4.70 KB, patch
|Details | Diff | Splinter Review|
1.27 KB, patch
|Details | Diff | Splinter Review|
1.63 MB, patch
Jeff Hammel: review+
|Details | Diff | Splinter Review|
Currently, python lives all over the source tree and, as needed, is hacked together to see each other with http://mxr.mozilla.org/mozilla-central/source/config/pythonpath.py Instead, virtualenv (http://www.virtualenv.org/en/latest/index.html) should be used to keep m-c land python separate from system python and allow cross-module imports easily and in a more robust way than the current ad hoc solution. This should make it easier to add python code to m-c without modifying makefiles (much if at all) as well as tend to a more modular architecture and consuming third-party modules (where third-party could include mozilla authors). As an illustration, mozmill is a typical modern python package included with its one dependency, simplejson: http://mxr.mozilla.org/mozilla-central/source/testing/mozmill/ This creates a virtualenv and appropriately installs the necessary software in it. This is just done as a one-off for this particular test harness. Really, there should be a unified virtualenv so that we can use python code from whereever we need it without PYTHONPATH munging or code duplication.
I really don't see what benefits virtualenv has over our current system, which is quite straightforward. The various python modules are over the tree for various good reasons, and the glue code is also pretty straightforward. I recommend WONTFIX.
Creating/updating the virtualenv in the $OBJDIR should give a unique environment per build to install python in. There are two ways I can think of accomplishing this: 1. Have virtualenv as part of the build prereqs: bug 689356; this has the advantage of not having to store it in m-c, although it has the obvious disadvantage that people will have to update their build environment. If it was bundled soon and pushed to m-c only later, announcing all of this, this should minimize the disruption 2. Bundle virtualenv in m-c; In fact, its already there http://mxr.mozilla.org/mozilla-central/source/testing/mozmill/virtualenv/, though see bug 676078 and bug 685903 on its pending removal. This has the advantage of portability -- you can just count on virtualenv being there -- but you're putting extra stuff in m-c for no real technical gain. In addition, you'd probably put the virtualenv in tests.zip which would result in loss of bandwidth and therefore build speed. Adding bug 689356 as a blocker assuming we're going that direction
I'd prefer we put virtualenv in the prereqs rather than distributing it. The advantage that virutalenv gives us over the current system is that we can use third party libraries without forking them into mozilla central and we can make our python system more maintainable/reusable with a more pythonic packaging mechanism for our own utilities. It will also help us better integrate with the changes coming from buildbot land for mozharness. As the python side of our test harnesses grow (I'm believe we'll soon be dealing with things like mock-server setup/teardown for sync, F1, identity tests) having a virtualenv will be a much simpler way to manage these packages in our $OBJDIR, since they are already python modules installed from an in-mozilla-network pypi server. Overall, I think virtualenv makes things simpler for the test harnesses as the scope of the python frameworks around our test harnesses grow more complex.
Created attachment 564917 [details] [diff] [review] WIP patch Not fully functional yet; all this does is set up a virtualenv and point $PYTHON at the virtualenv's python. It has a few holes and is not final form, just a pointer to the path i've been down
I would really like to see a more unified Python environment *inside* m-c. I see this effort as directly related to increasing developer involvement by reducing the barrier to entry and making the build system simpler. Here are some of the issues that go away if we do this: * Most Python scripts are currently marshaled through config/pythonpath.py. The path to pythonpath.py and all the extra -I arguments add a lot of extra line noise to build output. If we were inside a virtualenv, we could just call `python` and be guaranteed to have a sane environment. * Python paths are manually maintained in many Makefiles. Lots of redundant effort. When we change or introduce something, this can be non-trivial to update. * Bundled Python modules have known versions and are known to work. If we rely on packages provided by the OS, we may get an incompatible version and builds may break in subtle ways. If we have a consistent build environment, that's much easier to support. * If we bundle all Python packages with mozilla-central, there are fewer dependencies for casual contributors to worry about installing. It is also less work for release engineering since they won't be worrying about these things any more. * For licensing reasons, the locations of various Python modules already in m-c are scattered about. This is begging for consolidation because code like https://github.com/indygreg/mozilla-central/blob/fdfec261f83d8ef2a615342e8d0f01c9fa91a3e8/build/parse-tree.py#L40 is silly and prone to breakage. * We could move stuff out of mozilla-build and into mozilla-central. I /think/ reducing the complexity of mozilla-build will make the mozilla-build folks happier. * It allows us to be more liberal with the use of Python in tools checked in to m-c because we know they will work. For example, I'd love to have a build.py in the top-level directory that interactively generated your .mozconfig, did configure, etc. As far as implementation, I would prefer that the environment be made available outside of the object directory and available without having to run make or configure. I should just be able to say: ./build_env.py or at worst /path/to/my/python ./build_env.py and get the fully-functional Python environment. I would also request that the methods for configuring sys.path and other environment variables be exposed as a method callable from other Python code. I'm requesting these because there are tools (or will be tools - see my work in bug 687388) which need a Python environment *before* configure and the object directory is created. Who knows, maybe someday we replace or supplement autoconf with Python that interfaces with libclang (see http://eli.thegreenplace.net/2011/07/03/parsing-c-in-python-with-clang/). Or, maybe we rewrite the build system in Python entirely - something not as esoteric as it sounds. Anyway, by providing a well-defined and ready-to-go Python environment inside m-c, we leave the door open for these kinds of experiments. Let's encourage better tooling and a saner tree by moving forward on this.
Ted: does the assignment mean there will be progress on this bug? If so, what are the plans?
Yes, I'm going to fix this. My plan is just to do the simplest thing that works for now: use the existing in-tree virtualenv code (other-licenses/virtualenv), create the virtualenv under the objdir as part of configure, and then populate it. Your ideas are good, but a bit too far-reaching for my immediate needs. I think if we get *something* working it's easier to iterate on than reaching for a perfect solution to problems we don't yet have.
Works for me. I have two requests: 1) Grab a newer version of virtualenv. The one in the tree is 1.4.8, which is somewhat old. 2) Provide a way to easily activate the virtualenv outside of the build system. I'd love if I could just `source objdir/virtualenv/bin/activate` then go to any directory in the build system and run Python without having to muck with PYTHONPATH, etc.
We can update virtualenv, no problem. We might have to anyway since the in-tree one doesn't seem to install cleanly on Windows. $objdir/_virtualenv/bin/activate should work fine with this patch.
Apparently virtualenv doesn't work with the windows environment we're using. There was a pull request sent by jgriffin to fix this: https://github.com/pypa/virtualenv/pull/160 . While this is closed and supposed to be fixed (e.g. https://github.com/pypa/virtualenv/pull/197 ), this is apparently still an issue. Can anyone confirm this? And assuming this is still an issue, can it be fixed?
Created attachment 615896 [details] [diff] [review] add mozbase packages to virtualenv This patch installs all the packages in testing/mozbase into the virtualenv from the previous patch. I added simplejson because we require that on Python 2.5.
Created attachment 615913 [details] [diff] [review] add mozbase packages to virtualenv Oops, turns out we already had a copy of simplejson in the tree.
The virtualenv in tree doesn't work with python 2.7. We should upate that
Created attachment 615927 [details] [diff] [review] update virtualenv to the latest release The copy of virtualenv in-tree is too old, so we need to update it.
Comment on attachment 615927 [details] [diff] [review] update virtualenv to the latest release looks good to me
(In reply to Jeff Hammel [:jhammel] from comment #13) > Apparently virtualenv doesn't work with the windows environment we're using. > > There was a pull request sent by jgriffin to fix this: > https://github.com/pypa/virtualenv/pull/160 . While this is closed and > supposed to be fixed (e.g. https://github.com/pypa/virtualenv/pull/197 ), > this is apparently still an issue. > > Can anyone confirm this? And assuming this is still an issue, can it be > fixed? I just tried the latest version from https://github.com/pypa/virtualenv on Win7, and it works as-is. You call 'source Scripts/activate' (instead of bin/activate).
I hate to play devil's advocate, but do we need this if we run inside a mozharness script? It seems to me that mozharness creates a virtualenv for us as well, so I'm not clear on which we will want to go with here.
(In reply to Clint Talbert ( :ctalbert ) from comment #20) > I hate to play devil's advocate, but do we need this if we run inside a > mozharness script? > > It seems to me that mozharness creates a virtualenv for us as well, so I'm > not clear on which we will want to go with here. Many parts of the build system (makefiles) do not run inside mozharness and are currently creating virtualenv-like environments on-the-fly. It is ugly.
To echo comment 21, yes, I think we'll need this for stuff that is part of the build and not test harnesses even if all the test harnesses are transitioned to mozharness. I could be wrong, but this is my current opinion
Yes, this has value for the build system outside of our unit test harnesses anyway.
I tested these patches on Mac originally, and things worked fine there. I just did a build on Windows with them and that went fine as well. I'm spinning a build on Linux just to test, but it seems to be going fine, I don't expect any problems.
My Linux build worked fine, and I also pushed this to try just for sanity's sake.
Comment on attachment 615863 [details] [diff] [review] create a virtualenv as part of configure Review of attachment 615863 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +7900,1 @@ > COMPILER_DEPEND=1 I assume you've tested this with both gmake and pymake. @@ +8892,5 @@ > > +dnl Create a virtualenv where we can install local Python packages > +AC_MSG_RESULT([Creating Python virtualenv]) > +# Should we be rm'ing and re-creating this every time we run configure? > +mkdir -p _virtualenv I think so. This seems like the kind of thing that's going to require strange clobbers otherwise. @@ +8930,5 @@ > AC_OUTPUT($MAKEFILES) > > +# Populate the virtualenv > +AC_MSG_RESULT([Populating Python virtualenv]) > +$MAKE -C build/virtualenv || exit 1 Why can't we do this in tier_base?
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a27219d06c49 - Windows builds dying in make buildsymbols with "KeyError: 'Microsoft'" a la https://tbpl.mozilla.org/php/getParsedLog.php?id=11924238&tree=Mozilla-Inbound is funny as hell, but probably not something we should stick with doing.
I was seeing this error on OSX: Populating Python virtualenv cd ../../../../source/trunk/other-licenses/simplejson-2.1.1/; /Users/dave/mozilla/build/nightly/_virtualenv/bin/python setup.py develop /bin/sh: /Users/dave/mozilla/build/nightly/_virtualenv/bin/python: No such file or directory make: *** [default] Error 127 *** Fix above errors and then restart with "make -f client.mk build" make: *** [configure] Error 1 make: *** [Makefile] Error 2 make: *** [build] Error 2
(In reply to Phil Ringnalda (:philor) from comment #27) > Backed out in > https://hg.mozilla.org/integration/mozilla-inbound/rev/a27219d06c49 - > Windows builds dying in make buildsymbols with "KeyError: 'Microsoft'" a la > https://tbpl.mozilla.org/php/getParsedLog.php?id=11924238&tree=Mozilla- > Inbound is funny as hell, but probably not something we should stick with > doing. *sigh*, I saw that on try but forgot to leave myself a note, apparently. I worked out the root cause there (a bug in Python 2.5 that's only exposed if you don't have the win32api module installed), but never fixed/worked around it. Mossop: what version of OS X/Python do you have? It built fine for me on 10.6/2.6.6 (and on try as well).
(In reply to Ted Mielczarek [:ted] from comment #29) > Mossop: what version of OS X/Python do you have? It built fine for me on > 10.6/2.6.6 (and on try as well). OSX 10.7/2.7.1
I worked around that error in "make buildsymbols", it's actually a bug in Python 2.5. https://hg.mozilla.org/integration/mozilla-inbound/rev/fc6df4da2bc6 https://hg.mozilla.org/integration/mozilla-inbound/rev/a70c497939cf https://hg.mozilla.org/integration/mozilla-inbound/rev/52b9038fdc68
Bustage fix checked in to fix B2G with irc-r=ted https://hg.mozilla.org/integration/mozilla-inbound/rev/eca8e2875ef5
(In reply to Dave Townsend (:Mossop) from comment #28) > I was seeing this error on OSX: FTR this was patched in bug 758381.
This breaks builds -- "$MAKE" is not guaranteed/required to be defined at configure time. Only reason why this doesn't break tinderbox builds is that they likely do MAKE=make (and indeed running configure with that env succeeds).
Comment on attachment 615863 [details] [diff] [review] create a virtualenv as part of configure This patch uses $MAKE in configure.in but that variable is not set by default, which breaks most people who are not the tinderbox. I believe this should be backed out (when the trees unhork for infra issues).
I get this: error: invalid Python installation: unable to open /home/brad/projects/mozilla/obj-x86_64-unknown-linux-gnu/_virtualenv/include/multiarch-x86_64-linux/python2.7/pyconfig.h (No such file or directory) In /home/brad/projects/mozilla/obj-x86_64-unknown-linux-gnu/_virtualenv/include/ I have this symlink: python2.7 -> /usr/include/python2.7/ It is missing the multiarch-x86_64-linux dir.
When you say "most people" I don't think that's true. This would only break people running configure by hand and not using client.mk (which is what our build documentation says to use). I think a simple patch should fix you, so let's just do that.
I think this might have broken l10n repacks. We've just been spinning new nightlies to test a different thing on Thunderbird and they are getting: Creating Python virtualenv Traceback (most recent call last): File "/builds/slave/tb-comm-cen-osx64-l10n-ntly/build/comm-central/mozilla/other-licenses/virtualenv/virtualenv.py", line 2270, in <module> Using real prefix '/tools/python-2.7.2/Python.framework/Versions/2.7' New python executable in ./_virtualenv/bin/python2.7 Also creating executable in ./_virtualenv/bin/python Overwriting ./_virtualenv/lib/python2.7/distutils/__init__.py with new content main() File "/builds/slave/tb-comm-cen-osx64-l10n-ntly/build/comm-central/mozilla/other-licenses/virtualenv/virtualenv.py", line 928, in main never_download=options.never_download) File "/builds/slave/tb-comm-cen-osx64-l10n-ntly/build/comm-central/mozilla/other-licenses/virtualenv/virtualenv.py", line 1031, in create_environment install_distutils(home_dir) File "/builds/slave/tb-comm-cen-osx64-l10n-ntly/build/comm-central/mozilla/other-licenses/virtualenv/virtualenv.py", line 1481, in install_distutils writefile(os.path.join(distutils_path, '__init__.py'), DISTUTILS_INIT) File "/builds/slave/tb-comm-cen-osx64-l10n-ntly/build/comm-central/mozilla/other-licenses/virtualenv/virtualenv.py", line 451, in writefile f = open(dest, 'wb') IOError: [Errno 13] Permission denied: './_virtualenv/lib/python2.7/distutils/__init__.py' I don't think this has broken dep builds, but at this stage its hard for me to tell.
This should fix the "running configure not via client.mk" issue: https://hg.mozilla.org/integration/mozilla-inbound/rev/d4d9d00ee47c This was breaking Firefox l10n repacks, incidentally, since they do that. We tracked down the root cause of the Thunderbird l10n bustage, I'm going to file a RelEng bug on that.
Another bustage fix for PGO builds: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e08361c00e
This also breaks using the system ply library. /tmp/buildd/iceweasel-15.0~a1+20120525070245/build-xulrunner/_virtualenv/bin/python ../../../config/pythonpath.py \ \ ../../../xpcom/idl-parser/header.py --cachedir=. --regen Traceback (most recent call last): File "../../../config/pythonpath.py", line 56, in <module> main(sys.argv[1:]) File "../../../config/pythonpath.py", line 48, in main execfile(script, frozenglobals) File "../../../xpcom/idl-parser/header.py", line 10, in <module> import sys, os.path, re, xpidl, itertools, glob File "/tmp/buildd/iceweasel-15.0~a1+20120525070245/xpcom/idl-parser/xpidl.py", line 11, in <module> from ply import lex, yacc ImportError: No module named ply
So, what do I have to do about this error: distutils.errors.DistutilsPlatformError: invalid Python installation: unable to open /usr/lib/python2.5/config/Makefile (No such file or directory)
(In reply to Ted Mielczarek [:ted] from comment #40) > This should fix the "running configure not via client.mk" issue: > https://hg.mozilla.org/integration/mozilla-inbound/rev/d4d9d00ee47c https://hg.mozilla.org/mozilla-central/rev/d4d9d00ee47c (In reply to Ted Mielczarek [:ted] from comment #41) > Another bustage fix for PGO builds: > https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e08361c00e https://hg.mozilla.org/mozilla-central/rev/a8e08361c00e Leaving open since it's not clear to me that this bug is finished. Please close if it is.
Yes, that's enough bustage fixes for one bug. We can file followups for anything else.