Page MenuHomePhabricator

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

Authored by martinvonz on Jan 23 2020, 1:44 AM.

Details

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 (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.

Note that until fa9ad1da2e77 (merge: start using the per-side copy
dicts, 2020-01-23), it was non-deterministic (depending on iteration
order of the allsources set in copies._fullcopytracing()) which
source was used in the affected test case in test-rename-merge1.t. We
could easily have fixed that by sorting them, but now we can instead
detect the case (the TODO added in the previous patch).

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jan 23 2020, 1:44 AM

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

durin42 requested changes to this revision.Jan 29 2020, 5:57 PM
durin42 added a subscriber: durin42.

This one needs rebased.

This revision now requires changes to proceed.Jan 29 2020, 5:57 PM
martinvonz updated this revision to Diff 19689.Jan 29 2020, 6:32 PM

This one needs rebased.

Done.

pulkit accepted this revision.Jan 30 2020, 9:53 AM

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.

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 @?

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.

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

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?

martinvonz edited the summary of this revision. (Show Details)Jan 31 2020, 6:42 PM
martinvonz updated this revision to Diff 19803.
pulkit accepted this revision.Jan 31 2020, 6:43 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.