From c7642ecdd50823d5e6e550008ad60f75416bd689 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Mon, 18 Apr 2022 00:24:53 -0700 Subject: [PATCH] commands,t: gitignore matching for fetch filters The "lfs.fetchinclude" and "lfs.fetchexclude" Git configuration options, if set, are used to control the action of a number of Git LFS commands. Since PR #4556, the "git lfs clone", "git lfs fetch", and "git lfs pull" commands have strictly applied gitignore(5)-style matching rules to these configuration options. However, other commands including "git lfs filter-process" and "git lfs smudge" now apply gitattributes(5)-style matching rules to these same configuration options, leading to confusion. We therefore revise all remaining uses of these configuration options to also use gitignore-style matching rules. We also add new tests for the "git lfs filter-process" and "git lfs fsck" commands and adjust or expand existing tests for the "git lfs prune" and "git lfs smudge" commands in order to confirm that gitignore-style matching is used for all of them. These new and updated tests fail if gitattributes-style matching is used instead. (Note that the "git lfs migrate" command does not require any changes because it does not read the "lfs.fetch*" configuration options. Instead, it supplies a "false" value for the "useFetchOptions" flag to the determineIncludeExcludePaths() function, so any "lfs.fetch*" configuration values are ignored. This is significant because "git lfs migrate" deliberately uses gitattributes-style matching for any path patterns supplied via its -I/-X command-line arguments, unlike all other commands that accept -I/-X arguments as overrides for the "lfs.fetch*" configuration options.) --- commands/command_filter_process.go | 2 +- commands/command_fsck.go | 2 +- commands/command_prune.go | 2 +- commands/command_smudge.go | 2 +- t/t-filter-process.sh | 54 ++++++++++++++++++++++++++++ t/t-fsck.sh | 43 +++++++++++++++++++++++ t/t-prune-worktree.sh | 2 +- t/t-prune.sh | 10 +++--- t/t-smudge.sh | 56 ++++++++++++++++++++++++++++-- 9 files changed, 160 insertions(+), 13 deletions(-) diff --git a/commands/command_filter_process.go b/commands/command_filter_process.go index 27d87dee..eaf0d266 100644 --- a/commands/command_filter_process.go +++ b/commands/command_filter_process.go @@ -61,7 +61,7 @@ func filterCommand(cmd *cobra.Command, args []string) { } skip := filterSmudgeSkip || cfg.Os.Bool("GIT_LFS_SKIP_SMUDGE", false) - filter := filepathfilter.New(cfg.FetchIncludePaths(), cfg.FetchExcludePaths(), filepathfilter.GitAttributes) + filter := filepathfilter.New(cfg.FetchIncludePaths(), cfg.FetchExcludePaths(), filepathfilter.GitIgnore) ptrs := make(map[string]*lfs.Pointer) diff --git a/commands/command_fsck.go b/commands/command_fsck.go index 73a8e0c1..8c90b182 100644 --- a/commands/command_fsck.go +++ b/commands/command_fsck.go @@ -144,7 +144,7 @@ func doFsckObjects(include, exclude string, useIndex bool) []string { // objects), the "missing" ones will fail the fsck. // // Attach a filepathfilter to avoid _only_ the excluded paths. - gitscanner.Filter = filepathfilter.New(nil, cfg.FetchExcludePaths(), filepathfilter.GitAttributes) + gitscanner.Filter = filepathfilter.New(nil, cfg.FetchExcludePaths(), filepathfilter.GitIgnore) if exclude == "" { if err := gitscanner.ScanRef(include, nil); err != nil { diff --git a/commands/command_prune.go b/commands/command_prune.go index bc830fd4..e01a4939 100644 --- a/commands/command_prune.go +++ b/commands/command_prune.go @@ -98,7 +98,7 @@ func prune(fetchPruneConfig lfs.FetchPruneConfig, verifyRemote, dryRun, verbose retainChan := make(chan string, 100) gitscanner := lfs.NewGitScanner(cfg, nil) - gitscanner.Filter = filepathfilter.New(nil, cfg.FetchExcludePaths(), filepathfilter.GitAttributes) + gitscanner.Filter = filepathfilter.New(nil, cfg.FetchExcludePaths(), filepathfilter.GitIgnore) sem := semaphore.NewWeighted(int64(runtime.NumCPU() * 2)) diff --git a/commands/command_smudge.go b/commands/command_smudge.go index c2dba79a..0117b996 100644 --- a/commands/command_smudge.go +++ b/commands/command_smudge.go @@ -154,7 +154,7 @@ func smudgeCommand(cmd *cobra.Command, args []string) { if !smudgeSkip && cfg.Os.Bool("GIT_LFS_SKIP_SMUDGE", false) { smudgeSkip = true } - filter := filepathfilter.New(cfg.FetchIncludePaths(), cfg.FetchExcludePaths(), filepathfilter.GitAttributes) + filter := filepathfilter.New(cfg.FetchIncludePaths(), cfg.FetchExcludePaths(), filepathfilter.GitIgnore) gitfilter := lfs.NewGitFilter(cfg) if n, err := smudge(gitfilter, os.Stdout, os.Stdin, smudgeFilename(args), smudgeSkip, filter); err != nil { diff --git a/t/t-filter-process.sh b/t/t-filter-process.sh index b91d3e5d..2ff4bf17 100755 --- a/t/t-filter-process.sh +++ b/t/t-filter-process.sh @@ -65,6 +65,60 @@ begin_test "filter process: checking out a branch" ) end_test +begin_test "filter process: include/exclude" +( + set -e + + reponame="$(basename "$0" ".sh")-includeexclude" + setup_remote_repo "$reponame" + clone_repo "$reponame" "$reponame" + + git lfs track "*.dat" + mkdir -p foo/bar + + contents_a="contents_a" + contents_a_oid="$(calc_oid $contents_a)" + printf "%s" "$contents_a" > a.dat + cp a.dat foo + cp a.dat foo/bar + + git add .gitattributes a.dat foo + git commit -m "initial commit" + + git push origin main + + # The Git LFS objects for a.dat and foo/bar/a.dat would both download except + # we're going to prevent them from doing so with include/exclude. + # We also need to prevent MSYS from rewriting /foo into a Windows path. + MSYS_NO_PATHCONV=1 git config --global "lfs.fetchinclude" "/foo" + MSYS_NO_PATHCONV=1 git config --global "lfs.fetchexclude" "/foo/bar" + + pushd .. + # Git will choose filter.lfs.process over `filter.lfs.clean` and + # `filter.lfs.smudge` + GIT_TRACE_PACKET=1 git \ + -c "filter.lfs.process=git-lfs filter-process" \ + -c "filter.lfs.clean=false"\ + -c "filter.lfs.smudge=false" \ + -c "filter.lfs.required=true" \ + clone "$GITSERVER/$reponame" "$reponame-assert" + + cd "$reponame-assert" + + pointer="$(pointer "$contents_a_oid" 10)" + + [ "$pointer" = "$(cat a.dat)" ] + assert_pointer "main" "a.dat" "$contents_a_oid" 10 + + [ "$contents_a" = "$(cat foo/a.dat)" ] + assert_pointer "main" "foo/a.dat" "$contents_a_oid" 10 + + [ "$pointer" = "$(cat foo/bar/a.dat)" ] + assert_pointer "main" "foo/bar/a.dat" "$contents_a_oid" 10 + popd +) +end_test + begin_test "filter process: adding a file" ( set -e diff --git a/t/t-fsck.sh b/t/t-fsck.sh index 79444da1..bbe3687d 100755 --- a/t/t-fsck.sh +++ b/t/t-fsck.sh @@ -326,6 +326,49 @@ begin_test "fsck detects invalid objects" ) end_test +begin_test "fsck detects invalid objects except in excluded paths" +( + set -e + + reponame="fsck-objects-exclude" + setup_invalid_objects + + # We need to prevent MSYS from rewriting /foo into a Windows path. + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" + + set +e + git lfs fsck >test.log 2>&1 + RET=$? + set -e + + [ "$RET" -eq 1 ] + [ $(grep -c 'objects: corruptObject: a.dat (.*) is corrupt' test.log) -eq 1 ] + [ $(grep -c 'objects: openError: b.dat (.*) could not be checked: .*' test.log) -eq 1 ] + [ $(grep -c 'objects: corruptObject: foo/a.dat (.*) is corrupt' test.log) -eq 0 ] + [ $(grep -c 'objects: openError: foo/b.dat (.*) could not be checked: .*' test.log) -eq 0 ] + [ $(grep -c 'objects: repair: moving corrupt objects to .*' test.log) -eq 1 ] + + cd .. + rm -rf $reponame + setup_invalid_objects + + # We need to prevent MSYS from rewriting /foo into a Windows path. + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" + + set +e + git lfs fsck --objects >test.log 2>&1 + RET=$? + set -e + + [ "$RET" -eq 1 ] + [ $(grep -c 'objects: corruptObject: a.dat (.*) is corrupt' test.log) -eq 1 ] + [ $(grep -c 'objects: openError: b.dat (.*) could not be checked: .*' test.log) -eq 1 ] + [ $(grep -c 'objects: corruptObject: foo/a.dat (.*) is corrupt' test.log) -eq 0 ] + [ $(grep -c 'objects: openError: foo/b.dat (.*) could not be checked: .*' test.log) -eq 0 ] + [ $(grep -c 'objects: repair: moving corrupt objects to .*' test.log) -eq 1 ] +) +end_test + begin_test "fsck does not detect invalid objects with no LFS objects" ( set -e diff --git a/t/t-prune-worktree.sh b/t/t-prune-worktree.sh index e0240224..554ebdd2 100755 --- a/t/t-prune-worktree.sh +++ b/t/t-prune-worktree.sh @@ -82,7 +82,7 @@ begin_test "prune worktree" git config lfs.fetchrecentcommitsdays 0 # We need to prevent MSYS from rewriting /foo into a Windows path. - MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/**" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" # before worktree, everything except current checkout would be pruned git lfs prune --dry-run 2>&1 | tee prune.log diff --git a/t/t-prune.sh b/t/t-prune.sh index e88ded08..7256a424 100755 --- a/t/t-prune.sh +++ b/t/t-prune.sh @@ -144,7 +144,7 @@ begin_test "prune all excluded paths" git config lfs.pruneoffsetdays 2 # We need to prevent MSYS from rewriting /foo into a Windows path. - MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/**" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" git lfs prune --dry-run --verbose 2>&1 | tee prune.log @@ -264,7 +264,7 @@ begin_test "prune keep unpushed" git config lfs.pruneoffsetdays 2 # We need to prevent MSYS from rewriting /foo into a Windows path. - MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/**" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" # force color codes in git diff meta-information git config color.diff always @@ -800,7 +800,7 @@ begin_test "prune keep stashed changes" assert_local_object "$oid_stashedandexcludedbranch" "${#content_stashedandexcludedbranch}" # We need to prevent MSYS from rewriting /foo into a Windows path. - MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/**" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" # force color codes in git diff meta-information git config color.diff always @@ -924,7 +924,7 @@ begin_test "prune keep stashed changes in index" assert_local_object "$oid_stashedandexcludedbranch" "${#content_stashedandexcludedbranch}" # We need to prevent MSYS from rewriting /foo into a Windows path. - MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/**" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" # force color codes in git diff meta-information git config color.diff always @@ -1077,7 +1077,7 @@ begin_test "prune keep stashed untracked files" assert_local_object "$oid_untrackedstashedandexcludedbranch" "${#content_untrackedstashedandexcludedbranch}" # We need to prevent MSYS from rewriting /foo into a Windows path. - MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/**" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo" # force color codes in git diff meta-information git config color.diff always diff --git a/t/t-smudge.sh b/t/t-smudge.sh index 03fefc77..07e4215f 100755 --- a/t/t-smudge.sh +++ b/t/t-smudge.sh @@ -82,6 +82,25 @@ begin_test "smudge include/exclude" git config "lfs.fetchexclude" "a*" [ "$pointer" = "$(echo "$pointer" | git lfs smudge a.dat)" ] + + mkdir -p foo/bar + echo "smudge a" > foo/a.dat + echo "smudge a" > foo/bar/a.dat + git add foo + git commit -m 'add foo' + + git push origin main + + # The Git LFS objects for a.dat and foo/bar/a.dat would both download except + # we're going to prevent them from doing so with include/exclude. + rm -rf .git/lfs/objects + # We also need to prevent MSYS from rewriting /foo into a Windows path. + MSYS_NO_PATHCONV=1 git config "lfs.fetchinclude" "/foo" + MSYS_NO_PATHCONV=1 git config "lfs.fetchexclude" "/foo/bar" + + [ "$pointer" = "$(echo "$pointer" | git lfs smudge a.dat)" ] + [ "smudge a" = "$(echo "$pointer" | git lfs smudge foo/a.dat)" ] + [ "$pointer" = "$(echo "$pointer" | git lfs smudge foo/bar/a.dat)" ] ) end_test @@ -169,11 +188,42 @@ begin_test "smudge clone with include/exclude" clone="$TRASHDIR/clone_$reponame" git -c lfs.fetchexclude="a*" clone "$GITSERVER/$reponame" "$clone" - cd "$clone" + pushd "$clone" + # Should have succeeded but not downloaded + refute_local_object "$contents_oid" + popd + rm -rf "$clone" - # Should have succeeded but not downloaded - refute_local_object "$contents_oid" + contents2="b" + contents2_oid=$(calc_oid "$contents2") + contents3="c" + contents3_oid=$(calc_oid "$contents3") + mkdir -p foo/bar + printf "%s" "$contents2" > foo/b.dat + printf "%s" "$contents3" > foo/bar/c.dat + git add foo + git commit -m 'add foo' + + assert_local_object "$contents2_oid" 1 + assert_local_object "$contents3_oid" 1 + + git push origin main + + assert_server_object "$reponame" "$contents2_oid" + assert_server_object "$reponame" "$contents3_oid" + + # The Git LFS objects for a.dat and foo/bar/a.dat would both download except + # we're going to prevent them from doing so with include/exclude. + # We also need to prevent MSYS from rewriting /foo into a Windows path. + MSYS_NO_PATHCONV=1 git config --global "lfs.fetchinclude" "/foo" + MSYS_NO_PATHCONV=1 git config --global "lfs.fetchexclude" "/foo/bar" + git clone "$GITSERVER/$reponame" "$clone" + pushd "$clone" + refute_local_object "$contents_oid" + assert_local_object "$contents2_oid" 1 + refute_local_object "$contents3_oid" + popd ) end_test