feat: Query of core.logAllRefUpdates is now fallible. (#450)

This is the same behaviour as shown by `git`, which requires valid
values or aborts.

git-repository/src/clone/fetch.rs

@@ -72,7 +72,7 @@ impl PrepareFetch {
             Ok(config)
         }
 
-        fn replace_changed_local_config(repo: &mut Repository, config: git_config::File<'static>) {
+        fn replace_changed_local_config(repo: &mut Repository, mut config: git_config::File<'static>) {
             let repo_config = git_features::threading::OwnShared::make_mut(&mut repo.config.resolved);
             let ids_to_remove: Vec<_> = repo_config
                 .sections_and_ids()
@@ -83,9 +83,9 @@ impl PrepareFetch {
             for id in ids_to_remove {
                 repo_config.remove_section_by_id(id);
             }
-            repo_config.append(config);
-            crate::config::overrides::apply(repo_config, &repo.options.config_overrides, git_config::Source::Api)
+            crate::config::overrides::apply(&mut config, &repo.options.config_overrides, git_config::Source::Api)
                 .expect("applied once and can be applied again");
+            repo_config.append(config);
             repo.reread_values_and_clear_caches()
                 .expect("values could be read once and can be read again");
         }

git-repository/src/config/cache/incubate.rs

@@ -57,7 +57,7 @@ impl StageOne {
             .transpose()?
             .unwrap_or(git_hash::Kind::Sha1);
 
-        let reflog = util::query_refupdates(&config);
+        let reflog = util::query_refupdates(&config, lenient)?;
         Ok(StageOne {
             git_dir_config: config,
             buf,

git-repository/src/config/cache/init.rs

@@ -119,7 +119,7 @@ impl Cache {
         let hex_len = util::check_lenient(util::parse_core_abbrev(&config, object_hash), lenient_config)?;
 
         use util::config_bool;
-        let reflog = util::query_refupdates(&config);
+        let reflog = util::query_refupdates(&config, lenient_config)?;
         let ignore_case = config_bool(&config, "core.ignoreCase", false, lenient_config)?;
         let use_multi_pack_index = config_bool(&config, "core.multiPackIndex", true, lenient_config)?;
         let object_kind_hint = util::disambiguate_hint(&config);
@@ -170,7 +170,7 @@ impl Cache {
         use util::config_bool;
         let ignore_case = config_bool(&config, "core.ignoreCase", false, self.lenient_config)?;
         let object_kind_hint = util::disambiguate_hint(&config);
-        let reflog = util::query_refupdates(config);
+        let reflog = util::query_refupdates(config, self.lenient_config)?;
 
         self.hex_len = hex_len;
         self.ignore_case = ignore_case;

git-repository/src/config/cache/util.rs

@@ -41,17 +41,27 @@ pub(crate) fn config_bool(
     }
 }
 
-pub(crate) fn query_refupdates(config: &git_config::File<'static>) -> Option<git_ref::store::WriteReflog> {
-    config.string("core", None, "logAllRefUpdates").map(|val| {
-        (val.eq_ignore_ascii_case(b"always"))
-            .then(|| git_ref::store::WriteReflog::Always)
-            .or_else(|| {
-                git_config::Boolean::try_from(val)
-                    .ok()
-                    .and_then(|b| b.is_true().then(|| git_ref::store::WriteReflog::Normal))
-            })
-            .unwrap_or(git_ref::store::WriteReflog::Disable)
-    })
+pub(crate) fn query_refupdates(
+    config: &git_config::File<'static>,
+    lenient_config: bool,
+) -> Result<Option<git_ref::store::WriteReflog>, Error> {
+    match config
+        .boolean("core", None, "logAllRefUpdates")
+        .and_then(|b| b.ok())
+        .map(|b| {
+            b.then(|| git_ref::store::WriteReflog::Normal)
+                .unwrap_or(git_ref::store::WriteReflog::Disable)
+        }) {
+        Some(val) => Ok(Some(val)),
+        None => match config.string("core", None, "logAllRefUpdates") {
+            Some(val) if val.eq_ignore_ascii_case(b"always") => Ok(Some(git_ref::store::WriteReflog::Always)),
+            Some(_val) if lenient_config => Ok(None),
+            Some(val) => Err(Error::LogAllRefUpdates {
+                value: val.into_owned(),
+            }),
+            None => Ok(None),
+        },
+    }
 }
 
 pub(crate) fn check_lenient<T, E>(v: Result<Option<T>, E>, lenient: bool) -> Result<Option<T>, E> {

git-repository/src/config/mod.rs

@@ -67,6 +67,8 @@ pub enum Error {
     PathInterpolation(#[from] git_config::path::interpolate::Error),
     #[error("Configuration overrides at open or init time could not be applied.")]
     ConfigOverrides(#[from] overrides::Error),
+    #[error("Invalid value for 'core.logAllRefUpdates': \"{value}\"")]
+    LogAllRefUpdates { value: BString },
 }
 
 ///

git-repository/src/open.rs

@@ -463,6 +463,7 @@ impl Repository {
     ///
     /// Similar to `reread_values_and_clear_caches_replacing_config()`, but works on the existing instance instead of a passed
     /// in one that it them makes the default.
+    #[cfg(feature = "blocking-network-client")]
     pub(crate) fn reread_values_and_clear_caches(&mut self) -> Result<(), config::Error> {
         self.config.reread_values_and_clear_caches()?;
         self.apply_changed_values();

git-repository/tests/clone/mod.rs

@@ -75,7 +75,7 @@ mod blocking_io {
         }
 
         let head = repo.head()?;
-        let _head_logs = head.log_iter().all()?.expect("log present").collect::<Vec<_>>();
+        // let _head_logs = head.log_iter().all()?.expect("log present").collect::<Vec<_>>();
         let referent = head.try_into_referent().expect("symbolic ref is present");
         assert!(
             referent.id().object().is_ok(),