Skip to content

Prevent sys.prefix from leaking into child process on macOS#1648

Merged
gaborbernat merged 4 commits into
pypa:masterfrom
cjolowicz:1643
Feb 21, 2020
Merged

Prevent sys.prefix from leaking into child process on macOS#1648
gaborbernat merged 4 commits into
pypa:masterfrom
cjolowicz:1643

Conversation

@cjolowicz

@cjolowicz cjolowicz commented Feb 21, 2020

Copy link
Copy Markdown
Contributor

Fixes #1643

This PR fixes a bug on macOS where virtualenv, when run inside a virtual environment, identifies every Python interpreter as the interpreter used to generate the virtual environment.

There is a related upstream bug at bpo22490, with an open PR at python/cpython#9516.

Setup

  • Your system is macOS.
  • You have not run virtualenv on this system before, or you have removed the user data directory at ~/Library/Application\ Support/virtualenv/.
  • You have a framework build of Python on your PATH. Example: /usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/bin/python3.7 (via Homebrew)
    (This is usually accessed as /usr/local/opt/python/bin/python3.7.)
  • You have another version of Python on your PATH. Example: ~/.pyenv/versions/3.8.1/bin/python3.8 (via pyenv)

Repro

  1. Create a virtual environment using the framework build:
    /usr/local/opt/python/bin/python3.7 -m venv /tmp/venv
  2. Install virtualenv into the virtual environment.
    /tmp/venv/bin/python -m pip install virtualenv
  3. Create a virtual environment for 3.8 using virtualenv.
    /tmp/venv/bin/virtualenv -p python3.8 /tmp/venv-3.8

Step 3 fails with the following error message:

RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.8'

Analysis

Here is a step-by-step analysis.

  1. User invokes /tmp/venv/bin/virtualenv -p python3.8 /tmp/venv-3.8.
  2. OS executes /tmp/venv/bin/python, a symlink to the framework build of Python 3.7.
  3. Framework build executes the actual Python binary, passing the original interpreter path (/tmp/venv/bin/python) in the environment variable __PYVENV_LAUNCHER__.
  4. Virtualenv spawns a subprocess executing ~/.pyenv/shims/python3.8 /tmp/venv/lib/.../virtualenv/discovery/py_info.py to obtain information about the interpreter.
  5. The shim executes ~/.pyenv/versions/3.8.1/bin/python3.8 (via ~/.pyenv/libexec/pyenv-exec).
  6. The interpreter imports ~/.pyenv/versions/3.8.1/lib/python3.8/site.py during initialization.
  7. The site.py script sees that the platform is darwin and the __PYVENV_LAUNCHER__ environment variable is set. It sets sys._base_executable to the value of __PYVENV_LAUNCHER__, and sys.prefix to the grand-parent directory of this path. sys.prefix is now /tmp/venv, while sys.base_prefix still points to the previous value of sys.prefix, ~/.pyenv/versions/3.8.1.
  8. Virtualenv sees that sys.prefix and sys.base_prefix differ, and assumes that the interpreter is running in a virtual environment. It determines the system executable using sys._base_executable, /tmp/venv/bin/python.
  9. Virtualenv uses /tmp/venv/bin/python to identify the interpreter, overriding the information previously gathered from the pyenv-provided Python 3.8. The interpreter is identified as Python 3.7.

References to source code:

Step 7:

https://github.com/python/cpython/blob/d4d17fd2cf69e7c8f4cd03fbf2d575370945b952/Lib/site.py#L460-L461

Step 8:

def _fast_get_system_executable(self):
"""Try to get the system executable by just looking at properties"""
if self.real_prefix or (
self.base_prefix is not None and self.base_prefix != self.prefix
): # if this is a virtual environment
if self.real_prefix is None:
base_executable = getattr(sys, "_base_executable", None) # some platforms may set this to help us
if base_executable is not None: # use the saved system executable if present
if sys.executable != base_executable: # we know we're in a virtual environment, cannot be us
return base_executable
return None # in this case we just can't tell easily without poking around FS and calling them, bail
# if we're not in a virtual environment, this is already a system python, so return the original executable
# note we must choose the original and not the pure executable as shim scripts might throw us off
return self.original_executable

Step 9:

