Skip to content

Commit

Permalink
merge-ort: make informational messages from recursive merges clearer
Browse files Browse the repository at this point in the history
merge-recursive and merge-ort are both based on the same recursive idea:
if there is more than one merge base, merge the merge bases (which may
require first merging the merge bases of the merges bases, etc.).  The
depth of the inner merge is recorded via a variable called "call_depth",
which we'll bring up again later.  Naturally, the inner merges
themselves can have conflicts and various messages generated about those
files.

There is a bug (or two) with the printing of messages from inner merges
in merge-ort, though it is easy to accidentally ascribe the bug to
separate behavior of merge-ort that is intended.  So allow me to first
pause and describe that other behavior.

merge-recursive immediately prints to stdout as it goes, at the risk of
separating different types of conflicts for the exact same path far
apart from each other in the output -- even without any inner merges
involved.  An example of this was given in [1] and apparently caused
some confusion:

    CONFLICT (rename/add): Rename A->B in HEAD. B added in otherbranch
    ...dozens of conflicts for OTHER paths...
    CONFLICT (content): Merge conflicts in B

In contrast, merge-ort collects messages and stores them by path so that
it can print them grouped by path.  Thus, the above output would appear
more in the form

    CONFLICT (rename/add): Rename A->B in HEAD. B added in otherbranch
    CONFLICT (content): Merge conflicts in B
    ...dozens of conflicts for OTHER paths...

This is helpful in isolation, but makes a different bug (or bugs?) in
merge-ort for recursive merges more problematic.  In particular, while
merge-recursive might report the following for a recursive merge:

      Auto-merging dir.c
      Auto-merging midx.c
      CONFLICT (content): Merge conflict in midx.c
    Auto-merging diff.c
    Auto-merging dir.c
    CONFLICT (content): Merge conflict in dir.c

merge-ort would instead report:

    Auto-merging diff.c
    Auto-merging dir.c
    Auto-merging dir.c
    CONFLICT (content): Merge conflict in dir.c
    Auto-merging midx.c
    CONFLICT (content): Merge conflict in midx.c

and yes, I sadly lost the helpful indenting that merge-recursive did on
the output messages.  The lack of indenting might cause users to wonder:

  * Why is Git reporting that "dir.c" is being merged twice?
  * If midx.c has conflicts, why do I not see any when I open up the
    file and why are none shown in the index?

Fix this output confusion by changing the output to clearly
differentiate the messages for outer merges from the ones for inner
merges, changing the above output to:

    Auto-merging diff.c
      From inner merge:  Auto-merging dir.c
    Auto-merging dir.c
    CONFLICT (content): Merge conflict in dir.c
      From inner merge:  Auto-merging midx.c
      From inner merge:  CONFLICT (content): Merge conflict in midx.c

(Note: the number of spaces after the 'From inner merge:' is
2*call_depth).

One other thing to note here, that I didn't notice until typing up this
commit message, is that merge-recursive does not print any messages from
the inner merges by default; the extra verbosity has to be requested.
merge-ort has no verbosity controls and always prints these.  We may
also want to change that, but for now, just make the output clearer with
these extra markings and indentation.

Signed-off-by: Elijah Newren <newren@gmail.com>
  • Loading branch information
newren committed Jan 22, 2022
1 parent ea5df61 commit 931b3b8
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions merge-ort.c
Expand Up @@ -651,6 +651,11 @@ static void path_msg(struct merge_options *opt,
dest = (opt->record_conflict_msgs_as_headers ? &tmp : sb);

va_start(ap, fmt);
if (opt->priv->call_depth) {
strbuf_addchars(dest, ' ', 2);
strbuf_addstr(dest, "From inner merge:");
strbuf_addchars(dest, ' ', opt->priv->call_depth * 2);
}
strbuf_vaddf(dest, fmt, ap);
va_end(ap);

Expand Down

0 comments on commit 931b3b8

Please sign in to comment.