refactor(config): Set User config based on Git config instead of the opposite

Author: Ishirui

src/dda/cli/base.py

@@ -137,14 +137,17 @@ def __exit__(
             from dda.cli import START_TIME, START_TIMESTAMP
             from dda.utils.platform import join_command_args
 
+            username = app.config.user.name if app.config.user.name != "auto" else app.tools.git.author_name
+            email = app.config.user.email if app.config.user.email != "auto" else app.tools.git.author_email
+
             metadata = {
                 "cli.command": join_command_args(sys.argv[1:]),
                 "cli.exit_code": str(exit_code),
-                "author.name": app.config.user.name,
-                "author.emails": app.config.user.emails,
+                "author.name": username,
+                "author.email": email,
                 # TODO: Remove this once the new keys are fully rolled out
-                "git.author.name": app.config.user.name,
-                "git.author.email": app.config.user.emails[0],
+                "git.author.name": username,
+                "git.author.email": email,
             }
             if os.environ.get("PRE_COMMIT") == "1":
                 metadata["exec.source"] = "pre-commit"

src/dda/config/model/tools.py

@@ -21,21 +21,43 @@ class BazelConfig(Struct, frozen=True, forbid_unknown_fields=True):
     managed: bool | Literal["auto"] = "auto"
 
 
+def _get_name_from_git() -> str:
+    from os import environ
+
+    from dda.tools.git import Git
+    from dda.utils.process import static_capture
+
+    if name := environ.get(Git.AUTHOR_NAME_ENV_VAR):
+        return name
+
+    return static_capture(["git", "config", "--global", "--get", "user.name"]).strip()
+
+
+def _get_email_from_git() -> str:
+    from os import environ
+
+    from dda.tools.git import Git
+    from dda.utils.process import static_capture
+
+    if email := environ.get(Git.AUTHOR_EMAIL_ENV_VAR):
+        return email
+
+    return static_capture(["git", "config", "--global", "--get", "user.email"]).strip()
+
+
 class GitConfig(Struct, frozen=True):
     """
     /// tab | :octicons-file-code-16: config.toml
     ```toml
     [tools.git]
-    author_details = "inherit"
+    name = "U.N. Owen"
+    email = "void@some.where"
     ```
-    `author_details` can be either:
-    - `inherit`: The author name and email will be inherited from the main 'user' config (see [UserConfig](./user.md))
-    > The first email will be used if multiple emails are found.
-    - `system`: The author name and email will be inherited from the system git config
     ///
     """
 
-    author_details: Literal["inherit", "system"] = "system"
+    author_name: str = field(default_factory=_get_name_from_git)
+    author_email: str = field(default_factory=_get_email_from_git)
 
 
 class ToolsConfig(Struct, frozen=True):

src/dda/config/model/user.py

@@ -3,31 +3,7 @@
 # SPDX-License-Identifier: MIT
 from __future__ import annotations
 
-from msgspec import Struct, field
-
-
-def _get_name_from_git() -> str:
-    from os import environ
-
-    from dda.tools.git import Git
-    from dda.utils.process import static_capture
-
-    if name := environ.get(Git.AUTHOR_NAME_ENV_VAR):
-        return name
-
-    return static_capture(["git", "config", "--global", "--get", "user.name"]).strip()
-
-
-def _get_emails_from_git() -> list[str]:
-    from os import environ
-
-    from dda.tools.git import Git
-    from dda.utils.process import static_capture
-
-    if email := environ.get(Git.AUTHOR_EMAIL_ENV_VAR):
-        return [email]
-
-    return [static_capture(["git", "config", "--global", "--get", "user.email"]).strip()]
+from msgspec import Struct
 
 
 class UserConfig(Struct, frozen=True):
@@ -36,12 +12,13 @@ class UserConfig(Struct, frozen=True):
     ```toml
     [user]
     name = "U.N. Owen"
-    emails = ["void@some.where", "other@some.where"]
+    email = "void@some.where"
     ```
-    > The first email will be used for setting git author email if multiple emails are found.
+    These values will be used for dda-related functionality, such as telemetry.
+    Both `email` and `name` can be set to `auto`, in which case they will be equal to the values in [tools.git]
     ///
     """
 
-    # Default username and email are fetched from git config
-    name: str = field(default_factory=_get_name_from_git)
-    emails: list[str] = field(default_factory=_get_emails_from_git)
+    # Default username and email are equal to the values in [tools.git]
+    name: str = "auto"
+    email: str = "auto"

src/dda/tools/git.py

@@ -26,12 +26,10 @@ class Git(Tool):
     AUTHOR_EMAIL_ENV_VAR = "GIT_AUTHOR_EMAIL"
 
     def env_vars(self) -> dict[str, str]:
-        if self.app.config.tools.git.author_details == "inherit":
-            return {
-                self.AUTHOR_NAME_ENV_VAR: self.app.config.user.name.strip(),
-                self.AUTHOR_EMAIL_ENV_VAR: self.app.config.user.emails[0].strip(),
-            }
-        return {}
+        return {
+            self.AUTHOR_NAME_ENV_VAR: self.app.config.tools.git.author_name.strip(),
+            self.AUTHOR_EMAIL_ENV_VAR: self.app.config.tools.git.author_email.strip(),
+        }
 
     @cached_property
     def path(self) -> str:

tests/cli/config/test_show.py

@@ -11,8 +11,8 @@ def test_default_scrubbed(dda, config_file, helpers, default_cache_dir, default_
 
     # The default name and email are queried from the global git config on config initialization
     # We override them to make sure we have a known value
-    config_file.data["user"]["name"] = "Foo Bar"
-    config_file.data["user"]["emails"] = ["foo@bar.baz"]
+    config_file.data["tools"]["git"]["author_name"] = "Foo Bar"
+    config_file.data["tools"]["git"]["author_email"] = "foo@bar.baz"
     config_file.save()
 
     result = dda("config", "show")
@@ -35,7 +35,8 @@ def test_default_scrubbed(dda, config_file, helpers, default_cache_dir, default_
         managed = "auto"
 
         [tools.git]
-        author_details = "system"
+        author_name = "Foo Bar"
+        author_email = "foo@bar.baz"
 
         [storage]
         data = "{default_data_directory}"
@@ -46,8 +47,8 @@ def test_default_scrubbed(dda, config_file, helpers, default_cache_dir, default_
         token = "*****"
 
         [user]
-        name = "Foo Bar"
-        emails = ["foo@bar.baz"]
+        name = "auto"
+        email = "auto"
 
         [terminal]
         verbosity = 0
@@ -73,10 +74,10 @@ def test_default_scrubbed(dda, config_file, helpers, default_cache_dir, default_
 def test_reveal(dda, config_file, helpers, default_cache_dir, default_data_dir):
     config_file.data["github"]["auth"] = {"user": "foo", "token": "bar"}
 
-    # The default name and email are queried from the global git config on config initialization
+    # The default git author name and email are queried from the global git config on config initialization
     # We override them to make sure we have a known value
-    config_file.data["user"]["name"] = "Foo Bar"
-    config_file.data["user"]["emails"] = ["foo@bar.baz"]
+    config_file.data["tools"]["git"]["author_name"] = "Foo Bar"
+    config_file.data["tools"]["git"]["author_email"] = "foo@bar.baz"
     config_file.save()
 
     result = dda("config", "show", "-a")
@@ -99,7 +100,8 @@ def test_reveal(dda, config_file, helpers, default_cache_dir, default_data_dir):
         managed = "auto"
 
         [tools.git]
-        author_details = "system"
+        author_name = "Foo Bar"
+        author_email = "foo@bar.baz"
 
         [storage]
         data = "{default_data_directory}"
@@ -110,8 +112,8 @@ def test_reveal(dda, config_file, helpers, default_cache_dir, default_data_dir):
         token = "bar"
 
         [user]
-        name = "Foo Bar"
-        emails = ["foo@bar.baz"]
+        name = "auto"
+        email = "auto"
 
         [terminal]
         verbosity = 0

tests/tools/conftest.py

@@ -1,4 +1,3 @@
-import os
 import shutil
 from collections.abc import Callable, Generator
 
@@ -28,7 +27,7 @@ def clear_cached_config(app: Application) -> None:
 
 # Initialize a dummy repo in a temporary directory for the tests to use
 @pytest.fixture
-def temp_repo(app: Application, temp_dir: Path, set_system_git_author: None) -> Generator[Path, None, None]:  # noqa: ARG001
+def temp_repo(app: Application, temp_dir: Path, set_git_author: None) -> Generator[Path, None, None]:  # noqa: ARG001
     git: Git = app.tools.git
     repo_path = temp_dir / "dummy-repo"
     repo_path.mkdir()  # Don't do exist_ok, the directory should not exist
@@ -42,52 +41,22 @@ def temp_repo(app: Application, temp_dir: Path, set_system_git_author: None) ->
 
 
 @pytest.fixture
-def set_system_git_author(app: Application) -> Generator[None, None, None]:
-    # Make sure the config is set to "system" mode, so that the author details are not inherited from the user config
-    old_config = app.config_file.data["tools"]["git"]["author_details"]
-    app.config_file.data["tools"]["git"]["author_details"] = "system"
-    app.config_file.save()
-    clear_cached_config(app)
+def set_git_author(app: Application) -> Generator[None, None, None]:
+    """
+    Set the git author name and email to "Test Runner" and "test.runner@example.com" respectively.
+    This is done by setting the values in the config file.
+    Any commits made by `dda` will use these values, but any calls to `git config` will still return the global git config values.
+    """
+    old_name = app.config_file.data["tools"]["git"]["author_name"]
+    old_email = app.config_file.data["tools"]["git"]["author_email"]
+    app.config_file.data["tools"]["git"]["author_name"] = "Test Runner"
+    app.config_file.data["tools"]["git"]["author_email"] = "test.runner@example.com"
 
-    old_env_author = os.environ.pop(Git.AUTHOR_NAME_ENV_VAR, default=None)
-    old_env_email = os.environ.pop(Git.AUTHOR_EMAIL_ENV_VAR, default=None)
-    old_author_name = app.tools.git.capture(["config", "--get", "user.name"], check=False)
-    old_author_email = app.tools.git.capture(["config", "--get", "user.email"], check=False)
-    app.tools.git.run(["config", "--global", "user.name", "Test Runner"])
-    app.tools.git.run(["config", "--global", "user.email", "test.runner@example.com"])
-    yield
-
-    app.tools.git.run(["config", "--global", "--unset", "user.name"])
-    app.tools.git.run(["config", "--global", "--unset", "user.email"])
-    if old_author_name:
-        app.tools.git.run(["config", "--global", "user.name", old_author_name.strip()])
-    if old_author_email:
-        app.tools.git.run(["config", "--global", "user.email", old_author_email.strip()])
-    if old_env_author:
-        os.environ[Git.AUTHOR_NAME_ENV_VAR] = old_env_author.strip()
-    if old_env_email:
-        os.environ[Git.AUTHOR_EMAIL_ENV_VAR] = old_env_email.strip()
-
-    app.config_file.data["tools"]["git"]["author_details"] = old_config
-    app.config_file.save()
-    clear_cached_config(app)
-
-
-@pytest.fixture
-def set_inherit_git_author(app: Application) -> Generator[None, None, None]:
-    old_config = app.config_file.data["tools"]["git"]["author_details"]
-    app.config_file.data["tools"]["git"]["author_details"] = "inherit"
-
-    old_name = app.config_file.data["user"]["name"]
-    old_emails = app.config_file.data["user"]["emails"]
-    app.config_file.data["user"]["name"] = "Test Runner"
-    app.config_file.data["user"]["emails"] = ["test.runner@example.com"]
     app.config_file.save()
     clear_cached_config(app)
     yield
-    app.config_file.data["tools"]["git"]["author_details"] = old_config
-    app.config_file.data["user"]["name"] = old_name
-    app.config_file.data["user"]["emails"] = old_emails
+    app.config_file.data["tools"]["git"]["author_name"] = old_name
+    app.config_file.data["tools"]["git"]["author_email"] = old_email
     app.config_file.save()
     clear_cached_config(app)
 

tests/tools/test_git.py

@@ -49,18 +49,12 @@ def test_basic(
         assert f"Initial commit: {random_key}" in app.tools.git.capture(["log", "-1", "--oneline"])
 
 
-def test_author_details_from_system(app: Application, set_system_git_author: None) -> None:  # noqa: ARG001
+def test_author_details(app: Application, set_git_author: None) -> None:  # noqa: ARG001
     clear_cached_config(app)
     assert app.tools.git.author_name == "Test Runner"
     assert app.tools.git.author_email == "test.runner@example.com"
 
 
-def test_author_details_inherit(app: Application, set_inherit_git_author: None) -> None:  # noqa: ARG001
-    clear_cached_config(app)
-    assert app.tools.git.author_name == app.config.user.name
-    assert app.tools.git.author_email == app.config.user.emails[0]
-
-
 def test_get_remote_details(app: Application, temp_repo_with_remote: Path) -> None:
     with temp_repo_with_remote.as_cwd():
         assert app.tools.git.get_remote_details() == ("foo", "bar", "https://github.com/foo/bar")
@@ -77,7 +71,9 @@ def test_get_head_commit(
 
 
 def test_get_commit_details(
-    app: Application, temp_repo: Path, create_commit_dummy_file: Callable[[Path | str, str, str], None]
+    app: Application,
+    temp_repo: Path,
+    create_commit_dummy_file: Callable[[Path | str, str, str], None],
 ) -> None:
     with temp_repo.as_cwd():
         create_commit_dummy_file("dummy", "dummy content", "Initial commit")