On Wed, Jan 17, 2024 at 12:14 AM Michael Lohmann
<mi.al.lohmann@xxxxxxxxx> wrote:
>
> Co-authored-by: Johannes Sixt <j6t@xxxxxxxx>
> Signed-off-by: Michael Lohmann <mi.al.lohmann@xxxxxxxxx>
> ---
>
> On 12. Jan 2024, at 21:18, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > I like the way your other_merge_candidate() loops over an array of
> > possible candidates, which makes it a lot easier to extend, though,
> > admittedly the "die()" message needs auto-generating if we really
> > wanted to make it maintenance free ;-)
>
> I would certainly like that but I did not find an easy way of achieving
> this in C. Help wanted.
>
> Changes with respect to v2:
> - use read_ref_full instead of refs_resolve_ref_unsafe
> - check for symbolic ref
> - extract "other_name" in this patch, instead of patch 1
>
> revision.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index aa4c4dc778..c778413c7d 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1961,11 +1961,31 @@ static void add_pending_commit_list(struct rev_info *revs,
> }
> }
>
> +static const char *lookup_other_head(struct object_id *oid)
> +{
> + int i;
> + static const char *const other_head[] = {
> + "MERGE_HEAD", "REBASE_HEAD", "CHERRY_PICK_HEAD", "REVERT_HEAD"
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(other_head); i++)
> + if (!read_ref_full(other_head[i],
> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> + oid, NULL)) {
> + if (is_null_oid(oid))
> + die("%s is a symbolic ref???", other_head[i]);
> + return other_head[i];
> + }
> +
> + die("--merge without MERGE_HEAD, REBASE_HEAD, CHERRY_PICK_HEAD or REVERT_HEAD?");
> +}
> +
> static void prepare_show_merge(struct rev_info *revs)
> {
> struct commit_list *bases;
> struct commit *head, *other;
> struct object_id oid;
> + const char *other_name;
> const char **prune = NULL;
> int i, prune_num = 1; /* counting terminating NULL */
> struct index_state *istate = revs->repo->index;
> @@ -1973,15 +1993,10 @@ static void prepare_show_merge(struct rev_info *revs)
> if (repo_get_oid(the_repository, "HEAD", &oid))
> die("--merge without HEAD?");
> head = lookup_commit_or_die(&oid, "HEAD");
> - if (read_ref_full("MERGE_HEAD",
> - RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> - &oid, NULL))
> - die("--merge without MERGE_HEAD?");
> - if (is_null_oid(&oid))
> - die("MERGE_HEAD is a symbolic ref???");
> - other = lookup_commit_or_die(&oid, "MERGE_HEAD");
> + other_name = lookup_other_head(&oid);
> + other = lookup_commit_or_die(&oid, other_name);
> add_pending_object(revs, &head->object, "HEAD");
> - add_pending_object(revs, &other->object, "MERGE_HEAD");
> + add_pending_object(revs, &other->object, other_name);
> bases = repo_get_merge_bases(the_repository, head, other);
> add_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);
> add_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);
> --
> 2.39.3 (Apple Git-145)
I had to go look up previous versions to see the discussion of why
this was useful for things other than merge. I agree with Phillip
from https://lore.kernel.org/git/648774b5-5208-42d3-95c7-e0cba4d6a159@xxxxxxxxx/,
that the commit message _needs_ to explain this, likely using some of
Junio's explanation.
Also, what about cases where users do a cherry-pick in the middle of a
rebase, so that both REBASE_HEAD and CHERRY_PICK_HEAD exist? What
then?
