Launchpad itself

Code review comment for lp:~gary/launchpad/bug491705

bug491705
Merge into devel

Revision history for this message

Gary Poster (gary) wrote on 2010-02-25:

This branch merges my changes to zc.buildout. These completely change how we handle working with a system Python from my earlier approach. Now instead of mucking with sys.modules, and instead of throwing a bunch of paths to every script we write, and instead of having a site.py that duplicates effort that we maintain in the tree, we have a custom buildout-generated site.py that sets up our paths. We insert the path to the directory containing that file in sys.paths (via PYTHONPATH; or by starting with -S, inserting the path in Python, and importing the site module) when we want to use the Launchpad environment.

The Launchpad reviews of these zc.buildout branches are here, for reference:

https://code.edge.launchpad.net/~gary/zc.buildout/python-support-1-cleanup/+merge/19532
https://code.edge.launchpad.net/~gary/zc.buildout/python-support-2-bootstrap/+merge/19538
https://code.edge.launchpad.net/~gary/zc.buildout/python-support-3-options/+merge/19539
https://code.edge.launchpad.net/~gary/zc.buildout/python-support-4/+merge/19547
https://code.edge.launchpad.net/~gary/zc.buildout/python-support-5-initial-egg-control/+merge/20010
https://code.edge.launchpad.net/~gary/zc.buildout/python-support-6-egg-control/+merge/20013
https://code.edge.launchpad.net/~gary/zc.buildout/python-support-7/+merge/20092

Making these changes to zc.buildout meant several necessary changes to the Launchpad tree, and a couple of opportunistic ones.

_pythonpath.py.in can now be wildly simpler. It now just needs to insert the path with our custom site.py into sys.path and import site. This only works if (the wrong) site has not already been imported, so we double-check for that. We try to provide guidance for how to use the new approach. I believe that I have adjusted all scripts that use _pythonpath to use the new approach. Some, such as buildout-templates/bin/jstest.in, were standardized in their approach at the same time. Some, like cronscripts/foaf-update-karma-cache.py, also got trailing-whitespace clean-up courtesy of my editor.

All generated scripts in the bin directory should now be significantly easier to read, since they only insert the directory with the custom site.py, which now handles all of the path code.

The generated bin/py now also is significantly different. It is Python rather than a bash script, though the basic mechanism (exec) is the same. It is also generated by code in buildout itself, so buildout-templates no longer needs a file to generate it. This is why "interpreter = py" has been added to buildout.cfg.

The filetemplates no longer need to specify eggs in the buildout.cfg because the scripts all use the custom site.py instead.

The initialization has been changed to rely on a new module, lp_sitecustomize.py. per bug 496705. Not everything could go into the new module, for the reasons I clarify in comments.

I removed a couple of lines like this one: "env['PYTHONPATH'] = os.path.pathsep.join(sys.path)". It is no longer necessary, and in fact causes problems because pkg_resources gets confused with duplicate paths. This change felt a little risky, particularly because it touched the Mailman integration, which can get hairy. However, I manually tested Mailman with Barry's help, and all seemed well.

You'll notice I moved to an older, released version of z3c.recipe.filetemplates. While it might want to be adjusted to take advantage of the new buildout features when they are merged, it's not necessary for Launchpad. We can use the released version just fine.

Finally, it's worth mentioning that I don't intend to merge this until after we've branched for this release. I want to have plenty of time to QA, since this touches the vast majority of our (under-tested) scripts.

Thank you!

Gary

This branch merges my changes to zc.buildout.  These completely change how we handle working with a system Python from my earlier approach.  Now instead of mucking with sys.modules, and instead of throwing a bunch of paths to every script we write, and instead of having a site.py that duplicates effort that we maintain in the tree, we have a custom buildout-generated site.py that sets up our paths.  We insert the path to the directory containing that file in sys.paths (via PYTHONPATH; or by starting with -S, inserting the path in Python, and importing the site module) when we want to use the Launchpad environment.

The Launchpad reviews of these zc.buildout branches are here, for reference:

Making these changes to zc.buildout meant several necessary changes to the Launchpad tree, and a couple of opportunistic ones.

_pythonpath.py.in can now be wildly simpler.  It now just needs to insert the path with our custom site.py into sys.path and import site.  This only works if (the wrong) site has not already been imported, so we double-check for that.  We try to provide guidance for how to use the new approach.  I believe that I have adjusted all scripts that use _pythonpath to use the new approach.  Some, such as buildout-templates/bin/jstest.in, were standardized in their approach at the same time.  Some, like cronscripts/foaf-update-karma-cache.py, also got trailing-whitespace clean-up courtesy of my editor.

All generated scripts in the bin directory should now be significantly easier to read, since they only insert the directory with the custom site.py, which now handles all of the path code.

The generated bin/py now also is significantly different.  It is Python rather than a bash script, though the basic mechanism (exec) is the same.  It is also generated by code in buildout itself, so buildout-templates no longer needs a file to generate it.  This is why "interpreter = py" has been added to buildout.cfg.

The filetemplates no longer need to specify eggs in the buildout.cfg because the scripts all use the custom site.py instead.

The initialization has been changed to rely on a new module, lp_sitecustomize.py. per bug 496705.  Not everything could go into the new module, for the reasons I clarify in comments.

I removed a couple of lines like this one: "env['PYTHONPATH'] = os.path.pathsep.join(sys.path)".  It is no longer necessary, and in fact causes problems because pkg_resources gets confused with duplicate paths.  This change felt a little risky, particularly because it touched the Mailman integration, which can get hairy.  However, I manually tested Mailman with Barry's help, and all seemed well.

You'll notice I moved to an older, released version of z3c.recipe.filetemplates.  While it might want to be adjusted to take advantage of the new buildout features when they are merged, it's not necessary for Launchpad.  We can use the released version just fine.

Finally, it's worth mentioning that I don't intend to merge this until after we've branched for this release.  I want to have plenty of time to QA, since this touches the vast majority of our (under-tested) scripts.

Thank you!

Gary

« Back to merge proposal