From 2b0dceeb3aedef45b22815148c42c085aef0f79b Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Mon, 1 May 2023 21:12:03 +0200 Subject: [PATCH 01/36] Revert "fix typo (#165)" This reverts commit 5f091ed0952e4adac0c95bda4006bec0b4532995. --- {{cookiecutter.project_name}}/.github/workflows/sync.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/{{cookiecutter.project_name}}/.github/workflows/sync.yaml b/{{cookiecutter.project_name}}/.github/workflows/sync.yaml index 36471f21..da9b5206 100644 --- a/{{cookiecutter.project_name}}/.github/workflows/sync.yaml +++ b/{{cookiecutter.project_name}}/.github/workflows/sync.yaml @@ -36,7 +36,7 @@ jobs: title: Automated template update from cookiecutter-scverse body: | A new version of the [scverse cookiecutter template](https://github.com/scverse/cookiecutter-scverse/releases) - got released. This PR adds all new changes to your repository and helps to stay in sync with + got released. This PR adds all new changes to your repository and helps to to stay in sync with the latest best-practice template maintained by the scverse team. **If a merge conflict arised, a `.rej` file with the rejected patch is generated. You'll need to From c343b6c72f3c342409efcc44b4e81b659faf3053 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 15:45:47 +0300 Subject: [PATCH 02/36] Cruft PR workflow (#168) Co-authored-by: Gregor Sturm --- .github/workflows/cruft-prs.yml | 30 +++ .github/workflows/test.yaml | 140 +++++++----- .gitignore | 1 + .pre-commit-config.yaml | 32 ++- hooks/.make_rich_output.py | 93 -------- scripts/pyproject.toml | 108 +++++++++ .../src/scverse_template_scripts/__init__.py | 0 .../src/scverse_template_scripts/cruft_prs.py | 209 ++++++++++++++++++ .../make_rich_output.py | 63 ++++++ scripts/tests/test_cruft.py | 62 ++++++ .../.github/workflows/sync.yaml | 46 ---- 11 files changed, 574 insertions(+), 210 deletions(-) create mode 100644 .github/workflows/cruft-prs.yml delete mode 100644 hooks/.make_rich_output.py create mode 100644 scripts/pyproject.toml create mode 100644 scripts/src/scverse_template_scripts/__init__.py create mode 100644 scripts/src/scverse_template_scripts/cruft_prs.py create mode 100644 scripts/src/scverse_template_scripts/make_rich_output.py create mode 100644 scripts/tests/test_cruft.py delete mode 100644 {{cookiecutter.project_name}}/.github/workflows/sync.yaml diff --git a/.github/workflows/cruft-prs.yml b/.github/workflows/cruft-prs.yml new file mode 100644 index 00000000..d65dd904 --- /dev/null +++ b/.github/workflows/cruft-prs.yml @@ -0,0 +1,30 @@ +name: make cruft PRs for all projects using us +on: + release: + types: [published] + workflow_dispatch: + inputs: + release: + description: "Tag of the release PRs should me made for" + type: string + required: true +jobs: + cruft-prs: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Set up Python 3.11 + uses: actions/setup-python@v4 + with: + python-version: "3.11" + cache: "pip" + cache-dependency-path: "**/pyproject.toml" + - name: Install build dependencies + run: python -m pip install --upgrade pip wheel + - name: Install package with scripts + run: pip install ./scripts + - name: Update template repo registry + run: send-cruft-prs ${{ env.RELEASE }} + env: + RELEASE: ${{ github.event_name == 'release' && github.event.release.tag_name || github.event.inputs.release }} + GITHUB_TOKEN: ${{ secrets.BOT_GH_TOKEN }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 70eb9b87..1e034ac1 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -1,69 +1,87 @@ name: Test on: - push: - branches: [main] - pull_request: - branches: [main] + push: + branches: [main] + pull_request: + branches: [main] + +defaults: + run: + shell: bash -e {0} # -e to fail on error jobs: - test: - runs-on: ${{ matrix.os }} - defaults: - run: - shell: bash -e {0} # -e to fail on error + test: + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false + matrix: + python: ["3.9", "3.10"] + os: [ubuntu-latest] + # one that matches "project-name".lower().replace('-', '_'), one that doesn’t: + package-name: [project_name, package_alt] + env: + PROJECT_ROOT: project-name - strategy: - fail-fast: false - matrix: - python: ["3.8", "3.10"] - os: [ubuntu-latest] - # one that matches "project-name".lower().replace('-', '_'), one that doesn’t: - package-name: [project_name, package_alt] + steps: + # Setup + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python }} + cache: "pip" + cache-dependency-path: "{{cookiecutter.project_name}}/pyproject.toml" + - name: Install Ubuntu system dependencies + if: matrix.os == 'ubuntu-latest' + run: sudo apt-get install pandoc + - name: Install build & check dependencies + run: python -m pip install --upgrade pip wheel cookiecutter pre-commit + # Build + - name: Build from template + shell: python + run: | + from cookiecutter.main import cookiecutter + overrides = dict(package_name='${{ matrix.package-name }}') + cookiecutter('.', no_input=True, extra_context=overrides) + # Check + - name: Set up pre-commit cache + uses: actions/cache@v3 + with: + path: ~/.cache/pre-commit + key: pre-commit-3|${{ matrix.python }}|${{ hashFiles(format('{0}/.pre-commit-config.yaml', env.PROJECT_ROOT)) }} + - name: Run pre-commit + run: | + cd "$PROJECT_ROOT" + git add . + pre-commit run --verbose --color=always --show-diff-on-failure --all-files + - name: Install the package + run: | + cd $PROJECT_ROOT + pip install ".[doc]" + python -c "import ${{ matrix.package-name }}" + # Docs + - name: Build the documentation env: - PROJECT_ROOT: project-name + SPHINXOPTS: -W --keep-going + run: | + cd "$PROJECT_ROOT/docs" + make html - steps: - # Setup - - uses: actions/checkout@v2 - - name: Set up Python ${{ matrix.python }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python }} - cache: "pip" - cache-dependency-path: "**/pyproject.toml" - - name: Install Ubuntu system dependencies - if: matrix.os == 'ubuntu-latest' - run: sudo apt-get install pandoc - - name: Install build & check dependencies - run: python -m pip install --upgrade pip wheel cookiecutter pre-commit - # Build - - name: Build from template - shell: python - run: | - from cookiecutter.main import cookiecutter - overrides = dict(package_name='${{ matrix.package-name }}') - cookiecutter('.', no_input=True, extra_context=overrides) - # Check - - name: Set up pre-commit cache - uses: actions/cache@v3 - with: - path: ~/.cache/pre-commit - key: pre-commit-3|${{ matrix.python }}|${{ hashFiles(format('{0}/.pre-commit-config.yaml', env.PROJECT_ROOT)) }} - - name: Run pre-commit - run: | - cd "$PROJECT_ROOT" - git add . - pre-commit run --verbose --color=always --show-diff-on-failure --all-files - - name: Install the package - run: | - cd $PROJECT_ROOT - pip install ".[doc]" - python -c "import ${{ matrix.package-name }}" - # Docs - - name: Build the documentation - env: - SPHINXOPTS: -W --keep-going - run: | - cd "$PROJECT_ROOT/docs" - make html + test-scripts: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Python 3.11 + uses: actions/setup-python@v4 + with: + python-version: "3.11" + cache: "pip" + cache-dependency-path: "**/pyproject.toml" + - name: Install build dependencies + run: python -m pip install --upgrade pip wheel + - name: Install package + run: pip install ./scripts[test] + - name: Run tests + run: pytest -v --color=yes ./scripts diff --git a/.gitignore b/.gitignore index eaccc425..7bb0bd5e 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,4 @@ __pycache__/ # IDEs /.idea/ +/.vscode/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c2468b1a..c94bb010 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,11 +1,23 @@ repos: - - repo: https://github.com/pre-commit/mirrors-prettier - rev: v2.5.1 # Only update together with the pre-commit yaml in the tempalate! - hooks: - - id: prettier - # Newer versions of node don't work on systems that have an older version of GLIBC - # (in particular Ubuntu 18.04 and Centos 7) - # EOL of Centos 7 is in 2024-06, we can probably get rid of this then. - # See https://github.com/scverse/cookiecutter-scverse/issues/143 and - # https://github.com/jupyterlab/jupyterlab/issues/12675 - language_version: "17.9.1" + - repo: https://github.com/pre-commit/mirrors-prettier + rev: v2.5.1 # Only update together with the pre-commit yaml in the tempalate! + hooks: + - id: prettier + # Newer versions of node don't work on systems that have an older version of GLIBC + # (in particular Ubuntu 18.04 and Centos 7) + # EOL of Centos 7 is in 2024-06, we can probably get rid of this then. + # See https://github.com/scverse/cookiecutter-scverse/issues/143 and + # https://github.com/jupyterlab/jupyterlab/issues/12675 + language_version: "17.9.1" + # scripts + - repo: https://github.com/psf/black + rev: "23.3.0" + hooks: + - id: black + files: ^scripts/ + - repo: https://github.com/charliermarsh/ruff-pre-commit + rev: v0.0.263 + hooks: + - id: ruff + args: [--fix, --exit-non-zero-on-fix] + files: ^scripts/ diff --git a/hooks/.make_rich_output.py b/hooks/.make_rich_output.py deleted file mode 100644 index 6e974f06..00000000 --- a/hooks/.make_rich_output.py +++ /dev/null @@ -1,93 +0,0 @@ -# --- -# jupyter: -# jupytext: -# formats: ipynb,py:percent -# text_representation: -# extension: .py -# format_name: percent -# format_version: '1.3' -# jupytext_version: 1.14.0 -# kernelspec: -# display_name: Python 3 (ipykernel) -# language: python -# name: python3 -# --- - -# %% -from rich.console import Console -from rich.markdown import Markdown - -# %% [markdown] -# Set up online services -# -# Your repository is now ready. However, to use all features of the template you will need to set up the following online services. Clicking on the links will take you to the respective sections of the developer documentation. The developer documentation is also shipped as part of the template in docs/developer_docs.md. -# -# pre-commit.ci to check for inconsistencies and to enforce a code style -# readthedocs.org to build and host documentation -# codecov to generate test coverage reports -# -# All CI checks should pass, you are ready to start developing your new tool! -# Customizations -# -# Further instructions on using this template can be found in the dev docs included in the project. -# Committment -# -# We expect developers of scverse ecosystem packages to -# -# write unit tests -# provide documentation, including tutorials where applicable -# support users through github and the scverse discourse -# - -# %% -with open("/home/sturm/Downloads/report.txt", "wt") as report_file: - console = Console(file=report_file, width=72, force_terminal=True, color_system="standard", ) - console.print( - Markdown( - """ -# Set-up online services - -**Your repository is now ready. However, to use all features of the template you will need to set up the following online services.** -Clicking on the links will take you to the respective sections of the developer documentation. -The developer documentation is also shipped as part of the template in docs/developer_docs.md. - -1. [pre-commit.ci][setup-pre-commit] to check for inconsistencies and to enforce a code style -2. [readthedocs.org][setup-rtd] to build and host documentation -3. [codecov][setup-codecov] to generate test coverage reports - -All CI checks should pass, you are ready to start developing your new tool! - -# Install the package - -To run tests or build the documentation locally, you need to install your package and its dependencies. You can do so with - -```bash -pip install ".[test,dev,doc]" -``` - -# Customizations - -Further instructions on using this template can be found in the [dev docs included in the project](https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html). - -# Committment - -We expect developers of scverse ecosystem packages to - -- [write unit tests][write-tests] -- [provide documentation][write-docs], including tutorials where applicable -- support users through github and the [scverse discourse][] - -[setup-pre-commit]: https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html#pre-commit-checks -[setup-rtd]: https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html#documentation-on-readthedocs -[setup-codecov]: https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html#coverage-tests-with-codecov -[write-tests]: https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html#writing-tests -[write-docs]: https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html#writing-documentation -[scverse discourse]: https://discourse.scverse.org/ - -""" - ) - ) - -# %% - -# %% diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml new file mode 100644 index 00000000..30a9e687 --- /dev/null +++ b/scripts/pyproject.toml @@ -0,0 +1,108 @@ +[build-system] +requires = ["hatchling", "hatch-vcs"] +build-backend = "hatchling.build" + +[project] +name = "scverse-template-scripts" +dynamic = ["version"] +description = "scripts for ecosystem package data" +readme = "../README.md" +requires-python = ">=3.11" +license = "GPL-3.0" +authors = [ + { name = "Philipp A.", email = "flying-sheep@web.de" }, +] +urls.Documentation = "https://github.com/scverse/cookiecutter-scverse#readme" +urls.Issues = "https://github.com/scverse/cookiecutter-scverse/issues" +urls.Source = "https://github.com/scverse/cookiecutter-scverse" +dependencies = [ + "rich", + "typer", + "furl", + "GitPython", + "PyGitHub", + "PyYAML", +] + +[project.optional-dependencies] +test = [ + "pytest", + "pytest-git", + "pytest-network", + "pytest-responsemock", +] + +[project.scripts] +send-cruft-prs = "scverse_template_scripts.cruft_prs:cli" +make-rich-output = "scverse_template_scripts.make_rich_output:main" + +[tool.hatch.version] +source = "vcs" +fallback-version = "0.0" + +[tool.hatch.envs.default] +python = "3.11" + +[tool.hatch.envs.test] +features = ["test"] +[tool.hatch.envs.test.scripts] +test = "pytest" + +[tool.pytest.ini_options] +addopts = [ + "--import-mode=importlib", + "-pgit", + "-pnetwork", "--disable-network", + "-presponsemock" +] +filterwarnings = ["error"] + +[tool.black] +target-version = ["py310"] # py311 not supported yet +line-length = 120 + +[tool.ruff] +target-version = "py311" +line-length = 120 +allowed-confusables = ["’"] +select = [ + "A", + "ARG", + "B", + "C", + "DTZ", + "E", + "EM", + "F", + "FBT", + "I", + "ICN", + "ISC", + "N", + "PLC", + "PLE", + "PLR", + "PLW", + "Q", + "RUF", + "S", + "T", + "TID", + "UP", + "W", + "YTT", +] +ignore = [ + "S101", # assert should be allowed + "S603", # subprocess with shell=False should be allowed +] +unfixable = ["RUF001"] # never “fix” “confusables” + +[tool.ruff.isort] +known-first-party = ["scverse_template_scripts"] + +[tool.ruff.per-file-ignores] +"tests/*.py" = [ + "ARG001", # pytest fixtures don’t need to be used + "PLR0913", # allow as many pytest fixtures being used as one likes +] diff --git a/scripts/src/scverse_template_scripts/__init__.py b/scripts/src/scverse_template_scripts/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py new file mode 100644 index 00000000..437cc6be --- /dev/null +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -0,0 +1,209 @@ +"""Script to send cruft update PRs. + +Uses `template-repos.yml` from `scverse/ecosystem-packages`. +""" + +import os +from collections.abc import Generator +from dataclasses import InitVar, dataclass, field +from logging import basicConfig, getLogger +from pathlib import Path +from subprocess import CompletedProcess, run +from tempfile import TemporaryDirectory +from typing import IO, ClassVar, LiteralString, NotRequired, TypedDict, cast + +import typer +from furl import furl +from git.repo import Repo +from git.util import Actor +from github import ContentFile, Github +from github.AuthenticatedUser import AuthenticatedUser +from github.GitRelease import GitRelease as GHRelease +from github.NamedUser import NamedUser +from github.PullRequest import PullRequest +from github.Repository import Repository as GHRepo +from yaml import safe_load + +log = getLogger(__name__) + +PR_BODY_TEMPLATE = """\ +`cookiecutter-scverse` released [{release.title}]({release.html_url}). + +## Changes + +{release.body} + +## Additional remarks +* unsubscribe: If you don`t want to receive these PRs in the future, + add `skip: true` to [`template-repos.yml`][] using a PR or, + if you never want to sync from the template again, delete your `.cruft` file. +* If there are **merge conflicts**, + they either show up inline (`>>>>>>>`) or a `.rej` file will have been created for the respective files. + You need to address these conflicts manually. Make sure to enable pre-commit.ci (see below) to detect such files. +* The scverse template works best when the [pre-commit.ci][], [readthedocs][] and [codecov][] services are enabled. + Make sure to activate those apps if you haven't already. + +[template-repos.yml]: https://github.com/scverse/ecosystem-packages/blob/main/template-repos.yml +[pre-commit.ci]: {template_usage}#pre-commit-ci +[readthedocs]: {template_usage}#documentation-on-readthedocs +[codecov]: {template_usage}#coverage-tests-with-codecov +""" + + +@dataclass +class GitHubConnection: + name: InitVar[str] + token: str | None = field(repr=False, default=None) + gh: Github = field(init=False) + user: NamedUser | AuthenticatedUser = field(init=False) + sig: Actor = field(init=False) + + def __post_init__(self, name: str) -> None: + self.gh = Github(self.token) + self.user = self.gh.get_user(name) + self.sig = Actor(self.name, self.email) + + @property + def name(self) -> str: + return self.user.name + + @property + def email(self) -> str: + return self.user.email + + def auth(self, url_str: str) -> str: + url = furl(url_str) + if self.token: + url.username = self.name + url.password = self.token + return str(url) + + +@dataclass +class PR: + con: GitHubConnection + release: GHRelease + + title_prefix: ClassVar[LiteralString] = "Update template to " + branch_prefix: ClassVar[LiteralString] = "template-update-" + + @property + def title(self) -> str: + return f"{self.title_prefix}{self.release.tag_name}" + + @property + def branch(self) -> str: + return f"{self.branch_prefix}{self.release.tag_name}" + + @property + def body(self) -> str: + return PR_BODY_TEMPLATE.format( + release=self.release, + template_usage="https://cookiecutter-scverse-instance.readthedocs.io/en/latest/template_usage.html", + ) + + def matches(self, pr: PullRequest) -> bool: + # 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 + + +class RepoInfo(TypedDict): + url: str + skip: NotRequired[bool] + + +def get_template_release(gh: Github, tag_name: str) -> GHRelease: + template_repo = gh.get_repo("scverse/cookiecutter-scverse") + return template_repo.get_release(tag_name) + + +def parse_repos(f: IO[str] | str) -> list[RepoInfo]: + repos = cast(list[RepoInfo], safe_load(f)) + log.info(f"Found {len(repos)} known repos") + # TODO: return real repos + fake_repos = [RepoInfo(url="https://github.com/flying-sheep/cruft-pr-test")] + return fake_repos + + +def get_repo_urls(gh: Github) -> Generator[str]: + repo = gh.get_repo("scverse/ecosystem-packages") + file = cast(ContentFile, repo.get_contents("template-repos.yml")) + for repo in parse_repos(file.decoded_content): + if not repo.get("skip"): + yield repo["url"] + + +def run_cruft(cwd: Path) -> CompletedProcess: + args = ["cruft", "update", " --checkout=main", "--skip-apply-ask", "--project-dir=."] + return run(args, check=True, cwd=cwd) + + +def cruft_update(con: GitHubConnection, repo: GHRepo, path: Path, pr: PR) -> bool: + clone = Repo.clone_from(con.auth(repo.git_url), path) + branch = clone.create_head(pr.branch, clone.active_branch) + branch.checkout() + + run_cruft(path) + + if not clone.is_dirty(): + return False + + # Stage & Commit + # Maybe clean? changed_files = [diff.b_path or diff.a_path for diff in cast(list[Diff], clone.index.diff(None))] + # and then: clone.index.add([*clone.untracked_files, changed_files]) + clone.git.add(all=True) + commit = clone.index.commit(pr.title, parent_commits=[branch.commit], author=con.sig, committer=con.sig) + branch.commit = commit + + # Push + remote = clone.remote() + remote.set_url(con.auth(repo.clone_url)) + remote.push([branch.name]) + return True + + +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 + return repo.create_fork() + + +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}") + + # create fork, populate branch, do PR from it + origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) + repo = get_fork(con, origin) + with TemporaryDirectory() as td: + updated = cruft_update(con, repo, Path(td), pr) + if updated: + if old_pr := next((p for p in origin.get_pulls("open") if pr.matches(p)), None): + old_pr.edit(state="closed") + origin.create_pull(pr.title, pr.body, origin.default_branch, pr.branch) + + +def setup() -> None: + from rich.logging import RichHandler + from rich.traceback import install + + basicConfig(level="INFO", handlers=[RichHandler()]) + install(show_locals=True) + + +def main(tag_name: str) -> None: + token = os.environ["GITHUB_TOKEN"] + con = GitHubConnection("scverse-bot", token) + release = get_template_release(con.gh, tag_name) + repo_urls = get_repo_urls(con.gh) + for repo_url in repo_urls: + make_pr(con, release, repo_url) + + +def cli() -> None: + setup() + typer.run(main) + + +if __name__ == "__main__": + cli() diff --git a/scripts/src/scverse_template_scripts/make_rich_output.py b/scripts/src/scverse_template_scripts/make_rich_output.py new file mode 100644 index 00000000..2eaa489f --- /dev/null +++ b/scripts/src/scverse_template_scripts/make_rich_output.py @@ -0,0 +1,63 @@ +from rich.console import Console +from rich.markdown import Markdown + +dev_docs_url = "https://cookiecutter-scverse-instance.readthedocs.io/en/latest/developer_docs.html" + +message = f"""\ +# Set-up online services + +**Your repository is now ready. +However, to use all features of the template you will need to set up the following online services.** +Clicking on the links will take you to the respective sections of the developer documentation. +The developer documentation is also shipped as part of the template in docs/developer_docs.md. + +1. [pre-commit.ci][setup-pre-commit] to check for inconsistencies and to enforce a code style +2. [readthedocs.org][setup-rtd] to build and host documentation +3. [codecov][setup-codecov] to generate test coverage reports + +All CI checks should pass, you are ready to start developing your new tool! + +# Install the package + +To run tests or build the documentation locally, you need to install your package and its dependencies. +You can do so with + +```bash +pip install ".[test,dev,doc]" +``` + +# Customizations + +Further instructions on using this template can be found in the [dev docs included in the project][dev-docs]. + +# Commitment + +We expect developers of scverse ecosystem packages to + +- [write unit tests][write-tests] +- [provide documentation][write-docs], including tutorials where applicable +- support users through github and the [scverse discourse][] + +[dev-docs]: {dev_docs_url} +[setup-pre-commit]: {dev_docs_url}#pre-commit-checks +[setup-rtd]: {dev_docs_url}#documentation-on-readthedocs +[setup-codecov]: {dev_docs_url}#coverage-tests-with-codecov +[write-tests]: {dev_docs_url}#writing-tests +[write-docs]: {dev_docs_url}#writing-documentation +[scverse discourse]: https://discourse.scverse.org/ +""" + + +def main() -> None: + with open("report.txt", "w") as report_file: + console = Console( + file=report_file, + width=72, + force_terminal=True, + color_system="standard", + ) + console.print(Markdown(message)) + + +if __name__ == "__main__": + main() diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py new file mode 100644 index 00000000..83b2790d --- /dev/null +++ b/scripts/tests/test_cruft.py @@ -0,0 +1,62 @@ +import json +from dataclasses import dataclass +from typing import cast + +import pytest +from git import Commit, Diff +from github.GitRelease import GitRelease as GHRelease +from github.Repository import Repository as GHRepo +from pytest_git import GitRepo + +from scverse_template_scripts.cruft_prs import PR, GitHubConnection, cruft_update + + +@dataclass +class MockGHRepo: + git_url: str # git://github.com/foo/bar.git + clone_url: str # https://github.com/foo/bar.git + + +@dataclass +class MockRelease: + tag_name: str = "test-tag" + title: str = "A test release" + body: str = "* Some changelog entry" + html_url: str = "https://example.com" + + +@pytest.fixture +def con(response_mock) -> GitHubConnection: + resp = json.dumps({}) # TODO: enter info once the tests need it + with response_mock(f"GET https://api.github.com:443/users/scverse-bot -> 200 :{resp}"): + return GitHubConnection("scverse-bot") + + +@pytest.fixture +def repo(git_repo: GitRepo) -> GHRepo: + assert not git_repo.api.bare + (git_repo.workspace / "a").write_text("a content") + (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)) + + +@pytest.fixture +def pr(con) -> PR: + return PR(con, cast(GHRelease, MockRelease())) + + +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) + 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" + pr_branch = next(b for b in git_repo.api.branches if b.name == pr.branch) + commit = cast(Commit, pr_branch.commit) + assert list(commit.parents) == [main_branch.commit] + assert [ + (diff.change_type, diff.a_path, diff.b_path) for diff in cast(list[Diff], main_branch.commit.diff(commit)) + ] == [("M", "b", "b")] diff --git a/{{cookiecutter.project_name}}/.github/workflows/sync.yaml b/{{cookiecutter.project_name}}/.github/workflows/sync.yaml deleted file mode 100644 index da9b5206..00000000 --- a/{{cookiecutter.project_name}}/.github/workflows/sync.yaml +++ /dev/null @@ -1,46 +0,0 @@ -name: Sync Template - -on: - workflow_dispatch: - schedule: - - cron: "0 2 * * *" # every night at 2:00 UTC - -jobs: - sync: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: Set up Python 3.10 - uses: actions/setup-python@v4 - with: - python-version: "3.10" - - name: Install dependencies - # for now, pin cookiecutter version, due to https://github.com/cruft/cruft/issues/166 - run: python -m pip install --upgrade cruft "cookiecutter<2" pre-commit toml - - name: Find Latest Tag - uses: oprypin/find-latest-tag@v1.1.0 - id: get-latest-tag - with: - repository: scverse/cookiecutter-scverse - releases-only: false - sort-tags: true - regex: '^v\d+\.\d+\.\d+$' # vX.X.X - - name: Sync - run: | - cruft update --checkout ${{ steps.get-latest-tag.outputs.tag }} --skip-apply-ask --project-dir . - - name: Create Pull Request - uses: peter-evans/create-pull-request@v4 - with: - commit-message: Automated template update from cookiecutter-scverse - branch: template-update - title: Automated template update from cookiecutter-scverse - body: | - A new version of the [scverse cookiecutter template](https://github.com/scverse/cookiecutter-scverse/releases) - got released. This PR adds all new changes to your repository and helps to to stay in sync with - the latest best-practice template maintained by the scverse team. - - **If a merge conflict arised, a `.rej` file with the rejected patch is generated. You'll need to - manually merge these changes.** - - For more information about the template sync, please refer to the - [template documentation](https://cookiecutter-scverse-instance.readthedocs.io/en/latest/template_usage.html#automated-template-sync). From 40cb337f7226e4b25c8521c9aef9bbe2ffc59ca6 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 16:34:42 +0300 Subject: [PATCH 03/36] Fix `git clone` in cruft PR workflow (#196) --- .github/workflows/cruft-prs.yml | 2 ++ .gitignore | 9 ++------- scripts/src/scverse_template_scripts/cruft_prs.py | 7 +++---- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/.github/workflows/cruft-prs.yml b/.github/workflows/cruft-prs.yml index d65dd904..e3a77c24 100644 --- a/.github/workflows/cruft-prs.yml +++ b/.github/workflows/cruft-prs.yml @@ -28,3 +28,5 @@ jobs: env: RELEASE: ${{ github.event_name == 'release' && github.event.release.tag_name || github.event.inputs.release }} GITHUB_TOKEN: ${{ secrets.BOT_GH_TOKEN }} + FORCE_COLOR: "1" + COLUMNS: "150" diff --git a/.gitignore b/.gitignore index 7bb0bd5e..72cdf379 100644 --- a/.gitignore +++ b/.gitignore @@ -2,23 +2,18 @@ .DS_Store *~ -# Compiled files -__pycache__/ - # Distribution / packaging /build/ /dist/ /*.egg-info/ # Tests and coverage +__pycache__/ +.ruff_cache/ /.pytest_cache/ /.cache/ /data/ -# docs -/docs/generated/ -/docs/_build/ - # IDEs /.idea/ /.vscode/ diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 437cc6be..daff5f27 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -64,7 +64,7 @@ def __post_init__(self, name: str) -> None: self.sig = Actor(self.name, self.email) @property - def name(self) -> str: + def name(self) -> str | None: return self.user.name @property @@ -74,8 +74,7 @@ def email(self) -> str: def auth(self, url_str: str) -> str: url = furl(url_str) if self.token: - url.username = self.name - url.password = self.token + url.username = self.token return str(url) @@ -139,7 +138,7 @@ def run_cruft(cwd: Path) -> CompletedProcess: def cruft_update(con: GitHubConnection, repo: GHRepo, path: Path, pr: PR) -> bool: - clone = Repo.clone_from(con.auth(repo.git_url), path) + clone = Repo.clone_from(con.auth(repo.clone_url), path) branch = clone.create_head(pr.branch, clone.active_branch) branch.checkout() From aee3566ed60eb6fb0be387962b2626d9b0b20e8c Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 16:45:21 +0300 Subject: [PATCH 04/36] depend on cruft in cruft PRs workflow (#197) --- scripts/pyproject.toml | 1 + scripts/src/scverse_template_scripts/cruft_prs.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index 30a9e687..7052ee1a 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -16,6 +16,7 @@ urls.Documentation = "https://github.com/scverse/cookiecutter-scverse#readme" urls.Issues = "https://github.com/scverse/cookiecutter-scverse/issues" urls.Source = "https://github.com/scverse/cookiecutter-scverse" dependencies = [ + "cruft", "rich", "typer", "furl", diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index daff5f27..30b4a1d9 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -4,6 +4,7 @@ """ import os +import sys from collections.abc import Generator from dataclasses import InitVar, dataclass, field from logging import basicConfig, getLogger @@ -133,7 +134,7 @@ def get_repo_urls(gh: Github) -> Generator[str]: def run_cruft(cwd: Path) -> CompletedProcess: - args = ["cruft", "update", " --checkout=main", "--skip-apply-ask", "--project-dir=."] + args = [sys.executable, "-m", "cruft", "update", " --checkout=main", "--skip-apply-ask", "--project-dir=."] return run(args, check=True, cwd=cwd) From f2e3dcab13104c73cb13899016bd58d678e4b61b Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 16:50:28 +0300 Subject: [PATCH 05/36] Fix cruft invocation typo (#198) --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 30b4a1d9..f931915a 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -134,7 +134,7 @@ def get_repo_urls(gh: Github) -> Generator[str]: def run_cruft(cwd: Path) -> CompletedProcess: - args = [sys.executable, "-m", "cruft", "update", " --checkout=main", "--skip-apply-ask", "--project-dir=."] + args = [sys.executable, "-m", "cruft", "update", "--checkout=main", "--skip-apply-ask", "--project-dir=."] return run(args, check=True, cwd=cwd) From 66efb06dcd0360226b891309cea0a8df24aa366c Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 17:14:53 +0300 Subject: [PATCH 06/36] Use namespaced head in cruft PRs (#199) --- scripts/src/scverse_template_scripts/cruft_prs.py | 10 ++++++++-- scripts/tests/test_cruft.py | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index f931915a..a06e8ca2 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -63,9 +63,11 @@ def __post_init__(self, name: str) -> None: self.gh = Github(self.token) self.user = self.gh.get_user(name) self.sig = Actor(self.name, self.email) + assert isinstance(self.name, str) @property - def name(self) -> str | None: + def name(self) -> str: + assert self.user.name is not None return self.user.name @property @@ -95,6 +97,10 @@ def title(self) -> str: def branch(self) -> str: return f"{self.branch_prefix}{self.release.tag_name}" + @property + def namespaced_head(self) -> str: + return f"{self.con.name}:{self.branch}" + @property def body(self) -> str: return PR_BODY_TEMPLATE.format( @@ -180,7 +186,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: if updated: if old_pr := next((p for p in origin.get_pulls("open") if pr.matches(p)), None): old_pr.edit(state="closed") - origin.create_pull(pr.title, pr.body, origin.default_branch, pr.branch) + origin.create_pull(pr.title, pr.body, origin.default_branch, pr.namespaced_head) def setup() -> None: diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index 83b2790d..2bb035e6 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -27,7 +27,7 @@ class MockRelease: @pytest.fixture def con(response_mock) -> GitHubConnection: - resp = json.dumps({}) # TODO: enter info once the tests need it + resp = json.dumps({"name": "scverse-bot"}) with response_mock(f"GET https://api.github.com:443/users/scverse-bot -> 200 :{resp}"): return GitHubConnection("scverse-bot") From f9a029b7e02038b4bef4420b9bd6b775e842c4d6 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 17:29:35 +0300 Subject: [PATCH 07/36] Use user.login instead of user.name (#200) --- .../src/scverse_template_scripts/cruft_prs.py | 19 ++++++++----------- scripts/tests/test_cruft.py | 2 +- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index a06e8ca2..7f007e8c 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -18,7 +18,6 @@ from git.repo import Repo from git.util import Actor from github import ContentFile, Github -from github.AuthenticatedUser import AuthenticatedUser from github.GitRelease import GitRelease as GHRelease from github.NamedUser import NamedUser from github.PullRequest import PullRequest @@ -53,22 +52,20 @@ @dataclass class GitHubConnection: - name: InitVar[str] + login: InitVar[str] token: str | None = field(repr=False, default=None) gh: Github = field(init=False) - user: NamedUser | AuthenticatedUser = field(init=False) + user: NamedUser = field(init=False) sig: Actor = field(init=False) - def __post_init__(self, name: str) -> None: + def __post_init__(self, login: str) -> None: self.gh = Github(self.token) - self.user = self.gh.get_user(name) - self.sig = Actor(self.name, self.email) - assert isinstance(self.name, str) + self.user = self.gh.get_user(login) + self.sig = Actor(self.login, self.email) @property - def name(self) -> str: - assert self.user.name is not None - return self.user.name + def login(self) -> str: + return self.user.login @property def email(self) -> str: @@ -99,7 +96,7 @@ def branch(self) -> str: @property def namespaced_head(self) -> str: - return f"{self.con.name}:{self.branch}" + return f"{self.con.login}:{self.branch}" @property def body(self) -> str: diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index 2bb035e6..0615eb23 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -27,7 +27,7 @@ class MockRelease: @pytest.fixture def con(response_mock) -> GitHubConnection: - resp = json.dumps({"name": "scverse-bot"}) + resp = json.dumps({"login": "scverse-bot"}) with response_mock(f"GET https://api.github.com:443/users/scverse-bot -> 200 :{resp}"): return GitHubConnection("scverse-bot") From 1929d97848fd6a62fe2916dec21d6c3de99cd0a2 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 1 Jun 2023 17:41:19 +0300 Subject: [PATCH 08/36] Finish cruft PRs (#201) --- scripts/src/scverse_template_scripts/cruft_prs.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 7f007e8c..901f7b4b 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -27,14 +27,14 @@ log = getLogger(__name__) PR_BODY_TEMPLATE = """\ -`cookiecutter-scverse` released [{release.title}]({release.html_url}). +`cookiecutter-scverse` released [{release.tag_name}]({release.html_url}). ## Changes {release.body} ## Additional remarks -* unsubscribe: If you don`t want to receive these PRs in the future, +* unsubscribe: If you don’t want to receive these PRs in the future, add `skip: true` to [`template-repos.yml`][] using a PR or, if you never want to sync from the template again, delete your `.cruft` file. * If there are **merge conflicts**, @@ -43,7 +43,7 @@ * The scverse template works best when the [pre-commit.ci][], [readthedocs][] and [codecov][] services are enabled. Make sure to activate those apps if you haven't already. -[template-repos.yml]: https://github.com/scverse/ecosystem-packages/blob/main/template-repos.yml +[`template-repos.yml`]: https://github.com/scverse/ecosystem-packages/blob/main/template-repos.yml [pre-commit.ci]: {template_usage}#pre-commit-ci [readthedocs]: {template_usage}#documentation-on-readthedocs [codecov]: {template_usage}#coverage-tests-with-codecov @@ -123,9 +123,7 @@ def get_template_release(gh: Github, tag_name: str) -> GHRelease: def parse_repos(f: IO[str] | str) -> list[RepoInfo]: repos = cast(list[RepoInfo], safe_load(f)) log.info(f"Found {len(repos)} known repos") - # TODO: return real repos - fake_repos = [RepoInfo(url="https://github.com/flying-sheep/cruft-pr-test")] - return fake_repos + return repos def get_repo_urls(gh: Github) -> Generator[str]: From e03a54d9ec23e4479ea0fec3490929002b479e30 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Fri, 2 Jun 2023 14:43:06 +0300 Subject: [PATCH 09/36] Cruft PRs: wait for fork creation (#202) --- scripts/pyproject.toml | 3 ++- .../src/scverse_template_scripts/backoff.py | 23 +++++++++++++++++++ .../src/scverse_template_scripts/cruft_prs.py | 14 +++++++++-- 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 scripts/src/scverse_template_scripts/backoff.py diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index 7052ee1a..2aa44e15 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -65,7 +65,7 @@ line-length = 120 [tool.ruff] target-version = "py311" line-length = 120 -allowed-confusables = ["’"] +allowed-confusables = ["’", "×"] select = [ "A", "ARG", @@ -96,6 +96,7 @@ select = [ ignore = [ "S101", # assert should be allowed "S603", # subprocess with shell=False should be allowed + "S311", # we don’t need cryptographically secure RNG ] unfixable = ["RUF001"] # never “fix” “confusables” diff --git a/scripts/src/scverse_template_scripts/backoff.py b/scripts/src/scverse_template_scripts/backoff.py new file mode 100644 index 00000000..4bbb697b --- /dev/null +++ b/scripts/src/scverse_template_scripts/backoff.py @@ -0,0 +1,23 @@ +import random +import time +from collections.abc import Callable +from typing import TypeVar + +T = TypeVar("T") + + +def retry_with_backoff( + fn: Callable[[], T], + retries: int = 5, + backoff_in_seconds: int | float = 1, + exc_cls: type = Exception, +) -> T: + exc = None + for x in range(retries): + try: + return fn() + except exc_cls as _exc: + exc = _exc + sleep = backoff_in_seconds * 2**x + random.uniform(0, 1) + time.sleep(sleep) + raise exc diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 901f7b4b..52d5c502 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -3,6 +3,7 @@ Uses `template-repos.yml` from `scverse/ecosystem-packages`. """ +import math import os import sys from collections.abc import Generator @@ -17,13 +18,15 @@ from furl import furl from git.repo import Repo from git.util import Actor -from github import ContentFile, Github +from github import ContentFile, Github, UnknownObjectException from github.GitRelease import GitRelease as GHRelease from github.NamedUser import NamedUser from github.PullRequest import PullRequest from github.Repository import Repository as GHRepo from yaml import safe_load +from .backoff import retry_with_backoff + log = getLogger(__name__) PR_BODY_TEMPLATE = """\ @@ -163,10 +166,17 @@ def cruft_update(con: GitHubConnection, repo: GHRepo, path: Path, pr: PR) -> boo return True +# GitHub says that up to 5 minutes of wait are OK, +# So we error our once we wait longer, i.e. when 2ⁿ = 5 min × 60 sec/min +n_retries = math.ceil(math.log(5 * 60) / math.log(2)) # = ⌈~8.22⌉ = 9 +# Due to exponential backoff, we’ll maximally wait 2⁹ sec, or 8.5 min + + 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 - return repo.create_fork() + 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: From 03ef73af73ea4a927decc396232d9c2c5e958ee1 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Tue, 6 Jun 2023 16:55:55 +0200 Subject: [PATCH 10/36] Another backoff for cruft (#207) * Add retry with backoff also for clone * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../src/scverse_template_scripts/cruft_prs.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 52d5c502..32355f9c 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -16,6 +16,7 @@ import typer from furl import furl +from git.exc import GitCommandError from git.repo import Repo from git.util import Actor from github import ContentFile, Github, UnknownObjectException @@ -142,8 +143,16 @@ def run_cruft(cwd: Path) -> CompletedProcess: return run(args, check=True, cwd=cwd) +# GitHub says that up to 5 minutes of wait are OK, +# So we error our once we wait longer, i.e. when 2ⁿ = 5 min × 60 sec/min +n_retries = math.ceil(math.log(5 * 60) / math.log(2)) # = ⌈~8.22⌉ = 9 +# 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: - clone = Repo.clone_from(con.auth(repo.clone_url), path) + clone = retry_with_backoff( + 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) branch.checkout() @@ -166,12 +175,6 @@ def cruft_update(con: GitHubConnection, repo: GHRepo, path: Path, pr: PR) -> boo return True -# GitHub says that up to 5 minutes of wait are OK, -# So we error our once we wait longer, i.e. when 2ⁿ = 5 min × 60 sec/min -n_retries = math.ceil(math.log(5 * 60) / math.log(2)) # = ⌈~8.22⌉ = 9 -# Due to exponential backoff, we’ll maximally wait 2⁹ sec, or 8.5 min - - 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 From 891571c4f6f9ca4d98a89404284bbebc121ede38 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 11:19:13 +0200 Subject: [PATCH 11/36] Update template sync - better logging - check out release tag instead of main - put repo owner in PR branch name to deal with forks --- .github/workflows/cruft-prs.yml | 4 ++ scripts/pyproject.toml | 1 + .../src/scverse_template_scripts/cruft_prs.py | 43 ++++++++++++++----- .../.pre-commit-config.yaml | 3 ++ 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/.github/workflows/cruft-prs.yml b/.github/workflows/cruft-prs.yml index e3a77c24..762df2aa 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: logs diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index 2aa44e15..4add8575 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -23,6 +23,7 @@ dependencies = [ "GitPython", "PyGitHub", "PyYAML", + "pre-commit" ] [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..21a1bcd4 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -29,6 +29,8 @@ from .backoff import retry_with_backoff log = getLogger(__name__) +log_dir = Path("./log") +log_dir.mkdir() PR_BODY_TEMPLATE = """\ `cookiecutter-scverse` released [{release.tag_name}]({release.html_url}). @@ -86,6 +88,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 +99,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: @@ -138,9 +141,18 @@ 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, git_tag: str, log_name: str) -> CompletedProcess: + args = [ + sys.executable, + "-m", + "cruft", + "update", + f"--checkout={git_tag}", + "--skip-apply-ask", + "--project-dir=.", + ] + with open(log_dir / f"{log_name}.txt", "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 +161,16 @@ 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(con: GitHubConnection, release: GHRelease, repo: GHRepo, path: Path, pr: PR) -> 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) branch.checkout() - run_cruft(path) + run_cruft(path, release.tag_name, pr.branch) if not clone.is_dirty(): return False @@ -179,18 +193,23 @@ 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 = repo.create_fork() - return retry_with_backoff(lambda: con.gh.get_repo(fork.id), retries=n_retries, exc_cls=UnknownObjectException) + 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) + 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) with TemporaryDirectory() as td: - updated = cruft_update(con, repo, Path(td), pr) + updated = cruft_update(con, release, repo, Path(td), pr) if updated: if old_pr := next((p for p in origin.get_pulls("open") if pr.matches(p)), None): old_pr.edit(state="closed") @@ -211,7 +230,9 @@ def main(tag_name: str) -> None: release = get_template_release(con.gh, tag_name) repo_urls = get_repo_urls(con.gh) for repo_url in repo_urls: - make_pr(con, release, repo_url) + # TODO just use single-repo we control for testing + if repo_url.endswith("infercnvpy"): + make_pr(con, release, repo_url) def cli() -> None: diff --git a/{{cookiecutter.project_name}}/.pre-commit-config.yaml b/{{cookiecutter.project_name}}/.pre-commit-config.yaml index 87dcb883..0a95f977 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 From 9527ee9d714d1cfa765e0364d62ed407f87e1d32 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 11:45:24 +0200 Subject: [PATCH 12/36] fix tests --- scripts/tests/test_cruft.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index 0615eb23..978e7da9 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -44,12 +44,15 @@ def repo(git_repo: GitRepo) -> GHRepo: @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")) + 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) assert changed # TODO: add test for short circuit main_branch = git_repo.api.active_branch From 8875ce742c3bd322a7ba1ddee75cb42a36dd1684 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 12:03:42 +0200 Subject: [PATCH 13/36] fix tests --- scripts/src/scverse_template_scripts/cruft_prs.py | 8 ++++---- scripts/tests/test_cruft.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 21a1bcd4..31a00f19 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -30,7 +30,7 @@ log = getLogger(__name__) log_dir = Path("./log") -log_dir.mkdir() +log_dir.mkdir(exist_ok=True) PR_BODY_TEMPLATE = """\ `cookiecutter-scverse` released [{release.tag_name}]({release.html_url}). @@ -161,7 +161,7 @@ def run_cruft(cwd: Path, git_tag: str, log_name: str) -> CompletedProcess: # Due to exponential backoff, we’ll maximally wait 2⁹ sec, or 8.5 min -def cruft_update(con: GitHubConnection, release: GHRelease, repo: GHRepo, path: Path, pr: PR) -> bool: +def cruft_update(con: GitHubConnection, tag_name: str, repo: GHRepo, path: Path, pr: PR) -> bool: clone = retry_with_backoff( lambda: Repo.clone_from(con.auth(repo.clone_url), path), retries=n_retries, @@ -170,7 +170,7 @@ def cruft_update(con: GitHubConnection, release: GHRelease, repo: GHRepo, path: branch = clone.create_head(pr.branch, clone.active_branch) branch.checkout() - run_cruft(path, release.tag_name, pr.branch) + run_cruft(path, tag_name, pr.branch) if not clone.is_dirty(): return False @@ -209,7 +209,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) repo = get_fork(con, origin) with TemporaryDirectory() as td: - updated = cruft_update(con, release, repo, Path(td), pr) + updated = cruft_update(con, release.tag_name, repo, Path(td), pr) if updated: if old_pr := next((p for p in origin.get_pulls("open") if pr.matches(p)), None): old_pr.edit(state="closed") diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index 978e7da9..c3be029e 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -51,9 +51,9 @@ def test_cruft_update(con, repo, tmp_path, pr, git_repo: GitRepo, monkeypatch: p 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"), + lambda p, _, __: (p / "b").write_text("b modified"), ) - changed = cruft_update(con, repo, tmp_path, pr) + changed = cruft_update(con, "main", repo, tmp_path, pr) 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" From b89738eaf57244922cfcee4d4400f1a65cf405c7 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 12:16:04 +0200 Subject: [PATCH 14/36] Make name of forked repos include the original user --- scripts/src/scverse_template_scripts/cruft_prs.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 31a00f19..8e5dc41a 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -189,10 +189,13 @@ def cruft_update(con: GitHubConnection, tag_name: str, repo: GHRepo, path: Path, return True -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): +def get_fork(con: GitHubConnection, repo: GHRepo, name: str) -> GHRepo: + if fork := next( + (f for f in repo.get_forks() if f.owner.id == con.user.id and f.name == name), + None, + ): return fork - fork = repo.create_fork() + fork = repo.create_fork(name=name) return retry_with_backoff( lambda: con.gh.get_repo(fork.id), retries=n_retries, @@ -207,7 +210,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: # create fork, populate branch, do PR from it origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) - repo = get_fork(con, origin) + repo = get_fork(con, origin, pr.repo_id) with TemporaryDirectory() as td: updated = cruft_update(con, release.tag_name, repo, Path(td), pr) if updated: From a41458d00315208345be13e45b0b1310b4f985ce Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 12:24:02 +0200 Subject: [PATCH 15/36] debug log --- scripts/src/scverse_template_scripts/cruft_prs.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 8e5dc41a..ade2aa52 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -194,7 +194,10 @@ def get_fork(con: GitHubConnection, repo: GHRepo, name: str) -> GHRepo: (f for f in repo.get_forks() if f.owner.id == con.user.id and f.name == name), None, ): + log.info(f"Using existing fork for {repo} with name {fork.name}.") + log.debug(fork) return fork + log.info(f"Creating fork for {repo} with name {name}") fork = repo.create_fork(name=name) return retry_with_backoff( lambda: con.gh.get_repo(fork.id), @@ -206,11 +209,11 @@ def get_fork(con: GitHubConnection, repo: GHRepo, name: str) -> GHRepo: def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: 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}") + 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, pr.repo_id) + repo = get_fork(con, origin, repo_id) with TemporaryDirectory() as td: updated = cruft_update(con, release.tag_name, repo, Path(td), pr) if updated: @@ -234,7 +237,7 @@ def main(tag_name: str) -> None: repo_urls = get_repo_urls(con.gh) for repo_url in repo_urls: # TODO just use single-repo we control for testing - if repo_url.endswith("infercnvpy"): + if repo_url.endswith("icbi-lab/infercnvpy"): make_pr(con, release, repo_url) From cb46a7ce4b24802d0e1f49ffb56793d2c18299e6 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 12:24:57 +0200 Subject: [PATCH 16/36] fix artifact upload --- .github/workflows/cruft-prs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cruft-prs.yml b/.github/workflows/cruft-prs.yml index 762df2aa..a2f9ab98 100644 --- a/.github/workflows/cruft-prs.yml +++ b/.github/workflows/cruft-prs.yml @@ -33,4 +33,4 @@ jobs: - uses: actions/upload-artifact@v3 with: name: cruft-logs - path: logs + path: log/ From 809e9aaf6069787bada964293840181122cef316 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 14:10:24 +0200 Subject: [PATCH 17/36] Fetch main branch from upstream --- .../src/scverse_template_scripts/cruft_prs.py | 31 ++++++++----------- scripts/tests/test_cruft.py | 5 +-- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index ade2aa52..9e605737 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -161,13 +161,17 @@ def run_cruft(cwd: Path, git_tag: str, log_name: str) -> CompletedProcess: # Due to exponential backoff, we’ll maximally wait 2⁹ sec, or 8.5 min -def cruft_update(con: GitHubConnection, tag_name: str, repo: GHRepo, path: Path, pr: PR) -> bool: +def cruft_update( # noqa: PLR0913 + con: GitHubConnection, tag_name: str, repo: GHRepo, origin: GHRepo, path: Path, pr: PR +) -> bool: clone = retry_with_backoff( 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.git_url) + upstream.fetch() + branch = clone.create_head(pr.branch, f"{pr.repo_id}/{origin.default_branch}") branch.checkout() run_cruft(path, tag_name, pr.branch) @@ -189,21 +193,12 @@ def cruft_update(con: GitHubConnection, tag_name: str, repo: GHRepo, path: Path, return True -def get_fork(con: GitHubConnection, repo: GHRepo, name: str) -> GHRepo: - if fork := next( - (f for f in repo.get_forks() if f.owner.id == con.user.id and f.name == name), - None, - ): - log.info(f"Using existing fork for {repo} with name {fork.name}.") - log.debug(fork) - return fork - log.info(f"Creating fork for {repo} with name {name}") - fork = repo.create_fork(name=name) - return retry_with_backoff( - lambda: con.gh.get_repo(fork.id), - retries=n_retries, - exc_cls=UnknownObjectException, - ) +def get_fork(con: GitHubConnection, repo: GHRepo) -> GHRepo: + """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: @@ -215,7 +210,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) repo = get_fork(con, origin, repo_id) with TemporaryDirectory() as td: - updated = cruft_update(con, release.tag_name, repo, Path(td), pr) + updated = cruft_update(con, release.tag_name, repo, origin, Path(td), pr) if updated: if old_pr := next((p for p in origin.get_pulls("open") if pr.matches(p)), None): old_pr.edit(state="closed") diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index c3be029e..64dc4d75 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -15,6 +15,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 @@ -39,7 +40,7 @@ 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 @@ -53,7 +54,7 @@ def test_cruft_update(con, repo, tmp_path, pr, git_repo: GitRepo, monkeypatch: p "scverse_template_scripts.cruft_prs.run_cruft", lambda p, _, __: (p / "b").write_text("b modified"), ) - changed = cruft_update(con, "main", repo, tmp_path, pr) + changed = cruft_update(con, "main", repo, repo, tmp_path, pr) 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" From a400e6f8143acaff1f740181b26085fdf9329a4b Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 14:13:46 +0200 Subject: [PATCH 18/36] Fix get fork --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 9e605737..eaeac6cc 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -208,7 +208,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: # create fork, populate branch, do PR from it origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) - repo = get_fork(con, origin, repo_id) + repo = get_fork(con, origin) with TemporaryDirectory() as td: updated = cruft_update(con, release.tag_name, repo, origin, Path(td), pr) if updated: From e55346f032bae3a161ca5401a5ccc8f77009a480 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Fri, 4 Aug 2023 14:47:26 +0200 Subject: [PATCH 19/36] fix clone url --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index eaeac6cc..f8c5351b 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -169,7 +169,7 @@ def cruft_update( # noqa: PLR0913 retries=n_retries, exc_cls=GitCommandError, ) - upstream = clone.create_remote(name=pr.repo_id, url=origin.git_url) + 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() From c0c2c27da3e219f5df57971189de16aede58597f Mon Sep 17 00:00:00 2001 From: Philipp A Date: Mon, 7 Aug 2023 09:40:29 +0200 Subject: [PATCH 20/36] fmt --- scripts/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index 4add8575..20bca2c5 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -23,7 +23,7 @@ dependencies = [ "GitPython", "PyGitHub", "PyYAML", - "pre-commit" + "pre-commit", ] [project.optional-dependencies] From 3aeb28d48f4315606a89328fb5f15ac5205abdcd Mon Sep 17 00:00:00 2001 From: Philipp A Date: Mon, 7 Aug 2023 09:42:36 +0200 Subject: [PATCH 21/36] comment on why pre-commit is a runtime dep --- scripts/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index 20bca2c5..348174da 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -23,7 +23,7 @@ dependencies = [ "GitPython", "PyGitHub", "PyYAML", - "pre-commit", + "pre-commit", # is ran by cruft ] [project.optional-dependencies] From 5f2e04dd9276f961ba30d5370dd59ed735c2fdb8 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Mon, 7 Aug 2023 09:53:20 +0200 Subject: [PATCH 22/36] less positional --- .../src/scverse_template_scripts/cruft_prs.py | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index f8c5351b..9d6efae6 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -29,8 +29,6 @@ from .backoff import retry_with_backoff log = getLogger(__name__) -log_dir = Path("./log") -log_dir.mkdir(exist_ok=True) PR_BODY_TEMPLATE = """\ `cookiecutter-scverse` released [{release.tag_name}]({release.html_url}). @@ -141,17 +139,21 @@ def get_repo_urls(gh: Github) -> Generator[str]: yield repo["url"] -def run_cruft(cwd: Path, git_tag: str, log_name: str) -> CompletedProcess: +def run_cruft(cwd: Path, *, tag_name: str, log_name: str) -> CompletedProcess: args = [ sys.executable, "-m", "cruft", "update", - f"--checkout={git_tag}", + f"--checkout={tag_name}", "--skip-apply-ask", "--project-dir=.", ] - with open(log_dir / f"{log_name}.txt", "w") as log_file: + + 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) @@ -162,7 +164,13 @@ def run_cruft(cwd: Path, git_tag: str, log_name: str) -> CompletedProcess: def cruft_update( # noqa: PLR0913 - con: GitHubConnection, tag_name: str, repo: GHRepo, origin: GHRepo, path: Path, pr: PR + 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), @@ -174,7 +182,7 @@ def cruft_update( # noqa: PLR0913 branch = clone.create_head(pr.branch, f"{pr.repo_id}/{origin.default_branch}") branch.checkout() - run_cruft(path, tag_name, pr.branch) + run_cruft(path, tag_name=tag_name, log_name=pr.branch) if not clone.is_dirty(): return False @@ -210,7 +218,14 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) repo = get_fork(con, origin) with TemporaryDirectory() as td: - updated = cruft_update(con, release.tag_name, repo, origin, 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): old_pr.edit(state="closed") From e742888fa8447df6f26a9c727d11f0d115a479bb Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 7 Aug 2023 07:53:28 +0000 Subject: [PATCH 23/36] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 9d6efae6..8d7d1b98 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -149,7 +149,7 @@ def run_cruft(cwd: Path, *, tag_name: str, log_name: str) -> CompletedProcess: "--skip-apply-ask", "--project-dir=.", ] - + log_path = Path(f"./log/{log_name}.txt") log_path.parent.mkdir(exist_ok=True) From 98d2f88312a27486412ecfae60a15566f3f4585f Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 19:55:23 +0200 Subject: [PATCH 24/36] Wrap entire command in try/catch --- scripts/src/scverse_template_scripts/cruft_prs.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index f8c5351b..092c8bb7 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -230,10 +230,17 @@ 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: - # TODO just use single-repo we control for testing - if repo_url.endswith("icbi-lab/infercnvpy"): - make_pr(con, release, repo_url) + try: + # TODO just use single-repo we control for testing + if repo_url.endswith("icbi-lab/infercnvpy"): + make_pr(con, release, repo_url) + except Exception as e: + failed += 1 + log.error(f"Error updating {repo_url}.", e) + + sys.exit(failed > 0) def cli() -> None: From fccd4c379ee743a9f764ba430b54961a14e5dd84 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 20:08:38 +0200 Subject: [PATCH 25/36] Skip PR if already exists for current version. This allows to re-run the entire action on failures without creating noise in the repos where the update was successful. --- scripts/src/scverse_template_scripts/cruft_prs.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index e4f1ac5e..1feb998c 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -110,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 @@ -217,6 +222,11 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: # 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: #{pr.number} with branch name `{pr.head.ref}`. Skipping.") + return + with TemporaryDirectory() as td: updated = cruft_update( con, @@ -227,7 +237,8 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: 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 `{pr.head.ref}`.") old_pr.edit(state="closed") origin.create_pull(pr.title, pr.body, origin.default_branch, pr.namespaced_head) From 60b1c0c5200f6134f00f400a5a5a1acc35dc3c8c Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 20:12:15 +0200 Subject: [PATCH 26/36] Fix log messages --- scripts/src/scverse_template_scripts/cruft_prs.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 1feb998c..62755e47 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -224,7 +224,9 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: 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: #{pr.number} with branch name `{pr.head.ref}`. Skipping.") + 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: @@ -238,7 +240,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: ) if updated: 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 `{pr.head.ref}`.") + 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) From 419fb3119483dfd2a8f2e501f50b9124c6ae573e Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 20:24:22 +0200 Subject: [PATCH 27/36] Fix tests --- scripts/src/scverse_template_scripts/cruft_prs.py | 3 ++- scripts/tests/test_cruft.py | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 62755e47..1ce48884 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -242,7 +242,8 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> 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) + log.info(f"Created PR #{new_pr.number} with branch name `{new_pr.head.ref}`.") def setup() -> None: diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index 64dc4d75..f6deed25 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 import pytest @@ -50,11 +51,12 @@ def pr(con) -> PR: 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, "main", repo, repo, tmp_path, pr) + + def _mock_run_cruft(cwd: Path, *, tag_name, log_name): + (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" From cd609e2089830b5ffc8cd4db42f6e981c6ccf9ce Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 20:31:42 +0200 Subject: [PATCH 28/36] Try fix test --- scripts/tests/test_cruft.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/test_cruft.py b/scripts/tests/test_cruft.py index f6deed25..3710a090 100644 --- a/scripts/tests/test_cruft.py +++ b/scripts/tests/test_cruft.py @@ -53,7 +53,7 @@ def test_cruft_update(con, repo, tmp_path, pr, git_repo: GitRepo, monkeypatch: p old_active_branch_name = git_repo.api.active_branch.name def _mock_run_cruft(cwd: Path, *, tag_name, log_name): - (cwd / "b").write_text("b modified") + 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) From 121bcbf58a44863613069dff10ff1f32d8a110df Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 20:38:27 +0200 Subject: [PATCH 29/36] Arming bot: Apply to all repos --- scripts/src/scverse_template_scripts/cruft_prs.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 1ce48884..764c0b9b 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -262,9 +262,7 @@ def main(tag_name: str) -> None: failed = 0 for repo_url in repo_urls: try: - # TODO just use single-repo we control for testing - if repo_url.endswith("icbi-lab/infercnvpy"): - make_pr(con, release, repo_url) + make_pr(con, release, repo_url) except Exception as e: failed += 1 log.error(f"Error updating {repo_url}.", e) From 4fcf46675ea727221ec35a30e2d5c22859a1ca5d Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 20:51:33 +0200 Subject: [PATCH 30/36] Fix global exception handler --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 764c0b9b..144cd3c6 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -265,7 +265,7 @@ def main(tag_name: str) -> None: make_pr(con, release, repo_url) except Exception as e: failed += 1 - log.error(f"Error updating {repo_url}.", e) + log.exception(f"Error updating {repo_url}.", e) sys.exit(failed > 0) From 23a9eaaf101dc9b6d31976f32f7345e1b209b071 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 9 Aug 2023 21:33:45 +0200 Subject: [PATCH 31/36] Fix global exception handler --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 144cd3c6..2fed4246 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -265,7 +265,7 @@ def main(tag_name: str) -> None: make_pr(con, release, repo_url) except Exception as e: failed += 1 - log.exception(f"Error updating {repo_url}.", e) + log.exception(f"Error updating {repo_url}: %s", e) sys.exit(failed > 0) From 8be748d6071feff3d927811a0620ae6ee5b1fa04 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Thu, 10 Aug 2023 10:21:19 +0200 Subject: [PATCH 32/36] Simplify code --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 2fed4246..ee3b4a11 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -223,7 +223,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: 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): + if any(True for p in origin.get_pulls("open") if pr.matches_current_version(p)): log.info( f"PR for current version already exists: #{old_pr.number} with branch name `{old_pr.head.ref}`. Skipping." ) From ff882921cfbdfcf7aa8c4a95295dca4897fbe662 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 16 Aug 2023 08:28:42 +0200 Subject: [PATCH 33/36] Allow maintainer to modify the PR --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index ee3b4a11..85b045d5 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -242,7 +242,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> 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") - new_pr = 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}`.") From abf05fce92edf95e57db655ee670ad04a9aab498 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Aug 2023 06:28:53 +0000 Subject: [PATCH 34/36] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- scripts/src/scverse_template_scripts/cruft_prs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 85b045d5..08d1f4af 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -242,7 +242,9 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> 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") - new_pr = origin.create_pull(pr.title, pr.body, origin.default_branch, pr.namespaced_head, maintainer_can_modify=True) + 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}`.") From 605f95714130c960bce12eab98a4e023e8932249 Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Wed, 16 Aug 2023 08:31:46 +0200 Subject: [PATCH 35/36] Fix logging for existing PRs --- scripts/src/scverse_template_scripts/cruft_prs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/scverse_template_scripts/cruft_prs.py b/scripts/src/scverse_template_scripts/cruft_prs.py index 08d1f4af..91f326c5 100644 --- a/scripts/src/scverse_template_scripts/cruft_prs.py +++ b/scripts/src/scverse_template_scripts/cruft_prs.py @@ -223,7 +223,7 @@ def make_pr(con: GitHubConnection, release: GHRelease, repo_url: str) -> None: origin = con.gh.get_repo(repo_url.removeprefix("https://github.com/")) repo = get_fork(con, origin) - if any(True for p in origin.get_pulls("open") if pr.matches_current_version(p)): + 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." ) From 7539d2af5cabb3805c5747c496032d4dbc056bfc Mon Sep 17 00:00:00 2001 From: Gregor Sturm Date: Tue, 28 Nov 2023 14:09:18 +0100 Subject: [PATCH 36/36] Set git default branch in tests --- .github/workflows/test.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1e034ac1..6e40f48c 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -79,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