Improve discovery on Windows and provide escape hatchet (#2046)

Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>

docs/changelog/1986.bugfix.rst

@@ -0,0 +1 @@
+On Windows python ``3.7+`` distributions where the exe shim is missing fallback to the old ways - by :user:`gaborbernat`.

docs/changelog/2046.bugfix.rst

@@ -0,0 +1,2 @@
+When discovering interpreters on Windows, via the PEP-514, prefer ``PythonCore`` releases over other ones. virtualenv
+is used via pip mostly by this distribution, so prefer it over other such as conda - by :user:`gaborbernat`.

docs/changelog/2046.feature.rst

@@ -0,0 +1,3 @@
+The builtin discovery takes now a ``--try-first-with`` argument and is first attempted as valid interpreters. One can
+use this to force discovery of a given python executable when the discovery order/mechanism raises errors -
+by :user:`gaborbernat`.

src/virtualenv/create/via_global_ref/builtin/cpython/cpython3.py

@@ -55,20 +55,27 @@ class CPython3Windows(CPythonWindows, CPython3):
     def sources(cls, interpreter):
         for src in super(CPython3Windows, cls).sources(interpreter):
             yield src
-        if not cls.venv_37p(interpreter):
+        if not cls.has_shim(interpreter):
             for src in cls.include_dll_and_pyd(interpreter):
                 yield src
 
-    @staticmethod
-    def venv_37p(interpreter):
-        return interpreter.version_info.minor >= 7
+    @classmethod
+    def has_shim(cls, interpreter):
+        return interpreter.version_info.minor >= 7 and cls.shim(interpreter) is not None
+
+    @classmethod
+    def shim(cls, interpreter):
+        shim = Path(interpreter.system_stdlib) / "venv" / "scripts" / "nt" / "python.exe"
+        if shim.exists():
+            return shim
+        return None
 
     @classmethod
     def host_python(cls, interpreter):
-        if cls.venv_37p(interpreter):
+        if cls.has_shim(interpreter):
             # starting with CPython 3.7 Windows ships with a venvlauncher.exe that avoids the need for dll/pyd copies
             # it also means the wrapper must be copied to avoid bugs such as https://bugs.python.org/issue42013
-            return Path(interpreter.system_stdlib) / "venv" / "scripts" / "nt" / "python.exe"
+            return cls.shim(interpreter)
         return super(CPython3Windows, cls).host_python(interpreter)
 
     @classmethod

src/virtualenv/discovery/builtin.py

@@ -17,6 +17,7 @@ class Builtin(Discover):
         super(Builtin, self).__init__(options)
         self.python_spec = options.python if options.python else [sys.executable]
         self.app_data = options.app_data
+        self.try_first_with = options.try_first_with
 
     @classmethod
     def add_parser_arguments(cls, parser):
@@ -31,10 +32,19 @@ class Builtin(Discover):
             help="interpreter based on what to create environment (path/identifier) "
             "- by default use the interpreter where the tool is installed - first found wins",
         )
+        parser.add_argument(
+            "--try-first-with",
+            dest="try_first_with",
+            metavar="py_exe",
+            type=str,
+            action="append",
+            default=[],
+            help="try first these interpreters before starting the discovery",
+        )
 
     def run(self):
         for python_spec in self.python_spec:
-            result = get_interpreter(python_spec, self.app_data)
+            result = get_interpreter(python_spec, self.try_first_with, self.app_data)
             if result is not None:
                 return result
         return None
@@ -47,11 +57,11 @@ class Builtin(Discover):
         return "{} discover of python_spec={!r}".format(self.__class__.__name__, spec)
 
 
-def get_interpreter(key, app_data=None):
+def get_interpreter(key, try_first_with, app_data=None):
     spec = PythonSpec.from_string_spec(key)
     logging.info("find interpreter for spec %r", spec)
     proposed_paths = set()
-    for interpreter, impl_must_match in propose_interpreters(spec, app_data):
+    for interpreter, impl_must_match in propose_interpreters(spec, try_first_with, app_data):
         key = interpreter.system_executable, impl_must_match
         if key in proposed_paths:
             continue
@@ -62,7 +72,17 @@ def get_interpreter(key, app_data=None):
         proposed_paths.add(key)
 
 
-def propose_interpreters(spec, app_data):
+def propose_interpreters(spec, try_first_with, app_data):
+    # 0. try with first
+    for py_exe in try_first_with:
+        path = os.path.abspath(py_exe)
+        try:
+            os.lstat(path)  # Windows Store Python does not work with os.path.exists, but does for os.lstat
+        except OSError:
+            pass
+        else:
+            yield PythonInfo.from_exe(os.path.abspath(path), app_data), True
+
     # 1. if it's a path and exists
     if spec.path is not None:
         try:

src/virtualenv/discovery/windows/__init__.py

@@ -13,8 +13,11 @@ def propose_interpreters(spec, cache_dir):
     # see if PEP-514 entries are good
 
     # start with higher python versions in an effort to use the latest version available
+    # and prefer PythonCore over conda pythons (as virtualenv is mostly used by non conda tools)
     existing = list(discover_pythons())
-    existing.sort(key=lambda i: tuple(-1 if j is None else j for j in i[1:4]), reverse=True)
+    existing.sort(
+        key=lambda i: tuple(-1 if j is None else j for j in i[1:4]) + (1 if i[0] == "PythonCore" else 0,), reverse=True
+    )
 
     for name, major, minor, arch, exe, _ in existing:
         # pre-filter

tests/conftest.py

@@ -367,7 +367,7 @@ def temp_app_data(monkeypatch, tmp_path):
 @pytest.fixture(scope="session")
 def cross_python(is_inside_ci, session_app_data):
     spec = str(2 if sys.version_info[0] == 3 else 3)
-    interpreter = get_interpreter(spec, session_app_data)
+    interpreter = get_interpreter(spec, [], session_app_data)
     if interpreter is None:
         msg = "could not find {}".format(spec)
         if is_inside_ci:

tests/unit/discovery/test_discovery.py

@@ -36,14 +36,14 @@ def test_discovery_via_path(monkeypatch, case, tmp_path, caplog, session_app_dat
         (target / pyvenv_cfg.name).write_bytes(pyvenv_cfg.read_bytes())
     new_path = os.pathsep.join([str(target)] + os.environ.get(str("PATH"), str("")).split(os.pathsep))
     monkeypatch.setenv(str("PATH"), new_path)
-    interpreter = get_interpreter(core)
+    interpreter = get_interpreter(core, [])
 
     assert interpreter is not None
 
 
 def test_discovery_via_path_not_found(tmp_path, monkeypatch):
     monkeypatch.setenv(str("PATH"), str(tmp_path))
-    interpreter = get_interpreter(uuid4().hex)
+    interpreter = get_interpreter(uuid4().hex, [])
     assert interpreter is None
 
 
@@ -52,13 +52,13 @@ def test_relative_path(tmp_path, session_app_data, monkeypatch):
     cwd = sys_executable.parents[1]
     monkeypatch.chdir(str(cwd))
     relative = str(sys_executable.relative_to(cwd))
-    result = get_interpreter(relative, session_app_data)
+    result = get_interpreter(relative, [], session_app_data)
     assert result is not None
 
 
 def test_discovery_fallback_fail(session_app_data, caplog):
     caplog.set_level(logging.DEBUG)
-    builtin = Builtin(Namespace(app_data=session_app_data, python=["magic-one", "magic-two"]))
+    builtin = Builtin(Namespace(app_data=session_app_data, try_first_with=[], python=["magic-one", "magic-two"]))
 
     result = builtin.run()
     assert result is None
@@ -68,7 +68,7 @@ def test_discovery_fallback_fail(session_app_data, caplog):
 
 def test_discovery_fallback_ok(session_app_data, caplog):
     caplog.set_level(logging.DEBUG)
-    builtin = Builtin(Namespace(app_data=session_app_data, python=["magic-one", sys.executable]))
+    builtin = Builtin(Namespace(app_data=session_app_data, try_first_with=[], python=["magic-one", sys.executable]))
 
     result = builtin.run()
     assert result is not None, caplog.text