fix: fix handling of merge commits by using overlayworkingctx
authorKyle Lippincott <spectral@google.com>
Wed, 18 Dec 2019 14:07:58 -0800
changeset 43967 eebdd6709868
parent 43966 b69d5f3a41d0
child 43968 bbcf78c4ff90
fix: fix handling of merge commits by using overlayworkingctx Most of this code was conceptually copied from what rebase does, with one small difference: hgext.rebaserev.rebase uses branchmerge=True, while I had to use branchmerge=False, or else it got really confused about updating to the same revision in some situations. I believe that the difference is that rebase is always dealing with *some* form of update - it never gets to mergemod.update if the source and destination are the same, while we can encounter that situation with fix. This may imply that this code has some issues with named branches that should be investigated. Differential Revision: https://phab.mercurial-scm.org/D7703
hgext/fix.py
tests/test-fix.t
--- a/hgext/fix.py	Mon Dec 23 10:02:50 2019 -0800
+++ b/hgext/fix.py	Wed Dec 18 14:07:58 2019 -0800
@@ -730,36 +730,40 @@
     ):
         return
 
-    def filectxfn(repo, memctx, path):
-        if path not in ctx:
-            return None
-        fctx = ctx[path]
-        copysource = fctx.copysource()
-        return context.memfilectx(
-            repo,
-            memctx,
-            path=fctx.path(),
-            data=filedata.get(path, fctx.data()),
-            islink=fctx.islink(),
-            isexec=fctx.isexec(),
-            copysource=copysource,
-        )
-
     extra = ctx.extra().copy()
     extra[b'fix_source'] = ctx.hex()
 
