Speed up linux and darwin CI (#35971)

* Restrict clang-tidy

* Move changed-files action forward

* Update logic a bit to match tj-actions documentation

* Restyled by autopep8

* Update logic to skip more tests for linux to make linux CI finish sooner on pull requests

* Revert "Update logic to skip more tests for linux to make linux CI finish sooner on pull requests"

This reverts commit a9bc97b.

* Generate only for sanitizers

* Make sure we checkout a bit of history and persist credentials

* Fix up logic for tidy, should now be able to actually run based on github file diff

* Restyle

* Update .github/workflows/build.yaml

Co-authored-by: Boris Zbarsky <[email protected]>

* Fix env

* Code review update

---------

Co-authored-by: Andrei Litvin <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>

.github/workflows/build.yaml

@@ -157,6 +157,9 @@ jobs:
               run: echo "$CONCURRENCY_CONTEXT"
             - name: Checkout
               uses: actions/checkout@v4
+              with:
+                  fetch-depth: 2
+                  persist-credentials: true
             - name: Try to ensure the directories for core dumping exist and we
                   can write them.
               run: |
@@ -186,6 +189,10 @@ jobs:
             - name: Clean output
               run: rm -rf ./out
             - name: Run Tests with sanitizers
+              # Sanitizer tests are not likely to find extra issues so running the same tests
+              # as above repeatedly on every pull request seems extra time. Instead keep this run
+              # for master only
+              if: github.event.pull_request.number == null
               env:
                   LSAN_OPTIONS: detect_leaks=1
               run: |
@@ -201,17 +208,34 @@ jobs:
                       BUILD_TYPE=sanitizers scripts/build/gn_gen.sh --args="$GN_ARGS chip_data_model_check_die_on_failure=true" --export-compile-commands
                       BUILD_TYPE=sanitizers scripts/tests/gn_tests.sh
                   done
+            - name: Generate tests with sanitizers (for tidy)
+              if: github.event.pull_request.number != null
+              run: |
+                  rm -rf ./out/sanitizers
+                  BUILD_TYPE=sanitizers scripts/build/gn_gen.sh --args="is_clang=true is_asan=true chip_data_model_check_die_on_failure=true" --export-compile-commands
             - name: Ensure codegen is done for sanitize
               run: |
                   ./scripts/run_in_build_env.sh "./scripts/run_codegen_targets.sh out/sanitizers"
+            - name: Find changed files
+              id: changed-files
+              uses: tj-actions/changed-files@v45
             - name: Clang-tidy validation
               # NOTE: clang-tidy crashes on CodegenDataModel_Write due to Nullable/std::optional check.
               #       See https://github.com/llvm/llvm-project/issues/97426
+              env:
+                  ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
               run: |
+                  touch out/changed_files.txt
+                  for file in ${ALL_CHANGED_FILES}; do
+                      echo "$file changed and will be considered for tidy"
+                      echo "$file" >>out/changed_files.txt
+                  done
+
                   ./scripts/run_in_build_env.sh \
                     "./scripts/run-clang-tidy-on-compile-commands.py \
                        --compile-database out/sanitizers/compile_commands.json \
                        --file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|-ReadImpl|-InvokeSubscribeImpl|CodegenDataModel_Write|QuieterReporting' \
+                       --file-list-file out/changed_files.txt \
                        check \
                     "
             - name: Clean output
@@ -382,6 +406,9 @@ jobs:
         steps:
             - name: Checkout
               uses: actions/checkout@v4
+              with:
+                  fetch-depth: 2
+                  persist-credentials: true
             - name: Checkout submodules & Bootstrap
               uses: ./.github/actions/checkout-submodules-and-bootstrap
               with:
@@ -418,14 +445,26 @@ jobs:
             - name: Ensure codegen is done for default
               run: |
                   ./scripts/run_in_build_env.sh "./scripts/run_codegen_targets.sh out/default"
+            - name: Find changed files
+              id: changed-files
+              uses: tj-actions/changed-files@v45
             - name: Clang-tidy validation
               # NOTE: clang-tidy crashes on CodegenDataModel_Write due to Nullable/std::optional check.
               #       See https://github.com/llvm/llvm-project/issues/97426
+              env:
+                  ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
               run: |
+                  touch out/changed_files.txt
+                  for file in ${ALL_CHANGED_FILES}; do
+                      echo "$file changed and will be considered for tidy"
+                      echo "$file" >>out/changed_files.txt
+                  done
+
                   ./scripts/run_in_build_env.sh \
                     "./scripts/run-clang-tidy-on-compile-commands.py \
                        --compile-database out/default/compile_commands.json \
                        --file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|CodegenDataModel_Write|QuieterReporting' \
+                       --file-list-file out/changed_files.txt \
                        check \
                     "
             - name: Uploading diagnostic logs

scripts/run-clang-tidy-on-compile-commands.py

@@ -40,6 +40,7 @@
 import tempfile
 import threading
 import traceback
+from pathlib import Path
 
 import click
 import coloredlogs
@@ -92,20 +93,19 @@ def __init__(self, json_entry, gcc_sysroot=None):
         #
         # However that seems to potentially disable a lot, so for now just filter out the
         # offending argument
-        command_items = [arg for arg in command_items if arg not in {'-c', '-S'}]
+        command_items = [arg for arg in command_items if arg not in {"-c", "-S"}]
 
         # Allow gcc/g++ invocations to also be tidied - arguments should be
         # compatible and on darwin gcc/g++ is actually a symlink to clang
-        if compiler in ['clang++', 'clang', 'gcc', 'g++']:
+        if compiler in ["clang++", "clang", "gcc", "g++"]:
             self.valid = True
             self.clang_arguments = command_items[1:]
         else:
-            logging.warning(
-                "Cannot tidy %s - not a clang compile command", self.file)
+            logging.warning("Cannot tidy %s - not a clang compile command", self.file)
             return
 
-        if compiler in ['gcc', 'g++'] and gcc_sysroot:
-            self.clang_arguments.insert(0, '--sysroot='+gcc_sysroot)
+        if compiler in ["gcc", "g++"] and gcc_sysroot:
+            self.clang_arguments.insert(0, "--sysroot=" + gcc_sysroot)
 
     @property
     def full_path(self):
@@ -122,8 +122,12 @@ def SetChecks(self, checks: str):
     def Check(self):
         logging.debug("Running tidy on %s from %s", self.file, self.directory)
         try:
-            cmd = ["clang-tidy", self.file] + \
-                self.tidy_arguments + ["--"] + self.clang_arguments
+            cmd = (
+                ["clang-tidy", self.file]
+                + self.tidy_arguments
+                + ["--"]
+                + self.clang_arguments
+            )
             logging.debug("Executing: %r" % cmd)
 
             proc = subprocess.Popen(
@@ -156,7 +160,7 @@ def Check(self):
                     "Use -system-headers to display errors from system headers as well.",
                 ]
 
-                for line in err.decode('utf-8').split('\n'):
+                for line in err.decode("utf-8").split("\n"):
                     line = line.strip()
 
                     if any(map(lambda s: s in line, skip_strings)):
@@ -165,7 +169,7 @@ def Check(self):
                     if not line:
                         continue  # no empty lines
 
-                    logging.warning('TIDY %s: %s', self.file, line)
+                    logging.warning("TIDY %s: %s", self.file, line)
 
             if proc.returncode != 0:
                 if proc.returncode < 0:
@@ -203,18 +207,20 @@ def Failure(self, path: str):
 
 
 def find_darwin_gcc_sysroot():
-    for line in subprocess.check_output('xcodebuild -sdk -version'.split()).decode('utf8').split('\n'):
-        if not line.startswith('Path: '):
+    for line in subprocess.check_output(
+        "xcodebuild -sdk -version".split(), text=True
+    ).splitlines():
+        if not line.startswith("Path: "):
             continue
-        path = line[line.find(': ')+2:]
-        if '/MacOSX.platform/' not in path:
+        path = line[line.find(": ") + 2:]
+        if "/MacOSX.platform/" not in path:
             continue
         logging.info("Found %s" % path)
         return path
 
     # A hard-coded value that works on default installations
     logging.warning("Using default platform sdk path. This may be incorrect.")
-    return '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk'
+    return "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk"
 
 
 class ClangTidyRunner:
@@ -228,13 +234,12 @@ def __init__(self):
         self.gcc_sysroot = None
         self.file_names_to_check = set()
 
-        if sys.platform == 'darwin':
+        if sys.platform == "darwin":
             # Darwin gcc invocation will auto select a system root, however clang requires an explicit path since
             # we are using the built-in pigweed clang-tidy.
-            logging.info(
-                'Searching for a MacOS system root for gcc invocations...')
+            logging.info("Searching for a MacOS system root for gcc invocations...")
             self.gcc_sysroot = find_darwin_gcc_sysroot()
-            logging.info('  Chose: %s' % self.gcc_sysroot)
+            logging.info("  Chose: %s" % self.gcc_sysroot)
 
     def AddDatabase(self, compile_commands_json):
         database = json.load(open(compile_commands_json))
@@ -245,7 +250,7 @@ def AddDatabase(self, compile_commands_json):
                 continue
 
             if item.file in self.file_names_to_check:
-                logging.info('Ignoring additional request for checking %s', item.file)
+                logging.info("Ignoring additional request for checking %s", item.file)
                 continue
 
             self.file_names_to_check.add(item.file)
@@ -270,12 +275,12 @@ def Cleanup(self):
                 # Allow all diagnostics for distinct paths to be applied
                 # at once but never again for future paths
                 for d in diagnostics:
-                    if d['DiagnosticMessage']['FilePath'] not in already_seen:
+                    if d["DiagnosticMessage"]["FilePath"] not in already_seen:
                         all_diagnostics.append(d)
 
                 # in the future assume these files were already processed
                 for d in diagnostics:
-                    already_seen.add(d['DiagnosticMessage']['FilePath'])
+                    already_seen.add(d["DiagnosticMessage"]["FilePath"])
 
             if all_diagnostics:
                 with open(self.fixes_file, "w") as out:
@@ -304,8 +309,7 @@ def ExportFixesTo(self, f):
         for idx, e in enumerate(self.entries):
             e.ExportFixesTo(
                 os.path.join(
-                    self.fixes_temporary_file_dir.name, "fixes%d.yaml" % (
-                        idx + 1,)
+                    self.fixes_temporary_file_dir.name, "fixes%d.yaml" % (idx + 1,)
                 )
             )
 
@@ -407,6 +411,12 @@ def Check(self):
     type=str,
     help="Checks to run (passed in to clang-tidy). If not set the .clang-tidy file is used.",
 )
+@click.option(
+    "--file-list-file",
+    default=None,
+    type=click.Path(exists=True),
+    help="When provided, only tidy files that match files mentioned in this file.",
+)
 @click.pass_context
 def main(
     context,
@@ -417,6 +427,7 @@ def main(
     no_log_timestamps,
     export_fixes,
     checks,
+    file_list_file,
 ):
     log_fmt = "%(asctime)s %(levelname)-7s %(message)s"
     if no_log_timestamps:
@@ -452,6 +463,14 @@ def cleanup():
         r = re.compile(file_exclude_regex)
         runner.FilterEntries(lambda e: not r.search(e.file))
 
+    if file_list_file:
+        acceptable = set()
+        with open(file_list_file, "rt") as f:
+            for file_name in f.readlines():
+                acceptable.add(Path(file_name.strip()).resolve().as_posix())
+
+        runner.FilterEntries(lambda e: e.full_path in acceptable)
+
     if export_fixes:
         runner.ExportFixesTo(export_fixes)
 
@@ -492,4 +511,4 @@ def cmd_fix(context):
 
 
 if __name__ == "__main__":
-    main(auto_envvar_prefix='CHIP')
+    main(auto_envvar_prefix="CHIP")