change: represent GIT_(COMMITTER|AUTHOR)_(NAME|EMAIL|DATE) with git…

… configuration.

That way it becomes more obvious where values are coming from.

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

@@ -60,7 +60,7 @@ impl Cache {
 
     pub(crate) fn personas(&self) -> &identity::Personas {
         self.personas
-            .get_or_init(|| identity::Personas::from_config_and_env(&self.resolved, self.git_prefix))
+            .get_or_init(|| identity::Personas::from_config_and_env(&self.resolved))
     }
 
     pub(crate) fn url_rewrite(&self) -> &remote::url::Rewrite {

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

@@ -31,6 +31,7 @@ impl Cache {
             xdg_config_home: xdg_config_home_env,
             ssh_prefix: _,
             http_transport,
+            identity,
         }: repository::permissions::Environment,
         repository::permissions::Config {
             git_binary: use_installation,
@@ -132,7 +133,7 @@ impl Cache {
                         source: git_config::Source::Api,
                     })?;
             }
-            apply_environment_overrides(&mut globals, *git_prefix, http_transport)?;
+            apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity)?;
             globals
         };
 
@@ -163,7 +164,6 @@ impl Cache {
             #[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
             url_scheme: Default::default(),
             diff_algorithm: Default::default(),
-            git_prefix,
         })
     }
 
