1436777 - Set sys.executable after after activating the virtualenv

Status: RESOLVED WONTFIX

(Firefox Build System :: General, enhancement)

Description

[Andrew Halberstadt [:ahal]]

Currently when activating the build system virtualenv, sys.executable will still point to the original executable. This can cause problems for consumers who expect sys.executable to run from the build system virtualenv. This first came up around a year ago here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1290765#c23

Though we never ended up solving it for the general case.

Recently I landed this patch:
https://hg.mozilla.org/integration/autoland/rev/c9d150401eea

I forgot that sys.executable doesn't point to the virtualenv python, and we'll now be installing flake8 with the system pip.. oops! I'd like to just get this fixed in the general case.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1436777 - Set sys.executable to virtualenv python in VirtualenvManager.activate,

Currently when activating the build system virtualenv, sys.executable isn't updated. This
can cause issues for consumers, e.g:

self.virtualenv_manager.activate()
self.virtualenv_manager.install_pip_package('flake8')
subprocess.call([sys.executable, '-mflake8'])  # flake8 not installed

This example is contrived, but sometimes the code launching the subprocess is very far
removed from the mach command, and passing self.virtualenv_manager.python_path everywhere
would be a big pain.

Review commit: https://reviewboard.mozilla.org/r/218850/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218850/

Comment 2

[Andrew Halberstadt [:ahal]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=891f6f7b668fcf3614be3fa55a10cd1c59b4ec1b

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8949514 [details]
Bug 1436777 - Set sys.executable to virtualenv python in VirtualenvManager.activate,

https://reviewboard.mozilla.org/r/218850/#review225352

Here be dragons.

I'm pretty sure I attempted to do this once. It was either backed out for causing problems or I dug into what virtualenv was doing under the hood and realized it would be a bad idea. I may even have an email thread with Ian Bicking (original author of virtualenv) about it.

What I recall is that `sys.executable` is supposed to refer to the current Python executable. Various things key off of this. Enough that changing it so it "lies" can cause problems. And in this case, we would be lying because the *current* process is a non-virtualenv binary.

I also recall that virtualenv and other tools set additional attributes on `sys` (such as `sys.real_prefix`) when virtualenvs are activated. I can't recall if the virtualenv's `python` somehow stores a pointer to the *real* Python it was derived from. But if it does, than the naive/simple switch of `sys.executable` is incomplete.

I'm fine with having an option to change `sys.executable` when a virtualenv is activated. I just don't think it can be the default. So an optional named argument to `activate()` seems like one solution.

A better solution would be to use `VirtualenvManager.python_path` everywhere in place of `sys.executable`. But, yeah, `sys.executable` is convenient. So setting it for cases where we know it works is easier than passing the Python path through several function calls. That's why I'm OK optionally updating `sys.executable` when the virtualenv is activated.

Comment 4

[Andrew Halberstadt [:ahal]]

Ok, in that case I'll WONTFIX. We can either pass down python_path, or if that happens to be extra inconvenient we can always set sys.executable manually after calling activate.

Comment 5

[Andrew Halberstadt [:ahal]]

In case anyone stumbles across this, a third alternative is to use sys.prefix to find the python (which does get modified):
os.path.join(sys.prefix, 'bin', 'python')          # linux/osx
os.path.join(sys.prefix, 'Scripts', 'python.exe')  # Windows