Bazaar

Merge lp:~vila/bzr/82693-plugin-at-path into lp:bzr

  1. 82693-plugin-at-path
  2. Merge into bzr.dev
Proposed by Vincent Ladeuil
Status: Merged
Approved by: Martin Pool
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~vila/bzr/82693-plugin-at-path
Merge into: lp:bzr
Prerequisite: lp:~vila/bzr/411413-disable-plugin
Diff against target: 498 lines (+295/-77) (has conflicts)
5 files modified
NEWS (+10/-0)
bzrlib/help_topics/en/configuration.txt (+30/-4)
bzrlib/plugin.py (+161/-66)
bzrlib/tests/__init__.py (+1/-0)
bzrlib/tests/test_plugins.py (+93/-7)
Text conflict in NEWS
To merge this branch: bzr merge lp:~vila/bzr/82693-plugin-at-path
Reviewer Review Type Date Requested Status
Martin Pool Approve
Review via email: mp+21547@code.launchpad.net

Description of the change

This path fixes bug #82693 by introducing a BZR_PLUGINS_AT environment variable.

Doc excerpt:

+BZR_PLUGINS_AT
+~~~~~~~~~~~~~~
+
+When adding a new feature or working on a bug in a plugin,
+developers often need to use a specific version of a given
+plugin. Since python requires that the directory containing the
+code is named like the plugin itself this make it impossible to
+use arbitrary directory names (using a two-level directory scheme
+is inconvenient). ``BZR_PLUGINS_AT`` allows such directories even
+if they don't appear in ``BZR_PLUGIN_PATH`` .
+
+Plugins specified in this environment variable takes precedence
+over the ones in ``BZR_PLUGIN_PATH``.
+
+The variable specified a list of ``plugin_name@plugin path``,
+``plugin_name`` being the name of the plugin as it appears in
+python module paths, ``plugin_path`` being the path to the
+directory containing the plugin code itself
+(i.e. ``plugins/myplugin`` not ``plugins``). Use ':' as the list
+separator, use ';' on windows.

This requires https://code.edge.launchpad.net/~vila/bzr/411413-disable-plugin/+merge/21435 and reuse the same sys.meta_hook importer.

I've tested it against python2.4, 2.5 and 2.6 ans this seems robust enough there.
python3.1 introduces some simpler ways to achieve that but there is no urgency for that :)

This should assist plugin developers working on several branches in paralle.

To post a comment you must log in.
Revision history for this message
Gary van der Merwe (garyvdm) wrote :

This will be very usefull.
I tested it, and works as expected.

Revision history for this message
Martin Pool (mbp) wrote :

looks ok

