patchfix git add :!x exiting with error when x is in .gitignore

`git add :!x .`, which is also executed as part of `git stash :!x`, seems to treat pathspec with and without exclude magic the same, exiting with error when "x" exists and is in gitignore.

Git-add manpage doesn't specify that exclude pathspecs should be treated anyhow differently from normal ones, which seems like a bug. Two inconsistencies I noticed: `git add :!ignored .` succeeds when "ignored" file doesn't exist, and `git add :!ignored/x .` succeeds even when "ignored/x" file exists.

This commit makes makes `git add :!x` not error on x being excluded path.

| $ sh repro.sh
| [...]
| + echo x >.gitignore
| + echo x >x
| + git stash --include-untracked -- ':!x'
| Saved working directory and index state WIP on main: c8a842d Init
| The following paths are ignored by one of your .gitignore files:
| x
| hint: Use -f if you really want to add them.
| hint: Disable this message with "git config set advice.addIgnoredFile false"
| + echo exited with code 1
| exited with code 1

| # repro.sh
| rm -rf repro; mkdir repro; cd repro
| trap 'echo exited with code $?' EXIT
| set -euo pipefail -o xtrace
|
| git init
| git commit -m Init --allow-empty
|
| # Commenting out either of the following lines makes git add/stash below succeed
| echo x >.gitignore
| echo x >x
|
| # Git add . is executed as part of git stash, as can be seen using strace -ffeexecve:
| git add -- ":!x" . # fails
| # git stash --include-untracked -- ":!x" # fails

--- I'm not sure who else to cc, last commit touching this code is 2ec87741 from 10 year ago, being a mere refactoring. I think this bug was simply overlooked when introducing PATHSPEC_EXCLUDE.

Thanks to Tian Yuchen for looking at my earlier submission (and noticing
an awkwardly stupid bug there).
---
 dir.c                              |  3 +++
 t/t2204-add-ignored.sh             | 14 ++++++++++++++
 t/t3905-stash-include-untracked.sh | 23 +++++++++++++++++++++++
 3 files changed, 40 insertions(+)

"Remy D. Farley" <one-d-wide@protonmail.com> writes:

A question that immediately comes to mind is if it is appropriate for a negated pathspec element to recuse itself like this from the decision process and let other pathspec elements decide the fate of the path, or if a negated pathspec element should take a more active role of saying "no" (no, not by immediately returning 0, but this loop may have to become a two step process if we wanted to implement e.g., for the function to yield "yes", it has to match at least one positive pathspec element and zero negated one, or something like that).

What should

	git add "$x" ":!$y"

do when a path <matches, does not match> $X and <matches, does not match> $Y? We have four combinations to consider in such a case. The code in the patch says it should behave identically to

	git add "$x"

and negated ":!$y" should not make any difference. Is that what we want?

Thanks.

On 2/5/26 00:48, Junio C Hamano wrote:

You are right. To illustrate, if we run:

git add ignored_file ":!ignored_file"

Then following things might happen with the patch:
-> For the first item,
	- Does it match 'exclude'? No.
	- Does it match 'path'? Yes.
	- Return 1.
-> For the second item,
	- Is never reached
-> Git complain,
	'The following paths are ignored: ignored_file.'

In other word, it's not the expected silent no-op (returning 0).

As you suggested, The loop needs to verify that the path matches at least one positive item AND matches none of the negative items. A possible way to acheive it is: (Notice that we no longer return 1 in the half way)

 >bool matched_positive = false;
 >
 >for (item in pathspec) {
 >	if (item matches patch) {
 >		if (item is exclude) {
 >			return 0;
 >		} else {
 >			matched_positive = true;
 >		}
 >	}
 >}
 >
 >return matched_positive ? 1 : 0;

By the way, I think extreme cases like 'git add x :!x' should be added into the test scripts.

Regards,

Yuchen

Tian Yuchen <a3205153416@gmail.com> writes:

One caveat. The case without any positive pathspec entries needs special consideration. I suspect, but can be totally wrong as I didn't think things through thoroughly, that

    git add "!$y"

would want to behave as if an implicit "everything matches" was given, i.e.,

    git add "!$y" .

while a pathspec with one or more positive entries would not need and want such an implicit "everything" treatment.

True.

Junio C Hamano <gitster@pobox.com> wrote:

I indeed failed to consider cases where pathspecs could interfere with each other, sorry.

It does seems like we don't want to just blanketly ignore negated pathspecs, at least for the sake of consistency:

    git add -n :!a a/ignored/c # is ok (even on mainline, nothing is added)

    git add -n :!a/b a/b # ok (same)

Probably the same should hold after this patch if "a" or "a/b" were excluded. I'll try to think this through.

Tian Yuchen <a3205153416@gmail.com> wrote:

I think this was already covered, though with a simplistic (wrong) approach.

Junio C Hamano <gitster@pobox.com> wrote:

This case is actually already handled by the pathspec itself.

From pathspec.c:

"Remy D. Farley" <one-d-wide@protonmail.com> writes:

Yes, that is from Linus 9 years ago plus a bit of my work, in 859b7f1d (pathspec: don't error out on all-exclusionary pathspec patterns, 2017-02-07) and b02fdbc8 (pathspec: correct an empty string used as a pathspec element, 2022-05-29). No wonder it sounded familiar ;-).

Thanks.