Merge pull request #528 from Mic92/git-author

Always set git author/committer

Author: Mic92

nixpkgs_review/builddir.py

@@ -5,8 +5,9 @@
 from tempfile import TemporaryDirectory
 from typing import Any, Union
 
+from . import git
 from .overlay import Overlay
-from .utils import sh, warn
+from .utils import warn
 
 
 class DisableKeyboardInterrupt:
@@ -89,7 +90,7 @@ def __exit__(
 
         with DisableKeyboardInterrupt():
             if Path.exists(self.worktree_dir / ".git"):
-                res = sh(["git", "worktree", "remove", "-f", str(self.worktree_dir)])
+                res = git.run(["worktree", "remove", "-f", str(self.worktree_dir)])
                 if res.returncode != 0:
                     warn(
                         f"Failed to remove worktree at {self.worktree_dir}. Please remove it manually. Git failed with: {res.returncode}"

nixpkgs_review/cli/rev.py

@@ -1,16 +1,16 @@
 import argparse
 from pathlib import Path
 
+from nixpkgs_review import git
 from nixpkgs_review.allow import AllowedFeatures
 from nixpkgs_review.buildenv import Buildenv
 from nixpkgs_review.review import review_local_revision
-from nixpkgs_review.utils import verify_commit_hash
 
 
 def rev_command(args: argparse.Namespace) -> Path:
     allow = AllowedFeatures(args.allow)
     with Buildenv(allow.aliases, args.extra_nixpkgs_config) as nixpkgs_config:
-        commit = verify_commit_hash(args.commit)
+        commit = git.verify_commit_hash(args.commit)
         return review_local_revision(
             f"rev-{commit}",
             args,

nixpkgs_review/cli/wip.py

@@ -1,17 +1,17 @@
 import argparse
 from pathlib import Path
 
+from nixpkgs_review import git
 from nixpkgs_review.allow import AllowedFeatures
 from nixpkgs_review.buildenv import Buildenv
 from nixpkgs_review.review import review_local_revision
-from nixpkgs_review.utils import verify_commit_hash
 
 
 def wip_command(args: argparse.Namespace) -> Path:
     allow = AllowedFeatures(args.allow)
     with Buildenv(allow.aliases, args.extra_nixpkgs_config) as nixpkgs_config:
         return review_local_revision(
-            f"rev-{verify_commit_hash('HEAD')}-dirty",
+            f"rev-{git.verify_commit_hash('HEAD')}-dirty",
             args,
             allow,
             nixpkgs_config,

nixpkgs_review/git.py

@@ -0,0 +1,41 @@
+import subprocess
+from pathlib import Path
+
+from .errors import NixpkgsReviewError
+from .utils import sh
+
+
+def run(
+    command: list[str],
+    cwd: Path | str | None = None,
+    stdin: str | None = None,
+    stdout: int | None = None,
+    stderr: int | None = None,
+    quiet: bool = False,
+) -> subprocess.CompletedProcess[str]:
+    """Run a git command with nixpkgs-review identity."""
+    env = {
+        "GIT_AUTHOR_NAME": "nixpkgs-review",
+        "GIT_AUTHOR_EMAIL": "nixpkgs-review@example.com",
+        "GIT_COMMITTER_NAME": "nixpkgs-review",
+        "GIT_COMMITTER_EMAIL": "nixpkgs-review@example.com",
+    }
+    return sh(
+        ["git", *command],
+        cwd=cwd,
+        env=env,
+        stdin=stdin,
+        stdout=stdout,
+        stderr=stderr,
+        quiet=quiet,
+    )
+
+
+def verify_commit_hash(commit: str) -> str:
+    """Verify and return full commit hash."""
+    cmd = ["rev-parse", "--verify", commit]
+    proc = run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, quiet=True)
+    if proc.returncode != 0:
+        error_message = f"Failed to verify commit hash '{commit}': {proc.stderr or 'Invalid commit'}"
+        raise NixpkgsReviewError(error_message)
+    return proc.stdout.strip()

nixpkgs_review/review.py

@@ -18,6 +18,7 @@
 from typing import IO
 from xml.etree import ElementTree as ET
 
+from . import git
 from .allow import AllowedFeatures
 from .builddir import Builddir
 from .errors import NixpkgsReviewError
@@ -276,41 +277,36 @@ def _display_pr_info(self, pr: dict, pr_number: int) -> None:
         print(f"{'=' * 80}\n")
 
     def git_merge(self, commit: str) -> None:
-        res = sh(
-            ["git", "merge", "--no-commit", "--no-ff", commit], cwd=self.worktree_dir()
+        res = git.run(
+            ["merge", "--no-commit", "--no-ff", commit], cwd=self.worktree_dir()
         )
         if res.returncode != 0:
             msg = f"Failed to merge {commit} into {self.worktree_dir()}. git merge failed with exit code {res.returncode}"
             raise NixpkgsReviewError(msg)
 
     def git_checkout(self, commit: str) -> None:
-        res = sh(["git", "checkout", commit], cwd=self.worktree_dir())
+        res = git.run(["checkout", commit], cwd=self.worktree_dir())
         if res.returncode != 0:
             msg = f"Failed to checkout {commit} in {self.worktree_dir()}. git checkout failed with exit code {res.returncode}"
             raise NixpkgsReviewError(msg)
 
     def apply_unstaged(self, staged: bool = False) -> None:
         args = [
-            "git",
             "--no-pager",
             "diff",
             "--no-ext-diff",
             "--src-prefix=a/",
             "--dst-prefix=b/",
         ]
         args.extend(["--staged"] if staged else [])
-        with subprocess.Popen(args, stdout=subprocess.PIPE) as diff_proc:
-            assert diff_proc.stdout
-            diff = diff_proc.stdout.read()
+        diff = git.run(args, stdout=subprocess.PIPE).stdout
 
         if not diff:
             info("No diff detected, stopping review...")
             sys.exit(0)
 
         info("Applying `nixpkgs` diff...")
-        result = subprocess.run(
-            ["git", "apply"], cwd=self.worktree_dir(), input=diff, check=False
-        )
+        result = git.run(["apply"], cwd=self.worktree_dir(), stdin=diff)
 
         if result.returncode != 0:
             warn(f"Failed to apply diff in {self.worktree_dir()}")
@@ -386,7 +382,7 @@ def build_commit(
         return self.build(changed_attrs, self.build_args)
 
     def git_worktree(self, commit: str) -> None:
-        res = sh(["git", "worktree", "add", self.worktree_dir(), commit])
+        res = git.run(["worktree", "add", self.worktree_dir(), commit])
         if res.returncode != 0:
             msg = f"Failed to add worktree for {commit} in {self.worktree_dir()}. git worktree failed with exit code {res.returncode}"
             raise NixpkgsReviewError(msg)

nixpkgs_review/utils.py

@@ -1,4 +1,5 @@
 import functools
+import os
 import shlex
 import shutil
 import subprocess
@@ -36,16 +37,30 @@ def to_link(uri: str, text: str) -> str:
 
 
 def sh(
-    command: list[str], cwd: Path | str | None = None
+    command: list[str],
+    cwd: Path | str | None = None,
+    env: dict[str, str] | None = None,
+    stdin: str | None = None,
+    stdout: int | None = None,
+    stderr: int | None = None,
+    quiet: bool = False,
 ) -> "subprocess.CompletedProcess[str]":
-    info("$ " + shlex.join(command))
-    return subprocess.run(command, cwd=cwd, text=True, check=False)
-
-
-def verify_commit_hash(commit: str) -> str:
-    cmd = ["git", "rev-parse", "--verify", commit]
-    proc = subprocess.run(cmd, check=True, stdout=subprocess.PIPE, text=True)
-    return proc.stdout.strip()
+    if not quiet:
+        info("$ " + shlex.join(command))
+    if env is not None:
+        full_env = os.environ.copy()
+        full_env.update(env)
+        env = full_env
+    return subprocess.run(
+        command,
+        cwd=cwd,
+        text=True,
+        check=False,
+        env=env,
+        input=stdin,
+        stdout=stdout,
+        stderr=stderr,
+    )
 
 
 def escape_attr(attr: str) -> str:

pyproject.toml

@@ -43,8 +43,6 @@ lint.ignore = [
   # fixmes
   "FIX",
 
-  # Missing type annotation for `self` in method
-  "ANN101",
   # Dynamically typed expressions (typing.Any)
   "ANN401",
   # Trailing comma missing