Re: bug: git-add --patch any negative pathspec fails
On Thu, Dec 16, 2021 at 08:09:24PM +0100, Erik Cervin Edin wrote:
Show 9 quoted lines
> Steps to reproduce:
> git-add -p -- ':!foo'
>
> Expected:
> Add everything that doesn't match the pathspec.
>
> Actual:
> fatal: empty string is not a valid pathspec. please use . instead if
> you meant to match all paths
Hmm. This is a regression from 728c573803 (Merge branch 'ex/deprecate-empty-pathspec-as-match-all', 2017-11-06). But I think the fault is really in the earlier 859b7f1d0e (pathspec: don't error out on all-exclusionary pathspec patterns, 2017-02-07).
It works by adding an extra "match everything" pathspec internally, but it just passes the empty string as the "original" string to init_pathspec_item(). And "git add -p" works by iterating over the parsed pathspec list and adding the result to the helper script's argv.
So either:
- it should instead make sure to pass the true original set of
pathspecs we got (so just the exclusion, not the "match everything"
one). This would work because those pathspecs are then interpreted
by further Git programs anyway (so they'd have their own "match
everything" logic internally). - we should pass something better for the "match everything" pathspec.
The patch below does that, though I didn't think too hard on whether
the choice of "something better" would always be correct (e.g., if
we change directory internally, do we need to adjust "." to match
original cwd)? It does fix your case, and it passes the test suite, but I have a
feeling this area is not very well tested (I doubt we even look at
the "original" string all that often).diff --git a/pathspec.c b/pathspec.c
index ddeeba7911..5a0e083229 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -628,8 +628,16 @@ 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, "");
+ int plen;
+ const char *orig;
+ if (flags & PATHSPEC_PREFER_CWD) {
+ plen = prefixlen;
+ orig = ".";
+ } else {
+ plen = 0;
+ orig = ":/";
+ }
+ init_pathspec_item(item + n, 0, prefix, plen, orig);
pathspec->nr++;
}
