From 31a67f539f860168b133fdba0889a339c1f7bd16 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 8 Apr 2024 14:15:20 -0400 Subject: [PATCH] Remove unused local wheel types (#2906) ## Summary No behavior changes. Just removing unused code. --- .../src/distribution_database.rs | 37 +++--- crates/uv-distribution/src/download.rs | 113 ++---------------- crates/uv-distribution/src/lib.rs | 4 +- crates/uv-distribution/src/unzip.rs | 36 ------ crates/uv-installer/src/downloader.rs | 42 +------ 5 files changed, 34 insertions(+), 198 deletions(-) delete mode 100644 crates/uv-distribution/src/unzip.rs diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index 79ad0acf5..e5c75187a 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -19,7 +19,6 @@ use uv_cache::{ArchiveTarget, ArchiveTimestamp, CacheBucket, CacheEntry, WheelCa use uv_client::{CacheControl, CachedClientError, Connectivity, RegistryClient}; use uv_types::{BuildContext, NoBinary, NoBuild}; -use crate::download::UnzippedWheel; use crate::locks::Locks; use crate::{Error, LocalWheel, Reporter, SourceDistributionBuilder}; @@ -121,11 +120,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> let path = editable_wheel_dir.join(&disk_filename); let target = editable_wheel_dir.join(cache_key::digest(&editable.path)); let archive = self.unzip_wheel(&path, &target).await?; - let wheel = LocalWheel::Unzipped(UnzippedWheel { + let wheel = LocalWheel { dist, filename, archive, - }); + }; Ok((wheel, metadata)) } @@ -175,11 +174,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .stream_wheel(url.clone(), &wheel.filename, &wheel_entry, dist) .await { - Ok(archive) => Ok(LocalWheel::Unzipped(UnzippedWheel { + Ok(archive) => Ok(LocalWheel { dist: Dist::Built(dist.clone()), archive, filename: wheel.filename.clone(), - })), + }), Err(Error::Extract(err)) if err.is_http_streaming_unsupported() => { warn!( "Streaming unsupported for {dist}; downloading wheel to disk ({err})" @@ -190,11 +189,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> let archive = self .download_wheel(url, &wheel.filename, &wheel_entry, dist) .await?; - Ok(LocalWheel::Unzipped(UnzippedWheel { + Ok(LocalWheel { dist: Dist::Built(dist.clone()), archive, filename: wheel.filename.clone(), - })) + }) } Err(err) => Err(err), } @@ -213,11 +212,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> .stream_wheel(wheel.url.raw().clone(), &wheel.filename, &wheel_entry, dist) .await { - Ok(archive) => Ok(LocalWheel::Unzipped(UnzippedWheel { + Ok(archive) => Ok(LocalWheel { dist: Dist::Built(dist.clone()), archive, filename: wheel.filename.clone(), - })), + }), Err(Error::Client(err)) if err.is_http_streaming_unsupported() => { warn!( "Streaming unsupported for {dist}; downloading wheel to disk ({err})" @@ -233,11 +232,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> dist, ) .await?; - Ok(LocalWheel::Unzipped(UnzippedWheel { + Ok(LocalWheel { dist: Dist::Built(dist.clone()), archive, filename: wheel.filename.clone(), - })) + }) } Err(err) => Err(err), } @@ -271,24 +270,24 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> // cached under a unique build ID, so unzipped directories are never stale. match built_wheel.target.canonicalize() { Ok(archive) => { - return Ok(LocalWheel::Unzipped(UnzippedWheel { + return Ok(LocalWheel { dist: Dist::Source(dist.clone()), archive, filename: built_wheel.filename, - })); + }); } Err(err) if err.kind() == io::ErrorKind::NotFound => {} Err(err) => return Err(Error::CacheRead(err)), } // Otherwise, unzip the wheel. - Ok(LocalWheel::Unzipped(UnzippedWheel { + Ok(LocalWheel { dist: Dist::Source(dist.clone()), archive: self .unzip_wheel(&built_wheel.path, &built_wheel.target) .await?, filename: built_wheel.filename, - })) + }) } /// Fetch the wheel metadata from the index, or from the cache if possible. @@ -480,11 +479,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> if ArchiveTimestamp::up_to_date_with(path, ArchiveTarget::Cache(&archive)) .map_err(Error::CacheRead)? { - return Ok(LocalWheel::Unzipped(UnzippedWheel { + return Ok(LocalWheel { dist: Dist::Built(dist.clone()), archive, filename: filename.clone(), - })); + }); } } Err(err) if err.kind() == io::ErrorKind::NotFound => {} @@ -492,11 +491,11 @@ impl<'a, Context: BuildContext + Send + Sync> DistributionDatabase<'a, Context> } // Otherwise, unzip the wheel. - Ok(LocalWheel::Unzipped(UnzippedWheel { + Ok(LocalWheel { dist: Dist::Built(dist.clone()), archive: self.unzip_wheel(path, wheel_entry.path()).await?, filename: filename.clone(), - })) + }) } /// Unzip a wheel into the cache, returning the path to the unzipped directory. diff --git a/crates/uv-distribution/src/download.rs b/crates/uv-distribution/src/download.rs index 7ee3726b5..b7123ff7c 100644 --- a/crates/uv-distribution/src/download.rs +++ b/crates/uv-distribution/src/download.rs @@ -6,9 +6,9 @@ use pypi_types::Metadata23; use crate::Error; -/// A wheel that's been unzipped while downloading +/// A locally available wheel. #[derive(Debug, Clone)] -pub struct UnzippedWheel { +pub struct LocalWheel { /// The remote distribution from which this wheel was downloaded. pub(crate) dist: Dist, /// The parsed filename. @@ -18,113 +18,32 @@ pub struct UnzippedWheel { pub(crate) archive: PathBuf, } -/// A downloaded wheel that's stored on-disk. -#[derive(Debug, Clone)] -pub struct DiskWheel { - /// The remote distribution from which this wheel was downloaded. - pub(crate) dist: Dist, - /// The parsed filename. - pub(crate) filename: WheelFilename, - /// The path to the downloaded wheel. - pub(crate) path: PathBuf, - /// The expected path to the downloaded wheel's entry in the cache. - /// Typically, a symlink within the wheels or built wheels bucket. - pub(crate) target: PathBuf, -} - -/// A wheel built from a source distribution that's stored on-disk. -#[derive(Debug, Clone)] -pub struct BuiltWheel { - /// The remote source distribution from which this wheel was built. - pub(crate) dist: Dist, - /// The parsed filename. - pub(crate) filename: WheelFilename, - /// The path to the built wheel. - pub(crate) path: PathBuf, - /// The expected path to the downloaded wheel's entry in the cache. - /// Typically, a symlink within the wheels or built wheels bucket. - pub(crate) target: PathBuf, -} - -/// A downloaded or built wheel. -#[derive(Debug, Clone)] -pub enum LocalWheel { - Unzipped(UnzippedWheel), - Disk(DiskWheel), - Built(BuiltWheel), -} - impl LocalWheel { /// Return the path to the downloaded wheel's entry in the cache. pub fn target(&self) -> &Path { - match self { - Self::Unzipped(wheel) => &wheel.archive, - Self::Disk(wheel) => &wheel.target, - Self::Built(wheel) => &wheel.target, - } + &self.archive } /// Return the [`Dist`] from which this wheel was downloaded. pub fn remote(&self) -> &Dist { - match self { - Self::Unzipped(wheel) => wheel.remote(), - Self::Disk(wheel) => wheel.remote(), - Self::Built(wheel) => wheel.remote(), - } + &self.dist } /// Return the [`WheelFilename`] of this wheel. pub fn filename(&self) -> &WheelFilename { - match self { - Self::Unzipped(wheel) => &wheel.filename, - Self::Disk(wheel) => &wheel.filename, - Self::Built(wheel) => &wheel.filename, - } - } - - /// Convert a [`LocalWheel`] into a [`CachedDist`]. - pub fn into_cached_dist(self, archive: PathBuf) -> CachedDist { - match self { - Self::Unzipped(wheel) => CachedDist::from_remote(wheel.dist, wheel.filename, archive), - Self::Disk(wheel) => CachedDist::from_remote(wheel.dist, wheel.filename, archive), - Self::Built(wheel) => CachedDist::from_remote(wheel.dist, wheel.filename, archive), - } + &self.filename } /// Read the [`Metadata23`] from a wheel. pub fn metadata(&self) -> Result { - match self { - Self::Unzipped(wheel) => read_flat_wheel_metadata(&wheel.filename, &wheel.archive), - Self::Disk(wheel) => read_built_wheel_metadata(&wheel.filename, &wheel.path), - Self::Built(wheel) => read_built_wheel_metadata(&wheel.filename, &wheel.path), - } + read_flat_wheel_metadata(&self.filename, &self.archive) } } -impl UnzippedWheel { - /// Return the [`Dist`] from which this wheel was downloaded. - pub fn remote(&self) -> &Dist { - &self.dist - } - - /// Convert an [`UnzippedWheel`] into a [`CachedDist`]. - pub fn into_cached_dist(self) -> CachedDist { - CachedDist::from_remote(self.dist, self.filename, self.archive) - } -} - -impl DiskWheel { - /// Return the [`Dist`] from which this wheel was downloaded. - pub fn remote(&self) -> &Dist { - &self.dist - } -} - -impl BuiltWheel { - /// Return the [`Dist`] from which this source distribution that this wheel was built from was - /// downloaded. - pub fn remote(&self) -> &Dist { - &self.dist +/// Convert a [`LocalWheel`] into a [`CachedDist`]. +impl From for CachedDist { + fn from(wheel: LocalWheel) -> CachedDist { + CachedDist::from_remote(wheel.dist, wheel.filename, wheel.archive) } } @@ -134,18 +53,6 @@ impl std::fmt::Display for LocalWheel { } } -/// Read the [`Metadata23`] from a built wheel. -fn read_built_wheel_metadata( - filename: &WheelFilename, - wheel: impl AsRef, -) -> Result { - let file = fs_err::File::open(wheel.as_ref()).map_err(Error::CacheRead)?; - let reader = std::io::BufReader::new(file); - let mut archive = zip::ZipArchive::new(reader)?; - let metadata = install_wheel_rs::metadata::read_archive_metadata(filename, &mut archive)?; - Ok(Metadata23::parse_metadata(&metadata)?) -} - /// Read the [`Metadata23`] from an unzipped wheel. fn read_flat_wheel_metadata( filename: &WheelFilename, diff --git a/crates/uv-distribution/src/lib.rs b/crates/uv-distribution/src/lib.rs index 5c04678c4..f74b0fc9d 100644 --- a/crates/uv-distribution/src/lib.rs +++ b/crates/uv-distribution/src/lib.rs @@ -1,11 +1,10 @@ pub use distribution_database::DistributionDatabase; -pub use download::{BuiltWheel, DiskWheel, LocalWheel}; +pub use download::LocalWheel; pub use error::Error; pub use git::{is_same_reference, to_precise}; pub use index::{BuiltWheelIndex, RegistryWheelIndex}; pub use reporter::Reporter; pub use source::SourceDistributionBuilder; -pub use unzip::Unzip; mod distribution_database; mod download; @@ -15,4 +14,3 @@ mod index; mod locks; mod reporter; mod source; -mod unzip; diff --git a/crates/uv-distribution/src/unzip.rs b/crates/uv-distribution/src/unzip.rs deleted file mode 100644 index 92f5b4867..000000000 --- a/crates/uv-distribution/src/unzip.rs +++ /dev/null @@ -1,36 +0,0 @@ -use std::path::Path; - -use tracing::instrument; - -use uv_extract::Error; - -use crate::download::BuiltWheel; -use crate::{DiskWheel, LocalWheel}; - -pub trait Unzip { - /// Unzip a wheel into the target directory. - fn unzip(&self, target: &Path) -> Result<(), Error>; -} - -impl Unzip for DiskWheel { - fn unzip(&self, target: &Path) -> Result<(), Error> { - uv_extract::unzip(fs_err::File::open(&self.path)?, target) - } -} - -impl Unzip for BuiltWheel { - fn unzip(&self, target: &Path) -> Result<(), Error> { - uv_extract::unzip(fs_err::File::open(&self.path)?, target) - } -} - -impl Unzip for LocalWheel { - #[instrument(skip_all, fields(filename=self.filename().to_string()))] - fn unzip(&self, target: &Path) -> Result<(), Error> { - match self { - Self::Unzipped(_) => Ok(()), - Self::Disk(wheel) => wheel.unzip(target), - Self::Built(wheel) => wheel.unzip(target), - } - } -} diff --git a/crates/uv-installer/src/downloader.rs b/crates/uv-installer/src/downloader.rs index e84405be1..bba3c2fa7 100644 --- a/crates/uv-installer/src/downloader.rs +++ b/crates/uv-installer/src/downloader.rs @@ -3,7 +3,6 @@ use std::path::Path; use std::sync::Arc; use futures::{FutureExt, Stream, StreamExt, TryFutureExt, TryStreamExt}; -use tempfile::TempDir; use tokio::task::JoinError; use tracing::instrument; use url::Url; @@ -14,7 +13,7 @@ use distribution_types::{ use platform_tags::Tags; use uv_cache::Cache; use uv_client::RegistryClient; -use uv_distribution::{DistributionDatabase, LocalWheel, Unzip}; +use uv_distribution::DistributionDatabase; use uv_types::{BuildContext, InFlight}; use crate::editable::BuiltEditable; @@ -133,7 +132,7 @@ impl<'a, Context: BuildContext + Send + Sync> Downloader<'a, Context> { .build_wheel_editable(&editable, editable_wheel_dir) .await .map_err(Error::Editable)?; - let cached_dist = self.unzip_wheel(local_wheel).await?; + let cached_dist = CachedDist::from(local_wheel); if let Some(task_id) = task_id { if let Some(reporter) = &self.reporter { reporter.on_editable_build_complete(&editable, task_id); @@ -166,13 +165,13 @@ 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 download: LocalWheel = self + let result = self .database .get_or_build_wheel(&dist, self.tags) .boxed() .map_err(|err| Error::Fetch(dist.clone(), err)) - .await?; - let result = self.unzip_wheel(download).await; + .await + .map(CachedDist::from); match result { Ok(cached) => { in_flight.downloads.done(id, Ok(cached.clone())); @@ -196,37 +195,6 @@ impl<'a, Context: BuildContext + Send + Sync> Downloader<'a, Context> { } } } - - /// Unzip a locally-available wheel into the cache. - async fn unzip_wheel(&self, download: LocalWheel) -> Result { - // Just an optimization: Avoid spawning a blocking task if there is no work to be done. - if let LocalWheel::Unzipped(download) = download { - return Ok(download.into_cached_dist()); - } - - // Unzip the wheel. - let temp_dir = tokio::task::spawn_blocking({ - let download = download.clone(); - let cache = self.cache.clone(); - move || -> Result { - // Unzip the wheel into a temporary directory. - let temp_dir = tempfile::tempdir_in(cache.root())?; - download.unzip(temp_dir.path())?; - Ok(temp_dir) - } - }) - .await? - .map_err(|err| Error::Unzip(download.remote().clone(), err))?; - - // Persist the temporary directory to the directory store. - let archive = self - .cache - .persist(temp_dir.into_path(), download.target()) - .map_err(Error::CacheWrite) - .await?; - - Ok(download.into_cached_dist(archive)) - } } pub trait Reporter: Send + Sync {