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

Respect malformed .dist-info directories in tool installs #5756

Merged
merged 1 commit into from
Aug 3, 2024
Merged

Respect malformed .dist-info directories in tool installs #5756

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
5 changes: 5 additions & 0 deletions crates/uv-installer/src/site_packages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ impl SitePackages {
})
}

/// Returns the [`Interpreter`] used to install the packages.
pub fn interpreter(&self) -> &Interpreter {
&self.interpreter
}

/// Returns an iterator over the installed distributions.
pub fn iter(&self) -> impl Iterator<Item = &InstalledDist> {
self.distributions.iter().flatten()
Expand Down
36 changes: 16 additions & 20 deletions crates/uv-tool/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ pub enum Error {
MissingToolReceipt(String, PathBuf),
#[error("Failed to read tool environment packages at `{0}`: {1}")]
EnvironmentRead(PathBuf, String),
#[error("Failed find tool package `{0}` at `{1}`")]
MissingToolPackage(PackageName, PathBuf),
#[error("Failed find package `{0}` in tool environment")]
MissingToolPackage(PackageName),
#[error(transparent)]
Serialization(#[from] toml_edit::ser::Error),
}
Expand Down Expand Up @@ -273,6 +273,7 @@ impl InstalledTools {
))
}

/// Return the [`Version`] of an installed tool.
pub fn version(&self, name: &PackageName, cache: &Cache) -> Result<Version, Error> {
let environment_path = self.tool_dir(name);
let environment = PythonEnvironment::from_root(&environment_path, cache)?;
Expand All @@ -281,7 +282,7 @@ impl InstalledTools {
let packages = site_packages.get_packages(name);
let package = packages
.first()
.ok_or_else(|| Error::MissingToolPackage(name.clone(), environment_path))?;
.ok_or_else(|| Error::MissingToolPackage(name.clone()))?;
Ok(package.version().clone())
}

Expand Down Expand Up @@ -375,22 +376,17 @@ pub fn find_executable_directory() -> Result<PathBuf, Error> {
}

/// Find the `.dist-info` directory for a package in an environment.
fn find_dist_info(
environment: &PythonEnvironment,
fn find_dist_info<'a>(
site_packages: &'a SitePackages,
package_name: &PackageName,
package_version: &Version,
) -> Result<PathBuf, Error> {
let dist_info_prefix = format!(
"{}-{}.dist-info",
package_name.as_dist_info_name(),
package_version
);
environment
.interpreter()
.site_packages()
.map(|path| path.join(&dist_info_prefix))
.find(|path| path.is_dir())
.ok_or_else(|| Error::DistInfoMissing(dist_info_prefix, environment.root().to_path_buf()))
) -> Result<&'a Path, Error> {
site_packages
.get_packages(package_name)
.iter()
.find(|package| package.version() == package_version)
.map(|dist| dist.path())
.ok_or_else(|| Error::MissingToolPackage(package_name.clone()))
}

