Re: [PATCH v3 4/7] sequencer: treat error reading HEAD as unborn branch

[Junio C Hamano <gitster@pobox.com>]

Brian Lyles <brianmlyles@gmail.com> writes:

> 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.

When cherry-picking on top of an unborn branch without the option,
how does the code figure out that we are on an unborn branch?  We
must be doing the detection, as we'd need to at least know the fact
that we are on an unborn in order to make the resulting commit a
root commit and also in order to apply the cherry-picked changes
against an empty tree.

> Treat a failure reading HEAD as an unborn branch in
> `is_index_unchanged`. This is consistent with other sequencer logic such
> as `do_pick_commit`. When on an unborn branch, use the `empty_tree` as
> the tree to compare against.

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.

But if the existing sequencer code is littered with such loose logic
everywhere, perhaps imitating the looseness for now with a NEEDSWORK
comment to later clean them all up would be the least we could do
right now.  If we want to be more ambitious, a few clean-up patches
that refactors existing code paths that detect that we are on an
unborn branch into a call to a single helper function, and then
tightens its implementation, before doing this step would be even
better (but I offhand do not know how bad the current code is, so I
would understand it if it may turn out to be a lot more work than
you would want to invest in).