From 6db67f2ca50bf0a4fc886f0c17f69b34dad72779 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 31 Aug 2018 15:30:28 -0400 Subject: [PATCH 1/2] commands/command_ls_files.go: don't accept "git lfs ls-files -- --all" In 05d65e75 (commands/ls-files: do not accept '--all' after '--', 2018-03-20), we encountered an interesting situation in which the following command invocation: $ git lfs ls-files -- --all Was treated as if "--all" were a reference name, and thusly passed to git.ResolveRef(), and ultimately to "git rev-parse", as "git rev-parse --all", which is a valid execution of "git rev-parse". Thus, we'll get an odd result. Instead, let's trap that case early, and return an appropriate error. --- commands/command_ls_files.go | 9 +++++++++ t/t-ls-files.sh | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/commands/command_ls_files.go b/commands/command_ls_files.go index 0129070a..3c6649a9 100644 --- a/commands/command_ls_files.go +++ b/commands/command_ls_files.go @@ -26,7 +26,16 @@ func lsFilesCommand(cmd *cobra.Command, args []string) { if len(args) == 1 { if lsFilesScanAll { Exit("fatal: cannot use --all with explicit reference") + } else if args[0] == "--all" { + // Since --all is a valid argument to "git rev-parse", + // if we try to give it to git.ResolveRef below, we'll + // get an unexpected result. + // + // So, let's check early that the caller invoked the + // command correctly. + Exit("fatal: did you mean \"git lfs ls-files --all --\" ?") } + ref = args[0] } else { fullref, err := git.CurrentRef() diff --git a/t/t-ls-files.sh b/t/t-ls-files.sh index 947bbd3b..dd9f2250 100755 --- a/t/t-ls-files.sh +++ b/t/t-ls-files.sh @@ -326,7 +326,7 @@ begin_test "ls-files: invalid --all ordering" echo >&2 "fatal: expected \`git lfs ls-files -- --all\' to fail" exit 1 fi - grep "Could not scan for Git LFS tree" ls-files.out + grep "fatal: did you mean \"git lfs ls-files --all --\" ?" ls-files.out ) end_test From cc7b83a9b9cb220692d56ba050e6ca7ecbde0356 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 31 Aug 2018 15:42:18 -0400 Subject: [PATCH 2/2] commands/command_ls_files: hide index with arguments Since e2717688 (commands: scan repository index in git-lfs-ls-files(1), 2017-12-19), we have traversed the contents of the index when showing files in 'git lfs ls-files'. The purpose of this was to match behavior in 'git ls-files', where the index is also shown by default. However, when given an argument, 'git lfs ls-files' will traverse starting from a specific revision, so showing the index unconditionally causes a problem here. It does not make sense to show the current state of the index when listing files when given a starting point explicitly. So, let's skip calling into the gitscanner code when given an explicit reference to start from (even if that reference is HEAD, since this likely indicates that the user does not want to scan the index, too). --- commands/command_ls_files.go | 11 +++++-- t/t-ls-files.sh | 56 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/commands/command_ls_files.go b/commands/command_ls_files.go index 3c6649a9..66f7e2ec 100644 --- a/commands/command_ls_files.go +++ b/commands/command_ls_files.go @@ -97,8 +97,15 @@ func lsFilesCommand(cmd *cobra.Command, args []string) { includeArg, excludeArg := getIncludeExcludeArgs(cmd) gitscanner.Filter = buildFilepathFilter(cfg, includeArg, excludeArg) - if err := gitscanner.ScanIndex(ref, nil); err != nil { - Exit("Could not scan for Git LFS index: %s", err) + if len(args) == 0 { + // Only scan the index when "git lfs ls-files" was invoked with + // no arguments. + // + // Do so to avoid showing "mixed" results, e.g., ls-files output + // from a specific historical revision, and the index. + if err := gitscanner.ScanIndex(ref, nil); err != nil { + Exit("Could not scan for Git LFS index: %s", err) + } } if lsFilesScanAll { if err := gitscanner.ScanAll(nil); err != nil { diff --git a/t/t-ls-files.sh b/t/t-ls-files.sh index dd9f2250..b84535e5 100755 --- a/t/t-ls-files.sh +++ b/t/t-ls-files.sh @@ -113,6 +113,62 @@ begin_test "ls-files: indexed file with tree" ) end_test +begin_test "ls-files: historical reference ignores index" +( + set -e + + reponame="ls-files-historical-reference-ignores-index" + git init "$reponame" + cd "$reponame" + + git lfs track "*.txt" + echo "a.txt" > a.txt + echo "b.txt" > b.txt + echo "c.txt" > c.txt + + git add .gitattributes a.txt + git commit -m "a.txt: initial commit" + + git add b.txt + git commit -m "b.txt: initial commit" + + git add c.txt + + git lfs ls-files "$(git rev-parse HEAD~1)" 2>&1 | tee ls-files.log + + [ 1 -eq "$(grep -c "a.txt" ls-files.log)" ] + [ 0 -eq "$(grep -c "b.txt" ls-files.log)" ] + [ 0 -eq "$(grep -c "c.txt" ls-files.log)" ] +) +end_test + +begin_test "ls-files: non-HEAD reference referring to HEAD ignores index" +( + set -e + + reponame="ls-files-HEAD-ish-ignores-index" + git init "$reponame" + cd "$reponame" + + git lfs track "*.txt" + echo "a.txt" > a.txt + echo "b.txt" > b.txt + + git add .gitattributes a.txt + git commit -m "a.txt: initial commit" + + tagname="v1.0.0" + git tag "$tagname" + + git add b.txt + + git lfs ls-files "$tagname" 2>&1 | tee ls-files.log + + [ 1 -eq "$(grep -c "a.txt" ls-files.log)" ] + [ 0 -eq "$(grep -c "b.txt" ls-files.log)" ] +) +end_test + begin_test "ls-files: outside git repository" ( set +e