-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add support for explicit variants and --all-variants
flag with uv python list
#16002
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
base: main
Are you sure you want to change the base?
Conversation
// When --all-variants is used with a specific request, use Any to get all variants | ||
// The filtering logic will ensure only matching downloads are shown | ||
PythonDownloadRequest::from_request(&PythonRequest::Any) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wrong — we shouldn't drop the original request from the user. Though I see you filter using it later, then we'll need to duplicate a bunch of logic that happens in discovery. We'll also do a bunch of work querying interpreters that do not match the request. I'm not quite sure how best to achieve what you're trying to do here though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that was pretty troll mb :/
Reworked the implementation based on your feedback
I think this issue may be harder than it looks. Maybe it'd be a good place to start just adjusting the downloads we show based on an |
72a0645
to
2921685
Compare
Oh it looks like you are only changing the download listing logic here, so perhaps disregard that part (though we'll need to figure out how to show all variants we discover too at some point) |
Maybe you need to add something like uv/crates/uv-python/src/downloads.rs Lines 163 to 165 in dea1700
for variants? |
6cc18a0
to
70b7741
Compare
Testing was a bit annoying ; I didn't realize debug variants were not supported on Windows at first. Last minute split up a lot of test-cases based off target, but lmk if you think it's worth organizing this better |
70b7741
to
740f872
Compare
Hey @zanieb, just wanted to check whether there’s anything else you’d like me to do on this PR. Happy to rebase or tweak if needed :) |
crates/uv-python/src/discovery.rs
Outdated
match self { | ||
Self::Version(version) => version.variant(), | ||
Self::ImplementationVersion(_, version) => version.variant(), | ||
_ => None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally prefer to list all the variants so if we add a new one we're forced to revisit this and see if it needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use explicit listing.
crates/uv-python/src/downloads.rs
Outdated
/// not None, and false otherwise. | ||
pub(crate) prereleases: Option<bool>, | ||
|
||
/// Whether to allow all variants, or to filter by the requested variant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does "to filter by the requested variant" mean here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean like, one in the VersionRequest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was the intent. Sorry, the conflicting terminology / overload between Python's "variants" and Rust variants is a bit unfortunate here haha. Updated the comment a bit to further elaborate.
crates/uv-python/src/downloads.rs
Outdated
if variant != key.variant { | ||
return false; | ||
// Only filter by variant if `all_variants` is not requested | ||
if self.all_variants.is_none() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused that this is an Option
if we're not using the inner boolean value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed to use an explicit boolean
#[diagnostic( | ||
code(uv::python::list::conflicting_arguments), | ||
help( | ||
"Use `--all-variants` to show all variants for a Python version, or specify an exact variant like `3.10+debug`, but not both." | ||
) | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should use this — we're moving away from miette
. You can see other hint
formatting to see how we're handling this for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to use standard hint
formatting
} else { | ||
// If the user request cannot be mapped to a download request, we won't show any downloads | ||
PythonDownloadRequest::from_request(request.as_ref().unwrap_or(&PythonRequest::Any)) | ||
.map(|request| request.with_all_variants(all_variants)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a note for https://github.com/astral-sh/uv/pull/16002/files#r2411619048
all_variants | ||
|| request | ||
.as_ref() | ||
.and_then(uv_python::PythonRequest::variant) | ||
.unwrap_or(uv_python::PythonVariant::Default) | ||
== *download.key().variant() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused that we're changing the filtering here and at https://github.com/astral-sh/uv/pull/16002/files#r2411618419
Why do we need both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated and consolidated.
560a5a0
to
b602f77
Compare
Python list now supports explicit variant requests (e.g. `3.10+debug`, `3.13t`) to show specific debug or free-threaded builds. A new `--all-variants` flag lists every available variant for a version (e.g. `uv python list 3.10 --all-variants`). By default, only standard builds are shown, matching previous behavior. The `--all-variants` flag can't be combined with explicit variant requests (e.g. `uv python list 3.10+debug --all-variants`) to avoid ambiguity. Fixes astral-sh#15983
Adds comprehensive test coverage for variant functionality in python list.
Add example usage for --all-variants flag showing how to list debug and freethreaded Python builds alongside other uv python list options.
b602f77
to
aa9842b
Compare
--all-variants
flag with uv python list
@zanieb Wanted to quickly double-check about pre-existing behavior with the display of freethreaded variants. Currently:
This seems intentional and aligns with treating freethreaded as non-experimental in Python 3.14+ and uv 0.9+ (per #16142). Just wanted to confirm this is the intended behavior, since this PR makes the variant filtering more explicit. |
Adds
--all-variants
flag touv python list
to show debug Python builds that are normally hidden.Functional Requirements Met:
3.13+debug
works without needing the flag--all-versions
,--only-downloads
, etc.Key Behavior:
Fixes #15983