From 068f89307245d062bf2bf995de3726e33faef5d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Wed, 19 Apr 2023 15:10:36 +0200 Subject: [PATCH] CI: Speed up static checks by checking only changed files - file_format, header_guards and clang-format benefit from this short list. - dotnet-format, Python and JS checks don't, but they're only relevant for PRs changing a specific set of files, so we skip them when those files aren't modified. The logic to get changed files only works reliably for: - Pull request events - Non-force pushed push events So when force pushing a branch in your fork, or creating a new branch, it will still scan all files as fallback. Upgraded CI runner to Ubuntu 22.04 so we get clang-format 14 out of the box, so we don't need to install a custom version (saves ~15 s). We also cache the APT dependencies to speed up the build and avoid flaky Ubuntu/Microsoft repos. --- .github/workflows/runner.yml | 2 +- .github/workflows/static_checks.yml | 69 ++++++++++++++++++++--------- misc/scripts/clang_format.sh | 21 ++++++--- misc/scripts/file_format.sh | 14 ++++-- misc/scripts/header_guards.sh | 18 +++++--- 5 files changed, 87 insertions(+), 37 deletions(-) diff --git a/.github/workflows/runner.yml b/.github/workflows/runner.yml index 0487b1e090d..be255b54688 100644 --- a/.github/workflows/runner.yml +++ b/.github/workflows/runner.yml @@ -7,7 +7,7 @@ concurrency: jobs: static-checks: - name: 📊 Static + name: 📊 Static checks uses: ./.github/workflows/static_checks.yml android-build: diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index b2ab9132347..8ef49c4539a 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -8,38 +8,58 @@ concurrency: jobs: static-checks: - name: Static Checks (clang-format, black format, file format, documentation checks) - runs-on: ubuntu-20.04 + name: Code style, file formatting, and docs + runs-on: ubuntu-22.04 steps: - name: Checkout uses: actions/checkout@v3 + with: + fetch-depth: 2 - - name: Install dependencies + - name: Install APT dependencies + uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: dos2unix libxml2-utils moreutils + + - name: Install Python dependencies and general setup run: | - sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list - wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add - - sudo apt-add-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-15 main" - sudo apt-get install -qq dos2unix clang-format-15 libxml2-utils python3-pip moreutils - sudo update-alternatives --remove-all clang-format || true - sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-15 100 - sudo pip3 install black==22.3.0 pytest==7.1.2 mypy==0.971 + pip3 install black==22.3.0 pytest==7.1.2 mypy==0.971 git config diff.wsErrorHighlight all + - name: Get changed files + id: changed-files + run: | + if [ "${{ github.event_name }}" == "pull_request" ]; then + files=$(git diff-tree --no-commit-id --name-only -r ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }} 2> /dev/null) + elif [ "${{ github.event_name }}" == "push" -a "${{ github.event.forced }}" == "false" -a "${{ github.event.created }}" == "false" ]; then + files=$(git diff-tree --no-commit-id --name-only -r ${{ github.event.before }}..${{ github.event.after }} 2> /dev/null) + fi + echo "$files" >> changed.txt + cat changed.txt + - name: File formatting checks (file_format.sh) run: | - bash ./misc/scripts/file_format.sh + bash ./misc/scripts/file_format.sh changed.txt - name: Header guards formatting checks (header_guards.sh) run: | - bash ./misc/scripts/header_guards.sh + bash ./misc/scripts/header_guards.sh changed.txt - name: Python style checks via black (black_format.sh) run: | - bash ./misc/scripts/black_format.sh + if grep -qE '*\.py|SConstruct|SCsub' changed.txt || [ ! -s changed.txt ]; then + bash ./misc/scripts/black_format.sh + else + echo "Skipping Python formatting as no Python files were changed." + fi - name: Python scripts static analysis (mypy_check.sh) run: | - bash ./misc/scripts/mypy_check.sh + if grep -qE '*\.py|SConstruct|SCsub' changed.txt || [ ! -s changed.txt ]; then + bash ./misc/scripts/mypy_check.sh + else + echo "Skipping Python static analysis as no Python files were changed." + fi - name: Python builders checks via pytest (pytest_builders.sh) run: | @@ -47,10 +67,14 @@ jobs: - name: JavaScript style and documentation checks via ESLint and JSDoc run: | - cd platform/web - npm ci - npm run lint - npm run docs -- -d dry-run + if grep -q "platform/web" changed.txt || [ ! -s changed.txt ]; then + cd platform/web + npm ci + npm run lint + npm run docs -- -d dry-run + else + echo "Skipping JavaScript formatting as no Web/JS files were changed." + fi - name: Class reference schema checks run: | @@ -62,11 +86,16 @@ jobs: - name: Style checks via clang-format (clang_format.sh) run: | - bash ./misc/scripts/clang_format.sh + clang-format --version + bash ./misc/scripts/clang_format.sh changed.txt - name: Style checks via dotnet format (dotnet_format.sh) run: | - bash ./misc/scripts/dotnet_format.sh + if grep -q "modules/mono" changed.txt || [ ! -s changed.txt ]; then + bash ./misc/scripts/dotnet_format.sh + else + echo "Skipping dotnet format as no C# files were changed." + fi - name: Spell checks via codespell uses: codespell-project/actions-codespell@v1 diff --git a/misc/scripts/clang_format.sh b/misc/scripts/clang_format.sh index 318a78b8651..74a8e1a8a3a 100755 --- a/misc/scripts/clang_format.sh +++ b/misc/scripts/clang_format.sh @@ -5,15 +5,22 @@ set -uo pipefail -# Loops through all code files tracked by Git. -git ls-files -- '*.c' '*.h' '*.cpp' '*.hpp' '*.cc' '*.hh' '*.cxx' '*.m' '*.mm' '*.inc' '*.java' '*.glsl' \ +if [ $# -eq 0 ]; then + # Loop through all code files tracked by Git. + files=$(git ls-files -- '*.c' '*.h' '*.cpp' '*.hpp' '*.cc' '*.hh' '*.cxx' '*.m' '*.mm' '*.inc' '*.java' '*.glsl' \ ':!:.git/*' ':!:thirdparty/*' ':!:*/thirdparty/*' ':!:platform/android/java/lib/src/com/google/*' \ - ':!:*-so_wrap.*' ':!:tests/python_build/*' | -while read -r f; do - # Run clang-format. - clang-format --Wno-error=unknown -i "$f" + ':!:*-so_wrap.*' ':!:tests/python_build/*') +else + # $1 should be a file listing file paths to process. Used in CI. + files=$(cat "$1" | grep -v "thirdparty/" | grep -E "\.(c|h|cpp|hpp|cc|hh|cxx|m|mm|inc|java|glsl)$" | grep -v "platform/android/java/lib/src/com/google/" | grep -v "\-so_wrap\." | grep -v "tests/python_build/") +fi - # Fix copyright headers, but not all files get them. +if [ ! -z "$files" ]; then + clang-format --Wno-error=unknown -i $files +fi + +# Fix copyright headers, but not all files get them. +for f in $files; do if [[ "$f" == *"inc" ]]; then continue elif [[ "$f" == *"glsl" ]]; then diff --git a/misc/scripts/file_format.sh b/misc/scripts/file_format.sh index f394c4ad3f5..94a3affbd70 100755 --- a/misc/scripts/file_format.sh +++ b/misc/scripts/file_format.sh @@ -7,14 +7,20 @@ # We need dos2unix and isutf8. if [ ! -x "$(command -v dos2unix)" -o ! -x "$(command -v isutf8)" ]; then printf "Install 'dos2unix' and 'isutf8' (moreutils package) to use this script.\n" + exit 1 fi set -uo pipefail -IFS=$'\n\t' -# Loops through all text files tracked by Git. -git grep -zIl '' | -while IFS= read -rd '' f; do +if [ $# -eq 0 ]; then + # Loop through all code files tracked by Git. + mapfile -d '' files < <(git grep -zIl '') +else + # $1 should be a file listing file paths to process. Used in CI. + mapfile -d ' ' < <(cat "$1") +fi + +for f in "${files[@]}"; do # Exclude some types of files. if [[ "$f" == *"csproj" ]]; then continue diff --git a/misc/scripts/header_guards.sh b/misc/scripts/header_guards.sh index a063ff4b9c9..1f8aa6151c8 100755 --- a/misc/scripts/header_guards.sh +++ b/misc/scripts/header_guards.sh @@ -5,9 +5,17 @@ if [ ! -f "version.py" ]; then echo "Some of the paths checks may not work as intended from a different folder." fi +if [ $# -eq 0 ]; then + # Loop through all code files tracked by Git. + files=$(find -name "thirdparty" -prune -o -name "*.h" -print | sed "s@^\./@@g") +else + # $1 should be a file listing file paths to process. Used in CI. + files=$(cat "$1" | grep -v "thirdparty/" | grep -E "\.h$" | sed "s@^\./@@g") +fi + files_invalid_guard="" -for file in $(find -name "thirdparty" -prune -o -name "*.h" -print); do +for file in $files; do # Skip *.gen.h and *-so_wrap.h, they're generated. if [[ "$file" == *".gen.h" || "$file" == *"-so_wrap.h" ]]; then continue; fi # Has important define before normal header guards. @@ -20,16 +28,16 @@ for file in $(find -name "thirdparty" -prune -o -name "*.h" -print); do # Add custom prefix or suffix for generic filenames with a well-defined namespace. prefix= - if [[ "$file" == "./modules/"*"/register_types.h" ]]; then + if [[ "$file" == "modules/"*"/register_types.h" ]]; then module=$(echo $file | sed "s@.*modules/\([^/]*\).*@\1@") prefix="${module^^}_" fi - if [[ "$file" == "./platform/"*"/api/api.h" || "$file" == "./platform/"*"/export/"* ]]; then + if [[ "$file" == "platform/"*"/api/api.h" || "$file" == "platform/"*"/export/"* ]]; then platform=$(echo $file | sed "s@.*platform/\([^/]*\).*@\1@") prefix="${platform^^}_" fi - if [[ "$file" == "./modules/mono/utils/"* && "$bname" != *"mono"* ]]; then prefix="MONO_"; fi - if [[ "$file" == "./servers/rendering/storage/utilities.h" ]]; then prefix="RENDERER_"; fi + if [[ "$file" == "modules/mono/utils/"* && "$bname" != *"mono"* ]]; then prefix="MONO_"; fi + if [[ "$file" == "servers/rendering/storage/utilities.h" ]]; then prefix="RENDERER_"; fi suffix= if [[ "$file" == *"dummy"* && "$bname" != *"dummy"* ]]; then suffix="_DUMMY"; fi