Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 79 additions & 13 deletions crates/uv/src/commands/tool/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,18 @@ use tracing::{debug, trace};
use uv_cache::Cache;
use uv_client::BaseClientBuilder;
use uv_configuration::{Concurrency, Constraints, DryRun, TargetTriple};
use uv_distribution_types::{ExtraBuildRequires, Requirement};
use uv_distribution_types::{ExtraBuildRequires, Requirement, RequirementSource};
use uv_fs::CWD;
use uv_normalize::PackageName;
use uv_pep440::{Operator, Version};
use uv_preview::Preview;
use uv_python::{
EnvironmentPreference, Interpreter, PythonDownloads, PythonInstallation, PythonPreference,
PythonRequest,
};
use uv_requirements::RequirementsSpecification;
use uv_settings::{Combine, PythonInstallMirrors, ResolverInstallerOptions, ToolOptions};
use uv_tool::InstalledTools;
use uv_tool::{InstalledTools, Tool};
use uv_warnings::write_error_chain;
use uv_workspace::WorkspaceCache;

Expand Down Expand Up @@ -114,6 +115,9 @@ pub(crate) async fn upgrade(
// Determine whether we applied any upgrades.
let mut did_upgrade_environment = vec![];

// Track tools that could not be upgraded due to pinned versions.
let mut pinned = Vec::new();

let mut errors = Vec::new();
for (name, constraints) in &names {
debug!("Upgrading tool: `{name}`");
Expand All @@ -135,14 +139,26 @@ pub(crate) async fn upgrade(
.await;

match result {
Ok(UpgradeOutcome::UpgradeEnvironment) => {
did_upgrade_environment.push(name);
}
Ok(UpgradeOutcome::UpgradeDependencies | UpgradeOutcome::UpgradeTool) => {
did_upgrade_tool.push(name);
}
Ok(UpgradeOutcome::NoOp) => {
debug!("Upgrading `{name}` was a no-op");
Ok((outcome, pinned_version)) => {
if let Some(version) = pinned_version.as_ref() {
debug!("`{name}` remains pinned to version {version}; skipping tool upgrade");
}

match outcome {
UpgradeOutcome::UpgradeEnvironment => {
did_upgrade_environment.push(name);
}
UpgradeOutcome::UpgradeDependencies | UpgradeOutcome::UpgradeTool => {
did_upgrade_tool.push(name);
}
UpgradeOutcome::NoOp => {
debug!("Upgrading `{name}` was a no-op");
}
}

if let Some(version) = pinned_version {
pinned.push((name.clone(), version));
}
}
Err(err) => {
errors.push((name, err));
Expand Down Expand Up @@ -187,9 +203,24 @@ pub(crate) async fn upgrade(
}
}

for (name, version) in pinned {
let name_str = name.to_string();

let version_str = version.to_string();

let reinstall_command = format!("uv tool install {name_str}@latest");
Comment thread
terror marked this conversation as resolved.
Outdated

writeln!(
printer.stderr(),
"hint: `{}` is pinned to `{}` (installed with an exact version pin); reinstall with `{}` to pick up a newer release.",
Comment thread
terror marked this conversation as resolved.
Outdated
name_str.cyan(),
version_str.magenta(),
reinstall_command.green(),
)?;
}
Comment thread
terror marked this conversation as resolved.

Ok(ExitStatus::Success)
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum UpgradeOutcome {
/// The tool itself was upgraded.
Expand Down Expand Up @@ -217,7 +248,7 @@ async fn upgrade_tool(
installer_metadata: bool,
concurrency: Concurrency,
preview: Preview,
) -> Result<UpgradeOutcome> {
) -> Result<(UpgradeOutcome, Option<Version>)> {
// Ensure the tool is installed.
let existing_tool_receipt = match installed_tools.get_tool_receipt(name) {
Ok(Some(receipt)) => receipt,
Expand Down Expand Up @@ -398,5 +429,40 @@ async fn upgrade_tool(
)?;
}

Ok(outcome)
let pinned_version = match outcome {
UpgradeOutcome::UpgradeTool | UpgradeOutcome::UpgradeEnvironment => None,
UpgradeOutcome::UpgradeDependencies | UpgradeOutcome::NoOp => {
pinned_requirement_version(&existing_tool_receipt, name)
}
};

Ok((outcome, pinned_version))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find this change in signature a little awkward. I think I'd expect the version to be attached to the UpgradeOutcome variant rather than as a part of the signature of this function?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(especially if it's only possible for a subset of the variants)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah agreed, I'll change this up!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After implementing it still looked a bit questionable, so I've lifted out a general UpgradeReason (we could probably name this to something better) I put onto an UpgradeReport. This makes it so we can easily extend UpgradeReason for other things. What do you think?

}

fn pinned_requirement_version(tool: &Tool, name: &PackageName) -> Option<Version> {
pinned_version_from(tool.requirements(), name)
.or_else(|| pinned_version_from(tool.constraints(), name))
}

fn pinned_version_from(requirements: &[Requirement], name: &PackageName) -> Option<Version> {
requirements
.iter()
.filter(|requirement| requirement.name == *name)
.find_map(|requirement| match &requirement.source {
RequirementSource::Registry { specifier, .. } => {
let mut specifiers = specifier.iter();

let first = specifiers.next()?;

if specifiers.next().is_some() {
return None;
}

match first.operator() {
Operator::Equal | Operator::ExactEqual => Some(first.version().clone()),
_ => None,
}
Comment thread
terror marked this conversation as resolved.
Outdated
}
_ => None,
})
}
51 changes: 51 additions & 0 deletions crates/uv/tests/it/tool_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,56 @@ fn tool_upgrade_multiple_names() {
"###);
}

#[test]
fn tool_upgrade_pinned_hint() {
let context = TestContext::new("3.12")
.with_filtered_counts()
.with_filtered_exe_suffix();

let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

// Install a specific version of `babel` so the receipt records an exact pin.
uv_snapshot!(context.filters(), context.tool_install()
.arg("babel==2.6.0")
.arg("--index-url")
.arg("https://test.pypi.org/simple/")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str())
.env(EnvVars::PATH, bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ babel==2.6.0
+ pytz==2018.5
Installed 1 executable: pybabel
"###);

// Attempt to upgrade `babel`; it should remain pinned and emit a hint explaining why.
uv_snapshot!(context.filters(), context.tool_upgrade()
.arg("babel")
.arg("--index-url")
.arg("https://pypi.org/simple/")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str())
.env(EnvVars::PATH, bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
Modified babel environment
- pytz==2018.5
+ pytz==2024.1
hint: `babel` is pinned to `2.6.0` (installed with an exact version pin); reinstall with `uv tool install babel@latest` to pick up a newer release.
"###);
}

#[test]
fn tool_upgrade_all() {
let context = TestContext::new("3.12")
Expand Down Expand Up @@ -683,6 +733,7 @@ fn tool_upgrade_with() {
Modified babel environment
- pytz==2018.5
+ pytz==2024.1
hint: `babel` is pinned to `2.6.0` (installed with an exact version pin); reinstall with `uv tool install babel@latest` to pick up a newer release.
"###);
}

Expand Down
Loading