Hi, Elijah
I was going to send v3 a couple days ago, but there is still one issue
that I'm having a bit of trouble with:
On Wed, Feb 24, 2021 at 3:50 AM Elijah Newren <newren@xxxxxxxxx> wrote:
>
> Hmm...here's an interesting command sequence:
>
> git init lame
> cd lame
> mkdir baz
> touch baz/tracked
> git add baz/tracked
> git update-index --skip-worktree baz/tracked
> rm baz/tracked. # But leave the empty directory!
> echo baz >.gitignore
> git add --ignore-missing --dry-run baz
>
>
> Reports the following:
> """
> The following pathspecs only matched index entries outside the current
> sparse checkout:
> baz
> hint: Disable or modify the sparsity rules if you intend to update such entries.
> hint: Disable this message with "git config advice.updateSparsePath false"
> The following paths are ignored by one of your .gitignore files:
> baz
> hint: Use -f if you really want to add them.
> hint: Turn this message off by running
> hint: "git config advice.addIgnoredFile false"
> """
>
> That's probably okay because it does match both, but the "only
> matched" in the first message followed by saying it matched something
> else seems a little surprising at first.
After giving this a little more thought, I got to this idea [1]. With
this patch, git-add doesn't warn about sparse paths that are inside
ignored dirs (it only warns about the ignored dirs themselves). If
--force is used, then git-add recurses into the ignored directories, and
warns about any sparse path inside them. I think this is more consistent
with the way in which ignored dirs (and their contents) are handled with
and without --force. And we avoid the double warning.
But there is a performance penalty. The relevant code is:
@@ -2029,19 +2033,23 @@ static int exclude_matches_pathspec(const char *path, int pathlen,
PATHSPEC_ICASE |
PATHSPEC_EXCLUDE);
+ assert(matched_ignored);
+
for (i = 0; i < pathspec->nr; i++) {
const struct pathspec_item *item = &pathspec->items[i];
int len = item->nowildcard_len;
- if (len == pathlen &&
- !ps_strncmp(item, item->match, path, pathlen))
- return 1;
- if (len > pathlen &&
- item->match[pathlen] == '/' &&
- !ps_strncmp(item, item->match, path, pathlen))
- return 1;
+ if (matched && matched_ignored[i])
+ continue;
+
+ if ((len == pathlen ||
+ (len > pathlen && item->match[pathlen] == '/')) &&
+ !ps_strncmp(item, item->match, path, pathlen)) {
+ matched = 1;
+ matched_ignored[i] = 1;
+ }
}
- return 0;
+ return matched;
}
This function is called for each ignored path (not recursing into
completely ignored dirs, though). Before the patch, the internal loop
stops at the first pathspec match. But with the patch, the loop must
continue as we need to know about all pathspecs that had matches with
the ignored paths...
And there is also one odd case:
$ mkdir d
$ touch d/s d/i
$ git add d/s
$ git update-index --skip-worktree d/s
$ echo d/i >.gitinore
$ git add 'd/[is]'
This outputs:
The following pathspecs only matched index entries outside the current
sparse checkout:
d/[is]
(and no warning on ignored paths).
I don't think the warning here is 100% correct, as 'd/[is]' *does* match
other files, only they are ignored. And git-add only warns about an
ignored path when the pathspec is an exact match with it (no glob
patterns) or inside it.
So, I don't know... Perhaps should we just rephrase the sparse warning
to remove the "only" part (and don't try to avoid the double warnings)?
I'm open to any suggestions on alternative wordings or ideas :)
[1]: https://github.com/matheustavares/git/commit/5d4be321e1f69cb08b0666ffd3f8fa658e9f6953