@@ -243,6 +243,7 @@ fn apply_environment_overrides(
     config: &mut File<'static>,
     git_prefix: Permission,
     http_transport: Permission,
+    identity: Permission,
 ) -> Result<(), Error> {
     fn var_as_bstring(var: &str, perm: Permission) -> Option<BString> {
         perm.check_opt(var)
@@ -298,13 +299,106 @@ fn apply_environment_overrides(
         }
     }
 
+    {
+        let mut section = env_override
+            .new_section("gitoxide", Some(Cow::Borrowed("committer".into())))
+            .expect("statically known valid section name");
+
+        for (var, key) in [
+            ("GIT_COMMITTER_NAME", "nameFallback"),
+            ("GIT_COMMITTER_EMAIL", "emailFallback"),
+        ] {
+            if let Some(value) = var_as_bstring(var, git_prefix) {
+                section.push_with_comment(
+                    key.try_into().expect("statically known to be valid"),
+                    Some(value.as_ref()),
+                    format!("from {var}").as_str(),
+                );
+            }
+        }
+
+        if section.num_values() == 0 {
+            let id = section.id();
+            env_override.remove_section_by_id(id);
+        }
+    }
+
+    {
+        let mut section = env_override
+            .new_section("gitoxide", Some(Cow::Borrowed("author".into())))
+            .expect("statically known valid section name");
+
+        for (var, key) in [
+            ("GIT_AUTHOR_NAME", "nameFallback"),
+            ("GIT_AUTHOR_EMAIL", "emailFallback"),
+        ] {
+            if let Some(value) = var_as_bstring(var, git_prefix) {
+                section.push_with_comment(
+                    key.try_into().expect("statically known to be valid"),
+                    Some(value.as_ref()),
+                    format!("from {var}").as_str(),
+                );
+            }
+        }
+
+        if section.num_values() == 0 {
+            let id = section.id();
+            env_override.remove_section_by_id(id);
+        }
+    }
+
+    {
+        let mut section = env_override
+            .new_section("gitoxide", Some(Cow::Borrowed("commit".into())))
+            .expect("statically known valid section name");
+
+        for (var, key) in [
+            ("GIT_COMMITTER_DATE", "committerDate"),
+            ("GIT_AUTHOR_DATE", "authorDate"),
+        ] {
+            if let Some(value) = var_as_bstring(var, git_prefix) {
+                section.push_with_comment(
+                    key.try_into().expect("statically known to be valid"),
+                    Some(value.as_ref()),
+                    format!("from {var}").as_str(),
+                );
+            }
+        }
+
+        if section.num_values() == 0 {
+            let id = section.id();
+            env_override.remove_section_by_id(id);
+        }
+    }
+
     {
         let mut section = env_override
             .new_section("gitoxide", Some(Cow::Borrowed("allow".into())))
             .expect("statically known valid section name");
 
         for (var, key) in [("GIT_PROTOCOL_FROM_USER", "protocolFromUser")] {
-            if let Some(value) = var_as_bstring(var, http_transport) {
+            if let Some(value) = var_as_bstring(var, git_prefix) {
+                section.push_with_comment(
+                    key.try_into().expect("statically known to be valid"),
+                    Some(value.as_ref()),
+                    format!("from {var}").as_str(),
+                );
+            }
+        }
+
+        if section.num_values() == 0 {
+            let id = section.id();
+            env_override.remove_section_by_id(id);
+        }
+    }
+
+    {
+        let mut section = env_override
+            .new_section("gitoxide", Some(Cow::Borrowed("user".into())))
+            .expect("statically known valid section name");
+
+        for (var, key) in [("EMAIL", "emailFallback")] {
+            if let Some(value) = var_as_bstring(var, identity) {
                 section.push_with_comment(
                     key.try_into().expect("statically known to be valid"),
                     Some(value.as_ref()),
@@ -328,7 +422,7 @@ fn apply_environment_overrides(
             ("GIT_NO_REPLACE_OBJECTS", "noReplace"),
             ("GIT_REPLACE_REF_BASE", "replaceRefBase"),
         ] {
-            if let Some(value) = var_as_bstring(var, http_transport) {
+            if let Some(value) = var_as_bstring(var, git_prefix) {
                 section.push_with_comment(
                     key.try_into().expect("statically known to be valid"),
                     Some(value.as_ref()),

git-repository/src/config/mod.rs

@@ -208,7 +208,5 @@ pub(crate) struct Cache {
     xdg_config_home_env: git_sec::Permission,
     /// Define how we can use values obtained with `xdg_config(…)`. and its `HOME` variable.
     home_env: git_sec::Permission,
-    /// How to use git-prefixed environment variables
-    git_prefix: git_sec::Permission,
     // TODO: make core.precomposeUnicode available as well.
 }

git-repository/src/repository/identity.rs

@@ -1,6 +1,6 @@
-use std::{borrow::Cow, time::SystemTime};
+use std::time::SystemTime;
 
-use crate::bstr::BString;
+use crate::bstr::{BString, ByteSlice};
 
 /// Identity handling.
 impl crate::Repository {
@@ -105,43 +105,44 @@ pub(crate) struct Personas {
 }
 
 impl Personas {
-    pub fn from_config_and_env(config: &git_config::File<'_>, git_env: git_sec::Permission) -> Self {
-        fn env_var(name: &str) -> Option<BString> {
-            std::env::var_os(name).map(|value| git_path::into_bstr(Cow::Owned(value.into())).into_owned())
-        }
-        fn entity_in_section(name: &str, config: &git_config::File<'_>) -> (Option<BString>, Option<BString>) {
-            config
+    pub fn from_config_and_env(config: &git_config::File<'_>) -> Self {
+        fn entity_in_section(
+            name: &str,
+            config: &git_config::File<'_>,
+            fallback: bool,
+        ) -> (Option<BString>, Option<BString>) {
+            let fallback = fallback
+                .then(|| config.section("gitoxide", Some(name.into())).ok())
+                .flatten();
+            let (name, email) = config
                 .section(name, None)
-                .map(|section| {
-                    (
-                        section.value("name").map(|v| v.into_owned()),
-                        section.value("email").map(|v| v.into_owned()),
-                    )
-                })
-                .unwrap_or_default()
+                .map(|section| (section.value("name"), section.value("email")))
+                .unwrap_or_default();
+            (
+                name.or_else(|| fallback.as_ref().and_then(|s| s.value("nameFallback")))
+                    .map(|v| v.into_owned()),
+                email
+                    .or_else(|| fallback.as_ref().and_then(|s| s.value("emailFallback")))
+                    .map(|v| v.into_owned()),
+            )
         }
+        let now = SystemTime::now();
+        let parse_date = |key: &str| -> Option<git_date::Time> {
+            config.string_by_key(key).and_then(|date| {
+                date.to_str()
+                    .ok()
+                    .and_then(|date| git_date::parse(date, Some(now)).ok())
+            })
+        };
 
-        let (mut committer_name, mut committer_email) = entity_in_section("committer", config);
-        let mut committer_date = None;
-        let (mut author_name, mut author_email) = entity_in_section("author", config);
-        let mut author_date = None;
-        let (user_name, mut user_email) = entity_in_section("user", config);
-
-        if git_env.eq(&git_sec::Permission::Allow) {
-            committer_name = committer_name.or_else(|| env_var("GIT_COMMITTER_NAME"));
-            committer_email = committer_email.or_else(|| env_var("GIT_COMMITTER_EMAIL"));
-            committer_date = std::env::var("GIT_COMMITTER_DATE")
-                .ok()
-                .and_then(|date| git_date::parse(&date, Some(SystemTime::now())).ok());
+        let (committer_name, committer_email) = entity_in_section("committer", config, true);
+        let (author_name, author_email) = entity_in_section("author", config, true);
+        let (user_name, mut user_email) = entity_in_section("user", config, false);
 
-            author_name = author_name.or_else(|| env_var("GIT_AUTHOR_NAME"));
-            author_email = author_email.or_else(|| env_var("GIT_AUTHOR_EMAIL"));
-            author_date = std::env::var("GIT_AUTHOR_DATE")
-                .ok()
-                .and_then(|date| git_date::parse(&date, Some(SystemTime::now())).ok());
+        let committer_date = parse_date("gitoxide.commit.committerDate");
+        let author_date = parse_date("gitoxide.commit.authorDate");
 
-            user_email = user_email.or_else(|| env_var("EMAIL")); // NOTE: we don't have permission for this specific one…
-        }
+        user_email = user_email.or_else(|| config.string_by_key("gitoxide.user.email").map(|v| v.into_owned()));
         Personas {
             user: Entity {
                 name: user_name,

git-repository/src/repository/permissions.rs

@@ -74,9 +74,14 @@ pub struct Environment {
     pub ssh_prefix: git_sec::Permission,
     /// Control if environment variables to configure the HTTP transport, like `http_proxy` may be used.
     ///
-    /// Note that those http-transport related environment variables prefixed with `GIT_` are falling under the
+    /// Note that http-transport related environment variables prefixed with `GIT_` are falling under the
     /// `git_prefix` permission, like `GIT_HTTP_USER_AGENT`.
     pub http_transport: git_sec::Permission,
+    /// Control if the `EMAIL` environment variables may be read.
+    ///
+    /// Note that identity related environment variables prefixed with `GIT_` are falling under the
+    /// `git_prefix` permission, like `GIT_AUTHOR_NAME`.
+    pub identity: git_sec::Permission,
 }
 
 impl Environment {
@@ -88,6 +93,7 @@ impl Environment {
             git_prefix: git_sec::Permission::Allow,
             ssh_prefix: git_sec::Permission::Allow,
             http_transport: git_sec::Permission::Allow,
+            identity: git_sec::Permission::Allow,
         }
     }
 }
@@ -133,6 +139,7 @@ impl Permissions {
                     ssh_prefix: deny,
                     git_prefix: deny,
                     http_transport: deny,
+                    identity: deny,
                 }
             },
         }

git-repository/tests/repository/config/identity.rs

@@ -21,6 +21,10 @@ fn author_and_committer_and_fallback() {
             .set("GIT_AUTHOR_NAME", "author")
             .set("GIT_AUTHOR_EMAIL", "author@email")
             .set("GIT_AUTHOR_DATE", "1979-02-26 18:30:00")
+            .set("GIT_COMMITTER_NAME", "commiter-overrider-unused")
+            .set("GIT_COMMITTER_EMAIL", "committer-override-unused@email")
+            .set("GIT_COMMITTER_DATE", "1980-02-26 18:30:00")
+            .set("EMAIL", "general@email-unused")
             .set("GIT_CONFIG_COUNT", "1")
             .set("GIT_CONFIG_KEY_0", "include.path")
             .set("GIT_CONFIG_VALUE_0", work_dir.join("c.config").display().to_string());

git-repository/tests/repository/open.rs

@@ -57,6 +57,7 @@ mod with_overrides {
     #[test]
     #[serial]
     fn order_from_api_and_cli_and_environment() -> crate::Result {
+        let default_date = "1979-02-26 18:30:00";
         let _env = Env::new()
             .set("GIT_HTTP_USER_AGENT", "agent-from-env")
             .set("GIT_HTTP_LOW_SPEED_LIMIT", "1")
@@ -72,7 +73,14 @@ mod with_overrides {
             .set("NO_PROXY", "no-proxy")
             .set("GIT_PROTOCOL_FROM_USER", "file-allowed")
             .set("GIT_REPLACE_REF_BASE", "refs/replace-mine")
-            .set("GIT_NO_REPLACE_OBJECTS", "no-replace");
+            .set("GIT_NO_REPLACE_OBJECTS", "no-replace")
+            .set("GIT_COMMITTER_NAME", "committer name")
+            .set("GIT_COMMITTER_EMAIL", "committer email")
+            .set("GIT_COMMITTER_DATE", default_date)
+            .set("GIT_AUTHOR_NAME", "author name")
+            .set("GIT_AUTHOR_EMAIL", "author email")
+            .set("GIT_AUTHOR_DATE", default_date)
+            .set("EMAIL", "user email");
         let mut opts = git::open::Options::isolated()
             .config_overrides([
                 "http.userAgent=agent-from-api",
@@ -86,6 +94,7 @@ mod with_overrides {
             ]);
         opts.permissions.env.git_prefix = Permission::Allow;
         opts.permissions.env.http_transport = Permission::Allow;
+        opts.permissions.env.identity = Permission::Allow;
         let repo = named_subrepo_opts("make_config_repos.sh", "http-config", opts)?;
         let config = repo.config_snapshot();
         assert_eq!(
@@ -154,32 +163,30 @@ mod with_overrides {
                 cow_bstr(if cfg!(windows) { "no-proxy" } else { "no-proxy-lower" })
             ]
         );
-        assert_eq!(
-            config
-                .strings_by_key("gitoxide.http.verbose")
-                .expect("at least one value"),
-            [cow_bstr("true")]
-        );
-        assert_eq!(
-            config
-                .strings_by_key("gitoxide.allow.protocolFromUser")
-                .expect("at least one value"),
-            [cow_bstr("file-allowed")]
-        );
-        assert_eq!(
-            config
-                .string_by_key("gitoxide.objects.noReplace")
-                .expect("at least one value")
-                .as_ref(),
-            "no-replace"
-        );
-        assert_eq!(
-            config
-                .string_by_key("gitoxide.objects.replaceRefBase")
-                .expect("at least one value")
-                .as_ref(),
-            "refs/replace-mine"
-        );
+        for (key, expected) in [
+            ("gitoxide.http.verbose", "true"),
+            ("gitoxide.allow.protocolFromUser", "file-allowed"),
+            ("gitoxide.objects.noReplace", "no-replace"),
+            ("gitoxide.objects.replaceRefBase", "refs/replace-mine"),
+            ("gitoxide.committer.nameFallback", "committer name"),
+            ("gitoxide.committer.emailFallback", "committer email"),
+            ("gitoxide.author.nameFallback", "author name"),
+            ("gitoxide.author.emailFallback", "author email"),
+            ("gitoxide.commit.authorDate", default_date),
+            ("gitoxide.commit.committerDate", default_date),
+            ("gitoxide.user.emailFallback", "user email"),
+        ] {
+            assert_eq!(
+                config
+                    .string_by_key(key)
+                    .unwrap_or_else(|| panic!("no value for {key}"))
+                    .as_ref(),
+                expected,
+                "{} == {}",
+                key,
+                expected
+            );
+        }
         Ok(())
     }