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

Remove some Clap-level conflicts in argument groups #3001

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Apr 11, 2024

Summary

It turns out that if you have an environment variable set, Clap will consider that equivalent to passing the flag, even if it's set to (e.g.) something falsy or the default value.

So, e.g., this fails:

UV_SYSTEM=false uv pip install --python ./.venv/bin/python flask

Worse, this fails, because it thinks --no-index and --index-url are conflicting:

export UV_INDEX_URL=https://google.com
uv pip install flask --no-index

This PR removes some of the conflicts, namely those related to environment variables, such that:

  • You can pass mixes of --no-index, --index-url, etc. If --no-index is provided, all the index URLs will be ignored (but we won't error).
  • Passing --pre will always enable prereleases, even if --prerelease is also provided. (We could warn here, although honestly it's not trivial because we'd need to make --prerelease take an optional, then we'd lose the default argument from the --help.)
  • You can pass --system and --python. If --python is provided, we use that, and ignore --system. (We could warn here.)

I guess the underlying problem here is that we can't differentiate between arguments passed on the CLI and those set as environment variables. But making bigger changes here seems out-of-scope.

Closes #3000.

@charliermarsh charliermarsh added bug Something isn't working cli Related to the command line interface labels Apr 11, 2024
@charliermarsh charliermarsh marked this pull request as ready for review April 11, 2024 22:02
Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an improvement to me.

Does this take the order of flags into account? e.g., Are --no-index --index-url ... and --index-url ... --no-index the same? I think arguably the order should determine which one "wins." IIRC, this was difficult to do in Clap unfortunately. (Or at least, difficult to do in Clap 2. I'm unsure about Clap 4.)

@charliermarsh
Copy link
Member Author

It doesn't, though in that specific case I actually think --no-index should still win...

@charliermarsh
Copy link
Member Author

In general though, I agree that taking order into account would help here.

@charliermarsh charliermarsh enabled auto-merge (squash) April 12, 2024 19:39
@charliermarsh charliermarsh merged commit a95a8c8 into main Apr 12, 2024
37 checks passed
@charliermarsh charliermarsh deleted the charlie/arg branch April 12, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Related to the command line interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UV_SYSTEM_PYTHON cannot be switched off
3 participants