Skip to content

Conversation

@bk2204
Copy link
Member

@bk2204 bk2204 commented Aug 5, 2019

Normally, the arguments to git lfs track are taken as glob patterns. This is generally useful, but sometimes there are filenames that contain special characters. Add a --filename option that forces arguments to be taken as literal paths instead of glob patterns to make handling these files easier.

In the test helpers, pass -F to grep so that our unusual file names don't trigger undesired pattern matching. This has no effect on other tests, as they all pass a literal filename here.

Fixes #3506.

@bk2204 bk2204 requested a review from a team August 5, 2019 20:51
Copy link
Contributor

@PastelMobileSuit PastelMobileSuit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally, the arguments to "git lfs track" are taken as glob patterns.
This is generally useful, but sometimes there are filenames that contain
special characters. Add a "--filename" option that forces arguments to
be taken as literal paths instead of glob patterns to make handling
these files easier.

In the test helpers, pass -F to grep so that our unusual file names
don't trigger undesired pattern matching. This has no effect on other
tests, as they all pass a literal filename here.
@bk2204 bk2204 merged commit d415353 into git-lfs:master Aug 7, 2019
@bk2204 bk2204 deleted the escaped-track branch August 7, 2019 13:48
@gdlxn-ibm
Copy link

gdlxn-ibm commented Aug 21, 2019

I've found that the --filename option doesn't work properly if the file name contains spaces in that [ and ] characters in [[:space:]] are incorrectly escaped. Here's an example .getattribues file:

git-issue-103-component/200MiB filter=lfs diff=lfs merge=lfs -text
git-issue-103-component/200MiB+1 filter=lfs diff=lfs merge=lfs -text
git-issue-103-component/50MiB+1 filter=lfs diff=lfs merge=lfs -text
git-issue-103-component/55MiB\[1\].test filter=lfs diff=lfs merge=lfs -text
git-issue-103-component/test\[\[:space:\]\]folder/55MiB\[1\].test filter=lfs diff=lfs merge=lfs -text

The last entry was added by

git lfs track --filename "git-issue-103-component/test folder/55MiB[1].test"

@bk2204
Copy link
Member Author

bk2204 commented Aug 21, 2019

Thanks for the information. I've opened #3785 to fix that.

@gdlxn-ibm
Copy link

Thanks for the quick response. I'll test with the fix for #3785 and let you know how it goes.

@gdlxn-ibm
Copy link

The fix for #3785 works for me. Thanks for the quick turn around on getting this issue fixed.

chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 19, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent commit we will also need to ensure that the
file paths we provide as arguments to these functions (specifically,
to assert_pointer()) do not match just any part of the paths
output by "git ls-tree", but only at the start of the path.
That is, "a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the obvious limitation that
if a test uses a filename with a space character in it and
tries to match that, there could be confusion; however, none of
our existing tests require this at the moment.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 20, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent commit we will also need to ensure that the
file paths we provide as arguments to these functions (specifically,
to assert_pointer()) do not match just any part of the paths
output by "git ls-tree", but only at the start of the path.
That is, "a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the obvious limitation that
if a test uses a filename with a space character in it and
tries to match that, there could be confusion; however, none of
our existing tests require this at the moment.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 22, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent commit we will also need to ensure that the
file paths we provide as arguments to these functions (specifically,
to assert_pointer()) do not match just any part of the paths
output by "git ls-tree", but only at the start of the path.
That is, "a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the obvious limitation that
if a test uses a filename with a space character in it and
tries to match that, there could be confusion; however, none of
our existing tests require this at the moment.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 25, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent PR we will also need to ensure that the file paths
we provide as arguments to these functions (specifically, to
assert_pointer()) do not match just any part of the paths output
by "git ls-tree", but only at the start of the path.  That is,
"a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the obvious limitation that
if a test uses a filename with a space character in it and
tries to match that, there could be confusion; however, none of
our existing tests require this at the moment.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 25, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent PR we will also need to ensure that the file paths
we provide as arguments to these functions (specifically, to
assert_pointer()) do not match just any part of the paths output
by "git ls-tree", but only at the start of the path.  That is,
"a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the obvious limitation that
if a test uses a filename with a space character in it and
tries to match that, there could be confusion; however, none of
our existing tests require this at the moment.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 25, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent PR we will also need to ensure that the file paths
we provide as arguments to these functions (specifically, to
assert_pointer()) do not match just any part of the paths output
by "git ls-tree", but only at the start of the path.  That is,
"a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the obvious limitation that
if a test uses a filename with a space character in it and
tries to match that, there could be confusion; however, none of
our existing tests require this at the moment.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Apr 25, 2022
The assert_pointer() and refute_pointer() test helper functions
use grep to match specific lines of output from "git ls-tree".
However, while assert_pointer() matches with fixed patterns by using
grep's -F option, refute_pointer() does not.  This difference was
introduced in commit fc421da of
PR git-lfs#3756, so we update refute_pointer() here to match.

In a subsequent PR we will also need to ensure that the file paths
we provide as arguments to these functions (specifically, to
assert_pointer()) do not match just any part of the paths output
by "git ls-tree", but only at the start of the path.  That is,
"a.bin" should match "a.bin" but not "foo/a.bin".

Because the output from "git ls-tree" is space-separated, we
prepend a space to our fixed patterns so we match against the
start of the file path.  This has the limitation that if a test
has a file named "foo a.bin", and calls one of these functions with
just the filename "a.bin", it will incorrectly match the filename
with a space.

However, at present only one of our tests that calls either of these
functions, specifically the "track: escaped glob pattern with spaces
in .gitattributes" test in t/t-track.sh, has a filename with a
space in it, and does not create any additional files which might
be confused by that filename.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a way to automatically escape/quote filenames given to git lfs track

3 participants