settings: propagate configuration error of commit and operation parameters

CHANGELOG.md

@@ -18,8 +18,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * The following configuration variables are now parsed strictly:
   `colors.<labels>`, `git.abandon-unreachable-commits`,
   `git.auto-local-bookmark`, `git.push-bookmark-prefix`, `revsets.log`,
-  `revsets.short-prefixes` `signing.backend`, `ui.allow-init-native`,
-  `ui.color`, `ui.default-description`, `ui.progress-indicator`, `ui.quiet`
+  `revsets.short-prefixes` `signing.backend`, `operation.hostname`,
+  `operation.username`, `ui.allow-init-native`, `ui.color`,
+  `ui.default-description`, `ui.progress-indicator`, `ui.quiet`, `user.email`,
+  `user.name`
 
 * `jj config list` now prints inline tables `{ key = value, .. }` literally.
   Inner items of inline tables are no longer merged across configuration files.

lib/src/revset.rs

@@ -850,7 +850,7 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = 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<RevsetWorkspaceContext<'a>>,
@@ -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<RevsetWorkspaceContext<'a>>,
@@ -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,
-            "[email protected]".to_string(),
+            "[email protected]",
             chrono::Utc::now().fixed_offset().into(),
             &extensions,
             None,
@@ -2735,7 +2735,7 @@ mod tests {
         let extensions = RevsetExtensions::default();
         let context = RevsetParseContext::new(
             &aliases_map,
-            "[email protected]".to_string(),
+            "[email protected]",
             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,
-            "[email protected]".to_string(),
+            "[email protected]",
             chrono::Utc::now().fixed_offset().into(),
             &extensions,
             None,

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<Timestamp>,
     operation_timestamp: Option<Timestamp>,
+    operation_hostname: String,
+    operation_username: String,
     rng: Arc<JJRng>,
 }
 
@@ -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,45 @@ fn to_timestamp(value: ConfigValue) -> Result<Timestamp, Box<dyn std::error::Err
 
 impl UserSettings {
     pub fn from_config(config: StackedConfig) -> Result<Self, ConfigGetError> {
+        let user_name = config.get("user.name").optional()?.unwrap_or_default();
+        let user_email = config.get("user.email").optional()?.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()?;
+        // whoami::fallible::*() failure isn't a ConfigGetError, but user would
+        // have to set the corresponding config keys if these parameter can't be
+        // obtained from the system. Instead of handling environment data here,
+        // it might be better to load them by CLI and insert as a config layer.
+        let operation_hostname = config
+            .get("operation.hostname")
+            .optional()?
+            .map_or_else(whoami::fallible::hostname, Ok)
+            .map_err(|err| ConfigGetError::Type {
+                name: "operation.hostname".to_owned(),
+                error: err.into(),
+                source_path: None,
+            })?;
+        let operation_username = config
+            .get("operation.username")
+            .optional()?
+            .map_or_else(whoami::fallible::username, Ok)
+            .map_err(|err| ConfigGetError::Type {
+                name: "operation.username".to_owned(),
+                error: err.into(),
+                source_path: None,
+            })?;
         let rng_seed = config.get::<u64>("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 +200,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<FsmonitorSettings, ConfigGetError> {
@@ -195,21 +227,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,
         }
     }

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,

lib/tests/test_revset.rs

@@ -69,8 +69,7 @@ fn resolve_symbol_with_extensions(
 ) -> Result<Vec<CommitId>, 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()