From f2e5fa60c8d943af8daa9384a1db1baa7a9d8015 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 28 Oct 2026 14:07:37 -0600 Subject: [PATCH] ci: run the pull request tier for fork pull requests on the fork Nearly every pull request comes from a fork, and the pull request tier plus label runs were about 60% of the runner-hours ci.yml and ci_label.yml spent in the week to 2026-10-28. For a pull request from a fork into main, the tier now runs on the contributor's fork. The merge queue stays on ASF runners and still runs every suite on the merge result. - compute-changes.py: a push to a fork gets the `pr` tier, and a pull request from a fork into main loses it. Every `run-*` label suite and the `labeled` run are unchanged, as are pull requests from a branch of this repository, pull requests to a release branch, and pull requests opened inside a fork. - ci_fork.yml: calls ci.yml on a push to any fork branch except main. It is a separate workflow so a push to a branch of this repository cannot publish a skipped `Required Checks` next to a pull request's real one. Upstream its only job is skipped. - ci.yml: a fork push diffs against the fork's default branch, and its concurrency group is keyed on the branch so the next push cancels the run. - fork_ci_status.yml: mirrors the fork's run onto the pull request as an informational `CI (fork)` check, adapted from apache/arrow-rs#11788 and apache/datafusion#25591. It never checks out pull request code. The fork result is advisory because a contributor controls the workflow. - check-ci-config.py: policy cases for fork pushes and fork pull requests, the new environment wiring, and a check that the caller, the bridge and ci.yml's push trigger agree. - docs: contributor guide and workflows README. --- .github/workflows/README.md | 57 +++- .github/workflows/ci.yml | 29 +- .github/workflows/ci_fork.yml | 71 ++++ .github/workflows/fork_ci_status.yml | 238 +++++++++++++ dev/ci/check-ci-config.py | 237 ++++++++++++- dev/ci/compute-changes.py | 49 ++- dev/ci/test-fork-ci-status.cjs | 338 +++++++++++++++++++ docs/source/contributor-guide/ci.md | 56 ++- docs/source/contributor-guide/development.md | 4 + 9 files changed, 1053 insertions(+), 26 deletions(-) create mode 100644 .github/workflows/ci_fork.yml create mode 100644 .github/workflows/fork_ci_status.yml create mode 100644 dev/ci/test-fork-ci-status.cjs diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 1976ca2632a..ba3512b76b9 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,57 @@ 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. +- **The fork's push to `main` uses `ci.yml` directly,** since `ci.yml` already + triggers on push to `main`, and `ci_fork.yml` ignores `main` so the two never + both fire. That run is the PR tier for whatever the push changed, which also + writes the fork's cache entries. A branch restores caches saved on its own + ref or on the fork's default branch only, so a fork whose `main` has never + run starts every branch cold (about 27 minutes for the native build alone). + The bridge reads `ci.yml` for a pull request opened from a fork's `main`, and + `ci_fork.yml` for every other branch. +- **`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. +- **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 diffs against an older `main`, which selects more suites, never + fewer. +- **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 +353,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..57d43ed1387 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,9 +73,11 @@ 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 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.head_ref || (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 @@ -194,6 +196,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 +287,10 @@ 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 }} + PUSH_REF: ${{ github.ref }} 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 +338,22 @@ 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" && "$PUSH_REF" != "refs/heads/$DEFAULT_BRANCH" ]]; then + # A push to a feature branch of a contributor's fork (ci_fork.yml). + # There is no pull request to diff against, and the fork cannot know + # what upstream main is, so diff against the fork's own default + # branch. A fork that has not synced yet diffs against an older + # main, which can only select more suites than the pull request + # needs, never fewer. Without a default branch to diff against, or + # without a merge base, run everything the tier selects. + if git merge-base "origin/$DEFAULT_BRANCH" HEAD >/dev/null 2>&1; then + git diff --name-only "origin/$DEFAULT_BRANCH"..."$PUSH_AFTER" > changed_files.txt + else + git ls-tree -r --name-only "$PUSH_AFTER" > changed_files.txt + fi else - # push to main; first push to a branch has all-zero before sha + # push to main, upstream or a fork's own; 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..24e3bb5ebbc --- /dev/null +++ b/.github/workflows/ci_fork.yml @@ -0,0 +1,71 @@ +# 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. +# +# `main` is excluded because ci.yml already runs on push to main, in a fork as +# upstream. There it refreshes the fork's cache entries and runs the PR tier +# for what the push changed, which is what makes a fork's branch runs fast: a +# branch can restore caches saved on its own ref or on the default branch only. +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 + # Merge queue branches exist only in this repository; the guard below + # would skip them anyway. + - 'gh-readonly-queue/**' + +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. + 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/fork_ci_status.yml b/.github/workflows/fork_ci_status.yml new file mode 100644 index 00000000000..2c89eafdd1c --- /dev/null +++ b/.github/workflows/fork_ci_status.yml @@ -0,0 +1,238 @@ +# 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, or ci.yml for a PR from the +# fork's main) 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: + if: github.repository == 'apache/datafusion-comet' && (github.event_name != 'workflow_run' || github.event.workflow_run.event == 'pull_request') + name: Mirror the fork run + runs-on: ubuntu-slim + timeout-minutes: 10 + steps: + - name: Create or update the check + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + 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)'; + // A branch is built by ci_fork.yml; a fork's main, which that workflow + // skips, by ci.yml itself. + const workflowFor = (pr) => (pr.head.ref === 'main' ? 'ci.yml' : '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: workflowFor(pr), 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 { + const workflow = await optional(() => github.rest.actions.getWorkflow({ + ...fork, workflow_id: workflowFor(pr), + })); + 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 (repository === 'apache/datafusion-comet') { + if (context.eventName === 'workflow_run') await notify(); + else await update(); + } diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index c752a50c266..8a5ff9103ba 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 eight 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,11 @@ # 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. +# # Run from the repository root: python3 dev/ci/check-ci-config.py import importlib.util @@ -243,6 +248,9 @@ 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; see UPSTREAM in compute-changes.py. +UPSTREAM = "apache/datafusion-comet" + POLICY_CASES = [ # A manual run may exercise anything. ({"name": "workflow_dispatch"}, ALL_JOBS), @@ -511,6 +519,90 @@ }, 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. + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": [], + "base": "main", + "repository": UPSTREAM, + "head_repository": "contributor/datafusion-comet", + }, + set(), + ), + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": ["run-spark-4.1-tests", "run-delta-tests"], + "base": "main", + "repository": UPSTREAM, + "head_repository": "contributor/datafusion-comet", + }, + {"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. + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-all-spark-profiles", + "labels": ["run-all-spark-profiles"], + "base": "main", + "repository": UPSTREAM, + "head_repository": "contributor/datafusion-comet", + }, + {"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), one that targets a + # release branch, which has no queue or nightly to catch what a fork run + # misses, and one opened inside a fork, whose own CI is the only CI. + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": [], + "base": "main", + "repository": UPSTREAM, + "head_repository": UPSTREAM, + }, + PR_TIER, + ), + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": [], + "base": "branch-1.1", + "repository": UPSTREAM, + "head_repository": "contributor/datafusion-comet", + }, + QUEUE_TIER | NIGHTLY_TIER, + ), + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": [], + "base": "main", + "repository": "contributor/datafusion-comet", + "head_repository": "contributor/datafusion-comet", + }, + PR_TIER, + ), + # A push to a fork 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. The same holds on the fork's + # own main, which is why a sync of the fork refreshes its caches. + ({"name": "push", "repository": "contributor/datafusion-comet"}, PR_TIER), + # Upstream, push stays the cache refresh and the site deploy. + ({"name": "push", "repository": UPSTREAM}, {"docs", "build_linux", "build_macos"}), ] @@ -787,6 +879,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 +927,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 +1296,32 @@ def ci_triggers(): return triggers +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. @@ -1242,21 +1366,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 +1670,100 @@ 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" + ) + 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 re.search(r"^\s+uses:\s*\./\.github/workflows/ci\.yml\s*$", body, re.M): + failures.append(f"{FORK_WORKFLOW} does not call ./.github/workflows/ci.yml") + ignored = re.search(r"^ branches-ignore:\n((?: .*\n)+)", body, re.M) + entries = ( + [ + line.strip()[2:].strip() + for line in ignored.group(1).splitlines() + if line.strip().startswith("- ") + ] + if ignored + else [] + ) + for needed in ("main", "asf-site", "'gh-readonly-queue/**'"): + if needed not in entries: + failures.append(f"{FORK_WORKFLOW} must list {needed} under `push: branches-ignore`") + if not re.search( + r"^ if:\s*github\.repository\s*!=\s*'apache/datafusion-comet'\s*$", body, re.M + ): + 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 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 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 + + if __name__ == "__main__": ok = check_change_filters() ok = check_event_policy() and ok @@ -1571,6 +1775,7 @@ 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_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..3f504c307cd 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,35 @@ } +# 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): + """Is this a pull request from a fork of UPSTREAM into the default branch?""" + head = event.get("head_repository", "") + return ( + event.get("name") == "pull_request" + and event.get("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 +666,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 +678,11 @@ def event_allows(job, event): if name == "workflow_dispatch": return True if name == "push": + # A push to a fork is the contributor's own pull request tier. The + # "push" tier is main's cache refresh and site deploy, and a fork's + # main 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 +697,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 +739,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..022fdbff8cc --- /dev/null +++ b/dev/ci/test-fork-ci-status.cjs @@ -0,0 +1,338 @@ +// 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 = [], 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('the bridge does nothing when copied into a fork', async () => { + const result = await execute({ owner: 'contributor' }); + assert.deepEqual(result.calls, []); + assert.deepEqual(result.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, []); +}); + +test('a PR from the fork main branch reads ci.yml, the workflow that builds main', async () => { + const mainPr = { ...pr, head: { ...pr.head, ref: 'main' } }; + const asked = []; + const github = { rest: { checks: {}, actions: {}, pulls: {} } }; + github.rest.checks.listForRef = async () => {}; + github.rest.checks.create = async () => {}; + github.rest.pulls.list = async () => ({ data: [mainPr], headers: {} }); + github.rest.actions.listWorkflowRuns = async (params) => { + asked.push(params.workflow_id); + return { data: { workflow_runs: [] } }; + }; + github.rest.actions.getWorkflow = async (params) => { + asked.push(params.workflow_id); + return { data: { state: 'active' } }; + }; + github.paginate = async (fn) => (fn === github.rest.checks.listForRef ? [] : [mainPr]); + github.paginate.iterator = async function* () { yield { data: [mainPr] }; }; + await bridge(github, { + repo: { owner: 'apache', repo: 'datafusion-comet' }, eventName: 'schedule', payload: {}, + }, { info: () => {}, setFailed: (message) => assert.fail(message) }, Clock); + assert.deepEqual(asked, ['ci.yml', 'ci.yml']); +}); + +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: {}, head_ref: '', + ref: 'refs/heads/feature', sha: 'first', workflow: 'Comet CI (fork run)', +}; +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, queue runs and pull requests stay separate per commit or head', () => { + 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' })); + const pull = { ...upstream, event_name: 'pull_request', head_ref: 'feature' }; + assert.equal(group(pull), group({ ...pull, sha: 'second' })); +}); diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index c567e6f0a75..ee6c27b8e13 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,56 @@ 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. Keep your + fork's own `main` in sync as well, for the reason under "The first run is slow" below. +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. + +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. +- **The first run is slow.** Caches are scoped to a branch or to the repository's default branch. + A fork whose `main` has never run starts every branch with empty caches. A cold 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; with warm caches the native build takes about 4. Syncing your fork's `main` runs + the workflow there once, which writes the caches your branches then restore. +- **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** runs the full PR tier there, as it did before. + +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 +502,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