Merge lp:~bzr/bzr/412930-plugin-path into lp:~bzr/bzr/trunk-old

Proposed by Vincent Ladeuil on 2009-08-21
Status: Merged
Approved by: John A Meinel on 2009-09-04
Approved revision: 4633
Merge reported by: Vincent Ladeuil
Merged at revision: not available
Proposed branch: lp:~bzr/bzr/412930-plugin-path
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 535 lines
To merge this branch: bzr merge lp:~bzr/bzr/412930-plugin-path
Reviewer Review Type Date Requested Status
John A Meinel 2009-08-21 Approve on 2009-09-04
Review via email: mp+10518@code.launchpad.net

This proposal supersedes a proposal from 2009-08-20.

To post a comment you must log in.
Vincent Ladeuil (vila) wrote : Posted in a previous version of this proposal

This patch add the capability to fully control BZR_PLUGIN_PATH by defining magic values
for user, core and site directories and a way to remove them from the search path.

It's compatible with the actual implementation.

This will allow the test/build farm to test:
- the core plugins (the test are currently run with --no-plugins),
- a chosen set of plugins

Thanks in advance to the reviewer :-)

John A Meinel (jameinel) wrote : Posted in a previous version of this proposal
Download full text (4.2 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~bzr/bzr/412930-plugin-path into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> This patch add the capability to fully control BZR_PLUGIN_PATH by defining magic values
> for user, core and site directories and a way to remove them from the search path.
>
> It's compatible with the actual implementation.
>
> This will allow the test/build farm to test:
> - the core plugins (the test are currently run with --no-plugins),
> - a chosen set of plugins
>
> Thanks in advance to the reviewer :-)
>

So does this break the existing BZR_PLUGIN_PATH behavior? At first I
thought it did, but it looks like the existing code says "if
BZR_PLUGIN_PATH is set, override default location". Which matches your
"+user" version.

+By default all directories specified in ``BZR_PLUGIN_PATH``
+replace 'user' in the but keep searching in 'core' then 'site'.

^- bad grammar here.

By default if BZR_PLUGIN_PATH is set, it replaces searching in 'user'.
However it will continue to search in 'core' and 'site' unless they are
explicitly removed.

...

+ # Handle removals first
+ def unset_ref(p):
+ if p.startswith('-'):
+ key = p[1:]
+ if key in refs: # Needed to handle multiple removals
+ refs[key] = None
+ p = None
+ return p
+
+ defaults = [p for p in defaults if unset_ref(p) is not None]
+ env_paths = [p for p in env_paths if unset_ref(p) is not None]

^- This doesn't look right to me.

defaults = [p for p in defaults if unset_ref(p) is not None]

Specifically:
 1) I don't think you would ever have '-site' in defaults, for one
    thing, it is a local variable.
 2) If you did have '-site' in the list, then it would see that it is
    present in refs, set p to None, and then the 'if unset_ref(p) is not
    None' would return True, and '-site' is then included in the final
    defaults list.

I'm not 100% sure that doing:

BZR_PLUGIN_PATH='-site:+site'

Should ignore the site path. I would actually say 'last value wins'. So
that you can just do:

BZR_PLUGIN_PATH="$BZR_PLUGIN_PATH:+site" bzr test

I'm not stuck on this, but I certainly find it hard to follow exactly
what is going on in your get_standard_plugins_path() function. (The diff
makes things worse, because it mixes statements a little bit.)

I believe you are also trying to allow for altering the order of the
switch. So doing:

BZR_PLUGIN_PATH='+site:+user'

Would then search site locations before user locations.

I would probably do something more like:

env_path = os.environ.get('BZR_PLUGIN_PATH', '+user').split(os.pathsep)

default_core = True
default_site = True
plugin_paths = []
for path in env_path:
  if path.startswith('-'):
    if path == '-site':
      default_site = False
      if '+site' in plugin_paths:
        plugin_paths.remove('+site')
    elif path == '-core':
    ...
 elif path.startswith('+'):
    if path == '+site':
      default_site = False
      if '+site' in plugin_paths:
        plugin_paths.remove('+site')
    elif path == '+core':
      ...

    plugin_paths.append(path)
 else:
    ...

Read more...

review: Needs Fixing
Alexander Belchenko (bialix) wrote : Posted in a previous version of this proposal

30 +* the size specific plugin directory if applicable (containing
31 + the site plugins).

