Only run IWYU on affected files + minor get_affected_files.py refactor (CleverRaven#79690)

* Refactor get_affected_files.py slightly

Changes:
- It now uses argparse for argument parsing, making its configuration
  somewhat more flexible
- It can determine the list of changed files on its own, and does
  not need to have it passed explicitly (used for local testing,
  not so much for CI)
- It sorts the output, putting the directly changed files (if any) at
  the top of the list. The hope is that this decreases the feedback
  latency ever so slightly.

* Only run IWYU on potentially affected files

drive-by change: remove leftover skip-duplicate check from IWYU job
it's not doing nothing other than hard-failing the job on draft PRs.

* Forgor that get_affected_files needs to know includes, whoops

Author: moxian

.github/workflows/iwyu.yml

@@ -25,7 +25,6 @@ jobs:
         COMPILER: clang++-19
     steps:
     - name: install LLVM 19
-      if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
       run: |
           sudo apt install llvm-19 llvm-19-dev llvm-19-tools clang-19 clang-tidy-19 clang-tools-19 libclang-19-dev
           sudo apt install python3-pip ninja-build cmake
@@ -47,24 +46,55 @@ jobs:
         echo "IWYU_SRC_DIR=${PWD}/include-what-you-use-src">> "$GITHUB_OUTPUT"
         echo "IWYU_BIN_DIR=${PWD}/iwyu-build/bin">> "$GITHUB_OUTPUT"
     - uses: ammaraskar/gcc-problem-matcher@master
+    - name: determine changed files
+      if: ${{ github.event_name == 'pull_request' }}
+      uses: actions/github-script@v7
+      with:
+        script: |
+          var fs = require('fs');
+          const response = await github.paginate(github.rest.pulls.listFiles,
+            {
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              pull_number: context.issue.number,
+            }
+          );
+          const files = response.map(x => x.filename);
+          for (const path of files) {
+            console.log(path);
+          }
+          fs.writeFileSync("Cataclysm-DDA/files_changed", files.join('\n') + '\n');
     - name: create CDDA compilation database
       run: |
-        cmake -B Cataclysm-DDA/build \
+        cd Cataclysm-DDA
+        cmake -B build \
             -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
             -DCMAKE_CXX_COMPILER=$COMPILER \
             -DCMAKE_BUILD_TYPE="Release" \
             -DTILES=${TILES:-0} \
             -DSOUND=${SOUND:-0} \
             -DLOCALIZE=${LOCALIZE:-0} \
-            Cataclysm-DDA
+            .
+        # and include database too, for get_affected_files.py
+        make includes -j4 --silent TILES=${TILES:-0} SOUND=${SOUND:-0} LOCALIZE=${LOCALIZE:-0}
     - name: run the thing
       env:
         IWYU_SRC_DIR: ${{ steps.build-iwyu.outputs.IWYU_SRC_DIR }}
         IWYU_BIN_DIR: ${{ steps.build-iwyu.outputs.IWYU_BIN_DIR }}
       run: |
         PATH="${PATH}:${IWYU_BIN_DIR}"
-
         cd Cataclysm-DDA
-        FILES_LIST=$( find src/ tests/ -maxdepth 1 -name '*.cpp' | grep -v -f tools/iwyu/bad_files.txt | sort )
+
+        CHECK_ALL=no
+        if [[ -f ./files_changed ]]; then
+            CHECK_ALL=$( cat ./files_changed | grep -E -i 'tools/iwyu|github/iwyu.yml|CMakeLists.txt|build-scripts/get_affected_files.py' && echo yes || echo no )
+        fi
+        if [[ "${CHECK_ALL}" = "no" ]] && [[ -f ./files_changed ]]; then
+            FILES_LIST=$( python3 build-scripts/get_affected_files.py --changed-files-list ./files_changed )
+        else
+            FILES_LIST=$( find src/ tests/ -maxdepth 1 -name '*.cpp' | sort )
+        fi
+        FILES_LIST=$( printf "%s\n" ${FILES_LIST[@]} | grep -v -f tools/iwyu/bad_files.txt )
+
         python ${IWYU_SRC_DIR}/iwyu_tool.py ${FILES_LIST} -p build --output-format clang --jobs 4  -- -Xiwyu "--mapping_file=${PWD}/tools/iwyu/cata.imp" -Xiwyu --cxx17ns -Xiwyu --comment_style=long -Xiwyu --max_line_length=1000 -Xiwyu --error=1
 

build-scripts/clang-tidy-run.sh

@@ -75,7 +75,7 @@ else
         includes
 
     tidyable_cpp_files="$( \
-        ( test -f ./files_changed && ( build-scripts/get_affected_files.py ./files_changed ) ) || \
+        ( test -f ./files_changed && ( build-scripts/get_affected_files.py --changed-files-list ./files_changed ) ) || \
         echo unknown )"
 
     tidyable_cpp_files="$(echo -n "$tidyable_cpp_files" | grep -v third-party || true)"

build-scripts/get_affected_files.py

@@ -1,8 +1,10 @@
 #!/usr/bin/env python3
 
+import argparse
 import glob
 import os
 import sys
+import subprocess
 
 
 class IncludesParser:
@@ -87,9 +89,41 @@ def get_files_affected_by(self, changed_file):
         return self.includes_to_sources.get(changed_file, [])
 
 
-def main(changed_files_file):
-    parser = IncludesParser()
+def get_changed_files_from_list(changed_files_list):
+    with open(changed_files_list, 'r') as f:
+        changed_files = [line.strip() for line in f.read().splitlines()]
+        return changed_files
+
+
+def get_changed_files_from_git(merge_target):
+    out = subprocess.run(
+        ["git", "diff", "--merge-base", "--name-only", merge_target],
+        capture_output=True)
+    if out.returncode != 0:
+        print(out.stderr, file=sys.stderr)
+        raise Exception("git diff failed to run")
+    changed_files = [line.strip() for line in out.stdout.decode().splitlines()]
+    return changed_files
+
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--changed-files-list", help=("Path to a file"
+                        "containing list of changed files, one per line."))
+    parser.add_argument(
+        "--merge-target", default="master",
+        help=("Name of the branch we are intending to merge into."
+              "Will be used to determine the list of files changed. "
+              "Ignored if --changed-files-list specified explicilty."))
+    args = parser.parse_args()
+
+    changed_files = None
+    if args.changed_files_list:
+        changed_files = get_changed_files_from_list(args.changed_files_list)
+    else:
+        changed_files = get_changed_files_from_git(args.merge_target)
 
