Respect data scripts in uv tool install (#4693)

## Summary

Packages that provide scripts that _aren't_ Python entrypoints need to
respected in `uv tool install`. For example, Ruff ships a script in
`ruff-0.5.0.data/scripts`.

Unfortunately, the `.data` directory doesn't exist in the virtual
environment at all (it's removed, per the spec, after install). So this
PR changes the entry point detection to look at the `RECORD` file, which
is the only evidence that the scripts were installed.

Closes https://github.com/astral-sh/uv/issues/4691.

## Test Plan

`cargo run uv tool install ruff` (snapshot tests to-come)
This commit is contained in:
Charlie Marsh 2024-07-01 12:22:37 -04:00 committed by GitHub
parent 081f092781
commit 324e9fe5cf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 89 additions and 80 deletions

1
Cargo.lock generated
View file

@ -5037,6 +5037,7 @@ dependencies = [
"fs-err",
"install-wheel-rs",
"path-slash",
"pathdiff",
"pep440_rs",
"pep508_rs",
"pypi-types",

View file

@ -11,11 +11,10 @@ use zip::result::ZipError;
use pep440_rs::Version;
use platform_tags::{Arch, Os};
use pypi_types::Scheme;
pub use script::{scripts_from_ini, Script};
pub use uninstall::{uninstall_egg, uninstall_legacy_editable, uninstall_wheel, Uninstall};
use uv_fs::Simplified;
use uv_normalize::PackageName;
pub use wheel::{parse_wheel_file, LibKind};
pub use wheel::{parse_wheel_file, read_record_file, LibKind};
pub mod linker;
pub mod metadata;

View file

@ -1,7 +1,7 @@
//! Like `wheel.rs`, but for installing wheels that have already been unzipped, rather than
//! reading from a zip file.
use std::path::{Path, PathBuf};
use std::path::Path;
use std::str::FromStr;
use std::time::SystemTime;
@ -143,24 +143,6 @@ pub fn install_wheel(
Ok(())
}
/// Determine the absolute path to an entrypoint script.
pub fn entrypoint_path(entrypoint: &Script, layout: &Layout) -> PathBuf {
if cfg!(windows) {
// On windows we actually build an .exe wrapper
let script_name = entrypoint
.name
// FIXME: What are the in-reality rules here for names?
.strip_suffix(".py")
.unwrap_or(&entrypoint.name)
.to_string()
+ ".exe";
layout.scheme.scripts.join(script_name)
} else {
layout.scheme.scripts.join(&entrypoint.name)
}
}
/// Find the `dist-info` directory in an unzipped wheel.
///
/// See: <https://github.com/PyO3/python-pkginfo-rs>

View file

@ -8,9 +8,9 @@ use serde::{Deserialize, Serialize};
/// tqdm-4.62.3.dist-info/RECORD,,
/// ```
#[derive(Deserialize, Serialize, PartialOrd, PartialEq, Ord, Eq)]
pub(crate) struct RecordEntry {
pub(crate) path: String,
pub(crate) hash: Option<String>,
pub struct RecordEntry {
pub path: String,
pub hash: Option<String>,
#[allow(dead_code)]
pub(crate) size: Option<u64>,
pub size: Option<u64>,
}

View file

@ -9,10 +9,10 @@ use crate::{wheel, Error};
/// A script defining the name of the runnable entrypoint and the module and function that should be
/// run.
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct Script {
pub name: String,
pub module: String,
pub function: String,
pub(crate) struct Script {
pub(crate) name: String,
pub(crate) module: String,
pub(crate) function: String,
}
impl Script {
@ -64,7 +64,7 @@ impl Script {
}
}
pub fn scripts_from_ini(
pub(crate) fn scripts_from_ini(
extras: Option<&[String]>,
python_minor: u8,
ini: String,

View file

@ -17,7 +17,6 @@ use zip::ZipWriter;
use pypi_types::DirectUrl;
use uv_fs::{relative_to, Simplified};
use crate::linker::entrypoint_path;
use crate::record::RecordEntry;
use crate::script::Script;
use crate::{Error, Layout};
@ -247,6 +246,24 @@ fn get_script_executable(python_executable: &Path, is_gui: bool) -> PathBuf {
}
}
/// Determine the absolute path to an entrypoint script.
fn entrypoint_path(entrypoint: &Script, layout: &Layout) -> PathBuf {
if cfg!(windows) {
// On windows we actually build an .exe wrapper
let script_name = entrypoint
.name
// FIXME: What are the in-reality rules here for names?
.strip_suffix(".py")
.unwrap_or(&entrypoint.name)
.to_string()
+ ".exe";
layout.scheme.scripts.join(script_name)
} else {
layout.scheme.scripts.join(&entrypoint.name)
}
}
/// Create the wrapper scripts in the bin folder of the venv for launching console scripts.
pub(crate) fn write_script_entrypoints(
layout: &Layout,
@ -632,7 +649,7 @@ pub(crate) fn extra_dist_info(
/// Reads the record file
/// <https://www.python.org/dev/peps/pep-0376/#record>
pub(crate) fn read_record_file(record: &mut impl Read) -> Result<Vec<RecordEntry>, Error> {
pub fn read_record_file(record: &mut impl Read) -> Result<Vec<RecordEntry>, Error> {
csv::ReaderBuilder::new()
.has_headers(false)
.escape(Some(b'"'))

View file

@ -27,6 +27,7 @@ uv-warnings = { workspace = true }
dirs-sys = { workspace = true }
fs-err = { workspace = true }
path-slash = { workspace = true }
pathdiff = { workspace = true }
serde = { workspace = true }
thiserror = { workspace = true }
toml = { workspace = true }

View file

@ -1,22 +1,23 @@
use core::fmt;
use fs_err as fs;
use install_wheel_rs::linker::entrypoint_path;
use install_wheel_rs::{scripts_from_ini, Script};
use pep440_rs::Version;
use pep508_rs::PackageName;
use std::io::{self, Write};
use std::path::{Path, PathBuf};
use fs_err as fs;
use fs_err::File;
use thiserror::Error;
use tracing::debug;
use install_wheel_rs::read_record_file;
use pep440_rs::Version;
use pep508_rs::PackageName;
pub use receipt::ToolReceipt;
pub use tool::{Tool, ToolEntrypoint};
use uv_cache::Cache;
use uv_fs::{LockedFile, Simplified};
use uv_state::{StateBucket, StateStore};
use uv_toolchain::{Interpreter, PythonEnvironment};
use uv_warnings::warn_user_once;
pub use receipt::ToolReceipt;
pub use tool::{Tool, ToolEntrypoint};
use uv_state::{StateBucket, StateStore};
mod receipt;
mod tool;
@ -291,7 +292,7 @@ pub fn find_executable_directory() -> Result<PathBuf, Error> {
.ok_or(Error::NoExecutableDirectory)
}
/// Find the dist-info directory for a package in an environment.
/// Find the `.dist-info` directory for a package in an environment.
fn find_dist_info(
environment: &PythonEnvironment,
package_name: &PackageName,
@ -306,53 +307,61 @@ fn find_dist_info(
.interpreter()
.site_packages()
.map(|path| path.join(&dist_info_prefix))
.find(|path| path.exists())
.find(|path| path.is_dir())
.ok_or_else(|| Error::DistInfoMissing(dist_info_prefix, environment.root().to_path_buf()))
}
/// Parses the `entry_points.txt` entry for console scripts
///
/// Returns (`script_name`, module, function)
fn parse_scripts(
dist_info_path: &Path,
python_minor: u8,
) -> Result<(Vec<Script>, Vec<Script>), Error> {
let entry_points_path = dist_info_path.join("entry_points.txt");
// Read the entry points mapping. If the file doesn't exist, we just return an empty mapping.
let Ok(ini) = fs::read_to_string(&entry_points_path) else {
debug!(
"Failed to read entry points at {}",
entry_points_path.user_display()
);
return Ok((Vec::new(), Vec::new()));
};
Ok(scripts_from_ini(None, python_minor, ini)?)
}
/// Find the paths to the entry points provided by a package in an environment.
///
/// Entry points can either be true Python entrypoints (defined in `entrypoints.txt`) or scripts in
/// the `.data` directory.
///
/// Returns a list of `(name, path)` tuples.
pub fn entrypoint_paths(
environment: &PythonEnvironment,
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)?;
debug!("Looking at dist-info at {}", dist_info_path.user_display());
debug!(
"Looking at `.dist-info` at: {}",
dist_info_path.user_display()
);
let (console_scripts, gui_scripts) =
parse_scripts(&dist_info_path, environment.interpreter().python_minor())?;
// Read the RECORD file.
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 script_relative = pathdiff::diff_paths(&layout.scheme.scripts, &layout.scheme.purelib)
.ok_or_else(|| {
io::Error::new(
io::ErrorKind::Other,
format!(
"Could not find relative path for: {}",
layout.scheme.scripts.simplified_display()
),
)
})?;
Ok(console_scripts
.into_iter()
.chain(gui_scripts)
.map(|entrypoint| {
let path = entrypoint_path(&entrypoint, &layout);
(entrypoint.name, path)
})
.collect())
// Identify any installed binaries (both entrypoints and scripts from the `.data` directory).
let mut entrypoints = vec![];
for entry in record {
let relative_path = PathBuf::from(&entry.path);
let Ok(path_in_scripts) = relative_path.strip_prefix(&script_relative) else {
continue;
};
let absolute_path = layout.scheme.scripts.join(path_in_scripts);
let script_name = entry
.path
.rsplit(std::path::MAIN_SEPARATOR)
.next()
.unwrap_or(&entry.path)
.to_string();
entrypoints.push((script_name, absolute_path));
}
Ok(entrypoints)
}

View file

@ -8,7 +8,7 @@ mod common;
#[test]
fn tool_uninstall() {
let context = TestContext::new("3.12");
let context = TestContext::new("3.12").with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");
@ -71,7 +71,7 @@ fn tool_uninstall() {
#[test]
fn tool_uninstall_not_installed() {
let context = TestContext::new("3.12");
let context = TestContext::new("3.12").with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");
@ -90,7 +90,7 @@ fn tool_uninstall_not_installed() {
#[test]
fn tool_uninstall_missing_receipt() {
let context = TestContext::new("3.12");
let context = TestContext::new("3.12").with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");