Call for nodePackages maintainers

Hello.

The situation with nodePackages PRs is outrageous! They always have merge conflicts because they always have to edit the same node-packages.nix file.

Until some better technical solution is conceived of and implemented, here is my plan for improving this situation:

  1. Make node maintainer team, add code owners for node-packages.nix
  2. Make script that makes it easy for node maintainers to list a bunch of PRs, drop all their node-packages.nix changes, and then regenerate node-packages.nix on top of that, and make a PR.

I am willing to make the script and merge PRs it generates, but I need your help with these things:

  1. monitoring new node PRs
  2. generating the combined PRs
  3. following up on potential nodePackages regressions

Who is willing to step up and be a node maintainer?

Sincerely,
Ryan Mulligan

6 Likes

one possible solution is just have “project-local” deps.nix. However, this will mean that each package will have a 2-13k line file dedicated to them.

If there were a lot of them, I guess that would make ofborg/nixpkgs-review evaluations much more expensive?

There’s two aspects of it:

  • Sharing of common node packages
    • This might not actually be much of an issue, if the individual node packages are just fixed derivation outputs (which I think they are)
  • Reduce the checkout size of nixpkgs
    • nixpkgs is currently 170MB unpacked, which is undesirable for CI use cases, where minimal size is desirable.

The global node-packages file is already 3.3MB

$ du -hd0 pkgs/development/node-packages/node-packages.nix
3.3M	pkgs/development/node-packages/node-packages.nix

Since the generated node-packages file could be named anything, I’m not sure of a good way to collecting all node files and their share of nixpkgs’ size. But I’m assuming it’s non-trivial.

Also why I’m not a big fan of using poetry2nix within nixpkgs. It creates similarly sized generated files, and it’s unable to re-use any of the curated python nix packages.

EDIT:
I guess it’s 5.9MB:

$ rg -l -F "{nodeEnv," | xargs du -d0 | awk '{ sum += $1; } END { print sum; }'
5904
2 Likes

Another issue is that it is currently impossible to find somebody to review node-specific things. I never feel comfortable reviewing those PRs, and whoever I try to ping (based on prior contributions to node packages) tends to be unresponsive. We could really use some maintainers.

4 Likes

For what it’s worth, we rely quite heavily on JS at work and that would be a great opportunity to introduce people to Nix, if I could show them that Nix works well with JS. What I’m trying to say is, I’d love to be one of those maintainers but I’m currently still a Nix beginner.

Is there any way I can help? I can at least offer some time and energy.

:wave: Hi all. Maintainer of https://github.com/cprussin/nixjs here. Obviously I have a particular interest in node / nix – I work on node (and JS in the browser) professionally at work, and while my company doesn’t use nix, I use nix avidly for anything I can (including nixos pretty much everywhere, all side projects I own, & I check shell.nix files into my repos at work for myself and in case anyone else comes along and wants to use nix).

The point being, in terms of interest and expertise, I’d be pretty well suited to lend a hand here. And I’d love to do so, but since my company doesn’t use nix it would be entirely extracurricular. So I can’t guarantee my time is sufficient, but I can certainly take a stab.

I’m reasonably familiar with nixpkgs from reading code, but my contributions thus far have been limited to a single backport of a PR someone else put in. I don’t know if that bothers the “powers that be” but it doesn’t bother me to dig in.

tl;dr I’d be happy to pitch in, but I can’t guarantee my time will be sufficient.

EDIT: someone tell me what to do next :slight_smile:

2 Likes

@cideM @cprussin thank for volunteering :slight_smile:

You could add yourself as a CODEOWNER of the node-packages directory, similar to this commit here: https://github.com/NixOS/nixpkgs/commit/4c810e70efa185b814b843119a9b332ce3bb55d4#diff-5cfb934ca0ec4c1743905a51dc428b84

This will mean that github will add you as a reviewer whenever someone creates a PR against those files or directories.

After you tested the changes, you can cc myself or another active committer to get the PR merged

If you’re not sure about nixpkgs PR process, can review manual section: https://nixos.org/manual/nixpkgs/stable/#chap-submitting-changes

