diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 43b5be5a5c..163adf54c4 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -22,87 +22,92 @@ ruleset in `.asf.yaml`. That splits CI into two tiers: against the merge result rather than against the PR head. Every queue-only job has a `run-*` label that opts a pull request into it -early, listed in the diagram below. +early, listed in the table below. -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-registry, Maven and TPC-H/TPC-DS caches on `main` stale until the next unrelated change. ``` - pull_request | merge_group | push to main | workflow_dispatch +pull_request | merge_group | push to main | workflow_dispatch + | + preflight + | + changes + | + +---------------+--------------------+ + | | + Linux checks / macOS / build_linux_native + docs / benchmark (if selected) (if any consumer is selected) | - 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, for cache) docs pr_build_macos run-macos-tests - spark_4_1 (catalyst + sql_core) pr_benchmark_check run-benchmark-check - iceberg_1_11 spark_4_1 sql_hive run-spark-4.1-hive-tests - spark_3_4 run-spark-3.4-tests - 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 | - +-----------------------+ - - 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 - docs.yaml + +----------------+----------------+ + | | | + pr_build_linux spark_3_* / iceberg_1_* + spark_4_* + +Every job above reports to required_checks (except the docs deployment). ``` +`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, 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 writes on `main`. + ## What runs when -| Job in `ci.yml` | Triggered by | Routing rule | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | -| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | -| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | -| `pr_build_linux` | PR, merge group or push to main, paths matched | `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 | -| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | -| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | -| `spark_4_1` | PR or merge group, paths matched; the `sql_hive` shards only in the merge group **or** with `run-spark-4.1-hive-tests` | Spark 4.1 sources | -| `spark_3_4` | merge group, **or** PR with `run-spark-3.4-tests` | Spark 3.4 sources | -| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | -| `iceberg_1_11` | PR or merge group, paths matched | Iceberg sources | -| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `required_checks` | always, after every job above except `docs` | none (always runs) | +| Job in `ci.yml` | Triggered by | Routing rule | +| ----------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | +| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | +| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | +| `build_linux_native` | any selected Linux/Spark/Iceberg consumer | `dev/ci/compute-changes.py` | +| `pr_build_linux_checks` | PR, merge group or push to main, paths matched | `dev/ci/compute-changes.py` | +| `pr_build_linux` | PR, merge group or push to main, paths matched | `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 | +| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | +| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | +| `spark_4_1` | PR or merge group, paths matched; the `sql_hive` shards only in the merge group **or** with `run-spark-4.1-hive-tests` | Spark 4.1 sources | +| `spark_3_4` | merge group, **or** PR with `run-spark-3.4-tests` | Spark 3.4 sources | +| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | +| `iceberg_1_11` | PR or merge group, paths matched | Iceberg sources | +| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `required_checks` | always, after every job above except `docs` | none (always runs) | 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 @@ -112,12 +117,14 @@ 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-hive-tests`, `run-iceberg-tests`, `run-macos-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 @@ -163,6 +170,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` | @@ -172,13 +181,26 @@ 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. 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 @@ -205,22 +227,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 @@ -294,6 +318,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..08c11a0de4 --- /dev/null +++ b/.github/workflows/build_linux_native.yml @@ -0,0 +1,85 @@ +# 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 + 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') }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd3a0aa99c..52ab6d2a9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -115,7 +115,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 @@ -153,6 +156,7 @@ jobs: needs: preflight runs-on: ubuntu-slim outputs: + build_linux_native: ${{ steps.compute.outputs.build_linux_native }} build_linux: ${{ steps.compute.outputs.build_linux }} build_macos: ${{ steps.compute.outputs.build_macos }} benchmark: ${{ steps.compute.outputs.benchmark }} @@ -225,11 +229,31 @@ 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' + uses: ./.github/workflows/pr_build_linux_checks.yml + + # 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 + needs: [changes, build_linux_native] if: needs.changes.outputs.build_linux == 'true' uses: ./.github/workflows/pr_build_linux.yml + with: + native-library-artifact: native-lib-linux pr_build_macos: name: PR Build (macOS) @@ -254,42 +278,45 @@ jobs: spark_3_4: name: Spark SQL Tests (Spark 3.4) - needs: changes + needs: [changes, build_linux_native] # Queue-only by default; PRs need the `run-spark-3.4-tests` label. 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: 11 spark_3_5: name: Spark SQL Tests (Spark 3.5) - needs: changes + needs: [changes, build_linux_native] # Queue-only 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] # Queue-only by default; PRs need the `run-spark-4.0-tests` label. Swapped # with spark_4_1 on the `oom` branch to validate the memory caps against # Spark 4.1 by default. 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] # Two POLICY outputs feed one call, so the queue gets every module from a # single 40-minute build instead of two. `spark_4_1` (PR tier) covers # catalyst and the sql_core shards; `spark_4_1_hive` (queue-only, or the @@ -298,6 +325,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 @@ -308,11 +336,12 @@ jobs: iceberg_1_8: name: Iceberg Spark SQL Tests (Iceberg 1.8) - needs: changes + needs: [changes, build_linux_native] # Queue-only 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' @@ -321,11 +350,12 @@ jobs: iceberg_1_9: name: Iceberg Spark SQL Tests (Iceberg 1.9) - needs: changes + needs: [changes, build_linux_native] # Queue-only 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' @@ -334,12 +364,13 @@ jobs: iceberg_1_10: name: Iceberg Spark SQL Tests (Iceberg 1.10) - needs: changes + needs: [changes, build_linux_native] # Queue-only by default; PRs need the `run-iceberg-tests` label. Iceberg 1.11 # (Spark 4.1) is the PR-gated Iceberg job; 1.10 covers the Spark 3.5 path. 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' @@ -348,11 +379,12 @@ jobs: iceberg_1_11: name: Iceberg Spark SQL Tests (Iceberg 1.11) - needs: changes + needs: [changes, build_linux_native] # Runs on every PR: Iceberg 1.11 is our only Spark 4.1 Iceberg coverage. 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' @@ -398,6 +430,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 4cd30e66cd..5945fbbd5d 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -21,6 +21,11 @@ name: PR Build (Linux) # live in the umbrella workflow. on: workflow_call: + inputs: + native-library-artifact: + description: 'Default Linux CI library produced by the calling workflow' + required: true + type: string env: RUST_VERSION: stable @@ -32,316 +37,8 @@ env: jobs: - # Fast lint check - gates all other jobs - 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: - 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: - needs: lint - name: Lint Java (${{ matrix.profile.name }}) - runs-on: ubuntu-24.04 - container: - image: amd64/rust - env: - JAVA_TOOL_OPTIONS: ${{ (matrix.profile.java_version == '17' || matrix.profile.java_version == '21') && '--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 11, Scala 2.12" - java_version: "11" - 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 - 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 - 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 - 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 - 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') }} - - # Run Rust tests (runs in parallel with build-native, uses debug builds) - linux-test-rust: - 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 - if: github.ref == 'refs/heads/main' - 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: - needs: build-native strategy: matrix: # the goal with these profiles is to get coverage of all Java, Scala, and Spark @@ -527,7 +224,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/ @@ -553,7 +250,6 @@ jobs: # TPC-H correctness test - verifies benchmark queries produce correct results verify-benchmark-results-tpch: - needs: build-native name: Verify TPC-H Results runs-on: ubuntu-24.04 container: @@ -572,7 +268,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 @@ -613,7 +309,6 @@ jobs: # TPC-DS correctness tests - verifies benchmark queries produce correct results. # The three join strategies run sequentially in one job so the project is built once. verify-benchmark-results-tpcds: - needs: build-native name: Verify TPC-DS Results runs-on: ubuntu-24.04 container: @@ -632,7 +327,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..a68bed26e0 --- /dev/null +++ b/.github/workflows/pr_build_linux_checks.yml @@ -0,0 +1,293 @@ +# 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: + +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: + + # Rust formatting gates the build and test jobs below + 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: + 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: + needs: lint + name: Lint Java (${{ matrix.profile.name }}) + runs-on: ubuntu-24.04 + container: + image: amd64/rust + env: + JAVA_TOOL_OPTIONS: ${{ (matrix.profile.java_version == '17' || matrix.profile.java_version == '21') && '--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 11, Scala 2.12" + java_version: "11" + 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 + 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 + 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, not the shared CI library. + linux-test-rust: + 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 + if: github.ref == 'refs/heads/main' + 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 6ccd22979a..ad60145f72 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 five 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,6 +48,12 @@ # run only under an input or a label can carry that for a long time # before anyone runs them. # +# 6. Independent Linux checks. Lint, compile-only checks and debug Rust +# tests must remain runnable without waiting for the native CI build. +# +# 7. 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 import importlib.util @@ -104,8 +108,13 @@ # 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"}), + # 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"}), # 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"}), @@ -226,9 +235,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 @@ -238,6 +245,26 @@ 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", +} + + +LINUX_CHECKS_WORKFLOW = "pr_build_linux_checks.yml" +LINUX_CHECKS_JOB = "pr_build_linux_checks" +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(): @@ -330,50 +357,325 @@ 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 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 diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 3380a1e443..9e2cf91751 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/**", @@ -108,6 +110,7 @@ "spark/src/main/spark-*/**", ], "spark_3_4": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -135,6 +138,7 @@ "mvnw", ], "spark_3_5": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -162,6 +166,7 @@ "mvnw", ], "spark_4_0": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -189,6 +194,7 @@ "mvnw", ], "spark_4_1": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -221,6 +227,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", @@ -245,6 +252,7 @@ "mvnw", ], "iceberg_1_9": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -269,6 +277,7 @@ "mvnw", ], "iceberg_1_10": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -293,6 +302,7 @@ "mvnw", ], "iceberg_1_11": [ + ".github/workflows/build_linux_native.yml", "native/**/src/**", "native/**/Cargo.toml", "native/Cargo.lock", @@ -374,6 +384,23 @@ "iceberg_1_11": ["pr", "queue"], } +# 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. +# Spark 4.1 has two routes into one caller: core shards and opt-in Hive shards. +# 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",), + "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:")] @@ -416,11 +443,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(): @@ -475,12 +517,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..a5d747671b --- /dev/null +++ b/dev/ci/test-ci-config.py @@ -0,0 +1,450 @@ +# 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), []) + + 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_hive_route_must_remain_in_native_selector(self): + """Reject omitting Hive from Python while ci.yml still selects it. + + 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. + """ + selector = CHECK.load_filters() + selector.NATIVE_CONSUMERS = {**selector.NATIVE_CONSUMERS, "spark_4_1": ("spark_4_1",)} + with mock.patch.object(CHECK, "load_filters", return_value=selector): + self.assert_rejected("spark_4_1 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", "type: string", "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, " workflow_call:\n", + " workflow_call:\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..e6756a7e5e --- /dev/null +++ b/dev/ci/test-native-build-selection.py @@ -0,0 +1,402 @@ +# 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",), + "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", "spark_4_1", "iceberg_1_11"} +OPT_IN = ( + "run-spark-3.4-tests", "run-spark-3.5-tests", "run-spark-4.0-tests", + "run-spark-4.1-hive-tests", "run-iceberg-tests", +) + + +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 core only, + the Hive label selects Hive only, and subsequent labeled-PR updates, + the 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" + cases = ( + ({"name": "pull_request", "action": "synchronize"}, True, False), + ({"name": "pull_request", "action": "labeled", "label": hive_label, + "labels": [hive_label]}, False, True), + ({"name": "pull_request", "action": "synchronize", "labels": [hive_label]}, + True, True), + ({"name": "merge_group"}, True, True), + ({"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"}, + ) + + 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 default-tier Spark patch selects native without Linux tests.""" + self.assert_selection(["dev/diffs/4.1.3.diff"], {"spark_4_1"}) + + def test_legacy_patch_needs_opt_in(self): + """Assert queue-tier Spark patches need their label on ordinary PR runs. + + Each repository-relative patch path is tested with and without its + matching label. 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-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 macOS and benchmark label runs select no shared native build. + + Both 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"), + ): + with self.subTest(label=label): + flags = self.filters.compute( + ["native/core/src/lib.rs", "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_only_opt_in_versions(self): + self.assert_selection( + ["native/core/src/lib.rs"], {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10"}, + action="labeled", labels=OPT_IN, label="run-iceberg-tests", + ) + + def test_merge_queue_runs_include_legacy_consumers(self): + """Assert the queue selects all native consumers in compute() and CLI. + + A native source edit matches every consumer without opt-in labels. + Check both entry points; the CLI's temporary inputs are cleaned up + by cli_outputs(), and neither check changes the routing policy. + """ + files = ["native/core/src/lib.rs"] + self.assert_selection(files, set(CONSUMERS), event="merge_group") + self.assert_selected(self.cli_outputs(files, {"name": "merge_group"}), set(CONSUMERS)) + + 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 queue-only 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 every consumer in compute and CLI.""" + 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="schedule") + + 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"), ("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 Spark 3.5's queue opt-in, Spark 4.1's Hive-only route, Iceberg's + grouped opt-in, and an unrelated label. cli_outputs() cleans up the child + environment and temporary file; routing tables remain unchanged. + """ + for label, expected in ( + ("run-spark-3.5-tests", {"spark_3_5"}), + ("run-spark-4.1-hive-tests", {"spark_4_1"}), + ("run-iceberg-tests", {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10"}), + ("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 1,024 output path masks across event and label combinations. + + 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. + Core and Hive use independent synthetic paths to exercise either route + alone, even though their real path filters are identical. + Include every subset of native opt-in labels and the merge queue, + cache-refresh push, manual dispatch, and unsupported schedule events. + """ + keys = [key for routes in CONSUMERS.values() for key in routes] + label_sets = [ + tuple(label for label, selected in zip(OPT_IN, mask) if selected) + for mask in itertools.product((False, True), repeat=len(OPT_IN)) + ] + events = [ + {"name": "merge_group"}, {"name": "push"}, + {"name": "workflow_dispatch"}, {"name": "schedule"}, + ] + for labels in label_sets: + for action in ("opened", "synchronize", "reopened"): + events.append({"name": "pull_request", "action": action, "labels": labels}) + for label in (*OPT_IN, "dependencies"): + events.append({ + "name": "pull_request", "action": "labeled", + "labels": labels, "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()