From 71de653a714958d6649501b874009bafec74d3f4 Mon Sep 17 00:00:00 2001 From: Philipp A 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 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[a-zA-Z]+)?(?P[0-9.]+)?(?:-(?P32|64))?$") @@ -12,7 +11,7 @@ PATTERN = re.compile(r"^(?P[a-zA-Z]+)?(?P[0-9.]+)?(?:-(?P32 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})(?P{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 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})(?P{version}){suffix}$", + rf"(?P{impl})(?P{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