Last Comment Bug 661908 - unified virtualenv for m-c
: unified virtualenv for m-c
Status: RESOLVED FIXED
[mozbase]
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Ted Mielczarek [:ted.mielczarek]
:
: Gregory Szorc [:gps]
Mentors:
Depends on: 759320 758381 758694 758739 758823 758925 758949 763419
Blocks: 688667 688686 688688 746244 774106 807066
  Show dependency treegraph
 
Reported: 2011-06-03 12:21 PDT by Jeff Hammel
Modified: 2012-10-30 12:42 PDT (History)
19 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP patch (2.30 KB, patch)
2011-10-05 10:35 PDT, Jeff Hammel
no flags Details | Diff | Splinter Review
create a virtualenv as part of configure (4.70 KB, patch)
2012-04-17 13:56 PDT, Ted Mielczarek [:ted.mielczarek]
khuey: review+
Details | Diff | Splinter Review
add mozbase packages to virtualenv (499.42 KB, patch)
2012-04-17 15:11 PDT, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Splinter Review
add mozbase packages to virtualenv (1.27 KB, patch)
2012-04-17 15:41 PDT, Ted Mielczarek [:ted.mielczarek]
khuey: review+
Details | Diff | Splinter Review
update virtualenv to the latest release (1.63 MB, patch)
2012-04-17 16:00 PDT, Ted Mielczarek [:ted.mielczarek]
k0scist: review+
Details | Diff | Splinter Review

Description Jeff Hammel 2011-06-03 12:21:07 PDT
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 Benjamin Smedberg [:bsmedberg] 2011-06-03 12:29:50 PDT
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.
Comment 2 Jeff Hammel 2011-09-26 16:17:11 PDT
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
Comment 3 cmtalbert 2011-09-27 18:40:58 PDT
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.
Comment 4 Jeff Hammel 2011-10-05 10:35:35 PDT
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
Comment 5 Jeff Hammel 2011-10-24 09:22:43 PDT
see also https://bugzilla.mozilla.org/show_bug.cgi?id=696535
Comment 6 Gregory Szorc [:gps] 2011-11-14 16:06:49 PST
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.
Comment 7 Gregory Szorc [:gps] 2012-04-17 12:52:54 PDT
Ted: does the assignment mean there will be progress on this bug? If so, what are the plans?
Comment 8 Ted Mielczarek [:ted.mielczarek] 2012-04-17 13:12:02 PDT
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.
Comment 9 Jeff Hammel 2012-04-17 13:13:52 PDT
++
Comment 10 Gregory Szorc [:gps] 2012-04-17 13:37:08 PDT
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.
Comment 11 Ted Mielczarek [:ted.mielczarek] 2012-04-17 13:56:15 PDT
Created attachment 615863 [details] [diff] [review]
create a virtualenv as part of configure
Comment 12 Ted Mielczarek [:ted.mielczarek] 2012-04-17 13:57:35 PDT
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.
Comment 13 Jeff Hammel 2012-04-17 14:52:53 PDT
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?
Comment 14 Ted Mielczarek [:ted.mielczarek] 2012-04-17 15:11:23 PDT
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.
Comment 15 Ted Mielczarek [:ted.mielczarek] 2012-04-17 15:41:22 PDT
Created attachment 615913 [details] [diff] [review]
add mozbase packages to virtualenv

Oops, turns out we already had a copy of simplejson in the tree.
Comment 16 Jeff Hammel 2012-04-17 15:51:05 PDT
The virtualenv in tree doesn't work with python 2.7.  We should upate that
Comment 17 Ted Mielczarek [:ted.mielczarek] 2012-04-17 16:00:31 PDT
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 18 Jeff Hammel 2012-04-17 16:18:43 PDT
Comment on attachment 615927 [details] [diff] [review]
update virtualenv to the latest release

looks good to me
Comment 19 Jonathan Griffin (:jgriffin) 2012-04-17 16:24:40 PDT
(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 cmtalbert 2012-04-30 12:41:36 PDT
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 Gregory Szorc [:gps] 2012-04-30 12:44:50 PDT
(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.
Comment 22 Jeff Hammel 2012-04-30 12:51:37 PDT
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
Comment 23 Ted Mielczarek [:ted.mielczarek] 2012-04-30 14:08:22 PDT
Yes, this has value for the build system outside of our unit test harnesses anyway.
Comment 24 Ted Mielczarek [:ted.mielczarek] 2012-05-14 11:47:47 PDT
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.
Comment 25 Ted Mielczarek [:ted.mielczarek] 2012-05-14 13:17:29 PDT
My Linux build worked fine, and I also pushed this to try just for sanity's sake.
Comment 26 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-05-21 10:28:04 PDT
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?
Comment 27 Phil Ringnalda (:philor) 2012-05-21 14:39:56 PDT
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 Dave Townsend [:mossop] 2012-05-21 14:44:02 PDT
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
Comment 29 Ted Mielczarek [:ted.mielczarek] 2012-05-21 16:08:38 PDT
(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 Dave Townsend [:mossop] 2012-05-21 17:23:31 PDT
(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
Comment 31 Ted Mielczarek [:ted.mielczarek] 2012-05-24 11:38:34 PDT
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 Michael Wu [:mwu] 2012-05-24 13:48:40 PDT
Bustage fix checked in to fix B2G with irc-r=ted

https://hg.mozilla.org/integration/mozilla-inbound/rev/eca8e2875ef5
Comment 33 Ted Mielczarek [:ted.mielczarek] 2012-05-25 05:36:08 PDT
(In reply to Dave Townsend (:Mossop) from comment #28)
> I was seeing this error on OSX:

FTR this was patched in bug 758381.
Comment 34 Vladimir Vukicevic [:vlad] [:vladv] 2012-05-25 07:58:19 PDT
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 Benjamin Smedberg [:bsmedberg] 2012-05-25 07:59:23 PDT
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 Brad Jackson 2012-05-25 08:04:52 PDT
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.
Comment 37 Ted Mielczarek [:ted.mielczarek] 2012-05-25 08:06:52 PDT
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 39 Mark Banner (:standard8, afk until Dec) 2012-05-25 09:09:17 PDT
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.
Comment 40 Ted Mielczarek [:ted.mielczarek] 2012-05-25 10:47:23 PDT
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.
Comment 41 Ted Mielczarek [:ted.mielczarek] 2012-05-25 11:10:27 PDT
Another bustage fix for PGO builds:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8e08361c00e
Comment 42 Mike Hommey [:glandium] 2012-05-25 12:12:54 PDT
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 neil@parkwaycc.co.uk 2012-05-26 03:09:30 PDT
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 Ryan VanderMeulen [:RyanVM] 2012-05-26 05:16:51 PDT
(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.
Comment 45 Ted Mielczarek [:ted.mielczarek] 2012-05-26 06:20:53 PDT
Yes, that's enough bustage fixes for one bug. We can file followups for anything else.

Note You need to log in before you can comment on or make changes to this bug.