Subject: Re: [PATCH 3/5] describe: catch unborn branch in describe_blob()

From: Jeff King <[email-masked]>
Date: Mon, 18 Aug 2025 19:07:17 -0400

On Mon, Aug 18, 2025 at 02:19:42PM -0700, Junio C Hamano wrote:

> Jeff King <[email-masked]> writes:
> 
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -518,10 +518,16 @@ static void describe_blob(const struct object_id *oid, struct strbuf *dst)
> >  {
> >  	struct rev_info revs;
> >  	struct strvec args = STRVEC_INIT;
> > +	struct object_id head_oid;
> >  	struct process_commit_data pcd = { *null_oid(the_hash_algo), oid, dst, &revs};
> 
> An unrelated tangent, but it seems that we are copying the object
> name for the first member of this struct, even though 1/5 changed
> the second one.

Yep, agreed. It goes away in the final patch, though.

> > +	if (repo_get_oid(the_repository, "HEAD", &head_oid))
> > +		die(_("cannot search for blob '%s' on an unborn branch"),
> > +		    oid_to_hex(oid));
> 
> Makes sense.  I briefly wondered if there is really a point in doing
> the traversal only from HEAD, but this topic is not about enhancing
> and making the "describe <blob>" more useful, but it is a strict
> improvement.

Yeah. We do document the traversal from HEAD, so I wondered if anybody
would be upset at being more inclusive. So it might need a new option.
Somebody who is more interested in the option is welcome to pick up that
topic. :)

> When you first mentioned "resolve HEAD ourselves", I somehow expected
> you to ask the ref subsystem to resolve HEAD, but this should do fine,
> thanks to ref_rev_parse_rules[].

I hadn't really thought about the distinction. If you grep for '"HEAD"',
looks like we have a mix of both types. I don't think it should matter
much in practice.

Some even call lookup_commit_reference_by_name(). So we could perhaps do
that, in which case the segfault fix in patch 4 just happens naturally,
because we know we are always traversing from at least one commit. I
don't see any difference between the approaches compelling enough to
switch, though.

-Peff