From: phillip.wood123@gmail.com
Date: Fri, 12 Jan 2024 11:01:21 GMT
Subject: Re: [RFC PATCH] `log --merge` also for rebase/cherry pick/revert
Message-ID: <47a4418b-68bf-4850-ba8b-1a5264f923e4@gmail.com>
In-Reply-To: <20240111233311.64734-1-mi.al.lohmann@gmail.com>

Hi Michael

On 11/01/2024 23:33, Michael Lohmann wrote:
> 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@gmail.com>
> ---
> Hi everybody!
> 
> When Phillip Wood gave me some nice hints on his workflow concerning
> conflicts [1] (we discussed if `--show-current-patch` would make sense
> for cherry pick/revert). When I learned about `git log --merge` I was a
> bit sad to discover that those do not exist for rebase/revert/cherry
> pick since they look really valuable for getting an understanding on
> what was changed. It is basically the counterpart to
> `git show ${ACTION}_HEAD` for understanding the source of the conflict.
> 
> I am curious if also other people would be interested in having an easy
> way to get a log of only the relevant commits touching conflicting files
> for said actions.
> 
> With this patch I think the functionality is there, just hijacking the
> `--merge` code - maybe an alias like `git log --conflict` or similar
> might be more descriptive now?
> 
> What do you think about this idea? (Or am I maybe missing an easy way to
> achieve it with the existing code as well?)

I should start by saying that I didn't know "git log --merge" existed 
before I saw this message so please correct me if I've misunderstood 
what this patch is doing. If I understand correctly it shows the commits 
from each side of the merge and is equivalent to

     git log HEAD MERGE_HEAD ^$(git merge-base HEAD MERGE_HEAD)

When a commit is cherry-picked the merge base used is CHERRY_PICK_HEAD^ 
[*] so I'm not sure what

     git log HEAD CHERRY_PICK_HEAD ^$(git merge-base HEAD CHERRY_PICK_HEAD)

tells us. Indeed there HEAD and CHERRY_PICK_HEAD may not share a common 
ancestor. As I say I've not used "git log --merge" so maybe I'm missing 
the reason why it would be useful when cherry-picking or rebasing.

Best Wishes

Phillip

[*] assuming we're not picking a merge commit

> Michael
> 
> 
> [1] https://lore.kernel.org/git/cfba7098-3c23-4a81-933c-b7fefdedec8a@gmail.com/
> 
>   revision.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
> 
> 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);