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.

Sign up for GitHub

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

Jump to bottom

Support {package}@{version} in uv tool install #6762

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Support {package}@{version} in uv tool install #6762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion crates/uv-cli/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2758,7 +2758,7 @@ pub enum ToolCommand {
/// By default, the package to install is assumed to match the command name.
///
/// The name of the command can include an exact version in the format
/// `<package>@<version>`, e.g., `uv run [email protected]`. If more complex
/// `<package>@<version>`, e.g., `uv tool run [email protected]`. If more complex
/// version specification is desired or if the command is provided by a
/// different package, use `--from`.
///
Expand Down
30 changes: 29 additions & 1 deletion crates/uv-configuration/src/package_options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub enum Upgrade {
}

impl Upgrade {
/// Determine the upgrade strategy from the command-line arguments.
/// Determine the [`Upgrade`] strategy from the command-line arguments.
pub fn from_args(upgrade: Option<bool>, upgrade_package: Vec<Requirement>) -> Self {
match upgrade {
Some(true) => Self::All,
Expand All @@ -97,6 +97,15 @@ impl Upgrade {
}
}

/// Create an [`Upgrade`] strategy to upgrade a single package.
pub fn package(package_name: PackageName) -> Self {
Self::Packages({
let mut map = FxHashMap::default();
map.insert(package_name, vec![]);
map
})
}

/// Returns `true` if no packages should be upgraded.
pub fn is_none(&self) -> bool {
matches!(self, Self::None)
Expand Down Expand Up @@ -130,6 +139,25 @@ impl Upgrade {
Either::Left(std::iter::empty())
}
}

/// Combine a set of [`Upgrade`] values.
#[must_use]
pub fn combine(self, other: Self) -> Self {
match (self, other) {
// If both are `None`, the result is `None`.
(Self::None, Self::None) => Self::None,
// If either is `All`, the result is `All`.
(Self::All, _) | (_, Self::All) => Self::All,
// If one is `None`, the result is the other.
(Self::Packages(a), Self::None) => Self::Packages(a),
(Self::None, Self::Packages(b)) => Self::Packages(b),
// If both are `Packages`, the result is the union of the two.
(Self::Packages(mut a), Self::Packages(b)) => {
a.extend(b);
Self::Packages(a)
}
}
}
}

/// Create a [`Refresh`] policy by integrating the [`Upgrade`] policy.
Expand Down
225 changes: 147 additions & 78 deletions crates/uv/src/commands/tool/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ use std::str::FromStr;
use anyhow::{bail, Result};
use distribution_types::UnresolvedRequirementSpecification;
use owo_colors::OwoColorize;
use pep440_rs::{VersionSpecifier, VersionSpecifiers};
use pep508_rs::MarkerTree;
use pypi_types::{Requirement, RequirementSource};
use tracing::debug;

use uv_cache::Cache;
use uv_cache::{Cache, Refresh, Timestamp};
use uv_client::{BaseClientBuilder, Connectivity};
use uv_configuration::Concurrency;
use uv_configuration::{Concurrency, Upgrade};
use uv_normalize::PackageName;
use uv_python::{
EnvironmentPreference, PythonDownloads, PythonInstallation, PythonPreference, PythonRequest,
Expand All @@ -24,6 +26,7 @@ use crate::commands::project::{
resolve_environment, resolve_names, sync_environment, update_environment,
};
use crate::commands::tool::common::remove_entrypoints;
use crate::commands::tool::Target;
use crate::commands::{reporters::PythonDownloadReporter, tool::common::install_executables};
use crate::commands::{ExitStatus, SharedState};
use crate::printer::Printer;
Expand All @@ -44,7 +47,7 @@ pub(crate) async fn install(
connectivity: Connectivity,
concurrency: Concurrency,
native_tls: bool,
cache: &Cache,
cache: Cache,
printer: Printer,
) -> Result<ExitStatus> {
let client_builder = BaseClientBuilder::new()
Expand All @@ -63,7 +66,7 @@ pub(crate) async fn install(
python_preference,
python_downloads,
&client_builder,
cache,
&cache,
Some(&reporter),
)
.await?
Expand All @@ -76,24 +79,28 @@ pub(crate) async fn install(
.connectivity(connectivity)
.native_tls(native_tls);

// Resolve the `from` requirement.
let from = if let Some(from) = from {
// Parse the positional name. If the user provided more than a package name, it's an error
// (e.g., `uv install foo==1.0 --from foo`).
let Ok(package) = PackageName::from_str(&package) else {
bail!("Package requirement (`{from}`) provided with `--from` conflicts with install request (`{package}`)", from = from.cyan(), package = package.cyan())
};
// Parse the input requirement.
let target = Target::parse(&package, from.as_deref());

let source = if editable {
RequirementsSource::Editable(from)
} else {
RequirementsSource::Package(from)
};
let requirements = RequirementsSpecification::from_source(&source, &client_builder)
.await?
.requirements;
// If the user passed, e.g., `ruff@latest`, refresh the cache.
let cache = if target.is_latest() {
cache.with_refresh(Refresh::All(Timestamp::now()))
Copy link
Member

Choose a reason for hiding this comment

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

Refresh all? Or just refresh Ruff?

Copy link
Member Author

Choose a reason for hiding this comment

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

In uv tool run at least we refresh everything. I think we might have to refresh everything here because we don't know the package name yet...?

Copy link
Member

Choose a reason for hiding this comment

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

Okay. Seems fine.

} else {
cache
};

let from_requirement = {
// Resolve the `--from` requirement.
let from = match target {
// Ex) `ruff`
Target::Unspecified(name) => {
let source = if editable {
RequirementsSource::Editable(name.to_string())
} else {
RequirementsSource::Package(name.to_string())
};
let requirements = RequirementsSpecification::from_source(&source, &client_builder)
.await?
.requirements;
resolve_names(
requirements,
&interpreter,
Expand All @@ -102,49 +109,106 @@ pub(crate) async fn install(
connectivity,
concurrency,
native_tls,
cache,
&cache,
printer,
)
.await?
.pop()
.unwrap()
};
}
// Ex) `[email protected]`
Target::Version(name, ref version) => {
if editable {
bail!("`--editable` is only supported for local packages");
}

// Check if the positional name conflicts with `--from`.
if from_requirement.name != package {
// Determine if it's an entirely different package (e.g., `uv install foo --from bar`).
bail!(
"Package name (`{}`) provided with `--from` does not match install request (`{}`)",
from_requirement.name.cyan(),
package.cyan()
);
Requirement {
name: PackageName::from_str(name)?,
extras: vec![],
marker: MarkerTree::default(),
source: RequirementSource::Registry {
specifier: VersionSpecifiers::from(VersionSpecifier::equals_version(
version.clone(),
)),
index: None,
},
origin: None,
}
}
// Ex) `ruff@latest`
Target::Latest(name) => {
if editable {
bail!("`--editable` is only supported for local packages");
}

from_requirement
} else {
let source = if editable {
RequirementsSource::Editable(package.clone())
} else {
RequirementsSource::Package(package.clone())
};
let requirements = RequirementsSpecification::from_source(&source, &client_builder)
Requirement {
name: PackageName::from_str(name)?,
extras: vec![],
marker: MarkerTree::default(),
source: RequirementSource::Registry {
specifier: VersionSpecifiers::empty(),
index: None,
},
origin: None,
}
}
// Ex) `ruff>=0.6.0`
Target::UserDefined(package, from) => {
// Parse the positional name. If the user provided more than a package name, it's an error
// (e.g., `uv install foo==1.0 --from foo`).
let Ok(package) = PackageName::from_str(package) else {
bail!("Package requirement (`{from}`) provided with `--from` conflicts with install request (`{package}`)", from = from.cyan(), package = package.cyan())
};

let source = if editable {
RequirementsSource::Editable(from.to_string())
} else {
RequirementsSource::Package(from.to_string())
};
let requirements = RequirementsSpecification::from_source(&source, &client_builder)
.await?
.requirements;

// Parse the `--from` requirement.
let from_requirement = resolve_names(
requirements,
&interpreter,
&settings,
&state,
connectivity,
concurrency,
native_tls,
&cache,
printer,
)
.await?
.requirements;
.pop()
.unwrap();

// Check if the positional name conflicts with `--from`.
if from_requirement.name != package {
// Determine if it's an entirely different package (e.g., `uv install foo --from bar`).
bail!(
"Package name (`{}`) provided with `--from` does not match install request (`{}`)",
from_requirement.name.cyan(),
package.cyan()
);
}

resolve_names(
requirements,
&interpreter,
&settings,
&state,
connectivity,
concurrency,
native_tls,
cache,
printer,
)
.await?
.pop()
.unwrap()
from_requirement
}
};

// If the user passed, e.g., `ruff@latest`, we need to mark it as upgradable.
let settings = if target.is_latest() {
ResolverInstallerSettings {
upgrade: settings
.upgrade
.combine(Upgrade::package(from.name.clone())),
..settings
}
} else {
settings
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This part here doesn't exist in uv tool run, because there, we always resolve an ephemeral environment and recreate it if any dependencies changed. Here, we need to ensure that we upgrade when re-resolving the tool environment.


// Read the `--with` requirements.
Expand All @@ -163,7 +227,7 @@ pub(crate) async fn install(
connectivity,
concurrency,
native_tls,
cache,
&cache,
printer,
)
.await?,
Expand Down Expand Up @@ -209,10 +273,10 @@ pub(crate) async fn install(

let existing_environment =
installed_tools
.get_environment(&from.name, cache)?
.get_environment(&from.name, &cache)?
.filter(|environment| {
python_request.as_ref().map_or(true, |python_request| {
if python_request.satisfied(environment.interpreter(), cache) {
if python_request.satisfied(environment.interpreter(), &cache) {
debug!("Found existing environment for `{from}`", from = from.name.cyan());
true
} else {
Expand All @@ -227,29 +291,34 @@ pub(crate) async fn install(
});

// If the requested and receipt requirements are the same...
if existing_environment.is_some() {
if existing_environment
.as_ref()
.filter(|_| {
// And the user didn't request a reinstall or upgrade...
!force
&& !target.is_latest()
&& settings.reinstall.is_none()
&& settings.upgrade.is_none()
})
.is_some()
{
if let Some(tool_receipt) = existing_tool_receipt.as_ref() {
let receipt = tool_receipt.requirements().to_vec();
if requirements == receipt {
// And the user didn't request a reinstall or upgrade...
if !force && settings.reinstall.is_none() && settings.upgrade.is_none() {
if *tool_receipt.options() != options {
// ...but the options differ, we need to update the receipt.
installed_tools.add_tool_receipt(
&from.name,
tool_receipt.clone().with_options(options),
)?;
}
if *tool_receipt.options() != options {
// ...but the options differ, we need to update the receipt.
installed_tools
.add_tool_receipt(&from.name, tool_receipt.clone().with_options(options))?;
}

// We're done, though we might need to update the receipt.
writeln!(
printer.stderr(),
"`{from}` is already installed",
from = from.cyan()
)?;
// We're done, though we might need to update the receipt.
writeln!(
printer.stderr(),
"`{from}` is already installed",
from = from.cyan()
)?;

return Ok(ExitStatus::Success);
}
return Ok(ExitStatus::Success);
}
}
}
Expand Down Expand Up @@ -279,7 +348,7 @@ pub(crate) async fn install(
connectivity,
concurrency,
native_tls,
cache,
&cache,
printer,
)
.await?
Expand All @@ -304,7 +373,7 @@ pub(crate) async fn install(
connectivity,
concurrency,
native_tls,
cache,
&cache,
printer,
)
.await?;
Expand All @@ -327,7 +396,7 @@ pub(crate) async fn install(
connectivity,
concurrency,
native_tls,
cache,
&cache,
printer,
)
.await?
Expand Down
Loading
Loading