I also have a video where I create a PR for a new application, but my suggestions on how to test and create the PR still apply to all PRs:

EDIT:
For most PRs, I look at the diff, make sure that it’s following established or best standards, and then also do a nixpkgs-review pr <pr #> to see what the changes affect. If there’s no regressions and the changes look good then it’s probably good to merge.

What is considered best practices? depends on the maintainer, language framework, and problem domain. That skillset develops slowly over time. If you don’t fee very confident about practices, I would review other PRs to see what most committers consider best practices

EDIT:
@ryantm might have a better “plan of attack”

2 Likes

@cideM @cprussin Thank you for volunteering! I will try to write up some instructions for you soon.

@ryantm I would be happy to help as well. I use nodejs + nix at work on daily basis and should be able to commit around 2 hours a week to this.

1 Like

@jonringer @ryantm Cool, I’ve added myself in https://github.com/NixOS/nixpkgs/pull/99666.

Few questions:

  1. Is the expectation here purely to help review PRs or are there larger plans for improving the nodejs ecosystem in nixpkgs that maintainers should be thinking about? What level of freedom & discretion about those changes do maintainers have and where is the appropriate place to discuss significant changes before making them?

  2. Are there discussion forums I should be joining as a maintainer? I sit on the #nix channel on the functionalprogramming slack, but I don’t notice many maintainers in that channel apart from @cdepillabout. I’m happy to become more active elsewhere too.

1 Like

oh and @adrian-gierakowski and @cideM I’d be happy to add you two as CODEOWNERS in the PR I opened, just let me know if ya’ll are interested still.

Short term, just PRs, which is the intent of this discussion thread. Changes to the node ecosystem should probably done through an issue and bring in previous contributors to make sure there’s a marked improvement.

Remember that people will be “consuming” the changes, and that nixpkgs can be thought of as an “api”. Breaking changes should be few, and warranted

Discourse is the only official forum/mailing list. IRC is the only official chat platform. Generally most nixpkgs affecting discussions happen on github. The other chat channels are usually there for brain storming, collaboration, or help. You’re not required participate in the other chat circles. (E.g. I spend the most time in discord)

1 Like

Thanks a bunch, that’s helpful context.

FWIW when I was talking about “significant changes” I mostly had in mind internal plumbing or additions, not breaking changes. No question that breaking changes should be few and far between and well justified. But in terms of anything significant, it seems a github issue is the appropriate starting point.

1 Like

Moving the discussion back here as I’ve closed the PR since it seems github doesn’t support CODEOWNERS who don’t have write access to the repository.

Any suggestions on how to effectively contribute as a maintainer without being listed as a CODEOWNER?

2 Likes

Before we get some tools made, you could do these things:

  1. Ask people who made PRs to remove their changes to node-packages.nix.
  2. Consolidate the changes from the PRs yourself with no changes to node-packages.nix.
  3. Make separate PRs that only update node-packages.nix via the generate script.

You can ping me whenever you need something merged.

2 Likes

the node infrastructure heavily uses node2nix, so the node2nix issue tracker might also be a good place to discuss some things.

Personally, for me (as someone who is not really too familiar with node, but ended up maintaining one node package (mastodon-bot)) the main pain point is that when you use node2nix to update the dependencies for one package, it also updates the dependencies for all other packages.

I think that is a big cause of the many conflicts between PR’s updating node packages, and also makes reviewing harder: I can easily test the one package I maintain still works, but am reluctant to approve a PR that also updates the dependencies for all those other node packages.

See also this node2nix issue.

Sorry I was away from home on a communion and didn’t have time to check in. I’m still up for the task! I see you already said that apparently this CODEOWNER thing doesn’t work without write permissions so I think I’ll just start by trying to make some time for taking a look at existing node PRs and doing the steps that @ryantm outlined.

5 Likes

@ryantm Is there an easy way for you to massively label every PR that touches node-packages.nix ?

Could serve as a quick work around:

Edit: at the very least the 6:topic label doesn’t seem to work. I checked in one of my node submissions.

2 Likes

Also, as a start, the documentation should be updated to tell contributors to not run ./generate.sh: