[PATCH] Search user plugin directories before site plugin directories

[Aaron Bentley]

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

John Arbash Meinel wrote:

> === modified file bzrlib/plugin.py
> --- bzrlib/plugin.py
> +++ bzrlib/plugin.py
> @@ -84,8 +84,8 @@
>      """Set the path for plugins to be loaded from."""
>      path = os.environ.get('BZR_PLUGIN_PATH',
>                            get_default_plugin_path()).split(os.pathsep)
> -    # search the bzrlib installed dir before anything else.
> -    path.insert(0, os.path.dirname(plugins.__file__))
> +    # search the plugin path before the bzrlib installed dir
> +    path.append(os.path.dirname(plugins.__file__))
>      plugins.__path__ = path
>      return path
> 
> ^^^^^ This very explicitly sets "bzrlib.plugins.__path__" which affects
> how python -c "import bzrlib.plugins.foo" works.
> 
> Basically, this is the *same* thing as sys.path. It is just local to a
> specific module.
> 
> Now, there is a small issue that it is *also* being used as the search
> path. However, if you look at the search code it does one loop to find
> all possible plugin names, and then a second import which doesn't care
> where it was found.
> 
> That means that if you have a "bzrlib/plugins/foo" and a
> ~/.bazaar/plugins/foo it will find the name "foo" and then do:
> 
> exec "import bzrlib.plugins.foo"
> 
> And at *that* time, it is important that we have ~/.bazaar/plugins in
> plugins.__path__ ahead of /usr/lib/.../bzrlib/plugins/
> 
> I think you are remembering the old plugin code, which used to look for
> plugins, and note what directory it was found in, and then explicitly
> load the plugin from that directory.

Yes.

> Robert changed it to use more of a standard "import bzrlib.plugins.foo"
> so that plugins could import each other. And as long as they do so after
> bzrlib.plugins.__path__ is set, it should "just work".

Plugins used to be loaded in order of discovery.  It appears that Robert
has broken this by using a set.  We need to get it back, because all
site-wide plugins should be loaded before user plugins.

e.g. if site-wide plugin "foo" overrides "diff" and user plugin plugin
"bar" overrides "diff", then "bar" should win.  If we load user plugins
first:

1. "bar" is loaded
2. "bar" overrides the builtin diff
3. "foo" is loaded
4. "foo" overrides "bar"'s diff

But it appears we are loading plugins in random order, since we're using
a set.  And so this patch doesn't make us any more broken than we
already were.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF3yCZ0F+nu1YWqI0RAq2DAJ923BazPuxLkZe+saapFJTfnO8T0wCeOZWN
gZRp/F4xvw2jYidx2X6w7ZU=
=vFnh
-----END PGP SIGNATURE-----