Patchwork [8,of,8] mercurial: only load C modules if they have expected versions

login
register
mail settings
Submitter Jun Wu
Date May 3, 2017, 12:20 a.m.
Message ID <95e1615828bc8a2890f9.1493770805@x1c>
Download mbox | patch
Permalink /patch/20381/
State Deferred
Headers show

Comments

Jun Wu - May 3, 2017, 12:20 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1493181614 25200
#      Tue Apr 25 21:40:14 2017 -0700
# Node ID 95e1615828bc8a2890f9cc6d2536a7a8141cf013
# Parent  ecbbee35fdb88babfb4ae0223bf4ee0294135066
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 95e1615828bc
mercurial: only load C modules if they have expected versions

This patch makes the Python 2 module loader check versions of C modules. If
the version does not match what we specified in the Python code, refuse to
load them.

This should have no impact on end-users. For mercurial developers,
The C module will be treated as if it's absent, if its version is different
from what the Python code wants. It allows us to not worry about an
inconsistent state where the C code is newer or older than the Python code.
And certain breaking changes to C code could be made in a cleaner way.

For bisect users, they could set HGMODULEPOLICY to "allow" to test behavior
without recompiling, or they could write some script to detect the version
change and run "make local" in a semi-automatic way.

Effectively, this patch may require developers to run "make local" to
update their binary modules, which is a bit inconvenient. However, the
consistency guarantee seems worthwhile to justify that rare inconvenience.

Patch

diff --git a/mercurial/__init__.py b/mercurial/__init__.py
--- a/mercurial/__init__.py
+++ b/mercurial/__init__.py
@@ -21,4 +21,16 @@  from . import (
 modulepolicy = policy.policy
 
+# Versions for C modules. If a C module does not have the desired version,
+# treat it as "invalid" and do not use it. This makes it impossible to have
+# mismatched .so with .py files.
+_cmoduleversions = {
+    'mercurial.base85': 1,
+    'mercurial.bdiff': 1,
+    'mercurial.diffhelpers': 1,
+    'mercurial.mpatch': 1,
+    'mercurial.osutil': 1,
+    'mercurial.parsers': 1,
+}
+
 # Modules that have both Python and C implementations. See also the
 # set of .py files under mercurial/pure/.
@@ -32,4 +44,12 @@  modulepolicy = policy.policy
 ])
 
+def _checkcmoduleversion(modname, mod):
+    """check version of a C module. raise ImportError if version mismatch"""
+    expected = _cmoduleversions.get(modname, 0)
+    actual = getattr(mod, '_version', 0)
+    if expected != actual:
+        raise ImportError('%s: mismatched version: got %s, expected %s' %
+                          (modname, actual, expected))
+
 class hgimporter(object):
     """Object that conforms to import hook interface defined in PEP-302."""
@@ -68,4 +88,5 @@  class hgimporter(object):
                 # indicates the type of module. So just assume what we found
                 # is OK (even though it could be a pure Python module).
+                _checkcmoduleversion(name, mod)
             except ImportError:
                 if modulepolicy == b'c':
@@ -106,4 +127,6 @@  class hgimporter(object):
                                   'version should exist' % name)
 
+            mod = imp.load_module(name, *modinfo)
+            _checkcmoduleversion(name, mod)
         except ImportError:
             if modulepolicy == b'c':
@@ -117,6 +140,6 @@  class hgimporter(object):
                 raise ImportError('could not find mercurial module %s' %
                                   name)
+            mod = imp.load_module(name, *modinfo)
 
-        mod = imp.load_module(name, *modinfo)
         sys.modules[name] = mod
         return mod