diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 0f1f93fe94..c11daeef22 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -29,7 +29,7 @@ ruleset in `.asf.yaml`. That splits CI into three tiers: surfaces. Every queue-only and nightly job has a `run-*` label that opts a pull request -into it early, listed in the diagram below. The Lint Java matrix compiles +into it early, listed in the table below. The Lint Java matrix compiles Spark 3.4/3.5/4.0 on every pull request, so a shim that fails to build is caught there; only the runtime suites wait for the queue or the nightly. @@ -43,126 +43,119 @@ required one, so a red 3.4 there changes nothing. It is the next push with the label still applied that runs 3.4 under `Required Checks`, and with the queue run gone that push is the only thing that makes a 3.4 failure blocking. -Heavy jobs have no `push` tier. The queue already tested the exact tree that +Most heavy jobs have no `push` tier. The queue already tested the exact tree that lands, so re-running them on push to main would double the cost of every -merge. Two jobs are still on `push`: `docs`, because it deploys to `asf-site` -and has to run after the commit is on main, and `pr_build_linux`, because of -`actions/cache` scoping. A pull request can only restore caches saved on its -own branch or on `main`, and the queue runs on a throwaway +merge. Two routes are still on `push`: `docs`, because it deploys to `asf-site` +and has to run after the commit is on main, and `build_linux`, because of +`actions/cache` scoping. The Linux route selects `pr_build_linux_checks` and +`pr_build_linux`, with the shared `build_linux_native` producer supplying the +latter. Spark SQL and Iceberg consumers stay off on push. A pull request can +only restore caches saved on its own branch or on `main`, and the queue runs on a throwaway `gh-readonly-queue/*` branch whose caches are deleted with it. Without a push run, a `Cargo.lock` or `pom.xml` change would leave the cargo-ci, cargo-debug, Maven and TPC-H/TPC-DS caches on `main` stale until the next unrelated change. -Warming those caches is the only thing the push run is for, so on `push` the -Linux build runs in **cache-refresh-only** mode: `build-native`, -`linux-test-rust` and the two TPC-H/TPC-DS jobs, each stopping once its cache -entry is written, and nothing else. The lints, the `linux-test` matrix and -the TPC query runs are skipped, which takes the push tier from 587 -runner-minutes to about 73. Three POLICY outputs express this: `build_linux` -says whether the workflow runs at all, `build_linux_full` whether it runs the -lints and tests too, and `build_linux_all_profiles` whether the `linux-test` -matrix covers every Spark profile or only the default one. `ci.yml` folds the -second into the workflow's `cache-refresh-only` input and the third into its -`profiles` input. `dev/ci/check-ci-config.py` fails if a job is added to -`pr_build_linux.yml` without either the guard or an entry in -`CACHE_REFRESH_JOBS` naming the cache it writes. See issue #5929. +On a push to main, `build_linux` selects all three Linux workflows, but only +cache-refresh work survives their job guards: + +- `build_linux_native.yml`: `build-native` refreshes `cargo-ci`. +- `pr_build_linux_checks.yml`: `lint` gates `linux-test-rust`, which refreshes + `cargo-debug`. The Java/Scala lints, compile-only job and Celeborn checks skip. +- `pr_build_linux.yml`: the two TPC jobs populate their datasets and Maven + caches, then skip query execution. Profile selection and JVM tests skip. + +Both Linux callers receive `cache-refresh-only` and `profiles` from the same +routing outputs. `build_linux_full` enables the checks and tests; +`build_linux_all_profiles` also selects the Linux consumers for nightly and +profile-label runs. Those runs must stay out of cache-refresh mode even when +`build_linux_full` is false. `dev/ci/check-ci-config.py` checks the caller +inputs and the allowed jobs across all three workflows, so adding an +unguarded job cannot silently restore unnecessary work on every push. The profile rows of the `linux-test` matrix live in `dev/ci/linux-test-profiles.py` rather than in the workflow, because a -job-level `if:` cannot see `matrix`: the `lint` job runs the script with the +job-level `if:` cannot see `matrix`: the `prepare-matrix` job runs the script with the `profiles` input and publishes the rows as a job output that the matrix reads with `fromJSON`. Each row carries a tier, `pr` for the default build profile and `nightly` for the other four, and `check-ci-config.py` asserts that the two tiers partition the list and that the `pr` tier is exactly the default profile. ``` - pull_request | merge_group | push to main | schedule | workflow_dispatch - | - v - +-----------------------+ - | preflight | ubuntu-slim - | (RAT, prettier, | - | missing-suites, | - | actionlint) | - +-----------+-----------+ - | on success - v - +-----------------------+ - | changes | ubuntu-slim - | (compute-changes.py: | - | one boolean per | - | heavy job) | - +-----------+-----------+ - | - +-----------------------------------+-----------------------------------+ - | | | - v v v - PR + queue tier push to main only queue tier, or PR with label - --------------- ----------------- --------------------------- - pr_build_linux (+ push, cache only) docs pr_build_macos run-macos-tests - (Spark 4.1 profile only) pr_benchmark_check run-benchmark-check - delta_build_gate run-delta-build-gate - pyarrow_udf_test run-pyarrow-udf-tests - spark_4_1 run-spark-4.1-tests - label or dispatch only spark_4_1 sql_hive run-spark-4.1-hive-tests - ---------------------- iceberg_1_11 run-iceberg-tests - spark_3_4 run-spark-3.4-tests - nightly tier, or PR with label - ------------------------------ - pr_build_linux run-all-spark-profiles - (other profiles) - spark_3_5 run-spark-3.5-tests - spark_4_0 run-spark-4.0-tests - iceberg_1_8 run-iceberg-tests - iceberg_1_9 run-iceberg-tests - iceberg_1_10 run-iceberg-tests - - | | | - +-----------------------------------+-----------------------------------+ - v - +-----------------------+ - | required_checks | ubuntu-slim - | one flat name that | - | is safe to require | - +-----------+-----------+ - | schedule only, when red - v - +-----------------------+ - | nightly_report | ubuntu-slim - | opens or comments on | - | a ci-nightly-failure | - | issue | - +-----------------------+ - - reusable workflows invoked via `uses:`: - pr_build_linux.yml spark_sql_test_reusable.yml - pr_build_macos.yml iceberg_spark_test_reusable.yml - pr_benchmark_check.yml delta_build_gate.yml - docs.yaml pyarrow_udf_test.yml +pull_request | merge_group | push to main | schedule | workflow_dispatch + | + preflight + | + changes + | + +----------------------+-----------------------+ + | | | + Linux checks build_linux_native macOS / benchmark / + (independent) (if any consumer Delta / PyArrow / docs + is selected) (if selected) + | + +-------------+-------------+ + | | | + pr_build_linux spark_* iceberg_* + +Every gating job reports to required_checks. A failed scheduled run also +starts nightly_report. The table below lists each caller's event policy. ``` +`build_linux_native.yml` builds the default Linux `libcomet.so` once per run +with JDK 17, the Cargo `ci` profile, and the existing x86-64-v3/bfd flags. +Every selected Linux test, Spark SQL, and Iceberg caller waits for that producer +and receives `native-lib-linux` through its required `native-library-artifact` +input. Consumers keep their own Spark/JDK versions and download the library +into `native/target/release/`, where Maven expects it. Spark still pre-compiles +and shares its JVM test classes separately for each Spark/JDK version. + +`compute-changes.py` derives `build_linux_native` as the union of the selected +consumer outputs, after applying path and event/label policy. The workflow +reads that single output. A Spark-patch-only change therefore gets a native +build when its Spark caller is selected, even if the Linux build is not. +Documentation-only changes, benchmark-only changes, and unrelated label +events do not start an unused native build. The event-selection regression +test checks that the producer and its consumers stay in agreement across PR, +merge-group, push, nightly, and manual runs. A macOS-only or benchmark-only label run +also skips this producer because neither job consumes the Linux artifact. + +Linux lint, compile-only checks, Celeborn compatibility tests, and Rust debug +tests run in `pr_build_linux_checks.yml` as soon as change selection completes. +They run alongside the native producer and still report results if it fails. +Only the JVM/TPC test consumers in `pr_build_linux.yml` wait for the shared +artifact. Both Linux callers use the same path and event selection. Regression +checks preserve this separation and prevent independent checks from acquiring +a native-build dependency. + +Rust formatting runs before native compilation and before the independent +Linux build/test jobs. Rust debug tests, macOS, and feature-specific workflows +continue to build their own binaries. The shared producer is the only writer +of the Linux CI-profile Cargo cache, and only saves it on pushes to `main`. + ## What runs when -| Job in `ci.yml` | Triggered by | Routing rule | -| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------- | -| `preflight` | every PR / merge group / push / schedule / dispatch / label | none (always runs) | -| `changes` | every PR / merge group / push / schedule / dispatch / label | runs `dev/ci/compute-changes.py` | -| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full`; the test matrix's non-default Spark profiles only in the nightly run **or** with `run-all-spark-profiles`, via `build_linux_all_profiles` | `dev/ci/compute-changes.py` | -| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | -| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | -| `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | -| `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | -| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | -| `spark_3_5` | nightly, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | -| `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | -| `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | -| `spark_4_0` | nightly, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | -| `iceberg_1_11` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_8` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_9` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_10` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `required_checks` | always, after every job above except `docs` | none (always runs) | -| `nightly_report` | schedule only, after `required_checks`, when it is not green | none | +| Job in `ci.yml` | Triggered by | Routing rule | +| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------- | +| `preflight` | every PR / merge group / push / schedule / dispatch / label | none (always runs) | +| `changes` | every PR / merge group / push / schedule / dispatch / label | runs `dev/ci/compute-changes.py` | +| `build_linux_native` | any selected Linux/Spark/Iceberg consumer, including nightly and profile-label runs | `dev/ci/compute-changes.py` | +| `pr_build_linux_checks` | same selection as `pr_build_linux`; cache-refresh and nightly inputs skip unnecessary checks | `dev/ci/compute-changes.py` | +| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full`; the test matrix's non-default Spark profiles only in the nightly run **or** with `run-all-spark-profiles`, via `build_linux_all_profiles` | `dev/ci/compute-changes.py` | +| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | +| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | +| `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | +| `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | +| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | +| `spark_3_5` | nightly, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | +| `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | +| `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | +| `spark_4_0` | nightly, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | +| `iceberg_1_11` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_8` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_9` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_10` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `required_checks` | always, after every job above except `docs` | none (always runs) | +| `nightly_report` | schedule only, after `required_checks`, when it is not green | none | A heavy job appears in the PR's checks list as a `skipped` entry whenever its path filter or event criteria don't match. Skipped checks count as @@ -172,12 +165,15 @@ safe to make a required check. ### Label events `ci.yml` also fires on `pull_request.types: [labeled]`, so applying -`run-spark-3.4-tests`, `run-spark-4.0-tests` or `run-iceberg-tests` starts the -job that label gates without needing a new push. GitHub cannot filter a +`run-spark-3.4-tests`, `run-spark-3.5-tests`, `run-spark-4.0-tests`, +`run-spark-4.1-tests`, `run-spark-4.1-hive-tests`, `run-all-spark-profiles`, +`run-iceberg-tests`, `run-macos-tests`, `run-delta-build-gate`, +`run-pyarrow-udf-tests`, or `run-benchmark-check` starts the jobs that label gates without needing a new +push. GitHub cannot filter a `pull_request` trigger by label name, so **every** label added to a PR starts a run, including labels that gate nothing. -Two rules keep those runs from corrupting the PR's status: +Three rules keep those runs from corrupting the PR's status: - `preflight` and `changes` carry no event guard and run every time. A job held back by `if:` still publishes a check run under its own name with conclusion @@ -228,11 +224,13 @@ commit no nightly has covered yet skips the suites that commit needs, lets the run go green, and then hands that green head to `nightly-base.py` as tomorrow's base, so the coverage is dropped for good. -With `profiles: nightly`, `pr_build_linux.yml` runs only `lint`, `build-native` -and the `linux-test` matrix; the lints, Rust tests, Spark build and TPC runs -carry `if: ${{ inputs.profiles != 'nightly' }}` because the queue and the push -run already produced those verdicts at the same commit. `check-ci-config.py` -pins that the same way it pins the cache-refresh guard. +With `profiles: nightly`, `pr_build_linux.yml` runs only `prepare-matrix` +and `linux-test`, consuming the shared native producer's library. The +independent checks workflow runs only its short Rust formatting prerequisite. +Rust debug tests, other lints and both TPC jobs carry +`if: ${{ inputs.profiles != 'nightly' }}` because they already ran at this +commit. `check-ci-config.py` pins this scope across the three Linux workflows +alongside the cache-refresh guards. A red nightly has no pull request to appear on, and GitHub only emails a scheduled run's failure to whoever last touched the workflow file, so @@ -263,6 +261,8 @@ umbrella doesn't watch, or operate independently of the rest of CI: | File | Called from `ci.yml` job(s) | | --------------------------------- | ------------------------------------------------------------ | +| `build_linux_native.yml` | `build_linux_native` | +| `pr_build_linux_checks.yml` | `pr_build_linux_checks` | | `pr_build_linux.yml` | `pr_build_linux` | | `pr_build_macos.yml` | `pr_build_macos` | | `pr_benchmark_check.yml` | `pr_benchmark_check` | @@ -274,13 +274,28 @@ umbrella doesn't watch, or operate independently of the rest of CI: ## Changing what runs when -Every heavy job in `ci.yml` is gated on exactly one thing: +Consumer jobs in `ci.yml` use their routing outputs, for example: ```yaml if: needs.changes.outputs.spark_3_5 == 'true' ``` -That single boolean folds together two separate decisions, both of which live +Spark 4.1 has separate core and Hive outputs selecting the same caller: + +```yaml +if: needs.changes.outputs.spark_4_1 == 'true' || needs.changes.outputs.spark_4_1_hive == 'true' +``` + +`NATIVE_CONSUMERS` in `dev/ci/compute-changes.py` maps each native consumer +job to all outputs that can select it. The shared native producer runs when +any of those outputs is true. A Hive-only label event has `spark_4_1=false` +and `spark_4_1_hive=true`, so it still starts the native producer. Likewise, +`build_linux_all_profiles=true` starts it when `build_linux=false`, covering +nightly runs and newly applied `run-all-spark-profiles` labels. The +configuration guard checks the exact callers, output exports, dependencies, +and gates against this mapping. + +Each routing output folds together two separate decisions, both of which live in `dev/ci/compute-changes.py`: - **`FILTERS`** — which files the job covers. Pattern semantics match @@ -320,22 +335,24 @@ a routing table in `dev/ci/check-ci-config.py`, which `preflight` runs. ## Artifact names must be unique per producer Artifact names are scoped to the workflow **run**, not to the calling -workflow. `ci.yml` calls `spark_sql_test_reusable.yml` once per Spark -version and `iceberg_spark_test_reusable.yml` once per Iceberg version, all -inside the same run, so an unqualified name like `native-lib-linux` would be -claimed by several producers at once. That breaks two things: - -- `download-artifact` resolves a name to the highest matching artifact ID. - Nothing ties it to the producer the consumer declared in `needs`. -- `upload-artifact` with `overwrite: true` deletes the newest record with - that name before uploading, which can be a sibling's finished artifact. - The retry wrapper below forces `overwrite` on attempts 2 and 3. - -So every artifact published by a reusable workflow that `ci.yml` calls more -than once carries its version inputs, e.g. -`native-lib-spark-4.1.3-jdk17`. `dev/ci/check-ci-config.py` enforces this, -and also that every `download-artifact` name is produced by an upload in the -same workflow. +workflow. `build_linux_native.yml` is called exactly once and is the sole +producer of `native-lib-linux`. Its consumers declare a required +`native-library-artifact` input; `ci.yml` passes that name and makes every +consumer depend on the shared producer. They download the existing artifact +without publishing copies under version-specific names. + +Artifacts with multiple producers still carry their version inputs. For +example, `spark_sql_test_reusable.yml` publishes +`jvm-compiled-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}` because +each Spark version has different compiled classes. Publishing two artifacts +under the same name can make a download select a sibling's artifact and let +an upload retry overwrite that sibling's output. + +`dev/ci/check-ci-config.py` verifies both contracts: local uploads and +downloads must match, and shared native-library consumers must be wired to +the one declared producer. Artifact retention remains one day; a failed-job +rerun can reuse a successful producer's artifact during that retention +window. If it has expired, rerun the full workflow to rebuild it. ## Retrying flaky network operations @@ -409,6 +426,12 @@ Any job whose first Maven use is a bare `./mvnw` needs this step before it. the composite, because a local action invoking another local action is deliberately avoided here (see the artifact-upload note above). +The independent Java lint, Spark 4.1 compile, and Celeborn compatibility jobs +in `pr_build_linux_checks.yml` each bootstrap Maven, as do the two TPC jobs +in `pr_build_linux.yml`. The configuration guard scans both workflows and +requires an earlier unconditional bootstrap step for every direct `./mvnw` +command; a bootstrap that ignores failure does not satisfy the guard. + ## Merge queue `.asf.yaml` declares a `Merge Queue` ruleset for the default branch, so `main` diff --git a/.github/workflows/build_linux_native.yml b/.github/workflows/build_linux_native.yml new file mode 100644 index 0000000000..17077436cb --- /dev/null +++ b/.github/workflows/build_linux_native.yml @@ -0,0 +1,86 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Single producer for the default Linux CI native library. The umbrella runs +# this whenever any Linux, Spark SQL, or Iceberg test consumer is selected. +name: Build Linux Native Library + +on: + workflow_call: + +permissions: + contents: read + +env: + RUST_VERSION: stable + RUST_BACKTRACE: 1 + +jobs: + # Build native library once and share with all test jobs + build-native: + name: Build Native Library + runs-on: ubuntu-24.04 + container: + image: amd64/rust + steps: + - uses: actions/checkout@v7 + - name: Setup Rust toolchain + uses: ./.github/actions/setup-builder + with: + rust-version: ${{ env.RUST_VERSION }} + jdk-version: 17 # Matches the existing Linux producer, including libjvm linkage. + + - name: Check Rust formatting + run: cd native && cargo fmt --all -- --check + + - name: Restore Cargo cache + uses: actions/cache/restore@v6 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + native/target + key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} + restore-keys: | + ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- + + - name: Build native library (CI profile) + run: | + cd native + # CI profile: same overflow behavior as release, but faster compilation + # (no LTO, parallel codegen) + cargo build --profile ci + env: + RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" + + - name: Upload native library + uses: ./.github/actions/upload-artifact-retry + with: + name: native-lib-linux + path: native/target/ci/libcomet.so + retention-days: 1 + + - name: Save Cargo cache + uses: actions/cache/save@v6 + # Nightly runs reuse the cache already refreshed by the push run. + if: github.event_name == 'push' + with: + path: | + ~/.cargo/registry + ~/.cargo/git + native/target + key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5bdde5aa6..36521ba699 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,7 +135,10 @@ jobs: run: python3 dev/ci/test-iceberg-shards.py - name: Check CI config invariants - run: python3 dev/ci/check-ci-config.py + run: | + python3 dev/ci/check-ci-config.py + python3 dev/ci/test-ci-config.py + python3 dev/ci/test-native-build-selection.py - name: Install actionlint # Pure network, and preflight gates every other job, so a single reset @@ -178,6 +181,7 @@ jobs: actions: read contents: read outputs: + build_linux_native: ${{ steps.compute.outputs.build_linux_native }} build_linux: ${{ steps.compute.outputs.build_linux }} build_linux_full: ${{ steps.compute.outputs.build_linux_full }} build_linux_all_profiles: ${{ steps.compute.outputs.build_linux_all_profiles }} @@ -278,15 +282,37 @@ jobs: # can actually test it. # --------------------------------------------------------------------------- + # Independent checks start immediately after change selection. + pr_build_linux_checks: + name: PR Checks (Linux) + needs: changes + permissions: + contents: read + if: needs.changes.outputs.build_linux == 'true' || needs.changes.outputs.build_linux_all_profiles == 'true' + uses: ./.github/workflows/pr_build_linux_checks.yml + with: + cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' && needs.changes.outputs.build_linux_all_profiles != 'true' }} + profiles: >- + ${{ needs.changes.outputs.build_linux_all_profiles != 'true' && 'pr' + || needs.changes.outputs.build_linux_full != 'true' && 'nightly' + || 'all' }} + + # compute-changes.py derives this output from the selected native consumers. + build_linux_native: + name: Shared Linux Native Library + needs: changes + permissions: + contents: read + if: needs.changes.outputs.build_linux_native == 'true' + uses: ./.github/workflows/build_linux_native.yml + pr_build_linux: name: PR Build (Linux) - needs: changes - # Three POLICY outputs feed one call, the same shape as spark_4_1 below. - # `build_linux` decides whether the workflow runs at all; `build_linux_full` - # whether it runs the lints and the test matrix as well as the jobs that - # populate main's actions/cache entries; `build_linux_all_profiles` whether - # the test matrix covers every Spark profile or only the default one. The - # combinations that occur: + needs: [changes, build_linux_native] + # Three POLICY outputs configure both Linux callers above and here. + # `build_linux` or `build_linux_all_profiles` selects them; `build_linux_full` + # enables the independent checks and tests beyond main's cache writers. + # Together with `build_linux_all_profiles`, it selects the test profiles: # # pull request linux, full -> profiles: pr # ... with the label linux, full, all -> profiles: all @@ -302,6 +328,7 @@ jobs: if: needs.changes.outputs.build_linux == 'true' || needs.changes.outputs.build_linux_all_profiles == 'true' uses: ./.github/workflows/pr_build_linux.yml with: + native-library-artifact: native-lib-linux cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' && needs.changes.outputs.build_linux_all_profiles != 'true' }} profiles: >- ${{ needs.changes.outputs.build_linux_all_profiles != 'true' && 'pr' @@ -351,7 +378,7 @@ jobs: spark_3_4: name: Spark SQL Tests (Spark 3.4) - needs: changes + needs: [changes, build_linux_native] # Spark 3.4 is deprecated, so this is the one test job the merge queue does # not run: it needs the `run-spark-3.4-tests` label on a pull request, or a # manual workflow_dispatch. @@ -368,36 +395,39 @@ jobs: if: needs.changes.outputs.spark_3_4 == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: + native-library-artifact: native-lib-linux spark-short: '3.4' spark-full: '3.4.3' java: 17 spark_3_5: name: Spark SQL Tests (Spark 3.5) - needs: changes + needs: [changes, build_linux_native] # Nightly by default; PRs need the `run-spark-3.5-tests` label. if: needs.changes.outputs.spark_3_5 == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: + native-library-artifact: native-lib-linux spark-short: '3.5' spark-full: '3.5.9' java: 17 spark_4_0: name: Spark SQL Tests (Spark 4.0) - needs: changes + needs: [changes, build_linux_native] # Nightly by default; PRs need the `run-spark-4.0-tests` label. Spark 4.1 # is the one Spark SQL suite the queue runs, being the default profile. if: needs.changes.outputs.spark_4_0 == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: + native-library-artifact: native-lib-linux spark-short: '4.0' spark-full: '4.0.4' java: 17 spark_4_1: name: Spark SQL Tests (Spark 4.1) - needs: changes + needs: [changes, build_linux_native] # Queue-only by default, like every other Spark SQL suite. Two POLICY # outputs feed one call, so the queue gets every module from a single # 40-minute build instead of two. `spark_4_1` covers catalyst and the @@ -407,6 +437,7 @@ jobs: if: needs.changes.outputs.spark_4_1 == 'true' || needs.changes.outputs.spark_4_1_hive == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: + native-library-artifact: native-lib-linux spark-short: '4.1' spark-full: '4.1.3' java: 17 @@ -417,11 +448,12 @@ jobs: iceberg_1_8: name: Iceberg Spark SQL Tests (Iceberg 1.8) - needs: changes + needs: [changes, build_linux_native] # Nightly by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_8 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: + native-library-artifact: native-lib-linux iceberg-short: '1.8' iceberg-full: '1.8.1' spark-short: '3.4' @@ -430,11 +462,12 @@ jobs: iceberg_1_9: name: Iceberg Spark SQL Tests (Iceberg 1.9) - needs: changes + needs: [changes, build_linux_native] # Nightly by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_9 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: + native-library-artifact: native-lib-linux iceberg-short: '1.9' iceberg-full: '1.9.1' spark-short: '3.5' @@ -443,11 +476,12 @@ jobs: iceberg_1_10: name: Iceberg Spark SQL Tests (Iceberg 1.10) - needs: changes + needs: [changes, build_linux_native] # Nightly by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_10 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: + native-library-artifact: native-lib-linux iceberg-short: '1.10' iceberg-full: '1.10.0' spark-short: '3.5' @@ -456,13 +490,14 @@ jobs: iceberg_1_11: name: Iceberg Spark SQL Tests (Iceberg 1.11) - needs: changes + needs: [changes, build_linux_native] # Queue-only by default; PRs need the `run-iceberg-tests` label. Iceberg # 1.11 is our only Spark 4.1 Iceberg coverage, which is why it is the one # Iceberg version the queue runs while 1.8/1.9/1.10 run nightly. if: needs.changes.outputs.iceberg_1_11 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: + native-library-artifact: native-lib-linux iceberg-short: '1.11' iceberg-full: '1.11.0' spark-short: '4.1' @@ -508,6 +543,8 @@ jobs: needs: - preflight - changes + - pr_build_linux_checks + - build_linux_native - pr_build_linux - pr_build_macos - pr_benchmark_check diff --git a/.github/workflows/iceberg_spark_test_reusable.yml b/.github/workflows/iceberg_spark_test_reusable.yml index ae20b6d768..8235d55fcf 100644 --- a/.github/workflows/iceberg_spark_test_reusable.yml +++ b/.github/workflows/iceberg_spark_test_reusable.yml @@ -24,6 +24,10 @@ name: Iceberg Spark SQL Tests (reusable) on: workflow_call: inputs: + native-library-artifact: + description: 'Default Linux CI library produced by the calling workflow' + required: true + type: string iceberg-short: description: 'Iceberg minor version, e.g. 1.10' required: true @@ -59,72 +63,24 @@ env: RUSTFLAGS: "-Clink-arg=-fuse-ld=bfd" jobs: - # Build native library once and share with all test jobs - build-native: - name: Build Native Library - runs-on: ubuntu-24.04 + # The calling workflow has already built the shared native library. + prepare-shards: + name: Define Iceberg test shards + runs-on: ubuntu-slim outputs: shard-matrix: ${{ steps.shards.outputs.matrix }} shard-count: ${{ steps.shards.outputs.count }} - container: - image: amd64/rust steps: - uses: actions/checkout@v7 - - - name: Setup Rust & Java toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: 17 - - name: Define Iceberg test shards id: shards run: python3 dev/ci/check-iceberg-shards.py --github-output "$GITHUB_OUTPUT" - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library - # Use CI profile for faster builds (no LTO) and to share cache with pr_build_linux.yml. - run: | - cd native && cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" - - - name: Save Cargo cache - uses: actions/cache/save@v6 - if: github.ref == 'refs/heads/main' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - - - name: Upload native library - uses: ./.github/actions/upload-artifact-retry - with: - # Version-qualified: ci.yml calls this workflow once per Iceberg - # version inside a single run, and artifact names are scoped to the - # run, not to the calling workflow. See "Artifact names must be - # unique per producer" in .github/workflows/README.md. - name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} - path: native/target/ci/libcomet.so - retention-days: 1 - iceberg-spark: - needs: build-native + needs: prepare-shards strategy: fail-fast: false - matrix: ${{ fromJSON(needs.build-native.outputs.shard-matrix) }} + matrix: ${{ fromJSON(needs.prepare-shards.outputs.shard-matrix) }} name: iceberg-spark/iceberg-${{ inputs.iceberg-full }}/spark-${{ inputs.spark-full }}/scala-${{ inputs.scala }}/java-${{ inputs.java }}/shard-${{ matrix.shard }} runs-on: ubuntu-24.04 container: @@ -141,7 +97,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} + name: ${{ inputs.native-library-artifact }} path: native/target/release/ - name: Build Comet run: | @@ -158,7 +114,7 @@ jobs: :iceberg-spark:iceberg-spark-${{ inputs.spark-short }}_${{ inputs.scala }}:test \ --init-script ../dev/ci/iceberg-test-shards.gradle \ -PcometShardTask=:iceberg-spark:iceberg-spark-${{ inputs.spark-short }}_${{ inputs.scala }}:test \ - -PcometShardIndex=${{ matrix.shard }} -PcometShardCount=${{ needs.build-native.outputs.shard-count }} \ + -PcometShardIndex=${{ matrix.shard }} -PcometShardCount=${{ needs.prepare-shards.outputs.shard-count }} \ -Pquick=true -x javadoc - name: Upload Iceberg shard inventory and test reports if: ${{ !cancelled() }} @@ -192,7 +148,7 @@ jobs: --task :iceberg-spark:iceberg-spark-${{ inputs.spark-short }}_${{ inputs.scala }}:test iceberg-spark-extensions: - needs: build-native + needs: prepare-shards name: iceberg-spark-extensions/iceberg-${{ inputs.iceberg-full }}/spark-${{ inputs.spark-full }}/scala-${{ inputs.scala }}/java-${{ inputs.java }} runs-on: ubuntu-24.04 container: @@ -209,7 +165,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} + name: ${{ inputs.native-library-artifact }} path: native/target/release/ - name: Build Comet run: | @@ -227,7 +183,7 @@ jobs: -Pquick=true -x javadoc iceberg-spark-runtime: - needs: build-native + needs: prepare-shards name: iceberg-spark-runtime/iceberg-${{ inputs.iceberg-full }}/spark-${{ inputs.spark-full }}/scala-${{ inputs.scala }}/java-${{ inputs.java }} runs-on: ubuntu-24.04 container: @@ -244,7 +200,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} + name: ${{ inputs.native-library-artifact }} path: native/target/release/ - name: Build Comet run: | diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index 58422f40ab..e35991a321 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -17,49 +17,25 @@ name: PR Build (Linux) -# Reusable: invoked by ci.yml. Triggering, path filters, and concurrency -# live in the umbrella workflow. +# Reusable Linux JVM/TPC consumers of the shared native library. Triggering, +# path filters, and concurrency live in ci.yml. Independent checks live in +# pr_build_linux_checks.yml and the cargo-ci cache writer in build_linux_native.yml. # -# Two modes, selected by `cache-refresh-only`: +# `cache-refresh-only` keeps only the two TPC jobs, stopping after data generation +# and Maven cache population. The queue already tested the tree on a main push. # -# false the full pipeline: the lints, the linux-test matrix, the Rust -# tests and the TPC-H/TPC-DS correctness runs. Pull requests and the -# merge queue. -# true only the jobs that write an actions/cache entry, and each only far -# enough to populate it. This is the push-to-main tier, where the -# queue has already tested the exact tree that landed and the sole -# remaining purpose of the run is to leave main's caches warm for the -# next pull request. See the `build_linux` / `build_linux_full` -# comments in dev/ci/compute-changes.py for why that tier exists. -# -# Within the full pipeline, `profiles` picks which Spark profiles the -# linux-test matrix runs the Comet test suites against: `pr` is the default -# build profile alone, `all` is every profile, and `nightly` is the rest. -# The rows live in dev/ci/linux-test-profiles.py, which the `lint` job runs so -# that the matrix can be built from its output. A pull request and the merge -# queue run `pr`, the nightly run `nightly`, and a pull request carrying -# `run-all-spark-profiles` runs `all`. See the `build_linux_all_profiles` -# comment in dev/ci/compute-changes.py. -# -# `nightly` is also a third mode. Whenever ci.yml passes it, the default -# profile's pipeline has already run at this commit (in the queue and the push -# run for the scheduled event, in the PR tier for a label run), so only `lint`, -# `build-native` and the `linux-test` matrix run; every other job carries -# `if: ${{ inputs.profiles != 'nightly' }}`. -# -# Keeping all three modes in this file is deliberate. The cache keys are the -# whole point of the push tier, and a separate cache-warming workflow would -# have to repeat every one of them; here they cannot drift apart. -# dev/ci/check-ci-config.py pins which jobs survive `cache-refresh-only` and -# which run under `profiles: nightly`, so a job added without either guard -# cannot quietly put the full pipeline back on those tiers. +# `profiles` selects the linux-test rows from dev/ci/linux-test-profiles.py: +# `pr` runs the default profile, `nightly` the other four, and `all` every profile. +# The prepare-matrix job publishes those rows independently of the moved lint job. +# In nightly mode the TPC jobs are skipped because they already ran at this commit. +# dev/ci/check-ci-config.py checks both modes across all three Linux workflows. on: workflow_call: inputs: cache-refresh-only: description: >- - Populate main's actions/cache entries without running any lint or - test. Set by ci.yml on push to main. + Populate main's TPC dataset and Maven caches without running query + passes or the JVM test matrix. Set by ci.yml on push to main. type: boolean default: false profiles: @@ -71,6 +47,10 @@ on: description: 'Which Spark profiles linux-test runs: all, pr (the default build profile) or nightly (the rest)' type: string default: 'all' + native-library-artifact: + description: 'Default Linux CI library produced by the calling workflow' + required: true + type: string env: RUST_VERSION: stable @@ -82,345 +62,22 @@ env: jobs: - # Fast lint check - gates all other jobs. Runs in cache-refresh-only mode too: - # it is 40 seconds, and `build-native` and `linux-test-rust` both `needs:` it, - # so skipping it would skip the two jobs the push tier exists to run. - lint: - name: Lint - runs-on: ubuntu-24.04 - container: - image: amd64/rust + # Select profiles independently of the checks workflow and shared native build. + prepare-matrix: + name: Select Linux test profiles + if: ${{ !inputs.cache-refresh-only }} + runs-on: ubuntu-slim outputs: - # The `profile` dimension of the linux-test matrix, selected by the - # `profiles` input. Computed here because this job already gates - # linux-test and a job-level `if:` cannot see `matrix`. profile-matrix: ${{ steps.profiles.outputs.matrix }} steps: - uses: actions/checkout@v7 - - name: Select linux-test profiles id: profiles run: python3 dev/ci/linux-test-profiles.py --profiles "${{ inputs.profiles }}" --github-output "$GITHUB_OUTPUT" - - name: Check Rust formatting - run: | - rustup component add rustfmt - cd native && cargo fmt --all -- --check - - # Fast syntactic-only scalafix check. Parses sources without compiling, so it - # surfaces version-independent style issues (e.g. RedundantSyntax) in seconds, - # long before the lint-java matrix finishes its ~3.5 min build. It also scans - # the spark-4.1 / spark-4.2 sources that lint-java skips (those profiles can't - # run -Psemanticdb yet), so it is the only gate covering them. The full rule - # set, including the semantic rules, still runs in lint-java. - scalafix-syntactic: - name: Lint Scala (syntactic) - if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} - runs-on: ubuntu-24.04 - steps: - - uses: actions/checkout@v7 - - - name: Setup coursier - uses: coursier/setup-action@v3 - with: - jvm: temurin:21 - - # Split the network from the lint. `cs launch` resolves and downloads the - # scalafix artifacts from Maven Central, which resets connections now and - # then, so the warm-up retries a no-op `--version` run. The real check then - # runs `--mode offline` against the populated cache: no network, and a - # nonzero exit can only mean a lint violation, never a download failure. - - name: Fetch scalafix - run: | - for attempt in 1 2 3; do - if cs launch scalafix:0.14.6 -- --version; then - break - fi - if [ "$attempt" = 3 ]; then - echo "::error::scalafix download failed after 3 attempts." - exit 1 - fi - echo "::warning::scalafix download failed (attempt $attempt of 3); retrying in $((attempt * 15))s." - sleep $((attempt * 15)) - done - - - name: Run syntactic scalafix check (no compile) - run: | - cs launch --mode offline scalafix:0.14.6 -- \ - --check \ - --syntactic \ - --config .scalafix-syntactic.conf \ - --exclude '**/target/**' \ - spark - - lint-java: - needs: lint - name: Lint Java (${{ matrix.profile.name }}) - if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} - runs-on: ubuntu-24.04 - container: - image: amd64/rust - env: - JAVA_TOOL_OPTIONS: '--add-exports=java.base/sun.nio.ch=ALL-UNNAMED --add-exports=java.base/sun.util.calendar=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED' - strategy: - matrix: - profile: - - name: "Spark 3.4, JDK 17, Scala 2.12" - java_version: "17" - maven_opts: "-Pspark-3.4 -Pscala-2.12" - - name: "Spark 3.5, JDK 17, Scala 2.12" - java_version: "17" - maven_opts: "-Pspark-3.5 -Pscala-2.12" - - name: "Spark 4.0, JDK 17" - java_version: "17" - maven_opts: "-Pspark-4.0" - - name: "Spark 4.0, JDK 21" - java_version: "21" - maven_opts: "-Pspark-4.0" - # Spark 4.1 and 4.2 are intentionally absent: the lint job invokes -Psemanticdb, - # but semanticdb-scalac for those Scala patch versions (2.13.17 / 2.13.18) is not - # yet published, so we cannot currently run scalafix against the spark-4.1 or - # spark-4.2 profiles. - fail-fast: false - steps: - - uses: actions/checkout@v7 - - - name: Setup Rust & Java toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: ${{ matrix.profile.java_version }} - - - name: Cache Maven dependencies - uses: actions/cache@v6 - with: - path: | - ~/.m2/repository - /root/.m2/repository - key: ${{ runner.os }}-java-maven-${{ hashFiles('**/pom.xml') }}-lint - restore-keys: | - ${{ runner.os }}-java-maven- - - - name: Bootstrap Maven - uses: ./.github/actions/maven-bootstrap - - - name: Run scalafix check - run: | - ./mvnw -B package -DskipTests scalafix:scalafix -Dscalafix.mode=CHECK -Psemanticdb ${{ matrix.profile.maven_opts }} - - - name: Setup Node.js - uses: actions/setup-node@v7 - with: - node-version: '24' - - - name: Install prettier - run: | - npm install -g prettier - - - name: Run prettier - run: | - npx prettier "**/*.md" --write - - - name: Mark workspace as safe for git - run: | - git config --global --add safe.directory "$GITHUB_WORKSPACE" - - - name: Check for any local git changes (such as generated docs) - run: | - ./dev/ci/check-working-tree-clean.sh - - # Compile-only verification for Spark 4.1. Tests are intentionally skipped: the spark-4.1 - # profile is currently a build target only, and several runtime/test failures are tracked - # in follow-up PRs. Excluded from lint-java because semanticdb-scalac_2.13.17 is not yet - # published and the lint job activates -Psemanticdb. - build-spark-4-1: - needs: lint - name: Build Spark 4.1, JDK 17 - if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} - runs-on: ubuntu-24.04 - container: - image: amd64/rust - steps: - - uses: actions/checkout@v7 - - - name: Setup Rust & Java toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: 17 - - - name: Cache Maven dependencies - uses: actions/cache@v6 - with: - path: | - ~/.m2/repository - /root/.m2/repository - key: ${{ runner.os }}-java-maven-${{ hashFiles('**/pom.xml') }}-spark-4.1-build - restore-keys: | - ${{ runner.os }}-java-maven- - - - name: Bootstrap Maven - uses: ./.github/actions/maven-bootstrap - - - name: Compile (skip tests) - run: ./mvnw -B install -DskipTests -Dmaven.test.skip=true -Pspark-4.1 - - celeborn-reflection-compatibility: - needs: lint - name: Celeborn ${{ matrix.celeborn_version }} reflection compatibility - if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} - runs-on: ubuntu-24.04 - container: - image: amd64/rust - env: - JAVA_TOOL_OPTIONS: --add-exports=java.base/sun.nio.ch=ALL-UNNAMED --add-exports=java.base/sun.util.calendar=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED - strategy: - fail-fast: false - matrix: - celeborn_version: ["0.6.0", "0.7.0"] - steps: - - uses: actions/checkout@v7 - - - name: Setup Rust & Java toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: 17 - - - name: Cache Maven dependencies - uses: actions/cache@v6 - with: - path: | - ~/.m2/repository - /root/.m2/repository - key: ${{ runner.os }}-java-maven-${{ hashFiles('**/pom.xml') }}-celeborn-${{ matrix.celeborn_version }} - restore-keys: | - ${{ runner.os }}-java-maven- - - - name: Bootstrap Maven - uses: ./.github/actions/maven-bootstrap - - - name: Verify reflected Celeborn internals - env: - SPARK_LOCAL_HOSTNAME: localhost - SPARK_LOCAL_IP: 127.0.0.1 - run: | - SPARK_HOME="$GITHUB_WORKSPACE" ./mvnw -B clean test \ - -Pspark-3.5,scala-2.12,celeborn-reflection-compatibility \ - -Dceleborn.version="${{ matrix.celeborn_version }}" \ - -Dtest=none \ - -Dsuites=org.apache.comet.shuffle.CelebornReflectionCompatibilitySuite \ - -DfailIfNoTests=false - - # Build native library once and share with all test jobs. Also the owner of - # main's `cargo-ci` cache entry, which is why it runs in cache-refresh-only - # mode: rebuilding native/target from cold costs every pull request ~20 min. - build-native: - needs: lint - name: Build Native Library - runs-on: ubuntu-24.04 - container: - image: amd64/rust - steps: - - uses: actions/checkout@v7 - - name: Setup Rust toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: 17 # JDK only needed for JVM module proto generation - - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library (CI profile) - run: | - cd native - # CI profile: same overflow behavior as release, but faster compilation - # (no LTO, parallel codegen) - cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" - - - name: Upload native library - uses: ./.github/actions/upload-artifact-retry - with: - name: native-lib-linux - path: native/target/ci/libcomet.so - retention-days: 1 - - - name: Save Cargo cache - uses: actions/cache/save@v6 - # The push run is the cache warmer (see the header); a scheduled run - # at the same sha would only re-archive an entry that already exists. - if: github.event_name == 'push' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - - # Run Rust tests (runs in parallel with build-native, uses debug builds). - # Owns main's `cargo-debug` cache entry, so it runs in cache-refresh-only mode - # as well. The tests themselves are a small slice of its runtime -- almost all - # of it is the compile that populates the cache -- so there is nothing to gain - # from a compile-only variant, and running them keeps the cache contents - # identical to what a pull request restores. - linux-test-rust: - needs: lint - name: ubuntu-latest/rust-test - if: ${{ inputs.profiles != 'nightly' }} - runs-on: ubuntu-24.04 - container: - image: amd64/rust - steps: - - uses: actions/checkout@v7 - - - name: Setup Rust & Java toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: 17 - - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - # Note: Java version intentionally excluded - Rust target is JDK-independent - key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Rust test steps - uses: ./.github/actions/rust-test - - - name: Save Cargo cache - uses: actions/cache/save@v6 - # The push run is the cache warmer (see the header); a scheduled run - # at the same sha would only re-archive an entry that already exists. - if: github.event_name == 'push' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - + # Native artifact readiness and Rust formatting are checked by the shared producer. linux-test: - # `lint` is already upstream via build-native; it is listed here so this - # job can read its `profile-matrix` output. - needs: [lint, build-native] + needs: prepare-matrix if: ${{ !inputs.cache-refresh-only }} strategy: matrix: @@ -429,7 +86,7 @@ jobs: # pull request and in the merge queue, the other four in the nightly # run. The goal of the full list is coverage of every Java, Scala and # Spark version without testing every combination. - profile: ${{ fromJSON(needs.lint.outputs.profile-matrix) }} + profile: ${{ fromJSON(needs.prepare-matrix.outputs.profile-matrix) }} # Suites are grouped by functional area into balanced buckets so that no test # job runs much longer than ~23 min. See # docs/superpowers/specs/2026-05-22-pr-build-consolidation-design.md for the @@ -592,7 +249,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-linux + name: ${{ inputs.native-library-artifact }} # Download to release/ since Maven's -Prelease expects libcomet.so there path: native/target/release/ @@ -622,7 +279,6 @@ jobs: # the query run is skipped there; everything up to and including data # generation still has to happen for the dataset cache to exist. verify-benchmark-results-tpch: - needs: build-native name: Verify TPC-H Results if: ${{ inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 @@ -642,7 +298,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-linux + name: ${{ inputs.native-library-artifact }} path: native/target/release/ - name: Cache Maven dependencies @@ -687,7 +343,6 @@ jobs: # there the three query passes are skipped and only the dataset generation and # the Maven cache remain. verify-benchmark-results-tpcds: - needs: build-native name: Verify TPC-DS Results if: ${{ inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 @@ -707,7 +362,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-linux + name: ${{ inputs.native-library-artifact }} path: native/target/release/ - name: Cache Maven dependencies diff --git a/.github/workflows/pr_build_linux_checks.yml b/.github/workflows/pr_build_linux_checks.yml new file mode 100644 index 0000000000..25ed419fc9 --- /dev/null +++ b/.github/workflows/pr_build_linux_checks.yml @@ -0,0 +1,309 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: PR Checks (Linux) + +# Independent lint, compilation, compatibility, and Rust debug checks. +# Run alongside the shared native producer; these jobs do not consume its artifact. +# Triggering and path filters live in the umbrella workflow. +on: + workflow_call: + inputs: + cache-refresh-only: + description: 'Refresh the Rust debug cache without repeating the other checks' + type: boolean + default: false + profiles: + description: 'Linux profile selection; nightly skips checks already run at this commit' + type: string + default: 'all' + +permissions: + contents: read + +env: + RUST_VERSION: stable + RUST_BACKTRACE: 1 + # Force GNU ld on Linux: recent Rust stable defaults to rust-lld on + # x86_64-unknown-linux-gnu, and rust-lld cannot resolve -ljvm against the + # Zulu JDK layout installed by setup-java. Keep bfd for all cargo invocations. + RUSTFLAGS: "-Clink-arg=-fuse-ld=bfd" + +jobs: + + # Keep formatting in cache-refresh mode: it gates the Rust debug cache writer. + lint: + name: Lint + runs-on: ubuntu-24.04 + container: + image: amd64/rust + steps: + - uses: actions/checkout@v7 + + - name: Check Rust formatting + run: | + rustup component add rustfmt + cd native && cargo fmt --all -- --check + + # Fast syntactic-only scalafix check. Parses sources without compiling, so it + # surfaces version-independent style issues (e.g. RedundantSyntax) in seconds, + # long before the lint-java matrix finishes its ~3.5 min build. It also scans + # the spark-4.1 / spark-4.2 sources that lint-java skips (those profiles can't + # run -Psemanticdb yet), so it is the only gate covering them. The full rule + # set, including the semantic rules, still runs in lint-java. + scalafix-syntactic: + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} + name: Lint Scala (syntactic) + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v7 + + - name: Setup coursier + uses: coursier/setup-action@v3 + with: + jvm: temurin:21 + + # Split the network from the lint. `cs launch` resolves and downloads the + # scalafix artifacts from Maven Central, which resets connections now and + # then, so the warm-up retries a no-op `--version` run. The real check then + # runs `--mode offline` against the populated cache: no network, and a + # nonzero exit can only mean a lint violation, never a download failure. + - name: Fetch scalafix + run: | + for attempt in 1 2 3; do + if cs launch scalafix:0.14.6 -- --version; then + break + fi + if [ "$attempt" = 3 ]; then + echo "::error::scalafix download failed after 3 attempts." + exit 1 + fi + echo "::warning::scalafix download failed (attempt $attempt of 3); retrying in $((attempt * 15))s." + sleep $((attempt * 15)) + done + + - name: Run syntactic scalafix check (no compile) + run: | + cs launch --mode offline scalafix:0.14.6 -- \ + --check \ + --syntactic \ + --config .scalafix-syntactic.conf \ + --exclude '**/target/**' \ + spark + + lint-java: + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} + needs: lint + name: Lint Java (${{ matrix.profile.name }}) + runs-on: ubuntu-24.04 + container: + image: amd64/rust + env: + JAVA_TOOL_OPTIONS: '--add-exports=java.base/sun.nio.ch=ALL-UNNAMED --add-exports=java.base/sun.util.calendar=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED' + strategy: + matrix: + profile: + - name: "Spark 3.4, JDK 17, Scala 2.12" + java_version: "17" + maven_opts: "-Pspark-3.4 -Pscala-2.12" + - name: "Spark 3.5, JDK 17, Scala 2.12" + java_version: "17" + maven_opts: "-Pspark-3.5 -Pscala-2.12" + - name: "Spark 4.0, JDK 17" + java_version: "17" + maven_opts: "-Pspark-4.0" + - name: "Spark 4.0, JDK 21" + java_version: "21" + maven_opts: "-Pspark-4.0" + # Spark 4.1 and 4.2 are intentionally absent: the lint job invokes -Psemanticdb, + # but semanticdb-scalac for those Scala patch versions (2.13.17 / 2.13.18) is not + # yet published, so we cannot currently run scalafix against the spark-4.1 or + # spark-4.2 profiles. + fail-fast: false + steps: + - uses: actions/checkout@v7 + + - name: Setup Rust & Java toolchain + uses: ./.github/actions/setup-builder + with: + rust-version: ${{ env.RUST_VERSION }} + jdk-version: ${{ matrix.profile.java_version }} + + - name: Cache Maven dependencies + uses: actions/cache@v6 + with: + path: | + ~/.m2/repository + /root/.m2/repository + key: ${{ runner.os }}-java-maven-${{ hashFiles('**/pom.xml') }}-lint + restore-keys: | + ${{ runner.os }}-java-maven- + + - name: Bootstrap Maven + uses: ./.github/actions/maven-bootstrap + + - name: Run scalafix check + run: | + ./mvnw -B package -DskipTests scalafix:scalafix -Dscalafix.mode=CHECK -Psemanticdb ${{ matrix.profile.maven_opts }} + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '24' + + - name: Install prettier + run: | + npm install -g prettier + + - name: Run prettier + run: | + npx prettier "**/*.md" --write + + - name: Mark workspace as safe for git + run: | + git config --global --add safe.directory "$GITHUB_WORKSPACE" + + - name: Check for any local git changes (such as generated docs) + run: | + ./dev/ci/check-working-tree-clean.sh + + # Compile-only verification for Spark 4.1. Tests are intentionally skipped: the spark-4.1 + # profile is currently a build target only, and several runtime/test failures are tracked + # in follow-up PRs. Excluded from lint-java because semanticdb-scalac_2.13.17 is not yet + # published and the lint job activates -Psemanticdb. + build-spark-4-1: + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} + needs: lint + name: Build Spark 4.1, JDK 17 + runs-on: ubuntu-24.04 + container: + image: amd64/rust + steps: + - uses: actions/checkout@v7 + + - name: Setup Rust & Java toolchain + uses: ./.github/actions/setup-builder + with: + rust-version: ${{ env.RUST_VERSION }} + jdk-version: 17 + + - name: Cache Maven dependencies + uses: actions/cache@v6 + with: + path: | + ~/.m2/repository + /root/.m2/repository + key: ${{ runner.os }}-java-maven-${{ hashFiles('**/pom.xml') }}-spark-4.1-build + restore-keys: | + ${{ runner.os }}-java-maven- + + - name: Bootstrap Maven + uses: ./.github/actions/maven-bootstrap + + - name: Compile (skip tests) + run: ./mvnw -B install -DskipTests -Dmaven.test.skip=true -Pspark-4.1 + + celeborn-reflection-compatibility: + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} + needs: lint + name: Celeborn ${{ matrix.celeborn_version }} reflection compatibility + runs-on: ubuntu-24.04 + container: + image: amd64/rust + env: + JAVA_TOOL_OPTIONS: --add-exports=java.base/sun.nio.ch=ALL-UNNAMED --add-exports=java.base/sun.util.calendar=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED + strategy: + fail-fast: false + matrix: + celeborn_version: ["0.6.0", "0.7.0"] + steps: + - uses: actions/checkout@v7 + + - name: Setup Rust & Java toolchain + uses: ./.github/actions/setup-builder + with: + rust-version: ${{ env.RUST_VERSION }} + jdk-version: 17 + + - name: Cache Maven dependencies + uses: actions/cache@v6 + with: + path: | + ~/.m2/repository + /root/.m2/repository + key: ${{ runner.os }}-java-maven-${{ hashFiles('**/pom.xml') }}-celeborn-${{ matrix.celeborn_version }} + restore-keys: | + ${{ runner.os }}-java-maven- + + - name: Bootstrap Maven + uses: ./.github/actions/maven-bootstrap + + - name: Verify reflected Celeborn internals + env: + SPARK_LOCAL_HOSTNAME: localhost + SPARK_LOCAL_IP: 127.0.0.1 + run: | + SPARK_HOME="$GITHUB_WORKSPACE" ./mvnw -B clean test \ + -Pspark-3.5,scala-2.12,celeborn-reflection-compatibility \ + -Dceleborn.version="${{ matrix.celeborn_version }}" \ + -Dtest=none \ + -Dsuites=org.apache.comet.shuffle.CelebornReflectionCompatibilitySuite \ + -DfailIfNoTests=false + + # Rust tests use a separate debug build and refresh main's cargo-debug cache. + # Keep them in cache-refresh mode; compilation dominates their runtime. + linux-test-rust: + if: ${{ inputs.profiles != 'nightly' }} + needs: lint + name: ubuntu-latest/rust-test + runs-on: ubuntu-24.04 + container: + image: amd64/rust + steps: + - uses: actions/checkout@v7 + + - name: Setup Rust & Java toolchain + uses: ./.github/actions/setup-builder + with: + rust-version: ${{ env.RUST_VERSION }} + jdk-version: 17 + + - name: Restore Cargo cache + uses: actions/cache/restore@v6 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + native/target + # Note: Java version intentionally excluded - Rust target is JDK-independent + key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} + restore-keys: | + ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- + + - name: Rust test steps + uses: ./.github/actions/rust-test + + - name: Save Cargo cache + uses: actions/cache/save@v6 + # Nightly runs reuse the cache already refreshed by the push run. + if: github.event_name == 'push' + with: + path: | + ~/.cargo/registry + ~/.cargo/git + native/target + key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} diff --git a/.github/workflows/spark_sql_test_reusable.yml b/.github/workflows/spark_sql_test_reusable.yml index 9fa254f9b6..2f4576db4b 100644 --- a/.github/workflows/spark_sql_test_reusable.yml +++ b/.github/workflows/spark_sql_test_reusable.yml @@ -24,6 +24,10 @@ name: Spark SQL Tests (reusable) on: workflow_call: inputs: + native-library-artifact: + description: 'Default Linux CI library produced by the calling workflow' + required: true + type: string spark-short: description: 'Spark minor version, e.g. 3.5' required: true @@ -63,23 +67,15 @@ env: jobs: - # Build the native library AND pre-compile Spark sources + Test classes in a - # single runner, then publish two artifacts the matrix consumes: - # - native-lib-spark--jdk: libcomet.so (~50 MB) - # - jvm-compiled-spark--jdk: apache-spark.tar.gz (sources + - # target/ + Zinc state, ~500 MB-1 GB) - # Combining them avoids a second runner cold-start and an extra inter-job - # artifact round-trip for the native lib, since the JVM build already - # depends on it (the Comet Maven install bundles libcomet.so into the - # Comet JAR before SBT resolves Spark's classpath). - # - # Both names carry the Spark/JDK version because ci.yml calls this workflow - # once per Spark version inside a single run, and artifact names are scoped - # to the run, not to the calling workflow. See "Artifact names must be unique - # per producer" in .github/workflows/README.md. + # Pre-compile Spark sources and test classes once per Spark/JDK version. + # The native library comes from the umbrella's shared Linux producer; only + # the version-specific JVM artifact is published by this workflow. build: - name: Build Native + JVM Test Classes + name: Build JVM Test Classes runs-on: ubuntu-24.04 + # Preserve the container workspace and Maven-cache paths used by the test + # jobs that reuse this Spark compilation and its Zinc incremental analysis. + # This job packages the downloaded native library and only needs Java setup. container: image: amd64/rust outputs: @@ -91,55 +87,17 @@ jobs: id: modules run: python3 dev/ci/spark-sql-modules.py --modules "${{ inputs.modules }}" --github-output "$GITHUB_OUTPUT" - - name: Setup Rust & Java toolchain - uses: ./.github/actions/setup-builder - with: - rust-version: ${{ env.RUST_VERSION }} - jdk-version: ${{ inputs.java }} - - - name: Restore Cargo cache - uses: actions/cache/restore@v6 + - name: Setup Java toolchain + uses: actions/setup-java@v4 with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library (CI profile) - run: | - cd native - cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" + distribution: zulu + java-version: ${{ inputs.java }} - - name: Save Cargo cache - uses: actions/cache/save@v6 - if: github.ref == 'refs/heads/main' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - - - name: Upload native library - uses: ./.github/actions/upload-artifact-retry + - name: Download native library + uses: ./.github/actions/download-artifact-retry with: - name: native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} - path: native/target/ci/libcomet.so - retention-days: 1 - - - name: Stage native library at release path - run: | - # setup-spark-builder's `mvnw install -DskipTests` (skip-native-build - # path) bundles native/target/release/libcomet.so into the Comet JAR. - # We built with --profile ci to avoid LTO, so the file lives at - # native/target/ci/. Copy it to where the Maven build expects it. - mkdir -p native/target/release - cp native/target/ci/libcomet.so native/target/release/libcomet.so + name: ${{ inputs.native-library-artifact }} + path: native/target/release/ - name: Setup Spark uses: ./.github/actions/setup-spark-builder @@ -210,7 +168,7 @@ jobs: - name: Download native library uses: ./.github/actions/download-artifact-retry with: - name: native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} + name: ${{ inputs.native-library-artifact }} path: native/target/release/ - name: Download JVM compile artifact uses: ./.github/actions/download-artifact-retry diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index b271bc60f6..84c9c426ad 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -15,18 +15,15 @@ # specific language governing permissions and limitations # under the License. -# Guards six CI invariants that are silent when broken: +# Guards CI invariants that are silent when broken: # # 1. Change-filter routing. dev/ci/compute-changes.py decides which heavy # jobs run. A file that a job depends on but that no filter lists makes # that job skip, so the edit merges with only preflight having looked at # it. The table below pins the routing for the shared build inputs. # -# 2. Event policy. The same script decides which events may run each job. -# That used to be a `${{ }}` expression on every job in ci.yml, where it -# could not be tested; POLICY_CASES below is the test it never had. The -# expected sets are transcribed from the `if:` expressions ci.yml carried -# before the policy moved, so a regression here is a behaviour change. +# 2. Event policy. The routing script also decides which events may run +# each job; POLICY_CASES pins the behavior of the former workflow gates. # # 3. Required-check coverage. `Required Checks` in ci.yml is the job that # `.asf.yaml` can name in `required_status_checks` for main. A heavy job @@ -38,8 +35,9 @@ # GitHub keeps the most recent check run per name per commit, so a label # run publishing the required name would overwrite the real verdict. # -# 4. Artifact-name uniqueness. Artifact names are scoped to the *run*, not -# to the calling workflow, and ci.yml calls the Spark SQL and Iceberg +# 4. Artifact-name uniqueness and explicit shared-producer wiring. Names +# are scoped to the *run*, not the calling workflow. ci.yml calls the +# Spark SQL and Iceberg # reusable workflows several times in one run. Two producers sharing a # name make `download-artifact` pick by highest artifact ID rather than # by `needs`, and make the forced `overwrite` on an upload retry delete @@ -50,11 +48,16 @@ # run only under an input or a label can carry that for a long time # before anyone runs them. # -# 6. Push-tier scope. On push to main, ci.yml calls pr_build_linux.yml with -# `cache-refresh-only`, which reduces it to the jobs that write an -# actions/cache entry; the merge queue already tested that tree. A job -# added to that workflow without the guard starts running on every push -# again and nothing fails, so nothing tells you. +# 6. Linux mode scope. Both Linux callers receive `cache-refresh-only` and +# `profiles`, so push runs only refresh caches and nightly runs only test +# the non-default profiles. Per-workflow tables preserve the cache writers +# and prerequisites after the pipeline is split across three workflows. +# +# 7. Independent Linux checks. Lint, compile-only checks and debug Rust +# tests must remain runnable without waiting for the native CI build. +# +# 8. Direct Maven wrapper invocations in the two Linux workflows must run +# after the retrying bootstrap, including checks moved between them. # # Run from the repository root: python3 dev/ci/check-ci-config.py @@ -117,11 +120,19 @@ # to nothing at all and merges having been exercised by no consumer. ([".github/actions/upload-artifact-retry/action.yaml"], BUILD_JOBS), ([".github/actions/download-artifact-retry/action.yaml"], BUILD_JOBS), - # The Maven bootstrap composite is called only from pr_build_linux.yml. + # Both Linux workflows call the Maven bootstrap composite. They share the + # build_linux route, so moving a caller between them keeps this route. ( [".github/actions/maven-bootstrap/action.yaml"], {"build_linux", "build_linux_full", "build_linux_all_profiles"}, ), + # Editing the shared Linux producer must exercise every Linux consumer. + ([".github/workflows/build_linux_native.yml"], BUILD_JOBS - {"build_macos"}), + # The independent lint/compile/Rust workflow belongs only to Linux CI. + ( + [".github/workflows/pr_build_linux_checks.yml"], + {"build_linux", "build_linux_full", "build_linux_all_profiles"}, + ), # Spot checks that the additions above did not widen unrelated routes. (["docs/source/user-guide/overview.md"], {"docs"}), (["native/core/benches/parquet_read.rs"], {"benchmark"}), @@ -372,9 +383,7 @@ # `uses:` values that publish an artifact, and the one that consumes it. UPLOAD_USES = re.compile(r"uses:\s*(\./\.github/actions/upload-artifact-retry|actions/upload-artifact@)") DOWNLOAD_USES = re.compile(r"uses:\s*(\./\.github/actions/download-artifact-retry|actions/download-artifact@)") -# The artifact name is the first `name:` key of the step's `with:` block. A -# following step starts with `- `, which distinguishes it from a `with:` key. -WITH_NAME = re.compile(r"^\s+name:\s*(\S.*?)\s*$") +# A following step starts with `- `, unlike the current step's `with:` keys. NEW_STEP = re.compile(r"^\s*-\s") # A job id in a workflow file, and the two `uses:` shapes the checkout guard @@ -384,29 +393,48 @@ JOB_KEY = re.compile(r"^ ([A-Za-z0-9_-]+):\s*$") LOCAL_ACTION_USES = re.compile(r"uses:\s*(\./\.github/actions/\S+)") CHECKOUT_USES = re.compile(r"uses:\s*actions/checkout@") +SHARED_NATIVE_WORKFLOW = "build_linux_native.yml" +SHARED_NATIVE_JOB = "build_linux_native" +SHARED_NATIVE_INPUT = "native-library-artifact" +SHARED_NATIVE_ARTIFACT = "native-lib-linux" +SHARED_NATIVE_EXPRESSION = "${{ inputs.native-library-artifact }}" +SHARED_NATIVE_CONSUMERS = { + "pr_build_linux.yml", + "spark_sql_test_reusable.yml", + "iceberg_spark_test_reusable.yml", +} -# pr_build_linux.yml runs in two modes; see its header. These are the jobs that -# must survive `cache-refresh-only`, because each one writes an actions/cache -# entry that main needs warm for the next pull request. Anything else in that -# file has to carry the guard. -CACHE_REFRESH_WORKFLOW = WORKFLOWS / "pr_build_linux.yml" +# The Linux pipeline spans three workflows. Cache writers and their prerequisites +# must survive `cache-refresh-only`; every other job must carry the guard. CACHE_REFRESH_JOBS = { - "lint": "gates build-native and linux-test-rust, and costs 40 seconds", - "build-native": "writes the cargo-ci cache (native/target, CI profile)", - "linux-test-rust": "writes the cargo-debug cache (native/target, debug)", - "verify-benchmark-results-tpch": "writes the TPC-H SF=1 dataset and java-maven caches", - "verify-benchmark-results-tpcds": "writes the TPC-DS SF=1 dataset and java-maven caches", + "build_linux_native.yml": { + "build-native": "writes the cargo-ci cache (native/target, CI profile)", + }, + "pr_build_linux_checks.yml": { + "lint": "gates linux-test-rust", + "linux-test-rust": "writes the cargo-debug cache (native/target, debug)", + }, + "pr_build_linux.yml": { + "verify-benchmark-results-tpch": "writes the TPC-H SF=1 dataset and java-maven caches", + "verify-benchmark-results-tpcds": "writes the TPC-DS SF=1 dataset and java-maven caches", + }, } # Job-level `if:` only: step-level guards inside the two verify jobs are # indented further, and those are expected rather than a reason to exempt the # whole job. CACHE_REFRESH_GUARD = re.compile(r"^ if:.*!\s*inputs\.cache-refresh-only") -# The same file's third mode: with `profiles: nightly` only the jobs the -# linux-test matrix needs run. See check_nightly_scope. +# With `profiles: nightly`, only the matrix and its prerequisites run. NIGHTLY_JOBS = { - "lint": "publishes the profile matrix that linux-test reads", - "build-native": "builds the native library the matrix loads", - "linux-test": "the matrix itself", + "build_linux_native.yml": { + "build-native": "builds the native library the matrix loads", + }, + "pr_build_linux_checks.yml": { + "lint": "the short formatting prerequisite retained by the checks workflow", + }, + "pr_build_linux.yml": { + "prepare-matrix": "publishes the profile matrix that linux-test reads", + "linux-test": "the matrix itself", + }, } NIGHTLY_GUARD = re.compile(r"^ if:.*inputs\.profiles\s*!=\s*'nightly'") # The `schedule` case in ci.yml's `Detect changes` script, and what has to be @@ -434,10 +462,25 @@ def guarded_jobs(path, guard): if job and guard.match(line): guarded.add(job) return jobs, guarded -CACHE_REFRESH_INPUT = re.compile(r"^\s+cache-refresh-only:\s*\$\{\{") -# `profiles:` is passed as a folded scalar (`>-`) whose expression sits on the -# next line, so match the key alone. -PROFILES_INPUT = re.compile(r"^\s+profiles:\s*(>-|\$\{\{)") + + +LINUX_CHECKS_WORKFLOW = "pr_build_linux_checks.yml" +LINUX_CHECKS_JOB = "pr_build_linux_checks" +LINUX_MODE_CALLERS = ("pr_build_linux", LINUX_CHECKS_JOB) +CACHE_REFRESH_EXPRESSION = ( + "${{ needs.changes.outputs.build_linux_full != 'true' " + "&& needs.changes.outputs.build_linux_all_profiles != 'true' }}" +) +PROFILES_EXPRESSION = ( + "${{ needs.changes.outputs.build_linux_all_profiles != 'true' && 'pr' " + "|| needs.changes.outputs.build_linux_full != 'true' && 'nightly' || 'all' }}" +) +INDEPENDENT_LINUX_JOBS = { + "lint", "scalafix-syntactic", "lint-java", "build-spark-4-1", + "celeborn-reflection-compatibility", "linux-test-rust", +} +LINUX_MAVEN_WORKFLOWS = (LINUX_CHECKS_WORKFLOW, "pr_build_linux.yml") +MAVEN_BOOTSTRAP_ACTION = "./.github/actions/maven-bootstrap" def load_filters(): @@ -507,12 +550,7 @@ def check_linux_test_profiles(): for row in module.select("all"): if sorted(row) != ["java_version", "maven_opts", "name"]: failures.append(f"profile {row['name']!r} must carry exactly name, java_version and maven_opts") - ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8").splitlines() - if not any(PROFILES_INPUT.match(line) for line in ci): - failures.append( - "ci.yml never passes `profiles:` to pr_build_linux.yml. The input " - "defaults to all, so without it every pull request runs every profile again" - ) + failures.extend(linux_profile_failures(WORKFLOWS)) for failure in failures: print(f"linux test profiles: {failure}") return not failures @@ -580,50 +618,364 @@ def check_event_policy(): return not failures -def artifact_names(path): - """Return ([upload names], [download names]) for one workflow file.""" - uploads, downloads = [], [] +def artifact_steps(path): + """Return artifact steps and their direct `with:` inputs.""" + artifacts = [] lines = path.read_text(encoding="utf-8").splitlines() for index, line in enumerate(lines): if UPLOAD_USES.search(line): - bucket = uploads + kind = "upload" elif DOWNLOAD_USES.search(line): - bucket = downloads + kind = "download" else: continue + indent = len(line) - len(line.lstrip()) + body = [] for following in lines[index + 1:]: - if NEW_STEP.match(following): - break # step ended without a `name:`; download-all, or the default - match = WITH_NAME.match(following) - if match: - bucket.append(match.group(1)) - break - return uploads, downloads + if following.strip() and not following.lstrip().startswith("#"): + if NEW_STEP.match(following) or len(following) - len(following.lstrip()) < indent: + break + body.append(following) + text = "\n".join(body) + with_key = re.search(r"^( +)with:\s*$", text, re.MULTILINE) + if with_key: + with_indent = len(with_key.group(1)) + inputs = block_mapping(block_mapping(text, with_indent)["with"][1], with_indent + 2) + artifacts.append((kind, {key: scalar(value) for key, (value, _) in inputs.items()})) + return artifacts -def check_artifact_names(): - ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8") +def artifact_names(path): + """Return ([upload names], [download names]) for one workflow file.""" + steps = artifact_steps(path) + return tuple([inputs["name"] for kind, inputs in steps if kind == expected and "name" in inputs] + for expected in ("upload", "download")) + + +def block_mapping(text, indent): + """Read block-style keys at the workflow files' conventional indentation. + + This only inspects the small mapping subset needed by the guards below; + actionlint remains responsible for validating GitHub Actions YAML syntax. + Values are (inline value, indented body), so scalar inputs and nested + workflow/job mappings can be checked without a third-party YAML dependency. + """ + pattern = re.compile(r"^" + " " * indent + r"([\w-]+):[^\S\n]*(.*)$", re.MULTILINE) + matches = list(pattern.finditer(text)) + return { + match.group(1): (match.group(2).strip(), + text[match.end():matches[index + 1].start() + if index + 1 < len(matches) else len(text)]) + for index, match in enumerate(matches) + } + + +def scalar(value): + return value.strip().strip("\"'") + + +def linux_mode_input_failures(workflows, name, expected): + """Check each Linux caller's mode expression, including folded YAML scalars.""" + ci = (workflows / "ci.yml").read_text(encoding="utf-8") + jobs = block_mapping(block_mapping(ci, 0).get("jobs", ("", ""))[1], 2) + failures = [] + for job_id in LINUX_MODE_CALLERS: + fields = block_mapping(jobs.get(job_id, ("", ""))[1], 4) + inputs = block_mapping(fields.get("with", ("", ""))[1], 6) + value, body = inputs.get(name, ("", "")) + expression = body if value in {"|", "|-", ">", ">-"} else value + actual = " ".join(line.strip() for line in expression.splitlines() + if line.strip() and not line.lstrip().startswith("#")) + if actual != expected: + failures.append(f"ci.yml: {job_id} must pass {name}: {expected}") + return failures + + +def linux_profile_failures(workflows): + """Keep profile selection connected after splitting out the native producer.""" + failures = linux_mode_input_failures(workflows, "profiles", PROFILES_EXPRESSION) + path = workflows / "pr_build_linux.yml" + if not path.exists(): + return failures + [f"{path}: Linux test workflow is missing"] + text = path.read_text(encoding="utf-8") + jobs = block_mapping(block_mapping(text, 0).get("jobs", ("", ""))[1], 2) + prepare = jobs.get("prepare-matrix", ("", ""))[1] + outputs = block_mapping(block_mapping(prepare, 4).get("outputs", ("", ""))[1], 6) + if outputs.get("profile-matrix", ("", ""))[0] != "${{ steps.profiles.outputs.matrix }}": + failures.append(f"{path}: prepare-matrix must publish steps.profiles.outputs.matrix") + test = jobs.get("linux-test", ("", ""))[1] + strategy = block_mapping(block_mapping(test, 4).get("strategy", ("", ""))[1], 6) + matrix = block_mapping(strategy.get("matrix", ("", ""))[1], 8) + if ("prepare-matrix" not in dependencies(test) + or matrix.get("profile", ("", ""))[0] + != "${{ fromJSON(needs.prepare-matrix.outputs.profile-matrix) }}"): + failures.append(f"{path}: linux-test must consume prepare-matrix's profile-matrix output") + return failures + + +def dependencies(job): + value, body = block_mapping(job, 4).get("needs", ("", "")) + if value.startswith("[") and value.endswith("]"): + return {scalar(item) for item in value[1:-1].split(",")} + if value: + return {scalar(value)} + return {scalar(item) for item in re.findall(r"^\s+- (.+)$", body, re.MULTILINE)} + + +def shared_native_failures(workflows, jobs, artifacts): + """Validate the sole allowed cross-workflow artifact producer/consumer edge.""" + failures = [] + producer_uses = f"./.github/workflows/{SHARED_NATIVE_WORKFLOW}" + calls = [job_id for job_id, (_, body) in jobs.items() + if scalar(block_mapping(body, 4).get("uses", ("", ""))[0]) == producer_uses] + if calls != [SHARED_NATIVE_JOB]: + failures.append(f"ci.yml: expected exactly one {SHARED_NATIVE_JOB} call to {producer_uses}") + + if SHARED_NATIVE_JOB in jobs: + body = jobs[SHARED_NATIVE_JOB][1] + if "changes" not in dependencies(body): + failures.append(f"ci.yml: {SHARED_NATIVE_JOB} must need changes") + if "strategy" in block_mapping(body, 4): + failures.append(f"ci.yml: {SHARED_NATIVE_JOB} must not use a matrix") + for filename, (uploads, _) in artifacts.items(): + if filename != SHARED_NATIVE_WORKFLOW and SHARED_NATIVE_ARTIFACT in uploads: + failures.append(f"{filename}: only {SHARED_NATIVE_WORKFLOW} may upload '{SHARED_NATIVE_ARTIFACT}'") + + producer = workflows / SHARED_NATIVE_WORKFLOW + if not producer.exists(): + failures.append(f"{producer}: shared native producer is missing") + else: + producer_jobs = block_mapping( + block_mapping(producer.read_text(encoding="utf-8"), 0).get("jobs", ("", ""))[1], 2) + uploads = artifacts[SHARED_NATIVE_WORKFLOW][0] + if len(producer_jobs) != 1 or uploads != [SHARED_NATIVE_ARTIFACT]: + failures.append(f"{producer}: expected one job uploading '{SHARED_NATIVE_ARTIFACT}' once") + for _, body in producer_jobs.values(): + if "strategy" in block_mapping(body, 4): + failures.append(f"{producer}: shared native producer must not use a matrix") + + seen_consumers = set() + for job_id, (_, body) in jobs.items(): + fields = block_mapping(body, 4) + called = scalar(fields.get("uses", ("", ""))[0]).removeprefix("./.github/workflows/") + if called not in SHARED_NATIVE_CONSUMERS: + continue + seen_consumers.add(called) + if not {"changes", SHARED_NATIVE_JOB}.issubset(dependencies(body)): + failures.append(f"ci.yml: {job_id} must need changes and {SHARED_NATIVE_JOB}") + inputs = block_mapping(fields.get("with", ("", ""))[1], 6) + if scalar(inputs.get(SHARED_NATIVE_INPUT, ("", ""))[0]) != SHARED_NATIVE_ARTIFACT: + failures.append(f"ci.yml: {job_id} must pass {SHARED_NATIVE_INPUT}: {SHARED_NATIVE_ARTIFACT}") + + for filename in sorted(SHARED_NATIVE_CONSUMERS): + path = workflows / filename + if filename not in seen_consumers: + failures.append(f"ci.yml: shared native consumer {filename} is not called") + if not path.exists(): + failures.append(f"{path}: shared native consumer is missing") + continue + text = path.read_text(encoding="utf-8") + declaration = text + for key, indent in (("on", 0), ("workflow_call", 2), ("inputs", 4), + (SHARED_NATIVE_INPUT, 6)): + declaration = block_mapping(declaration, indent).get(key, ("", ""))[1] + fields = block_mapping(declaration, 8) + if (scalar(fields.get("required", ("", ""))[0]) != "true" + or scalar(fields.get("type", ("", ""))[0]) != "string"): + failures.append(f"{path}: {SHARED_NATIVE_INPUT} must be a required string input") + uploads, downloads = artifacts[filename] + if SHARED_NATIVE_EXPRESSION not in downloads: + failures.append(f"{path}: must download {SHARED_NATIVE_EXPRESSION}") + if any(name.startswith("native-lib") or name == SHARED_NATIVE_EXPRESSION for name in uploads): + failures.append(f"{path}: native library must only be uploaded by {SHARED_NATIVE_WORKFLOW}") + native_destinations = [inputs.get("name") for kind, inputs in artifact_steps(path) + if kind == "download" and inputs.get("path", "").startswith("native/target")] + if (any(name.startswith("native-lib") for name in downloads) + or any(name != SHARED_NATIVE_EXPRESSION for name in native_destinations)): + failures.append(f"{path}: native downloads must use {SHARED_NATIVE_EXPRESSION}") + if re.search(r"^\s*(?:cargo build\b|make (?:release|core)\b)", text, re.MULTILINE): + failures.append(f"{path}: must consume the shared native library instead of building it") + return failures + + +def linux_checks_failures(workflows, jobs): + """Keep lint, compile-only and debug Rust checks independent of native CI.""" + failures = [] + body = jobs.get(LINUX_CHECKS_JOB, ("", ""))[1] + fields = block_mapping(body, 4) + expected_uses = f"./.github/workflows/{LINUX_CHECKS_WORKFLOW}" + if scalar(fields.get("uses", ("", ""))[0]) != expected_uses: + failures.append(f"ci.yml: {LINUX_CHECKS_JOB} must call {expected_uses}") + if dependencies(body) != {"changes"}: + failures.append(f"ci.yml: {LINUX_CHECKS_JOB} must need only changes, independently of native CI") + + def condition(job): + value, body = block_mapping(job, 4).get("if", ("", "")) + text = body if value in {"|", ">"} else value + return " ".join(line.strip() for line in text.splitlines() + if line.strip() and not line.lstrip().startswith("#")) + + if condition(body) != condition(jobs.get("pr_build_linux", ("", ""))[1]): + failures.append(f"ci.yml: {LINUX_CHECKS_JOB} must use the Linux test selection condition") + + checks = workflows / LINUX_CHECKS_WORKFLOW + if not checks.exists(): + failures.append(f"{checks}: independent Linux checks workflow is missing") + return failures + text = checks.read_text(encoding="utf-8") + check_jobs = block_mapping(block_mapping(text, 0).get("jobs", ("", ""))[1], 2) + missing = INDEPENDENT_LINUX_JOBS - check_jobs.keys() + if missing: + failures.append(f"{checks}: independent jobs are missing: {', '.join(sorted(missing))}") + events = block_mapping(text, 0).get("on", ("", ""))[1] + workflow_call = block_mapping(events, 2).get("workflow_call", ("", ""))[1] + inputs = block_mapping(block_mapping(workflow_call, 4).get("inputs", ("", ""))[1], 6) + if SHARED_NATIVE_INPUT in inputs or any( + kind == "download" and (inputs.get("name", "").startswith("native-lib") + or inputs.get("path", "").startswith("native/target")) + for kind, inputs in artifact_steps(checks)): + failures.append(f"{checks}: independent Linux checks must not consume the shared native artifact") + + consumers = workflows / "pr_build_linux.yml" + if consumers.exists(): + consumer_jobs = block_mapping( + block_mapping(consumers.read_text(encoding="utf-8"), 0).get("jobs", ("", ""))[1], 2) + misplaced = INDEPENDENT_LINUX_JOBS & consumer_jobs.keys() + if misplaced: + failures.append(f"{consumers}: independent jobs must stay in {LINUX_CHECKS_WORKFLOW}: " + f"{', '.join(sorted(misplaced))}") + return failures + + +def native_selection_failures(jobs): + """Return errors when workflow gates diverge from the Python selector. + + `jobs` is the ci.yml job mapping returned by block_mapping, with each value + holding an inline scalar and indented body. The selector maps actual caller + IDs to tuples of selection outputs; each caller must use exactly their OR + (a single comparison for one output), and export every output directly. + For example, spark_4_1 accepts either its core or Hive output, while the + producer has one derived output. Expressions are compared, not evaluated. + Inputs and files are not mutated; import/read errors propagate. + """ + consumers = load_filters().NATIVE_CONSUMERS + actual_consumers = { + job_id for job_id, (_, body) in jobs.items() + if scalar(block_mapping(body, 4).get("uses", ("", ""))[0]) + .removeprefix("./.github/workflows/") in SHARED_NATIVE_CONSUMERS + } + failures = [] + if actual_consumers != consumers.keys(): + failures.append("ci.yml: native consumer calls must match NATIVE_CONSUMERS " + "in compute-changes.py") + changes = block_mapping(jobs.get("changes", ("", ""))[1], 4) + outputs = block_mapping(changes.get("outputs", ("", ""))[1], 6) + selections = {SHARED_NATIVE_JOB: (SHARED_NATIVE_JOB,), **consumers} + for job_id, routes in selections.items(): + fields = block_mapping(jobs.get(job_id, ("", ""))[1], 4) + expected = " || ".join(f"needs.changes.outputs.{output} == 'true'" for output in routes) + if fields.get("if", ("", ""))[0] != expected: + failures.append(f"ci.yml: {job_id} must select exactly {expected}") + for output in routes: + if outputs.get(output, ("", ""))[0] != f"${{{{ steps.compute.outputs.{output} }}}}": + failures.append(f"ci.yml: changes must export steps.compute.outputs.{output}") + return failures + + +def job_steps(job): + """Return ordered step mappings from a conventional workflow job body. + + `job` is the indented text from block_mapping. Only the direct `steps:` + list at six spaces is read; each result uses block_mapping's (scalar, + body) values. Replacing each list marker with spaces lets that existing + parser read step keys at eight spaces without inspecting nested actions. + This reads a string, mutates nothing, and returns an empty list if absent; + actionlint remains responsible for other YAML layouts and syntax errors. + """ + body = block_mapping(job, 4).get("steps", ("", ""))[1] + starts = list(re.finditer(r"^ - ", body, re.MULTILINE)) + return [block_mapping(" " + body[start.end(): + starts[index + 1].start() if index + 1 < len(starts) else len(body)], 8) + for index, start in enumerate(starts)] + + +def linux_maven_bootstrap_failures(workflows): + """Return errors for Linux Maven runs without a prior reliable bootstrap. + + `workflows` is a directory Path. Read only the two Linux reusable workflows + and their direct job steps; composite actions own their internal bootstrap. + Each direct ./mvnw run needs an earlier maven-bootstrap step in the same job + with no `if` and with failures propagated. Comments and non-run fields do + not count as commands. No files or mappings are mutated; missing workflows + produce errors, other file-read errors propagate, and success returns []. + """ + failures = [] + for filename in LINUX_MAVEN_WORKFLOWS: + path = workflows / filename + if not path.exists(): + failures.append(f"{path}: Linux Maven workflow is missing") + continue + jobs = block_mapping(block_mapping(path.read_text(encoding="utf-8"), 0) + .get("jobs", ("", ""))[1], 2) + for job_id, (_, body) in jobs.items(): + bootstrapped = False + for step in job_steps(body): + if (scalar(step.get("uses", ("", ""))[0]) == MAVEN_BOOTSTRAP_ACTION + and "if" not in step + and scalar(step.get("continue-on-error", ("false", ""))[0]) == "false"): + bootstrapped = True + value, script = step.get("run", ("", "")) + script = script if value in {"|", ">", "|-", ">-", "|+", ">+"} else scalar(value) + commands = "\n".join(line for line in script.splitlines() + if not line.lstrip().startswith("#")) + if re.search(r"(? 1: + artifacts = {path.name: artifact_names(path) for path in sorted(workflows.glob("*.y*ml"))} + failures = shared_native_failures(workflows, jobs, artifacts) + failures.extend(linux_checks_failures(workflows, jobs)) + failures.extend(native_selection_failures(jobs)) + failures.extend(linux_maven_bootstrap_failures(workflows)) + shared_wiring_valid = not failures + for filename, (uploads, downloads) in artifacts.items(): + path = workflows / filename + if call_counts.get(filename, 0) > 1: for name in uploads: if "inputs." not in name: failures.append( f"{path}: artifact '{name}' is uploaded by a workflow ci.yml calls " - f"{call_counts[path.name]} times; qualify the name with an input " + f"{call_counts[filename]} times; qualify the name with an input " f"(e.g. ${{{{ inputs.spark-full }}}}) so the parallel producers stay distinct" ) for name in downloads: - if name not in uploads: + explicitly_shared = (shared_wiring_valid and filename in SHARED_NATIVE_CONSUMERS + and name == SHARED_NATIVE_EXPRESSION) + if name not in uploads and not explicitly_shared: failures.append( f"{path}: artifact '{name}' is downloaded but never uploaded in the same " f"workflow; a producer rename probably missed its consumer" ) + return failures + + +def check_artifact_names(): + failures = artifact_failures(WORKFLOWS) for failure in failures: print(f"artifact name: {failure}") return not failures @@ -838,96 +1190,54 @@ def check_required_checks(): return not failures -def check_cache_refresh_scope(): - """Every job in pr_build_linux.yml is either a cache writer or guarded. - - On push to main the merge queue has already tested the exact tree that - landed, so the only thing left for that run to do is leave main's - actions/cache entries warm -- a pull request can restore caches saved on - its own branch or on main and nowhere else, and the queue's throwaway - branch takes its own with it. ci.yml therefore calls the workflow with - `cache-refresh-only` on push, and every job that is not a cache writer - has to opt out with `if: ${{ !inputs.cache-refresh-only }}`. - - A job added without the guard runs on every push again. Nothing fails when - that happens; the runner bill just quietly goes back up by up to ~500 - minutes a push, which is what this check exists to notice. - """ +def linux_scope_failures(workflows, allowed, guard, mode): + """Require each workflow's retained jobs and guard every other job in that mode.""" failures = [] - jobs, guarded = guarded_jobs(CACHE_REFRESH_WORKFLOW, CACHE_REFRESH_GUARD) - - for stale in sorted(set(CACHE_REFRESH_JOBS) - set(jobs)): - failures.append( - f"CACHE_REFRESH_JOBS names `{stale}`, which no longer exists in " - f"{CACHE_REFRESH_WORKFLOW}; drop it here, or restore the job" - ) - for name in jobs: - if name in CACHE_REFRESH_JOBS and name in guarded: - failures.append( - f"{CACHE_REFRESH_WORKFLOW}: job `{name}` is listed in " - f"CACHE_REFRESH_JOBS ({CACHE_REFRESH_JOBS[name]}) but carries " - f"the cache-refresh-only guard, so it is skipped on push and " - f"the cache it owns goes stale on main" - ) - if name not in CACHE_REFRESH_JOBS and name not in guarded: - failures.append( - f"{CACHE_REFRESH_WORKFLOW}: job `{name}` has no " - f"`if: ${{{{ !inputs.cache-refresh-only }}}}`, so it runs on " - f"every push to main where the merge queue has already tested " - f"the same tree. Add the guard, or add the job to " - f"CACHE_REFRESH_JOBS with the cache entry it writes" - ) + for filename, retained in allowed.items(): + path = workflows / filename + if not path.exists(): + failures.append(f"{path}: {mode} workflow is missing") + continue + jobs, guarded = guarded_jobs(path, guard) + for name in sorted(retained.keys() - set(jobs)): + failures.append(f"{path}: {mode} requires job `{name}` ({retained[name]})") + for name in jobs: + if name in retained and name in guarded: + failures.append(f"{path}: {mode} must retain job `{name}` ({retained[name]}), " + "but it carries the mode's skip guard") + if name not in retained and name not in guarded: + failures.append(f"{path}: job `{name}` has no {mode} guard, so it repeats " + "work the earlier CI tier already ran") + return failures + + +def cache_refresh_failures(workflows): + """Keep main's cache writers across all three workflows and skip other work. + + The caller expressions are checked separately for both Linux calls: finding + one input elsewhere in ci.yml does not protect the other workflow's jobs. + """ + failures = linux_scope_failures( + workflows, CACHE_REFRESH_JOBS, CACHE_REFRESH_GUARD, "cache-refresh-only") + failures.extend(linux_mode_input_failures( + workflows, "cache-refresh-only", CACHE_REFRESH_EXPRESSION)) + return failures - # The guards above do nothing unless the caller actually sets the input; - # its default is false, so a dropped `with:` block silently restores the - # full pipeline on push. - ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8").splitlines() - if not any(CACHE_REFRESH_INPUT.match(line) for line in ci): - failures.append( - "ci.yml never passes `cache-refresh-only:` to pr_build_linux.yml. " - "The input defaults to false, so without it every push to main runs " - "the full pipeline again" - ) +def check_cache_refresh_scope(): + failures = cache_refresh_failures(WORKFLOWS) for failure in failures: print(f"cache refresh scope: {failure}") return not failures +def nightly_scope_failures(workflows): + """Keep the nightly matrix and prerequisites while skipping previously run checks.""" + return linux_scope_failures(workflows, NIGHTLY_JOBS, NIGHTLY_GUARD, "nightly") + + def check_nightly_scope(): - """With `profiles: nightly`, pr_build_linux.yml runs the test matrix alone. - - That input value means the default-profile pipeline already ran at this - commit: in the queue and the push run for the nightly, in the PR tier for - a `run-all-spark-profiles` label run. The lints, the Rust tests, the - Spark build and the TPC-H/TPC-DS runs would repeat a verdict, so every - job other than the three the matrix needs carries - `if: ${{ inputs.profiles != 'nightly' }}`. Silent when broken, like the - cache-refresh guard: the nightly just costs 100-odd runner-minutes more. - """ - failures = [] - jobs, guarded = guarded_jobs(CACHE_REFRESH_WORKFLOW, NIGHTLY_GUARD) - for stale in sorted(set(NIGHTLY_JOBS) - set(jobs)): - failures.append( - f"NIGHTLY_JOBS names `{stale}`, which no longer exists in " - f"{CACHE_REFRESH_WORKFLOW}; drop it here, or restore the job" - ) - for name in jobs: - if name in NIGHTLY_JOBS and name in guarded: - failures.append( - f"{CACHE_REFRESH_WORKFLOW}: job `{name}` is listed in " - f"NIGHTLY_JOBS ({NIGHTLY_JOBS[name]}) but carries the " - f"`profiles != 'nightly'` guard, so the nightly test matrix " - f"cannot run" - ) - if name not in NIGHTLY_JOBS and name not in guarded: - failures.append( - f"{CACHE_REFRESH_WORKFLOW}: job `{name}` has no " - f"`if: ${{{{ inputs.profiles != 'nightly' }}}}`, so it repeats " - f"on every nightly a verdict the queue already produced. Add " - f"the guard, or add the job to NIGHTLY_JOBS with the reason " - f"the test matrix needs it" - ) + failures = nightly_scope_failures(WORKFLOWS) for failure in failures: print(f"nightly scope: {failure}") return not failures diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 799b791624..5487f20db2 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -40,6 +40,7 @@ FILTERS = { "build_linux": [ + ".github/workflows/build_linux_native.yml", "native/**", "common/**", "spark/**", @@ -53,6 +54,7 @@ "dev/ci/**", ".github/workflows/ci.yml", ".github/workflows/pr_build_linux.yml", + ".github/workflows/pr_build_linux_checks.yml", ".github/actions/setup-builder/**", ".github/actions/java-test/**", ".github/actions/maven-bootstrap/**", @@ -179,6 +181,7 @@ "spark/src/main/spark-*/**", ], "spark_3_4": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -206,6 +209,7 @@ "mvnw", ], "spark_3_5": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -233,6 +237,7 @@ "mvnw", ], "spark_4_0": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -260,6 +265,7 @@ "mvnw", ], "spark_4_1": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -292,6 +298,7 @@ # input. Populated below, after the dict, so the two lists cannot drift. "spark_4_1_hive": [], "iceberg_1_8": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -316,6 +323,7 @@ "mvnw", ], "iceberg_1_9": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -340,6 +348,7 @@ "mvnw", ], "iceberg_1_10": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -364,6 +373,7 @@ "mvnw", ], "iceberg_1_11": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -421,7 +431,7 @@ # in the queue and once after, which is the thing the queue was adopted to # avoid. POLICY = { - # The one test job that also runs on push to main, and only because of + # The Linux workflows also run on push to main, and only because of # actions/cache scoping: a pull request can restore caches saved on its # own branch or on main, and nowhere else. The queue runs on a throwaway # gh-readonly-queue/* branch, so whatever it saves is deleted with that @@ -433,13 +443,16 @@ # On push that is the *only* thing it is for. The queue already tested the # exact tree that landed, so re-running the lints and the linux-test # matrix there tests nothing, and they are 514 of the 587 runner-minutes a - # push run costs. The split below keeps the cache writers on push and moves - # everything else behind `build_linux_full`. + # push run costs. The cache writers now span build_linux_native.yml, + # pr_build_linux_checks.yml (cargo-debug), and pr_build_linux.yml (Maven + # and TPC-H/TPC-DS), so push must select all three workflows. The split + # below keeps those writers on push and moves everything else behind + # `build_linux_full`. "build_linux": ["pr", "queue", "push"], - # The lints and the test matrix inside pr_build_linux.yml. Deliberately no - # "push": ci.yml turns this output into the workflow's `cache-refresh-only` - # input, so dropping "push" here is what trims the push tier down to the - # jobs that write an actions/cache entry. See issue #5929. + # The lints in pr_build_linux_checks.yml and test matrix in pr_build_linux.yml. + # Deliberately no "push": ci.yml uses this output for both workflows' + # `cache-refresh-only` inputs, trimming push down to jobs needed to write an + # actions/cache entry. See issue #5929. "build_linux_full": ["pr", "queue"], # The linux-test matrix's Spark profiles other than the default one. The # five profiles cost about the same each, roughly 2,300 runner-minutes a @@ -502,6 +515,24 @@ "iceberg_1_11": ["queue", "label:run-iceberg-tests"], } +# Map each caller job that downloads the shared Linux native library to every +# output key that can select it. Insertion order follows the callers in ci.yml. +# Linux has separate default/all-profile routes, and Spark 4.1 has core/Hive +# routes; either route must select the shared producer for its caller. +# The producer has no independent path or event policy: it runs exactly when +# any route of at least one consumer is selected after FILTERS and POLICY apply. +NATIVE_CONSUMERS = { + "pr_build_linux": ("build_linux", "build_linux_all_profiles"), + "spark_3_4": ("spark_3_4",), + "spark_3_5": ("spark_3_5",), + "spark_4_0": ("spark_4_0",), + "spark_4_1": ("spark_4_1", "spark_4_1_hive"), + "iceberg_1_8": ("iceberg_1_8",), + "iceberg_1_9": ("iceberg_1_9",), + "iceberg_1_10": ("iceberg_1_10",), + "iceberg_1_11": ("iceberg_1_11",), +} + def gating_labels(job): return [t[len("label:"):] for t in POLICY[job] if t.startswith("label:")] @@ -546,11 +577,26 @@ def event_allows(job, event): def compute(files, event): - """Return {job: bool}, folding the path filter and the event policy.""" - return { - name: event_allows(name, event) and matches(patterns, files) + """Return a new {output: bool} mapping for consumers and their native build. + + `files` is a reusable sequence of repository-relative changed paths; + `event` has the fields described by event_allows(). Neither input is + mutated. Manual dispatch selects every route even with no changed files; + other events require both path and event matches. The shared native build + is selected only after those decisions, taking the union of every caller's + routes, so Hive alone can start its producer and a denied opt-in route + cannot start an unused producer. Unknown events select nothing. Configuration + lookup failures propagate as KeyError rather than returning partial output. + """ + manual = event.get("name") == "workflow_dispatch" + outputs = { + name: event_allows(name, event) and (manual or matches(patterns, files)) for name, patterns in FILTERS.items() } + outputs["build_linux_native"] = any( + outputs[name] for routes in NATIVE_CONSUMERS.values() for name in routes + ) + return outputs def event_from_env(): @@ -605,12 +651,12 @@ def matches(patterns, files): if __name__ == "__main__": event = event_from_env() # workflow_dispatch has no meaningful base to diff against, so the caller - # passes an empty list and every path filter is treated as matched. + # passes an empty list. compute() applies its override before deriving the + # native producer, and every event emits the same complete set of outputs. if event["name"] == "workflow_dispatch": - for name in FILTERS: - print(f"{name}=true") - sys.exit(0) - files_path = Path(sys.argv[1]) - files = [line.strip() for line in files_path.read_text().splitlines() if line.strip()] + files = [] + else: + files_path = Path(sys.argv[1]) + files = [line.strip() for line in files_path.read_text().splitlines() if line.strip()] for name, flag in compute(files, event).items(): print(f"{name}={'true' if flag else 'false'}") diff --git a/dev/ci/test-ci-config.py b/dev/ci/test-ci-config.py new file mode 100644 index 0000000000..a0e63b8cd0 --- /dev/null +++ b/dev/ci/test-ci-config.py @@ -0,0 +1,560 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Regression tests for shared artifacts using mutations of the real workflows.""" + +import importlib.util +from pathlib import Path +import shutil +import tempfile +import unittest +from unittest import mock + + +SPEC = importlib.util.spec_from_file_location( + "check_ci_config", Path(__file__).with_name("check-ci-config.py")) +CHECK = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(CHECK) +WORKFLOWS = Path(__file__).resolve().parents[2] / ".github/workflows" + +# Pin every direct Maven caller affected by splitting the Linux workflow. +MAVEN_JOBS = { + "pr_build_linux_checks.yml": ( + "lint-java", "build-spark-4-1", "celeborn-reflection-compatibility"), + "pr_build_linux.yml": ("verify-benchmark-results-tpch", "verify-benchmark-results-tpcds"), +} + + +class SharedNativeArtifactTest(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="comet-ci-config-test-") + self.addCleanup(self.temp.cleanup) + self.workflows = Path(self.temp.name) / "workflows" + shutil.copytree(WORKFLOWS, self.workflows) + + def replace(self, filename, old, new): + path = self.workflows / filename + text = path.read_text(encoding="utf-8") + self.assertIn(old, text, f"Fixture changed: {filename}") + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + def assert_rejected(self, expected): + failures = CHECK.artifact_failures(self.workflows) + self.assertTrue(any(expected in failure for failure in failures), failures) + + def test_real_workflows_have_valid_shared_artifacts(self): + self.assertEqual(CHECK.artifact_failures(self.workflows), []) + self.assertEqual(CHECK.cache_refresh_failures(self.workflows), []) + self.assertEqual(CHECK.nightly_scope_failures(self.workflows), []) + self.assertEqual(CHECK.linux_profile_failures(self.workflows), []) + + def test_linux_modes_reject_new_unguarded_jobs_in_each_workflow(self): + for filename in CHECK.CACHE_REFRESH_JOBS: + path = self.workflows / filename + original = path.read_text(encoding="utf-8") + with self.subTest(workflow=filename): + path.write_text(original + "\n extra-check:\n runs-on: ubuntu-24.04\n" + " steps:\n - run: true\n", encoding="utf-8") + for check in (CHECK.cache_refresh_failures, CHECK.nightly_scope_failures): + failures = check(self.workflows) + self.assertTrue(any("extra-check` has no" in item for item in failures), failures) + path.write_text(original, encoding="utf-8") + + def test_cache_refresh_preserves_each_cache_owner_and_prerequisite(self): + """Reject both removing a retained job and guarding it off on main pushes.""" + for filename, retained in CHECK.CACHE_REFRESH_JOBS.items(): + path = self.workflows / filename + original = path.read_text(encoding="utf-8") + jobs = CHECK.block_mapping(CHECK.block_mapping(original, 0)["jobs"][1], 2) + for job_id in retained: + body = jobs[job_id][1] + guarded = (body.replace("inputs.profiles != 'nightly'", + "!inputs.cache-refresh-only && inputs.profiles != 'nightly'", 1) + if "if" in CHECK.block_mapping(body, 4) else + "\n if: ${{ !inputs.cache-refresh-only }}" + body) + mutations = ( + (original.replace(f" {job_id}:{body}", "", 1), f"requires job `{job_id}`"), + (original.replace(body, guarded, 1), f"must retain job `{job_id}`"), + ) + for mutation, expected in mutations: + with self.subTest(workflow=filename, job=job_id, expected=expected): + self.assertNotEqual(original, mutation) + path.write_text(mutation, encoding="utf-8") + failures = CHECK.cache_refresh_failures(self.workflows) + self.assertTrue(any(expected in item for item in failures), failures) + path.write_text(original, encoding="utf-8") + + def test_existing_checks_cannot_lose_cache_or_nightly_guards(self): + cases = ( + ("pr_build_linux_checks.yml", "lint-java", "!inputs.cache-refresh-only && ", + CHECK.cache_refresh_failures), + ("pr_build_linux.yml", "linux-test", " if: ${{ !inputs.cache-refresh-only }}\n", + CHECK.cache_refresh_failures), + ("pr_build_linux_checks.yml", "linux-test-rust", + " if: ${{ inputs.profiles != 'nightly' }}\n", CHECK.nightly_scope_failures), + ("pr_build_linux.yml", "verify-benchmark-results-tpch", + " if: ${{ inputs.profiles != 'nightly' }}\n", CHECK.nightly_scope_failures), + ) + for filename, job_id, guard, check in cases: + path = self.workflows / filename + original = path.read_text(encoding="utf-8") + body = CHECK.block_mapping(CHECK.block_mapping(original, 0)["jobs"][1], 2)[job_id][1] + with self.subTest(workflow=filename, job=job_id): + self.assertIn(guard, body) + path.write_text(original.replace(body, body.replace(guard, "", 1), 1), encoding="utf-8") + failures = check(self.workflows) + self.assertTrue(any(f"job `{job_id}` has no" in item for item in failures), failures) + path.write_text(original, encoding="utf-8") + + def test_linux_callers_each_require_the_correct_mode_inputs(self): + """One intact caller must not hide missing or miswired inputs on its sibling.""" + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + jobs = CHECK.block_mapping(CHECK.block_mapping(original, 0)["jobs"][1], 2) + for job_id in CHECK.LINUX_MODE_CALLERS: + body = jobs[job_id][1] + fields = CHECK.block_mapping(body, 4) + inputs = CHECK.block_mapping(fields["with"][1], 6) + for name, check in (("cache-refresh-only", CHECK.cache_refresh_failures), + ("profiles", CHECK.linux_profile_failures)): + value, nested = inputs[name] + declaration = f" {name}: {value}{nested}" + self.assertIn(declaration, body) + for replacement in ("", f" {name}: false\n"): + with self.subTest(job=job_id, input=name, replacement=replacement): + mutation = body.replace(declaration, replacement, 1) + path.write_text(original.replace(body, mutation, 1), encoding="utf-8") + failures = check(self.workflows) + self.assertTrue(any(f"{job_id} must pass {name}:" in item + for item in failures), failures) + path.write_text(original, encoding="utf-8") + + def test_linux_profile_matrix_stays_connected_and_available_at_night(self): + path = self.workflows / "pr_build_linux.yml" + original = path.read_text(encoding="utf-8") + mutations = ( + ("${{ steps.profiles.outputs.matrix }}", "[]", CHECK.linux_profile_failures, + "prepare-matrix must publish"), + ("${{ fromJSON(needs.prepare-matrix.outputs.profile-matrix) }}", "[]", + CHECK.linux_profile_failures, "linux-test must consume prepare-matrix"), + (" if: ${{ !inputs.cache-refresh-only }}\n", + " if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }}\n", + CHECK.nightly_scope_failures, "must retain job `prepare-matrix`"), + ) + for old, new, check, expected in mutations: + with self.subTest(expected=expected): + self.assertIn(old, original) + path.write_text(original.replace(old, new, 1), encoding="utf-8") + failures = check(self.workflows) + self.assertTrue(any(expected in item for item in failures), failures) + path.write_text(original, encoding="utf-8") + + def test_native_gates_cannot_bypass_selected_outputs(self): + """Reject an inverted gate for each producer/consumer in a temporary copy. + + Each mutation starts from the original workflow and must report the + changed job, including each side of the Spark 4.1 core/Hive OR. The + source tree is untouched and the fixture is restored on success, or + removed by tear-down if an assertion fails. + """ + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + selections = {CHECK.SHARED_NATIVE_JOB: (CHECK.SHARED_NATIVE_JOB,), + **CHECK.load_filters().NATIVE_CONSUMERS} + for job_id, routes in selections.items(): + for output in routes: + with self.subTest(job=job_id, output=output): + start = original.index(f"\n {job_id}:\n") + expression = f"needs.changes.outputs.{output} == 'true'" + self.assertIn(expression, original[start:]) + body = original[start:].replace( + expression, f"needs.changes.outputs.{output} == 'false'", 1) + path.write_text(original[:start] + body, encoding="utf-8") + self.assert_rejected(f"{job_id} must select exactly") + path.write_text(original, encoding="utf-8") + + def test_native_outputs_must_be_exported_without_remapping(self): + """Reject missing and remapped output exports in temporary workflows. + + Every producer/consumer output, including Hive's second route into + Spark 4.1, is checked independently. This covers Python selecting work + that ci.yml never starts. Only the temporary fixture is mutated; it is + restored on success and removed by tear-down on any assertion failure. + """ + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + outputs = {CHECK.SHARED_NATIVE_JOB} | { + output for routes in CHECK.load_filters().NATIVE_CONSUMERS.values() for output in routes} + for output in sorted(outputs): + expression = f"${{{{ steps.compute.outputs.{output} }}}}" + self.assertIn(expression, original) + for replacement in ("", "${{ steps.compute.outputs.docs }}"): + with self.subTest(output=output, replacement=replacement): + path.write_text(original.replace(expression, replacement, 1), encoding="utf-8") + self.assert_rejected(f"changes must export steps.compute.outputs.{output}") + path.write_text(original, encoding="utf-8") + + def test_hive_or_cannot_omit_or_add_selection_routes(self): + """Reject narrowed or broadened Spark 4.1 gates in temporary ci.yml. + + Each mutation starts with the real core/Hive OR and tests missing + branches, AND, a wrong Hive output, and an unrelated extra branch. + No source files change; the fixture is restored on success and cleaned + up by tear-down after either success or an assertion failure. + """ + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + core = "needs.changes.outputs.spark_4_1 == 'true'" + hive = "needs.changes.outputs.spark_4_1_hive == 'true'" + gate = f"{core} || {hive}" + self.assertIn(gate, original) + for replacement in (core, hive, f"{core} && {hive}", + f"{core} || needs.changes.outputs.docs == 'true'", + f"{gate} || needs.changes.outputs.docs == 'true'"): + with self.subTest(gate=replacement): + path.write_text(original.replace(gate, replacement, 1), encoding="utf-8") + self.assert_rejected("spark_4_1 must select exactly") + path.write_text(original, encoding="utf-8") + + def test_secondary_routes_must_remain_in_native_selector(self): + """Reject omitting Hive or non-default profiles from the producer's selector. + + Patch only the freshly imported selector returned to the checker; + temporary and repository workflows remain unchanged. The mock scope + restores load_filters even if the expected diagnostic is not raised. + """ + for job_id in ("spark_4_1", "pr_build_linux"): + with self.subTest(job=job_id): + selector = CHECK.load_filters() + selector.NATIVE_CONSUMERS = { + **selector.NATIVE_CONSUMERS, + job_id: selector.NATIVE_CONSUMERS[job_id][:1], + } + with mock.patch.object(CHECK, "load_filters", return_value=selector): + self.assert_rejected(f"{job_id} must select exactly") + + def test_native_consumer_calls_must_match_selector_ids(self): + """Reject missing, renamed, or unrelated workflow calls for Spark 4.1. + + Keep the real Hive OR intact while mutating its caller in temporary + ci.yml. The checker must identify the caller/selector mismatch rather + than exempt the combined gate. Only the fixture is mutated; it is + restored on success and removed by tear-down after assertion failures. + """ + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + body = CHECK.block_mapping(CHECK.block_mapping(original, 0)["jobs"][1], 2)["spark_4_1"][1] + mutations = ( + original.replace(f" spark_4_1:{body}", "", 1), + original.replace(" spark_4_1:\n", " spark_4_1_renamed:\n", 1), + original.replace(body, body.replace("spark_sql_test_reusable.yml", "unrelated.yml"), 1), + ) + for index, mutation in enumerate(mutations): + with self.subTest(mutation=index): + self.assertNotEqual(original, mutation) + path.write_text(mutation, encoding="utf-8") + self.assert_rejected("native consumer calls must match NATIVE_CONSUMERS") + path.write_text(original, encoding="utf-8") + + def test_hive_gate_does_not_allow_missing_native_producer_call(self): + """Reject a removed producer call even with valid core/Hive routing. + + Remove only the producer job from temporary ci.yml and require a + producer diagnostic. The repository remains unchanged; tear-down + releases the fixture directory even if the assertion fails. + """ + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + body = CHECK.block_mapping(CHECK.block_mapping(original, 0)["jobs"][1], 2)[ + CHECK.SHARED_NATIVE_JOB][1] + declaration = f" {CHECK.SHARED_NATIVE_JOB}:{body}" + self.assertIn(declaration, original) + path.write_text(original.replace(declaration, "", 1), encoding="utf-8") + self.assert_rejected("expected exactly one build_linux_native call") + + def test_new_native_consumer_must_join_selector(self): + """Reject an extra consumer absent from Python's producer union. + + Append a caller to the temporary workflow only. The fixture directory + is removed by tear-down; no repository file is changed. + """ + path = self.workflows / "ci.yml" + with path.open("a", encoding="utf-8") as stream: + stream.write("\n spark_future:\n needs: [changes, build_linux_native]\n" + " uses: ./.github/workflows/spark_sql_test_reusable.yml\n" + " with:\n native-library-artifact: native-lib-linux\n") + self.assert_rejected("native consumer calls must match NATIVE_CONSUMERS") + + def test_missing_shared_producer_is_rejected(self): + (self.workflows / CHECK.SHARED_NATIVE_WORKFLOW).unlink() + self.assert_rejected("shared native producer is missing") + + def test_second_producer_call_is_rejected(self): + path = self.workflows / "ci.yml" + with path.open("a", encoding="utf-8") as stream: + stream.write("\n duplicate_native:\n needs: changes\n" + " uses: ./.github/workflows/build_linux_native.yml\n") + self.assert_rejected("expected exactly one") + + def test_another_workflow_cannot_publish_the_shared_name(self): + shutil.copyfile(self.workflows / CHECK.SHARED_NATIVE_WORKFLOW, + self.workflows / "duplicate_native.yml") + self.assert_rejected("only build_linux_native.yml may upload") + + def test_producer_matrix_is_rejected(self): + self.replace(CHECK.SHARED_NATIVE_WORKFLOW, " runs-on:", + " strategy:\n matrix:\n duplicate: [1, 2]\n runs-on:") + self.assert_rejected("shared native producer must not use a matrix") + + def test_producer_artifact_rename_is_rejected(self): + self.replace(CHECK.SHARED_NATIVE_WORKFLOW, "name: native-lib-linux", + "name: native-lib-renamed") + self.assert_rejected("uploading 'native-lib-linux' once") + + def test_missing_dependency_in_each_consumer_call_is_rejected(self): + """Reject each caller dropping its producer dependency independently. + + Caller IDs come from the selector mapping, not its output keys: Hive + shares the spark_4_1 caller. Each mutation starts from real ci.yml in + the temporary directory, restored on success and cleaned on failure. + """ + path = self.workflows / "ci.yml" + original = path.read_text(encoding="utf-8") + job_ids = CHECK.load_filters().NATIVE_CONSUMERS + for job_id in sorted(job_ids): + with self.subTest(job=job_id): + start = original.index(f"\n {job_id}:\n") + before, body = original[:start], original[start:] + self.assertIn("needs: [changes, build_linux_native]", body) + body = body.replace("needs: [changes, build_linux_native]", "needs: changes", 1) + path.write_text(before + body, encoding="utf-8") + self.assert_rejected(f"{job_id} must need changes and build_linux_native") + path.write_text(original, encoding="utf-8") + + def test_wrong_caller_artifact_is_rejected(self): + self.replace("ci.yml", "native-library-artifact: native-lib-linux", + "native-library-artifact: native-lib-wrong") + self.assert_rejected("must pass native-library-artifact: native-lib-linux") + + def test_missing_caller_artifact_is_rejected(self): + self.replace("ci.yml", " native-library-artifact: native-lib-linux\n", "") + self.assert_rejected("must pass native-library-artifact: native-lib-linux") + + def test_consumer_input_must_be_required(self): + self.replace("pr_build_linux.yml", "required: true", "required: false") + self.assert_rejected("native-library-artifact must be a required string input") + + def test_consumer_input_must_be_string(self): + self.replace("pr_build_linux.yml", "required: true\n type: string", + "required: true\n type: boolean") + self.assert_rejected("native-library-artifact must be a required string input") + + def test_literal_native_download_is_rejected(self): + self.replace("pr_build_linux.yml", "name: ${{ inputs.native-library-artifact }}", + "name: native-lib-linux") + self.assert_rejected("native downloads must use") + + def test_jvm_artifact_cannot_replace_a_native_download(self): + path = self.workflows / "spark_sql_test_reusable.yml" + uploads, _ = CHECK.artifact_names(path) + jvm_name = next(name for name in uploads if name.startswith("jvm-compiled-spark-")) + self.replace(path.name, "name: ${{ inputs.native-library-artifact }}", f"name: {jvm_name}") + self.assert_rejected("native downloads must use") + + def test_unrelated_input_does_not_bypass_producer_check(self): + self.replace("pr_build_linux.yml", "name: ${{ inputs.native-library-artifact }}", + "name: ${{ inputs.unrelated-artifact }}") + self.assert_rejected("artifact '${{ inputs.unrelated-artifact }}' is downloaded but never uploaded") + + def test_native_build_in_consumer_is_rejected(self): + path = self.workflows / "iceberg_spark_test_reusable.yml" + with path.open("a", encoding="utf-8") as stream: + stream.write("\n - run: |\n cargo build --profile ci\n") + self.assert_rejected("consume the shared native library instead of building it") + + def test_jvm_artifact_rename_still_fails(self): + self.replace("spark_sql_test_reusable.yml", "name: jvm-compiled-spark-", + "name: renamed-jvm-compiled-spark-") + self.assert_rejected("is downloaded but never uploaded in the same workflow") + + def test_unqualified_jvm_upload_still_fails(self): + path = self.workflows / "spark_sql_test_reusable.yml" + text = path.read_text(encoding="utf-8") + uploads, _ = CHECK.artifact_names(path) + name = next(name for name in uploads if name.startswith("jvm-compiled-spark-")) + path.write_text(text.replace(f"name: {name}", "name: jvm-compiled-spark"), encoding="utf-8") + self.assert_rejected("qualify the name with an input") + + + def test_independent_checks_cannot_wait_for_native(self): + path = self.workflows / "ci.yml" + text = path.read_text(encoding="utf-8") + start = text.index("\n pr_build_linux_checks:\n") + before, body = text[:start], text[start:] + self.assertIn("needs: changes", body) + path.write_text(before + body.replace("needs: changes", + "needs: [changes, build_linux_native]", 1), encoding="utf-8") + self.assert_rejected("pr_build_linux_checks must need only changes") + + def test_missing_independent_checks_call_is_rejected(self): + self.replace("ci.yml", "uses: ./.github/workflows/pr_build_linux_checks.yml", + "uses: ./.github/workflows/pr_build_linux.yml") + self.assert_rejected("pr_build_linux_checks must call") + + def test_missing_independent_checks_workflow_is_rejected(self): + (self.workflows / CHECK.LINUX_CHECKS_WORKFLOW).unlink() + self.assert_rejected("independent Linux checks workflow is missing") + + def test_independent_checks_keep_linux_selection(self): + path = self.workflows / "ci.yml" + text = path.read_text(encoding="utf-8") + start = text.index("\n pr_build_linux_checks:\n") + before, body = text[:start], text[start:] + body = body.replace("needs.changes.outputs.build_linux == 'true'", + "needs.changes.outputs.build_linux == 'false'", 1) + path.write_text(before + body, encoding="utf-8") + self.assert_rejected("pr_build_linux_checks must use the Linux test selection condition") + + def test_each_independent_job_must_remain_available(self): + path = self.workflows / CHECK.LINUX_CHECKS_WORKFLOW + original = path.read_text(encoding="utf-8") + for job_id in sorted(CHECK.INDEPENDENT_LINUX_JOBS): + with self.subTest(job=job_id): + self.assertIn(f"\n {job_id}:\n", original) + path.write_text(original.replace(f"\n {job_id}:\n", + f"\n missing-{job_id}:\n", 1), encoding="utf-8") + self.assert_rejected(f"independent jobs are missing: {job_id}") + path.write_text(original, encoding="utf-8") + + def test_independent_job_cannot_return_to_native_consumer(self): + path = self.workflows / "pr_build_linux.yml" + with path.open("a", encoding="utf-8") as stream: + stream.write("\n linux-test-rust:\n runs-on: ubuntu-24.04\n" + " steps:\n - run: true\n") + self.assert_rejected("independent jobs must stay in pr_build_linux_checks.yml") + + def test_independent_checks_cannot_require_native_artifact_input(self): + self.replace(CHECK.LINUX_CHECKS_WORKFLOW, " inputs:\n", + " inputs:\n native-library-artifact:\n" + " required: true\n type: string\n") + self.assert_rejected("independent Linux checks must not consume the shared native artifact") + + def test_independent_checks_cannot_download_native_artifact(self): + """Reject direct and retried downloads of native artifacts by independent checks. + + Each action is appended to a fresh copy of the temporary workflow. The + original fixture is restored after successful assertions and its whole + temporary directory is cleaned up even if a check fails. + """ + path = self.workflows / CHECK.LINUX_CHECKS_WORKFLOW + original = path.read_text(encoding="utf-8") + for action in ("actions/download-artifact@v8", "./.github/actions/download-artifact-retry"): + with self.subTest(action=action): + path.write_text(original + f"\n - uses: {action}\n" + " with:\n name: native-lib-linux\n" + " path: native/target/release/\n", encoding="utf-8") + self.assert_rejected("independent Linux checks must not consume the shared native artifact") + path.write_text(original, encoding="utf-8") + + def test_every_linux_maven_job_keeps_earlier_reliable_bootstrap(self): + """Reject four lost-bootstrap cases for all five direct Maven callers. + + For each explicitly named job, independently remove the bootstrap, + move it after its Maven commands, make it conditional, or allow its + errors. This exercises inline, multiline, and env-prefixed Maven runs. + Only temporary workflow copies are mutated, restored between jobs on + success and removed by tear-down after either success or failure. + """ + bootstrap = f" - name: Bootstrap Maven\n uses: {CHECK.MAVEN_BOOTSTRAP_ACTION}\n" + for filename, job_ids in MAVEN_JOBS.items(): + path = self.workflows / filename + original = path.read_text(encoding="utf-8") + jobs = CHECK.block_mapping(CHECK.block_mapping(original, 0)["jobs"][1], 2) + for job_id in job_ids: + body = jobs[job_id][1] + self.assertIn(bootstrap, body) + removed = body.replace(bootstrap, "", 1) + mutations = { + "missing": removed, + "late": removed + "\n" + bootstrap, + "conditional": body.replace(bootstrap, bootstrap + " if: false\n", 1), + "ignored-failure": body.replace( + bootstrap, bootstrap + " continue-on-error: true\n", 1), + } + for name, mutation in mutations.items(): + with self.subTest(workflow=filename, job=job_id, mutation=name): + path.write_text(original.replace(body, mutation, 1), encoding="utf-8") + self.assert_rejected(f"{job_id} must bootstrap Maven unconditionally") + path.write_text(original, encoding="utf-8") + + def test_new_direct_maven_job_cannot_borrow_another_jobs_bootstrap(self): + """Reject a new direct Maven step in each scoped Linux workflow. + + Earlier jobs already bootstrap, so appending an unprotected job also + verifies that successful bootstrap state does not cross job boundaries. + Only temporary fixtures change; they are restored after assertions + and their directory is removed by tear-down even on failure. + """ + for filename in MAVEN_JOBS: + path = self.workflows / filename + original = path.read_text(encoding="utf-8") + with self.subTest(workflow=filename): + path.write_text(original + "\n another-maven-job:\n runs-on: ubuntu-24.04\n" + " steps:\n - run: ./mvnw -B validate\n", encoding="utf-8") + self.assert_rejected("another-maven-job must bootstrap Maven unconditionally") + path.write_text(original, encoding="utf-8") + + def test_maven_bootstrap_guard_is_scoped_to_direct_linux_commands(self): + """Accept composite-only jobs and ignore Maven outside the two workflows. + + Temporary fixtures add a composite caller and a job mentioning ./mvnw + only in a shell comment and env value, plus another workflow with an + unbootstrapped direct run. These need no direct Linux bootstrap; the + existing five protected jobs must still pass. No repository files are + changed and tear-down removes all added files even on assertion error. + """ + for filename in MAVEN_JOBS: + path = self.workflows / filename + with path.open("a", encoding="utf-8") as stream: + stream.write("\n composite-only:\n runs-on: ubuntu-24.04\n" + " steps:\n - uses: ./.github/actions/java-test\n" + "\n no-direct-maven:\n runs-on: ubuntu-24.04\n steps:\n" + " - run: |\n # ./mvnw is owned by the composite.\n" + " echo complete\n env:\n COMMAND: ./mvnw\n") + (self.workflows / "unrelated.yml").write_text( + "jobs:\n other-maven:\n steps:\n - run: ./mvnw validate\n", encoding="utf-8") + self.assertEqual(CHECK.linux_maven_bootstrap_failures(self.workflows), []) + + def test_maven_bootstrap_can_explicitly_propagate_failure(self): + """Accept explicit continue-on-error false on Linux bootstrap steps. + + Mutate only temporary workflows to spell out the default failure + behavior, then check the protected direct Maven callers still pass. + Tear-down removes the fixtures after success or assertion failure. + """ + for filename in MAVEN_JOBS: + path = self.workflows / filename + original = path.read_text(encoding="utf-8") + action = f" uses: {CHECK.MAVEN_BOOTSTRAP_ACTION}\n" + self.assertIn(action, original) + path.write_text(original.replace(action, action + " continue-on-error: false\n"), + encoding="utf-8") + self.assertEqual(CHECK.linux_maven_bootstrap_failures(self.workflows), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/dev/ci/test-native-build-selection.py b/dev/ci/test-native-build-selection.py new file mode 100644 index 0000000000..3e9ffc2afc --- /dev/null +++ b/dev/ci/test-native-build-selection.py @@ -0,0 +1,467 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Exercise shared native-build selection through the routing script and CLI. + +Consumer outputs already include path and event policy. The native producer +must be their union, including for manual dispatch. check-ci-config.py checks +the workflow's output wiring; actionlint validates its syntax and dependencies. +""" + +import importlib.util +import itertools +import json +import os +import subprocess +import sys +import tempfile +import unittest +from unittest import mock +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CONSUMERS = { + "pr_build_linux": ("build_linux", "build_linux_all_profiles"), + "spark_3_4": ("spark_3_4",), + "spark_3_5": ("spark_3_5",), + "spark_4_0": ("spark_4_0",), + "spark_4_1": ("spark_4_1", "spark_4_1_hive"), + "iceberg_1_8": ("iceberg_1_8",), + "iceberg_1_9": ("iceberg_1_9",), + "iceberg_1_10": ("iceberg_1_10",), + "iceberg_1_11": ("iceberg_1_11",), +} +DEFAULT = {"pr_build_linux"} +QUEUE = {"pr_build_linux", "spark_4_1", "iceberg_1_11"} +NIGHTLY = {"pr_build_linux", "spark_3_5", "spark_4_0", + "iceberg_1_8", "iceberg_1_9", "iceberg_1_10"} +OPT_IN = ( + "run-spark-3.4-tests", "run-spark-3.5-tests", "run-spark-4.0-tests", + "run-spark-4.1-tests", "run-spark-4.1-hive-tests", "run-iceberg-tests", + "run-all-spark-profiles", +) + + +class NativeBuildSelectionTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + """Load one routing module for this suite; import errors fail setup. + + Store it on the test class. Individual policy/filter patches restore + this module's dictionaries when their context exits, including failure. + """ + spec = importlib.util.spec_from_file_location( + "compute_changes", ROOT / "dev/ci/compute-changes.py" + ) + cls.filters = importlib.util.module_from_spec(spec) + spec.loader.exec_module(cls.filters) + + def cli_outputs(self, files, event): + """Return validated boolean outputs from one real CLI invocation. + + Pass repository-relative `files` through a temporary text file and the + event fields through a child-only environment. Neither input nor the + parent environment changes. The temporary file closes on success or + failure; a failed process raises, and malformed or missing outputs fail + assertions rather than being interpreted as a skipped native build. + """ + env = { + **os.environ, + "EVENT_NAME": event["name"], + "EVENT_ACTION": event.get("action", ""), + "LABEL_NAME": event.get("label", ""), + "PR_LABELS": json.dumps(event.get("labels", [])), + } + with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8") as changed: + changed.write("\n".join(files)) + changed.flush() + result = subprocess.run( + [sys.executable, str(ROOT / "dev/ci/compute-changes.py"), changed.name], + env=env, + check=True, + capture_output=True, + text=True, + ) + flags = {} + for line in result.stdout.splitlines(): + key, value = line.split("=", 1) + self.assertIn(value, ("true", "false")) + self.assertNotIn(key, flags) + flags[key] = value == "true" + self.assertEqual(set(flags), set(self.filters.FILTERS) | {"build_linux_native"}) + return flags + + def assert_selected(self, flags, expected): + """Assert consumer job IDs and producer selection without mutating inputs. + + `flags` is the complete output-to-bool mapping; `expected` contains CI + consumer job IDs, whose possible output keys are pinned in CONSUMERS. + A caller is selected if any of its routes is true. Return + None on success; any missing key or selection mismatch fails the test. + """ + self.assertEqual( + {job for job, routes in CONSUMERS.items() if any(flags[key] for key in routes)}, + expected, + ) + self.assertEqual(flags["build_linux_native"], bool(expected)) + + def assert_selection( + self, files, expected, event="pull_request", action="synchronize", labels=(), label="" + ): + """Check compute() for changed paths, event fields, and expected job IDs. + + Build a fresh event mapping from the supplied values and assert the + complete native selection. No caller input changes; return None or + propagate the computation/assertion failure. CLI behavior is exercised + separately so both entry points cover manual dispatch's empty input. + """ + event = {"name": event, "action": action, "labels": labels, "label": label} + flags = self.filters.compute(files, event) + self.assert_selected(flags, expected) + + def test_native_change_uses_default_pr_coverage(self): + self.assert_selection(["native/core/src/lib.rs"], DEFAULT) + + def test_consumer_mapping_matches_actual_caller_routes(self): + """Pin caller IDs and all of their routes independently of production data. + + Compare ordered items with the reviewed fixture without mutating either + mapping. A missing, extra, reordered, or misassigned route fails the test. + """ + self.assertEqual(list(self.filters.NATIVE_CONSUMERS.items()), list(CONSUMERS.items())) + + def test_spark_4_1_core_and_hive_event_routes(self): + """Check both Spark 4.1 output flags and the producer through both APIs. + + A Spark 4.1 patch isolates this caller. Ordinary PRs select neither, + the Hive label selects Hive only, and the full-suite label, queue, + and manual dispatch select both. Fixtures remain unchanged; + CLI inputs are temporary, and any routing mismatch fails assertions. + """ + files = ["dev/diffs/4.1.3.diff"] + hive_label = "run-spark-4.1-hive-tests" + suite_label = "run-spark-4.1-tests" + cases = ( + ({"name": "pull_request", "action": "synchronize"}, False, False), + ({"name": "pull_request", "action": "labeled", "label": hive_label, + "labels": [hive_label]}, False, True), + ({"name": "pull_request", "action": "synchronize", "labels": [hive_label]}, + False, True), + ({"name": "pull_request", "action": "labeled", "label": suite_label, + "labels": [suite_label]}, True, True), + ({"name": "pull_request", "action": "synchronize", "labels": [suite_label]}, + True, True), + ({"name": "merge_group"}, True, True), + ({"name": "schedule"}, False, False), + ({"name": "workflow_dispatch"}, True, True), + ) + for event, core, hive in cases: + for invoke in (self.filters.compute, self.cli_outputs): + with self.subTest(event=event, api=invoke.__name__): + flags = invoke(files, event) + self.assertEqual(flags["spark_4_1"], core) + self.assertEqual(flags["spark_4_1_hive"], hive) + self.assert_selected( + flags, + set(CONSUMERS) if event["name"] == "workflow_dispatch" + else {"spark_4_1"} if core or hive else set(), + ) + + def test_docs_and_benchmarks_do_not_build_native(self): + for path in ("docs/source/user-guide/overview.md", "native/core/benches/parquet_read.rs"): + with self.subTest(path=path): + self.assert_selection([path], set()) + + def test_spark_patch_does_not_require_linux_test_workflow(self): + """Assert a queued Spark patch selects native without Linux tests.""" + self.assert_selection(["dev/diffs/4.1.3.diff"], {"spark_4_1"}, event="merge_group") + + def test_legacy_patch_needs_opt_in(self): + """Assert older Spark patches need their label on ordinary PR runs. + + Each repository-relative patch path is tested with and without its + matching label, including deprecated Spark 3.4, which is excluded from + the merge queue. Inputs and routing tables stay unchanged; a mismatch + in either the consumer or producer selection fails the assertion. + """ + for version, job, label in ( + ("3.4.3", "spark_3_4", "run-spark-3.4-tests"), + ("3.5.9", "spark_3_5", "run-spark-3.5-tests"), + ("4.0.4", "spark_4_0", "run-spark-4.0-tests"), + ): + with self.subTest(version=version): + files = [f"dev/diffs/{version}.diff"] + self.assert_selection(files, set()) + self.assert_selection(files, {job}, labels=(label,)) + + def test_unrelated_label_does_not_duplicate_existing_runs(self): + self.assert_selection( + ["native/core/src/lib.rs"], set(), action="labeled", + labels=(*OPT_IN, "dependencies"), label="dependencies", + ) + + def test_new_spark_label_runs_only_selected_version(self): + """Assert each new Spark label selects only its consumer and native build. + + Use a shared native source path with every opt-in label present, so + the event's newly added label must narrow the selection. No fixtures + are mutated; incorrect selection fails through assert_selection(). + """ + for job, label in ( + ("spark_3_4", "run-spark-3.4-tests"), + ("spark_3_5", "run-spark-3.5-tests"), + ("spark_4_0", "run-spark-4.0-tests"), + ("spark_4_1", "run-spark-4.1-tests"), + ("spark_4_1", "run-spark-4.1-hive-tests"), + ): + with self.subTest(label=label): + self.assert_selection( + ["native/core/src/lib.rs"], {job}, action="labeled", + labels=OPT_IN, label=label, + ) + + def test_nonconsumer_labels_do_not_build_native(self): + """Assert independent label runs select no shared native build. + + All real routes match the changed paths, and all consumer opt-ins are + present. Check that the newly labeled route runs while every native + consumer stays off. No input or routing configuration is changed. + """ + for label, key in ( + ("run-macos-tests", "build_macos"), + ("run-benchmark-check", "benchmark"), + ("run-delta-build-gate", "delta_gate"), + ("run-pyarrow-udf-tests", "pyarrow_udf"), + ): + with self.subTest(label=label): + flags = self.filters.compute( + ["pom.xml", "native/core/benches/parquet_read.rs"], + {"name": "pull_request", "action": "labeled", + "labels": (*OPT_IN, label), "label": label}, + ) + self.assertTrue(flags[key]) + self.assert_selected(flags, set()) + + def test_new_iceberg_label_runs_all_iceberg_versions(self): + self.assert_selection( + ["native/core/src/lib.rs"], + {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"}, + action="labeled", labels=OPT_IN, label="run-iceberg-tests", + ) + + def test_merge_queue_uses_default_versions(self): + """Assert queue selection uses default versions through compute() and CLI. + + A shared native edit selects the three default callers and the producer; + a Spark 3.4-only patch selects no caller and skips the producer. + Supplying opt-in labels cannot restore deprecated coverage to a queue + event. Fixtures and policy stay unchanged, cli_outputs() owns and + removes temporary inputs, and every mismatch fails an assertion. + """ + for files, expected in ( + (["native/core/src/lib.rs"], QUEUE), + (["dev/diffs/3.4.3.diff"], set()), + ): + for labels in ((), OPT_IN): + event = {"name": "merge_group", "labels": labels} + for invoke in (self.filters.compute, self.cli_outputs): + with self.subTest(files=files, labels=labels, api=invoke.__name__): + flags = invoke(files, event) + self.assertFalse(flags["spark_3_4"]) + self.assert_selected(flags, expected) + + def test_nightly_runs_other_versions(self): + """Assert nightly callers, including Linux profiles, get their producer.""" + for files, expected in ( + (["native/core/src/lib.rs"], NIGHTLY), + (["common/src/test/ExampleTest.java"], {"pr_build_linux"}), + (["dev/diffs/3.4.3.diff"], set()), + ): + event = {"name": "schedule"} + for invoke in (self.filters.compute, self.cli_outputs): + with self.subTest(files=files, api=invoke.__name__): + flags = invoke(files, event) + self.assertFalse(flags["build_linux"]) + self.assert_selected(flags, expected) + + def test_all_profiles_label_selects_linux_producer(self): + """Assert the new all-profile label selects Linux without its default route.""" + event = { + "name": "pull_request", "action": "labeled", + "labels": OPT_IN, "label": "run-all-spark-profiles", + } + for invoke in (self.filters.compute, self.cli_outputs): + with self.subTest(api=invoke.__name__): + flags = invoke(["native/core/src/lib.rs"], event) + self.assertFalse(flags["build_linux"]) + self.assertTrue(flags["build_linux_all_profiles"]) + self.assert_selected(flags, {"pr_build_linux"}) + + def test_main_runs_only_linux_consumers_to_refresh_caches(self): + """Assert push selects the Linux consumer and native build in both APIs. + + Main's cache refresh needs the shared producer, while other test + consumers stay off. The native source path and event are read-only; + cli_outputs() owns and cleans up its temporary changed-files input. + """ + files = ["native/core/src/lib.rs"] + self.assert_selection(files, {"pr_build_linux"}, event="push") + self.assert_selected(self.cli_outputs(files, {"name": "push"}), {"pr_build_linux"}) + + def test_manual_runs_include_legacy_consumers_without_changed_files(self): + """Assert empty-input dispatch selects all nine callers in both APIs. + + Deprecated Spark 3.4 remains runnable manually even though it is absent + from the merge queue. Inputs and routing policy stay unchanged; the CLI + helper removes temporary inputs and a missing output fails assertions. + """ + self.assert_selection([], set(CONSUMERS), event="workflow_dispatch") + flags = self.cli_outputs([], {"name": "workflow_dispatch"}) + self.assert_selected(flags, set(CONSUMERS)) + self.assertTrue(all(flags.values())) + + def test_empty_changes_and_unsupported_events_skip_native(self): + """Assert ordinary empty diffs and unsupported events select no consumers.""" + self.assert_selection([], set()) + self.assert_selection(["native/core/src/lib.rs"], set(), event="repository_dispatch") + + def test_nonconsumer_outputs_do_not_select_native(self): + """Select each unrelated route alone and ensure it cannot start native CI. + + Temporarily give every filter a distinct synthetic path to separate + macOS from its normally overlapping Linux inputs. Use each route's + permitted event so the assertion checks an active unrelated job. The + patch restores the real filters on exit, including assertion failure. + """ + filters = {key: [key] for key in self.filters.FILTERS} + with mock.patch.dict(self.filters.FILTERS, filters, clear=True): + for key, event in ( + ("build_macos", "merge_group"), ("benchmark", "merge_group"), + ("delta_gate", "merge_group"), ("pyarrow_udf", "merge_group"), ("docs", "push") + ): + with self.subTest(key=key): + flags = self.filters.compute([key], {"name": event}) + self.assertEqual({name for name, selected in flags.items() if selected}, {key}) + + def test_cli_emits_native_output_for_selected_and_skipped_runs(self): + """Assert real CLI output includes the producer on both true and false paths.""" + event = {"name": "pull_request", "action": "synchronize", "labels": []} + for files, expected in ((["native/core/src/lib.rs"], DEFAULT), ([], set())): + with self.subTest(files=files): + self.assert_selected(self.cli_outputs(files, event), expected) + + def test_producer_change_exercises_all_default_linux_consumers(self): + self.assert_selection([".github/workflows/build_linux_native.yml"], DEFAULT) + + def test_independent_checks_change_selects_linux_checks_and_tests(self): + self.assert_selection( + [".github/workflows/pr_build_linux_checks.yml"], {"pr_build_linux"} + ) + + def test_label_event_cli_uses_only_the_new_gating_label(self): + """Assert the CLI derives native selection from the new label only. + + Exercise deprecated Spark 3.4's retained opt-in, Spark 3.5's nightly + opt-in, Spark 4.1's core/Hive routes, Iceberg's grouped opt-in, and an + unrelated label. cli_outputs() cleans up the child environment and + temporary file; routing tables remain unchanged and mismatches fail. + """ + for label, expected in ( + ("run-spark-3.4-tests", {"spark_3_4"}), + ("run-spark-3.5-tests", {"spark_3_5"}), + ("run-spark-4.1-tests", {"spark_4_1"}), + ("run-spark-4.1-hive-tests", {"spark_4_1"}), + ("run-iceberg-tests", {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"}), + ("run-all-spark-profiles", {"pr_build_linux"}), + ("dependencies", set()), + ): + with self.subTest(label=label): + event = { + "name": "pull_request", + "action": "labeled", + "labels": [*OPT_IN, "dependencies"], + "label": label, + } + flags = self.cli_outputs(["native/core/src/lib.rs"], event) + self.assert_selected(flags, expected) + + def test_producer_follows_policy_changes_without_workflow_edits(self): + with mock.patch.dict(self.filters.POLICY, {"spark_3_4": ["pr", "push"]}): + self.assert_selection(["dev/diffs/3.4.3.diff"], {"spark_3_4"}) + with mock.patch.dict( + self.filters.POLICY, {"build_linux": ["push", "label:run-linux-tests"]} + ): + files = ["common/src/test/ExampleTest.java"] + self.assert_selection(files, set()) + self.assert_selection( + files, {"pr_build_linux"}, action="labeled", + labels=("run-linux-tests",), label="run-linux-tests", + ) + + def test_producer_output_matches_all_consumer_combinations(self): + """Exhaust all 2,048 output path masks across event and label routes. + + Synthetic one-path filters exercise compute() without relying on real + paths overlapping particular consumers. Unrelated routes also match, + guarding against accidentally including them in the native union. Only + FILTERS is patched, and it is restored on success or assertion failure; + the actual event policy and native-output computation always execute. + Linux profiles and Spark core/Hive use independent synthetic paths to + exercise either route alone, despite their identical real filters. + Include absent, individual and combined opt-in labels, each newly + added label, the merge queue, cache-refresh push, manual dispatch, + nightly schedule and an unsupported event. Exhaustive label subsets + would repeat the same independent per-job decisions. + """ + keys = [key for routes in CONSUMERS.values() for key in routes] + events = [ + {"name": "merge_group"}, {"name": "push"}, + {"name": "workflow_dispatch"}, {"name": "schedule"}, + {"name": "repository_dispatch"}, + ] + for labels in ((), OPT_IN): + for action in ("opened", "synchronize", "reopened"): + events.append({"name": "pull_request", "action": action, "labels": labels}) + for label in OPT_IN: + events.append({ + "name": "pull_request", "action": "synchronize", "labels": (label,), + }) + for label in (*OPT_IN, "dependencies"): + events.append({ + "name": "pull_request", "action": "labeled", + "labels": OPT_IN, "label": label, + }) + filters = {key: [key] for key in self.filters.FILTERS} + unrelated = sorted(set(filters) - set(keys)) + with mock.patch.dict(self.filters.FILTERS, filters, clear=True): + for mask in itertools.product((False, True), repeat=len(keys)): + raw_flags = dict(zip(keys, mask)) + files = [key for key, selected in raw_flags.items() if selected] + unrelated + for event in events: + flags = self.filters.compute(files, event) + expected = { + job for job, routes in CONSUMERS.items() + if event["name"] == "workflow_dispatch" or any( + raw_flags[key] and self.filters.event_allows(key, event) + for key in routes + ) + } + self.assert_selected(flags, expected) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/source/contributor-guide/adding_a_new_spark_version.md b/docs/source/contributor-guide/adding_a_new_spark_version.md index 64f90bec5c..1edb47d03f 100644 --- a/docs/source/contributor-guide/adding_a_new_spark_version.md +++ b/docs/source/contributor-guide/adding_a_new_spark_version.md @@ -109,7 +109,7 @@ logic and to skip tests. Add the matching helper for the new version ### Add a Compile-Only CI Job -Edit `.github/workflows/pr_build_linux.yml` and `pr_build_macos.yml` to add +Edit `.github/workflows/pr_build_linux_checks.yml` and `pr_build_macos.yml` to add the new Spark version to the `build-spark` (or equivalent compile-only) job matrix. Do not add it to the heavier test matrices yet. A compile-only job keeps the CI cost of stage 1 small and prevents test failures on the new diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index 20f2eedf80..2779b459ad 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -59,13 +59,18 @@ is the Linux build, which also runs on push so that the dependency caches on `ma pull request can only restore caches saved on its own branch or on `main`, and the queue's temporary branch takes its caches with it when it is deleted. -That push run is for the caches and nothing else, so it runs in **cache-refresh-only** mode: only -the four jobs that own a cache entry (the native CI build, the Rust tests, and the two TPC-H/TPC-DS -jobs, the last two stopping before their query passes), plus the short `Lint` job the native jobs -depend on. The lints and the Comet test matrix are skipped, which is the difference between 587 -runner-minutes a push and about 73. If you add a job to `pr_build_linux.yml`, give it -`if: ${{ !inputs.cache-refresh-only }}` unless it writes a cache that `main` needs; -`dev/ci/check-ci-config.py` fails the build if you forget. +That push run uses **cache-refresh-only** mode across three Linux workflows. +`build_linux_native.yml` builds the shared library and refreshes the native CI cache. +`pr_build_linux_checks.yml` runs Rust formatting and Rust tests to refresh the debug cache. +`pr_build_linux.yml` runs the two TPC-H/TPC-DS jobs through dataset generation and Maven +cache population, then skips their query passes. Java/Scala lints, compile-only and Celeborn +checks, profile selection, and the JVM test matrix are skipped. + +When adding a job to either `pr_build_linux.yml` or `pr_build_linux_checks.yml`, give it +`if: ${{ !inputs.cache-refresh-only }}` unless it is needed to refresh a cache on `main`. +Also preserve the `profiles: nightly` guards so nightly runs do not repeat checks already run +at that commit. `dev/ci/check-ci-config.py` records the allowed jobs by workflow across all +three files and verifies that both Linux callers pass the mode inputs. Spark 3.4 is the one suite in neither tier. [Spark 3.4 support is deprecated](../user-guide/latest/compatibility/spark-versions.md#spark-34), so its Spark SQL suite no longer gates a merge. It remains available on demand: apply the @@ -258,8 +263,9 @@ The umbrella workflow, the reusable workflows it calls, and the routing tables a `dev/ci/check-ci-config.py`, which runs in preflight. It enforces that every job feeds `Required Checks`, that the required check name in `.asf.yaml` matches the job that publishes it, that artifact names are unique per producer, that the routing policy matches its test cases, and -that every job in `pr_build_linux.yml` is either a cache writer or skipped on push. Run it locally -before pushing a CI change: +that the three Linux workflows preserve cache-refresh and nightly job selection. It also checks +that both Linux callers pass the mode inputs, so moving a job cannot silently restore work on +push. Run it locally before pushing a CI change: ```sh python3 dev/ci/check-ci-config.py