-    memctx = context.memctx(
+    wctx = context.overlayworkingctx(repo)
+    wctx.setbase(repo[newp1node])
+    merge.update(
         repo,
-        parents=(newp1node, newp2node),
+        ctx.rev(),
+        branchmerge=False,
+        force=True,
+        ancestor=p1rev,
+        mergeancestor=False,
+        wc=wctx,
+    )
+    copies.duplicatecopies(
+        repo, wctx, ctx.rev(), ctx.p1().rev(), skiprev=newp1node
+    )
+
+    for path in filedata.keys():
+        fctx = ctx[path]
+        copysource = fctx.copysource()
+        wctx.write(path, filedata[path], flags=fctx.flags())
+        if copysource:
+            wctx.markcopied(path, copysource)
+
+    memctx = wctx.tomemctx(
         text=ctx.description(),
-        files=set(ctx.files()) | set(filedata.keys()),
-        filectxfn=filectxfn,
-        user=ctx.user(),
+        branch=ctx.branch(),
+        extra=extra,
         date=ctx.date(),
-        extra=extra,
-        branch=ctx.branch(),
-        editor=None,
+        parents=(newp1node, newp2node),
+        user=ctx.user(),
     )
+
     sucnode = memctx.commit()
     prenode = ctx.node()
     if prenode == sucnode:
--- a/tests/test-fix.t	Mon Dec 23 10:02:50 2019 -0800
+++ b/tests/test-fix.t	Wed Dec 18 14:07:58 2019 -0800
@@ -1456,3 +1456,242 @@
   2 through 2
 
   $ cd ..
+
+Test various cases around merges. We were previously dropping files if they were
+created on only the p2 side of the merge, so let's test permutations of:
+*   added, was fixed
+*   added, considered for fixing but was already good
+*   added, not considered for fixing
+*   modified, was fixed
+*   modified, considered for fixing but was already good
+*   modified, not considered for fixing
+
+Before the bug was fixed where we would drop files, this test demonstrated the
+following issues:
+*   new_in_r1.ignored, new_in_r1_already_good.changed, and
+>   mod_in_r1_already_good.changed were NOT in the manifest for the merge commit
+*   mod_in_r1.ignored had its contents from r0, NOT r1.
+
+We're also setting a named branch for every commit to demonstrate that the
+branch is kept intact and there aren't issues updating to another branch in the
+middle of fix.
+
+  $ hg init merge_keeps_files
+  $ cd merge_keeps_files
+  $ for f in r0 mod_in_r1 mod_in_r2 mod_in_merge mod_in_child; do
+  >   for c in changed whole ignored; do
+  >     printf "hello\n" > $f.$c
+  >   done
+  >   printf "HELLO\n" > "mod_in_${f}_already_good.changed"
+  > done
+  $ hg branch -q r0
+  $ hg ci -Aqm 'r0'
+  $ hg phase -p
+  $ make_test_files() {
+  >   printf "world\n" >> "mod_in_$1.changed"
+  >   printf "world\n" >> "mod_in_$1.whole"
+  >   printf "world\n" >> "mod_in_$1.ignored"
+  >   printf "WORLD\n" >> "mod_in_$1_already_good.changed"
+  >   printf "new in $1\n" > "new_in_$1.changed"
+  >   printf "new in $1\n" > "new_in_$1.whole"
+  >   printf "new in $1\n" > "new_in_$1.ignored"
+  >   printf "ALREADY GOOD, NEW IN THIS REV\n" > "new_in_$1_already_good.changed"
+  > }
+  $ make_test_commit() {
+  >   make_test_files "$1"
+  >   hg branch -q "$1"
+  >   hg ci -Aqm "$2"
+  > }
+  $ make_test_commit r1 "merge me, pt1"
+  $ hg co -q ".^"
+  $ make_test_commit r2 "merge me, pt2"
+  $ hg merge -qr 1
+  $ make_test_commit merge "evil merge"
+  $ make_test_commit child "child of merge"
+  $ make_test_files wdir
+  $ hg fix -r 'not public()' -w
+  $ hg log -G -T'{rev}:{shortest(node,8)}: branch:{branch} desc:{desc}'
+  @  8:c22ce900: branch:child desc:child of merge
+  |
+  o    7:5a30615a: branch:merge desc:evil merge
+  |\
+  | o  6:4e5acdc4: branch:r2 desc:merge me, pt2
+  | |
+  o |  5:eea01878: branch:r1 desc:merge me, pt1
+  |/
+  o  0:0c548d87: branch:r0 desc:r0
+  
+  $ hg files -r tip
+  mod_in_child.changed
+  mod_in_child.ignored
+  mod_in_child.whole
+  mod_in_child_already_good.changed
+  mod_in_merge.changed
+  mod_in_merge.ignored
+  mod_in_merge.whole
+  mod_in_merge_already_good.changed
+  mod_in_mod_in_child_already_good.changed
+  mod_in_mod_in_merge_already_good.changed
+  mod_in_mod_in_r1_already_good.changed
+  mod_in_mod_in_r2_already_good.changed
+  mod_in_r0_already_good.changed
+  mod_in_r1.changed
+  mod_in_r1.ignored
+  mod_in_r1.whole
+  mod_in_r1_already_good.changed
+  mod_in_r2.changed
+  mod_in_r2.ignored
+  mod_in_r2.whole
+  mod_in_r2_already_good.changed
+  new_in_child.changed
+  new_in_child.ignored
+  new_in_child.whole
+  new_in_child_already_good.changed
+  new_in_merge.changed
+  new_in_merge.ignored
+  new_in_merge.whole
+  new_in_merge_already_good.changed
+  new_in_r1.changed
+  new_in_r1.ignored
+  new_in_r1.whole
+  new_in_r1_already_good.changed
+  new_in_r2.changed
+  new_in_r2.ignored
+  new_in_r2.whole
+  new_in_r2_already_good.changed
+  r0.changed
+  r0.ignored
+  r0.whole
+  $ for f in "$(hg files -r tip)"; do hg cat -r tip $f -T'{path}:\n{data}\n'; done
+  mod_in_child.changed:
+  hello
+  WORLD
+  
+  mod_in_child.ignored:
+  hello
+  world
+  
+  mod_in_child.whole:
+  HELLO
+  WORLD
+  
+  mod_in_child_already_good.changed:
+  WORLD
+  
+  mod_in_merge.changed:
+  hello
+  WORLD
+  
+  mod_in_merge.ignored:
+  hello
+  world
+  
+  mod_in_merge.whole:
+  HELLO
+  WORLD
+  
+  mod_in_merge_already_good.changed:
+  WORLD
+  
+  mod_in_mod_in_child_already_good.changed:
+  HELLO
+  
+  mod_in_mod_in_merge_already_good.changed:
+  HELLO
+  
+  mod_in_mod_in_r1_already_good.changed:
+  HELLO
+  
+  mod_in_mod_in_r2_already_good.changed:
+  HELLO
+  
+  mod_in_r0_already_good.changed:
+  HELLO
+  
+  mod_in_r1.changed:
+  hello
+  WORLD
+  
+  mod_in_r1.ignored:
+  hello
+  world
+  
+  mod_in_r1.whole:
+  HELLO
+  WORLD
+  
+  mod_in_r1_already_good.changed:
+  WORLD
+  
+  mod_in_r2.changed:
+  hello
+  WORLD
+  
+  mod_in_r2.ignored:
+  hello
+  world
+  
+  mod_in_r2.whole:
+  HELLO
+  WORLD
+  
+  mod_in_r2_already_good.changed:
+  WORLD
+  
+  new_in_child.changed:
+  NEW IN CHILD
+  
+  new_in_child.ignored:
+  new in child
+  
+  new_in_child.whole:
+  NEW IN CHILD
+  
+  new_in_child_already_good.changed:
+  ALREADY GOOD, NEW IN THIS REV
+  
+  new_in_merge.changed:
+  NEW IN MERGE
+  
+  new_in_merge.ignored:
+  new in merge
+  
+  new_in_merge.whole:
+  NEW IN MERGE
+  
+  new_in_merge_already_good.changed:
+  ALREADY GOOD, NEW IN THIS REV
+  
+  new_in_r1.changed:
+  NEW IN R1
+  
+  new_in_r1.ignored:
+  new in r1
+  
+  new_in_r1.whole:
+  NEW IN R1
+  
+  new_in_r1_already_good.changed:
+  ALREADY GOOD, NEW IN THIS REV
+  
+  new_in_r2.changed:
+  NEW IN R2
+  
+  new_in_r2.ignored:
+  new in r2
+  
+  new_in_r2.whole:
+  NEW IN R2
+  
+  new_in_r2_already_good.changed:
+  ALREADY GOOD, NEW IN THIS REV
+  
+  r0.changed:
+  hello
+  
+  r0.ignored:
+  hello
+  
+  r0.whole:
+  hello
+