From 228e31fb20879cac1c5600100e5dcd792d051cfd Mon Sep 17 00:00:00 2001
From: "brian m. carlson" <bk2204@github.com>
Date: Mon, 18 Nov 2019 16:41:49 +0000
Subject: [PATCH] status: update index before showing status

When running `git lfs status`, we perform a `git diff-index`.  However,
we don't update the index first, so any changes, such as permissions
changes due to locking, cause the file to be listed as modified.  Since
these changes don't represent actual changes that we're interested in,
refresh the index before running diff-index so that it doesn't produce
spurious output.
---
 commands/command_status.go |  4 ++--
 git/git.go                 |  9 +++++++-
 lfs/diff_index_scanner.go  |  9 ++++++--
 lfs/gitscanner_index.go    |  2 +-
 t/t-status.sh              | 45 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/commands/command_status.go b/commands/command_status.go
index e5661f24..677826bc 100644
--- a/commands/command_status.go
+++ b/commands/command_status.go
@@ -170,12 +170,12 @@ func blobInfo(s *lfs.PointerScanner, blobSha, name string) (sha, from string, er
 }
 
 func scanIndex(ref string) (staged, unstaged []*lfs.DiffIndexEntry, err error) {
-	uncached, err := lfs.NewDiffIndexScanner(ref, false)
+	uncached, err := lfs.NewDiffIndexScanner(ref, false, true)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	cached, err := lfs.NewDiffIndexScanner(ref, true)
+	cached, err := lfs.NewDiffIndexScanner(ref, true, false)
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/git/git.go b/git/git.go
index c87f9404..817e5bd1 100644
--- a/git/git.go
+++ b/git/git.go
@@ -178,7 +178,14 @@ func CatFile() (*subprocess.BufferedCmd, error) {
 	return gitNoLFSBuffered("cat-file", "--batch-check")
 }
 
-func DiffIndex(ref string, cached bool) (*bufio.Scanner, error) {
+func DiffIndex(ref string, cached bool, refresh bool) (*bufio.Scanner, error) {
+	if refresh {
+		_, err := gitSimple("update-index", "-q", "--refresh")
+		if err != nil {
+			return nil, lfserrors.Wrap(err, "Failed to run git update-index")
+		}
+	}
+
 	args := []string{"diff-index", "-M"}
 	if cached {
 		args = append(args, "--cached")
diff --git a/lfs/diff_index_scanner.go b/lfs/diff_index_scanner.go
index c1472d8e..1558b058 100644
--- a/lfs/diff_index_scanner.go
+++ b/lfs/diff_index_scanner.go
@@ -119,11 +119,16 @@ type DiffIndexScanner struct {
 // scan for differences between the given ref and the currently checked out
 // tree.
 //
+// If "refresh" is given, the DiffIndexScanner will refresh the index.  This is
+// probably what you want in all cases except fsck, where invoking a filtering
+// operation would be undesirable due to the possibility of corruption. It can
+// also be disabled where another operation will have refreshed the index.
+//
 // If any error was encountered in starting the command or closing its `stdin`,
 // that error will be returned immediately. Otherwise, a `*DiffIndexScanner`
 // will be returned with a `nil` error.
-func NewDiffIndexScanner(ref string, cached bool) (*DiffIndexScanner, error) {
-	scanner, err := git.DiffIndex(ref, cached)
+func NewDiffIndexScanner(ref string, cached bool, refresh bool) (*DiffIndexScanner, error) {
+	scanner, err := git.DiffIndex(ref, cached, refresh)
 	if err != nil {
 		return nil, err
 	}
diff --git a/lfs/gitscanner_index.go b/lfs/gitscanner_index.go
index 801bc827..512d39b7 100644
--- a/lfs/gitscanner_index.go
+++ b/lfs/gitscanner_index.go
@@ -110,7 +110,7 @@ func scanIndex(cb GitScannerFoundPointer, ref string, f *filepathfilter.Filter,
 // for in the indexf. It returns a channel from which sha1 strings can be read.
 // The namMap will be filled indexFile pointers mapping sha1s to indexFiles.
 func revListIndex(atRef string, cache bool, indexMap *indexFileMap) (*StringChannelWrapper, error) {
-	scanner, err := NewDiffIndexScanner(atRef, cache)
+	scanner, err := NewDiffIndexScanner(atRef, cache, false)
 	if err != nil {
 		return nil, err
 	}
diff --git a/t/t-status.sh b/t/t-status.sh
index c80d5481..c03cfcd9 100755
--- a/t/t-status.sh
+++ b/t/t-status.sh
@@ -548,3 +548,48 @@ Git LFS objects not staged for commit:"
   [ "$expected" = "$(git lfs status)" ]
 )
 end_test
+
+
+begin_test "status: permission change"
+(
+  set -e
+
+  # We're using chmod below.
+  if [ "$IS_WINDOWS" -eq 1 ]; then
+    exit 0
+  fi
+
+  reponame="status-permission-change"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  contents="contents"
+
+  git lfs track "*.dat"
+  git add .gitattributes
+  git commit -m "track *.dat"
+
+  printf "%s" "$contents" > a.dat
+  git add a.dat
+  git commit -m "add a.dat"
+
+  chmod 400 a.dat
+
+  # A permission change should not result in any output.
+  git lfs status 2>&1 | tee status.log
+  if [ "0" -ne "${PIPESTATUS[0]}" ]; then
+    echo >&2 "git lfs status should have succeeded, didn't ..."
+    exit 1
+  fi
+
+  expected="On branch master
+
+Git LFS objects to be committed:
+
+
+Git LFS objects not staged for commit:"
+  actual="$(cat status.log)"
+
+  [ "$expected" = "$actual" ]
+)
+end_test