From 91e1d893dc5fc45c1c0e29948d2ff1ba219d3ca5 Mon Sep 17 00:00:00 2001 From: sdf-jkl Date: Wed, 21 Oct 2026 18:42:12 -0400 Subject: [PATCH 1/4] Run fork PR test suites in contributor forks --- .github/ci/scripts/check-ci-config.py | 93 ++++++- .github/ci/scripts/compute-changes.py | 31 ++- .github/ci/scripts/test-fork-ci-status.cjs | 271 +++++++++++++++++++++ .github/pull_request_template.md | 4 + .github/workflows/README.md | 21 +- .github/workflows/ci.yml | 36 ++- .github/workflows/docs.yml | 2 +- .github/workflows/fork_ci_status.yml | 213 ++++++++++++++++ .github/workflows/rust.yml | 8 + CONTRIBUTING.md | 14 ++ 10 files changed, 673 insertions(+), 20 deletions(-) create mode 100644 .github/ci/scripts/test-fork-ci-status.cjs create mode 100644 .github/workflows/fork_ci_status.yml diff --git a/.github/ci/scripts/check-ci-config.py b/.github/ci/scripts/check-ci-config.py index 2648251b4fb4..f027d51772fe 100644 --- a/.github/ci/scripts/check-ci-config.py +++ b/.github/ci/scripts/check-ci-config.py @@ -56,6 +56,31 @@ "parquet": {"pyspark-integration-test": {"Install Python dependencies"}}, } FULL_RUN = "${{ !inputs.cache-refresh-only }}" +CI_EVENT = "github.event_name != 'push' || github.repository != 'apache/arrow-rs' || github.ref == 'refs/heads/main'" +LINT_ONLY = "${{ needs.changes.outputs.fork-pr == 'true' }}" + + +def validate_fork_ci(workflows, ci): + """Fork feedback cannot replace the upstream merge-queue gate.""" + jobs = ci["jobs"] + assert jobs["changes"]["if"] == CI_EVENT, "Unexpected CI event guard" + assert jobs["changes"]["outputs"].get("fork-pr") == "${{ steps.select.outputs.fork-pr }}", "Missing fork PR output" + assert (jobs["rust"].get("with") or {}).get("lint-only") == LINT_ONLY, "Missing Rust lint mode" + rust = load_yaml(workflows / "rust.yml") + for name, job in rust["jobs"].items(): + if name in {"lint", "unused-dependencies"}: + assert "if" not in job, f"rust/{name}: lightweight check must run upstream" + else: + assert job.get("if") == "${{ !inputs.lint-only }}", f"rust/{name}: runs during lint-only mode" + docs = load_yaml(workflows / "docs.yml") + assert docs["jobs"]["deploy"]["if"] == "github.repository == 'apache/arrow-rs' && github.ref_name == 'main' && github.event_name == 'push'", "Forks must not deploy docs" + bridge = load_yaml(workflows / "fork_ci_status.yml") + assert set(bridge[True]) == {"workflow_run", "schedule", "workflow_dispatch"}, "Unexpected fork bridge triggers" + assert bridge[True]["workflow_run"] == {"workflows": ["CI"], "types": ["completed"]}, "Bridge must follow CI" + assert bridge["permissions"] == {"actions": "read", "checks": "write", "pull-requests": "read"}, "Unexpected bridge permissions" + steps = bridge["jobs"]["bridge"]["steps"] + assert len(steps) == 1 and steps[0]["uses"].startswith("actions/github-script@"), "Bridge must not check out or execute PR code" + assert "${{" not in steps[0]["with"]["script"], "Do not interpolate untrusted values into JavaScript" def validate_cache_refresh(ci, suite, workflow): @@ -106,18 +131,28 @@ def validate_config(root=ROOT): # PyYAML's YAML 1.1 loader treats the key 'on' as the boolean True. events = ci[True] assert jobs["changes"]["outputs"].get("cache-refresh-only") == "${{ steps.select.outputs.cache-refresh-only }}", "Missing cache mode output" - assert set(events) == {"pull_request", "push", "merge_group"}, "Unexpected CI triggers" + assert set(events) == {"pull_request", "push", "merge_group", "workflow_dispatch"}, "Unexpected CI triggers" assert events["pull_request"] is None, "Required CI must run on every PR" assert events["merge_group"] == {"types": ["checks_requested"]}, "Missing queue trigger" - assert events["push"] == {"branches": ["main"]}, "Only main pushes should run CI" + assert events["push"] == {"branches-ignore": ["asf-site"]}, "Fork branches must trigger CI" + validate_fork_ci(workflows, ci) gate = jobs["required-checks"] assert set(gate["needs"]) == set(jobs) - {"required-checks"}, "Required Checks omits a job" - assert gate["if"] == "always()", "Required Checks must run after failures and cancellations" + assert gate["if"] == f"always() && ({CI_EVENT})", "Required Checks must run after failures and cancellations" asf = load_yaml(root / ".asf.yaml")["github"] checks = asf["protected_branches"]["main"]["required_status_checks"] assert checks["contexts"] == [gate["name"]], "Required check name does not match .asf.yaml" assert checks["strict"] is False, "The queue handles up-to-date checks" + assert any( + ruleset.get("enforcement") == "active" + and ruleset.get("target") == "branch" + and ruleset.get("conditions", {}).get("ref_name") == { + "include": ["~DEFAULT_BRANCH"], "exclude": [], + } + and any(rule.get("type") == "merge_queue" for rule in ruleset.get("rules", [])) + for ruleset in asf.get("rulesets", []) + ), "Fork PRs require an active merge queue on the default branch" called = set() for suite in suites: @@ -134,6 +169,11 @@ def validate_config(root=ROOT): "type": "boolean", "default": False, }}} validate_cache_refresh(ci, suite, workflow) + if suite == "rust": + call = {"inputs": {"lint-only": { + "description": "Run only formatting and unused dependency checks", + "type": "boolean", "default": False, + }}} assert workflow[True] == {"workflow_call": call}, f"{suite}: duplicate or missing triggers/inputs" assert "concurrency" not in workflow, f"{suite}: concurrency is owned by ci.yml" if suite in filters["suites"]: @@ -171,19 +211,51 @@ def test_event_policy(self): with self.assertRaises(ValueError): self.select("pull_request_target", []) + def test_fork_pr_policy(self): + for base, expected in [("main", {"dev", "rust"}), + ("maintenance", self.all_suites)]: + selected = changes.select_suites( + "pull_request", ["Cargo.toml"], self.filters, + head_repository="contributor/arrow-rs", base_ref=base) + self.assertEqual(set(selected), expected) + # PRs inside a contributor's fork retain ordinary path-based testing. + selected = changes.select_suites( + "pull_request", ["Cargo.toml"], self.filters, + repository="contributor/arrow-rs", head_repository="other/arrow-rs") + self.assertEqual(set(selected), self.all_suites) + + def test_fork_push_and_manual_runs(self): + for repository, event in [("contributor/arrow-rs", "push"), + ("contributor/arrow-rs", "workflow_dispatch"), + (changes.UPSTREAM, "workflow_dispatch")]: + selected = changes.select_suites(event, [], self.filters, repository=repository) + self.assertEqual(set(selected), self.all_suites) + # Even a queue event with fork metadata must run the full validation. + selected = changes.select_suites("merge_group", [], self.filters, + head_repository="contributor/arrow-rs") + self.assertEqual(set(selected), self.all_suites | {"miri"}) + def test_selector_outputs(self): - for event, expected in [("push", {"docs", "integration", "parquet"}), - ("merge_group", self.all_suites | {"miri"}), - ("pull_request", self.always)]: - with self.subTest(event=event), tempfile.TemporaryDirectory() as directory: + cases = [ + ("push", changes.UPSTREAM, "", {"docs", "integration", "parquet"}, True, False), + ("push", "contributor/arrow-rs", "", self.all_suites, False, False), + ("merge_group", changes.UPSTREAM, "", self.all_suites | {"miri"}, False, False), + ("pull_request", changes.UPSTREAM, changes.UPSTREAM, self.always, False, False), + ("pull_request", changes.UPSTREAM, "contributor/arrow-rs", {"dev", "rust"}, False, True), + ("workflow_dispatch", "contributor/arrow-rs", "", self.all_suites, False, False), + ] + for event, repo, head_repo, expected, cache_only, fork_pr in cases: + with self.subTest(event=event, repo=repo, head_repo=head_repo), tempfile.TemporaryDirectory() as directory: output = Path(directory) / "output" env = dict(os.environ, GITHUB_EVENT_NAME=event, GITHUB_OUTPUT=str(output), + GITHUB_REPOSITORY=repo, PR_HEAD_REPOSITORY=head_repo, PR_BASE_REF="main", PR_BASE_SHA="HEAD", PR_HEAD_SHA="HEAD") subprocess.run([sys.executable, str(ROOT / ".github/ci/scripts/compute-changes.py")], env=env, check=True, capture_output=True) values = dict(line.split("=", 1) for line in output.read_text().splitlines()) self.assertEqual(set(json.loads(values["suites"])), expected) - self.assertEqual(json.loads(values["cache-refresh-only"]), event == "push") + self.assertEqual(json.loads(values["cache-refresh-only"]), cache_only) + self.assertEqual(json.loads(values["fork-pr"]), fork_pr) def test_pr_path_routing(self): cases = [ @@ -234,6 +306,11 @@ def test_current_configuration(self): def test_configuration_regressions(self): cases = [ + (".asf.yaml", "enforcement: active", "enforcement: disabled"), + (".asf.yaml", "type: merge_queue", "type: deletion"), + (".github/workflows/rust.yml", " if: ${{ !inputs.lint-only }}\n", ""), + (".github/workflows/ci.yml", " lint-only: ${{ needs.changes.outputs.fork-pr == 'true' }}\n", ""), + (".github/workflows/docs.yml", "github.repository == 'apache/arrow-rs' && ", ""), (".asf.yaml", '"Required Checks"', '"Missing Check"'), (".github/workflows/ci.yml", " - miri\n", ""), (".github/workflows/ci.yml", " pull_request:\n", " pull_request:\n paths: ['arrow/**']\n"), diff --git a/.github/ci/scripts/compute-changes.py b/.github/ci/scripts/compute-changes.py index 0b8706206c5d..b9ba40fbbfa3 100644 --- a/.github/ci/scripts/compute-changes.py +++ b/.github/ci/scripts/compute-changes.py @@ -25,6 +25,13 @@ ROOT = Path(__file__).resolve().parents[3] ALWAYS = {"dev", "rust", "docs"} +UPSTREAM = "apache/arrow-rs" + + +def is_fork_pr(event, repository, head_repository, base_ref): + return (event == "pull_request" and repository == UPSTREAM + and bool(head_repository) and head_repository != repository + and base_ref == "main") def load_yaml(path): @@ -33,16 +40,24 @@ def load_yaml(path): return yaml.safe_load(path.read_text()) -def select_suites(event, paths, filters): +def select_suites(event, paths, filters, *, repository=UPSTREAM, + head_repository=UPSTREAM, base_ref="main"): suites = set(filters["suites"]) | ALWAYS if event == "merge_group": return sorted(suites | {"miri"}) + if event == "workflow_dispatch" or (event == "push" and repository != UPSTREAM): + # Fork pushes test the branch head, including pushes to the fork's main. + # Unlike upstream main, these commits have not passed the merge queue. + return sorted(suites) if event == "push": # The queue already tested the merge result. Publish docs and populate # main's caches through the reusable workflows' cache-refresh-only mode. return ["docs", "integration", "parquet"] if event != "pull_request": raise ValueError(f"Unsupported CI event: {event}") + if is_fork_pr(event, repository, head_repository, base_ref): + # Keep quick checks upstream; the queue still selects every suite. + return ["dev", "rust"] def matches(patterns): return any(fnmatch.fnmatchcase(path, pattern) for path in paths for pattern in patterns) @@ -54,8 +69,12 @@ def matches(patterns): def main(): event = os.environ["GITHUB_EVENT_NAME"] + repository = os.environ["GITHUB_REPOSITORY"] + head_repository = os.environ.get("PR_HEAD_REPOSITORY", "") + base_ref = os.environ.get("PR_BASE_REF", "") + fork_pr = is_fork_pr(event, repository, head_repository, base_ref) paths = [] - if event == "pull_request": + if event == "pull_request" and not fork_pr: base = os.environ["PR_BASE_SHA"] head = os.environ["PR_HEAD_SHA"] # Full checkout history includes both PR parents. Disable rename @@ -65,10 +84,14 @@ def main(): cwd=ROOT, ) paths = changed.decode("utf-8", errors="surrogateescape").split("\0")[:-1] - suites = select_suites(event, paths, load_yaml(ROOT / ".github/ci/paths.yaml")) + suites = select_suites(event, paths, load_yaml(ROOT / ".github/ci/paths.yaml"), + repository=repository, head_repository=head_repository, + base_ref=base_ref) + cache_refresh = event == "push" and repository == UPSTREAM with open(os.environ["GITHUB_OUTPUT"], "a") as output: output.write(f"suites={json.dumps(suites)}\n") - output.write(f"cache-refresh-only={json.dumps(event == 'push')}\n") + output.write(f"cache-refresh-only={json.dumps(cache_refresh)}\n") + output.write(f"fork-pr={json.dumps(fork_pr)}\n") print(f"Selected suites: {', '.join(suites)}") diff --git a/.github/ci/scripts/test-fork-ci-status.cjs b/.github/ci/scripts/test-fork-ci-status.cjs new file mode 100644 index 000000000000..406e235d54ed --- /dev/null +++ b/.github/ci/scripts/test-fork-ci-status.cjs @@ -0,0 +1,271 @@ +// 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 workflow(name) { + return JSON.parse(execFileSync('python3', [ + '-c', 'import json, sys, yaml; print(json.dumps(yaml.safe_load(sys.stdin)))', + ], { + input: readFileSync(path.join(__dirname, '../../workflows', name)), + encoding: 'utf8', + })); +} +const bridgeWorkflow = workflow('fork_ci_status.yml'); +const ciWorkflow = workflow('ci.yml'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const bridge = new AsyncFunction('github', 'context', 'core', 'Date', + bridgeWorkflow.jobs.bridge.steps[0].with.script); +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: 'arrow-rs', full_name: 'contributor/arrow-rs' }, + }, + 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/arrow-rs/actions/runs/' + id, + ...extra, +}); + +async function execute({ + event = 'schedule', checks = [], runs = [run()], workflowState = 'active', + recent = [pr], slice = [pr], candidates = [pr], repo = 'arrow-rs', + owner = 'apache', runError, count = 1, notification = {}, +} = {}) { + const writes = [], calls = []; + 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.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]); + 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: () => {} }, Clock); + return { writes, calls, sweepPage }; +} +async function existing(runValue = run()) { + const { writes } = await execute({ runs: [runValue] }); + return { + id: 100, app: { slug: 'github-actions' }, external_id: 'arrow-rs-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]) { + await assert.rejects(execute({ runError }), /API failure/); + } +}); +for (const conclusion of ['failure', 'cancelled', 'timed_out', 'startup_failure']) { + test('mirrors ' + conclusion + ' without reporting success', async () => { + const { writes } = await execute({ runs: [run(1, { conclusion })] }); + assert.equal(writes[0].conclusion, conclusion === 'startup_failure' ? 'failure' : conclusion); + }); +} +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 () => { + assert.deepEqual((await execute({ checks: [await existing()] })).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: 'arrow-rs-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, []); +}); + +function group(github) { + return ciWorkflow.concurrency.group.replace(/\$\{\{\s*(.*?)\s*\}\}/g, + (_, expression) => new Function('github', 'return (' + expression + ');')(github)); +} +const forkPush = { + // GitHub expressions resolve absent properties to an empty string. + repository: 'contributor/arrow-rs', event_name: 'push', event: { pull_request: { number: '' } }, + ref: 'refs/heads/feature', sha: 'first', workflow: 'CI', +}; +test('successive fork pushes share a cancellation group, separate branches do not', () => { + assert.equal(ciWorkflow.concurrency['cancel-in-progress'], true); + assert.equal(group(forkPush), group({ ...forkPush, sha: 'second' })); + assert.notEqual(group(forkPush), group({ ...forkPush, ref: 'refs/heads/other' })); +}); +test('upstream main and merge queue runs remain separate per commit', () => { + for (const event_name of ['push', 'merge_group']) { + const upstream = { ...forkPush, repository: 'apache/arrow-rs', event_name }; + assert.notEqual(group(upstream), group({ ...upstream, sha: 'second' })); + } +}); +test('same-named branches from different fork PRs cannot cancel each other', () => { + const event = { pull_request: { number: 1 } }; + const context = { ...forkPush, repository: 'apache/arrow-rs', event_name: 'pull_request', event }; + assert.notEqual(group(context), group({ ...context, event: { pull_request: { number: 2 } } })); +}); diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index c3720f6cc704..44727862b05a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,3 +1,7 @@ +> [!IMPORTANT] +> +> Fork PRs to `main` require [enabling CI in your fork](https://github.com/apache/arrow-rs/blob/main/CONTRIBUTING.md#pull-requests-from-forks). + # Which issue does this PR close? -CI runs a subset of checks on pull requests, selected by the changed files. +CI runs lightweight checks upstream for fork pull requests to `main`. Fork +branch pushes and manual CI runs execute all regular suites, excluding Miri. +Other pull requests run suites selected by their changed files upstream. The merge queue runs the full test suite against the proposed merge result. -Pushes to `main` populate caches and publish documentation without repeating tests. +Pushes to upstream `main` populate caches and publish documentation without repeating tests. A single required status check combines the results and blocks merging on failure. + +`fork_ci_status.yml` mirrors fork results in an informational `CI (fork)` check, +using `workflow_run` and scheduled polling. It reads fork runs and writes upstream +checks through GitHub APIs without checking out PR code or consuming PR artifacts. +See [fork CI setup](../../CONTRIBUTING.md#pull-requests-from-forks). + +Run the routing and bridge tests with Python 3.11+, PyYAML, and Node.js 24: + +```shell +python3 .github/ci/scripts/check-ci-config.py +node --test .github/ci/scripts/test-fork-ci-status.cjs +``` + +Bridge tests mock GitHub APIs. End-to-end testing requires a test repository +with the upstream guards adapted and the bridge installed on its default branch. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcd624568c49..839e4618937a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,10 +22,13 @@ on: merge_group: types: [checks_requested] push: - branches: [main] + # Forks need CI on feature branches as well as main. The selector job + # ignores upstream feature-branch pushes, which are tested by their PRs. + branches-ignore: [asf-site] + workflow_dispatch: concurrency: - group: ${{ github.repository }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.sha }}-${{ github.workflow }} + group: ${{ github.repository }}-${{ github.event_name }}-${{ github.event.pull_request.number || (github.repository != 'apache/arrow-rs' && github.ref) || github.sha }}-${{ github.workflow }} cancel-in-progress: true permissions: @@ -34,10 +37,12 @@ permissions: jobs: changes: name: Select and validate CI suites + if: github.event_name != 'push' || github.repository != 'apache/arrow-rs' || github.ref == 'refs/heads/main' runs-on: ubuntu-latest outputs: suites: ${{ steps.select.outputs.suites }} cache-refresh-only: ${{ steps.select.outputs.cache-refresh-only }} + fork-pr: ${{ steps.select.outputs.fork-pr }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -56,11 +61,18 @@ jobs: done - name: Validate CI configuration and routing run: python3 .github/ci/scripts/check-ci-config.py + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: '24' + - name: Test fork CI status bridge + run: node --test .github/ci/scripts/test-fork-ci-status.cjs - name: Select suites id: select env: PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} run: python3 .github/ci/scripts/compute-changes.py arrow: @@ -101,6 +113,8 @@ jobs: needs: changes if: contains(fromJSON(needs.changes.outputs.suites), 'rust') uses: ./.github/workflows/rust.yml + with: + lint-only: ${{ needs.changes.outputs.fork-pr == 'true' }} docs: needs: changes @@ -119,7 +133,7 @@ jobs: # Like datafusion-comet: one stable check name even when a reusable # workflow is skipped (its individual check names would be different). name: Required Checks - if: always() + if: always() && (github.event_name != 'push' || github.repository != 'apache/arrow-rs' || github.ref == 'refs/heads/main') permissions: {} needs: - changes @@ -138,8 +152,20 @@ jobs: env: NEEDS: ${{ toJSON(needs) }} run: echo "$NEEDS" - # Path-filtered suites and Miri outside the queue are intentionally - # skipped. A failed/cancelled selector or suite must still block merging. + - name: Link to fork CI + if: needs.changes.outputs.fork-pr == 'true' + env: + FORK_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + { + printf 'Full test results: [CI on %s](https://github.com/%s/actions/workflows/ci.yml).\n\n' "$FORK_REPOSITORY" "$FORK_REPOSITORY" + printf 'Look for commit `%s`. Enable Actions in your fork and push your branch if no run exists.\n\n' "$PR_HEAD_SHA" + printf 'This upstream PR run checks formatting and contributor hygiene. The merge queue runs the full suite before merge.\n' + } >> "$GITHUB_STEP_SUMMARY" + # Fork PRs to main only run lightweight checks here. The informational + # CI (fork) check is separate; the merge queue runs every suite here. + # A failed/cancelled selector or suite must still block merging. - name: Fail if any suite failed or was cancelled if: needs.changes.result != 'success' || contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') run: | diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 9838bf6de349..69021a3d330f 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -57,7 +57,7 @@ jobs: deploy: # Only deploy if a push to main - if: github.ref_name == 'main' && github.event_name == 'push' + if: github.repository == 'apache/arrow-rs' && github.ref_name == 'main' && github.event_name == 'push' needs: docs permissions: contents: write diff --git a/.github/workflows/fork_ci_status.yml b/.github/workflows/fork_ci_status.yml new file mode 100644 index 000000000000..114f8769e411 --- /dev/null +++ b/.github/workflows/fork_ci_status.yml @@ -0,0 +1,213 @@ +# 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 fork results using reviewed default-branch code and public API reads. +# No checkout, artifacts, or code from the pull request are used here. +name: Fork CI status + +on: + workflow_run: + workflows: [CI] + types: [completed] + schedule: + - cron: '7,22,37,52 * * * *' + workflow_dispatch: + +permissions: + actions: read + checks: write + pull-requests: read + +# Serialize notifications and sweeps so they cannot create duplicate checks. +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +jobs: + bridge: + if: github.repository == 'apache/arrow-rs' && (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)'; + const WORKFLOW = 'ci.yml'; + const upstream = { owner: context.repo.owner, repo: context.repo.repo }; + const repository = upstream.owner + '/' + upstream.repo; + const GUIDE = 'https://github.com/apache/arrow-rs/blob/main/CONTRIBUTING.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) => 'arrow-rs-fork-ci:' + pr.number; + const conclusions = new Set([ + 'action_required', 'cancelled', 'failure', 'neutral', 'skipped', 'stale', 'success', 'timed_out', + ]); + const UPDATE_NOTE = + 'This check is informational: the contributor controls the fork workflow. ' + + 'The upstream merge queue runs every suite, including Miri, on the proposed merge result. ' + + 'Active or missing runs on recently updated PRs are polled every 15 minutes; ' + + 'a rotating sweep also discovers reruns on older PRs. Scheduled updates may be delayed.'; + + // An absent workflow/run is expected on a fork with Actions disabled. Other + // API errors must fail the bridge rather than falsely reporting missing CI. + async function optional(request) { + try { + return (await request()).data; + } catch (error) { + if (error.status === 404) return null; + throw error; + } + } + + async function syncCheck(pr, includeCompleted = true) { + if (!isForkPr(pr)) return; + const fork = { owner: pr.head.repo.owner.login, repo: pr.head.repo.name }; + const sha = pr.head.sha; + const checks = await github.paginate(github.rest.checks.listForRef, { + ...upstream, ref: sha, check_name: CHECK, per_page: 100, + }); + const check = checks.find((item) => + item.app?.slug === 'github-actions' && item.external_id === externalId(pr)); + let previous = {}; + try { previous = JSON.parse(check?.output?.text || '{}'); } catch {} + // A missing run must keep being polled, even though its check is completed. + if (!includeCompleted && check?.status === 'completed' && previous.run_id) return; + + const data = await optional(() => github.rest.actions.listWorkflowRuns({ + ...fork, workflow_id: WORKFLOW, head_sha: sha, branch: pr.head.ref, per_page: 100, + })); + // Ignore PR merge-ref runs. The named CI 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: 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 or manually run CI on that 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.details_url === payload.details_url && + 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 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 syncCheck(pr, sweepNumbers.has(pr.number)); + } + if (stale) break; + } + for (const pr of slice) { + if (!seen.has(pr.number)) await syncCheck(pr); + } + } + + if (repository === 'apache/arrow-rs') { + if (context.eventName === 'workflow_run') await notify(); + else await update(); + } diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 25882c4c4e46..d811263684f9 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -21,6 +21,11 @@ name: rust # Triggered by ci.yml, which owns event routing and concurrency. on: workflow_call: + inputs: + lint-only: + description: Run only formatting and unused dependency checks + type: boolean + default: false permissions: contents: read @@ -29,6 +34,7 @@ jobs: # Check workspace wide compile and test with default features for # mac macos: + if: ${{ !inputs.lint-only }} name: Test on Mac runs-on: macos-latest steps: @@ -52,6 +58,7 @@ jobs: # Check workspace wide compile and test with default features for # windows windows: + if: ${{ !inputs.lint-only }} name: Test on Windows runs-on: windows-latest steps: @@ -120,6 +127,7 @@ jobs: run: cargo shear --deny-warnings arrow-pyarrow-integration-testing msrv: + if: ${{ !inputs.lint-only }} name: Verify MSRV (Minimum Supported Rust Version) runs-on: ubuntu-latest container: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 581498be232f..a36f7503d016 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,6 +73,20 @@ When possible, we recommend splitting your contributions into multiple smaller focused PRs rather than large PRs, because they are more likely to be reviewed quickly and are easier to understand and review. +### Pull Requests from Forks + +To reduce load on ASF's shared runners, pull requests from forks to `main` +run tests in the fork and lightweight checks upstream. + +1. Open the **Actions** tab in your fork and enable workflows before pushing. +2. Sync your fork and rebase your branch onto upstream `main` to include the + current CI workflows. +3. Push your branch, or manually run `CI` from your fork's **Actions** tab. + View results and rerun failed jobs there; the PR's `CI (fork)` check may lag. + +Fork results are informational. The merge queue tests the proposed merge with +`main`, including Miri. Same-repository and release-branch PRs retain upstream testing. + ## Developer's guide to Arrow Rust ### Setting Up Your Build Environment From ee6f0b8e029fa3b14503e74866d51dd6bcb862ca Mon Sep 17 00:00:00 2001 From: Kosta Tarasov <33369833+sdf-jkl@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:25:19 -0400 Subject: [PATCH 2/4] Update CONTRIBUTING.md Co-authored-by: Andrew Lamb --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a36f7503d016..b8fe45501b29 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,7 +76,8 @@ quickly and are easier to understand and review. ### Pull Requests from Forks To reduce load on ASF's shared runners, pull requests from forks to `main` -run tests in the fork and lightweight checks upstream. +run the CPU intensive tests on your fork (using the forks github runners). To +enable actions in your fork: 1. Open the **Actions** tab in your fork and enable workflows before pushing. 2. Sync your fork and rebase your branch onto upstream `main` to include the From db1c466f6cbfeeefc27f8b835bfce0fd1a33a226 Mon Sep 17 00:00:00 2001 From: sdf-jkl Date: Fri, 23 Oct 2026 11:01:52 -0400 Subject: [PATCH 3/4] Address fork CI review feedback --- .github/ci/scripts/check-ci-config.py | 2 +- .github/ci/scripts/compute-changes.py | 2 +- .github/ci/scripts/test-fork-ci-status.cjs | 44 ++++++++++++++++++---- .github/workflows/README.md | 22 ++--------- .github/workflows/ci.yml | 4 +- .github/workflows/fork_ci_status.yml | 26 ++++++++++--- CONTRIBUTING.md | 10 +++-- 7 files changed, 72 insertions(+), 38 deletions(-) diff --git a/.github/ci/scripts/check-ci-config.py b/.github/ci/scripts/check-ci-config.py index 6081696d93ba..ba72b863d778 100644 --- a/.github/ci/scripts/check-ci-config.py +++ b/.github/ci/scripts/check-ci-config.py @@ -17,7 +17,7 @@ """Check CI configuration and routing policy, following Comet's structure. -Run from the repository root: +Run from the repository root with Python 3.11+ and PyYAML: python3 .github/ci/scripts/check-ci-config.py """ diff --git a/.github/ci/scripts/compute-changes.py b/.github/ci/scripts/compute-changes.py index 9b88140ebbb9..5c1eac29f074 100644 --- a/.github/ci/scripts/compute-changes.py +++ b/.github/ci/scripts/compute-changes.py @@ -56,7 +56,7 @@ def select_suites(event, paths, filters, *, repository=UPSTREAM, if event != "pull_request": raise ValueError(f"Unsupported CI event: {event}") if is_fork_pr(event, repository, head_repository, base_ref): - # Keep quick checks upstream; the queue still selects every suite. + # Keep quick checks upstream; the queue still selects all regular suites. return ["dev", "rust"] def matches(patterns): diff --git a/.github/ci/scripts/test-fork-ci-status.cjs b/.github/ci/scripts/test-fork-ci-status.cjs index 406e235d54ed..52637e5b0544 100644 --- a/.github/ci/scripts/test-fork-ci-status.cjs +++ b/.github/ci/scripts/test-fork-ci-status.cjs @@ -55,9 +55,9 @@ const run = (id = 1, extra = {}) => ({ async function execute({ event = 'schedule', checks = [], runs = [run()], workflowState = 'active', recent = [pr], slice = [pr], candidates = [pr], repo = 'arrow-rs', - owner = 'apache', runError, count = 1, notification = {}, + owner = 'apache', runError, count = 1, notification = {}, checkErrors = {}, } = {}) { - const writes = [], calls = []; + const writes = [], calls = [], failures = []; let sweepPage; const github = { rest: { checks: {}, actions: {}, pulls: {} } }; github.rest.checks.listForRef = async () => {}; @@ -89,6 +89,7 @@ async function execute({ 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); @@ -104,8 +105,8 @@ async function execute({ event: 'pull_request', head_sha: sha, head_branch: 'feature', head_repository: pr.head.repo, pull_requests: [], ...notification, } }, - }, { info: () => {} }, Clock); - return { writes, calls, sweepPage }; + }, { info: () => {}, setFailed: (message) => failures.push(message) }, Clock); + return { writes, calls, sweepPage, failures }; } async function existing(runValue = run()) { const { writes } = await execute({ runs: [runValue] }); @@ -169,13 +170,42 @@ test('a missing workflow endpoint is handled as missing CI', async () => { }); test('permission, rate limit and server errors are not reported as missing CI', async () => { for (const runError of [403, 429, 500]) { - await assert.rejects(execute({ runError }), /API failure/); + 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']) { +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, conclusion === 'startup_failure' ? 'failure' : 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, 'arrow-rs-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 () => { diff --git a/.github/workflows/README.md b/.github/workflows/README.md index f0d2827e2425..9665f150a530 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -17,28 +17,12 @@ under the License. --> -CI runs lightweight checks upstream for fork pull requests to `main`. Fork -branch pushes and manual CI runs execute all regular suites, excluding Miri. -Other pull requests run suites selected by their changed files upstream. -The merge queue runs the full test suite against the proposed merge result. +CI selects checks for pull requests according to their source and changed files. +See the [contributor guide](../../CONTRIBUTING.md#pull-requests-from-forks) for fork CI setup. +The merge queue runs the full test suite, except Miri, against the proposed merge result. Pushes to upstream `main` populate caches and publish documentation without repeating tests. A single required status check combines the results and blocks merging on failure. -`fork_ci_status.yml` mirrors fork results in an informational `CI (fork)` check, -using `workflow_run` and scheduled polling. It reads fork runs and writes upstream -checks through GitHub APIs without checking out PR code or consuming PR artifacts. -See [fork CI setup](../../CONTRIBUTING.md#pull-requests-from-forks). - -Run the routing and bridge tests with Python 3.11+, PyYAML, and Node.js 24: - -```shell -python3 .github/ci/scripts/check-ci-config.py -node --test .github/ci/scripts/test-fork-ci-status.cjs -``` - -Bridge tests mock GitHub APIs. End-to-end testing requires a test repository -with the upstream guards adapted and the bridge installed on its default branch. - Miri runs separately every day at 03:17 UTC and on demand through the `miri` workflow's **Run workflow** button or `gh workflow run miri.yaml --repo apache/arrow-rs --ref `. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 57ed068950b1..38b5f3a52eb3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -155,10 +155,10 @@ jobs: { printf 'Full test results: [CI on %s](https://github.com/%s/actions/workflows/ci.yml).\n\n' "$FORK_REPOSITORY" "$FORK_REPOSITORY" printf 'Look for commit `%s`. Enable Actions in your fork and push your branch if no run exists.\n\n' "$PR_HEAD_SHA" - printf 'This upstream PR run checks formatting and contributor hygiene. The merge queue runs the full suite before merge.\n' + printf 'This upstream PR run checks formatting and contributor hygiene. The merge queue runs the required CI checks before merge.\n' } >> "$GITHUB_STEP_SUMMARY" # Fork PRs to main only run lightweight checks here. The informational - # CI (fork) check is separate; the merge queue runs every suite here. + # CI (fork) check is separate; the merge queue runs the required suites here. # A failed/cancelled selector or suite must still block merging. # Miri runs separately to reduce runner time: # https://github.com/apache/arrow-rs/issues/11548 diff --git a/.github/workflows/fork_ci_status.yml b/.github/workflows/fork_ci_status.yml index 114f8769e411..6e56073f929b 100644 --- a/.github/workflows/fork_ci_status.yml +++ b/.github/workflows/fork_ci_status.yml @@ -15,8 +15,14 @@ # specific language governing permissions and limitations # under the License. -# Mirror fork results using reviewed default-branch code and public API reads. +# Mirror fork results as the informational CI (fork) check after upstream PR CI +# completes and through scheduled polling for late runs and reruns. Read fork +# runs and write upstream checks using reviewed default-branch code. # No checkout, artifacts, or code from the pull request are used here. +# Test with Python 3.11+, PyYAML, and Node.js 24: +# node --test .github/ci/scripts/test-fork-ci-status.cjs +# Tests mock GitHub APIs. End-to-end testing requires a test repository with +# the upstream guards adapted and this workflow installed on its default branch. name: Fork CI status on: @@ -61,11 +67,11 @@ jobs: pr.head.repo.full_name.toLowerCase() !== repository.toLowerCase() && pr.base.ref === 'main'; const externalId = (pr) => 'arrow-rs-fork-ci:' + pr.number; const conclusions = new Set([ - 'action_required', 'cancelled', 'failure', 'neutral', 'skipped', 'stale', 'success', 'timed_out', + '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 every suite, including Miri, on the proposed merge result. ' + + '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.'; @@ -178,6 +184,15 @@ jobs: } 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; @@ -198,13 +213,14 @@ jobs: for (const pr of page.data) { if (new Date(pr.updated_at).getTime() < cutoff) { stale = true; break; } seen.add(pr.number); - await syncCheck(pr, sweepNumbers.has(pr.number)); + await syncPr(pr, sweepNumbers.has(pr.number)); } if (stale) break; } for (const pr of slice) { - if (!seen.has(pr.number)) await syncCheck(pr); + if (!seen.has(pr.number)) await syncPr(pr); } + if (errors.length) core.setFailed(errors.join('\n')); } if (repository === 'apache/arrow-rs') { diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a36f7503d016..b4559f11cc20 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,7 +76,8 @@ quickly and are easier to understand and review. ### Pull Requests from Forks To reduce load on ASF's shared runners, pull requests from forks to `main` -run tests in the fork and lightweight checks upstream. +run CPU-intensive tests on your fork using your fork's GitHub runners, with +lightweight checks upstream. To enable Actions in your fork: 1. Open the **Actions** tab in your fork and enable workflows before pushing. 2. Sync your fork and rebase your branch onto upstream `main` to include the @@ -84,8 +85,11 @@ run tests in the fork and lightweight checks upstream. 3. Push your branch, or manually run `CI` from your fork's **Actions** tab. View results and rerun failed jobs there; the PR's `CI (fork)` check may lag. -Fork results are informational. The merge queue tests the proposed merge with -`main`, including Miri. Same-repository and release-branch PRs retain upstream testing. +Fork branch pushes and manual runs execute all regular suites, excluding Miri. +The `CI (fork)` check links to the fork run and is informational. When your PR is +approved and added to the merge queue, the final CI checks run using ASF's runners +against the proposed merge with `main`. Same-repository and release-branch PRs +retain upstream testing. Miri runs separately on a daily schedule and on demand. ## Developer's guide to Arrow Rust From 9f4972538866756186f6f7dff5c6cac2679b3ce5 Mon Sep 17 00:00:00 2001 From: sdf-jkl Date: Fri, 23 Oct 2026 13:36:38 -0400 Subject: [PATCH 4/4] Fix fork CI rerun cancellation and redundant check updates --- .github/ci/scripts/test-fork-ci-status.cjs | 18 +++++++++++++++++- .github/workflows/ci.yml | 3 ++- .github/workflows/fork_ci_status.yml | 8 ++++---- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.github/ci/scripts/test-fork-ci-status.cjs b/.github/ci/scripts/test-fork-ci-status.cjs index 52637e5b0544..7cd872517ae3 100644 --- a/.github/ci/scripts/test-fork-ci-status.cjs +++ b/.github/ci/scripts/test-fork-ci-status.cjs @@ -229,7 +229,11 @@ test('a deleted run clears its stale success result', async () => { assert.equal(writes[0].conclusion, 'neutral'); }); test('unchanged results do not consume check write requests', async () => { - assert.deepEqual((await execute({ checks: [await existing()] })).writes, []); + 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/arrow-rs/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(); @@ -288,11 +292,23 @@ test('successive fork pushes share a cancellation group, separate branches do no assert.equal(group(forkPush), group({ ...forkPush, sha: 'second' })); assert.notEqual(group(forkPush), group({ ...forkPush, ref: 'refs/heads/other' })); }); +test('fork push and manual runs replace each other on the same branch', () => { + const manual = { ...forkPush, event_name: 'workflow_dispatch' }; + assert.equal(group(forkPush), group(manual)); + assert.equal(group({ ...forkPush, sha: 'second' }), group(manual)); + assert.notEqual(group(forkPush), group({ ...manual, ref: 'refs/heads/other' })); + assert.notEqual(group(forkPush), group({ ...manual, event_name: 'pull_request', + event: { pull_request: { number: 1 } } })); +}); test('upstream main and merge queue runs remain separate per commit', () => { for (const event_name of ['push', 'merge_group']) { const upstream = { ...forkPush, repository: 'apache/arrow-rs', event_name }; assert.notEqual(group(upstream), group({ ...upstream, sha: 'second' })); } + const upstream = { ...forkPush, repository: 'apache/arrow-rs' }; + for (const event_name of ['workflow_dispatch', 'merge_group']) { + assert.notEqual(group(upstream), group({ ...upstream, event_name })); + } }); test('same-named branches from different fork PRs cannot cancel each other', () => { const event = { pull_request: { number: 1 } }; diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38b5f3a52eb3..4d04dd30d153 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,8 @@ on: workflow_dispatch: concurrency: - group: ${{ github.repository }}-${{ github.event_name }}-${{ github.event.pull_request.number || (github.repository != 'apache/arrow-rs' && github.ref) || github.sha }}-${{ github.workflow }} + # Fork pushes and manual runs share a branch group so newer runs replace both. + group: ${{ github.repository }}-${{ github.repository != 'apache/arrow-rs' && github.event_name == 'workflow_dispatch' && 'push' || github.event_name }}-${{ github.event.pull_request.number || (github.repository != 'apache/arrow-rs' && github.ref) || github.sha }}-${{ github.workflow }} cancel-in-progress: true permissions: diff --git a/.github/workflows/fork_ci_status.yml b/.github/workflows/fork_ci_status.yml index 6e56073f929b..b731c329a3ae 100644 --- a/.github/workflows/fork_ci_status.yml +++ b/.github/workflows/fork_ci_status.yml @@ -38,10 +38,11 @@ permissions: checks: write pull-requests: read -# Serialize notifications and sweeps so they cannot create duplicate checks. +# 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 }} - cancel-in-progress: false + 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: @@ -153,7 +154,6 @@ jobs: } if (check && check.status === payload.status && check.conclusion === (payload.conclusion || null) && - check.details_url === payload.details_url && check.output?.text === payload.output.text && check.output?.summary === payload.output.summary) return; if (check) {