Commit Graph

6 Commits

Author SHA1 Message Date
Chris Darroch
b8d97e3dd7 git/gitattr,t: handle unspecified macro attributes
In commit 1ff52542da97d6418689a5528112b57e37536014 of PR #3391 we
introduced the MacroProcessor type and methods to support the use of
macro attributes in .gitattributes files.

However, we do not currently support the case where a macro attributes
is specified with a "!" prefix, which Git handles by setting all
attributes defined by the macro attribute back to the unspecified state.
(Note that the "-" prefix is not supported by Git for macro attributes,
only the "!" one.)

To mimic the same behaviour in Git LFS we add a check for a macro
attribute with its Unspecified bool set to "true", and when this is
detected we iterate through the set of attributes defined by the macro
attribute and set them all to the same unspecified state.

We also add tests to confirm this new handling works as expected, both
a new Go test and a new "fsck does not detect invalid pointers with
negated macro patterns" test in t/t-fsck.sh that will not succeed without
the changes to the MacroProcessor in this commit.  Without these changes,
any patterns that reference a macro attribute with the "!" prefix are not
processed as making the macro's attributes all unspecified again, and so
non-pointer files matching those patterns are reported as invalid Git LFS
pointers.

In the new test in t/t-fsck.sh we include comments describing how the
"git lfs fsck" command currently processes .gitattributes files in
the order returned by "git ls-tree", and so a .gitattributes file in
a subdirectory such as .dir/ will be parsed before the top-level
.gitattributes one because it appears first in the "git ls-tree" output.
The result is that any macro attribute references in the
.dir/.gitattributes file will not be resolved properly, and so our
test succeeds but not quite for the right reasons.

We also add a new "macros with unspecified flag" test in the
t/t-attributes.sh test script, but this test ultimately is only a
placeholder as it can not actually test that the "git lfs track" command
will not overwrite a pattern in a .gitattributes file in a subdirectory
if it references a macro attribute defined in the top-level .gitattributes
file and the reference has the "!" prefix.  This is due to the fact that
the "git lfs track" command parses .gitattributes files in the order
of the length of their full paths, from longest to shortest, and so
macro attribute references can not be resolved except within the top-level
.gitattributes file (with some caveats regarding the .git/info/attributes
file and the global and system attributes files).

For now we defer resolution of both this issue and the one described
regarding the "git lfs fsck" command to the future.
2022-11-08 01:07:05 -08:00
Chris Darroch
12d9ecd736 t/t-{attributes,fsck}.sh: test macro support
In commit f4b8938fd60dfb705d83a62f153acd59fe067a51 of PR #3391 the
t/t-attributes.sh test script was introduced with its initial "macros"
test, which validates that the "git lfs track" command is able to parse
macro attribute definitions in the top-level .gitattributes file and
resolve references to those macros in the same file.  It also confirms
that the command does not accept macro definitions in .gitattributes
files in subdirectories, as Git does not accept these either.

However, Git does resolve macro attribute references from .gitattributes
files in subdirectories, so long as they refer to macro attributes
defined in the top-level .gitattributes (or one of the other files where
definitions are accepted, such as the .git/info/attributes file).  But
the "git lfs track" command at present does not resolve such references
consistently because it sorts the attributes files by path length and
then processes them strictly in that order, from longest to shortest.
Thus references to macro attributes defined in the top-level .gitattributes
file from other attributes files never succeed because the top-level file
is always parsed last (except for the global and system attributes files).

We therefore add a note to this effect in the "macros" test to explain
why we do not test valid macro attribute references in a .gitattributes
file in a subdirectory.

(There is also an inconsistency in how "git lfs track" handles references
to macro attributes defined in the .git/info/attributes file, because if
the references appear in .gitattributes files whose full file path in the
repository is longer than ".git/info/attributes", then the references are
not resolved as these files are parsed before the .git/info/attributes one,
whereas references from other .gitattributes files are resolved.)

