From 498d806dcc2ff74811667984112d9c7e5a400e4d Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Mon, 10 Feb 2020 12:35:26 +0000 Subject: [PATCH 1/3] fix system executable discovery Signed-off-by: Bernat Gabor --- docs/changelog/1552.bugfix.rst | 1 + docs/changelog/1553.bugfix.rst | 2 ++ src/virtualenv/create/creator.py | 2 +- src/virtualenv/discovery/py_info.py | 3 ++- tests/unit/create/test_creator.py | 8 +++++++- 5 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/1552.bugfix.rst create mode 100644 docs/changelog/1553.bugfix.rst diff --git a/docs/changelog/1552.bugfix.rst b/docs/changelog/1552.bugfix.rst new file mode 100644 index 000000000..a57f2a757 --- /dev/null +++ b/docs/changelog/1552.bugfix.rst @@ -0,0 +1 @@ +Virtual environments created via relative path on Windows creates bad console executables - by :user:`gaborbernat`. diff --git a/docs/changelog/1553.bugfix.rst b/docs/changelog/1553.bugfix.rst new file mode 100644 index 000000000..2084dc977 --- /dev/null +++ b/docs/changelog/1553.bugfix.rst @@ -0,0 +1,2 @@ +Seems sometimes venvs created set their base executable to themselves; we accept these without question, so we handle +virtual environments as system pythons causing issues - by :user:`gaborbernat`. diff --git a/src/virtualenv/create/creator.py b/src/virtualenv/create/creator.py index 7965bd0b8..ab4545cfb 100644 --- a/src/virtualenv/create/creator.py +++ b/src/virtualenv/create/creator.py @@ -131,7 +131,7 @@ def non_write_able(dest, value): # pre 3.6 resolve is always strict, aka must exists, sidestep by using os.path operation dest = Path(os.path.realpath(raw_value)) else: - dest = value.resolve() + dest = value.resolve().absolute() # on Windows absolute does not imply resolve so use both value = dest while dest: if dest.exists(): diff --git a/src/virtualenv/discovery/py_info.py b/src/virtualenv/discovery/py_info.py index a922e47ef..eb1534147 100644 --- a/src/virtualenv/discovery/py_info.py +++ b/src/virtualenv/discovery/py_info.py @@ -96,7 +96,8 @@ def _fast_get_system_executable(self): 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 - return base_executable + 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 diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 75e4ca977..c9c1f44dd 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -14,7 +14,7 @@ import six from virtualenv.__main__ import run -from virtualenv.create.creator import DEBUG_SCRIPT, get_env_debug_info +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, fs_supports_symlink @@ -291,3 +291,9 @@ def create(count): thread.start() for thread in threads: thread.join() + + +def test_creator_input_passed_is_abs(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + result = Creator.validate_dest("venv") + assert str(result) == str(tmp_path / "venv") From 1088ba516dfec682267fc96d7e753548e787bc8d Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Mon, 10 Feb 2020 13:40:25 +0000 Subject: [PATCH 2/3] time to eat our own dogfood Signed-off-by: Bernat Gabor --- azure-pipelines.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 30f79b506..6a2dd8a2a 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -77,6 +77,8 @@ jobs: pathtoPublish: $(Build.ArtifactStagingDirectory) ArtifactName: virtualenv.pyz before: + - script: '$(toxPython.pythonLocation)/python -m pip install .' + displayName: tox uses this virtualenv (eat our own dogfood) - script: 'sudo apt-get update -y && sudo apt-get install fish csh' condition: and(succeeded(), eq(variables['image_name'], 'linux'), in(variables['TOXENV'], 'py38', 'py37', 'py36', 'py35', 'py27', 'pypy', 'pypy3')) displayName: install fish and csh via apt-get From 6f9a7c39d1c6d3f1fb9a9ddb163a97d59afc509c Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Mon, 10 Feb 2020 13:48:25 +0000 Subject: [PATCH 3/3] seems Path.absolute should not be used Signed-off-by: Bernat Gabor --- azure-pipelines.yml | 4 ++++ src/virtualenv/create/creator.py | 4 ++-- .../create/via_global_ref/builtin/python2/python2.py | 2 +- src/virtualenv/discovery/cached_py_info.py | 3 ++- src/virtualenv/util/path/_pathlib/via_os_path.py | 3 --- tests/unit/create/test_creator.py | 2 +- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 6a2dd8a2a..c9ab77269 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -77,8 +77,12 @@ jobs: pathtoPublish: $(Build.ArtifactStagingDirectory) ArtifactName: virtualenv.pyz before: + - script: '$(toxPython.pythonLocation)/python -m pip install "virtualenv < 20"' + displayName: pypy3 tox needs fix to accomodate separate bin and Script + condition: and(succeeded(), eq(variables['image_name'], 'windows'), in(variables['TOXENV'], 'pypy3')) - script: '$(toxPython.pythonLocation)/python -m pip install .' displayName: tox uses this virtualenv (eat our own dogfood) + condition: not(and(succeeded(), eq(variables['image_name'], 'windows'), in(variables['TOXENV'], 'pypy3'))) - script: 'sudo apt-get update -y && sudo apt-get install fish csh' condition: and(succeeded(), eq(variables['image_name'], 'linux'), in(variables['TOXENV'], 'py38', 'py37', 'py36', 'py35', 'py27', 'pypy', 'pypy3')) displayName: install fish and csh via apt-get diff --git a/src/virtualenv/create/creator.py b/src/virtualenv/create/creator.py index ab4545cfb..bcf65ec19 100644 --- a/src/virtualenv/create/creator.py +++ b/src/virtualenv/create/creator.py @@ -22,7 +22,7 @@ from virtualenv.util.zipapp import ensure_file_on_disk from virtualenv.version import __version__ -HERE = Path(__file__).absolute().parent +HERE = Path(os.path.abspath(__file__)).parent DEBUG_SCRIPT = HERE / "debug.py" @@ -131,7 +131,7 @@ def non_write_able(dest, value): # pre 3.6 resolve is always strict, aka must exists, sidestep by using os.path operation dest = Path(os.path.realpath(raw_value)) else: - dest = value.resolve().absolute() # on Windows absolute does not imply resolve so use both + dest = Path(os.path.abspath(str(value))).resolve() # on Windows absolute does not imply resolve so use both value = dest while dest: if dest.exists(): diff --git a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py index 9b14c2e70..621c68ec5 100644 --- a/src/virtualenv/create/via_global_ref/builtin/python2/python2.py +++ b/src/virtualenv/create/via_global_ref/builtin/python2/python2.py @@ -14,7 +14,7 @@ from ..via_global_self_do import ViaGlobalRefVirtualenvBuiltin -HERE = Path(__file__).absolute().parent +HERE = Path(os.path.abspath(__file__)).parent @six.add_metaclass(abc.ABCMeta) diff --git a/src/virtualenv/discovery/cached_py_info.py b/src/virtualenv/discovery/cached_py_info.py index 03e3d57c1..73b2310de 100644 --- a/src/virtualenv/discovery/cached_py_info.py +++ b/src/virtualenv/discovery/cached_py_info.py @@ -8,6 +8,7 @@ import json import logging +import os import pipes import sys from collections import OrderedDict @@ -99,7 +100,7 @@ def _get_fs_path(): def _run_subprocess(cls, exe): - resolved_path = Path(__file__).parent.absolute().absolute() / "py_info.py" + resolved_path = Path(os.path.abspath(__file__)).parent / "py_info.py" with ensure_file_on_disk(resolved_path) as resolved_path: cmd = [exe, "-s", str(resolved_path)] diff --git a/src/virtualenv/util/path/_pathlib/via_os_path.py b/src/virtualenv/util/path/_pathlib/via_os_path.py index 44bd2bf49..56315c1a8 100644 --- a/src/virtualenv/util/path/_pathlib/via_os_path.py +++ b/src/virtualenv/util/path/_pathlib/via_os_path.py @@ -52,9 +52,6 @@ def __hash__(self): def exists(self): return os.path.exists(self._path) - def absolute(self): - return Path(os.path.abspath(self._path)) - @property def parent(self): return Path(os.path.abspath(os.path.join(self._path, os.path.pardir))) diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index c9c1f44dd..90b5db1c2 100644 --- a/tests/unit/create/test_creator.py +++ b/tests/unit/create/test_creator.py @@ -69,7 +69,7 @@ def test_destination_not_write_able(tmp_path, capsys): def cleanup_sys_path(paths): from virtualenv.create.creator import HERE - paths = [Path(i).absolute() for i in paths] + paths = [Path(os.path.abspath(i)) for i in paths] to_remove = [Path(HERE)] if os.environ.get(str("PYCHARM_HELPERS_DIR")): to_remove.append(Path(os.environ[str("PYCHARM_HELPERS_DIR")]).parent)