Patchwork D7974: merge: when rename was made on both sides, use ancestor as merge base

login
register
mail settings
Submitter phabricator
Date Jan. 23, 2020, 6:44 a.m.
Message ID <differential-rev-PHID-DREV-eojyghoysoirurpwsm2y-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44583/
State Superseded
Headers show

Comments

phabricator - Jan. 23, 2020, 6:44 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  When both sides of a merge have renamed a file to the same place, we
  would treat that as a "both created" action in merge.py. That means
  that we'd use an empty diffbase. It seems better to use the copy
  source as diffbase. That can be done by simply dropping code that
  prevented us from doing that. I think I did it that way in
  57203e0210f8 <https://phab.mercurial-scm.org/rHG57203e0210f832b34438534770a583de35d0d6c6> (copies: calculate mergecopies() based on pathcopies(),
  2019-04-11) only to preserve the existing behavior. I also suspect it
  was just an accident that it behaved that way before that commit.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

AFFECTED FILES
  mercurial/copies.py
  tests/test-rename-merge2.t

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 24, 2020, 6:06 p.m.
martinvonz added a comment.


  I'll insert another patch with another fun test case before this patch soon

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers
Cc: marmoute, mercurial-devel
phabricator - Jan. 29, 2020, 10:57 p.m.
This revision now requires changes to proceed.
durin42 added a comment.
durin42 requested changes to this revision.


  This one needs rebased.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, marmoute, mercurial-devel
phabricator - Jan. 29, 2020, 11:32 p.m.
martinvonz added a comment.


  In D7974#118492 <https://phab.mercurial-scm.org/D7974#118492>, @durin42 wrote:
  
  > This one needs rebased.
  
  Done.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, marmoute, mercurial-devel
phabricator - Jan. 30, 2020, 3:17 p.m.
pulkit added a comment.


  `test-rename-merge1.t` fails with this patch. Looks like there is one more instance of prompt which is not needed anymore. I didn't amend that in flight as I was not sure.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42, pulkit
Cc: durin42, marmoute, mercurial-devel
phabricator - Jan. 30, 2020, 3:28 p.m.
martinvonz added a comment.


  In D7974#118682 <https://phab.mercurial-scm.org/D7974#118682>, @pulkit wrote:
  
  > `test-rename-merge1.t` fails with this patch. Looks like there is one more instance of prompt which is not needed anymore. I didn't amend that in flight as I was not sure.
  
  I'm pretty sure it's flaky for some reason. Augie also mentioned that that test failed on one of my patches. I ran it 100 times with different python hashes and it never failed for me. I don't know why it behaves differently sometimes. I'm not even sure it started failing with this patch. Does it fail consistently on this one for you? Does it pass consistently on @?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42, pulkit
Cc: durin42, marmoute, mercurial-devel
phabricator - Jan. 30, 2020, 3:29 p.m.
martinvonz added a comment.


  In D7974#118685 <https://phab.mercurial-scm.org/D7974#118685>, @martinvonz wrote:
  
  > In D7974#118682 <https://phab.mercurial-scm.org/D7974#118682>, @pulkit wrote:
  >
  >> `test-rename-merge1.t` fails with this patch. Looks like there is one more instance of prompt which is not needed anymore. I didn't amend that in flight as I was not sure.
  >
  > I'm pretty sure it's flaky for some reason. Augie also mentioned that that test failed on one of my patches. I ran it 100 times with different python hashes and it never failed for me. I don't know why it behaves differently sometimes. I'm not even sure it started failing with this patch. Does it fail consistently on this one for you? Does it pass consistently on @?
  
  Oh, and note that `--runs-per-test=100` will run the test 100 times with the *same* python hash.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42, pulkit
