settings: propagate error from git_settings() and signing_backend()

CHANGELOG.md

@@ -16,9 +16,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   not converted to a string `"true"`, and vice versa.
 
 * The following configuration variables are now parsed strictly:
-  `colors.<labels>`, `git.push-bookmark-prefix`, `revsets.log`,
-  `revsets.short-prefixes` `ui.allow-init-native`, `ui.color`,
-  `ui.default-description`, `ui.progress-indicator`, `ui.quiet`
+  `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`
 
 * `jj config list` now prints inline tables `{ key = value, .. }` literally.
   Inner items of inline tables are no longer merged across configuration files.

cli/src/cli_util.rs

@@ -1117,7 +1117,7 @@ impl WorkspaceCommandHelper {
     /// the working copy parent if the repository is colocated.
     #[instrument(skip_all)]
     fn import_git_refs(&mut self, ui: &Ui) -> Result<(), CommandError> {
-        let git_settings = self.settings().git_settings();
+        let git_settings = self.settings().git_settings()?;
         let mut tx = self.start_transaction();
         // Automated import shouldn't fail because of reserved remote name.
         let stats = git::import_some_refs(tx.repo_mut(), &git_settings, |ref_name| {

cli/src/commands/git/clone.rs

@@ -214,6 +214,7 @@ fn do_git_clone(
     maybe_add_gitignore(&workspace_command)?;
     git_repo.remote(remote_name, source).unwrap();
     let mut fetch_tx = workspace_command.start_transaction();
+    let git_settings = command.settings().git_settings()?;
 
     let stats = with_remote_git_callbacks(ui, None, |cb| {
         git::fetch(
@@ -222,7 +223,7 @@ fn do_git_clone(
             remote_name,
             &[StringPattern::everything()],
             cb,
-            &command.settings().git_settings(),
+            &git_settings,
             depth,
         )
     })

cli/src/commands/git/import.rs

@@ -36,7 +36,7 @@ pub fn cmd_git_import(
     // In non-colocated repo, Git HEAD will never be moved internally by jj.
     // That's why cmd_git_export() doesn't export the HEAD ref.
     git::import_head(tx.repo_mut())?;
-    let stats = git::import_refs(tx.repo_mut(), &command.settings().git_settings())?;
+    let stats = git::import_refs(tx.repo_mut(), &command.settings().git_settings()?)?;
     print_git_import_stats(ui, tx.repo(), &stats, true)?;
     tx.finish(ui, "import git refs")?;
     Ok(())

cli/src/commands/git/init.rs

@@ -205,7 +205,7 @@ fn init_git_refs(
 ) -> Result<Arc<ReadonlyRepo>, CommandError> {
     let mut tx = start_repo_transaction(&repo, command.settings(), command.string_args());
     // There should be no old refs to abandon, but enforce it.
-    let mut git_settings = command.settings().git_settings();
+    let mut git_settings = command.settings().git_settings()?;
     git_settings.abandon_unreachable_commits = false;
     let stats = git::import_some_refs(
         tx.repo_mut(),

cli/src/git_util.rs

@@ -463,7 +463,7 @@ pub fn git_fetch(
     remotes: &[String],
     branch: &[StringPattern],
 ) -> Result<(), CommandError> {
-    let git_settings = tx.settings().git_settings();
+    let git_settings = tx.settings().git_settings()?;
 
     for remote in remotes {
         let stats = with_remote_git_callbacks(ui, None, |cb| {

lib/src/settings.rs

@@ -58,18 +58,20 @@ pub struct GitSettings {
 }
 
 impl GitSettings {
-    pub fn from_settings(settings: &UserSettings) -> Self {
-        let auto_local_bookmark = settings
-            .get_bool("git.auto-local-bookmark")
-            .or_else(|_| settings.get_bool("git.auto-local-branch"))
-            .unwrap_or(false);
+    pub fn from_settings(settings: &UserSettings) -> Result<Self, ConfigGetError> {
+        let auto_local_bookmark = {
+            let opt1 = settings.get_bool("git.auto-local-bookmark").optional()?;
+            let opt2 = settings.get_bool("git.auto-local-branch").optional()?;
+            opt1.or(opt2).unwrap_or(false)
+        };
         let abandon_unreachable_commits = settings
             .get_bool("git.abandon-unreachable-commits")
+            .optional()?
             .unwrap_or(true);
-        GitSettings {
+        Ok(GitSettings {
             auto_local_bookmark,
             abandon_unreachable_commits,
-        }
+        })
     }
 }
 
@@ -219,18 +221,22 @@ impl UserSettings {
         &self.config
     }
 
-    pub fn git_settings(&self) -> GitSettings {
+    pub fn git_settings(&self) -> Result<GitSettings, ConfigGetError> {
         GitSettings::from_settings(self)
     }
 
     // separate from sign_settings as those two are needed in pretty different
     // places
-    pub fn signing_backend(&self) -> Option<String> {
-        let backend = self.get_string("signing.backend").ok()?;
-        (backend.as_str() != "none").then_some(backend)
+    pub fn signing_backend(&self) -> Result<Option<String>, ConfigGetError> {
+        let maybe_backend = self.get_string("signing.backend").optional()?;
+        match maybe_backend.as_deref() {
+            Some("none") | None => Ok(None),
+            Some(_) => Ok(maybe_backend),
+        }
     }
 
     pub fn sign_settings(&self) -> SignSettings {
+        // TODO: propagate config error
         SignSettings::from_settings(self)
     }
 }

lib/src/signing.rs

@@ -170,6 +170,7 @@ impl Signer {
 
         let main_backend = settings
             .signing_backend()
+            .map_err(SignInitError::BackendConfig)?
             .map(|backend| {
                 backends
                     .iter()