From c7a504fef3e261aa3c5f22acb96604c86ce96d81 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Date: Sat, 20 Jan 2024 18:34:03 +0200 Subject: [PATCH] Lint on GitHub Actions via pre-commit (#93) Co-authored-by: Ezio Melotti Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Hugo van Kemenade --- .flake8 | 4 ++ .github/workflows/lint.yml | 30 ++++++++++ .github/workflows/lint_python.yml | 26 --------- .pre-commit-config.yaml | 88 +++++++++++++++++++++++++++++ cherry_picker/__init__.py | 2 + cherry_picker/__main__.py | 2 + cherry_picker/cherry_picker.py | 46 ++++++++++----- cherry_picker/test_cherry_picker.py | 71 +++++++++++++++++------ pyproject.toml | 3 + readme.rst | 2 +- 10 files changed, 214 insertions(+), 60 deletions(-) create mode 100644 .flake8 create mode 100644 .github/workflows/lint.yml delete mode 100644 .github/workflows/lint_python.yml create mode 100644 .pre-commit-config.yaml diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..3ed1b92 --- /dev/null +++ b/.flake8 @@ -0,0 +1,4 @@ +[flake8] +extend-ignore = C408,E203,F841,W503 +max-complexity = 10 +max-line-length = 88 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..a2bc9cb --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,30 @@ +name: Lint + +on: [push, pull_request, workflow_dispatch] + +env: + FORCE_COLOR: 1 + +permissions: + contents: read + +jobs: + lint: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.x" + cache: pip + cache-dependency-path: .github/workflows/lint.yml + - uses: pre-commit/action@v3.0.0 + - name: Install dependencies + run: | + python -m pip install --upgrade pip wheel + # TODO: remove setuptools installation when safety==2.4.0 is released + python -m pip install --upgrade safety setuptools + python -m pip install --editable . + # Ignore CVE-2023-5752, we're not using that pip or feature + - run: safety check --ignore 62044 diff --git a/.github/workflows/lint_python.yml b/.github/workflows/lint_python.yml deleted file mode 100644 index afa4a8a..0000000 --- a/.github/workflows/lint_python.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: lint_python -on: [pull_request, push, workflow_dispatch] -jobs: - lint_python: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - cache: pip - cache-dependency-path: .github/workflows/lint_python.yml - - run: pip install --upgrade pip wheel - # TODO: remove setuptools installation when safety==2.4.0 is released - - run: pip install --upgrade bandit black codespell flake8 flake8-bugbear - flake8-comprehensions isort mypy pyupgrade safety setuptools - - run: bandit --recursive --skip B101,B404,B603 . - - run: black --diff . - - run: codespell --ignore-words-list="commitish" - - run: flake8 . --count --ignore=C408,E203,F841,W503 --max-complexity=10 - --max-line-length=143 --show-source --statistics - - run: isort --check-only --profile black . - - run: pip install --editable . - - run: mypy --ignore-missing-imports --install-types --non-interactive . - - run: shopt -s globstar && pyupgrade --py38-plus **/*.py || true - # # Ignore CVE-2023-5752, we're not using that pip or feature - - run: safety check --ignore 62044 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..c5fbb02 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,88 @@ +repos: + - repo: https://github.com/asottile/pyupgrade + rev: v3.15.0 + hooks: + - id: pyupgrade + args: [--py38-plus] + + - repo: https://github.com/psf/black-pre-commit-mirror + rev: 23.12.1 + hooks: + - id: black + + - repo: https://github.com/PyCQA/isort + rev: 5.13.2 + hooks: + - id: isort + args: [--add-import=from __future__ import annotations] + + - repo: https://github.com/PyCQA/bandit + rev: 1.7.6 + hooks: + - id: bandit + args: ["--skip=B101,B404,B603"] + + - repo: https://github.com/PyCQA/flake8 + rev: 7.0.0 + hooks: + - id: flake8 + additional_dependencies: + [ + flake8-2020, + flake8-bugbear, + flake8-comprehensions, + flake8-implicit-str-concat, + flake8-logging, + ] + + - repo: https://github.com/pre-commit/pygrep-hooks + rev: v1.10.0 + hooks: + - id: python-check-blanket-noqa + + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: check-case-conflict + - id: check-executables-have-shebangs + - id: check-merge-conflict + - id: check-json + - id: check-toml + - id: check-yaml + - id: debug-statements + - id: end-of-file-fixer + - id: trailing-whitespace + + - repo: https://github.com/pre-commit/mirrors-mypy + rev: v1.8.0 + hooks: + - id: mypy + args: + [ + --ignore-missing-imports, + --install-types, + --non-interactive, + --pretty, + --show-error-codes, + ., + ] + pass_filenames: false + + - repo: https://github.com/tox-dev/pyproject-fmt + rev: 1.6.0 + hooks: + - id: pyproject-fmt + + - repo: https://github.com/abravalheri/validate-pyproject + rev: v0.15 + hooks: + - id: validate-pyproject + + - repo: https://github.com/codespell-project/codespell + rev: v2.2.6 + hooks: + - id: codespell + args: [--ignore-words-list=commitish] + +ci: + autoupdate_schedule: quarterly diff --git a/cherry_picker/__init__.py b/cherry_picker/__init__.py index b654ebc..9dfa236 100644 --- a/cherry_picker/__init__.py +++ b/cherry_picker/__init__.py @@ -1,4 +1,6 @@ """Backport CPython changes from main to maintenance branches.""" +from __future__ import annotations + import importlib.metadata __version__ = importlib.metadata.version(__name__) diff --git a/cherry_picker/__main__.py b/cherry_picker/__main__.py index cc02b31..b5ff54f 100644 --- a/cherry_picker/__main__.py +++ b/cherry_picker/__main__.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from .cherry_picker import cherry_pick_cli if __name__ == "__main__": diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index c3c6e60..66bde15 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -1,5 +1,7 @@ #!/usr/bin/env python3 +from __future__ import annotations + import collections import enum import os @@ -91,7 +93,6 @@ class InvalidRepoException(Exception): class CherryPicker: - ALLOWED_STATES = WORKFLOW_STATES.BACKPORT_PAUSED, WORKFLOW_STATES.UNSET """The list of states expected at the start of the app.""" @@ -151,7 +152,7 @@ def set_paused_state(self): set_state(WORKFLOW_STATES.BACKPORT_PAUSED) def remember_previous_branch(self): - """Save the current branch into Git config to be able to get back to it later.""" + """Save the current branch into Git config, to be used later.""" current_branch = get_current_branch() save_cfg_vals_to_git_cfg(previous_branch=current_branch) @@ -160,7 +161,8 @@ def upstream(self): """Get the remote name to use for upstream branches Uses the remote passed to `--upstream-remote`. - If this flag wasn't passed, it uses "upstream" if it exists or "origin" otherwise. + If this flag wasn't passed, it uses "upstream" if it exists or "origin" + otherwise. """ # the cached calculated value of the property if self._upstream is not None: @@ -203,7 +205,10 @@ def get_cherry_pick_branch(self, maint_branch): return f"backport-{self.commit_sha1[:7]}-{maint_branch}" def get_pr_url(self, base_branch, head_branch): - return f"https://github.com/{self.config['team']}/{self.config['repo']}/compare/{base_branch}...{self.username}:{head_branch}?expand=1" + return ( + f"https://github.com/{self.config['team']}/{self.config['repo']}" + f"/compare/{base_branch}...{self.username}:{head_branch}?expand=1" + ) def fetch_upstream(self): """git fetch """ @@ -323,7 +328,9 @@ def get_updated_commit_message(self, cherry_pick_branch): commit_prefix = "" if self.prefix_commit: commit_prefix = f"[{get_base_branch(cherry_pick_branch)}] " - updated_commit_message = f"{commit_prefix}{self.get_commit_message(self.commit_sha1)}" + updated_commit_message = ( + f"{commit_prefix}{self.get_commit_message(self.commit_sha1)}" + ) # Add '(cherry picked from commit ...)' to the message # and add new Co-authored-by trailer if necessary. @@ -349,7 +356,9 @@ def get_updated_commit_message(self, cherry_pick_branch): # # This needs to be done because `git interpret-trailers` required us to add `:` # to `cherry_pick_information` when we don't actually want it. - before, after = output.strip().decode().rsplit(f"\n{cherry_pick_information}", 1) + before, after = ( + output.strip().decode().rsplit(f"\n{cherry_pick_information}", 1) + ) if not before.endswith("\n"): # ensure that we still have a newline between cherry pick information # and commit headline @@ -359,7 +368,7 @@ def get_updated_commit_message(self, cherry_pick_branch): return updated_commit_message def amend_commit_message(self, cherry_pick_branch): - """ prefix the commit message with (X.Y) """ + """Prefix the commit message with (X.Y)""" updated_commit_message = self.get_updated_commit_message(cherry_pick_branch) if self.dry_run: @@ -442,7 +451,7 @@ def create_gh_pr(self, base_branch, head_branch, *, commit_message, gh_auth): if response.status_code == requests.codes.created: response_data = response.json() click.echo(f"Backport PR created at {response_data['html_url']}") - self.pr_number = response_data['number'] + self.pr_number = response_data["number"] else: click.echo(response.status_code) click.echo(response.text) @@ -542,7 +551,9 @@ def abort_cherry_pick(self): state = self.get_state_and_verify() if state != WORKFLOW_STATES.BACKPORT_PAUSED: raise ValueError( - f"One can only abort a paused process. Current state: {state}. Expected state: {WORKFLOW_STATES.BACKPORT_PAUSED}" + f"One can only abort a paused process. " + f"Current state: {state}. " + f"Expected state: {WORKFLOW_STATES.BACKPORT_PAUSED}" ) try: @@ -575,7 +586,9 @@ def continue_cherry_pick(self): state = self.get_state_and_verify() if state != WORKFLOW_STATES.BACKPORT_PAUSED: raise ValueError( - f"One can only continue a paused process. Current state: {state}. Expected state: {WORKFLOW_STATES.BACKPORT_PAUSED}" + "One can only continue a paused process. " + f"Current state: {state}. " + f"Expected state: {WORKFLOW_STATES.BACKPORT_PAUSED}" ) cherry_pick_branch = get_current_branch() @@ -623,7 +636,8 @@ def continue_cherry_pick(self): else: click.echo( - f"Current branch ({cherry_pick_branch}) is not a backport branch. Will not continue. \U0001F61B" + f"Current branch ({cherry_pick_branch}) is not a backport branch. " + "Will not continue. \U0001F61B" ) set_state(WORKFLOW_STATES.CONTINUATION_FAILED) @@ -635,8 +649,8 @@ def check_repo(self): """ Check that the repository is for the project we're configured to operate on. - This function performs the check by making sure that the sha specified in the config - is present in the repository that we're operating on. + This function performs the check by making sure that the sha specified in the + config is present in the repository that we're operating on. """ try: validate_sha(self.config["check_sha"]) @@ -823,7 +837,8 @@ def get_base_branch(cherry_pick_branch): if prefix != "backport": raise ValueError( - 'branch name is not prefixed with "backport-". Is this a cherry_picker branch?' + 'branch name is not prefixed with "backport-". ' + "Is this a cherry_picker branch?" ) if not re.match("[0-9a-f]{7,40}", sha): @@ -851,7 +866,8 @@ def validate_sha(sha): subprocess.check_output(cmd, stderr=subprocess.STDOUT) except subprocess.SubprocessError: raise ValueError( - f"The sha listed in the branch name, {sha}, is not present in the repository" + f"The sha listed in the branch name, {sha}, " + "is not present in the repository" ) diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index d99ea81..bffb11d 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import os import pathlib import re @@ -131,7 +133,7 @@ def tmp_git_repo_dir(tmpdir, cd, git_init, git_commit, git_config): except subprocess.CalledProcessError: version = subprocess.run(("git", "--version"), capture_output=True) # the output looks like "git version 2.34.1" - v = version.stdout.decode("utf-8").removeprefix('git version ').split('.') + v = version.stdout.decode("utf-8").removeprefix("git version ").split(".") if (int(v[0]), int(v[1])) < (2, 28): warnings.warn( "You need git 2.28.0 or newer to run the full test suite.", @@ -264,7 +266,9 @@ def test_get_cherry_pick_branch(os_path_exists, config): ("python", "python"), ), ) -def test_upstream_name(remote_name, upstream_remote, config, tmp_git_repo_dir, git_remote): +def test_upstream_name( + remote_name, upstream_remote, config, tmp_git_repo_dir, git_remote +): git_remote("add", remote_name, "https://github.com/python/cpython.git") if remote_name != "origin": git_remote("add", "origin", "https://github.com/miss-islington/cpython.git") @@ -292,10 +296,14 @@ def test_upstream_name(remote_name, upstream_remote, config, tmp_git_repo_dir, g (None, "python", None), ), ) -def test_error_on_missing_remote(remote_to_add, remote_name, upstream_remote, config, tmp_git_repo_dir, git_remote): +def test_error_on_missing_remote( + remote_to_add, remote_name, upstream_remote, config, tmp_git_repo_dir, git_remote +): git_remote("add", "some-remote-name", "https://github.com/python/cpython.git") if remote_to_add is not None: - git_remote("add", remote_to_add, "https://github.com/miss-islington/cpython.git") + git_remote( + "add", remote_to_add, "https://github.com/miss-islington/cpython.git" + ) branches = ["3.6"] with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): @@ -381,7 +389,8 @@ def test_get_updated_commit_message_without_links_replacement(config): @mock.patch("subprocess.check_output") def test_is_cpython_repo(subprocess_check_output): - subprocess_check_output.return_value = """commit 7f777ed95a19224294949e1b4ce56bbffcb1fe9f + subprocess_check_output.return_value = """\ +commit 7f777ed95a19224294949e1b4ce56bbffcb1fe9f Author: Guido van Rossum Date: Thu Aug 9 14:25:15 1990 +0000 @@ -495,7 +504,8 @@ def test_load_config_no_head_sha(tmp_git_repo_dir, git_add, git_commit): def test_normalize_long_commit_message(): - commit_message = """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) + commit_message = """\ +[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) The `Show Source` was broken because of a change made in sphinx 1.5.1 In Sphinx 1.4.9, the sourcename was "index.txt". @@ -521,7 +531,8 @@ def test_normalize_long_commit_message(): def test_normalize_short_commit_message(): - commit_message = """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) + commit_message = """\ +[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) (cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) @@ -610,7 +621,9 @@ def test_normalize_short_commit_message(): ), ), ) -def test_get_updated_commit_message_with_trailers(commit_message, expected_commit_message): +def test_get_updated_commit_message_with_trailers( + commit_message, expected_commit_message +): cherry_pick_branch = "backport-22a594a-3.6" commit = "b9ff498793611d1c6a9b99df464812931a1e2d69" @@ -625,7 +638,9 @@ def test_get_updated_commit_message_with_trailers(commit_message, expected_commi "cherry_picker.cherry_picker.get_author_info_from_short_sha", return_value="PR Author ", ): - updated_commit_message = cherry_picker.get_updated_commit_message(cherry_pick_branch) + updated_commit_message = cherry_picker.get_updated_commit_message( + cherry_pick_branch + ) assert updated_commit_message == expected_commit_message @@ -764,7 +779,9 @@ def test_cleanup_branch(tmp_git_repo_dir, git_checkout): assert get_current_branch() == "main" -def test_cleanup_branch_checkout_previous_branch(tmp_git_repo_dir, git_checkout, git_worktree): +def test_cleanup_branch_checkout_previous_branch( + tmp_git_repo_dir, git_checkout, git_worktree +): assert get_state() == WORKFLOW_STATES.UNSET with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): @@ -790,7 +807,9 @@ def test_cleanup_branch_fail(tmp_git_repo_dir): assert get_state() == WORKFLOW_STATES.REMOVING_BACKPORT_BRANCH_FAILED -def test_cleanup_branch_checkout_fail(tmp_git_repo_dir, tmpdir, git_checkout, git_worktree): +def test_cleanup_branch_checkout_fail( + tmp_git_repo_dir, tmpdir, git_checkout, git_worktree +): assert get_state() == WORKFLOW_STATES.UNSET with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): @@ -845,7 +864,7 @@ class tested_state: set_state(tested_state) expected_msg_regexp = ( - fr"^Run state cherry-picker.state={tested_state.name} in Git config " + rf"^Run state cherry-picker.state={tested_state.name} in Git config " r"is not known." "\n" r"Perhaps it has been set by a newer " @@ -863,7 +882,7 @@ class tested_state: with mock.patch( "cherry_picker.cherry_picker.validate_sha", return_value=True ), pytest.raises(InvalidRepoException, match=expected_msg_regexp): - cherry_picker = CherryPicker("origin", "xxx", []) + CherryPicker("origin", "xxx", []) def test_push_to_remote_fail(tmp_git_repo_dir): @@ -1008,7 +1027,9 @@ def test_backport_cherry_pick_branch_already_exists( pr_remote, scm_revision, cherry_pick_target_branches ) - backport_branch_name = cherry_picker.get_cherry_pick_branch(cherry_pick_target_branches[0]) + backport_branch_name = cherry_picker.get_cherry_pick_branch( + cherry_pick_target_branches[0] + ) git_branch(backport_branch_name) with mock.patch.object(cherry_picker, "fetch_upstream"), pytest.raises( @@ -1055,7 +1076,15 @@ def test_backport_success( @pytest.mark.parametrize("already_committed", (True, False)) @pytest.mark.parametrize("push", (True, False)) def test_backport_pause_and_continue( - tmp_git_repo_dir, git_branch, git_add, git_commit, git_checkout, git_reset, git_remote, already_committed, push + tmp_git_repo_dir, + git_branch, + git_add, + git_commit, + git_checkout, + git_reset, + git_remote, + already_committed, + push, ): cherry_pick_target_branches = ("3.8",) pr_remote = "origin" @@ -1087,7 +1116,9 @@ def test_backport_pause_and_continue( if not already_committed: git_reset("HEAD~1") - assert len(get_commits_from_backport_branch(cherry_pick_target_branches[0])) == 0 + assert ( + len(get_commits_from_backport_branch(cherry_pick_target_branches[0])) == 0 + ) with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): cherry_picker = CherryPicker(pr_remote, "", [], push=push) @@ -1142,7 +1173,9 @@ def test_continue_cherry_pick_invalid_state(tmp_git_repo_dir): assert get_state() == WORKFLOW_STATES.UNSET - with pytest.raises(ValueError, match=re.compile(r"^One can only continue a paused process.")): + with pytest.raises( + ValueError, match=re.compile(r"^One can only continue a paused process.") + ): cherry_picker.continue_cherry_pick() assert get_state() == WORKFLOW_STATES.UNSET # success @@ -1168,7 +1201,9 @@ def test_abort_cherry_pick_invalid_state(tmp_git_repo_dir): assert get_state() == WORKFLOW_STATES.UNSET - with pytest.raises(ValueError, match=re.compile(r"^One can only abort a paused process.")): + with pytest.raises( + ValueError, match=re.compile(r"^One can only abort a paused process.") + ): cherry_picker.abort_cherry_pick() diff --git a/pyproject.toml b/pyproject.toml index 808cafa..0499ac8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,3 +48,6 @@ tag-pattern = '^cherry-picker-(?P[vV]?\d+(?:\.\d+){0,2}[^\+]*)(?:\+.*)? [tool.hatch.version.raw-options] local_scheme = "no-local-version" + +[tool.isort] +profile = "black" diff --git a/readme.rst b/readme.rst index 8815a90..d6c1322 100644 --- a/readme.rst +++ b/readme.rst @@ -18,7 +18,7 @@ of the maintenance branches (``3.6``, ``3.5``, ``2.7``). workflow as CPython. See the configuration file options below for more details. The maintenance branch names should contain some sort of version number (X.Y). -For example: ``3.6``, ``3.5``, ``2.7``, ``stable-2.6``, ``2.5-lts``, are all +For example: ``3.6``, ``3.5``, ``2.7``, ``stable-2.6``, ``2.5-lts``, are all supported branch names. It will prefix the commit message with the branch, e.g. ``[3.6]``, and then