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

[John Arbash Meinel]

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

Robert Collins wrote:
> On Fri, 2007-02-02 at 13:43 -0600, John Arbash Meinel wrote:
>
..

>> 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.

What is the traceback in ~/.bzr.log? Having "unable to load" at least
tells someone something is wrong.

If you are going to have a check, I would actually prefer:

if not re.match('\w+'):
  warning('Unable to load plugin %r from %r: '
          'It is not a valid python module name.' % (name, d))

That way we handle 'foo.bar' which is also invalid. And might actually
cause problems based on what you are doing (import bzrlib.foo.bar)
imports something different.

> 
> === 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()
>  

...

>>> +    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


__import__ returns something different than 'import' does. I think it
has more to do with what it does to locals() and globals() than anything
else.

I just avoid exec() and eval(). If I see it, it makes me wonder if the
code is correct.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFw5nlJdeBCYSNAAMRAn4sAJ9TFp++ebelINv/sI/UQitBMPSBXQCfQM8b
fHRsFIUG6fLRhp8PToSHuIM=
=IRJO
-----END PGP SIGNATURE-----