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 <wxiaoguang@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

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
 }
 

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}
 

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
 	}

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)

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