From 997c265cfbf030d595f2318d5e3ceddeea4393e1 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 17 Dec 2025 21:48:03 +0100 Subject: [PATCH] refactor(git): introduce GitRunError hierarchy, surface non-repo errors, and improve verification queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replace legacy GitError usage with a clearer exception hierarchy: * GitBaseError as the common root for all git-related failures * GitRunError for subprocess execution failures * GitQueryError for read-only query failures * GitCommandError for state-changing command failures * GitNotRepositoryError to explicitly signal “not a git repository” situations * Update git runner to detect “not a git repository” stderr and raise GitNotRepositoryError with rich context (cwd, command, stderr) * Refactor repository verification to use dedicated query helpers instead of ad-hoc subprocess calls: * get_remote_head_commit (ls-remote) for pull mode * get_head_commit for local mode * get_latest_signing_key (%GK) for signature verification * Add strict vs best-effort behavior in verify_repository: * Best-effort collection for reporting (does not block when no verification config exists) * Strict retrieval and explicit error messages when verification is configured * Clear failure cases when commit/signing key cannot be determined * Add new unit tests covering: * get_latest_signing_key output stripping and error wrapping * get_remote_head_commit parsing, empty output, and error wrapping * verify_repository success/failure scenarios and “do not swallow GitNotRepositoryError” * Adjust imports and exception handling across actions/commands/queries to align with GitRunError-based handling while keeping GitNotRepositoryError uncaught for debugging clarity https://chatgpt.com/share/6943173c-508c-800f-8879-af75d131c79b --- src/pkgmgr/actions/branch/close_branch.py | 6 +- src/pkgmgr/actions/branch/drop_branch.py | 4 +- src/pkgmgr/actions/branch/open_branch.py | 2 +- src/pkgmgr/actions/mirror/git_remote.py | 6 +- src/pkgmgr/actions/release/workflow.py | 6 +- src/pkgmgr/core/git/__init__.py | 4 +- src/pkgmgr/core/git/commands/__init__.py | 6 +- src/pkgmgr/core/git/commands/add.py | 4 +- src/pkgmgr/core/git/commands/add_all.py | 5 +- src/pkgmgr/core/git/commands/add_remote.py | 4 +- .../core/git/commands/add_remote_push_url.py | 4 +- src/pkgmgr/core/git/commands/branch_move.py | 5 +- src/pkgmgr/core/git/commands/checkout.py | 4 +- src/pkgmgr/core/git/commands/clone.py | 4 +- src/pkgmgr/core/git/commands/commit.py | 4 +- src/pkgmgr/core/git/commands/create_branch.py | 4 +- .../core/git/commands/delete_local_branch.py | 4 +- .../core/git/commands/delete_remote_branch.py | 4 +- src/pkgmgr/core/git/commands/fetch.py | 4 +- src/pkgmgr/core/git/commands/init.py | 5 +- src/pkgmgr/core/git/commands/merge_no_ff.py | 4 +- src/pkgmgr/core/git/commands/pull.py | 4 +- src/pkgmgr/core/git/commands/pull_args.py | 4 +- src/pkgmgr/core/git/commands/pull_ff_only.py | 4 +- src/pkgmgr/core/git/commands/push.py | 4 +- src/pkgmgr/core/git/commands/push_upstream.py | 5 +- .../core/git/commands/set_remote_url.py | 4 +- src/pkgmgr/core/git/commands/tag_annotated.py | 4 +- .../core/git/commands/tag_force_annotated.py | 4 +- src/pkgmgr/core/git/errors.py | 15 ++- src/pkgmgr/core/git/queries/__init__.py | 30 +++-- src/pkgmgr/core/git/queries/get_changelog.py | 6 +- .../core/git/queries/get_config_value.py | 6 +- .../core/git/queries/get_current_branch.py | 4 +- .../core/git/queries/get_head_commit.py | 4 +- .../core/git/queries/get_latest_commit.py | 4 +- .../git/queries/get_latest_signing_key.py | 25 ++++ .../git/queries/get_remote_head_commit.py | 33 ++++++ .../core/git/queries/get_remote_push_urls.py | 3 +- src/pkgmgr/core/git/queries/get_repo_root.py | 5 +- src/pkgmgr/core/git/queries/get_tags.py | 5 +- .../core/git/queries/get_tags_at_ref.py | 6 +- .../core/git/queries/get_upstream_ref.py | 4 +- src/pkgmgr/core/git/queries/list_remotes.py | 2 +- .../git/queries/probe_remote_reachable.py | 4 +- .../core/git/queries/resolve_base_branch.py | 13 +-- src/pkgmgr/core/git/run.py | 21 +++- src/pkgmgr/core/repository/verify.py | 110 ++++++++---------- tests/e2e/test_changelog_commands.py | 2 +- .../actions/branch/test_close_branch.py | 4 +- .../pkgmgr/actions/branch/test_drop_branch.py | 4 +- .../queries/test_get_latest_signing_key.py | 25 ++++ .../queries/test_get_remote_head_commit.py | 32 +++++ .../core/git/queries/test_remote_check.py | 4 +- .../git/queries/test_resolve_base_branch.py | 8 +- tests/unit/pkgmgr/core/git/test_run.py | 4 +- .../core/repository/test_verify_repository.py | 87 ++++++++++++++ tests/unit/pkgmgr/core/test_git_utils.py | 8 +- 58 files changed, 408 insertions(+), 196 deletions(-) create mode 100644 src/pkgmgr/core/git/queries/get_latest_signing_key.py create mode 100644 src/pkgmgr/core/git/queries/get_remote_head_commit.py create mode 100644 tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py create mode 100644 tests/unit/pkgmgr/core/git/queries/test_get_remote_head_commit.py create mode 100644 tests/unit/pkgmgr/core/repository/test_verify_repository.py diff --git a/src/pkgmgr/actions/branch/close_branch.py b/src/pkgmgr/actions/branch/close_branch.py index c15e3fa..c9353f3 100644 --- a/src/pkgmgr/actions/branch/close_branch.py +++ b/src/pkgmgr/actions/branch/close_branch.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Optional -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.queries import get_current_branch from pkgmgr.core.git.commands import ( GitDeleteRemoteBranchError, @@ -32,7 +32,7 @@ def close_branch( if not name: try: name = get_current_branch(cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise RuntimeError(f"Failed to detect current branch: {exc}") from exc if not name: @@ -55,7 +55,7 @@ def close_branch( print("Aborted closing branch.") return - # Execute workflow (commands raise specific GitError subclasses) + # Execute workflow (commands raise specific GitRunError subclasses) fetch("origin", cwd=cwd) checkout(target_base, cwd=cwd) pull("origin", target_base, cwd=cwd) diff --git a/src/pkgmgr/actions/branch/drop_branch.py b/src/pkgmgr/actions/branch/drop_branch.py index f6a678c..a941f3d 100644 --- a/src/pkgmgr/actions/branch/drop_branch.py +++ b/src/pkgmgr/actions/branch/drop_branch.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Optional -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.queries import get_current_branch from pkgmgr.core.git.commands import ( GitDeleteRemoteBranchError, @@ -26,7 +26,7 @@ def drop_branch( if not name: try: name = get_current_branch(cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise RuntimeError(f"Failed to detect current branch: {exc}") from exc if not name: diff --git a/src/pkgmgr/actions/branch/open_branch.py b/src/pkgmgr/actions/branch/open_branch.py index 7531cc2..1a13fa1 100644 --- a/src/pkgmgr/actions/branch/open_branch.py +++ b/src/pkgmgr/actions/branch/open_branch.py @@ -30,7 +30,7 @@ def open_branch( resolved_base = resolve_base_branch(base_branch, fallback_base, cwd=cwd) - # Workflow (commands raise specific GitError subclasses) + # Workflow (commands raise specific GitBaseError subclasses) fetch("origin", cwd=cwd) checkout(resolved_base, cwd=cwd) pull("origin", resolved_base, cwd=cwd) diff --git a/src/pkgmgr/actions/mirror/git_remote.py b/src/pkgmgr/actions/mirror/git_remote.py index 49d878d..bed95d5 100644 --- a/src/pkgmgr/actions/mirror/git_remote.py +++ b/src/pkgmgr/actions/mirror/git_remote.py @@ -3,7 +3,7 @@ from __future__ import annotations import os from typing import Optional, Set -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.commands import ( GitAddRemoteError, GitAddRemotePushUrlError, @@ -90,7 +90,7 @@ def determine_primary_remote_url( def has_origin_remote(repo_dir: str) -> bool: try: return "origin" in list_remotes(cwd=repo_dir) - except GitError: + except GitRunError: return False @@ -122,7 +122,7 @@ def _ensure_additional_push_urls( try: existing = get_remote_push_urls("origin", cwd=repo_dir) - except GitError: + except GitRunError: existing = set() for url in sorted(desired - existing): diff --git a/src/pkgmgr/actions/release/workflow.py b/src/pkgmgr/actions/release/workflow.py index d1aae33..e62b3e5 100644 --- a/src/pkgmgr/actions/release/workflow.py +++ b/src/pkgmgr/actions/release/workflow.py @@ -5,7 +5,7 @@ import sys from typing import Optional from pkgmgr.actions.branch import close_branch -from pkgmgr.core.git import GitError +from pkgmgr.core.git import GitRunError from pkgmgr.core.git.commands import add, commit, push, tag_annotated from pkgmgr.core.git.queries import get_current_branch from pkgmgr.core.repository.paths import resolve_repo_paths @@ -40,7 +40,7 @@ def _release_impl( # Determine current branch early try: branch = get_current_branch() or "main" - except GitError: + except GitRunError: branch = "main" print(f"Releasing on branch: {branch}") @@ -158,7 +158,7 @@ def _release_impl( update_latest_tag(new_tag, preview=False) else: print(f"[INFO] Skipping 'latest' update (tag {new_tag} is not the highest).") - except GitError as exc: + except GitRunError as exc: print(f"[WARN] Failed to update floating 'latest' tag for {new_tag}: {exc}") print("'latest' tag was not updated.") diff --git a/src/pkgmgr/core/git/__init__.py b/src/pkgmgr/core/git/__init__.py index 0a7c6e1..1e39cc6 100644 --- a/src/pkgmgr/core/git/__init__.py +++ b/src/pkgmgr/core/git/__init__.py @@ -1,6 +1,6 @@ from __future__ import annotations -from .errors import GitError +from .errors import GitRunError from .run import run """ @@ -12,6 +12,6 @@ details of subprocess handling. """ __all__ = [ - "GitError", + "GitRunError", "run", ] diff --git a/src/pkgmgr/core/git/commands/__init__.py b/src/pkgmgr/core/git/commands/__init__.py index 7cd37aa..94d6b56 100644 --- a/src/pkgmgr/core/git/commands/__init__.py +++ b/src/pkgmgr/core/git/commands/__init__.py @@ -16,7 +16,7 @@ from .fetch import GitFetchError, fetch from .init import GitInitError, init from .merge_no_ff import GitMergeError, merge_no_ff from .pull import GitPullError, pull -from .pull_args import GitPullArgsError, pull_args # <-- add +from .pull_args import GitPullArgsError, pull_args from .pull_ff_only import GitPullFfOnlyError, pull_ff_only from .push import GitPushError, push from .push_upstream import GitPushUpstreamError, push_upstream @@ -30,7 +30,7 @@ __all__ = [ "fetch", "checkout", "pull", - "pull_args", # <-- add + "pull_args", "pull_ff_only", "merge_no_ff", "push", @@ -52,7 +52,7 @@ __all__ = [ "GitFetchError", "GitCheckoutError", "GitPullError", - "GitPullArgsError", # <-- add + "GitPullArgsError", "GitPullFfOnlyError", "GitMergeError", "GitPushError", diff --git a/src/pkgmgr/core/git/commands/add.py b/src/pkgmgr/core/git/commands/add.py index d79bb4d..1f2da9c 100644 --- a/src/pkgmgr/core/git/commands/add.py +++ b/src/pkgmgr/core/git/commands/add.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Iterable, List, Sequence, Union -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -37,7 +37,7 @@ def add( try: run(["add", *normalized], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitAddError( f"Failed to add paths to staging area: {normalized!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/add_all.py b/src/pkgmgr/core/git/commands/add_all.py index c03b5b1..5061b51 100644 --- a/src/pkgmgr/core/git/commands/add_all.py +++ b/src/pkgmgr/core/git/commands/add_all.py @@ -1,7 +1,6 @@ -# src/pkgmgr/core/git/commands/add_all.py from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitCommandError, GitRunError from ..run import run @@ -18,5 +17,5 @@ def add_all(*, cwd: str = ".", preview: bool = False) -> None: """ try: run(["add", "-A"], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitAddAllError("Failed to stage all changes with `git add -A`.", cwd=cwd) from exc diff --git a/src/pkgmgr/core/git/commands/add_remote.py b/src/pkgmgr/core/git/commands/add_remote.py index 72056be..586ae06 100644 --- a/src/pkgmgr/core/git/commands/add_remote.py +++ b/src/pkgmgr/core/git/commands/add_remote.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -27,7 +27,7 @@ def add_remote( cwd=cwd, preview=preview, ) - except GitError as exc: + except GitRunError as exc: raise GitAddRemoteError( f"Failed to add remote {name!r} with URL {url!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/add_remote_push_url.py b/src/pkgmgr/core/git/commands/add_remote_push_url.py index a401059..8de6681 100644 --- a/src/pkgmgr/core/git/commands/add_remote_push_url.py +++ b/src/pkgmgr/core/git/commands/add_remote_push_url.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -27,7 +27,7 @@ def add_remote_push_url( cwd=cwd, preview=preview, ) - except GitError as exc: + except GitRunError as exc: raise GitAddRemotePushUrlError( f"Failed to add push url {url!r} to remote {remote!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/branch_move.py b/src/pkgmgr/core/git/commands/branch_move.py index a8fa38f..62180bc 100644 --- a/src/pkgmgr/core/git/commands/branch_move.py +++ b/src/pkgmgr/core/git/commands/branch_move.py @@ -1,7 +1,6 @@ -# src/pkgmgr/core/git/commands/branch_move.py from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -18,5 +17,5 @@ def branch_move(branch: str, *, cwd: str = ".", preview: bool = False) -> None: """ try: run(["branch", "-M", branch], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitBranchMoveError(f"Failed to move/rename current branch to {branch!r}.", cwd=cwd) from exc diff --git a/src/pkgmgr/core/git/commands/checkout.py b/src/pkgmgr/core/git/commands/checkout.py index 8506449..e7700a2 100644 --- a/src/pkgmgr/core/git/commands/checkout.py +++ b/src/pkgmgr/core/git/commands/checkout.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -11,7 +11,7 @@ class GitCheckoutError(GitCommandError): def checkout(branch: str, cwd: str = ".") -> None: try: run(["checkout", branch], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitCheckoutError( f"Failed to checkout branch {branch!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/clone.py b/src/pkgmgr/core/git/commands/clone.py index d904c70..a8b66a8 100644 --- a/src/pkgmgr/core/git/commands/clone.py +++ b/src/pkgmgr/core/git/commands/clone.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import List -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -25,7 +25,7 @@ def clone( """ try: run(["clone", *args], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitCloneError( f"Git clone failed with args={args!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/commit.py b/src/pkgmgr/core/git/commands/commit.py index 73bb522..836610e 100644 --- a/src/pkgmgr/core/git/commands/commit.py +++ b/src/pkgmgr/core/git/commands/commit.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -30,7 +30,7 @@ def commit( try: run(args, cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitCommitError( "Failed to create commit.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/create_branch.py b/src/pkgmgr/core/git/commands/create_branch.py index 359adff..31a9eb7 100644 --- a/src/pkgmgr/core/git/commands/create_branch.py +++ b/src/pkgmgr/core/git/commands/create_branch.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -16,7 +16,7 @@ def create_branch(branch: str, base: str, cwd: str = ".") -> None: """ try: run(["checkout", "-b", branch, base], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitCreateBranchError( f"Failed to create branch {branch!r} from base {base!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/delete_local_branch.py b/src/pkgmgr/core/git/commands/delete_local_branch.py index 5c6340d..45402d5 100644 --- a/src/pkgmgr/core/git/commands/delete_local_branch.py +++ b/src/pkgmgr/core/git/commands/delete_local_branch.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -12,7 +12,7 @@ def delete_local_branch(branch: str, cwd: str = ".", force: bool = False) -> Non flag = "-D" if force else "-d" try: run(["branch", flag, branch], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitDeleteLocalBranchError( f"Failed to delete local branch {branch!r} (flag {flag}).", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/delete_remote_branch.py b/src/pkgmgr/core/git/commands/delete_remote_branch.py index 1f9e28a..0ba758d 100644 --- a/src/pkgmgr/core/git/commands/delete_remote_branch.py +++ b/src/pkgmgr/core/git/commands/delete_remote_branch.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -11,7 +11,7 @@ class GitDeleteRemoteBranchError(GitCommandError): def delete_remote_branch(remote: str, branch: str, cwd: str = ".") -> None: try: run(["push", remote, "--delete", branch], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitDeleteRemoteBranchError( f"Failed to delete remote branch {branch!r} on {remote!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/fetch.py b/src/pkgmgr/core/git/commands/fetch.py index 7e1d1a0..37f1c80 100644 --- a/src/pkgmgr/core/git/commands/fetch.py +++ b/src/pkgmgr/core/git/commands/fetch.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -33,7 +33,7 @@ def fetch( try: run(args, cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitFetchError( f"Failed to fetch from remote {remote!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/init.py b/src/pkgmgr/core/git/commands/init.py index 5a592de..a3683c9 100644 --- a/src/pkgmgr/core/git/commands/init.py +++ b/src/pkgmgr/core/git/commands/init.py @@ -1,7 +1,6 @@ -# src/pkgmgr/core/git/commands/init.py from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -18,5 +17,5 @@ def init(*, cwd: str = ".", preview: bool = False) -> None: """ try: run(["init"], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitInitError("Failed to initialize git repository.", cwd=cwd) from exc diff --git a/src/pkgmgr/core/git/commands/merge_no_ff.py b/src/pkgmgr/core/git/commands/merge_no_ff.py index cc9f30d..19b3f7b 100644 --- a/src/pkgmgr/core/git/commands/merge_no_ff.py +++ b/src/pkgmgr/core/git/commands/merge_no_ff.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -11,7 +11,7 @@ class GitMergeError(GitCommandError): def merge_no_ff(branch: str, cwd: str = ".") -> None: try: run(["merge", "--no-ff", branch], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitMergeError( f"Failed to merge branch {branch!r} with --no-ff.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/pull.py b/src/pkgmgr/core/git/commands/pull.py index ff5753f..6754e9e 100644 --- a/src/pkgmgr/core/git/commands/pull.py +++ b/src/pkgmgr/core/git/commands/pull.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -11,7 +11,7 @@ class GitPullError(GitCommandError): def pull(remote: str, branch: str, cwd: str = ".") -> None: try: run(["pull", remote, branch], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitPullError( f"Failed to pull {remote!r}/{branch!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/pull_args.py b/src/pkgmgr/core/git/commands/pull_args.py index b09e9fb..2e679a9 100644 --- a/src/pkgmgr/core/git/commands/pull_args.py +++ b/src/pkgmgr/core/git/commands/pull_args.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import List -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -28,7 +28,7 @@ def pull_args( extra = args or [] try: run(["pull", *extra], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitPullArgsError( f"Failed to run `git pull` with args={extra!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/pull_ff_only.py b/src/pkgmgr/core/git/commands/pull_ff_only.py index 9ea1d2a..eba343e 100644 --- a/src/pkgmgr/core/git/commands/pull_ff_only.py +++ b/src/pkgmgr/core/git/commands/pull_ff_only.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -17,7 +17,7 @@ def pull_ff_only(*, cwd: str = ".", preview: bool = False) -> None: """ try: run(["pull", "--ff-only"], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitPullFfOnlyError( "Failed to pull with --ff-only.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/push.py b/src/pkgmgr/core/git/commands/push.py index 1dc394c..0265072 100644 --- a/src/pkgmgr/core/git/commands/push.py +++ b/src/pkgmgr/core/git/commands/push.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -28,7 +28,7 @@ def push( try: run(args, cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitPushError( f"Failed to push ref {ref!r} to remote {remote!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/push_upstream.py b/src/pkgmgr/core/git/commands/push_upstream.py index dc04c5d..4991f5d 100644 --- a/src/pkgmgr/core/git/commands/push_upstream.py +++ b/src/pkgmgr/core/git/commands/push_upstream.py @@ -1,7 +1,6 @@ -# src/pkgmgr/core/git/commands/push_upstream.py from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -24,7 +23,7 @@ def push_upstream( """ try: run(["push", "-u", remote, branch], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitPushUpstreamError( f"Failed to push branch {branch!r} to {remote!r} with upstream tracking.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/set_remote_url.py b/src/pkgmgr/core/git/commands/set_remote_url.py index 61090c3..8d5c48a 100644 --- a/src/pkgmgr/core/git/commands/set_remote_url.py +++ b/src/pkgmgr/core/git/commands/set_remote_url.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -35,7 +35,7 @@ def set_remote_url( cwd=cwd, preview=preview, ) - except GitError as exc: + except GitRunError as exc: mode = "push" if push else "fetch" raise GitSetRemoteUrlError( f"Failed to set {mode} url for remote {remote!r} to {url!r}.", diff --git a/src/pkgmgr/core/git/commands/tag_annotated.py b/src/pkgmgr/core/git/commands/tag_annotated.py index 5939a6f..88f9fa9 100644 --- a/src/pkgmgr/core/git/commands/tag_annotated.py +++ b/src/pkgmgr/core/git/commands/tag_annotated.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -23,7 +23,7 @@ def tag_annotated( """ try: run(["tag", "-a", tag, "-m", message], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitTagAnnotatedError( f"Failed to create annotated tag {tag!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/commands/tag_force_annotated.py b/src/pkgmgr/core/git/commands/tag_force_annotated.py index aaeed58..7c15f4e 100644 --- a/src/pkgmgr/core/git/commands/tag_force_annotated.py +++ b/src/pkgmgr/core/git/commands/tag_force_annotated.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError, GitCommandError +from ..errors import GitRunError, GitCommandError from ..run import run @@ -24,7 +24,7 @@ def tag_force_annotated( """ try: run(["tag", "-f", "-a", name, target, "-m", message], cwd=cwd, preview=preview) - except GitError as exc: + except GitRunError as exc: raise GitTagForceAnnotatedError( f"Failed to force annotated tag {name!r} at {target!r}.", cwd=cwd, diff --git a/src/pkgmgr/core/git/errors.py b/src/pkgmgr/core/git/errors.py index be192f6..54ef96a 100644 --- a/src/pkgmgr/core/git/errors.py +++ b/src/pkgmgr/core/git/errors.py @@ -1,11 +1,19 @@ from __future__ import annotations -class GitError(RuntimeError): +class GitBaseError(RuntimeError): """Base error raised for Git related failures.""" +class GitRunError(GitBaseError): + """Base error raised for Git related failures.""" -class GitCommandError(GitError): +class GitNotRepositoryError(GitBaseError): + """Raised when the current working directory is not a git repository.""" + +class GitQueryError(GitRunError): + """Base class for read-only git query failures.""" + +class GitCommandError(GitRunError): """ Base class for state-changing git command failures. @@ -13,4 +21,5 @@ class GitCommandError(GitError): """ def __init__(self, message: str, *, cwd: str = ".") -> None: super().__init__(message) - self.cwd = cwd \ No newline at end of file + if cwd in locals(): + self.cwd = cwd diff --git a/src/pkgmgr/core/git/queries/__init__.py b/src/pkgmgr/core/git/queries/__init__.py index fcf50e0..96bb7d4 100644 --- a/src/pkgmgr/core/git/queries/__init__.py +++ b/src/pkgmgr/core/git/queries/__init__.py @@ -1,24 +1,36 @@ from __future__ import annotations +from .get_changelog import GitChangelogQueryError, get_changelog +from .get_config_value import get_config_value from .get_current_branch import get_current_branch from .get_head_commit import get_head_commit from .get_latest_commit import get_latest_commit -from .get_tags import get_tags -from .resolve_base_branch import GitBaseBranchNotFoundError, resolve_base_branch -from .list_remotes import list_remotes +from .get_latest_signing_key import ( + GitLatestSigningKeyQueryError, + get_latest_signing_key, +) +from .get_remote_head_commit import ( + GitRemoteHeadCommitQueryError, + get_remote_head_commit, +) from .get_remote_push_urls import get_remote_push_urls -from .probe_remote_reachable import probe_remote_reachable -from .get_changelog import get_changelog, GitChangelogQueryError -from .get_tags_at_ref import get_tags_at_ref, GitTagsAtRefQueryError -from .get_config_value import get_config_value -from .get_upstream_ref import get_upstream_ref -from .list_tags import list_tags from .get_repo_root import get_repo_root +from .get_tags import get_tags +from .get_tags_at_ref import GitTagsAtRefQueryError, get_tags_at_ref +from .get_upstream_ref import get_upstream_ref +from .list_remotes import list_remotes +from .list_tags import list_tags +from .probe_remote_reachable import probe_remote_reachable +from .resolve_base_branch import GitBaseBranchNotFoundError, resolve_base_branch __all__ = [ "get_current_branch", "get_head_commit", "get_latest_commit", + "get_latest_signing_key", + "GitLatestSigningKeyQueryError", + "get_remote_head_commit", + "GitRemoteHeadCommitQueryError", "get_tags", "resolve_base_branch", "GitBaseBranchNotFoundError", diff --git a/src/pkgmgr/core/git/queries/get_changelog.py b/src/pkgmgr/core/git/queries/get_changelog.py index 0344581..750f96b 100644 --- a/src/pkgmgr/core/git/queries/get_changelog.py +++ b/src/pkgmgr/core/git/queries/get_changelog.py @@ -2,11 +2,11 @@ from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitQueryError, GitRunError from ..run import run -class GitChangelogQueryError(GitError): +class GitChangelogQueryError(GitQueryError): """Raised when querying the git changelog fails.""" @@ -38,7 +38,7 @@ def get_changelog( try: return run(cmd, cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitChangelogQueryError( f"Failed to query changelog for range {rev_range!r}.", ) from exc diff --git a/src/pkgmgr/core/git/queries/get_config_value.py b/src/pkgmgr/core/git/queries/get_config_value.py index c859d5e..79d35ed 100644 --- a/src/pkgmgr/core/git/queries/get_config_value.py +++ b/src/pkgmgr/core/git/queries/get_config_value.py @@ -2,11 +2,11 @@ from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitRunError from ..run import run -def _is_missing_key_error(exc: GitError) -> bool: +def _is_missing_key_error(exc: GitRunError) -> bool: msg = str(exc).lower() # Ensure we only swallow the expected case for THIS command. @@ -25,7 +25,7 @@ def get_config_value(key: str, *, cwd: str = ".") -> Optional[str]: """ try: output = run(["config", "--get", key], cwd=cwd) - except GitError as exc: + except GitRunError as exc: if _is_missing_key_error(exc): return None raise diff --git a/src/pkgmgr/core/git/queries/get_current_branch.py b/src/pkgmgr/core/git/queries/get_current_branch.py index 36db71e..e9a8263 100644 --- a/src/pkgmgr/core/git/queries/get_current_branch.py +++ b/src/pkgmgr/core/git/queries/get_current_branch.py @@ -1,7 +1,7 @@ from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -13,6 +13,6 @@ def get_current_branch(cwd: str = ".") -> Optional[str]: """ try: output = run(["rev-parse", "--abbrev-ref", "HEAD"], cwd=cwd) - except GitError: + except GitRunError: return None return output or None \ No newline at end of file diff --git a/src/pkgmgr/core/git/queries/get_head_commit.py b/src/pkgmgr/core/git/queries/get_head_commit.py index c7b7e55..84d4117 100644 --- a/src/pkgmgr/core/git/queries/get_head_commit.py +++ b/src/pkgmgr/core/git/queries/get_head_commit.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -12,6 +12,6 @@ def get_head_commit(cwd: str = ".") -> Optional[str]: """ try: output = run(["rev-parse", "HEAD"], cwd=cwd) - except GitError: + except GitRunError: return None return output or None diff --git a/src/pkgmgr/core/git/queries/get_latest_commit.py b/src/pkgmgr/core/git/queries/get_latest_commit.py index a748fb2..a0a82c6 100644 --- a/src/pkgmgr/core/git/queries/get_latest_commit.py +++ b/src/pkgmgr/core/git/queries/get_latest_commit.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -19,7 +19,7 @@ def get_latest_commit(cwd: str = ".") -> Optional[str]: """ try: output = run(["log", "-1", "--format=%H"], cwd=cwd) - except GitError: + except GitRunError: return None output = output.strip() diff --git a/src/pkgmgr/core/git/queries/get_latest_signing_key.py b/src/pkgmgr/core/git/queries/get_latest_signing_key.py new file mode 100644 index 0000000..9b84b99 --- /dev/null +++ b/src/pkgmgr/core/git/queries/get_latest_signing_key.py @@ -0,0 +1,25 @@ +from __future__ import annotations + +from ..errors import GitQueryError, GitRunError +from ..run import run + + +class GitLatestSigningKeyQueryError(GitQueryError): + """Raised when querying the latest commit signing key fails.""" + + +def get_latest_signing_key(*, cwd: str = ".") -> str: + """ + Return the GPG signing key ID of the latest commit, via: + + git log -1 --format=%GK + + Returns: + The key id string (may be empty if commit is not signed). + """ + try: + return run(["log", "-1", "--format=%GK"], cwd=cwd).strip() + except GitRunError as exc: + raise GitLatestSigningKeyQueryError( + "Failed to query latest signing key.", + ) from exc diff --git a/src/pkgmgr/core/git/queries/get_remote_head_commit.py b/src/pkgmgr/core/git/queries/get_remote_head_commit.py new file mode 100644 index 0000000..c056b12 --- /dev/null +++ b/src/pkgmgr/core/git/queries/get_remote_head_commit.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +from ..errors import GitQueryError, GitRunError +from ..run import run + + +class GitRemoteHeadCommitQueryError(GitQueryError): + """Raised when querying the remote HEAD commit fails.""" + + +def get_remote_head_commit( + *, + remote: str = "origin", + ref: str = "HEAD", + cwd: str = ".", +) -> str: + """ + Return the commit hash for via: + + git ls-remote + + Returns: + The commit hash string (may be empty if remote/ref yields no output). + """ + try: + out = run(["ls-remote", remote, ref], cwd=cwd).strip() + except GitRunError as exc: + raise GitRemoteHeadCommitQueryError( + f"Failed to query remote head commit for {remote!r} {ref!r}.", + ) from exc + + # minimal parsing: first token is the hash + return (out.split()[0].strip() if out else "") diff --git a/src/pkgmgr/core/git/queries/get_remote_push_urls.py b/src/pkgmgr/core/git/queries/get_remote_push_urls.py index d2a5e8c..77b779c 100644 --- a/src/pkgmgr/core/git/queries/get_remote_push_urls.py +++ b/src/pkgmgr/core/git/queries/get_remote_push_urls.py @@ -4,7 +4,6 @@ from typing import Set from ..run import run - def get_remote_push_urls(remote: str, cwd: str = ".") -> Set[str]: """ Return all push URLs configured for a remote. @@ -12,7 +11,7 @@ def get_remote_push_urls(remote: str, cwd: str = ".") -> Set[str]: Equivalent to: git remote get-url --push --all - Raises GitError if the command fails. + Raises GitBaseError if the command fails. """ output = run(["remote", "get-url", "--push", "--all", remote], cwd=cwd) if not output: diff --git a/src/pkgmgr/core/git/queries/get_repo_root.py b/src/pkgmgr/core/git/queries/get_repo_root.py index 3de4cee..55ae311 100644 --- a/src/pkgmgr/core/git/queries/get_repo_root.py +++ b/src/pkgmgr/core/git/queries/get_repo_root.py @@ -1,9 +1,8 @@ -# src/pkgmgr/core/git/queries/get_repo_root.py from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -16,7 +15,7 @@ def get_repo_root(*, cwd: str = ".") -> Optional[str]: """ try: out = run(["rev-parse", "--show-toplevel"], cwd=cwd) - except GitError: + except GitRunError: return None out = out.strip() diff --git a/src/pkgmgr/core/git/queries/get_tags.py b/src/pkgmgr/core/git/queries/get_tags.py index 7ceb65a..4ebc19c 100644 --- a/src/pkgmgr/core/git/queries/get_tags.py +++ b/src/pkgmgr/core/git/queries/get_tags.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import List -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -14,11 +14,10 @@ def get_tags(cwd: str = ".") -> List[str]: """ try: output = run(["tag"], cwd=cwd) - except GitError as exc: + except GitRunError as exc: # If the repo is not a git repo, surface a clear error. if "not a git repository" in str(exc): raise - # Otherwise, treat as "no tags" (e.g., empty stdout). return [] if not output: diff --git a/src/pkgmgr/core/git/queries/get_tags_at_ref.py b/src/pkgmgr/core/git/queries/get_tags_at_ref.py index 70175f1..8ce74c7 100644 --- a/src/pkgmgr/core/git/queries/get_tags_at_ref.py +++ b/src/pkgmgr/core/git/queries/get_tags_at_ref.py @@ -2,11 +2,11 @@ from __future__ import annotations from typing import List -from ..errors import GitError +from ..errors import GitQueryError, GitRunError from ..run import run -class GitTagsAtRefQueryError(GitError): +class GitTagsAtRefQueryError(GitQueryError): """Raised when querying tags for a ref fails.""" @@ -19,7 +19,7 @@ def get_tags_at_ref(ref: str, *, cwd: str = ".") -> List[str]: """ try: output = run(["tag", "--points-at", ref], cwd=cwd) - except GitError as exc: + except GitRunError as exc: raise GitTagsAtRefQueryError( f"Failed to query tags at ref {ref!r}.", ) from exc diff --git a/src/pkgmgr/core/git/queries/get_upstream_ref.py b/src/pkgmgr/core/git/queries/get_upstream_ref.py index 4416f3b..ef6dd1d 100644 --- a/src/pkgmgr/core/git/queries/get_upstream_ref.py +++ b/src/pkgmgr/core/git/queries/get_upstream_ref.py @@ -2,7 +2,7 @@ from __future__ import annotations from typing import Optional -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -18,7 +18,7 @@ def get_upstream_ref(*, cwd: str = ".") -> Optional[str]: ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"], cwd=cwd, ) - except GitError: + except GitRunError: return None out = out.strip() diff --git a/src/pkgmgr/core/git/queries/list_remotes.py b/src/pkgmgr/core/git/queries/list_remotes.py index b114ebb..c982124 100644 --- a/src/pkgmgr/core/git/queries/list_remotes.py +++ b/src/pkgmgr/core/git/queries/list_remotes.py @@ -9,7 +9,7 @@ def list_remotes(cwd: str = ".") -> List[str]: """ Return a list of configured git remotes (e.g. ['origin', 'upstream']). - Raises GitError if the command fails. + Raises GitBaseError if the command fails. """ output = run(["remote"], cwd=cwd) if not output: diff --git a/src/pkgmgr/core/git/queries/probe_remote_reachable.py b/src/pkgmgr/core/git/queries/probe_remote_reachable.py index 8133a93..a6c87cf 100644 --- a/src/pkgmgr/core/git/queries/probe_remote_reachable.py +++ b/src/pkgmgr/core/git/queries/probe_remote_reachable.py @@ -1,6 +1,6 @@ from __future__ import annotations -from ..errors import GitError +from ..errors import GitRunError from ..run import run @@ -17,5 +17,5 @@ def probe_remote_reachable(url: str, cwd: str = ".") -> bool: try: run(["ls-remote", "--exit-code", url], cwd=cwd) return True - except GitError: + except GitRunError: return False diff --git a/src/pkgmgr/core/git/queries/resolve_base_branch.py b/src/pkgmgr/core/git/queries/resolve_base_branch.py index b680774..35d4070 100644 --- a/src/pkgmgr/core/git/queries/resolve_base_branch.py +++ b/src/pkgmgr/core/git/queries/resolve_base_branch.py @@ -1,15 +1,14 @@ -# src/pkgmgr/core/git/queries/resolve_base_branch.py from __future__ import annotations -from ..errors import GitError +from ..errors import GitQueryError, GitRunError from ..run import run -class GitBaseBranchNotFoundError(GitError): +class GitBaseBranchNotFoundError(GitQueryError): """Raised when neither preferred nor fallback base branch exists.""" -def _is_branch_missing_error(exc: GitError) -> bool: +def _is_branch_missing_error(exc: GitRunError) -> bool: """ Heuristic: Detect errors that indicate the branch/ref does not exist. @@ -46,15 +45,15 @@ def resolve_base_branch( fall back to `fallback` (default: master). Raises GitBaseBranchNotFoundError if neither exists. - Raises GitError for other git failures (e.g., not a git repository). + Raises GitRunError for other git failures (e.g., not a git repository). """ - last_missing_error: GitError | None = None + last_missing_error: GitRunError | None = None for candidate in (preferred, fallback): try: run(["rev-parse", "--verify", candidate], cwd=cwd) return candidate - except GitError as exc: + except GitRunError as exc: if _is_branch_missing_error(exc): last_missing_error = exc continue diff --git a/src/pkgmgr/core/git/run.py b/src/pkgmgr/core/git/run.py index d6d72b6..e9844a6 100644 --- a/src/pkgmgr/core/git/run.py +++ b/src/pkgmgr/core/git/run.py @@ -3,7 +3,12 @@ from __future__ import annotations import subprocess from typing import List -from .errors import GitError +from .errors import GitRunError, GitNotRepositoryError + + +def _is_not_repo_error(stderr: str) -> bool: + msg = (stderr or "").lower() + return "not a git repository" in msg def run( @@ -17,7 +22,7 @@ def run( If preview=True, the command is printed but NOT executed. - Raises GitError if execution fails. + Raises GitRunError (or a subclass) if execution fails. """ cmd = ["git"] + args cmd_str = " ".join(cmd) @@ -36,11 +41,19 @@ def run( text=True, ) except subprocess.CalledProcessError as exc: - raise GitError( + stderr = exc.stderr or "" + if _is_not_repo_error(stderr): + raise GitNotRepositoryError( + f"Not a git repository: {cwd!r}\n" + f"Command: {cmd_str}\n" + f"STDERR:\n{stderr}" + ) from exc + + raise GitRunError( f"Git command failed in {cwd!r}: {cmd_str}\n" f"Exit code: {exc.returncode}\n" f"STDOUT:\n{exc.stdout}\n" - f"STDERR:\n{exc.stderr}" + f"STDERR:\n{stderr}" ) from exc return result.stdout.strip() diff --git a/src/pkgmgr/core/repository/verify.py b/src/pkgmgr/core/repository/verify.py index a86baea..c93f156 100644 --- a/src/pkgmgr/core/repository/verify.py +++ b/src/pkgmgr/core/repository/verify.py @@ -1,48 +1,37 @@ -import subprocess +from __future__ import annotations + +from pkgmgr.core.git.queries import ( + get_head_commit, + get_latest_signing_key, + get_remote_head_commit, + GitLatestSigningKeyQueryError, + GitRemoteHeadCommitQueryError, +) + def verify_repository(repo, repo_dir, mode="local", no_verification=False): - """ - Verifies the repository based on its 'verified' field. - - The 'verified' field can be a dictionary with the following keys: - commit: The expected commit hash. - gpg_keys: A list of valid GPG key IDs (at least one must match the signing key). + _ = no_verification - If mode == "pull", the remote HEAD commit is checked via "git ls-remote origin HEAD". - Otherwise (mode "local", used for install and clone), the local HEAD commit is checked via "git rev-parse HEAD". - - Returns a tuple: - (verified_ok, error_details, commit_hash, signing_key) - - verified_ok: True if the verification passed (or no verification info is set), False otherwise. - - error_details: A list of error messages for any failed checks. - - commit_hash: The obtained commit hash. - - signing_key: The GPG key ID that signed the latest commit (obtained via "git log -1 --format=%GK"). - """ verified_info = repo.get("verified") - if not verified_info: - # Nothing to verify. - commit_hash = "" - signing_key = "" + + commit_hash = "" + signing_key = "" + + # best-effort info collection + try: if mode == "pull": - try: - result = subprocess.run("git ls-remote origin HEAD", cwd=repo_dir, shell=True, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - commit_hash = result.stdout.split()[0].strip() - except Exception: - commit_hash = "" + commit_hash = get_remote_head_commit(cwd=repo_dir) else: - try: - result = subprocess.run("git rev-parse HEAD", cwd=repo_dir, shell=True, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - commit_hash = result.stdout.strip() - except Exception: - commit_hash = "" - try: - result = subprocess.run(["git", "log", "-1", "--format=%GK"], cwd=repo_dir, shell=False, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - signing_key = result.stdout.strip() - except Exception: - signing_key = "" + commit_hash = get_head_commit(cwd=repo_dir) or "" + except GitRemoteHeadCommitQueryError: + commit_hash = "" + + try: + signing_key = get_latest_signing_key(cwd=repo_dir) + except GitLatestSigningKeyQueryError: + signing_key = "" + + if not verified_info: return True, [], commit_hash, signing_key expected_commit = None @@ -51,47 +40,42 @@ def verify_repository(repo, repo_dir, mode="local", no_verification=False): expected_commit = verified_info.get("commit") expected_gpg_keys = verified_info.get("gpg_keys") else: - # If verified is a plain string, treat it as the expected commit. expected_commit = verified_info - error_details = [] + error_details: list[str] = [] - # Get commit hash according to the mode. - commit_hash = "" + # strict retrieval when verification is configured if mode == "pull": try: - result = subprocess.run("git ls-remote origin HEAD", cwd=repo_dir, shell=True, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - commit_hash = result.stdout.split()[0].strip() - except Exception as e: - error_details.append(f"Error retrieving remote commit: {e}") + commit_hash = get_remote_head_commit(cwd=repo_dir) + except GitRemoteHeadCommitQueryError as exc: + error_details.append(str(exc)) + commit_hash = "" else: - try: - result = subprocess.run("git rev-parse HEAD", cwd=repo_dir, shell=True, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - commit_hash = result.stdout.strip() - except Exception as e: - error_details.append(f"Error retrieving local commit: {e}") + commit_hash = get_head_commit(cwd=repo_dir) or "" - # Get the signing key using "git log -1 --format=%GK" - signing_key = "" try: - result = subprocess.run(["git", "log", "-1", "--format=%GK"], cwd=repo_dir, shell=False, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - signing_key = result.stdout.strip() - except Exception as e: - error_details.append(f"Error retrieving signing key: {e}") + signing_key = get_latest_signing_key(cwd=repo_dir) + except GitLatestSigningKeyQueryError as exc: + error_details.append(str(exc)) + signing_key = "" commit_check_passed = True gpg_check_passed = True if expected_commit: - if commit_hash != expected_commit: + if not commit_hash: + commit_check_passed = False + error_details.append(f"Expected commit: {expected_commit}, but could not determine current commit.") + elif commit_hash != expected_commit: commit_check_passed = False error_details.append(f"Expected commit: {expected_commit}, found: {commit_hash}") if expected_gpg_keys: - if signing_key not in expected_gpg_keys: + if not signing_key: + gpg_check_passed = False + error_details.append(f"Expected one of GPG keys: {expected_gpg_keys}, but no signing key was found.") + elif signing_key not in expected_gpg_keys: gpg_check_passed = False error_details.append(f"Expected one of GPG keys: {expected_gpg_keys}, found: {signing_key}") diff --git a/tests/e2e/test_changelog_commands.py b/tests/e2e/test_changelog_commands.py index ac78f84..85d9136 100644 --- a/tests/e2e/test_changelog_commands.py +++ b/tests/e2e/test_changelog_commands.py @@ -89,7 +89,7 @@ class TestIntegrationChangelogCommands(unittest.TestCase): """ Run 'pkgmgr changelog HEAD~5..HEAD' inside the pkgmgr repo. Selbst wenn HEAD~5 nicht existiert, sollte der Befehl den - GitError intern behandeln und mit Exit-Code 0 beenden + GitBaseError intern behandeln und mit Exit-Code 0 beenden (es wird dann eine [ERROR]-Zeile gedruckt). Wird übersprungen, wenn das pkgmgr-Repo nicht lokal vorhanden ist. diff --git a/tests/unit/pkgmgr/actions/branch/test_close_branch.py b/tests/unit/pkgmgr/actions/branch/test_close_branch.py index 3cf946f..c1a0687 100644 --- a/tests/unit/pkgmgr/actions/branch/test_close_branch.py +++ b/tests/unit/pkgmgr/actions/branch/test_close_branch.py @@ -2,7 +2,7 @@ import unittest from unittest.mock import patch from pkgmgr.actions.branch.close_branch import close_branch -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.commands import GitDeleteRemoteBranchError @@ -90,7 +90,7 @@ class TestCloseBranch(unittest.TestCase): delete_local_branch.assert_called_once_with("feature-x", cwd=".", force=False) delete_remote_branch.assert_called_once_with("origin", "feature-x", cwd=".") - @patch("pkgmgr.actions.branch.close_branch.get_current_branch", side_effect=GitError("fail")) + @patch("pkgmgr.actions.branch.close_branch.get_current_branch", side_effect=GitRunError("fail")) def test_close_branch_errors_if_cannot_detect_branch(self, _current) -> None: with self.assertRaises(RuntimeError): close_branch(None) diff --git a/tests/unit/pkgmgr/actions/branch/test_drop_branch.py b/tests/unit/pkgmgr/actions/branch/test_drop_branch.py index a157370..facc505 100644 --- a/tests/unit/pkgmgr/actions/branch/test_drop_branch.py +++ b/tests/unit/pkgmgr/actions/branch/test_drop_branch.py @@ -2,7 +2,7 @@ import unittest from unittest.mock import patch from pkgmgr.actions.branch.drop_branch import drop_branch -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.commands import GitDeleteRemoteBranchError @@ -50,7 +50,7 @@ class TestDropBranch(unittest.TestCase): delete_local.assert_called_once_with("feature-x", cwd=".", force=False) delete_remote.assert_called_once_with("origin", "feature-x", cwd=".") - @patch("pkgmgr.actions.branch.drop_branch.get_current_branch", side_effect=GitError("fail")) + @patch("pkgmgr.actions.branch.drop_branch.get_current_branch", side_effect=GitRunError("fail")) def test_drop_branch_errors_if_no_branch_detected(self, _current) -> None: with self.assertRaises(RuntimeError): drop_branch(None) diff --git a/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py b/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py new file mode 100644 index 0000000..65650af --- /dev/null +++ b/tests/unit/pkgmgr/core/git/queries/test_get_latest_signing_key.py @@ -0,0 +1,25 @@ +import unittest +from unittest.mock import patch + +from pkgmgr.core.git.errors import GitNotRepositoryError, GitRunError +from pkgmgr.core.git.queries.get_latest_signing_key import ( + GitLatestSigningKeyQueryError, + get_latest_signing_key, +) + + +class TestGetLatestSigningKey(unittest.TestCase): + @patch("pkgmgr.core.git.queries.get_latest_signing_key.run", return_value="ABCDEF1234567890\n") + def test_strips_output(self, _mock_run) -> None: + out = get_latest_signing_key(cwd="/tmp/repo") + self.assertEqual(out, "ABCDEF1234567890") + + @patch("pkgmgr.core.git.queries.get_latest_signing_key.run", side_effect=GitRunError("boom")) + def test_wraps_git_run_error(self, _mock_run) -> None: + with self.assertRaises(GitLatestSigningKeyQueryError): + get_latest_signing_key(cwd="/tmp/repo") + + @patch("pkgmgr.core.git.queries.get_latest_signing_key.run", side_effect=GitNotRepositoryError("no repo")) + def test_does_not_catch_not_repository_error(self, _mock_run) -> None: + with self.assertRaises(GitNotRepositoryError): + get_latest_signing_key(cwd="/tmp/no-repo") diff --git a/tests/unit/pkgmgr/core/git/queries/test_get_remote_head_commit.py b/tests/unit/pkgmgr/core/git/queries/test_get_remote_head_commit.py new file mode 100644 index 0000000..81157da --- /dev/null +++ b/tests/unit/pkgmgr/core/git/queries/test_get_remote_head_commit.py @@ -0,0 +1,32 @@ +import unittest +from unittest.mock import patch + +from pkgmgr.core.git.errors import GitNotRepositoryError, GitRunError +from pkgmgr.core.git.queries.get_remote_head_commit import ( + GitRemoteHeadCommitQueryError, + get_remote_head_commit, +) + + +class TestGetRemoteHeadCommit(unittest.TestCase): + @patch("pkgmgr.core.git.queries.get_remote_head_commit.run", return_value="abc123\tHEAD\n") + def test_parses_first_token_as_hash(self, mock_run) -> None: + out = get_remote_head_commit(cwd="/tmp/repo") + self.assertEqual(out, "abc123") + mock_run.assert_called_once() + + @patch("pkgmgr.core.git.queries.get_remote_head_commit.run", return_value="") + def test_returns_empty_string_on_empty_output(self, _mock_run) -> None: + out = get_remote_head_commit(cwd="/tmp/repo") + self.assertEqual(out, "") + + @patch("pkgmgr.core.git.queries.get_remote_head_commit.run", side_effect=GitRunError("boom")) + def test_wraps_git_run_error(self, _mock_run) -> None: + with self.assertRaises(GitRemoteHeadCommitQueryError) as ctx: + get_remote_head_commit(cwd="/tmp/repo") + self.assertIn("Failed to query remote head commit", str(ctx.exception)) + + @patch("pkgmgr.core.git.queries.get_remote_head_commit.run", side_effect=GitNotRepositoryError("no repo")) + def test_does_not_catch_not_repository_error(self, _mock_run) -> None: + with self.assertRaises(GitNotRepositoryError): + get_remote_head_commit(cwd="/tmp/no-repo") diff --git a/tests/unit/pkgmgr/core/git/queries/test_remote_check.py b/tests/unit/pkgmgr/core/git/queries/test_remote_check.py index 4e3955f..ce36e55 100644 --- a/tests/unit/pkgmgr/core/git/queries/test_remote_check.py +++ b/tests/unit/pkgmgr/core/git/queries/test_remote_check.py @@ -6,7 +6,7 @@ from __future__ import annotations import unittest from unittest.mock import patch -from pkgmgr.core.git import GitError +from pkgmgr.core.git import GitRunError from pkgmgr.core.git.queries.probe_remote_reachable import probe_remote_reachable @@ -32,7 +32,7 @@ class TestProbeRemoteReachable(unittest.TestCase): @patch("pkgmgr.core.git.queries.probe_remote_reachable.run") def test_probe_remote_reachable_failure_returns_false(self, mock_run) -> None: - mock_run.side_effect = GitError("Git command failed (simulated)") + mock_run.side_effect = GitRunError("Git command failed (simulated)") ok = probe_remote_reachable( "ssh://git@code.example.org:2201/alice/repo.git", diff --git a/tests/unit/pkgmgr/core/git/queries/test_resolve_base_branch.py b/tests/unit/pkgmgr/core/git/queries/test_resolve_base_branch.py index d73bdc7..a1f6e0a 100644 --- a/tests/unit/pkgmgr/core/git/queries/test_resolve_base_branch.py +++ b/tests/unit/pkgmgr/core/git/queries/test_resolve_base_branch.py @@ -3,7 +3,7 @@ from __future__ import annotations import unittest from unittest.mock import patch -from pkgmgr.core.git import GitError +from pkgmgr.core.git import GitRunError from pkgmgr.core.git.queries.resolve_base_branch import ( GitBaseBranchNotFoundError, resolve_base_branch, @@ -21,7 +21,7 @@ class TestResolveBaseBranch(unittest.TestCase): @patch("pkgmgr.core.git.queries.resolve_base_branch.run") def test_resolves_fallback(self, mock_run): mock_run.side_effect = [ - GitError("fatal: Needed a single revision"), # treat as "missing" + GitRunError("fatal: Needed a single revision"), # treat as "missing" "dummy", ] result = resolve_base_branch("main", "master", cwd=".") @@ -32,8 +32,8 @@ class TestResolveBaseBranch(unittest.TestCase): @patch("pkgmgr.core.git.queries.resolve_base_branch.run") def test_raises_when_no_branch_exists(self, mock_run): mock_run.side_effect = [ - GitError("fatal: Needed a single revision"), - GitError("fatal: Needed a single revision"), + GitRunError("fatal: Needed a single revision"), + GitRunError("fatal: Needed a single revision"), ] with self.assertRaises(GitBaseBranchNotFoundError): resolve_base_branch("main", "master", cwd=".") diff --git a/tests/unit/pkgmgr/core/git/test_run.py b/tests/unit/pkgmgr/core/git/test_run.py index eeaadfc..15ec348 100644 --- a/tests/unit/pkgmgr/core/git/test_run.py +++ b/tests/unit/pkgmgr/core/git/test_run.py @@ -1,7 +1,7 @@ import unittest from unittest.mock import MagicMock, patch -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.run import run @@ -55,7 +55,7 @@ class TestGitRun(unittest.TestCase): exc.stderr = "ERR!" with patch("pkgmgr.core.git.run.subprocess.run", side_effect=exc): - with self.assertRaises(GitError) as ctx: + with self.assertRaises(GitRunError) as ctx: run(["status"], cwd="/bad/repo", preview=False) msg = str(ctx.exception) diff --git a/tests/unit/pkgmgr/core/repository/test_verify_repository.py b/tests/unit/pkgmgr/core/repository/test_verify_repository.py new file mode 100644 index 0000000..045b470 --- /dev/null +++ b/tests/unit/pkgmgr/core/repository/test_verify_repository.py @@ -0,0 +1,87 @@ +import unittest +from unittest.mock import patch + +from pkgmgr.core.git.errors import GitNotRepositoryError +from pkgmgr.core.git.queries.get_latest_signing_key import GitLatestSigningKeyQueryError +from pkgmgr.core.git.queries.get_remote_head_commit import GitRemoteHeadCommitQueryError +from pkgmgr.core.repository.verify import verify_repository + + +class TestVerifyRepository(unittest.TestCase): + def test_no_verified_info_returns_ok_and_best_effort_values(self) -> None: + repo = {"id": "demo"} # no "verified" + with patch("pkgmgr.core.repository.verify.get_head_commit", return_value="deadbeef"), patch( + "pkgmgr.core.repository.verify.get_latest_signing_key", + return_value="KEYID", + ): + ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local") + self.assertTrue(ok) + self.assertEqual(errors, []) + self.assertEqual(commit, "deadbeef") + self.assertEqual(key, "KEYID") + + def test_best_effort_swallows_query_errors_when_no_verified_info(self) -> None: + repo = {"id": "demo"} + with patch( + "pkgmgr.core.repository.verify.get_head_commit", + return_value=None, + ), patch( + "pkgmgr.core.repository.verify.get_latest_signing_key", + side_effect=GitLatestSigningKeyQueryError("fail signing key"), + ): + ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local") + self.assertTrue(ok) + self.assertEqual(errors, []) + self.assertEqual(commit, "") + self.assertEqual(key, "") + + def test_verified_commit_mismatch_fails(self) -> None: + repo = {"verified": {"commit": "expected", "gpg_keys": None}} + with patch("pkgmgr.core.repository.verify.get_head_commit", return_value="actual"), patch( + "pkgmgr.core.repository.verify.get_latest_signing_key", + return_value="", + ): + ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local") + + self.assertFalse(ok) + self.assertIn("Expected commit: expected, found: actual", errors) + self.assertEqual(commit, "actual") + self.assertEqual(key, "") + + def test_verified_gpg_key_missing_fails(self) -> None: + repo = {"verified": {"commit": None, "gpg_keys": ["ABC"]}} + with patch("pkgmgr.core.repository.verify.get_head_commit", return_value=""), patch( + "pkgmgr.core.repository.verify.get_latest_signing_key", + return_value="", + ): + ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local") + + self.assertFalse(ok) + self.assertTrue(any("no signing key was found" in e for e in errors)) + self.assertEqual(commit, "") + self.assertEqual(key, "") + + def test_strict_pull_collects_remote_error_message(self) -> None: + repo = {"verified": {"commit": "expected", "gpg_keys": None}} + with patch( + "pkgmgr.core.repository.verify.get_remote_head_commit", + side_effect=GitRemoteHeadCommitQueryError("remote fail"), + ), patch( + "pkgmgr.core.repository.verify.get_latest_signing_key", + return_value="", + ): + ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="pull") + + self.assertFalse(ok) + self.assertIn("remote fail", " ".join(errors)) + self.assertEqual(commit, "") + self.assertEqual(key, "") + + def test_not_repository_error_is_not_caught(self) -> None: + repo = {"verified": {"commit": "expected", "gpg_keys": None}} + with patch( + "pkgmgr.core.repository.verify.get_head_commit", + side_effect=GitNotRepositoryError("no repo"), + ): + with self.assertRaises(GitNotRepositoryError): + verify_repository(repo, "/tmp/no-repo", mode="local") diff --git a/tests/unit/pkgmgr/core/test_git_utils.py b/tests/unit/pkgmgr/core/test_git_utils.py index 08ea844..34be761 100644 --- a/tests/unit/pkgmgr/core/test_git_utils.py +++ b/tests/unit/pkgmgr/core/test_git_utils.py @@ -4,7 +4,7 @@ import unittest from unittest.mock import patch -from pkgmgr.core.git.errors import GitError +from pkgmgr.core.git.errors import GitRunError from pkgmgr.core.git.run import run from pkgmgr.core.git.queries import get_tags, get_head_commit, get_current_branch @@ -35,7 +35,7 @@ class TestGitRun(unittest.TestCase): stderr="error\n", ) - with self.assertRaises(GitError) as ctx: + with self.assertRaises(GitRunError) as ctx: run(["status"], cwd="/tmp/repo") msg = str(ctx.exception) @@ -66,7 +66,7 @@ class TestGitQueries(unittest.TestCase): @patch("pkgmgr.core.git.queries.get_head_commit.run") def test_get_head_commit_failure_returns_none(self, mock_run): - mock_run.side_effect = GitError("fail") + mock_run.side_effect = GitRunError("fail") commit = get_head_commit(cwd="/tmp/repo") self.assertIsNone(commit) @@ -78,7 +78,7 @@ class TestGitQueries(unittest.TestCase): @patch("pkgmgr.core.git.queries.get_current_branch.run") def test_get_current_branch_failure_returns_none(self, mock_run): - mock_run.side_effect = GitError("fail") + mock_run.side_effect = GitRunError("fail") branch = get_current_branch(cwd="/tmp/repo") self.assertIsNone(branch)