mergestate: store about files resolved in favour of other

Committing a merge sometimes wrongly creates a new filenode where it can re-use
an existing one. This happens because the commit code does it's own calculation
and does not know what happened on merge.

This starts storing information in mergestate about files which were
automatically merged and the other/remote version of file was used.
We need this information at commit to pick the filenode parent for the new
commit.

This issue was found by Pierre-Yves David and idea to store the relevant parts
in mergestate is also suggested by him.

Somethings which can be further investigated are:

1) refactoring of commit logic more to depend on this information
2) maybe a more generic solution?

Differential Revision: https://phab.mercurial-scm.org/D8392

mercurial/commands.py

@@ -5955,6 +5955,8 def resolve(ui, repo, *pats, **opts):
             if not m(f):
                 continue
 
+            if ms[f] == mergemod.MERGE_RECORD_MERGED_OTHER:
+                continue
             label, key = mergestateinfo[ms[f]]
             fm.startitem()
             fm.context(ctx=wctx)
@@ -6002,6 +6004,9 def resolve(ui, repo, *pats, **opts):
 
             didwork = True
 
+            if ms[f] == mergemod.MERGE_RECORD_MERGED_OTHER:
+                continue
+
             # don't let driver-resolved files be marked, and run the conclude
             # step if asked to resolve
             if ms[f] == mergemod.MERGE_RECORD_DRIVER_RESOLVED:

mercurial/localrepo.py

@@ -2855,6 +2855,14 class localrepository(object):
                 fparent1, fparent2 = fparent2, nullid
             elif fparent2 in fparentancestors:
                 fparent2 = nullid
+            elif not fparentancestors:
+                # TODO: this whole if-else might be simplified much more
+                ms = mergemod.mergestate.read(self)
+                if (
+                    fname in ms
+                    and ms[fname] == mergemod.MERGE_RECORD_MERGED_OTHER
+                ):
+                    fparent1, fparent2 = fparent2, nullid
 
         # is the file changed?
         text = fctx.data()

mercurial/merge.py

@@ -64,6 +64,7 RECORD_LABELS = b'l'
 RECORD_OVERRIDE = b't'
 RECORD_UNSUPPORTED_MANDATORY = b'X'
 RECORD_UNSUPPORTED_ADVISORY = b'x'
+RECORD_RESOLVED_OTHER = b'R'
 
 MERGE_DRIVER_STATE_UNMARKED = b'u'
 MERGE_DRIVER_STATE_MARKED = b'm'
@@ -74,6 +75,9 MERGE_RECORD_RESOLVED = b'r'
 MERGE_RECORD_UNRESOLVED_PATH = b'pu'
 MERGE_RECORD_RESOLVED_PATH = b'pr'
 MERGE_RECORD_DRIVER_RESOLVED = b'd'
+# represents that the file was automatically merged in favor
+# of other version. This info is used on commit.
+MERGE_RECORD_MERGED_OTHER = b'o'
 
 ACTION_FORGET = b'f'
 ACTION_REMOVE = b'r'
@@ -91,6 +95,8 ACTION_DIR_RENAME_MOVE_LOCAL = b'dm'
 ACTION_KEEP = b'k'
 ACTION_EXEC = b'e'
 ACTION_CREATED_MERGE = b'cm'
+# GET the other/remote side and store this info in mergestate
+ACTION_GET_OTHER_AND_STORE = b'gs'
 
 
 class mergestate(object):
@@ -227,6 +233,7 class mergestate(object):
                 RECORD_CHANGEDELETE_CONFLICT,
                 RECORD_PATH_CONFLICT,
                 RECORD_MERGE_DRIVER_MERGE,
+                RECORD_RESOLVED_OTHER,
             ):
                 bits = record.split(b'\0')
                 self._state[bits[0]] = bits[1:]
@@ -453,6 +460,10 class mergestate(object):
                 records.append(
                     (RECORD_PATH_CONFLICT, b'\0'.join([filename] + v))
                 )
+            elif v[0] == MERGE_RECORD_MERGED_OTHER:
+                records.append(
+                    (RECORD_RESOLVED_OTHER, b'\0'.join([filename] + v))
+                )
             elif v[1] == nullhex or v[6] == nullhex:
                 # Change/Delete or Delete/Change conflicts. These are stored in
                 # 'C' records. v[1] is the local file, and is nullhex when the
@@ -551,6 +562,10 class mergestate(object):
         self._state[path] = [MERGE_RECORD_UNRESOLVED_PATH, frename, forigin]
         self._dirty = True
 
