Copilot commented on code in PR #12667:
URL: https://github.com/apache/gluten/pull/12667#discussion_r3749981817
##
.github/workflows/velox_backend_x86.yml:
##
@@ -55,7 +55,57 @@ concurrency:
cancel-in-progress: true
jobs:
+ # Detect which parts of the codebase were modified so downstream jobs can
+ # skip work that is irrelevant to the change (e.g. a pure C++ fix does not
+ # need to run any spark-test-* job, and a Spark-3.3-shim-only change does
+ # not need to run spark-test-spark34/35/40/41).
+ #
+ # Output flags:
+ # cpp – C++ sources, Velox build scripts, or dev tooling changed
+ # java – any cross-version Java/Scala/Maven source changed
+ # shims33/34/35/40/41 – version-specific shim layer changed
+ # tools_it – tools/gluten-it changed
+ #
+ # A job runs when ANY of the relevant flags is true, so:
+ # • gluten-core change → java=true → all spark-test jobs run ✓
+ # • shims/spark35 only → shims35=true → only spark35 jobs run ✓
+ # • pure cpp change → cpp=true → only native/tpc jobs run ✓
+ detect-changes:
+runs-on: ubuntu-22.04
+outputs:
+ cpp: ${{ steps.filter.outputs.cpp }}
+ java: ${{ steps.filter.outputs.java }}
+ shims33: ${{ steps.filter.outputs.shims33 }}
+ shims34: ${{ steps.filter.outputs.shims34 }}
+ shims35: ${{ steps.filter.outputs.shims35 }}
+ shims40: ${{ steps.filter.outputs.shims40 }}
+ shims41: ${{ steps.filter.outputs.shims41 }}
+ tools_it: ${{ steps.filter.outputs.tools_it }}
+steps:
+ - uses: actions/checkout@v4
+with:
+ fetch-depth: 0
+ - name: Detect changed paths
+id: filter
+run: |
+ BASE=${{ github.event.pull_request.base.sha }}
+ HEAD=${{ github.sha }}
+ changed=$(git diff --name-only "$BASE" "$HEAD")
+ echo "$changed"
+
+ match() { echo "$changed" | grep -qE "$1" && echo true || echo
false; }
+
+ echo "cpp=$(match '^(cpp/|ep/build-velox/|dev/)')"
>> $GITHUB_OUTPUT
+ echo "java=$(match
'^(pom\.xml|backends-velox/|gluten-(uniffle|celeborn|ras|core|substrait|arrow|delta|iceberg|hudi|paimon|ut)/|package/|build/mvn)')"
>> $GITHUB_OUTPUT
Review Comment:
The `java` change-detection regex matches `gluten-ut/**` via `...|ut)/`,
which makes `java=true` even for version-scoped changes under
`gluten-ut/sparkXX/`. That defeats the intended optimization where a
`gluten-ut/spark35/`-only change should only run Spark 3.5 jobs (via `shims35`)
instead of all `spark-test-*` jobs. It also doesn’t treat
`.github/workflows/**` (including `util/` scripts in this workflow’s
`on.pull_request.paths`) as relevant, so CI-only changes can cause downstream
jobs to be skipped and the workflow changes to go unvalidated.
##
.github/workflows/velox_backend_enhanced.yml:
##
@@ -52,7 +52,36 @@ concurrency:
cancel-in-progress: true
jobs:
+ # Detect which parts of the codebase were modified so downstream jobs can
+ # skip work that is irrelevant to the change.
+ detect-changes:
+runs-on: ubuntu-22.04
+outputs:
+ cpp: ${{ steps.filter.outputs.cpp }}
+ java:${{ steps.filter.outputs.java }}
+ shims35: ${{ steps.filter.outputs.shims35 }}
+ shims40: ${{ steps.filter.outputs.shims40 }}
+steps:
+ - uses: actions/checkout@v4
+with:
+ fetch-depth: 0
+ - name: Detect changed paths
+id: filter
+run: |
+ BASE=${{ github.event.pull_request.base.sha }}
+ HEAD=${{ github.sha }}
+ changed=$(git diff --name-only "$BASE" "$HEAD")
+ echo "$changed"
+
+ match() { echo "$changed" | grep -qE "$1" && echo true || echo
false; }
+
+ echo "cpp=$(match '^(cpp/|ep/build-velox/|dev/)')"
>> $GITHUB_OUTPUT
+ echo "java=$(match
'^(pom\.xml|backends-velox/|gluten-(uniffle|celeborn|ras|core|substrait|arrow|delta|iceberg|hudi|ut)/|package/|build/mvn)')"
>> $GITHUB_OUTPUT
Review Comment:
The `java` change-detection regex matches `gluten-ut/**` via `...|ut)/`,
which makes `java=true` even for version-scoped changes under
`gluten-ut/spark35/` or `gluten-ut/spark40/`. That prevents shims-only changes
from skipping the other Spark-version job in this workflow. Also, changes to
`.github/workflows/**` aren’t considered, so edits to this workflow file can
skip the very jobs being modified.
##
.github/workflows/velox_backend_x86.yml:
##
@@ -55,7 +55,57 @@ concurrency:
cancel-in-progress: true
jobs:
+ # Detect which parts of the codebase were modified so downstream jobs can
+ # skip work that is irrelevant to the change (e.g. a pure C++ fix does not
+ # need to run any spark-test-* job, and a Spark-3.3-shim-only change does
+ # not need to run spark-test-spark34/35/40/41).
Review
