feat!: remove SnapshotMut::apply_cli_overrides() in favor of open::Options::cli_overrides().

Author: Byron

git-repository/src/clone/fetch/util.rs

@@ -38,7 +38,7 @@ pub fn replace_changed_local_config_file(repo: &mut Repository, mut config: git_
     for id in ids_to_remove {
         repo_config.remove_section_by_id(id);
     }
-    crate::config::overrides::apply(&mut config, &repo.options.config_overrides, git_config::Source::Api)
+    crate::config::overrides::append(&mut config, &repo.options.api_config_overrides, git_config::Source::Api)
         .expect("applied once and can be applied again");
     repo_config.append(config);
     repo.reread_values_and_clear_caches()

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

@@ -37,7 +37,8 @@ impl Cache {
             includes: use_includes,
         }: repository::permissions::Config,
         lenient_config: bool,
-        config_overrides: &[BString],
+        api_config_overrides: &[BString],
+        cli_config_overrides: &[BString],
     ) -> Result<Self, Error> {
         let options = git_config::file::init::Options {
             includes: if use_includes {
@@ -113,8 +114,11 @@ impl Cache {
             if use_env {
                 globals.append(git_config::File::from_env(options)?.unwrap_or_default());
             }
-            if !config_overrides.is_empty() {
-                crate::config::overrides::apply(&mut globals, config_overrides, git_config::Source::Api)?;
+            if !cli_config_overrides.is_empty() {
+                crate::config::overrides::append(&mut globals, cli_config_overrides, git_config::Source::Cli)?;
+            }
+            if !api_config_overrides.is_empty() {
+                crate::config::overrides::append(&mut globals, api_config_overrides, git_config::Source::Api)?;
             }
             globals
         };

git-repository/src/config/overrides.rs

@@ -17,7 +17,7 @@ pub enum Error {
     SectionHeader(#[from] git_config::parse::section::header::Error),
 }
 
-pub(crate) fn apply(
+pub(crate) fn append(
     config: &mut git_config::File<'static>,
     values: impl IntoIterator<Item = impl AsRef<BStr>>,
     source: git_config::Source,

git-repository/src/config/snapshot/access.rs

@@ -88,13 +88,17 @@ impl<'repo> Snapshot<'repo> {
 
 /// Utilities
 impl<'repo> SnapshotMut<'repo> {
-    /// Apply configuration values of the form `core.abbrev=5` or `remote.origin.url = foo` or `core.bool-implicit-true`
-    /// to the repository configuration, marked with [source CLI][git_config::Source::Cli].
-    pub fn apply_cli_overrides(
+    /// Append configuration values of the form `core.abbrev=5` or `remote.origin.url = foo` or `core.bool-implicit-true`
+    /// to the end of the repository configuration, with each section marked with the given `source`.
+    ///
+    /// Note that doing so applies the configuration at the very end, so it will always override what came before it
+    /// even though the `source` is of lower priority as what's there.
+    pub fn append_config(
         &mut self,
         values: impl IntoIterator<Item = impl AsRef<BStr>>,
+        source: git_config::Source,
     ) -> Result<&mut Self, crate::config::overrides::Error> {
-        crate::config::overrides::apply(&mut self.config, values, git_config::Source::Cli)?;
+        crate::config::overrides::append(&mut self.config, values, source)?;
         Ok(self)
     }
     /// Apply all changes made to this instance.

git-repository/src/open.rs

@@ -75,7 +75,8 @@ pub struct Options {
     pub(crate) lossy_config: Option<bool>,
     pub(crate) lenient_config: bool,
     pub(crate) bail_if_untrusted: bool,
-    pub(crate) config_overrides: Vec<BString>,
+    pub(crate) api_config_overrides: Vec<BString>,
+    pub(crate) cli_config_overrides: Vec<BString>,
     /// Internal to pass an already obtained CWD on to where it may also be used. This avoids the CWD being queried more than once per repo.
     pub(crate) current_dir: Option<PathBuf>,
 }
@@ -91,7 +92,8 @@ impl Default for Options {
             lossy_config: None,
             lenient_config: true,
             bail_if_untrusted: false,
-            config_overrides: Vec::new(),
+            api_config_overrides: Vec::new(),
+            cli_config_overrides: Vec::new(),
             current_dir: None,
         }
     }
@@ -138,7 +140,15 @@ impl Options {
     /// as the configuration is initialized to allow affecting the repository instantiation phase, both on disk or when opening.
     /// The configuration is marked with [source API][git_config::Source::Api].
     pub fn config_overrides(mut self, values: impl IntoIterator<Item = impl Into<BString>>) -> Self {
-        self.config_overrides = values.into_iter().map(Into::into).collect();
+        self.api_config_overrides = values.into_iter().map(Into::into).collect();
+        self
+    }
+
+    /// Set configuration values of the form `core.abbrev=5` or `remote.origin.url = foo` or `core.bool-implicit-true` for application
+    /// as CLI overrides to the repository configuration, marked with [source CLI][git_config::Source::Cli].
+    /// These are equivalent to CLI overrides passed with `-c` in `git`, for example.
+    pub fn cli_overrides(mut self, values: impl IntoIterator<Item = impl Into<BString>>) -> Self {
+        self.cli_config_overrides = values.into_iter().map(Into::into).collect();
         self
     }
 
@@ -241,7 +251,8 @@ impl git_sec::trust::DefaultForLevel for Options {
                 lossy_config: None,
                 bail_if_untrusted: false,
                 lenient_config: true,
-                config_overrides: Vec::new(),
+                api_config_overrides: Vec::new(),
+                cli_config_overrides: Vec::new(),
                 current_dir: None,
             },
             git_sec::Trust::Reduced => Options {
@@ -253,7 +264,8 @@ impl git_sec::trust::DefaultForLevel for Options {
                 bail_if_untrusted: false,
                 lenient_config: true,
                 lossy_config: None,
-                config_overrides: Vec::new(),
+                api_config_overrides: Vec::new(),
+                cli_config_overrides: Vec::new(),
                 current_dir: None,
             },
         }
@@ -361,7 +373,8 @@ impl ThreadSafeRepository {
             lenient_config,
             bail_if_untrusted,
             permissions: Permissions { ref env, config },
-            ref config_overrides,
+            ref api_config_overrides,
+            ref cli_config_overrides,
             ref current_dir,
         } = options;
         let current_dir = current_dir.as_deref().expect("BUG: current_dir must be set by caller");
@@ -402,7 +415,8 @@ impl ThreadSafeRepository {
             env.clone(),
             config,
             lenient_config,
-            config_overrides,
+            api_config_overrides,
+            cli_config_overrides,
         )?;
 
         if bail_if_untrusted && git_dir_trust != git_sec::Trust::Full {
@@ -571,7 +585,7 @@ mod tests {
     #[test]
     fn size_of_options() {
         let actual = std::mem::size_of::<Options>();
-        let limit = 140;
+        let limit = 160;
         assert!(
             actual <= limit,
             "{} <= {}: size shouldn't change without us knowing (on windows, it's bigger)",

git-repository/tests/repository/config/config_snapshot/mod.rs

@@ -84,12 +84,15 @@ fn values_are_set_in_memory_only() {
 #[test]
 fn apply_cli_overrides() -> crate::Result {
     let mut repo = named_repo("make_config_repo.sh").unwrap();
-    repo.config_snapshot_mut().apply_cli_overrides([
-        "a.b=c",
-        "remote.origin.url = url",
-        "implicit.bool-true",
-        "implicit.bool-false = ",
-    ])?;
+    repo.config_snapshot_mut().append_config(
+        [
+            "a.b=c",
+            "remote.origin.url = url",
+            "implicit.bool-true",
+            "implicit.bool-false = ",
+        ],
+        git_config::Source::Cli,
+    )?;
 
     let config = repo.config_snapshot();
     assert_eq!(config.string("a.b").expect("present").as_ref(), "c");