Patchwork [1,of,1] merge: confirm continuation when conflict of divergent operations is detected

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Nov. 12, 2013, 5:38 a.m.
Message ID <a1fd26f1794b1158021b.1384234736@juju>
Download mbox | patch
Permalink /patch/2923/
State Superseded
Headers show

Comments

Katsunori FUJIWARA - Nov. 12, 2013, 5:38 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1384234132 -32400
#      Tue Nov 12 14:28:52 2013 +0900
# Node ID a1fd26f1794b1158021ba09cd3bab7aff6296418
# Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
merge: confirm continuation when conflict of divergent operations is detected

Before this patch, "hg merge" just shows warning message, when
conflict of divergent operations (multiple renaming, or renaming and
deleting) on same file is detected.

This (silently) successful divergence in merging may be overlooked and
confuse users long after merging.

This patch confirms about allowance for detected conflict of divergent
operations on same file, and abort merging if users don't allow it.

This patch shouldn't break existing tool chains, because default value
of confirmation for non-interactive mode is 'yes' and it doesn't abort
merging.
Mads Kiilerich - Nov. 12, 2013, 9:54 a.m.
On 11/12/2013 06:38 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1384234132 -32400
> #      Tue Nov 12 14:28:52 2013 +0900
> # Node ID a1fd26f1794b1158021ba09cd3bab7aff6296418
> # Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
> merge: confirm continuation when conflict of divergent operations is detected
>
> Before this patch, "hg merge" just shows warning message, when
> conflict of divergent operations (multiple renaming, or renaming and
> deleting) on same file is detected.
>
> This (silently) successful divergence in merging may be overlooked and
> confuse users long after merging.
>
> This patch confirms about allowance for detected conflict of divergent
> operations on same file, and abort merging if users don't allow it.
>
> This patch shouldn't break existing tool chains, because default value
> of confirmation for non-interactive mode is 'yes' and it doesn't abort
> merging.

Do you have a real world case where it really matters? Can you describe it?

My gut feeling is that this kind of "conflicts" not is more serious than 
the case of the same change being done on both sides and less serious 
than the default of having premerge enabled. The new behaviour is thus 
not what I would expect and don't think this prompt would be an 
improvement ... but it would only happen rarely and thus also not do 
much harm ;-)

/Mads
Katsunori FUJIWARA - Nov. 12, 2013, 6:43 p.m.
At Tue, 12 Nov 2013 10:54:48 +0100,
Mads Kiilerich wrote:
> 
> On 11/12/2013 06:38 AM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1384234132 -32400
> > #      Tue Nov 12 14:28:52 2013 +0900
> > # Node ID a1fd26f1794b1158021ba09cd3bab7aff6296418
> > # Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
> > merge: confirm continuation when conflict of divergent operations is detected
> >
> > Before this patch, "hg merge" just shows warning message, when
> > conflict of divergent operations (multiple renaming, or renaming and
> > deleting) on same file is detected.
> >
> > This (silently) successful divergence in merging may be overlooked and
> > confuse users long after merging.
> >
> > This patch confirms about allowance for detected conflict of divergent
> > operations on same file, and abort merging if users don't allow it.
> >
> > This patch shouldn't break existing tool chains, because default value
> > of confirmation for non-interactive mode is 'yes' and it doesn't abort
> > merging.
> 
> Do you have a real world case where it really matters? Can you describe it?
> 
> My gut feeling is that this kind of "conflicts" not is more serious than 
> the case of the same change being done on both sides and less serious 
> than the default of having premerge enabled. The new behaviour is thus 
> not what I would expect and don't think this prompt would be an 
> improvement ... but it would only happen rarely and thus also not do 
> much harm ;-)
> 
> /Mads

This patch comes from the tweet I found: it said "Mercurial seems not
to track renaming".

After some Q & A with owner of the tweet, we found that the root cause
of his problem was not "rename tracking of Mercurial" but "overlooking
divergent renaming in merging".

In his case:

  1. execute divergent renaming accidentally

  2. execute merging, and it succeed silently (for him)

     unfortunately, he uses TortoseHg: it shows warning/notice
     messages only in a moment, if "Automatically advance to next page
     ...." is checked on dialog for merging.

     in the other hand, TortoiseHg can show dialog window corresponded
     to each "ui.promptchoice()" invocations :-)

  3. change renaming destination files individually, and then

  4, notice divergent renaming long after merging

Even if he used Mercurial from CUI, he might not notice divergent
renaming at merging: users often ignore messages just shown, even if
they are warning for some problems :-<


Now, he understands that Mercurial tracks renaming correctly, but
still feels that Mercurial doesn't "manage" renaming well, because "hg
merge" don't confirm about divergent renaming, even if it is detected.

IMHO, for novice users (ordinary users, too?):

  - successful "hg merge" means that there is no (potential) problem, but

  - accidental divergent renaming is problematic operation in many cases

Then, they may feel that Mercurial doesn't manage renaming well:
"record/recognize well" in logic's view may differ from "manage well"
in user's view.


Of course:

  - validity of divergent renaming depends on projects and contexts

  - users can trace renaming by history, and fix problems afterward

But:

  - divergent renaming may occur accidentally

    especially in concurrent working with others, and/or in operations
    by novice users

  - delay of knowing divergence increases cost to fix

    destinations of renaming may be changed individually

So, "hg merge" should prompt about detected divergent renaming for
safety, IMHO.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -166,13 +166,11 @@ 
     opmap = {
         "a": addop,
         "d": renameop,
-        "dr": nop,
         "e": nop,
         "f": addop, # untracked file should be kept in working directory
         "g": addop,
         "m": mergeop,
         "r": removeop,
-        "rd": nop,
     }
     for f, m, args, msg in actions:
         op = opmap.get(m)
@@ -208,7 +206,7 @@ 
     """
 
     overwrite = force and not branchmerge
-    actions, copy, movewithdir = [], {}, {}
+    criticals, copy, movewithdir = [], {}, {}
 
     followcopies = False
     if overwrite:
@@ -228,9 +226,9 @@ 
         ret = copies.mergecopies(repo, wctx, p2, pa)
         copy, movewithdir, diverge, renamedelete = ret
         for of, fl in diverge.iteritems():
-            actions.append((of, "dr", (fl,), "divergent renames"))
+            criticals.append((of, "dr", (fl,)))
         for of, fl in renamedelete.iteritems():
-            actions.append((of, "rd", (fl,), "rename and delete"))
+            criticals.append((of, "rd", (fl,)))
 
     repo.ui.note(_("resolving manifests\n"))
     repo.ui.debug(" branchmerge: %s, force: %s, partial: %s\n"
@@ -248,7 +246,7 @@ 
                 m1['.hgsubstate'] += "+"
                 break
 
-    aborts, prompts = [], []
+    actions, aborts, prompts = [], [], []
     # Compare manifests
     fdiff = dicthelpers.diff(m1, m2)
     flagsdiff = m1.flagsdiff(m2)
@@ -361,6 +359,27 @@ 
         else:
             _checkcollision(repo, m1, actions, prompts)
 
+    for f, m, args in sorted(criticals):
+        if m == "dr": # divergent renames
+            fl, = args
+            query = _("possible conflict - %s was renamed multiple times to:\n"
+                      " %s\n"
+                      "allow this conflict and continue merging? [Yn]"
+                      "$$ &Yes $$ &No") % (f, "\n ".join(fl))
+            if repo.ui.promptchoice(query):
+                raise util.Abort(_("resolve divergent operations "
+                                   "before merging: %s") % f)
+        elif m == "rd": # rename and delete
+            fl, = args
+            query = _("possible conflict - %s was deleted and renamed to:\n"
+                      " %s\n"
+                      "allow this conflict and continue merging? [Yn]"
+                      "$$ &Yes $$ &No") % (f, "\n ".join(fl))
+            if repo.ui.promptchoice(query):
+                raise util.Abort(_("resolve divergent operations"
+                                   " before merging: %s") % f)
+        else: assert False, m
+
     for f, m in sorted(prompts):
         if m == "cd":
             if acceptremote:
@@ -534,18 +553,6 @@ 
                 repo.ui.note(_("getting %s to %s\n") % (f2, fd))
                 repo.wwrite(fd, mctx.filectx(f2).data(), flags)
             updated += 1
-        elif m == "dr": # divergent renames
-            fl, = args
-            repo.ui.warn(_("note: possible conflict - %s was renamed "
-                           "multiple times to:\n") % f)
-            for nf in fl:
-                repo.ui.warn(" %s\n" % nf)
-        elif m == "rd": # rename and delete
-            fl, = args
-            repo.ui.warn(_("note: possible conflict - %s was deleted "
-                           "and renamed to:\n") % f)
-            for nf in fl:
-                repo.ui.warn(" %s\n" % nf)
         elif m == "e": # exec
             flags, = args
             audit(f)
diff --git a/tests/test-bundle-r.t b/tests/test-bundle-r.t
--- a/tests/test-bundle-r.t
+++ b/tests/test-bundle-r.t
@@ -294,9 +294,10 @@ 
 
   $ cd ../test
   $ hg merge 7
-  note: possible conflict - afile was renamed multiple times to:
+  possible conflict - afile was renamed multiple times to:
    anotherfile
    adifferentfile
+  allow this conflict and continue merging? [Yn] y
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
   $ hg ci -m merge
diff --git a/tests/test-issue1175.t b/tests/test-issue1175.t
--- a/tests/test-issue1175.t
+++ b/tests/test-issue1175.t
@@ -13,9 +13,10 @@ 
 
   $ hg mv a a2
   $ hg up
-  note: possible conflict - a was renamed multiple times to:
+  possible conflict - a was renamed multiple times to:
    a2
    a1
+  allow this conflict and continue merging? [Yn] y
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
   $ hg ci -m2
diff --git a/tests/test-rename-merge1.t b/tests/test-rename-merge1.t
--- a/tests/test-rename-merge1.t
+++ b/tests/test-rename-merge1.t
@@ -21,6 +21,14 @@ 
   $ hg ci -m "modify"
   created new head
 
+  $ hg merge --config ui.interactive=true <<EOF
+  > n
+  > EOF
+  possible conflict - a2 was renamed multiple times to:
+   c2
+   b2
+  allow this conflict and continue merging? [Yn] abort: resolve divergent operations before merging: a2
+  [255]
   $ hg merge -y --debug
     searching for copies back to rev 1
     unmatched files in local:
@@ -36,22 +44,21 @@ 
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: af1939970a1c, local: 044f8520aeeb+, remote: 85c198ef2f6c
+  possible conflict - a2 was renamed multiple times to:
+   c2
+   b2
+  allow this conflict and continue merging? [Yn] y
    a: remote moved to b -> m
     preserving a for resolve of b
-   a2: divergent renames -> dr
    b2: remote created -> g
   removing a
   getting b2
-  updating: b2 1/3 files (33.33%)
-  updating: a 2/3 files (66.67%)
+  updating: b2 1/2 files (50.00%)
+  updating: a 2/2 files (100.00%)
   picked tool 'internal:merge' for b (binary False symlink False)
   merging a and b to b
   my b@044f8520aeeb+ other b@85c198ef2f6c ancestor a@af1939970a1c
    premerge successful
-  updating: a2 3/3 files (100.00%)
-  note: possible conflict - a2 was renamed multiple times to:
-   c2
-   b2
   1 files updated, 1 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
 
@@ -171,6 +178,13 @@ 
   $ hg rm file
   $ hg commit -m "deleted file"
   created new head
+  $ hg merge --config ui.interactive=true <<EOF
+  > n
+  > EOF
+  possible conflict - file was deleted and renamed to:
+   newfile
+  allow this conflict and continue merging? [Yn] abort: resolve divergent operations before merging: file
+  [255]
   $ hg merge --debug
     searching for copies back to rev 1
     unmatched files in other:
@@ -181,13 +195,12 @@ 
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 19d7f95df299, local: 0084274f6b67+, remote: 5d32493049f0
-   file: rename and delete -> rd
+  possible conflict - file was deleted and renamed to:
+   newfile
+  allow this conflict and continue merging? [Yn] y
    newfile: remote created -> g
   getting newfile
-  updating: newfile 1/2 files (50.00%)
-  updating: file 2/2 files (100.00%)
-  note: possible conflict - file was deleted and renamed to:
-   newfile
+  updating: newfile 1/1 files (100.00%)
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
   $ hg status
diff --git a/tests/test-rename-merge2.t b/tests/test-rename-merge2.t
--- a/tests/test-rename-merge2.t
+++ b/tests/test-rename-merge2.t
@@ -376,17 +376,16 @@ 
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 924404dff337, local: 02963e448370+, remote: fe905ef2c33e
-   a: divergent renames -> dr
+  possible conflict - a was renamed multiple times to:
+   b
+   c
+  allow this conflict and continue merging? [Yn] y
    c: remote created -> g
    rev: versions differ -> m
     preserving rev for resolve of rev
   getting c
-  updating: c 1/3 files (33.33%)
-  updating: a 2/3 files (66.67%)
-  note: possible conflict - a was renamed multiple times to:
-   b
-   c
-  updating: rev 3/3 files (100.00%)
+  updating: c 1/2 files (50.00%)
+  updating: rev 2/2 files (100.00%)
   picked tool 'python ../merge' for rev (binary False symlink False)
   merging rev
   my rev@02963e448370+ other rev@fe905ef2c33e ancestor rev@924404dff337