diff --git a/.github/workflows/cruft-prs.yml b/.github/workflows/cruft-prs.yml index e3a77c24..a2f9ab98 100644 --- a/.github/workflows/cruft-prs.yml +++ b/.github/workflows/cruft-prs.yml @@ -30,3 +30,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.BOT_GH_TOKEN }} FORCE_COLOR: "1" COLUMNS: "150" + - uses: actions/upload-artifact@v3 + with: + name: cruft-logs + path: log/ diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 80f3bd20..6e40f48c 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -5,8 +5,6 @@ on: branches: [main] pull_request: branches: [main] - schedule: - - cron: "0 5 1,15 * *" defaults: run: @@ -81,6 +79,8 @@ jobs: python-version: "3.11" cache: "pip" cache-dependency-path: "**/pyproject.toml" + - name: set python default branch + run: git config --global init.defaultBranch main - name: Install build dependencies run: python -m pip install --upgrade pip wheel - name: Install package diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index d148592d..6c7e6ad5 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -23,6 +23,7 @@ dependencies = [ "GitPython", "PyGitHub", "PyYAML", + "pre-commit", # is ran by cruft ] [project.optional-dependencies] diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 32355f9c..91f326c5 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -86,6 +86,7 @@ def auth(self, url_str: str) -> str: class PR: con: GitHubConnection release: GHRelease + repo_id: str # something like grst-infercnvpy title_prefix: ClassVar[LiteralString] = "Update template to " branch_prefix: ClassVar[LiteralString] = "template-update-" @@ -96,7 +97,7 @@ def title(self) -> str: @property def branch(self) -> str: - return f"{self.branch_prefix}{self.release.tag_name}" + return f"{self.branch_prefix}{self.repo_id}-{self.release.tag_name}" @property def namespaced_head(self) -> str: @@ -109,10 +110,15 @@ def body(self) -> str: template_usage="https://cookiecutter-scverse-instance.readthedocs.io/en/latest/template_usage.html", ) - def matches(self, pr: PullRequest) -> bool: + def matches_prefix(self, pr: PullRequest) -> bool: + """Check if `pr` is either a current or previous template update PR by matching the branch name""" # Don’t compare title prefix, people might rename PRs return pr.head.ref.startswith(self.branch_prefix) and pr.user.id == self.con.user.id + def matches_current_version(self, pr: PullRequest) -> bool: + """Check if `pr` is a template update PR for the current version""" + return pr.head.ref == self.branch and pr.user.id == self.con.user.id + class RepoInfo(TypedDict): url: str @@ -138,9 +144,22 @@ def get_repo_urls(gh: Github) -> Generator[str]: yield repo["url"] -def run_cruft(cwd: Path) -> CompletedProcess: - args = [sys.executable, "-m", "cruft", "update", "--checkout=main", "--skip-apply-ask", "--project-dir=."] - return run(args, check=True, cwd=cwd) +def run_cruft(cwd: Path, *, tag_name: str, log_name: str) -> CompletedProcess: + args = [ + sys.executable, + "-m", + "cruft", + "update", + f"--checkout={tag_name}", + "--skip-apply-ask", + "--project-dir=.", + ] + + log_path = Path(f"./log/{log_name}.txt") + log_path.parent.mkdir(exist_ok=True) + + with log_path.open("w") as log_file: + return run(args, check=True, cwd=cwd, stdout=log_file, stderr=log_file) # GitHub says that up to 5 minutes of wait are OK, @@ -149,14 +168,26 @@ def run_cruft(cwd: Path) -> CompletedProcess: # Due to exponential backoff, we’ll maximally wait 2⁹ sec, or 8.5 min -def cruft_update(con: GitHubConnection, repo: GHRepo, path: Path, pr: PR) -> bool: +def cruft_update( # noqa: PLR0913 + con: GitHubConnection, + pr: PR, + *, + tag_name: str, + repo: GHRepo, + origin: GHRepo, + path: Path, +) -> bool: clone = retry_with_backoff( - lambda: Repo.clone_from(con.auth(repo.clone_url), path), retries=n_retries, exc_cls=GitCommandError + lambda: Repo.clone_from(con.auth(repo.clone_url), path), + retries=n_retries, + exc_cls=GitCommandError, ) - branch = clone.create_head(pr.branch, clone.active_branch) + upstream = clone.create_remote(name=pr.repo_id, url=origin.clone_url) + upstream.fetch() + branch = clone.create_head(pr.branch, f"{pr.repo_id}/{origin.default_branch}") branch.checkout() - run_cruft(path) + run_cruft(path, tag_name=tag_name, log_name=pr.branch) if not clone.is_dirty(): return False @@ -176,25 +207,45 @@ def cruft_update(con: GitHubConnection, repo: GHRepo, path: Path, pr: PR) -> boo def get_fork(con: GitHubConnection, repo: GHRepo) -> GHRepo: - if fork := next((f for f in repo.get_forks() if f.owner.id == con.user.id), None): - return fork + """Fork target repo into the scverse-bot namespace and wait until the fork has been created. + If the fork already exists it is reused. + """ fork = repo.create_fork() return retry_with_backoff(lambda: con.gh.get_repo(fork.id), retries=n_retries, exc_cls=UnknownObjectException) def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: - pr = PR(con, release) - log.info(f"Sending PR to {repo_url}: {pr.title}") + repo_id = repo_url.replace("https://github.com/", "").replace("/", "-") + pr = PR(con, release, repo_id) + log.info(f"Sending PR to {repo_url} : {pr.title}") # create fork, populate branch, do PR from it origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) repo = get_fork(con, origin) + + if old_pr := next((p for p in origin.get_pulls("open") if pr.matches_current_version(p)), None): + log.info( + f"PR for current version already exists: #{old_pr.number} with branch name `{old_pr.head.ref}`. Skipping." + ) + return + with TemporaryDirectory() as td: - updated = cruft_update(con, repo, Path(td), pr) + updated = cruft_update( + con, + pr, + tag_name=release.tag_name, + repo=repo, + origin=origin, + path=Path(td), + ) if updated: - if old_pr := next((p for p in origin.get_pulls("open") if pr.matches(p)), None): + if old_pr := next((p for p in origin.get_pulls("open") if pr.matches_prefix(p)), None): + log.info(f"Closing old PR #{old_pr.number} with branch name `{old_pr.head.ref}`.") old_pr.edit(state="closed") - origin.create_pull(pr.title, pr.body, origin.default_branch, pr.namespaced_head) + new_pr = origin.create_pull( + pr.title, pr.body, origin.default_branch, pr.namespaced_head, maintainer_can_modify=True + ) + log.info(f"Created PR #{new_pr.number} with branch name `{new_pr.head.ref}`.") def setup() -> None: @@ -210,8 +261,15 @@ def main(tag_name: str) -> None: con = GitHubConnection("scverse-bot", token) release = get_template_release(con.gh, tag_name) repo_urls = get_repo_urls(con.gh) + failed = 0 for repo_url in repo_urls: - make_pr(con, release, repo_url) + try: + make_pr(con, release, repo_url) + except Exception as e: + failed += 1 + log.exception(f"Error updating {repo_url}: %s", e) + + sys.exit(failed > 0) def cli() -> None: diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index db532014..cf035a1f 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -1,5 +1,6 @@ import json from dataclasses import dataclass +from pathlib import Path from typing import cast from warnings import catch_warnings, filterwarnings @@ -16,6 +17,7 @@ class MockGHRepo: git_url: str # git://github.com/foo/bar.git clone_url: str # https://github.com/foo/bar.git + default_branch: str # main @dataclass @@ -43,18 +45,22 @@ def repo(git_repo: GitRepo) -> GHRepo: (git_repo.workspace / "b").write_text("b content") git_repo.api.index.add(["a", "b"]) git_repo.api.index.commit("initial commit") - return cast(GHRepo, MockGHRepo(git_repo.uri, git_repo.uri)) + return cast(GHRepo, MockGHRepo(git_repo.uri, git_repo.uri, "main")) @pytest.fixture def pr(con) -> PR: - return PR(con, cast(GHRelease, MockRelease())) + return PR(con, cast(GHRelease, MockRelease()), "scverse-test") def test_cruft_update(con, repo, tmp_path, pr, git_repo: GitRepo, monkeypatch: pytest.MonkeyPatch): old_active_branch_name = git_repo.api.active_branch.name - monkeypatch.setattr("scverse_template_scripts.cruft_prs.run_cruft", lambda p: (p / "b").write_text("b modified")) - changed = cruft_update(con, repo, tmp_path, pr) + + def _mock_run_cruft(cwd: Path, *, tag_name, log_name): + return (cwd / "b").write_text("b modified") + + monkeypatch.setattr("scverse_template_scripts.cruft_prs.run_cruft", _mock_run_cruft) + changed = cruft_update(con, pr, tag_name="main", repo=repo, origin=repo, path=tmp_path) assert changed # TODO: add test for short circuit main_branch = git_repo.api.active_branch assert main_branch.name == old_active_branch_name, "Shouldn’t change active branch" diff --git a/{{cookiecutter.project_name}}/.github/workflows/test.yaml b/{{cookiecutter.project_name}}/.github/workflows/test.yaml index 790cdd31..263bbf8b 100644 --- a/{{cookiecutter.project_name}}/.github/workflows/test.yaml +++ b/{{cookiecutter.project_name}}/.github/workflows/test.yaml @@ -5,6 +5,8 @@ on: branches: [main] pull_request: branches: [main] + schedule: + - cron: "0 5 1,15 * *" concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/{{cookiecutter.project_name}}/.pre-commit-config.yaml b/{{cookiecutter.project_name}}/.pre-commit-config.yaml index d69018c2..94c745ad 100644 --- a/{{cookiecutter.project_name}}/.pre-commit-config.yaml +++ b/{{cookiecutter.project_name}}/.pre-commit-config.yaml @@ -39,6 +39,9 @@ repos: args: [--fix=lf] - id: trailing-whitespace - id: check-case-conflict + # Check that there are no merge conflicts (could be generated by template sync) + - id: check-merge-conflict + args: [--assume-in-merge] - repo: local hooks: - id: forbid-to-commit