1268493 - VirtualenvManager can't bootstrap non-msys python virtualenv with msys python

Status: NEW

(Firefox Build System :: General, defect)

Description

[Anthony Ramine [:nox]]

https://dxr.mozilla.org/mozilla-central/source/js/src/configure#9

The python executable used to be overridable through PYTHON.

Comment 1

[Mike Hommey [:glandium]]

You'll have to be more specific about what your problem is (as opposed to what you think your problem is) because what you're pointing to is the very beginning of a complex process. configure.py does honor PYTHON, initializes a virtualenv with it, and reexecutes itself. See build/moz.configure/init.configure.

Comment 2

[Anthony Ramine [:nox]]

I said configure, not configure.py. Cf. the link I pasted which just does "exec python2.7" and "exec python" if it doesn't find python2.7.

Comment 3

[Mike Hommey [:glandium]]

(In reply to nox from comment #2)
> I said configure, not configure.py. Cf. the link I pasted which just does
> "exec python2.7" and "exec python" if it doesn't find python2.7.

Those execs run... configure.py. With a reasonable python. And then configure.py does what's written in comment 1. So what's the problem?

Comment 4

[Anthony Ramine [:nox]]

The problem is that the first python that is in our PATH for Servo is a msys Python because we need it elsewhere. virtualenv.py doesn't like running from an msys python and guess a wrong path in virtualenv.python_path. That would be easily fixed if I could specify which Python to run from the very start.

So the problem is that it uses whichever Python is on the path, as mentioned in the title.

And what if I didn't even have any Python in path?

Comment 5

[Mike Hommey [:glandium]]

(In reply to nox from comment #4)
> The problem is that the first python that is in our PATH for Servo is a msys
> Python because we need it elsewhere. virtualenv.py doesn't like running from
> an msys python and guess a wrong path in virtualenv.python_path. That would
> be easily fixed if I could specify which Python to run from the very start.

You can specify a python with the PYTHON environment variable, and that is what
is going to be used by virtualenv, not the python that configure picked.

There might be a bug there with the mix of native python and msys, but it is not what you think it is.

Comment 6

[Anthony Ramine [:nox]]

But configure.py itself will never run with $PYTHON, given it does "exec python2.7". I need configure.py to run with not-msys-python.

I agree that it would be even better if configure.py could just cope with msys python (it probably can by just adding 'msys' to ('win32', 'cygwin')), but don't say that it is absolutely normal to hardcode "exec python" in that dummy configure file!

Comment 7

[Mike Hommey [:glandium]]

(In reply to nox from comment #6)
> But configure.py itself will never run with $PYTHON, given it does "exec
> python2.7". I need configure.py to run with not-msys-python.

Please re-read comment 2.

Comment 8

[Anthony Ramine [:nox]]

The virtualenv cannot be initialised by msys-python because of path problems (sys.platform is 'msys' in msys python), I can't avoid that with current code except by changing my path because 'python2.7' and 'python' are hardcoded in js/src/configure.

configure.py fails to configure virtualenv with msys-python, so the things you say in comment 2 don't even happen.

Comment 9

[Mike Hommey [:glandium]]

PYTHON environment variable is taken from here:
https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/init.configure#160-164

and mozconfig, just in case:
https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/init.configure#182-189

then we actually try to find the file that PYTHON points to
https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/init.configure#208-212

then create the virtualenv with /that/ python
https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/init.configure#216-218

and reexecute configure.py with the virtualenv python derived from $PYTHON, which starts all of configure.py over again.
https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/init.configure#222-228

Comment 10

[Anthony Ramine [:nox]]

virtualenv.build calls virtualenv.python_path
https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/virtualenv.py#441-442

virtualenv.python_path is called with the initial Python which was called by js/src/configure, and sys.platform is neither win32 nor cygwin so it guesses the wrong Python path.
https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/virtualenv.py#80-86

Comment 11

[Mike Hommey [:glandium]]

So here is the actual problem:
$PYTHON *is* used to create the virtualenv, and since it's a non-msys python, it's put in _virtualenv/Scripts/python.exe
Then to populate the virtualenv, the VirtualenvManager's python_path is used, which is populated from the msys python, and thus points to _virtualenv/bin/python.exe, which doesn't exist.

Comment 12

[Anthony Ramine [:nox]]

Thus why it would fix the problem to just be able to use the non-msys python that we have on our build machines to execute the first run of configure.py...

Comment 13

[Mike Hommey [:glandium]]

Which you can if you would just change your $PATH. But that's a workaround. The bug to be fixed is what is said in comment 11.

Comment 14

[Anthony Ramine [:nox]]

Attachment: 0001-Properly-consider-msys-Python-when-bootstrapping-vir.patch

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8748635 [details] [diff] [review]
0001-Properly-consider-msys-Python-when-bootstrapping-vir.patch

Review of attachment 8748635 [details] [diff] [review]:
-----------------------------------------------------------------

This is definitely not the right fix. sys.platform == 'msys' corresponds to msys python, which, if you run virtualenv with *will* use the /bin/ directory, not /Scripts/. Again, the problem is that the VirtualenvManager tries to find the path of the virtualenv python according to the type of python that is running, not the type of python that is in the virtualenv, which in your corner case, was different.

Comment 16

[Anthony Ramine [:nox]]

With msys Python, virtualenv used /Scripts/, otherwise I wouldn't have had a problem from the very start. This is still about the original Python first initialising virtualenv etc.

Comment 17

[Anthony Ramine [:nox]]

If this is definitely not the right fix, can I at least have an explanation as to why it... fixed my problem?

Comment 18

[Mike Hommey [:glandium]]

Here is what happens in your case:
- you set PYTHON to the native python
- running python uses the msys python
- configure.py starts with msys python, up to init.configure
- init.configure picks PYTHON, and since there is no virtualenv, it uses the virtualenv manager to create one.
- the virtualenv manager creates the virtualenv by running python/virtualenv/virtualenv.py with the native python. Doing so makes the python path in the virtualenv _virtualenv/Scripts/python.exe.
- then we're back to the msys python which is still running init.configure, and wants to reexecute with the python in the virtualenv. To do that, it asks the virtualenv manager what the virtualenv python path is, and, since that part is still running in msys python, it says _virtualenv/bin/python. Which is obviously wrong.

Now, the reason your patch works *for you* is that it changes that last step such that the virtualenv manager says "_virtualenv/Scripts/python". But you're creating the exact opposite problem in return: if PYTHON is the msys python (or not set, for that matter), virtualenv.py will run with msys python, and put python in _virtualenv/bin/python. And then the virtualenv manager will tell the virtualenv python is in _virtualenv/Scripts/python.exe, which it is not.