[PATCH v2] py3: conditionalize the cPickle import

Martijn Pieters mj at zopatista.com
Mon Jun 6 09:08:34 UTC 2016


On 29 May 2016 at 14:42, Augie Fackler <raf at durin42.com> wrote:
>
>> On May 29, 2016, at 12:29, Pulkit Goyal <7895pulkit at gmail.com> wrote:
>>
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit at gmail.com>
>> # Date 1464550061 -19800
>> #      Mon May 30 00:57:41 2016 +0530
>> # Node ID a881a47f98e8306443c93b7d7cc8660d01c6253a
>> # Parent  9da137faaa9c9593f821923189316e7fcb38cf67
>> py3: conditionalize the cPickle import
>>
>> The cPickle is renamed to _pickle in python3 and this C extension is available
>
> Why are you importing _pickle? Shouldn't we just import pickle and trust that in python3 it'll correctly pick the fastest available implementation?

Absolutely; please replace `import _pickle as pickle` with `import
pickle`. Don't ever import the C implementations directly if there is
a Python wrapper.

>> in pickle which was not included in earlier versions. So imports are conditionalized
>> to import cPickle in py2 and pickle in py3. Moreover the use of pickle in py2 is
>> switched to cPickle as the C extension is faster.
>> Adding the hack in util.py and pycompat.py will be a overkill because just 5 files
>> uses the library.
>>
>> diff --git a/hgext/convert/common.py b/hgext/convert/common.py
>> --- a/hgext/convert/common.py
>> +++ b/hgext/convert/common.py
>> @@ -7,7 +7,6 @@
>> from __future__ import absolute_import
>>
>> import base64
>> -import cPickle as pickle
>> import datetime
>> import errno
>> import os
>> @@ -21,6 +20,13 @@
>>     util,
>> )
>>
>> +try:
>> +    import cPickle as pickle
>> +    pickle.dumps
>> +except ImportError:
>> +    import _pickle as pickle
>> +    pickle.dumps
>> +
>> propertycache = util.propertycache
>>
>> def encodeargs(args):
>> diff --git a/hgext/convert/cvsps.py b/hgext/convert/cvsps.py
>> --- a/hgext/convert/cvsps.py
>> +++ b/hgext/convert/cvsps.py
>> @@ -6,7 +6,6 @@
>> # GNU General Public License version 2 or any later version.
>> from __future__ import absolute_import
>>
>> -import cPickle as pickle
>> import os
>> import re
>>
>> @@ -16,6 +15,13 @@
>>     util,
>> )
>>
>> +try:
>> +    import cPickle as pickle
>> +    pickle.dumps
>> +except ImportError:
>> +    import _pickle as pickle
>> +    pickle.dumps
>> +
>> class logentry(object):
>>     '''Class logentry has the following attributes:
>>         .author    - author name as CVS knows it
>> diff --git a/hgext/convert/subversion.py b/hgext/convert/subversion.py
>> --- a/hgext/convert/subversion.py
>> +++ b/hgext/convert/subversion.py
>> @@ -3,7 +3,6 @@
>> # Copyright(C) 2007 Daniel Holth et al
>> from __future__ import absolute_import
>>
>> -import cPickle as pickle
>> import os
>> import re
>> import sys
>> @@ -21,6 +20,13 @@
>>
>> from . import common
>>
>> +try:
>> +    import cPickle as pickle
>> +    pickle.dumps
>> +except ImportError:
>> +    import _pickle as pickle
>> +    pickle.dumps
>> +
>> stringio = util.stringio
>> propertycache = util.propertycache
>> urlerr = util.urlerr
>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>> --- a/hgext/histedit.py
>> +++ b/hgext/histedit.py
>> @@ -173,7 +173,6 @@
>>
>> import errno
>> import os
>> -import pickle
>> import sys
>>
>> from mercurial.i18n import _
>> @@ -197,6 +196,13 @@
>>     util,
>> )
>>
>> +try:
>> +    import cPickle as pickle
>> +    pickle.dumps
>> +except ImportError:
>> +    import _pickle as pickle
>> +    pickle.dumps
>> +
>> release = lock.release
>> cmdtable = {}
>> command = cmdutil.command(cmdtable)
>> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
>> --- a/mercurial/formatter.py
>> +++ b/mercurial/formatter.py
>> @@ -7,7 +7,6 @@
>>
>> from __future__ import absolute_import
>>
>> -import cPickle
>> import os
>>
>> from .i18n import _
>> @@ -22,6 +21,13 @@
>>     templater,
>> )
>>
>> +try:
>> +    import cPickle as pickle
>> +    pickle.dumps
>> +except ImportError:
>> +    import _pickle as pickle
>> +    pickle.dumps
>> +
>> class baseformatter(object):
>>     def __init__(self, ui, topic, opts):
>>         self._ui = ui
>> @@ -107,7 +113,7 @@
>>         self._data.append(self._item)
>>     def end(self):
>>         baseformatter.end(self)
>> -        self._ui.write(cPickle.dumps(self._data))
>> +        self._ui.write(pickle.dumps(self._data))
>>
>> def _jsonifyobj(v):
>>     if isinstance(v, tuple):
>> diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
>> --- a/tests/test-check-py3-compat.t
>> +++ b/tests/test-check-py3-compat.t
>> @@ -45,10 +45,9 @@
>>   hgext/clonebundles.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   hgext/color.py: invalid syntax: invalid syntax (<unknown>, line *) (glob)
>>   hgext/convert/bzr.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>> -  hgext/convert/common.py: error importing module: <ImportError> No module named 'cPickle' (line *) (glob)
>>   hgext/convert/convcmd.py: error importing: <SyntaxError> invalid syntax (bundle*.py, line *) (error at bundlerepo.py:*) (glob)
>>   hgext/convert/cvs.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>> -  hgext/convert/cvsps.py: error importing module: <ImportError> No module named 'cPickle' (line *) (glob)
>> +  hgext/convert/cvsps.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   hgext/convert/darcs.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>>   hgext/convert/filemap.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>>   hgext/convert/git.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>> @@ -56,7 +55,7 @@
>>   hgext/convert/hg.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   hgext/convert/monotone.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>>   hgext/convert/p*.py: error importing module: <SystemError> Parent module 'hgext.convert' not loaded, cannot perform relative import (line *) (glob)
>> -  hgext/convert/subversion.py: error importing module: <ImportError> No module named 'cPickle' (line *) (glob)
>> +  hgext/convert/subversion.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   hgext/convert/transport.py: error importing module: <ImportError> No module named 'svn.client' (line *) (glob)
>>   hgext/eol.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   hgext/extdiff.py: error importing module: <SyntaxError> invalid syntax (archival.py, line *) (line *) (glob)
>> @@ -109,9 +108,9 @@
>>   mercurial/exchange.py: error importing module: <SyntaxError> invalid syntax (bundle*.py, line *) (line *) (glob)
>>   mercurial/extensions.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/filelog.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>> -  mercurial/filemerge.py: error importing: <ImportError> No module named 'cPickle' (error at formatter.py:*) (glob)
>> +  mercurial/filemerge.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/fileset.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>> -  mercurial/formatter.py: error importing module: <ImportError> No module named 'cPickle' (line *) (glob)
>> +  mercurial/formatter.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/graphmod.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/help.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/hg.py: error importing: <SyntaxError> invalid syntax (bundle*.py, line *) (error at bundlerepo.py:*) (glob)
>> @@ -151,7 +150,7 @@
>>   mercurial/templatefilters.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/templatekw.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/templater.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>> -  mercurial/ui.py: error importing: <ImportError> No module named 'cPickle' (error at formatter.py:*) (glob)
>> +  mercurial/ui.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/unionrepo.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>>   mercurial/url.py: error importing module: <ImportError> No module named 'httplib' (line *) (glob)
>>   mercurial/verify.py: error importing: <AttributeError> 'dict' object has no attribute 'iteritems' (error at revset.py:*) (glob)
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel



-- 
Martijn Pieters


More information about the Mercurial-devel mailing list