Separately, in commit 608bc8d53efe0bfc789dae4b934e18af69ebd8e5 of PR #4525
support for scanning the repository contents using the output of the
"git ls-tree" command was added to help enable the "git lfs fsck" to
search for invalid Git LFS pointer files.  The GitScanner.ScanRefByTree()
method invokes a chain of functions, of which catFileBatchTreeForPointers()
reads Git blob metadata and examines each blob in turn to see if it is
a Git LFS pointer or a .gitattributes file, and if it is the latter it
reads and parses its contents, including macro attribute definitions if
the file is the top-level .gitattributes file.

We therefore add a "fsck detects invalid pointers with macro patterns"
test to the t/t-fsck.sh test script which validates the ability of the
"git lfs fsck" command to report as invalid pointers any files matching
patterns with a "filter=lfs" attribute defined by reference to a macro
attribute defined in the top-level .gitattributes file.

To do this we refactor the setup_invalid_pointers() helper function so
that we can reuse some of its code in a new, smaller function that just
creates invalid pointers.

However, we also add a note explaining that we can not yet test this
behaviour with a .gitattributes file whose parent directory sorts
before the top-level .gitattributes one in the output from "git ls-tree".
Because that command outputs its results sorted by filepath, a file such
as .dir/.gitattributes will be listed before the top-level .gitattributes
file, and so any macro attribute references from the .dir/.gitattributes
file to macro attributes defined in the top-level .gitattributes file
will not be resolved in the way that Git resolves them.

For now we defer resolution of this issue and the ones described regarding
the "git lfs track" command to the future.
2022-11-08 00:41:21 -08:00
Chris Darroch
3c46a4641c t/t-attributes.sh: test check top-level macro rule
In commit f4b8938fd60dfb705d83a62f153acd59fe067a51 of PR #3391 the
t/t-attributes.sh file was added with an initial "macros" test, and
part of that test confirms that macro attribute definitions are only
processed when they appear in the top-level .giattributes file in
a repository.

The test does confirm this in that it creates both an "lfs2" macro
attribute definition and assignment of that attribute name to a
file pattern in a .gitattributes file in a subdirectory, and then
validates that a file matching that pattern in the subdirectory is
not converted into an LFS object.

The test also includes a second check of this logic in which it confirms
that a "git lfs track" command for the file pattern in the subdirectory
succeeds, i.e., that it does not fail because the file pattern was already
assigned the normal "filter=lfs" attribute by the "lfs2" macro attribute.

However, this particular check will always succeed, even if macro
attribute definitions like the "lfs2" one are incorrectly accepted from
.gitattributes files other than the top-level one.  This is because the
"git lfs track" command is run in the top-level directory and only sets
a pattern that includes the subdirectory in its path (i.e., "dir/*.bin").
This will succeed regardless of whether the "*.bin" pattern is assigned
to LFS attributes in the dir/.gitattributes file.

We therefore make this second check more sensitive to potential future
regressions by running the "git lfs track" command in the subdirectory.
Now if the macro definition in the .gitattributes file in that directory
is (incorrectly) read as a valid definition, and the check which tests that
the file in the subdirectory has not been converted into an LFS object is
skipped, then this second check fails as expected.
2022-11-06 22:33:19 -08:00
brian m. carlson
c38ad0f0fd
commands/track: read all attributes files to parse macros
Currently, when checking for files being tracked, we don't read
gitattributes files that are not in the working directory, so we don't
expand any macros that might be in them.  Ensure that when we read any
gitattributes file, we read all of them, and in the right order, to
expand macros in later files that have been defined in earlier ones.
2018-12-03 17:18:50 +00:00
brian m. carlson
4a5f7e43a3
git: look up default global attributes file if none specified
Git documents the default global attributes file if core.attributesFile
is not specified as $XDG_CONFIG_HOME/git/attributes, or
$HOME/.config/git/attributes if $XDG_CONFIG_HOME is not set or empty.
Teach our attributes file code about this file and read it as normal.
2018-12-03 17:18:50 +00:00
brian m. carlson
f4b8938fd6
git: expand macros when reading gitattributes files
Read and expand macros when reading the gitattributes files so that
users can use shorthands such as "lfs" for "filter=lfs diff=lfs
merge=lfs".  Pass the same macro processor into each attribute lookup
function, since macros can be defined in the system or global file and
then used in per-repository files.
2018-12-03 17:18:49 +00:00