Closed Bug 774106 Opened 7 years ago Closed 7 years ago

Change how virtualenv is populated


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: gps, Assigned: gps)




(1 file, 2 obsolete files)

In order to support alternate build backends, we can't rely on's because Makefiles may not be generated by alternate build backends. Since the virtualenv today is populated in configure immediately after aclocal-fast creates build/virtualenv/Makefile, we are in a bit of a bind.

We need to change how the virtualenv is populated such that it doesn't rely on the mass Makefile generation as it is currently implemented in configure.

We could still use a make file if we really wanted to. However, the existing make file logic is dead simple and it could easily be ported to a Python module/script. Having it exist as an importable module would also likely benefit mach. Since mach will run before configure, it needs a way of knowing where all the Python source lives so it can import its own modules. This likely involves some kind of self bootstrapping. It's entirely possible mach will want to create and populate the virtualenv itself, before configure is executed. At the very least, mach will need to adjust sys.path on first run in order to be able to import its own modules.

Anyway, the mach work will be in a follow-up bug. Just know that its usage needs to be considered when changing this.

Our options depend on what :glandium is doing for refactoring autoconf output file generation. If we lump virtualenv's make file into the always-generated set, we should be cool. That being said, I'm advocating for an importable Python module (which can also be executed as a script from configure) so mach can do magic without having to fork a new process.
I don't really care, honestly. I just wanted to have an easy place for people to rebuild if they wanted to re-populate the virtualenv. It doesn't have to be a Makefile.
Replaced with packages.json manifest and a Python file. Population of virtualenv in configure now happens before Makefile generation.

This works fine for me on Python 2.7 on OS X. I don't doubt the first revision will cause burnage. That's why we have
Assignee: nobody → gps
Attachment #643701 - Flags: review?(ted.mielczarek)
Comment on attachment 643701 [details] [diff] [review]
refactor how virtualenv is populated

Review of attachment 643701 [details] [diff] [review]:

This will need a hook-up somewhere in or elsewhere so virtualenv population is retriggered if the manifest file changes. I'm not sure where all I need to hook that up.
Bit by Python 2.5 once again. I completely forgot the json module isn't present in 2.5. I guess I'll go with a flat file...
Now with text file and dependencies in I also may need to ensure something gets written to config.status - I don't know how all this magic is hooked up.
Attachment #643701 - Attachment is obsolete: true
Attachment #643701 - Flags: review?(ted.mielczarek)
Attachment #643711 - Flags: review?(ted.mielczarek)
Comment on attachment 643711 [details] [diff] [review]
refactor how virtualenv is populated, v2

Review of attachment 643711 [details] [diff] [review]:

::: build/virtualenv/
@@ +14,5 @@
> +
> +    The manifest file is opened and read. For each package defined in the
> +    manifest, we install it into the current environment.
> +
> +    Note that the Python interpretter running this function should be the one


@@ +32,5 @@
> +    setup = os.path.join(directory, '')
> +    result =[sys.executable, setup, 'develop'], cwd=directory)
> +
> +    if result != 0:
> +        raise Exception('Error installing package: %s' % directory)

I guess using check_call would give a worse error message
Current patch has green builds for all arches on Try.
I think Mike is interested in using this (due to dependent bugs), so I'll put him in the review path.

I changed the manifest format to facilitate multiple install actions. Currently, we just support <arg0> [<arg1>[...<argN>]]. But, we'll need to support raw .pth in the near future. And, who knows what the distant future holds (maybe even symlinks to individual files?). Anyway, we now have a plan for tackling that when it becomes a problem.

Try at
Attachment #643711 - Attachment is obsolete: true
Attachment #643711 - Flags: review?(ted.mielczarek)
Attachment #644453 - Flags: review?(mh+mozilla)
Try build was all green.
Comment on attachment 644453 [details] [diff] [review]
refactor how virtualenv is populated, v3

Review of attachment 644453 [details] [diff] [review]:

@@ +8782,5 @@
> +# Populate the virtualenv
> +AC_MSG_RESULT([Populating Python virtualenv])

While at it, you should also set CC and CXX.
Attachment #644453 - Flags: review?(mh+mozilla) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Blocks: 777465
Blocks: 1195264
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.