Michael Lohmann <mi.al.lohmann@xxxxxxxxx> writes:
> This extends the functionality of `git log --merge` to also work with
> conflicts for rebase, cherry pick and revert.
>
> Signed-off-by: Michael Lohmann <mi.al.lohmann@xxxxxxxxx>
> ---
> ... It is basically the counterpart to
> `git show ${ACTION}_HEAD` for understanding the source of the conflict.
I do not know about the validity of that approach to use *_HEAD, but
we may want to tighten the original's use of repo_get_oid() here ...
> - 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
(1) ensure MERGE_HEAD is a ref,
(2) obtain the oid without any prefixing by refs.c:repo_dwim_ref(),
and optionally
(3) error out when MERGE_HEAD is a symref.
As your patch makes the problem even worse, if we were to do such a
tightening (and I do not see a reason not to), it may want to be
done before, not after, this patch.
I won't comment on the coding style violations in the patch below in
this message.
Thanks.
> diff --git a/revision.c b/revision.c
> index 2424c9bd67..2e5c00dabd 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1961,23 +1961,37 @@ static void add_pending_commit_list(struct rev_info *revs,
> }
> }
>
> +static char* get_action_head_name(struct object_id* oid)
> +{
> + if (!repo_get_oid(the_repository, "MERGE_HEAD", oid)) {
> + return "MERGE_HEAD";
> + } else if (!repo_get_oid(the_repository, "REBASE_HEAD", oid)) {
> + return "REBASE_HEAD";
> + } else if (!repo_get_oid(the_repository, "CHERRY_PICK_HEAD", oid)) {
> + return "CHERRY_PICK_HEAD";
> + } else if (!repo_get_oid(the_repository, "REVERT_HEAD", oid)) {
> + return "REVERT_HEAD";
> + } else
> + 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 **prune = NULL;
> + const char *action_head_name;
> 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))
> - die("--merge without MERGE_HEAD?");
> - other = lookup_commit_or_die(&oid, "MERGE_HEAD");
> + action_head_name = get_action_head_name(&oid);
> + other = lookup_commit_or_die(&oid, action_head_name);
> add_pending_object(revs, &head->object, "HEAD");
> - add_pending_object(revs, &other->object, "MERGE_HEAD");
> + add_pending_object(revs, &other->object, action_head_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);
