Use git-ls-files to enumerate repo contents.

Due to the complexities of handling .gitignores, rely directly on
git-ls-files rather than a custom gitignore-aware directory walking
routine.

As a side-effect, there has been a minor behaviorial change related to
locking. Previously, if a file was locked, then a matching pattern was
added to a .gitignore, that file was no longer considered locked and the
writeable attribute was set on that file. With this change, files which
match a .gitignore, but have been addded to the repository,
will still have their write bit cleared.

Fixes #3797

Author: Seamus Connor

fs/cleanup.go

@@ -17,7 +17,7 @@ func (f *Filesystem) cleanupTmp() error {
 	}
 
 	var walkErr error
-	tools.FastWalkGitRepo(tmpdir, func(parentDir string, info os.FileInfo, err error) {
+	tools.FastWalkDir(tmpdir, func(parentDir string, info os.FileInfo, err error) {
 		if err != nil {
 			walkErr = err
 		}

fs/fs.go

@@ -45,7 +45,7 @@ type Filesystem struct {
 
 func (f *Filesystem) EachObject(fn func(Object) error) error {
 	var eachErr error
-	tools.FastWalkGitRepo(f.LFSObjectDir(), func(parentDir string, info os.FileInfo, err error) {
+	tools.FastWalkDir(f.LFSObjectDir(), func(parentDir string, info os.FileInfo, err error) {
 		if err != nil {
 			eachErr = err
 			return

git/attribs.go

@@ -3,6 +3,7 @@ package git
 import (
 	"os"
 	"path/filepath"
+	"sort"
 
 	"github.com/git-lfs/git-lfs/filepathfilter"
 	"github.com/git-lfs/git-lfs/git/gitattr"
@@ -172,27 +173,27 @@ func findAttributeFiles(workingDir, gitDir string) []attrFile {
 		paths = append(paths, attrFile{path: repoAttributes, readMacros: true})
 	}
 
-	tools.FastWalkGitRepo(workingDir, func(parentDir string, info os.FileInfo, err error) {
-		if err != nil {
-			tracerx.Printf("Error finding .gitattributes: %v", err)
-			return
-		}
+	lsFiles, err := NewLsFiles(workingDir, true)
+	if err != nil {
+		tracerx.Printf("Error finding .gitattributes: %v", err)
+		return paths
+	}
 
-		if info.IsDir() || info.Name() != ".gitattributes" {
-			return
+	if gitattributesFiles, present := lsFiles.FilesByName[".gitattributes"]; present {
+		for _, f := range gitattributesFiles {
+			tracerx.Printf("findAttributeFiles: located %s", f.FullPath)
+			paths = append(paths, attrFile{
+				path: filepath.Join(workingDir, f.FullPath),
+				readMacros: f.FullPath == ".gitattributes", // Read macros from the top-level attributes
+			})
 		}
-
-		paths = append(paths, attrFile{
-			path:       filepath.Join(parentDir, info.Name()),
-			readMacros: parentDir == workingDir,
-		})
-	})
+	}
 
 	// reverse the order of the files so more specific entries are found first
 	// when iterating from the front (respects precedence)
-	for i, j := 0, len(paths)-1; i < j; i, j = i+1, j-1 {
-		paths[i], paths[j] = paths[j], paths[i]
-	}
+	sort.Slice(paths[:], func(i, j int) bool {
+		return len(paths[i].path) > len(paths[j].path)
+	})
 
 	return paths
 }

git/ls_files.go

@@ -0,0 +1,90 @@
+package git
+
+import (
+	"bufio"
+	"strings"
+	"path"
+	"io/ioutil"
+
+	"github.com/git-lfs/git-lfs/errors"
+	"github.com/git-lfs/git-lfs/tools"
+	"github.com/rubyist/tracerx"
+)
+
+type lsFileInfo struct {
+	BaseName string
+	FullPath string
+}
+type LsFiles struct {
+	Files map[string]*lsFileInfo
+	FilesByName map[string][]*lsFileInfo
+}
+
+func NewLsFiles(workingDir string, standardExclude bool) (*LsFiles, error) {
+
+	args := []string{
+		"ls-files",
+		"-z", // Use a NUL separator. This also disables the escaping of special characters.
+		"--others",
+		"--cached",
+	}
+
+	if standardExclude {
+		args = append(args, "--exclude-standard")
+	}
+	cmd := gitNoLFS(args...)
+	cmd.Dir = workingDir
+
+    tracerx.Printf("NewLsFiles: running in %s git %s",
+			workingDir, strings.Join(args, " "))
+
+	// Capture stdout and stderr
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		return nil, err
+	}
+	stderr, err := cmd.StderrPipe()
+	if err != nil {
+		return nil, err
+	}
+
+	scanner := bufio.NewScanner(stdout)
+	scanner.Split(tools.SplitOnNul)
+
+	if err := cmd.Start(); err != nil {
+		return nil, err
+	}
+
+	rv := &LsFiles {
+		Files: make(map[string]*lsFileInfo),
+		FilesByName: make(map[string][]*lsFileInfo),
+	}
+
+	// Setup a goroutine to drain stderr as large amounts of error output may cause
+	// the subprocess to block.
+	errorMessages := make(chan []byte)
+	go func() {
+		msg, _ := ioutil.ReadAll(stderr)
+		errorMessages <- msg
+	}()
+
+	// Read all files
+	for scanner.Scan() {
+		base := path.Base(scanner.Text())
+		finfo := &lsFileInfo {
+			BaseName: base,
+			FullPath: scanner.Text(),
+		}
+		rv.Files[scanner.Text()] = finfo
+		rv.FilesByName[base] = append(rv.FilesByName[base], finfo)
+	}
+
+	// Check the output of the subprocess, output stderr if the command failed.
+	msg := <-errorMessages
+	if err := cmd.Wait(); err != nil {
+		return nil, errors.Errorf("Error in git %s: %v %s",
+			strings.Join(args, " "), err, msg)
+	}
+
+	return rv, nil
+}

locking/lockable.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	"github.com/git-lfs/git-lfs/errors"
 	"github.com/git-lfs/git-lfs/filepathfilter"
@@ -113,46 +112,20 @@ func (c *Client) FixFileWriteFlagsInDir(dir string, lockablePatterns, unlockable
 // Internal implementation of fixing file write flags with precompiled filters
 func (c *Client) fixFileWriteFlags(absPath, workingDir string, lockable, unlockable *filepathfilter.Filter) error {
 
-	var errs []error
-	var errMux sync.Mutex
-
-	addErr := func(err error) {
-		errMux.Lock()
-		defer errMux.Unlock()
-
-		errs = append(errs, err)
-	}
-
-	recursor := tools.FastWalkGitRepo
-	if c.ModifyIgnoredFiles {
-		recursor = tools.FastWalkGitRepoAll
+	// Build a list of files
+	lsFiles, err := git.NewLsFiles(workingDir, !c.ModifyIgnoredFiles)
+	if err != nil {
+		return err
 	}
 
-	recursor(absPath, func(parentDir string, fi os.FileInfo, err error) {
-		if err != nil {
-			addErr(err)
-			return
-		}
-		// Skip dirs, we only need to check files
-		if fi.IsDir() {
-			return
-		}
-		abschild := filepath.Join(parentDir, fi.Name())
-
-		// This is a file, get relative to repo root
-		relpath, err := filepath.Rel(workingDir, abschild)
-		if err != nil {
-			addErr(err)
-			return
-		}
-
-		err = c.fixSingleFileWriteFlags(relpath, lockable, unlockable)
+	for f := range lsFiles.Files {
+		err = c.fixSingleFileWriteFlags(f, lockable, unlockable)
 		if err != nil {
-			addErr(err)
+			return err
 		}
+	}
 
-	})
-	return errors.Combine(errs)
+	return nil
 }
 
 // FixLockableFileWriteFlags checks each file in the provided list, and for

t/t-lock.sh

@@ -250,7 +250,7 @@ begin_test "lock with .gitignore"
   git add .gitignore
   git commit -m ".gitignore: ignore 'a.txt'"
   rm -f a.txt && git checkout a.txt
-  assert_file_writeable a.txt
+  refute_file_writeable a.txt
 )
 end_test
 

tools/filetools.go

@@ -3,7 +3,6 @@
 package tools
 
 import (
-	"bufio"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -239,7 +238,7 @@ func VerifyFileHash(oid, path string) error {
 // FastWalkCallback is the signature for the callback given to FastWalkGitRepo()
 type FastWalkCallback func(parentDir string, info os.FileInfo, err error)
 
-// FastWalkGitRepo is a more optimal implementation of filepath.Walk for a Git
+// FastWalkDir is a more optimal implementation of filepath.Walk for a Git
 // repo. The callback guaranteed to be called sequentially. The function returns
 // once all files and errors have triggered callbacks.
 // It differs in the following ways:
@@ -248,18 +247,10 @@ type FastWalkCallback func(parentDir string, info os.FileInfo, err error)
 //    there are no other guarantees. Use parentDir argument in the callback to
 //    determine absolute path rather than tracking it yourself
 //  * Automatically ignores any .git directories
-//  * Respects .gitignore contents and skips ignored files/dirs
 //
 // rootDir - Absolute path to the top of the repository working directory
-func FastWalkGitRepo(rootDir string, cb FastWalkCallback) {
-	fastWalkCallback(fastWalkWithExcludeFiles(rootDir, ".gitignore"), cb)
-}
-
-// FastWalkGitRepoAll behaves as FastWalkGitRepo, with the additional caveat
-// that it does not ignore paths and directories found in .gitignore file(s)
-// throughout the repository.
-func FastWalkGitRepoAll(rootDir string, cb FastWalkCallback) {
-	fastWalkCallback(fastWalkWithExcludeFiles(rootDir, ""), cb)
+func FastWalkDir(rootDir string, cb FastWalkCallback) {
+	fastWalkCallback(fastWalkWithExcludeFiles(rootDir), cb)
 }
 
 // fastWalkCallback calls the FastWalkCallback "cb" for all files found by the
@@ -281,19 +272,17 @@ type fastWalkInfo struct {
 
 type fastWalker struct {
 	rootDir         string
-	excludeFilename string
 	ch              chan fastWalkInfo
 	limit           int32
 	cur             *int32
 	wg              *sync.WaitGroup
 }
 
 // fastWalkWithExcludeFiles walks the contents of a dir, respecting
-// include/exclude patterns and also loading new exlude patterns from files
-// named excludeFilename in directories walked
+// include/exclude patterns.
 //
 // rootDir - Absolute path to the top of the repository working directory
-func fastWalkWithExcludeFiles(rootDir, excludeFilename string) *fastWalker {
+func fastWalkWithExcludeFiles(rootDir string) *fastWalker {
 	excludePaths := []filepathfilter.Pattern{
 		filepathfilter.NewPattern(".git"),
 		filepathfilter.NewPattern("**/.git"),
@@ -307,7 +296,6 @@ func fastWalkWithExcludeFiles(rootDir, excludeFilename string) *fastWalker {
 	c := int32(0)
 	w := &fastWalker{
 		rootDir:         rootDir,
-		excludeFilename: excludeFilename,
 		limit:           int32(limit),
 		cur:             &c,
 		ch:              make(chan fastWalkInfo, 256),
@@ -327,12 +315,9 @@ func fastWalkWithExcludeFiles(rootDir, excludeFilename string) *fastWalker {
 	return w
 }
 
-// Walk is the main recursive implementation of fast walk.
-// Sends the file/dir and any contents to the channel so long as it passes the
-// include/exclude filter. If a dir, parses any excludeFilename found and updates
-// the excludePaths with its content before (parallel) recursing into contents
-// Also splits large directories into multiple goroutines.
-// Increments waitg.Add(1) for each new goroutine launched internally
+// Walk is the main recursive implementation of fast walk.  Sends the file/dir
+// and any contents to the channel so long as it passes the include/exclude
+// filter.  Increments waitg.Add(1) for each new goroutine launched internally
 //
 // workDir - Relative path inside the repository
 func (w *fastWalker) Walk(isRoot bool, workDir string, itemFi os.FileInfo,
@@ -373,15 +358,6 @@ func (w *fastWalker) Walk(isRoot bool, workDir string, itemFi os.FileInfo,
 		childWorkDir = join(workDir, itemFi.Name())
 	}
 
-	if len(w.excludeFilename) > 0 {
-		possibleExcludeFile := join(fullPath, w.excludeFilename)
-		var err error
-		excludePaths, err = loadExcludeFilename(possibleExcludeFile, childWorkDir, excludePaths)
-		if err != nil {
-			w.ch <- fastWalkInfo{Err: err}
-		}
-	}
-
 	// The absolute optimal way to scan would be File.Readdirnames but we
 	// still need the Stat() to know whether something is a dir, so use
 	// File.Readdir instead. Means we can provide os.FileInfo to callers like
@@ -430,51 +406,6 @@ func (w *fastWalker) Wait() {
 	close(w.ch)
 }
 
-// loadExcludeFilename reads the given file in gitignore format and returns a
-// revised array of exclude paths if there are any changes.
-// If any changes are made a copy of the array is taken so the original is not
-// modified
-func loadExcludeFilename(filename, workDir string, excludePaths []filepathfilter.Pattern) ([]filepathfilter.Pattern, error) {
-	f, err := os.OpenFile(filename, os.O_RDONLY, 0644)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return excludePaths, nil
-		}
-		return excludePaths, err
-	}
-	defer f.Close()
-
-	retPaths := excludePaths
-	modified := false
-
-	scanner := bufio.NewScanner(f)
-	for scanner.Scan() {
-		line := strings.TrimSpace(scanner.Text())
-		// Skip blanks, comments and negations (not supported right now)
-		if len(line) == 0 || strings.HasPrefix(line, "#") || strings.HasPrefix(line, "!") {
-			continue
-		}
-
-		if !modified {
-			// copy on write
-			retPaths = make([]filepathfilter.Pattern, len(excludePaths))
-			copy(retPaths, excludePaths)
-			modified = true
-		}
-
-		path := line
-		// Add pattern in context if exclude has separator, or no wildcard
-		// Allow for both styles of separator at this point
-		if strings.ContainsAny(path, "/\\") ||
-			!strings.Contains(path, "*") {
-			path = join(workDir, line)
-		}
-		retPaths = append(retPaths, filepathfilter.NewPattern(path))
-	}
-
-	return retPaths, nil
-}
-
 func join(paths ...string) string {
 	ne := make([]string, 0, len(paths))