[PATCH 2 of 6 import-refactor V2] mercurial: implement import hook for handling C/Python modules

[Yuya Nishihara]

On Tue, 24 Nov 2015 23:31:46 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1448432444 28800
> #      Tue Nov 24 22:20:44 2015 -0800
> # Node ID 18fcf978ddd46ba5df67210a8586051c77653b5f
> # Parent  82b37470a224dc1e496b86b0ba14df391a3f3ecd
> mercurial: implement import hook for handling C/Python modules

> We choose the main mercurial/__init__.py module for this code out of
> necessity: in a future world, if the custom module importer isn't
> registered, we'll fail to find/import certain modules when running
> from a pure installation. Without the magical import-time side-effects,
> *any* importer of mercurial.* modules would be required to call a
> function to register our importer. I'm not a fan of import time side
> effects and I initially attempted to do this. However, I was foiled by
> our own test harness, which has numerous `python` invoked scripts that
> "import mercurial" and fail because the importer isn't registered.
> Realizing this problem is probably present in random Python scripts
> that have been written over the years, I decided that sacrificing
> purity for backwards compatibility is necessary. Plus, if you are
> programming Python, "import" should probably "just work."

Yeah, I agree that the import-time side effect isn't nice but we would have
no better alternative.

> +class hgimporter(object):
> +    """Object that conforms to import hook interface defined in PEP-302."""
> +    def find_module(self, name, path=None):
> +        # We only care about modules that have both C and pure implementations.
> +        if name in _dualmodules:
> +            return self
> +        return None
> +
> +    def load_module(self, name):
> +        mod = sys.modules.get(name, None)
> +        if mod:
> +            return mod
> +
> +        # imp.find_module doesn't support submodules (modules with ".").
> +        # Instead you have to pass the parent package's __path__ attribute
> +        # as the path argument.
> +        stem = name.split('.')[-1]
> +
> +        mercurial = sys.modules['mercurial']
> +
> +        # Unlike the default importer which searches special locations and
> +        # sys.path, we only look in the directory where "mercurial" was
> +        # imported from.
> +
> +        try:
> +            if modulepolicy == 'py':
> +                raise ImportError()
> +
> +            modinfo = imp.find_module(stem, mercurial.__path__)
> +
> +            # The Mercurial installer used to copy files from
> +            # mercurial/pure/*.py to mercurial/*.py. Therefore, it's possible
> +            # for some installations to have .py files under mercurial/*.
> +            # Loading Python modules when we expected C versions could result
> +            # in a) poor performance b) loading a version from a previous
> +            # Mercurial version, potentially leading to incompatibility. Either
> +            # scenario is bad. So we verify that modules loaded from
> +            # mercurial/* are C extensions. If the current policy allows the
> +            # loading of .py modules, the module will be re-imported from
> +            # mercurial/pure/* below.
> +            # TODO uncomment once setup.py is updated to actually install
> +            # into mercurial/pure.
> +            #if modinfo[2][2] != imp.C_EXTENSION:
> +            #    raise ImportError('.py version of %s found where C '
> +            #                      'version should exist' % name)
> +
> +        except ImportError:
> +            if modulepolicy == 'c':
> +                raise
> +
> +            # Could not load the C extension and pure Python is allowed. So
> +            # try to load them.
> +            from . import pure
> +            modinfo = imp.find_module(stem, pure.__path__)
> +            if not modinfo:
> +                raise ImportError('could not find mercurial module %s' %
> +                                  name)
> +
> +        mod = imp.load_module(name, *modinfo)
> +        sys.modules[name] = mod
> +        return mod

PEP 302 says "the __loader__ attribute must be set to the loader object", but
I don't know it is true for Python 2.

> +# We automagically register our custom importer as a side-effect of loading.
> +# This is necessary to ensure that any entry points are able to import
> +# mercurial.* modules without having to perform this registration themselves.
> +if not any(isinstance(x, hgimporter) for x in sys.meta_path):
> +    # meta_path is used before any implicit finders and before sys.path.
> +    sys.meta_path.insert(0, hgimporter())

It appears py2exe hates hgimporter. Can you investigate it?

> --- a/setup.py
> +++ b/setup.py
> -# Execute hg out of this directory with a custom environment which
> -# includes the pure Python modules in mercurial/pure. We also take
> -# care to not use any hgrc files and do no localization.
> -pypath = ['mercurial', os.path.join('mercurial', 'pure')]
> -env = {'PYTHONPATH': os.pathsep.join(pypath),
> +# Execute hg out of this directory with a custom environment which takes care
> +# to not use any hgrc files and do no localization.
> +env = {'HGMODULEPOLICY': 'py',

Is there any reason you change it to 'py' instead of 'allow' ?