Repository navigation
🐛 fix(discovery): let -p replace the python set in config or env - #3494
Open
darrenhuai wants to merge 1 commit into
Open
darrenhuai wants to merge 1 commit into
darrenhuai wants to merge 1 commit into
Conversation
--python and --try-first-with were plain append actions, so a value from virtualenv.ini or VIRTUALENV_PYTHON became the starting list and the command-line values were added after it. Discovery takes the first spec that resolves, so with python = 3.13 in virtualenv.ini, virtualenv -p 3.14 env built a 3.13 environment, and -p 3.12 on a machine without 3.12 succeeded with 3.13 instead of reporting that no interpreter was found. Builtin worked around this for the env var alone by rotating the list by one whenever VIRTUALENV_PYTHON was set. That covered a single env value, but with several values it moved the second one to the front, with or without -p, and it never helped the config file. Both options now use the ExtendAction from pypa#3440, which already does this for --extra-search-dir: the first command-line value replaces the configured list and later ones are added in order. nargs=1 hands the action a one-item list per flag, since it extends with whatever it receives. A configured list is still tried in order when the command line gives none, and several -p are still all tried in order. The rotation in Builtin is gone because the parser now gets the order right. One visible change follows from the precedence: a -p that matches nothing now fails the same way it does without any configuration, instead of falling back to VIRTUALENV_PYTHON.
darrenhuai
marked this pull request as ready for review
October 30, 2026 01:47
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With
python = 3.13invirtualenv.ini, a-pon the command line loses to the config file. This machine has 3.13 and 3.14 installed and no 3.12. Run against main:Without the config file,
-p 3.14builds 3.14.3 and-p 3.12stops withfailed to find interpreter for Builtin discover of python_spec='3.12'. The env var goes wrong in a different way:VIRTUALENV_PYTHON=3.14,3.13with no-pat all builds 3.13, so the second value wins.-p/--pythonand--try-first-withareappendactions._fix_defaultseeds the namespace with the env var or ini value, argparse appends the command-line values after it, andBuiltin.runreturns the first spec that resolves, so the configured interpreter wins whenever it exists. #2821 patched this forVIRTUALENV_PYTHONonly, by rotatingpython_specby one whenever the variable is set. That puts a single env value behind-p, but with two or more env values it moves the second one to the front, and it never covered the config file. tox passes itsbase_pythonlist as a newline-joinedVIRTUALENV_PYTHONwith no-p, so a tox env with more than onebase_pythongets the rotated order too. In #2285, the issue #2821 closed, the answer was that a CLI value should overwrite the env var.Both options now use the
ExtendActionfrom #3440, which already does this for--extra-search-dir: the first command-line value replaces the configured list and later ones are appended in order.nargs=1is there becauseExtendActionextends the list with whatever it receives, and a bare3.14would otherwise be split into characters. Since the parser now gets the order right, the rotation inBuiltinis gone. A configured list is still tried in order when there is no-p, several-pare still all tried in order, and the help output doesn't change.One behavior does change:
VIRTUALENV_PYTHON=3.13plus a-pthat matches nothing used to fall back to 3.13 and now fails, the same as without the env var. That follows the documented priority (command line, then env vars, then the config file), and the changelog fragment says so. With this branch the two runs above build 3.14.3 and fail withfailed to find interpreter,VIRTUALENV_PYTHON=3.14,3.13builds 3.14.3, and the ini on its own still builds 3.13.7.test_cli_replaces_configured_interpretersrecords what discovery tries for env and ini values with zero, one or two values on the command line, for both options. On main it fails whenever the command line has a value, and for two env values without one. Theenv-var-and-clicase intest_returns_python_specifiedencoded the old merged list plus the rotation, so I replaced it with anenv-var-multiplecase that expects the first env value.Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)
ran the linter to address style issues (
tox -e fix). I ran the same hooks throughprekon the changed files; the semgrep hook only looks atactivate.sh.wrote descriptive pull request text
ensured there are test(s) validating the fix
added a
docs/changelog/+short-description.<type>.rstfragment for each user-facing change, or appliedskip newsupdated/extended the documentation (the configuration priority section already puts the command line first; this change makes
-pand--try-first-withfollow it)for changes to activation scripts,
pyvenv.cfg, wheel downloads, app-data or release workflows: explained in thePR text which outside input the change handles, such as a path, a prompt or a downloaded file, and how it stops that
input from running as code. See the
security scope. (none of these are touched; the change is in how the discovery options are parsed)
ran tests on Python 3.9 and 3.15 (
tox run -e 3.9,3.15). I ran the discovery, config and seed-embed test files on 3.9 and 3.15 and the full suite on 3.13, and left the full 3.9 and 3.15 runs to CI.linked upstream issues beside any temporary interpreter overrides (none)
kept the PR in draft until CI passes