Skip to content

Commit

Permalink
commands,t: gitignore matching for fetch filters
Browse files Browse the repository at this point in the history
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 git-lfs#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.)
  • Loading branch information
chrisd8088 committed Apr 28, 2022
1 parent 0df26c1 commit c7642ec
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 13 deletions.
2 changes: 1 addition & 1 deletion commands/command_filter_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion commands/command_fsck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion commands/command_prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
2 changes: 1 addition & 1 deletion commands/command_smudge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
54 changes: 54 additions & 0 deletions t/t-filter-process.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions t/t-fsck.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion t/t-prune-worktree.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions t/t-prune.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
56 changes: 53 additions & 3 deletions t/t-smudge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"

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

# Should have succeeded but not downloaded
refute_local_object "$contents_oid"
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

Expand Down

0 comments on commit c7642ec

Please sign in to comment.