Bot running nixpkgs-review on most nixpkgs PRs

Hi All,

I’ve started building a system that runs nixpkgs-review on each PR to nixpkgs on both x86_64-linux and aarch64-linux. It does have some cutoffs that exclude building certain PRs that cause mass rebuilds, but I believe it covers approximately 75% of all PRs currently.

I’m still working on the system, but you can see a live shot of the builds that have completed here: r-rmcgibbo’s gists · GitHub.

It’s currently not activated to actually comment on github, because I wanted to iron out the bugs in the orchestration and ask the community for feedback first.

@Sandro – I know specifically that you review a lot of PRs so I want to confirm that this would be helpful to you before pushing it.

Here’s for example the comment it would have posted on github for one of the 300 PRs that it has reviewed in the past 3 days:

Result of `nixpkgs-review pr 110223` at 7ab66f91 run on aarch64-linux [1](https://github.com/Mic92/nixpkgs-review)
<details>
  <summary>1 package built:</summary>
  <ul>
    <li><a href="https://gist.github.com/83cc5ffd8d63c75638539d33d06be302">xfig</a></li></li>
  </ul>
</details>
12 Likes

Aha! I’ve just found your extra checks, Sandro. I’ll add these into the environment for the bot: GitHub - SuperSandro2000/nixpkgs-review-checks: This project does additional checks and adds more information about the build logs and outputs to the reports generated by nixpkgs-review.

3 Likes

Please check them carefully before you add them. So far I have no feedback if they worked for anyone else. I try to keep them stable and not break them with every other commit but so far they have zero tests and every change could break something.

1 Like

You might also have a look at SIG Workflow automation - NixOS Discourse — currently unfortunately a little dormant, though.

1 Like

Another association to the word “orchestration” you used and that I’d like to share: once in some shape, maybe (out of curiosity) you might want to have a look at zeebe.io, specifically how job client could execute jobs with independent scaling after an initial helper kicks off the workflow from a github event

FYI: I’ve “turned on” this bot,and it’s now commenting on github. I’ve tried to put in place a few different checks so as to not upload comments to github if they’re completely duplicative (i.e. if someone else has posted a nixpkgs-review bot for x86_64-linux my bot will skip posting for that arch), but these checks are unlikely to be perfect.

Please reach out to me on over email, IRC, or any other mechanism if it’s bothering you or if there are any changes you’d like me to make to it.

3 Likes

I currently have it configured to attempt to build PRs that it believes it can finish in less than 2 hours of build time, and I’m working on adding a feature such that it can attempt to partially build PRs that are are larger, if they’re composed of many derivations.

I think it probably does not make sense to show warnings about expressions in files that were not touched by the PR.

Example of where this is happening: nx-libs: 3.5.99.25 -> 3.5.99.26 by r-ryantm · Pull Request #112348 · NixOS/nixpkgs · GitHub

1 Like

Yeah, that’s a good point. I will try to filter those.

2 Likes

Update:

The bot has been running stably for the last few days and I haven’t heard any major complaints.

  • @ryantm’s suggestion above was incorporated.
  • Numerous improvements to @jtojnar’s nixpkgs-hammering have been landing, so the bot will be getting smarter with suggestions (although there’s lots of room for improvement of course).

Some bugs, areas for improvement and future plans:

  • I do receive some HTTP 422 errors from the github API when posting comments that I don’t understand.
  • There are some cases of truncated logs (cc @danieldk who pointed one out this morning to me and @Sandro who pointed out a few as well). I believe, but have not confirmed, that this is relates to timeouts (I have a 1 hour limit per drv in place). The uploaded logs and the “failure message” in the github comment really ought to be more helpful here.
  • When a build fails because one of its dependencies failed to build (e.g. because of a hash mismatch in the src), the bot often doesn’t upload useful logs showing the failure of the dependency. This needs to be fixed.
  • For PRs that trigger a massive number of rebuilds, I’d like to add a feature to the bot such that it can try to build a subset of the changed attrs that it believes it can finish in time. I plan to do this by (approximately) solving a precedence-constrained knapsack problem (GitHub - rmcgibbo/precedenceConstrainedKnapsack: Precedence constrained knapsack problem solver).

Let me know if there’s any other features or bugs I should be aware of!

6 Likes

some features I would like to have which are most likely for nixpkgs-hammering:

  • typo detection for some very common errors (eg pythonImportsCheck)
  • log grepping for:
    • stale substituteInPlace
    • maybe 0 tests run by pytest
  • variable conversions which could be avoided
  • separate new and already failing builds which probably should go into nixpkgs-review

That are most features from nixpkgs-review-checks/nixpkgs-review-checks at 266b98215fb85a74590108a011c35dc913861a3e · SuperSandro2000/nixpkgs-review-checks · GitHub.
I am not sure if the ldd and bloatfly analysis is something worth to port.

2 Likes

In cudatoolkit_11, cudnn_cudatoolkit_11: 11.1 -> 11.2 by danieldk · Pull Request #112648 · NixOS/nixpkgs · GitHub the builder ran out of disk space.

1 Like

I can add your builder to my prometheus monitoring. Alerts would go to #nix-community on freenode. This would log out-of-disk cases.

1 Like

Thanks @danieldk and @mic92. Most of the builds are run in fresh spot VMs, so I don’t know how much monitoring would help (it’s not very actionable). I think the best bet is either to skip those builds or provision machines with larger disks.

How about making substituteInPlace fail in that case, so maintainers notice that themselves?

Note: If anyone – especially core maintainers who file lots of PRs – would prefer to not be notified with a github comment when the build succeeds, just let me know. You can find me on IRC, twitter, or look up my email address.

It’s a 1-line change for me to make the bot refrain from commenting on PRs that you’ve filed unless the build actually fails.

(Other changes to reduce the amount of comment spam like integration with the Github Checks API are possible but more delicate.)

1 Like

I would be up for that. Maybe we should try to push that.