Check format: only check modified files, v2 (#1680)

* Check format: only check modified files, v2

* cheeky z_play edit

* format

* rename z_play.c -> z_play_.c

* Revert "rename z_play.c -> z_play_.c"

This reverts commit 9da9e76309.

* rm z_moji.c

* Revert "rm z_moji.c"

This reverts commit bd2d5acb5a.

Jenkinsfile

@@ -10,7 +10,7 @@ pipeline {
             }
             steps {
                 echo 'Checking formatting on all files...'
-                sh 'tools/check_format.sh full'
+                sh 'python3 tools/check_format.py'
             }
         }
         stage('Check formatting (modified)') {
@@ -21,7 +21,7 @@ pipeline {
             }
             steps {
                 echo 'Checking formatting on modified files...'
-                sh 'tools/check_format.sh modified'
+                sh 'python3 tools/check_format.py --verbose --compare-to origin/main'
             }
         }
         stage('Setup') {

format.py

@@ -9,7 +9,6 @@ import shutil
 import subprocess
 import sys
 import tempfile
-from pathlib import Path
 from functools import partial
 from typing import List
 
@@ -151,15 +150,6 @@ def main():
         action="store_true",
         help="Print the paths to the clang-* binaries used",
     )
-    parser.add_argument(
-        "--files-list",
-        dest="files_list",
-        help="A file listing the files to format",
-    )
-    parser.add_argument(
-        "--verbose",
-        action="store_true",
-    )
     parser.add_argument("files", metavar="file", nargs="*")
     parser.add_argument(
         "-j",
@@ -185,18 +175,10 @@ def main():
                 f"Error: neither clang-apply-replacements nor clang-apply-replacements-{CLANG_VER} found (required to use -j)"
             )
 
-    if args.files or args.files_list:
+    if args.files:
-        files = []
+        files = args.files
-        files.extend(args.files)
-        if args.files_list:
-            files.extend(Path(args.files_list).read_text().split())
-        files = list(files)
         extra_files = []
-        if args.verbose:
-            print("Formatting specific files:", len(files), files)
     else:
-        if args.verbose:
-            print("Formatting all files")
         files = glob.glob("src/**/*.c", recursive=True)
         extra_files = glob.glob("assets/**/*.xml", recursive=True)
 

tools/check_format.py

@@ -0,0 +1,77 @@
+# SPDX-FileCopyrightText: 2024 zeldaret
+# SPDX-License-Identifier: CC0-1.0
+
+import subprocess
+import argparse
+import difflib
+import multiprocessing
+import glob
+import os.path
+import sys
+
+sys.path.insert(0, os.curdir)
+import format
+
+sys.path.pop(0)
+
+
+def get_git_status():
+    return subprocess.check_output("git status --porcelain".split(), text=True)
+
+
+def get_modified_files_to_format(compare_to):
+    modified_files_str = subprocess.check_output(
+        ["git", "diff", "--name-only", compare_to], text=True
+    )
+    modified_files = modified_files_str.splitlines()
+    modified_c_files = [
+        f
+        for f in modified_files
+        if f.startswith("src" + os.path.sep) and f.endswith(".c")
+    ]
+    modified_c_files_existing = [f for f in modified_c_files if os.path.exists(f)]
+    return modified_c_files_existing
+
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--verbose", action="store_true")
+    parser.add_argument("--compare-to", dest="compare_to")
+    args = parser.parse_args()
+
+    if args.compare_to:
+        src_files = get_modified_files_to_format(args.compare_to)
+        if args.verbose:
+            print("Formatting specific files:", len(src_files), src_files)
+        if not src_files:
+            if args.verbose:
+                print("Nothing to format")
+            exit(0)
+    else:
+        src_files = glob.glob("src/**/*.c", recursive=True)
+
+    nb_jobs = multiprocessing.cpu_count()
+
+    git_status_pre = get_git_status()
+
+    format.format_files(src_files, [], nb_jobs)
+
+    git_status_post = get_git_status()
+
+    if git_status_pre != git_status_post:
+        print(
+            "Misformatted files found."
+            " Run ./format.py and verify codegen is not impacted."
+        )
+        for l in difflib.unified_diff(
+            git_status_pre.splitlines(),
+            git_status_post.splitlines(),
+            "Old git status",
+            "New git status",
+        ):
+            print(l)
+        exit(1)
+
+
+if __name__ == "__main__":
+    main()

tools/check_format.sh

@@ -1,50 +0,0 @@
-#!/bin/bash
-
-if [ $# -ne 1 ]
-then
-    echo "Usage: $0 <full | modified>. Exiting."
-    exit -1
-fi
-
-STATUSOLD=`git status --porcelain`
-
-if [ $1 = 'modified' ]
-then
-    FILES_LIST=.tmp_format_modified_files_list.txt
-    git show --name-only --format='' origin/main.. | grep 'src/.*\.c' | sort | uniq > $FILES_LIST
-    if [ ${PIPESTATUS[0]} -ne 0 ]  # git show failed
-    then
-        echo "Listing modified files failed. Exiting."
-        exit -1
-    fi
-    ./format.py --verbose -j`nproc` --files-list $FILES_LIST
-    FORMAT_EXIT_CODE=$?
-    rm $FILES_LIST
-elif [ $1 = 'full' ]
-then
-    ./format.py -j
-    FORMAT_EXIT_CODE=$?
-else
-    echo "Unknown argument $1. Exiting."
-    exit -1
-fi
-
-if [ $FORMAT_EXIT_CODE -ne 0 ]
-then
-   echo "Formatter failed. Exiting."
-   exit -1
-fi
-STATUSNEW=`git status --porcelain`
-
-if [ "${STATUSOLD}" != "${STATUSNEW}" ];
-then
-    echo ""
-    echo "Misformatted files found. Run ./format.py and verify codegen is not impacted."
-    echo ""
-    diff --unified=0  --label "Old git status" <(echo "${STATUSOLD}") --label "New git status" <(echo "${STATUSNEW}")
-    echo ""
-    echo "Exiting."
-    exit 1
-fi
-
-exit 0