diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 1976ca2632a..01a7452ac29 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,72 @@ 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 everything on the merge result before anything +lands. 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 `main` and calls `ci.yml`. Upstream its only job is skipped. | +| `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. +- **A push to a fork's `main` runs nothing.** It is a sync of commits the queue + already tested, and `ci.yml` triggers on it because it triggers on push to + `main`, so its `Preflight` skips the run, and with it every job. The same + skip covers a fork whose default branch is not `main`. The cost is that a + fork never has warm caches, so 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. A sync that did warm them would cost about + as much as it saved on its first run. The bridge answers a pull request from + a fork's default branch with a neutral "open the pull request from a branch", + because nothing tests it. +- **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 diffs against the fork's default branch.** There is no pull + request to diff against, and a fork does not know what upstream `main` is, so + `Detect changes` uses `git diff origin/...`. A fork that + has not synced selects more suites, not fewer. With nothing to go on, or a diff + that is empty, it runs everything: a branch whose commits are already on the + default branch, such as `git push fork main:my-fix`, would otherwise skip every + suite and report a green run for a change nobody built. +- **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` and `codeql.yml` carry a + `github.repository == 'apache/datafusion-comet'` guard on a scheduled + event, and `check-ci-config.py` fails any workflow with a `schedule` trigger + that lacks one. A guard on `Preflight` skips the whole nightly, since every + other job needs `changes`, which needs it. +- **`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 +368,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..6a719accdb7 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 everything 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,13 @@ 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 branch instead of its commit, so the +# next push to the branch cancels the run in flight. 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.ref) || 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 +129,29 @@ 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 two runs that belong to no pull request, and + # with it the whole run, since every other job needs `changes`, which needs + # this: a scheduled run in a fork, which would otherwise run the whole nightly + # tier for want of an earlier run to diff against, and a push to a fork's + # default branch, which is a sync of commits the queue already tested. # --------------------------------------------------------------------------- preflight: name: Preflight + if: >- + (github.event_name != 'schedule' || github.repository == 'apache/datafusion-comet') + && (github.event_name != 'push' || github.repository == 'apache/datafusion-comet' + || github.ref_name != github.event.repository.default_branch) runs-on: ubuntu-slim steps: - uses: actions/checkout@v7 @@ -194,6 +215,9 @@ 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 Iceberg shard inventory validation run: python3 dev/ci/test-iceberg-shards.py @@ -282,6 +306,9 @@ 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 }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} 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 +356,17 @@ 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). The + # fork cannot know what upstream main is, so diff against its own + # default branch; a fork that has not synced selects more suites, + # not fewer. With nothing to go on, run everything the tier selects: + # a branch already on the default branch (`git push fork main:fix`) + # diffs to nothing, and that would report green for an unbuilt change. + git diff --name-only "origin/$DEFAULT_BRANCH"..."$PUSH_AFTER" > changed_files.txt 2>/dev/null || true + [[ -s changed_files.txt ]] || git ls-tree -r --name-only "$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 diff --git a/.github/workflows/ci_fork.yml b/.github/workflows/ci_fork.yml new file mode 100644 index 00000000000..80fe1d9b004 --- /dev/null +++ b/.github/workflows/ci_fork.yml @@ -0,0 +1,70 @@ +# 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 every suite 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. +# +# A fork's default branch is excluded because a push to it is a sync of upstream, +# which the merge queue already tested. ci.yml still triggers on a push to `main` +# there, but its Preflight job skips that run. A fork's caches are therefore never +# warm; see "Pull requests from forks" in docs/source/contributor-guide/ci.md for +# what that costs. +name: Comet CI (fork run) + +# No `concurrency` here. ci.yml declares its own group, which for a push to a +# fork is keyed on the branch, so the next push to the branch supersedes this +# one. The same group on the caller as well would deadlock the two. +on: + push: + branches-ignore: + - main + - asf-site + +permissions: + contents: read + +jobs: + ci: + name: Fork run + # 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. In a fork the default branch is skipped, whatever it is + # called; `branches-ignore` above can only name `main`. + if: github.repository != 'apache/datafusion-comet' && github.ref_name != github.event.repository.default_branch + # 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..319d58d3a36 --- /dev/null +++ b/.github/workflows/fork_ci_status.yml @@ -0,0 +1,260 @@ +# 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 every suite 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: + +permissions: + actions: read + checks: write + pull-requests: read + +# 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 + 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; + + // A fork's default branch is never tested: ci_fork.yml skips it, and a + // push there is a sync, which runs nothing. Looking for a run would + // find none, or worse, one that tested nothing and passed. + const fromMain = pr.head.ref === pr.head.repo.default_branch; + const data = fromMain ? null : 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. The fork workflow on push/manual events + // 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 && + ['push', 'workflow_dispatch'].includes(item.event) + ).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 if (fromMain) { + payload = { + status: 'completed', + conclusion: 'neutral', + details_url: GUIDE, + output: { + title: 'Open the pull request from a branch', + summary: [ + 'This pull request comes from the ' + pr.head.ref + ' branch of ' + pr.head.repo.full_name + + ', and CI does not run on a fork’s default branch: a push there is a sync of upstream, ' + + 'so nothing tests this change before the merge queue does.', + 'Push the change to a branch of your fork and open the pull request from it.', + '[Fork CI setup](' + GUIDE + ')', + ].join('\n\n'), + text: JSON.stringify({ head_sha: sha, run_id: null }), + }, + }; + } 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..a67fc5d9f73 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,6 +73,14 @@ # 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 importlib.util @@ -243,6 +251,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 +540,40 @@ }, 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 run adds the non-default Spark profiles to the fork's default + # profile run, exactly as it does for any other pull request. + ( + 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. A push to the + # fork's default branch, a sync, never reaches this table: Preflight skips it + # (see check_fork_runs). + ({"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 +840,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 +855,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 +903,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 +1272,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 +1333,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 +1346,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 +1650,144 @@ 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") + + +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 push to a fork's default branch, a sync of commits the + # queue already tested, is dropped: ci.yml triggers on push to main, so it + # fires there too, and every other job needs Preflight through `changes`. + preflight = re.search(r"^ preflight:\n(?: (?!runs-on)[^\n]*\n|\n)*", ci, re.M) + if not preflight or "github.event.repository.default_branch" not in preflight.group(0): + failures.append( + "ci.yml's Preflight must skip a push to a fork's default branch " + "(`github.ref_name != github.event.repository.default_branch`); " + "otherwise every sync of a fork runs CI" + ) + if FORK_TEST.name not in ci: + failures.append(f"ci.yml's Preflight does not run {FORK_TEST}, so the bridge 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") + ignored = re.search(r"^ branches-ignore:\n((?: .*\n)+)", body, re.M) + for needed in ("main", "asf-site"): + if not ignored or f"- {needed}\n" not in ignored.group(1): + failures.append(f"{FORK_WORKFLOW} must list {needed} under `push: branches-ignore`") + conditions = " ".join(job_conditions(body)) + if "github.repository != 'apache/datafusion-comet'" not in conditions: + failures.append( + f"{FORK_WORKFLOW} has no `if: github.repository != 'apache/datafusion-comet'` " + f"guard, so every upstream branch push would run the PR tier a second time" + ) + if "github.event.repository.default_branch" not in conditions: + failures.append( + f"{FORK_WORKFLOW} does not skip the fork's default branch, so a sync of a fork " + f"whose default branch is not `main` would start a run; `branches-ignore` " + f"can only name `main`" + ) + if re.search(r"^concurrency:", body, re.M): + failures.append( + f"{FORK_WORKFLOW} declares `concurrency`; ci.yml's own group is keyed on the " + f"branch for a fork push, and a 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" + ) + if re.search(r"^\s+(contents|pull-requests|issues):\s*write\s*$", body, re.M): + failures.append(f"{BRIDGE_WORKFLOW} asks for more than `checks: write` and read access") + + for failure in failures: + print(f"fork runs: {failure}") + return not 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. Every workflow with a `schedule` +# trigger therefore needs a job-level guard on the repository. +SCHEDULE_TRIGGER = re.compile(r"^ schedule:\s*$", re.M) +UPSTREAM_GUARD = re.compile(r"github\.repository\s*==\s*'apache/datafusion-comet'") +# 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) + + +def job_conditions(body): + return JOB_CONDITION.findall(body) + + +def check_scheduled_workflows_stay_upstream(): + failures = [] + for path in sorted(WORKFLOWS.glob("*.y*ml")): + body = path.read_text(encoding="utf-8") + if SCHEDULE_TRIGGER.search(body) and not any( + UPSTREAM_GUARD.search(c) for c in job_conditions(body) + ): + failures.append( + f"{path} has a `schedule` trigger but no job-level " + f"`if: ... github.repository == 'apache/datafusion-comet'`, so it " + f"runs in every fork that enables scheduled workflows" + ) + 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 +1799,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..d1ad802420f 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 everything 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,12 @@ 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. A push to a fork's default branch never gets + # here: Preflight skips it, as a sync of commits the queue already tested. + if is_fork_push(event): + return "pr" in tiers return "push" in tiers if name == "merge_group": return "queue" in tiers @@ -660,7 +708,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 +750,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/test-fork-ci-status.cjs b/dev/ci/test-fork-ci-status.cjs new file mode 100644 index 00000000000..278bde75ccf --- /dev/null +++ b/dev/ci/test-fork-ci-status.cjs @@ -0,0 +1,343 @@ +// 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', default_branch: 'main', + }, + }, + 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 = [], runs = [run()], 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, 'feature'); + 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:feature'); + 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: 'feature', + 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 runs for the exact branch head on push/manual events are 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(5).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 manual run can replace a failed push run', async () => { + const check = await existing(run(1, { conclusion: 'failure' })); + const { writes } = await execute({ + checks: [check], runs: [run(2, { event: 'workflow_dispatch' }), run(1)], + }); + assert.equal(writes[0].details_url, run(2).html_url); +}); +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, []); +}); + +for (const defaultBranch of ['main', 'develop']) { + test('a PR from the fork default branch ' + defaultBranch + ' is told to use a branch', async () => { + const head = { + ...pr.head, ref: defaultBranch, repo: { ...pr.head.repo, default_branch: defaultBranch }, + }; + const mainPr = { ...pr, head }; + const { writes, calls } = await execute({ recent: [mainPr], slice: [mainPr] }); + assert.equal(calls.filter(([kind]) => kind === 'runs').length, 0, 'a default branch has no run to read'); + assert.equal(writes.length, 1); + assert.equal(writes[0].conclusion, 'neutral'); + assert.equal(writes[0].output.title, 'Open the pull request from a branch'); + }); +} + +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', 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('successive fork pushes share a cancellation group, separate branches do not', () => { + assert.equal(ciConcurrency[2], 'true'); + assert.equal(group(forkPush), group({ ...forkPush, sha: 'second' })); + assert.notEqual(group(forkPush), group({ ...forkPush, ref: 'refs/heads/other' })); +}); +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'))); +}); diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index c567e6f0a75..8278dc12c39 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,64 @@ 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 every suite on the merge result before anything +lands. 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 other than `main` 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 your fork's `main`: the Linux build, + lints, Rust tests, TPC-H and TPC-DS, and the Comet test suites against Spark 4.1. If that + comparison finds nothing, because the commits are already on your fork's `main`, it runs the + whole tier. + +On the pull request, a `CI (fork)` check links to that run and mirrors its result. It is updated when +the upstream run finishes and every 15 minutes after, so it can lag the fork. Read the details in +your fork 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. +- **Runs are cold.** Caches are scoped to a branch or to the repository's default branch, and a + push to your fork's `main` runs nothing, because it is a sync of commits the merge queue already + tested. So the Cargo and Maven caches are never warm on a fork, and a run rebuilds the native + library: a run of the PR tier on a fork 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. +- **Open the pull request from a branch.** Nothing runs on your fork's `main`, so a pull request + from it gets no CI before the merge queue. `CI (fork)` says so. +- **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 + skip. 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, are unchanged. 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 +510,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