as per review

Signed-off-by: Andrew Thornton <art27@cantab.net>

Author: zeripath

modules/git/pipeline/lfs.go

@@ -103,12 +103,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 	if err != nil {
 		return nil, err
 	}
-
-	defer func() {
-		for _, pipe := range pipes {
-			_ = pipe.Close()
-		}
-	} ()
+	defer pipes.Close()
 
 	shasToNameReader, shasToNameWriter := pipes[0].ReaderWriter()
 	nameRevStdinReader, nameRevStdinWriter := pipes[1].ReaderWriter()

modules/git/pipeline/lfs_nogogit.go

@@ -202,12 +202,7 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 	if err != nil {
 		return nil, err
 	}
-
-	defer func() {
-		for _, pipe := range pipes {
-			_ = pipe.Close()
-		}
-	}()
+	defer pipes.Close()
 
 	shasToNameReader, shasToNameWriter := pipes[0].ReaderWriter()
 	nameRevStdinReader, nameRevStdinWriter := pipes[1].ReaderWriter()

modules/git/utils_pipe.go

@@ -210,7 +210,7 @@ func NewPipePair() (*PipePair, error) {
 }
 
 // NewPipePairs will return a slice of n PipePairs or an error
-func NewPipePairs(n int) ([]*PipePair, error) {
+func NewPipePairs(n int) (PipePairs, error) {
 	pipePairs := make([]*PipePair, 0, n)
 	for i := 0; i < n; i++ {
 		pipe, err := NewPipePair()
@@ -225,3 +225,19 @@ func NewPipePairs(n int) ([]*PipePair, error) {
 	}
 	return pipePairs, nil
 }
+
+type PipePairs []*PipePair
+
+// Close closes the PipePairs
+func (pairs PipePairs) Close() error {
+	return pairs.CloseWithError(nil)
+}
+
+// CloseWithError closes the pipe pair
+func (pairs PipePairs) CloseWithError(err error) error {
+	for _, p := range pairs {
+		_ = p.closeRead(err)
+		_ = p.closeWrite(err)
+	}
+	return nil
+}

modules/lfs/pointer_scanner_nogogit.go

@@ -23,24 +23,24 @@ import (
 func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan chan<- PointerBlob, errChan chan<- error) {
 	basePath := repo.Path
 
-	fail := func(err error) {
-		errChan <- err
-		close(pointerChan)
-		close(errChan)
+	// We will need to run batch-check on all the objects in the repository
+	// in Git 2.6.0+ we can use `git cat-file --batch-check --batch-all-objects`
+	// However in earlier versions we'll need to use rev-list to get the objects.
+	gitHasBatchCheckAllObjects := git.CheckGitVersionAtLeast("2.6.0") == nil
+
+	numPipesRequired := 3
+	if !gitHasBatchCheckAllObjects {
+		numPipesRequired += 2
 	}
 
-	pipes, err := git.NewPipePairs(3)
+	pipes, err := git.NewPipePairs(numPipesRequired)
 	if err != nil {
-		fail(err)
+		errChan <- err
+		close(pointerChan)
+		close(errChan)
 		return
 	}
-
-	closeAll := func(err error) {
-		for _, closer := range pipes {
-			_ = closer.CloseWithError(err)
-		}
-	}
-	defer closeAll(nil)
+	defer pipes.Close()
 
 	catFileCheckReader, catFileCheckWriter := pipes[0].ReaderWriter()
 	shasToBatchReader, shasToBatchWriter := pipes[1].ReaderWriter()
@@ -62,20 +62,9 @@ func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan c
 	go pipeline.BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg)
 
 	// 1. Run batch-check on all objects in the repository
-	if git.CheckGitVersionAtLeast("2.6.0") != nil {
-
-		morePipes, err := git.NewPipePairs(2)
-		if err != nil {
-			wg.Done()
-			closeAll(err)
-			wg.Wait()
-			fail(err)
-			return
-		}
-
-		revListReader, revListWriter := morePipes[0].ReaderWriter()
-		shasToCheckReader, shasToCheckWriter := morePipes[1].ReaderWriter()
-		pipes = append(pipes, morePipes...)
+	if !gitHasBatchCheckAllObjects {
+		revListReader, revListWriter := pipes[3].ReaderWriter()
+		shasToCheckReader, shasToCheckWriter := pipes[4].ReaderWriter()
 
 		wg.Add(2)
 		go pipeline.CatFileBatchCheck(ctx, shasToCheckReader, catFileCheckWriter, &wg, basePath)

services/pull/lfs.go

@@ -32,13 +32,7 @@ func LFSPush(ctx context.Context, tmpBasePath, mergeHeadSHA, mergeBaseSHA string
 	if err != nil {
 		return err
 	}
-
-	closeAll := func(err error) {
-		for _, closer := range pipes {
-			_ = closer.CloseWithError(err)
-		}
-	}
-	defer closeAll(nil)
+	defer pipes.Close()
 
 	revListReader, revListWriter := pipes[0].ReaderWriter()
 	shasToCheckReader, shasToCheckWriter := pipes[1].ReaderWriter()