git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH 2/2alt] dir: do not feed path suffix to pathspec match
Date: Fri, 07 Jul 2023 16:45:32 -0700	[thread overview]
Message-ID: <xmqqttuf70bn.fsf_-_@gitster.g> (raw)
In-Reply-To: <20230707220457.3655121-3-gitster@pobox.com> (Junio C. Hamano's message of "Fri, 7 Jul 2023 15:04:57 -0700")

Junio C Hamano <gitster@pobox.com> 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))
 		return 0;
 
 	/* If the match was just the prefix, we matched */

  reply	other threads:[~2023-07-07 23:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-07 22:04 [PATCH 0/2] Fix attr magic combined with pathspec prefix Junio C Hamano
2023-07-07 22:04 ` [PATCH 1/2] t6135: attr magic with path pattern Junio C Hamano
2023-07-07 22:04 ` [PATCH 2/2] dir: do not feed path suffix to pathspec match Junio C Hamano
2023-07-07 23:45   ` Junio C Hamano [this message]
2023-07-08  7:16     ` [PATCH 2/2alt] " René Scharfe
2023-07-08 21:35       ` [PATCH 2alt/2] dir: match "attr" pathspec magic with correct paths Junio C Hamano
2023-07-09  5:35         ` René Scharfe
2023-07-09  9:28           ` Junio C Hamano
2023-07-08  7:16   ` [PATCH 2/2] dir: do not feed path suffix to pathspec match René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqttuf70bn.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).