Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix setting PYTHONPATH on Python 2 not working #1673

Merged
merged 6 commits into from
Mar 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/1673.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ``PYTHONPATH`` being overriden on Python 2 — by :user:`jd`.
2 changes: 1 addition & 1 deletion src/virtualenv/create/via_global_ref/builtin/pypy/pypy2.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def skip_rewrite(self):
PyPy2 built-in imports are handled by this path entry, don't overwrite to not disable it
see: https://github.com/pypa/virtualenv/issues/1652
"""
return 'or value.endswith("lib_pypy{}__extensions__")'.format(os.sep)
return 'or path.endswith("lib_pypy{}__extensions__") # PyPy2 built-in import marker'.format(os.sep)


class PyPy2Posix(PyPy2, PosixSupports):
Expand Down
73 changes: 46 additions & 27 deletions src/virtualenv/create/via_global_ref/builtin/python2/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,40 +78,59 @@ def read_pyvenv():

def rewrite_standard_library_sys_path():
"""Once this site file is loaded the standard library paths have already been set, fix them up"""
exe = abs_path(sys.executable)
exe, prefix, exec_prefix = get_exe_prefixes(base=False)
base_exe, base_prefix, base_exec = get_exe_prefixes(base=True)
exe_dir = exe[: exe.rfind(sep)]
prefix, exec_prefix = abs_path(sys.prefix), abs_path(sys.exec_prefix)
base_prefix, base_exec_prefix = abs_path(sys.base_prefix), abs_path(sys.base_exec_prefix)
base_executable = abs_path(sys.base_executable)
for at, value in enumerate(sys.path):
value = abs_path(value)
# replace old sys prefix path starts with new
skip_rewrite = value == exe_dir # don't fix the current executable location, notably on Windows this gets added
for at, path in enumerate(sys.path):
path = abs_path(path) # replace old sys prefix path starts with new
skip_rewrite = path == exe_dir # don't fix the current executable location, notably on Windows this gets added
skip_rewrite = skip_rewrite # ___SKIP_REWRITE____
if not skip_rewrite:
if value.startswith(exe_dir):
# content inside the exe folder needs to remap to original executables folder
orig_exe_folder = base_executable[: base_executable.rfind(sep)]
value = "{}{}".format(orig_exe_folder, value[len(exe_dir) :])
elif value.startswith(prefix):
value = "{}{}".format(base_prefix, value[len(prefix) :])
elif value.startswith(exec_prefix):
value = "{}{}".format(base_exec_prefix, value[len(exec_prefix) :])
sys.path[at] = value
sys.path[at] = map_path(path, base_exe, exe_dir, exec_prefix, base_prefix, prefix, base_exec)

# the rewrite above may have changed elements from PYTHONPATH, revert these if on
if sys.flags.ignore_environment:
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved
return
import os

python_paths = []
if "PYTHONPATH" in os.environ and os.environ["PYTHONPATH"]:
for path in os.environ["PYTHONPATH"].split(os.pathsep):
if path not in python_paths:
python_paths.append(path)
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved
sys.path[: len(python_paths)] = python_paths
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved


def get_exe_prefixes(base=False):
return tuple(abs_path(getattr(sys, ("base_" if base else "") + i)) for i in ("executable", "prefix", "exec_prefix"))


def abs_path(value):
keep = []
values = value.split(sep)
i = len(values) - 1
while i >= 0:
if values[i] == "..":
i -= 1
values, keep = value.split(sep), []
at = len(values) - 1
while at >= 0:
if values[at] == "..":
at -= 1
else:
keep.append(values[i])
i -= 1
value = sep.join(keep[::-1])
return value
keep.append(values[at])
at -= 1
return sep.join(keep[::-1])


def map_path(path, base_executable, exe_dir, exec_prefix, base_prefix, prefix, base_exec_prefix):
if path_starts_with(path, exe_dir):
# content inside the exe folder needs to remap to original executables folder
orig_exe_folder = base_executable[: base_executable.rfind(sep)]
return "{}{}".format(orig_exe_folder, path[len(exe_dir) :])
elif path_starts_with(path, prefix):
return "{}{}".format(base_prefix, path[len(prefix) :])
elif path_starts_with(path, exec_prefix):
return "{}{}".format(base_exec_prefix, path[len(exec_prefix) :])
return path


def path_starts_with(directory, value):
return directory.startswith(value if value[-1] == sep else value + sep)


def disable_user_site_package():
Expand Down
16 changes: 8 additions & 8 deletions tests/unit/create/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@


# noinspection PyUnusedLocal
def root(tmp_path_factory):
def root(tmp_path_factory, session_app_data):
return CURRENT.system_executable


def venv(tmp_path_factory):
def venv(tmp_path_factory, session_app_data):
if CURRENT.is_venv:
return sys.executable
elif CURRENT.version_info.major == 3:
Expand All @@ -40,7 +40,7 @@ def venv(tmp_path_factory):
return exe_path


def old_virtualenv(tmp_path_factory):
def old_virtualenv(tmp_path_factory, session_app_data):
if CURRENT.is_old_virtualenv:
return CURRENT.executable
else:
Expand Down Expand Up @@ -77,18 +77,18 @@ def old_virtualenv(tmp_path_factory):
process = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
_, __ = process.communicate()
assert not process.returncode
exe_path = CURRENT.discover_exe(prefix=str(old_virtualenv_at)).original_executable
exe_path = CURRENT.discover_exe(session_app_data, prefix=str(old_virtualenv_at)).original_executable
return exe_path
except Exception:
return RuntimeError("failed to create old virtualenv")
except Exception as exception:
return RuntimeError("failed to create old virtualenv %r".format(exception))


PYTHON = {"root": root, "venv": venv, "old_virtualenv": old_virtualenv}


@pytest.fixture(params=list(PYTHON.values()), ids=list(PYTHON.keys()), scope="session")
def python(request, tmp_path_factory):
result = request.param(tmp_path_factory)
def python(request, tmp_path_factory, session_app_data):
result = request.param(tmp_path_factory, session_app_data)
if isinstance(result, Exception):
pytest.skip("could not resolve interpreter based on {} because {}".format(request.param.__name__, result))
if result is None:
Expand Down
62 changes: 60 additions & 2 deletions tests/unit/create/test_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

import difflib
import gc
import json
import logging
import os
import shutil
import stat
import subprocess
import sys
from collections import OrderedDict
from itertools import product
from stat import S_IREAD, S_IRGRP, S_IROTH
from textwrap import dedent
Expand All @@ -19,11 +21,11 @@
from virtualenv.create.creator import DEBUG_SCRIPT, Creator, get_env_debug_info
from virtualenv.discovery.builtin import get_interpreter
from virtualenv.discovery.py_info import PythonInfo
from virtualenv.info import IS_PYPY, PY3, fs_supports_symlink
from virtualenv.info import IS_PYPY, PY3, fs_is_case_sensitive, fs_supports_symlink
from virtualenv.pyenv_cfg import PyEnvCfg
from virtualenv.run import cli_run, session_via_cli
from virtualenv.util.path import Path
from virtualenv.util.six import ensure_text
from virtualenv.util.six import ensure_str, ensure_text

CURRENT = PythonInfo.current_system()

Expand Down Expand Up @@ -386,3 +388,59 @@ def test_create_distutils_cfg(creator, tmp_path, monkeypatch):

package_folder = result.creator.platlib / "demo" # prefix is set to the virtualenv prefix for install
assert package_folder.exists()


@pytest.mark.parametrize("python_path_on", [True, False], ids=["on", "off"])
@pytest.mark.skipif(PY3, reason="we rewrite sys.path only on PY2")
def test_python_path(monkeypatch, tmp_path, python_path_on):
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved
result = cli_run([ensure_text(str(tmp_path)), "--without-pip", "--activators", ""])
monkeypatch.chdir(tmp_path)
case_sensitive = fs_is_case_sensitive()

def _get_sys_path(flag=None):
cmd = [str(result.creator.exe)]
if flag:
cmd.append(flag)
cmd.extend(["-c", "import json; import sys; print(json.dumps(sys.path))"])
return [i if case_sensitive else i.lower() for i in json.loads(subprocess.check_output(cmd))]

monkeypatch.delenv(str("PYTHONPATH"), raising=False)
base = _get_sys_path()

# note the value result.creator.interpreter.system_stdlib cannot be set, as that would disable our custom site.py
python_paths = [
str(Path(result.creator.interpreter.prefix)),
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved
str(Path(result.creator.interpreter.system_stdlib) / "b"),
str(result.creator.purelib / "a"),
str(result.creator.purelib),
str(result.creator.bin_dir),
str(tmp_path / "base"),
str(tmp_path / "base_sep") + os.sep,
"name",
"name{}".format(os.sep),
str(tmp_path.parent / (ensure_text(tmp_path.name) + "_suffix")),
".",
"..",
"",
]
python_path_env = os.pathsep.join(ensure_str(i) for i in python_paths)
monkeypatch.setenv(str("PYTHONPATH"), python_path_env)

extra_all = _get_sys_path(None if python_path_on else "-E")
if python_path_on:
assert extra_all[0] == "" # the cwd is always injected at start as ''
extra_all = extra_all[1:]
assert base[0] == ""
base = base[1:]

assert not (set(base) - set(extra_all)) # all base paths are present
abs_python_paths = list(OrderedDict((os.path.abspath(ensure_text(i)), None) for i in python_paths).keys())
abs_python_paths = [i if case_sensitive else i.lower() for i in abs_python_paths]

extra_as_python_path = extra_all[: len(abs_python_paths)]
assert abs_python_paths == extra_as_python_path # python paths are there at the start

non_python_path = extra_all[len(abs_python_paths) :]
assert non_python_path == [i for i in base if i not in extra_as_python_path]
else:
assert base == extra_all
gaborbernat marked this conversation as resolved.
Show resolved Hide resolved