@classmethod
def from_exe(cls, exe, raise_on_error=True, ignore_cache=False, resolve_to_host=True):
"""Given a path to an executable get the python information"""
# this method is not used by itself, so here and called functions can import stuff locally
from virtualenv.discovery.cached_py_info import from_exe
proposed = from_exe(cls, exe, raise_on_error=raise_on_error, ignore_cache=ignore_cache)
# noinspection PyProtectedMember
if isinstance(proposed, PythonInfo) and resolve_to_host:
proposed = proposed._resolve_to_system(proposed)
return proposed

@classmethod
def _resolve_to_system(cls, target):
start_executable = target.executable
prefixes = OrderedDict()
while target.system_executable is None:
prefix = target.real_prefix or target.base_prefix or target.prefix
if prefix in prefixes:
for at, (p, t) in enumerate(prefixes.items(), start=1):
logging.error("%d: prefix=%s, info=%r", at, p, t)
logging.error("%d: prefix=%s, info=%r", len(prefixes) + 1, prefix, target)
raise RuntimeError("prefixes are causing a circle {}".format("|".join(prefixes.keys())))
prefixes[prefix] = target
target = target.discover_exe(prefix=prefix, exact=False)
if target.executable != target.system_executable:
target = cls.from_exe(target.system_executable)
target.executable = start_executable
return target


Thanks for contributing a pull request, see checklist all is good!

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added news fragment in docs/changelog folder
Comment thread src/virtualenv/discovery/cached_py_info.py Outdated
Claudio Jolowicz and others added 3 commits February 21, 2020 13:32
@gaborbernat gaborbernat merged commit 4053f05 into pypa:master Feb 21, 2020
@cjolowicz

Copy link
Copy Markdown
Contributor Author

Thanks for merging! ...and sorry, did not get around to writing test and changelog earlier.

Updated PR description with repro and analysis.

@cjolowicz cjolowicz deleted the 1643 branch February 21, 2020 16:45
rdesgroppes added a commit to DataDog/datadog-agent that referenced this pull request Aug 14, 2025
The present change is a follow-up of #39563, where it was found that:
> We need to have a default value here because these unit tests are also
> called from `macos` runners  but [`system` is patched as
> `linux`](https://github.com/DataDog/datadog-agent/pull/39563/files#diff-407acb07e932b50660fc4999612d12a05d6046a1716a53125e1010050c898cebR33).
> So even though the unit tests are run in macos runners(run on EC2)
> they try to access vault and dont have K8 `POD_NAMESPACE` env var set.

It turns out the temporary fix introduced by #5010 in 2020 is no longer
needed:
- upstream bug report:
  pypa/virtualenv#1643
- upstream fix:
  pypa/virtualenv#1648
- released with `virtualenv` v20.0.5:
  https://virtualenv.pypa.io/en/latest/changelog.html#v20-0-5-2020-02-21

This therefore reverts commit d16a8a4:
"[omnibus] Fix MacOS build conflicting with system Python 3", and
adjusts consequences accordingly:
- the `POD_NAMESPACE` environment variable must be present (no default)
  for `linux`, and only for `linux`,
- patches in tests get tailored to the very needs. (3 `platforms` => 3
  "sub" tests)
rdesgroppes added a commit to DataDog/datadog-agent that referenced this pull request Aug 14, 2025
The present change is a follow-up of #39563, where it was found that:
> We need to have a default value here because these unit tests are also
> called from `macos` runners  but [`system` is patched as
> `linux`](https://github.com/DataDog/datadog-agent/pull/39563/files#diff-407acb07e932b50660fc4999612d12a05d6046a1716a53125e1010050c898cebR33).
> So even though the unit tests are run in macos runners(run on EC2)
> they try to access vault and dont have K8 `POD_NAMESPACE` env var set.
    
It turns out the temporary fix introduced by #5010 in 2020 is no longer
needed:
- upstream bug report:
  pypa/virtualenv#1643
- upstream fix:
  pypa/virtualenv#1648
- released with `virtualenv` v20.0.5:
  https://virtualenv.pypa.io/en/latest/changelog.html#v20-0-5-2020-02-21
    
This therefore reverts commit d16a8a4:
"[omnibus] Fix MacOS build conflicting with system Python 3", and
adjusts consequences accordingly:
- the `POD_NAMESPACE` environment variable must be present (no default)
  for `linux`, and only for `linux`,
- patches in tests get tailored to the very needs, with 3 `platforms` to
  cover implying 3 "sub" tests,
- covering `win32` unearthes unmatched command patterns due to suffixes
  (`.bat`, `.exe`) being used for cetain commands (like `aws.exe` and
  `omnibus.bat`, unlike `git`) which requires further adaptation,
  especially to understand which patterns are not found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants