Patchwork [4,of,4,stable?] merge: before cd/dc prompt, check that changed side really changed

login
register
mail settings
Submitter Mads Kiilerich
Date Dec. 1, 2014, 1:33 a.m.
Message ID <03a765f8fc576d3f5e06.1417397581@localhost.localdomain>
Download mbox | patch
Permalink /patch/6925/
State Accepted
Commit 902554884335e5ca3661d63be9978eb4aec3f68a
Headers show

Comments

Mads Kiilerich - Dec. 1, 2014, 1:33 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1417397421 -3600
#      Mon Dec 01 02:30:21 2014 +0100
# Branch stable
# Node ID 03a765f8fc576d3f5e0663f4a2b3a419eee49d7d
# Parent  f4e4b477208f723b5edefda8ae1ff23266abc127
merge: before cd/dc prompt, check that changed side really changed

Before, merging would in some cases ask "wrong" questions about
"changed/deleted" conflicts ... and even do it before the resolve phase where
they can be postponed, re"resolved" or answered in bulk operations.

Instead, check that the content of the changed file really did change.

Reading and comparing file content is expensive and should be avoided before
the resolve phase. Prompting the user is however even more expensive. Checking
the content here is thus better.

The 'f in ancestors[0]' should not be necessary but is included to be extra
safe.
Matt Mackall - Dec. 1, 2014, 11:54 p.m.
On Mon, 2014-12-01 at 02:33 +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1417397421 -3600
> #      Mon Dec 01 02:30:21 2014 +0100
> # Branch stable
> # Node ID 03a765f8fc576d3f5e0663f4a2b3a419eee49d7d
> # Parent  f4e4b477208f723b5edefda8ae1ff23266abc127
> merge: before cd/dc prompt, check that changed side really changed
> 
> Before, merging would in some cases ask "wrong" questions about
> "changed/deleted" conflicts ... and even do it before the resolve phase where
> they can be postponed, re"resolved" or answered in bulk operations.
> 
> Instead, check that the content of the changed file really did change.
> 
> Reading and comparing file content is expensive and should be avoided before
> the resolve phase. Prompting the user is however even more expensive. Checking
> the content here is thus better.
> 
> The 'f in ancestors[0]' should not be necessary but is included to be extra
> safe.

> +        if f in ancestors[0] and wctx[f].data() == ancestors[0][f].data():

As discussed on IRC, I've tweaked these in flight to use "not
fctx1.cmp(fctx2)" which has extra smarts relative to raw data()
comparison. So, queued for stable.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -827,7 +827,10 @@  def calculateupdates(repo, wctx, mctx, a
 
     # Prompt and create actions. TODO: Move this towards resolve phase.
     for f, args, msg in actions['cd']:
-        if repo.ui.promptchoice(
+        if f in ancestors[0] and wctx[f].data() == ancestors[0][f].data():
+            # local did change but ended up with same content
+            actions['r'].append((f, None, "prompt same"))
+        elif repo.ui.promptchoice(
             _("local changed %s which remote deleted\n"
               "use (c)hanged version or (d)elete?"
               "$$ &Changed $$ &Delete") % f, 0):
@@ -838,7 +841,10 @@  def calculateupdates(repo, wctx, mctx, a
 
     for f, args, msg in actions['dc']:
         flags, = args
-        if repo.ui.promptchoice(
+        if f in ancestors[0] and mctx[f].data() == ancestors[0][f].data():
+            # remote did change but ended up with same content
+            pass # don't get = keep local deleted
+        elif repo.ui.promptchoice(
             _("remote changed %s which local deleted\n"
               "use (c)hanged version or leave (d)eleted?"
               "$$ &Changed $$ &Deleted") % f, 0) == 0:
diff --git a/tests/test-issue3084.t b/tests/test-issue3084.t
--- a/tests/test-issue3084.t
+++ b/tests/test-issue3084.t
@@ -262,8 +262,6 @@  Ancestor: normal  Parent: normal-same  P
 
   $ hg up -Cqr normal-same
   $ hg merge -r large
-  local changed f which remote deleted
-  use (c)hanged version or (d)elete? c
   getting changed largefiles
   1 largefiles updated, 0 removed
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
@@ -275,11 +273,9 @@  swap
 
   $ hg up -Cqr large
   $ hg merge -r normal-same
-  remote changed f which local deleted
-  use (c)hanged version or leave (d)eleted? c
   getting changed largefiles
-  1 largefiles updated, 0 removed
-  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  0 largefiles updated, 0 removed
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
   $ cat f
   large
@@ -387,8 +383,6 @@  Ancestor: large   Parent: large-same   P
 
   $ hg up -Cqr large-same
   $ hg merge -r normal
-  local changed .hglf/f which remote deleted
-  use (c)hanged version or (d)elete? c
   getting changed largefiles
   0 largefiles updated, 0 removed
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
@@ -400,11 +394,7 @@  swap
 
   $ hg up -Cqr normal
   $ hg merge -r large-same
-  remote changed .hglf/f which local deleted
-  use (c)hanged version or leave (d)eleted? c
-  getting changed largefiles
-  0 largefiles updated, 0 removed
-  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
   $ cat f
   normal