From 232859250c915a33349e7a25b7fdb1c6883b61bd Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Tue, 25 Jul 2023 12:33:28 +1000 Subject: [PATCH 1/4] tools/codeformat.py: Remove git state detection. This was added to speed up running codeformat.py when only a small number of files are changed, but it breaks running the tool on the master branch. The pre-commit tool handles this correctly, and it's working well in the main repo, so we can remove the special handling. This makes codeformat.py behave identically to the main repository, but without additional code for handling .c/.h files. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared --- tools/codeformat.py | 167 +++++++------------------------------------- 1 file changed, 26 insertions(+), 141 deletions(-) diff --git a/tools/codeformat.py b/tools/codeformat.py index 63c9c5988..ebcb22416 100755 --- a/tools/codeformat.py +++ b/tools/codeformat.py @@ -5,7 +5,7 @@ # The MIT License (MIT) # # Copyright (c) 2020 Damien P. George -# Copyright (c) 2020 Jim Mussared +# Copyright (c) 2023 Jim Mussared # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -25,6 +25,9 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. +# This is based on tools/codeformat.py from the main micropython/micropython +# repository but without support for .c/.h files. + import argparse import glob import itertools @@ -34,9 +37,6 @@ # Relative to top-level repo dir. PATHS = [ - # C - "**/*.[ch]", - # Python "**/*.py", ] @@ -45,19 +45,9 @@ # Path to repo top-level dir. TOP = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) -UNCRUSTIFY_CFG = os.path.join(TOP, "tools/uncrustify.cfg") - -C_EXTS = ( - ".c", - ".h", -) PY_EXTS = (".py",) -MAIN_BRANCH = "master" -BASE_BRANCH = os.environ.get("GITHUB_BASE_REF", MAIN_BRANCH) - - def list_files(paths, exclusions=None, prefix=""): files = set() for pattern in paths: @@ -67,128 +57,33 @@ def list_files(paths, exclusions=None, prefix=""): return sorted(files) -def fixup_c(filename): - # Read file. - with open(filename) as f: - lines = f.readlines() - - # Write out file with fixups. - with open(filename, "w", newline="") as f: - dedent_stack = [] - while lines: - # Get next line. - l = lines.pop(0) - - # Dedent #'s to match indent of following line (not previous line). - m = re.match(r"( +)#(if |ifdef |ifndef |elif |else|endif)", l) - if m: - indent = len(m.group(1)) - directive = m.group(2) - if directive in ("if ", "ifdef ", "ifndef "): - l_next = lines[0] - indent_next = len(re.match(r"( *)", l_next).group(1)) - if indent - 4 == indent_next and re.match(r" +(} else |case )", l_next): - # This #-line (and all associated ones) needs dedenting by 4 spaces. - l = l[4:] - dedent_stack.append(indent - 4) - else: - # This #-line does not need dedenting. - dedent_stack.append(-1) - else: - if dedent_stack[-1] >= 0: - # This associated #-line needs dedenting to match the #if. - indent_diff = indent - dedent_stack[-1] - assert indent_diff >= 0 - l = l[indent_diff:] - if directive == "endif": - dedent_stack.pop() - - # Write out line. - f.write(l) - - assert not dedent_stack, filename - - -def query_git_files(verbose): - def cmd_result_set(cmd): - ret = subprocess.run(cmd, capture_output=True).stdout.strip().decode() - if not ret: - return set() - return {f.strip() for f in ret.split("\n")} - - def rel_paths(files, root): - return {os.path.relpath(os.path.join(root, f.strip()), ".") for f in files} - - try: - ret = set() - - # get path to root of repository - root_dir = ( - subprocess.run(["git", "rev-parse", "--show-toplevel"], capture_output=True) - .stdout.strip() - .decode() - ) - - # Check locally modified files - status = cmd_result_set(["git", "status", "--porcelain"]) - dirty_files = rel_paths({line.split(" ", 1)[-1] for line in status}, root_dir) - ret |= dirty_files - - # Current commit and branch - current_commit = ( - subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True) - .stdout.strip() - .decode() - ) - current_branches = cmd_result_set(["git", "branch", "--contains", current_commit]) - if MAIN_BRANCH in current_branches: - if ret: - if verbose: - print("Local changes detected, only scanning them.") - return ret - - # We're on clean master, run on entire repo - if verbose: - print("Scanning whole repository") - return None - - # List the files modified on current branch - if verbose: - print("Scanning changes from current branch and any local changes") - files_on_branch = rel_paths( - cmd_result_set(["git", "diff", "--name-only", BASE_BRANCH]), root_dir - ) - ret |= files_on_branch - return ret - except: - # Git not available, run on entire repo - return None - - def main(): - cmd_parser = argparse.ArgumentParser(description="Auto-format C and Python files.") - cmd_parser.add_argument("-c", action="store_true", help="Format C code only") - cmd_parser.add_argument("-p", action="store_true", help="Format Python code only") + cmd_parser = argparse.ArgumentParser(description="Auto-format Python files.") cmd_parser.add_argument("-v", action="store_true", help="Enable verbose output") cmd_parser.add_argument( - "files", - nargs="*", - help="Run on specific globs. If not specied current branch changes will be used", + "-f", + action="store_true", + help="Filter files provided on the command line against the default list of files to check.", ) + cmd_parser.add_argument("files", nargs="*", help="Run on specific globs") args = cmd_parser.parse_args() - # Setting only one of -c or -p disables the other. If both or neither are set, then do both. - format_c = args.c or not args.p - format_py = args.p or not args.c - # Expand the globs passed on the command line, or use the default globs above. files = [] if args.files: files = list_files(args.files) + if args.f: + # Filter against the default list of files. This is a little fiddly + # because we need to apply both the inclusion globs given in PATHS + # as well as the EXCLUSIONS, and use absolute paths + files = set(os.path.abspath(f) for f in files) + all_files = set(list_files(PATHS, EXCLUSIONS, TOP)) + if args.v: # In verbose mode, log any files we're skipping + for f in files - all_files: + print("Not checking: {}".format(f)) + files = list(files & all_files) else: - files = query_git_files(verbose=args.v) - if not files: - files = list_files(PATHS, EXCLUSIONS, TOP) + files = list_files(PATHS, EXCLUSIONS, TOP) # Extract files matching a specific language. def lang_files(exts): @@ -204,23 +99,13 @@ def batch(cmd, files, N=200): break subprocess.check_call(cmd + file_args) - # Format C files with uncrustify. - if format_c: - command = ["uncrustify", "-c", UNCRUSTIFY_CFG, "-lC", "--no-backup"] - if not args.v: - command.append("-q") - batch(command, lang_files(C_EXTS)) - for file in lang_files(C_EXTS): - fixup_c(file) - # Format Python files with black. - if format_py: - command = ["black", "--fast", "--line-length=99"] - if args.v: - command.append("-v") - else: - command.append("-q") - batch(command, lang_files(PY_EXTS)) + command = ["black", "--fast", "--line-length=99"] + if args.v: + command.append("-v") + else: + command.append("-q") + batch(command, lang_files(PY_EXTS)) if __name__ == "__main__": From 5cdfe715364844ad783d4ad63394099b7dc33109 Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Tue, 25 Jul 2023 12:37:06 +1000 Subject: [PATCH 2/4] top: Add pre-commit config. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared --- .pre-commit-config.yaml | 11 +++++++++++ CONTRIBUTING.md | 18 ++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..e017b86e8 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,11 @@ +repos: + - repo: local + hooks: + - id: codeformat + name: MicroPython codeformat.py for changed files + entry: tools/codeformat.py -v -f + language: python + - repo: https://github.com/charliermarsh/ruff-pre-commit + rev: v0.0.280 + hooks: + - id: ruff diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 804a26bef..d590754e9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,11 +25,21 @@ or packages from micropython-lib, please post at the ### Pull requests The same rules for commit messages, signing-off commits, and commit structure -apply as for the main MicroPython repository. All Python code is formatted -using `black`. See [`tools/codeformat.py`](tools/codeformat.py) to apply -`black` automatically before submitting a PR. +apply [as for the main MicroPython repository](https://github.com/micropython/micropython/blob/master/CODECONVENTIONS.md). -There are some specific conventions and guidelines for micropython-lib: +All Python code is formatted using the [black](https://github.com/psf/black) +tool. You can run [`tools/codeformat.py`](tools/codeformat.py) to apply +`black` automatically before submitting a PR. The GitHub CI will also run the +[ruff](https://github.com/astral-sh/ruff) tool to apply further "linting" +checks. + +Similar to the main repository, a configuration is provided for the +[pre-commit](https://pre-commit.com/) tool to apply `black` code formatting +rules and run `ruff` automatically. See the documentation for using pre-commit +in [the code conventions document](https://github.com/micropython/micropython/blob/master/CODECONVENTIONS.md#automatic-pre-commit-hooks) + +In addition to the conventions from the main repository, there are some +specific conventions and guidelines for micropython-lib: * The first line of the commit message should start with the name of the package, followed by a short description of the commit. Package names are From efa04028466b8aae30adb63a215b6c4f52c52d0f Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Tue, 25 Jul 2023 12:46:49 +1000 Subject: [PATCH 3/4] tools/codeformat.py: Fix ruff warnings. Signed-off-by: Jim Mussared --- tools/codeformat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/codeformat.py b/tools/codeformat.py index ebcb22416..2bc0c7f44 100755 --- a/tools/codeformat.py +++ b/tools/codeformat.py @@ -76,7 +76,7 @@ def main(): # Filter against the default list of files. This is a little fiddly # because we need to apply both the inclusion globs given in PATHS # as well as the EXCLUSIONS, and use absolute paths - files = set(os.path.abspath(f) for f in files) + files = {os.path.abspath(f) for f in files} all_files = set(list_files(PATHS, EXCLUSIONS, TOP)) if args.v: # In verbose mode, log any files we're skipping for f in files - all_files: From ce3f282967ded48d21bf6588834893c066e4be8b Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Tue, 25 Jul 2023 12:53:15 +1000 Subject: [PATCH 4/4] github/workflows: Split ruff into its own action. This matches the main repo, and conceputually ruff is not strictly doing "code formatting". Signed-off-by: Jim Mussared --- .github/workflows/code_formatting.yml | 2 -- .github/workflows/ruff.yml | 10 ++++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/ruff.yml diff --git a/.github/workflows/code_formatting.yml b/.github/workflows/code_formatting.yml index 7e74776af..71c50aa1b 100644 --- a/.github/workflows/code_formatting.yml +++ b/.github/workflows/code_formatting.yml @@ -7,8 +7,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - run: pip install --user ruff - - run: ruff --format=github . - uses: actions/setup-python@v4 - name: Install packages run: source tools/ci.sh && ci_code_formatting_setup diff --git a/.github/workflows/ruff.yml b/.github/workflows/ruff.yml new file mode 100644 index 000000000..b8e43dc78 --- /dev/null +++ b/.github/workflows/ruff.yml @@ -0,0 +1,10 @@ +# https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python +name: Python code lint with ruff +on: [push, pull_request] +jobs: + ruff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - run: pip install --user ruff + - run: ruff --format=github .