Merge lp:~mars/tarmac/fix-plugin-import into lp:tarmac

[Māris Fogels]

Hi,

This branch presents a quick and dirty fix to the Tarmac plugin activation code. It adds the Tarmac plugin directories to the system path so that Tarmac can import the plugins.

There may be a better way to do this, but I am leaving that better implementation for someone else to tackle. The code passed manual testing on my local system.

Maris

[Paul Hummer (rockstar)]

So, the idea here was to create a similar environment to the way bzr imports its plugins (from bzrlib.plugins import whatever). This patch breaks that idea. I think we need to dig deeper into the bzrlib plugin registration code to see why this is not working (I'm pretty sure it worked at some point, so maybe we broke it).

I appreciate that you've found the bug, and that you've proposed a fix. I think this breaks the api we want to use (even though that API is broken as well), so I'm not sure this is the correct way to do this.

review: Disapprove

[dobey (dobey)]

I think the code (probably in bzrlib too), needs to perhaps switch over to using imp.find_module() and imp.load_module(), to do the plug-in loading. Doing the exec 'import foo' is extra nasty. Even just doing __import__() instead would at least be better code, even if it doesn't solve this specific bug.

Unmerged revisions

=== modified file 'tarmac/plugin.py'
--- tarmac/plugin.py	2010-09-02 15:18:07 +0000
+++ tarmac/plugin.py	2010-12-07 15:04:10 +0000
@@ -18,6 +18,7 @@
 
 import imp
 import os
+import sys
 
 from tarmac import plugins as _mod_plugins
 
@@ -40,6 +41,9 @@
 
     plugin_names = set()
     for path in TARMAC_PLUGIN_PATHS:
+        if path not in sys.path:
+            sys.path.insert(0, path)
+
         try:
             for _file in os.listdir(path):
                 full_path = os.path.join(path, _file)
@@ -75,6 +79,6 @@
 
     for name in plugin_names:
         try:
-            exec 'import tarmac.plugins.%s' % name in {}
+            exec 'import %s' % name in {}
         except KeyboardInterrupt:
             raise