From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
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, 01 Jun 2023 14:38:05 +0900 [thread overview]
Message-ID: <xmqqsfbbd9oi.fsf@gitster.g> (raw)
In-Reply-To: <20230527173903.GB1460206@coredump.intra.peff.net> (Jeff King's message of "Sat, 27 May 2023 13:39:03 -0400")
Jeff King <peff@peff.net> writes:
>> 13a1b11 (HEAD -> master) third commit
>> BUG: tree-diff.c:596: unsupported magic 8
>> error: git died of signal 6
> ...
> Thanks for your report. I had trouble reproducing at first, but there is
> one extra twist. By default, the command you gave produces:
> But this also seems to be wading into a spot that is not actually
> supported by --follow. The code near where the BUG() is triggered looks
> like this:
>
> static void try_to_follow_renames([...])
> {
> [...]
> /*
> * follow-rename code is very specific, we need exactly one
> * path. Magic that matches more than one path is not
> * supported.
> */
> GUARD_PATHSPEC(&opt->pathspec, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);
> #if 0
> /*
> * We should reject wildcards as well. Unfortunately we
> * haven't got a reliable way to detect that 'foo\*bar' in
> * fact has no wildcards. nowildcard_len is merely a hint for
> * optimization. Let it slip for now until wildmatch is taught
> * about dry-run mode and returns wildcard info.
> */
> if (opt->pathspec.has_wildcard)
> BUG("wildcards are not supported");
> #endif
>
> 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.
> That would avoid the BUG() and make things work consistently. But it
> still would not produce the output you expect, because --follow simply
> does not work with wildcard pathspecs. And fixing that would presumably
> be a much bigger change (I didn't dig into it).
True. And wildcard is not the only thing it is broken for; the
biggest one is that it only follows one path at the time across all
the traversal paths, which has interesting implications like it can
give inconsistent result depending on the traversal order in a mergy
history.
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.
next prev parent reply other threads:[~2023-06-01 5:39 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 ` Junio C Hamano [this message]
2023-06-01 13:30 ` git 2.40.1 tree-diff crashes with (glob) magic on pathspecs Jeff King
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=xmqqsfbbd9oi.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=dubiousjim@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/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).