the size specific plugin directory? What is it?

John A Meinel (jameinel) wrote : Posted in a previous version of this proposal

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alexander Belchenko wrote:
> 30 +* the size specific plugin directory if applicable (containing
> 31 + the site plugins).
>
> the size specific plugin directory? What is it?

site-specific. It only really exists on Linux where they split out the
python path because of wanting a separate "/usr/lib64" versus "/usr/lib"
and other such tweaks for the different between 64-bit compiled code,
and pure python code.

At least, that is how I understand it.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqNlz0ACgkQJdeBCYSNAAMxxQCeOJopPvaV0JgNHuf+5pRpyl6m
5y4An0jzQ0IjHY4ZVHIm/+370zinGdpX
=WfNs
-----END PGP SIGNATURE-----

Vincent Ladeuil (vila) wrote : Posted in a previous version of this proposal
Download full text (7.0 KiB)

>>>>> "jam" == John A Meinel <email address hidden> writes:

    jam> Review: Needs Fixing
    jam> Vincent Ladeuil wrote:
    >> Vincent Ladeuil has proposed merging lp:~bzr/bzr/412930-plugin-path into lp:bzr.
    >>
    >> Requested reviews:
    >> bzr-core (bzr-core)
    >>
    >> This patch add the capability to fully control BZR_PLUGIN_PATH by defining magic values
    >> for user, core and site directories and a way to remove them from the search path.
    >>
    >> It's compatible with the actual implementation.
    >>
    >> This will allow the test/build farm to test:
    >> - the core plugins (the test are currently run with --no-plugins),
    >> - a chosen set of plugins
    >>
    >> Thanks in advance to the reviewer :-)
    >>

    jam> So does this break the existing BZR_PLUGIN_PATH behavior?

No. But it makes the code more complex.

    jam> At first I thought it did, but it looks like the
    jam> existing code says "if BZR_PLUGIN_PATH is set, override
    jam> default location". Which matches your "+user" version.

Yes. That's the complicated part, 'core' and 'site' cannot be
removed without trickery.

    jam> +By default all directories specified in ``BZR_PLUGIN_PATH``
    jam> +replace 'user' in the but keep searching in 'core' then 'site'.

    jam> ^- bad grammar here.

    jam> By default if BZR_PLUGIN_PATH is set, it replaces searching in 'user'.
    jam> However it will continue to search in 'core' and 'site' unless they are
    jam> explicitly removed.

Thanks, fixed.

    jam> ...

    jam> + # Handle removals first
    jam> + def unset_ref(p):
    jam> + if p.startswith('-'):
    jam> + key = p[1:]
    jam> + if key in refs: # Needed to handle multiple removals
    jam> + refs[key] = None
    jam> + p = None
    jam> + return p
    jam> +
    jam> + defaults = [p for p in defaults if unset_ref(p) is not None]
    jam> + env_paths = [p for p in env_paths if unset_ref(p) is not None]

    jam> ^- This doesn't look right to me.

Then forget about it I removed that part :)

    jam> defaults = [p for p in defaults if unset_ref(p) is not None]

    jam> Specifically:

    jam> 1) I don't think you would ever have '-site' in defaults, for one
    jam> thing, it is a local variable.

You won't have '-site', but you have '+site', and you want it to
get removed when '-site' is specified. That's even the only way
to ensure that s 'site' plugin is not involved.

Both https://bugs.edge.launchpad.net/bzr/+bug/412930 and
https://bugs.edge.launchpad.net/bzr/+bug/316192 needs that
feature.

    jam> 2) If you did have '-site' in the list, then it would
    jam> see that it is present in refs, set p to None, and then
    jam> the 'if unset_ref(p) is not None' would return True,

???

if p is None, 'unset_ref(p) is not None' is False

    jam> and '-site' is then included in the final defaults
    jam> list.

No.

    jam> I'm not 100% sure that doing:

    jam> BZR_PLUGIN_PATH='-site:+site'

    jam> Should ignore the site path. I would actually say 'last
    jam> value wins'. So that you can just do:

    jam> BZR_PLUGIN_PATH="$BZR_PLU...

Read more...

Vincent Ladeuil (vila) wrote : Posted in a previous version of this proposal

>>>>> "jam" == John A Meinel <email address hidden> writes:

    jam> Alexander Belchenko wrote:
    >> 30 +* the size specific plugin directory if applicable (containing
    >> 31 + the site plugins).
    >>
    >> the size specific plugin directory? What is it?

    jam> site-specific.

