From 9346946c7f4edac72aaf5a0dc255a5431cfe7f39 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 6 Aug 2024 14:14:12 -0400 Subject: [PATCH] Allow downloading wheels for metadata with `--no-binary` (#5707) ## Summary We allow the use of (e.g.) `.whl.metadata` files when `--no-binary` is enabled, so it makes sense that we'd also also allow wheels to be downloaded for metadata extraction. So now, we validate `--no-binary` at install time, rather than metadata-fetch time. Closes https://github.com/astral-sh/uv/issues/5699. --- PIP_COMPATIBILITY.md | 9 ++++ crates/uv-dispatch/src/lib.rs | 1 + .../src/distribution_database.rs | 8 ---- crates/uv-distribution/src/error.rs | 2 - crates/uv-installer/src/preparer.rs | 46 +++++++++++++++---- crates/uv/src/commands/pip/operations.rs | 1 + 6 files changed, 48 insertions(+), 19 deletions(-) diff --git a/PIP_COMPATIBILITY.md b/PIP_COMPATIBILITY.md index 8352d98867ba..87e7db20d09b 100644 --- a/PIP_COMPATIBILITY.md +++ b/PIP_COMPATIBILITY.md @@ -337,6 +337,15 @@ package is "allowed" in such cases without building its metadata. Both pip and uv allow editables requirements to be built and installed even when `--only-binary` is provided. For example, `uv pip install -e . --only-binary :all:` is allowed. +## `--no-binary` enforcement + +The `--no-binary` argument is used to restrict installation to source distributions. When +`--no-binary` is provided, uv will refuse to install pre-built binary distributions, but _will_ +reuse any binary distributions that are already present in the local cache. + +Additionally, and in contrast to pip, uv's resolver will still read metadata from pre-built binary +distributions when `--no-binary` is provided. + ## Bytecode compilation Unlike pip, uv does not compile `.py` files to `.pyc` files during installation by default (i.e., uv diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index b880a457a364..79d36e97f841 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -249,6 +249,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { self.cache, tags, &HashStrategy::None, + self.build_options, DistributionDatabase::new( self.client, self, diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index d479f7071d34..64c88f6f59cb 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -145,14 +145,6 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { dist: &BuiltDist, hashes: HashPolicy<'_>, ) -> Result { - if self - .build_context - .build_options() - .no_binary_package(dist.name()) - { - return Err(Error::NoBinary); - } - match dist { BuiltDist::Registry(wheels) => { let wheel = wheels.best_wheel(); diff --git a/crates/uv-distribution/src/error.rs b/crates/uv-distribution/src/error.rs index 36ad15161d77..eb64f9292834 100644 --- a/crates/uv-distribution/src/error.rs +++ b/crates/uv-distribution/src/error.rs @@ -16,8 +16,6 @@ use uv_normalize::PackageName; pub enum Error { #[error("Building source distributions is disabled")] NoBuild, - #[error("Using pre-built wheels is disabled")] - NoBinary, // Network error #[error("Failed to parse URL: {0}")] diff --git a/crates/uv-installer/src/preparer.rs b/crates/uv-installer/src/preparer.rs index 5f7aed512ac8..7980e63e39f4 100644 --- a/crates/uv-installer/src/preparer.rs +++ b/crates/uv-installer/src/preparer.rs @@ -4,26 +4,33 @@ use std::sync::Arc; use futures::{stream::FuturesUnordered, FutureExt, Stream, TryFutureExt, TryStreamExt}; use pep508_rs::PackageName; use tokio::task::JoinError; -use tracing::instrument; +use tracing::{debug, instrument}; use url::Url; -use distribution_types::{BuildableSource, CachedDist, Dist, Hashed, Identifier, RemoteSource}; +use distribution_types::{ + BuildableSource, CachedDist, Dist, Hashed, Identifier, Name, RemoteSource, +}; use platform_tags::Tags; use uv_cache::Cache; +use uv_configuration::BuildOptions; use uv_distribution::{DistributionDatabase, LocalWheel}; use uv_types::{BuildContext, HashStrategy, InFlight}; #[derive(thiserror::Error, Debug)] pub enum Error { + #[error("Building source distributions is disabled, but attempted to build `{0}`")] + NoBuild(PackageName), + #[error("Using pre-built wheels is disabled, but attempted to use `{0}`")] + NoBinary(PackageName), #[error("Failed to unzip wheel: {0}")] - Unzip(Dist, #[source] uv_extract::Error), + Unzip(Dist, #[source] Box), #[error("Failed to fetch wheel: {0}")] - Fetch(Dist, #[source] uv_distribution::Error), + Fetch(Dist, #[source] Box), /// Should not occur; only seen when another task panicked. #[error("The task executor is broken, did some other task panic?")] Join(#[from] JoinError), #[error(transparent)] - Editable(#[from] uv_distribution::Error), + Editable(#[from] Box), #[error("Failed to write to the client cache")] CacheWrite(#[source] std::io::Error), #[error("Unzip failed in another thread: {0}")] @@ -37,6 +44,7 @@ pub struct Preparer<'a, Context: BuildContext> { tags: &'a Tags, cache: &'a Cache, hashes: &'a HashStrategy, + build_options: &'a BuildOptions, database: DistributionDatabase<'a, Context>, reporter: Option>, } @@ -46,12 +54,14 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { cache: &'a Cache, tags: &'a Tags, hashes: &'a HashStrategy, + build_options: &'a BuildOptions, database: DistributionDatabase<'a, Context>, ) -> Self { Self { tags, cache, hashes, + build_options, database, reporter: None, } @@ -65,6 +75,7 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { tags: self.tags, cache: self.cache, hashes: self.hashes, + build_options: self.build_options, database: self.database.with_reporter(Facade::from(reporter.clone())), reporter: Some(reporter.clone()), } @@ -110,10 +121,27 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { Ok(wheels) } - /// Download, build, and unzip a single wheel. #[instrument(skip_all, fields(name = % dist, size = ? dist.size(), url = dist.file().map(| file | file.url.to_string()).unwrap_or_default()))] pub async fn get_wheel(&self, dist: Dist, in_flight: &InFlight) -> Result { + // Validate that the distribution is compatible with the build options. + match dist { + Dist::Built(ref dist) => { + if self.build_options.no_binary_package(dist.name()) { + return Err(Error::NoBinary(dist.name().clone())); + } + } + Dist::Source(ref dist) => { + if self.build_options.no_build_package(dist.name()) { + if dist.is_editable() { + debug!("Allowing build for editable source distribution: {dist}"); + } else { + return Err(Error::NoBuild(dist.name().clone())); + } + } + } + } + let id = dist.distribution_id(); if in_flight.downloads.register(id.clone()) { let policy = self.hashes.get(&dist); @@ -122,7 +150,7 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { .database .get_or_build_wheel(&dist, self.tags, policy) .boxed_local() - .map_err(|err| Error::Fetch(dist.clone(), err)) + .map_err(|err| Error::Fetch(dist.clone(), Box::new(err))) .await .and_then(|wheel: LocalWheel| { if wheel.satisfies(policy) { @@ -130,11 +158,11 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> { } else { Err(Error::Fetch( dist.clone(), - uv_distribution::Error::hash_mismatch( + Box::new(uv_distribution::Error::hash_mismatch( dist.to_string(), policy.digests(), wheel.hashes(), - ), + )), )) } }) diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 21390138fac3..375bf29c1b12 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -400,6 +400,7 @@ pub(crate) async fn install( cache, tags, hasher, + build_options, DistributionDatabase::new(client, build_dispatch, concurrency.downloads, preview), ) .with_reporter(PrepareReporter::from(printer).with_length(remote.len() as u64));