default plugin path

[Robert Collins]

On Mon, 2009-01-26 at 22:51 -0500, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > On Mon, 2009-01-26 at 22:12 -0500, Aaron Bentley wrote:
> >> Robert Collins wrote:
> >>>>> which is a dead chicken.
> >>>> If you mean it's voodoo, I disagree.  It's reasonably easy to understand.
> >>> Its easy to understand but its replicating logic that in the common case
> >>> isn't needed.
> >> There is no common case.
> > 
> > Sure there is - pqm, every microscript I've seen, bzr itself, all want
> > the bzrlib default path setting logic. AFAIK only launchpad and
> > bundlebuggy want something different.
> 
> Every bzrlib client I've worked on was afflicted by something like this.
>  Including hitchhiker.

Sounds like we've had diametrically opposed experiences. James
Henstridge on IRC just reported suffering a related thing - he thought
importing plugins wouldn't work *because he was using bzrlib 1.10*.

 
> >>>>> What harmful effect will setting the path have?
> >>>> It will cause bzrlib clients to load bzr plugins from the system path,
> >>>> which will be incompatible with their local version of bzrlib.
> >>> If I put a patch forward that doesn't spuriously add the system python
> >>> to the path
> >> Adding the system python to the path is *right* for many cases.  It's
> >> also *wrong* for many cases.  Therefore the client should decide.
> > 
> > Alternatively, which is what I'm proposing, the clients that consider it
> > wrong can just change it.
> 
> The client won't know that they consider it wrong until a bunch of their
> users break simultaneously due to a package upgrade.

I really don't follow here. Can you unpack what you mean?

> > Their code looks identical in both cases. So
> > its transparent to them.
> > 
> >>> (there is a bug in the path setting at the moment affecting
> >>> normal bzr as well as bzr clients), what harmful effects would setting
> >>> the path by default at module load have?
> >> It would be setting bzrlib to load plugins that the client author hasn't
> >> requested.
> > 
> > No. Its telling bzrlib *where* plugins are found, but in itself isn't
> > loading any.
> 
> Yes, exactly.

I really can't get any sense out of this - I disagree, and you say that
my statement about what its doing is correct. 

> > One of the following applies:
> >  - the client author doesn't load plugins at all - non issue
> >  - the client author wants the same plugins bzr loads - no action needed
> >    vis-a-vis the path
> >  - the client author wants a specific path - they set the path as 
> >    desired
> > 
> >>>> Launchpad and Bundle Buggy have both been hit by bad default plugin
> >>>> paths, even though Launchpad tried to control the plugin path.
> >>> I know that - but I'm not proposing to *load plugins*, only to set the
> >>> path.
> >> Which would mean that the wrong plugins would be loaded if we used the
> >> "from foo import bar" method.
> > 
> > The same logic is used for 'from bzrlib.plugins.foo import bar' as for
> > 'load_plugins()' - a search path is searched to find foo, and it is
> > imported into bzrlib.plugins.foo.
> 
> Right.  And the wrong version of a plugin can be loaded that way, when
> instead what you should get is an import error.

No version of a plugin is *also* wrong. And if the default path was
being used, which most library clients want, then they will *still* get
the wrong version of a plugin loaded, instead of an import error. There
are precisely three cases:
 - default path wanted
 - a specific path wanted
 - no plugins wanted

The third case is clearly uninteresting (just don't import anything from
any plugin).

The first case is what I am asserting is the common case. Any library
client wanting that case will before 1.10 have done nothing special, and
from 1.10 will be calling set_plugins_path, which is what is actually
causing the import failure you had to deal with, so they will *still be
broken*. And the second case, wanting a specific path is not affected at
all by setting the path by default.

> >>> And both can set it to whatever they like *anyway*.
> >> Yeah, but they need to remember to do that.  These are the sort of bad
> >> defaults that look right up until the point that stuff starts breaking
> >> for all users.
> > 
> > There are currently two bits of damage:
> >  - the path is wrong for *all users* including bzr itself. bug
> > https://bugs.edge.launchpad.net/bzr/+bug/316192 has discussion on this. 
> 
> I don't agree.  It looks completely correct for a system-installed bzr.

Sorry, I should have said 'all users of bzrlib which hasn't been system
installed including bzr running from source'.

> >  - clients have to know to call a function to allow callouts to *any*
> > plugin to work.
> 
> Clients are expecting that.  Having "from bzrlib.plugins import
> bzrtools" Just Work without any kind of set-up is weird.

I don't think its weird at all. Every plugin has a name, and its code is
accessible under that name. It works great.

> > I'm happy to make the second depend on fixing the first, to avoid
> > breaking clients like lp that were affected by bug 316192. But I don't
> > think that the situation today is better than what we had in e.g. bzrlib
> > 1.0!
> 
> And I do, and that's why I landed the patch I did.
> 
> > And the simplest comparison is that more code is needed now than
> > then - clearly a regression IMO.
> 
> I don't agree with the premise that requiring more code makes something
> worse.  I believe that explicit is better than implicit, even though it
> tends to be more wordy.

It is indeed in 'import this' - but its rule of thumb. Another great one
is sane defaults - and this default isn't sane because it works for
noone. 

-Rob


-- 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090127/8fbb9842/attachment.pgp