Merge lp:~gary/launchpad/buildout-py into lp:launchpad

The main goal of this branch was to get the build working when lazr packages were installed into the system. When you have the same namespace ("lazr," for instance) in site-packages and in eggs, the site-package namespace package would mask all of the eggs in the same namespace.

Fixing this happened first in our upstream zc.buildout branch. Then we also have the same kind of problem when we start subprocesses; and when we start standalone scripts in the "scripts," "utilities," and "cron" directories. For those, we use PYTHONPATH and _pythonpath.py, respectively. The branch needed to make these approaches also work around the namespace package problem.

This work happened on Karmic, because that's where the problem was first discovered, because that's what we are supposed to be dogfooding, and because some lazr packages are installed by default in Karmic because of apport (and maybe other things) so the problem happens consistently there. Trying to get tests to pass in Karmic discovered a few other issues, which I addressed here. I should have been using a pipeline and then put this work in an earlier branch, but I did not. If you want me to divide this work up after the fact, that will take some time, but I will completely understand the request. For now, though, we have a diff that is almost 1300 lines long.

When reviewing, in addition to looking at the diff in Launchpad, you should probably also look at the changes in the scripts that buildout is producing. The ``bin/py`` script, for instance, is wildly different now: it used to use PYTHONPATH, and now it uses the faux-interpreter that buildout produces.

The ``bin/py`` script is worth calling out for another reason. Codehosting noticed a marked increase in slowness when 3.0 launched, and investigated to find that at least some of the problem may be put on setuptools' door. Michael Hudson reported that the speed to start a Python interpreter increased quadratically based on the length of sys.path. One of the codehosting tools relies on starting an Python process for every operation, so this start-up time is noticeable. I was a bit worried that my changes in this branch would exacerbate the problem, because ``bin/py`` has moved from a bash script relying on PYTHONPATH to pass its configuration to a buildout-generated faux interpreter. According to tests of ``time bin/py -c ''`` in the old and new versions, the speed is about the same (between .2 and .3 seconds on my machine, about a factor of ten slower than ``time python2.4 -c ''``). Therefore, at least I have not made the problem worse.

Now I'll start describing the changes to this branch in more detail.

Makefile:

Since bin/py is now a faux-interpreter, it does not support the -t option ("issue warnings about inconsistent tab usage"). We use that option a few times in the Makefile, so I call the faux Python interpreter with the actual Python executable, and give the -t flag to the real executable.

buildmailman.py:

Thanks to Barry Warsaw's inspiration, I was able to have Mailman use the correct paths (and get the fix for namespace packages described at the start of this comment) by simply telling it to configure usin...

On Oct 16, 2009, at 02:30 PM, Gary Poster wrote:

>Gary Poster has proposed merging lp:~gary/launchpad/buildout-py into lp:launchpad/devel.
>
> Requested reviews:
> Barry Warsaw (barry)
>
>
>The main goal of this branch was to get the build working when lazr packages were installed into the system. When you have the same namespace ("lazr," for instance) in site-packages and in eggs, the site-package namespace package would mask all of the eggs in the same namespace.

Hi Gary,

Thanks so much for working on this branch. From all the discussions we had about it, I'm sure it was "fun". :)

>When reviewing, in addition to looking at the diff in Launchpad, you should probably also look at the changes in the scripts that buildout is producing. The ``bin/py`` script, for instance, is wildly different now: it used to use PYTHONPATH, and now it uses the faux-interpreter that buildout produces.

I'm going to try the branch out as part of the review, though I might do that in two separate responses.

>The ``bin/py`` script is worth calling out for another reason. Codehosting noticed a marked increase in slowness when 3.0 launched, and investigated to find that at least some of the problem may be put on setuptools' door. Michael Hudson reported that the speed to start a Python interpreter increased quadratically based on the length of sys.path. One of the codehosting tools relies on starting an Python process for every operation, so this start-up time is noticeable. I was a bit worried that my changes in this branch would exacerbate the problem, because ``bin/py`` has moved from a bash script relying on PYTHONPATH to pass its configuration to a buildout-generated faux interpreter. According to tests of ``time bin/py -c ''`` in the old and new versions, the speed is about the same (between .2 and .3 seconds on my machine, about a factor of ten slower than ``time python2.4 -c ''``). Therefore, at least I have not made the problem worse.

That's good. I'm fairly concerned about the quadratic impact of long sys.path entries. This is something we should pursue in upstream, especially if super long sys.paths are going to be a common occurrence with buildout (or virtualenv or pip) based development.

>Since bin/py is now a faux-interpreter, it does not support the -t option ("issue warnings about inconsistent tab usage"). We use that option a few times in the Makefile, so I call the faux Python interpreter with the actual Python executable, and give the -t flag to the real executable.

Does it make sense to try to extend bin/py to understand -t? That's probably an upstream change, and if so, out of scope right now for this branch. IWBNI bin/py understood all the command line options that `python` understood.

>buildmailman.py:
>
>Thanks to Barry Warsaw's inspiration, I was able to have Mailman use the correct paths (and get the fix for namespace packages described at the start of this comment) by simply telling it to configure using bin/py rather than the actual Python executable.

