From 4f851f8e6350aff8944339e8ac35a15b6fb7a4ef Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 19 Mar 2024 14:33:25 +0100 Subject: [PATCH 1/2] feat: wire up global config for jlap / zstd and bzip2 support in repodata --- docs/advanced/global_configuration.md | 8 ++++ src/cli/global/common.rs | 10 +++-- src/cli/search.rs | 11 ++++- src/config.rs | 43 ++++++++++++++----- src/lock_file/update.rs | 1 + src/repodata.rs | 32 +++++++++++--- .../pixi__config__tests__config_merge.snap | 11 +++++ tests/config/config_2.toml | 4 ++ 8 files changed, 98 insertions(+), 22 deletions(-) diff --git a/docs/advanced/global_configuration.md b/docs/advanced/global_configuration.md index e18fb49bf..b2e9a1bd2 100644 --- a/docs/advanced/global_configuration.md +++ b/docs/advanced/global_configuration.md @@ -55,6 +55,14 @@ authentication_override_file = "/path/to/your/override.json" "oci://ghcr.io/channel-mirrors/bioconda", "https://prefix.dev/bioconda", ] + +[repodata_options] +# disable fetching of jlap, bz2 or zstd repodata files. +# This should only be used for specific old versions of artifactory and other non-compliant +# servers. +disable_jlap = true # don't try to download repodata.jlap +disable_bzip2 = true # don't try to download repodata.json.bz2 +disable_zstd = true # don't try to download repodata.json.zst ``` ## Mirror configuration diff --git a/src/cli/global/common.rs b/src/cli/global/common.rs index a11f8d3cb..34a14eb6d 100644 --- a/src/cli/global/common.rs +++ b/src/cli/global/common.rs @@ -182,9 +182,13 @@ pub(super) async fn get_client_and_sparse_repodata( IndexMap<(Channel, Platform), SparseRepoData>, )> { let authenticated_client = build_reqwest_clients(Some(config)).1; - let platform_sparse_repodata = - repodata::fetch_sparse_repodata(channels, [Platform::current()], &authenticated_client) - .await?; + let platform_sparse_repodata = repodata::fetch_sparse_repodata( + channels, + [Platform::current()], + &authenticated_client, + Some(config), + ) + .await?; Ok((authenticated_client, platform_sparse_repodata)) } diff --git a/src/cli/search.rs b/src/cli/search.rs index a7394a112..371c3f39a 100644 --- a/src/cli/search.rs +++ b/src/cli/search.rs @@ -143,8 +143,15 @@ pub async fn execute(args: Args) -> miette::Result<()> { build_reqwest_clients(None).1 }; - let repo_data = - Arc::new(fetch_sparse_repodata(channels.iter(), [args.platform], &client).await?); + let repo_data = Arc::new( + fetch_sparse_repodata( + channels.iter(), + [args.platform], + &client, + project.as_ref().map(|p| p.config()), + ) + .await?, + ); // When package name filter contains * (wildcard), it will search and display a list of packages matching this filter if package_name_filter.contains('*') { diff --git a/src/config.rs b/src/config.rs index b519ac4f8..2245a8abc 100644 --- a/src/config.rs +++ b/src/config.rs @@ -10,6 +10,17 @@ use url::Url; use crate::consts; +/// If other is None, keep the value, otherwise assign the value of other to self. +macro_rules! assign_if_some { + ($self:expr, $other:expr, $($field:ident),+) => { + $( + if $other.$field.is_some() { + $self.$field = $other.$field.clone(); + } + )+ + } +} + /// Determines the default author based on the default git author. Both the name and the email /// address of the author are returned. pub fn get_default_author() -> Option<(String, String)> { @@ -88,6 +99,16 @@ pub struct ConfigCliPrompt { change_ps1: Option, } +#[derive(Clone, Default, Debug, Deserialize)] +pub struct RepodataConfig { + /// Disable JLAP compression for repodata. + pub disable_jlap: Option, + /// Disable bzip2 compression for repodata. + pub disable_bzip2: Option, + /// Disable zstd compression for repodata. + pub disable_zstd: Option, +} + #[derive(Clone, Default, Debug, Deserialize)] pub struct Config { #[serde(default)] @@ -113,6 +134,9 @@ pub struct Config { #[serde(skip)] pub channel_config: ChannelConfig, + + /// Configuration for repodata fetching. + pub repodata_config: Option, } impl From for Config { @@ -210,17 +234,14 @@ impl Config { self.default_channels = other.default_channels.clone(); } - if other.change_ps1.is_some() { - self.change_ps1 = other.change_ps1; - } - - if other.tls_no_verify.is_some() { - self.tls_no_verify = other.tls_no_verify; - } - - if other.authentication_override_file.is_some() { - self.authentication_override_file = other.authentication_override_file.clone(); - } + assign_if_some!( + self, + other, + change_ps1, + tls_no_verify, + authentication_override_file, + repodata_config + ); self.mirrors.extend(other.mirrors.clone()); diff --git a/src/lock_file/update.rs b/src/lock_file/update.rs index ba2033abb..6c7e216d9 100644 --- a/src/lock_file/update.rs +++ b/src/lock_file/update.rs @@ -470,6 +470,7 @@ pub async fn ensure_up_to_date_lock_file( .into_iter() .filter(|target| !options.existing_repo_data.contains_key(target)), project.authenticated_client(), + Some(project.config()), ) .await?; diff --git a/src/repodata.rs b/src/repodata.rs index da2903f13..f0cda4c18 100644 --- a/src/repodata.rs +++ b/src/repodata.rs @@ -1,3 +1,4 @@ +use crate::config::Config; use crate::project::Environment; use crate::{config, progress, project::Project}; use futures::{stream, StreamExt, TryStreamExt}; @@ -26,7 +27,13 @@ impl Environment<'_> { ) -> miette::Result> { let channels = self.channels(); let platforms = self.platforms(); - fetch_sparse_repodata(channels, platforms, self.project().authenticated_client()).await + fetch_sparse_repodata( + channels, + platforms, + self.project().authenticated_client(), + Some(self.project().config()), + ) + .await } } @@ -34,6 +41,7 @@ pub async fn fetch_sparse_repodata( channels: impl IntoIterator, target_platforms: impl IntoIterator, authenticated_client: &ClientWithMiddleware, + config: Option<&Config>, ) -> miette::Result> { let channels = channels.into_iter(); let target_platforms = target_platforms.into_iter().collect_vec(); @@ -54,12 +62,13 @@ pub async fn fetch_sparse_repodata( } } - fetch_sparse_repodata_targets(fetch_targets, authenticated_client).await + fetch_sparse_repodata_targets(fetch_targets, authenticated_client, config).await } pub async fn fetch_sparse_repodata_targets( fetch_targets: impl IntoIterator, authenticated_client: &ClientWithMiddleware, + config: Option<&Config>, ) -> miette::Result> { let mut fetch_targets = fetch_targets.into_iter().peekable(); if fetch_targets.peek().is_none() { @@ -78,6 +87,17 @@ pub async fn fetch_sparse_repodata_targets( let multi_progress = progress::global_multi_progress(); let mut progress_bars = Vec::new(); + let fetch_repodata_options = config + .as_ref() + .and_then(|config| config.repodata_config.as_ref()) + .map(|config| FetchRepoDataOptions { + bz2_enabled: !config.disable_bzip2.unwrap_or_default(), + jlap_enabled: !config.disable_jlap.unwrap_or_default(), + zstd_enabled: !config.disable_zstd.unwrap_or_default(), + ..Default::default() + }) + .unwrap_or_default(); + let repo_data = stream::iter(fetch_targets) .map(|(channel, platform)| { // Construct a progress bar for the fetch @@ -93,7 +113,7 @@ pub async fn fetch_sparse_repodata_targets( let repodata_cache = repodata_cache_path.clone(); let download_client = authenticated_client.clone(); let top_level_progress = top_level_progress.clone(); - + let fetch_options = fetch_repodata_options.clone(); async move { let result = fetch_repo_data_records_with_progress( channel.clone(), @@ -102,6 +122,7 @@ pub async fn fetch_sparse_repodata_targets( download_client, progress_bar.clone(), platform != Platform::NoArch, + fetch_options, ) .await; @@ -133,6 +154,7 @@ async fn fetch_repo_data_records_with_progress( client: ClientWithMiddleware, progress_bar: indicatif::ProgressBar, allow_not_found: bool, + fetch_options: FetchRepoDataOptions, ) -> miette::Result> { // Download the repodata.json let download_progress_progress_bar = progress_bar.clone(); @@ -140,9 +162,7 @@ async fn fetch_repo_data_records_with_progress( channel.platform_url(platform), client, repodata_cache.to_path_buf(), - FetchRepoDataOptions { - ..FetchRepoDataOptions::default() - }, + fetch_options, Some(Box::new(move |fetch::DownloadProgress { total, bytes }| { download_progress_progress_bar.set_length(total.unwrap_or(bytes)); download_progress_progress_bar.set_position(bytes); diff --git a/src/snapshots/pixi__config__tests__config_merge.snap b/src/snapshots/pixi__config__tests__config_merge.snap index ef874a75a..1acfd5f18 100644 --- a/src/snapshots/pixi__config__tests__config_merge.snap +++ b/src/snapshots/pixi__config__tests__config_merge.snap @@ -37,4 +37,15 @@ Config { fragment: None, }, }, + repodata_config: Some( + RepodataConfig { + disable_jlap: Some( + true, + ), + disable_bzip2: None, + disable_zstd: Some( + true, + ), + }, + ), } diff --git a/tests/config/config_2.toml b/tests/config/config_2.toml index 90f64b623..b4455099d 100644 --- a/tests/config/config_2.toml +++ b/tests/config/config_2.toml @@ -1,2 +1,6 @@ tls_no_verify = false change_ps1 = true + +[repodata_config] +disable_jlap = true +disable_zstd = true From 6e192535379ee53188e00fdd262e674f5202ef7f Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Wed, 20 Mar 2024 11:56:23 +0100 Subject: [PATCH 2/2] improve merging --- src/config.rs | 65 ++++++++++++++++++++-------------------------- src/project/mod.rs | 2 +- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/config.rs b/src/config.rs index 2245a8abc..03a968e4e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -10,17 +10,6 @@ use url::Url; use crate::consts; -/// If other is None, keep the value, otherwise assign the value of other to self. -macro_rules! assign_if_some { - ($self:expr, $other:expr, $($field:ident),+) => { - $( - if $other.$field.is_some() { - $self.$field = $other.$field.clone(); - } - )+ - } -} - /// Determines the default author based on the default git author. Both the name and the email /// address of the author are returned. pub fn get_default_author() -> Option<(String, String)> { @@ -181,7 +170,7 @@ impl Config { tracing::info!("Loading global config from {}", location.display()); let global_config = fs::read_to_string(&location).unwrap_or_default(); if let Ok(config) = Config::from_toml(&global_config, &location) { - merged_config.merge_config(&config); + merged_config = merged_config.merge_config(config); } else { tracing::warn!( "Could not load global config (invalid toml): {}", @@ -197,16 +186,13 @@ impl Config { // This will add any environment variables defined in the `clap` attributes to the config let mut default_cli = ConfigCli::default(); default_cli.update_from(std::env::args().take(0)); - merged_config.merge_config(&default_cli.into()); - - merged_config + merged_config.merge_config(default_cli.into()) } /// Load the global config and layer the given cli config on top of it. pub fn with_cli_config(cli: &ConfigCli) -> Config { - let mut config = Config::load_global(); - config.merge_config(&cli.clone().into()); - config + let config = Config::load_global(); + config.merge_config(cli.clone().into()) } /// Load the config from the given path pixi folder and merge it with the global config. @@ -217,7 +203,7 @@ impl Config { if local_config.exists() { let s = fs::read_to_string(&local_config).into_diagnostic()?; let local = Config::from_toml(&s, &local_config)?; - config.merge_config(&local); + config = config.merge_config(local); } Ok(config) @@ -229,23 +215,28 @@ impl Config { } /// Merge the given config into the current one. - pub fn merge_config(&mut self, other: &Config) { - if !other.default_channels.is_empty() { - self.default_channels = other.default_channels.clone(); - } + #[must_use] + pub fn merge_config(mut self, other: Config) -> Self { + self.mirrors.extend(other.mirrors); + self.loaded_from.extend(other.loaded_from); - assign_if_some!( - self, - other, - change_ps1, - tls_no_verify, - authentication_override_file, - repodata_config - ); - - self.mirrors.extend(other.mirrors.clone()); - - self.loaded_from.extend(other.loaded_from.iter().cloned()); + Self { + default_channels: if other.default_channels.is_empty() { + self.default_channels + } else { + other.default_channels + }, + tls_no_verify: other.tls_no_verify.or(self.tls_no_verify), + change_ps1: other.change_ps1.or(self.change_ps1), + authentication_override_file: other + .authentication_override_file + .or(self.authentication_override_file), + mirrors: self.mirrors, + loaded_from: self.loaded_from, + // currently this is always the default so just use the current value + channel_config: self.channel_config, + repodata_config: other.repodata_config.or(self.repodata_config), + } } /// Retrieve the value for the default_channels field (defaults to the ["conda-forge"]). @@ -345,7 +336,7 @@ mod tests { tls_no_verify: Some(true), ..Default::default() }; - config.merge_config(&other); + config = config.merge_config(other); assert_eq!(config.default_channels, vec!["conda-forge"]); assert_eq!(config.tls_no_verify, Some(true)); @@ -357,7 +348,7 @@ mod tests { let config_2 = Config::from_path(&d.join("config_2.toml")).unwrap(); let mut merged = config_1.clone(); - merged.merge_config(&config_2); + merged = merged.merge_config(config_2); let debug = format!("{:#?}", merged); let debug = debug.replace("\\\\", "/"); diff --git a/src/project/mod.rs b/src/project/mod.rs index cadffe949..0a1ea34fa 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -222,7 +222,7 @@ impl Project { where C: Into, { - self.config.merge_config(&config.into()); + self.config = self.config.merge_config(config.into()); self }