Closed
Bug 661908
Opened 13 years ago
Closed 12 years ago
unified virtualenv for m-c
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla15
People
(Reporter: k0scist, Assigned: ted)
References
Details
(Whiteboard: [mozbase])
Attachments
(3 files, 2 obsolete files)
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
k0scist
:
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.
Comment 1•13 years ago
|
||
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.
Reporter | ||
Updated•13 years ago
|
Whiteboard: [mozbase]
Reporter | ||
Comment 2•13 years ago
|
||
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
Depends on: 689356
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.
Reporter | ||
Comment 4•13 years ago
|
||
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
Reporter | ||
Updated•13 years ago
|
Reporter | ||
Comment 5•13 years ago
|
||
Comment 6•13 years ago
|
||
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.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → ted.mielczarek
Comment 7•13 years ago
|
||
Ted: does the assignment mean there will be progress on this bug? If so, what are the plans?
Assignee | ||
Updated•13 years ago
|
Attachment #564917 -
Attachment is obsolete: true
Assignee | ||
Comment 8•13 years ago
|
||
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.
Reporter | ||
Comment 9•13 years ago
|
||
++
Assignee | ||
Updated•13 years ago
|
Comment 10•13 years ago
|
||
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.
Assignee | ||
Comment 11•13 years ago
|
||
Assignee | ||
Comment 12•13 years ago
|
||
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.
Reporter | ||
Comment 13•13 years ago
|
||
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?
Assignee | ||
Comment 14•13 years ago
|
||
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.
Assignee | ||
Comment 15•13 years ago
|
||
Oops, turns out we already had a copy of simplejson in the tree.
Assignee | ||
Updated•13 years ago
|
Attachment #615896 -
Attachment is obsolete: true
Reporter | ||
Comment 16•13 years ago
|
||
The virtualenv in tree doesn't work with python 2.7. We should upate that
Assignee | ||
Comment 17•13 years ago
|
||
The copy of virtualenv in-tree is too old, so we need to update it.
Attachment #615927 -
Flags: review?(jhammel)
Reporter | ||
Comment 18•13 years ago
|
||
Comment on attachment 615927 [details] [diff] [review]
update virtualenv to the latest release
looks good to me
Attachment #615927 -
Flags: review?(jhammel) → review+
Comment 19•13 years ago
|
||
(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).
Comment 20•13 years ago
|
||
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.
Comment 21•13 years ago
|
||
(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.
Reporter | ||
Comment 22•13 years ago
|
||
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
Assignee | ||
Comment 23•13 years ago
|
||
Yes, this has value for the build system outside of our unit test harnesses anyway.
Assignee | ||
Updated•12 years ago
|
Attachment #615913 -
Flags: review?(khuey)
Assignee | ||
Updated•12 years ago
|
Attachment #615863 -
Flags: review?(khuey)
Assignee | ||
Comment 24•12 years ago
|
||
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.
Assignee | ||
Comment 25•12 years ago
|
||
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?
Attachment #615913 -
Flags: review?(khuey) → review+
Attachment #615863 -
Flags: review?(khuey) → review+
Comment 27•12 years ago
|
||
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.
Comment 28•12 years ago
|
||
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[3]: *** [default] Error 127
*** Fix above errors and then restart with "make -f client.mk build"
make[2]: *** [configure] Error 1
make[1]: *** [Makefile] Error 2
make: *** [build] Error 2
Assignee | ||
Comment 29•12 years ago
|
||
(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).
Comment 30•12 years ago
|
||
(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
Assignee | ||
Comment 31•12 years ago
|
||
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
Comment 32•12 years ago
|
||
Bustage fix checked in to fix B2G with irc-r=ted
https://hg.mozilla.org/integration/mozilla-inbound/rev/eca8e2875ef5
Assignee | ||
Updated•12 years ago
|
Target Milestone: --- → mozilla15
Assignee | ||
Comment 33•12 years ago
|
||
(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 35•12 years ago
|
||
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).
Comment 36•12 years ago
|
||
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.
Assignee | ||
Comment 37•12 years ago
|
||
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.
Comment 38•12 years ago
|
||
Comment 39•12 years ago
|
||
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.
Assignee | ||
Comment 40•12 years ago
|
||
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.
Assignee | ||
Comment 41•12 years ago
|
||
Another bustage fix for PGO builds:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e08361c00e
Comment 42•12 years ago
|
||
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
Comment 43•12 years ago
|
||
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)
Comment 44•12 years ago
|
||
(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.
Flags: in-testsuite-
Assignee | ||
Comment 45•12 years ago
|
||
Yes, that's enough bustage fixes for one bug. We can file followups for anything else.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•