Hi Philippe
On 10/02/2024 23:35, Philippe Blain wrote:
From: Michael Lohmann <mi.al.lohmann@xxxxxxxxx>
'git log' learned in ae3e5e1ef2 (git log -p --merge [[--] paths...],
2006-07-03) to show commits touching conflicted files in the range
HEAD...MERGE_HEAD, an addition documented in d249b45547 (Document
rev-list's option --merge, 2006-08-04).
It can be useful to look at the commit history to understand what lead
to merge conflicts also for other mergy operations besides merges, like
cherry-pick, revert and rebase.
For rebases, an interesting range to look at is HEAD...REBASE_HEAD,
since the conflicts are usually caused by how the code changed
differently on HEAD since REBASE_HEAD forked from it.
For cherry-picks and revert, it is less clear that
HEAD...CHERRY_PICK_HEAD and HEAD...REVERT_HEAD are indeed interesting
ranges, since these commands are about applying or unapplying a single
(or a few, for cherry-pick) commit(s) on top of HEAD. However, conflicts
encountered during these operations can indeed be caused by changes
introduced in preceding commits on both sides of the history.
I tend to think that there isn't much difference between rebase and
cherry-pick here - they are both cherry-picking commits and it is
perfectly possible to rebase a branch onto an unrelated upstream. The
important part for me is that we're showing these commits because even
though they aren't part of the 3-way merge they are relevant for
investigating where any merge conflicts come from.
For revert I'd argue that the only sane use is reverting an ancestor of
HEAD but maybe I'm missing something. In that case REVERT_HEAD...HEAD is
the same as REVERT_HEAD..HEAD so it shows the changes since the commit
that is being reverted which will be the ones causing the conflict.
Adjust the code in prepare_show_merge so it constructs the range
HEAD...$OTHER for each of OTHER={MERGE_HEAD, CHERRY_PICK_HEAD,
REVERT_HEAD or REBASE_HEAD}. Note that we try these pseudorefs in order,
so keep REBASE_HEAD last since the three other operations can be
performed during a rebase. Note also that in the uncommon case where
$OTHER and HEAD do not share a common ancestor, this will show the
complete histories of both sides since their root commits, which is the
same behaviour as currently happens in that case for HEAD and
MERGE_HEAD.
Adjust the documentation of this option accordingly.
Thanks for the comprehensive commit message.
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 2bf239ff03..5b4672c346 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -341,8 +341,10 @@ See also linkgit:git-reflog[1].
Under `--pretty=reference`, this information will not be shown at all.
--merge::
- After a failed merge, show refs that touch files having a
- conflict and don't exist on all heads to merge.
+ Show commits touching conflicted paths in the range `HEAD...$OTHER`,
+ where `$OTHER` is the first existing pseudoref in `MERGE_HEAD`,
+ `CHERRY_PICK_HEAD`, `REVERT_HEAD` or `REBASE_HEAD`. Only works
+ when the index has unmerged entries.
Do you know what "and don't exist on all heads to merge" in the original
is referring to? The new text doesn't mention anything that sounds like
that but I don't understand what the original was trying to say.
It might be worth adding a sentence explaining when this option is useful.
This option can be used to show the commits that are relevant
when resolving conflicts from a 3-way merge
or something like that.
--boundary::
Output excluded boundary commits. Boundary commits are
diff --git a/revision.c b/revision.c
index aa4c4dc778..36dc2f94f7 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", "CHERRY_PICK_HEAD", "REVERT_HEAD", "REBASE_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]);
This would benefit from being translated and I think one '?' would
suffice (I'm not sure we even need that - are there other possible
causes of a null oid here?)
+ return other_head[i];
+ }
+
+ die("--merge without MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD or REBASE_HEAD?");
This is not a question and would also benefit from translation. It might
be more helpful to say that "--merge" requires one of those pseudorefs.
Thanks for pick this series up and polishing it
Phillip
