From b5e53210c20c0e3fc9ea9e81aa0b1ec8519acf28 Mon Sep 17 00:00:00 2001 From: Shudhanshu Singh Date: Wed, 21 Oct 2026 15:42:41 +0530 Subject: [PATCH] fix(lfs): send the default branch as ref in batch download requests (#39891) Fixes https://github.com/go-gitea/gitea/issues/38085 --------- Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang --- modules/lfs/client.go | 2 +- modules/lfs/filesystem_client.go | 2 +- modules/lfs/http_client.go | 24 ++++++++++++------------ modules/lfs/http_client_test.go | 4 +++- modules/repository/repo.go | 7 ++++++- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/modules/lfs/client.go b/modules/lfs/client.go index b1fba18e81b..fe0ca536d5b 100644 --- a/modules/lfs/client.go +++ b/modules/lfs/client.go @@ -22,7 +22,7 @@ type UploadCallback func(p Pointer, objectError error) (io.ReadCloser, error) // Client is used to communicate with a LFS source type Client interface { BatchSize() int - Download(ctx context.Context, objects []Pointer, callback DownloadCallback) error + Download(ctx context.Context, ref string, objects []Pointer, callback DownloadCallback) error // ref is the full git ref name the objects belong to, may be empty Upload(ctx context.Context, objects []Pointer, callback UploadCallback) error } diff --git a/modules/lfs/filesystem_client.go b/modules/lfs/filesystem_client.go index b10ca817aef..d905a5dd91e 100644 --- a/modules/lfs/filesystem_client.go +++ b/modules/lfs/filesystem_client.go @@ -38,7 +38,7 @@ func (c *FilesystemClient) objectPath(oid string) (string, error) { } // Download reads the specific LFS object from the target path -func (c *FilesystemClient) Download(ctx context.Context, objects []Pointer, callback DownloadCallback) error { +func (c *FilesystemClient) Download(ctx context.Context, _ string, objects []Pointer, callback DownloadCallback) error { for _, object := range objects { p := Pointer{object.Oid, object.Size} diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index c4e4faf4f67..a8922c90117 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -67,18 +67,18 @@ func (c *HTTPClient) transferNames() []string { return keys } -func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Pointer) (*BatchResponse, error) { +func (c *HTTPClient) batch(ctx context.Context, operation, ref string, objects []Pointer) (*BatchResponse, error) { log.Trace("BATCH operation with objects: %v", objects) url := c.endpoint + "/objects/batch" - // Original: In some lfs server implementations, they require the ref attribute. #32838 - // `ref` is an "optional object describing the server ref that the objects belong to" - // but some (incorrect) lfs servers like aliyun require it, so maybe adding an empty ref here doesn't break the correct ones. + // `ref` is optional, but some servers like aliyun require it, while others like Hugging Face reject an empty or null one // https://github.com/git-lfs/git-lfs/blob/a32a02b44bf8a511aa14f047627c49e1a7fd5021/docs/api/batch.md?plain=1#L37 - // - // UPDATE: it can't use "empty ref" here because it breaks others like https://github.com/go-gitea/gitea/issues/33453 - request := &BatchRequest{operation, c.transferNames(), nil, objects} + // Related issues: #32838, #33453, #39891 + request := &BatchRequest{Operation: operation, Transfers: c.transferNames(), Objects: objects} + if ref != "" { + request.Ref = &Reference{Name: ref} + } payload := new(bytes.Buffer) err := json.NewEncoder(payload).Encode(request) @@ -113,17 +113,17 @@ func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Poin } // Download reads the specific LFS object from the LFS server -func (c *HTTPClient) Download(ctx context.Context, objects []Pointer, callback DownloadCallback) error { - return c.performOperation(ctx, objects, callback, nil) +func (c *HTTPClient) Download(ctx context.Context, ref string, objects []Pointer, callback DownloadCallback) error { + return c.performOperation(ctx, ref, objects, callback, nil) } // Upload sends the specific LFS object to the LFS server func (c *HTTPClient) Upload(ctx context.Context, objects []Pointer, callback UploadCallback) error { - return c.performOperation(ctx, objects, nil, callback) + return c.performOperation(ctx, "", objects, nil, callback) } // performOperation takes a slice of LFS object pointers, batches them, and performs the upload/download operations concurrently in each batch -func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc DownloadCallback, uc UploadCallback) error { +func (c *HTTPClient) performOperation(ctx context.Context, ref string, objects []Pointer, dc DownloadCallback, uc UploadCallback) error { if len(objects) == 0 { return nil } @@ -133,7 +133,7 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc operation = "upload" } - result, err := c.batch(ctx, operation, objects) + result, err := c.batch(ctx, operation, ref, objects) if err != nil { return err } diff --git a/modules/lfs/http_client_test.go b/modules/lfs/http_client_test.go index a234ceb5529..b6500cd6e96 100644 --- a/modules/lfs/http_client_test.go +++ b/modules/lfs/http_client_test.go @@ -175,6 +175,7 @@ func TestHTTPClientDownload(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "download", batchRequest.Operation) + assert.Equal(t, &Reference{Name: "refs/heads/main"}, batchRequest.Ref) assert.Len(t, batchRequest.Objects, 1) assert.Equal(t, p.Oid, batchRequest.Objects[0].Oid) assert.Equal(t, p.Size, batchRequest.Objects[0].Size) @@ -248,7 +249,7 @@ func TestHTTPClientDownload(t *testing.T) { }, } - err := client.Download(t.Context(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error { + err := client.Download(t.Context(), "refs/heads/main", []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error { if objectError != nil { return objectError } @@ -283,6 +284,7 @@ func TestHTTPClientUpload(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "upload", batchRequest.Operation) + assert.Nil(t, batchRequest.Ref) assert.Len(t, batchRequest.Objects, 1) assert.Equal(t, p.Oid, batchRequest.Objects[0].Oid) assert.Equal(t, p.Size, batchRequest.Objects[0].Size) diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 02534587614..14dc1fba1fa 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -67,8 +67,13 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re errChan <- lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan) }() + // some servers require `ref`, send the default branch for objects from all refs like + // `git lfs fetch --all` does on the main branch: it uses current branch as `ref`: + // https://github.com/git-lfs/git-lfs/blob/58a763d09d5fe4b7ea85c89ec799d71c5b0dc1da/commands/commands.go#L124-L137 + // `man git-lfs-fetch` said that "--all" can fetch all objects, so the default branch should work + ref := git.RefNameFromBranch(repo.DefaultBranch).String() downloadObjects := func(pointers []lfs.Pointer) error { - err := lfsClient.Download(ctx, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { + err := lfsClient.Download(ctx, ref, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { if errors.Is(objectError, lfs.ErrObjectNotExist) { log.Warn("Ignoring missing upstream LFS object %-v: %v", p, objectError) return nil