+    def addmergedother(self, path):
+        self._state[path] = [MERGE_RECORD_MERGED_OTHER, nullhex, nullhex]
+        self._dirty = True
+
     def __contains__(self, dfile):
         return dfile in self._state
 
@@ -594,6 +609,8 class mergestate(object):
         """rerun merge process for file path `dfile`"""
         if self[dfile] in (MERGE_RECORD_RESOLVED, MERGE_RECORD_DRIVER_RESOLVED):
             return True, 0
+        if self._state[dfile][0] == MERGE_RECORD_MERGED_OTHER:
+            return True, 0
         stateentry = self._state[dfile]
         state, localkey, lfile, afile, anode, ofile, onode, flags = stateentry
         octx = self._repo[self._other]
@@ -1209,7 +1226,7 def _filternarrowactions(narrowmatch, br
     narrowed.
     """
     nooptypes = {b'k'}  # TODO: handle with nonconflicttypes
-    nonconflicttypes = set(b'a am c cm f g r e'.split())
+    nonconflicttypes = set(b'a am c cm f g gs r e'.split())
     # We mutate the items in the dict during iteration, so iterate
     # over a copy.
     for f, action in list(actions.items()):
@@ -1348,14 +1365,22 def manifestmerge(
                         )
                     else:
                         actions[f] = (
-                            ACTION_GET,
+                            ACTION_GET_OTHER_AND_STORE
+                            if branchmerge
+                            else ACTION_GET,
                             (fl2, False),
                             b'remote is newer',
                         )
                 elif nol and n2 == a:  # remote only changed 'x'
                     actions[f] = (ACTION_EXEC, (fl2,), b'update permissions')
                 elif nol and n1 == a:  # local only changed 'x'
-                    actions[f] = (ACTION_GET, (fl1, False), b'remote is newer')
+                    actions[f] = (
+                        ACTION_GET_OTHER_AND_STORE
+                        if branchmerge
+                        else ACTION_GET,
+                        (fl1, False),
+                        b'remote is newer',
+                    )
                 else:  # both changed something
                     actions[f] = (
                         ACTION_MERGE,
@@ -1588,6 +1613,8 def calculateupdates(
 
             for f, a in sorted(pycompat.iteritems(actions)):
                 m, args, msg = a
+                if m == ACTION_GET_OTHER_AND_STORE:
+                    m = ACTION_GET
                 repo.ui.debug(b' %s: %s -> %s\n' % (f, msg, m))
                 if f in fbids:
                     d = fbids[f]
@@ -1813,6 +1840,7 def emptyactions():
             ACTION_KEEP,
             ACTION_PATH_CONFLICT,
             ACTION_PATH_CONFLICT_RESOLVE,
+            ACTION_GET_OTHER_AND_STORE,
         )
     }
 
@@ -1835,6 +1863,11 def applyupdates(
 
     updated, merged, removed = 0, 0, 0
     ms = mergestate.clean(repo, wctx.p1().node(), mctx.node(), labels)
+
+    # add ACTION_GET_OTHER_AND_STORE to mergestate
+    for e in actions[ACTION_GET_OTHER_AND_STORE]:
+        ms.addmergedother(e[0])
+
     moves = []
     for m, l in actions.items():
         l.sort()
@@ -2415,6 +2448,7 def update(
                     ACTION_EXEC,
                     ACTION_REMOVE,
                     ACTION_PATH_CONFLICT_RESOLVE,
+                    ACTION_GET_OTHER_AND_STORE,
                 ):
                     msg = _(b"conflicting changes")
                     hint = _(b"commit or update --clean to discard changes")
@@ -2477,6 +2511,10 def update(
                 actions[m] = []
             actions[m].append((f, args, msg))
 
+        # ACTION_GET_OTHER_AND_STORE is a ACTION_GET + store in mergestate
+        for e in actions[ACTION_GET_OTHER_AND_STORE]:
+            actions[ACTION_GET].append(e)
+
         if not util.fscasesensitive(repo.path):
             # check collision between files only in p2 for clean update
             if not branchmerge and (

tests/test-convert-hg-source.t

@@ -62,9 +62,9 Make and delete some tags
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ cd new
@@ -76,7 +76,7 Make and delete some tags
 #if execbit
   $ hg bookmarks
      premerge1                 3:973ef48a98a4
-     premerge2                 8:91d107c423ba
+     premerge2                 8:c4968fdf2e5d
 #else
 Different hash because no x bit
   $ hg bookmarks
@@ -96,19 +96,19 Test that redoing a convert results in a
   6 make bar and baz copies of foo
   5 merge local copy
   4 merge remote copy
-  3 Added tag that for changeset 88586c4e9f02
+  3 Added tag that for changeset 8601262d7472
   2 Removed tag that
-  1 Added tag this for changeset c56a7f387039
+  1 Added tag this for changeset 706614b458c1
   0 mark baz executable
   updating bookmarks
   $ hg -R new log -G -T '{rev} {desc}'
   o  8 mark baz executable
   |
-  o  7 Added tag this for changeset c56a7f387039
+  o  7 Added tag this for changeset 706614b458c1
   |
   o  6 Removed tag that
   |
-  o  5 Added tag that for changeset 88586c4e9f02
+  o  5 Added tag that for changeset 8601262d7472
   |
   o    4 merge remote copy
   |\

tests/test-copies-chain-merge.t

@@ -319,7 +319,7 Note:
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBDm-0 simple merge - one way'
   $ hg up 'desc("d-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -348,7 +348,6 Note:
   M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mBDm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("d-2")' --rev 'desc("mDBm-0")'
   M b
   $ hg status --copies --rev 'desc("i-2")' --rev 'desc("mBDm-0")'
@@ -361,7 +360,7 Note:
 The bugs makes recorded copy is different depending of where we started the merge from since
 
   $ hg manifest --debug --rev 'desc("mBDm-0")' | grep '644   d'
-  0bb5445dc4d02f4e0d86cf16f9f3a411d0f17744 644   d
+  b004912a8510032a0350a74daa2803dadfb00e12 644   d
   $ hg manifest --debug --rev 'desc("mDBm-0")' | grep '644   d'
   b004912a8510032a0350a74daa2803dadfb00e12 644   d
 
@@ -378,21 +377,13 not a merge.
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1       8 b004912a8510 000000000000 000000000000
-       2      17 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
 
 (This `hg log` output if wrong, since no merge actually happened).
 
   $ hg log -Gfr 'desc("mBDm-0")' d
-  o    17 mBDm-0 simple merge - one way
-  |\
-  o :  8 d-2 re-add d
-  :/
-  o  2 i-2: c -move-> d
+  o  8 d-2 re-add d
   |
-  o  1 i-1: a -move-> c
-  |
-  o  0 i-0 initial commit: a b h
-  
+  ~
 
 This `hg log` output is correct
 
@@ -404,7 +395,6 This `hg log` output is correct
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBDm-0")'
   M b
   A d
-    a
   R a
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mDBm-0")'
   M b
@@ -525,8 +515,7 Merge:
      rev linkrev nodeid       p1           p2
        0       2 01c2f5eabdc4 000000000000 000000000000
        1       8 b004912a8510 000000000000 000000000000
-       2      17 0bb5445dc4d0 01c2f5eabdc4 b004912a8510
-       3      22 c72365ee036f 000000000000 000000000000
+       2      22 c72365ee036f 000000000000 000000000000
   $ hg up 'desc("b-1")'
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("f-2")'
@@ -534,7 +523,7 Merge:
   (branch merge, don't forget to commit)
   $ hg ci -m 'mBFm-0 simple merge - one way'
   $ hg up 'desc("f-2")'
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg merge 'desc("b-1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
@@ -562,7 +551,7 The overwriting should take over. Howeve
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBFm-0")'
   M b
   A d
-    a (true !)
+    h
     h (false !)
   R a
   R h
@@ -577,7 +566,6 The overwriting should take over. Howeve
   R h
   $ hg status --copies --rev 'desc("f-2")' --rev 'desc("mBFm-0")'
   M b
-  M d
   $ hg status --copies --rev 'desc("f-1")' --rev 'desc("mBFm-0")'
   M b
   M d
@@ -595,16 +583,10 The overwriting should take over. Howeve
 The following graphlog is wrong, the "a -> c -> d" chain was overwritten and should not appear.
 
   $ hg log -Gfr 'desc("mBFm-0")' d
-  o    23 mBFm-0 simple merge - one way
-  |\
-  o :  22 f-2: rename i -> d
-  | :
-  o :  21 f-1: rename h -> i
-  :/
-  o  2 i-2: c -move-> d
+  o  22 f-2: rename i -> d
   |
-  o  1 i-1: a -move-> c
-  |
+  o  21 f-1: rename h -> i
+  :
   o  0 i-0 initial commit: a b h