Right. The intent is to give access to plugins installed by
whoever administer the site, this directory contains plugins that
can be used by all users.

    jam> It only really exists on Linux

Wrong.

The get_python_lib() doc string in distutils says:

    """Return the directory containing the Python library (standard or
    site additions).

    If 'plat_specific' is true, return the directory containing
    platform-specific modules, i.e. any module from a non-pure-Python
    module distribution; otherwise, return the platform-shared library
    directory. If 'standard_lib' is true, return the directory
    containing standard Python library modules; otherwise, return the
    directory for site-specific modules.

    If 'prefix' is supplied, use it instead of sys.prefix or
    sys.exec_prefix -- i.e., ignore 'plat_specific'.
    """

On linux it can be 'dist-packages' or 'site-packages' in the
python hierarchy.

On mac it's 'Lib/site-packages' but again the installers may be
wrong here (I don't know them enough to be sure, they may well be
right).

On os.name == "nt" (windows right ?) it seems to be
'Lib/site-packages'.

So, as I understand it, it makes no sense for bzr.exe (but we may
want to say it's 'All Users/Application Data/Bazaar/Plugins') and
when running from sources (or from easy_install when it will
work), we may want to activate it (in addition to 'All Users/...')

In the mean time, I kept the compatibility with previous versions
and add the win32 check that I accidentally removed in my patch.

Feedback welcome even if I think that should be the subject of
another submission are there are certainly tweaks to be done in
the windows installers.

Alexander Belchenko (bialix) wrote : Posted in a previous version of this proposal

Vincent Ladeuil пишет:
>>>>>> "jam" == John A Meinel <email address hidden> writes:
>
> jam> Alexander Belchenko wrote:
> >> 30 +* the size specific plugin directory if applicable (containing
> >> 31 + the site plugins).
> >>
> >> the size specific plugin directory? What is it?
>
> jam> site-specific.

I understand, it was light joke. But it was not very good joke. Move on.

> Right. The intent is to give access to plugins installed by
> whoever administer the site, this directory contains plugins that
> can be used by all users.
>
> jam> It only really exists on Linux
>
> Wrong.

Really?

> The get_python_lib() doc string in distutils says:
[...]
>
> On linux it can be 'dist-packages' or 'site-packages' in the
> python hierarchy.
>
> On mac it's 'Lib/site-packages' but again the installers may be
> wrong here (I don't know them enough to be sure, they may well be
> right).
>
> On os.name == "nt" (windows right ?) it seems to be
> 'Lib/site-packages'.

Yes, so there is no distinction between "site" and "core" for Windows?

Because if I run bzr from sources current bzrlib/plugin.py (as I know)
even don't try to load plugins from
C:\PythonXX\Lib\site-packages\bzrlib\plugins

And perhaps it's right? No? Why?

If yes, what should be the right thing if I run bzr 3.0 from sources and
have installed bzr 2.x in site-packages? Do you think they will be
compatible? Quick responses on IRC reveal that it's not.

> So, as I understand it, it makes no sense for bzr.exe

For bzr.exe there is only "core" plugins I think. They are installed as
$(INSTALL_DIR)\plugins\, e.g. C:\Program Files\Bazaar\plugins

> (but we may
> want to say it's 'All Users/Application Data/Bazaar/Plugins')

I'm recall vaguely it was even discussed. But honestly: I don't want
this. Actually the less places bzr will use to look for plugins the
faster its startup time will be. Because on Windows it makes huge
difference if you need to read 1 file of 10MB and 100 files of 100K. The
less disk IO the faster application.

> and
> when running from sources (or from easy_install when it will
> work), we may want to activate it (in addition to 'All Users/...')

I have no opinion on using bzr in easy_install way. I love bzr.exe and
think it's the best we have today.

> In the mean time, I kept the compatibility with previous versions
> and add the win32 check that I accidentally removed in my patch.

If you return this code back then I should not worried?

> Feedback welcome even if I think that should be the subject of
> another submission are there are certainly tweaks to be done in
> the windows installers.

I'm still trying to understand what question I should answer.
I hope said above will help you. Ask me more and preferably directly,
I'm not subscribed on merge proposal comments.

Alexander Belchenko (bialix) wrote : Posted in a previous version of this proposal

My 5 kopecks: as hardcore windows user I don't really like idea of Vincent example:

BZR_PLUGIN_PATH=-site bzr selftest ...

Because on pure Windows I can't set env variable only for one command. But I can't imagine why I need the proposed feature so I'm actually indifferent on this. But as windows users I'd prefer global command-line options similar to --no-plugins. Maybe:

--plugins=-site

would work as alternate syntax. Or maybe it will be even more obscure.
I dunno.

Vincent Ladeuil (vila) wrote :

That version is simpler, see the superseded proposal for relevant comments.

John A Meinel (jameinel) :
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 2009-08-30 23:51:10 +0000
3+++ NEWS 2009-08-31 04:35:30 +0000
4@@ -237,6 +237,11 @@
5 New Features
6 ************
7
8+* Give more control on BZR_PLUGIN_PATH by providing a way to refer to or
9+ disable the user, site and core plugin directories.
10+ (Vincent Ladeuil, #412930, #316192, #145612)
11+
12+
13 Bug Fixes
14 *********
15
16
17=== modified file 'bzrlib/help_topics/en/configuration.txt'
18--- bzrlib/help_topics/en/configuration.txt 2009-06-26 18:13:41 +0000
19+++ bzrlib/help_topics/en/configuration.txt 2009-08-31 04:35:30 +0000
20@@ -63,6 +63,60 @@
21 ~~~~~~~~~~~~~~~
22
23 The path to the plugins directory that Bazaar should use.
24+If not set, Bazaar will search for plugins in:
25+
26+* the user specific plugin directory (containing the ``user`` plugins),
27+
28+* the bzrlib directory (containing the ``core`` plugins),
29+
30+* the site specific plugin directory if applicable (containing
31+ the ``site`` plugins).
32+
33+If ``BZR_PLUGIN_PATH`` is set in any fashion, it will change the
34+the way the plugin are searched.
35+
36+As for the ``PATH`` variables, if multiple directories are
37+specified in ``BZR_PLUGIN_PATH`` they should be separated by the
38+platform specific appropriate character (':' on Unix/Linux/etc,
39+';' on windows)
40+
41+By default if ``BZR_PLUGIN_PATH`` is set, it replaces searching
42+in ``user``. However it will continue to search in ``core`` and
43+``site`` unless they are explicitly removed.
44+
45+If you need to change the order or remove one of these
46+directories, you should use special values:
47+
48+* ``-user``, ``-core``, ``-site`` will remove the corresponding
49+ path from the default values,
50+
51+* ``+user``, ``+core``, ``+site`` will add the corresponding path
52+ before the remaining default values (and also remove it from
53+ the default values).
54+
55+Note that the special values 'user', 'core' and 'site' should be
56+used literally, they will be substituted by the corresponding,
57+platform specific, values.
58+
59+Examples:
60+^^^^^^^^^
61+
62+The examples below uses ':' as the separator, windows users
63+should use ';'.
64+
65+Overriding the default user plugin directory:
66+``BZR_PLUGIN_PATH='/path/to/my/other/plugins'``
67+
68+Disabling the site directory while retaining the user directory:
69+``BZR_PLUGIN_PATH='-site:+user'``
70+
71+Disabling all plugins (better achieved with --no-plugins):
72+``BZR_PLUGIN_PATH='-user:-core:-site'``
73+
74+Overriding the default site plugin directory:
75+``BZR_PLUGIN_PATH='/path/to/my/site/plugins:-site':+user``
76+
77+
78
79 BZRPATH
80 ~~~~~~~
81
82=== modified file 'bzrlib/plugin.py'
83--- bzrlib/plugin.py 2009-03-24 01:53:42 +0000
84+++ bzrlib/plugin.py 2009-08-31 04:35:30 +0000
85@@ -52,12 +52,16 @@
86 from bzrlib import plugins as _mod_plugins
87 """)
88
89-from bzrlib.symbol_versioning import deprecated_function
90+from bzrlib.symbol_versioning import (
91+ deprecated_function,
92+ deprecated_in,
93+ )
94
95
96 DEFAULT_PLUGIN_PATH = None
97 _loaded = False
98
99+@deprecated_function(deprecated_in((2, 0, 0)))
100 def get_default_plugin_path():
101 """Get the DEFAULT_PLUGIN_PATH"""
102 global DEFAULT_PLUGIN_PATH
103@@ -91,13 +95,15 @@
104 return path
105
106
107-def get_standard_plugins_path():
108- """Determine a plugin path suitable for general use."""
109- path = os.environ.get('BZR_PLUGIN_PATH',
110- get_default_plugin_path()).split(os.pathsep)
111- # Get rid of trailing slashes, since Python can't handle them when
112- # it tries to import modules.
113- path = map(_strip_trailing_sep, path)
114+def _append_new_path(paths, new_path):
115+ """Append a new path if it set and not already known."""
116+ if new_path is not None and new_path not in paths:
117+ paths.append(new_path)
118+ return paths
119+
120+
121+def get_core_plugin_path():
122+ core_path = None
123 bzr_exe = bool(getattr(sys, 'frozen', None))
124 if bzr_exe: # expand path for bzr.exe
125 # We need to use relative path to system-wide plugin
126@@ -110,25 +116,83 @@
127 # then plugins directory is
128 # C:\Program Files\Bazaar\plugins
129 # so relative path is ../../../plugins
130- path.append(osutils.abspath(osutils.pathjoin(
131- osutils.dirname(__file__), '../../../plugins')))
132- if not bzr_exe: # don't look inside library.zip
133+ core_path = osutils.abspath(osutils.pathjoin(
134+ osutils.dirname(__file__), '../../../plugins'))
135+ else: # don't look inside library.zip
136 # search the plugin path before the bzrlib installed dir
137- path.append(os.path.dirname(_mod_plugins.__file__))
138- # search the arch independent path if we can determine that and
139- # the plugin is found nowhere else
140- if sys.platform != 'win32':
141- try:
142- from distutils.sysconfig import get_python_lib
143- except ImportError:
144- # If distutuils is not available, we just won't add that path
145- pass
146- else:
147- archless_path = osutils.pathjoin(get_python_lib(), 'bzrlib',
148- 'plugins')
149- if archless_path not in path:
150- path.append(archless_path)
151- return path
152+ core_path = os.path.dirname(_mod_plugins.__file__)
153+ return core_path
154+
155+
156+def get_site_plugin_path():
157+ """Returns the path for the site installed plugins."""
158+ if sys.platform == 'win32':
159+ # We don't have (yet) a good answer for windows since that is certainly
160+ # related to the way we build the installers. -- vila20090821
161+ return None
162+ site_path = None
163+ try:
164+ from distutils.sysconfig import get_python_lib
165+ except ImportError:
166+ # If distutuils is not available, we just don't know where they are
167+ pass
168+ else:
169+ site_path = osutils.pathjoin(get_python_lib(), 'bzrlib', 'plugins')
170+ return site_path
171+
172+
173+def get_user_plugin_path():
174+ return osutils.pathjoin(config.config_dir(), 'plugins')
175+
176+
177+def get_standard_plugins_path():
178+ """Determine a plugin path suitable for general use."""
179+ # Ad-Hoc default: core is not overriden by site but user can overrides both
180+ # The rationale is that:
181+ # - 'site' comes last, because these plugins should always be available and
182+ # are supposed to be in sync with the bzr installed on site.
183+ # - 'core' comes before 'site' so that running bzr from sources or a user
184+ # installed version overrides the site version.
185+ # - 'user' comes first, because... user is always right.
186+ # - the above rules clearly defines which plugin version will be loaded if
187+ # several exist. Yet, it is sometimes desirable to disable some directory
188+ # so that a set of plugin is disabled as once. This can be done via
189+ # -site, -core, -user.
190+
191+ env_paths = os.environ.get('BZR_PLUGIN_PATH', '+user').split(os.pathsep)
192+ defaults = ['+core', '+site']
193+
194+ # The predefined references
195+ refs = dict(core=get_core_plugin_path(),
196+ site=get_site_plugin_path(),
197+ user=get_user_plugin_path())
198+
199+ # Unset paths that should be removed
200+ for k,v in refs.iteritems():
201+ removed = '-%s' % k
202+ # defaults can never mention removing paths as that will make it
203+ # impossible for the user to revoke these removals.
204+ if removed in env_paths:
205+ env_paths.remove(removed)
206+ refs[k] = None
207+
208+ # Expand references
209+ paths = []
210+ for p in env_paths + defaults:
211+ if p.startswith('+'):
212+ # Resolve reference if they are known
213+ try:
214+ p = refs[p[1:]]
215+ except KeyError:
216+ # Leave them untouched otherwise, user may have paths starting
217+ # with '+'...
218+ pass
219+ _append_new_path(paths, p)
220+
221+ # Get rid of trailing slashes, since Python can't handle them when
222+ # it tries to import modules.
223+ paths = map(_strip_trailing_sep, paths)
224+ return paths
225
226
227 def load_plugins(path=None):
228
229=== modified file 'bzrlib/tests/test_plugins.py'
230--- bzrlib/tests/test_plugins.py 2009-06-10 03:56:49 +0000
231+++ bzrlib/tests/test_plugins.py 2009-08-31 04:35:31 +0000
232@@ -26,7 +26,11 @@
233 import sys
234 import zipfile
235
236-from bzrlib import plugin, tests
237+from bzrlib import (
238+ osutils,
239+ plugin,
240+ tests,
241+ )
242 import bzrlib.plugin
243 import bzrlib.plugins
244 import bzrlib.commands
245@@ -454,41 +458,6 @@
246 delattr(bzrlib.plugins, 'myplug')
247
248
249-class TestSetPluginsPath(TestCase):
250-
251- def test_set_plugins_path(self):
252- """set_plugins_path should set the module __path__ correctly."""
253- old_path = bzrlib.plugins.__path__
254- try:
255- bzrlib.plugins.__path__ = []
256- expected_path = bzrlib.plugin.set_plugins_path()
257- self.assertEqual(expected_path, bzrlib.plugins.__path__)
258- finally:
259- bzrlib.plugins.__path__ = old_path
260-
261- def test_set_plugins_path_with_trailing_slashes(self):
262- """set_plugins_path should set the module __path__ based on
263- BZR_PLUGIN_PATH after removing all trailing slashes."""
264- old_path = bzrlib.plugins.__path__
265- old_env = os.environ.get('BZR_PLUGIN_PATH')
266- try:
267- bzrlib.plugins.__path__ = []
268- os.environ['BZR_PLUGIN_PATH'] = "first\\//\\" + os.pathsep + \
269- "second/\\/\\/"
270- bzrlib.plugin.set_plugins_path()
271- # We expect our nominated paths to have all path-seps removed,
272- # and this is testing only that.
273- expected_path = ['first', 'second']
274- self.assertEqual(expected_path,
275- bzrlib.plugins.__path__[:len(expected_path)])
276- finally:
277- bzrlib.plugins.__path__ = old_path
278- if old_env is not None:
279- os.environ['BZR_PLUGIN_PATH'] = old_env
280- else:
281- del os.environ['BZR_PLUGIN_PATH']
282-
283-
284 class TestHelpIndex(tests.TestCase):
285 """Tests for the PluginsHelpIndex class."""
286
287@@ -597,41 +566,42 @@
288 self.assertEqual('foo_bar', topic.get_help_topic())
289
290
291-def clear_plugins(test_case):
292- # Save the attributes that we're about to monkey-patch.
293- old_plugins_path = bzrlib.plugins.__path__
294- old_loaded = plugin._loaded
295- old_load_from_path = plugin.load_from_path
296- # Change bzrlib.plugin to think no plugins have been loaded yet.
297- bzrlib.plugins.__path__ = []
298- plugin._loaded = False
299- # Monkey-patch load_from_path to stop it from actually loading anything.
300- def load_from_path(dirs):
301- pass
302- plugin.load_from_path = load_from_path
303- def restore_plugins():
304- bzrlib.plugins.__path__ = old_plugins_path
305- plugin._loaded = old_loaded
306- plugin.load_from_path = old_load_from_path
307- test_case.addCleanup(restore_plugins)
308-
309-
310-class TestPluginPaths(tests.TestCase):
311+class TestLoadFromPath(tests.TestCaseInTempDir):
312+
313+ def setUp(self):
314+ super(TestLoadFromPath, self).setUp()
315+ # Save the attributes that we're about to monkey-patch.
316+ old_plugins_path = bzrlib.plugins.__path__
317+ old_loaded = plugin._loaded
318+ old_load_from_path = plugin.load_from_path
319+
320+ def restore():
321+ bzrlib.plugins.__path__ = old_plugins_path
322+ plugin._loaded = old_loaded
323+ plugin.load_from_path = old_load_from_path
324+
325+ self.addCleanup(restore)
326+
327+ # Change bzrlib.plugin to think no plugins have been loaded yet.
328+ bzrlib.plugins.__path__ = []
329+ plugin._loaded = False
330+
331+ # Monkey-patch load_from_path to stop it from actually loading anything.
332+ def load_from_path(dirs):
333+ pass
334+ plugin.load_from_path = load_from_path
335
336 def test_set_plugins_path_with_args(self):
337- clear_plugins(self)
338 plugin.set_plugins_path(['a', 'b'])
339 self.assertEqual(['a', 'b'], bzrlib.plugins.__path__)
340
341 def test_set_plugins_path_defaults(self):
342- clear_plugins(self)
343 plugin.set_plugins_path()
344 self.assertEqual(plugin.get_standard_plugins_path(),
345 bzrlib.plugins.__path__)
346
347 def test_get_standard_plugins_path(self):
348 path = plugin.get_standard_plugins_path()
349- self.assertEqual(plugin.get_default_plugin_path(), path[0])
350 for directory in path:
351 self.assertNotContainsRe(directory, r'\\/$')
352 try:
353@@ -649,13 +619,11 @@
354
355 def test_get_standard_plugins_path_env(self):
356 os.environ['BZR_PLUGIN_PATH'] = 'foo/'
357- self.assertEqual('foo', plugin.get_standard_plugins_path()[0])
358-
359-
360-class TestLoadPlugins(tests.TestCaseInTempDir):
361+ path = plugin.get_standard_plugins_path()
362+ for directory in path:
363+ self.assertNotContainsRe(directory, r'\\/$')
364
365 def test_load_plugins(self):
366- clear_plugins(self)
367 plugin.load_plugins(['.'])
368 self.assertEqual(bzrlib.plugins.__path__, ['.'])
369 # subsequent loads are no-ops
370@@ -663,7 +631,107 @@
371 self.assertEqual(bzrlib.plugins.__path__, ['.'])
372
373 def test_load_plugins_default(self):
374- clear_plugins(self)
375 plugin.load_plugins()
376 path = plugin.get_standard_plugins_path()
377 self.assertEqual(path, bzrlib.plugins.__path__)
378+
379+
380+class TestEnvPluginPath(tests.TestCaseInTempDir):
381+
382+ def setUp(self):
383+ super(TestEnvPluginPath, self).setUp()
384+ old_default = plugin.DEFAULT_PLUGIN_PATH
385+
386+ def restore():
387+ plugin.DEFAULT_PLUGIN_PATH = old_default
388+
389+ self.addCleanup(restore)
390+
391+ plugin.DEFAULT_PLUGIN_PATH = None
392+
393+ self.user = plugin.get_user_plugin_path()
394+ self.site = plugin.get_site_plugin_path()
395+ self.core = plugin.get_core_plugin_path()
396+
397+ def _list2paths(self, *args):
398+ paths = []
399+ for p in args:
400+ plugin._append_new_path(paths, p)
401+ return paths
402+
403+ def _set_path(self, *args):
404+ path = os.pathsep.join(self._list2paths(*args))
405+ osutils.set_or_unset_env('BZR_PLUGIN_PATH', path)
406+
407+ def check_path(self, expected_dirs, setting_dirs):
408+ if setting_dirs:
409+ self._set_path(*setting_dirs)
410+ actual = plugin.get_standard_plugins_path()
411+ self.assertEquals(self._list2paths(*expected_dirs), actual)
412+
413+ def test_default(self):
414+ self.check_path([self.user, self.core, self.site],
415+ None)
416+
417+ def test_adhoc_policy(self):
418+ self.check_path([self.user, self.core, self.site],
419+ ['+user', '+core', '+site'])
420+
421+ def test_fallback_policy(self):
422+ self.check_path([self.core, self.site, self.user],
423+ ['+core', '+site', '+user'])
424+
425+ def test_override_policy(self):
426+ self.check_path([self.user, self.site, self.core],
427+ ['+user', '+site', '+core'])
428+
429+ def test_disable_user(self):
430+ self.check_path([self.core, self.site], ['-user'])
431+
432+ def test_disable_user_twice(self):
433+ # Ensures multiple removals don't left cruft
434+ self.check_path([self.core, self.site], ['-user', '-user'])
435+
436+ def test_duplicates_are_removed(self):
437+ self.check_path([self.user, self.core, self.site],
438+ ['+user', '+user'])
439+ # And only the first reference is kept (since the later references will
440+ # onnly produce <plugin> already loaded mutters)
441+ self.check_path([self.user, self.core, self.site],
442+ ['+user', '+user', '+core',
443+ '+user', '+site', '+site',
444+ '+core'])
445+
446+ def test_disable_overrides_disable(self):
447+ self.check_path([self.core, self.site], ['-user', '+user'])
448+
449+ def test_disable_core(self):
450+ self.check_path([self.site], ['-core'])
451+ self.check_path([self.user, self.site], ['+user', '-core'])
452+
453+ def test_disable_site(self):
454+ self.check_path([self.core], ['-site'])
455+ self.check_path([self.user, self.core], ['-site', '+user'])
456+
457+ def test_override_site(self):
458+ self.check_path(['mysite', self.user, self.core],
459+ ['mysite', '-site', '+user'])
460+ self.check_path(['mysite', self.core],
461+ ['mysite', '-site'])
462+
463+ def test_override_core(self):
464+ self.check_path(['mycore', self.user, self.site],
465+ ['mycore', '-core', '+user', '+site'])
466+ self.check_path(['mycore', self.site],
467+ ['mycore', '-core'])
468+
469+ def test_my_plugin_only(self):
470+ self.check_path(['myplugin'], ['myplugin', '-user', '-core', '-site'])
471+
472+ def test_my_plugin_first(self):
473+ self.check_path(['myplugin', self.core, self.site, self.user],
474+ ['myplugin', '+core', '+site', '+user'])
475+
476+ def test_bogus_references(self):
477+ self.check_path(['+foo', '-bar', self.core, self.site],
478+ ['+foo', '-bar'])
479
480=== modified file 'doc/en/user-guide/plugins.txt'
481--- doc/en/user-guide/plugins.txt 2007-12-14 07:35:49 +0000
482+++ doc/en/user-guide/plugins.txt 2009-08-31 04:35:31 +0000
483@@ -56,23 +56,11 @@
484 Alternative plugin locations
485 ----------------------------
486
487-If you have the necessary permissions, plugins can also be installed on
488-a system-wide basis. Two locations are currently checked for plugins:
489-
490- 1. the system location - bzrlib/plugins
491- 2. the personal location - $BZR_HOME/plugins.
492-
493-On a Linux installation, these locations are typically
494-``/usr/lib/python2.4/site-packages/bzrlib/plugins/`` and
495-``$HOME/.bazaar/plugins/``.
496-On a Windows installation, the system location might be
497-``C:\\Program Files\\Bazaar\\plugins``
498-while the personal location might be
499-``C:\Documents and Settings\<username>\Application Data\Bazaar\2.0\plugins``.
500-
501-One can additionally override the personal plugins location
502-by setting the environment variable ``BZR_PLUGIN_PATH``
503-to a directory that contains plugins.
504+If you have the necessary permissions, plugins can also be installed on a
505+system-wide basis. One can additionally override the personal plugins
506+location by setting the environment variable ``BZR_PLUGIN_PATH`` (see `User
507+Reference <../user-reference/bzr_man.html#bzr-plugin-path>`_ for a detailed
508+explanation).
509
510 Listing the installed plugins
511 -----------------------------
512
513=== modified file 'doc/en/user-guide/writing_a_plugin.txt'
514--- doc/en/user-guide/writing_a_plugin.txt 2008-10-15 19:34:48 +0000
515+++ doc/en/user-guide/writing_a_plugin.txt 2009-08-31 04:35:31 +0000
516@@ -32,11 +32,14 @@
517 Plugin searching rules
518 ----------------------
519
520-Bzr will scan ``bzrlib/plugins`` and ``~/.bazaar/plugins`` for plugins
521-by default. You can override this with ``BZR_PLUGIN_PATH``. Plugins
522-may be either modules or packages. If your plugin is a single file,
523-you can structure it as a module. If it has multiple files, or if you
524-want to distribute it as a bzr branch, you should structure it as a
525+Bzr will scan ``~/.bazaar/plugins`` and ``bzrlib/plugins`` for plugins
526+by default. You can override this with ``BZR_PLUGIN_PATH``
527+(see `User Reference <../user-reference/bzr_man.html#bzr-plugin-path>`_
528+for details).
529+
530+Plugins may be either modules or packages. If your plugin is a single
531+file, you can structure it as a module. If it has multiple files, or if
532+you want to distribute it as a bzr branch, you should structure it as a
533 package, i.e. a directory with an ``__init__.py`` file.
534
535 More information