From b0286a89398a242430e40f2b948db2dcb816c7e8 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 7 Nov 2023 06:17:31 -0800 Subject: [PATCH] Add user feedback when building source distributions in the resolver (#347) It looks like Cargo, notice the bold green lines at the top (which appear during the resolution, to indicate Git fetches and source distribution builds): Screen Shot 2023-11-06 at 11 28 47 PM Screen Shot 2023-11-06 at 11 28 51 PM Closes https://github.com/astral-sh/puffin/issues/287 although we can do a lot more here. --- Cargo.lock | 92 ++++++++++------ crates/puffin-cli/src/commands/reporters.rs | 101 +++++++++++++++++- crates/puffin-cli/tests/pip_compile.rs | 2 +- crates/puffin-git/src/git.rs | 13 +++ crates/puffin-git/src/lib.rs | 2 +- crates/puffin-git/src/source.rs | 47 +++++++- .../puffin-resolver/src/distribution/mod.rs | 4 +- .../src/distribution/source_distribution.rs | 86 ++++++++++++--- crates/puffin-resolver/src/lib.rs | 2 +- crates/puffin-resolver/src/resolver.rs | 55 +++++++++- scripts/benchmarks/requirements.in | 2 + 11 files changed, 341 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 718c8bf23..4892586ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1515,9 +1515,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.64" +version = "0.3.65" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5f195fe497f702db0f318b07fdd68edb16955aed830df8363d837542f8f935a" +checksum = "54c0c35952f67de54bb584e9fd912b3023117cbafc0a77d8f3dee1fb5f572fe8" dependencies = [ "wasm-bindgen", ] @@ -1548,6 +1548,17 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "libredox" +version = "0.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85c833ca1e66078851dba29046874e38f08b2c883700aa29a03ddd3b23814ee8" +dependencies = [ + "bitflags 2.4.1", + "libc", + "redox_syscall 0.4.1", +] + [[package]] name = "libssh2-sys" version = "0.3.0" @@ -1814,9 +1825,9 @@ dependencies = [ [[package]] name = "openssl-sys" -version = "0.9.93" +version = "0.9.95" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db4d56a4c0478783083cfafcc42493dd4a981d41669da64b4572a2a089b51b1d" +checksum = "40a4130519a360279579c2053038317e40eff64d13fd3f004f9e1b72b8a6aaf9" dependencies = [ "cc", "libc", @@ -2175,7 +2186,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", - "toml 0.8.6", + "toml 0.8.8", "tracing", "which", "zip", @@ -2231,7 +2242,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", - "toml 0.8.6", + "toml 0.8.8", "tracing", "tracing-subscriber", "tracing-tree", @@ -2687,12 +2698,12 @@ dependencies = [ [[package]] name = "redox_users" -version = "0.4.3" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b033d837a7cf162d7993aded9304e30a83213c648b6e389db233191f891e5c2b" +checksum = "a18479200779601e498ada4e8c1e1f50e3ee19deb0259c25825a98b5603b2cb4" dependencies = [ "getrandom", - "redox_syscall 0.2.16", + "libredox", "thiserror", ] @@ -3015,18 +3026,18 @@ checksum = "1c107b6f4780854c8b126e228ea8869f4d7b71260f962fefb57b996b8959ba6b" [[package]] name = "serde" -version = "1.0.190" +version = "1.0.191" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91d3c334ca1ee894a2c6f6ad698fe8c435b76d504b13d436f0685d648d6d96f7" +checksum = "a834c4821019838224821468552240d4d95d14e751986442c816572d39a080c9" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.190" +version = "1.0.191" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67c5609f394e5c2bd7fc51efda478004ea80ef42fee983d5c67a65e34f32c0e3" +checksum = "46fa52d5646bce91b680189fe5b1c049d2ea38dabb4e2e7c8d00ca12cfbfbcfd" dependencies = [ "proc-macro2", "quote", @@ -3545,14 +3556,14 @@ dependencies = [ [[package]] name = "toml" -version = "0.8.6" +version = "0.8.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ff9e3abce27ee2c9a37f9ad37238c1bdd4e789c84ba37df76aa4d528f5072cc" +checksum = "a1a195ec8c9da26928f773888e0742ca3ca1040c6cd859c919c9f59c1954ab35" dependencies = [ "serde", "serde_spanned", "toml_datetime", - "toml_edit 0.20.7", + "toml_edit 0.21.0", ] [[package]] @@ -3590,6 +3601,19 @@ dependencies = [ "winnow", ] +[[package]] +name = "toml_edit" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d34d383cd00a163b4a5b85053df514d45bc330f6de7737edfe0a93311d1eaa03" +dependencies = [ + "indexmap 2.1.0", + "serde", + "serde_spanned", + "toml_datetime", + "winnow", +] + [[package]] name = "tower-service" version = "0.3.2" @@ -3869,9 +3893,9 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" [[package]] name = "wasm-bindgen" -version = "0.2.87" +version = "0.2.88" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7706a72ab36d8cb1f80ffbf0e071533974a60d0a308d01a5d0375bf60499a342" +checksum = "7daec296f25a1bae309c0cd5c29c4b260e510e6d813c286b19eaadf409d40fce" dependencies = [ "cfg-if 1.0.0", "wasm-bindgen-macro", @@ -3879,9 +3903,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-backend" -version = "0.2.87" +version = "0.2.88" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ef2b6d3c510e9625e5fe6f509ab07d66a760f0885d858736483c32ed7809abd" +checksum = "e397f4664c0e4e428e8313a469aaa58310d302159845980fd23b0f22a847f217" dependencies = [ "bumpalo", "log", @@ -3894,9 +3918,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.37" +version = "0.4.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c02dbc21516f9f1f04f187958890d7e6026df8d16540b7ad9492bc34a67cea03" +checksum = "9afec9963e3d0994cac82455b2b3502b81a7f40f9a0d32181f7528d9f4b43e02" dependencies = [ "cfg-if 1.0.0", "js-sys", @@ -3906,9 +3930,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.87" +version = "0.2.88" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dee495e55982a3bd48105a7b947fd2a9b4a8ae3010041b9e0faab3f9cd028f1d" +checksum = "5961017b3b08ad5f3fe39f1e79877f8ee7c23c5e5fd5eb80de95abc41f1f16b2" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -3916,9 +3940,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.87" +version = "0.2.88" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" +checksum = "c5353b8dab669f5e10f5bd76df26a9360c748f054f862ff5f3f8aae0c7fb3907" dependencies = [ "proc-macro2", "quote", @@ -3929,9 +3953,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.87" +version = "0.2.88" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca6ad05a4870b2bf5fe995117d3728437bd27d7cd5f06f13c17443ef369775a1" +checksum = "0d046c5d029ba91a1ed14da14dca44b68bf2f124cfbaf741c54151fdb3e0750b" [[package]] name = "wasm-streams" @@ -3963,9 +3987,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.64" +version = "0.3.65" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b85cbef8c220a6abc02aefd892dfc0fc23afb1c6a426316ec33253a3877249b" +checksum = "5db499c5f66323272151db0e666cd34f78617522fb0c1604d31a27c50c206a85" dependencies = [ "js-sys", "wasm-bindgen", @@ -4217,18 +4241,18 @@ dependencies = [ [[package]] name = "zerocopy" -version = "0.7.20" +version = "0.7.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd66a62464e3ffd4e37bd09950c2b9dd6c4f8767380fabba0d523f9a775bc85a" +checksum = "8cd369a67c0edfef15010f980c3cbe45d7f651deac2cd67ce097cd801de16557" dependencies = [ "zerocopy-derive", ] [[package]] name = "zerocopy-derive" -version = "0.7.20" +version = "0.7.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "255c4596d41e6916ced49cfafea18727b24d67878fa180ddfd69b9df34fd1726" +checksum = "c2f140bda219a26ccc0cdb03dba58af72590c53b22642577d88a927bc5c87d6b" dependencies = [ "proc-macro2", "quote", diff --git a/crates/puffin-cli/src/commands/reporters.rs b/crates/puffin-cli/src/commands/reporters.rs index 54235770d..8e195293d 100644 --- a/crates/puffin-cli/src/commands/reporters.rs +++ b/crates/puffin-cli/src/commands/reporters.rs @@ -1,8 +1,13 @@ +use colored::Colorize; +use std::sync::{Arc, Mutex}; use std::time::Duration; -use indicatif::{ProgressBar, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use url::Url; -use puffin_distribution::{CachedDistribution, RemoteDistribution, VersionOrUrl}; +use puffin_distribution::{ + CachedDistribution, RemoteDistribution, RemoteDistributionRef, VersionOrUrl, +}; use puffin_normalize::ExtraName; use puffin_normalize::PackageName; @@ -185,12 +190,17 @@ impl puffin_installer::BuildReporter for BuildReporter { #[derive(Debug)] pub(crate) struct ResolverReporter { + printer: Printer, + multi_progress: MultiProgress, progress: ProgressBar, + bars: Arc>>, } impl From for ResolverReporter { fn from(printer: Printer) -> Self { - let progress = ProgressBar::with_draw_target(None, printer.target()); + let multi_progress = MultiProgress::with_draw_target(printer.target()); + + let progress = multi_progress.add(ProgressBar::with_draw_target(None, printer.target())); progress.enable_steady_tick(Duration::from_millis(200)); progress.set_style( ProgressStyle::with_template("{spinner:.white} {wide_msg:.dim}") @@ -198,7 +208,13 @@ impl From for ResolverReporter { .tick_strings(&["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]), ); progress.set_message("Resolving dependencies..."); - Self { progress } + + Self { + printer, + multi_progress, + progress, + bars: Arc::new(Mutex::new(Vec::new())), + } } } @@ -230,4 +246,81 @@ impl puffin_resolver::ResolverReporter for ResolverReporter { fn on_complete(&self) { self.progress.finish_and_clear(); } + + fn on_build_start(&self, distribution: &RemoteDistributionRef<'_>) -> usize { + let progress = self.multi_progress.insert_before( + &self.progress, + ProgressBar::with_draw_target(None, self.printer.target()), + ); + + progress.set_style(ProgressStyle::with_template("{wide_msg}").unwrap()); + progress.set_message(format!( + "{} {}", + "Building".bold().green(), + distribution.to_color_string() + )); + + let mut bars = self.bars.lock().unwrap(); + bars.push(progress); + bars.len() - 1 + } + + fn on_build_complete(&self, distribution: &RemoteDistributionRef<'_>, index: usize) { + let mut bars = self.bars.lock().unwrap(); + let progress = bars.remove(index); + progress.finish_with_message(format!( + "{} {}", + "Built".bold().green(), + distribution.to_color_string() + )); + } + + fn on_checkout_start(&self, url: &Url, rev: &str) -> usize { + let progress = self.multi_progress.insert_before( + &self.progress, + ProgressBar::with_draw_target(None, self.printer.target()), + ); + + progress.set_style(ProgressStyle::with_template("{wide_msg}").unwrap()); + progress.set_message(format!( + "{} {} ({})", + "Updating".bold().green(), + url, + rev.dimmed() + )); + progress.finish(); + + let mut bars = self.bars.lock().unwrap(); + bars.push(progress); + bars.len() - 1 + } + + fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize) { + let mut bars = self.bars.lock().unwrap(); + let progress = bars.remove(index); + progress.finish_with_message(format!( + "{} {} ({})", + "Updated".bold().green(), + url, + rev.dimmed() + )); + } +} + +/// Like [`std::fmt::Display`], but with colors. +trait ColorDisplay { + fn to_color_string(&self) -> String; +} + +impl ColorDisplay for &RemoteDistributionRef<'_> { + fn to_color_string(&self) -> String { + match self { + RemoteDistributionRef::Registry(name, version, _file) => { + format!("{}{}", name, format!("=={version}").dimmed()) + } + RemoteDistributionRef::Url(name, url) => { + format!("{}{}", name, format!(" @ {url}").dimmed()) + } + } + } } diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index e2ee82789..22e861b27 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -1136,7 +1136,7 @@ fn conflicting_repeated_url_dependency_version_match() -> Result<()> { let requirements_in = temp_dir.child("requirements.in"); requirements_in.touch()?; - requirements_in.write_str("werkzeug @ git+https://github.com/pallets/werkzeug.git@2.0.0 \nwerkzeug @ https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl")?; + requirements_in.write_str("werkzeug @ git+https://github.com/pallets/werkzeug.git@2.0.0\nwerkzeug @ https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl")?; insta::with_settings!({ filters => INSTA_FILTERS.to_vec() diff --git a/crates/puffin-git/src/git.rs b/crates/puffin-git/src/git.rs index 007fda856..51702969a 100644 --- a/crates/puffin-git/src/git.rs +++ b/crates/puffin-git/src/git.rs @@ -56,6 +56,19 @@ impl GitReference { Self::BranchOrTag(rev.to_owned()) } } + + /// Views the short ID as a `str`. + pub(crate) fn as_str(&self) -> &str { + match self { + GitReference::Branch(rev) + | GitReference::Tag(rev) + | GitReference::BranchOrTag(rev) + | GitReference::FullCommit(rev) + | GitReference::ShortCommit(rev) + | GitReference::Ref(rev) => rev, + GitReference::DefaultBranch => "HEAD", + } + } } /// A short abbreviated OID. diff --git a/crates/puffin-git/src/lib.rs b/crates/puffin-git/src/lib.rs index 0a72202d0..8638da3d7 100644 --- a/crates/puffin-git/src/lib.rs +++ b/crates/puffin-git/src/lib.rs @@ -1,7 +1,7 @@ use url::Url; use crate::git::GitReference; -pub use crate::source::GitSource; +pub use crate::source::{GitSource, Reporter}; mod git; mod source; diff --git a/crates/puffin-git/src/source.rs b/crates/puffin-git/src/source.rs index 6f4fae711..595abbf2d 100644 --- a/crates/puffin-git/src/source.rs +++ b/crates/puffin-git/src/source.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use anyhow::Result; use reqwest::Client; use tracing::debug; +use url::Url; use puffin_cache::{digest, RepositoryUrl}; @@ -22,44 +23,65 @@ pub struct GitSource { strategy: FetchStrategy, /// The path to the Git source database. cache: PathBuf, + /// The reporter to use for this source. + reporter: Option>, } impl GitSource { + /// Initialize a new Git source. pub fn new(git: Git, cache: impl Into) -> Self { Self { git, client: Client::new(), strategy: FetchStrategy::Libgit2, cache: cache.into(), + reporter: None, } } + /// Set the [`Reporter`] to use for this `GIt` source. + #[must_use] + pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self { + Self { + reporter: Some(Box::new(reporter)), + ..self + } + } + + /// Fetch the underlying Git repository at the given revision. pub fn fetch(self) -> Result { // The path to the repo, within the Git database. let ident = digest(&RepositoryUrl::new(&self.git.url)); let db_path = self.cache.join("db").join(&ident); let remote = GitRemote::new(&self.git.url); - let (db, actual_rev) = match (self.git.precise, remote.db_at(&db_path).ok()) { + let (db, actual_rev, task) = match (self.git.precise, remote.db_at(&db_path).ok()) { // If we have a locked revision, and we have a preexisting database // which has that revision, then no update needs to happen. - (Some(rev), Some(db)) if db.contains(rev) => (db, rev), + (Some(rev), Some(db)) if db.contains(rev) => (db, rev, None), // ... otherwise we use this state to update the git database. Note // that we still check for being offline here, for example in the // situation that we have a locked revision but the database // doesn't have it. (locked_rev, db) => { - debug!("Updating Git source: `{:?}`", remote); + debug!("Updating git source `{:?}`", self.git.url); - remote.checkout( + // Report the checkout operation to the reporter. + let task = self.reporter.as_ref().map(|reporter| { + reporter.on_checkout_start(remote.url(), self.git.reference.as_str()) + }); + + let (db, actual_rev) = remote.checkout( &db_path, db, &self.git.reference, locked_rev, self.strategy, &self.client, - )? + )?; + + (db, actual_rev, task) } }; @@ -77,6 +99,13 @@ impl GitSource { .join(short_id.as_str()); db.copy_to(actual_rev, &checkout_path, self.strategy, &self.client)?; + // Report the checkout operation to the reporter. + if let Some(task) = task { + if let Some(reporter) = self.reporter.as_ref() { + reporter.on_checkout_complete(remote.url(), short_id.as_str(), task); + } + } + Ok(Fetch { git: self.git.with_precise(actual_rev), path: checkout_path, @@ -102,3 +131,11 @@ impl From for PathBuf { fetch.path } } + +pub trait Reporter: Send + Sync { + /// Callback to invoke when a repository checkout begins. + fn on_checkout_start(&self, url: &Url, rev: &str) -> usize; + + /// Callback to invoke when a repository checkout completes. + fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize); +} diff --git a/crates/puffin-resolver/src/distribution/mod.rs b/crates/puffin-resolver/src/distribution/mod.rs index 7f12a10ba..458c92dd5 100644 --- a/crates/puffin-resolver/src/distribution/mod.rs +++ b/crates/puffin-resolver/src/distribution/mod.rs @@ -1,4 +1,6 @@ -pub(crate) use source_distribution::SourceDistributionFetcher; +pub(crate) use source_distribution::{ + Reporter as SourceDistributionReporter, SourceDistributionFetcher, +}; pub(crate) use wheel::WheelFetcher; mod cached_wheel; diff --git a/crates/puffin-resolver/src/distribution/source_distribution.rs b/crates/puffin-resolver/src/distribution/source_distribution.rs index e6b5d2c92..89c259b0f 100644 --- a/crates/puffin-resolver/src/distribution/source_distribution.rs +++ b/crates/puffin-resolver/src/distribution/source_distribution.rs @@ -3,9 +3,11 @@ //! TODO(charlie): Unify with `crates/puffin-installer/src/sdist_builder.rs`. use std::str::FromStr; +use std::sync::Arc; use anyhow::Result; use fs_err::tokio as fs; + use tempfile::tempdir_in; use tokio_util::compat::FuturesAsyncReadCompatExt; use tracing::debug; @@ -27,12 +29,27 @@ const BUILT_WHEELS_CACHE: &str = "built-wheels-v0"; const GIT_CACHE: &str = "git-v0"; /// Fetch and build a source distribution from a remote source, or from a local cache. -pub(crate) struct SourceDistributionFetcher<'a, T: BuildContext>(&'a T); +pub(crate) struct SourceDistributionFetcher<'a, T: BuildContext> { + build_context: &'a T, + reporter: Option>, +} impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { /// Initialize a [`SourceDistributionFetcher`] from a [`BuildContext`]. pub(crate) fn new(build_context: &'a T) -> Self { - Self(build_context) + Self { + build_context, + reporter: None, + } + } + + /// Set the [`Reporter`] to use for this source distribution fetcher. + #[must_use] + pub(crate) fn with_reporter(self, reporter: impl Reporter + 'static) -> Self { + Self { + reporter: Some(Arc::new(reporter)), + ..self + } } /// Read the [`Metadata21`] from a built source distribution, if it exists in the cache. @@ -41,10 +58,14 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { distribution: &RemoteDistributionRef<'_>, tags: &Tags, ) -> Result> { - CachedWheel::find_in_cache(distribution, tags, self.0.cache().join(BUILT_WHEELS_CACHE)) - .as_ref() - .map(CachedWheel::read_dist_info) - .transpose() + CachedWheel::find_in_cache( + distribution, + tags, + self.build_context.cache().join(BUILT_WHEELS_CACHE), + ) + .as_ref() + .map(CachedWheel::read_dist_info) + .transpose() } /// Download and build a source distribution, storing the built wheel in the cache. @@ -65,7 +86,7 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { let mut reader = tokio::io::BufReader::new(reader.compat()); // Download the source distribution. - let temp_dir = tempdir_in(self.0.cache())?.into_path(); + let temp_dir = tempdir_in(self.build_context.cache())?.into_path(); let sdist_filename = distribution.filename()?; let sdist_file = temp_dir.join(sdist_filename.as_ref()); let mut writer = tokio::fs::File::create(&sdist_file).await?; @@ -83,7 +104,7 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { let mut reader = tokio::io::BufReader::new(reader.compat()); // Download the source distribution. - let temp_dir = tempdir_in(self.0.cache())?.into_path(); + let temp_dir = tempdir_in(self.build_context.cache())?.into_path(); let sdist_filename = distribution.filename()?; let sdist_file = temp_dir.join(sdist_filename.as_ref()); let mut writer = tokio::fs::File::create(&sdist_file).await?; @@ -94,8 +115,12 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { Source::Git(git, subdirectory) => { debug!("Fetching source distribution from Git: {git}"); - let git_dir = self.0.cache().join(GIT_CACHE); - let source = GitSource::new(git, git_dir); + let git_dir = self.build_context.cache().join(GIT_CACHE); + let source = if let Some(reporter) = &self.reporter { + GitSource::new(git, git_dir).with_reporter(Facade::from(reporter.clone())) + } else { + GitSource::new(git, git_dir) + }; let sdist_file = tokio::task::spawn_blocking(move || source.fetch()) .await?? .into(); @@ -106,7 +131,7 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { // Create a directory for the wheel. let wheel_dir = self - .0 + .build_context .cache() .join(BUILT_WHEELS_CACHE) .join(distribution.id()); @@ -114,7 +139,7 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { // Build the wheel. let disk_filename = self - .0 + .build_context .build_source(&sdist_file, subdirectory.as_deref(), &wheel_dir) .await?; @@ -153,8 +178,12 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { // Fetch the precise SHA of the Git reference (which could be a branch, a tag, a partial // commit, etc.). - let dir = self.0.cache().join(GIT_CACHE); - let source = GitSource::new(git, dir); + let git_dir = self.build_context.cache().join(GIT_CACHE); + let source = if let Some(reporter) = &self.reporter { + GitSource::new(git, git_dir).with_reporter(Facade::from(reporter.clone())) + } else { + GitSource::new(git, git_dir) + }; let precise = tokio::task::spawn_blocking(move || source.fetch()).await??; let git = Git::from(precise); @@ -163,3 +192,32 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> { Ok(Some(source.into())) } } + +pub(crate) trait Reporter: Send + Sync { + /// Callback to invoke when a repository checkout begins. + fn on_checkout_start(&self, url: &Url, rev: &str) -> usize; + + /// Callback to invoke when a repository checkout completes. + fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize); +} + +/// A facade for converting from [`Reporter`] to [`puffin_git::Reporter`]. +struct Facade { + reporter: Arc, +} + +impl From> for Facade { + fn from(reporter: Arc) -> Self { + Self { reporter } + } +} + +impl puffin_git::Reporter for Facade { + fn on_checkout_start(&self, url: &Url, rev: &str) -> usize { + self.reporter.on_checkout_start(url, rev) + } + + fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize) { + self.reporter.on_checkout_complete(url, rev, index); + } +} diff --git a/crates/puffin-resolver/src/lib.rs b/crates/puffin-resolver/src/lib.rs index f936b6614..b2fcc879f 100644 --- a/crates/puffin-resolver/src/lib.rs +++ b/crates/puffin-resolver/src/lib.rs @@ -5,7 +5,7 @@ pub use prerelease_mode::PreReleaseMode; pub use pubgrub::ResolutionFailureReporter; pub use resolution::Graph; pub use resolution_mode::ResolutionMode; -pub use resolver::{Reporter as ResolverReporter, Resolver}; +pub use resolver::{BuildId, Reporter as ResolverReporter, Resolver}; mod candidate_selector; mod distribution; diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 708e549c0..d1af6e55a 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -29,7 +29,7 @@ use puffin_traits::BuildContext; use pypi_types::{File, Metadata21, SimpleJson}; use crate::candidate_selector::CandidateSelector; -use crate::distribution::{SourceDistributionFetcher, WheelFetcher}; +use crate::distribution::{SourceDistributionFetcher, SourceDistributionReporter, WheelFetcher}; use crate::error::ResolveError; use crate::file::{DistributionFile, SdistFile, WheelFile}; use crate::manifest::Manifest; @@ -50,7 +50,7 @@ pub struct Resolver<'a, Context: BuildContext + Sync> { index: Arc, locks: Arc, build_context: &'a Context, - reporter: Option>, + reporter: Option>, } impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { @@ -93,7 +93,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { #[must_use] pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self { Self { - reporter: Some(Box::new(reporter)), + reporter: Some(Arc::new(reporter)), ..self } } @@ -684,7 +684,14 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { let lock = self.locks.acquire(&url).await; let _guard = lock.lock().await; - let fetcher = SourceDistributionFetcher::new(self.build_context); + let fetcher = if let Some(reporter) = &self.reporter { + SourceDistributionFetcher::new(self.build_context).with_reporter(Facade { + reporter: reporter.clone(), + }) + } else { + SourceDistributionFetcher::new(self.build_context) + }; + let precise = fetcher .precise(&RemoteDistributionRef::from_url(&package_name, &url)) .await @@ -698,6 +705,11 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { precise.as_ref().unwrap_or(&url), ); + let task = self + .reporter + .as_ref() + .map(|reporter| reporter.on_build_start(&distribution)); + let metadata = match fetcher.find_dist_info(&distribution, self.tags) { Ok(Some(metadata)) => { debug!("Found source distribution metadata in cache: {url}"); @@ -734,6 +746,12 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { }); } + if let Some(task) = task { + if let Some(reporter) = self.reporter.as_ref() { + reporter.on_build_complete(&distribution, task); + } + } + Ok(Response::SdistUrl(url, precise, metadata)) } // Fetch wheel metadata from a remote URL. @@ -807,12 +825,41 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { } } +pub type BuildId = usize; + pub trait Reporter: Send + Sync { /// Callback to invoke when a dependency is resolved. fn on_progress(&self, name: &PackageName, extra: Option<&ExtraName>, version: VersionOrUrl); /// Callback to invoke when the resolution is complete. fn on_complete(&self); + + /// Callback to invoke when a source distribution build is kicked off. + fn on_build_start(&self, distribution: &RemoteDistributionRef<'_>) -> usize; + + /// Callback to invoke when a source distribution build is complete. + fn on_build_complete(&self, distribution: &RemoteDistributionRef<'_>, id: usize); + + /// Callback to invoke when a repository checkout begins. + fn on_checkout_start(&self, url: &Url, rev: &str) -> usize; + + /// Callback to invoke when a repository checkout completes. + fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize); +} + +/// A facade for converting from [`Reporter`] to [`puffin_git::Reporter`]. +struct Facade { + reporter: Arc, +} + +impl SourceDistributionReporter for Facade { + fn on_checkout_start(&self, url: &Url, rev: &str) -> usize { + self.reporter.on_checkout_start(url, rev) + } + + fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize) { + self.reporter.on_checkout_complete(url, rev, index); + } } /// Fetch the metadata for an item diff --git a/scripts/benchmarks/requirements.in b/scripts/benchmarks/requirements.in index 87f6e1d6f..7173573c4 100644 --- a/scripts/benchmarks/requirements.in +++ b/scripts/benchmarks/requirements.in @@ -5,6 +5,7 @@ packaging>=23.1 pygls>=1.0.1 lsprotocol>=2023.0.0a1 ruff>=0.0.274 +flask @ git+https://github.com/pallets/flask.git@d92b64a typing_extensions scipy numpy @@ -28,3 +29,4 @@ typer pydantic uvicorn traitlets +