Merge lp:~bzr/bzr/412930-plugin-path into lp:~bzr/bzr/trunk-old
- 412930-plugin-path
- Merge into trunk-old
Status: | Merged | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Approved by: | John A Meinel | ||||||||||||
Approved revision: | no longer in the source branch. | ||||||||||||
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 | ||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
John A Meinel | Approve | ||
Review via email: mp+10518@code.launchpad.net |
This proposal supersedes a proposal from 2009-08-20.
Commit message
Description of the change
Vincent Ladeuil (vila) wrote : Posted in a previous version of this proposal | # |
John A Meinel (jameinel) wrote : Posted in a previous version of this proposal | # |
-----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_
Should ignore the site path. I would actually say 'last value wins'. So
that you can just do:
BZR_PLUGIN_
I'm not stuck on this, but I certainly find it hard to follow exactly
what is going on in your get_standard_
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_
Would then search site locations before user locations.
I would probably do something more like:
env_path = os.environ.
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:
elif path == '-core':
...
elif path.startswith
if path == '+site':
default_site = False
if '+site' in plugin_paths:
elif path == '+core':
...
plugin_
else:
...
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://
iEYEARECAAYFAkq
5y4An0jzQ0IjHY4
=WfNs
-----END PGP SIGNATURE-----
Vincent Ladeuil (vila) wrote : Posted in a previous version of this proposal | # |
>>>>> "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:/
https:/
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_
jam> Should ignore the site path. I would actually say 'last
jam> value wins'. So that you can just do:
jam> BZR_PLUGIN_
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-
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-
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/
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-
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\
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_
> (but we may
> want to say it's 'All Users/Application Data/Bazaar/
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_
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) : | # |
Preview Diff
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 | 237 | New Features | 237 | New Features |
6 | 238 | ************ | 238 | ************ |
7 | 239 | 239 | ||
8 | 240 | * Give more control on BZR_PLUGIN_PATH by providing a way to refer to or | ||
9 | 241 | disable the user, site and core plugin directories. | ||
10 | 242 | (Vincent Ladeuil, #412930, #316192, #145612) | ||
11 | 243 | |||
12 | 244 | |||
13 | 240 | Bug Fixes | 245 | Bug Fixes |
14 | 241 | ********* | 246 | ********* |
15 | 242 | 247 | ||
16 | 243 | 248 | ||
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 | 63 | ~~~~~~~~~~~~~~~ | 63 | ~~~~~~~~~~~~~~~ |
22 | 64 | 64 | ||
23 | 65 | The path to the plugins directory that Bazaar should use. | 65 | The path to the plugins directory that Bazaar should use. |
24 | 66 | If not set, Bazaar will search for plugins in: | ||
25 | 67 | |||
26 | 68 | * the user specific plugin directory (containing the ``user`` plugins), | ||
27 | 69 | |||
28 | 70 | * the bzrlib directory (containing the ``core`` plugins), | ||
29 | 71 | |||
30 | 72 | * the site specific plugin directory if applicable (containing | ||
31 | 73 | the ``site`` plugins). | ||
32 | 74 | |||
33 | 75 | If ``BZR_PLUGIN_PATH`` is set in any fashion, it will change the | ||
34 | 76 | the way the plugin are searched. | ||
35 | 77 | |||
36 | 78 | As for the ``PATH`` variables, if multiple directories are | ||
37 | 79 | specified in ``BZR_PLUGIN_PATH`` they should be separated by the | ||
38 | 80 | platform specific appropriate character (':' on Unix/Linux/etc, | ||
39 | 81 | ';' on windows) | ||
40 | 82 | |||
41 | 83 | By default if ``BZR_PLUGIN_PATH`` is set, it replaces searching | ||
42 | 84 | in ``user``. However it will continue to search in ``core`` and | ||
43 | 85 | ``site`` unless they are explicitly removed. | ||
44 | 86 | |||
45 | 87 | If you need to change the order or remove one of these | ||
46 | 88 | directories, you should use special values: | ||
47 | 89 | |||
48 | 90 | * ``-user``, ``-core``, ``-site`` will remove the corresponding | ||
49 | 91 | path from the default values, | ||
50 | 92 | |||
51 | 93 | * ``+user``, ``+core``, ``+site`` will add the corresponding path | ||
52 | 94 | before the remaining default values (and also remove it from | ||
53 | 95 | the default values). | ||
54 | 96 | |||
55 | 97 | Note that the special values 'user', 'core' and 'site' should be | ||
56 | 98 | used literally, they will be substituted by the corresponding, | ||
57 | 99 | platform specific, values. | ||
58 | 100 | |||
59 | 101 | Examples: | ||
60 | 102 | ^^^^^^^^^ | ||
61 | 103 | |||
62 | 104 | The examples below uses ':' as the separator, windows users | ||
63 | 105 | should use ';'. | ||
64 | 106 | |||
65 | 107 | Overriding the default user plugin directory: | ||
66 | 108 | ``BZR_PLUGIN_PATH='/path/to/my/other/plugins'`` | ||
67 | 109 | |||
68 | 110 | Disabling the site directory while retaining the user directory: | ||
69 | 111 | ``BZR_PLUGIN_PATH='-site:+user'`` | ||
70 | 112 | |||
71 | 113 | Disabling all plugins (better achieved with --no-plugins): | ||
72 | 114 | ``BZR_PLUGIN_PATH='-user:-core:-site'`` | ||
73 | 115 | |||
74 | 116 | Overriding the default site plugin directory: | ||
75 | 117 | ``BZR_PLUGIN_PATH='/path/to/my/site/plugins:-site':+user`` | ||
76 | 118 | |||
77 | 119 | |||
78 | 66 | 120 | ||
79 | 67 | BZRPATH | 121 | BZRPATH |
80 | 68 | ~~~~~~~ | 122 | ~~~~~~~ |
81 | 69 | 123 | ||
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 | 52 | from bzrlib import plugins as _mod_plugins | 52 | from bzrlib import plugins as _mod_plugins |
87 | 53 | """) | 53 | """) |
88 | 54 | 54 | ||
90 | 55 | from bzrlib.symbol_versioning import deprecated_function | 55 | from bzrlib.symbol_versioning import ( |
91 | 56 | deprecated_function, | ||
92 | 57 | deprecated_in, | ||
93 | 58 | ) | ||
94 | 56 | 59 | ||
95 | 57 | 60 | ||
96 | 58 | DEFAULT_PLUGIN_PATH = None | 61 | DEFAULT_PLUGIN_PATH = None |
97 | 59 | _loaded = False | 62 | _loaded = False |
98 | 60 | 63 | ||
99 | 64 | @deprecated_function(deprecated_in((2, 0, 0))) | ||
100 | 61 | def get_default_plugin_path(): | 65 | def get_default_plugin_path(): |
101 | 62 | """Get the DEFAULT_PLUGIN_PATH""" | 66 | """Get the DEFAULT_PLUGIN_PATH""" |
102 | 63 | global DEFAULT_PLUGIN_PATH | 67 | global DEFAULT_PLUGIN_PATH |
103 | @@ -91,13 +95,15 @@ | |||
104 | 91 | return path | 95 | return path |
105 | 92 | 96 | ||
106 | 93 | 97 | ||
114 | 94 | def get_standard_plugins_path(): | 98 | def _append_new_path(paths, new_path): |
115 | 95 | """Determine a plugin path suitable for general use.""" | 99 | """Append a new path if it set and not already known.""" |
116 | 96 | path = os.environ.get('BZR_PLUGIN_PATH', | 100 | if new_path is not None and new_path not in paths: |
117 | 97 | get_default_plugin_path()).split(os.pathsep) | 101 | paths.append(new_path) |
118 | 98 | # Get rid of trailing slashes, since Python can't handle them when | 102 | return paths |
119 | 99 | # it tries to import modules. | 103 | |
120 | 100 | path = map(_strip_trailing_sep, path) | 104 | |
121 | 105 | def get_core_plugin_path(): | ||
122 | 106 | core_path = None | ||
123 | 101 | bzr_exe = bool(getattr(sys, 'frozen', None)) | 107 | bzr_exe = bool(getattr(sys, 'frozen', None)) |
124 | 102 | if bzr_exe: # expand path for bzr.exe | 108 | if bzr_exe: # expand path for bzr.exe |
125 | 103 | # We need to use relative path to system-wide plugin | 109 | # We need to use relative path to system-wide plugin |
126 | @@ -110,25 +116,83 @@ | |||
127 | 110 | # then plugins directory is | 116 | # then plugins directory is |
128 | 111 | # C:\Program Files\Bazaar\plugins | 117 | # C:\Program Files\Bazaar\plugins |
129 | 112 | # so relative path is ../../../plugins | 118 | # so relative path is ../../../plugins |
133 | 113 | path.append(osutils.abspath(osutils.pathjoin( | 119 | core_path = osutils.abspath(osutils.pathjoin( |
134 | 114 | osutils.dirname(__file__), '../../../plugins'))) | 120 | osutils.dirname(__file__), '../../../plugins')) |
135 | 115 | if not bzr_exe: # don't look inside library.zip | 121 | else: # don't look inside library.zip |
136 | 116 | # search the plugin path before the bzrlib installed dir | 122 | # search the plugin path before the bzrlib installed dir |
152 | 117 | path.append(os.path.dirname(_mod_plugins.__file__)) | 123 | core_path = os.path.dirname(_mod_plugins.__file__) |
153 | 118 | # search the arch independent path if we can determine that and | 124 | return core_path |
154 | 119 | # the plugin is found nowhere else | 125 | |
155 | 120 | if sys.platform != 'win32': | 126 | |
156 | 121 | try: | 127 | def get_site_plugin_path(): |
157 | 122 | from distutils.sysconfig import get_python_lib | 128 | """Returns the path for the site installed plugins.""" |
158 | 123 | except ImportError: | 129 | if sys.platform == 'win32': |
159 | 124 | # If distutuils is not available, we just won't add that path | 130 | # We don't have (yet) a good answer for windows since that is certainly |
160 | 125 | pass | 131 | # related to the way we build the installers. -- vila20090821 |
161 | 126 | else: | 132 | return None |
162 | 127 | archless_path = osutils.pathjoin(get_python_lib(), 'bzrlib', | 133 | site_path = None |
163 | 128 | 'plugins') | 134 | try: |
164 | 129 | if archless_path not in path: | 135 | from distutils.sysconfig import get_python_lib |
165 | 130 | path.append(archless_path) | 136 | except ImportError: |
166 | 131 | return path | 137 | # If distutuils is not available, we just don't know where they are |
167 | 138 | pass | ||
168 | 139 | else: | ||
169 | 140 | site_path = osutils.pathjoin(get_python_lib(), 'bzrlib', 'plugins') | ||
170 | 141 | return site_path | ||
171 | 142 | |||
172 | 143 | |||
173 | 144 | def get_user_plugin_path(): | ||
174 | 145 | return osutils.pathjoin(config.config_dir(), 'plugins') | ||
175 | 146 | |||
176 | 147 | |||
177 | 148 | def get_standard_plugins_path(): | ||
178 | 149 | """Determine a plugin path suitable for general use.""" | ||
179 | 150 | # Ad-Hoc default: core is not overriden by site but user can overrides both | ||
180 | 151 | # The rationale is that: | ||
181 | 152 | # - 'site' comes last, because these plugins should always be available and | ||
182 | 153 | # are supposed to be in sync with the bzr installed on site. | ||
183 | 154 | # - 'core' comes before 'site' so that running bzr from sources or a user | ||
184 | 155 | # installed version overrides the site version. | ||
185 | 156 | # - 'user' comes first, because... user is always right. | ||
186 | 157 | # - the above rules clearly defines which plugin version will be loaded if | ||
187 | 158 | # several exist. Yet, it is sometimes desirable to disable some directory | ||
188 | 159 | # so that a set of plugin is disabled as once. This can be done via | ||
189 | 160 | # -site, -core, -user. | ||
190 | 161 | |||
191 | 162 | env_paths = os.environ.get('BZR_PLUGIN_PATH', '+user').split(os.pathsep) | ||
192 | 163 | defaults = ['+core', '+site'] | ||
193 | 164 | |||
194 | 165 | # The predefined references | ||
195 | 166 | refs = dict(core=get_core_plugin_path(), | ||
196 | 167 | site=get_site_plugin_path(), | ||
197 | 168 | user=get_user_plugin_path()) | ||
198 | 169 | |||
199 | 170 | # Unset paths that should be removed | ||
200 | 171 | for k,v in refs.iteritems(): | ||
201 | 172 | removed = '-%s' % k | ||
202 | 173 | # defaults can never mention removing paths as that will make it | ||
203 | 174 | # impossible for the user to revoke these removals. | ||
204 | 175 | if removed in env_paths: | ||
205 | 176 | env_paths.remove(removed) | ||
206 | 177 | refs[k] = None | ||
207 | 178 | |||
208 | 179 | # Expand references | ||
209 | 180 | paths = [] | ||
210 | 181 | for p in env_paths + defaults: | ||
211 | 182 | if p.startswith('+'): | ||
212 | 183 | # Resolve reference if they are known | ||
213 | 184 | try: | ||
214 | 185 | p = refs[p[1:]] | ||
215 | 186 | except KeyError: | ||
216 | 187 | # Leave them untouched otherwise, user may have paths starting | ||
217 | 188 | # with '+'... | ||
218 | 189 | pass | ||
219 | 190 | _append_new_path(paths, p) | ||
220 | 191 | |||
221 | 192 | # Get rid of trailing slashes, since Python can't handle them when | ||
222 | 193 | # it tries to import modules. | ||
223 | 194 | paths = map(_strip_trailing_sep, paths) | ||
224 | 195 | return paths | ||
225 | 132 | 196 | ||
226 | 133 | 197 | ||
227 | 134 | def load_plugins(path=None): | 198 | def load_plugins(path=None): |
228 | 135 | 199 | ||
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 | 26 | import sys | 26 | import sys |
234 | 27 | import zipfile | 27 | import zipfile |
235 | 28 | 28 | ||
237 | 29 | from bzrlib import plugin, tests | 29 | from bzrlib import ( |
238 | 30 | osutils, | ||
239 | 31 | plugin, | ||
240 | 32 | tests, | ||
241 | 33 | ) | ||
242 | 30 | import bzrlib.plugin | 34 | import bzrlib.plugin |
243 | 31 | import bzrlib.plugins | 35 | import bzrlib.plugins |
244 | 32 | import bzrlib.commands | 36 | import bzrlib.commands |
245 | @@ -454,41 +458,6 @@ | |||
246 | 454 | delattr(bzrlib.plugins, 'myplug') | 458 | delattr(bzrlib.plugins, 'myplug') |
247 | 455 | 459 | ||
248 | 456 | 460 | ||
249 | 457 | class TestSetPluginsPath(TestCase): | ||
250 | 458 | |||
251 | 459 | def test_set_plugins_path(self): | ||
252 | 460 | """set_plugins_path should set the module __path__ correctly.""" | ||
253 | 461 | old_path = bzrlib.plugins.__path__ | ||
254 | 462 | try: | ||
255 | 463 | bzrlib.plugins.__path__ = [] | ||
256 | 464 | expected_path = bzrlib.plugin.set_plugins_path() | ||
257 | 465 | self.assertEqual(expected_path, bzrlib.plugins.__path__) | ||
258 | 466 | finally: | ||
259 | 467 | bzrlib.plugins.__path__ = old_path | ||
260 | 468 | |||
261 | 469 | def test_set_plugins_path_with_trailing_slashes(self): | ||
262 | 470 | """set_plugins_path should set the module __path__ based on | ||
263 | 471 | BZR_PLUGIN_PATH after removing all trailing slashes.""" | ||
264 | 472 | old_path = bzrlib.plugins.__path__ | ||
265 | 473 | old_env = os.environ.get('BZR_PLUGIN_PATH') | ||
266 | 474 | try: | ||
267 | 475 | bzrlib.plugins.__path__ = [] | ||
268 | 476 | os.environ['BZR_PLUGIN_PATH'] = "first\\//\\" + os.pathsep + \ | ||
269 | 477 | "second/\\/\\/" | ||
270 | 478 | bzrlib.plugin.set_plugins_path() | ||
271 | 479 | # We expect our nominated paths to have all path-seps removed, | ||
272 | 480 | # and this is testing only that. | ||
273 | 481 | expected_path = ['first', 'second'] | ||
274 | 482 | self.assertEqual(expected_path, | ||
275 | 483 | bzrlib.plugins.__path__[:len(expected_path)]) | ||
276 | 484 | finally: | ||
277 | 485 | bzrlib.plugins.__path__ = old_path | ||
278 | 486 | if old_env is not None: | ||
279 | 487 | os.environ['BZR_PLUGIN_PATH'] = old_env | ||
280 | 488 | else: | ||
281 | 489 | del os.environ['BZR_PLUGIN_PATH'] | ||
282 | 490 | |||
283 | 491 | |||
284 | 492 | class TestHelpIndex(tests.TestCase): | 461 | class TestHelpIndex(tests.TestCase): |
285 | 493 | """Tests for the PluginsHelpIndex class.""" | 462 | """Tests for the PluginsHelpIndex class.""" |
286 | 494 | 463 | ||
287 | @@ -597,41 +566,42 @@ | |||
288 | 597 | self.assertEqual('foo_bar', topic.get_help_topic()) | 566 | self.assertEqual('foo_bar', topic.get_help_topic()) |
289 | 598 | 567 | ||
290 | 599 | 568 | ||
311 | 600 | def clear_plugins(test_case): | 569 | class TestLoadFromPath(tests.TestCaseInTempDir): |
312 | 601 | # Save the attributes that we're about to monkey-patch. | 570 | |
313 | 602 | old_plugins_path = bzrlib.plugins.__path__ | 571 | def setUp(self): |
314 | 603 | old_loaded = plugin._loaded | 572 | super(TestLoadFromPath, self).setUp() |
315 | 604 | old_load_from_path = plugin.load_from_path | 573 | # Save the attributes that we're about to monkey-patch. |
316 | 605 | # Change bzrlib.plugin to think no plugins have been loaded yet. | 574 | old_plugins_path = bzrlib.plugins.__path__ |
317 | 606 | bzrlib.plugins.__path__ = [] | 575 | old_loaded = plugin._loaded |
318 | 607 | plugin._loaded = False | 576 | old_load_from_path = plugin.load_from_path |
319 | 608 | # Monkey-patch load_from_path to stop it from actually loading anything. | 577 | |
320 | 609 | def load_from_path(dirs): | 578 | def restore(): |
321 | 610 | pass | 579 | bzrlib.plugins.__path__ = old_plugins_path |
322 | 611 | plugin.load_from_path = load_from_path | 580 | plugin._loaded = old_loaded |
323 | 612 | def restore_plugins(): | 581 | plugin.load_from_path = old_load_from_path |
324 | 613 | bzrlib.plugins.__path__ = old_plugins_path | 582 | |
325 | 614 | plugin._loaded = old_loaded | 583 | self.addCleanup(restore) |
326 | 615 | plugin.load_from_path = old_load_from_path | 584 | |
327 | 616 | test_case.addCleanup(restore_plugins) | 585 | # Change bzrlib.plugin to think no plugins have been loaded yet. |
328 | 617 | 586 | bzrlib.plugins.__path__ = [] | |
329 | 618 | 587 | plugin._loaded = False | |
330 | 619 | class TestPluginPaths(tests.TestCase): | 588 | |
331 | 589 | # Monkey-patch load_from_path to stop it from actually loading anything. | ||
332 | 590 | def load_from_path(dirs): | ||
333 | 591 | pass | ||
334 | 592 | plugin.load_from_path = load_from_path | ||
335 | 620 | 593 | ||
336 | 621 | def test_set_plugins_path_with_args(self): | 594 | def test_set_plugins_path_with_args(self): |
337 | 622 | clear_plugins(self) | ||
338 | 623 | plugin.set_plugins_path(['a', 'b']) | 595 | plugin.set_plugins_path(['a', 'b']) |
339 | 624 | self.assertEqual(['a', 'b'], bzrlib.plugins.__path__) | 596 | self.assertEqual(['a', 'b'], bzrlib.plugins.__path__) |
340 | 625 | 597 | ||
341 | 626 | def test_set_plugins_path_defaults(self): | 598 | def test_set_plugins_path_defaults(self): |
342 | 627 | clear_plugins(self) | ||
343 | 628 | plugin.set_plugins_path() | 599 | plugin.set_plugins_path() |
344 | 629 | self.assertEqual(plugin.get_standard_plugins_path(), | 600 | self.assertEqual(plugin.get_standard_plugins_path(), |
345 | 630 | bzrlib.plugins.__path__) | 601 | bzrlib.plugins.__path__) |
346 | 631 | 602 | ||
347 | 632 | def test_get_standard_plugins_path(self): | 603 | def test_get_standard_plugins_path(self): |
348 | 633 | path = plugin.get_standard_plugins_path() | 604 | path = plugin.get_standard_plugins_path() |
349 | 634 | self.assertEqual(plugin.get_default_plugin_path(), path[0]) | ||
350 | 635 | for directory in path: | 605 | for directory in path: |
351 | 636 | self.assertNotContainsRe(directory, r'\\/$') | 606 | self.assertNotContainsRe(directory, r'\\/$') |
352 | 637 | try: | 607 | try: |
353 | @@ -649,13 +619,11 @@ | |||
354 | 649 | 619 | ||
355 | 650 | def test_get_standard_plugins_path_env(self): | 620 | def test_get_standard_plugins_path_env(self): |
356 | 651 | os.environ['BZR_PLUGIN_PATH'] = 'foo/' | 621 | os.environ['BZR_PLUGIN_PATH'] = 'foo/' |
361 | 652 | self.assertEqual('foo', plugin.get_standard_plugins_path()[0]) | 622 | path = plugin.get_standard_plugins_path() |
362 | 653 | 623 | for directory in path: | |
363 | 654 | 624 | self.assertNotContainsRe(directory, r'\\/$') | |
360 | 655 | class TestLoadPlugins(tests.TestCaseInTempDir): | ||
364 | 656 | 625 | ||
365 | 657 | def test_load_plugins(self): | 626 | def test_load_plugins(self): |
366 | 658 | clear_plugins(self) | ||
367 | 659 | plugin.load_plugins(['.']) | 627 | plugin.load_plugins(['.']) |
368 | 660 | self.assertEqual(bzrlib.plugins.__path__, ['.']) | 628 | self.assertEqual(bzrlib.plugins.__path__, ['.']) |
369 | 661 | # subsequent loads are no-ops | 629 | # subsequent loads are no-ops |
370 | @@ -663,7 +631,107 @@ | |||
371 | 663 | self.assertEqual(bzrlib.plugins.__path__, ['.']) | 631 | self.assertEqual(bzrlib.plugins.__path__, ['.']) |
372 | 664 | 632 | ||
373 | 665 | def test_load_plugins_default(self): | 633 | def test_load_plugins_default(self): |
374 | 666 | clear_plugins(self) | ||
375 | 667 | plugin.load_plugins() | 634 | plugin.load_plugins() |
376 | 668 | path = plugin.get_standard_plugins_path() | 635 | path = plugin.get_standard_plugins_path() |
377 | 669 | self.assertEqual(path, bzrlib.plugins.__path__) | 636 | self.assertEqual(path, bzrlib.plugins.__path__) |
378 | 637 | |||
379 | 638 | |||
380 | 639 | class TestEnvPluginPath(tests.TestCaseInTempDir): | ||
381 | 640 | |||
382 | 641 | def setUp(self): | ||
383 | 642 | super(TestEnvPluginPath, self).setUp() | ||
384 | 643 | old_default = plugin.DEFAULT_PLUGIN_PATH | ||
385 | 644 | |||
386 | 645 | def restore(): | ||
387 | 646 | plugin.DEFAULT_PLUGIN_PATH = old_default | ||
388 | 647 | |||
389 | 648 | self.addCleanup(restore) | ||
390 | 649 | |||
391 | 650 | plugin.DEFAULT_PLUGIN_PATH = None | ||
392 | 651 | |||
393 | 652 | self.user = plugin.get_user_plugin_path() | ||
394 | 653 | self.site = plugin.get_site_plugin_path() | ||
395 | 654 | self.core = plugin.get_core_plugin_path() | ||
396 | 655 | |||
397 | 656 | def _list2paths(self, *args): | ||
398 | 657 | paths = [] | ||
399 | 658 | for p in args: | ||
400 | 659 | plugin._append_new_path(paths, p) | ||
401 | 660 | return paths | ||
402 | 661 | |||
403 | 662 | def _set_path(self, *args): | ||
404 | 663 | path = os.pathsep.join(self._list2paths(*args)) | ||
405 | 664 | osutils.set_or_unset_env('BZR_PLUGIN_PATH', path) | ||
406 | 665 | |||
407 | 666 | def check_path(self, expected_dirs, setting_dirs): | ||
408 | 667 | if setting_dirs: | ||
409 | 668 | self._set_path(*setting_dirs) | ||
410 | 669 | actual = plugin.get_standard_plugins_path() | ||
411 | 670 | self.assertEquals(self._list2paths(*expected_dirs), actual) | ||
412 | 671 | |||
413 | 672 | def test_default(self): | ||
414 | 673 | self.check_path([self.user, self.core, self.site], | ||
415 | 674 | None) | ||
416 | 675 | |||
417 | 676 | def test_adhoc_policy(self): | ||
418 | 677 | self.check_path([self.user, self.core, self.site], | ||
419 | 678 | ['+user', '+core', '+site']) | ||
420 | 679 | |||
421 | 680 | def test_fallback_policy(self): | ||
422 | 681 | self.check_path([self.core, self.site, self.user], | ||
423 | 682 | ['+core', '+site', '+user']) | ||
424 | 683 | |||
425 | 684 | def test_override_policy(self): | ||
426 | 685 | self.check_path([self.user, self.site, self.core], | ||
427 | 686 | ['+user', '+site', '+core']) | ||
428 | 687 | |||
429 | 688 | def test_disable_user(self): | ||
430 | 689 | self.check_path([self.core, self.site], ['-user']) | ||
431 | 690 | |||
432 | 691 | def test_disable_user_twice(self): | ||
433 | 692 | # Ensures multiple removals don't left cruft | ||
434 | 693 | self.check_path([self.core, self.site], ['-user', '-user']) | ||
435 | 694 | |||
436 | 695 | def test_duplicates_are_removed(self): | ||
437 | 696 | self.check_path([self.user, self.core, self.site], | ||
438 | 697 | ['+user', '+user']) | ||
439 | 698 | # And only the first reference is kept (since the later references will | ||
440 | 699 | # onnly produce <plugin> already loaded mutters) | ||
441 | 700 | self.check_path([self.user, self.core, self.site], | ||
442 | 701 | ['+user', '+user', '+core', | ||
443 | 702 | '+user', '+site', '+site', | ||
444 | 703 | '+core']) | ||
445 | 704 | |||
446 | 705 | def test_disable_overrides_disable(self): | ||
447 | 706 | self.check_path([self.core, self.site], ['-user', '+user']) | ||
448 | 707 | |||
449 | 708 | def test_disable_core(self): | ||
450 | 709 | self.check_path([self.site], ['-core']) | ||
451 | 710 | self.check_path([self.user, self.site], ['+user', '-core']) | ||
452 | 711 | |||
453 | 712 | def test_disable_site(self): | ||
454 | 713 | self.check_path([self.core], ['-site']) | ||
455 | 714 | self.check_path([self.user, self.core], ['-site', '+user']) | ||
456 | 715 | |||
457 | 716 | def test_override_site(self): | ||
458 | 717 | self.check_path(['mysite', self.user, self.core], | ||
459 | 718 | ['mysite', '-site', '+user']) | ||
460 | 719 | self.check_path(['mysite', self.core], | ||
461 | 720 | ['mysite', '-site']) | ||
462 | 721 | |||
463 | 722 | def test_override_core(self): | ||
464 | 723 | self.check_path(['mycore', self.user, self.site], | ||
465 | 724 | ['mycore', '-core', '+user', '+site']) | ||
466 | 725 | self.check_path(['mycore', self.site], | ||
467 | 726 | ['mycore', '-core']) | ||
468 | 727 | |||
469 | 728 | def test_my_plugin_only(self): | ||
470 | 729 | self.check_path(['myplugin'], ['myplugin', '-user', '-core', '-site']) | ||
471 | 730 | |||
472 | 731 | def test_my_plugin_first(self): | ||
473 | 732 | self.check_path(['myplugin', self.core, self.site, self.user], | ||
474 | 733 | ['myplugin', '+core', '+site', '+user']) | ||
475 | 734 | |||
476 | 735 | def test_bogus_references(self): | ||
477 | 736 | self.check_path(['+foo', '-bar', self.core, self.site], | ||
478 | 737 | ['+foo', '-bar']) | ||
479 | 670 | 738 | ||
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 | 56 | Alternative plugin locations | 56 | Alternative plugin locations |
485 | 57 | ---------------------------- | 57 | ---------------------------- |
486 | 58 | 58 | ||
504 | 59 | If you have the necessary permissions, plugins can also be installed on | 59 | If you have the necessary permissions, plugins can also be installed on a |
505 | 60 | a system-wide basis. Two locations are currently checked for plugins: | 60 | system-wide basis. One can additionally override the personal plugins |
506 | 61 | 61 | location by setting the environment variable ``BZR_PLUGIN_PATH`` (see `User | |
507 | 62 | 1. the system location - bzrlib/plugins | 62 | Reference <../user-reference/bzr_man.html#bzr-plugin-path>`_ for a detailed |
508 | 63 | 2. the personal location - $BZR_HOME/plugins. | 63 | explanation). |
492 | 64 | |||
493 | 65 | On a Linux installation, these locations are typically | ||
494 | 66 | ``/usr/lib/python2.4/site-packages/bzrlib/plugins/`` and | ||
495 | 67 | ``$HOME/.bazaar/plugins/``. | ||
496 | 68 | On a Windows installation, the system location might be | ||
497 | 69 | ``C:\\Program Files\\Bazaar\\plugins`` | ||
498 | 70 | while the personal location might be | ||
499 | 71 | ``C:\Documents and Settings\<username>\Application Data\Bazaar\2.0\plugins``. | ||
500 | 72 | |||
501 | 73 | One can additionally override the personal plugins location | ||
502 | 74 | by setting the environment variable ``BZR_PLUGIN_PATH`` | ||
503 | 75 | to a directory that contains plugins. | ||
509 | 76 | 64 | ||
510 | 77 | Listing the installed plugins | 65 | Listing the installed plugins |
511 | 78 | ----------------------------- | 66 | ----------------------------- |
512 | 79 | 67 | ||
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 | 32 | Plugin searching rules | 32 | Plugin searching rules |
518 | 33 | ---------------------- | 33 | ---------------------- |
519 | 34 | 34 | ||
525 | 35 | Bzr will scan ``bzrlib/plugins`` and ``~/.bazaar/plugins`` for plugins | 35 | Bzr will scan ``~/.bazaar/plugins`` and ``bzrlib/plugins`` for plugins |
526 | 36 | by default. You can override this with ``BZR_PLUGIN_PATH``. Plugins | 36 | by default. You can override this with ``BZR_PLUGIN_PATH`` |
527 | 37 | may be either modules or packages. If your plugin is a single file, | 37 | (see `User Reference <../user-reference/bzr_man.html#bzr-plugin-path>`_ |
528 | 38 | you can structure it as a module. If it has multiple files, or if you | 38 | for details). |
529 | 39 | want to distribute it as a bzr branch, you should structure it as a | 39 | |
530 | 40 | Plugins may be either modules or packages. If your plugin is a single | ||
531 | 41 | file, you can structure it as a module. If it has multiple files, or if | ||
532 | 42 | you want to distribute it as a bzr branch, you should structure it as a | ||
533 | 40 | package, i.e. a directory with an ``__init__.py`` file. | 43 | package, i.e. a directory with an ``__init__.py`` file. |
534 | 41 | 44 | ||
535 | 42 | More information | 45 | More information |
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 :-)