Reland: [Workflow] Add new code format helper.

[tru]

I landed this format helper, but unfortunately, it didn't work because of permissions, it could not add comments on a fork's PR. @cor3ntin informed me there are fixes for this that you had worked on @tstellar - but I didn't have time to read up on it too much. Can you explain what changes are needed to get the action to be able to write comments on fork's PR?

[llvmbot]

@llvm/pr-subscribers-github-workflow

Changes

I landed this format helper, but unfortunately, it didn't work because of permissions, it could not add comments on a fork's PR. @cor3ntin informed me there are fixes for this that you had worked on @tstellar - but I didn't have time to read up on it too much. Can you explain what changes are needed to get the action to be able to write comments on fork's PR?

---

Full diff: https://github.com/llvm/llvm-project/pull/66869.diff

5 Files Affected:

• (added) .github/workflows/pr-code-format.yml (+54)
• (removed) .github/workflows/pr-python-format.yml (-39)
• (added) llvm/utils/git/code-format-helper.py (+233)
• (added) llvm/utils/git/requirements_formatting.txt (+52)
• (added) llvm/utils/git/requirements_formatting.txt.in (+3)

diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml new file mode 100644 index 000000000000000..102e1a263b15a88 --- /dev/null +++ b/.github/workflows/pr-code-format.yml @@ -0,0 +1,54 @@ +name: "Check code formatting" +on: pull_request +permissions: + pull-requests: write + +jobs: + code_formatter: + runs-on: ubuntu-latest + steps: + - name: Fetch LLVM sources + uses: actions/checkout@v4 + with: + persist-credentials: false + fetch-depth: 2 + + - name: Get changed files + id: changed-files + uses: tj-actions/changed-files@v39 + with: + separator: "," + + - name: "Listed files" + run: | + echo "Formatting files:" + echo "${{ steps.changed-files.outputs.all_changed_files }}" + + - name: Install clang-format + uses: aminya/setup-cpp@v1 + with: + clangformat: 16.0.6 + + - name: Setup Python env + uses: actions/setup-python@v4 + with: + python-version: '3.11' + cache: 'pip' + cache-dependency-path: 'llvm/utils/git/requirements_formatting.txt' + + - name: Install python dependencies + run: pip install -r llvm/utils/git/requirements_formatting.txt + + - name: Run code formatter + env: + GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }} + START_REV: ${{ github.event.pull_request.base.sha }} + END_REV: ${{ github.event.pull_request.head.sha }} + CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }} + run: | + python llvm/utils/git/code-format-helper.py \ + --token ${{ secrets.GITHUB_TOKEN }} \ + --issue-number $GITHUB_PR_NUMBER \ + --start-rev $START_REV \ + --end-rev $END_REV \ + --changed-files "$CHANGED_FILES" diff --git a/.github/workflows/pr-python-format.yml b/.github/workflows/pr-python-format.yml deleted file mode 100644 index c6122958826545c..000000000000000 --- a/.github/workflows/pr-python-format.yml +++ /dev/null @@ -1,39 +0,0 @@ -name: "Check Python Formatting" -on: - pull_request: - # run on .py - paths: - - '**.py' - -jobs: - python_formatting: - runs-on: ubuntu-latest - steps: - - name: Fetch LLVM sources - uses: actions/checkout@v4 - with: - persist-credentials: false - fetch-depth: 2 - - - name: Get changed files - id: changed-files - uses: tj-actions/changed-files@v39 - with: - files: '**/*.py' - - - name: "Listed files" - run: | - echo "Formatting files:" - echo "${{ steps.changed-files.outputs.all_changed_files }}" - - - name: Setup Python env - uses: actions/setup-python@v4 - with: - python-version: '3.11' - - - name: Python Formatting - uses: akaihola/darker@1.7.2 - with: - options: "--check --diff --color" - version: "~=1.7.2" - src: "${{ steps.changed-files.outputs.all_changed_files }}" diff --git a/llvm/utils/git/code-format-helper.py b/llvm/utils/git/code-format-helper.py new file mode 100644 index 000000000000000..8d3c30b309d015d --- /dev/null +++ b/llvm/utils/git/code-format-helper.py @@ -0,0 +1,233 @@ +#!/usr/bin/env python3 +# +# ====- code-format-helper, runs code formatters from the ci --*- python -*--==# +# +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +# +# ==-------------------------------------------------------------------------==# + +import argparse +import os +import subprocess +import sys +from functools import cached_property + +import github +from github import IssueComment, PullRequest + + +class FormatHelper: + COMMENT_TAG = "<!--LLVM CODE FORMAT COMMENT: {fmt}-->" + name = "unknown" + + @property + def comment_tag(self) -> str: + return self.COMMENT_TAG.replace("fmt", self.name) + + def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None: + pass + + def pr_comment_text(self, diff: str) -> str: + return f""" +{self.comment_tag} + +:warning: {self.friendly_name}, {self.name} found issues in your code. :warning: + +<details> +<summary> +You can test this locally with the following command: +</summary> + +``````````bash +{self.instructions} +`````````` + +</details> + +<details> +<summary> +View the diff from {self.name} here. +</summary> + +``````````diff +{diff} +`````````` + +</details> +""" + + def find_comment( + self, pr: PullRequest.PullRequest + ) -> IssueComment.IssueComment | None: + for comment in pr.as_issue().get_comments(): + if self.comment_tag in comment.body: + return comment + return None + + def update_pr(self, diff: str, args: argparse.Namespace): + repo = github.Github(args.token).get_repo(args.repo) + pr = repo.get_issue(args.issue_number).as_pull_request() + + existing_comment = self.find_comment(pr) + pr_text = self.pr_comment_text(diff) + + if existing_comment: + existing_comment.edit(pr_text) + else: + pr.as_issue().create_comment(pr_text) + + def update_pr_success(self, args: argparse.Namespace): + repo = github.Github(args.token).get_repo(args.repo) + pr = repo.get_issue(args.issue_number).as_pull_request() + + existing_comment = self.find_comment(pr) + if existing_comment: + existing_comment.edit( + f""" +{self.comment_tag} +:white_check_mark: With the latest revision this PR passed the {self.friendly_name}. +""" + ) + + def run(self, changed_files: [str], args: argparse.Namespace): + diff = self.format_run(changed_files, args) + if diff: + self.update_pr(diff, args) + return False + else: + self.update_pr_success(args) + return True + + +class ClangFormatHelper(FormatHelper): + name = "clang-format" + friendly_name = "C/C++ code formatter" + + @property + def instructions(self): + return " ".join(self.cf_cmd) + + @cached_property + def libcxx_excluded_files(self): + with open("libcxx/utils/data/ignore_format.txt", "r") as ifd: + return [excl.strip() for excl in ifd.readlines()] + + def should_be_excluded(self, path: str) -> bool: + if path in self.libcxx_excluded_files: + print(f"Excluding file {path}") + return True + return False + + def filter_changed_files(self, changed_files: [str]) -> [str]: + filtered_files = [] + for path in changed_files: + _, ext = os.path.splitext(path) + if ext in (".cpp", ".c", ".h", ".hpp", ".hxx", ".cxx"): + if not self.should_be_excluded(path): + filtered_files.append(path) + return filtered_files + + def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None: + cpp_files = self.filter_changed_files(changed_files) + if not cpp_files: + return + cf_cmd = [ + "git-clang-format", + "--diff", + args.start_rev, + args.end_rev, + "--", + ] + cpp_files + print(f"Running: {' '.join(cf_cmd)}") + self.cf_cmd = cf_cmd + proc = subprocess.run(cf_cmd, capture_output=True) + + # formatting needed + if proc.returncode == 1: + return proc.stdout.decode("utf-8") + + return None + + +class DarkerFormatHelper(FormatHelper): + name = "darker" + friendly_name = "Python code formatter" + + @property + def instructions(self): + return " ".join(self.darker_cmd) + + def filter_changed_files(self, changed_files: [str]) -> [str]: + filtered_files = [] + for path in changed_files: + name, ext = os.path.splitext(path) + if ext == ".py": + filtered_files.append(path) + + return filtered_files + + def format_run(self, changed_files: [str], args: argparse.Namespace) -> str | None: + py_files = self.filter_changed_files(changed_files) + if not py_files: + return + darker_cmd = [ + "darker", + "--check", + "--diff", + "-r", + f"{args.start_rev}..{args.end_rev}", + ] + py_files + print(f"Running: {' '.join(darker_cmd)}") + self.darker_cmd = darker_cmd + proc = subprocess.run(darker_cmd, capture_output=True) + + # formatting needed + if proc.returncode == 1: + return proc.stdout.decode("utf-8") + + return None + + +ALL_FORMATTERS = (DarkerFormatHelper(), ClangFormatHelper()) + +if __name__ == "__main__": + parser = argparse.ArgumentParser() + parser.add_argument( + "--token", type=str, required=True, help="GitHub authentiation token" + ) + parser.add_argument( + "--repo", + type=str, + default=os.getenv("GITHUB_REPOSITORY", "llvm/llvm-project"), + help="The GitHub repository that we are working with in the form of <owner>/<repo> (e.g. llvm/llvm-project)", + ) + parser.add_argument("--issue-number", type=int, required=True) + parser.add_argument( + "--start-rev", + type=str, + required=True, + help="Compute changes from this revision.", + ) + parser.add_argument( + "--end-rev", type=str, required=True, help="Compute changes to this revision" + ) + parser.add_argument( + "--changed-files", + type=str, + help="Comma separated list of files that has been changed", + ) + + args = parser.parse_args() + + changed_files = [] + if args.changed_files: + changed_files = args.changed_files.split(",") + + exit_code = 0 + for fmt in ALL_FORMATTERS: + if not fmt.run(changed_files, args): + exit_code = 1 + + sys.exit(exit_code) diff --git a/llvm/utils/git/requirements_formatting.txt b/llvm/utils/git/requirements_formatting.txt new file mode 100644 index 000000000000000..ff744f0d4225f59 --- /dev/null +++ b/llvm/utils/git/requirements_formatting.txt @@ -0,0 +1,52 @@ +# +# This file is autogenerated by pip-compile with Python 3.11 +# by the following command: +# +# pip-compile --output-file=llvm/utils/git/requirements_formatting.txt llvm/utils/git/requirements_formatting.txt.in +# +black==23.9.1 + # via + # -r llvm/utils/git/requirements_formatting.txt.in + # darker +certifi==2023.7.22 + # via requests +cffi==1.15.1 + # via + # cryptography + # pynacl +charset-normalizer==3.2.0 + # via requests +click==8.1.7 + # via black +cryptography==41.0.3 + # via pyjwt +darker==1.7.2 + # via -r llvm/utils/git/requirements_formatting.txt.in +deprecated==1.2.14 + # via pygithub +idna==3.4 + # via requests +mypy-extensions==1.0.0 + # via black +packaging==23.1 + # via black +pathspec==0.11.2 + # via black +platformdirs==3.10.0 + # via black +pycparser==2.21 + # via cffi +pygithub==1.59.1 + # via -r llvm/utils/git/requirements_formatting.txt.in +pyjwt[crypto]==2.8.0 + # via pygithub +pynacl==1.5.0 + # via pygithub +requests==2.31.0 + # via pygithub +toml==0.10.2 + # via darker +urllib3==2.0.4 + # via requests +wrapt==1.15.0 + # via deprecated diff --git a/llvm/utils/git/requirements_formatting.txt.in b/llvm/utils/git/requirements_formatting.txt.in new file mode 100644 index 000000000000000..4aac571af1cf51c --- /dev/null +++ b/llvm/utils/git/requirements_formatting.txt.in @@ -0,0 +1,3 @@ +black~=23.0 +darker==1.7.2 +PyGithub==1.59.1 

[tru]

Tested with forks in my tree and pull_request_target works fine. Will merge this again and keep an eye on it.