Hi Junio,
On Mon, Mar 11, 2024 at 11:54 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> It is not a good code hygiene to assume that a failure to read HEAD
>> always means we are on an unborn branch, even if that is the most
>> likely cause of the failure. We may instead want to positively
>> determine that we are on an unborn state, by seeing if the HEAD is a
>> symbolic ref that points at a ref in refs/heads/* hierarchy, and
>> that ref does not exist.
>
> I suspect that you are almost there.
>
> + if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
> + /*
> + * Treat an error reading HEAD as an unborn branch.
> + */
>
> After we see this error, if we make a call to resolve_ref_unsafe()
> with RESOLVE_REF_NO_RECURSE, the call should return the branch that
> we are on but is not yet born, and &oid will get the null_oid. I am
> not sure if there is a way to combine the two calls into one, but
> because the failure case (i.e. doing anything on an unborn branch)
> is a rare case that happens only once before actually giving birth
> to a new branch, it probably is not worth spending extra brain
> cycles on it and just use a simple and stupid "when resolving fails,
> see if we are in a rare salvageable case with extra code" approach.
If I'm understanding you correctly, it sounds like you're hinting at
something like this:
if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
/*
* Check to see if this is an unborn branch
*/
if (resolve_ref_unsafe("HEAD", RESOLVE_REF_NO_RECURSE, &head_oid, NULL)
&& is_null_oid(&head_oid)) {
head_tree_oid = the_hash_algo->empty_tree;
} else {
return error(_("could not resolve HEAD commit"));
}
} else {
...
}
This does in fact seem to have the same effect as the original patch in
this case. If this is in line with what you are looking for, I can
include this in v4. The commit message would be adjusted slightly to be:
sequencer: handle unborn branch with `--allow-empty`
When using git-cherry-pick(1) with `--allow-empty` while on an
unborn branch, an error is thrown. This is inconsistent with the
same cherry-pick when `--allow-empty` is not specified.
Detect unborn branches in `is_index_unchanged`. When on an unborn
branch, use the `empty_tree` as the tree to compare against.
...
Do these changes look good?
--
Thank you,
Brian Lyles
