Przeglądaj źródła

Try using same_pkg_direct_rdeps for clang-tidy action (#5144)

This is trying to reduce how much we run tidy over due to poor
performance of tidy.

It's opening the door for issues where a header file is modified in a
way that introduces tidy issues in a different target. However, that's
not the typical tidy issue we see.

There may also be a risk where a source file indirectly becomes a source
file for a target in a way that `same_pkg_direct_rdeps` doesn't return,
but I'm not sure that's applicable for how we use targets.

An example of this locating a diagnostic error can be found in the "Add
tidy issue" commit's run (which I cancelled before it finished running,
but note the error):

https://github.com/carbon-language/carbon-lang/actions/runs/13932649182/job/38993348130?pr=5144
Jon Ross-Perkins 1 rok temu
rodzic
commit
24c173b10f

+ 46 - 3
.github/actions/test-setup/action.yml

@@ -13,6 +13,8 @@ inputs:
     required: true
   targets_file:
     required: true
+  use_direct_targets:
+    default: 'false'
 
 outputs:
   has_code:
@@ -90,7 +92,7 @@ runs:
     # Build and run all targets on branch pushes to ensure we always have a
     # clean tree. We don't expect this to be an interactive path and so don't
     # optimize the latency of this step.
-    - name: Compute impacted pull request targets (for push)
+    - name: Using all targets for push
       if: steps.filter.outputs.has_code == 'true' && github.event_name == 'push'
       shell: bash
       env:
@@ -102,8 +104,10 @@ runs:
     # Bazel-based diffing. This lets PRs and the merge queue have a much more
     # efficient test CI action by avoiding even enumerating (and downloading)
     # all of the unaffected Bazel targets.
-    - name: Compute impacted pull request targets
-      if: steps.filter.outputs.has_code == 'true' && github.event_name != 'push'
+    - name: Compute indirect pull request targets
+      if:
+        steps.filter.outputs.has_code == 'true' && github.event_name != 'push'
+        && inputs.use_direct_targets != 'true'
       shell: bash
       env:
         # Compute the base SHA from the different event structures.
@@ -123,3 +127,42 @@ runs:
         # no targets or there may only be non-test targets that we want to
         # build, so simply inject an explicit no-op test target.
         echo "//scripts:no_op_test" >> $TARGETS_FILE
+
+    # Run the query to generate the targets file.
+    - name: Compute direct pull request targets
+      if:
+        steps.filter.outputs.has_code == 'true' && github.event_name != 'push'
+        && inputs.use_direct_targets == 'true'
+      shell: bash
+      env:
+        GIT_BASE_SHA: ${{ inputs.base_sha }}
+        QUERY_FILE: ${{ inputs.targets_file }}.query
+        TARGETS_FILE: ${{ inputs.targets_file }}
+      run: |
+        # First fetch the relevant base into the git repository.
+        git fetch --depth=1 origin $GIT_BASE_SHA
+
+        # Generate the query file. `same_pkg_direct_rdeps` is used to try to
+        # only get targets that contain modified files as srcs or hdrs (or
+        # similar artifacts).
+        echo 'same_pkg_direct_rdeps(' > $QUERY_FILE
+
+        # Start with an uninteresting file so that we can `union` below.
+        echo '  scripts/no_op_test.py' >> $QUERY_FILE
+
+        # Use `union` to join the list of files. Add quotes to defend against
+        # spaces. Note we can filter to the intersection of Carbon extensions
+        # and the supported list at:
+        # https://github.com/erenon/bazel_clang_tidy/blob/master/clang_tidy/clang_tidy.bzl#L65
+        for f in $(
+            git diff --name-only $GIT_BASE_SHA -- '**/*.h' '**/*.cpp'); do
+          echo "  union '$f'" >> $QUERY_FILE
+        done
+
+        echo ')' >> $QUERY_FILE
+
+        # Use query because cquery doesn't support `same_pkg_direct_rdeps`.
+        ./scripts/run_bazel.py \
+          --attempts=5 \
+          query --query_file=$QUERY_FILE \
+          > $TARGETS_FILE

+ 4 - 3
.github/workflows/clang_tidy.yaml

@@ -58,6 +58,7 @@ jobs:
             github.event.merge_group.base_sha }}
           remote_cache_key: ${{ secrets.CARBON_BUILDS_GITHUB }}
           targets_file: ${{ runner.temp }}/targets
+          use_direct_targets: true
 
       # Run in the clang-tidy config. This is done as part of tests so that we
       # aren't duplicating bazel/llvm setup.
@@ -69,9 +70,9 @@ jobs:
           TARGETS_FILE: ${{ runner.temp }}/targets
         run: |
           ./scripts/run_bazel.py \
-            --attempts=5 \
-            build --config=clang-tidy -k \
-            --target_pattern_file=$TARGETS_FILE
+              --attempts=5 \
+              build --config=clang-tidy -k \
+              --target_pattern_file=$TARGETS_FILE
 
       # See "Disk space before build" in `test-setup`.
       - name: Disk space after build