The default bug view has changed. See this FAQ.

Change how virtualenv is populated

RESOLVED FIXED in mozilla17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks: 1 bug)

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
In order to support alternate build backends, we can't rely on Makefile.in'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.
(Assignee)

Comment 2

5 years ago
Created attachment 643701 [details] [diff] [review]
refactor how virtualenv is populated

Replaced Makefile.in 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 https://tbpl.mozilla.org/?tree=Try&rev=f3fa3ee4237b
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #643701 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 3

5 years ago
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 client.mk or elsewhere so virtualenv population is retriggered if the manifest file changes. I'm not sure where all I need to hook that up.
(Assignee)

Comment 4

5 years ago
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...
(Assignee)

Comment 5

5 years ago
Created attachment 643711 [details] [diff] [review]
refactor how virtualenv is populated, v2

Now with text file and dependencies in client.mk. I also may need to ensure something gets written to config.status - I don't know how all this magic is hooked up.

http://tbpl.mozilla.org/?tree=Try&rev=6f4e3ae19cef
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/populate_virtualenv.py
@@ +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

interpreter

@@ +32,5 @@
> +    setup = os.path.join(directory, 'setup.py')
> +    result = subprocess.call([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
(Assignee)

Comment 7

5 years ago
Current patch has green builds for all arches on Try.
Blocks: 776046
(Assignee)

Comment 8

5 years ago
Created attachment 644453 [details] [diff] [review]
refactor how virtualenv is populated, v3

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 setup.py <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 https://tbpl.mozilla.org/?tree=Try&rev=6416aec3c12d
Attachment #643711 - Attachment is obsolete: true
Attachment #643711 - Flags: review?(ted.mielczarek)
Attachment #644453 - Flags: review?(mh+mozilla)
(Assignee)

Comment 9

5 years ago
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]:
-----------------------------------------------------------------

::: configure.in
@@ +8782,5 @@
>  
> +# Populate the virtualenv
> +AC_MSG_RESULT([Populating Python virtualenv])
> +MACOSX_DEPLOYMENT_TARGET= LDFLAGS="${HOST_LDFLAGS}" \
> +  CFLAGS="${HOST_CFLAGS}" CXXFLAGS="${HOST_CXXFLAGS}" \

While at it, you should also set CC and CXX.
Attachment #644453 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fda9279d7031
Target Milestone: --- → mozilla17
Depends on: 776537
https://hg.mozilla.org/mozilla-central/rev/fda9279d7031
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Blocks: 777465

Updated

2 years ago
Blocks: 1195264
You need to log in before you can comment on or make changes to this bug.