Code review comment for lp:~bzr/bzr/412930-plugin-path

Revision history for this message
Alexander Belchenko (bialix) wrote :

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

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\Lib\site-packages\bzrlib\plugins

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_DIR)\plugins\, e.g. C:\Program Files\Bazaar\plugins

> (but we may
> want to say it's 'All Users/Application Data/Bazaar/Plugins')

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.

« Back to merge proposal