Don't fail if we lack objects the server has

A Git LFS client may not have the entire history of the objects for the
repository. However, in some situations, we traverse the entire history
of a branch when pushing it, meaning that we need to process every
LFS object in the history of that branch. If the objects for the entire
history are not present, we currently fail to push.

Instead, let's mark objects we don't have on disk as missing and only
fail when we would need to upload those objects. We'll know the server
has the objects if the batch response provides no actions to take for
them when we request an upload. Pass the missing flag down through the
code, and always set it to false for non-uploads.

If for some reason we fail to properly flag a missing object, we will
still fail later on when we cannot open the file, just in a messier and
more poorly controlled way. The technique used here will attempt to
abort the batch as soon as we notice a problem, which means that in the
common case (less than 100 objects) we won't have transferred any
objects, so the user can notice the failure as soon as possible.

Update the tests to look for a string which will occur in the error
message, since we no longer produce the system error message for ENOENT.

Author: bk2204

commands/command_migrate_export.go

@@ -136,7 +136,7 @@ func migrateExportCommand(cmd *cobra.Command, args []string) {
 			}
 
 			if _, err := os.Stat(downloadPath); os.IsNotExist(err) {
-				q.Add(p.Name, downloadPath, p.Oid, p.Size)
+				q.Add(p.Name, downloadPath, p.Oid, p.Size, false)
 			}
 		})
 		gs.ScanRefs(opts.Include, opts.Exclude, nil)

commands/command_smudge.go

