diff --git a/docs/changelog/+cli-python-overrides-config.bugfix.rst b/docs/changelog/+cli-python-overrides-config.bugfix.rst new file mode 100644 index 000000000..155bf95ed --- /dev/null +++ b/docs/changelog/+cli-python-overrides-config.bugfix.rst @@ -0,0 +1,4 @@ +Make ``-p``/``--python`` and ``--try-first-with`` replace the interpreters set by ``VIRTUALENV_PYTHON``, +``VIRTUALENV_TRY_FIRST_WITH`` or the configuration file instead of being merged with them, so the configured interpreter +no longer wins over the one asked for and a ``-p`` that finds nothing fails instead of falling back to it. Try several +``VIRTUALENV_PYTHON`` values in the order given rather than starting with the second. diff --git a/docs/how-to/usage.rst b/docs/how-to/usage.rst index d979b5873..d11fb7ac7 100644 --- a/docs/how-to/usage.rst +++ b/docs/how-to/usage.rst @@ -252,6 +252,9 @@ Specify multiple values on separate lines: /path/to/dists /path/to/other/dists +A value given on the command line replaces the configured list instead of adding to it. Repeat the option to pass +several, for example ``-p 3.14 -p 3.13``. + Environment variables ===================== diff --git a/src/virtualenv/discovery/builtin.py b/src/virtualenv/discovery/builtin.py index 395e77116..07c41c009 100644 --- a/src/virtualenv/discovery/builtin.py +++ b/src/virtualenv/discovery/builtin.py @@ -7,6 +7,8 @@ from python_discovery import get_interpreter as _get_interpreter +from virtualenv.config.cli.parser import ExtendAction + from .discover import Discover if TYPE_CHECKING: @@ -36,8 +38,6 @@ class Builtin(Discover): def __init__(self, options: VirtualEnvOptions) -> None: super().__init__(options) self.python_spec = options.python or [sys.executable] - if self._env.get("VIRTUALENV_PYTHON"): - self.python_spec = self.python_spec[1:] + self.python_spec[:1] self.app_data = options.app_data self.try_first_with = options.try_first_with @@ -49,7 +49,9 @@ def add_parser_arguments(cls, parser: ArgumentParser) -> None: dest="python", metavar="py", type=str, - action="append", + # ExtendAction extends the list with the values it gets, so a lone string would split into characters + nargs=1, + action=ExtendAction, default=[], help="interpreter based on what to create environment (path/identifier/version-specifier) " "- by default use the interpreter where the tool is installed - first found wins. " @@ -60,7 +62,8 @@ def add_parser_arguments(cls, parser: ArgumentParser) -> None: dest="try_first_with", metavar="py_exe", type=str, - action="append", + nargs=1, + action=ExtendAction, default=[], help="try first these interpreters before starting the discovery", ) diff --git a/tests/unit/discovery/test_discovery.py b/tests/unit/discovery/test_discovery.py index a5aba7e8f..e5343278d 100644 --- a/tests/unit/discovery/test_discovery.py +++ b/tests/unit/discovery/test_discovery.py @@ -5,6 +5,7 @@ import sys from argparse import Namespace from pathlib import Path +from typing import TYPE_CHECKING, Final import pytest from python_discovery import PythonInfo @@ -13,6 +14,9 @@ from virtualenv.info import IS_WIN from virtualenv.run import cli_run, session_via_cli +if TYPE_CHECKING: + from pytest_mock import MockerFixture + def test_relative_path(session_app_data, monkeypatch) -> None: sys_executable = Path(PythonInfo.current_system(session_app_data).system_executable) @@ -61,9 +65,7 @@ def mock_get_interpreter(mocker): ("env_var", "python", "expected"), [ pytest.param("python_from_env_var", ["python_from_env_var"], "python_from_env_var", id="env-var-only"), - pytest.param( - "python_from_env_var", ["python_from_env_var", "python_from_cli"], "python_from_cli", id="env-var-and-cli" - ), + pytest.param("first,second", ["first", "second"], "first", id="env-var-multiple"), pytest.param(None, ["python_from_cli"], "python_from_cli", id="cli-only"), ], ) @@ -79,6 +81,57 @@ def test_returns_python_specified( # ruff:ignore[too-many-arguments] assert builtin.run() == getattr(mocker.sentinel, expected) +@pytest.mark.parametrize( + ("variable", "value"), + [ + pytest.param("VIRTUALENV_{dest}", "configured-1,configured-2", id="env"), + pytest.param("VIRTUALENV_CONFIG_FILE", "{config}", id="ini"), + ], +) +@pytest.mark.parametrize( + ("dest", "cli", "tried"), + [ + pytest.param("python", [], [("configured-1", []), ("configured-2", [])], id="python-configured"), + pytest.param("python", ["-p", "first"], [("first", [])], id="python-cli-replaces"), + pytest.param("python", ["-p", "first", "-p", "second"], [("first", []), ("second", [])], id="python-cli-adds"), + pytest.param( + "try_first_with", [], [(sys.executable, ["configured-1", "configured-2"])], id="try-first-configured" + ), + pytest.param( + "try_first_with", ["--try-first-with", "first"], [(sys.executable, ["first"])], id="try-first-cli-replaces" + ), + pytest.param( + "try_first_with", + ["--try-first-with", "first", "--try-first-with", "second"], + [(sys.executable, ["first", "second"])], + id="try-first-cli-adds", + ), + ], +) +def test_cli_replaces_configured_interpreters( # ruff:ignore[too-many-arguments] + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + mocker: MockerFixture, + variable: str, + value: str, + dest: str, + cli: list[str], + tried: list[tuple[str, list[str]]], +) -> None: + (config := tmp_path / "virtualenv.ini").write_text( + f"[virtualenv]\n{dest} =\n configured-1\n configured-2\n", encoding="utf-8" + ) + monkeypatch.setenv(variable.format(dest=dest.upper()), value.format(config=config)) + get_interpreter: Final = mocker.patch( + "virtualenv.discovery.builtin.get_interpreter", autospec=True, return_value=None + ) + + with pytest.raises(RuntimeError, match="failed to find interpreter"): + session_via_cli([*cli, str(tmp_path / "env")]) + + assert [call.args for call in get_interpreter.call_args_list] == tried + + def test_discovery_absolute_path_with_try_first(tmp_path, session_app_data) -> None: good_env = tmp_path / "good" bad_env = tmp_path / "bad"