From 8583fdf172e3f8b2f2eb0d1d01763f0376193621 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 16 Dec 2025 14:19:19 +0100 Subject: [PATCH] feat(mirror,create): make MIRRORS single source of truth and exclude PyPI from git config - Treat MIRRORS as the only authority for mirror URLs - Filter non-git URLs (e.g. PyPI) from git remotes and push URLs - Prefer SSH git URLs when determining primary origin - Ensure mirror probing only targets valid git remotes - Refactor repository create into service-based architecture - Write PyPI metadata exclusively to MIRRORS, never to git config - Add integration test verifying PyPI is not written into .git/config - Update preview and unit tests to match new create flow https://chatgpt.com/share/69415c61-1c5c-800f-86dd-0405edec25db --- src/pkgmgr/actions/mirror/git_remote.py | 70 +++-- src/pkgmgr/actions/mirror/setup_cmd.py | 31 ++- src/pkgmgr/actions/repository/create.py | 245 ------------------ .../actions/repository/create/__init__.py | 28 ++ .../repository/create/config_writer.py | 84 ++++++ .../repository/create/git_bootstrap.py | 35 +++ .../actions/repository/create/mirrors.py | 54 ++++ src/pkgmgr/actions/repository/create/model.py | 12 + .../actions/repository/create/parser.py | 68 +++++ .../actions/repository/create/planner.py | 52 ++++ .../actions/repository/create/service.py | 97 +++++++ .../actions/repository/create/templates.py | 80 ++++++ src/pkgmgr/actions/repository/scaffold.py | 97 ------- .../test_repos_create_preview_output.py | 54 +++- ...est_repos_create_pypi_not_in_git_config.py | 115 ++++++++ .../actions/repository/create/__init__.py | 0 .../{ => create}/test_create_parsing.py | 10 +- .../create/test_scaffold_render_preview.py | 43 +++ .../test_scaffold_render_preview.py | 35 --- 19 files changed, 792 insertions(+), 418 deletions(-) delete mode 100644 src/pkgmgr/actions/repository/create.py create mode 100644 src/pkgmgr/actions/repository/create/__init__.py create mode 100644 src/pkgmgr/actions/repository/create/config_writer.py create mode 100644 src/pkgmgr/actions/repository/create/git_bootstrap.py create mode 100644 src/pkgmgr/actions/repository/create/mirrors.py create mode 100644 src/pkgmgr/actions/repository/create/model.py create mode 100644 src/pkgmgr/actions/repository/create/parser.py create mode 100644 src/pkgmgr/actions/repository/create/planner.py create mode 100644 src/pkgmgr/actions/repository/create/service.py create mode 100644 src/pkgmgr/actions/repository/create/templates.py delete mode 100644 src/pkgmgr/actions/repository/scaffold.py create mode 100644 tests/integration/test_repos_create_pypi_not_in_git_config.py create mode 100644 tests/unit/pkgmgr/actions/repository/create/__init__.py rename tests/unit/pkgmgr/actions/repository/{ => create}/test_create_parsing.py (89%) create mode 100644 tests/unit/pkgmgr/actions/repository/create/test_scaffold_render_preview.py delete mode 100644 tests/unit/pkgmgr/actions/repository/test_scaffold_render_preview.py diff --git a/src/pkgmgr/actions/mirror/git_remote.py b/src/pkgmgr/actions/mirror/git_remote.py index edcbd34..49d878d 100644 --- a/src/pkgmgr/actions/mirror/git_remote.py +++ b/src/pkgmgr/actions/mirror/git_remote.py @@ -12,14 +12,38 @@ from pkgmgr.core.git.commands import ( add_remote_push_url, set_remote_url, ) -from pkgmgr.core.git.queries import ( - get_remote_push_urls, - list_remotes, -) +from pkgmgr.core.git.queries import get_remote_push_urls, list_remotes from .types import MirrorMap, RepoMirrorContext, Repository +def _is_git_remote_url(url: str) -> bool: + """ + True only for URLs that should become git remotes / push URLs. + + Accepted: + - git@host:owner/repo(.git) (SCP-like SSH) + - ssh://git@host(:port)/owner/repo(.git) (SSH URL) + - https://host/owner/repo.git (HTTPS git remote) + - http://host/owner/repo.git (rare, but possible) + Everything else (e.g. PyPI project page) stays metadata only. + """ + u = (url or "").strip() + if not u: + return False + + if u.startswith("git@"): + return True + + if u.startswith("ssh://"): + return True + + if (u.startswith("https://") or u.startswith("http://")) and u.endswith(".git"): + return True + + return False + + def build_default_ssh_url(repo: Repository) -> Optional[str]: provider = repo.get("provider") account = repo.get("account") @@ -35,25 +59,29 @@ def build_default_ssh_url(repo: Repository) -> Optional[str]: return f"git@{provider}:{account}/{name}.git" +def _git_mirrors_only(m: MirrorMap) -> MirrorMap: + return {k: v for k, v in m.items() if v and _is_git_remote_url(v)} + + def determine_primary_remote_url( repo: Repository, ctx: RepoMirrorContext, ) -> Optional[str]: """ - Priority order: - 1. origin from resolved mirrors - 2. MIRRORS file order - 3. config mirrors order + Priority order (GIT URLS ONLY): + 1. origin from resolved mirrors (if it is a git URL) + 2. first git URL from MIRRORS file (in file order) + 3. first git URL from config mirrors (in config order) 4. default SSH URL """ resolved = ctx.resolved_mirrors - - if resolved.get("origin"): - return resolved["origin"] + origin = resolved.get("origin") + if origin and _is_git_remote_url(origin): + return origin for mirrors in (ctx.file_mirrors, ctx.config_mirrors): for _, url in mirrors.items(): - if url: + if url and _is_git_remote_url(url): return url return build_default_ssh_url(repo) @@ -82,10 +110,13 @@ def _ensure_additional_push_urls( preview: bool, ) -> None: """ - Ensure all mirror URLs (except primary) are configured as additional push URLs for origin. - Preview is handled by the underlying git runner. + Ensure all *git* mirror URLs (except primary) are configured as additional + push URLs for origin. + + Non-git URLs (like PyPI) are ignored and will never land in git config. """ - desired: Set[str] = {u for u in mirrors.values() if u and u != primary} + git_only = _git_mirrors_only(mirrors) + desired: Set[str] = {u for u in git_only.values() if u and u != primary} if not desired: return @@ -110,8 +141,8 @@ def ensure_origin_remote( return primary = determine_primary_remote_url(repo, ctx) - if not primary: - print("[WARN] No primary mirror URL could be determined.") + if not primary or not _is_git_remote_url(primary): + print("[WARN] No valid git primary mirror URL could be determined.") return # 1) Ensure origin exists @@ -122,14 +153,13 @@ def ensure_origin_remote( print(f"[WARN] Failed to add origin remote: {exc}") return # without origin we cannot reliably proceed - # 2) Ensure origin fetch+push URLs are correct (ALWAYS, even if origin already existed) + # 2) Ensure origin fetch+push URLs are correct try: _set_origin_fetch_and_push(repo_dir, primary, preview) except GitSetRemoteUrlError as exc: - # Do not abort: still try to add additional push URLs print(f"[WARN] Failed to set origin URLs: {exc}") - # 3) Ensure additional push URLs for mirrors + # 3) Ensure additional push URLs for mirrors (git urls only) try: _ensure_additional_push_urls(repo_dir, ctx.resolved_mirrors, primary, preview) except GitAddRemotePushUrlError as exc: diff --git a/src/pkgmgr/actions/mirror/setup_cmd.py b/src/pkgmgr/actions/mirror/setup_cmd.py index 77d58b2..bf282d6 100644 --- a/src/pkgmgr/actions/mirror/setup_cmd.py +++ b/src/pkgmgr/actions/mirror/setup_cmd.py @@ -2,13 +2,29 @@ from __future__ import annotations from typing import List +from pkgmgr.core.git.queries import probe_remote_reachable + from .context import build_context from .git_remote import ensure_origin_remote, determine_primary_remote_url -from pkgmgr.core.git.queries import probe_remote_reachable from .remote_provision import ensure_remote_repository from .types import Repository +def _is_git_remote_url(url: str) -> bool: + # Keep the same filtering semantics as in git_remote.py (duplicated on purpose + # to keep setup_cmd independent of private helpers). + u = (url or "").strip() + if not u: + return False + if u.startswith("git@"): + return True + if u.startswith("ssh://"): + return True + if (u.startswith("https://") or u.startswith("http://")) and u.endswith(".git"): + return True + return False + + def _setup_local_mirrors_for_repo( repo: Repository, repositories_base_dir: str, @@ -48,16 +64,23 @@ def _setup_remote_mirrors_for_repo( preview, ) - if not ctx.resolved_mirrors: + # Probe only git URLs (do not try ls-remote against PyPI etc.) + # If there are no mirrors at all, probe the primary git URL. + git_mirrors = {k: v for k, v in ctx.resolved_mirrors.items() if _is_git_remote_url(v)} + + if not git_mirrors: primary = determine_primary_remote_url(repo, ctx) - if not primary: + if not primary or not _is_git_remote_url(primary): + print("[INFO] No git mirrors to probe.") + print() return + ok = probe_remote_reachable(primary, cwd=ctx.repo_dir) print("[OK]" if ok else "[WARN]", primary) print() return - for name, url in ctx.resolved_mirrors.items(): + for name, url in git_mirrors.items(): ok = probe_remote_reachable(url, cwd=ctx.repo_dir) print(f"[OK] {name}: {url}" if ok else f"[WARN] {name}: {url}") diff --git a/src/pkgmgr/actions/repository/create.py b/src/pkgmgr/actions/repository/create.py deleted file mode 100644 index 8c89d78..0000000 --- a/src/pkgmgr/actions/repository/create.py +++ /dev/null @@ -1,245 +0,0 @@ -# src/pkgmgr/actions/repository/create.py -from __future__ import annotations - -import os -import re -from dataclasses import dataclass -from typing import Any, Dict, Optional, Tuple -from urllib.parse import urlparse - -import yaml - -from pkgmgr.actions.mirror.io import write_mirrors_file -from pkgmgr.actions.mirror.setup_cmd import setup_mirrors -from pkgmgr.actions.repository.scaffold import render_default_templates -from pkgmgr.core.command.alias import generate_alias -from pkgmgr.core.config.save import save_user_config -from pkgmgr.core.git.commands import ( - GitCommitError, - GitPushUpstreamError, - add_all, - branch_move, - commit, - init, - push_upstream, -) -from pkgmgr.core.git.queries import get_config_value - -Repository = Dict[str, Any] - -_NAME_RE = re.compile(r"^[a-z0-9_-]+$") - - -@dataclass(frozen=True) -class RepoParts: - host: str - port: Optional[str] - owner: str - name: str - - -def _split_host_port(host_with_port: str) -> Tuple[str, Optional[str]]: - if ":" in host_with_port: - host, port = host_with_port.split(":", 1) - return host, port or None - return host_with_port, None - - -def _strip_git_suffix(name: str) -> str: - return name[:-4] if name.endswith(".git") else name - - -def _parse_git_url(url: str) -> RepoParts: - if url.startswith("git@") and "://" not in url: - left, right = url.split(":", 1) - host = left.split("@", 1)[1] - path = right.lstrip("/") - owner, name = path.split("/", 1) - return RepoParts(host=host, port=None, owner=owner, name=_strip_git_suffix(name)) - - parsed = urlparse(url) - host = (parsed.hostname or "").strip() - port = str(parsed.port) if parsed.port else None - path = (parsed.path or "").strip("/") - - if not host or not path or "/" not in path: - raise ValueError(f"Could not parse git URL: {url}") - - owner, name = path.split("/", 1) - return RepoParts(host=host, port=port, owner=owner, name=_strip_git_suffix(name)) - - -def _parse_identifier(identifier: str) -> RepoParts: - ident = identifier.strip() - - if "://" in ident or ident.startswith("git@"): - return _parse_git_url(ident) - - parts = ident.split("/") - if len(parts) != 3: - raise ValueError("Identifier must be URL or 'provider(:port)/owner/repo'.") - - host_with_port, owner, name = parts - host, port = _split_host_port(host_with_port) - return RepoParts(host=host, port=port, owner=owner, name=name) - - -def _ensure_valid_repo_name(name: str) -> None: - if not name or not _NAME_RE.fullmatch(name): - raise ValueError("Repository name must match: lowercase a-z, 0-9, '_' and '-'.") - - -def _repo_homepage(host: str, owner: str, name: str) -> str: - return f"https://{host}/{owner}/{name}" - - -def _build_default_primary_url(parts: RepoParts) -> str: - if parts.port: - return f"ssh://git@{parts.host}:{parts.port}/{parts.owner}/{parts.name}.git" - return f"git@{parts.host}:{parts.owner}/{parts.name}.git" - - -def _write_default_mirrors(repo_dir: str, primary: str, name: str, preview: bool) -> None: - mirrors = {"origin": primary, "pypi": f"https://pypi.org/project/{name}/"} - write_mirrors_file(repo_dir, mirrors, preview=preview) - - -def _git_init_and_initial_commit(repo_dir: str, preview: bool) -> None: - init(cwd=repo_dir, preview=preview) - add_all(cwd=repo_dir, preview=preview) - - try: - commit("Initial commit", cwd=repo_dir, preview=preview) - except GitCommitError as exc: - print(f"[WARN] Initial commit failed (continuing): {exc}") - - -def _git_push_main_or_master(repo_dir: str, preview: bool) -> None: - try: - branch_move("main", cwd=repo_dir, preview=preview) - push_upstream("origin", "main", cwd=repo_dir, preview=preview) - return - except GitPushUpstreamError: - pass - - try: - branch_move("master", cwd=repo_dir, preview=preview) - push_upstream("origin", "master", cwd=repo_dir, preview=preview) - except GitPushUpstreamError as exc: - print(f"[WARN] Push failed: {exc}") - - -def create_repo( - identifier: str, - config_merged: Dict[str, Any], - user_config_path: str, - bin_dir: str, - *, - remote: bool = False, - preview: bool = False, -) -> None: - parts = _parse_identifier(identifier) - _ensure_valid_repo_name(parts.name) - - directories = config_merged.get("directories") or {} - base_dir = os.path.expanduser(str(directories.get("repositories", "~/Repositories"))) - repo_dir = os.path.join(base_dir, parts.host, parts.owner, parts.name) - - author_name = get_config_value("user.name") or "Unknown Author" - author_email = get_config_value("user.email") or "unknown@example.invalid" - - homepage = _repo_homepage(parts.host, parts.owner, parts.name) - primary_url = _build_default_primary_url(parts) - - repositories = config_merged.get("repositories") or [] - exists = any( - ( - r.get("provider") == parts.host - and r.get("account") == parts.owner - and r.get("repository") == parts.name - ) - for r in repositories - ) - - if not exists: - new_entry: Repository = { - "provider": parts.host, - "port": parts.port, - "account": parts.owner, - "repository": parts.name, - "homepage": homepage, - "alias": generate_alias( - {"repository": parts.name, "provider": parts.host, "account": parts.owner}, - bin_dir, - existing_aliases=set(), - ), - "verified": {}, - } - - if os.path.exists(user_config_path): - with open(user_config_path, "r", encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} - else: - user_config = {"repositories": []} - - user_config.setdefault("repositories", []) - user_config["repositories"].append(new_entry) - - if preview: - print(f"[Preview] Would save user config: {user_config_path}") - else: - save_user_config(user_config, user_config_path) - - config_merged.setdefault("repositories", []).append(new_entry) - repo = new_entry - print(f"[INFO] Added repository to configuration: {parts.host}/{parts.owner}/{parts.name}") - else: - repo = next( - r - for r in repositories - if ( - r.get("provider") == parts.host - and r.get("account") == parts.owner - and r.get("repository") == parts.name - ) - ) - print(f"[INFO] Repository already in configuration: {parts.host}/{parts.owner}/{parts.name}") - - if preview: - print(f"[Preview] Would ensure directory exists: {repo_dir}") - else: - os.makedirs(repo_dir, exist_ok=True) - - tpl_context = { - "provider": parts.host, - "port": parts.port, - "account": parts.owner, - "repository": parts.name, - "homepage": homepage, - "author_name": author_name, - "author_email": author_email, - "license_text": f"All rights reserved by {author_name}", - "primary_remote": primary_url, - } - - render_default_templates(repo_dir, context=tpl_context, preview=preview) - _git_init_and_initial_commit(repo_dir, preview=preview) - - _write_default_mirrors(repo_dir, primary=primary_url, name=parts.name, preview=preview) - - repo.setdefault("mirrors", {}) - repo["mirrors"].setdefault("origin", primary_url) - repo["mirrors"].setdefault("pypi", f"https://pypi.org/project/{parts.name}/") - - setup_mirrors( - selected_repos=[repo], - repositories_base_dir=base_dir, - all_repos=config_merged.get("repositories", []), - preview=preview, - local=True, - remote=True, - ensure_remote=bool(remote), - ) - - if remote: - _git_push_main_or_master(repo_dir, preview=preview) diff --git a/src/pkgmgr/actions/repository/create/__init__.py b/src/pkgmgr/actions/repository/create/__init__.py new file mode 100644 index 0000000..0ca6a49 --- /dev/null +++ b/src/pkgmgr/actions/repository/create/__init__.py @@ -0,0 +1,28 @@ +from __future__ import annotations + +from typing import Any, Dict + +from .service import CreateRepoService + +RepositoryConfig = Dict[str, Any] + +__all__ = [ + "CreateRepoService", + "create_repo", +] + + +def create_repo( + identifier: str, + config_merged: RepositoryConfig, + user_config_path: str, + bin_dir: str, + *, + remote: bool = False, + preview: bool = False, +) -> None: + CreateRepoService( + config_merged=config_merged, + user_config_path=user_config_path, + bin_dir=bin_dir, + ).run(identifier=identifier, preview=preview, remote=remote) diff --git a/src/pkgmgr/actions/repository/create/config_writer.py b/src/pkgmgr/actions/repository/create/config_writer.py new file mode 100644 index 0000000..622ccf8 --- /dev/null +++ b/src/pkgmgr/actions/repository/create/config_writer.py @@ -0,0 +1,84 @@ +from __future__ import annotations + +import os +from typing import Dict, Any, Set + +import yaml + +from pkgmgr.core.command.alias import generate_alias +from pkgmgr.core.config.save import save_user_config + +Repository = Dict[str, Any] + + +class ConfigRepoWriter: + def __init__( + self, + *, + config_merged: Dict[str, Any], + user_config_path: str, + bin_dir: str, + ): + self.config_merged = config_merged + self.user_config_path = user_config_path + self.bin_dir = bin_dir + + def ensure_repo_entry( + self, + *, + host: str, + port: str | None, + owner: str, + name: str, + homepage: str, + preview: bool, + ) -> Repository: + repositories = self.config_merged.setdefault("repositories", []) + + for repo in repositories: + if ( + repo.get("provider") == host + and repo.get("account") == owner + and repo.get("repository") == name + ): + return repo + + existing_aliases: Set[str] = { + str(r.get("alias")) for r in repositories if r.get("alias") + } + + repo: Repository = { + "provider": host, + "port": port, + "account": owner, + "repository": name, + "homepage": homepage, + "alias": generate_alias( + { + "repository": name, + "provider": host, + "account": owner, + }, + self.bin_dir, + existing_aliases=existing_aliases, + ), + "verified": {}, + } + + if preview: + print(f"[Preview] Would add repository to config: {repo}") + return repo + + if os.path.exists(self.user_config_path): + with open(self.user_config_path, "r", encoding="utf-8") as f: + user_cfg = yaml.safe_load(f) or {} + else: + user_cfg = {} + + user_cfg.setdefault("repositories", []).append(repo) + save_user_config(user_cfg, self.user_config_path) + + repositories.append(repo) + print(f"[INFO] Added repository to configuration: {host}/{owner}/{name}") + + return repo diff --git a/src/pkgmgr/actions/repository/create/git_bootstrap.py b/src/pkgmgr/actions/repository/create/git_bootstrap.py new file mode 100644 index 0000000..9c63a16 --- /dev/null +++ b/src/pkgmgr/actions/repository/create/git_bootstrap.py @@ -0,0 +1,35 @@ +from __future__ import annotations + +from pkgmgr.core.git.commands import ( + GitCommitError, + GitPushUpstreamError, + add_all, + branch_move, + commit, + init, + push_upstream, +) + + +class GitBootstrapper: + def init_repo(self, repo_dir: str, preview: bool) -> None: + init(cwd=repo_dir, preview=preview) + add_all(cwd=repo_dir, preview=preview) + try: + commit("Initial commit", cwd=repo_dir, preview=preview) + except GitCommitError as exc: + print(f"[WARN] Initial commit failed (continuing): {exc}") + + def push_default_branch(self, repo_dir: str, preview: bool) -> None: + try: + branch_move("main", cwd=repo_dir, preview=preview) + push_upstream("origin", "main", cwd=repo_dir, preview=preview) + return + except GitPushUpstreamError: + pass + + try: + branch_move("master", cwd=repo_dir, preview=preview) + push_upstream("origin", "master", cwd=repo_dir, preview=preview) + except GitPushUpstreamError as exc: + print(f"[WARN] Push failed: {exc}") diff --git a/src/pkgmgr/actions/repository/create/mirrors.py b/src/pkgmgr/actions/repository/create/mirrors.py new file mode 100644 index 0000000..fbe15d4 --- /dev/null +++ b/src/pkgmgr/actions/repository/create/mirrors.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from typing import Any, Dict + +from pkgmgr.actions.mirror.io import write_mirrors_file +from pkgmgr.actions.mirror.setup_cmd import setup_mirrors + +Repository = Dict[str, Any] + + +class MirrorBootstrapper: + """ + MIRRORS is the single source of truth. + + We write defaults to MIRRORS and then call mirror setup which will + configure git remotes based on MIRRORS content (but only for git URLs). + """ + + def write_defaults( + self, + *, + repo_dir: str, + primary: str, + name: str, + preview: bool, + ) -> None: + mirrors = { + # preferred SSH url is supplied by CreateRepoPlanner.primary_remote + "origin": primary, + # metadata only: must NEVER be configured as a git remote + "pypi": f"https://pypi.org/project/{name}/", + } + write_mirrors_file(repo_dir, mirrors, preview=preview) + + def setup( + self, + *, + repo: Repository, + repositories_base_dir: str, + all_repos: list[Repository], + preview: bool, + remote: bool, + ) -> None: + # IMPORTANT: do NOT set repo["mirrors"] here. + # MIRRORS file is the single source of truth. + setup_mirrors( + selected_repos=[repo], + repositories_base_dir=repositories_base_dir, + all_repos=all_repos, + preview=preview, + local=True, + remote=True, + ensure_remote=remote, + ) diff --git a/src/pkgmgr/actions/repository/create/model.py b/src/pkgmgr/actions/repository/create/model.py new file mode 100644 index 0000000..02911aa --- /dev/null +++ b/src/pkgmgr/actions/repository/create/model.py @@ -0,0 +1,12 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Optional + + +@dataclass(frozen=True) +class RepoParts: + host: str + port: Optional[str] + owner: str + name: str diff --git a/src/pkgmgr/actions/repository/create/parser.py b/src/pkgmgr/actions/repository/create/parser.py new file mode 100644 index 0000000..9bc90b3 --- /dev/null +++ b/src/pkgmgr/actions/repository/create/parser.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import re +from typing import Tuple +from urllib.parse import urlparse + +from .model import RepoParts + +_NAME_RE = re.compile(r"^[a-z0-9_-]+$") + + +def parse_identifier(identifier: str) -> RepoParts: + ident = identifier.strip() + + if "://" in ident or ident.startswith("git@"): + return _parse_git_url(ident) + + parts = ident.split("/") + if len(parts) != 3: + raise ValueError("Identifier must be URL or 'provider(:port)/owner/repo'.") + + host_with_port, owner, name = parts + host, port = _split_host_port(host_with_port) + _ensure_valid_repo_name(name) + + return RepoParts(host=host, port=port, owner=owner, name=name) + + +def _parse_git_url(url: str) -> RepoParts: + if url.startswith("git@") and "://" not in url: + left, right = url.split(":", 1) + host = left.split("@", 1)[1] + owner, name = right.lstrip("/").split("/", 1) + name = _strip_git_suffix(name) + _ensure_valid_repo_name(name) + return RepoParts(host=host, port=None, owner=owner, name=name) + + parsed = urlparse(url) + host = parsed.hostname or "" + port = str(parsed.port) if parsed.port else None + path = (parsed.path or "").strip("/") + + if not host or "/" not in path: + raise ValueError(f"Could not parse git URL: {url}") + + owner, name = path.split("/", 1) + name = _strip_git_suffix(name) + _ensure_valid_repo_name(name) + + return RepoParts(host=host, port=port, owner=owner, name=name) + + +def _split_host_port(host: str) -> Tuple[str, str | None]: + if ":" in host: + h, p = host.split(":", 1) + return h, p or None + return host, None + + +def _strip_git_suffix(name: str) -> str: + return name[:-4] if name.endswith(".git") else name + + +def _ensure_valid_repo_name(name: str) -> None: + if not _NAME_RE.fullmatch(name): + raise ValueError( + "Repository name must match: lowercase a-z, 0-9, '_' and '-'." + ) diff --git a/src/pkgmgr/actions/repository/create/planner.py b/src/pkgmgr/actions/repository/create/planner.py new file mode 100644 index 0000000..782e85b --- /dev/null +++ b/src/pkgmgr/actions/repository/create/planner.py @@ -0,0 +1,52 @@ +from __future__ import annotations + +import os +from typing import Dict, Any + +from .model import RepoParts + + +class CreateRepoPlanner: + def __init__(self, parts: RepoParts, repositories_base_dir: str): + self.parts = parts + self.repositories_base_dir = os.path.expanduser(repositories_base_dir) + + @property + def repo_dir(self) -> str: + return os.path.join( + self.repositories_base_dir, + self.parts.host, + self.parts.owner, + self.parts.name, + ) + + @property + def homepage(self) -> str: + return f"https://{self.parts.host}/{self.parts.owner}/{self.parts.name}" + + @property + def primary_remote(self) -> str: + if self.parts.port: + return ( + f"ssh://git@{self.parts.host}:{self.parts.port}/" + f"{self.parts.owner}/{self.parts.name}.git" + ) + return f"git@{self.parts.host}:{self.parts.owner}/{self.parts.name}.git" + + def template_context( + self, + *, + author_name: str, + author_email: str, + ) -> Dict[str, Any]: + return { + "provider": self.parts.host, + "port": self.parts.port, + "account": self.parts.owner, + "repository": self.parts.name, + "homepage": self.homepage, + "author_name": author_name, + "author_email": author_email, + "license_text": f"All rights reserved by {author_name}", + "primary_remote": self.primary_remote, + } diff --git a/src/pkgmgr/actions/repository/create/service.py b/src/pkgmgr/actions/repository/create/service.py new file mode 100644 index 0000000..548d55e --- /dev/null +++ b/src/pkgmgr/actions/repository/create/service.py @@ -0,0 +1,97 @@ +from __future__ import annotations + +import os +from typing import Dict, Any + +from pkgmgr.core.git.queries import get_config_value + +from .parser import parse_identifier +from .planner import CreateRepoPlanner +from .config_writer import ConfigRepoWriter +from .templates import TemplateRenderer +from .git_bootstrap import GitBootstrapper +from .mirrors import MirrorBootstrapper + + +class CreateRepoService: + def __init__( + self, + *, + config_merged: Dict[str, Any], + user_config_path: str, + bin_dir: str, + ): + self.config_merged = config_merged + self.user_config_path = user_config_path + self.bin_dir = bin_dir + + self.templates = TemplateRenderer() + self.git = GitBootstrapper() + self.mirrors = MirrorBootstrapper() + + def run( + self, + *, + identifier: str, + preview: bool, + remote: bool, + ) -> None: + parts = parse_identifier(identifier) + + base_dir = self.config_merged.get("directories", {}).get( + "repositories", "~/Repositories" + ) + + planner = CreateRepoPlanner(parts, base_dir) + + writer = ConfigRepoWriter( + config_merged=self.config_merged, + user_config_path=self.user_config_path, + bin_dir=self.bin_dir, + ) + + repo = writer.ensure_repo_entry( + host=parts.host, + port=parts.port, + owner=parts.owner, + name=parts.name, + homepage=planner.homepage, + preview=preview, + ) + + if preview: + print(f"[Preview] Would ensure directory exists: {planner.repo_dir}") + else: + os.makedirs(planner.repo_dir, exist_ok=True) + + author_name = get_config_value("user.name") or "Unknown Author" + author_email = get_config_value("user.email") or "unknown@example.invalid" + + self.templates.render( + repo_dir=planner.repo_dir, + context=planner.template_context( + author_name=author_name, + author_email=author_email, + ), + preview=preview, + ) + + self.git.init_repo(planner.repo_dir, preview=preview) + + self.mirrors.write_defaults( + repo_dir=planner.repo_dir, + primary=planner.primary_remote, + name=parts.name, + preview=preview, + ) + + self.mirrors.setup( + repo=repo, + repositories_base_dir=os.path.expanduser(base_dir), + all_repos=self.config_merged.get("repositories", []), + preview=preview, + remote=remote, + ) + + if remote: + self.git.push_default_branch(planner.repo_dir, preview=preview) diff --git a/src/pkgmgr/actions/repository/create/templates.py b/src/pkgmgr/actions/repository/create/templates.py new file mode 100644 index 0000000..967650e --- /dev/null +++ b/src/pkgmgr/actions/repository/create/templates.py @@ -0,0 +1,80 @@ +from __future__ import annotations + +import os +from pathlib import Path +from typing import Dict, Any + +from pkgmgr.core.git.queries import get_repo_root + +try: + from jinja2 import Environment, FileSystemLoader, StrictUndefined +except Exception as exc: # pragma: no cover + Environment = None # type: ignore + FileSystemLoader = None # type: ignore + StrictUndefined = None # type: ignore + _JINJA_IMPORT_ERROR = exc +else: + _JINJA_IMPORT_ERROR = None + + +class TemplateRenderer: + def __init__(self) -> None: + self.templates_dir = self._resolve_templates_dir() + + def render( + self, + *, + repo_dir: str, + context: Dict[str, Any], + preview: bool, + ) -> None: + if preview: + self._preview() + return + + if Environment is None: + raise RuntimeError( + "Jinja2 is required but not available. " + f"Import error: {_JINJA_IMPORT_ERROR}" + ) + + env = Environment( + loader=FileSystemLoader(self.templates_dir), + undefined=StrictUndefined, + autoescape=False, + keep_trailing_newline=True, + ) + + for root, _, files in os.walk(self.templates_dir): + for fn in files: + if not fn.endswith(".j2"): + continue + + abs_src = os.path.join(root, fn) + rel_src = os.path.relpath(abs_src, self.templates_dir) + rel_out = rel_src[:-3] + abs_out = os.path.join(repo_dir, rel_out) + + os.makedirs(os.path.dirname(abs_out), exist_ok=True) + template = env.get_template(rel_src) + rendered = template.render(**context) + + with open(abs_out, "w", encoding="utf-8") as f: + f.write(rendered) + + def _preview(self) -> None: + for root, _, files in os.walk(self.templates_dir): + for fn in files: + if fn.endswith(".j2"): + rel = os.path.relpath( + os.path.join(root, fn), self.templates_dir + ) + print(f"[Preview] Would render template: {rel} -> {rel[:-3]}") + + @staticmethod + def _resolve_templates_dir() -> str: + here = Path(__file__).resolve().parent + root = get_repo_root(cwd=str(here)) + if not root: + raise RuntimeError("Could not determine repository root for templates.") + return os.path.join(root, "templates", "default") diff --git a/src/pkgmgr/actions/repository/scaffold.py b/src/pkgmgr/actions/repository/scaffold.py deleted file mode 100644 index 8bfd703..0000000 --- a/src/pkgmgr/actions/repository/scaffold.py +++ /dev/null @@ -1,97 +0,0 @@ -# src/pkgmgr/actions/repository/scaffold.py -from __future__ import annotations - -import os -from pathlib import Path -from typing import Any, Dict, Optional - -from pkgmgr.core.git.queries import get_repo_root - -try: - from jinja2 import Environment, FileSystemLoader, StrictUndefined -except Exception as exc: # pragma: no cover - Environment = None # type: ignore[assignment] - FileSystemLoader = None # type: ignore[assignment] - StrictUndefined = None # type: ignore[assignment] - _JINJA_IMPORT_ERROR = exc -else: - _JINJA_IMPORT_ERROR = None - - -def _repo_root_from_here(anchor: Optional[Path] = None) -> str: - """ - Prefer git root (robust in editable installs / different layouts). - Fallback to a conservative relative parent lookup. - """ - here = (anchor or Path(__file__)).resolve().parent - - top = get_repo_root(cwd=str(here)) - if top: - return top - - # Fallback: src/pkgmgr/actions/repository/scaffold.py -> = parents[5] - p = (anchor or Path(__file__)).resolve() - if len(p.parents) < 6: - raise RuntimeError(f"Unexpected path depth for: {p}") - return str(p.parents[5]) - - -def _templates_dir() -> str: - return os.path.join(_repo_root_from_here(), "templates", "default") - - -def render_default_templates( - repo_dir: str, - *, - context: Dict[str, Any], - preview: bool, -) -> None: - """ - Render templates/default/*.j2 into repo_dir. - Keeps create.py clean: create.py calls this function only. - """ - tpl_dir = _templates_dir() - if not os.path.isdir(tpl_dir): - raise RuntimeError(f"Templates directory not found: {tpl_dir}") - - # Preview mode: do not require Jinja2 at all. We only print planned outputs. - if preview: - for root, _, files in os.walk(tpl_dir): - for fn in files: - if not fn.endswith(".j2"): - continue - abs_src = os.path.join(root, fn) - rel_src = os.path.relpath(abs_src, tpl_dir) - rel_out = rel_src[:-3] - print(f"[Preview] Would render template: {rel_src} -> {rel_out}") - return - - if Environment is None or FileSystemLoader is None or StrictUndefined is None: - raise RuntimeError( - "Jinja2 is required for repo templates but is not available. " - f"Import error: {_JINJA_IMPORT_ERROR}" - ) - - env = Environment( - loader=FileSystemLoader(tpl_dir), - undefined=StrictUndefined, - autoescape=False, - keep_trailing_newline=True, - ) - - for root, _, files in os.walk(tpl_dir): - for fn in files: - if not fn.endswith(".j2"): - continue - - abs_src = os.path.join(root, fn) - rel_src = os.path.relpath(abs_src, tpl_dir) - rel_out = rel_src[:-3] - abs_out = os.path.join(repo_dir, rel_out) - - os.makedirs(os.path.dirname(abs_out), exist_ok=True) - template = env.get_template(rel_src) - rendered = template.render(**context) - - with open(abs_out, "w", encoding="utf-8") as f: - f.write(rendered) diff --git a/tests/integration/test_repos_create_preview_output.py b/tests/integration/test_repos_create_preview_output.py index 22a833b..d4ed8a8 100644 --- a/tests/integration/test_repos_create_preview_output.py +++ b/tests/integration/test_repos_create_preview_output.py @@ -15,17 +15,47 @@ class TestCreateRepoPreviewOutput(unittest.TestCase): out = io.StringIO() with ( redirect_stdout(out), - patch("pkgmgr.actions.repository.create.os.path.exists", return_value=False), - patch("pkgmgr.actions.repository.create.generate_alias", return_value="repo"), - patch("pkgmgr.actions.repository.create.save_user_config"), - patch("pkgmgr.actions.repository.create.os.makedirs"), - patch("pkgmgr.actions.repository.create.render_default_templates"), - patch("pkgmgr.actions.repository.create.write_mirrors_file"), - patch("pkgmgr.actions.repository.create.setup_mirrors"), - patch("pkgmgr.actions.repository.create.get_config_value", return_value=None), - patch("pkgmgr.actions.repository.create.init"), - patch("pkgmgr.actions.repository.create.add_all"), - patch("pkgmgr.actions.repository.create.commit"), + patch( + "pkgmgr.actions.repository.create.config_writer.generate_alias", + return_value="repo", + ), + patch( + "pkgmgr.actions.repository.create.config_writer.save_user_config", + ), + patch( + "pkgmgr.actions.repository.create.config_writer.os.path.exists", + return_value=False, + ), + patch( + "pkgmgr.actions.repository.create.service.os.makedirs", + ), + patch( + "pkgmgr.actions.repository.create.templates.TemplateRenderer._resolve_templates_dir", + return_value="/tpl", + ), + patch( + "pkgmgr.actions.repository.create.templates.os.walk", + return_value=[("/tpl", [], ["README.md.j2"])], + ), + patch( + "pkgmgr.actions.repository.create.git_bootstrap.init", + ), + patch( + "pkgmgr.actions.repository.create.git_bootstrap.add_all", + ), + patch( + "pkgmgr.actions.repository.create.git_bootstrap.commit", + ), + patch( + "pkgmgr.actions.repository.create.mirrors.write_mirrors_file", + ), + patch( + "pkgmgr.actions.repository.create.mirrors.setup_mirrors", + ), + patch( + "pkgmgr.actions.repository.create.service.get_config_value", + return_value=None, + ), ): create_repo( "github.com/acme/repo", @@ -37,7 +67,7 @@ class TestCreateRepoPreviewOutput(unittest.TestCase): ) s = out.getvalue() - self.assertIn("[Preview] Would save user config:", s) + self.assertIn("[Preview] Would add repository to config:", s) self.assertIn("[Preview] Would ensure directory exists:", s) diff --git a/tests/integration/test_repos_create_pypi_not_in_git_config.py b/tests/integration/test_repos_create_pypi_not_in_git_config.py new file mode 100644 index 0000000..42dc944 --- /dev/null +++ b/tests/integration/test_repos_create_pypi_not_in_git_config.py @@ -0,0 +1,115 @@ +# tests/integration/test_repos_create_pypi_not_in_git_config.py +from __future__ import annotations + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from pkgmgr.actions.repository.create import create_repo + + +class TestCreateRepoPypiNotInGitConfig(unittest.TestCase): + def test_create_repo_writes_pypi_to_mirrors_but_not_git_config(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + + # Repositories base dir used by create flow + repos_base = tmp_path / "Repositories" + user_cfg = tmp_path / "user.yml" + bin_dir = tmp_path / "bin" + bin_dir.mkdir(parents=True, exist_ok=True) + + cfg = { + "directories": {"repositories": str(repos_base)}, + "repositories": [], + } + + # Provide a minimal templates directory so TemplateRenderer can run + tpl_dir = tmp_path / "tpl" + tpl_dir.mkdir(parents=True, exist_ok=True) + (tpl_dir / "README.md.j2").write_text( + "# {{ repository }}\n", encoding="utf-8" + ) + + # Expected repo dir for identifier github.com/acme/repo + repo_dir = repos_base / "github.com" / "acme" / "repo" + + with ( + # Avoid any real network calls during mirror "remote probing" + patch( + "pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable", + return_value=True, + ), + # Force templates to come from our temp directory + patch( + "pkgmgr.actions.repository.create.templates.TemplateRenderer._resolve_templates_dir", + return_value=str(tpl_dir), + ), + # Make git commit deterministic without depending on global git config + patch.dict( + os.environ, + { + "GIT_AUTHOR_NAME": "Test Author", + "GIT_AUTHOR_EMAIL": "author@example.invalid", + "GIT_COMMITTER_NAME": "Test Author", + "GIT_COMMITTER_EMAIL": "author@example.invalid", + }, + clear=False, + ), + ): + create_repo( + "github.com/acme/repo", + cfg, + str(user_cfg), + str(bin_dir), + remote=False, + preview=False, + ) + + # --- Assertions: MIRRORS file --- + mirrors_file = repo_dir / "MIRRORS" + self.assertTrue(mirrors_file.exists(), "MIRRORS file was not created") + + mirrors_content = mirrors_file.read_text(encoding="utf-8") + self.assertIn( + "pypi https://pypi.org/project/repo/", + mirrors_content, + "PyPI mirror entry must exist in MIRRORS", + ) + self.assertIn( + "origin git@github.com:acme/repo.git", + mirrors_content, + "origin SSH URL must exist in MIRRORS", + ) + + # --- Assertions: git config must NOT contain PyPI --- + git_config = repo_dir / ".git" / "config" + self.assertTrue(git_config.exists(), ".git/config was not created") + + git_config_content = git_config.read_text(encoding="utf-8") + self.assertNotIn( + "pypi.org/project", + git_config_content, + "PyPI must never be written into git config", + ) + + # --- Assertions: origin remote exists and points to SSH --- + remotes = subprocess.check_output( + ["git", "-C", str(repo_dir), "remote"], + text=True, + ).splitlines() + + self.assertIn("origin", remotes, "origin remote was not created") + + remote_v = subprocess.check_output( + ["git", "-C", str(repo_dir), "remote", "-v"], + text=True, + ) + self.assertIn("git@github.com:acme/repo.git", remote_v) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/pkgmgr/actions/repository/create/__init__.py b/tests/unit/pkgmgr/actions/repository/create/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/pkgmgr/actions/repository/test_create_parsing.py b/tests/unit/pkgmgr/actions/repository/create/test_create_parsing.py similarity index 89% rename from tests/unit/pkgmgr/actions/repository/test_create_parsing.py rename to tests/unit/pkgmgr/actions/repository/create/test_create_parsing.py index 8c52ce0..d59b61a 100644 --- a/tests/unit/pkgmgr/actions/repository/test_create_parsing.py +++ b/tests/unit/pkgmgr/actions/repository/create/test_create_parsing.py @@ -2,9 +2,9 @@ from __future__ import annotations import unittest -from pkgmgr.actions.repository.create import ( - RepoParts, - _parse_identifier, +from pkgmgr.actions.repository.create.model import RepoParts +from pkgmgr.actions.repository.create.parser import ( + parse_identifier, _parse_git_url, _strip_git_suffix, _split_host_port, @@ -22,7 +22,7 @@ class TestRepositoryCreateParsing(unittest.TestCase): self.assertEqual(_split_host_port("example.com:"), ("example.com", None)) def test_parse_identifier_plain(self) -> None: - parts = _parse_identifier("github.com/owner/repo") + parts = parse_identifier("github.com/owner/repo") self.assertIsInstance(parts, RepoParts) self.assertEqual(parts.host, "github.com") self.assertEqual(parts.port, None) @@ -30,7 +30,7 @@ class TestRepositoryCreateParsing(unittest.TestCase): self.assertEqual(parts.name, "repo") def test_parse_identifier_with_port(self) -> None: - parts = _parse_identifier("gitea.example.com:2222/org/repo") + parts = parse_identifier("gitea.example.com:2222/org/repo") self.assertEqual(parts.host, "gitea.example.com") self.assertEqual(parts.port, "2222") self.assertEqual(parts.owner, "org") diff --git a/tests/unit/pkgmgr/actions/repository/create/test_scaffold_render_preview.py b/tests/unit/pkgmgr/actions/repository/create/test_scaffold_render_preview.py new file mode 100644 index 0000000..13719e5 --- /dev/null +++ b/tests/unit/pkgmgr/actions/repository/create/test_scaffold_render_preview.py @@ -0,0 +1,43 @@ +from __future__ import annotations + +import unittest +from unittest.mock import patch + +from pkgmgr.actions.repository.create.templates import TemplateRenderer + + +class TestTemplateRendererPreview(unittest.TestCase): + def test_render_preview_does_not_write(self) -> None: + # Ensure TemplateRenderer does not try to resolve real repo root. + with ( + patch( + "pkgmgr.actions.repository.create.templates.TemplateRenderer._resolve_templates_dir", + return_value="/tpl", + ), + patch( + "pkgmgr.actions.repository.create.templates.os.walk", + return_value=[("/tpl", [], ["README.md.j2"])], + ), + patch( + "pkgmgr.actions.repository.create.templates.os.path.relpath", + return_value="README.md.j2", + ), + patch("pkgmgr.actions.repository.create.templates.os.makedirs") as mk, + patch("pkgmgr.actions.repository.create.templates.open", create=True) as op, + patch("pkgmgr.actions.repository.create.templates.Environment") as env_cls, + ): + renderer = TemplateRenderer() + + renderer.render( + repo_dir="/repo", + context={"repository": "x"}, + preview=True, + ) + + mk.assert_not_called() + op.assert_not_called() + env_cls.assert_not_called() + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/unit/pkgmgr/actions/repository/test_scaffold_render_preview.py b/tests/unit/pkgmgr/actions/repository/test_scaffold_render_preview.py deleted file mode 100644 index 0b81ef0..0000000 --- a/tests/unit/pkgmgr/actions/repository/test_scaffold_render_preview.py +++ /dev/null @@ -1,35 +0,0 @@ -from __future__ import annotations - -import unittest -from unittest.mock import patch - -from pkgmgr.actions.repository.scaffold import render_default_templates - - -class TestScaffoldRenderPreview(unittest.TestCase): - def test_render_preview_does_not_write(self) -> None: - with ( - patch("pkgmgr.actions.repository.scaffold._templates_dir", return_value="/tpl"), - patch("pkgmgr.actions.repository.scaffold.os.path.isdir", return_value=True), - patch("pkgmgr.actions.repository.scaffold.os.walk", return_value=[("/tpl", [], ["README.md.j2"])]), - patch("pkgmgr.actions.repository.scaffold.os.path.relpath", return_value="README.md.j2"), - patch("pkgmgr.actions.repository.scaffold.os.makedirs") as mk, - patch("pkgmgr.actions.repository.scaffold.open", create=True) as op, - patch("pkgmgr.actions.repository.scaffold.Environment") as env_cls, - ): - env = env_cls.return_value - env.get_template.return_value.render.return_value = "X" - - render_default_templates( - "/repo", - context={"repository": "x"}, - preview=True, - ) - - mk.assert_not_called() - op.assert_not_called() - env.get_template.assert_not_called() - - -if __name__ == "__main__": - unittest.main()