From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jim Pryor <dubiousjim@gmail.com>, git@vger.kernel.org
Subject: Re: git 2.40.1 tree-diff crashes with (glob) magic on pathspecs
Date: Thu, 1 Jun 2023 09:30:57 -0400 [thread overview]
Message-ID: <20230601133057.GA2458601@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqsfbbd9oi.fsf@gitster.g>
On Thu, Jun 01, 2023 at 02:38:05PM +0900, Junio C Hamano wrote:
> > So follow-mode does not actually work with wildcards, but we err on the
> > side of not rejecting them outright. In that sense, the simplest "fix"
> > here would be to allow :(glob) to match the '#if 0' section, like:
>
> Is it "fix" or widening the wound, though? The runtime BUG is very
> unpleasant to see, but silently giving a possibly wrong result would
> be even worse, I am afraid.
I think it's the same wrong result we'd give in other circumstances,
which in some ways makes it orthogonal. I.e., this is already wrong:
git log --follow 'foo*'
Doing:
git --noglob-pathspecs --follow 'foo*'
makes it right (assuming you really have a literal '*' in your
pathname). And then doing:
git --noglob-pathspecs --follow ':(glob)foo*'
makes it wrong again, but in exactly the same way as the first case.
Which is why I said "orthogonal" above, because it is really just
twiddling options to cancel each other out and reach the same broken
state. ;)
All of that said, I do think the patch I showed earlier is pretty ugly,
and it is not even fixing all of the problems (you can trigger the same
BUG() for ":(icase)", etc). I have a series which moves us in a better
direction, but I need to put a few finishing touches on it. I'll
hopefully send it later today.
> If somebody cares about the "--follow" mode very deeply, the "upon
> finding that the path disappears, run the rename detection with the
> parent and switch the (single) path to follow to the old path in the
> parent" logic must be updated to keep track of these pathspecs per
> traversal paths. If false positives are allowed, an approximation
> that may be easier to implement is to add paths to the set of paths
> to be followed every time such a rename is found.
Yes, it would be nice to fix all of these --follow bugs once and for
all. But that's a pretty big task. I think in the meantime, it is not
too hard to make Jim's case behave more sensibly.
-Peff
prev parent reply other threads:[~2023-06-01 13:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-27 13:53 git 2.40.1 tree-diff crashes with (glob) magic on pathspecs Jim Pryor
2023-05-27 17:39 ` Jeff King
2023-05-27 19:00 ` Jim Pryor
2023-06-01 17:37 ` [PATCH 0/3] handling pathspec magic with --follow Jeff King
2023-06-01 17:38 ` [PATCH 1/3] pathspec: factor out magic-to-name function Jeff King
2023-06-01 17:41 ` [PATCH 2/3] diff: factor out --follow pathspec check Jeff King
2023-06-01 17:43 ` [PATCH 3/3] diff: detect pathspec magic not supported by --follow Jeff King
2023-06-02 7:27 ` Junio C Hamano
2023-06-15 7:26 ` Jeff King
2023-06-15 19:00 ` Junio C Hamano
2023-06-01 5:38 ` git 2.40.1 tree-diff crashes with (glob) magic on pathspecs Junio C Hamano
2023-06-01 13:30 ` Jeff King [this message]
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=20230601133057.GA2458601@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=dubiousjim@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).