Michael Lohmann <mi.al.lohmann@xxxxxxxxx> writes:
>> but we may want to tighten the original's use of repo_get
>> > - if (repo_get_oid(the_repository, "MERGE_HEAD", &oid))
>> > - die("--merge without MERGE_HEAD?");
>> > - other = lookup_commit_or_die(&oid, "MERGE_HEAD");
>>
>> ... so that we won't be confused in a repository that has a tag
>> whose name happens to be MERGE_HEAD. We probably should be using
>> refs.c:refs_resolve_ref_unsafe() instead to _oid() here ...
>
> Here I am really at the limit of my understanding of the core functions.
> Is this roughly what you had in mind? From my testing it seems to do the
> job, but I don't understand the details "refs_resolve_ref_unsafe"...
Perhaps there are others who are more familiar with the ref API than
I am, but I was just looking at refs.h today and wonder if something
along the lines of this ...
if (read_ref_full("MERGE_HEAD",
RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
&oid, NULL))
die("no MERGE_HEAD");
if (is_null_oid(&oid))
die("MERGE_HEAD is a symbolic ref???");
... would be simpler.
With the above, we are discarding the refname read_ref_full()
obtains from its refs_resolve_ref_unsafe(), but I think that is
totally fine. We expect it to be "MERGE_HEAD" in the good case.
The returned value can be different from "MERGE_HEAD" if it is
a symbolic ref, but if the comment in refs.h on NO_RECURSE is to be
trusted, we should catch that case with the NULL-ness check on oid.
Which would mean that we do not need a separate "other_head"
variable, and instead can use "MERGE_HEAD".
> revision.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index 2424c9bd67..786e1a3e89 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1967,17 +1967,23 @@ static void prepare_show_merge(struct rev_info *revs)
> struct commit *head, *other;
> struct object_id oid;
> const char **prune = NULL;
> + const char *other_head;
> int i, prune_num = 1; /* counting terminating NULL */
> struct index_state *istate = revs->repo->index;
>
> if (repo_get_oid(the_repository, "HEAD", &oid))
> die("--merge without HEAD?");
> head = lookup_commit_or_die(&oid, "HEAD");
> - if (repo_get_oid(the_repository, "MERGE_HEAD", &oid))
> + other_head = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> + "MERGE_HEAD",
> + RESOLVE_REF_READING,
> + &oid,
> + NULL);
> + if (!other_head)
> die("--merge without MERGE_HEAD?");
> - other = lookup_commit_or_die(&oid, "MERGE_HEAD");
> + other = lookup_commit_or_die(&oid, other_head);
> add_pending_object(revs, &head->object, "HEAD");
> - add_pending_object(revs, &other->object, "MERGE_HEAD");
> + add_pending_object(revs, &other->object, other_head);
> 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);