+    parser = IncludesParser()
     # Load includes files list.
     if not parser.parse_includes_files_from("obj", "src", "."):
         return 1
@@ -98,25 +132,22 @@ def main(changed_files_file):
     if not parser.parse_includes_files_from("tests/obj", "tests", "tests"):
         return 1
 
-    changed_files = []
     lintable_files = set()
-    with open(changed_files_file, 'r') as f:
-        changed_files = f.read().splitlines()
-        for changed_file in changed_files:
-            for affected_file in parser.get_files_affected_by(changed_file):
-                lintable_files.add(affected_file)
+    for changed_file in changed_files:
+        for affected_file in parser.get_files_affected_by(changed_file):
+            lintable_files.add(affected_file)
+    changed_files = set(changed_files)
 
-    for lintable_file in lintable_files:
+    # Lightly sort the output list so that directly changed files appear first
+    lintable_files_sorted = []
+    lintable_files_sorted.extend(sorted(lintable_files & changed_files))
+    lintable_files_sorted.extend(sorted(lintable_files - changed_files))
+
+    for lintable_file in lintable_files_sorted:
         print(lintable_file)
 
     return 0
 
 
 if __name__ == '__main__':
-    if len(sys.argv) <= 1:
-        print(
-            "First argument must be a path to a list of changed files",
-            file=sys.stderr,
-        )
-        sys.exit(1)
-    sys.exit(main(sys.argv[1]))
+    sys.exit(main())