@@ -59,7 +59,7 @@ func delayedSmudge(gf *lfs.GitFilter, s *git.FilterProcessScanner, to io.Writer,
 
 	if !skip && filter.Allows(filename) {
 		if _, statErr := os.Stat(path); statErr != nil {
-			q.Add(filename, path, ptr.Oid, ptr.Size)
+			q.Add(filename, path, ptr.Oid, ptr.Size, false)
 			return 0, true, ptr, nil
 		}
 

commands/commands.go

@@ -131,9 +131,9 @@ func buildFilepathFilter(config *config.Configuration, includeArg, excludeArg *s
 	return filepathfilter.New(inc, exc)
 }
 
-func downloadTransfer(p *lfs.WrappedPointer) (name, path, oid string, size int64) {
+func downloadTransfer(p *lfs.WrappedPointer) (name, path, oid string, size int64, missing bool) {
 	path, _ = cfg.Filesystem().ObjectPath(p.Oid)
-	return p.Name, path, p.Oid, p.Size
+	return p.Name, path, p.Oid, p.Size, false
 }
 
 // Get user-readable manual install steps for hooks

commands/uploader.go

@@ -246,7 +246,7 @@ func (c *uploadContext) UploadPointers(q *tq.TransferQueue, unfiltered ...*lfs.W
 			ExitWithError(err)
 		}
 
-		q.Add(t.Name, t.Path, t.Oid, t.Size)
+		q.Add(t.Name, t.Path, t.Oid, t.Size, t.Missing)
 		c.SetUploaded(p.Oid)
 	}
 }
@@ -338,6 +338,8 @@ var (
 )
 
 func (c *uploadContext) uploadTransfer(p *lfs.WrappedPointer) (*tq.Transfer, error) {
+	var missing bool
+
 	filename := p.Name
 	oid := p.Oid
 
@@ -347,40 +349,38 @@ func (c *uploadContext) uploadTransfer(p *lfs.WrappedPointer) (*tq.Transfer, err
 	}
 
 	if len(filename) > 0 {
-		if err = c.ensureFile(filename, localMediaPath, oid); err != nil && !errors.IsCleanPointerError(err) {
+		if missing, err = c.ensureFile(filename, localMediaPath, oid); err != nil && !errors.IsCleanPointerError(err) {
 			return nil, err
 		}
 	}
 
 	return &tq.Transfer{
-		Name: filename,
-		Path: localMediaPath,
-		Oid:  oid,
-		Size: p.Size,
+		Name:    filename,
+		Path:    localMediaPath,
+		Oid:     oid,
+		Size:    p.Size,
+		Missing: missing,
 	}, nil
 }
 
 // ensureFile makes sure that the cleanPath exists before pushing it.  If it
 // does not exist, it attempts to clean it by reading the file at smudgePath.
-func (c *uploadContext) ensureFile(smudgePath, cleanPath, oid string) error {
+func (c *uploadContext) ensureFile(smudgePath, cleanPath, oid string) (bool, error) {
 	if _, err := os.Stat(cleanPath); err == nil {
-		return nil
+		return false, nil
 	}
 
 	localPath := filepath.Join(cfg.LocalWorkingDir(), smudgePath)
 	file, err := os.Open(localPath)
 	if err != nil {
-		if c.allowMissing {
-			return nil
-		}
-		return errors.Wrapf(err, "Unable to find source for object %v (try running git lfs fetch --all)", oid)
+		return !c.allowMissing, nil
 	}
 
 	defer file.Close()
 
 	stat, err := file.Stat()
 	if err != nil {
-		return err
+		return false, err
 	}
 
 	cleaned, err := c.gitfilter.Clean(file, file.Name(), stat.Size(), nil)
@@ -389,9 +389,9 @@ func (c *uploadContext) ensureFile(smudgePath, cleanPath, oid string) error {
 	}
 
 	if err != nil {
-		return err
+		return false, err
 	}
-	return nil
+	return false, nil
 }
 
 // supportsLockingAPI returns whether or not a given url is known to support

lfs/gitfilter_smudge.go

@@ -96,7 +96,7 @@ func (f *GitFilter) downloadFile(writer io.Writer, ptr *Pointer, workingfile, me
 		tq.WithProgressCallback(cb),
 		tq.RemoteRef(f.RemoteRef()),
 	)
-	q.Add(filepath.Base(workingfile), mediafile, ptr.Oid, ptr.Size)
+	q.Add(filepath.Base(workingfile), mediafile, ptr.Oid, ptr.Size, false)
 	q.Wait()
 
 	if errs := q.Errors(); len(errs) > 0 {

t/git-lfs-test-server-api/main.go

@@ -206,7 +206,7 @@ func buildTestData(repo *t.Repo, manifest *tq.Manifest) (oidsExist, oidsMissing
 		if err != nil {
 			return nil, nil, err
 		}
-		uploadQueue.Add(t.Name, t.Path, t.Oid, t.Size)
+		uploadQueue.Add(t.Name, t.Path, t.Oid, t.Size, false)
 	}
 	uploadQueue.Wait()
 

t/t-pre-push.sh

@@ -412,8 +412,7 @@ begin_test "pre-push reject missing pointers (lfs.allowincompletepush default)"
     exit 1
   fi
 
-  grep "no such file or directory" push.log || # unix
-    grep "cannot find the file" push.log       # windows
+  grep 'Unable to find source' push.log
 
   refute_server_object "$reponame" "$present_oid"
   refute_server_object "$reponame" "$missing_oid"

t/t-push-failures-local.sh

@@ -90,9 +90,7 @@ begin_test "push reject missing objects (lfs.allowincompletepush false)"
     exit 1
   fi
 
-  grep "no such file or directory" push.log || # unix
-    grep "cannot find the file" push.log       # windows
-  grep "failed to push some refs" push.log
+  grep 'Unable to find source' push.log
 
   refute_server_object "$reponame" "$present_oid"
   refute_server_object "$reponame" "$missing_oid"

t/t-push.sh

@@ -694,3 +694,49 @@ begin_test "push with invalid pushInsteadof"
   git lfs push origin master
 )
 end_test
+
+begin_test 'push with data the server already has'
+(
+  set -e
+
+  reponame="push-server-data"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  git lfs track "*.dat"
+  git add .gitattributes
+  git commit -m "initial commit"
+
+  contents="abc123"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" > a.dat
+  git add a.dat
+  git commit -m "add a.dat"
+
+  git push origin master
+
+  assert_server_object "$reponame" "$contents_oid"
+
+  git checkout -b side
+
+  # Use a different file name for the second file; otherwise, this test
+  # unexpectedly passes with the old code, since we fail to notice that the
+  # object we run through the clean filter is not the object we wanted.
+  contents2="def456"
+  contents2_oid="$(calc_oid "$contents2")"
+  printf "%s" "$contents2" > b.dat
+  git add b.dat
+  git commit -m "add b.dat"
+
+  # We remove the original object. The server already has this.
+  delete_local_object "$contents_oid"
+
+  # We use the URL so that we cannot take advantage of the existing "origin/*"
+  # refs that we know the server must have. We will traverse the entire history
+  # for this push, and we should not fail because the server already has the
+  # object we deleted above.
+  git push "$(git config remote.origin.url)" side
+
+  assert_server_object "$reponame" "$contents2_oid"
+)
+end_test

tq/api.go

@@ -55,6 +55,11 @@ func (c *tqClient) Batch(remote string, bReq *batchRequest) (*BatchResponse, err
 		bReq.TransferAdapterNames = nil
 	}
 
+	missing := make(map[string]bool)
+	for _, obj := range bReq.Objects {
+		missing[obj.Oid] = obj.Missing
+	}
+
 	bRes.endpoint = c.Endpoints.Endpoint(bReq.Operation, remote)
 	requestedAt := time.Now()
 
@@ -81,6 +86,7 @@ func (c *tqClient) Batch(remote string, bReq *batchRequest) (*BatchResponse, err
 	}
 
 	for _, obj := range bRes.Objects {
+		obj.Missing = missing[obj.Oid]
 		for _, a := range obj.Actions {
 			a.createdAt = requestedAt
 		}