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