"Diogo Castro via GitGitGadget" <[email protected]> writes:

> From: Diogo Castro <[email protected]>
>
> `git ls-files` calls `common_prefix()` / `get_common_prefix_len()` which
> calculate the length of the common prefix of all *positive* pathspecs,
> `max_prefix_len`.
> ...
> Solution: in `do_match_pathspec()`, only strip the prefix when handling
> positive pathspecs, not when handling negative pathspecs.

Hmph, if the command line were

	git ls-files -- a/b/c a/b/d !a/b/

shouldn't we strip a/b/ from all three?  Would it make sense to
leave the negative one relative to the full tree?  I am wondering
if the solution is to compute common prefix across both positive and
negative ones instead.

I don't think so.

As far as I can tell, the "strip the common prefix" feature is a
performance optimization aimed at avoiding walking the working
directory needlessly.
So for `git add -- a/b/c a/b/d`, there's no need to look anywhere
other than in `a/b/`.

But extending the "strip the common prefix" to negative pathspecs
could end up negating the benefits we get from this perf optimization.
E.g. in `git add -- a/b/c a/b/d ':!*.md'`, there is no prefix common
to *all* pathspecs, so we'd revert to walking the entire working
directory, even though `a/b/` would still suffice.


On Sun, 30 Aug 2026 at 15:25, Junio C Hamano <[email protected]> wrote:
>
> "Diogo Castro via GitGitGadget" <[email protected]> writes:
>
> > From: Diogo Castro <[email protected]>
> >
> > `git ls-files` calls `common_prefix()` / `get_common_prefix_len()` which
> > calculate the length of the common prefix of all *positive* pathspecs,
> > `max_prefix_len`.
> > ...
> > Solution: in `do_match_pathspec()`, only strip the prefix when handling
> > positive pathspecs, not when handling negative pathspecs.
>
> Hmph, if the command line were
>
>         git ls-files -- a/b/c a/b/d !a/b/
>
> shouldn't we strip a/b/ from all three?  Would it make sense to
> leave the negative one relative to the full tree?  I am wondering
> if the solution is to compute common prefix across both positive and
> negative ones instead.
>

Diogo Castro <[email protected]> writes:

> I don't think so.
>
> As far as I can tell, the "strip the common prefix" feature is a
> performance optimization aimed at avoiding walking the working
> directory needlessly.
> So for `git add -- a/b/c a/b/d`, there's no need to look anywhere
> other than in `a/b/`.
>
> But extending the "strip the common prefix" to negative pathspecs
> could end up negating the benefits we get from this perf optimization.
> E.g. in `git add -- a/b/c a/b/d ':!*.md'`, there is no prefix common
> to *all* pathspecs, so we'd revert to walking the entire working
> directory, even though `a/b/` would still suffice.

I was wondering more about case like this:

    $ git add -- a/b/c a/b/d ':!a/b/x

I agree that it is nonsense to compute the common prefix over only
positive ones, and then to strip the common prefix from both
positive and negative ones, and it needs to be corrected.

I think there's some misunderstanding, please allow me to take a step
back and attempt to clarify.
My previous message was a reply to this:

> I am wondering if the solution is to compute common prefix across both positive and negative ones instead.

As far as I can tell, this "common prefix" feature does not affect the
semantics of "ls-files" or "add", it doesn't affect which files are
reported.
It only affects the performance.

Your first example of "git ls-files -- a/b/c a/b/d :!a/b/" already
works correctly, the pattern ":!a/b/" excludes everything from the
first 2 pathspecs.


So the discussion to be had is purely about performance.
My point was that computing the common prefix across both positive
*and* negative pathspecs would not improve performance, and might
actually make it worse.

The "common prefix" is mainly used to avoid walking the entire working
directory.
A couple of examples to illustrate:

* "git add -- a/b/c a/b/d ':!a/b/x'"
    * Under the current implementation, the common prefix is "a/b/",
so as a performance optimization, we can look only into the "a/b/"
directory and ignore the others.
    * Under your proposal of computing the "common prefix across both
positive and negative ones", the common prefix would still be "a/b/",
so performance wouldn't be affected.
* "git add -- a/b/c a/b/d ':!a/**/x'"
    * Under the current implementation, the common prefix is "a/b/",
like in the example above.
    * Under your proposal, the common prefix would be "a/", so we'd
have to walk _more_ directories, which would hurt performance.

Does that answer your question? Or perhaps I misunderstood your point?

Diogo Castro <[email protected]> writes:

> My point was that computing the common prefix across both positive
> *and* negative pathspecs would not improve performance, and might
> actually make it worse.

OK.  Then that points at the right solution.  Ignore negative ones
when finding what the common prefix is, strip it only from positive
ones to reduce the width of the traversal to come up with the list
of possible match candidates, and match them as full paths against
the negative ones to cull "within the positive set but is excluded"
paths, and the posted patch looks good.

I still wonder if we need different implementation when we have many
more negative patterns than the positive ones.  In such a case, the
stage to filter paths that matched one positive pattern by finding
matches with a negative pattern among many of them, which may
benefit from having a similar common prefix (among negative
patterns) optimization, but that is a separate topic.

Thanks.