Nixpkgs merge bot testing and plan

Announcements
1

After receiving a lot of concerned feedback in a previous attempt, it’s time to clear things up and give it another chance.

Starting today, the Nixpkgs merge bot will be enabled in dry-run mode.
This means that it is not privileged to actually merge anything at all just yet, and instead only prints a comment explaining what it would’ve done if merges were enabled.

The people behind this effort are @Lassulus and @Mic92.
This announcement was written with @infinisil’s help, who’s also in favor of going ahead with this plan.

It got new wind during Thaigersprint with the help from @Scriptkiddi and @Luis-Hebendanz.

How does it work?

This bot will allow Nixpkgs PRs to be merged automatically if they fulfill these main requirements:

  • The PR is created by @r-ryantm, which is @ryantm’s bot that automatically creates PRs using nixpkgs-update.
    This relies on various heuristics to update packages, the most notable of which being updateScript.
    Check out the manual how to define it for your package.
    Currently this bot doesn’t run on official infrastructure, but there are plans to change that.

  • The PR only changes files in pkgs/by-name, the new way to declare top-level packages.
    While most packages aren’t defined there yet, @infinisil is working on an automated migration that is expected to happen soon and will avoid a lot of manual labor.
    Restricting to pkgs/by-name is necessary because it’s the only way to reliably determine the updated package from the changed files.

  • A maintainer of the updated package posts a comment containing:

    @NixOS/nixpkgs-merge-bot merge

    This means that if you want to be able to merge automatic updates to the packages you maintain, you need to add yourself to the lib.maintainers field of the package.
    This also has the effect that you’ll automatically get a review request for new PRs.

The bot will then either merge the PR, or reply with a reason why it can’t be merged.
Since it’s only running in dry-run mode right now, instead of merging, it will reply saying that it can’t be merged because it’s running in dry-run mode.

The reason behind these strict requirements is to both keep it reasonably secure, but also to encourage improving package update automation, which is a key part in making Nixpkgs sustainable in the long-term.

Enabling automated merges

Unless any major problems with this approach are brought up, we plan to start allowing automated PR merging starting 2024-02-23T00:00:00Z.

If you have any feedback, please either reply in this thread, come chat on Matrix, or directly open issues or pull requests in the repository.

Future

The previous announcement has shown that it’s not easy to design such a bot in a secure way.
Because of this, we will not make substantial changes to the merge requirements without getting broader community consensus, such as via an RFC.

70 Likes
2

Thank you for your work on this! I’m relieved to hear that the… less than warm reception to the first attempt didn’t stop this second iteration from happening. And it sounds like most of the feedback was clearly taken into account.

One quick question:

Any reason to not just use the GH review/approval feature? I’m sure there is since it’s quite an obvious option, but I couldn’t find it documented anywhere. IMO it would be preferable to not have two different ways to mark approval on PRs.

7 Likes
3

@delroth I think it’s good as is, because:

  • There may be multiple maintainers for a single package, and they might want to wait with merging until each maintainer approved it. This wouldn’t be possible if the first approval merges it.
  • Not everybody will know about this merge bot, we don’t want maintainers to accidentally trigger a merge when they didn’t intend to. A merge is not just an approval of the PR, it’s also making you responsible for having merged it.
8 Likes
4

I’m not sure I buy the second argument (I view reviews also as something important that I should be able to trust), but the first one makes a lot of sense. Thanks!

1 Like
5

Thanks for moving this forward.

6

I’m happy we found a path forward. Automation such as this is critical for nixpkgs given its size and rate of growth. I agree with many of the security concerns in the original thread but feel like they were also taken into account with the current state. Thanks! :slightly_smiling_face:

Was there any discussion on providing package maintainers to customize the behavior of some of this bot for their packages? After rereading the previous thread again now, I feel like having customization could go a long way towards meeting more people’s preferences on how the bot decides what to do. One team may want two approvals for auto merge, as an example. Another package may consider Darwin support optional, etc

1 Like
7

my plan is to start writing an RFC for more mergebot features. IMHO a start could be the introduction of more meta fields in the packages to enable certain behavior. For example:

  • a list of people allowed to use the bot (for this package), in my draft I called them trusted-maintainers for now
  • automerge strategies: Maybe it’s enough for some packages to just merge it if ofborg succeeds and the PR was created by r-ryantm. Maybe a package wants all maintainers to approve first. Different strategies could be codified here

These are just 2 ideas, more ideas are welcomed although I like to keep the scope of the first RFC small so we can iterate.

6 Likes
8

Can’t wait for this!

1 Like
9

Very nice. I’m glad that this approach is carefully driven forward.

10

Would it be possible to make the bot announce itself on such PRs after the first approval, so that maintainers learn about it within their workflow?

4 Likes
11

Yes that’s possible. If we would go forward with the different merge strategies idea I had above, I would only announce it in the circumstances where it would make sense?
Maybe the bot automatically introduces itself in a PR if it would be helpful for the defined ruleset? like the automerge if PR from r-ryantm and ofborg passes, then it would maybe write a message after opening the PR and before actually merging. Or it would keep track how many persons are needed until it automerges if we have the full consensus rule active? but these are just rough ideas in my head. We can discuss this further when I have the draft RFC ready :slight_smile:

1 Like
12

Alright it is 2024-02-23T00:00:00Z. So I have enabled automatic merging inside the mergebot:

With these changes deployed, maintainers are now able to self merge PRs created by r-ryantm.
They can trigger this automatic merging by writing a comment with the content:
@NixOS/nixpkgs-merge-bot merge

We will continue to monitor all PRs where the bot is mentioned if any unexpected behavior occurs and will stop and fix the bot in that case. I will also create an RFC shortly for the next steps in extending the mergebot.

19 Likes
13
3 Likes