Re: Excluding paths with wildcard not working with add -p

From: Junio C Hamano <gitster@pobox.com>
Date: May 29, 2022, 18:42 UTC
Message-ID: <xmqq8rqkyyc2.fsf@gitster.g>
In-Reply-To: <xmqqh758yz4u.fsf@gitster.g>

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

Show 5 quoted lines

> Does this command
>
>     git add -p -- . ':^*.cs'
>
> work as you expect?

We used to error out when you give a pathspec with only negative elements in it, like the one you gave above. Later, we tweaked this logic at 859b7f1d (pathspec: don't error out on all-exclusionary pathspec patterns, 2017-02-07) so that we add an empty string as an extra element when your pathspec has only negative elements.

At around the same time, we were migrating from "an empty string is a valid pathspec element that matches everything" to "either a dot or ":/" is used for that purpose, and an empty string is rejected", between d426430e (pathspec: warn on empty strings as pathspec, 2016-06-22) and 9e4e8a64 (pathspec: die on empty strings as pathspec, 2017-06-06). I think 9e4e8a64 was not careful enough to turn the empty string 859b7f1d added to either a dot or ":/"

For the purpose of "add -p", I _think_ adding a "dot" is correct, but depending on the command, the code needs to add ":/".

Here is a quick trial patch, which seems to compile and pass all the tests we have, but the fact that this lingered with us for the past 5 years is a strong sign that we lack coverage in this area, so it may be breaking something else in a big way.

 pathspec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git c/pathspec.c w/pathspec.c
index ddeeba7911..1b0ae51aa4 100644
--- c/pathspec.c
+++ w/pathspec.c
@@ -628,8 +628,10 @@ void parse_pathspec(struct pathspec *pathspec,
 	 * that matches everything. We allocated an extra one for this.
 	 */
 	if (nr_exclude == n) {
-		int plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;
-		init_pathspec_item(item + n, 0, prefix, plen, "");
+		if (!(flags & PATHSPEC_PREFER_CWD))
+			init_pathspec_item(item + n, 0, NULL, 0, ":/");
+		else
+			init_pathspec_item(item + n, 0, prefix, prefixlen, ".");
 		pathspec->nr++;
 	}