Am 08.07.23 um 01:45 schrieb Junio C Hamano: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> ... >> * do_match_pathspec() calls match_pathspec_item() _after_ stripping >> the common prefix "sub/" from the path, giving it "file", plus >> the length of the common prefix (4-bytes), so that the pathspec >> element "(attr:label)sub/" can be treated as if it were "(attr:label)". >> >> The last one is what breaks the match, as the pathspec subsystem >> ends up asking the attribute subsystem to find the attribute >> attached to the path "file". >> ... >> Update do_match_pathspec() so that it does not strip the prefix from >> the path, and always feeding the full pathname to match_pathspec_item(). > > Here is an alternative approach with a lot smaller code footprint. > Instead of teaching do_match_pathspec() not to strip the common > prefix from the pathname, we teach match_pathspec_item() how to > recover the original pathname before stripping, and use that when > calling match_pathspec_attrs() function. The same trick is already > used in an earlier part of the same function, so even though it > looks somewhat dirty, it is unlikely that it would introduce > more breakage. > > As the test part is the same, I'll just show the code change > relative to the 'master' branch. > > I am undecided which one is better. > > dir.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git c/dir.c w/dir.c > index a7469df3ac..635d1b058c 100644 > --- c/dir.c > +++ w/dir.c > @@ -374,7 +374,7 @@ static int match_pathspec_item(struct index_state *istate, > return 0; > > if (item->attr_match_nr && > - !match_pathspec_attrs(istate, name, namelen, item)) > + !match_pathspec_attrs(istate, name - prefix, namelen + prefix, item)) match_pathspec_item() has only one caller, and it did the opposite, so this is safe. And a minimal fix like that is less likely to have side effects. Removing the trick will surely improve the code, though. If match_pathspec_item() needs the full name then we should pass it on, and if the "prefix" offset needs to be added then it can happen right there in that function. > return 0; > > /* If the match was just the prefix, we matched */
