Patchwork [4,of,4,mergedriver] mergestate: handle additional record types specially

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 18, 2015, 6:41 a.m.
Message ID <c95cea9046503b324619.1447828899@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11471/
State Superseded
Commit a01ecbcfaf847390660d66bfd8c9150d938f21bf
Delegated to: Martin von Zweigbergk
Headers show

Comments

Siddharth Agarwal - Nov. 18, 2015, 6:41 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1447804144 28800
#      Tue Nov 17 15:49:04 2015 -0800
# Node ID c95cea9046503b32461910270ed8966d7b790073
# Parent  5a8a002259aa129e3c3a332df860d09db62c2964
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r c95cea904650
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.
Martin von Zweigbergk - Nov. 18, 2015, 6:40 p.m.
On Tue, Nov 17, 2015 at 10:45 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1447804144 28800
> #      Tue Nov 17 15:49:04 2015 -0800
> # Node ID c95cea9046503b32461910270ed8966d7b790073
> # Parent  5a8a002259aa129e3c3a332df860d09db62c2964
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r c95cea904650
> 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.
>
> 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/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
>

Should this patch include a fakemergerecord.py?


> +  $ 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
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>

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/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