/// Find the paths to the entry points provided by a package in an environment.
Expand All @@ -400,12 +396,12 @@ fn find_dist_info(
///
/// Returns a list of `(name, path)` tuples.
pub fn entrypoint_paths(
environment: &PythonEnvironment,
site_packages: &SitePackages,
package_name: &PackageName,
package_version: &Version,
) -> Result<Vec<(String, PathBuf)>, Error> {
// Find the `.dist-info` directory in the installed environment.
let dist_info_path = find_dist_info(environment, package_name, package_version)?;
let dist_info_path = find_dist_info(site_packages, package_name, package_version)?;
debug!(
"Looking at `.dist-info` at: {}",
dist_info_path.user_display()
Expand All @@ -415,7 +411,7 @@ pub fn entrypoint_paths(
let record = read_record_file(&mut File::open(dist_info_path.join("RECORD"))?)?;

// The RECORD file uses relative paths, so we're looking for the relative path to be a prefix.
let layout = environment.interpreter().layout();
let layout = site_packages.interpreter().layout();
let script_relative = pathdiff::diff_paths(&layout.scheme.scripts, &layout.scheme.purelib)
.ok_or_else(|| {
io::Error::new(
Expand Down
15 changes: 4 additions & 11 deletions crates/uv/src/commands/tool/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
use distribution_types::{InstalledDist, Name};
use uv_installer::SitePackages;
use uv_python::PythonEnvironment;
use uv_tool::entrypoint_paths;

/// Return all packages which contain an executable with the given name.
pub(super) fn matching_packages(
name: &str,
environment: &PythonEnvironment,
) -> anyhow::Result<Vec<InstalledDist>> {
let site_packages = SitePackages::from_environment(environment)?;
let packages = site_packages
pub(super) fn matching_packages(name: &str, site_packages: &SitePackages) -> Vec<InstalledDist> {
site_packages
.iter()
.filter_map(|package| {
entrypoint_paths(environment, package.name(), package.version())
entrypoint_paths(site_packages, package.name(), package.version())
.ok()
.and_then(|entrypoints| {
entrypoints
Expand All @@ -26,7 +21,5 @@ pub(super) fn matching_packages(
.then(|| package.clone())
})
})
.collect();

Ok(packages)
.collect()
}
55 changes: 25 additions & 30 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 @@ -19,8 +19,7 @@ use uv_fs::Simplified;
use uv_installer::SitePackages;
use uv_normalize::PackageName;
use uv_python::{
EnvironmentPreference, PythonEnvironment, PythonFetch, PythonInstallation, PythonPreference,
PythonRequest,
EnvironmentPreference, PythonFetch, PythonInstallation, PythonPreference, PythonRequest,
};
use uv_requirements::{RequirementsSource, RequirementsSpecification};
use uv_shell::Shell;
Expand Down Expand Up @@ -350,7 +349,7 @@ pub(crate) async fn install(
);

let entry_points = entrypoint_paths(
&environment,
&site_packages,
installed_dist.name(),
installed_dist.version(),
)?;
Expand All @@ -377,7 +376,7 @@ pub(crate) async fn install(
from = from.name.cyan()
)?;

hint_executable_from_dependency(&from, &environment, printer)?;
hint_executable_from_dependency(&from, &site_packages, printer)?;

// Clean up the environment we just created.
installed_tools.remove_environment(&from.name)?;
Expand Down Expand Up @@ -508,40 +507,36 @@ fn remove_entrypoints(tool: &Tool) {
/// Displays a hint if an executable matching the package name can be found in a dependency of the package.
fn hint_executable_from_dependency(
from: &Requirement,
environment: &PythonEnvironment,
site_packages: &SitePackages,
printer: Printer,
) -> Result<()> {
match matching_packages(from.name.as_ref(), environment) {
Ok(packages) => match packages.as_slice() {
[] => {}
[package] => {
let command = format!("uv tool install {}", package.name());
writeln!(
printer.stdout(),
"However, an executable with the name `{}` is available via dependency `{}`.\nDid you mean `{}`?",
from.name.cyan(),
package.name().cyan(),
command.bold(),
)?;
}
packages => {
writeln!(
let packages = matching_packages(from.name.as_ref(), site_packages);
match packages.as_slice() {
[] => {}
[package] => {
let command = format!("uv tool install {}", package.name());
writeln!(
printer.stdout(),
"However, an executable with the name `{}` is available via the following dependencies::",
"However, an executable with the name `{}` is available via dependency `{}`.\nDid you mean `{}`?",
from.name.cyan(),
package.name().cyan(),
command.bold(),
)?;

for package in packages {
writeln!(printer.stdout(), "- {}", package.name().cyan())?;
}
writeln!(
}
packages => {
writeln!(
printer.stdout(),
"Did you mean to install one of them instead?"
"However, an executable with the name `{}` is available via the following dependencies::",
from.name.cyan(),
)?;

for package in packages {
writeln!(printer.stdout(), "- {}", package.name().cyan())?;
}
},
Err(err) => {
warn!("Failed to determine executables for packages: {err}");
writeln!(
printer.stdout(),
"Did you mean to install one of them instead?"
)?;
}
}

Expand Down
86 changes: 43 additions & 43 deletions crates/uv/src/commands/tool/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,22 @@ pub(crate) async fn run(
args.iter().map(|arg| arg.to_string_lossy()).join(" ")
);

let site_packages = SitePackages::from_environment(&environment)?;

// We check if the provided command is not part of the executables for the `from` package.
// If the command is found in other packages, we warn the user about the correct package to use.

warn_executable_not_provided_by_package(
&executable.to_string_lossy(),
&from.name,
&environment,
&site_packages,
&invocation_source,
);

let mut handle = match process.spawn() {
Ok(handle) => Ok(handle),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
match get_entrypoints(&from.name, &environment) {
match get_entrypoints(&from.name, &site_packages) {
Ok(entrypoints) => {
writeln!(
printer.stdout(),
Expand Down Expand Up @@ -209,17 +212,15 @@ pub(crate) async fn run(
/// Return the entry points for the specified package.
fn get_entrypoints(
from: &PackageName,
environment: &PythonEnvironment,
site_packages: &SitePackages,
) -> Result<Vec<(String, PathBuf)>> {
let site_packages = SitePackages::from_environment(environment)?;

let installed = site_packages.get_packages(from);
let Some(installed_dist) = installed.first().copied() else {
bail!("Expected at least one requirement")
};

Ok(entrypoint_paths(
environment,
site_packages,
installed_dist.name(),
installed_dist.version(),
)?)
Expand All @@ -231,45 +232,44 @@ fn get_entrypoints(
fn warn_executable_not_provided_by_package(
executable: &str,
from_package: &PackageName,
environment: &PythonEnvironment,
site_packages: &SitePackages,
invocation_source: &ToolRunCommand,
) {
if let Ok(packages) = matching_packages(executable, environment) {
if !packages
.iter()
.any(|package| package.name() == from_package)
{
match packages.as_slice() {
[] => {}
[package] => {
let suggested_command = format!(
"{invocation_source} --from {} {}",
package.name(),
executable
);
warn_user!(
"An executable named `{}` is not provided by package `{}` but is available via the dependency `{}`. Consider using `{}` instead.",
executable.cyan(),
from_package.cyan(),
package.name().cyan(),
suggested_command.green()
);
}
packages => {
let suggested_command = format!("{invocation_source} --from PKG {executable}");
let provided_by = packages
.iter()
.map(distribution_types::Name::name)
.map(|name| format!("- {}", name.cyan()))
.join("\n");
warn_user!(
"An executable named `{}` is not provided by package `{}` but is available via the following dependencies:\n- {}\nConsider using `{}` instead.",
executable.cyan(),
from_package.cyan(),
provided_by,
suggested_command.green(),
);
}
let packages = matching_packages(executable, site_packages);
if !packages
.iter()
.any(|package| package.name() == from_package)
{
match packages.as_slice() {
[] => {}
[package] => {
let suggested_command = format!(
"{invocation_source} --from {} {}",
package.name(),
executable
);
warn_user!(
"An executable named `{}` is not provided by package `{}` but is available via the dependency `{}`. Consider using `{}` instead.",
executable.cyan(),
from_package.cyan(),
package.name().cyan(),
suggested_command.green()
);
}
packages => {
let suggested_command = format!("{invocation_source} --from PKG {executable}");
let provided_by = packages
.iter()
.map(distribution_types::Name::name)
.map(|name| format!("- {}", name.cyan()))
.join("\n");
warn_user!(
"An executable named `{}` is not provided by package `{}` but is available via the following dependencies:\n- {}\nConsider using `{}` instead.",
executable.cyan(),
from_package.cyan(),
provided_by,
suggested_command.green(),
);
}
}
}
Expand Down
Loading
Loading