Rev 5134: Stricter rules for loading plugins from BZR_PLUGINS_AT in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Apr 6 09:40:58 BST 2010


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5134 [merge]
revision-id: pqm at pqm.ubuntu.com-20100406084057-8bjovmj2hjaee1bb
parent: pqm at pqm.ubuntu.com-20100406065903-y9dxgwmog1pmw7dz
parent: v.ladeuil+lp at free.fr-20100406072626-hzjn0cla4xpx5kr0
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2010-04-06 09:40:57 +0100
message:
  Stricter rules for loading plugins from BZR_PLUGINS_AT
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/plugin.py               plugin.py-20050622060424-829b654519533d69
  bzrlib/tests/test_plugins.py   plugins.py-20050622075746-32002b55e5e943e9
=== modified file 'NEWS'
--- a/NEWS	2010-04-06 06:59:03 +0000
+++ b/NEWS	2010-04-06 07:26:26 +0000
@@ -194,6 +194,10 @@
 * Fix ``log`` to better check ancestors even if merged revisions are involved.
   (Vincent Ladeuil, #476293)
 
+* Loading a plugin from a given path with ``BZR_PLUGINS_AT`` doesn't depend
+  on os.lisdir() order and is now reliable.
+  (Vincent Ladeuil, #552922).
+
 * Many IO operations that returned ``EINTR`` were retried even if it
   wasn't safe to do so via careless use of ``until_no_eintr``.  Bazaar now
   only retries operations that are safe to retry, and in some cases has

=== modified file 'bzrlib/plugin.py'
--- a/bzrlib/plugin.py	2010-03-24 13:57:35 +0000
+++ b/bzrlib/plugin.py	2010-04-06 07:22:31 +0000
@@ -303,7 +303,7 @@
 
 
 def _load_plugin_module(name, dir):
-    """Load plugine name from dir.
+    """Load plugin name from dir.
 
     :param name: The plugin name in the bzrlib.plugins namespace.
     :param dir: The directory the plugin is loaded from for error messages.
@@ -560,32 +560,34 @@
     def load_module(self, fullname):
         """Load a plugin from a specific directory."""
         # We are called only for specific paths
-        plugin_dir = self.specific_paths[fullname]
-        candidate = None
-        maybe_package = False
-        for p in os.listdir(plugin_dir):
-            if os.path.isdir(osutils.pathjoin(plugin_dir, p)):
-                # We're searching for files only and don't want submodules to
-                # be recognized as plugins (they are submodules inside the
-                # plugin).
-                continue
-            name, path, (
-                suffix, mode, kind) = _find_plugin_module(plugin_dir, p)
-            if name is not None:
-                candidate = (name, path, suffix, mode, kind)
-                if kind == imp.PY_SOURCE:
-                    # We favour imp.PY_SOURCE (which will use the compiled
-                    # version if available) over imp.PY_COMPILED (which is used
-                    # only if the source is not available)
-                    break
-        if candidate is None:
+        plugin_path = self.specific_paths[fullname]
+        loading_path = None
+        package = False
+        if os.path.isdir(plugin_path):
+            for suffix, mode, kind in imp.get_suffixes():
+                if kind not in (imp.PY_SOURCE, imp.PY_COMPILED):
+                    # We don't recognize compiled modules (.so, .dll, etc)
+                    continue
+                init_path = osutils.pathjoin(plugin_path, '__init__' + suffix)
+                if os.path.isfile(init_path):
+                    loading_path = init_path
+                    package = True
+                    break
+        else:
+            for suffix, mode, kind in imp.get_suffixes():
+                if plugin_path.endswith(suffix):
+                    loading_path = plugin_path
+                    break
+        if loading_path is None:
             raise ImportError('%s cannot be loaded from %s'
-                              % (fullname, plugin_dir))
-        f = open(path, mode)
+                              % (fullname, plugin_path))
+        f = open(loading_path, mode)
         try:
-            mod = imp.load_module(fullname, f, path, (suffix, mode, kind))
-            # The plugin can contain modules, so be ready
-            mod.__path__ = [plugin_dir]
+            mod = imp.load_module(fullname, f, loading_path,
+                                  (suffix, mode, kind))
+            if package:
+                # The plugin can contain modules, so be ready
+                mod.__path__ = [plugin_path]
             mod.__package__ = fullname
             return mod
         finally:

=== modified file 'bzrlib/tests/test_plugins.py'
--- a/bzrlib/tests/test_plugins.py	2010-03-24 11:55:49 +0000
+++ b/bzrlib/tests/test_plugins.py	2010-04-06 07:22:31 +0000
@@ -810,21 +810,23 @@
         self.overrideAttr(plugin, '_loaded', False)
         # Create the same plugin in two directories
         self.create_plugin_package('test_foo', dir='non-standard-dir')
-        self.create_plugin_package('test_foo', dir='b/test_foo')
+        # The "normal" directory, we use 'standard' instead of 'plugins' to
+        # avoid depending on the precise naming.
+        self.create_plugin_package('test_foo', dir='standard/test_foo')
 
-    def assertTestFooLoadedFrom(self, dir):
+    def assertTestFooLoadedFrom(self, path):
         self.assertPluginKnown('test_foo')
         self.assertEqual('This is the doc for test_foo',
                          bzrlib.plugins.test_foo.__doc__)
-        self.assertEqual(dir, bzrlib.plugins.test_foo.dir_source)
+        self.assertEqual(path, bzrlib.plugins.test_foo.dir_source)
 
     def test_regular_load(self):
-        plugin.load_plugins(['b'])
-        self.assertTestFooLoadedFrom('b/test_foo')
+        plugin.load_plugins(['standard'])
+        self.assertTestFooLoadedFrom('standard/test_foo')
 
     def test_import(self):
         osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo at non-standard-dir')
-        plugin.set_plugins_path(['b'])
+        plugin.set_plugins_path(['standard'])
         try:
             import bzrlib.plugins.test_foo
         except ImportError:
@@ -833,12 +835,12 @@
 
     def test_loading(self):
         osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo at non-standard-dir')
-        plugin.load_plugins(['b'])
+        plugin.load_plugins(['standard'])
         self.assertTestFooLoadedFrom('non-standard-dir')
 
     def test_compiled_loaded(self):
         osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo at non-standard-dir')
-        plugin.load_plugins(['b'])
+        plugin.load_plugins(['standard'])
         self.assertTestFooLoadedFrom('non-standard-dir')
         self.assertEqual('non-standard-dir/__init__.py',
                          bzrlib.plugins.test_foo.__file__)
@@ -846,7 +848,7 @@
         # Try importing again now that the source has been compiled
         self._unregister_plugin('test_foo')
         plugin._loaded = False
-        plugin.load_plugins(['b'])
+        plugin.load_plugins(['standard'])
         self.assertTestFooLoadedFrom('non-standard-dir')
         if __debug__:
             suffix = 'pyc'
@@ -859,10 +861,35 @@
         # We create an additional directory under the one for test_foo
         self.create_plugin_package('test_bar', dir='non-standard-dir/test_bar')
         osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo at non-standard-dir')
-        plugin.set_plugins_path(['b'])
+        plugin.set_plugins_path(['standard'])
         import bzrlib.plugins.test_foo
         self.assertEqual('bzrlib.plugins.test_foo',
                          bzrlib.plugins.test_foo.__package__)
         import bzrlib.plugins.test_foo.test_bar
         self.assertEqual('non-standard-dir/test_bar/__init__.py',
                          bzrlib.plugins.test_foo.test_bar.__file__)
+
+    def test_loading_from___init__only(self):
+        # We rename the existing __init__.py file to ensure that we don't load
+        # a random file
+        init = 'non-standard-dir/__init__.py'
+        random = 'non-standard-dir/setup.py'
+        os.rename(init, random)
+        self.addCleanup(os.rename, random, init)
+        osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo at non-standard-dir')
+        plugin.load_plugins(['standard'])
+        self.assertPluginUnknown('test_foo')
+
+    def test_loading_from_specific_file(self):
+        plugin_dir = 'non-standard-dir'
+        plugin_file_name = 'iamtestfoo.py'
+        plugin_path = osutils.pathjoin(plugin_dir, plugin_file_name)
+        source = '''\
+"""This is the doc for %s"""
+dir_source = '%s'
+''' % ('test_foo', plugin_path)
+        self.create_plugin('test_foo', source=source,
+                           dir=plugin_dir, file_name=plugin_file_name)
+        osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo@%s' % plugin_path)
+        plugin.load_plugins(['standard'])
+        self.assertTestFooLoadedFrom(plugin_path)




More information about the bazaar-commits mailing list