Patchwork mergestate: handle additional record types specially

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 18, 2015, 7:46 p.m.
Message ID <2a2a7a0cc1fa629aa4e0.1447875993@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11488/
State Superseded
Delegated to: Pierre-Yves David
Headers show

Comments

Siddharth Agarwal - Nov. 18, 2015, 7:46 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1447875906 28800
#      Wed Nov 18 11:45:06 2015 -0800
# Node ID 2a2a7a0cc1fa629aa4e096bd6045af510c05467c
# Parent  65351015f3ca03102f39e00167864e139d48e477
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r 2a2a7a0cc1fa
mergestate: handle additional record types specially

This works around a bug in older Mercurial versions' handling of the v2 merge
state.

We also add a bunch of tests that make sure that
(1) we correctly abort when the merge state has an unsupported record type
(2) aborting the merge, rebase or histedit continues to work and clears out the
    merge state.
Siddharth Agarwal - Nov. 18, 2015, 7:52 p.m.
On 11/18/15 11:46, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal<sid0@fb.com>
> # Date 1447875906 28800
> #      Wed Nov 18 11:45:06 2015 -0800
> # Node ID 2a2a7a0cc1fa629aa4e096bd6045af510c05467c
> # Parent  65351015f3ca03102f39e00167864e139d48e477
> # Available Athttp://42.netv6.net/sid0-wip/hg/
> #              hg pullhttp://42.netv6.net/sid0-wip/hg/  -r 2a2a7a0cc1fa
> mergestate: handle additional record types specially

Meant to flag this as V2.
Pierre-Yves David - Nov. 18, 2015, 10:51 p.m.
On 11/18/2015 11:46 AM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1447875906 28800
> #      Wed Nov 18 11:45:06 2015 -0800
> # Node ID 2a2a7a0cc1fa629aa4e096bd6045af510c05467c
> # Parent  65351015f3ca03102f39e00167864e139d48e477
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r 2a2a7a0cc1fa
> mergestate: handle additional record types specially

Patch seems solid, but I've questions regarding its documentation. See 
below.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -65,6 +65,8 @@  class mergestate(object):
        (experimental)
     m: the external merge driver defined for this merge plus its run state
        (experimental)
+    X: unsupported mandatory record type (used in tests)
+    x: unsupported advisory record type (used in tests)
 
     Merge driver run states (experimental):
     u: driver-resolved files unmarked -- needs to be run next time we're about
@@ -226,7 +228,15 @@  class mergestate(object):
     def _readrecordsv2(self):
         """read on disk merge state for version 2 file
 
-        returns list of record [(TYPE, data), ...]
+        Mercurial versions prior to 3.7 have a bug where if there are
+        unsupported mandatory merge records, attempting to clear out the merge
+        state with hg update --clean or similar aborts. The 't' record type
+        works around that by writing out what those versions treat as an
+        advisory record, but later versions interpret as special: the first
+        character is the 'real' record type and everything onwards is the data.
+
+        Returns list of records [(TYPE, data), ...].
+
         """
         records = []
         try:
@@ -241,6 +251,8 @@  class mergestate(object):
                 off += 4
                 record = data[off:(off + length)]
                 off += length
+                if rtype == 't':
+                    rtype, record = record[0], record[1:]
                 records.append((rtype, record))
             f.close()
         except IOError as err:
@@ -322,10 +334,16 @@  class mergestate(object):
         f.close()
 
     def _writerecordsv2(self, records):
-        """Write current state on disk in a version 2 file"""
+        """Write current state on disk in a version 2 file
+
+        See the docstring for _readrecordsv2 for why we use 't'."""
+        # these are the records that all version 2 clients can read
+        whitelist = 'LOF'
         f = self._repo.vfs(self.statepathv2, 'w')
         for key, data in records:
             assert len(key) == 1
+            if key not in whitelist:
+                key, data = 't', '%s%s' % (key, data)
             format = '>sI%is' % len(data)
             f.write(_pack(format, key, len(data), data))
         f.close()
diff --git a/tests/fakemergerecord.py b/tests/fakemergerecord.py
new file mode 100644
--- /dev/null
+++ b/tests/fakemergerecord.py
@@ -0,0 +1,25 @@ 
+# Extension to write out fake unsupported records into the merge state
+#
+#
+
+from __future__ import absolute_import
+
+from mercurial import (
+    cmdutil,
+    merge,
+)
+
+cmdtable = {}
+command = cmdutil.command(cmdtable)
+
+@command('fakemergerecord',
+         [('X', 'mandatory', None, 'add a fake mandatory record'),
+          ('x', 'advisory', None, 'add a fake advisory record')], '')
+def fakemergerecord(ui, repo, *pats, **opts):
+    ms = merge.mergestate.read(repo)
+    records = ms._makerecords()
+    if opts.get('mandatory'):
+        records.append(('X', 'mandatory record'))
+    if opts.get('advisory'):
+        records.append(('x', 'advisory record'))
+    ms._writerecords(records)
diff --git a/tests/test-histedit-non-commute-abort.t b/tests/test-histedit-non-commute-abort.t
--- a/tests/test-histedit-non-commute-abort.t
+++ b/tests/test-histedit-non-commute-abort.t
@@ -75,10 +75,45 @@  edit the history
   warning: conflicts while merging e! (edit, then use 'hg resolve --mark')
   Fix up the change and run hg histedit --continue
 
+insert unsupported advisory merge record
+  $ hg --config extensions.fakemergerecord=$TESTDIR/fakemergerecord.py fakemergerecord -x
+  $ hg debugmergestate
+  * version 2 records
+  local: 8f7551c7e4a2f2efe0bc8c741baf7f227d65d758
+  other: e860deea161a2f77de56603b340ebbb4536308ae
+  unrecognized entry: x	advisory record
+  file: e (record type "F", state "u", hash 58e6b3a414a1e090dfc6029add0f3555ccba127f)
+    local path: e (flags "")
+    ancestor path: e (node 0000000000000000000000000000000000000000)
+    other path: e (node 6b67ccefd5ce6de77e7ead4f5292843a0255329f)
+  $ hg resolve -l
+  U e
 
