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.

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

Allow downloading wheels for metadata with --no-binary #5707

Merged
merged 1 commit into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions PIP_COMPATIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.cache,
tags,
&HashStrategy::None,
self.build_options,
DistributionDatabase::new(
self.client,
self,
Expand Down
8 changes: 0 additions & 8 deletions crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,6 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
dist: &BuiltDist,
hashes: HashPolicy<'_>,
) -> Result<LocalWheel, Error> {
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();
Expand Down
2 changes: 0 additions & 2 deletions crates/uv-distribution/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ use uv_normalize::PackageName;
pub enum Error {
#[error("Building source distributions is disabled")]
NoBuild,
Copy link
Member Author

Choose a reason for hiding this comment

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

We do, however, need to be aggressive about rejecting builds at metadata-fetch time.

#[error("Using pre-built wheels is disabled")]
NoBinary,

// Network error
#[error("Failed to parse URL: {0}")]
Expand Down
46 changes: 37 additions & 9 deletions crates/uv-installer/src/preparer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<uv_extract::Error>),
#[error("Failed to fetch wheel: {0}")]
Fetch(Dist, #[source] uv_distribution::Error),
Fetch(Dist, #[source] Box<uv_distribution::Error>),
/// 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<uv_distribution::Error>),
Copy link
Member Author

Choose a reason for hiding this comment

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

(Clippy yelling at me about variant size.)

#[error("Failed to write to the client cache")]
CacheWrite(#[source] std::io::Error),
#[error("Unzip failed in another thread: {0}")]
Expand All @@ -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<Arc<dyn Reporter>>,
}
Expand All @@ -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,
}
Expand All @@ -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()),
}
Expand Down Expand Up @@ -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<CachedDist, Error> {
// 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);
Expand All @@ -122,19 +150,19 @@ 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) {
Ok(wheel)
} 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(),
),
)),
))
}
})
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading