From 3cb7852cb4e5d0691b1ddcd502ef27c96ad32768 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 16 Dec 2025 19:49:09 +0100 Subject: [PATCH] feat(mirrors): support URL-only MIRRORS entries and keep git config clean - Allow MIRRORS to contain plain URLs (one per line) in addition to legacy "NAME URL" - Treat strings as single URLs to avoid iterable pitfalls - Write PyPI URLs as metadata-only entries (never added to git config) - Keep MIRRORS as the single source of truth for mirror setup - Update integration test to assert URL-only MIRRORS output https://chatgpt.com/share/6941a9aa-b8b4-800f-963d-2486b34856b1 --- src/pkgmgr/actions/mirror/io.py | 48 +++++++++++++++++-- .../actions/repository/create/mirrors.py | 13 +++-- ...est_repos_create_pypi_not_in_git_config.py | 4 +- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/pkgmgr/actions/mirror/io.py b/src/pkgmgr/actions/mirror/io.py index d93ebf2..c5c9098 100644 --- a/src/pkgmgr/actions/mirror/io.py +++ b/src/pkgmgr/actions/mirror/io.py @@ -1,8 +1,9 @@ from __future__ import annotations import os +from collections.abc import Iterable, Mapping +from typing import Union from urllib.parse import urlparse -from typing import Mapping from .types import MirrorMap, Repository @@ -32,7 +33,7 @@ def read_mirrors_file(repo_dir: str, filename: str = "MIRRORS") -> MirrorMap: """ Supports: NAME URL - URL → auto name = hostname + URL -> auto-generate name from hostname """ path = os.path.join(repo_dir, filename) mirrors: MirrorMap = {} @@ -52,7 +53,8 @@ def read_mirrors_file(repo_dir: str, filename: str = "MIRRORS") -> MirrorMap: # Case 1: "name url" if len(parts) == 2: name, url = parts - # Case 2: "url" → auto-generate name + + # Case 2: "url" -> auto name elif len(parts) == 1: url = parts[0] parsed = urlparse(url) @@ -67,21 +69,56 @@ def read_mirrors_file(repo_dir: str, filename: str = "MIRRORS") -> MirrorMap: continue mirrors[name] = url + except OSError as exc: print(f"[WARN] Could not read MIRRORS file at {path}: {exc}") return mirrors +MirrorsInput = Union[Mapping[str, str], Iterable[str]] + + def write_mirrors_file( repo_dir: str, - mirrors: Mapping[str, str], + mirrors: MirrorsInput, filename: str = "MIRRORS", preview: bool = False, ) -> None: + """ + Write MIRRORS in one of two formats: + 1) Mapping[str, str] -> "NAME URL" per line (legacy / compatible) + 2) Iterable[str] -> "URL" per line (new preferred) + + Strings are treated as a single URL (not iterated character-by-character). + """ path = os.path.join(repo_dir, filename) - lines = [f"{name} {url}" for name, url in sorted(mirrors.items())] + + lines: list[str] + + if isinstance(mirrors, Mapping): + items = [ + (str(name), str(url)) + for name, url in mirrors.items() + if url is not None and str(url).strip() + ] + items.sort(key=lambda x: (x[0], x[1])) + lines = [f"{name} {url}" for name, url in items] + + else: + if isinstance(mirrors, (str, bytes)): + urls = [str(mirrors).strip()] + else: + urls = [ + str(url).strip() + for url in mirrors + if url is not None and str(url).strip() + ] + + urls = sorted(set(urls)) + lines = urls + content = "\n".join(lines) + ("\n" if lines else "") if preview: @@ -94,5 +131,6 @@ def write_mirrors_file( with open(path, "w", encoding="utf-8") as fh: fh.write(content) print(f"[INFO] Wrote MIRRORS file at {path}") + except OSError as exc: print(f"[ERROR] Failed to write MIRRORS file at {path}: {exc}") diff --git a/src/pkgmgr/actions/repository/create/mirrors.py b/src/pkgmgr/actions/repository/create/mirrors.py index fbe15d4..97917bb 100644 --- a/src/pkgmgr/actions/repository/create/mirrors.py +++ b/src/pkgmgr/actions/repository/create/mirrors.py @@ -12,8 +12,8 @@ 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). + Defaults are written to MIRRORS and mirror setup derives + git remotes exclusively from that file (git URLs only). """ def write_defaults( @@ -25,10 +25,8 @@ class MirrorBootstrapper: 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}/", + primary, + f"https://pypi.org/project/{name}/", } write_mirrors_file(repo_dir, mirrors, preview=preview) @@ -41,7 +39,8 @@ class MirrorBootstrapper: preview: bool, remote: bool, ) -> None: - # IMPORTANT: do NOT set repo["mirrors"] here. + # IMPORTANT: + # Do NOT set repo["mirrors"] here. # MIRRORS file is the single source of truth. setup_mirrors( selected_repos=[repo], 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 index 42dc944..9e4e5f5 100644 --- a/tests/integration/test_repos_create_pypi_not_in_git_config.py +++ b/tests/integration/test_repos_create_pypi_not_in_git_config.py @@ -75,12 +75,12 @@ class TestCreateRepoPypiNotInGitConfig(unittest.TestCase): mirrors_content = mirrors_file.read_text(encoding="utf-8") self.assertIn( - "pypi https://pypi.org/project/repo/", + "https://pypi.org/project/repo/", mirrors_content, "PyPI mirror entry must exist in MIRRORS", ) self.assertIn( - "origin git@github.com:acme/repo.git", + "git@github.com:acme/repo.git", mirrors_content, "origin SSH URL must exist in MIRRORS", )