Skip to content

workflows/{labels,reviewers}: fix concurrency groups for nested workflows#416448

Merged
philiptaron merged 2 commits intoNixOS:masterfrom
wolfgangwalther:ci-label-concurrency
Jun 13, 2025
Merged

workflows/{labels,reviewers}: fix concurrency groups for nested workflows#416448
philiptaron merged 2 commits intoNixOS:masterfrom
wolfgangwalther:ci-label-concurrency

Conversation

@wolfgangwalther
Copy link
Contributor

This didn't work as intended. When a workflow is run with workflow_call, it will have github.workflow set to the parent workflow. So the caller input that we passed, resulted in this concurrency key:

Eval-Eval-...

But that's bad, because the labels and reviewers workflows will cancel each other!

What we actually want is this:

  • Label and Reviewers workflow should have different groups.
  • Reviewers called via Eval and called directly via undraft should have different groups.

We can't use the default condition we use everywhere else, because github.workflow is the same for Label and Reviewers. Thus, we hardcode the workflow's name as well. This essentially means we have this as a key:

<name-of-running-workflow>-<name-of-triggering-workflow>-<name-of-event>-<name-of-head-branch>

This should do what we want.

Things done


Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs backport release-24.11 labels Jun 13, 2025
This can happen when two PRs run at the same time, which come from
different forks, but have the same head branch name.

github.head_ref is suggested by GitHub's docs, but.. that's not really
useful for cases with forks.
@github-actions github-actions bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 13, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

Different users could have the same head branch name in their forks and run CI for their PRs at the same time.

I assumed that head_ref would be refs/pull/1234/head. Is it actually the branch name? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, apparently. See for example the summary page here, section annotations: https://github.com/NixOS/nixpkgs/actions/runs/15634880254?pr=413785

There you can see the actual concurrency key used.

…lows

This didn't work as intended. When a workflow is run with
`workflow_call`, it will have `github.workflow` set to the *parent*
workflow. So the `caller` input that we passed, resulted in this
concurrency key:

```
Eval-Eval-...
```

But that's bad, because the labels and reviewers workflows will cancel
each other!

What we actually want is this:
- Label and Reviewers workflow should have different groups.
- Reviewers called via Eval and called directly via undraft should have
*different* groups.

We can't use the default condition we use everywhere else, because
`github.workflow` is the same for Label and Reviewers. Thus, we hardcode
the workflow's name as well. This essentially means we have this as a
key:

```
<name-of-running-workflow>-<name-of-triggering-workflow>-<name-of-event>-<name-of-head-branch>
```

This should do what we want.

Since workflows can be made reusable workflows later on, we add those
hardcoded names to *all* concurrency groups. This avoids copy&paste
errors later on.
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Thank you for becoming an expert at this, @wolfgangwalther.

@philiptaron philiptaron merged commit a921965 into NixOS:master Jun 13, 2025
23 of 27 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jun 13, 2025

Backport failed for release-24.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.11
git worktree add -d .worktree/backport-416448-to-release-24.11 origin/release-24.11
cd .worktree/backport-416448-to-release-24.11
git switch --create backport-416448-to-release-24.11
git cherry-pick -x 7ba7720b28cc03d44d5ad1f5931ad4a88e068470 6793e238faeffcb58f1f6e1241c7390649fb8197

@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jun 13, 2025

Backport failed for release-25.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.05
git worktree add -d .worktree/backport-416448-to-release-25.05 origin/release-25.05
cd .worktree/backport-416448-to-release-25.05
git switch --create backport-416448-to-release-25.05
git cherry-pick -x 7ba7720b28cc03d44d5ad1f5931ad4a88e068470 6793e238faeffcb58f1f6e1241c7390649fb8197

@github-actions github-actions bot added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 13, 2025
@wolfgangwalther wolfgangwalther deleted the ci-label-concurrency branch June 13, 2025 16:32
This was referenced Jun 13, 2025
@wolfgangwalther wolfgangwalther added the 8.has: port to stable This PR already has a backport to the stable release. label Jun 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants