Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions .github/workflows/fork_ci_status.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,20 @@
# A pull request from a fork to `main` runs `rust.yml` on the fork. This
# workflow mirrors that run as one `Rust (fork)` check on the pull request, so
# reviewers see the result without opening the fork. It never checks out or
# runs code from the pull request: on `pull_request_target` it reads the fork's
# public run list and creates the check, and on a schedule it updates the
# active or missing runs of recently updated pull requests plus a rotating
# slice of all open pull requests. The check is informational. The merge
# runs code from the pull request: on `workflow_run` of
# `pull_request_signal.yml` it reads the fork's public run list and creates the
# check (`pull_request_target` is disabled by ASF infra policy), and on a
# schedule it updates the active or missing runs of recently updated pull
# requests plus a rotating slice of all open pull requests. The check is informational. The merge
# queue runs the full workflow on the merged result and stays the gate.
#
# See docs/source/contributor-guide/index.md, "Pull Requests from Forks".
name: Fork CI status

on:
pull_request_target:
types: [opened, reopened, synchronize]
workflow_run:
workflows: ["Pull request signal"]
types: [requested]
schedule:
- cron: "*/15 * * * *"
workflow_dispatch:
Expand All @@ -47,12 +49,13 @@ permissions:
pull-requests: read

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || 'update' }}
cancel-in-progress: ${{ github.event_name == 'pull_request_target' }}
group: ${{ github.workflow }}-${{ github.event.workflow_run.head_repository.full_name && format('{0}-{1}', github.event.workflow_run.head_repository.full_name, github.event.workflow_run.head_branch) || 'update' }}
cancel-in-progress: ${{ github.event_name == 'workflow_run' }}

jobs:
bridge:
name: Mirror the fork run
if: github.event_name != 'workflow_run' || github.event.workflow_run.event == 'pull_request'
runs-on: ubuntu-slim
timeout-minutes: 10
steps:
Expand Down Expand Up @@ -211,8 +214,24 @@ jobs:
core.info(`Created ${CHECK} for ${sha}: ${payload.conclusion || payload.status}`);
}

// `workflow_run.pull_requests` is empty for pull requests from
// forks, so find the pull request by its head branch and commit.
async function pullRequestOf(run) {
const { data } = await github.rest.pulls.list({
...upstream,
state: 'open',
head: `${run.head_repository.owner.login}:${run.head_branch}`,
});
return data.find((pr) => pr.head.sha === run.head_sha);
}

async function notify() {
const pr = context.payload.pull_request;
// An outdated commit matches nothing, since a newer run mirrors the new head.
const pr = await pullRequestOf(context.payload.workflow_run);
if (!pr) {
core.info('No open pull request for this commit; nothing to mirror.');
return;
}
// Only fork PRs to main skip jobs upstream; release branches have
// no merge queue and keep the full run.
if (!isForkPrToMain(pr)) {
Expand Down Expand Up @@ -312,7 +331,7 @@ jobs:
}
}

if (context.eventName === 'pull_request_target') {
if (context.eventName === 'workflow_run') {
await notify();
} else {
await update();
Expand Down
45 changes: 33 additions & 12 deletions .github/workflows/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,57 @@

name: Labeler

# `pull_request_target` is disabled by ASF infra policy. This workflow runs from
# the default branch when `pull_request_signal.yml` starts, so it never runs the
# code of a pull request. See that file for details.
concurrency:
group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}
group: ${{ github.repository }}-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }}-${{ github.workflow }}
cancel-in-progress: true

on:
pull_request_target:
types:
- opened
- edited
- synchronize
workflow_run:
workflows: ["Pull request signal"]
types: [requested]

permissions: {}

jobs:
process:
name: Process
runs-on: ubuntu-slim
# only run for users whose permissions allow them to update PRs
# otherwise labeler is failing:
if: github.event.workflow_run.event == 'pull_request'
# The token of a `workflow_run` workflow can update PRs from forks.
# Previously `pull_request_target` was used for the same reason, see
# https://github.com/apache/datafusion/issues/3743
permissions:
contents: read
pull-requests: write
steps:
# `workflow_run.pull_requests` is empty for pull requests from forks, so
# find the pull request by its head branch and commit. An outdated commit
# matches nothing, since a newer run labels the new head.
- name: Find the pull request
id: pr
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
const run = context.payload.workflow_run;
const { data } = await github.rest.pulls.list({
owner: context.repo.owner,
repo: context.repo.repo,
state: 'open',
head: `${run.head_repository.owner.login}:${run.head_branch}`,
});
const pr = data.find((pr) => pr.head.sha === run.head_sha);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A head branch and SHA do not uniquely identify a PR: the same branch can target main and a release branch. pulls.list defaults to newest first, so find() selects the newer PR for every signal and the older PR never receives labels.
The equivalent lookup in fork_ci_status.yml should filter to main before selecting, otherwise it can select the release PR and skip the main notification.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, confirmed. A fork can have open PRs to main and to a release branch from the same branch, and both match head + head_sha. pulls.list returns newest first, so find() always picks the newer PR.

There is one more piece: both PRs share the new concurrency group (head repo + branch) with cancel-in-progress: true, so the two signal runs from one push cancel each other. Fixing find() alone is not enough. The run that survives has to handle every matching PR.

Suggested changes:

labeler.yml: label every matching PR. actions/labeler v7 accepts several numbers in pr-number, one per line:

-            const pr = data.find((pr) => pr.head.sha === run.head_sha);
-            if (pr) core.setOutput('number', String(pr.number));
-            else core.info(`No open pull request for ${run.head_sha}`);
+            const numbers = data
+              .filter((pr) => pr.head.sha === run.head_sha)
+              .map((pr) => String(pr.number));
+            if (numbers.length) core.setOutput('numbers', numbers.join('\n'));
+            else core.info(`No open pull request for ${run.head_sha}`);
       - name: Assign GitHub labels
-        if: steps.pr.outputs.number != ''
+        if: steps.pr.outputs.numbers != ''
         uses: actions/labeler@bf12e9b00b37c5c0ca2b87b79b2daf7891dbda13  # v7.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
-          pr-number: ${{ steps.pr.outputs.number }}
+          pr-number: ${{ steps.pr.outputs.numbers }}

fork_ci_status.yml: only PRs to main get the mirrored check, so filter on the base in the lookup:

               const { data } = await github.rest.pulls.list({
                 ...upstream,
                 state: 'open',
                 head: `${run.head_repository.owner.login}:${run.head_branch}`,
+                base: 'main',
               });

With that, whichever signal run survives the cancellation finds the main PR.

test_fork_ci_status.cjs: add a case with two open PRs from the same head branch, one to main and one to a release branch, where the release PR is newer. The Rust (fork) check should still be created for the main PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to draft PR to your PR if you prefer.

if (pr) core.setOutput('number', String(pr.number));
else core.info(`No open pull request for ${run.head_sha}`);

- name: Assign GitHub labels
if: |
github.event_name == 'pull_request_target' &&
(github.event.action == 'opened' ||
github.event.action == 'synchronize')
if: steps.pr.outputs.number != ''
uses: actions/labeler@bf12e9b00b37c5c0ca2b87b79b2daf7891dbda13 # v7.0.0
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
pr-number: ${{ steps.pr.outputs.number }}
configuration-path: .github/workflows/labeler/labeler-config.yml
sync-labels: true

Expand Down
42 changes: 42 additions & 0 deletions .github/workflows/pull_request_signal.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# 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.

# Signal for the workflows that need a write token on a pull request.
#
# ASF infra policy disables `pull_request_target`
# (https://infra.apache.org/github-actions-policy.html), and GitHub is adding a
# default policy that blocks it. The `workflow_run` workflows below run from the
# default branch with a write token and never check out or run pull request
# code. They trigger on the `requested` event of this workflow, which GitHub
# sends at once for every run, even for a first-time contributor whose run waits
# for approval:
# - labeler.yml
# - fork_ci_status.yml
name: Pull request signal

on:
pull_request:
types: [opened, reopened, synchronize]

permissions: {}

jobs:
signal:
name: Signal pull request update
runs-on: ubuntu-slim
steps:
- run: echo "This run triggers the workflow_run workflows that handle the pull request."
7 changes: 3 additions & 4 deletions .github/workflows/release_version_labeler.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,15 @@ concurrency:
group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}
cancel-in-progress: true

# `pull_request_target` is disabled by ASF infra policy, so label on the push of
# the merge commit to `main` and look up the merged pull request from it.
on:
pull_request_target:
types:
- closed
push:
branches:
- main

jobs:
label:
if: github.event.pull_request.merged == true
runs-on: ubuntu-slim
permissions:
pull-requests: write
Expand Down
14 changes: 11 additions & 3 deletions ci/scripts/release_version_labeler.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,18 @@ module.exports = async ({ github, context }) => {
}
}

await github.rest.issues.addLabels({
// A squash merge or a merge queue commit maps to the merged pull request.
const { data: pulls } = await github.rest.repos.listPullRequestsAssociatedWithCommit({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.pull_request.number,
labels: [target_version]
commit_sha: context.sha
});
for (const pull of pulls.filter((pull) => pull.merged_at && pull.base.ref === 'main')) {
await github.rest.issues.addLabels({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pull.number,
labels: [target_version]
});
}
};
25 changes: 22 additions & 3 deletions ci/scripts/test_fork_ci_status.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ const pr = {
},
base: { ref: "main" },
};
const workflowRun = {
head_sha: sha,
head_branch: "feature",
head_repository: { owner: { login: "contributor" } },
};
const run = (id, event = "push", conclusion = "success", run_attempt = 1) => ({
id,
event,
Expand Down Expand Up @@ -92,6 +97,7 @@ async function execute({
workflowState = "active",
recentPrs = [pr],
slice = [pr],
openPrs = [pr],
calls = [],
}) {
const writes = [];
Expand Down Expand Up @@ -123,7 +129,11 @@ async function execute({
getWorkflow: async () => ({ data: { state: workflowState } }),
},
pulls: {
list: async ({ per_page }) => {
list: async ({ per_page, head }) => {
if (head) {
assert.equal(head, "contributor:feature");
return { data: openPrs };
}
if (per_page === 1) return { data: [pr], headers: {} };
assert.equal(per_page, 25);
return { data: slice };
Expand All @@ -141,7 +151,7 @@ async function execute({
{
repo: { owner: "apache", repo: "datafusion" },
eventName: event,
payload: { pull_request: pr },
payload: { workflow_run: workflowRun },
},
{ info: () => {} },
Clock
Expand All @@ -161,13 +171,22 @@ function assertRun(write, method, expected) {
test("notification selects a newer manual run instead of a push run", async () => {
const latest = run(2, "workflow_dispatch");
const writes = await execute({
event: "pull_request_target",
event: "workflow_run",
runs: [latest, run(1)],
});
assert.equal(writes.length, 1);
assertRun(writes[0], "create", latest);
});

test("notification skips a commit that is no longer a pull request head", async () => {
const writes = await execute({
event: "workflow_run",
openPrs: [{ ...pr, head: { ...pr.head, sha: "b".repeat(40) } }],
runs: [run(1)],
});
assert.deepEqual(writes, []);
});

for (const conclusion of ["cancelled", "failure"]) {
test(`sweep replaces a ${conclusion} push run with a newer manual run`, async () => {
const previous = run(1, "push", conclusion);
Expand Down
Loading