This migrates the bash code to request reviewers to github-script. This
will allow multiple nice improvements later on, but at this stage it's
mostly a reduction in code and complexity.
This makes reactions to merge comments and all the labeling a bit
quicker. Lower the number of backlog items to process per run
accordingly, so that we don't really need more API requests for it.
The last user to touch the schedule will be notified about errors in the
scheduled run. I used to be that, but somehow when renaming the workflow
this didn't happen anymore, so I didn't noticed the workflow failing for
quite some time.
All other conditions equal, there is no reason to prevent maintainers
from backporting changes to their packages. Maintainers are probably in
the *best* position to tell whether a certain change is backportable or
not - because they know the package well.
We still get plenty of errors in the prepare step for the diff requests
to take way too long - let's increase the number of retries massively
now. These are repeated with exponential backoff, so hopefully they will
succeed with some cooldown, when GitHub had time to compute the right
diff in the background.
The nixpkgs-ci token used in CI doesn't have permissions to read the
member list of the nixpkgs-committers team, yet. Let's request this
permission and see whether that fixes the errors we get so far.
Running the nixpkgs-merge-bot in GitHub Actions instead of a separate
workflow has multiple advantages:
- A much better development workflow, with improved testability.
- The ability to label PRs with a "merge-bot eligible" label from the
same codebase.
- Using more data for merge strategy decisions, for example the number
of rebuilds.
This commits re-implements most of the features from the current
nxipkgs-merge-bot directly in the bot workflow. Instead of reacting to
webhook events, this now runs on the regular 10 minute schedule. Some
merges might be delayed a few minutes, but that should not be a problem
in practice.
To give the user early feedback, there are additional workflows running
when a comment or review is posted. These react with "eyes" to make the
user aware that the comment has been recognized.
The only feature not taken over was the size check for files in the PR.
This kind of check is not really relevant for maintainer merges only -
if we want to prevent bigger files from making it into the tree, then we
need a generic CI check, which is out of scope for the merge-bot.
Other than that, everything should be implemented - any omissions are by
accident.
This is just a separate commit for a better diff. This workflow will be
extended with the `submitted` type in the next commit, so needs a
different naming.
While at it, also remove the useless concurrency group: This workflow is
never run in a PR context, so will never be cancelled.
This workflow / script is already doing more than must labeling: it's
already auto-closing package request issues.
Since we're going to migrate the nixpkgs-merge-bot into this workflow,
we'll rename things to a more generic name.
This moves the parsing of ci/OWNERS into the Nix sandbox. We also get
rid of checking out the nixpkgs repo another time in the reviewers
workflow - we already have everything we need in the eval/compare job.
The creation of owners.txt in this way is only temporary, it should
eventually be moved further, similar to how maintainers.json is
currently migrating to a maintainer map for the whole repo stored on the
target branch as artifact.
All the github related logic is now bundled in `request-reviewers.sh`.
This allows moving the `get-code-owners.sh` file into the eval/compare
step in the next commit.
Instead of requesting owners and maintainer separately, each with their
own limit of 10 review requests, we now run this together. This unties
the logic and allows easier refactoring. Also, it gives us a consistent
threshold of when not to request reviews anymore, which I set to 15.
Before, this could have been anything between 10 and 20, depending on
how the reviewers distributed over owners and maintainers.
To be able to disable the pr.yml workflow on GitHub, we need to rename
it to a different name. Let's use the long name for consistency with
merge-group.yml. This only affects the GitHub-internal name, not the
visible name in the PR checklist, which is still "PR". This visible name
is also used by nixpkgs-review, so that won't break.
In f7d6d11e8e I wrongly assumed that
running from the untrusted checkout should be fine for the codeowners
validator, because we removed all the logic for privileged tokens.
However, this job also contains access to the cachix secret, which could
be used to push malicious code to cachix, which would then be pulled by
a more privileged workflow like reviewers.yml later.
Instead of setting the maintainer-related labels based entirely on Eval
results, this uses the new maintainer map from the target branch. This
allows labeling PRs correctly, that had been created *before* a
contributor became a maintainer of the respective package.
This creates a simple mapping of all packages to github ids of all their
maintainers. This is uploaded as an artifact and is then available for
download on the latest commit of each branch with a merge queue. This
will allow scheduled jobs to use this information for setting
maintainer-related labels, to request reviewers and to implement the
merge-bot.
The advantage over querying this information directly via Nix in each
case: The scheduled job does not need to install Nix and does not need
to checkout the target branch.
Compared to obtaining the maintainer information just for a single PR
during Eval, this will allow retroactively changing maintainers for a
package: For example, it allows to request a new maintainer as reviewer
for a PR that was created before they became maintainer, but is still
open - and similarly for maintainer labels and merge-bot rights.
None of these extensions are implemented by this PR, yet.
This runs the compare step in the merge queue, to get a combined diff
against the previous queue item. This can be used as a base for multiple
different features in the future, for example preventing accidental mass
rebuilds in the merge queue, or diffing of packages' meta.
I recently tried to increase retries to 3 to fix the 422 "Sorry, this
diff is taking too long to generate" errors that appear frequently as of
late. Little did I know that, this specific status code would not be
retried by default. Thus, try again.
Once we start running similar jobs in the PR and Merge Queue workflows,
the Test workflow will run into a problem: Both nested workflows will
try to upload the same artifacts, which errors out.
By providing a prefix to use for artifact names, this will be a
non-issue when we later run more of Eval in the merge queue or when we
add the build jobs to the queue as well.
Creates a team sync workflow that pushes the current state of teams to a
JSON file, which can then be ingested by `lib.teams` to expose member
lists.
Co-Authored-By: Alexander Bantyev <alexander.bantyev@tweag.io>
This was added when introducing zizmor. It appears that the backport
action actually needs these credentials to persist, to be able to push
the branch.
`zizmor` is a tool that uses static analysis to find potential security
issues in GitHub Actions [0]. (Yes, it's a bit absurd that GitHub
made a CI system so complicated that tools like this were created, but
I digress.)
Given our increase in GHA usage recently, I think this is a good step
towards keeping our security posture in tip-top shape. (It also keeps
with the theme of automating as many things as possible!)
The rule related to the usages of dangerous-triggers have been disabled
to avoid false-positives. Explanations about the usage of
`pull_request_target` and expectations around its usage can be found in
`.github/workflows/README.md`.
[0]: https://woodruffw.github.io/zizmor/
Co-authored-by: Thomas Gerbet <thomas@gerbet.me>
The Eval report which tests performance between Nix/Lix versions on
update of `ci/pinned` wrongly returned errors, when only the special
attribute `release-checks` changed. Since this reads in all of Nixpkgs,
it will change with any formatting change that is introduced at the same
time via update of any of `treefmt`'s formatters.
This change runs Lix on the target commit and Nix on the merged commit.
This does two things for us at once:
- We test both Lix and Nix.
- We ensure that both Lix and Nix produce the same output hashes.
If Lix and Nix were to produce different output hashes at some point,
this would show up as rebuilds in every PR.
This was only separate to work around possible delays from the target
branch's eval workflow. With the switch to the merge queue, this delay
is impossible - the relevant target commit will only appear once Eval
has completed in the merge queue, so Eval will be guaranteed to have
finished.
By running attrpaths and outpaths in the same step, we share ~10 seconds
of eval time, traversing through all of Nixpkgs.
Running eval in the merge queue prevents eval failures after merging
multiple PRs with semantic merge conflicts.
It's also the base for allowing more sophisticated checks about rebuild
counts in the merge queue later.
When branches are directly pushed to, such as for periodic merges, this
will not run Eval immediately. However, the next PR will run it as part
of its Eval step and will cache the results. Any further PR will then
benefit from the same caching again.
This also avoids running Eval twice when pushing to staging-next or
haskell-updates, where a PR is open at the same time. Here, the PR's
Eval still runs on the push, of course.
