From 7665afa771c26096f06e9816663326001b4fb945 Mon Sep 17 00:00:00 2001 From: Wackyator Date: Thu, 7 Sep 2023 19:51:21 +0530 Subject: [PATCH 1/4] fix: make FetchRepoDataOptions clonable --- crates/rattler-bin/src/commands/create.rs | 11 +++--- .../rattler_repodata_gateway/src/fetch/mod.rs | 36 +++++++++++-------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/crates/rattler-bin/src/commands/create.rs b/crates/rattler-bin/src/commands/create.rs index 5cb41882a..4c16a9983 100644 --- a/crates/rattler-bin/src/commands/create.rs +++ b/crates/rattler-bin/src/commands/create.rs @@ -28,6 +28,7 @@ use std::{ io::ErrorKind, path::{Path, PathBuf}, str::FromStr, + sync::Arc, time::Duration, }; use tokio::task::JoinHandle; @@ -589,10 +590,12 @@ async fn fetch_repo_data_records_with_progress( client, repodata_cache, FetchRepoDataOptions { - download_progress: Some(Box::new(move |DownloadProgress { total, bytes }| { - download_progress_progress_bar.set_length(total.unwrap_or(bytes)); - download_progress_progress_bar.set_position(bytes); - })), + download_progress: Some(Arc::new(Box::new( + move |DownloadProgress { total, bytes }| { + download_progress_progress_bar.set_length(total.unwrap_or(bytes)); + download_progress_progress_bar.set_position(bytes); + }, + ))), ..Default::default() }, ) diff --git a/crates/rattler_repodata_gateway/src/fetch/mod.rs b/crates/rattler_repodata_gateway/src/fetch/mod.rs index 22cf7a985..9fc2049b7 100644 --- a/crates/rattler_repodata_gateway/src/fetch/mod.rs +++ b/crates/rattler_repodata_gateway/src/fetch/mod.rs @@ -14,6 +14,7 @@ use reqwest::{ use std::{ io::ErrorKind, path::{Path, PathBuf}, + sync::Arc, time::SystemTime, }; use tempfile::NamedTempFile; @@ -24,6 +25,9 @@ use url::Url; mod cache; pub mod jlap; +/// Type alias for function to report progress while downloading repodata +pub type ProgressFunc = Arc>; + /// RepoData could not be found for given channel and platform #[derive(Debug, thiserror::Error)] pub enum RepoDataNotFoundError { @@ -136,14 +140,14 @@ impl Variant { } /// Additional knobs that allow you to tweak the behavior of [`fetch_repo_data`]. -#[derive(Default)] +#[derive(Default, Clone)] pub struct FetchRepoDataOptions { /// How to use the cache. By default it will cache and reuse downloaded repodata.json (if the /// server allows it). pub cache_action: CacheAction, /// A function that is called during downloading of the repodata.json to report progress. - pub download_progress: Option>, + pub download_progress: Option, /// Determines which variant to download. See [`Variant`] for more information. pub variant: Variant, @@ -578,16 +582,18 @@ async fn stream_and_decode_to_file( response: Response, content_encoding: Encoding, temp_dir: &Path, - mut progress: Option>, + mut progress: Option, ) -> Result<(NamedTempFile, blake2::digest::Output), FetchRepoDataError> { // Determine the length of the response in bytes and notify the listener that a download is // starting. The response may be compressed. Decompression happens below. let content_size = response.content_length(); - if let Some(progress) = progress.as_mut() { - progress(DownloadProgress { - bytes: 0, - total: content_size, - }) + if let Some(progress) = &mut progress { + if let Some(progress) = ProgressFunc::get_mut(progress) { + progress(DownloadProgress { + bytes: 0, + total: content_size, + }) + } } // Determine the encoding of the response @@ -605,11 +611,13 @@ async fn stream_and_decode_to_file( let total_bytes_mut = &mut total_bytes; let bytes_stream = bytes_stream.inspect_ok(move |bytes| { *total_bytes_mut += bytes.len() as u64; - if let Some(progress) = progress.as_mut() { - progress(DownloadProgress { - bytes: *total_bytes_mut, - total: content_size, - }) + if let Some(progress) = &mut progress { + if let Some(progress) = ProgressFunc::get_mut(progress) { + progress(DownloadProgress { + bytes: *total_bytes_mut, + total: content_size, + }) + } } }); @@ -1367,7 +1375,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), FetchRepoDataOptions { - download_progress: Some(Box::new(download_progress)), + download_progress: Some(Arc::new(Box::new(download_progress))), ..Default::default() }, ) From c1a72b7a755a53c7551abd17ef0ca8d5f4a577e0 Mon Sep 17 00:00:00 2001 From: Wackyator Date: Fri, 8 Sep 2023 16:46:02 +0530 Subject: [PATCH 2/4] fix: remove download_progress field from FetchRepoDataOptions --- crates/rattler-bin/src/commands/create.rs | 15 ++---- .../rattler_repodata_gateway/src/fetch/mod.rs | 54 ++++++++++++------- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/crates/rattler-bin/src/commands/create.rs b/crates/rattler-bin/src/commands/create.rs index 4c16a9983..292ed3e89 100644 --- a/crates/rattler-bin/src/commands/create.rs +++ b/crates/rattler-bin/src/commands/create.rs @@ -28,7 +28,6 @@ use std::{ io::ErrorKind, path::{Path, PathBuf}, str::FromStr, - sync::Arc, time::Duration, }; use tokio::task::JoinHandle; @@ -589,15 +588,11 @@ async fn fetch_repo_data_records_with_progress( channel.platform_url(platform), client, repodata_cache, - FetchRepoDataOptions { - download_progress: Some(Arc::new(Box::new( - move |DownloadProgress { total, bytes }| { - download_progress_progress_bar.set_length(total.unwrap_or(bytes)); - download_progress_progress_bar.set_position(bytes); - }, - ))), - ..Default::default() - }, + FetchRepoDataOptions::default(), + Some(Box::new(move |DownloadProgress { total, bytes }| { + download_progress_progress_bar.set_length(total.unwrap_or(bytes)); + download_progress_progress_bar.set_position(bytes); + })), ) .await; diff --git a/crates/rattler_repodata_gateway/src/fetch/mod.rs b/crates/rattler_repodata_gateway/src/fetch/mod.rs index 9fc2049b7..24a5bd07a 100644 --- a/crates/rattler_repodata_gateway/src/fetch/mod.rs +++ b/crates/rattler_repodata_gateway/src/fetch/mod.rs @@ -14,7 +14,6 @@ use reqwest::{ use std::{ io::ErrorKind, path::{Path, PathBuf}, - sync::Arc, time::SystemTime, }; use tempfile::NamedTempFile; @@ -26,7 +25,7 @@ mod cache; pub mod jlap; /// Type alias for function to report progress while downloading repodata -pub type ProgressFunc = Arc>; +pub type ProgressFunc = Box; /// RepoData could not be found for given channel and platform #[derive(Debug, thiserror::Error)] @@ -146,13 +145,17 @@ pub struct FetchRepoDataOptions { /// server allows it). pub cache_action: CacheAction, - /// A function that is called during downloading of the repodata.json to report progress. - pub download_progress: Option, - /// Determines which variant to download. See [`Variant`] for more information. pub variant: Variant, } +impl FetchRepoDataOptions { + /// A function that is called during downloading of the repodata.json to report progress. + pub fn report(&self, progress_func: &mut ProgressFunc, progress: DownloadProgress) { + progress_func(progress); + } +} + /// A struct that provides information about download progress. #[derive(Debug, Clone)] pub struct DownloadProgress { @@ -276,6 +279,7 @@ pub async fn fetch_repo_data( client: AuthenticatedClient, cache_path: &Path, options: FetchRepoDataOptions, + progress: Option, ) -> Result { let subdir_url = normalize_subdir_url(subdir_url); @@ -521,7 +525,8 @@ pub async fn fetch_repo_data( Encoding::Passthrough }, cache_path, - options.download_progress, + &options, + progress, ) .await?; @@ -582,18 +587,20 @@ async fn stream_and_decode_to_file( response: Response, content_encoding: Encoding, temp_dir: &Path, - mut progress: Option, + options: &FetchRepoDataOptions, + mut progress_func: Option, ) -> Result<(NamedTempFile, blake2::digest::Output), FetchRepoDataError> { // Determine the length of the response in bytes and notify the listener that a download is // starting. The response may be compressed. Decompression happens below. let content_size = response.content_length(); - if let Some(progress) = &mut progress { - if let Some(progress) = ProgressFunc::get_mut(progress) { - progress(DownloadProgress { + if let Some(progress_func) = progress_func.as_mut() { + options.report( + progress_func, + DownloadProgress { bytes: 0, total: content_size, - }) - } + }, + ); } // Determine the encoding of the response @@ -611,13 +618,14 @@ async fn stream_and_decode_to_file( let total_bytes_mut = &mut total_bytes; let bytes_stream = bytes_stream.inspect_ok(move |bytes| { *total_bytes_mut += bytes.len() as u64; - if let Some(progress) = &mut progress { - if let Some(progress) = ProgressFunc::get_mut(progress) { - progress(DownloadProgress { + if let Some(progress_func) = progress_func.as_mut() { + options.report( + progress_func, + DownloadProgress { bytes: *total_bytes_mut, total: content_size, - }) - } + }, + ) } }); @@ -1110,6 +1118,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1139,6 +1148,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1151,6 +1161,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1174,6 +1185,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1202,6 +1214,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1243,6 +1256,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1291,6 +1305,7 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1343,6 +1358,7 @@ mod test { authenticated_client, cache_dir.path(), Default::default(), + None, ) .await .unwrap(); @@ -1375,9 +1391,9 @@ mod test { AuthenticatedClient::default(), cache_dir.path(), FetchRepoDataOptions { - download_progress: Some(Arc::new(Box::new(download_progress))), ..Default::default() }, + Some(Box::new(download_progress)), ) .await .unwrap(); @@ -1402,6 +1418,7 @@ mod test { FetchRepoDataOptions { ..Default::default() }, + None, ) .await; @@ -1425,6 +1442,7 @@ mod test { FetchRepoDataOptions { ..Default::default() }, + None, ) .await; From 1eed78cca42a447dcedaebe26b8b9f1f9c52eca3 Mon Sep 17 00:00:00 2001 From: Wackyator Date: Fri, 8 Sep 2023 17:33:51 +0530 Subject: [PATCH 3/4] fix: remove report method and inline it --- .../rattler_repodata_gateway/src/fetch/mod.rs | 31 +++++-------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/crates/rattler_repodata_gateway/src/fetch/mod.rs b/crates/rattler_repodata_gateway/src/fetch/mod.rs index 24a5bd07a..d8002ccb2 100644 --- a/crates/rattler_repodata_gateway/src/fetch/mod.rs +++ b/crates/rattler_repodata_gateway/src/fetch/mod.rs @@ -149,13 +149,6 @@ pub struct FetchRepoDataOptions { pub variant: Variant, } -impl FetchRepoDataOptions { - /// A function that is called during downloading of the repodata.json to report progress. - pub fn report(&self, progress_func: &mut ProgressFunc, progress: DownloadProgress) { - progress_func(progress); - } -} - /// A struct that provides information about download progress. #[derive(Debug, Clone)] pub struct DownloadProgress { @@ -525,7 +518,6 @@ pub async fn fetch_repo_data( Encoding::Passthrough }, cache_path, - &options, progress, ) .await?; @@ -587,20 +579,16 @@ async fn stream_and_decode_to_file( response: Response, content_encoding: Encoding, temp_dir: &Path, - options: &FetchRepoDataOptions, mut progress_func: Option, ) -> Result<(NamedTempFile, blake2::digest::Output), FetchRepoDataError> { // Determine the length of the response in bytes and notify the listener that a download is // starting. The response may be compressed. Decompression happens below. let content_size = response.content_length(); if let Some(progress_func) = progress_func.as_mut() { - options.report( - progress_func, - DownloadProgress { - bytes: 0, - total: content_size, - }, - ); + progress_func(DownloadProgress { + bytes: 0, + total: content_size, + }); } // Determine the encoding of the response @@ -619,13 +607,10 @@ async fn stream_and_decode_to_file( let bytes_stream = bytes_stream.inspect_ok(move |bytes| { *total_bytes_mut += bytes.len() as u64; if let Some(progress_func) = progress_func.as_mut() { - options.report( - progress_func, - DownloadProgress { - bytes: *total_bytes_mut, - total: content_size, - }, - ) + progress_func(DownloadProgress { + bytes: *total_bytes_mut, + total: content_size, + }); } }); From cc2f172776ee6b3aeebbe236839c73e97c4d3eb0 Mon Sep 17 00:00:00 2001 From: Wackyator Date: Fri, 8 Sep 2023 17:37:05 +0530 Subject: [PATCH 4/4] test: fix rattler_repodata_gateway doc test --- crates/rattler_repodata_gateway/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/rattler_repodata_gateway/src/lib.rs b/crates/rattler_repodata_gateway/src/lib.rs index bd4953ff3..86db18582 100644 --- a/crates/rattler_repodata_gateway/src/lib.rs +++ b/crates/rattler_repodata_gateway/src/lib.rs @@ -41,7 +41,8 @@ //! repodata_url, //! client, //! cache, -//! fetch::FetchRepoDataOptions { ..Default::default() } +//! fetch::FetchRepoDataOptions { ..Default::default() }, +//! None, //! ).await; //! //! let result = match result {