And you trusted that Barry guy? You're brave and gullible. But okay, I'll let this one slide.

:)

>We now use a buildout-generated faux-interpreter for bin/py,...

On October 16, 2009, Gary Poster wrote:
> lib/site.py:
>
> This is the hack--forking the Python standard library site.py--that I am
> saddest about in this branch. I talked about it with Francis Lacoste and
> he blessed the approach, under the circumstances. I also have described
> it to Barry Warsaw and Björn Tillenius, so they are at least aware, if not
> necessarily accepting.
>
> The comment at the top describes the rationale, and highlights the
> change. If PEP 382 (namespace packages,
> http://www.python.org/dev/peps/pep-0382/) fixes the underlying problem
> then hopefully we can get rid of this fork, as well as some of the other
> more unpleasant bits in this branch. That won't be till Python 2.7 at the
> soonest, though.
>
> You should not have to review the file after this block, near the very top:
>
> +try:
> + __import__('pkg_resources') # Use __import__ to not pollute the
> namespace. +except ImportError:
> + pass
>
> Everything else is from the Python (2.5) standard library.
>

Instead of copying the file content couldn't we simply use execfile
"/usr/lib/python2.5/site.py". Bonus points if there is a way to find
"/usr/lib/python2.5" dynamicall :-)

--
Francis J. Lacoste
<email address hidden>

On Oct 16, 2009, at 3:15 PM, Francis J. Lacoste wrote:

> Instead of copying the file content couldn't we simply use execfile
> "/usr/lib/python2.5/site.py". Bonus points if there is a way to find
> "/usr/lib/python2.5" dynamicall :-)

We're way ahead of ya Francis! :)

-Barry

On October 16, 2009, Barry Warsaw wrote:
> On Oct 16, 2009, at 3:15 PM, Francis J. Lacoste wrote:
> > Instead of copying the file content couldn't we simply use execfile
> > "/usr/lib/python2.5/site.py". Bonus points if there is a way to find
> > "/usr/lib/python2.5" dynamicall :-)
>
> We're way ahead of ya Francis! :)
>
> -Barry
>
Yeah, I noticed :-)

--
Francis J. Lacoste
<email address hidden>

On Oct 16, 2009, at 1:08 PM, Barry Warsaw wrote:

> On Oct 16, 2009, at 02:30 PM, Gary Poster wrote:
>
>> Gary Poster has proposed merging lp:~gary/launchpad/buildout-py
>> into lp:launchpad/devel.
>>
>> Requested reviews:
>> Barry Warsaw (barry)
>>
>>
>> The main goal of this branch was to get the build working when lazr
>> packages were installed into the system. When you have the same
>> namespace ("lazr," for instance) in site-packages and in eggs, the
>> site-package namespace package would mask all of the eggs in the
>> same namespace.
>
> Hi Gary,
>
> Thanks so much for working on this branch. From all the discussions
> we had about it, I'm sure it was "fun". :)

:-) yup.

>> When reviewing, in addition to looking at the diff in Launchpad,
>> you should probably also look at the changes in the scripts that
>> buildout is producing. The ``bin/py`` script, for instance, is
>> wildly different now: it used to use PYTHONPATH, and now it uses
>> the faux-interpreter that buildout produces.
>
> I'm going to try the branch out as part of the review, though I
> might do that in two separate responses.
>
>> The ``bin/py`` script is worth calling out for another reason.
>> Codehosting noticed a marked increase in slowness when 3.0
>> launched, and investigated to find that at least some of the
>> problem may be put on setuptools' door. Michael Hudson reported
>> that the speed to start a Python interpreter increased
>> quadratically based on the length of sys.path. One of the
>> codehosting tools relies on starting an Python process for every
>> operation, so this start-up time is noticeable. I was a bit
>> worried that my changes in this branch would exacerbate the
>> problem, because ``bin/py`` has moved from a bash script relying on
>> PYTHONPATH to pass its configuration to a buildout-generated faux
>> interpreter. According to tests of ``time bin/py -c ''`` in the
>> old and new versions, the speed is about the same (between .2 and .
>> 3 seconds on my machine, about a factor of ten slower than ``time
>> python2.4 -c ''``). Therefore, at least I have not made the
>> problem worse.
>
> That's good. I'm fairly concerned about the quadratic impact of
> long sys.path entries. This is something we should pursue in
> upstream, especially if super long sys.paths are going to be a
> common occurrence with buildout (or virtualenv or pip) based
> development.

Agreed. Francis has already mentioned maybe pursuing Distribute.
Maybe we should be working with them and trying to fix this there,

>> Since bin/py is now a faux-interpreter, it does not support the -t
>> option ("issue warnings about inconsistent tab usage"). We use
>> that option a few times in the Makefile, so I call the faux Python
>> interpreter with the actual Python executable, and give the -t flag
>> to the real executable.
>
> Does it make sense to try to extend bin/py to understand -t? That's
> probably an upstream change, and if so, out of scope right now for
> this branch. IWBNI bin/py understood all the command line options
> that `python` understood.

