[MERGE] allow 'import bzrlib.plugins.NAME'

[Robert Collins]

On Fri, 2007-02-02 at 13:43 -0600, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > This should be non-contentious: it just allows 'import
> > bzrlib.plugins.NAME' at any point after import bzrlib is performed.
> > 
> > This allows plugin X to depend on plugin Y, if Y has a predictable
> > name :).
> > 
> > No attempt is made to enforce naming policy or consistency.
> > 
> > There are two minor related changes.
> > 
> > One is that the zip entries on the path are now correctly scanned in
> > path order, rather than after all directories.
> > 
> > Secondly, plugins with names that are not legal python identifiers will
> > no longer load - and I consider this rather positive ;).
> 
> Well, we should at least give a warning about it. Because otherwise
> people will install, and not understand why it won't load.

We give the current warning. I can check for '-' in the name and warn
explicitly I guess.

=== modified file 'bzrlib/plugin.py'
--- bzrlib/plugin.py    2007-02-02 16:17:55 +0000
+++ bzrlib/plugin.py    2007-02-02 19:53:40 +0000
@@ -181,7 +181,11 @@
             raise
         except Exception, e:
             ## import pdb; pdb.set_trace()
-            warning('Unable to load plugin %r from %r' % (name, d))
+            if '-' in name:
+                warning('Unable to load plugin %r from %r: '
+                    'It is not a valid python module name.' % (name,
d))
+            else:
+                warning('Unable to load plugin %r from %r' % (name, d))
             log_exception_quietly()
 

> Especially since all Launchpad project names cannot contain underscore,
> so we have to default to using '-'. (Which is weird because I can use
> 'bzr register-branch' to register a branch with an underscore, and
> everything seems to work just fine. But if I try to set any other
> details like Status, I have to rename it first)
> 
> It looks like you will be giving a warning, but I'm wondering if it will
> be clear. If it is, then good enough.

The current one is:

~/source/baz/in-review/plugin-hook/bzr help
Unable to load plugin 'pqm-submit' from '/home/robertc/.bazaar/plugins'


> > +    for name in plugin_names:
> > +        try:
> > +            exec "import bzrlib.plugins.%s" % name
> > +        except KeyboardInterrupt:
> > +            raise
> > +        except Exception, e:
> > +            ## import pdb; pdb.set_trace()
> > +            warning('Unable to load plugin %r from %r' % (name, d))
> > +            log_exception_quietly()
> 
> Wouldn't __import__('bzrlib.plugins.%s' % name) be better? You don't
> really want 'bzrlib.plugins.foo' in the current namespace anyway.

Huh? It puts 'foo' into bzrlib.plugins - the global system module, which
is precisely correct.

> And I just tested, and it does put the module into sys.modules[], and it
> should also attach it to bzrlib.plugins.* properly.

So does the current code, but __import__ behaves different from 'import
foo' in some subtle ways I can't remember. So I'd rather have the more
obvious code.

Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070203/9d8d3d64/attachment.pgp