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):

<img width="868" alt="Screen Shot 2023-11-06 at 11 28 47 PM"
src="9647a480-7be7-41e9-b1d3-69faefd054ae">

<img width="868" alt="Screen Shot 2023-11-06 at 11 28 51 PM"
src="6bc491aa-5b51-4b37-9ee1-257f1bc1c049">

Closes https://github.com/astral-sh/puffin/issues/287 although we can do
a lot more here.
This commit is contained in:
Charlie Marsh 2023-11-07 06:17:31 -08:00 committed by GitHub
parent 2c32bc5a86
commit b0286a8939
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 341 additions and 65 deletions

View file

@ -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;

View file

@ -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<Arc<dyn Reporter>>,
}
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<Option<Metadata21>> {
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<dyn Reporter>,
}
impl From<Arc<dyn Reporter>> for Facade {
fn from(reporter: Arc<dyn Reporter>) -> 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);
}
}

View file

@ -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;

View file

@ -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<Index>,
locks: Arc<Locks>,
build_context: &'a Context,
reporter: Option<Box<dyn Reporter>>,
reporter: Option<Arc<dyn Reporter>>,
}
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<dyn Reporter>,
}
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