Subject: Re: Segfault in master due to 4fbcca4eff

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 22 Sep 2018 01:37:17 +0200

On Fri, Sep 21 2018, Junio C Hamano wrote:

> Derrick Stolee <[email-masked]> writes:
>
>> On 9/21/2018 10:40 AM, Ævar Arnfjörð Bjarmason wrote:
>>> On Fri, Sep 21 2018, Derrick Stolee wrote:
>>>
>>>>
>>>> This error was reported by Peff [1] and fixed in [2], but as stated
>>>> [3] I was waiting for more review before sending a v3. I'll send that
>>>> v3 shortly, responding to the feedback so far.
>>>>
>>>> -Stolee
>>>>
>>>> [1]
>>>> https://public-inbox.org/git/[email-masked]/T/#u
>>>>
>>>> [2] https://public-inbox.org/git/[email-masked]/
>>>>
>>>> [3]
>>>> https://public-inbox.org/git/[email-masked]/
>>> Thanks and sorry for the duplicate report. I can confirm that applying
>>> the v2 of that fixes the segfault for the test case I posted.
>>
>> Thanks for the report! You are a good dogfooder.
>
> 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).