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

Use dynamic dispatch to simplify reporters #10086

Merged
merged 1 commit into from
Jan 6, 2025
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
7 changes: 3 additions & 4 deletions crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,10 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {

/// Set the [`Reporter`] to use for the [`DistributionDatabase`].
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
let reporter = Arc::new(reporter);
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
reporter: Some(reporter.clone()),
builder: self.builder.with_reporter(reporter),
builder: self.builder.with_reporter(reporter.clone()),
reporter: Some(reporter),
..self
}
}
Expand Down
17 changes: 10 additions & 7 deletions crates/uv-distribution/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,18 @@ pub trait Reporter: Send + Sync {
fn on_download_complete(&self, name: &PackageName, id: usize);
}

/// A facade for converting from [`Reporter`] to [`uv_git::Reporter`].
pub(crate) struct Facade {
reporter: Arc<dyn Reporter>,
impl dyn Reporter {
/// Converts this reporter to a [`uv_git::Reporter`].
pub(crate) fn into_git_reporter(self: Arc<dyn Reporter>) -> Arc<dyn uv_git::Reporter> {
Arc::new(Facade {
reporter: self.clone(),
})
}
}

impl From<Arc<dyn Reporter>> for Facade {
fn from(reporter: Arc<dyn Reporter>) -> Self {
Self { reporter }
}
/// A facade for converting from [`Reporter`] to [`uv_git::Reporter`].
struct Facade {
reporter: Arc<dyn Reporter>,
}

impl uv_git::Reporter for Facade {
Expand Down
25 changes: 20 additions & 5 deletions crates/uv-distribution/src/source/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
//! Fetch and build source distributions from remote sources.

// This is to squash warnings about `|r| r.into_git_reporter()`. Clippy wants
// me to eta-reduce that and write it as
// `<(dyn reporter::Reporter + 'static)>::into_git_reporter`
// instead. But that's a monster. On the other hand, applying this suppression
// instruction more granularly is annoying. So we just slap it on the module
// for now. ---AG
#![allow(clippy::redundant_closure_for_method_calls)]

use std::borrow::Cow;
use std::ops::Bound;
use std::path::{Path, PathBuf};
Expand All @@ -9,7 +17,6 @@ use std::sync::Arc;
use crate::distribution_database::ManagedClient;
use crate::error::Error;
use crate::metadata::{ArchiveMetadata, GitWorkspaceMember, Metadata};
use crate::reporter::Facade;
use crate::source::built_wheel_metadata::BuiltWheelMetadata;
use crate::source::revision::Revision;
use crate::{Reporter, RequiresDist};
Expand Down Expand Up @@ -1330,7 +1337,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
resource.git,
client.unmanaged.uncached_client(resource.url).clone(),
self.build_context.cache().bucket(CacheBucket::Git),
self.reporter.clone().map(Facade::from),
self.reporter
.clone()
.map(|reporter| reporter.into_git_reporter()),
)
.await?;

Expand Down Expand Up @@ -1427,7 +1436,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
resource.git,
client.unmanaged.uncached_client(resource.url).clone(),
self.build_context.cache().bucket(CacheBucket::Git),
self.reporter.clone().map(Facade::from),
self.reporter
.clone()
.map(|reporter| reporter.into_git_reporter()),
)
.await?;

Expand Down Expand Up @@ -1603,7 +1614,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
&source.git,
client.unmanaged.uncached_client(&source.url).clone(),
self.build_context.cache().bucket(CacheBucket::Git),
self.reporter.clone().map(Facade::from),
self.reporter
.clone()
.map(|reporter| reporter.into_git_reporter()),
)
.await?;
}
Expand All @@ -1614,7 +1627,9 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
source.git,
client.unmanaged.uncached_client(source.url).clone(),
self.build_context.cache().bucket(CacheBucket::Git),
self.reporter.clone().map(Facade::from),
self.reporter
.clone()
.map(|reporter| reporter.into_git_reporter()),
)
.await?;
}
Expand Down
6 changes: 3 additions & 3 deletions crates/uv-git/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl GitResolver {
url: &GitUrl,
client: ClientWithMiddleware,
cache: PathBuf,
reporter: Option<impl Reporter + 'static>,
reporter: Option<Arc<dyn Reporter>>,
) -> Result<GitSha, GitResolverError> {
debug!("Resolving source distribution from Git: {url}");

Expand Down Expand Up @@ -88,7 +88,7 @@ impl GitResolver {
url: &GitUrl,
client: ClientWithMiddleware,
cache: PathBuf,
reporter: Option<impl Reporter + 'static>,
reporter: Option<Arc<dyn Reporter>>,
) -> Result<Fetch, GitResolverError> {
debug!("Fetching source distribution from Git: {url}");

Expand Down Expand Up @@ -138,7 +138,7 @@ impl GitResolver {
/// For example, given a Git dependency with a reference to a branch or tag, return a URL
/// with a precise reference to the current commit of that branch or tag.
///
/// This method takes into account various normalizations that are independent from the Git
/// This method takes into account various normalizations that are independent of the Git
/// layer. For example: removing `#subdirectory=pkg_dir`-like fragments, and removing `git+`
/// prefix kinds.
///
Expand Down
11 changes: 6 additions & 5 deletions crates/uv-git/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use std::borrow::Cow;
use std::path::{Path, PathBuf};
use std::sync::Arc;

use anyhow::Result;
use reqwest_middleware::ClientWithMiddleware;
Expand All @@ -24,11 +25,11 @@ pub struct GitSource {
/// The path to the Git source database.
cache: PathBuf,
/// The reporter to use for this source.
reporter: Option<Box<dyn Reporter>>,
reporter: Option<Arc<dyn Reporter>>,
}

impl GitSource {
/// Initialize a new Git source.
/// Initialize a [`GitSource`] with the given Git URL, HTTP client, and cache path.
pub fn new(
git: GitUrl,
client: impl Into<ClientWithMiddleware>,
Expand All @@ -42,11 +43,11 @@ impl GitSource {
}
}

/// Set the [`Reporter`] to use for this `GIt` source.
/// Set the [`Reporter`] to use for the [`GitSource`].
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
reporter: Some(Box::new(reporter)),
reporter: Some(reporter),
..self
}
}
Expand Down
17 changes: 9 additions & 8 deletions crates/uv-installer/src/installer.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
use std::convert;
use std::sync::{Arc, LazyLock};

use anyhow::{Context, Error, Result};
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
use std::convert;
use std::sync::LazyLock;
use tokio::sync::oneshot;
use tracing::instrument;
use uv_install_wheel::{linker::LinkMode, Layout};

use uv_cache::Cache;
use uv_configuration::RAYON_INITIALIZE;
use uv_distribution_types::CachedDist;
use uv_install_wheel::{linker::LinkMode, Layout};
use uv_python::PythonEnvironment;

pub struct Installer<'a> {
venv: &'a PythonEnvironment,
link_mode: LinkMode,
cache: Option<&'a Cache>,
reporter: Option<Box<dyn Reporter>>,
reporter: Option<Arc<dyn Reporter>>,
installer_name: Option<String>,
installer_metadata: bool,
}
Expand Down Expand Up @@ -50,9 +51,9 @@ impl<'a> Installer<'a> {

/// Set the [`Reporter`] to use for this installer.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
reporter: Some(Box::new(reporter)),
reporter: Some(reporter),
..self
}
}
Expand All @@ -66,7 +67,7 @@ impl<'a> Installer<'a> {
}
}

