{"thread":{"id":"64914","subject":"[PATCH] fix git add :!x exiting with error when x is in .gitignore","startedAt":"2026-02-04T13:30:52Z","lastAt":"2026-02-04T20:47:50Z","messageCount":6,"participants":["Remy D. Farley","Junio C Hamano","Tian Yuchen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"535147","messageId":"20260204132747.1564157-1-one-d-wide@protonmail.com","threadId":"64914","inReplyTo":null,"subject":"[PATCH] fix git add :!x exiting with error when x is in .gitignore","fromName":"Remy D. Farley","fromEmail":"one-d-wide@protonmail.com","sentAt":"2026-02-04T13:30:38Z","receivedAt":"2026-02-04T13:30:52Z","isPatch":true,"sender":{"key":"one-d-wide@protonmail.com","avatar":null},"body":"`git add :!x .`, which is also executed as part of `git stash :!x`,\nseems to treat pathspec with and without exclude magic the same, exiting\nwith error when \"x\" exists and is in gitignore.\n\nGit-add manpage doesn't specify that exclude pathspecs should be treated\nanyhow differently from normal ones, which seems like a bug. Two\ninconsistencies I noticed: `git add :!ignored .` succeeds when \"ignored\"\nfile doesn't exist, and `git add :!ignored/x .` succeeds even when\n\"ignored/x\" file exists.\n\nThis commit makes makes `git add :!x` not error on x being excluded path.\n\n\n| $ sh repro.sh\n| [...]\n| + echo x >.gitignore\n| + echo x >x\n| + git stash --include-untracked -- ':!x'\n| Saved working directory and index state WIP on main: c8a842d Init\n| The following paths are ignored by one of your .gitignore files:\n| x\n| hint: Use -f if you really want to add them.\n| hint: Disable this message with \"git config set advice.addIgnoredFile false\"\n| + echo exited with code 1\n| exited with code 1\n\n\n| # repro.sh\n| rm -rf repro; mkdir repro; cd repro\n| trap 'echo exited with code $?' EXIT\n| set -euo pipefail -o xtrace\n|\n| git init\n| git commit -m Init --allow-empty\n|\n| # Commenting out either of the following lines makes git add/stash below succeed\n| echo x >.gitignore\n| echo x >x\n|\n| # Git add . is executed as part of git stash, as can be seen using strace -ffeexecve:\n| git add -- \":!x\" . # fails\n| # git stash --include-untracked -- \":!x\" # fails\n\n---\nI'm not sure who else to cc, last commit touching this code is 2ec87741\nfrom 10 year ago, being a mere refactoring. I think this bug was simply\noverlooked when introducing PATHSPEC_EXCLUDE.\n\nThanks to Tian Yuchen for looking at my earlier submission (and noticing\nan awkwardly stupid bug there).\n---\n dir.c                              |  3 +++\n t/t2204-add-ignored.sh             | 14 ++++++++++++++\n t/t3905-stash-include-untracked.sh | 23 +++++++++++++++++++++++\n 3 files changed, 40 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex b00821f294..ed6b99e337 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2280,6 +2280,9 @@ static int exclude_matches_pathspec(const char *path, int pathlen,\n \t\tconst struct pathspec_item *item = &pathspec->items[i];\n \t\tint len = item->nowildcard_len;\n \n+\t\tif (item->magic & PATHSPEC_EXCLUDE)\n+\t\t\tcontinue;\n+\n \t\tif (len == pathlen &&\n \t\t    !ps_strncmp(item, item->match, path, pathlen))\n \t\t\treturn 1;\ndiff --git a/t/t2204-add-ignored.sh b/t/t2204-add-ignored.sh\nindex 31eb233df5..76c53fbfde 100755\n--- a/t/t2204-add-ignored.sh\n+++ b/t/t2204-add-ignored.sh\n@@ -47,6 +47,20 @@ do\n \ttest_expect_success \"complaints for ignored $i with unignored file output\" '\n \t\ttest_grep -e \"Use -f if\" err\n \t'\n+\n+\ttest_expect_success \"no complaints for unignored file with ignored :!$i\" '\n+\t\trm -f .git/index &&\n+\t\tgit add file \":!$i\" &&\n+\t\tgit ls-files file \"$i\" >out &&\n+\t\ttest -s out\n+\t'\n+\n+\ttest_expect_success \"complaints for ignored $i with ignored :!ign\" '\n+\t\trm -f .git/index &&\n+\t\ttest_must_fail git add \"$i\" :!ign 2>err &&\n+\t\tgit ls-files \"$i\" ign >out &&\n+\t\ttest_must_be_empty out\n+\t'\n done\n \n for i in sub sub/*\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 7704709054..028ff3efc0 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -206,6 +206,29 @@ test_expect_success 'stash push --include-untracked with pathspec' '\n \ttest_path_is_file foo\n '\n \n+test_expect_success 'stash push --include-untracked with :!pathspec' '\n+\t>foo &&\n+\t>bar &&\n+\tgit stash push --include-untracked -- :!bar &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_file foo\n+'\n+\n+test_expect_success 'stash push --include-untracked with :!pathspec in .gitignore' '\n+\techo ignored > .gitignore &&\n+\t>foo &&\n+\t>ignored &&\n+\tgit stash push --include-untracked -- :!ignored &&\n+\ttest_path_is_file ignored &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file ignored &&\n+\ttest_path_is_file foo\n+'\n+\n test_expect_success 'stash push with $IFS character' '\n \t>\"foo bar\" &&\n \t>foo &&\n-- \n2.51.2\n\n\n"},{"id":"535167","messageId":"xmqqo6m4pi84.fsf@gitster.g","threadId":"64914","inReplyTo":"20260204132747.1564157-1-one-d-wide@protonmail.com","subject":"Re: [PATCH] fix git add :!x exiting with error when x is in .gitignore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-04T16:48:59Z","receivedAt":"2026-02-04T16:49:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Remy D. Farley\" <one-d-wide@protonmail.com> writes:\n\n> diff --git a/dir.c b/dir.c\n> index b00821f294..ed6b99e337 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -2280,6 +2280,9 @@ static int exclude_matches_pathspec(const char *path, int pathlen,\n>  \t\tconst struct pathspec_item *item = &pathspec->items[i];\n>  \t\tint len = item->nowildcard_len;\n>  \n> +\t\tif (item->magic & PATHSPEC_EXCLUDE)\n> +\t\t\tcontinue;\n> +\n>  \t\tif (len == pathlen &&\n>  \t\t    !ps_strncmp(item, item->match, path, pathlen))\n>  \t\t\treturn 1;\n\nA question that immediately comes to mind is if it is appropriate\nfor a negated pathspec element to recuse itself like this from the\ndecision process and let other pathspec elements decide the fate of\nthe path, or if a negated pathspec element should take a more active\nrole of saying \"no\" (no, not by immediately returning 0, but this\nloop may have to become a two step process if we wanted to implement\ne.g., for the function to yield \"yes\", it has to match at least one\npositive pathspec element and zero negated one, or something like\nthat).\n\nWhat should\n\n\tgit add \"$x\" \":!$y\"\n\ndo when a path <matches, does not match> $X and <matches, does not\nmatch> $Y?  We have four combinations to consider in such a case.\nThe code in the patch says it should behave identically to\n\n\tgit add \"$x\"\n\nand negated \":!$y\" should not make any difference.  Is that what we\nwant?\n\nThanks.\n"},{"id":"535184","messageId":"9c5be231-f340-4a97-850e-d43c78b2c889@gmail.com","threadId":"64914","inReplyTo":"xmqqo6m4pi84.fsf@gitster.g","subject":"Re: [PATCH] fix git add :!x exiting with error when x is in .gitignore","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-04T17:53:05Z","receivedAt":"2026-02-04T17:53:09Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"On 2/5/26 00:48, Junio C Hamano wrote:\n> \"Remy D. Farley\" <one-d-wide@protonmail.com> writes:\n\n> A question that immediately comes to mind is if it is appropriate\n> for a negated pathspec element to recuse itself like this from the\n> decision process and let other pathspec elements decide the fate of\n> the path, or if a negated pathspec element should take a more active\n> role of saying \"no\" (no, not by immediately returning 0, but this\n> loop may have to become a two step process if we wanted to implement\n> e.g., for the function to yield \"yes\", it has to match at least one\n> positive pathspec element and zero negated one, or something like\n> that).\n\nYou are right. To illustrate, if we run:\n\ngit add ignored_file \":!ignored_file\"\n\nThen following things might happen with the patch:\n-> For the first item,\n\t- Does it match 'exclude'? No.\n\t- Does it match 'path'? Yes.\n\t- Return 1.\n-> For the second item,\n\t- Is never reached\n-> Git complain,\n\t'The following paths are ignored: ignored_file.'\n\nIn other word, it's not the expected silent no-op (returning 0).\n\nAs you suggested, The loop needs to verify that the path matches at \nleast one positive item AND matches none of the negative items. A \npossible way to acheive it is:\n(Notice that we no longer return 1 in the half way)\n\n\n >bool matched_positive = false;\n >\n >for (item in pathspec) {\n >\tif (item matches patch) {\n >\t\tif (item is exclude) {\n >\t\t\treturn 0;\n >\t\t} else {\n >\t\t\tmatched_positive = true;\n >\t\t}\n >\t}\n >}\n >\n >return matched_positive ? 1 : 0;\n\nBy the way, I think extreme cases like 'git add x :!x' should be added \ninto the test scripts.\n\nRegards,\n\nYuchen\n\n\n\n\n"},{"id":"535186","messageId":"xmqq5x8cpcrd.fsf@gitster.g","threadId":"64914","inReplyTo":"9c5be231-f340-4a97-850e-d43c78b2c889@gmail.com","subject":"Re: [PATCH] fix git add :!x exiting with error when x is in .gitignore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-04T18:47:02Z","receivedAt":"2026-02-04T18:47:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen <a3205153416@gmail.com> writes:\n\n> As you suggested, The loop needs to verify that the path matches at \n> least one positive item AND matches none of the negative items. A \n> possible way to acheive it is:\n> (Notice that we no longer return 1 in the half way)\n>\n>  >bool matched_positive = false;\n>  >\n>  >for (item in pathspec) {\n>  >\tif (item matches patch) {\n>  >\t\tif (item is exclude) {\n>  >\t\t\treturn 0;\n>  >\t\t} else {\n>  >\t\t\tmatched_positive = true;\n>  >\t\t}\n>  >\t}\n>  >}\n>  >\n>  >return matched_positive ? 1 : 0;\n\nOne caveat.  The case without any positive pathspec entries needs\nspecial consideration.  I suspect, but can be totally wrong as I\ndidn't think things through thoroughly, that\n\n    git add \"!$y\"\n\nwould want to behave as if an implicit \"everything matches\" was\ngiven, i.e.,\n\n    git add \"!$y\" .\n\nwhile a pathspec with one or more positive entries would not need\nand want such an implicit \"everything\" treatment.\n\n> By the way, I think extreme cases like 'git add x :!x' should be added \n> into the test scripts.\n\nTrue.\n"},{"id":"535190","messageId":"24VdqZCRHE7M9q7Rp-IH60MmQrEOW5lzhtd1-SUNqEhV_OTzGiCUkVDL5ngVJbyWRMDZ2GlWCJ9wkMSJLsJh8QYO4gRhDMGyzhfuGAODOs8=@protonmail.com","threadId":"64914","inReplyTo":"xmqq5x8cpcrd.fsf@gitster.g","subject":"[PATCH] fix git add :!x exiting with error when x is in .gitignore","fromName":"Remy D. Farley","fromEmail":"one-d-wide@protonmail.com","sentAt":"2026-02-04T20:11:32Z","receivedAt":"2026-02-04T20:11:44Z","isPatch":true,"sender":{"key":"one-d-wide@protonmail.com","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> A question that immediately comes to mind is if it is appropriate\n> for a negated pathspec element to recuse itself like this from the\n> decision process and let other pathspec elements decide the fate of\n> the path, or if a negated pathspec element should take a more active\n> role of saying \"no\" (no, not by immediately returning 0, but this\n> loop may have to become a two step process if we wanted to implement\n> e.g., for the function to yield \"yes\", it has to match at least one\n> positive pathspec element and zero negated one, or something like\n> that).\n>\n> What should\n>\n>     git add \"$x\" \":!$y\"\n>\n> do when a path <matches, does not match> $X and <matches, does not\n> match> $Y? We have four combinations to consider in such a case.\n> The code in the patch says it should behave identically to\n>\n>     git add \"$x\"\n>\n> and negated \":!$y\" should not make any difference. Is that what we\n> want?\n\n\nI indeed failed to consider cases where pathspecs could interfere with each\nother, sorry.\n\nIt does seems like we don't want to just blanketly ignore negated pathspecs,\nat least for the sake of consistency:\n\n    git add -n :!a a/ignored/c # is ok (even on mainline, nothing is added)\n\n    git add -n :!a/b a/b # ok (same)\n\nProbably the same should hold after this patch if \"a\" or \"a/b\" were excluded.\nI'll try to think this through.\n\n\nTian Yuchen <a3205153416@gmail.com> wrote:\n> By the way, I think extreme cases like 'git add x :!x' should be added\n> into the test scripts.\n\n\nI think this was already covered, though with a simplistic (wrong) approach.\n\n>  for i in ign dir/ign dir/sub dir/sub/*ign sub/file sub sub/*\n>  do\n>  \t[...]\n> +\ttest_expect_success \"complaints for ignored $i with ignored :!ign\" '\n> +\t\trm -f .git/index &&\n> +\t\ttest_must_fail git add \"$i\" :!ign 2>err &&\n> +\t\tgit ls-files \"$i\" ign >out &&\n> +\t\ttest_must_be_empty out\n> +\t'\n>  done\n\n\nJunio C Hamano <gitster@pobox.com> wrote:\n> One caveat.  The case without any positive pathspec entries needs\n> special consideration.  I suspect, but can be totally wrong as I\n> didn't think things through thoroughly, that\n> \n>     git add \"!$y\"\n> \n> would want to behave as if an implicit \"everything matches\" was\n> given, i.e.,\n> \n>     git add \"!$y\" .\n>\n> while a pathspec with one or more positive entries would not need\n> and want such an implicit \"everything\" treatment.\n\n\nThis case is actually already handled by the pathspec itself.\n\n\nFrom pathspec.c:\n> void parse_pathspec(struct pathspec *pathspec,\n> \t\t    unsigned magic_mask, unsigned flags,\n> \t\t    const char *prefix, const char **argv)\n> {\n> \t[...]\n> \t/*\n> \t * If everything is an exclude pattern, add one positive pattern\n> \t * that matches everything. We allocated an extra one for this.\n> \t */\n> \tif (nr_exclude == n) {\n> \t\tint plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;\n> \t\tinit_pathspec_item(item + n, 0, prefix, plen, \".\");\n> \t\tpathspec->nr++;\n> \t}\n"},{"id":"535191","messageId":"xmqqikccnsln.fsf@gitster.g","threadId":"64914","inReplyTo":"24VdqZCRHE7M9q7Rp-IH60MmQrEOW5lzhtd1-SUNqEhV_OTzGiCUkVDL5ngVJbyWRMDZ2GlWCJ9wkMSJLsJh8QYO4gRhDMGyzhfuGAODOs8=@protonmail.com","subject":"Re: [PATCH] fix git add :!x exiting with error when x is in .gitignore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-04T20:47:48Z","receivedAt":"2026-02-04T20:47:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Remy D. Farley\" <one-d-wide@protonmail.com> writes:\n\n> This case is actually already handled by the pathspec itself.\n>\n>\n> From pathspec.c:\n>> void parse_pathspec(struct pathspec *pathspec,\n>> \t\t    unsigned magic_mask, unsigned flags,\n>> \t\t    const char *prefix, const char **argv)\n>> {\n>> \t[...]\n>> \t/*\n>> \t * If everything is an exclude pattern, add one positive pattern\n>> \t * that matches everything. We allocated an extra one for this.\n>> \t */\n>> \tif (nr_exclude == n) {\n>> \t\tint plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;\n>> \t\tinit_pathspec_item(item + n, 0, prefix, plen, \".\");\n>> \t\tpathspec->nr++;\n>> \t}\n\nYes, that is from Linus 9 years ago plus a bit of my work, in\n859b7f1d (pathspec: don't error out on all-exclusionary pathspec\npatterns, 2017-02-07) and b02fdbc8 (pathspec: correct an empty\nstring used as a pathspec element, 2022-05-29).  No wonder it\nsounded familiar ;-).\n\nThanks.\n"}]}