Gary, feel free to approve things

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-03-24 07:27:44 +0000
3+++ NEWS 2010-03-24 14:00:53 +0000
4@@ -58,10 +58,20 @@
5 a list of plugin names separated by ':' (';' on windows).
6 (Vincent Ladeuil, #411413)
7
8+<<<<<<< TREE
9 * Tag names can now be determined automatically by ``automatic_tag_name``
10 hooks on ``Branch`` if they are not specified on the command line.
11 (Jelmer Vernooij)
12
13+=======
14+* Plugins can be loaded from arbitrary locations by defining
15+ ``BZR_PLUGINS_AT`` as a list of name@path separated by ':' (';' on
16+ windows). This takes precedence over ``BZR_PLUGIN_PATH`` for the
17+ specified plugins. This is targeted at plugin developers for punctual
18+ needs and *not* intended to replace ``BZR_PLUGIN_PATH``.
19+ (Vincent Ladeuil, #82693)
20+
21+>>>>>>> MERGE-SOURCE
22 * Tree-shape conflicts can be resolved by providing ``--take-this`` and
23 ``--take-other`` to the ``bzr resolve`` command. Just marking the conflict
24 as resolved is still accessible via the ``--done`` default action.
25
26=== modified file 'bzrlib/help_topics/en/configuration.txt'
27--- bzrlib/help_topics/en/configuration.txt 2010-03-19 12:09:05 +0000
28+++ bzrlib/help_topics/en/configuration.txt 2010-03-24 14:00:53 +0000
29@@ -120,10 +120,10 @@
30 BZR_DISABLE_PLUGINS
31 ~~~~~~~~~~~~~~~~~~~
32
33-Under special circumstances, it's better to disable a plugin (or
34-several) rather than uninstalling them completely. Such plugins
35-can be specified in the ``BZR_DISABLE_PLUGINS`` environment
36-variable.
37+Under special circumstances (mostly when trying to diagnose a
38+bug), it's better to disable a plugin (or several) rather than
39+uninstalling them completely. Such plugins can be specified in
40+the ``BZR_DISABLE_PLUGINS`` environment variable.
41
42 In that case, ``bzr`` will stop loading the specified plugins and
43 will raise an import error if they are explicitly imported (by
44@@ -133,6 +133,32 @@
45
46 BZR_DISABLE_PLUGINS='myplugin:yourplugin'
47
48+BZR_PLUGINS_AT
49+~~~~~~~~~~~~~~
50+
51+When adding a new feature or working on a bug in a plugin,
52+developers often need to use a specific version of a given
53+plugin. Since python requires that the directory containing the
54+code is named like the plugin itself this make it impossible to
55+use arbitrary directory names (using a two-level directory scheme
56+is inconvenient). ``BZR_PLUGINS_AT`` allows such directories even
57+if they don't appear in ``BZR_PLUGIN_PATH`` .
58+
59+Plugins specified in this environment variable takes precedence
60+over the ones in ``BZR_PLUGIN_PATH``.
61+
62+The variable specified a list of ``plugin_name@plugin path``,
63+``plugin_name`` being the name of the plugin as it appears in
64+python module paths, ``plugin_path`` being the path to the
65+directory containing the plugin code itself
66+(i.e. ``plugins/myplugin`` not ``plugins``). Use ':' as the list
67+separator, use ';' on windows.
68+
69+Example:
70+~~~~~~~~
71+
72+Using a specific version of ``myplugin``:
73+``BZR_PLUGINS_AT='myplugin@/home/me/bugfixes/123456-myplugin``
74
75 BZRPATH
76 ~~~~~~~
77
78=== modified file 'bzrlib/plugin.py'
79--- bzrlib/plugin.py 2010-03-17 07:16:32 +0000
80+++ bzrlib/plugin.py 2010-03-24 14:00:53 +0000
81@@ -91,12 +91,19 @@
82 if path is None:
83 path = get_standard_plugins_path()
84 _mod_plugins.__path__ = path
85- # Set up a blacklist for disabled plugins if any
86- PluginBlackListImporter.blacklist = {}
87+ PluginImporter.reset()
88+ # Set up a blacklist for disabled plugins
89 disabled_plugins = os.environ.get('BZR_DISABLE_PLUGINS', None)
90 if disabled_plugins is not None:
91 for name in disabled_plugins.split(os.pathsep):
92- PluginBlackListImporter.blacklist['bzrlib.plugins.' + name] = True
93+ PluginImporter.blacklist.add('bzrlib.plugins.' + name)
94+ # Set up a the specific paths for plugins
95+ specific_plugins = os.environ.get('BZR_PLUGINS_AT', None)
96+ if specific_plugins is not None:
97+ for spec in specific_plugins.split(os.pathsep):
98+ plugin_name, plugin_path = spec.split('@')
99+ PluginImporter.specific_paths[
100+ 'bzrlib.plugins.%s' % plugin_name] = plugin_path
101 return path
102
103
104@@ -237,6 +244,11 @@
105
106 The python module path for bzrlib.plugins will be modified to be 'dirs'.
107 """
108+ # Explicitly load the plugins with a specific path
109+ for fullname, path in PluginImporter.specific_paths.iteritems():
110+ name = fullname[len('bzrlib.plugins.'):]
111+ _load_plugin_module(name, path)
112+
113 # We need to strip the trailing separators here as well as in the
114 # set_plugins_path function because calling code can pass anything in to
115 # this function, and since it sets plugins.__path__, it should set it to
116@@ -256,72 +268,99 @@
117 load_from_dirs = load_from_path
118
119
120+def _find_plugin_module(dir, name):
121+ """Check if there is a valid python module that can be loaded as a plugin.
122+
123+ :param dir: The directory where the search is performed.
124+ :param path: An existing file path, either a python file or a package
125+ directory.
126+
127+ :return: (name, path, description) name is the module name, path is the
128+ file to load and description is the tuple returned by
129+ imp.get_suffixes().
130+ """
131+ path = osutils.pathjoin(dir, name)
132+ if os.path.isdir(path):
133+ # Check for a valid __init__.py file, valid suffixes depends on -O and
134+ # can be .py, .pyc and .pyo
135+ for suffix, mode, kind in imp.get_suffixes():
136+ if kind not in (imp.PY_SOURCE, imp.PY_COMPILED):
137+ # We don't recognize compiled modules (.so, .dll, etc)
138+ continue
139+ init_path = osutils.pathjoin(path, '__init__' + suffix)
140+ if os.path.isfile(init_path):
141+ return name, init_path, (suffix, mode, kind)
142+ else:
143+ for suffix, mode, kind in imp.get_suffixes():
144+ if name.endswith(suffix):
145+ # Clean up the module name
146+ name = name[:-len(suffix)]
147+ if kind == imp.C_EXTENSION and name.endswith('module'):
148+ name = name[:-len('module')]
149+ return name, path, (suffix, mode, kind)
150+ # There is no python module here
151+ return None, None, (None, None, None)
152+
153+
154+def _load_plugin_module(name, dir):
155+ """Load plugine name from dir.
156+
157+ :param name: The plugin name in the bzrlib.plugins namespace.
158+ :param dir: The directory the plugin is loaded from for error messages.
159+ """
160+ if ('bzrlib.plugins.%s' % name) in PluginImporter.blacklist:
161+ return
162+ try:
163+ exec "import bzrlib.plugins.%s" % name in {}
164+ except KeyboardInterrupt:
165+ raise
166+ except errors.IncompatibleAPI, e:
167+ trace.warning("Unable to load plugin %r. It requested API version "
168+ "%s of module %s but the minimum exported version is %s, and "
169+ "the maximum is %s" %
170+ (name, e.wanted, e.api, e.minimum, e.current))
171+ except Exception, e:
172+ trace.warning("%s" % e)
173+ if re.search('\.|-| ', name):
174+ sanitised_name = re.sub('[-. ]', '_', name)
175+ if sanitised_name.startswith('bzr_'):
176+ sanitised_name = sanitised_name[len('bzr_'):]
177+ trace.warning("Unable to load %r in %r as a plugin because the "
178+ "file path isn't a valid module name; try renaming "
179+ "it to %r." % (name, dir, sanitised_name))
180+ else:
181+ trace.warning('Unable to load plugin %r from %r' % (name, dir))
182+ trace.log_exception_quietly()
183+ if 'error' in debug.debug_flags:
184+ trace.print_exception(sys.exc_info(), sys.stderr)
185+
186+
187 def load_from_dir(d):
188 """Load the plugins in directory d.
189
190 d must be in the plugins module path already.
191+ This function is called once for each directory in the module path.
192 """
193- # Get the list of valid python suffixes for __init__.py?
194- # this includes .py, .pyc, and .pyo (depending on if we are running -O)
195- # but it doesn't include compiled modules (.so, .dll, etc)
196- valid_suffixes = [suffix for suffix, mod_type, flags in imp.get_suffixes()
197- if flags in (imp.PY_SOURCE, imp.PY_COMPILED)]
198- package_entries = ['__init__'+suffix for suffix in valid_suffixes]
199 plugin_names = set()
200- for f in os.listdir(d):
201- path = osutils.pathjoin(d, f)
202- if os.path.isdir(path):
203- for entry in package_entries:
204- # This directory should be a package, and thus added to
205- # the list
206- if os.path.isfile(osutils.pathjoin(path, entry)):
207- break
208- else: # This directory is not a package
209- continue
210- else:
211- for suffix_info in imp.get_suffixes():
212- if f.endswith(suffix_info[0]):
213- f = f[:-len(suffix_info[0])]
214- if suffix_info[2] == imp.C_EXTENSION and f.endswith('module'):
215- f = f[:-len('module')]
216- break
217+ for p in os.listdir(d):
218+ name, path, desc = _find_plugin_module(d, p)
219+ if name is not None:
220+ if name == '__init__':
221+ # We do nothing with the __init__.py file in directories from
222+ # the bzrlib.plugins module path, we may want to, one day
223+ # -- vila 20100316.
224+ continue # We don't load __init__.py in the plugins dirs
225+ elif getattr(_mod_plugins, name, None) is not None:
226+ # The module has already been loaded from another directory
227+ # during a previous call.
228+ # FIXME: There should be a better way to report masked plugins
229+ # -- vila 20100316
230+ trace.mutter('Plugin name %s already loaded', name)
231 else:
232- continue
233- if f == '__init__':
234- continue # We don't load __init__.py again in the plugin dir
235- elif getattr(_mod_plugins, f, None):
236- trace.mutter('Plugin name %s already loaded', f)
237- else:
238- # trace.mutter('add plugin name %s', f)
239- plugin_names.add(f)
240+ plugin_names.add(name)
241
242 for name in plugin_names:
243- if ('bzrlib.plugins.%s' % name) in PluginBlackListImporter.blacklist:
244- continue
245- try:
246- exec "import bzrlib.plugins.%s" % name in {}
247- except KeyboardInterrupt:
248- raise
249- except errors.IncompatibleAPI, e:
250- trace.warning("Unable to load plugin %r. It requested API version "
251- "%s of module %s but the minimum exported version is %s, and "
252- "the maximum is %s" %
253- (name, e.wanted, e.api, e.minimum, e.current))
254- except Exception, e:
255- trace.warning("%s" % e)
256- ## import pdb; pdb.set_trace()
257- if re.search('\.|-| ', name):
258- sanitised_name = re.sub('[-. ]', '_', name)
259- if sanitised_name.startswith('bzr_'):
260- sanitised_name = sanitised_name[len('bzr_'):]
261- trace.warning("Unable to load %r in %r as a plugin because the "
262- "file path isn't a valid module name; try renaming "
263- "it to %r." % (name, d, sanitised_name))
264- else:
265- trace.warning('Unable to load plugin %r from %r' % (name, d))
266- trace.log_exception_quietly()
267- if 'error' in debug.debug_flags:
268- trace.print_exception(sys.exc_info(), sys.stderr)
269+ _load_plugin_module(name, d)
270
271
272 def plugins():
273@@ -486,17 +525,73 @@
274 __version__ = property(_get__version__)
275
276
277-class _PluginBlackListImporter(object):
278+class _PluginImporter(object):
279+ """An importer tailored to bzr specific needs.
280+
281+ This is a singleton that takes care of:
282+ - disabled plugins specified in 'blacklist',
283+ - plugins that needs to be loaded from specific directories.
284+ """
285
286 def __init__(self):
287- self.blacklist = {}
288+ self.reset()
289+
290+ def reset(self):
291+ self.blacklist = set()
292+ self.specific_paths = {}
293
294 def find_module(self, fullname, parent_path=None):
295+ """Search a plugin module.
296+
297+ Disabled plugins raise an import error, plugins with specific paths
298+ returns a specific loader.
299+
300+ :return: None if the plugin doesn't need special handling, self
301+ otherwise.
302+ """
303+ if not fullname.startswith('bzrlib.plugins.'):
304+ return None
305 if fullname in self.blacklist:
306 raise ImportError('%s is disabled' % fullname)
307+ if fullname in self.specific_paths:
308+ return self
309 return None
310
311-PluginBlackListImporter = _PluginBlackListImporter()
312-sys.meta_path.append(PluginBlackListImporter)
313-
314-
315+ def load_module(self, fullname):
316+ """Load a plugin from a specific directory."""
317+ # We are called only for specific paths
318+ plugin_dir = self.specific_paths[fullname]
319+ candidate = None
320+ maybe_package = False
321+ for p in os.listdir(plugin_dir):
322+ if os.path.isdir(osutils.pathjoin(plugin_dir, p)):
323+ # We're searching for files only and don't want submodules to
324+ # be recognized as plugins (they are submodules inside the
325+ # plugin).
326+ continue
327+ name, path, (
328+ suffix, mode, kind) = _find_plugin_module(plugin_dir, p)
329+ if name is not None:
330+ candidate = (name, path, suffix, mode, kind)
331+ if kind == imp.PY_SOURCE:
332+ # We favour imp.PY_SOURCE (which will use the compiled
333+ # version if available) over imp.PY_COMPILED (which is used
334+ # only if the source is not available)
335+ break
336+ if candidate is None:
337+ raise ImportError('%s cannot be loaded from %s'
338+ % (fullname, plugin_dir))
339+ f = open(path, mode)
340+ try:
341+ mod = imp.load_module(fullname, f, path, (suffix, mode, kind))
342+ # The plugin can contain modules, so be ready
343+ mod.__path__ = [plugin_dir]
344+ mod.__package__ = fullname
345+ return mod
346+ finally:
347+ f.close()
348+
349+
350+# Install a dedicated importer for plugins requiring special handling
351+PluginImporter = _PluginImporter()
352+sys.meta_path.append(PluginImporter)
353
354=== modified file 'bzrlib/tests/__init__.py'
355--- bzrlib/tests/__init__.py 2010-03-24 07:27:44 +0000
356+++ bzrlib/tests/__init__.py 2010-03-24 14:00:53 +0000
357@@ -1520,6 +1520,7 @@
358 'BZR_LOG': None,
359 'BZR_PLUGIN_PATH': None,
360 'BZR_DISABLE_PLUGINS': None,
361+ 'BZR_PLUGINS_AT': None,
362 'BZR_CONCURRENCY': None,
363 # Make sure that any text ui tests are consistent regardless of
364 # the environment the test case is run in; you may want tests that
365
366=== modified file 'bzrlib/tests/test_plugins.py'
367--- bzrlib/tests/test_plugins.py 2010-03-17 07:16:32 +0000
368+++ bzrlib/tests/test_plugins.py 2010-03-24 14:00:53 +0000
369@@ -39,7 +39,11 @@
370
371 class TestPluginMixin(object):
372
373- def create_plugin(self, name, source='', dir='.', file_name=None):
374+ def create_plugin(self, name, source=None, dir='.', file_name=None):
375+ if source is None:
376+ source = '''\
377+"""This is the doc for %s"""
378+''' % (name)
379 if file_name is None:
380 file_name = name + '.py'
381 # 'source' must not fail to load
382@@ -51,11 +55,20 @@
383 finally:
384 f.close()
385
386- def create_plugin_package(self, name, source='', dir='.'):
387- plugin_dir = osutils.pathjoin(dir, name)
388- os.mkdir(plugin_dir)
389- self.addCleanup(osutils.rmtree, plugin_dir)
390- self.create_plugin(name, source, dir=plugin_dir,
391+ def create_plugin_package(self, name, dir=None, source=None):
392+ if dir is None:
393+ dir = name
394+ if source is None:
395+ source = '''\
396+"""This is the doc for %s"""
397+dir_source = '%s'
398+''' % (name, dir)
399+ os.makedirs(dir)
400+ def cleanup():
401+ # Workaround lazy import random? madness
402+ osutils.rmtree(dir)
403+ self.addCleanup(cleanup)
404+ self.create_plugin(name, source, dir,
405 file_name='__init__.py')
406
407 def _unregister_plugin(self, name):
408@@ -767,16 +780,89 @@
409 self.overrideAttr(plugin, '_loaded', False)
410 plugin.load_plugins(['.'])
411 self.assertPluginKnown('test_foo')
412+ self.assertEqual("This is the doc for test_foo",
413+ bzrlib.plugins.test_foo.__doc__)
414
415 def test_not_loaded(self):
416 self.warnings = []
417 def captured_warning(*args, **kwargs):
418 self.warnings.append((args, kwargs))
419 self.overrideAttr(trace, 'warning', captured_warning)
420+ # Reset the flag that protect against double loading
421 self.overrideAttr(plugin, '_loaded', False)
422 osutils.set_or_unset_env('BZR_DISABLE_PLUGINS', 'test_foo')
423- plugin.load_plugins(plugin.set_plugins_path(['.']))
424+ plugin.load_plugins(['.'])
425 self.assertPluginUnknown('test_foo')
426 # Make sure we don't warn about the plugin ImportError since this has
427 # been *requested* by the user.
428 self.assertLength(0, self.warnings)
429+
430+
431+class TestLoadPluginAt(tests.TestCaseInTempDir, TestPluginMixin):
432+
433+ def setUp(self):
434+ super(TestLoadPluginAt, self).setUp()
435+ # Make sure we don't pollute the plugins namespace
436+ self.overrideAttr(plugins, '__path__')
437+ # Be paranoid in case a test fail
438+ self.addCleanup(self._unregister_plugin, 'test_foo')
439+ # Reset the flag that protect against double loading
440+ self.overrideAttr(plugin, '_loaded', False)
441+ # Create the same plugin in two directories
442+ self.create_plugin_package('test_foo', dir='non-standard-dir')
443+ self.create_plugin_package('test_foo', dir='b/test_foo')
444+
445+ def assertTestFooLoadedFrom(self, dir):
446+ self.assertPluginKnown('test_foo')
447+ self.assertEqual('This is the doc for test_foo',
448+ bzrlib.plugins.test_foo.__doc__)
449+ self.assertEqual(dir, bzrlib.plugins.test_foo.dir_source)
450+
451+ def test_regular_load(self):
452+ plugin.load_plugins(['b'])
453+ self.assertTestFooLoadedFrom('b/test_foo')
454+
455+ def test_import(self):
456+ osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo@non-standard-dir')
457+ plugin.set_plugins_path(['b'])
458+ try:
459+ import bzrlib.plugins.test_foo
460+ except ImportError:
461+ pass
462+ self.assertTestFooLoadedFrom('non-standard-dir')
463+
464+ def test_loading(self):
465+ osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo@non-standard-dir')
466+ plugin.load_plugins(['b'])
467+ self.assertTestFooLoadedFrom('non-standard-dir')
468+
469+ def test_compiled_loaded(self):
470+ osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo@non-standard-dir')
471+ plugin.load_plugins(['b'])
472+ self.assertTestFooLoadedFrom('non-standard-dir')
473+ self.assertEqual('non-standard-dir/__init__.py',
474+ bzrlib.plugins.test_foo.__file__)
475+
476+ # Try importing again now that the source has been compiled
477+ self._unregister_plugin('test_foo')
478+ plugin._loaded = False
479+ plugin.load_plugins(['b'])
480+ self.assertTestFooLoadedFrom('non-standard-dir')
481+ if __debug__:
482+ suffix = 'pyc'
483+ else:
484+ suffix = 'pyo'
485+ self.assertEqual('non-standard-dir/__init__.%s' % suffix,
486+ bzrlib.plugins.test_foo.__file__)
487+
488+ def test_submodule_loading(self):
489+ # We create an additional directory under the one for test_foo
490+ self.create_plugin_package('test_bar', dir='non-standard-dir/test_bar')
491+ osutils.set_or_unset_env('BZR_PLUGINS_AT', 'test_foo@non-standard-dir')
492+ plugin.set_plugins_path(['b'])
493+ import bzrlib.plugins.test_foo
494+ self.assertEqual('bzrlib.plugins.test_foo',
495+ bzrlib.plugins.test_foo.__package__)
496+ import bzrlib.plugins.test_foo.test_bar
497+ self.assertEqual('non-standard-dir/test_bar/__init__.py',
498+ bzrlib.plugins.test_foo.test_bar.__file__)