It would be nice if it understood ...

Barry, we've got problems. Mailman problems, specifically.

Karmic accepts interpreters that themselves are interpreted scripts in the shebang line. Hardy, not so much, it seems.

This means that the nice Mailman build hack of ``'--with-python=' + os.path.abspath('bin/py'),`` doesn't work on Hardy. The Mailman Python scripts are interpreted as shell scripts, to poor effect.

I'm not quite sure what to do about this yet, but it definitely means I can't land the branch as is.

/me cries. Okay, will think about this over the weekend.

Here's an idea: use ``#! /usr/bin/env {path-to-bin/py}`` instead of ``#! {path-to-bin/py}``. Make that modification after Mailman's ``make`` and before ``make install`` (because Mailman's ``make install`` tries to run ``bin/update``). It seemed to work as a technique when I experimented yesterday on the ec2 instance. I don't see a downside yet (other than the small amount of code to do the hack). It should work. I'll push and give it a try on ec2.

The incremental diff below also switches bin/py back to a shell script. This seemed to make the Mailman pofile creation happier on hardy than the zc.buildout Python version that I had switched to for this branch. It also meant that I could go back to using -t normally in the Makefile. (I had switched from the shell version primarily because I didn't know I would have to get a shell version working; now that we have the site.py hack, it works fine.)

=== modified file 'Makefile'
--- Makefile 2009-10-13 18:50:35 +0000
+++ Makefile 2009-10-17 17:46:38 +0000
@@ -82,7 +82,7 @@
 check: clean build
  # Run all tests. test_on_merge.py takes care of setting up the
  # database.
- ${PYTHON} -t ${PY} ./test_on_merge.py $(VERBOSITY)
+ ${PY} -t ./test_on_merge.py $(VERBOSITY)

 jscheck: build
  # Run all JavaScript integration tests. The test runner takes care of
@@ -103,7 +103,7 @@
 check_mailman: build
  # Run all tests, including the Mailman integration
  # tests. test_on_merge.py takes care of setting up the database.
- ${PYTHON} -t ${PY} ./test_on_merge.py $(VERBOSITY) --layer=MailmanLayer
+ ${PY} -t ./test_on_merge.py $(VERBOSITY) --layer=MailmanLayer

 lint: ${PY}
  @bash ./bin/lint.sh
@@ -112,7 +112,7 @@
  @bash ./bin/lint.sh -v

 xxxreport: $(PY)
- ${PYTHON} -t ${PY} ./utilities/xxxreport.py -f csv -o xxx-report.csv ./
+ ${PY} -t ./utilities/xxxreport.py -f csv -o xxx-report.csv ./

 check-configs: $(PY)
  ${PY} utilities/check-configs.py
@@ -149,7 +149,7 @@
 compile: $(PY)
  ${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
      PYTHON_VERSION=${PYTHON_VERSION} LPCONFIG=${LPCONFIG}
- ${SHHH} LPCONFIG=${LPCONFIG} ${PYTHON} -t ${PY} buildmailman.py
+ ${SHHH} LPCONFIG=${LPCONFIG} ${PY} -t buildmailman.py
  ${SHHH} $(PY) sourcecode/lazr-js/tools/build.py \
   -n launchpad -s lib/canonical/launchpad/javascript \
   -b lib/canonical/launchpad/icing/build $(EXTRA_JS_FILES)

=== modified file 'buildmailman.py'
--- buildmailman.py 2009-10-15 04:19:54 +0000
+++ buildmailman.py 2009-10-18 00:59:56 +0000
@@ -84,25 +84,52 @@

     # Build and install the Mailman software. Note that we don't care about
     # --with-cgi-gid because we're not going to use that Mailman subsystem.
+ executable = os.path.abspath('bin/py')
     configure_args = (
         './configure',
         '--prefix', mailman_path,
         '--with-var-prefix=' + var_dir,
- '--with-python=' + os.path.abspath('bin/py'),
+ '--with-python=' + executable,
         '--with-username=' + user,
         '--with-groupname=' + group,
         '--with-mail-gid=' + group,
         '--with-mailhost=' + build_host_name,
         '--with-urlhost=' + build_host_name,
         )
+ # Configure.
     retcode = subprocess.call(...

That builds!

It turns out you also need this to start the tests, as you might expect:

=== modified file 'buildout-templates/bin/test.in'
--- buildout-templates/bin/test.in 2009-10-16 20:26:32 +0000
+++ buildout-templates/bin/test.in 2009-10-18 01:28:48 +0000
@@ -1,4 +1,4 @@
-#!${buildout:directory}/bin/py
+#! /usr/bin/env ${buildout:directory}/bin/py
 ##############################################################################
 #
 # Copyright (c) 2004 Zope Corporation and Contributors.

The tests pass on ec2.

I think the changes in the last two incremental diffs are substantive enough that I should wait for a mini-review; but once I have a blessing, I'll submit directly to PQM.

Gary

Nice hack! If it works, JFDI.