Rev 2258: Allow 'import bzrlib.plugins.NAME' to work when the plugin NAME has not in file:///home/robertc/source/baz/plugin-hook/

Robert Collins robertc at robertcollins.net
Fri Feb 2 16:18:23 GMT 2007


------------------------------------------------------------
revno: 2258
revision-id: robertc at robertcollins.net-20070202161755-zy2d68928wqwehj6
parent: robertc at robertcollins.net-20070202104806-2lfxhf4zsocxuc43
committer: Robert Collins <robertc at robertcollins.net>
branch nick: plugin-hook
timestamp: Sat 2007-02-03 03:17:55 +1100
message:
  Allow 'import bzrlib.plugins.NAME' to work when the plugin NAME has not
  yet been loaded by load_plugins(). This allows plugins to depend on each
  other for code reuse without requiring users to perform file-renaming
  gymnastics. (Robert Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/__init__.py             __init__.py-20050309040759-33e65acf91bbcd5d
  bzrlib/plugin.py               plugin.py-20050622060424-829b654519533d69
  bzrlib/tests/test_plugins.py   plugins.py-20050622075746-32002b55e5e943e9
=== modified file 'NEWS'
--- a/NEWS	2007-02-01 21:29:19 +0000
+++ b/NEWS	2007-02-02 16:17:55 +0000
@@ -34,6 +34,11 @@
       branch as it makes performance and policy decisions to match the UI
       level command ``push``. (Robert Collins).
 
+    * Allow 'import bzrlib.plugins.NAME' to work when the plugin NAME has not
+      yet been loaded by load_plugins(). This allows plugins to depend on each
+      other for code reuse without requiring users to perform file-renaming
+      gymnastics. (Robert Collins)
+
   BUGFIXES:
 
     * ``bzr annotate`` now uses dotted revnos from the viewpoint of the

=== modified file 'bzrlib/__init__.py'
--- a/bzrlib/__init__.py	2007-01-24 19:42:26 +0000
+++ b/bzrlib/__init__.py	2007-02-02 16:17:55 +0000
@@ -54,6 +54,11 @@
                     'Consider using bzrlib.ignores.add_unique_user_ignores'
                     ' or bzrlib.ignores.add_runtime_ignores')
 
+# allow bzrlib plugins to be imported.
+import bzrlib.plugin
+bzrlib.plugin.set_plugins_path()
+
+
 def test_suite():
     import tests
     return tests.test_suite()

=== modified file 'bzrlib/plugin.py'
--- a/bzrlib/plugin.py	2007-01-03 00:23:45 +0000
+++ b/bzrlib/plugin.py	2007-02-02 16:17:55 +0000
@@ -15,30 +15,20 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 
-"""bzr python plugin support
-
-Any python module in $BZR_PLUGIN_PATH will be imported upon initialization of
-bzrlib. The module will be imported as 'bzrlib.plugins.$BASENAME(PLUGIN)'.
-In the plugin's main body, it should update any bzrlib registries it wants to
-extend; for example, to add new commands, import bzrlib.commands and add your
-new command to the plugin_cmds variable.
+"""bzr python plugin support.
+
+When load_plugins() is invoked, any python module in any directory in
+$BZR_PLUGIN_PATH will be imported.  The module will be imported as
+'bzrlib.plugins.$BASENAME(PLUGIN)'.  In the plugin's main body, it should
+update any bzrlib registries it wants to extend; for example, to add new
+commands, import bzrlib.commands and add your new command to the plugin_cmds
+variable.
+
+BZR_PLUGIN_PATH is also honoured for any plugins imported via
+'import bzrlib.plugins.PLUGINNAME', as long as set_plugins_path has been 
+called.
 """
 
-# TODO: Refactor this to make it more testable.  The main problem at the
-# moment is that loading plugins affects the global process state -- for bzr
-# in general use it's a reasonable assumption that all plugins are loaded at
-# startup and then stay loaded, but this is less good for testing.
-# 
-# Several specific issues:
-#  - plugins can't be unloaded and will continue to effect later tests
-#  - load_plugins does nothing if called a second time
-#  - plugin hooks can't be removed
-#
-# Our options are either to remove these restrictions, or work around them by
-# loading the plugins into a different space than the one running the tests.
-# That could be either a separate Python interpreter or perhaps a new
-# namespace inside this interpreter.
-
 import os
 import sys
 
@@ -46,6 +36,7 @@
 lazy_import(globals(), """
 import imp
 import types
