Re: Segfault in master due to 4fbcca4eff

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 21, 2018, 23:37 UTC
Message-ID: <87lg7ujuc2.fsf@evledraar.gmail.com>
In-Reply-To: <xmqqefdmyhe9.fsf@gitster-ct.c.googlers.com>

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

Show 27 quoted lines

> Derrick Stolee <stolee@gmail.com> 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/0bf9103c-9377-506b-7ad7-e5273d8e94fc@gmail.com/T/#u
>>>>
>>>> [2] https://public-inbox.org/git/pull.39.git.gitgitgadget@gmail.com/
>>>>
>>>> [3]
>>>> https://public-inbox.org/git/8d6061de-1654-577c-40c6-211dbd03aa36@gmail.com/
>>> 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).