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

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 22, 2024, 09:02 UTC
Message-ID: <ZdcNtxw04MtybTWZ@tanuki>
In-Reply-To: <xmqqv86hogpi.fsf@gitster.g>

On Wed, Feb 21, 2024 at 09:44:09AM -0800, Junio C Hamano wrote:

Show 21 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > with an empty reflog file (added by that same commit). The code in
> > get_oid_basic() to parse reflogs doesn't suffer from the same bugs: it
> > checks up front that the reflog file exists, it preloads the output oid
> > with the current ref value, and it doesn't look at other fields (like
> > the reflog message).
> 
> It is a usability hack to allow foo@{0} to resolve successfully,
> instead of causing a failure, when there is no reflog entries for
> foo, I would think.  As to the "show-branch -g", the intent is to
> see the recent evolution of the branch, so I am fine if we do not
> give any output when no reflog entries exist (i.e. "no evolution
> behind the current state---it just is"), or just one entry output
> for "foo@{0}" to say "there is only one recent state".
> 
> Even though it may feel wrong to successfully resolve foo@{0} when
> reflog for foo does not exist at the mechanical level (read: the
> implementors of reflog mechanism may find the usability hack a bad
> idea), I suspect at the end-user level it may be closer to what
> people expect out of foo@{0} (i.e. "give me the latest").

Hum, I dunno. I don't really understand what the benefit of this fallback is. If a user wants to know the latest object ID of the ref they shouldn't ask for `foo@{0}`, they should ask for `foo`. On the other hand, if I want to know "What is the latest entry in the ref's log", I want to ask for `foo@{0}`.

For one, I think that this is adding complexity to the user interface. If you were to explain this feature to a user who has never encountered it, you need to now also explain special cases: "It gives you the latest reflog entry, except when the reflog doesn't exist or is missing, then we return the object ID of the corresponding ref." This is a lot more mental overhead than "It gives you the latest reflog entry."

We also have to consider a potential future where we stop deleting reflogs together with their ref in the "reftable" backend. What do we return when the reflog is empty and the ref is missing? "It gives you the latest reflog entry, except when the reflog doesn't exist or is missing, then we give you the ref except if that is missing, too". It's getting even harder to explain now.

Another angle to me is scripting. If I really want to get the latest reflog entry, then I now have to execute two commands. First I have to check whether the reflog is non-empty, and only then can I ask for the latest reflog entry. Otherwise, I might not get a reflog entry but the resolved ref instead. And I wouldn't even know how to check whether the reflog is non-empty.

So overall, I think the interface is a lot easier to understand and use correctly if we didn't have this fallback.

Patrick