From 41957c37779f5e1caed11d674091e994daec77c3 Mon Sep 17 00:00:00 2001 From: comphead Date: Tue, 8 Sep 2026 16:49:26 -0700 Subject: [PATCH] chore: run Preflight mandatory --- .github/workflows/README.md | 103 ++++++++++++++++++++++++------------ .github/workflows/ci.yml | 63 ++++++++++++++-------- 2 files changed, 111 insertions(+), 55 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 8a8791e7118..1953248a8ba 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -30,16 +30,17 @@ is a `workflow_call` reusable invoked from the umbrella. | heavy job) | +-----------+-----------+ | - +-----------+-----------+-----------+-----------+-----------+-----------+ - | | | | | | | - v v v v v v v - pr_build_ pr_build_ pr_benchmark_ docs spark_3_5 spark_4_0 iceberg_1_10 - linux macos check (push) (PR+push) (PR+push) (PR+push) - (PR+push) (PR+push) (PR+push) - | | | - v v v - spark_3_4 / spark_4_1 iceberg_1_8 / 1_9 - (push or PR + label) (push only) + +-----------------------------------+-----------------------------------+ + | | | + v v v + every PR + push push to main only PR with label, or push + --------------- ----------------- ---------------------- + pr_build_linux docs spark_3_4 run-spark-3.4-tests + pr_build_macos spark_4_0 run-spark-4.0-tests + pr_benchmark_check iceberg_1_8 run-iceberg-tests + spark_3_5 iceberg_1_9 run-iceberg-tests + spark_4_1 iceberg_1_10 run-iceberg-tests + iceberg_1_11 reusable workflows invoked via `uses:`: pr_build_linux.yml spark_sql_test_reusable.yml @@ -50,25 +51,50 @@ is a `workflow_call` reusable invoked from the umbrella. ## What runs when -| Job in `ci.yml` | Triggered by | Path filter source | -| -------------------- | ------------------------------------------------ | ----------------------------------- | -| `preflight` | every PR / push to main / dispatch | none (always runs) | -| `changes` | every PR / push to main / dispatch | runs `dev/ci/compute-changes.py` | -| `pr_build_linux` | PR or push, paths matched | `dev/ci/compute-changes.py` | -| `pr_build_macos` | PR or push, paths matched | `dev/ci/compute-changes.py` | -| `pr_benchmark_check` | PR or push, paths matched | benchmark sources only | -| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | -| `spark_3_5` | PR or push, paths matched | Spark 3.5 sources | -| `spark_4_0` | PR or push, paths matched | Spark 4.0 sources | -| `spark_3_4` | push, **or** PR with `run-spark-3.4-tests` label | Spark 3.4 sources | -| `spark_4_1` | push, **or** PR with `run-spark-4.1-tests` label | Spark 4.1 sources | -| `iceberg_1_10` | PR or push, paths matched | Iceberg sources | -| `iceberg_1_8` | push only | Iceberg sources | -| `iceberg_1_9` | push only | Iceberg sources | +| Job in `ci.yml` | Triggered by | Path filter source | +| -------------------- | --------------------------------------------------- | ----------------------------------- | +| `preflight` | every PR / push to main / dispatch / PR label added | none (always runs) | +| `changes` | every PR / push to main / dispatch / PR label added | runs `dev/ci/compute-changes.py` | +| `pr_build_linux` | PR or push, paths matched | `dev/ci/compute-changes.py` | +| `pr_build_macos` | PR or push, paths matched | `dev/ci/compute-changes.py` | +| `pr_benchmark_check` | PR or push, paths matched | benchmark sources only | +| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | +| `spark_3_5` | PR or push, paths matched | Spark 3.5 sources | +| `spark_4_1` | PR or push, paths matched | Spark 4.1 sources | +| `spark_3_4` | push, **or** PR with `run-spark-3.4-tests` label | Spark 3.4 sources | +| `spark_4_0` | push, **or** PR with `run-spark-4.0-tests` label | Spark 4.0 sources | +| `iceberg_1_11` | PR or push, paths matched | Iceberg sources | +| `iceberg_1_8` | push, **or** PR with `run-iceberg-tests` label | Iceberg sources | +| `iceberg_1_9` | push, **or** PR with `run-iceberg-tests` label | Iceberg sources | +| `iceberg_1_10` | push, **or** PR with `run-iceberg-tests` label | Iceberg sources | 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 -passing for branch protection. +passing for branch protection, so a name that can report `skipped` is not +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 +`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: + +- `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 + `skipped`, and the newest check run for a name is what the merge box, + `gh pr checks` and required-status-check evaluation read. Guarding + `preflight` on the label name used to let any unrelated label overwrite the + commit run's real `Preflight` verdict with `skipped`, see + [#5007](https://github.com/apache/datafusion-comet/issues/5007). +- Every heavy job excludes `labeled` events unless the label just added is the + one that gates it. Without that, applying a single label re-ran the entire + heavy pipeline at a commit that had already been tested. + +`run-spark-4.1-tests` gates nothing: `spark_4_1` already runs on every PR. ## Standalone workflows (not under the umbrella) @@ -83,17 +109,18 @@ umbrella doesn't watch, or operate independently of the rest of CI: | `stale.yml` | Daily stale-PR closer. | | `take.yml` | Issue-comment trigger for `take` / `untake`. | | `label_new_issues.yml` | Issue trigger to apply `requires-triage`. | +| `label_prs.yml` | Runs on `pull_request_target` so it can label pull requests opened from forks. | ## Reusable workflows (called by `ci.yml`) -| File | Called from `ci.yml` job(s) | -| --------------------------------- | -------------------------------------------------- | -| `pr_build_linux.yml` | `pr_build_linux` | -| `pr_build_macos.yml` | `pr_build_macos` | -| `pr_benchmark_check.yml` | `pr_benchmark_check` | -| `docs.yaml` | `docs` | -| `spark_sql_test_reusable.yml` | `spark_3_4`, `spark_3_5`, `spark_4_0`, `spark_4_1` | -| `iceberg_spark_test_reusable.yml` | `iceberg_1_8`, `iceberg_1_9`, `iceberg_1_10` | +| File | Called from `ci.yml` job(s) | +| --------------------------------- | ------------------------------------------------------------ | +| `pr_build_linux.yml` | `pr_build_linux` | +| `pr_build_macos.yml` | `pr_build_macos` | +| `pr_benchmark_check.yml` | `pr_benchmark_check` | +| `docs.yaml` | `docs` | +| `spark_sql_test_reusable.yml` | `spark_3_4`, `spark_3_5`, `spark_4_0`, `spark_4_1` | +| `iceberg_spark_test_reusable.yml` | `iceberg_1_8`, `iceberg_1_9`, `iceberg_1_10`, `iceberg_1_11` | ## Modifying path filters @@ -111,3 +138,11 @@ umbrella exposes per-job names like `CI / pr_build_linux / Lint`, protection rules to point at the new names; the old standalone workflow names (`Spark SQL Tests (Spark 3.5)`, `PR Build (Linux)`, ...) no longer exist as top-level workflows. + +`.asf.yaml` currently declares no `required_status_checks` for `main`, only +`required_approving_review_count: 1`. Anything added there must be a name that +never legitimately reports `skipped`, because GitHub counts a skipped check as +passing. The bare caller-job names (`PR Build (Linux)`, `Spark SQL Tests +(Spark 3.5)`, ...) are not such names: a reusable workflow that actually runs +publishes only its child jobs, so the bare name shows up solely when the caller +was skipped. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8eb9973117..87a4ec53fd9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,20 +45,20 @@ jobs: # pr_rat_check, pr_markdown_format, pr_missing_suites, and validate_workflows. # pr_title_check stays a standalone workflow because it needs to fire on PR # `edited` events. + # + # This job deliberately carries no `if:`. A job held back by `if:` still + # publishes a check run under its own name with conclusion `skipped`, and the + # newest check run for a name is the one the merge box, `gh pr checks` and + # required-status-check evaluation read. Guarding this job on the label name + # therefore let any non-gating label overwrite the commit run's real + # `Preflight` verdict with `skipped` (issue #5007). Running it unconditionally + # costs about a minute of ubuntu-slim time per label event and keeps the + # reported verdict truthful. Skipping the redundant work is the heavy jobs' + # job, and they filter `labeled` events themselves below. # --------------------------------------------------------------------------- preflight: name: Preflight runs-on: ubuntu-slim - # On a `labeled` event, only proceed for the gating labels. Any other label - # (e.g. dependabot's `dependencies`) skips the whole pipeline rather than - # spawning a redundant run alongside the opened/synchronize one. - if: >- - github.event_name != 'pull_request' || - github.event.action != 'labeled' || - github.event.label.name == 'run-spark-3.4-tests' || - github.event.label.name == 'run-spark-4.0-tests' || - github.event.label.name == 'run-spark-4.1-tests' || - github.event.label.name == 'run-iceberg-tests' steps: - uses: actions/checkout@v7 @@ -166,6 +166,11 @@ jobs: # Heavy jobs: each is a thin caller of an existing reusable workflow. The # `if:` expressions encode the same event/label/path criteria the # standalone trigger workflows used to encode in their `on:` blocks. + # + # On a `labeled` event only the job that the newly added label gates runs. + # Every other heavy job already ran on the opened/synchronize event at the + # same commit, so letting them through would duplicate an entire pipeline + # each time somebody applies a label. # --------------------------------------------------------------------------- pr_build_linux: @@ -175,7 +180,8 @@ jobs: needs.changes.outputs.build_linux == 'true' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || - github.event_name == 'pull_request') + (github.event_name == 'pull_request' && + github.event.action != 'labeled')) uses: ./.github/workflows/pr_build_linux.yml pr_build_macos: @@ -185,7 +191,8 @@ jobs: needs.changes.outputs.build_macos == 'true' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || - github.event_name == 'pull_request') + (github.event_name == 'pull_request' && + github.event.action != 'labeled')) uses: ./.github/workflows/pr_build_macos.yml pr_benchmark_check: @@ -195,7 +202,8 @@ jobs: needs.changes.outputs.benchmark == 'true' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || - github.event_name == 'pull_request') + (github.event_name == 'pull_request' && + github.event.action != 'labeled')) uses: ./.github/workflows/pr_benchmark_check.yml docs: @@ -216,7 +224,9 @@ jobs: (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && - contains(github.event.pull_request.labels.*.name, 'run-spark-3.4-tests'))) + contains(github.event.pull_request.labels.*.name, 'run-spark-3.4-tests') && + (github.event.action != 'labeled' || + github.event.label.name == 'run-spark-3.4-tests'))) uses: ./.github/workflows/spark_sql_test_reusable.yml with: spark-short: '3.4' @@ -230,7 +240,8 @@ jobs: needs.changes.outputs.spark_3_5 == 'true' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || - github.event_name == 'pull_request') + (github.event_name == 'pull_request' && + github.event.action != 'labeled')) uses: ./.github/workflows/spark_sql_test_reusable.yml with: spark-short: '3.5' @@ -248,7 +259,9 @@ jobs: (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && - contains(github.event.pull_request.labels.*.name, 'run-spark-4.0-tests'))) + contains(github.event.pull_request.labels.*.name, 'run-spark-4.0-tests') && + (github.event.action != 'labeled' || + github.event.label.name == 'run-spark-4.0-tests'))) uses: ./.github/workflows/spark_sql_test_reusable.yml with: spark-short: '4.0' @@ -262,7 +275,8 @@ jobs: needs.changes.outputs.spark_4_1 == 'true' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || - github.event_name == 'pull_request') + (github.event_name == 'pull_request' && + github.event.action != 'labeled')) uses: ./.github/workflows/spark_sql_test_reusable.yml with: spark-short: '4.1' @@ -278,7 +292,9 @@ jobs: (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && - contains(github.event.pull_request.labels.*.name, 'run-iceberg-tests'))) + contains(github.event.pull_request.labels.*.name, 'run-iceberg-tests') && + (github.event.action != 'labeled' || + github.event.label.name == 'run-iceberg-tests'))) uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: iceberg-short: '1.8' @@ -296,7 +312,9 @@ jobs: (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && - contains(github.event.pull_request.labels.*.name, 'run-iceberg-tests'))) + contains(github.event.pull_request.labels.*.name, 'run-iceberg-tests') && + (github.event.action != 'labeled' || + github.event.label.name == 'run-iceberg-tests'))) uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: iceberg-short: '1.9' @@ -315,7 +333,9 @@ jobs: (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && - contains(github.event.pull_request.labels.*.name, 'run-iceberg-tests'))) + contains(github.event.pull_request.labels.*.name, 'run-iceberg-tests') && + (github.event.action != 'labeled' || + github.event.label.name == 'run-iceberg-tests'))) uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: iceberg-short: '1.10' @@ -332,7 +352,8 @@ jobs: needs.changes.outputs.iceberg_1_11 == 'true' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch' || - github.event_name == 'pull_request') + (github.event_name == 'pull_request' && + github.event.action != 'labeled')) uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: iceberg-short: '1.11'