From 96c3c2e77415a7e2afc9d2f15e261db1d41acabb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 11 Apr 2024 11:26:50 -0400 Subject: [PATCH] Support unnamed requirements in `--require-hashes` (#2993) ## Summary This PR enables `--require-hashes` with unnamed requirements. The key change is that `PackageId` becomes `VersionId` (since it refers to a package at a specific version), and the new `PackageId` consists of _either_ a package name _or_ a URL. The hashes are keyed by `PackageId`, so we can generate the `RequiredHashes` before we have names for all packages, and enforce them throughout. Closes #2979. --- Cargo.lock | 1 + crates/distribution-types/src/id.rs | 46 ++++++- crates/distribution-types/src/traits.rs | 24 +++- crates/uv-build/src/lib.rs | 44 +++--- crates/uv-dispatch/src/lib.rs | 6 +- .../src/index/built_wheel_index.rs | 6 +- .../src/index/registry_wheel_index.rs | 6 +- crates/uv-installer/src/downloader.rs | 4 +- crates/uv-installer/src/plan.rs | 5 +- crates/uv-requirements/src/lookahead.rs | 6 +- crates/uv-requirements/src/source_tree.rs | 7 +- crates/uv-requirements/src/unnamed.rs | 20 +-- crates/uv-resolver/src/flat_index.rs | 4 +- crates/uv-resolver/src/resolution.rs | 28 ++-- .../src/resolver/batch_prefetch.rs | 2 +- crates/uv-resolver/src/resolver/index.rs | 12 +- crates/uv-resolver/src/resolver/mod.rs | 52 ++++--- crates/uv-resolver/src/resolver/provider.rs | 4 +- crates/uv-resolver/src/version_map.rs | 2 +- crates/uv-types/Cargo.toml | 1 + crates/uv-types/src/hash.rs | 128 ++++++++++++------ crates/uv-types/src/traits.rs | 4 +- crates/uv/src/commands/pip_install.rs | 7 +- crates/uv/src/commands/pip_sync.rs | 7 +- crates/uv/tests/pip_sync.rs | 15 +- 25 files changed, 256 insertions(+), 185 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 79b79ad7f..6a58eb025 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4842,6 +4842,7 @@ dependencies = [ "serde", "serde_json", "thiserror", + "url", "uv-cache", "uv-configuration", "uv-interpreter", diff --git a/crates/distribution-types/src/id.rs b/crates/distribution-types/src/id.rs index 9920ae2d5..cca3237da 100644 --- a/crates/distribution-types/src/id.rs +++ b/crates/distribution-types/src/id.rs @@ -1,32 +1,64 @@ use std::fmt::{Display, Formatter}; +use cache_key::CanonicalUrl; use url::Url; use pep440_rs::Version; use uv_normalize::PackageName; -/// A unique identifier for a package at a specific version (e.g., `black==23.10.0`). +/// A unique identifier for a package. A package can either be identified by a name (e.g., `black`) +/// or a URL (e.g., `git+https://github.com/psf/black`). #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum PackageId { - /// The identifier consists of a package name and version. - NameVersion(PackageName, Version), + /// The identifier consists of a package name. + Name(PackageName), /// The identifier consists of a URL. Url(String), } impl PackageId { /// Create a new [`PackageId`] from a package name and version. - pub fn from_registry(name: PackageName, version: Version) -> Self { - Self::NameVersion(name, version) + pub fn from_registry(name: PackageName) -> Self { + Self::Name(name) } /// Create a new [`PackageId`] from a URL. pub fn from_url(url: &Url) -> Self { - Self::Url(cache_key::digest(&cache_key::CanonicalUrl::new(url))) + Self::Url(cache_key::digest(&CanonicalUrl::new(url))) } } impl Display for PackageId { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::Name(name) => write!(f, "{name}"), + Self::Url(url) => write!(f, "{url}"), + } + } +} + +/// A unique identifier for a package at a specific version (e.g., `black==23.10.0`). +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum VersionId { + /// The identifier consists of a package name and version. + NameVersion(PackageName, Version), + /// The identifier consists of a URL. + Url(String), +} + +impl VersionId { + /// Create a new [`VersionId`] from a package name and version. + pub fn from_registry(name: PackageName, version: Version) -> Self { + Self::NameVersion(name, version) + } + + /// Create a new [`VersionId`] from a URL. + pub fn from_url(url: &Url) -> Self { + Self::Url(cache_key::digest(&CanonicalUrl::new(url))) + } +} + +impl Display for VersionId { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Self::NameVersion(name, version) => write!(f, "{name}-{version}"), @@ -73,7 +105,7 @@ impl ResourceId { } } -impl From<&Self> for PackageId { +impl From<&Self> for VersionId { /// Required for `WaitMap::wait`. fn from(value: &Self) -> Self { value.clone() diff --git a/crates/distribution-types/src/traits.rs b/crates/distribution-types/src/traits.rs index c5beacbf5..5d87219dc 100644 --- a/crates/distribution-types/src/traits.rs +++ b/crates/distribution-types/src/traits.rs @@ -10,7 +10,8 @@ use crate::{ BuiltDist, CachedDirectUrlDist, CachedDist, CachedRegistryDist, DirectUrlBuiltDist, DirectUrlSourceDist, Dist, DistributionId, GitSourceDist, InstalledDirectUrlDist, InstalledDist, InstalledRegistryDist, InstalledVersion, LocalDist, PackageId, PathBuiltDist, - PathSourceDist, RegistryBuiltDist, RegistrySourceDist, ResourceId, SourceDist, VersionOrUrl, + PathSourceDist, RegistryBuiltDist, RegistrySourceDist, ResourceId, SourceDist, VersionId, + VersionOrUrl, }; pub trait Name { @@ -25,16 +26,29 @@ pub trait DistributionMetadata: Name { /// for URL-based distributions. fn version_or_url(&self) -> VersionOrUrl; - /// Returns a unique identifier for the package (e.g., `black==23.10.0`). + /// Returns a unique identifier for the package at the given version (e.g., `black==23.10.0`). /// /// Note that this is not equivalent to a unique identifier for the _distribution_, as multiple /// registry-based distributions (e.g., different wheels for the same package and version) - /// will return the same package ID, but different distribution IDs. - fn package_id(&self) -> PackageId { + /// will return the same version ID, but different distribution IDs. + fn version_id(&self) -> VersionId { match self.version_or_url() { VersionOrUrl::Version(version) => { - PackageId::from_registry(self.name().clone(), version.clone()) + VersionId::from_registry(self.name().clone(), version.clone()) } + VersionOrUrl::Url(url) => VersionId::from_url(url), + } + } + + /// Returns a unique identifier for a package. A package can either be identified by a name + /// (e.g., `black`) or a URL (e.g., `git+https://github.com/psf/black`). + /// + /// Note that this is not equivalent to a unique identifier for the _distribution_, as multiple + /// registry-based distributions (e.g., different wheels for the same package and version) + /// will return the same version ID, but different distribution IDs. + fn package_id(&self) -> PackageId { + match self.version_or_url() { + VersionOrUrl::Version(_) => PackageId::from_registry(self.name().clone()), VersionOrUrl::Url(url) => PackageId::from_url(url), } } diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index 5933511aa..7d8ff3795 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -112,7 +112,7 @@ pub enum MissingLibrary { #[derive(Debug, Error)] pub struct MissingHeaderCause { missing_library: MissingLibrary, - package_id: String, + version_id: String, } impl Display for MissingHeaderCause { @@ -122,22 +122,22 @@ impl Display for MissingHeaderCause { write!( f, "This error likely indicates that you need to install a library that provides \"{}\" for {}", - header, self.package_id + header, self.version_id ) } MissingLibrary::Linker(library) => { write!( f, "This error likely indicates that you need to install the library that provides a shared library \ - for {library} for {package_id} (e.g. lib{library}-dev)", - library = library, package_id = self.package_id + for {library} for {version_id} (e.g. lib{library}-dev)", + library = library, version_id = self.version_id ) } MissingLibrary::PythonPackage(package) => { write!( f, - "This error likely indicates that you need to `uv pip install {package}` into the build environment for {package_id}", - package = package, package_id = self.package_id + "This error likely indicates that you need to `uv pip install {package}` into the build environment for {version_id}", + package = package, version_id = self.version_id ) } } @@ -148,7 +148,7 @@ impl Error { fn from_command_output( message: String, output: &Output, - package_id: impl Into, + version_id: impl Into, ) -> Self { let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); @@ -178,7 +178,7 @@ impl Error { stderr, missing_header_cause: MissingHeaderCause { missing_library, - package_id: package_id.into(), + version_id: version_id.into(), }, }; } @@ -364,7 +364,7 @@ pub struct SourceBuild { /// > it created. metadata_directory: Option, /// Package id such as `foo-1.2.3`, for error reporting - package_id: String, + version_id: String, /// Whether we do a regular PEP 517 build or an PEP 660 editable build build_kind: BuildKind, /// Modified PATH that contains the `venv_bin`, `user_path` and `system_path` variables in that order @@ -385,7 +385,7 @@ impl SourceBuild { interpreter: &Interpreter, build_context: &impl BuildContext, source_build_context: SourceBuildContext, - package_id: String, + version_id: String, setup_py: SetupPyStrategy, config_settings: ConfigSettings, build_isolation: BuildIsolation<'_>, @@ -477,7 +477,7 @@ impl SourceBuild { &venv, pep517_backend, build_context, - &package_id, + &version_id, build_kind, &config_settings, &environment_variables, @@ -497,7 +497,7 @@ impl SourceBuild { build_kind, config_settings, metadata_directory: None, - package_id, + version_id, environment_variables, modified_path, }) @@ -695,7 +695,7 @@ impl SourceBuild { return Err(Error::from_command_output( "Build backend failed to determine metadata through `prepare_metadata_for_build_wheel`".to_string(), &output, - &self.package_id, + &self.version_id, )); } @@ -714,7 +714,7 @@ impl SourceBuild { /// dir. /// /// - #[instrument(skip_all, fields(package_id = self.package_id))] + #[instrument(skip_all, fields(version_id = self.version_id))] pub async fn build_wheel(&self, wheel_dir: &Path) -> Result { // The build scripts run with the extracted root as cwd, so they need the absolute path. let wheel_dir = fs::canonicalize(wheel_dir)?; @@ -750,7 +750,7 @@ impl SourceBuild { return Err(Error::from_command_output( "Failed building wheel through setup.py".to_string(), &output, - &self.package_id, + &self.version_id, )); } let dist = fs::read_dir(self.source_tree.join("dist"))?; @@ -761,7 +761,7 @@ impl SourceBuild { "Expected exactly wheel in `dist/` after invoking setup.py, found {dist_dir:?}" ), &output, - &self.package_id) + &self.version_id) ); }; @@ -831,7 +831,7 @@ impl SourceBuild { self.build_kind ), &output, - &self.package_id, + &self.version_id, )); } @@ -843,7 +843,7 @@ impl SourceBuild { self.build_kind ), &output, - &self.package_id, + &self.version_id, )); } Ok(distribution_filename) @@ -873,7 +873,7 @@ async fn create_pep517_build_environment( venv: &PythonEnvironment, pep517_backend: &Pep517Backend, build_context: &impl BuildContext, - package_id: &str, + version_id: &str, build_kind: BuildKind, config_settings: &ConfigSettings, environment_variables: &FxHashMap, @@ -927,7 +927,7 @@ async fn create_pep517_build_environment( return Err(Error::from_command_output( format!("Build backend failed to determine extra requires with `build_{build_kind}()`"), &output, - package_id, + version_id, )); } @@ -938,7 +938,7 @@ async fn create_pep517_build_environment( "Build backend failed to read extra requires from `get_requires_for_build_{build_kind}`: {err}" ), &output, - package_id, + version_id, ) })?; @@ -949,7 +949,7 @@ async fn create_pep517_build_environment( "Build backend failed to return extra requires with `get_requires_for_build_{build_kind}`: {err}" ), &output, - package_id, + version_id, ) })?; diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index dab5b2cbe..b204b9a1f 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -269,12 +269,12 @@ impl<'a> BuildContext for BuildDispatch<'a> { Ok(()) } - #[instrument(skip_all, fields(package_id = package_id, subdirectory = ?subdirectory))] + #[instrument(skip_all, fields(version_id = version_id, subdirectory = ?subdirectory))] async fn setup_build<'data>( &'data self, source: &'data Path, subdirectory: Option<&'data Path>, - package_id: &'data str, + version_id: &'data str, dist: Option<&'data SourceDist>, build_kind: BuildKind, ) -> Result { @@ -304,7 +304,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { self.interpreter, self, self.source_build_context.clone(), - package_id.to_string(), + version_id.to_string(), self.setup_py, self.config_settings.clone(), self.build_isolation, diff --git a/crates/uv-distribution/src/index/built_wheel_index.rs b/crates/uv-distribution/src/index/built_wheel_index.rs index 16f44ff20..48777ba22 100644 --- a/crates/uv-distribution/src/index/built_wheel_index.rs +++ b/crates/uv-distribution/src/index/built_wheel_index.rs @@ -47,7 +47,7 @@ impl<'a> BuiltWheelIndex<'a> { // Enforce hash-checking by omitting any wheels that don't satisfy the required hashes. let revision = pointer.into_revision(); - if !revision.satisfies(self.hasher.get(&source_dist.name)) { + if !revision.satisfies(self.hasher.get(source_dist)) { return Ok(None); } @@ -81,7 +81,7 @@ impl<'a> BuiltWheelIndex<'a> { // Enforce hash-checking by omitting any wheels that don't satisfy the required hashes. let revision = pointer.into_revision(); - if !revision.satisfies(self.hasher.get(&source_dist.name)) { + if !revision.satisfies(self.hasher.get(source_dist)) { return Ok(None); } @@ -91,7 +91,7 @@ impl<'a> BuiltWheelIndex<'a> { /// Return the most compatible [`CachedWheel`] for a given source distribution at a git URL. pub fn git(&self, source_dist: &GitSourceDist) -> Option { // Enforce hash-checking, which isn't supported for Git distributions. - if self.hasher.get(&source_dist.name).is_validate() { + if self.hasher.get(source_dist).is_validate() { return None; } diff --git a/crates/uv-distribution/src/index/registry_wheel_index.rs b/crates/uv-distribution/src/index/registry_wheel_index.rs index 29b815d1a..0149a0cfb 100644 --- a/crates/uv-distribution/src/index/registry_wheel_index.rs +++ b/crates/uv-distribution/src/index/registry_wheel_index.rs @@ -123,7 +123,7 @@ impl<'a> RegistryWheelIndex<'a> { CachedWheel::from_http_pointer(wheel_dir.join(file), cache) { // Enforce hash-checking based on the built distribution. - if wheel.satisfies(hasher.get(package)) { + if wheel.satisfies(hasher.get_package(package)) { Self::add_wheel(wheel, tags, &mut versions); } } @@ -139,7 +139,7 @@ impl<'a> RegistryWheelIndex<'a> { CachedWheel::from_local_pointer(wheel_dir.join(file), cache) { // Enforce hash-checking based on the built distribution. - if wheel.satisfies(hasher.get(package)) { + if wheel.satisfies(hasher.get_package(package)) { Self::add_wheel(wheel, tags, &mut versions); } } @@ -184,7 +184,7 @@ impl<'a> RegistryWheelIndex<'a> { if let Some(revision) = revision { // Enforce hash-checking based on the source distribution. - if revision.satisfies(hasher.get(package)) { + if revision.satisfies(hasher.get_package(package)) { for wheel_dir in symlinks(cache_shard.join(revision.id())) { if let Some(wheel) = CachedWheel::from_built_source(wheel_dir) { Self::add_wheel(wheel, tags, &mut versions); diff --git a/crates/uv-installer/src/downloader.rs b/crates/uv-installer/src/downloader.rs index 5862bb210..e83d0bec8 100644 --- a/crates/uv-installer/src/downloader.rs +++ b/crates/uv-installer/src/downloader.rs @@ -8,7 +8,7 @@ use tracing::instrument; use url::Url; use distribution_types::{ - BuildableSource, CachedDist, Dist, Hashed, Identifier, LocalEditable, LocalEditables, Name, + BuildableSource, CachedDist, Dist, Hashed, Identifier, LocalEditable, LocalEditables, RemoteSource, }; use platform_tags::Tags; @@ -170,7 +170,7 @@ impl<'a, Context: BuildContext + Send + Sync> Downloader<'a, Context> { pub async fn get_wheel(&self, dist: Dist, in_flight: &InFlight) -> Result { let id = dist.distribution_id(); if in_flight.downloads.register(id.clone()) { - let policy = self.hashes.get(dist.name()); + let policy = self.hashes.get(&dist); let result = self .database .get_or_build_wheel(&dist, self.tags, policy) diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index 9895ac3be..21970eafb 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -13,7 +13,6 @@ use distribution_types::{ use pep508_rs::{Requirement, VersionOrUrl}; use platform_tags::Tags; use uv_cache::{ArchiveTarget, ArchiveTimestamp, Cache, CacheBucket, WheelCache}; - use uv_configuration::{NoBinary, Reinstall}; use uv_distribution::{ BuiltWheelIndex, HttpArchivePointer, LocalArchivePointer, RegistryWheelIndex, @@ -259,7 +258,7 @@ impl<'a> Planner<'a> { // Read the HTTP pointer. if let Some(pointer) = HttpArchivePointer::read_from(&cache_entry)? { let archive = pointer.into_archive(); - if archive.satisfies(hasher.get(&requirement.name)) { + if archive.satisfies(hasher.get(&wheel)) { let cached_dist = CachedDirectUrlDist::from_url( wheel.filename, wheel.url, @@ -301,7 +300,7 @@ impl<'a> Planner<'a> { let timestamp = ArchiveTimestamp::from_file(&wheel.path)?; if pointer.is_up_to_date(timestamp) { let archive = pointer.into_archive(); - if archive.satisfies(hasher.get(&requirement.name)) { + if archive.satisfies(hasher.get(&wheel)) { let cached_dist = CachedDirectUrlDist::from_url( wheel.filename, wheel.url, diff --git a/crates/uv-requirements/src/lookahead.rs b/crates/uv-requirements/src/lookahead.rs index 7d8ae63bf..496b6506a 100644 --- a/crates/uv-requirements/src/lookahead.rs +++ b/crates/uv-requirements/src/lookahead.rs @@ -5,7 +5,7 @@ use futures::stream::FuturesUnordered; use futures::StreamExt; use rustc_hash::FxHashSet; -use distribution_types::{Dist, DistributionMetadata, LocalEditable, Name}; +use distribution_types::{Dist, DistributionMetadata, LocalEditable}; use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl}; use pypi_types::Metadata23; use uv_client::RegistryClient; @@ -138,7 +138,7 @@ impl<'a, Context: BuildContext + Send + Sync> LookaheadResolver<'a, Context> { // Fetch the metadata for the distribution. let requires_dist = { - let id = dist.package_id(); + let id = dist.version_id(); if let Some(archive) = self .index .get_metadata(&id) @@ -157,7 +157,7 @@ impl<'a, Context: BuildContext + Send + Sync> LookaheadResolver<'a, Context> { // Run the PEP 517 build process to extract metadata from the source distribution. let archive = self .database - .get_or_build_wheel_metadata(&dist, self.hasher.get(dist.name())) + .get_or_build_wheel_metadata(&dist, self.hasher.get(&dist)) .await .with_context(|| match &dist { Dist::Built(built) => format!("Failed to download: {built}"), diff --git a/crates/uv-requirements/src/source_tree.rs b/crates/uv-requirements/src/source_tree.rs index 79ad535e0..920c42993 100644 --- a/crates/uv-requirements/src/source_tree.rs +++ b/crates/uv-requirements/src/source_tree.rs @@ -5,7 +5,7 @@ use anyhow::{Context, Result}; use futures::{StreamExt, TryStreamExt}; use url::Url; -use distribution_types::{BuildableSource, HashPolicy, PackageId, PathSourceUrl, SourceUrl}; +use distribution_types::{BuildableSource, HashPolicy, PathSourceUrl, SourceUrl, VersionId}; use pep508_rs::Requirement; use uv_client::RegistryClient; use uv_distribution::{DistributionDatabase, Reporter}; @@ -92,8 +92,7 @@ impl<'a, Context: BuildContext + Send + Sync> SourceTreeResolver<'a, Context> { let hashes = match self.hasher { HashStrategy::None => HashPolicy::None, HashStrategy::Generate => HashPolicy::Generate, - HashStrategy::Validate(_) => { - // TODO(charlie): Support `--require-hashes` for unnamed requirements. + HashStrategy::Validate { .. } => { return Err(anyhow::anyhow!( "Hash-checking is not supported for local directories: {}", source_tree.user_display() @@ -103,7 +102,7 @@ impl<'a, Context: BuildContext + Send + Sync> SourceTreeResolver<'a, Context> { // Fetch the metadata for the distribution. let metadata = { - let id = PackageId::from_url(source.url()); + let id = VersionId::from_url(source.url()); if let Some(archive) = self .index .get_metadata(&id) diff --git a/crates/uv-requirements/src/unnamed.rs b/crates/uv-requirements/src/unnamed.rs index deeb847e5..d51460e50 100644 --- a/crates/uv-requirements/src/unnamed.rs +++ b/crates/uv-requirements/src/unnamed.rs @@ -10,8 +10,8 @@ use tracing::debug; use distribution_filename::{SourceDistFilename, WheelFilename}; use distribution_types::{ - BuildableSource, DirectSourceUrl, GitSourceUrl, HashPolicy, PackageId, PathSourceUrl, - RemoteSource, SourceUrl, + BuildableSource, DirectSourceUrl, GitSourceUrl, PathSourceUrl, RemoteSource, SourceUrl, + VersionId, }; use pep508_rs::{ Requirement, RequirementsTxtRequirement, Scheme, UnnamedRequirement, VersionOrUrl, @@ -241,7 +241,7 @@ impl<'a, Context: BuildContext + Send + Sync> NamedRequirementsResolver<'a, Cont // Fetch the metadata for the distribution. let name = { - let id = PackageId::from_url(source.url()); + let id = VersionId::from_url(source.url()); if let Some(archive) = index.get_metadata(&id).as_deref().and_then(|response| { if let MetadataResponse::Found(archive) = response { Some(archive) @@ -252,20 +252,8 @@ impl<'a, Context: BuildContext + Send + Sync> NamedRequirementsResolver<'a, Cont // If the metadata is already in the index, return it. archive.metadata.name.clone() } else { - // Determine the hash policy. Since we don't have a package name, we perform a - // manual match. - let hashes = match hasher { - HashStrategy::None => HashPolicy::None, - HashStrategy::Generate => HashPolicy::Generate, - HashStrategy::Validate(_) => { - // TODO(charlie): Support `--require-hashes` for unnamed requirements. - return Err(anyhow::anyhow!( - "Unnamed requirements are not supported with `--require-hashes`" - )); - } - }; - // Run the PEP 517 build process to extract metadata from the source distribution. + let hashes = hasher.get_url(source.url()); let source = BuildableSource::Url(source); let archive = database.build_wheel_metadata(&source, hashes).await?; diff --git a/crates/uv-resolver/src/flat_index.rs b/crates/uv-resolver/src/flat_index.rs index 4a0093471..f37577c85 100644 --- a/crates/uv-resolver/src/flat_index.rs +++ b/crates/uv-resolver/src/flat_index.rs @@ -132,7 +132,7 @@ impl FlatIndex { } // Check if hashes line up - let hash = if let HashPolicy::Validate(required) = hasher.get(&filename.name) { + let hash = if let HashPolicy::Validate(required) = hasher.get_package(&filename.name) { if hashes.is_empty() { Hash::Missing } else if required.iter().any(|hash| hashes.contains(hash)) { @@ -174,7 +174,7 @@ impl FlatIndex { }; // Check if hashes line up - let hash = if let HashPolicy::Validate(required) = hasher.get(&filename.name) { + let hash = if let HashPolicy::Validate(required) = hasher.get_package(&filename.name) { if hashes.is_empty() { Hash::Missing } else if required.iter().any(|hash| hashes.contains(hash)) { diff --git a/crates/uv-resolver/src/resolution.rs b/crates/uv-resolver/src/resolution.rs index 7203668b1..533a979b0 100644 --- a/crates/uv-resolver/src/resolution.rs +++ b/crates/uv-resolver/src/resolution.rs @@ -12,7 +12,7 @@ use pubgrub::type_aliases::SelectedDependencies; use rustc_hash::{FxHashMap, FxHashSet}; use distribution_types::{ - Dist, DistributionMetadata, IndexUrl, LocalEditable, Name, PackageId, ResolvedDist, Verbatim, + Dist, DistributionMetadata, IndexUrl, LocalEditable, Name, ResolvedDist, Verbatim, VersionId, VersionOrUrl, }; use once_map::OnceMap; @@ -66,7 +66,7 @@ impl ResolutionGraph { selection: &SelectedDependencies, pins: &FilePins, packages: &OnceMap, - distributions: &OnceMap, + distributions: &OnceMap, state: &State, preferences: &Preferences, editables: Editables, @@ -135,7 +135,7 @@ impl ResolutionGraph { { hashes.insert(package_name.clone(), digests.to_vec()); } else if let Some(metadata_response) = - distributions.get(&pinned_package.package_id()) + distributions.get(&pinned_package.version_id()) { if let MetadataResponse::Found(ref archive) = *metadata_response { let mut digests = archive.hashes.clone(); @@ -168,17 +168,17 @@ impl ResolutionGraph { }); } } else { - let response = distributions.get(&dist.package_id()).unwrap_or_else(|| { + let response = distributions.get(&dist.version_id()).unwrap_or_else(|| { panic!( "Every package should have metadata: {:?}", - dist.package_id() + dist.version_id() ) }); let MetadataResponse::Found(archive) = &*response else { panic!( "Every package should have metadata: {:?}", - dist.package_id() + dist.version_id() ) }; @@ -222,17 +222,17 @@ impl ResolutionGraph { }); } } else { - let response = distributions.get(&dist.package_id()).unwrap_or_else(|| { + let response = distributions.get(&dist.version_id()).unwrap_or_else(|| { panic!( "Every package should have metadata: {:?}", - dist.package_id() + dist.version_id() ) }); let MetadataResponse::Found(archive) = &*response else { panic!( "Every package should have metadata: {:?}", - dist.package_id() + dist.version_id() ) }; @@ -429,20 +429,20 @@ impl ResolutionGraph { let mut seen_marker_values = FxHashSet::default(); for i in self.petgraph.node_indices() { let dist = &self.petgraph[i]; - let package_id = match dist.version_or_url() { + let version_id = match dist.version_or_url() { VersionOrUrl::Version(version) => { - PackageId::from_registry(dist.name().clone(), version.clone()) + VersionId::from_registry(dist.name().clone(), version.clone()) } - VersionOrUrl::Url(verbatim_url) => PackageId::from_url(verbatim_url.raw()), + VersionOrUrl::Url(verbatim_url) => VersionId::from_url(verbatim_url.raw()), }; let res = index .distributions - .get(&package_id) + .get(&version_id) .expect("every package in resolution graph has metadata"); let MetadataResponse::Found(archive, ..) = &*res else { panic!( "Every package should have metadata: {:?}", - dist.package_id() + dist.version_id() ) }; for req in manifest.apply(&archive.metadata.requires_dist) { diff --git a/crates/uv-resolver/src/resolver/batch_prefetch.rs b/crates/uv-resolver/src/resolver/batch_prefetch.rs index 93d32c4fa..558cdc2b7 100644 --- a/crates/uv-resolver/src/resolver/batch_prefetch.rs +++ b/crates/uv-resolver/src/resolver/batch_prefetch.rs @@ -141,7 +141,7 @@ impl BatchPrefetcher { dist ); prefetch_count += 1; - if index.distributions.register(candidate.package_id()) { + if index.distributions.register(candidate.version_id()) { let request = match dist { ResolvedDistRef::Installable(dist) => Request::Dist(dist.clone()), ResolvedDistRef::Installed(dist) => Request::Installed(dist.clone()), diff --git a/crates/uv-resolver/src/resolver/index.rs b/crates/uv-resolver/src/resolver/index.rs index af42c9a6f..614baf206 100644 --- a/crates/uv-resolver/src/resolver/index.rs +++ b/crates/uv-resolver/src/resolver/index.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use distribution_types::PackageId; +use distribution_types::VersionId; use once_map::OnceMap; use uv_normalize::PackageName; @@ -14,7 +14,7 @@ pub struct InMemoryIndex { pub(crate) packages: OnceMap, /// A map from package ID to metadata for that distribution. - pub(crate) distributions: OnceMap, + pub(crate) distributions: OnceMap, } impl InMemoryIndex { @@ -24,8 +24,8 @@ impl InMemoryIndex { } /// Insert a [`Metadata23`] into the index. - pub fn insert_metadata(&self, package_id: PackageId, response: MetadataResponse) { - self.distributions.done(package_id, response); + pub fn insert_metadata(&self, version_id: VersionId, response: MetadataResponse) { + self.distributions.done(version_id, response); } /// Get the [`VersionsResponse`] for a given package name, without waiting. @@ -34,7 +34,7 @@ impl InMemoryIndex { } /// Get the [`MetadataResponse`] for a given package ID, without waiting. - pub fn get_metadata(&self, package_id: &PackageId) -> Option> { - self.distributions.get(package_id) + pub fn get_metadata(&self, version_id: &VersionId) -> Option> { + self.distributions.get(version_id) } } diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 2ddf6c365..74a564682 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -523,29 +523,27 @@ impl< match package { PubGrubPackage::Root(_) => {} PubGrubPackage::Python(_) => {} - PubGrubPackage::Package(package_name, _extra, None) => { - // Validate that the package is permitted under hash-checking mode. - if !self.hasher.allows(package_name) { - return Err(ResolveError::UnhashedPackage(package_name.clone())); + PubGrubPackage::Package(name, _extra, None) => { + // Verify that the package is allowed under the hash-checking policy. + if !self.hasher.allows_package(name) { + return Err(ResolveError::UnhashedPackage(name.clone())); } // Emit a request to fetch the metadata for this package. - if self.index.packages.register(package_name.clone()) { - priorities.add(package_name.clone()); - request_sink - .send(Request::Package(package_name.clone())) - .await?; + if self.index.packages.register(name.clone()) { + priorities.add(name.clone()); + request_sink.send(Request::Package(name.clone())).await?; } } - PubGrubPackage::Package(package_name, _extra, Some(url)) => { - // Validate that the package is permitted under hash-checking mode. - if !self.hasher.allows(package_name) { - return Err(ResolveError::UnhashedPackage(package_name.clone())); + PubGrubPackage::Package(name, _extra, Some(url)) => { + // Verify that the package is allowed under the hash-checking policy. + if !self.hasher.allows_url(url) { + return Err(ResolveError::UnhashedPackage(name.clone())); } // Emit a request to fetch the metadata for this distribution. - let dist = Dist::from_url(package_name.clone(), url.clone())?; - if self.index.distributions.register(dist.package_id()) { + let dist = Dist::from_url(name.clone(), url.clone())?; + if self.index.distributions.register(dist.version_id()) { priorities.add(dist.name().clone()); request_sink.send(Request::Dist(dist)).await?; } @@ -646,7 +644,7 @@ impl< let response = self .index .distributions - .wait(&dist.package_id()) + .wait(&dist.version_id()) .await .ok_or(ResolveError::Unregistered)?; @@ -796,7 +794,7 @@ impl< let version = candidate.version().clone(); // Emit a request to fetch the metadata for this version. - if self.index.distributions.register(candidate.package_id()) { + if self.index.distributions.register(candidate.version_id()) { let request = match dist.for_resolution() { ResolvedDistRef::Installable(dist) => Request::Dist(dist.clone()), ResolvedDistRef::Installed(dist) => Request::Installed(dist.clone()), @@ -880,13 +878,13 @@ impl< Some(url) => PubGrubDistribution::from_url(package_name, url), None => PubGrubDistribution::from_registry(package_name, version), }; - let package_id = dist.package_id(); + let version_id = dist.version_id(); // Wait for the metadata to be available. self.index .distributions - .wait(&package_id) - .instrument(info_span!("distributions_wait", %package_id)) + .wait(&version_id) + .instrument(info_span!("distributions_wait", %version_id)) .await .ok_or(ResolveError::Unregistered)?; } @@ -931,7 +929,7 @@ impl< Some(url) => PubGrubDistribution::from_url(package_name, url), None => PubGrubDistribution::from_registry(package_name, version), }; - let package_id = dist.package_id(); + let version_id = dist.version_id(); // If the package does not exist in the registry or locally, we cannot fetch its dependencies if self.unavailable_packages.get(package_name).is_some() @@ -953,8 +951,8 @@ impl< let response = self .index .distributions - .wait(&package_id) - .instrument(info_span!("distributions_wait", %package_id)) + .wait(&version_id) + .instrument(info_span!("distributions_wait", %version_id)) .await .ok_or(ResolveError::Unregistered)?; @@ -1061,7 +1059,7 @@ impl< Some(Response::Installed { dist, metadata }) => { trace!("Received installed distribution metadata for: {dist}"); self.index.distributions.done( - dist.package_id(), + dist.version_id(), MetadataResponse::Found(ArchiveMetadata::from(metadata)), ); } @@ -1079,7 +1077,7 @@ impl< } _ => {} } - self.index.distributions.done(dist.package_id(), metadata); + self.index.distributions.done(dist.version_id(), metadata); } Some(Response::Dist { dist: Dist::Source(dist), @@ -1095,7 +1093,7 @@ impl< } _ => {} } - self.index.distributions.done(dist.package_id(), metadata); + self.index.distributions.done(dist.version_id(), metadata); } None => {} } @@ -1200,7 +1198,7 @@ impl< }; // Emit a request to fetch the metadata for this version. - if self.index.distributions.register(candidate.package_id()) { + if self.index.distributions.register(candidate.version_id()) { let dist = dist.for_resolution().to_owned(); let response = match dist { diff --git a/crates/uv-resolver/src/resolver/provider.rs b/crates/uv-resolver/src/resolver/provider.rs index 566da4894..3479620ac 100644 --- a/crates/uv-resolver/src/resolver/provider.rs +++ b/crates/uv-resolver/src/resolver/provider.rs @@ -3,7 +3,7 @@ use std::future::Future; use anyhow::Result; use chrono::{DateTime, Utc}; -use distribution_types::{Dist, IndexLocations, Name}; +use distribution_types::{Dist, IndexLocations}; use platform_tags::Tags; use uv_client::RegistryClient; @@ -181,7 +181,7 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider async fn get_or_build_wheel_metadata<'io>(&'io self, dist: &'io Dist) -> WheelMetadataResult { match self .fetcher - .get_or_build_wheel_metadata(dist, self.hasher.get(dist.name())) + .get_or_build_wheel_metadata(dist, self.hasher.get(dist)) .await { Ok(metadata) => Ok(MetadataResponse::Found(metadata)), diff --git a/crates/uv-resolver/src/version_map.rs b/crates/uv-resolver/src/version_map.rs index e6309f00c..91e41c4e2 100644 --- a/crates/uv-resolver/src/version_map.rs +++ b/crates/uv-resolver/src/version_map.rs @@ -112,7 +112,7 @@ impl VersionMap { .allowed_versions(package_name) .cloned() .unwrap_or_default(); - let required_hashes = hasher.get(package_name).digests().to_vec(); + let required_hashes = hasher.get_package(package_name).digests().to_vec(); Self { inner: VersionMapInner::Lazy(VersionMapLazy { map, diff --git a/crates/uv-types/Cargo.toml b/crates/uv-types/Cargo.toml index 2ee16ce44..c517c0bf0 100644 --- a/crates/uv-types/Cargo.toml +++ b/crates/uv-types/Cargo.toml @@ -30,6 +30,7 @@ rustc-hash = { workspace = true } serde = { workspace = true, optional = true } serde_json = { workspace = true, optional = true } thiserror = { workspace = true } +url = { workspace = true } [features] default = [] diff --git a/crates/uv-types/src/hash.rs b/crates/uv-types/src/hash.rs index 4e0e8532d..592682998 100644 --- a/crates/uv-types/src/hash.rs +++ b/crates/uv-types/src/hash.rs @@ -1,8 +1,10 @@ -use distribution_types::HashPolicy; -use rustc_hash::FxHashMap; use std::str::FromStr; -use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl}; +use rustc_hash::FxHashMap; +use url::Url; + +use distribution_types::{DistributionMetadata, HashPolicy, PackageId}; +use pep508_rs::{MarkerEnvironment, RequirementsTxtRequirement, VersionOrUrl}; use pypi_types::{HashDigest, HashError}; use uv_normalize::PackageName; @@ -14,74 +16,115 @@ pub enum HashStrategy { Generate, /// Hashes should be validated against a pre-defined list of hashes. If necessary, hashes should /// be generated so as to ensure that the archive is valid. - Validate(FxHashMap>), + Validate(FxHashMap>), } impl HashStrategy { - /// Return the [`HashPolicy`] for the given package. - pub fn get(&self, package_name: &PackageName) -> HashPolicy { + /// Return the [`HashPolicy`] for the given distribution. + pub fn get(&self, distribution: &T) -> HashPolicy { match self { Self::None => HashPolicy::None, Self::Generate => HashPolicy::Generate, Self::Validate(hashes) => hashes - .get(package_name) + .get(&distribution.package_id()) .map(Vec::as_slice) .map_or(HashPolicy::None, HashPolicy::Validate), } } - /// Returns `true` if the given package is allowed. - pub fn allows(&self, package_name: &PackageName) -> bool { + /// Return the [`HashPolicy`] for the given registry-based package. + pub fn get_package(&self, name: &PackageName) -> HashPolicy { match self { - Self::None => true, - Self::Generate => true, - Self::Validate(hashes) => hashes.contains_key(package_name), + Self::None => HashPolicy::None, + Self::Generate => HashPolicy::Generate, + Self::Validate(hashes) => hashes + .get(&PackageId::from_registry(name.clone())) + .map(Vec::as_slice) + .map_or(HashPolicy::None, HashPolicy::Validate), } } - /// Generate the required hashes from a set of [`Requirement`] entries. + /// Return the [`HashPolicy`] for the given direct URL package. + pub fn get_url(&self, url: &Url) -> HashPolicy { + match self { + Self::None => HashPolicy::None, + Self::Generate => HashPolicy::Generate, + Self::Validate(hashes) => hashes + .get(&PackageId::from_url(url)) + .map(Vec::as_slice) + .map_or(HashPolicy::None, HashPolicy::Validate), + } + } + + /// Returns `true` if the given registry-based package is allowed. + pub fn allows_package(&self, name: &PackageName) -> bool { + match self { + Self::None => true, + Self::Generate => true, + Self::Validate(hashes) => hashes.contains_key(&PackageId::from_registry(name.clone())), + } + } + + /// Returns `true` if the given direct URL package is allowed. + pub fn allows_url(&self, url: &Url) -> bool { + match self { + Self::None => true, + Self::Generate => true, + Self::Validate(hashes) => hashes.contains_key(&PackageId::from_url(url)), + } + } + + /// Generate the required hashes from a set of [`RequirementsTxtRequirement`] entries. pub fn from_requirements( - requirements: impl Iterator)>, + requirements: impl Iterator)>, markers: &MarkerEnvironment, ) -> Result { - let mut hashes = FxHashMap::>::default(); + let mut hashes = FxHashMap::>::default(); // For each requirement, map from name to allowed hashes. We use the last entry for each // package. - // - // For now, unnamed requirements are unsupported. This should be fine, since `--require-hashes` - // tends to be used after `pip-compile`, which will always output named requirements. - // - // TODO(charlie): Preserve hashes from `requirements.txt` through to this pass, so that we - // can iterate over requirements directly, rather than iterating over the entries. for (requirement, digests) in requirements { if !requirement.evaluate_markers(markers, &[]) { continue; } // Every requirement must be either a pinned version or a direct URL. - match requirement.version_or_url.as_ref() { - Some(VersionOrUrl::Url(_)) => { - // Direct URLs are always allowed. - } - Some(VersionOrUrl::VersionSpecifier(specifiers)) => { - if specifiers - .iter() - .any(|specifier| matches!(specifier.operator(), pep440_rs::Operator::Equal)) - { - // Pinned versions are allowed. - } else { - return Err(HashStrategyError::UnpinnedRequirement( - requirement.to_string(), - )); + let id = match &requirement { + RequirementsTxtRequirement::Pep508(requirement) => { + match requirement.version_or_url.as_ref() { + Some(VersionOrUrl::Url(url)) => { + // Direct URLs are always allowed. + PackageId::from_url(url) + } + Some(VersionOrUrl::VersionSpecifier(specifiers)) => { + // Must be a single specifier. + let [specifier] = specifiers.as_ref() else { + return Err(HashStrategyError::UnpinnedRequirement( + requirement.to_string(), + )); + }; + + // Must be pinned to a specific version. + if *specifier.operator() != pep440_rs::Operator::Equal { + return Err(HashStrategyError::UnpinnedRequirement( + requirement.to_string(), + )); + } + + PackageId::from_registry(requirement.name.clone()) + } + None => { + return Err(HashStrategyError::UnpinnedRequirement( + requirement.to_string(), + )) + } } } - None => { - return Err(HashStrategyError::UnpinnedRequirement( - requirement.to_string(), - )) + RequirementsTxtRequirement::Unnamed(requirement) => { + // Direct URLs are always allowed. + PackageId::from_url(&requirement.url) } - } + }; // Every requirement must include a hash. if digests.is_empty() { @@ -95,8 +138,7 @@ impl HashStrategy { .collect::, _>>() .unwrap(); - // TODO(charlie): Extract hashes from URL fragments. - hashes.insert(requirement.name, digests); + hashes.insert(id, digests); } Ok(Self::Validate(hashes)) @@ -107,8 +149,6 @@ impl HashStrategy { pub enum HashStrategyError { #[error(transparent)] Hash(#[from] HashError), - #[error("Unnamed requirements are not supported in `--require-hashes`")] - UnnamedRequirement, #[error("In `--require-hashes` mode, all requirement must have their versions pinned with `==`, but found: {0}")] UnpinnedRequirement(String), #[error("In `--require-hashes` mode, all requirement must have a hash, but none were provided for: {0}")] diff --git a/crates/uv-types/src/traits.rs b/crates/uv-types/src/traits.rs index 62b2ea818..bd5498f52 100644 --- a/crates/uv-types/src/traits.rs +++ b/crates/uv-types/src/traits.rs @@ -95,13 +95,13 @@ pub trait BuildContext: Sync { /// /// For PEP 517 builds, this calls `get_requires_for_build_wheel`. /// - /// `package_id` is for error reporting only. + /// `version_id` is for error reporting only. /// `dist` is for safety checks and may be null for editable builds. fn setup_build<'a>( &'a self, source: &'a Path, subdirectory: Option<&'a Path>, - package_id: &'a str, + version_id: &'a str, dist: Option<&'a SourceDist>, build_kind: BuildKind, ) -> impl Future> + Send + 'a; diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index cffa3d8ea..b2389d8e0 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -15,7 +15,7 @@ use distribution_types::{ LocalEditables, Name, Resolution, }; use install_wheel_rs::linker::LinkMode; -use pep508_rs::{MarkerEnvironment, Requirement, RequirementsTxtRequirement}; +use pep508_rs::{MarkerEnvironment, Requirement}; use platform_tags::Tags; use pypi_types::{Metadata23, Yanked}; use requirements_txt::EditableRequirement; @@ -191,10 +191,7 @@ pub(crate) async fn pip_install( HashStrategy::from_requirements( entries .into_iter() - .filter_map(|requirement| match requirement.requirement { - RequirementsTxtRequirement::Pep508(req) => Some((req, requirement.hashes)), - RequirementsTxtRequirement::Unnamed(_) => None, - }), + .map(|entry| (entry.requirement, entry.hashes)), markers, )? } else { diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index 2d401a646..b2295c1e7 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -10,7 +10,7 @@ use distribution_types::{ IndexLocations, InstalledMetadata, LocalDist, LocalEditable, LocalEditables, Name, ResolvedDist, }; use install_wheel_rs::linker::LinkMode; -use pep508_rs::RequirementsTxtRequirement; + use platform_tags::Tags; use pypi_types::Yanked; use requirements_txt::EditableRequirement; @@ -140,10 +140,7 @@ pub(crate) async fn pip_sync( HashStrategy::from_requirements( entries .into_iter() - .filter_map(|requirement| match requirement.requirement { - RequirementsTxtRequirement::Pep508(req) => Some((req, requirement.hashes)), - RequirementsTxtRequirement::Unnamed(_) => None, - }), + .map(|entry| (entry.requirement, entry.hashes)), markers, )? } else { diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index d39c103e2..811a681f5 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -3855,24 +3855,29 @@ fn require_hashes_source_path_mismatch() -> Result<()> { Ok(()) } -/// `--require-hashes` isn't supported for unnamed requirements (yet). +/// We allow `--require-hashes` for direct URL dependencies. #[test] fn require_hashes_unnamed() -> Result<()> { let context = TestContext::new("3.12"); let requirements_txt = context.temp_dir.child("requirements.txt"); requirements_txt - .write_str("https://foo.com --hash=sha256:cfdb2b588b9fc25ede96d8db56ed50848b0b649dca3dd1df0b11f683bb9e0b5f")?; + .write_str(indoc::indoc! {r" + https://files.pythonhosted.org/packages/36/55/ad4de788d84a630656ece71059665e01ca793c04294c463fd84132f40fe6/anyio-4.0.0-py3-none-any.whl --hash=sha256:cfdb2b588b9fc25ede96d8db56ed50848b0b649dca3dd1df0b11f683bb9e0b5f + "} )?; uv_snapshot!(command(&context) .arg("requirements.txt") .arg("--require-hashes"), @r###" - success: false - exit_code: 2 + success: true + exit_code: 0 ----- stdout ----- ----- stderr ----- - error: Unnamed requirements are not supported with `--require-hashes` + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + anyio==4.0.0 (from https://files.pythonhosted.org/packages/36/55/ad4de788d84a630656ece71059665e01ca793c04294c463fd84132f40fe6/anyio-4.0.0-py3-none-any.whl) "### );