From 4a69d0178c7e873c038eb0c8a21da7789c99c0cd Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 23 Dec 2024 17:30:21 +0900 Subject: [PATCH] settings: cache commit and operation parameters by UserSettings This helps propagate configuration error. RevsetParseContext is also updated because it was easier to pass &str in to it. --- lib/src/revset.rs | 16 ++++++++-------- lib/src/settings.rs | 40 +++++++++++++++++++++++++++------------- lib/src/transaction.rs | 4 ++-- lib/tests/test_revset.rs | 5 ++--- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/src/revset.rs b/lib/src/revset.rs index f67b1d04e7..7ae195c7e4 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -850,7 +850,7 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: // are generally (although not universally) treated as case‐insensitive too, so // we use a case‐insensitive match here. Ok(RevsetExpression::filter(RevsetFilterPredicate::Author( - StringPattern::exact_i(&context.user_email), + StringPattern::exact_i(context.user_email), ))) }); map.insert("committer", |diagnostics, function, _context| { @@ -2629,7 +2629,7 @@ impl RevsetExtensions { #[derive(Clone)] pub struct RevsetParseContext<'a> { aliases_map: &'a RevsetAliasesMap, - user_email: String, + user_email: &'a str, date_pattern_context: DatePatternContext, extensions: &'a RevsetExtensions, workspace: Option>, @@ -2638,7 +2638,7 @@ pub struct RevsetParseContext<'a> { impl<'a> RevsetParseContext<'a> { pub fn new( aliases_map: &'a RevsetAliasesMap, - user_email: String, + user_email: &'a str, date_pattern_context: DatePatternContext, extensions: &'a RevsetExtensions, workspace: Option>, @@ -2656,8 +2656,8 @@ impl<'a> RevsetParseContext<'a> { self.aliases_map } - pub fn user_email(&self) -> &str { - &self.user_email + pub fn user_email(&self) -> &'a str { + self.user_email } pub fn date_pattern_context(&self) -> &DatePatternContext { @@ -2706,7 +2706,7 @@ mod tests { let extensions = RevsetExtensions::default(); let context = RevsetParseContext::new( &aliases_map, - "test.user@example.com".to_string(), + "test.user@example.com", chrono::Utc::now().fixed_offset().into(), &extensions, None, @@ -2735,7 +2735,7 @@ mod tests { let extensions = RevsetExtensions::default(); let context = RevsetParseContext::new( &aliases_map, - "test.user@example.com".to_string(), + "test.user@example.com", chrono::Utc::now().fixed_offset().into(), &extensions, Some(workspace_ctx), @@ -2760,7 +2760,7 @@ mod tests { let extensions = RevsetExtensions::default(); let context = RevsetParseContext::new( &aliases_map, - "test.user@example.com".to_string(), + "test.user@example.com", chrono::Utc::now().fixed_offset().into(), &extensions, None, diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 0deb2631e1..374382a354 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -41,8 +41,12 @@ use crate::signing::SignBehavior; #[derive(Debug, Clone)] pub struct UserSettings { config: StackedConfig, + user_name: String, + user_email: String, commit_timestamp: Option, operation_timestamp: Option, + operation_hostname: String, + operation_username: String, rng: Arc, } @@ -106,7 +110,7 @@ impl SignSettings { } else { SignBehavior::Keep }, - user_email: settings.user_email(), + user_email: settings.user_email().to_owned(), key: settings.get_string("signing.key").ok(), } } @@ -142,17 +146,29 @@ fn to_timestamp(value: ConfigValue) -> Result Result { + let user_name = config.get("user.name").unwrap_or_default(); + let user_email = config.get("user.email").unwrap_or_default(); let commit_timestamp = config .get_value_with("debug.commit-timestamp", to_timestamp) .optional()?; let operation_timestamp = config .get_value_with("debug.operation-timestamp", to_timestamp) .optional()?; + let operation_hostname = config + .get("operation.hostname") + .unwrap_or_else(|_| whoami::fallible::hostname().expect("valid hostname")); + let operation_username = config + .get("operation.username") + .unwrap_or_else(|_| whoami::username()); let rng_seed = config.get::("debug.randomness-seed").optional()?; Ok(UserSettings { config, + user_name, + user_email, commit_timestamp, operation_timestamp, + operation_hostname, + operation_username, rng: Arc::new(JJRng::new(rng_seed)), }) } @@ -168,15 +184,15 @@ impl UserSettings { self.rng.clone() } - pub fn user_name(&self) -> String { - self.get_string("user.name").unwrap_or_default() + pub fn user_name(&self) -> &str { + &self.user_name } // Must not be changed to avoid git pushing older commits with no set name pub const USER_NAME_PLACEHOLDER: &'static str = "(no name configured)"; - pub fn user_email(&self) -> String { - self.get_string("user.email").unwrap_or_default() + pub fn user_email(&self) -> &str { + &self.user_email } pub fn fsmonitor_settings(&self) -> Result { @@ -195,21 +211,19 @@ impl UserSettings { self.operation_timestamp } - pub fn operation_hostname(&self) -> String { - self.get_string("operation.hostname") - .unwrap_or_else(|_| whoami::fallible::hostname().expect("valid hostname")) + pub fn operation_hostname(&self) -> &str { + &self.operation_hostname } - pub fn operation_username(&self) -> String { - self.get_string("operation.username") - .unwrap_or_else(|_| whoami::username()) + pub fn operation_username(&self) -> &str { + &self.operation_username } pub fn signature(&self) -> Signature { let timestamp = self.commit_timestamp.unwrap_or_else(Timestamp::now); Signature { - name: self.user_name(), - email: self.user_email(), + name: self.user_name().to_owned(), + email: self.user_email().to_owned(), timestamp, } } diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 1d708e6d62..9f65d6423f 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -155,8 +155,8 @@ pub fn create_op_metadata( .operation_timestamp() .unwrap_or_else(Timestamp::now); let end_time = start_time; - let hostname = user_settings.operation_hostname(); - let username = user_settings.operation_username(); + let hostname = user_settings.operation_hostname().to_owned(); + let username = user_settings.operation_username().to_owned(); OperationMetadata { start_time, end_time, diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 3063abea3c..b8f63eb265 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -69,8 +69,7 @@ fn resolve_symbol_with_extensions( ) -> Result, RevsetResolutionError> { let aliases_map = RevsetAliasesMap::default(); let now = chrono::Local::now(); - let context = - RevsetParseContext::new(&aliases_map, String::new(), now.into(), extensions, None); + let context = RevsetParseContext::new(&aliases_map, "", now.into(), extensions, None); let expression = parse(&mut RevsetDiagnostics::new(), symbol, &context).unwrap(); assert_matches!(*expression, RevsetExpression::CommitRef(_)); let symbol_resolver = DefaultSymbolResolver::new(repo, extensions.symbol_resolvers()); @@ -2971,7 +2970,7 @@ fn test_evaluate_expression_mine() { .set_parents(vec![commit1.id().clone()]) .set_author(Signature { name: "name2".to_string(), - email: settings.user_email(), + email: settings.user_email().to_owned(), timestamp, }) .write()