Re: [PATCH 1/7] merge-ort: mark a few more conflict messages as omittable

2 messages, 2 authors, 2021-09-01 · open the first message on its own page

Re: [PATCH 1/7] merge-ort: mark a few more conflict messages as omittable

From: Junio C Hamano <hidden>
Date: 2021-08-31 21:06:46

"Elijah Newren via GitGitGadget" [off-list ref] writes:
From: Elijah Newren <redacted>

path_msg() has the ability to mark messages as omittable when recording
conflict messages in an in-tree file.  This conflict message file will
then be shown by diffing the merge-created tree to the actual merge
commit that is created.  While all the messages touched in this commit
are very useful when trying to create a merge initially, early use with
the --remerge-diff feature (the only user of this omittable conflict
message capability), suggests that the particular messages marked in
this commit are just noise when trying to see what changes users made to
create a merge commit.  Mark them as omittable.
Sorry for asking something that may be obvious, but if you recreate
an automated and potentially conflicting merge in-core, in order to
then compare the recorded merge result, is there *any* message that
is worth showing while doing the first step, and where in the output
do users see them?

Re: [PATCH 1/7] merge-ort: mark a few more conflict messages as omittable

From: Elijah Newren <hidden>
Date: 2021-09-01 00:04:38

On Tue, Aug 31, 2021 at 2:06 PM Junio C Hamano [off-list ref] wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted
From: Elijah Newren <redacted>

path_msg() has the ability to mark messages as omittable when recording
conflict messages in an in-tree file.  This conflict message file will
then be shown by diffing the merge-created tree to the actual merge
commit that is created.  While all the messages touched in this commit
are very useful when trying to create a merge initially, early use with
the --remerge-diff feature (the only user of this omittable conflict
message capability), suggests that the particular messages marked in
this commit are just noise when trying to see what changes users made to
create a merge commit.  Mark them as omittable.
Sorry for asking something that may be obvious
No, it's not obvious.  I had a hard time figuring out the right way to
split up this series and order the patches in part because so many
little pieces are needed before I can show the solution.  So some of
this comes from the later patches, but let me see if I can motivate
the issue and solution I picked a bit more right here.
but if you recreate
an automated and potentially conflicting merge in-core, in order to
then compare the recorded merge result, is there *any* message that
is worth showing while doing the first step,
Yes, absolutely.  While three-way *content* conflicts are easily
representable within a file in the working tree using conflict
markers, *non-content* conflicts are usually not easily representable
in such a fashion.  For example, a rename/delete conflict will not
result in any conflict markers, and will result in the renamed version
of the file being left in the working tree; the only way you know
there is a conflict for that file is either because of the stage in
the index or the the message that is printed to the terminal:

    CONFLICT (rename/delete): %s renamed to %s in %s, but deleted in %s.

But relying on higher order stages in the index and messages printed
to the terminal present a bit of a problem for --remerge-diff; as
described, it ignores both those sources of input.  Here's a few other
conflict types that face similar issues:

  * modify/delete conflicts
  * failure to merge submodule
  * directory rename detection (i.e. new file added to old directory
that other side renamed; which directory should file end up in?)
  * distinct types of files (e.g. regular file and symlink) located at
the same path
  * rename/rename conflicts

In all these cases (and some others), relying on a diff of "what the
working directory looks like at the end of a merge" to "the tree
recorded in the merge commit" does not convey enough (if any)
information about the above types of conflicts.
and where in the output do users see them?
For --remerge-diff, I chose to handle the fact that the working
directory didn't naturally have enough information, by augmenting the
working directory with additional information.  So, for example, if
there was a file named `dir/my.file` that had a modify/delete conflict
(and the user who did the real merge edited that file a bit as part of
conflict resolution), then I would also add a `dir/my.fil
e.conflict_msg` file whose contents are

"""
== Conflict notices for my.file ==
CONFLICT (modify/delete): dir/my.file deleted in HASH1 (SHORT
SUMMARY1) and modified in HASH2 (SHORT SUMMARY 2).  Version HASH2
(SHORT SUMMARY2) of  dir/my.file left in tree.
"""

Creating this file means you see something like this in the
remerge-diff (note there are diffs for two files, with the synthetic
file appearing just before the file it has messages about):
diff --git a/dir/my.fil e.conflict_msg b/dir/my.fil e.conflict_msg
deleted file mode 100644
index 2bd215a32f06..000000000000
--- a/dir/my.fil e.conflict_msg
+++ /dev/null
@@ -1,2 +0,0 @@
-== Conflict notices for my.file ==
-CONFLICT (modify/delete): dir/my.file deleted in HASH1 (SHORT
SUMMARY1) and modified in HASH2 (SHORT SUMMARY 2).  Version HASH2
(SHORT SUMMARY2) of  dir/my.file left in tree.
diff --git a/dir/my.file b/dir/my.file
index 09c78c725a3b..79f9d1fb7611 100644
--- a/dir/my.file
+++ b/dir/my.file
@@ -950,15 +912,15 @@ int my_func(struct stuff *data,
                                        rq->timeline,
                                        rq);

-               old_func(data, 1);
+               new_func("for funsies", data, VALID);
                obj->value = 8;
                obj->read_attempts = 0;
        } else {
-               err = old_func(data, 0);
+               err = new_func("for funsies", data, INVALID);
                if (unlikely(err))
                        return err;

-               another_old_func(data, obj->value);
+               another_new_func(data, obj->value, INVALID);
                obj->value++;
        }
        obj->read_attempts += 1;

The `dir/my.fil e.conflict_msg` file is definitely slightly weird, but
any solution here would be.  I think it's fairly intuitive what is
meant.  No one has commented on this choice in the 9+ months it's been
in use internally by ~50 users (even with -p implying --remerge-diff
automatically to make it more likely people have used this option), so
either it really is intuitive, or it doesn't come up much.  It could
well be the latter.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help