diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 1976ca2632a..d50ef7cd58a 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -16,7 +16,9 @@ ruleset in `.asf.yaml`. That splits CI into three tiers: - **PR tier** (`pr`): fast feedback while a change is being iterated on. The Linux build, with the Comet test suites run against the default Spark - profile (4.1) only, and nothing else. + profile (4.1) only, and nothing else. For a pull request from a fork this + tier runs on the contributor's fork instead of here; see + [Fork runs](#fork-runs). - **Queue tier** (`queue`): the authoritative gate. The PR tier plus the macOS build, the benchmark compile check, the Delta contrib build gate, the PyArrow UDF suite, Spark SQL on Spark 4.1 and Iceberg 1.11, evaluated @@ -198,6 +200,92 @@ still needs its label, so a `labeled` run there adds `spark_3_4` or nothing. `check-ci-config.py` pins that wiring, because a dropped variable reads as an empty string and would route the pull request as if it targeted `main`. +## Fork runs + +Nearly every pull request comes from a fork, and the PR tier is the +largest share of the runner-hours CI spends (about 60% in the week to +2026-10-28, label runs included). A fork pull request therefore does not run +the PR tier on ASF's runners. The contributor's fork runs it, and the merge +queue, which stays on ASF, runs the queue tier on the merge result before +anything lands. That is the PR tier plus Spark SQL on 4.1, Iceberg 1.11, macOS and +the other queue-tier jobs, with path filters still applied; the nightly-only and +label-only suites are not part of it. The design follows `apache/datafusion` (#25591) and `apache/arrow-rs` +(#11788). Three files carry it: + +| File | Role | +| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ci_fork.yml` | Fires on a push to any branch of a fork except `asf-site` and calls `ci.yml`, after cancelling the branch's older runs. Its jobs are skipped upstream. | +| `compute-changes.py` | `is_fork_push` gives a fork the `pr` tier. `is_fork_pull_request` takes the `pr` tier away upstream, and leaves every `run-*` label and the `labeled` run. | +| `fork_ci_status.yml` | Mirrors the fork's run onto the pull request as the `CI (fork)` check, on `workflow_run` and every 15 minutes. | + +Things that follow from that, and are easy to break: + +- **The fork result is advisory.** `Required Checks` does not depend on + `CI (fork)`, because a contributor controls the workflow that produced it. A + green fork run that never ran the tests costs one queue slot, not a bad merge. +- **Every branch of a fork runs through `ci_fork.yml`, `main` included.** + `ci.yml` also triggers on push to `main`, in a fork as upstream, so its + `Preflight` skips any fork push that did not come through `ci_fork.yml`, which + it tells apart by the caller's `github.workflow`, and `required_checks` skips + itself when `Preflight` does. That starts no runner for a fork's schedule or + for a second copy of a push, and leaves the bridge one workflow to look up. A + push that adds nothing to upstream `main`, such as a sync of the fork, selects + no build or test suite and writes to no build cache. A fork's caches are therefore cold unless a run on its `main` builds, + and each branch run rebuilds the native library: 59 minutes of wall time and + 240 runner-minutes cold against 50 and 213 warm, measured 2026-10-28 on one + fork. +- **Branch names compare without regard to case in Actions,** in `==`, `!=` and + concurrency group names alike, and a fork can hold both `feature` and + `Feature`. So a fork push has no concurrency group shared with another push: it + is keyed on its run, which also stops a backup branch created at the commit a + pull request is building from cancelling that build. `ci_fork.yml`'s `supersede` + job cancels the older runs of exactly the same branch instead, which it can + compare with case. `check-ci-config.py` evaluates + conditions with the same case-insensitive comparison, and checks that the push + trigger admits `main`. +- **A pull request opened inside a fork skips the PR tier,** because the branch + push already ran it, and `is_fork_pull_request` covers both that and a fork + pull request into `main` here. A `run-*` label still runs its suite. +- **A fork push is diffed against upstream main, as its pull request will be.** + There is no pull request to diff against on a push, and the fork's own default + branch is the wrong base: a fork that has not synced is behind upstream, so a + branch cut from newer upstream code that reverts one of its files matches the + fork's old copy and a diff against it omits the file. + `dev/ci/fork-push-changed-files.sh` fetches upstream `main` and diffs against + that. It lists every file when it has nothing to go on (the fetch fails or + there is no merge base), because selecting no suites then would report a green + run for a change nobody could see. An empty diff with a merge base is a branch + that adds nothing to upstream, which selects nothing. + `dev/ci/test-fork-push-changed-files.py` runs it against real repositories, + including the stale-baseline revert and a branch whose commits only the fork's + `main` has. +- **Scheduled workflows stay upstream.** A fork that enables scheduled + workflows would run them on its owner's minutes, and with no earlier scheduled + run to diff against, a fork's nightly runs the whole nightly tier. `ci.yml`'s + `Preflight`, `miri.yml`, `stale.yml`, `codeql.yml` and the bridge skip a + scheduled run outside this repository. Skipping `Preflight` skips the whole + nightly, since every other job needs `changes`, which needs it, and + `nightly_report` treats a skipped aggregator as nothing to report. + `check-ci-config.py` does not match text for this. It evaluates each root job's + `if:` for a scheduled run in a fork, and for the pushes and events above, and + fails when a job would run that should not or the other way round. +- **`ci.yml` must not trigger on every push.** A skipped job still publishes a + check run under its own name, so a push to a branch of this repository that + lands on a pull request's commit would put a skipped `Required Checks` next to + the real one. `ci_fork.yml` is a separate workflow so the only name that can + be skipped is its own, and `check-ci-config.py` pins its trigger and the + `github.repository != 'apache/datafusion-comet'` guard. +- **Exceptions stay upstream:** a pull request from a branch of this + repository (a committer's, dependabot's), one that targets a release branch, + and the opt-in `run-*` suites. A fork cannot see the labels on its pull + request, and a release branch has no queue to catch what a fork run misses. +- **The bridge never checks out or runs anything from the pull request.** It + runs with `checks: write` from the default branch and reads the fork's public + run list. Its logic is tested by `dev/ci/test-fork-ci-status.cjs`, which + Preflight runs, against mocked GitHub APIs. `workflow_run` and `schedule` + events use the copy on the default branch, so a change to the bridge cannot be + exercised by the pull request that makes it. + ### Label events `ci_label.yml` fires on `pull_request.types: [labeled]` and calls `ci.yml` @@ -300,6 +388,8 @@ umbrella doesn't watch, or operate independently of the rest of CI: | `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. | +| `ci_fork.yml` | Runs `ci.yml` on a push to a fork's branch; skipped upstream. See [Fork runs](#fork-runs). | +| `fork_ci_status.yml` | Mirrors a fork's run onto its pull request as `CI (fork)`. See [Fork runs](#fork-runs). | ## Reusable workflows (called by `ci.yml`) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 936b804272d..0fc66aba7f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,13 @@ # green nightly. A failure opens (or comments on) an issue # labelled `ci-nightly-failure`. # +# A pull request from a fork into main does not run the PR tier here. The +# contributor's fork runs it (ci_fork.yml), and fork_ci_status.yml mirrors that +# run onto the pull request as the informational `CI (fork)` check, which +# `Required Checks` does not depend on. The merge queue still runs the queue tier +# on the merge result. Opt-in `run-*` suites, pull requests from a branch of this +# repository and pull requests to a release branch are unaffected. +# # Spark 3.4 is deprecated and sits outside all three tiers: it runs only when # a pull request carries `run-spark-3.4-tests`, or from a manual dispatch. # @@ -73,9 +80,17 @@ name: Comet CI # each other. Every other run is keyed on its event name: opened, synchronize # and reopened all map to `pull_request`, so a new push still supersedes its # predecessor, while a scheduled run at the tip of main never cancels that -# commit's push run. +# commit's push run. A pull request is keyed on its number, and not on its +# branch name: nearly every pull request comes from a fork, and two forks' +# branches can share a name, which would have one cancel the other's run. A push +# to a fork (ci_fork.yml) is keyed on its run, so no other push shares its group: +# a backup branch created at the commit a pull request is building would +# otherwise cancel that build. A newer push to the same branch cancels the run in +# flight through ci_fork.yml's `supersede` job instead, because group names +# compare without regard to case and a fork can hold both `feature` and +# `Feature`. concurrency: - group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}-${{ github.event.action == 'labeled' && github.event.label.name || github.event_name }} + group: ${{ github.repository }}-${{ github.event.pull_request.number || (github.event_name == 'push' && github.repository != 'apache/datafusion-comet' && github.run_id) || github.sha }}-${{ github.workflow }}-${{ github.event.action == 'labeled' && github.event.label.name || github.event_name }} cancel-in-progress: true # No `labeled` here: ci_label.yml handles that event and calls this file @@ -118,19 +133,35 @@ jobs: # 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; POLICY in dev/ci/compute-changes.py drops every job a `labeled` event - # does not gate. + # This job deliberately carries no `if:` that reads the pull request or its + # labels. 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; POLICY in dev/ci/compute-changes.py + # drops every job a `labeled` event does not gate. + # + # The `if:` it does carry skips runs that belong to no pull request, and with + # them the whole run, since every other job needs `changes`, which needs this + # (`required_checks` skips itself when this is skipped, below): + # + # - A scheduled run in a fork, which would otherwise run the whole nightly + # tier for want of an earlier run to diff against. + # - A push to a fork that did not come through ci_fork.yml. ci.yml triggers + # on push to main, so it fires in a fork for its `main` as well, while + # ci_fork.yml runs every branch of a fork, `main` included. Running both + # would test the push twice. The workflow name tells the two entry points + # apart, since a called workflow sees its caller's `github.workflow`. # --------------------------------------------------------------------------- preflight: name: Preflight + if: >- + (github.event_name != 'schedule' || github.repository == 'apache/datafusion-comet') + && (github.event_name != 'push' || github.repository == 'apache/datafusion-comet' + || github.workflow == 'Comet CI (fork run)') runs-on: ubuntu-slim steps: - uses: actions/checkout@v7 @@ -194,6 +225,12 @@ jobs: - name: Check pull request type labeling run: node --test dev/ci/pr-type-label.test.mjs + - name: Check fork CI status bridge + run: node --test dev/ci/test-fork-ci-status.cjs + + - name: Check the changed files of a fork push + run: python3 dev/ci/test-fork-push-changed-files.py + - name: Check Iceberg shard inventory validation run: python3 dev/ci/test-iceberg-shards.py @@ -282,6 +319,8 @@ jobs: LABEL_NAME: ${{ github.event.label.name }} PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} PR_BASE_REF: ${{ github.event.pull_request.base.ref }} + PR_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + REPOSITORY: ${{ github.repository }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} MQ_BASE_SHA: ${{ github.event.merge_group.base_sha }} @@ -329,8 +368,12 @@ jobs: # merge_group runs; the queue's ALLGREEN strategy is what makes # that enough (see the Merge Queue ruleset in .asf.yaml). git diff --name-only "$MQ_BASE_SHA"..."$MQ_HEAD_SHA" > changed_files.txt + elif [[ "$REPOSITORY" != "apache/datafusion-comet" ]]; then + # A push to a branch of a contributor's fork (ci_fork.yml): diff it + # against upstream main, as its pull request will be. + bash dev/ci/fork-push-changed-files.sh "$PUSH_AFTER" > changed_files.txt else - # push to main; first push to a branch has all-zero before sha + # push to main; the first push to a branch has an all-zero before sha if [[ "$PUSH_BEFORE" =~ ^0+$ ]]; then git ls-tree -r --name-only "$PUSH_AFTER" > changed_files.txt else @@ -366,6 +409,10 @@ jobs: # pull request linux, full -> profiles: pr # ... with the label linux, full, all -> profiles: all # ... to branch-N.M linux, full, all -> profiles: all + # ... from a fork (nothing) -> no call: the fork runs the tier + # ... from a fork, with the label + # all -> profiles: nightly, the ones the + # fork's default-profile run skips # `labeled` run all -> profiles: nightly # merge queue linux, full -> profiles: pr # nightly all -> profiles: nightly @@ -625,7 +672,9 @@ jobs: # nested name would block every docs-only change. Both hang rather than fail, # and a required context that never reports also locks `.asf.yaml` itself, # which then needs an INFRA ticket to unwedge. Aggregating into one flat job, - # whose name is published on every event, avoids the whole class of problem. + # whose name is published on every event that can reach the merge box, avoids + # the whole class of problem. The exception is a fork run that is no pull + # request's tier, which Preflight skips and this job follows. # # `if: always()` is what makes this work: without it the job inherits the # default `success()` and is itself skipped the moment any dependency fails. @@ -643,7 +692,10 @@ jobs: # --------------------------------------------------------------------------- required_checks: name: ${{ github.event.action == 'labeled' && 'Required Checks (label run)' || 'Required Checks' }} - if: always() + # `always()` so a failed or cancelled dependency still reaches it. It skips + # itself only when Preflight did, which is a fork run that is not a pull + # request's tier (see Preflight), so those start no runner at all. + if: always() && needs.preflight.result != 'skipped' # Reads only the `needs` context, so it needs no token access at all. permissions: {} needs: @@ -699,7 +751,11 @@ jobs: nightly_report: name: Nightly failure report needs: required_checks - if: always() && github.event_name == 'schedule' && needs.required_checks.result != 'success' + # `skipped` is not a failure here: the aggregator skips itself on a fork's + # scheduled run, and that run has nothing to report. + if: >- + always() && github.event_name == 'schedule' + && (needs.required_checks.result == 'failure' || needs.required_checks.result == 'cancelled') permissions: actions: read issues: write diff --git a/.github/workflows/ci_fork.yml b/.github/workflows/ci_fork.yml new file mode 100644 index 00000000000..a7b67b5b48f --- /dev/null +++ b/.github/workflows/ci_fork.yml @@ -0,0 +1,109 @@ +# 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. + +# Fork runs: a push to a branch of a contributor's fork runs the pull request +# tier on the contributor's own GitHub runners, so a fork pull request does not +# spend ASF's shared capacity on it. ci.yml upstream skips the PR tier for a +# fork pull request into main (see `is_fork_pull_request` in +# dev/ci/compute-changes.py), keeping only the preflight checks and the opt-in +# `run-*` suites, and fork_ci_status.yml mirrors this run onto the pull request +# as the informational `CI (fork)` check. The merge queue runs the queue tier on +# the merge result, so a fork run is feedback and never a gate. +# +# This is a separate workflow, and not a wider `push` trigger on ci.yml, for the +# same reason ci_label.yml is one. A skipped job still publishes a check run +# under its own name, and a push to a branch of this repository can land on the +# very commit a pull request is testing. A skipped `Required Checks` from that +# push would compete with the pull request's real verdict. Here the only job +# that can be skipped is named after this workflow's own run, and upstream it is +# always skipped. +# +# Every branch of a fork is run the same way, the default branch included. ci.yml +# also triggers on a push to `main`, in a fork as upstream, but its Preflight job +# skips that run unless it came through this workflow, so a fork push is tested +# once and the bridge has one workflow to look up. A push that adds nothing to +# upstream main, such as a sync of the fork, selects no suite; see +# dev/ci/fork-push-changed-files.sh. +name: Comet CI (fork run) + +# No `concurrency` here, and none for a fork push in ci.yml either: see the +# `supersede` job. A group on the caller as well as in ci.yml would deadlock +# the two. +on: + push: + branches-ignore: + - asf-site + +permissions: + contents: read + +jobs: + # A newer push to a branch supersedes the run in flight, as it does for a pull + # request upstream, but not through a concurrency group. Group names compare + # without regard to case, and a fork can hold both `feature` and `Feature`, + # which are two branches: keying a group on the branch would let a push to one + # cancel the other's run, and no expression can make the two keys differ. So the + # runs get no shared group, and this job cancels the older runs of exactly this + # branch, which it can compare with case. + supersede: + name: Cancel superseded runs + # Upstream, a push to any branch other than main is a pull request's own + # business: the pull request run tests it, and a second run here would + # test it twice. + if: github.repository != 'apache/datafusion-comet' + runs-on: ubuntu-slim + # Failing to cancel only leaves an older run to finish, so it never blocks the run. + continue-on-error: true + permissions: + actions: write + steps: + - uses: actions/github-script@v9 + with: + script: | + const { owner, repo } = context.repo; + const branch = context.ref.replace(/^refs\/heads\//, ''); + for (const status of ['queued', 'in_progress']) { + const runs = await github.paginate(github.rest.actions.listWorkflowRuns, { + owner, repo, workflow_id: 'ci_fork.yml', event: 'push', status, per_page: 100, + }); + for (const run of runs) { + // The API's branch filter ignores case, so filter here, where `feature` + // and `Feature` stay two branches. + if (run.head_branch !== branch || run.run_number >= context.runNumber) continue; + try { + await github.rest.actions.cancelWorkflowRun({ owner, repo, run_id: run.id }); + core.info('Cancelled run ' + run.run_number + ' of ' + branch); + } catch (error) { + // 409: it finished between the listing and the cancel. + if (error.status !== 409) throw error; + } + } + } + + ci: + name: Fork run + needs: supersede + if: github.repository != 'apache/datafusion-comet' + # The union of what ci.yml's jobs request, for the same reason as in + # ci_label.yml: GitHub checks a called workflow's permissions against + # this ceiling for every job when the run starts, including the ones this + # event skips. + permissions: + actions: read + contents: write + issues: write + uses: ./.github/workflows/ci.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index c834da33407..00c38cb697d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -36,6 +36,9 @@ permissions: jobs: analyze: name: Analyze Actions + # The weekly scan runs upstream only; a fork's own pushes and pull requests + # still get one. + if: github.event_name != 'schedule' || github.repository == 'apache/datafusion-comet' runs-on: ubuntu-slim permissions: contents: read diff --git a/.github/workflows/fork_ci_status.yml b/.github/workflows/fork_ci_status.yml new file mode 100644 index 00000000000..b01787d100c --- /dev/null +++ b/.github/workflows/fork_ci_status.yml @@ -0,0 +1,242 @@ +# 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. + +# Mirror a contributor's fork run (ci_fork.yml) onto its pull request as the informational `CI (fork)` check, +# after upstream PR CI completes and through scheduled polling for late runs and +# reruns. The check is feedback only: `Required Checks` does not depend on it, +# because a contributor controls the workflow that produced it, and the merge +# queue runs the queue tier on the merge result. +# +# Reads fork runs and writes upstream checks using reviewed default-branch code. +# No checkout, artifacts, or code from the pull request are used here. +# +# Adapted from apache/arrow-rs#11788 and apache/datafusion#25591. +# Test with Node.js 24: +# node --test dev/ci/test-fork-ci-status.cjs +# Tests mock GitHub APIs. End-to-end testing needs a fork pull request after this +# workflow is installed on the default branch. +name: Fork CI status + +on: + workflow_run: + workflows: [Comet CI] + types: [completed] + schedule: + - cron: '7,22,37,52 * * * *' + workflow_dispatch: + +# No scopes by default: the job below asks for the two it needs. Every call to the +# fork's runs and workflows reads public data of another repository, which a token +# does not need a scope for. +permissions: {} + +# Like DataFusion, group notifications separately from scheduled/manual sweeps. +# workflow_run identifies the fork and branch rather than the PR number. +concurrency: + group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name || 'update' }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: ${{ github.event_name == 'workflow_run' }} + +jobs: + bridge: + # A workflow_run for a pull request from a branch of this repository has no + # fork run to mirror, so it never starts a job. + if: >- + github.repository == 'apache/datafusion-comet' + && (github.event_name != 'workflow_run' + || (github.event.workflow_run.event == 'pull_request' + && github.event.workflow_run.head_repository.full_name != github.repository)) + name: Mirror the fork run + permissions: + checks: write + pull-requests: read + runs-on: ubuntu-slim + timeout-minutes: 10 + steps: + - name: Create or update the check + uses: actions/github-script@v9 + with: + script: | + // Adapted from apache/datafusion#25591. Fork results are informational: + // contributors control their workflows; only upstream merge-queue CI gates merging. + const CHECK = 'CI (fork)'; + const WORKFLOW = 'ci_fork.yml'; + const upstream = { owner: context.repo.owner, repo: context.repo.repo }; + const repository = upstream.owner + '/' + upstream.repo; + const GUIDE = 'https://github.com/apache/datafusion-comet/blob/main/docs/source/contributor-guide/ci.md#pull-requests-from-forks'; + const SWEEP_SIZE = 25; + const listing = { ...upstream, state: 'open', base: 'main', sort: 'updated', direction: 'desc' }; + const isForkPr = (pr) => pr.state === 'open' && pr.head.repo && + pr.head.repo.full_name.toLowerCase() !== repository.toLowerCase() && pr.base.ref === 'main'; + const externalId = (pr) => 'comet-fork-ci:' + pr.number; + const conclusions = new Set([ + 'action_required', 'cancelled', 'failure', 'neutral', 'skipped', 'success', 'timed_out', + ]); + const UPDATE_NOTE = + 'This check is informational: the contributor controls the fork workflow. ' + + 'The upstream merge queue runs the required CI checks on the proposed merge result. ' + + 'Active or missing runs on recently updated PRs are polled every 15 minutes; ' + + 'a rotating sweep also discovers reruns on older PRs. Scheduled updates may be delayed.'; + + // An absent workflow/run is expected on a fork with Actions disabled. Other + // API errors must fail the bridge rather than falsely reporting missing CI. + async function optional(request) { + try { + return (await request()).data; + } catch (error) { + if (error.status === 404) return null; + throw error; + } + } + + async function syncCheck(pr, includeCompleted = true) { + if (!isForkPr(pr)) return; + const fork = { owner: pr.head.repo.owner.login, repo: pr.head.repo.name }; + const sha = pr.head.sha; + const checks = await github.paginate(github.rest.checks.listForRef, { + ...upstream, ref: sha, check_name: CHECK, per_page: 100, + }); + const check = checks.find((item) => + item.app?.slug === 'github-actions' && item.external_id === externalId(pr)); + let previous = {}; + try { previous = JSON.parse(check?.output?.text || '{}'); } catch {} + // A missing run must keep being polled, even though its check is completed. + if (!includeCompleted && check?.status === 'completed' && previous.run_id) return; + + const data = await optional(() => github.rest.actions.listWorkflowRuns({ + ...fork, workflow_id: WORKFLOW, head_sha: sha, branch: pr.head.ref, per_page: 100, + })); + // Ignore PR merge-ref runs. ci_fork.yml runs on push only, and tests + // the branch head; contributors still control its code. + const run = (data?.workflow_runs || []).filter((item) => + item.head_sha === sha && item.head_branch === pr.head.ref && + item.event === 'push' + ).sort((a, b) => b.id - a.id)[0]; + let payload; + if (run) { + const status = run.status === 'completed' ? 'completed' : + run.status === 'in_progress' ? 'in_progress' : 'queued'; + payload = { + status, + ...(status === 'completed' ? { + conclusion: conclusions.has(run.conclusion) ? run.conclusion : 'failure', + } : {}), + details_url: run.html_url, + output: { + title: 'Fork CI: ' + (status === 'completed' ? run.conclusion : status), + summary: [ + '[CI run on ' + pr.head.repo.full_name + '](' + run.html_url + ') for ' + sha + '.', + 'Rerun failed checks in the fork. A green fork run tests the branch head, not its merge with main.', + UPDATE_NOTE, + ].join('\n\n'), + text: JSON.stringify({ head_sha: sha, run_id: run.id, run_attempt: run.run_attempt }), + }, + }; + } else { + const workflow = await optional(() => github.rest.actions.getWorkflow({ + ...fork, workflow_id: WORKFLOW, + })); + const disabled = !workflow || workflow.state.startsWith('disabled'); + payload = { + status: 'completed', + conclusion: disabled ? 'action_required' : 'neutral', + details_url: 'https://github.com/' + pr.head.repo.full_name + '/actions', + output: { + title: disabled ? 'Enable CI on your fork' : 'No fork CI run for this commit', + summary: [ + 'No CI run was found for ' + sha + ' on ' + pr.head.repo.full_name + '.', + 'Enable workflows in the fork’s Actions tab, sync/rebase to include the fork CI workflow, ' + + 'then push your branch. A newly pushed run may take a moment to appear.', + UPDATE_NOTE, + '[Fork CI setup](' + GUIDE + ')', + ].join('\n\n'), + text: JSON.stringify({ head_sha: sha, run_id: null }), + }, + }; + } + if (check && check.status === payload.status && + check.conclusion === (payload.conclusion || null) && + check.output?.text === payload.output.text && + check.output?.summary === payload.output.summary) return; + if (check) { + await github.rest.checks.update({ ...upstream, check_run_id: check.id, ...payload }); + } else { + await github.rest.checks.create({ + ...upstream, name: CHECK, head_sha: sha, external_id: externalId(pr), ...payload, + }); + } + core.info('#' + pr.number + ': ' + (payload.conclusion || payload.status)); + } + + async function notify() { + const run = context.payload.workflow_run; + if (run.event !== 'pull_request' || !run.head_repository || + run.head_repository.full_name.toLowerCase() === repository.toLowerCase()) return; + // GitHub can leave workflow_run.pull_requests empty for fork PRs. Resolve + // from the API instead, and recheck the current head to ignore stale runs. + const prs = await github.paginate(github.rest.pulls.list, { + ...listing, head: run.head_repository.owner.login + ':' + run.head_branch, per_page: 100, + }); + for (const pr of prs) { + if (pr.head.sha === run.head_sha && + pr.head.repo?.full_name.toLowerCase() === run.head_repository.full_name.toLowerCase()) { + await syncCheck(pr); + } + } + } + + async function update() { + const errors = []; + // A failed update must not prevent unrelated PRs from being refreshed. + async function syncPr(pr, includeCompleted) { + try { + await syncCheck(pr, includeCompleted); + } catch (error) { + errors.push('#' + pr.number + ': ' + error.message); + } + } + const count = await github.rest.pulls.list({ ...listing, per_page: 1 }); + const last = /[?&]page=(\d+)>;\s*rel="last"/.exec(count.headers.link || ''); + const total = last ? Number(last[1]) : count.data.length; + if (!total) return; + const slices = Math.ceil(total / SWEEP_SIZE); + const slot = Math.floor(Date.now() / (15 * 60 * 1000)); + const { data: slice } = await github.rest.pulls.list({ + ...listing, per_page: SWEEP_SIZE, page: (slot % slices) + 1, + }); + const sweepNumbers = new Set(slice.map((pr) => pr.number)); + const seen = new Set(); + const cutoff = Date.now() - 6 * 3600 * 1000; + // Poll active and missing checks on recently updated PRs, and completed + // checks only in the rotating slice. This bounds expensive fork API calls. + const pages = github.paginate.iterator(github.rest.pulls.list, { ...listing, per_page: 100 }); + for await (const page of pages) { + let stale = false; + for (const pr of page.data) { + if (new Date(pr.updated_at).getTime() < cutoff) { stale = true; break; } + seen.add(pr.number); + await syncPr(pr, sweepNumbers.has(pr.number)); + } + if (stale) break; + } + for (const pr of slice) { + if (!seen.has(pr.number)) await syncPr(pr); + } + if (errors.length) core.setFailed(errors.join('\n')); + } + + if (context.eventName === 'workflow_run') await notify(); + else await update(); diff --git a/.github/workflows/miri.yml b/.github/workflows/miri.yml index 5310af2ee40..3d8cf9b1bc7 100644 --- a/.github/workflows/miri.yml +++ b/.github/workflows/miri.yml @@ -35,6 +35,8 @@ permissions: jobs: miri: name: Miri (${{ matrix.suite }}) + # The nightly runs upstream only. A fork can still run it by dispatch. + if: github.event_name != 'schedule' || github.repository == 'apache/datafusion-comet' runs-on: ubuntu-24.04 strategy: fail-fast: false diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index a22c42043c8..e6d50fb82aa 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -22,6 +22,8 @@ on: jobs: close-stale-prs: + # Upstream only: in a fork this would close the fork owner's own stale PRs. + if: github.repository == 'apache/datafusion-comet' runs-on: ubuntu-slim permissions: issues: write diff --git a/AGENTS.md b/AGENTS.md index b7a732268dc..8e6e8fda096 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,9 +46,11 @@ remote changes before retrying; do not bypass it with `--force`. See ## Checking a change against CI A green pull request does not mean a change is safe to queue. The pull request tier runs the Comet -suites against the default Spark profile only. Spark's own SQL suite and the Iceberg suites first -report in the merge queue, where a failure evicts the pull request and blocks everyone else's -merges, or in the nightly run, after the change has already landed. +suites against the default Spark profile only. For a pull request from a fork it runs on the fork, +where `CI (fork)` reports it as advisory, and `Required Checks` here passes without a build. Spark's +own SQL suite and the Iceberg suites first report in the merge queue, where a failure evicts the +pull request and blocks everyone else's merges, or in the nightly run, after the change has already +landed. So when a change touches the serde, the planner, a native operator, a Spark shim, an Iceberg code path, or anything under `dev/diffs/`, get a verdict first. Either run the suite locally: diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index c752a50c266..9823869bf8c 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -# Guards seven CI invariants that are silent when broken: +# Guards nine 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 @@ -73,8 +73,17 @@ # which is how every native build in the queue came to miss its cargo # cache and pay a cold ~26 minute compile. # +# 8. Fork runs. A pull request from a fork runs no PR tier upstream; the +# contributor's fork does (ci_fork.yml), and fork_ci_status.yml mirrors it +# as an informational check. The caller, the bridge and ci.yml's own push +# trigger have to agree, or the PR tier runs twice, or nowhere. +# +# 9. Scheduled workflows. A fork that enables scheduled workflows runs them on +# its owner's minutes, so each one needs a guard on the repository. +# # Run from the repository root: python3 dev/ci/check-ci-config.py +import ast import importlib.util import re import subprocess @@ -243,6 +252,27 @@ ALL_JOBS = QUEUE_TIER | NIGHTLY_TIER | SPARK_DEPRECATED | {"docs"} assert not QUEUE_TIER & NIGHTLY_TIER, "a job is queue or nightly, never both" +# The repository whose queue gates main; check_event_policy asserts this matches +# UPSTREAM in compute-changes.py. +UPSTREAM = "apache/datafusion-comet" +FORK = "contributor/datafusion-comet" + + +def fork_pr(action="synchronize", label=None, labels=(), base="main", repository=UPSTREAM, head=FORK): + """A pull_request event for POLICY_CASES, from a fork of UPSTREAM by default.""" + event = { + "name": "pull_request", + "action": action, + "labels": list(labels), + "base": base, + "repository": repository, + "head_repository": head, + } + if label: + event["label"] = label + return event + + POLICY_CASES = [ # A manual run may exercise anything. ({"name": "workflow_dispatch"}, ALL_JOBS), @@ -511,6 +541,43 @@ }, PR_TIER, ), + # A pull request from a fork into main runs no PR tier upstream: the + # contributor's fork runs it (ci_fork.yml) and fork_ci_status.yml mirrors it + # as the informational `CI (fork)` check. Every opt-in label still works, + # because a fork cannot see the labels on the pull request. + (fork_pr(), set()), + ( + fork_pr(labels=["run-spark-4.1-tests", "run-delta-tests"]), + {"spark_4_1", "spark_4_1_hive"} | DELTA_OPT_IN, + ), + # The label brings the non-default Spark profiles forward here, while the + # fork's run covers the default profile and the lints, so between them every + # profile runs. Only the profiles are on, which ci.yml turns into + # `profiles: nightly`; the PR tier's default-profile row is the fork's. + (fork_pr(labels=["run-all-spark-profiles"]), {"build_linux_all_profiles"}), + ( + fork_pr(action="labeled", label="run-all-spark-profiles", labels=["run-all-spark-profiles"]), + {"build_linux_all_profiles"}, + ), + # The exceptions keep the PR tier upstream: a pull request from a branch of + # this repository (a committer's, or dependabot's), and one that targets a + # release branch, which has no queue or nightly to catch what a fork run + # misses. + (fork_pr(head=UPSTREAM), PR_TIER), + (fork_pr(base="branch-1.1"), QUEUE_TIER | NIGHTLY_TIER), + # A push to a fork's branch is the contributor's pull request tier and + # nothing more: not main's cache refresh or site deploy (`docs` publishes to + # asf-site), and not the queue's or the nightly's suites. That holds for the + # fork's default branch too; a push that adds nothing to upstream selects no + # file (see dev/ci/fork-push-changed-files.sh). + ({"name": "push", "repository": FORK}, PR_TIER), + # A pull request opened inside a fork leaves the PR tier to the branch push + # that already ran it, so the same commit is not tested twice, but a label + # still runs its suite there. + (fork_pr(repository=FORK, head=FORK), set()), + (fork_pr(repository=FORK, head=FORK, labels=["run-iceberg-tests"]), ICEBERG_OPT_IN), + # Upstream, push stays the cache refresh and the site deploy. + ({"name": "push", "repository": UPSTREAM}, {"docs", "build_linux", "build_macos"}), ] @@ -777,6 +844,11 @@ def check_change_filters(): def check_event_policy(): module = load_filters() failures = [] + if module.UPSTREAM != UPSTREAM: + failures.append( + f"UPSTREAM is {module.UPSTREAM!r} in compute-changes.py but {UPSTREAM!r} here; " + f"the fork cases below would test the wrong repository" + ) for event, expected in POLICY_CASES: actual = {job for job in module.POLICY if module.event_allows(job, event)} if actual != expected: @@ -787,6 +859,10 @@ def check_event_policy(): label += f" +{event['label']}" if event.get("base"): label += f" base={event['base']}" + if event.get("repository"): + label += f" in {event['repository']}" + if event.get("head_repository"): + label += f" from {event['head_repository']}" failures.append( f"{label} labels={event.get('labels', [])}: " f"unexpectedly allowed {sorted(actual - expected) or 'nothing'}, " @@ -831,6 +907,8 @@ def check_event_policy(): "LABEL_NAME": "github.event.label.name", "PR_LABELS": "toJSON(github.event.pull_request.labels.*.name)", "PR_BASE_REF": "github.event.pull_request.base.ref", + "PR_HEAD_REPO": "github.event.pull_request.head.repo.full_name", + "REPOSITORY": "github.repository", } ENV_READ = re.compile(r'os\.environ\.get\("([A-Z_]+)"') ENV_SET = re.compile(r"^\s+([A-Z_]+):\s+\$\{\{\s*(.+?)\s*\}\}\s*$") @@ -1198,6 +1276,36 @@ def ci_triggers(): return triggers +# A job that calls ci.yml as a reusable workflow. +CALLS_CI = re.compile(r"^\s+uses:\s*\./\.github/workflows/ci\.yml\s*$", re.M) + + +def permission_ceiling_failures(caller): + """Failures for each ci.yml permission that `caller` does not grant. + + A workflow that calls ci.yml passes it a `permissions:` ceiling, and GitHub + refuses to start the run if any job in ci.yml asks for more, even a job the + event would skip. + """ + granted = permission_blocks(caller.read_text(encoding="utf-8").splitlines()) + ceiling = {} + for block in granted: + for scope, level in block.items(): + ceiling[scope] = max(ceiling.get(scope, 0), PERMISSION_RANK[level]) + failures = [] + ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8").splitlines() + for block in permission_blocks(ci): + for scope, level in block.items(): + if PERMISSION_RANK[level] > ceiling.get(scope, 0): + failures.append( + f"ci.yml requests `{scope}: {level}`, but {caller.name} grants " + f"the call at most `{scope}: " + f"{PERMISSION_NAME[ceiling.get(scope, 0)]}`, so every run it " + f"starts fails at startup. Raise the grant in {caller.name}" + ) + return failures + + def check_label_runs_separate(): """`labeled` runs come from ci_label.yml, never from ci.yml's own trigger. @@ -1229,7 +1337,7 @@ def check_label_runs_separate(): failures.append(f"{label_workflow} is missing; nothing runs on `labeled`") else: body = label_workflow.read_text(encoding="utf-8") - if not re.search(r"^\s+uses:\s*\./\.github/workflows/ci\.yml\s*$", body, re.M): + if not CALLS_CI.search(body): failures.append(f"{label_workflow} does not call ./.github/workflows/ci.yml") if not re.search(r"^\s+types:\s*\[\s*labeled\s*\]\s*$", body, re.M): failures.append( @@ -1242,21 +1350,7 @@ def check_label_runs_separate(): # even a job the `labeled` event would skip. That is how the `docs` job's # `contents: write` stopped every label run on 2026-09-29. if label_workflow.exists(): - granted = permission_blocks(label_workflow.read_text(encoding="utf-8").splitlines()) - ceiling = {} - for block in granted: - for scope, level in block.items(): - ceiling[scope] = max(ceiling.get(scope, 0), PERMISSION_RANK[level]) - ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8").splitlines() - for block in permission_blocks(ci): - for scope, level in block.items(): - if PERMISSION_RANK[level] > ceiling.get(scope, 0): - failures.append( - f"ci.yml requests `{scope}: {level}`, but ci_label.yml grants " - f"the call at most `{scope}: " - f"{PERMISSION_NAME[ceiling.get(scope, 0)]}`, so every label " - f"run fails at startup. Raise the grant in ci_label.yml" - ) + failures.extend(permission_ceiling_failures(label_workflow)) for failure in failures: print(f"label runs: {failure}") @@ -1560,6 +1654,280 @@ def check_cache_save_scope(): return not failures +# Fork runs. A push to a contributor's fork runs the PR tier there (ci_fork.yml), +# upstream skips that tier for a fork pull request (compute-changes.py), and +# fork_ci_status.yml mirrors the fork's run onto the pull request. The pieces +# are in three files and break silently apart: a caller that also fires upstream +# runs the PR tier twice, a caller that fires on main runs it twice in a fork, a +# caller that cannot start leaves a fork pull request with no PR tier anywhere, +# and a bridge that reads the wrong workflow name reports "no fork CI" on every +# pull request. +FORK_WORKFLOW = WORKFLOWS / "ci_fork.yml" +BRIDGE_WORKFLOW = WORKFLOWS / "fork_ci_status.yml" +FORK_TEST = Path("dev/ci/test-fork-ci-status.cjs") +FORK_PUSH_SCRIPT = Path("dev/ci/fork-push-changed-files.sh") +FORK_PUSH_TEST = Path("dev/ci/test-fork-push-changed-files.py") + + +def workflow_name(path): + match = re.search(r"^name:\s*(.+?)\s*$", path.read_text(encoding="utf-8"), re.M) + return match.group(1) if match else None + + +def check_fork_runs(): + failures = [] + ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8") + + # ci.yml itself must stay a push-to-main trigger: it is what runs on a + # fork's main, and a wider one would run the PR tier twice on every push to + # a fork's branch and publish skipped `Required Checks` upstream. + if not re.search(r"^ push:\n branches:\n - main\n", ci, re.M): + failures.append( + "ci.yml must trigger on `push` to `branches: [main]` only; " + "fork branches go through ci_fork.yml" + ) + # Preflight is where a fork run that is not a pull request's tier is dropped, + # and with it the whole run, since every other job needs Preflight through + # `changes`. ci.yml triggers on push to main, so it fires in a fork for its + # `main` as well as ci_fork.yml does; only a push that came through + # ci_fork.yml, whose `github.workflow` is its own name, is tested, once. + ci_name = workflow_name(WORKFLOWS / "ci.yml") + fork_name = workflow_name(FORK_WORKFLOW) + failures.extend( + condition_failures( + WORKFLOWS / "ci.yml", + "preflight", + [ + (actions_context("schedule", UPSTREAM, ci_name), True, "the upstream nightly"), + (actions_context("schedule", FORK, ci_name), False, "a scheduled run in a fork"), + (actions_context("push", UPSTREAM, ci_name), True, "a push to upstream main"), + (actions_context("push", FORK, ci_name), False, "ci.yml's own push trigger in a fork"), + (actions_context("push", FORK, fork_name), True, "a push through ci_fork.yml"), + (actions_context("pull_request", UPSTREAM, ci_name), True, "a pull request"), + (actions_context("pull_request", FORK, ci_name), True, "a pull request inside a fork"), + (actions_context("merge_group", UPSTREAM, ci_name), True, "the merge queue"), + (actions_context("workflow_dispatch", FORK, ci_name), True, "a dispatch in a fork"), + ], + ) + ) + # `needs.preflight.result` is how the aggregator follows Preflight. + aggregator = job_condition(job_blocks(WORKFLOWS / "ci.yml").get(AGGREGATOR_JOB, "")) + if not aggregator or "needs.preflight.result" not in aggregator: + failures.append( + f"ci.yml's {AGGREGATOR_JOB} must skip itself when Preflight does " + f"(`needs.preflight.result != 'skipped'`), or a fork's skipped run still starts a runner" + ) + for needed in (FORK_TEST.name, FORK_PUSH_SCRIPT.name, FORK_PUSH_TEST.name): + if needed not in ci: + failures.append(f"ci.yml does not use or run {needed}, so a fork push goes untested") + + if not FORK_WORKFLOW.exists(): + failures.append(f"{FORK_WORKFLOW} is missing; a fork pull request would run no PR tier anywhere") + else: + body = FORK_WORKFLOW.read_text(encoding="utf-8") + if not CALLS_CI.search(body): + failures.append(f"{FORK_WORKFLOW} does not call ./.github/workflows/ci.yml") + # The trigger comes first: a branch it ignores never reaches a job's `if:`, + # so a condition table alone cannot see an exclusion that is too wide. + ignored = re.search(r"^ branches-ignore:\n((?: .*\n)+)", body, re.M) + patterns = re.findall(r"^\s+- (\S+)\s*$", ignored.group(1), re.M) if ignored else [] + patterns = [p.strip("'\"") for p in patterns] + compute = load_filters() + for branch, admitted in ( + ("feature", True), ("main", True), ("Main", True), ("release/1.1", True), ("asf-site", False), + ): + actual = not any(re.match(compute.glob_to_regex(p), branch) for p in patterns) + if actual != admitted: + failures.append( + f"{FORK_WORKFLOW}'s `push: branches-ignore` {patterns} " + f"{'admits' if actual else 'ignores'} a push to `{branch}`, and it should " + f"{'ignore' if actual else 'admit'} it; a fork's `main` is an ordinary branch" + ) + cases = [ + (actions_context("push", UPSTREAM, fork_name, "feature"), False, "an upstream branch push"), + (actions_context("push", UPSTREAM, fork_name, "main"), False, "an upstream push to main"), + (actions_context("push", FORK, fork_name, "feature"), True, "a fork branch"), + (actions_context("push", FORK, fork_name, "main"), True, "a push to a fork's main"), + ] + for job in ("supersede", "ci"): + failures.extend(condition_failures(FORK_WORKFLOW, job, cases)) + if re.search(r"^concurrency:", body, re.M): + failures.append( + f"{FORK_WORKFLOW} declares `concurrency`; ci.yml has its own group, and a " + f"second group on the caller deadlocks the pair" + ) + if re.search(r"^ (pull_request|pull_request_target|workflow_dispatch|schedule):", body, re.M): + failures.append(f"{FORK_WORKFLOW} must trigger on `push` and nothing else") + failures.extend(permission_ceiling_failures(FORK_WORKFLOW)) + + if not BRIDGE_WORKFLOW.exists(): + failures.append(f"{BRIDGE_WORKFLOW} is missing; fork pull requests would show no fork result") + else: + body = BRIDGE_WORKFLOW.read_text(encoding="utf-8") + listed = re.search(r"^ workflows:\s*\[(.*?)\]\s*$", body, re.M) + ci_name = workflow_name(WORKFLOWS / "ci.yml") + if not listed or [w.strip() for w in listed.group(1).split(",")] != [ci_name]: + failures.append( + f"{BRIDGE_WORKFLOW} must trigger on `workflow_run` of exactly ci.yml's name, " + f"{ci_name!r}; a rename there silences the bridge" + ) + if "github.repository == 'apache/datafusion-comet'" not in body: + failures.append(f"{BRIDGE_WORKFLOW} has no repository guard, so it would run in every fork") + if f"const WORKFLOW = '{FORK_WORKFLOW.name}';" not in body: + failures.append( + f"{BRIDGE_WORKFLOW} must read runs of {FORK_WORKFLOW.name}; any other name " + f"reports \"no fork CI\" on every pull request" + ) + if re.search(r"uses:\s*actions/checkout@", body): + failures.append( + f"{BRIDGE_WORKFLOW} checks out code. It runs with `checks: write` on data from a " + f"fork and must never check out or run anything from the pull request" + ) + # It writes check runs from a trigger a fork can reach, so it starts with no + # scopes and the one job asks for exactly what it uses. `checks: write` + # creates the check; `pull-requests: read` lists the open pull requests. + # Everything it reads from the fork is public. + if not re.search(r"^permissions:\s*\{\}\s*$", body, re.M): + failures.append(f"{BRIDGE_WORKFLOW} must set `permissions: {{}}` at the top level") + scopes = permission_blocks(job_blocks(BRIDGE_WORKFLOW).get("bridge", "").splitlines()) + if scopes != [{"checks": "write", "pull-requests": "read"}]: + failures.append( + f"{BRIDGE_WORKFLOW}'s bridge job must ask for exactly `checks: write` and " + f"`pull-requests: read`, not {scopes}" + ) + + for failure in failures: + print(f"fork runs: {failure}") + return not failures + + +# Job-level `if:` conditions, evaluated for concrete events. A text match on a +# condition cannot tell the clause that guards an event from one that merely +# mentions the same repository, so the checks below run the expression instead. +SCHEDULE_TRIGGER = re.compile(r"^ schedule:\s*$", re.M) +# A job-level `if:` at four spaces of indent, with its continuation lines when it +# is a folded block (`if: >-`). +JOB_CONDITION = re.compile(r"^ if:[^\n]*\n(?: [^\n]*\n)*", re.M) +JOB_START = re.compile(r"^ ([A-Za-z0-9_-]+):\s*$", re.M) +CONTEXT_PROPERTY = re.compile(r"\b(?:github|inputs|needs)(?:\.[A-Za-z_][A-Za-z0-9_-]*)+") +EXPRESSION_NODES = ( + ast.Expression, ast.BoolOp, ast.And, ast.Or, ast.Compare, ast.Eq, ast.NotEq, + ast.Constant, ast.Call, ast.Name, ast.Load, +) + + +def job_blocks(path): + """{job: the text of its block} for the `jobs:` section of a workflow.""" + section = path.read_text(encoding="utf-8").split("\njobs:\n", 1)[1] + parts = JOB_START.split(section) + return dict(zip(parts[1::2], parts[2::2])) + + +def job_condition(block): + """The text of a job's `if:` with the YAML scalar markers dropped, or None.""" + match = JOB_CONDITION.search(block) + if not match: + return None + text = match.group(0).split("if:", 1)[1] + return re.sub(r"^\s*[>|][-+]?\s*", "", text) + + +class _Comparisons(ast.NodeTransformer): + """Rewrite `a == b` and `a != b` into calls that ignore case, as Actions does.""" + + def visit_Compare(self, node): + self.generic_visit(node) + name = "_eq" if isinstance(node.ops[0], ast.Eq) else "_ne" + call = ast.Call(func=ast.Name(id=name, ctx=ast.Load()), + args=[node.left, node.comparators[0]], keywords=[]) + return ast.copy_location(call, node) + + +def evaluate_condition(text, context): + """Evaluate an `if:` made of ==, !=, &&, ||, parentheses, string literals and + context properties. A property absent from `context` is the empty string, and + strings compare without regard to case, as in Actions: `Main` equals `main`. + Anything else, a function call say, is an error: better to say a condition + cannot be checked than to pass it.""" + expression = " ".join(text.split()) + expression = re.sub(r"^\$\{\{\s*|\s*\}\}$", "", expression) + expression = CONTEXT_PROPERTY.sub(lambda m: f"_v('{m.group(0)}')", expression) + expression = expression.replace("&&", " and ").replace("||", " or ") + tree = ast.parse(expression, mode="eval") + for node in ast.walk(tree): + if not isinstance(node, EXPRESSION_NODES): + raise ValueError(f"unsupported syntax {type(node).__name__} in `{text.strip()}`") + if isinstance(node, ast.Call) and not ( + isinstance(node.func, ast.Name) and node.func.id == "_v" + ): + raise ValueError(f"unsupported call in `{text.strip()}`") + if isinstance(node, ast.Compare) and ( + len(node.ops) != 1 or not isinstance(node.ops[0], (ast.Eq, ast.NotEq)) + ): + raise ValueError(f"unsupported comparison in `{text.strip()}`") + tree = ast.fix_missing_locations(_Comparisons().visit(tree)) + values = { + "_v": lambda name: context.get(name, ""), + "_eq": lambda a, b: str(a).lower() == str(b).lower(), + "_ne": lambda a, b: str(a).lower() != str(b).lower(), + } + return bool(eval(compile(tree, "", "eval"), {"__builtins__": {}}, values)) # noqa: S307 + + +def actions_context(event_name, repository, workflow="", ref_name="feature"): + return { + "github.event_name": event_name, + "github.repository": repository, + "github.workflow": workflow, + "github.ref_name": ref_name, + } + + +def condition_failures(path, job, cases): + """Failures for each (context, expected, description) a job's `if:` gets wrong. + + A job with no `if:` always runs. + """ + block = job_blocks(path).get(job) + if block is None: + return [f"{path} has no job `{job}`"] + condition = job_condition(block) + failures = [] + for context, expected, description in cases: + try: + actual = True if condition is None else evaluate_condition(condition, context) + except (ValueError, SyntaxError) as error: + return [f"{path} job `{job}`: cannot check its `if:`: {error}"] + if actual != expected: + failures.append( + f"{path} job `{job}` {'runs' if actual else 'is skipped'} for {description}, " + f"and it should be {'run' if expected else 'skipped'}" + ) + return failures + + +# Scheduled workflows. GitHub runs `schedule` in any fork that has scheduled +# workflows enabled, on the fork owner's minutes. With no earlier scheduled run +# of its own to diff against, a fork's nightly runs the whole nightly tier, and +# a fork's Miri run costs twenty minutes a day. So every job of a workflow with +# a `schedule` trigger that does not wait on another must be skipped there. +def check_scheduled_workflows_stay_upstream(): + failures = [] + for path in sorted(WORKFLOWS.glob("*.y*ml")): + if not SCHEDULE_TRIGGER.search(path.read_text(encoding="utf-8")): + continue + fork_schedule = actions_context("schedule", FORK, workflow=workflow_name(path)) + for job, block in job_blocks(path).items(): + if re.search(r"^ needs:", block, re.M): + continue + failures.extend( + condition_failures(path, job, [(fork_schedule, False, "a scheduled run in a fork")]) + ) + for failure in failures: + print(f"scheduled workflows: {failure}") + return not failures + + if __name__ == "__main__": ok = check_change_filters() ok = check_event_policy() and ok @@ -1571,6 +1939,8 @@ def check_cache_save_scope(): ok = check_local_actions_have_checkout() and ok ok = check_required_checks() and ok ok = check_label_runs_separate() and ok + ok = check_fork_runs() and ok + ok = check_scheduled_workflows_stay_upstream() and ok ok = check_cache_refresh_scope() and ok ok = check_nightly_scope() and ok ok = check_nightly_base_fallback() and ok diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 2e91d4c91a0..f07abdb8329 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -478,7 +478,8 @@ # Which events may run each job, independent of the path filters above. # -# "pr" every pull request +# "pr" every pull request, except one from a fork (see +# UPSTREAM below), plus a push to a fork # "queue" the merge queue, i.e. a merge_group event # "nightly" the scheduled run against main, once a day # "push" push to main @@ -622,6 +623,45 @@ } +# Pull requests from a contributor's fork do not run the PR tier on ASF runners. +# The contributor's fork runs it instead, on a push to the PR branch (see +# ci_fork.yml), and a bridge workflow (fork_ci_status.yml) mirrors that run onto +# the pull request as the informational `CI (fork)` check. The merge queue still +# runs the queue tier on the merge result, so the fork's verdict is feedback and +# never a gate: a contributor controls their own workflows. A fork PR keeps +# every opt-in label job, and a PR from a branch of this repository, or one that +# targets a release branch, keeps the PR tier upstream. +UPSTREAM = "apache/datafusion-comet" + + +def is_fork_pull_request(event): + """Does the pull request's PR tier run on a fork, not here? + + That is a pull request from a fork of UPSTREAM into the default branch. It + is also any pull request opened inside a fork, whose own branch push has + already run the tier through ci_fork.yml: running it again on the + `pull_request` event would test the same commit twice. + """ + if event.get("name") != "pull_request": + return False + repository = event.get("repository", "") + if repository and repository != UPSTREAM: + return True + head = event.get("head_repository", "") + return ( + repository == UPSTREAM + and bool(head) + and head != UPSTREAM + and event.get("base") == "main" + ) + + +def is_fork_push(event): + """Is this a push to a contributor's fork, rather than to UPSTREAM?""" + repository = event.get("repository", "") + return event.get("name") == "push" and bool(repository) and repository != UPSTREAM + + # Release branches are named `branch-.`, as in branch-0.17 and # branch-1.0. A pull request against one runs these tiers; see the end of the # comment above POLICY. @@ -636,9 +676,11 @@ def gating_labels(job): def event_allows(job, event): """Does `event` permit `job` to run, ignoring which files changed? - `event` is {"name", "action", "label", "labels", "base"}: the workflow event - name, the pull_request action, the label just added on a `labeled` event, - the labels currently on the pull request, and the branch it targets. + `event` is {"name", "action", "label", "labels", "base", "repository", + "head_repository"}: the workflow event name, the pull_request action, the + label just added on a `labeled` event, the labels currently on the pull + request, the branch it targets, the repository the workflow runs in, and + the repository the pull request's branch lives in. """ tiers = POLICY[job] name = event.get("name") @@ -646,6 +688,11 @@ def event_allows(job, event): if name == "workflow_dispatch": return True if name == "push": + # A push to a fork's branch is the contributor's own pull request tier. + # The "push" tier is main's cache refresh and site deploy, and a fork + # has no site to deploy. + if is_fork_push(event): + return "pr" in tiers return "push" in tiers if name == "merge_group": return "queue" in tiers @@ -660,7 +707,7 @@ def event_allows(job, event): if gates: if not any(label in event.get("labels", []) for label in gates): return False - elif "pr" not in tiers: + elif "pr" not in tiers or is_fork_pull_request(event): return False # A `labeled` event fires at the same commit as the opened/synchronize run @@ -702,6 +749,8 @@ def event_from_env(): "label": os.environ.get("LABEL_NAME", ""), "labels": json.loads(labels) if labels.strip() else [], "base": os.environ.get("PR_BASE_REF", ""), + "repository": os.environ.get("REPOSITORY", ""), + "head_repository": os.environ.get("PR_HEAD_REPO", ""), } diff --git a/dev/ci/fork-push-changed-files.sh b/dev/ci/fork-push-changed-files.sh new file mode 100755 index 00000000000..ff89332f907 --- /dev/null +++ b/dev/ci/fork-push-changed-files.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash +# +# 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. + +# Prints the files a push to a branch of a contributor's fork changed, as the +# pull request from that branch would show them: relative to upstream main. +# +# There is no pull request to diff against on a push, and the fork's own default +# branch is the wrong base. A fork that has not synced is behind upstream, so a +# branch based on newer upstream code that reverts one of its files matches the +# fork's old copy, and a diff against it omits the file. Fetching upstream main +# gives the diff the pull request will have. The fetch drops the credentials +# actions/checkout leaves in the git config: they belong to the fork, and the +# upstream repository is public. +# +# Prints every tracked file when there is nothing to go on: the fetch fails or +# there is no merge base. Selecting no suites then would report a green run for a +# change nobody could see. An empty diff is not that case. With a merge base it +# means the branch adds nothing to upstream main, such as a branch pushed before +# its first commit, and the pull request will show nothing either. +# +# Usage: fork-push-changed-files.sh +# UPSTREAM_URL overrides the repository to diff against, for the tests. + +set -euo pipefail + +head="${1:?usage: fork-push-changed-files.sh }" +upstream="${UPSTREAM_URL:-https://github.com/apache/datafusion-comet.git}" +ref="refs/remotes/upstream/main" + +if git -c "http.https://github.com/.extraheader=" fetch --quiet --no-tags "$upstream" "+main:$ref" \ + && git merge-base "$ref" "$head" >/dev/null 2>&1; then + git diff --name-only "$ref"..."$head" +else + git ls-tree -r --name-only "$head" +fi diff --git a/dev/ci/test-fork-ci-status.cjs b/dev/ci/test-fork-ci-status.cjs new file mode 100644 index 00000000000..a8f16bbf70e --- /dev/null +++ b/dev/ci/test-fork-ci-status.cjs @@ -0,0 +1,385 @@ +// 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. + +const assert = require('node:assert/strict'); +const { execFileSync } = require('node:child_process'); +const { readFileSync } = require('node:fs'); +const path = require('node:path'); +const { test } = require('node:test'); + +function workflowText(name) { + return readFileSync(path.join(__dirname, '../../.github/workflows', name), 'utf8'); +} +// The bridge is one github-script step, so its `script: |` block is the code +// under test. Read it textually rather than parse the YAML: that keeps this test +// free of a YAML dependency. +function scriptOf(text) { + const lines = text.split('\n'); + const start = lines.findIndex((line) => /^\s+script: \|\s*$/.test(line)); + assert.notEqual(start, -1, 'no `script: |` block'); + const indent = lines[start + 1].match(/^ */)[0].length; + const body = []; + for (const line of lines.slice(start + 1)) { + if (line.trim() !== '' && !line.startsWith(' '.repeat(indent))) break; + body.push(line.slice(indent)); + } + return body.join('\n'); +} +const bridgeScript = scriptOf(workflowText('fork_ci_status.yml')); +const ciConcurrency = /^concurrency:\n group: (.*)\n cancel-in-progress: (.*)$/m.exec( + workflowText('ci.yml')); +assert.ok(ciConcurrency, 'ci.yml has no top-level concurrency block'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const bridge = new AsyncFunction('github', 'context', 'core', 'Date', bridgeScript); +const now = Date.UTC(2026, 9, 20, 12); +class Clock extends Date { static now() { return now; } } +const sha = 'a'.repeat(40); +const pr = { + number: 10, state: 'open', updated_at: new Date(now).toISOString(), + head: { + sha, ref: 'feature', + repo: { + owner: { login: 'contributor' }, name: 'datafusion-comet', + full_name: 'contributor/datafusion-comet', + }, + }, + base: { ref: 'main' }, +}; +const run = (id = 1, extra = {}) => ({ + id, event: 'push', head_sha: sha, head_branch: 'feature', run_attempt: 1, + status: 'completed', conclusion: 'success', + html_url: 'https://github.com/contributor/datafusion-comet/actions/runs/' + id, + ...extra, +}); + +async function execute({ + event = 'schedule', checks = [], branch = 'feature', runs = [run(1, { head_branch: branch })], + workflowState = 'active', + recent = [pr], slice = [pr], candidates = [pr], repo = 'datafusion-comet', + owner = 'apache', runError, count = 1, notification = {}, checkErrors = {}, +} = {}) { + const writes = [], calls = [], failures = []; + let sweepPage; + const github = { rest: { checks: {}, actions: {}, pulls: {} } }; + github.rest.checks.listForRef = async () => {}; + github.rest.pulls.list = async (params) => { + calls.push(['pulls', params]); + if (params.per_page === 1) return { + data: count ? [pr] : [], + headers: count ? { link: '; rel="last"' } : {}, + }; + assert.equal(params.per_page, 25); + sweepPage = params.page; + return { data: slice }; + }; + github.rest.actions.listWorkflowRuns = async (params) => { + calls.push(['runs', params]); + assert.equal(params.owner, 'contributor'); + assert.equal(params.workflow_id, 'ci_fork.yml'); + assert.equal(params.head_sha, sha); + assert.equal(params.branch, branch); + if (runError) throw Object.assign(new Error('API failure'), { status: runError }); + return { data: { workflow_runs: runs } }; + }; + github.rest.actions.getWorkflow = async () => { + if (workflowState === null) throw Object.assign(new Error('Not Found'), { status: 404 }); + return { data: { state: workflowState } }; + }; + github.rest.checks.create = async (payload) => writes.push({ method: 'create', ...payload }); + github.rest.checks.update = async (payload) => writes.push({ method: 'update', ...payload }); + github.paginate = async (fn, params) => { + if (fn === github.rest.checks.listForRef) { + calls.push(['checks', params]); + if (checkErrors[params.ref]) throw new Error(checkErrors[params.ref]); + return checks; + } + assert.equal(fn, github.rest.pulls.list); + assert.equal(params.head, 'contributor:' + branch); + return candidates; + }; + github.paginate.iterator = async function* () { + for (let i = 0; i < recent.length; i += 100) yield { data: recent.slice(i, i + 100) }; + }; + await bridge(github, { + repo: { owner, repo }, eventName: event, + payload: { workflow_run: { + event: 'pull_request', head_sha: sha, head_branch: branch, + head_repository: pr.head.repo, pull_requests: [], ...notification, + } }, + }, { info: () => {}, setFailed: (message) => failures.push(message) }, Clock); + return { writes, calls, sweepPage, failures }; +} +async function existing(runValue = run()) { + const { writes } = await execute({ runs: [runValue] }); + return { + id: 100, app: { slug: 'github-actions' }, external_id: 'comet-fork-ci:10', + ...writes[0], conclusion: writes[0].conclusion || null, + }; +} + +test('CI completion discovers a fork PR even when the event PR list is empty', async () => { + const { writes } = await execute({ event: 'workflow_run' }); + assert.equal(writes.length, 1); + assert.equal(writes[0].head_sha, sha); + assert.equal(writes[0].name, 'CI (fork)'); + assert.equal(writes[0].conclusion, 'success'); + assert.equal(writes[0].details_url, run().html_url); +}); +test('notifications ignore stale branch heads, release PRs, closed PRs and deleted forks', async () => { + for (const candidate of [ + { ...pr, head: { ...pr.head, sha: 'b'.repeat(40) } }, + { ...pr, base: { ref: 'maintenance' } }, + { ...pr, state: 'closed' }, + { ...pr, head: { ...pr.head, repo: null } }, + ]) { + assert.deepEqual((await execute({ event: 'workflow_run', candidates: [candidate] })).writes, []); + } +}); +test('upstream pushes and merge-queue completions cannot create fork checks', async () => { + for (const event of ['push', 'merge_group']) { + assert.deepEqual((await execute({ event: 'workflow_run', notification: { event } })).writes, []); + } +}); +test('only a push run for the exact branch head is mirrored', async () => { + const result = await execute({ runs: [ + run(8, { head_sha: 'b'.repeat(40) }), + run(7, { head_branch: 'other' }), + run(6, { event: 'pull_request' }), + run(5, { event: 'workflow_dispatch' }), + run(1), + ] }); + assert.equal(result.writes[0].details_url, run(1).html_url); +}); +for (const [state, expected] of [ + ['active', 'neutral'], ['disabled_fork', 'action_required'], [null, 'action_required'], +]) { + test('missing run reports workflow state ' + state, async () => { + const { writes } = await execute({ runs: [], workflowState: state }); + assert.equal(writes[0].conclusion, expected); + assert.match(writes[0].output.summary, /Enable workflows/); + assert.equal(JSON.parse(writes[0].output.text).run_id, null); + }); +} +test('a missing workflow endpoint is handled as missing CI', async () => { + const { writes } = await execute({ runError: 404, workflowState: null }); + assert.equal(writes[0].conclusion, 'action_required'); +}); +test('permission, rate limit and server errors are not reported as missing CI', async () => { + for (const runError of [403, 429, 500]) { + const result = await execute({ runError }); + assert.deepEqual(result.writes, []); + assert.deepEqual(result.failures, ['#10: API failure']); + await assert.rejects(execute({ event: 'workflow_run', runError }), /API failure/); + } +}); +for (const conclusion of ['failure', 'cancelled', 'timed_out', 'startup_failure', 'stale']) { + test('mirrors ' + conclusion + ' without reporting success', async () => { + const { writes } = await execute({ runs: [run(1, { conclusion })] }); + assert.equal(writes[0].conclusion, + ['startup_failure', 'stale'].includes(conclusion) ? 'failure' : conclusion); + }); +} +test('a stale run updates an existing check with failure, not the reserved conclusion', async () => { + const { writes } = await execute({ + checks: [await existing()], runs: [run(1, { conclusion: 'stale' })], + }); + assert.equal(writes[0].method, 'update'); + assert.equal(writes[0].conclusion, 'failure'); +}); +for (const source of ['recent', 'slice']) { + test('the sweep continues after errors in the ' + source + ' PRs and reports every failure', async () => { + const broken = ['b', 'c'].map((letter, index) => ({ + ...pr, number: 20 + index, head: { ...pr.head, sha: letter.repeat(40) }, + })); + const result = await execute({ + recent: [], slice: [], [source]: [...broken, pr], count: 3, + checkErrors: { + [broken[0].head.sha]: 'First API failure', + [broken[1].head.sha]: 'Second API failure', + }, + }); + assert.equal(result.writes.length, 1); + assert.equal(result.writes[0].external_id, 'comet-fork-ci:10'); + assert.equal(result.writes[0].conclusion, 'success'); + assert.deepEqual(result.failures, ['#20: First API failure\n#21: Second API failure']); + }); +} +test('a rerun of the same workflow run updates the existing check', async () => { + const check = await existing(run(1, { conclusion: 'failure' })); + const { writes } = await execute({ checks: [check], runs: [run(1, { run_attempt: 2 })] }); + assert.equal(writes[0].method, 'update'); + assert.equal(writes[0].check_run_id, 100); + assert.equal(writes[0].conclusion, 'success'); + assert.equal(JSON.parse(writes[0].output.text).run_attempt, 2); +}); +test('a deleted run clears its stale success result', async () => { + const { writes } = await execute({ checks: [await existing()], runs: [] }); + assert.equal(writes[0].method, 'update'); + assert.equal(writes[0].conclusion, 'neutral'); +}); +test('unchanged results do not consume check write requests', async () => { + const check = await existing(); + // GitHub can replace details_url with the check's own URL. + for (const details_url of [check.details_url, 'https://github.com/apache/datafusion-comet/runs/100']) { + assert.deepEqual((await execute({ checks: [{ ...check, details_url }] })).writes, []); + } +}); +test('a same-name check from another app or PR is not overwritten', async () => { + const check = await existing(); + for (const foreign of [ + { ...check, app: { slug: 'other-app' } }, + { ...check, external_id: 'comet-fork-ci:20' }, + ]) { + assert.equal((await execute({ checks: [foreign] })).writes[0].method, 'create'); + } +}); +test('recent completed checks wait for the rotating sweep', async () => { + const { writes, calls } = await execute({ checks: [await existing()], slice: [] }); + assert.deepEqual(writes, []); + assert.equal(calls.filter(([kind]) => kind === 'runs').length, 0); +}); +test('late runs are recovered outside the sweep slice', async () => { + const missing = await execute({ runs: [] }); + const check = { ...await existing(), ...missing.writes[0] }; + const { writes } = await execute({ checks: [check], slice: [] }); + assert.equal(writes[0].conclusion, 'success'); +}); +for (const status of ['queued', 'in_progress']) { + test('polls recent ' + status + ' runs outside the sweep slice', async () => { + const check = await existing(run(1, { status, conclusion: null })); + const { writes } = await execute({ checks: [check], slice: [] }); + assert.equal(writes[0].conclusion, 'success'); + }); +} +test('the sweep reaches old PRs beyond 500 open PRs', async () => { + const old = { ...pr, updated_at: new Date(now - 24 * 3600 * 1000).toISOString() }; + const { writes, sweepPage } = await execute({ count: 600, recent: [old], slice: [old] }); + assert.equal(sweepPage, Math.floor(now / (15 * 60 * 1000)) % 24 + 1); + assert.equal(writes.length, 1); +}); +test('an active PR appearing in both lists is polled once', async () => { + const { calls } = await execute(); + assert.equal(calls.filter(([kind]) => kind === 'runs').length, 1); +}); +test('no open PRs means no fork API calls', async () => { + const result = await execute({ count: 0 }); + assert.equal(result.calls.length, 1); + assert.deepEqual(result.writes, []); +}); + +test('a PR from a branch called main, the default branch included, is mirrored', async () => { + // ci_fork.yml runs every branch of a fork, so there is always one workflow to look up. + const mainPr = { ...pr, head: { ...pr.head, ref: 'main' } }; + const { writes } = await execute({ + branch: 'main', recent: [mainPr], slice: [mainPr], candidates: [mainPr], + }); + assert.equal(writes.length, 1); + assert.equal(writes[0].conclusion, 'success'); + assert.equal(writes[0].details_url, run().html_url); +}); + +function group(github) { + return ciConcurrency[1].replace(/\$\{\{\s*(.*?)\s*\}\}/g, + (_, expression) => new Function('github', 'return (' + expression + ');')(github)); +} +const forkPush = { + // GitHub expressions resolve absent properties to an empty string. + repository: 'contributor/datafusion-comet', event_name: 'push', event: { pull_request: { number: '' } }, + ref: 'refs/heads/feature', sha: 'first', run_id: 1, workflow: 'Comet CI (fork run)', +}; +const pullRequest = (number, extra = {}) => ({ + ...forkPush, repository: 'apache/datafusion-comet', event_name: 'pull_request', + event: { pull_request: { number } }, ref: 'refs/pull/' + number + '/merge', ...extra, +}); +test('every fork push has a group of its own, so none cancels another', () => { + // A backup branch created at the commit a pull request is building has the same + // sha, so a group keyed on the commit would let its run cancel the build. + // ci_fork.yml's supersede job cancels older runs of exactly one branch instead, + // which a group cannot: names compare without regard to case. + assert.notEqual(group(forkPush), group({ ...forkPush, run_id: 2 })); + assert.notEqual(group(forkPush), group({ ...forkPush, run_id: 2, ref: 'refs/heads/backup' })); + assert.notEqual(group(forkPush), group({ ...forkPush, sha: 'second', run_id: 2 })); +}); +test('upstream pushes and queue runs stay separate per commit', () => { + for (const event_name of ['push', 'merge_group']) { + const upstream = { ...forkPush, repository: 'apache/datafusion-comet', event_name }; + assert.notEqual(group(upstream), group({ ...upstream, sha: 'second' })); + } + const upstream = { ...forkPush, repository: 'apache/datafusion-comet' }; + assert.notEqual(group(upstream), group({ ...upstream, event_name: 'merge_group' })); +}); +test('a push to a pull request supersedes the last one, whatever the commit', () => { + assert.equal(group(pullRequest(7, { sha: 'first' })), group(pullRequest(7, { sha: 'second' }))); +}); +test('same-named branches from different forks cannot cancel each other', () => { + // github.head_ref is the branch name alone, so it is the same for both. + const one = pullRequest(7, { head_ref: 'patch-1' }); + const other = pullRequest(8, { head_ref: 'patch-1' }); + assert.notEqual(group(one), group(other)); +}); +test('a label run is not cancelled by the commit run, or by another label', () => { + const labeled = (name) => pullRequest(7, { event: { action: 'labeled', label: { name }, pull_request: { number: 7 } } }); + assert.notEqual(group(labeled('run-iceberg-tests')), group(pullRequest(7))); + assert.notEqual(group(labeled('run-iceberg-tests')), group(labeled('run-delta-tests'))); +}); + +// The supersede job in ci_fork.yml cancels older runs of exactly one branch. +const supersedeScript = scriptOf(workflowText('ci_fork.yml')); +const supersede = new AsyncFunction('github', 'context', 'core', supersedeScript); +async function runSupersede({ runs, cancelError, branch = 'feature', runNumber = 5 }) { + const cancelled = []; + const github = { rest: { actions: {} } }; + github.rest.actions.listWorkflowRuns = async () => {}; + github.rest.actions.cancelWorkflowRun = async ({ run_id }) => { + if (cancelError) throw Object.assign(new Error('conflict'), { status: cancelError }); + cancelled.push(run_id); + }; + let calls = 0; + github.paginate = async (fn, params) => { + assert.equal(fn, github.rest.actions.listWorkflowRuns); + assert.equal(params.workflow_id, 'ci_fork.yml'); + // One listing per status, so each run appears once. + return runs.filter((r) => r.status === params.status); + }; + await supersede(github, { repo: { owner: 'contributor', repo: 'datafusion-comet' }, + ref: 'refs/heads/' + branch, runNumber }, { info: () => { calls += 1; } }); + return cancelled; +} +const branchRun = (id, extra = {}) => ({ + id, run_number: id, head_branch: 'feature', status: 'in_progress', ...extra, +}); +test('a push cancels the older queued and running runs of its branch', async () => { + const cancelled = await runSupersede({ runs: [ + branchRun(2), branchRun(3, { status: 'queued' }), branchRun(4), + ] }); + assert.deepEqual(cancelled.sort(), [2, 3, 4]); +}); +test('a push does not cancel itself, a newer run or a rerun of itself', async () => { + const cancelled = await runSupersede({ runs: [branchRun(5), branchRun(6), branchRun(7)] }); + assert.deepEqual(cancelled, []); +}); +test('a push cancels no run of a branch that differs only by case', async () => { + const cancelled = await runSupersede({ runs: [ + branchRun(2, { head_branch: 'Feature' }), branchRun(3, { head_branch: 'other' }), branchRun(4), + ] }); + assert.deepEqual(cancelled, [4]); +}); +test('a run that finished before it was cancelled is not an error', async () => { + const cancelled = await runSupersede({ runs: [branchRun(2)], cancelError: 409 }); + assert.deepEqual(cancelled, []); + await assert.rejects(runSupersede({ runs: [branchRun(2)], cancelError: 500 }), /conflict/); +}); diff --git a/dev/ci/test-fork-push-changed-files.py b/dev/ci/test-fork-push-changed-files.py new file mode 100755 index 00000000000..11671c397bd --- /dev/null +++ b/dev/ci/test-fork-push-changed-files.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +# +# 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 dev/ci/fork-push-changed-files.sh, on real git repositories.""" + +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + +SCRIPT = Path(__file__).with_name("fork-push-changed-files.sh").resolve() +GIT_ENV = { + "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@example.com", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@example.com", + "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_SYSTEM": os.devnull, +} + + +def git(cwd, *args): + result = subprocess.run( + ["git", *args], cwd=cwd, check=True, capture_output=True, text=True, + env={**os.environ, **GIT_ENV}) + return result.stdout.strip() + + +def write(repo, path, text): + file = Path(repo, path) + file.parent.mkdir(parents=True, exist_ok=True) + file.write_text(text) + + +def commit(repo, message, files): + for path, text in files.items(): + write(repo, path, text) + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", message) + return git(repo, "rev-parse", "HEAD") + + +class ForkPushChangedFiles(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + root = Path(self.tmp.name) + self.upstream = root / "upstream" + self.fork = root / "fork" + git(root, "init", "-q", "-b", "main", str(self.upstream)) + commit(self.upstream, "base", {"README.md": "a\n", "native/lib.rs": "1\n", "spark/Plan.scala": "1\n"}) + git(root, "clone", "-q", str(self.upstream), str(self.fork)) + git(self.fork, "checkout", "-q", "-b", "work") + + def changed(self, head=None, url=None): + head = head or git(self.fork, "rev-parse", "HEAD") + result = subprocess.run( + [str(SCRIPT), head], cwd=self.fork, check=True, capture_output=True, text=True, + env={**os.environ, **GIT_ENV, "UPSTREAM_URL": url or str(self.upstream)}) + return set(result.stdout.split()) + + def test_lists_what_the_branch_changed(self): + commit(self.fork, "native", {"native/lib.rs": "2\n"}) + self.assertEqual(self.changed(), {"native/lib.rs"}) + + def test_docs_only_branch_selects_no_code(self): + commit(self.fork, "docs", {"README.md": "b\n"}) + self.assertEqual(self.changed(), {"README.md"}) + + def test_revert_of_newer_upstream_code_is_kept_when_the_fork_is_stale(self): + # The fork's main stays at the base while upstream moves on and changes a + # native file. A branch cut from the newer upstream that reverts the file + # matches the fork's stale main, so a diff against it would omit the file. + commit(self.upstream, "upstream native change", {"native/lib.rs": "2\n"}) + # Fetch by URL so the fork's own origin/main stays at the base. + git(self.fork, "fetch", "-q", str(self.upstream), "main") + git(self.fork, "checkout", "-q", "-b", "revert", "FETCH_HEAD") + commit(self.fork, "revert", {"native/lib.rs": "1\n", "README.md": "b\n"}) + self.assertEqual(self.changed(), {"native/lib.rs", "README.md"}) + + def test_commits_made_on_the_fork_main_are_kept_when_pushed_to_a_branch(self): + # `git push fork main:my-fix`: the fork's main holds commits upstream does + # not have, and the pull request from the branch will show them. + commit(self.fork, "native", {"native/lib.rs": "2\n"}) + git(self.fork, "checkout", "-q", "main") + git(self.fork, "merge", "-q", "--ff-only", "work") + self.assertEqual(self.changed(), {"native/lib.rs"}) + + def test_code_then_docs_commits_keep_the_code_change(self): + commit(self.fork, "native", {"native/lib.rs": "2\n"}) + commit(self.fork, "docs", {"README.md": "b\n"}) + self.assertEqual(self.changed(), {"native/lib.rs", "README.md"}) + + def test_an_unreachable_upstream_runs_everything(self): + commit(self.fork, "docs", {"README.md": "b\n"}) + self.assertEqual( + self.changed(url=str(Path(self.tmp.name, "missing"))), + {"README.md", "native/lib.rs", "spark/Plan.scala"}) + + def test_a_branch_that_adds_nothing_to_upstream_selects_nothing(self): + # A branch pushed before its first commit: the pull request will show no + # change, so there is nothing to test. + self.assertEqual(self.changed(), set()) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index c567e6f0a75..9088ddbcf22 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -100,6 +100,9 @@ verdict like any other suite. See [Opting a pull request into a suite the PR tier skips](#opting-a-pull-request-into-a-suite-the-pr-tier-skips) for the mechanics. +For a pull request from a fork, the pull request column's "yes" rows run on your fork rather than +here; see [Pull requests from forks](#pull-requests-from-forks). + Every job's result feeds one flat job named `Required Checks`, and that is the only status check `main` requires. A job that is skipped because the change did not touch its inputs counts as a pass. A job that fails or is cancelled turns `Required Checks` red. @@ -111,6 +114,69 @@ cases that pin both down. A pull request against a release branch runs all three tiers at once; see [Release branches](#release-branches). +## Pull requests from forks + +Nearly every pull request comes from a fork, and the PR tier is the largest share of the runner time +the project spends on CI. So for a pull request from a fork into `main`, the PR tier runs on **your +fork's** GitHub runners, not on the Apache Software Foundation's shared ones. What stays here is the +preflight checks, `Required Checks`, and any `run-*` suite a maintainer opts the pull request +into. The merge queue, which also stays here, runs the queue tier on the merge result before anything +lands: the PR tier plus Spark SQL on 4.1, Iceberg 1.11, macOS and the other queue-tier suites in the +table above. The nightly-only suites and the opt-in `run-*` suites are not part of it, and path +filters still apply. This is the arrangement `apache/datafusion` and `apache/arrow-rs` use. + +To get it working: + +1. **Enable GitHub Actions on your fork.** GitHub disables workflows on a new fork. Open the + fork's **Actions** tab and choose **I understand my workflows, go ahead and enable them**. +2. **Sync your fork and rebase your branch onto `main`.** The workflows that run on your fork are + the ones in your branch, so a branch cut before this arrangement existed runs nothing. +3. **Push your branch.** Each push to a branch of your fork starts a `Comet CI (fork run)` in + your fork's **Actions** tab, and a newer push to the same branch cancels the run in flight. It runs + the PR tier for the files your branch changes relative to upstream `main`, which is what the pull + request will show: the Linux build, lints, Rust tests, TPC-H and TPC-DS, and the Comet test suites + against Spark 4.1. If that comparison cannot be made, it runs the whole tier. + +On the pull request, a `CI (fork)` check links to that run and mirrors its result. It is first +written when the upstream run finishes. After that, a pull request updated in the last six hours +whose check is still running or has no run yet is rechecked every 15 minutes. A check that already +shows a result is rechecked only by a sweep that takes 25 open pull requests every 15 minutes, which +is about two hours to go round 200 of them. So after you re-run a failed job in your fork, the pull +request can keep showing the old result for hours. Your fork's **Actions** tab is the source of truth: +read the details and re-run failed jobs there. If the check says **Enable CI on your fork** or **No fork CI +run for this commit**, no run was found for the pull request's head commit: Actions is disabled, the +branch does not include the fork workflows, or the run has not appeared yet. + +Some things to know: + +- **`CI (fork)` is informational.** `Required Checks` does not depend on it, because a contributor + controls the workflow that produced it. A green fork run is a good sign, not a merge gate. The + queue is the gate, and it tests your change merged with `main` and everything queued ahead of it, + where the fork run tests your branch head alone. +- **Caches are cold unless `main` builds.** A branch restores caches saved on its own ref or on your + fork's default branch, and a run on your fork's `main` saves them only when it builds. A push to + `main` that adds nothing to upstream, such as syncing your fork, selects no suite, so it warms + nothing, and a branch run rebuilds the native library. A run of the PR tier on a fork with cold + caches took about an hour of wall time and 240 runner-minutes, of which the native build alone was + 23 minutes. The same run with warm caches took 50 minutes and 213 runner-minutes. +- **Scheduled runs stay upstream.** The nightly run, Miri, the stale-PR closer and the weekly + CodeQL scan run in `apache/datafusion-comet` only. If your fork has scheduled workflows enabled they + start no job. Miri can still be run on your fork by dispatching it. +- **Opt-in suites still run here.** A fork cannot see the `run-*` labels on your pull request, so + those suites, and the `labeled` runs that start when a maintainer applies one, run here. For + `run-all-spark-profiles` that is the other four Spark profiles, while your fork runs the default + profile and the lints, so together they cover every profile. See + [Opting a pull request into a suite the PR tier skips](#opting-a-pull-request-into-a-suite-the-pr-tier-skips). +- **Not everything moves.** A pull request from a branch of this repository, such as a + committer's or dependabot's, and a pull request that targets a release branch, keep the PR tier here. + A release branch has no queue to catch what a fork run misses. +- **Opening a pull request inside your own fork** leaves the PR tier to your branch push, so the + same commit is not tested twice. A `run-*` label applied there still runs its suite. + +If you cannot enable Actions on your fork, say so on the pull request. Nothing else will run the PR +tier for you before the queue does, so a maintainer may ask you to reproduce a failure with +[`dev/local-ci.sh`](#reproducing-a-suite-failure-locally) first. + ## Opting a pull request into a suite the PR tier skips Each suite outside the PR tier has a label that runs it on a pull request: @@ -449,7 +515,8 @@ The umbrella workflow, the reusable workflows it calls, and the routing tables a `dev/ci/check-ci-config.py`, which runs in preflight. It enforces that every job feeds `Required Checks`, that the required check name in `.asf.yaml` matches the job that publishes it, that artifact names are unique per producer, that the routing policy matches its test cases, and -that every job in `pr_build_linux.yml` is either a cache writer or skipped on push. Run it locally +that every job in `pr_build_linux.yml` is either a cache writer or skipped on push, and that the fork +workflows (`ci_fork.yml`, `fork_ci_status.yml`) and `ci.yml`'s push trigger agree. Run it locally before pushing a CI change: ```sh diff --git a/docs/source/contributor-guide/development.md b/docs/source/contributor-guide/development.md index 199ec017d40..8efec3ea7c3 100644 --- a/docs/source/contributor-guide/development.md +++ b/docs/source/contributor-guide/development.md @@ -646,6 +646,10 @@ the risk of accidentally overwriting another maintainer's commits when multiple to the same PR branch. If the lease check rejects the push, inspect and integrate the remote changes before retrying; do not switch to `--force` to bypass the check. +For a pull request from a fork, the pull request tier of CI runs on your fork's GitHub runners +rather than the project's. Enable GitHub Actions on your fork and keep its `main` in sync before you +push; see [Pull requests from forks](ci.md#pull-requests-from-forks). + Before submitting a pull request, follow this checklist to ensure your changes are ready: ### 1. Format Your Code