Potential fix for pylauncher with python version (#1186)

* Potential fix for pylauncher with python version

* Validate provided python version at startup

* Add hint when version looks like path but is not found

* add tests

* fix interpreter resolution test on windows

* remove redundant error handling from install command

* Reduce complexity for checking path

* Fix changelog

* Add news fragment

* Split doc and fix part into seperate fragments

---------

Co-authored-by: Bernát Gábor <bgabor8@bloomberg.net>

changelog.d/1150.bugfix.md

@@ -0,0 +1 @@
+Autofix python version for pylauncher, when version is provided prefixed with `python`

changelog.d/1150.doc.md

@@ -0,0 +1 @@
+Provide useful error messages when unresolvable python version is passed

src/pipx/commands/install.py

@@ -1,12 +1,11 @@
 from pathlib import Path
 from typing import List, Optional
 
-from pipx import constants, emojis
+from pipx import constants
 from pipx.commands.common import package_name_from_spec, run_post_install_actions
 from pipx.constants import (
     EXIT_CODE_INSTALL_VENV_EXISTS,
     EXIT_CODE_OK,
-    EXIT_CODE_SPECIFIED_PYTHON_EXECUTABLE_NOT_FOUND,
     ExitCode,
 )
 from pipx.interpreter import DEFAULT_PYTHON
@@ -107,23 +106,6 @@ def install(
                 include_dependencies,
                 force=force,
             )
-        except FileNotFoundError as e:
-            venv.remove_venv()
-            if python in str(e) or "The system cannot find the file specified" in str(e):
-                print(
-                    pipx_wrap(
-                        f"""
-                           {emojis.hazard} No executable for the provided Python version '{python}' found.
-                           Please make sure the executable name is on your PATH /
-                           the path to the executable is correct.
-                        """,
-                        subsequent_indent=" " * 4,
-                    )
-                )
-                return EXIT_CODE_SPECIFIED_PYTHON_EXECUTABLE_NOT_FOUND
-            else:
-                print()
-                raise
         except (Exception, KeyboardInterrupt):
             print()
             venv.remove_venv()

src/pipx/interpreter.py

@@ -1,12 +1,16 @@
+import logging
 import os
 import shutil
 import subprocess
 import sys
+from pathlib import Path
 from typing import Optional
 
 from pipx.constants import WINDOWS
 from pipx.util import PipxError
 
+logger = logging.getLogger(__name__)
+
 
 def has_venv() -> bool:
     try:
@@ -17,6 +21,46 @@ def has_venv() -> bool:
         return False
 
 
+class InterpreterResolutionError(PipxError):
+    def __init__(self, source: str, version: str, wrap_message: bool = True):
+        self.source = source
+        self.version = version
+        potentially_path = "/" in version
+        potentially_pylauncher = "python" not in version and not potentially_path
+
+        message = (
+            f"No executable for the provided Python version '{version}' found in {source}."
+            " Please make sure the provided version is "
+        )
+        if source == "py launcher":
+            message += "listed when running `py --list`."
+        if source == "PATH":
+            message += "on your PATH or the file path is valid. "
+            if potentially_path:
+                message += "The provided version looks like a path, but no executable was found there."
+            if potentially_pylauncher:
+                message += (
+                    "The provided version looks like a version for Python Launcher, " "but `py` was not found on PATH."
+                )
+        super().__init__(message, wrap_message)
+
+
+def find_python_interpreter(python_version: str) -> str:
+    if Path(python_version).is_file():
+        return python_version
+
+    try:
+        py_executable = find_py_launcher_python(python_version)
+        if py_executable:
+            return py_executable
+    except (subprocess.CalledProcessError, FileNotFoundError) as e:
+        raise InterpreterResolutionError(source="py launcher", version=python_version) from e
+
+    if shutil.which(python_version):
+        return python_version
+    raise InterpreterResolutionError(source="PATH", version=python_version)
+
+
 # The following code was copied from https://github.com/uranusjr/pipx-standalone
 # which uses the same technique to build a completely standalone pipx
 # distribution.
@@ -30,8 +74,14 @@ def has_venv() -> bool:
 def find_py_launcher_python(python_version: Optional[str] = None) -> Optional[str]:
     py = shutil.which("py")
     if py and python_version:
+        python_semver = python_version
+        if python_version.startswith("python"):
+            logging.warn(
+                "Removing `python` from the start of the version, as pylauncher just expects the semantic version"
+            )
+            python_semver = python_semver.lstrip("python")
         py = subprocess.run(
-            [py, f"-{python_version}", "-c", "import sys; print(sys.executable)"],
+            [py, f"-{python_semver}", "-c", "import sys; print(sys.executable)"],
             capture_output=True,
             text=True,
             check=True,

src/pipx/main.py

@@ -23,9 +23,9 @@
 from pipx import commands, constants
 from pipx.animate import hide_cursor, show_cursor
 from pipx.colors import bold, green
-from pipx.constants import MINIMUM_PYTHON_VERSION, WINDOWS, ExitCode
+from pipx.constants import EXIT_CODE_SPECIFIED_PYTHON_EXECUTABLE_NOT_FOUND, MINIMUM_PYTHON_VERSION, WINDOWS, ExitCode
 from pipx.emojis import hazard
-from pipx.interpreter import DEFAULT_PYTHON, find_py_launcher_python
+from pipx.interpreter import DEFAULT_PYTHON, InterpreterResolutionError, find_python_interpreter
 from pipx.util import PipxError, mkdir, pipx_wrap, rmdir
 from pipx.venv import VenvContainer
 from pipx.version import __version__
@@ -187,11 +187,18 @@ def run_pipx_command(args: argparse.Namespace) -> ExitCode:  # noqa: C901
     if "skip" in args:
         skip_list = [canonicalize_name(x) for x in args.skip]
 
-    if "python" in args:
-        if args.python is not None and not Path(args.python).is_file():
-            py_launcher_python = find_py_launcher_python(args.python)
-            if py_launcher_python:
-                args.python = py_launcher_python
+    if "python" in args and args.python is not None:
+        try:
+            interpreter = find_python_interpreter(args.python)
+            args.python = interpreter
+        except InterpreterResolutionError as e:
+            print(
+                pipx_wrap(
+                    f"{hazard} {e}",
+                    subsequent_indent=" " * 4,
+                )
+            )
+            return EXIT_CODE_SPECIFIED_PYTHON_EXECUTABLE_NOT_FOUND
 
     if args.command == "run":
         commands.run(

tests/test_install.py

@@ -337,9 +337,3 @@ def test_passed_python_and_force_flag_warning(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", "pycowsay", "--force"])
     captured = capsys.readouterr()
     assert "--python is ignored when --force is passed." not in captured.out
-
-
-def test_passed_python_not_executable(pipx_temp_env, capsys):
-    assert run_pipx_cli(["install", "--python", "py_not_real", "pycowsay"])
-    captured = capsys.readouterr()
-    assert "No executable for the provided Python version" in captured.out

tests/test_interpreter.py

@@ -7,9 +7,10 @@
 
 import pipx.interpreter
 from pipx.interpreter import (
+    InterpreterResolutionError,
     _find_default_windows_python,
     _get_absolute_python_interpreter,
-    find_py_launcher_python,
+    find_python_interpreter,
 )
 from pipx.util import PipxError
 
@@ -24,12 +25,43 @@ def which(name):
     minor = sys.version_info.minor
     monkeypatch.setattr(pipx.interpreter, "has_venv", lambda: venv)
     monkeypatch.setattr(shutil, "which", which)
-    python_path = find_py_launcher_python(f"{major}.{minor}")
+    python_path = find_python_interpreter(f"{major}.{minor}")
     assert python_path is not None
     assert f"{major}.{minor}" in python_path or f"{major}{minor}" in python_path
     assert python_path.endswith("python.exe")
 
 
+@pytest.mark.skipif(not sys.platform.startswith("win"), reason="Looks for Python.exe")
+@pytest.mark.parametrize("venv", [True, False])
+def test_windows_python_with_python_and_version(monkeypatch, venv):
+    def which(name):
+        return "py"
+
+    major = sys.version_info.major
+    minor = sys.version_info.minor
+    monkeypatch.setattr(pipx.interpreter, "has_venv", lambda: venv)
+    monkeypatch.setattr(shutil, "which", which)
+    python_path = find_python_interpreter(f"python{major}.{minor}")
+    assert python_path is not None
+    assert f"{major}.{minor}" in python_path or f"{major}{minor}" in python_path
+    assert python_path.endswith("python.exe")
+
+
+@pytest.mark.skipif(not sys.platform.startswith("win"), reason="Looks for Python.exe")
+@pytest.mark.parametrize("venv", [True, False])
+def test_windows_python_with_python_and_unavailable_version(monkeypatch, venv):
+    def which(name):
+        return "py"
+
+    major = sys.version_info.major + 99
+    minor = sys.version_info.minor
+    monkeypatch.setattr(pipx.interpreter, "has_venv", lambda: venv)
+    monkeypatch.setattr(shutil, "which", which)
+    with pytest.raises(InterpreterResolutionError) as e:
+        find_python_interpreter(f"python{major}.{minor}")
+        assert "py --list" in str(e)
+
+
 def test_windows_python_no_version_with_venv(monkeypatch):
     monkeypatch.setattr(pipx.interpreter, "has_venv", lambda: True)
     assert _find_default_windows_python() == sys.executable
@@ -108,3 +140,30 @@ def test_bad_env_python(monkeypatch):
 def test_good_env_python(monkeypatch, capsys):
     good_exec = _get_absolute_python_interpreter(sys.executable)
     assert good_exec == sys.executable
+
+
+def test_find_python_interpreter_by_path(monkeypatch):
+    interpreter_path = sys.executable
+    assert interpreter_path == find_python_interpreter(interpreter_path)
+
+
+def test_find_python_interpreter_by_version(monkeypatch):
+    major = sys.version_info.major
+    minor = sys.version_info.minor
+    python_path = find_python_interpreter(f"python{major}.{minor}")
+    assert python_path == f"python{major}.{minor}" or f"Python\\{major}.{minor}" in python_path
+
+
+def test_find_python_interpreter_by_wrong_path_raises(monkeypatch):
+    interpreter_path = sys.executable + "99"
+    with pytest.raises(InterpreterResolutionError) as e:
+        find_python_interpreter(interpreter_path)
+        assert "like a path" in str(e)
+
+
+def test_find_python_interpreter_missing_on_path_raises(monkeypatch):
+    interpreter = "1.1"
+    with pytest.raises(InterpreterResolutionError) as e:
+        find_python_interpreter(interpreter)
+        assert "Python Launcher" in str(e)
+        assert "on your PATH" in str(e)