Re: Segfault in master due to 4fbcca4eff

From: Jeff King <peff@peff.net>
Date: Sep 21, 2018, 23:45 UTC
Message-ID: <20180921234543.GA3412@sigill.intra.peff.net>
In-Reply-To: <87lg7ujuc2.fsf@evledraar.gmail.com>

On Sat, Sep 22, 2018 at 01:37:17AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 30 quoted lines

> > Thanks, both of you ;-).  I was aware of the issue and proposed fix
> > but forgot about it when merging things down to 'master'.  Sorry
> > about that.
> 
> Just a follow-up question, in your merge commit you just pushed to
> "next" you say:
> 
>     Recent update broke the reachability algorithm when tags pointing
>     at objects that are not commit were involved, which has been fixed.
> 
> And in Derrick's commit message it says:
> 
>     [...]but incorrectly assumed that all objects provided were commits[...]
> 
> I just wanted to double check (without having the time to dig myself at
> this point) whether this bug was understood & tested for, or whether the
> case I had was just /also/ fixed for unexpected reasons.
> 
> I.e. in my upthread test case I have two annotated tags pointing at
> commits, whereas the merge to "next" says "when tags pointing at objects
> that are not commit were involved", which I I assume means say annotated
> tags pointing at blobs..., but that's not what I had.
> 
> Wasn't this just a bug fix that had nothing to do with tags not pointing
> to commits, but just ones where we had the simple case of tags pointing
> to commits, but they just weren't peeled?
> 
> I'm hoping for a "Junio skimmed the fix and wrote a merge message that
> wasn't quite accurate" here, but maybe that's not the case and something
> might be missing (e.g. missing test code).

I think it's a combination of the merge message being slightly inaccurate, and you slightly misreading it. :)

I think by "tags pointing", Junio meant "tag refs". Which of course, often point at tag objects, but can also point at trees, etc.

But the problem is not limited to tag refs. I think it's a problem with any "want" that is a non-commit. So really any ref pointing to a non-commit is a problem. But of course tags are the likely way for that to happen, since refs/heads is generally limited to commits.

So in short, yeah, the bug was triggered by fetching any annotated tag.

-Peff