-abort the edit
+insert unsupported mandatory merge record
+  $ hg --config extensions.fakemergerecord=$TESTDIR/fakemergerecord.py fakemergerecord -X
+  $ hg debugmergestate
+  * version 2 records
+  local: 8f7551c7e4a2f2efe0bc8c741baf7f227d65d758
+  other: e860deea161a2f77de56603b340ebbb4536308ae
+  file: e (record type "F", state "u", hash 58e6b3a414a1e090dfc6029add0f3555ccba127f)
+    local path: e (flags "")
+    ancestor path: e (node 0000000000000000000000000000000000000000)
+    other path: e (node 6b67ccefd5ce6de77e7ead4f5292843a0255329f)
+  unrecognized entry: X	mandatory record
+  $ hg resolve -l
+  abort: unsupported merge state records: X
+  (see https://mercurial-scm.org/wiki/MergeStateRecords for more information)
+  [255]
+  $ hg resolve -ma
+  abort: unsupported merge state records: X
+  (see https://mercurial-scm.org/wiki/MergeStateRecords for more information)
+  [255]
+
+abort the edit (should clear out merge state)
   $ hg histedit --abort 2>&1 | fixbundle
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg debugmergestate
+  no merge state found
 
 log after abort
   $ hg resolve -l
diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
--- a/tests/test-rebase-abort.t
+++ b/tests/test-rebase-abort.t
@@ -68,11 +68,49 @@  Conflicting rebase:
   unresolved conflicts (see hg resolve, then hg rebase --continue)
   [1]
 
-Abort:
+Insert unsupported advisory merge record:
+
+  $ hg --config extensions.fakemergerecord=$TESTDIR/fakemergerecord.py fakemergerecord -x
+  $ hg debugmergestate
+  * version 2 records
+  local: 3e046f2ecedb793b97ed32108086edd1a162f8bc
+  other: 46f0b057b5c061d276b91491c22151f78698abd2
+  unrecognized entry: x	advisory record
+  file: common (record type "F", state "u", hash 94c8c21d08740f5da9eaa38d1f175c592692f0d1)
+    local path: common (flags "")
+    ancestor path: common (node de0a666fdd9c1a0b0698b90d85064d8bd34f74b6)
+    other path: common (node 2f6411de53677f6f1048fef5bf888d67a342e0a5)
+  $ hg resolve -l
+  U common
+
+Insert unsupported mandatory merge record:
+
+  $ hg --config extensions.fakemergerecord=$TESTDIR/fakemergerecord.py fakemergerecord -X
+  $ hg debugmergestate
+  * version 2 records
+  local: 3e046f2ecedb793b97ed32108086edd1a162f8bc
+  other: 46f0b057b5c061d276b91491c22151f78698abd2
+  file: common (record type "F", state "u", hash 94c8c21d08740f5da9eaa38d1f175c592692f0d1)
+    local path: common (flags "")
+    ancestor path: common (node de0a666fdd9c1a0b0698b90d85064d8bd34f74b6)
+    other path: common (node 2f6411de53677f6f1048fef5bf888d67a342e0a5)
+  unrecognized entry: X	mandatory record
+  $ hg resolve -l
+  abort: unsupported merge state records: X
+  (see https://mercurial-scm.org/wiki/MergeStateRecords for more information)
+  [255]
+  $ hg resolve -ma
+  abort: unsupported merge state records: X
+  (see https://mercurial-scm.org/wiki/MergeStateRecords for more information)
+  [255]
+
+Abort (should clear out unsupported merge state):
 
   $ hg rebase --abort
   saved backup bundle to $TESTTMP/a/.hg/strip-backup/3e046f2ecedb-6beef7d5-backup.hg (glob)
   rebase aborted
+  $ hg debugmergestate
+  no merge state found
 
   $ hg tglog
   @  4:draft 'L2'
diff --git a/tests/test-resolve.t b/tests/test-resolve.t
--- a/tests/test-resolve.t
+++ b/tests/test-resolve.t
@@ -226,9 +226,69 @@  resolve <file> should do nothing if 'fil
   $ cat file1
   resolved
 
+insert unsupported advisory merge record
+
+  $ hg --config extensions.fakemergerecord=$TESTDIR/fakemergerecord.py fakemergerecord -x
+  $ hg debugmergestate
+  * version 2 records
+  local: 57653b9f834a4493f7240b0681efcb9ae7cab745
+  other: dc77451844e37f03f5c559e3b8529b2b48d381d1
+  unrecognized entry: x	advisory record
+  file: file1 (record type "F", state "r", hash 60b27f004e454aca81b0480209cce5081ec52390)
+    local path: file1 (flags "")
+    ancestor path: file1 (node 2ed2a3912a0b24502043eae84ee4b279c18b90dd)
+    other path: file1 (node 6f4310b00b9a147241b071a60c28a650827fb03d)
+  file: file2 (record type "F", state "u", hash cb99b709a1978bd205ab9dfd4c5aaa1fc91c7523)
+    local path: file2 (flags "")
+    ancestor path: file2 (node 2ed2a3912a0b24502043eae84ee4b279c18b90dd)
+    other path: file2 (node 6f4310b00b9a147241b071a60c28a650827fb03d)
+  $ hg resolve -l
+  R file1
+  U file2
+
+insert unsupported mandatory merge record
+
+  $ hg --config extensions.fakemergerecord=$TESTDIR/fakemergerecord.py fakemergerecord -X
+  $ hg debugmergestate
+  * version 2 records
+  local: 57653b9f834a4493f7240b0681efcb9ae7cab745
+  other: dc77451844e37f03f5c559e3b8529b2b48d381d1
+  file: file1 (record type "F", state "r", hash 60b27f004e454aca81b0480209cce5081ec52390)
+    local path: file1 (flags "")
+    ancestor path: file1 (node 2ed2a3912a0b24502043eae84ee4b279c18b90dd)
+    other path: file1 (node 6f4310b00b9a147241b071a60c28a650827fb03d)
+  file: file2 (record type "F", state "u", hash cb99b709a1978bd205ab9dfd4c5aaa1fc91c7523)
+    local path: file2 (flags "")
+    ancestor path: file2 (node 2ed2a3912a0b24502043eae84ee4b279c18b90dd)
+    other path: file2 (node 6f4310b00b9a147241b071a60c28a650827fb03d)
+  unrecognized entry: X	mandatory record
+  $ hg resolve -l
+  abort: unsupported merge state records: X
+  (see https://mercurial-scm.org/wiki/MergeStateRecords for more information)
+  [255]
+  $ hg resolve -ma
+  abort: unsupported merge state records: X
+  (see https://mercurial-scm.org/wiki/MergeStateRecords for more information)
+  [255]
+  $ hg summary
+  parent: 2:57653b9f834a 
+   append baz to files
+  parent: 1:dc77451844e3 
+   append bar to files
+  branch: default
+  warning: merge state has unsupported record types: X
+  commit: 2 modified, 2 unknown (merge)
+  update: 2 new changesets (update)
+  phases: 5 draft
+
+update --clean shouldn't abort on unsupported records
+
+  $ hg up -qC 1
+  $ hg debugmergestate
+  no merge state found
+
 test crashed merge with empty mergestate
 
-  $ hg up -qC 1
   $ mkdir .hg/merge
   $ touch .hg/merge/state