diff --git a/copier_update/github.py b/copier_update/github.py index 507a760..261bf58 100644 --- a/copier_update/github.py +++ b/copier_update/github.py @@ -19,6 +19,7 @@ class Repository: full_name: str default_branch: str private: bool = False + fork: bool = False archived: bool = False disabled: bool = False @@ -72,6 +73,7 @@ def repositories(self, token: str) -> list[Repository]: full_name=str(item["full_name"]), default_branch=str(item.get("default_branch") or "main"), private=bool(item.get("private", False)), + fork=bool(item.get("fork", False)), archived=bool(item.get("archived", False)), disabled=bool(item.get("disabled", False)), ) @@ -99,14 +101,25 @@ def has_open_update(self, repository: Repository, token: str, branch_prefix: str return True return False - def create_pull_request(self, repository: Repository, token: str, branch: str, title: str) -> str: + def create_pull_request( + self, + repository: Repository, + token: str, + branch: str, + title: str, + *, + invalid: bool = False, + ) -> str: + body = "Automated update from the repository's Copier template." + if invalid: + body += "\n\n> [!WARNING]\n> Copier generated changes that fail `git diff --check`. Review them before merging." response = self._request( "POST", f"/repos/{repository.full_name}/pulls", token=token, json={ "base": repository.default_branch, - "body": "Automated update from the repository's Copier template.", + "body": body, "head": branch, "title": title, }, diff --git a/copier_update/tests/test_github.py b/copier_update/tests/test_github.py index 3776579..f3f6fd7 100644 --- a/copier_update/tests/test_github.py +++ b/copier_update/tests/test_github.py @@ -56,7 +56,7 @@ def test_lists_installation_repositories(): { "repositories": [ {"id": 1, "full_name": "owner/active", "default_branch": "trunk", "private": True}, - {"id": 2, "full_name": "owner/archived", "default_branch": "main", "archived": True}, + {"id": 2, "full_name": "owner/archived", "default_branch": "main", "fork": True, "archived": True}, ] }, ) @@ -65,7 +65,7 @@ def test_lists_installation_repositories(): assert client.repositories("installation-token") == [ Repository(id=1, full_name="owner/active", default_branch="trunk", private=True), - Repository(id=2, full_name="owner/archived", default_branch="main", archived=True), + Repository(id=2, full_name="owner/archived", default_branch="main", fork=True, archived=True), ] assert session.requests[0][2]["headers"]["Authorization"] == "Bearer installation-token" @@ -92,3 +92,14 @@ def test_detects_open_update_pull_request(): client = GitHubAppClient("123", "private-key", session=session) assert client.has_open_update(repository, "token", "copier-update") + + +def test_marks_invalid_update_pull_request(): + repository = Repository(id=1, full_name="owner/repository", default_branch="main") + session = FakeSession(FakeResponse(201, {"html_url": "https://github.com/owner/repository/pull/1"})) + client = GitHubAppClient("123", "private-key", session=session) + + url = client.create_pull_request(repository, "token", "copier-update", "Update from Copier", invalid=True) + + assert url == "https://github.com/owner/repository/pull/1" + assert "fail `git diff --check`" in session.requests[0][2]["json"]["body"] diff --git a/copier_update/tests/test_updater.py b/copier_update/tests/test_updater.py index f0df90d..b9df7ef 100644 --- a/copier_update/tests/test_updater.py +++ b/copier_update/tests/test_updater.py @@ -15,7 +15,7 @@ def __init__(self, repositories: list[Repository]) -> None: self._repositories = repositories self.answers = {repository.full_name: ".copier-answers.yaml" for repository in repositories} self.open_updates: set[str] = set() - self.pull_requests: list[tuple[str, str, str]] = [] + self.pull_requests: list[tuple[str, str, str, bool]] = [] self.token_requests: list[tuple[int, int | None]] = [] def installations(self) -> list[Installation]: @@ -35,8 +35,16 @@ def copier_answers_file(self, repository: Repository, token: str) -> str | None: def has_open_update(self, repository: Repository, token: str, branch_prefix: str) -> bool: return repository.full_name in self.open_updates - def create_pull_request(self, repository: Repository, token: str, branch: str, title: str) -> str: - self.pull_requests.append((repository.full_name, branch, title)) + def create_pull_request( + self, + repository: Repository, + token: str, + branch: str, + title: str, + *, + invalid: bool = False, + ) -> str: + self.pull_requests.append((repository.full_name, branch, title, invalid)) return f"https://github.com/{repository.full_name}/pull/1" @@ -85,16 +93,17 @@ def test_updates_eligible_repository_with_restricted_token(): def test_skips_inactive_unmanaged_and_open_repositories(): archived = Repository(id=1, full_name="owner/archived", default_branch="main", archived=True) - unmanaged = Repository(id=2, full_name="owner/unmanaged", default_branch="main") - open_update = Repository(id=3, full_name="owner/open", default_branch="main") - client = FakeClient([archived, unmanaged, open_update]) + fork = Repository(id=2, full_name="owner/fork", default_branch="main", fork=True) + unmanaged = Repository(id=3, full_name="owner/unmanaged", default_branch="main") + open_update = Repository(id=4, full_name="owner/open", default_branch="main") + client = FakeClient([archived, fork, unmanaged, open_update]) client.answers.pop(unmanaged.full_name) client.open_updates.add(open_update.full_name) updater = RecordingUpdater(client) summary = updater.run() - assert (summary.checked, summary.updated, summary.skipped, summary.failed) == (3, 0, 3, 0) + assert (summary.checked, summary.updated, summary.skipped, summary.failed) == (4, 0, 4, 0) assert updater.updated == [] assert client.token_requests == [(10, None)] @@ -110,6 +119,21 @@ def test_continues_after_repository_failure(): assert updater.updated[0][0] == second.full_name +def test_counts_conflicted_pull_request_as_updated_and_failed(): + repository = Repository(id=1, full_name="owner/repository", default_branch="main") + client = FakeClient([repository]) + updater = CommandRecordingUpdater( + client, + status=" M README.md\n", + invalid_diff="README.md:1: leftover conflict marker\n", + ) + + summary = updater.run() + + assert (summary.checked, summary.updated, summary.skipped, summary.failed) == (1, 1, 0, 1) + assert client.pull_requests[0][-1] is True + + def test_repository_filter_must_match_installation(): updater = RecordingUpdater(FakeClient([]), repository_filter="owner/missing") @@ -170,6 +194,11 @@ def fake_run(command, **kwargs): assert "COPIER_APP_ID" not in captured["environment"] assert "COPIER_APP_PRIVATE_KEY" not in captured["environment"] assert captured["environment"]["GIT_CONFIG_VALUE_0"].startswith("Authorization: Basic ") + assert captured["environment"]["GIT_CONFIG_COUNT"] == "3" + assert captured["environment"]["GIT_CONFIG_KEY_1"] == "url.https://github.com/.insteadOf" + assert captured["environment"]["GIT_CONFIG_VALUE_1"] == "git@github.com:" + assert captured["environment"]["GIT_CONFIG_KEY_2"] == "url.https://github.com/.insteadOf" + assert captured["environment"]["GIT_CONFIG_VALUE_2"] == "ssh://git@github.com/" def test_repository_update_runs_copier_and_opens_pull_request(): @@ -186,6 +215,7 @@ def test_repository_update_runs_copier_and_opens_pull_request(): "owner/repository", "copier-update-2026-08-01T12-34-56Z", "Update from Copier (2026-08-01T12-34-56Z)", + False, ) ] @@ -200,7 +230,7 @@ def test_repository_update_stops_when_copier_changes_only_ignored_files(): assert client.pull_requests == [] -def test_repository_update_stops_when_copier_leaves_conflicts(): +def test_repository_update_opens_pull_request_when_copier_leaves_conflicts(): repository = Repository(id=20, full_name="owner/repository", default_branch="main") client = FakeClient([repository]) updater = CommandRecordingUpdater( @@ -209,8 +239,15 @@ def test_repository_update_stops_when_copier_leaves_conflicts(): invalid_diff="README.md:1: leftover conflict marker\n", ) - with pytest.raises(RuntimeError, match="Copier produced invalid changes"): + with pytest.raises(RuntimeError, match="Opened .* with changes that fail git diff --check"): updater.update_repository(repository, ".copier-answers.yaml", "repository-token") - assert not any(command[:2] == ["git", "push"] for command, _ in updater.commands) - assert client.pull_requests == [] + assert any(command[:2] == ["git", "push"] for command, _ in updater.commands) + assert client.pull_requests == [ + ( + "owner/repository", + "copier-update-2026-08-01T12-34-56Z", + "Update from Copier (2026-08-01T12-34-56Z)", + True, + ) + ] diff --git a/copier_update/updater.py b/copier_update/updater.py index 50ebbf2..f6075c2 100644 --- a/copier_update/updater.py +++ b/copier_update/updater.py @@ -30,6 +30,10 @@ class UpdateSummary: failed: int = 0 +class InvalidUpdateError(RuntimeError): + """Raised after opening an update pull request containing invalid changes.""" + + class Updater: def __init__( self, @@ -76,6 +80,10 @@ def run(self) -> UpdateSummary: summary.checked += 1 try: result = self._consider_repository(installation.id, repository, installation_token) + except InvalidUpdateError as error: + summary.updated += 1 + summary.failed += 1 + LOGGER.error("%s", error) except Exception: summary.failed += 1 LOGGER.exception("Failed to update %s", repository.full_name) @@ -94,6 +102,9 @@ def run(self) -> UpdateSummary: return summary def _consider_repository(self, installation_id: int, repository: Repository, installation_token: str) -> bool: + if repository.fork: + LOGGER.info("Skipping fork %s", repository.full_name) + return False if repository.archived or repository.disabled: LOGGER.info("Skipping inactive repository %s", repository.full_name) return False @@ -130,11 +141,11 @@ def update_repository(self, repository: Repository, answers_file: str, token: st token=token, ) self._run(["copier", "update", "-A", "-f", "-a", answers_file], cwd=repository_path, token=token) + invalid_changes = "" try: self._run(["git", "diff", "--check"], cwd=repository_path, capture_output=True) except subprocess.CalledProcessError as error: - details = (error.stdout or error.stderr or "").strip() - raise RuntimeError(f"Copier produced invalid changes for {repository.full_name}:\n{details}") from error + invalid_changes = (error.stdout or error.stderr or "").strip() if not self._has_meaningful_changes(repository_path): LOGGER.info("No update available for %s", repository.full_name) @@ -148,8 +159,16 @@ def update_repository(self, repository: Repository, answers_file: str, token: st self._run(["git", "commit", "-s", "-m", title], cwd=repository_path) self._run(["git", "push", "origin", branch], cwd=repository_path, token=token) - pull_request_url = self.client.create_pull_request(repository, token, branch, title) + pull_request_url = self.client.create_pull_request( + repository, + token, + branch, + title, + invalid=bool(invalid_changes), + ) LOGGER.info("Opened %s", pull_request_url) + if invalid_changes: + raise InvalidUpdateError(f"Opened {pull_request_url} with changes that fail git diff --check:\n{invalid_changes}") return True def _has_meaningful_changes(self, repository_path: Path) -> bool: @@ -190,9 +209,13 @@ def _run( credentials = base64.b64encode(f"x-access-token:{token}".encode()).decode() environment.update( { - "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_COUNT": "3", "GIT_CONFIG_KEY_0": "http.https://github.com/.extraheader", "GIT_CONFIG_VALUE_0": f"Authorization: Basic {credentials}", + "GIT_CONFIG_KEY_1": "url.https://github.com/.insteadOf", + "GIT_CONFIG_VALUE_1": "git@github.com:", + "GIT_CONFIG_KEY_2": "url.https://github.com/.insteadOf", + "GIT_CONFIG_VALUE_2": "ssh://git@github.com/", } ) return subprocess.run(