From 28ccfca53f3beddd135cc4d5999cae90215772b2 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Thu, 5 Sep 2024 09:06:35 -0400 Subject: [PATCH 1/3] chore(config): use merge instead of handwritten merge --- Cargo.lock | 23 +++++++++ Cargo.toml | 1 + crates/turborepo-lib/Cargo.toml | 1 + crates/turborepo-lib/src/config/mod.rs | 64 ++------------------------ 4 files changed, 29 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 128d9683cc4d4..8b1404c2d8639 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2992,6 +2992,16 @@ dependencies = [ "autocfg", ] +[[package]] +name = "merge" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10bbef93abb1da61525bbc45eeaff6473a41907d19f8f9aa5168d214e10693e9" +dependencies = [ + "merge_derive", + "num-traits", +] + [[package]] name = "merge-streams" version = "0.1.2" @@ -3002,6 +3012,18 @@ dependencies = [ "pin-project", ] +[[package]] +name = "merge_derive" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "209d075476da2e63b4b29e72a2ef627b840589588e71400a25e3565c4f849d07" +dependencies = [ + "proc-macro-error", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "miette" version = "5.10.0" @@ -5883,6 +5905,7 @@ dependencies = [ "jsonc-parser 0.21.0", "lazy_static", "libc", + "merge", "miette", "nix 0.26.2", "notify", diff --git a/Cargo.toml b/Cargo.toml index 5e34955203a0d..f5aa5f2296c00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -137,6 +137,7 @@ lightningcss = { version = "1.0.0-alpha.57", features = [ "visitor", "into_owned", ] } +merge = "0.1.0" mime = "0.3.16" notify = "6.1.1" once_cell = "1.17.1" diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index 0785c49941ca7..448dc7429615e 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -70,6 +70,7 @@ itertools = { workspace = true } jsonc-parser = { version = "0.21.0" } lazy_static = { workspace = true } libc = "0.2.140" +merge = { workspace = true } miette = { workspace = true, features = ["fancy"] } nix = "0.26.2" notify = { workspace = true } diff --git a/crates/turborepo-lib/src/config/mod.rs b/crates/turborepo-lib/src/config/mod.rs index 398754bea9c4d..dd9272799046f 100644 --- a/crates/turborepo-lib/src/config/mod.rs +++ b/crates/turborepo-lib/src/config/mod.rs @@ -8,6 +8,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use convert_case::{Case, Casing}; use env::{EnvVars, OverrideEnvVars}; use file::{AuthFile, ConfigFile}; +use merge::Merge; use miette::{Diagnostic, NamedSource, SourceSpan}; use serde::Deserialize; use struct_iterable::Iterable; @@ -190,7 +191,7 @@ const DEFAULT_UPLOAD_TIMEOUT: u64 = 60; // We intentionally don't derive Serialize so that different parts // of the code that want to display the config can tune how they // want to display and what fields they want to include. -#[derive(Deserialize, Default, Debug, PartialEq, Eq, Clone, Iterable)] +#[derive(Deserialize, Default, Debug, PartialEq, Eq, Clone, Iterable, Merge)] #[serde(rename_all = "camelCase")] pub struct ConfigurationOptions { #[serde(alias = "apiurl")] @@ -336,44 +337,6 @@ impl ConfigurationOptions { } } -macro_rules! create_set_if_empty { - ($func_name:ident, $property_name:ident, $type:ty) => { - fn $func_name(&mut self, value: &mut Option<$type>) { - if self.$property_name.is_none() { - if let Some(value) = value.take() { - self.$property_name = Some(value); - } - } - } - }; -} - -// Private setters used only for construction -impl ConfigurationOptions { - create_set_if_empty!(set_api_url, api_url, String); - create_set_if_empty!(set_login_url, login_url, String); - create_set_if_empty!(set_team_slug, team_slug, String); - create_set_if_empty!(set_team_id, team_id, String); - create_set_if_empty!(set_token, token, String); - create_set_if_empty!(set_signature, signature, bool); - create_set_if_empty!(set_enabled, enabled, bool); - create_set_if_empty!(set_preflight, preflight, bool); - create_set_if_empty!(set_timeout, timeout, u64); - create_set_if_empty!(set_ui, ui, UIMode); - create_set_if_empty!(set_allow_no_package_manager, allow_no_package_manager, bool); - create_set_if_empty!(set_daemon, daemon, bool); - create_set_if_empty!(set_env_mode, env_mode, EnvMode); - create_set_if_empty!(set_cache_dir, cache_dir, Utf8PathBuf); - create_set_if_empty!(set_scm_base, scm_base, String); - create_set_if_empty!(set_scm_head, scm_head, String); - create_set_if_empty!(set_spaces_id, spaces_id, String); - create_set_if_empty!( - set_root_turbo_json_path, - root_turbo_json_path, - AbsoluteSystemPathBuf - ); -} - // Maps Some("") to None to emulate how Go handles empty strings fn non_empty_str(s: Option<&str>) -> Option<&str> { s.filter(|s| !s.is_empty()) @@ -483,27 +446,8 @@ impl TurborepoConfigBuilder { let config = sources.into_iter().try_fold( ConfigurationOptions::default(), |mut acc, current_source| { - let mut current_source_config = current_source.get_configuration_options(&acc)?; - acc.set_api_url(&mut current_source_config.api_url); - acc.set_login_url(&mut current_source_config.login_url); - acc.set_team_slug(&mut current_source_config.team_slug); - acc.set_team_id(&mut current_source_config.team_id); - acc.set_token(&mut current_source_config.token); - acc.set_signature(&mut current_source_config.signature); - acc.set_enabled(&mut current_source_config.enabled); - acc.set_preflight(&mut current_source_config.preflight); - acc.set_timeout(&mut current_source_config.timeout); - acc.set_spaces_id(&mut current_source_config.spaces_id); - acc.set_ui(&mut current_source_config.ui); - acc.set_allow_no_package_manager( - &mut current_source_config.allow_no_package_manager, - ); - acc.set_daemon(&mut current_source_config.daemon); - acc.set_env_mode(&mut current_source_config.env_mode); - acc.set_scm_base(&mut current_source_config.scm_base); - acc.set_scm_head(&mut current_source_config.scm_head); - acc.set_cache_dir(&mut current_source_config.cache_dir); - acc.set_root_turbo_json_path(&mut current_source_config.root_turbo_json_path); + let current_source_config = current_source.get_configuration_options(&acc)?; + acc.merge(current_source_config); Ok(acc) }, ); From 2f4600a4a831fd4a08e9bdc15140cdee769820c5 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Thu, 5 Sep 2024 09:19:17 -0400 Subject: [PATCH 2/3] chore(config): use derive_setters instead of hand written --- Cargo.lock | 13 ++++++++ Cargo.toml | 1 + crates/turborepo-lib/Cargo.toml | 1 + crates/turborepo-lib/src/config/mod.rs | 41 +++++--------------------- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b1404c2d8639..b7778fa8c0eeb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1533,6 +1533,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "derive_setters" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e8ef033054e131169b8f0f9a7af8f5533a9436fadf3c500ed547f730f07090d" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn 2.0.58", +] + [[package]] name = "dialoguer" version = "0.10.4" @@ -5887,6 +5899,7 @@ dependencies = [ "convert_case 0.6.0", "crossterm 0.26.1", "ctrlc", + "derive_setters", "dialoguer", "dirs-next", "dunce", diff --git a/Cargo.toml b/Cargo.toml index f5aa5f2296c00..07b9e607e1d4e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -119,6 +119,7 @@ console-subscriber = "0.1.8" criterion = "0.4.0" crossbeam-channel = "0.5.8" dashmap = "5.4.0" +derive_setters = "0.1.6" dialoguer = "0.10.3" dunce = "1.0.3" either = "1.9.0" diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index 448dc7429615e..43f802ab32780 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -52,6 +52,7 @@ const_format = "0.2.30" convert_case = "0.6.0" crossterm = "0.26" ctrlc = { version = "3.4.0", features = ["termination"] } +derive_setters = { workspace = true } dialoguer = { workspace = true, features = ["fuzzy-select"] } dirs-next = "2.0.0" dunce = { workspace = true } diff --git a/crates/turborepo-lib/src/config/mod.rs b/crates/turborepo-lib/src/config/mod.rs index dd9272799046f..0ecb9974ebc9b 100644 --- a/crates/turborepo-lib/src/config/mod.rs +++ b/crates/turborepo-lib/src/config/mod.rs @@ -6,6 +6,7 @@ use std::{collections::HashMap, ffi::OsString, io}; use camino::{Utf8Path, Utf8PathBuf}; use convert_case::{Case, Casing}; +use derive_setters::Setters; use env::{EnvVars, OverrideEnvVars}; use file::{AuthFile, ConfigFile}; use merge::Merge; @@ -174,15 +175,6 @@ pub enum Error { }, } -macro_rules! create_builder { - ($func_name:ident, $property_name:ident, $type:ty) => { - pub fn $func_name(mut self, value: $type) -> Self { - self.override_config.$property_name = value; - self - } - }; -} - const DEFAULT_API_URL: &str = "https://vercel.com/api"; const DEFAULT_LOGIN_URL: &str = "https://vercel.com"; const DEFAULT_TIMEOUT: u64 = 30; @@ -191,8 +183,13 @@ const DEFAULT_UPLOAD_TIMEOUT: u64 = 60; // We intentionally don't derive Serialize so that different parts // of the code that want to display the config can tune how they // want to display and what fields they want to include. -#[derive(Deserialize, Default, Debug, PartialEq, Eq, Clone, Iterable, Merge)] +#[derive(Deserialize, Default, Debug, PartialEq, Eq, Clone, Iterable, Merge, Setters)] #[serde(rename_all = "camelCase")] +// Generate setters for the builder type the set these values on it's override_config field +#[setters( + prefix = "with_", + generate_delegates(ty = "TurborepoConfigBuilder", field = "override_config") +)] pub struct ConfigurationOptions { #[serde(alias = "apiurl")] #[serde(alias = "ApiUrl")] @@ -391,30 +388,6 @@ impl TurborepoConfigBuilder { .unwrap_or_else(get_lowercased_env_vars) } - create_builder!(with_api_url, api_url, Option); - create_builder!(with_login_url, login_url, Option); - create_builder!(with_team_slug, team_slug, Option); - create_builder!(with_team_id, team_id, Option); - create_builder!(with_token, token, Option); - create_builder!(with_signature, signature, Option); - create_builder!(with_enabled, enabled, Option); - create_builder!(with_preflight, preflight, Option); - create_builder!(with_timeout, timeout, Option); - create_builder!(with_ui, ui, Option); - create_builder!( - with_allow_no_package_manager, - allow_no_package_manager, - Option - ); - create_builder!(with_daemon, daemon, Option); - create_builder!(with_env_mode, env_mode, Option); - create_builder!(with_cache_dir, cache_dir, Option); - create_builder!( - with_root_turbo_json_path, - root_turbo_json_path, - Option - ); - pub fn build(&self) -> Result { // Priority, from least significant to most significant: // - shared configuration (turbo.json) From e93b0aa68619a4d796f121c09dfd58608ad709d3 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Tue, 10 Sep 2024 10:50:32 -0400 Subject: [PATCH 3/3] Update crates/turborepo-lib/src/config/mod.rs Co-authored-by: Nicholas Yang --- crates/turborepo-lib/src/config/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/turborepo-lib/src/config/mod.rs b/crates/turborepo-lib/src/config/mod.rs index 0ecb9974ebc9b..6fefa2d435e69 100644 --- a/crates/turborepo-lib/src/config/mod.rs +++ b/crates/turborepo-lib/src/config/mod.rs @@ -185,7 +185,7 @@ const DEFAULT_UPLOAD_TIMEOUT: u64 = 60; // want to display and what fields they want to include. #[derive(Deserialize, Default, Debug, PartialEq, Eq, Clone, Iterable, Merge, Setters)] #[serde(rename_all = "camelCase")] -// Generate setters for the builder type the set these values on it's override_config field +// Generate setters for the builder type that set these values on its override_config field #[setters( prefix = "with_", generate_delegates(ty = "TurborepoConfigBuilder", field = "override_config")