+import zipimport
 
 from bzrlib import (
     config,
@@ -58,7 +49,7 @@
 
 
 DEFAULT_PLUGIN_PATH = None
-
+_loaded = False
 
 def get_default_plugin_path():
     """Get the DEFAULT_PLUGIN_PATH"""
@@ -68,9 +59,6 @@
     return DEFAULT_PLUGIN_PATH
 
 
-_loaded = False
-
-
 def all_plugins():
     """Return a dictionary of the plugins."""
     result = {}
@@ -91,6 +79,16 @@
     _loaded = True
 
 
+def set_plugins_path():
+    """Set the path for plugins to be loaded from."""
+    path = os.environ.get('BZR_PLUGIN_PATH',
+                          get_default_plugin_path()).split(os.pathsep)
+    # search the bzrlib installed dir before anything else.
+    path.insert(0, os.path.dirname(plugins.__file__))
+    plugins.__path__ = path
+    return path
+
+
 def load_plugins():
     """Load bzrlib plugins.
 
@@ -108,15 +106,11 @@
         return
     _loaded = True
 
-    dirs = os.environ.get('BZR_PLUGIN_PATH',
-                          get_default_plugin_path()).split(os.pathsep)
-    dirs.insert(0, os.path.dirname(plugins.__file__))
-
-    load_from_dirs(dirs)
-    load_from_zips(dirs)
-
-
-def load_from_dirs(dirs):
+    # scan for all plugins in the path.
+    load_from_path(set_plugins_path())
+
+
+def load_from_path(dirs):
     """Load bzrlib plugins found in each dir in dirs.
 
     Loading a plugin means importing it into the python interpreter.
@@ -125,145 +119,146 @@
 
     Plugins are loaded into bzrlib.plugins.NAME, and can be found there
     for future reference.
+
+    The python module path for bzrlib.plugins will be modified to be 'dirs'.
     """
+    plugins.__path__ = dirs
+    for d in dirs:
+        if not d:
+            continue
+        mutter('looking for plugins in %s', d)
+        if os.path.isdir(d):
+            load_from_dir(d)
+        else:
+            # it might be a zip: try loading from the zip.
+            load_from_zip(d)
+            continue
+
+
+# backwards compatability: load_from_dirs was the old name
+# This was changed in 0.15
+load_from_dirs = load_from_path
+
+
+def load_from_dir(d):
+    """Load the plugins in directory d."""
     # Get the list of valid python suffixes for __init__.py?
     # this includes .py, .pyc, and .pyo (depending on if we are running -O)
     # but it doesn't include compiled modules (.so, .dll, etc)
     valid_suffixes = [suffix for suffix, mod_type, flags in imp.get_suffixes()
                               if flags in (imp.PY_SOURCE, imp.PY_COMPILED)]
     package_entries = ['__init__'+suffix for suffix in valid_suffixes]
-    for d in dirs:
-        if not d:
-            continue
-        mutter('looking for plugins in %s', d)
-        plugin_names = set()
-        if not os.path.isdir(d):
-            continue
-        for f in os.listdir(d):
-            path = osutils.pathjoin(d, f)
-            if os.path.isdir(path):
-                for entry in package_entries:
-                    # This directory should be a package, and thus added to
-                    # the list
-                    if os.path.isfile(osutils.pathjoin(path, entry)):
-                        break
-                else: # This directory is not a package
-                    continue
-            else:
-                for suffix_info in imp.get_suffixes():
-                    if f.endswith(suffix_info[0]):
-                        f = f[:-len(suffix_info[0])]
-                        if suffix_info[2] == imp.C_EXTENSION and f.endswith('module'):
-                            f = f[:-len('module')]
-                        break
-                else:
-                    continue
-            if getattr(plugins, f, None):
-                mutter('Plugin name %s already loaded', f)
-            else:
-                # mutter('add plugin name %s', f)
-                plugin_names.add(f)
-
-        plugin_names = list(plugin_names)
-        plugin_names.sort()
-        for name in plugin_names:
-            try:
-                plugin_info = imp.find_module(name, [d])
-                # mutter('load plugin %r', plugin_info)
-                try:
-                    plugin = imp.load_module('bzrlib.plugins.' + name,
-                                             *plugin_info)
-                    setattr(plugins, name, plugin)
-                finally:
-                    if plugin_info[0] is not None:
-                        plugin_info[0].close()
-                # mutter('loaded succesfully')
-            except KeyboardInterrupt:
-                raise
-            except Exception, e:
-                ## import pdb; pdb.set_trace()
-                warning('Unable to load plugin %r from %r' % (name, d))
-                log_exception_quietly()
-
-
-def load_from_zips(zips):
-    """Load bzr plugins from zip archives with zipimport.
-    It's similar to load_from_dirs but plugins searched inside archives.
-    """
-    import zipfile
-    import zipimport
-
+    plugin_names = set()
+    for f in os.listdir(d):
+        path = osutils.pathjoin(d, f)
+        if os.path.isdir(path):
+            for entry in package_entries:
+                # This directory should be a package, and thus added to
+                # the list
+                if os.path.isfile(osutils.pathjoin(path, entry)):
+                    break
+            else: # This directory is not a package
+                continue
+        else:
+            for suffix_info in imp.get_suffixes():
+                if f.endswith(suffix_info[0]):
+                    f = f[:-len(suffix_info[0])]
+                    if suffix_info[2] == imp.C_EXTENSION and f.endswith('module'):
+                        f = f[:-len('module')]
+                    break
+            else:
+                continue
+        if getattr(plugins, f, None):
+            mutter('Plugin name %s already loaded', f)
+        else:
+            # mutter('add plugin name %s', f)
+            plugin_names.add(f)
+    
+    for name in plugin_names:
+        try:
+            exec "import bzrlib.plugins.%s" % name
+        except KeyboardInterrupt:
+            raise
+        except Exception, e:
+            ## import pdb; pdb.set_trace()
+            warning('Unable to load plugin %r from %r' % (name, d))
+            log_exception_quietly()
+
+
+def load_from_zip(zip_name):
+    """Load all the plugins in a zip."""
     valid_suffixes = ('.py', '.pyc', '.pyo')    # only python modules/packages
                                                 # is allowed
-    for zip_name in zips:
-        if '.zip' not in zip_name:
-            continue
+    if '.zip' not in zip_name:
+        return
+    try:
+        ziobj = zipimport.zipimporter(zip_name)
+    except zipimport.ZipImportError:
+        # not a valid zip
+        return
+    mutter('Looking for plugins in %r', zip_name)
+    
+    import zipfile
+
+    # use zipfile to get list of files/dirs inside zip
+    z = zipfile.ZipFile(ziobj.archive)
+    namelist = z.namelist()
+    z.close()
+    
+    if ziobj.prefix:
+        prefix = ziobj.prefix.replace('\\','/')
+        ix = len(prefix)
+        namelist = [name[ix:]
+                    for name in namelist
+                    if name.startswith(prefix)]
+    
+    mutter('Names in archive: %r', namelist)
+    
+    for name in namelist:
+        if not name or name.endswith('/'):
+            continue
+    
+        # '/' is used to separate pathname components inside zip archives
+        ix = name.rfind('/')
+        if ix == -1:
+            head, tail = '', name
+        else:
+            head, tail = name.rsplit('/',1)
+        if '/' in head:
+            # we don't need looking in subdirectories
+            continue
+    
+        base, suffix = osutils.splitext(tail)
+        if suffix not in valid_suffixes:
+            continue
+    
+        if base == '__init__':
+            # package
+            plugin_name = head
+        elif head == '':
+            # module
+            plugin_name = base
+        else:
+            continue
+    
+        if not plugin_name:
+            continue
+        if getattr(plugins, plugin_name, None):
+            mutter('Plugin name %s already loaded', plugin_name)
+            continue
+    
         try:
-            ziobj = zipimport.zipimporter(zip_name)
-        except zipimport.ZipImportError:
-            # not a valid zip
+            plugin = ziobj.load_module(plugin_name)
+            setattr(plugins, plugin_name, plugin)
+            mutter('Load plugin %s from zip %r', plugin_name, zip_name)
+        except zipimport.ZipImportError, e:
+            mutter('Unable to load plugin %r from %r: %s',
+                   plugin_name, zip_name, str(e))
             continue
-        mutter('Looking for plugins in %r', zip_name)
-
-        # use zipfile to get list of files/dirs inside zip
-        z = zipfile.ZipFile(ziobj.archive)
-        namelist = z.namelist()
-        z.close()
-
-        if ziobj.prefix:
-            prefix = ziobj.prefix.replace('\\','/')
-            ix = len(prefix)
-            namelist = [name[ix:]
-                        for name in namelist
-                        if name.startswith(prefix)]
-
-        mutter('Names in archive: %r', namelist)
-
-        for name in namelist:
-            if not name or name.endswith('/'):
-                continue
-
-            # '/' is used to separate pathname components inside zip archives
-            ix = name.rfind('/')
-            if ix == -1:
-                head, tail = '', name
-            else:
-                head, tail = name.rsplit('/',1)
-            if '/' in head:
-                # we don't need looking in subdirectories
-                continue
-
-            base, suffix = osutils.splitext(tail)
-            if suffix not in valid_suffixes:
-                continue
-
-            if base == '__init__':
-                # package
-                plugin_name = head
-            elif head == '':
-                # module
-                plugin_name = base
-            else:
-                continue
-
-            if not plugin_name:
-                continue
-            if getattr(plugins, plugin_name, None):
-                mutter('Plugin name %s already loaded', plugin_name)
-                continue
-
-            try:
-                plugin = ziobj.load_module(plugin_name)
-                setattr(plugins, plugin_name, plugin)
-                mutter('Load plugin %s from zip %r', plugin_name, zip_name)
-            except zipimport.ZipImportError, e:
-                mutter('Unable to load plugin %r from %r: %s',
-                       plugin_name, zip_name, str(e))
-                continue
-            except KeyboardInterrupt:
-                raise
-            except Exception, e:
-                ## import pdb; pdb.set_trace()
-                warning('Unable to load plugin %r from %r'
-                        % (name, zip_name))
-                log_exception_quietly()
+        except KeyboardInterrupt:
+            raise
+        except Exception, e:
+            ## import pdb; pdb.set_trace()
+            warning('Unable to load plugin %r from %r'
+                    % (name, zip_name))
+            log_exception_quietly()

=== modified file 'bzrlib/tests/test_plugins.py'
--- a/bzrlib/tests/test_plugins.py	2007-02-02 10:48:06 +0000
+++ b/bzrlib/tests/test_plugins.py	2007-02-02 16:17:55 +0000
@@ -22,13 +22,14 @@
 
 import os
 from StringIO import StringIO
+import sys
 import zipfile
 
 import bzrlib.plugin
 import bzrlib.plugins
 import bzrlib.commands
 import bzrlib.help
-from bzrlib.tests import TestCaseInTempDir
+from bzrlib.tests import TestCase, TestCaseInTempDir
 from bzrlib.osutils import pathjoin, abspath
 
 
@@ -43,21 +44,21 @@
 
 # TODO: Write a test for plugin decoration of commands.
 
-class TestOneNamedPluginOnly(TestCaseInTempDir):
+class TestLoadingPlugins(TestCaseInTempDir):
 
     activeattributes = {}
 
     def test_plugins_with_the_same_name_are_not_loaded(self):
-        # This test tests that having two plugins in different
-        # directories does not result in both being loaded.
-        # get a file name we can use which is also a valid attribute
-        # for accessing in activeattributes. - we cannot give import parameters.
+        # This test tests that having two plugins in different directories does
+        # not result in both being loaded when they have the same name.  get a
+        # file name we can use which is also a valid attribute for accessing in
+        # activeattributes. - we cannot give import parameters.
         tempattribute = "0"
         self.failIf(tempattribute in self.activeattributes)
         # set a place for the plugins to record their loading, and at the same
         # time validate that the location the plugins should record to is
         # valid and correct.
-        bzrlib.tests.test_plugins.TestOneNamedPluginOnly.activeattributes \
+        bzrlib.tests.test_plugins.TestLoadingPlugins.activeattributes \
             [tempattribute] = []
         self.failUnless(tempattribute in self.activeattributes)
         # create two plugin directories
@@ -65,13 +66,51 @@
         os.mkdir('second')
         # write a plugin that will record when its loaded in the 
         # tempattribute list.
-        template = ("from bzrlib.tests.test_plugins import TestOneNamedPluginOnly\n"
-                    "TestOneNamedPluginOnly.activeattributes[%r].append('%s')\n")
+        template = ("from bzrlib.tests.test_plugins import TestLoadingPlugins\n"
+                    "TestLoadingPlugins.activeattributes[%r].append('%s')\n")
         print >> file(os.path.join('first', 'plugin.py'), 'w'), template % (tempattribute, 'first')
         print >> file(os.path.join('second', 'plugin.py'), 'w'), template % (tempattribute, 'second')
         try:
-            bzrlib.plugin.load_from_dirs(['first', 'second'])
-            self.assertEqual(['first'], self.activeattributes[tempattribute])
+            bzrlib.plugin.load_from_path(['first', 'second'])
+            self.assertEqual(['first'], self.activeattributes[tempattribute])
+        finally:
+            # remove the plugin 'plugin'
+            del self.activeattributes[tempattribute]
+            if getattr(bzrlib.plugins, 'plugin', None):
+                del bzrlib.plugins.plugin
+        self.failIf(getattr(bzrlib.plugins, 'plugin', None))
+
+    def test_plugins_from_different_dirs_can_demand_load(self):
+        # This test tests that having two plugins in different
+        # directories with different names allows them both to be loaded, when
+        # we do a direct import statement.
+        # Determine a file name we can use which is also a valid attribute
+        # for accessing in activeattributes. - we cannot give import parameters.
+        tempattribute = "different-dirs"
+        self.failIf(tempattribute in self.activeattributes)
+        # set a place for the plugins to record their loading, and at the same
+        # time validate that the location the plugins should record to is
+        # valid and correct.
+        bzrlib.tests.test_plugins.TestLoadingPlugins.activeattributes \
+            [tempattribute] = []
+        self.failUnless(tempattribute in self.activeattributes)
+        # create two plugin directories
+        os.mkdir('first')
+        os.mkdir('second')
+        # write plugins that will record when they are loaded in the 
+        # tempattribute list.
+        template = ("from bzrlib.tests.test_plugins import TestLoadingPlugins\n"
+                    "TestLoadingPlugins.activeattributes[%r].append('%s')\n")
+        print >> file(os.path.join('first', 'pluginone.py'), 'w'), template % (tempattribute, 'first')
+        print >> file(os.path.join('second', 'plugintwo.py'), 'w'), template % (tempattribute, 'second')
+        oldpath = bzrlib.plugins.__path__
+        try:
+            bzrlib.plugins.__path__ = ['first', 'second']
+            exec "import bzrlib.plugins.pluginone"
+            self.assertEqual(['first'], self.activeattributes[tempattribute])
+            exec "import bzrlib.plugins.plugintwo"
+            self.assertEqual(['first', 'second'],
+                self.activeattributes[tempattribute])
         finally:
             # remove the plugin 'plugin'
             del self.activeattributes[tempattribute]
@@ -89,13 +128,15 @@
         # write a plugin that _cannot_ fail to load.
         print >> file('plugin.py', 'w'), ""
         try:
-            bzrlib.plugin.load_from_dirs(['.'])
+            bzrlib.plugin.load_from_path(['.'])
             self.failUnless('plugin' in bzrlib.plugin.all_plugins())
             self.failUnless(getattr(bzrlib.plugins, 'plugin', None))
             self.assertEqual(bzrlib.plugin.all_plugins()['plugin'],
                              bzrlib.plugins.plugin)
         finally:
             # remove the plugin 'plugin'
+            if 'bzrlib.plugins.plugin' in sys.modules:
+                del sys.modules['bzrlib.plugins.plugin']
             if getattr(bzrlib.plugins, 'plugin', None):
                 del bzrlib.plugins.plugin
         self.failIf(getattr(bzrlib.plugins, 'plugin', None))
@@ -143,7 +184,7 @@
 
         try:
             # Check its help
-            bzrlib.plugin.load_from_dirs(['plugin_test'])
+            bzrlib.plugin.load_from_path(['plugin_test'])
             bzrlib.commands.register_command( bzrlib.plugins.myplug.cmd_myplug)
             help = self.capture('help myplug')
             self.assertContainsRe(help, 'From plugin "myplug"')
@@ -169,7 +210,7 @@
         self.assertFalse(plugin_name in dir(bzrlib.plugins),
                          'Plugin already loaded')
         try:
-            bzrlib.plugin.load_from_zips([zip_name])
+            bzrlib.plugin.load_from_zip(zip_name)
             self.assertTrue(plugin_name in dir(bzrlib.plugins),
                             'Plugin is not loaded')
         finally:
@@ -184,3 +225,16 @@
     def test_load_package(self):
         self.make_zipped_plugin('./test.zip', 'ziplug/__init__.py')
         self.check_plugin_load('./test.zip', 'ziplug')
+
+
+class TestSetPluginsPath(TestCase):
+    
+    def test_set_plugins_path(self):
+        """set_plugins_path should set the module __path__ correctly."""
+        old_path = bzrlib.plugins.__path__
+        try:
+            bzrlib.plugins.__path__ = []
+            expected_path = bzrlib.plugin.set_plugins_path()
+            self.assertEqual(expected_path, bzrlib.plugins.__path__)
+        finally:
+            bzrlib.plugins.__path__ = old_path




More information about the bazaar-commits mailing list