Cc: durin42, marmoute, mercurial-devel
phabricator - Jan. 31, 2020, 5 p.m.
pulkit added a comment.


  I am seeing this failure, hope it helps:
  
    (vm)~/repo/hgpush/tests$ ./run-tests.py test-rename-merge1.t          
    running 1 tests using 1 parallel processes                                                      
                                                                                
    --- /home/pulkit/repo/hgpush/tests/test-rename-merge1.t    
    +++ /home/pulkit/repo/hgpush/tests/test-rename-merge1.t.err                  
    @@ -219,16 +219,6 @@                                                                  
        ancestor: 5151c134577e, local: 07fcbc9a74ed+, remote: f21419739508
        preserving z for resolve of z                              
       starting 4 threads for background file closing (?)                     
    -   x: prompt changed/deleted -> m (premerge)                           
    -  picked tool ':prompt' for x (binary False symlink False changedelete True)
    -  file 'x' was deleted in other [merge rev] but was modified in local [working copy].
    -  You can use (c)hanged version, (d)elete, or leave (u)nresolved.                
    -  What do you want to do? u                                              
    -   y: prompt deleted/changed -> m (premerge)                                
    -  picked tool ':prompt' for y (binary False symlink False changedelete True)        
    -  file 'y' was deleted in local [working copy] but was modified in other [merge rev].
    -  You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
    -  What do you want to do? u                                 
        z: both renamed from y -> m (premerge)                            
       picked tool ':merge3' for z (binary False symlink False changedelete False)                  
       merging z                                                                
                                                                             
    ERROR: test-rename-merge1.t output changed                                   
    !                                                                                     
    Failed test-rename-merge1.t: output changed
    # Ran 1 tests, 0 skipped, 1 failed.
    python hash seed: 3001008644

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42, pulkit
Cc: durin42, marmoute, mercurial-devel
phabricator - Jan. 31, 2020, 5:20 p.m.
martinvonz added a comment.


  In D7974#118853 <https://phab.mercurial-scm.org/D7974#118853>, @pulkit wrote:
  
  > I am seeing this failure, hope it helps:
  >
  >   (vm)~/repo/hgpush/tests$ ./run-tests.py test-rename-merge1.t          
  >   running 1 tests using 1 parallel processes                                                      
  >   --- /home/pulkit/repo/hgpush/tests/test-rename-merge1.t    
  >   +++ /home/pulkit/repo/hgpush/tests/test-rename-merge1.t.err                  
  >   @@ -219,16 +219,6 @@                                                                  
  >       ancestor: 5151c134577e, local: 07fcbc9a74ed+, remote: f21419739508
  >       preserving z for resolve of z                              
  >      starting 4 threads for background file closing (?)                     
  >   -   x: prompt changed/deleted -> m (premerge)                           
  >   -  picked tool ':prompt' for x (binary False symlink False changedelete True)
  >   -  file 'x' was deleted in other [merge rev] but was modified in local [working copy].
  >   -  You can use (c)hanged version, (d)elete, or leave (u)nresolved.                
  >   -  What do you want to do? u                                              
  >   -   y: prompt deleted/changed -> m (premerge)                                
  >   -  picked tool ':prompt' for y (binary False symlink False changedelete True)        
  >   -  file 'y' was deleted in local [working copy] but was modified in other [merge rev].
  >   -  You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
  >   -  What do you want to do? u                                 
  >       z: both renamed from y -> m (premerge)                            
  >      picked tool ':merge3' for z (binary False symlink False changedelete False)                  
  >      merging z                                                                
  >   ERROR: test-rename-merge1.t output changed                                   
  >   !                                                                                     
  >   Failed test-rename-merge1.t: output changed
  >   # Ran 1 tests, 0 skipped, 1 failed.
  >   python hash seed: 3001008644
  
  Consistently?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7974/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7974

To: martinvonz, #hg-reviewers, durin42, pulkit
Cc: durin42, marmoute, mercurial-devel

Patch

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
@@ -513,7 +513,7 @@ 
   test L:nc a b R:up a b W:       - 14 merge b no ancestor
   --------------
     all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'a' -> dst: 'b' 
+     src: 'a' -> dst: 'b' *
     checking for directory renames
   resolving manifests
    branchmerge: True, force: False, partial: False
@@ -522,19 +522,15 @@ 
    preserving rev for resolve of rev
    a: remote is newer -> g
   getting a
-   b: both created -> m (premerge)
+   b: both renamed from a -> m (premerge)
   picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
   merging b
