Changeset 20920

Timestamp:

08/02/2007 08:12:35 AM (3 years ago)

Author:

exarkun

Message:

Merge plugin-collision-2339

Author: exarkun
Reviewer: therve
Fixes #2339

Add a function, twisted.plugin.pluginPackagePaths, which returns a list of
additional directories to search for plugin modules for a particular plugin
package. Change twisted.plugins.init to use this function instead of
looping over sys.path directly.

Unlike the loop previously in twisted.plugins.init, pluginPackagePaths
will not include any directory which is a Python package. This prevents
plugins from being discovered from packages which Python may not actually
be able to import and limits plugin discovery to those installed on the
version of the package which is in use by an application. It also allows
a site installation to co-exist with another installation (for example,
a personal user installation) without interfering with each other.

Location:

trunk/twisted

Files:

• plugin.py (modified) (4 diffs)
• plugins/__init__.py (modified) (2 diffs)
• test/test_plugin.py (modified) (10 diffs)

trunk/twisted/plugin.py

@@ -1 +1 @@
 # -*- test-case-name: twisted.test.test_plugin -*-
 # Copyright (c) 2005 Divmod, Inc.
+# Copyright (c) 2007 Twisted Matrix Laboratories.
 # See LICENSE for details.
 
@@ -10 +11 @@
 """
 
-from __future__ import generators
+import os
+import sys
 
 from zope.interface import Interface, providedBy
@@ -80 +82 @@
     getComponent = __conform__
 
+
+
 class CachedDropin(object):
+    """
+    A collection of L{CachedPlugin} instances from a particular module in a
+    plugin package.
+
+    @type moduleName: C{str}
+    @ivar moduleName: The fully qualified name of the plugin module this
+        represents.
+
+    @type description: C{str} or C{NoneType}
+    @ivar description: A brief explanation of this collection of plugins
+        (probably the plugin module's docstring).
+
+    @type plugins: C{list}
+    @ivar plugins: The L{CachedPlugin} instances which were loaded from this
+        dropin.
+    """
     def __init__(self, moduleName, description):
         self.moduleName = moduleName
         self.description = description
         self.plugins = []
+
+
 
 def _generateCacheEntry(provider):
@@ -200 +222 @@
 
 
-__all__ = ['getPlugins']
+def pluginPackagePaths(name):
+    """
+    Return a list of additional directories which should be searched for
+    modules to be included as part of the named plugin package.
+
+    @type name: C{str}
+    @param name: The fully-qualified Python name of a plugin package, eg
+        C{'twisted.plugins'}.
+
+    @rtype: C{list} of C{str}
+    @return: The absolute paths to other directories which may contain plugin
+        modules for the named plugin package.
+    """
+    package = name.split('.')
+    # Note that this may include directories which do not exist.  It may be
+    # preferable to remove such directories at this point, rather than allow
+    # them to be searched later on.
+    #
+    # Note as well that only '__init__.py' will be considered to make a
+    # directory a package (and thus exclude it from this list).  This means
+    # that if you create a master plugin package which has some other kind of
+    # __init__ (eg, __init__.pyc) it will be incorrectly treated as a
+    # supplementary plugin directory.
+    return [
+        os.path.abspath(os.path.join(x, *package))
+        for x
+        in sys.path
+        if
+        not os.path.exists(os.path.join(x, *package + ['__init__.py']))]
+
+__all__ = ['getPlugins', 'pluginPackagePaths']

trunk/twisted/plugins/__init__.py

@@ -1 +1 @@
 # -*- test-case-name: twisted.test.test_plugin -*-
 # Copyright (c) 2005 Divmod, Inc.
+# Copyright (c) 2007 Twisted Matrix Laboratories.
 # See LICENSE for details.
 
@@ -12 +13 @@
 """
 
-import os, sys
-__path__ = [os.path.abspath(os.path.join(x, 'twisted', 'plugins')) for x in sys.path]
-
+from twisted.plugin import pluginPackagePaths
+__path__.extend(pluginPackagePaths(__name__))
 __all__ = []                    # nothing to see here, move along, move along

trunk/twisted/test/test_plugin.py

@@ -1 +1 @@
 # Copyright (c) 2005 Divmod, Inc.
+# Copyright (c) 2007 Twisted Matrix Laboratories.
 # See LICENSE for details.
 
@@ -282 +283 @@
 # This is something like the Twisted plugins file.
 pluginInitFile = """
-import os, sys
-__path__ = [os.path.abspath(os.path.join(x, 'plugindummy', 'plugins')) for x in sys.path]
+from twisted.plugin import pluginPackagePaths
+__path__.extend(pluginPackagePaths(__name__))
 __all__ = []
 """
 
-systemPluginFile = """
-from zope.interface import classProvides
-from twisted.plugin import IPlugin, ITestPlugin
-
-class system(object):
-    classProvides(IPlugin, ITestPlugin)
-"""
-
-devPluginFile = """
-from zope.interface import classProvides
-from twisted.plugin import IPlugin, ITestPlugin
-
-class system(object):
-    classProvides(IPlugin, ITestPlugin)
-
-class dev(object):
-    classProvides(IPlugin, ITestPlugin)
-"""
-
-appPluginFile = """
-from zope.interface import classProvides
-from twisted.plugin import IPlugin, ITestPlugin
-
-class app(object):
-    classProvides(IPlugin, ITestPlugin)
-"""
-
-onePluginFile = """
-from zope.interface import classProvides
-from twisted.plugin import IPlugin, ITestPlugin
-
-class one(object):
-    classProvides(IPlugin, ITestPlugin)
-"""
-
-twoPluginFile = """
-from zope.interface import classProvides
-from twisted.plugin import IPlugin, ITestPlugin
-
-class two(object):
-    classProvides(IPlugin, ITestPlugin)
-"""
-
-def _createPluginDummy(entrypath, pluginContent, real=True):
+def pluginFileContents(name):
+    return (
+        "from zope.interface import classProvides\n"
+        "from twisted.plugin import IPlugin, ITestPlugin\n"
+        "\n"
+        "class %s(object):\n"
+        "    classProvides(IPlugin, ITestPlugin)\n") % (name,)
+
+
+def _createPluginDummy(entrypath, pluginContent, real, pluginModule):
     """
     Create a plugindummy package.
@@ -343 +310 @@
     if real:
         plugs.child('__init__.py').setContent(pluginInitFile)
-    if real:
-        n = 'plugindummy_builtin.py'
-    else:
-        n = 'plugindummy_app.py'
-    plugs.child(n).setContent(pluginContent)
+    plugs.child(pluginModule + '.py').setContent(pluginContent)
     return plugs
+
 
 
@@ -372 +336 @@
         self.devPath = self.fakeRoot.child('development_path')
         self.appPath = self.fakeRoot.child('application_path')
-        self.systemPackage = _createPluginDummy(self.systemPath,
-                                                systemPluginFile)
-        self.devPackage = _createPluginDummy(self.devPath, devPluginFile)
-        self.appPackage = _createPluginDummy(self.appPath, appPluginFile, False)
+        self.systemPackage = _createPluginDummy(
+            self.systemPath, pluginFileContents('system'),
+            True, 'plugindummy_builtin')
+        self.devPackage = _createPluginDummy(
+            self.devPath, pluginFileContents('dev'),
+            True, 'plugindummy_builtin')
+        self.appPackage = _createPluginDummy(
+            self.appPath, pluginFileContents('app'),
+            False, 'plugindummy_app')
 
         # Now we're going to do the system installation.
@@ -458 +427 @@
 
 
-    def test_newPluginsNotClobbered(self):
-        """
-        Verify that plugins added in the 'development' path are loadable, even when
-        the (now non-importable) system path contains its own idea of the list
-        of plugins for a package.
+    def test_developmentPluginAvailability(self):
+        """
+        Plugins added in the development path should be loadable, even when
+        the (now non-importable) system path contains its own idea of the
+        list of plugins for a package.  Inversely, plugins added in the
+        system path should not be available.
         """
         # Run 3 times: uncached, cached, and then cached again to make sure we
@@ -468 +438 @@
         for x in range(3):
             names = self.getAllPlugins()
-            self.assertEqual(len(names), 3)
-            self.assertIn('app', names)
-            self.assertIn('dev', names)
-            self.assertIn('system', names)
+            names.sort()
+            self.assertEqual(names, ['app', 'dev'])
 
 
@@ -481 +449 @@
         """
         mypath = self.appPackage.child("stale.py")
-        mypath.setContent(onePluginFile)
+        mypath.setContent(pluginFileContents('one'))
         # Make it super stale
         x = time.time() - 1000
@@ -497 +465 @@
         self.failIfIn('two', self.getAllPlugins())
         self.resetEnvironment()
-        mypath.setContent(twoPluginFile)
+        mypath.setContent(pluginFileContents('two'))
         self.failIfIn('one', self.getAllPlugins())
         self.assertIn('two', self.getAllPlugins())
@@ -512 +480 @@
         """
         self.unlockSystem()
-        self.sysplug.child('newstuff.py').setContent(onePluginFile)
+        self.sysplug.child('newstuff.py').setContent(pluginFileContents('one'))
         self.lockSystem()
+
+        # Take the developer path out, so that the system plugins are actually
+        # examined.
+        sys.path.remove(self.devPath.path)
+
         # Sanity check to make sure we're only flushing the error logged
         # below...
@@ -519 +492 @@
         self.assertIn('one', self.getAllPlugins())
         self.assertEqual(len(self.flushLoggedErrors()), 1)
+
+
+
+class AdjacentPackageTests(unittest.TestCase):
+    """
+    Tests for the behavior of the plugin system when there are multiple
+    installed copies of the package containing the plugins being loaded.
+    """
+    def setUp(self):
+        """
+        Save the elements of C{sys.path} and the items of C{sys.modules}.
+        """
+        self.originalPath = sys.path[:]
+        self.savedModules = sys.modules.copy()
+
+
+    def tearDown(self):
+        """
+        Restore C{sys.path} and C{sys.modules} to their original values.
+        """
+        sys.path[:] = self.originalPath
+        sys.modules.clear()
+        sys.modules.update(self.savedModules)
+
+
+    def createDummyPackage(self, root, name, pluginName):
+        """
+        Create a directory containing a Python package named I{dummy} with a
+        I{plugins} subpackage.
+
+        @type root: L{FilePath}
+        @param root: The directory in which to create the hierarchy.
+
+        @type name: C{str}
+        @param name: The name of the directory to create which will contain
+            the package.
+
+        @type pluginName: C{str}
+        @param pluginName: The name of a module to create in the
+            I{dummy.plugins} package.
+
+        @rtype: L{FilePath}
+        @return: The directory which was created to contain the I{dummy}
+            package.
+        """
+        directory = root.child(name)
+        package = directory.child('dummy')
+        package.makedirs()
+        package.child('__init__.py').setContent('')
+        plugins = package.child('plugins')
+        plugins.makedirs()
+        plugins.child('__init__.py').setContent(pluginInitFile)
+        pluginModule = plugins.child(pluginName + '.py')
+        pluginModule.setContent(pluginFileContents(name))
+        return directory
+
+
+    def test_hiddenPackageSamePluginModuleNameObscured(self):
+        """
+        Only plugins from the first package in sys.path should be returned by
+        getPlugins in the case where there are two Python packages by the same
+        name installed, each with a plugin module by a single name.
+        """
+        root = FilePath(self.mktemp())
+        root.makedirs()
+
+        firstDirectory = self.createDummyPackage(root, 'first', 'someplugin')
+        secondDirectory = self.createDummyPackage(root, 'second', 'someplugin')
+
+        sys.path.append(firstDirectory.path)
+        sys.path.append(secondDirectory.path)
+
+        import dummy.plugins
+
+        plugins = list(plugin.getPlugins(plugin.ITestPlugin, dummy.plugins))
+        self.assertEqual(['first'], [p.__name__ for p in plugins])
+
+
+    def test_hiddenPackageDifferentPluginModuleNameObscured(self):
+        """
+        Plugins from the first package in sys.path should be returned by
+        getPlugins in the case where there are two Python packages by the same
+        name installed, each with a plugin module by a different name.
+        """
+        root = FilePath(self.mktemp())
+        root.makedirs()
+
+        firstDirectory = self.createDummyPackage(root, 'first', 'thisplugin')
+        secondDirectory = self.createDummyPackage(root, 'second', 'thatplugin')
+
+        sys.path.append(firstDirectory.path)
+        sys.path.append(secondDirectory.path)
+
+        import dummy.plugins
+
+        plugins = list(plugin.getPlugins(plugin.ITestPlugin, dummy.plugins))
+        self.assertEqual(['first'], [p.__name__ for p in plugins])
+
+
+
+class PackagePathTests(unittest.TestCase):
+    """
+    Tests for L{plugin.pluginPackagePaths} which constructs search paths for
+    plugin packages.
+    """
+    def setUp(self):
+        """
+        Save the elements of C{sys.path}.
+        """
+        self.originalPath = sys.path[:]
+
+
+    def tearDown(self):
+        """
+        Restore C{sys.path} to its original value.
+        """
+        sys.path[:] = self.originalPath
+
+
+    def test_pluginDirectories(self):
+        """
+        L{plugin.pluginPackagePaths} should return a list containing each
+        directory in C{sys.path} with a suffix based on the supplied package
+        name.
+        """
+        foo = FilePath('foo')
+        bar = FilePath('bar')
+        sys.path = [foo.path, bar.path]
+        self.assertEqual(
+            plugin.pluginPackagePaths('dummy.plugins'),
+            [foo.child('dummy').child('plugins').path,
+             bar.child('dummy').child('plugins').path])
+
+
+    def test_pluginPackagesExcluded(self):
+        """
+        L{plugin.pluginPackagePaths} should exclude directories which are
+        Python packages.  The only allowed plugin package (the only one
+        associated with a I{dummy} package which Python will allow to be
+        imported) will already be known to the caller of
+        L{plugin.pluginPackagePaths} and will most commonly already be in
+        the C{__path__} they are about to mutate.
+        """
+        root = FilePath(self.mktemp())
+        foo = root.child('foo').child('dummy').child('plugins')
+        foo.makedirs()
+        foo.child('__init__.py').setContent('')
+        sys.path = [root.child('foo').path, root.child('bar').path]
+        self.assertEqual(
+            plugin.pluginPackagePaths('dummy.plugins'),
+            [root.child('bar').child('dummy').child('plugins').path])