This lets users limit added/stashed files or exclude files based on file attributes. For example, the chromium project would like to use this like "git add --all ':(exclude,attr:submodule)'", as submodules are managed in a unique way and often results in submodule changes that users do not want in their commits. This does not change any attr magic implementation. It is only adding attr as an allowed pathspec in git-add/stash, which was previously blocked (for stash/add) by GUARD_PATHSPEC and (for add only) a pathspec mask in parse_pathspec()). With this patch, attr is supported for the two commands. It is possible that when the attr pathspec feature was first added in b0db704652 (pathspec: allow querying for attributes, 2017-03-13), "PATHSPEC_ATTR" was just unintentionally left out of a few GUARD_PATHSPEC() invocations. Later, to get a more user-friendly error message when attr was used with git-add, PATHSPEC_ATTR was added as a mask to git-add's invocation of parse_pathspec() in 84d938b732 (add: do not accept pathspec magic 'attr', 2018-09-18). Any bugs this may trigger will be fixed in follow-up patches. Signed-off-by: Joanna Wang <jojwang@xxxxxxxxxx> --- I have tested this thoroughly with different flags, non-root directories, and other magic pathspecs, but may be unaware of some edge cases. The GUARD_PATHSPEC() in dir.c is a potential part of the call stack of fill_directory(), which is called by add, stash, grep, ls-files, sparse-checkout, and status. However, this patch only seems to add new behavior to git-add and git-stash. grep, ls-files, and status already support attr pathspecs and running some of these commands with the debugger confirms this GUARD_PATHSPEC() is never reached. But again I am not aware of all edge cases. For sparse-checkout I'm not sure if/how pathspec can be used, but either way did not see new behavior. For context, attr magic support for git-add is part of work discussed here: https://lore.kernel.org/git/xmqqfs2yjgvr.fsf@gitster.g/T/#t builtin/add.c | 5 +++-- dir.c | 8 +------- t/t6135-pathspec-with-attrs.sh | 34 ++++++++++++++++++++++++++-------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index c27254a5cd..ef0b8d55fd 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -424,7 +424,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) * Check the "pathspec '%s' did not match any files" block * below before enabling new magic. */ - parse_pathspec(&pathspec, PATHSPEC_ATTR, + parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_FULL | PATHSPEC_SYMLINK_LEADING_PATH, prefix, argv); @@ -504,7 +504,8 @@ int cmd_add(int argc, const char **argv, const char *prefix) PATHSPEC_LITERAL | PATHSPEC_GLOB | PATHSPEC_ICASE | - PATHSPEC_EXCLUDE); + PATHSPEC_EXCLUDE | + PATHSPEC_ATTR); for (i = 0; i < pathspec.nr; i++) { const char *path = pathspec.items[i].match; diff --git a/dir.c b/dir.c index 8486e4d56f..55c9607b1a 100644 --- a/dir.c +++ b/dir.c @@ -2173,13 +2173,7 @@ static int exclude_matches_pathspec(const char *path, int pathlen, if (!pathspec || !pathspec->nr) return 0; - GUARD_PATHSPEC(pathspec, - PATHSPEC_FROMTOP | - PATHSPEC_MAXDEPTH | - PATHSPEC_LITERAL | - PATHSPEC_GLOB | - PATHSPEC_ICASE | - PATHSPEC_EXCLUDE); + GUARD_PATHSPEC(pathspec, PATHSPEC_ALL_MAGIC); for (i = 0; i < pathspec->nr; i++) { const struct pathspec_item *item = &pathspec->items[i]; diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh index f70c395e75..e11d8cb119 100755 --- a/t/t6135-pathspec-with-attrs.sh +++ b/t/t6135-pathspec-with-attrs.sh @@ -64,12 +64,23 @@ test_expect_success 'setup .gitattributes' ' fileSetLabel label fileValue label=foo fileWrongLabel label☺ + newFileA* labelA + newFileB* labelB EOF echo fileSetLabel label1 >sub/.gitattributes && git add .gitattributes sub/.gitattributes && git commit -m "add attributes" ' +test_expect_success 'setup .gitignore' ' + cat <<-\EOF >.gitignore && + actual + expect + EOF + git add .gitignore && + git commit -m "add gitignore" +' + test_expect_success 'check specific set attr' ' cat <<-\EOF >expect && fileSetLabel @@ -150,6 +161,7 @@ test_expect_success 'check specific value attr (2)' ' test_expect_success 'check unspecified attr' ' cat <<-\EOF >expect && .gitattributes + .gitignore fileA fileAB fileAC @@ -175,6 +187,7 @@ test_expect_success 'check unspecified attr' ' test_expect_success 'check unspecified attr (2)' ' cat <<-\EOF >expect && HEAD:.gitattributes + HEAD:.gitignore HEAD:fileA HEAD:fileAB HEAD:fileAC @@ -200,6 +213,7 @@ test_expect_success 'check unspecified attr (2)' ' test_expect_success 'check multiple unspecified attr' ' cat <<-\EOF >expect && .gitattributes + .gitignore fileC fileNoLabel fileWrongLabel @@ -239,14 +253,18 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' ' test_i18ngrep "Only one" actual ' -test_expect_success 'fail if attr magic is used places not implemented' ' - # The main purpose of this test is to check that we actually fail - # when you attempt to use attr magic in commands that do not implement - # attr magic. This test does not advocate git-add to stay that way, - # though, but git-add is convenient as it has its own internal pathspec - # parsing. - test_must_fail git add ":(attr:labelB)" 2>actual && - test_i18ngrep "magic not supported" actual +test_expect_success 'check that attr magic works for git-add' ' + # attr magic was previously blocked for git-add. With attr magic + # enabled for git-add, add a basic test to make sure it works as + # expected and add more tests if more bugs are discovered. + cat <<-\EOF >expect && + sub/newFileA-foo + EOF + touch sub/newFileA-foo && + touch sub/newFileB-foo && + git add --all ":(exclude,attr:labelB)" && + git diff --name-only --cached >actual && + test_cmp expect actual ' test_expect_success 'abort on giving invalid label on the command line' ' -- 2.42.0.582.g8ccd20d70d-goog
