Subject: Re: [PATCH v4 4/7] sequencer: handle unborn branch with `--allow-empty`

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 Mar 2024 09:22:35 -0700

Dirk Gouders <[email-masked]> writes:

> Brian Lyles <[email-masked]> writes:
>
>> +	if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
>> +		/*
>> +		 * Check to see if this is an unborn branch
>> +		 */
> In the above example, there is a short but outstanding comment that
> announces a check (and if I understood correctly by [1] it is a kind of
> trick that could deserve some more information) and it does _not_
> comment on the result.  Of course, I have an idea where the correct
> place for a comment /* This is an unborn branch -- handle it as if... */
> could be, but I'm not sure.

You mean "Check to see if this is an unborn branch, and if so, use
an empty tree to compare against, instead of the tree of the HEAD
that does not yet exist"?

I think that is possible, but the use of the_hash_algo->empty_tree
indicates that clearly enough.  But we need to stop somewhere and
what we see above may be a reasonable place to do so.

If anything, we may want to say why we want to continue as if we had
an empty tree (as opposed to fail and return with an error()), or
the tree to compare with is computed here for what purpose.  But the
name of the function may tell what this whole computation and
comparison is for, so it probably is not needed, either.