Jeff King <peff@xxxxxxxx> 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.
