From 71de653a714958d6649501b874009bafec74d3f4 Mon Sep 17 00:00:00 2001
From: Philipp A <flying-sheep@web.de>
Date: Tue, 23 Apr 2024 18:46:41 +0200
Subject: [PATCH 1/2] Allow builtin interpreter discovery to find specific
Python versions given a general spec (#2709)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Bernát Gábor <gaborjbernat@gmail.com>
Backported to 20.21.1 (without typing improvements).
---
docs/changelog/2709.bugfix.rst | 1 +
src/virtualenv/discovery/builtin.py | 78 +++++++++++++-------------
src/virtualenv/discovery/py_spec.py | 42 ++++++--------
tests/unit/discovery/test_discovery.py | 15 ++++-
4 files changed, 67 insertions(+), 69 deletions(-)
create mode 100644 docs/changelog/2709.bugfix.rst
diff --git a/docs/changelog/2709.bugfix.rst b/docs/changelog/2709.bugfix.rst
new file mode 100644
index 0000000..904da84
--- /dev/null
+++ b/docs/changelog/2709.bugfix.rst
@@ -0,0 +1 @@
+allow builtin discovery to discover specific interpreters (e.g. ``python3.12``) given an unspecific spec (e.g. ``python3``) - by :user:`flying-sheep`.
diff --git a/src/virtualenv/discovery/builtin.py b/src/virtualenv/discovery/builtin.py
index 40320d3..f79b5a5 100644
--- a/src/virtualenv/discovery/builtin.py
+++ b/src/virtualenv/discovery/builtin.py
@@ -1,6 +1,7 @@
import logging
import os
import sys
+from pathlib import Path
from virtualenv.info import IS_WIN
@@ -67,7 +68,7 @@ def get_interpreter(key, try_first_with, app_data=None, env=None):
proposed_paths.add(key)
-def propose_interpreters(spec, try_first_with, app_data, env=None):
+def propose_interpreters(spec, try_first_with, app_data=None, env=None): # noqa: C901, PLR0912
# 0. try with first
env = os.environ if env is None else env
for py_exe in try_first_with:
@@ -101,20 +102,17 @@ def propose_interpreters(spec, try_first_with, app_data, env=None):
for interpreter in propose_interpreters(spec, app_data, env):
yield interpreter, True
# finally just find on path, the path order matters (as the candidates are less easy to control by end user)
- paths = get_paths(env)
tested_exes = set()
- for pos, path in enumerate(paths):
- path_str = str(path)
- logging.debug(LazyPathDump(pos, path_str, env))
- for candidate, match in possible_specs(spec):
- found = check_path(candidate, path_str)
- if found is not None:
- exe = os.path.abspath(found)
- if exe not in tested_exes:
- tested_exes.add(exe)
- interpreter = PathPythonInfo.from_exe(exe, app_data, raise_on_error=False, env=env)
- if interpreter is not None:
- yield interpreter, match
+ find_candidates = path_exe_finder(spec)
+ for pos, path in enumerate(get_paths(env)):
+ logging.debug(LazyPathDump(pos, path, env))
+ for exe, impl_must_match in find_candidates(path):
+ if exe in tested_exes:
+ continue
+ tested_exes.add(exe)
+ interpreter = PathPythonInfo.from_exe(str(exe), app_data, raise_on_error=False, env=env)
+ if interpreter is not None:
+ yield interpreter, impl_must_match
def get_paths(env):
@@ -125,14 +123,14 @@ def get_paths(env):
except (AttributeError, ValueError):
path = os.defpath
if not path:
- paths = []
- else:
- paths = [p for p in path.split(os.pathsep) if os.path.exists(p)]
- return paths
+ return None
+ for p in map(Path, path.split(os.pathsep)):
+ if p.exists():
+ yield p
class LazyPathDump:
- def __init__(self, pos, path, env):
+ def __init__(self, pos, path, env) -> None:
self.pos = pos
self.path = path
self.env = env
@@ -141,35 +139,35 @@ class LazyPathDump:
content = f"discover PATH[{self.pos}]={self.path}"
if self.env.get("_VIRTUALENV_DEBUG"): # this is the over the board debug
content += " with =>"
- for file_name in os.listdir(self.path):
+ for file_path in self.path.iterdir():
try:
- file_path = os.path.join(self.path, file_name)
- if os.path.isdir(file_path) or not os.access(file_path, os.X_OK):
+ if file_path.is_dir() or not (file_path.stat().st_mode & os.X_OK):
continue
except OSError:
pass
content += " "
- content += file_name
+ content += file_path.name
return content
-def check_path(candidate, path):
- _, ext = os.path.splitext(candidate)
- if sys.platform == "win32" and ext != ".exe":
- candidate = candidate + ".exe"
- if os.path.isfile(candidate):
- return candidate
- candidate = os.path.join(path, candidate)
- if os.path.isfile(candidate):
- return candidate
- return None
-
-
-def possible_specs(spec):
- # 4. then maybe it's something exact on PATH - if it was direct lookup implementation no longer counts
- yield spec.str_spec, False
- # 5. or from the spec we can deduce a name on path that matches
- yield from spec.generate_names()
+def path_exe_finder(spec):
+ """Given a spec, return a function that can be called on a path to find all matching files in it."""
+ pat = spec.generate_re(windows=sys.platform == "win32")
+ direct = spec.str_spec
+ if sys.platform == "win32":
+ direct = f"{direct}.exe"
+
+ def path_exes(path):
+ # 4. then maybe it's something exact on PATH - if it was direct lookup implementation no longer counts
+ yield (path / direct), False
+ # 5. or from the spec we can deduce if a name on path matches
+ for exe in path.iterdir():
+ match = pat.fullmatch(exe.name)
+ if match:
+ # the implementation must match when we find “python[ver]”
+ yield exe.absolute(), match["impl"] == "python"
+
+ return path_exes
class PathPythonInfo(PythonInfo):
diff --git a/src/virtualenv/discovery/py_spec.py b/src/virtualenv/discovery/py_spec.py
index 103c7ae..cbfdfb8 100644
--- a/src/virtualenv/discovery/py_spec.py
+++ b/src/virtualenv/discovery/py_spec.py
@@ -1,10 +1,9 @@
"""A Python specification is an abstract requirement definition of an interpreter"""
+from __future__ import annotations
+
import os
import re
-from collections import OrderedDict
-
-from virtualenv.info import fs_is_case_sensitive
PATTERN = re.compile(r"^(?P<impl>[a-zA-Z]+)?(?P<version>[0-9.]+)?(?:-(?P<arch>32|64))?$")
@@ -12,7 +11,7 @@ PATTERN = re.compile(r"^(?P<impl>[a-zA-Z]+)?(?P<version>[0-9.]+)?(?:-(?P<arch>32
class PythonSpec:
"""Contains specification about a Python Interpreter"""
- def __init__(self, str_spec, implementation, major, minor, micro, architecture, path):
+ def __init__(self, str_spec, implementation, major, minor, micro, architecture, path) -> None: # noqa: PLR0913
self.str_spec = str_spec
self.implementation = implementation
self.major = major
@@ -22,7 +21,7 @@ class PythonSpec:
self.path = path
@classmethod
- def from_string_spec(cls, string_spec):
+ def from_string_spec(cls, string_spec): # noqa: C901, PLR0912
impl, major, minor, micro, arch, path = None, None, None, None, None, None
if os.path.isabs(string_spec):
path = string_spec
@@ -64,27 +63,18 @@ class PythonSpec:
return cls(string_spec, impl, major, minor, micro, arch, path)
- def generate_names(self):
- impls = OrderedDict()
- if self.implementation:
- # first consider implementation as it is
- impls[self.implementation] = False
- if fs_is_case_sensitive():
- # for case sensitive file systems consider lower and upper case versions too
- # trivia: MacBooks and all pre 2018 Windows-es were case insensitive by default
- impls[self.implementation.lower()] = False
- impls[self.implementation.upper()] = False
- impls["python"] = True # finally consider python as alias, implementation must match now
- version = self.major, self.minor, self.micro
- try:
- version = version[: version.index(None)]
- except ValueError:
- pass
- for impl, match in impls.items():
- for at in range(len(version), -1, -1):
- cur_ver = version[0:at]
- spec = f"{impl}{'.'.join(str(i) for i in cur_ver)}"
- yield spec, match
+ def generate_re(self, *, windows):
+ """Generate a regular expression for matching against a filename."""
+ version = r"{}(\.{}(\.{})?)?".format(
+ *(r"\d+" if v is None else v for v in (self.major, self.minor, self.micro))
+ )
+ impl = "python" if self.implementation is None else f"python|{re.escape(self.implementation)}"
+ suffix = r"\.exe" if windows else ""
+ # Try matching `direct` first, so the `direct` group is filled when possible.
+ return re.compile(
+ rf"(?P<impl>{impl})(?P<v>{version}){suffix}$",
+ flags=re.IGNORECASE,
+ )
@property
def is_abs(self):
diff --git a/tests/unit/discovery/test_discovery.py b/tests/unit/discovery/test_discovery.py
index c4a2cc7..51bae24 100644
--- a/tests/unit/discovery/test_discovery.py
+++ b/tests/unit/discovery/test_discovery.py
@@ -14,16 +14,25 @@ from virtualenv.info import fs_supports_symlink
@pytest.mark.skipif(not fs_supports_symlink(), reason="symlink not supported")
@pytest.mark.parametrize("case", ["mixed", "lower", "upper"])
-def test_discovery_via_path(monkeypatch, case, tmp_path, caplog, session_app_data):
+@pytest.mark.parametrize("specificity", ["more", "less"])
+def test_discovery_via_path(monkeypatch, case, specificity, tmp_path, caplog, session_app_data): # noqa: PLR0913
caplog.set_level(logging.DEBUG)
current = PythonInfo.current_system(session_app_data)
- core = f"somethingVeryCryptic{'.'.join(str(i) for i in current.version_info[0:3])}"
name = "somethingVeryCryptic"
if case == "lower":
name = name.lower()
elif case == "upper":
name = name.upper()
- exe_name = f"{name}{current.version_info.major}{'.exe' if sys.platform == 'win32' else ''}"
+ if specificity == "more":
+ # e.g. spec: python3, exe: /bin/python3.12
+ core_ver = current.version_info.major
+ exe_ver = ".".join(str(i) for i in current.version_info[0:2])
+ elif specificity == "less":
+ # e.g. spec: python3.12.1, exe: /bin/python3
+ core_ver = ".".join(str(i) for i in current.version_info[0:3])
+ exe_ver = current.version_info.major
+ core = f"somethingVeryCryptic{core_ver}"
+ exe_name = f"{name}{exe_ver}{'.exe' if sys.platform == 'win32' else ''}"
target = tmp_path / current.install_path("scripts")
target.mkdir(parents=True)
executable = target / exe_name
--
2.45.2
From 4bb73d175614aa6041b1e9e57d7302b9c253b5ef Mon Sep 17 00:00:00 2001
From: Ofek Lev <ofekmeister@gmail.com>
Date: Sat, 27 Apr 2024 14:43:00 -0400
Subject: [PATCH 2/2] Fix PATH-based Python discovery on Windows (#2712)
---
docs/changelog/2712.bugfix.rst | 1 +
src/virtualenv/discovery/builtin.py | 44 ++++++++++++++++++++------
src/virtualenv/discovery/py_spec.py | 10 +++++-
src/virtualenv/info.py | 5 +++
tests/unit/discovery/test_discovery.py | 8 +++--
5 files changed, 55 insertions(+), 13 deletions(-)
create mode 100644 docs/changelog/2712.bugfix.rst
diff --git a/docs/changelog/2712.bugfix.rst b/docs/changelog/2712.bugfix.rst
new file mode 100644
index 0000000..fee5436
--- /dev/null
+++ b/docs/changelog/2712.bugfix.rst
@@ -0,0 +1 @@
+fix PATH-based Python discovery on Windows - by :user:`ofek`.
diff --git a/src/virtualenv/discovery/builtin.py b/src/virtualenv/discovery/builtin.py
index f79b5a5..456c68b 100644
--- a/src/virtualenv/discovery/builtin.py
+++ b/src/virtualenv/discovery/builtin.py
@@ -3,7 +3,7 @@ import os
import sys
from pathlib import Path
-from virtualenv.info import IS_WIN
+from virtualenv.info import IS_WIN, fs_path_id
from .discover import Discover
from .py_info import PythonInfo
@@ -68,9 +68,10 @@ def get_interpreter(key, try_first_with, app_data=None, env=None):
proposed_paths.add(key)
-def propose_interpreters(spec, try_first_with, app_data=None, env=None): # noqa: C901, PLR0912
+def propose_interpreters(spec, try_first_with, app_data=None, env=None): # noqa: C901, PLR0912, PLR0915
# 0. try with first
env = os.environ if env is None else env
+ tested_exes: set[str] = set()
for py_exe in try_first_with:
path = os.path.abspath(py_exe)
try:
@@ -78,7 +79,12 @@ def propose_interpreters(spec, try_first_with, app_data=None, env=None): # noqa
except OSError:
pass
else:
- yield PythonInfo.from_exe(os.path.abspath(path), app_data, env=env), True
+ exe_raw = os.path.abspath(path)
+ exe_id = fs_path_id(exe_raw)
+ if exe_id in tested_exes:
+ continue
+ tested_exes.add(exe_id)
+ yield PythonInfo.from_exe(exe_raw, app_data, env=env), True
# 1. if it's a path and exists
if spec.path is not None:
@@ -88,29 +94,44 @@ def propose_interpreters(spec, try_first_with, app_data=None, env=None): # noqa
if spec.is_abs:
raise
else:
- yield PythonInfo.from_exe(os.path.abspath(spec.path), app_data, env=env), True
+ exe_raw = os.path.abspath(spec.path)
+ exe_id = fs_path_id(exe_raw)
+ if exe_id not in tested_exes:
+ tested_exes.add(exe_id)
+ yield PythonInfo.from_exe(exe_raw, app_data, env=env), True
if spec.is_abs:
return
else:
# 2. otherwise try with the current
- yield PythonInfo.current_system(app_data), True
+ current_python = PythonInfo.current_system(app_data)
+ exe_raw = str(current_python.executable)
+ exe_id = fs_path_id(exe_raw)
+ if exe_id not in tested_exes:
+ tested_exes.add(exe_id)
+ yield current_python, True
# 3. otherwise fallback to platform default logic
if IS_WIN:
from .windows import propose_interpreters
for interpreter in propose_interpreters(spec, app_data, env):
+ exe_raw = str(interpreter.executable)
+ exe_id = fs_path_id(exe_raw)
+ if exe_id in tested_exes:
+ continue
+ tested_exes.add(exe_id)
yield interpreter, True
# finally just find on path, the path order matters (as the candidates are less easy to control by end user)
- tested_exes = set()
find_candidates = path_exe_finder(spec)
for pos, path in enumerate(get_paths(env)):
logging.debug(LazyPathDump(pos, path, env))
for exe, impl_must_match in find_candidates(path):
- if exe in tested_exes:
+ exe_raw = str(exe)
+ exe_id = fs_path_id(exe_raw)
+ if exe_id in tested_exes:
continue
- tested_exes.add(exe)
- interpreter = PathPythonInfo.from_exe(str(exe), app_data, raise_on_error=False, env=env)
+ tested_exes.add(exe_id)
+ interpreter = PathPythonInfo.from_exe(exe_raw, app_data, raise_on_error=False, env=env)
if interpreter is not None:
yield interpreter, impl_must_match
@@ -159,7 +180,10 @@ def path_exe_finder(spec):
def path_exes(path):
# 4. then maybe it's something exact on PATH - if it was direct lookup implementation no longer counts
- yield (path / direct), False
+ direct_path = path / direct
+ if direct_path.exists():
+ yield direct_path, False
+
# 5. or from the spec we can deduce if a name on path matches
for exe in path.iterdir():
match = pat.fullmatch(exe.name)
diff --git a/src/virtualenv/discovery/py_spec.py b/src/virtualenv/discovery/py_spec.py
index cbfdfb8..04b63b8 100644
--- a/src/virtualenv/discovery/py_spec.py
+++ b/src/virtualenv/discovery/py_spec.py
@@ -70,9 +70,17 @@ class PythonSpec:
)
impl = "python" if self.implementation is None else f"python|{re.escape(self.implementation)}"
suffix = r"\.exe" if windows else ""
+ version_conditional = (
+ "?"
+ # Windows Python executables are almost always unversioned
+ if windows
+ # Spec is an empty string
+ or self.major is None
+ else ""
+ )
# Try matching `direct` first, so the `direct` group is filled when possible.
return re.compile(
- rf"(?P<impl>{impl})(?P<v>{version}){suffix}$",
+ rf"(?P<impl>{impl})(?P<v>{version}){version_conditional}{suffix}$",
flags=re.IGNORECASE,
)
diff --git a/src/virtualenv/info.py b/src/virtualenv/info.py
index a4fc4bf..dd96f10 100644
--- a/src/virtualenv/info.py
+++ b/src/virtualenv/info.py
@@ -47,11 +47,16 @@ def fs_supports_symlink():
return _CAN_SYMLINK
+def fs_path_id(path: str) -> str:
+ return path.casefold() if fs_is_case_sensitive() else path
+
+
__all__ = (
"IS_PYPY",
"IS_CPYTHON",
"IS_WIN",
"fs_is_case_sensitive",
+ "fs_path_id",
"fs_supports_symlink",
"ROOT",
"IS_ZIPAPP",
diff --git a/tests/unit/discovery/test_discovery.py b/tests/unit/discovery/test_discovery.py
index 51bae24..6c64b40 100644
--- a/tests/unit/discovery/test_discovery.py
+++ b/tests/unit/discovery/test_discovery.py
@@ -14,7 +14,7 @@ from virtualenv.info import fs_supports_symlink
@pytest.mark.skipif(not fs_supports_symlink(), reason="symlink not supported")
@pytest.mark.parametrize("case", ["mixed", "lower", "upper"])
-@pytest.mark.parametrize("specificity", ["more", "less"])
+@pytest.mark.parametrize("specificity", ["more", "less", "none"])
def test_discovery_via_path(monkeypatch, case, specificity, tmp_path, caplog, session_app_data): # noqa: PLR0913
caplog.set_level(logging.DEBUG)
current = PythonInfo.current_system(session_app_data)
@@ -31,7 +31,11 @@ def test_discovery_via_path(monkeypatch, case, specificity, tmp_path, caplog, se
# e.g. spec: python3.12.1, exe: /bin/python3
core_ver = ".".join(str(i) for i in current.version_info[0:3])
exe_ver = current.version_info.major
- core = f"somethingVeryCryptic{core_ver}"
+ elif specificity == "none":
+ # e.g. spec: python3.12.1, exe: /bin/python
+ core_ver = ".".join(str(i) for i in current.version_info[0:3])
+ exe_ver = ""
+ core = "" if specificity == "none" else f"{name}{core_ver}"
exe_name = f"{name}{exe_ver}{'.exe' if sys.platform == 'win32' else ''}"
target = tmp_path / current.install_path("scripts")
target.mkdir(parents=True)
--
2.45.2