Re: Segfault: git show-branch --reflog refs/pullreqs/1

[Jeff King <peff@peff.net>]

On Wed, Feb 21, 2024 at 11:05:56AM +0100, Patrick Steinhardt wrote:

> > I am still trying to wrap my head around how it can get such wrong
> > results for show-branch when asking for "git rev-parse branch@{0}", etc,
> > are correct. I think it is that "rev-parse branch@{0}" is only looking
> > at the output oid and does not consider the reflog message at all. So I
> > think it is subtly broken, but in a way that happens to work for that
> > caller. But I'm not sure of the correct fix. At least not at this time
> > of night.
> > 
> > Cc-ing folks involved in 6436a20284.
> 
> Ah, our mails crossed, but we came to the same conclusion. Things indeed
> are subtly broken here and work just by chance because all callers pre
> initialize the object ID. So in the case where the reflog is missing or
> empty we'd use that pre-initialized object ID because `read_ref_at()`
> does not indicate the failure to the callers.
> 
> I think that this behaviour is not sensible in the first place. When
> asking for the reflog, we should only ever return object IDs parsed from
> the reflog. Falling back to parsing the ref itself does not make much
> sense. I've thus sent a patch series that changes the behaviour here.

I left some comments on your patches. But assuming we are OK to change
the branch@{0} behavior for the empty log, the approach is sound.

That still leaves us with the bug in showing the message (which is
easily fixed), and the weird off-by-one output caused by 6436a20284.
Obviously the segfault fix can be taken independently of the rest, but I
wonder if some deeper refactoring of what 6436a20284 did will be
necessary.

-Peff