-  my b@86a2aa42fc76+ other b@8dbce441892a ancestor b@000000000000
+  my b@86a2aa42fc76+ other b@8dbce441892a ancestor a@924404dff337
+   premerge successful
    rev: versions differ -> m (premerge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   merging rev
   my rev@86a2aa42fc76+ other rev@8dbce441892a ancestor rev@924404dff337
-   b: both created -> m (merge)
-  picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
-  my b@86a2aa42fc76+ other b@8dbce441892a ancestor b@000000000000
-  launching merge tool: * ../merge *$TESTTMP/t/t/b* * * (glob)
-  merge tool returned: 0
    rev: versions differ -> m (merge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   my rev@86a2aa42fc76+ other rev@8dbce441892a ancestor rev@924404dff337
@@ -593,7 +589,7 @@ 
   test L:nc a b R:up a b W:       - 16 get a, merge b no ancestor
   --------------
     all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'a' -> dst: 'b' 
+     src: 'a' -> dst: 'b' *
     checking for directory renames
   resolving manifests
    branchmerge: True, force: False, partial: False
@@ -602,19 +598,15 @@ 
    preserving rev for resolve of rev
    a: remote is newer -> g
   getting a
-   b: both created -> m (premerge)
+   b: both renamed from a -> m (premerge)
   picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
   merging b
-  my b@86a2aa42fc76+ other b@8dbce441892a ancestor b@000000000000
+  my b@86a2aa42fc76+ other b@8dbce441892a ancestor a@924404dff337
+   premerge successful
    rev: versions differ -> m (premerge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   merging rev
   my rev@86a2aa42fc76+ other rev@8dbce441892a ancestor rev@924404dff337
-   b: both created -> m (merge)
-  picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
-  my b@86a2aa42fc76+ other b@8dbce441892a ancestor b@000000000000
-  launching merge tool: * ../merge *$TESTTMP/t/t/b* * * (glob)
-  merge tool returned: 0
    rev: versions differ -> m (merge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   my rev@86a2aa42fc76+ other rev@8dbce441892a ancestor rev@924404dff337
@@ -633,7 +625,7 @@ 
   test L:up a b R:nc a b W:       - 17 keep a, merge b no ancestor
   --------------
     all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'a' -> dst: 'b' 
+     src: 'a' -> dst: 'b' *
     checking for directory renames
   resolving manifests
    branchmerge: True, force: False, partial: False
@@ -641,19 +633,15 @@ 
    preserving b for resolve of b
    preserving rev for resolve of rev
   starting 4 threads for background file closing (?)
-   b: both created -> m (premerge)
+   b: both renamed from a -> m (premerge)
   picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
   merging b
-  my b@0b76e65c8289+ other b@4ce40f5aca24 ancestor b@000000000000
+  my b@0b76e65c8289+ other b@4ce40f5aca24 ancestor a@924404dff337
+   premerge successful
    rev: versions differ -> m (premerge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   merging rev
   my rev@0b76e65c8289+ other rev@4ce40f5aca24 ancestor rev@924404dff337
-   b: both created -> m (merge)
-  picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
-  my b@0b76e65c8289+ other b@4ce40f5aca24 ancestor b@000000000000
-  launching merge tool: * ../merge *$TESTTMP/t/t/b* * * (glob)
-  merge tool returned: 0
    rev: versions differ -> m (merge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   my rev@0b76e65c8289+ other rev@4ce40f5aca24 ancestor rev@924404dff337
@@ -672,7 +660,7 @@ 
   test L:nm a b R:up a b W:       - 18 merge b no ancestor
   --------------
     all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'a' -> dst: 'b' 
+     src: 'a' -> dst: 'b' *
     checking for directory renames
   resolving manifests
    branchmerge: True, force: False, partial: False
@@ -680,35 +668,24 @@ 
    preserving b for resolve of b
    preserving rev for resolve of rev
   starting 4 threads for background file closing (?)
-   a: prompt deleted/changed -> m (premerge)
-  picked tool ':prompt' for a (binary False symlink False changedelete True)
-  file 'a' was deleted in local [working copy] but was modified in other [merge rev].
-  You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
-  What do you want to do? u
-   b: both created -> m (premerge)
+   b: both renamed from a -> m (premerge)
   picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
   merging b
-  my b@02963e448370+ other b@8dbce441892a ancestor b@000000000000
+  my b@02963e448370+ other b@8dbce441892a ancestor a@924404dff337
+   premerge successful
    rev: versions differ -> m (premerge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   merging rev
   my rev@02963e448370+ other rev@8dbce441892a ancestor rev@924404dff337
-   b: both created -> m (merge)
-  picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
-  my b@02963e448370+ other b@8dbce441892a ancestor b@000000000000
-  launching merge tool: * ../merge *$TESTTMP/t/t/b* * * (glob)
-  merge tool returned: 0
    rev: versions differ -> m (merge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   my rev@02963e448370+ other rev@8dbce441892a ancestor rev@924404dff337
   launching merge tool: * ../merge *$TESTTMP/t/t/rev* * * (glob)
   merge tool returned: 0
-  0 files updated, 2 files merged, 0 files removed, 1 files unresolved
-  use 'hg resolve' to retry unresolved file merges or 'hg merge --abort' to abandon
+  0 files updated, 2 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
   --------------
-  M a
   M b
-  abort: unresolved merge conflicts (see 'hg help resolve')
   --------------
   
   $ tm "up a b" "nm a b" "      " "19 merge b no ancestor, prompt remove a"
@@ -717,44 +694,33 @@ 
   test L:up a b R:nm a b W:       - 19 merge b no ancestor, prompt remove a
   --------------
     all copies found (* = to merge, ! = divergent, % = renamed and deleted):
-     src: 'a' -> dst: 'b' 
+     src: 'a' -> dst: 'b' *
     checking for directory renames
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 924404dff337, local: 0b76e65c8289+, remote: bdb19105162a
-   preserving a for resolve of a
    preserving b for resolve of b
    preserving rev for resolve of rev
+   b: both renamed from a -> m (premerge)
   starting 4 threads for background file closing (?)
-   a: prompt changed/deleted -> m (premerge)
-  picked tool ':prompt' for a (binary False symlink False changedelete True)
-  file 'a' was deleted in other [merge rev] but was modified in local [working copy].
-  You can use (c)hanged version, (d)elete, or leave (u)nresolved.
-  What do you want to do? u
-   b: both created -> m (premerge)
   picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
   merging b
-  my b@0b76e65c8289+ other b@bdb19105162a ancestor b@000000000000
+  my b@0b76e65c8289+ other b@bdb19105162a ancestor a@924404dff337
+   premerge successful
    rev: versions differ -> m (premerge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   merging rev
   my rev@0b76e65c8289+ other rev@bdb19105162a ancestor rev@924404dff337
-   b: both created -> m (merge)
-  picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
-  my b@0b76e65c8289+ other b@bdb19105162a ancestor b@000000000000
-  launching merge tool: * ../merge *$TESTTMP/t/t/b* * * (glob)
-  merge tool returned: 0
    rev: versions differ -> m (merge)
   picked tool '* ../merge' for rev (binary False symlink False changedelete False) (glob)
   my rev@0b76e65c8289+ other rev@bdb19105162a ancestor rev@924404dff337
   launching merge tool: * ../merge *$TESTTMP/t/t/rev* * * (glob)
   merge tool returned: 0
-  0 files updated, 2 files merged, 0 files removed, 1 files unresolved
-  use 'hg resolve' to retry unresolved file merges or 'hg merge --abort' to abandon
+  0 files updated, 2 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
   --------------
   M b
   C a
-  abort: unresolved merge conflicts (see 'hg help resolve')
   --------------
   
   $ tm "up a  " "um a b" "      " "20 merge a and b to b, remove a"
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -542,10 +542,7 @@ 
             return
         # modified on side 2
         for dst in dsts1:
-            if dst not in m2:
-                # dst not added on side 2 (handle as regular
-                # "both created" case in manifestmerge otherwise)
-                copy[dst] = src
+            copy[dst] = src
 
 
 def _fullcopytracing(repo, c1, c2, base):