potiuk commented on code in PR #26683:
URL: https://github.com/apache/datafusion/pull/26683#discussion_r4366907403
##########
.github/workflows/labeler.yml:
##########
@@ -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);
Review Comment:
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:
```diff
- 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}`);
```
```diff
- 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:
```diff
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
