# HG changeset patch # User Philippe Pepiot # Date 1486471623 -3600 # Node ID f85ec84355db8499e235f7fedc80863333c23d58 # Parent e0d708fb20e880fc38ad922ffea691a0b6e54b43 Fix possible double import of cubes modules When cubes using the new layout are imported with 'cubicweb_' and with 'cubes.', the same module is imported twice. Handle this by adding 'cubes.' to sys.modules when importing from 'cubicweb_'. Move load_module() to a sub class _CubesLoader to share informations computed in find_modules(). Don't handle subpackages in _CubesImporter and rely on normal import machinery instead. Add a test and use unittest from cubicweb.devtools.testlib which resolve to unittest2 on PY2 with assertLogs() method. diff -r e0d708fb20e8 -r f85ec84355db cubicweb/__init__.py --- a/cubicweb/__init__.py Wed Feb 08 10:31:26 2017 +0100 +++ b/cubicweb/__init__.py Tue Feb 07 13:47:03 2017 +0100 @@ -24,7 +24,6 @@ import logging import os import pickle -import pkgutil import sys import types import warnings @@ -283,6 +282,37 @@ # Import hook for "legacy" cubes ############################################## +class _CubesLoader(object): + + def __init__(self, *modinfo): + self.modinfo = modinfo + + def load_module(self, fullname): + try: + # If there is an existing module object named 'fullname' in + # sys.modules , the loader must use that existing module. + # Otherwise, the reload() builtin will not work correctly. + return sys.modules[fullname] + except KeyError: + pass + if fullname == 'cubes': + mod = sys.modules[fullname] = types.ModuleType( + fullname, doc='CubicWeb cubes') + else: + modname, file, pathname, description = self.modinfo + try: + mod = sys.modules[fullname] = imp.load_module( + modname, file, pathname, description) + finally: + # https://docs.python.org/2/library/imp.html#imp.load_module + # Important: the caller is responsible for closing the file + # argument, if it was not None, even when an exception is + # raised. This is best done using a try ... finally statement + if file is not None: + file.close() + return mod + + class _CubesImporter(object): """Module finder handling redirection of import of "cubes." to "cubicweb_". @@ -296,25 +326,12 @@ def find_module(self, fullname, path=None): if fullname == 'cubes': - return self - elif fullname.startswith('cubes.'): + return _CubesLoader() + elif fullname.startswith('cubes.') and fullname.count('.') == 1: modname = 'cubicweb_' + fullname.split('.', 1)[1] try: modinfo = imp.find_module(modname) except ImportError: return None else: - return pkgutil.ImpLoader(fullname, *modinfo) - - def load_module(self, fullname): - try: - # If there is an existing module object named 'fullname' in - # sys.modules , the loader must use that existing module. - # Otherwise, the reload() builtin will not work correctly. - return sys.modules[fullname] - except KeyError: - pass - if fullname != 'cubes': - raise ImportError('No module named {0}'.format(fullname)) - mod = sys.modules[fullname] = types.ModuleType(fullname, doc='CubicWeb cubes') - return mod + return _CubesLoader(modname, *modinfo) diff -r e0d708fb20e8 -r f85ec84355db cubicweb/test/unittest_cubes.py --- a/cubicweb/test/unittest_cubes.py Wed Feb 08 10:31:26 2017 +0100 +++ b/cubicweb/test/unittest_cubes.py Tue Feb 07 13:47:03 2017 +0100 @@ -21,13 +21,12 @@ import os from os import path import sys -import unittest from six import PY2 from cubicweb import _CubesImporter from cubicweb.cwconfig import CubicWebConfiguration -from cubicweb.devtools.testlib import TemporaryDirectory +from cubicweb.devtools.testlib import TemporaryDirectory, TestCase @contextmanager @@ -37,17 +36,20 @@ libdir = path.join(tempdir, 'libpython') cubedir = path.join(libdir, 'cubicweb_foo') os.makedirs(cubedir) + check_code = ("import logging\n" + "logging.getLogger('cubicweb_foo')" + ".warn('imported %s', __name__)\n") with open(path.join(cubedir, '__init__.py'), 'w') as f: - f.write('"""cubicweb_foo application package"""') + f.write("'cubicweb_foo application package'\n" + check_code) with open(path.join(cubedir, 'bar.py'), 'w') as f: - f.write('baz = 1') + f.write(check_code + 'baz = 1\n') sys.path.append(libdir) yield cubedir finally: sys.path.remove(libdir) -class CubesImporterTC(unittest.TestCase): +class CubesImporterTC(TestCase): def setUp(self): # During discovery, CubicWebConfiguration.cls_adjust_sys_path may be @@ -104,6 +106,27 @@ new = importlib.reload(cubes) self.assertIs(new, cubes) + def test_no_double_import(self): + """Check new and legacy import the same module once""" + with temp_cube(): + CubicWebConfiguration.cls_adjust_sys_path() + with self.assertLogs('cubicweb_foo', 'WARNING') as cm: + from cubes.foo import bar + from cubicweb_foo import bar as bar2 + self.assertIs(bar, bar2) + self.assertIs(sys.modules['cubes.foo'], + sys.modules['cubicweb_foo']) + self.assertEqual(cm.output, [ + 'WARNING:cubicweb_foo:imported cubicweb_foo', + # module __name__ for subpackage differ along python version + # for PY2 it's based on how the module was imported "from + # cubes.foo import bar" and for PY3 based on __name__ of parent + # module "cubicweb_foo". Not sure if it's an issue, but PY3 + # behavior looks better. + 'WARNING:cubicweb_foo:imported ' + ( + 'cubes.foo.bar' if PY2 else 'cubicweb_foo.bar') + ]) + def test_import_legacy_cube(self): """Check that importing a legacy cube works when sys.path got adjusted. """