git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>, Elijah Newren <newren@gmail.com>
Subject: [PATCH] merge-ort: make informational messages from recursive merges clearer
Date: Thu, 17 Feb 2022 06:38:42 +0000	[thread overview]
Message-ID: <pull.1121.git.1645079923090.gitgitgadget@gmail.com> (raw)

From: Elijah Newren <newren@gmail.com>

This is another simple change with a long explanation...

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.

merge-recursive immediately prints to stdout as it goes, at the risk of
printing multiple conflict notices for the same path separated far apart
from each other with many intervenining conflict notices for other paths
between them.  And this is true even if there are no 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 same case handled by
merge-ort would have output of 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 generally helpful, but does make a separate bug 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

The fact that messages for the same file are together is probably
helpful in general, but with the indentation missing for the inner
merge it unfortunately serves to confuse.  This probably would lead
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 no conflicts 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 from merge-ort 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 currently 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.

[1] https://lore.kernel.org/git/CAGyf7-He4in8JWUh9dpAwvoPkQz9hr8nCBpxOxhZEd8+jtqTpg@mail.gmail.com/

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    merge-ort: make informational messages from recursive merges clearer
    
    Sorry for any confusion caused by this bug (though I'm surprised no one
    else reported it).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1121%2Fnewren%2Fmerge-ort-clearer-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1121/newren/merge-ort-clearer-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1121

 merge-ort.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/merge-ort.c b/merge-ort.c
index d85b1cd99e9..55decb2587e 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -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);
 

base-commit: 45fe28c951c3e70666ee4ef8379772851a8e4d32
-- 
gitgitgadget

             reply	other threads:[~2022-02-17  6:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-17  6:38 Elijah Newren via GitGitGadget [this message]
2022-02-17 23:10 ` [PATCH] merge-ort: make informational messages from recursive merges clearer Junio C Hamano
2022-03-01  6:44   ` Junio C Hamano
2022-03-02  2:32     ` Elijah Newren
2022-03-02  4:21       ` Elijah Newren
2022-03-02  6:53       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1121.git.1645079923090.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).