/// Set the whether to link Uv specific files in dist-info
/// Set whether to install uv-specifier files in the dist-info directory.
#[must_use]
pub fn with_installer_metadata(self, installer_metadata: bool) -> Self {
Self {
Expand Down Expand Up @@ -151,7 +152,7 @@ fn install(
layout: Layout,
installer_name: Option<String>,
link_mode: LinkMode,
reporter: Option<Box<dyn Reporter>>,
reporter: Option<Arc<dyn Reporter>>,
relocatable: bool,
installer_metadata: bool,
) -> Result<Vec<CachedDist>> {
Expand Down
28 changes: 17 additions & 11 deletions crates/uv-installer/src/preparer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,16 @@ impl<'a, Context: BuildContext> Preparer<'a, Context> {

/// Set the [`Reporter`] to use for operations.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
let reporter: Arc<dyn Reporter> = Arc::new(reporter);
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
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()),
database: self
.database
.with_reporter(reporter.clone().into_distribution_reporter()),
reporter: Some(reporter),
}
}

Expand Down Expand Up @@ -271,15 +272,20 @@ pub trait Reporter: Send + Sync {
fn on_checkout_complete(&self, url: &Url, rev: &str, index: usize);
}

/// A facade for converting from [`Reporter`] to [`uv_git::Reporter`].
struct Facade {
reporter: Arc<dyn Reporter>,
impl dyn Reporter {
/// Converts this reporter to a [`uv_distribution::Reporter`].
pub(crate) fn into_distribution_reporter(
self: Arc<dyn Reporter>,
) -> Arc<dyn uv_distribution::Reporter> {
Arc::new(Facade {
reporter: self.clone(),
})
}
}

impl From<Arc<dyn Reporter>> for Facade {
fn from(reporter: Arc<dyn Reporter>) -> Self {
Self { reporter }
}
/// A facade for converting from [`Reporter`] to [`uv_distribution::Reporter`].
struct Facade {
reporter: Arc<dyn Reporter>,
}

impl uv_distribution::Reporter for Facade {
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-requirements/src/extras.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl<'a, Context: BuildContext> ExtrasResolver<'a, Context> {

/// Set the [`Reporter`] to use for this resolver.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
database: self.database.with_reporter(reporter),
..self
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-requirements/src/lookahead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl<'a, Context: BuildContext> LookaheadResolver<'a, Context> {

/// Set the [`Reporter`] to use for this resolver.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
database: self.database.with_reporter(reporter),
..self
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-requirements/src/source_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> {

/// Set the [`Reporter`] to use for this resolver.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
database: self.database.with_reporter(reporter),
..self
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-requirements/src/unnamed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl<'a, Context: BuildContext> NamedRequirementsResolver<'a, Context> {

/// Set the [`Reporter`] to use for this resolver.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
database: self.database.with_reporter(reporter),
..self
Expand Down
9 changes: 4 additions & 5 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ pub use crate::resolver::provider::{
DefaultResolverProvider, MetadataResponse, PackageVersionsResult, ResolverProvider,
VersionsResponse, WheelMetadataResult,
};
use crate::resolver::reporter::Facade;
pub use crate::resolver::reporter::{BuildId, Reporter};
use crate::yanks::AllowedYanks;
use crate::{marker, DependencyMode, Exclusions, FlatIndex, Options, ResolutionMode, VersionMap};
Expand Down Expand Up @@ -243,15 +242,15 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>

/// Set the [`Reporter`] to use for this installer.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
let reporter = Arc::new(reporter);

pub fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
state: ResolverState {
reporter: Some(reporter.clone()),
..self.state
},
provider: self.provider.with_reporter(Facade { reporter }),
provider: self
.provider
.with_reporter(reporter.into_distribution_reporter()),
}
}

Expand Down
10 changes: 5 additions & 5 deletions crates/uv-resolver/src/resolver/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::future::Future;
use std::sync::Arc;

use uv_configuration::BuildOptions;
use uv_distribution::{ArchiveMetadata, DistributionDatabase};
use uv_distribution::{ArchiveMetadata, DistributionDatabase, Reporter};
use uv_distribution_types::{Dist, IndexCapabilities, IndexUrl, InstalledDist, RequestedDist};
use uv_normalize::PackageName;
use uv_pep440::{Version, VersionSpecifiers};
Expand Down Expand Up @@ -97,9 +97,9 @@ pub trait ResolverProvider {
dist: &'io InstalledDist,
) -> impl Future<Output = WheelMetadataResult> + 'io;

/// Set the [`uv_distribution::Reporter`] to use for this installer.
/// Set the [`Reporter`] to use for this installer.
#[must_use]
fn with_reporter(self, reporter: impl uv_distribution::Reporter + 'static) -> Self;
fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self;
}

/// The main IO backend for the resolver, which does cached requests network requests using the
Expand Down Expand Up @@ -273,9 +273,9 @@ impl<'a, Context: BuildContext> ResolverProvider for DefaultResolverProvider<'a,
}
}

/// Set the [`uv_distribution::Reporter`] to use for this installer.
/// Set the [`Reporter`] to use for this installer.
#[must_use]
fn with_reporter(self, reporter: impl uv_distribution::Reporter + 'static) -> Self {
fn with_reporter(self, reporter: Arc<dyn Reporter>) -> Self {
Self {
fetcher: self.fetcher.with_reporter(reporter),
..self
Expand Down
15 changes: 13 additions & 2 deletions crates/uv-resolver/src/resolver/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,20 @@ pub trait Reporter: Send + Sync {
fn on_checkout_complete(&self, url: &Url, rev: &str, id: usize);
}

impl dyn Reporter {
/// Converts this reporter to a [`uv_distribution::Reporter`].
pub(crate) fn into_distribution_reporter(
self: Arc<dyn Reporter>,
) -> Arc<dyn uv_distribution::Reporter> {
Arc::new(Facade {
reporter: self.clone(),
})
}
}

/// A facade for converting from [`Reporter`] to [`uv_distribution::Reporter`].
pub(crate) struct Facade {
pub(crate) reporter: Arc<dyn Reporter>,
struct Facade {
reporter: Arc<dyn Reporter>,
}

impl uv_distribution::Reporter for Facade {
Expand Down
Loading
Loading