-
Notifications
You must be signed in to change notification settings - Fork 3k
Defer stabilization of the uv python install --default behavior
#14681
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -166,6 +166,16 @@ pub(crate) async fn install( | |
| ) -> Result<ExitStatus> { | ||
| let start = std::time::Instant::now(); | ||
|
|
||
| // TODO(zanieb): We should consider marking the Python installation as the default when | ||
| // `--default` is used. It's not clear how this overlaps with a global Python pin, but I'd be | ||
| // surprised if `uv python find` returned the "newest" Python version rather than the one I just | ||
| // installed with the `--default` flag. | ||
| if default && !preview.is_enabled() { | ||
| warn_user!( | ||
| "The `--default` option is experimental and may change without warning. Pass `--preview` to disable this warning" | ||
| ); | ||
| } | ||
|
|
||
| if upgrade && preview.is_disabled() { | ||
| warn_user!( | ||
| "`uv python upgrade` is experimental and may change without warning. Pass `--preview` to disable this warning" | ||
|
|
@@ -214,6 +224,8 @@ pub(crate) async fn install( | |
| .map(PythonVersionFile::into_versions) | ||
| .unwrap_or_else(|| { | ||
| // If no version file is found and no requests were made | ||
| // TODO(zanieb): We should consider differentiating between a global Python version | ||
| // file here, allowing a request from there to enable `is_default_install`. | ||
| is_default_install = true; | ||
| vec![if reinstall { | ||
| // On bare `--reinstall`, reinstall all Python versions | ||
|
|
@@ -443,10 +455,10 @@ pub(crate) async fn install( | |
| } | ||
| } | ||
|
|
||
| let bin_dir = if !matches!(bin, Some(false)) { | ||
| Some(python_executable_dir()?) | ||
| } else { | ||
| let bin_dir = if matches!(bin, Some(false)) { | ||
| None | ||
| } else { | ||
| Some(python_executable_dir()?) | ||
| }; | ||
|
|
||
| let installations: Vec<_> = downloaded.iter().chain(satisfied.iter().copied()).collect(); | ||
|
|
@@ -727,7 +739,10 @@ fn create_bin_links( | |
| errors: &mut Vec<(InstallErrorKind, PythonInstallationKey, Error)>, | ||
| preview: PreviewMode, | ||
| ) { | ||
| let targets = if (default || (is_default_install && !reinstall)) | ||
| // TODO(zanieb): We want more feedback on the `is_default_install` behavior before stabilizing | ||
| // it. In particular, it may be confusing because it does not apply when versions are loaded | ||
| // from a `.python-version` file. | ||
| let targets = if (default || (is_default_install && preview.is_enabled())) | ||
|
Comment on lines
+742
to
+745
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could stabilize the |
||
| && first_request.matches_installation(installation) | ||
| { | ||
| vec![ | ||
|
|
||
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 feel like this is supposed to end in a period since it's multiple sentences, but not sure if we do that consistently for these preview warnings.
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 do. I can do a pass for that everywhere separately.