| Message ID | 20180918173159.30300-1-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add: do not accept pathspec magic 'attr' | expand |
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > Commit b0db704652 (pathspec: allow querying for attributes - > 2017-03-13) adds new pathspec magic 'attr' but only with > match_pathspec(). "git add" has some pathspec related code that still > does not know about 'attr' and will bail out: > > $ git add ':(attr:foo)' > fatal: BUG:dir.c:1584: unsupported magic 40 > > A better solution would be making this code support 'attr'. But I > don't know how much work is needed (I'm not familiar with this new > magic). For now, let's simply reject this magic with a friendlier > message: > > $ git add ':(attr:foo)' > fatal: :(attr:foo): pathspec magic not supported by this command: 'attr' > > Reported-by: smaudet@sebastianaudet.com > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > Since Brandon is currently unreachable, let's do this for now. Thanks. This certainly make it better than the status quo.
Junio C Hamano <gitster@pobox.com> writes: > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > >> Commit b0db704652 (pathspec: allow querying for attributes - >> 2017-03-13) adds new pathspec magic 'attr' but only with >> match_pathspec(). "git add" has some pathspec related code that still >> does not know about 'attr' and will bail out: >> >> $ git add ':(attr:foo)' >> fatal: BUG:dir.c:1584: unsupported magic 40 >> >> A better solution would be making this code support 'attr'. But I >> don't know how much work is needed (I'm not familiar with this new >> magic). For now, let's simply reject this magic with a friendlier >> message: >> >> $ git add ':(attr:foo)' >> fatal: :(attr:foo): pathspec magic not supported by this command: 'attr' >> >> Reported-by: smaudet@sebastianaudet.com >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> >> --- >> Since Brandon is currently unreachable, let's do this for now. > > Thanks. This certainly make it better than the status quo. And of course, there is an interesting glitch found after I almost finish day's integration cycle X-<. -- >8 -- Subject: [PATCH] fixup! add: do not accept pathspec magic 'attr' [Add the following when squashing] Update t6135 so that the expected error message is from the "graceful" rejection codepath, not "oops, we were supposed to reject the request to trigger this magic" codepath. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t6135-pathspec-with-attrs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh index 77b8cef661..e436a73962 100755 --- a/t/t6135-pathspec-with-attrs.sh +++ b/t/t6135-pathspec-with-attrs.sh @@ -164,11 +164,11 @@ test_expect_success 'fail if attr magic is used places not implemented' ' # 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 "unsupported magic" actual + test_i18ngrep "magic not supported" actual ' test_expect_success 'abort on giving invalid label on the command line' ' test_must_fail git ls-files . ":(attr:☺)" '
diff --git a/builtin/add.c b/builtin/add.c index 9916498a29..0b64bcdebe 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -454,7 +454,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, 0, + parse_pathspec(&pathspec, PATHSPEC_ATTR, PATHSPEC_PREFER_FULL | PATHSPEC_SYMLINK_LEADING_PATH, prefix, argv);
Commit b0db704652 (pathspec: allow querying for attributes - 2017-03-13) adds new pathspec magic 'attr' but only with match_pathspec(). "git add" has some pathspec related code that still does not know about 'attr' and will bail out: $ git add ':(attr:foo)' fatal: BUG:dir.c:1584: unsupported magic 40 A better solution would be making this code support 'attr'. But I don't know how much work is needed (I'm not familiar with this new magic). For now, let's simply reject this magic with a friendlier message: $ git add ':(attr:foo)' fatal: :(attr:foo): pathspec magic not supported by this command: 'attr' Reported-by: smaudet@sebastianaudet.com Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Since Brandon is currently unreachable, let's do this for now. I might eventually find time to go over pathspec code and see if I can add 'attr' support to the rest of the commands, but no promise. builtin/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)