Patchwork [04,of,10,PyPy] histedit: only use pickle if not using the modern save format

login
register
mail settings
Submitter Bryan O'Sullivan
Date Dec. 24, 2015, 12:22 a.m.
Message ID <c4f5fdce149644e65ff4.1450916539@bryano-mbp.local>
Download mbox | patch
Permalink /patch/12338/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Bryan O'Sullivan - Dec. 24, 2015, 12:22 a.m.
This avoids a case where PyPy's cPickle module throws a more confusing
error than CPython's.
Yuya Nishihara - Dec. 27, 2015, 4:10 p.m.
On Wed, 23 Dec 2015 16:22:19 -0800, Bryan O'Sullivan wrote:
> This avoids a case where PyPy's cPickle module throws a more confusing
> error than CPython's.

I've queued the first 4, thanks. I'll read the remainder tomorrow.

BTW, do you know why patch headers were lost?

> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -252,19 +252,19 @@ class histeditstate(object):
>      def read(self):
>          """Load histedit state from disk and set fields appropriately."""
>          try:
> -            fp = self.repo.vfs('histedit-state', 'r')
> +            state = self.repo.vfs.read('histedit-state')
>          except IOError as err:
>              if err.errno != errno.ENOENT:
>                  raise
>              raise error.Abort(_('no histedit in progress'))
>  
> -        try:
> -            data = pickle.load(fp)
> +        if state.startswith('v1\n'):
> +            data = self._load()
> +            parentctxnode, rules, keep, topmost, replacements, backupfile = data
> +        else:
> +            data = pickle.loads(state)
>              parentctxnode, rules, keep, topmost, replacements = data
>              backupfile = None
> -        except pickle.UnpicklingError:
> -            data = self._load()
> -            parentctxnode, rules, keep, topmost, replacements, backupfile = data

This is off topic, but the old pickle format seems not work anymore probably
because the old format doesn't keep a full hash.

  % hg-3.3/hg clone https://selenic.com/repo/hello/
  % cd hello
  % hg-3.3/hg phase -fdr0
  % hg-3.3/hg histedit 0  # edit all
  % hg-dev/hg histedit --continue
  abort: unknown revision 'xxx'!  # xxx: binary data

I've tried it because pickle.loads() isn't covered by our tests.

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -252,19 +252,19 @@  class histeditstate(object):
     def read(self):
         """Load histedit state from disk and set fields appropriately."""
         try:
-            fp = self.repo.vfs('histedit-state', 'r')
+            state = self.repo.vfs.read('histedit-state')
         except IOError as err:
             if err.errno != errno.ENOENT:
                 raise
             raise error.Abort(_('no histedit in progress'))
 
-        try:
-            data = pickle.load(fp)
+        if state.startswith('v1\n'):
+            data = self._load()
+            parentctxnode, rules, keep, topmost, replacements, backupfile = data
+        else:
+            data = pickle.loads(state)
             parentctxnode, rules, keep, topmost, replacements = data
             backupfile = None
-        except pickle.UnpicklingError:
-            data = self._load()
-            parentctxnode, rules, keep, topmost, replacements, backupfile = data
 
         self.parentctxnode = parentctxnode
         rules = "\n".join(["%s %s" % (verb, rest) for [verb, rest] in rules])