Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The --python command-line flag should take precedence over config file python and VIRTUALENV_PYTHON variable #2285

Open
filiplajszczak opened this issue Jan 14, 2022 · 3 comments

Comments

@filiplajszczak
Copy link

Current state

Currently each python provided by --python cli flag is appended to the list . It's meant to provide fallback functionality for interpreter discovery. The side effect of it is that any python defined in configuration file or in the VIRTUALENV_PYTHON environment variable is used as default because the one provided by cli flag is at the end of the list.

I believe that it's a sane expectation for command line flags to take precedence over environment variables.

Example

Consider system with system python 3.9 where Alice for some reason prefers to create venvs with python 3.10, so she sets VIRTUALENV_PYTHON in her shell environment to /usr/bin/python3.10.

That way running:

virtualenv some-name

always creates venv with python3.10

That's fine until she needs to create virtual environment with python3.8, by running:

virtualenv --python python3.8 some-other-name

It unexpectedly produces virtual environment with python3.10

She can use:

virtualenv --try-first-with /usr/bin/python3.8 some-completely-different-name

but not:

virtualenv --try-first-with python3.8 some-other-completely-different-name

as --try-first-with works with paths only, and not with pythonX.Y specs.

All of that is hard to get from the discovery docs, and introduction to cli flags section mentions that "Environment variables takes priority over the configuration file values" but is silent about precedence of values provided in the cli itself.

Possible solutions

I am happy to fix it and propose pull request but I would like first to discuss the right way to approach it. The question is how to express the intent of the user in the best possible way without ambiguity and doubts.

One possibility is to call it a feature not a bug and just add more extensive description of behavior with examples to the docs. That way anyone who wants to change it has to write his own discovery class and the current pluggable architecture allows it.

Other possibility is to extend --try-first-with to accept not only paths but also just pythonX.Y specs. It should be similarly documented as in solution above. On the other hand when I'm reading previous discussion about that flag I get the impression that it's not desired.

The final possibility is to modify the code so the python provided by cli flag is the first considered one and configuration file and environment variable ones are treated as fallback. I prefer that solution over the first two but I'm ready to be convinced otherwise.

@gaborbernat
Copy link
Contributor

The intent here was that the env-var changes the default, however, if a CLI is set that should overwrite the env-var. A PR for that would be welcome, Not sure why --python merges rather than replaces.

@filiplajszczak
Copy link
Author

What do you think about keeping lists of sources for options populated by "append" action? (in VirtualEnvOptions._sources["python"] and VirtualEnvOptions._sources["try_first_with"]) Currently these are always a strings that are overwritten with the source of the last item that was appended.

It would be possible then to build correct Builtin.python_spec without making very special case out of the python option.

@gaborbernat
Copy link
Contributor

PR welcome, otherwise I'll close this as no longer wanted within a months time.

samueljsb added a commit to samueljsb/qaz that referenced this issue Aug 10, 2023
Prior to this change, we used an env var to select the python version to
use. This takes precedence over the version passed as a CLI option (see
pypa/virtualenv#2285) so breaks thing like nox.

This change switches to putting the config in a config file instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@gaborbernat @filiplajszczak and others