buildout/distribute recursion error fixed

[Reinout van Rees]

Hi all,

Several people have seen a hard-to-reproduce recursion error when running
buildout in combination with distribute.  You'd get 200 lines of
"distribute".

Good news: the problem has been identified.  And I've fixed it on a
zc.buildout branch (branches/reinout_distribute_upgrade_check).  Tarek will
test it and probably/hopefully release it real soon, wink, wink.


Buildout's easy_install.py module detects the location of setuptools and
buildout at import time and uses those paths when creating scripts.
Setuptools's path is also used when calling easy_install to install .tgz
files.

Problem: the install() method that calls easy_install detects the setuptools
location all over again at runtime.  And there's a recursion in there.  The
method that searches for the available setuptools detects that there's a new
version available and tries to install() it.  And searches for the setuptools
location.  And tries to install() it.  Repeat.

I fixed it by simply using the pre-detected setuptools location.

Note that setuptools did not have this recursion problem solely because it was
packaged as an ``.egg``, which does not have to go through the easy_install
step!  So distribute's (necessary) packaging as a tgz exposed this bug.


Reinout

--
Reinout van Rees - reinout@... - http://reinout.vanrees.org
Software developer at http://www.thehealthagency.com
"Military engineers build missiles. Civil engineers build targets"

_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@...
http://mail.python.org/mailman/listinfo/distutils-sig

[Tarek Ziadé]

On Thu, Nov 5, 2009 at 9:32 AM, Reinout van Rees <reinout@...> wrote:
> Hi all,
>
> Several people have seen a hard-to-reproduce recursion error when running
> buildout in combination with distribute.  You'd get 200 lines of
> "distribute".
>
> Good news: the problem has been identified.  And I've fixed it on a
> zc.buildout branch (branches/reinout_distribute_upgrade_check).  Tarek will
> test it and probably/hopefully release it real soon, wink, wink.

Yes, thanks for the fix/work ! It looks nice and also simplifies the code.

Before it's merged in trunk and eventually released, we need to :

- add a test demonstrating this bug in easy_install.txt

- manually test the various scenarii (these are not fully covered with
zc.buildout unittests because the assmption is made that python don't
have setuptools installed):

    - nude python, buildout without --distribute, plone buildout cfg file
    - python with setuptools, buildout witout --distribute, plone
buildout cfg file
    - python with distribute < 0.6.8, buildout witout --distribute,
plone buildout cfg file
    - these 3 tests *with* --distribute

Regards
Tarek
_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@...
http://mail.python.org/mailman/listinfo/distutils-sig

[Reinout van Rees]

On 2009-11-05, Tarek Ziadé <ziade.tarek@...> wrote:

> On Thu, Nov 5, 2009 at 9:32 AM, Reinout van Rees <reinout@...> wrote:
>> Hi all,
>>
>> Several people have seen a hard-to-reproduce recursion error when running
>> buildout in combination with distribute.  You'd get 200 lines of
>> "distribute".
>>
>> Good news: the problem has been identified.  And I've fixed it on a
>> zc.buildout branch (branches/reinout_distribute_upgrade_check).  Tarek will
>> test it and probably/hopefully release it real soon, wink, wink.
>
> Yes, thanks for the fix/work ! It looks nice and also simplifies the code.
>
> Before it's merged in trunk and eventually released, we need to :
>
> - add a test demonstrating this bug in easy_install.txt

Buildout's test aren't setup to test this kind of things directly.  I *did*
add a test to easy_install.txt that tests whether the offending _get_dist()
(which starts the recursion) gets called.  This fails before my fix and
succeeds afterwards.

On Tarek's suggestion, I moved the test out of easy_install.txt to a separate
file to keep the corner case test out of the user documentation.

> - manually test the various scenarii (these are not fully covered with
> zc.buildout unittests because the assmption is made that python don't
> have setuptools installed):
>
>     - nude python, buildout without --distribute, plone buildout cfg file
>     - python with setuptools, buildout witout --distribute, plone
> buildout cfg file
>     - python with distribute < 0.6.8, buildout witout --distribute,
> plone buildout cfg file
>     - these 3 tests *with* --distribute

I did not test all the variants, that would cost me waaaay too much time for
such a small fix.  To be honest.

I did reproduce it with setuptools + distribute with and without the fix with
the development buildout itself.  That executes the changed code point enough.


Reinout


--
Reinout van Rees - reinout@... - http://reinout.vanrees.org
Software developer at http://www.thehealthagency.com
"Military engineers build missiles. Civil engineers build targets"

_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@...
http://mail.python.org/mailman/listinfo/distutils-sig

[Tarek Ziadé]

On Tue, Nov 10, 2009 at 3:00 PM, Reinout van Rees <reinout@...> wrote:
[..]

>>
>> Before it's merged in trunk and eventually released, we need to :
>>
>> - add a test demonstrating this bug in easy_install.txt
>
> Buildout's test aren't setup to test this kind of things directly.  I *did*
> add a test to easy_install.txt that tests whether the offending _get_dist()
> (which starts the recursion) gets called.  This fails before my fix and
> succeeds afterwards.
>
> On Tarek's suggestion, I moved the test out of easy_install.txt to a separate
> file to keep the corner case test out of the user documentation.
>

Sounds enough,

>> - manually test the various scenarii (these are not fully covered with
>> zc.buildout unittests because the assmption is made that python don't
>> have setuptools installed):
>>
>>     - nude python, buildout without --distribute, plone buildout cfg file
>>     - python with setuptools, buildout witout --distribute, plone
>> buildout cfg file
>>     - python with distribute < 0.6.8, buildout witout --distribute,
>> plone buildout cfg file
>>     - these 3 tests *with* --distribute
>
> I did not test all the variants, that would cost me waaaay too much time for
> such a small fix.  To be honest.
>
> I did reproduce it with setuptools + distribute with and without the fix with
> the development buildout itself.  That executes the changed code point enough.

Can you merge it into trunk ?

Thanks

Tarek
_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@...
http://mail.python.org/mailman/listinfo/distutils-sig

[Reinout van Rees]

On 2009-11-11, Tarek Ziadé <ziade.tarek@...> wrote:
>
> Can you merge it into trunk ?

Done!


Reinout

--
Reinout van Rees - reinout@... - http://reinout.vanrees.org
Software developer at http://www.thehealthagency.com
"Military engineers build missiles. Civil engineers build targets"

_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@...
http://mail.python.org/mailman/listinfo/distutils-sig