Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Update nodePackages with node2nix 1.10.0 #146440

Closed
wants to merge 8 commits into from

Conversation

svanderburg
Copy link
Member

Motivation for this change

I'm working on updating the NPM packages to be regenerated with node2nix 1.10.0

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@happysalada
Copy link
Contributor

Great to see you around! (Thanks a lot for the node2nix contribution btw)
You might be aware of this issue #145432
a missing bin for nodePackages built with node16.
since 16 is lts, there are efforts to change the default node version from 14 to 16. My understanding is that the next node2nix version handles node 16 correctly.
Let me know if I can be of any help.
Thanks again!

@svanderburg
Copy link
Member Author

@happysalada Yes I've added an option to handle Node.js 16.x out of the box. Some of the stuff that I need seems to work, but we still have to see how well the integration works.

I also intend make the modifications so that we can package for Node.js 16.x. We probably should start with a small set of packages first and then gradually expand it.

@happysalada
Copy link
Contributor

Amazing thanks! I think prettier was one of the problematic packages. If it works properly for prettier, then the potential problem might have been fixed.

@shinzui
Copy link

shinzui commented Feb 5, 2022

Hi. Is there anything blocking updating to node2nix 1.10.0?

@dotlambda
Copy link
Member

This fixes node2nix when using Node.js >= 15, right?
Since electron 14, which is the last version supporting Node.js 14, is now EOL (#166825), we should really try to get this merged.
cc @prusnak @teutat3s

@CMCDragonkai
Copy link
Member

What's blocking this?

@CMCDragonkai
Copy link
Member

I'm wondering why node2nix even on master of nixpkgs (a5774e7) is still 1.9.0 even though it claims to be 1.10.0.

nix-repl> pkgs.nodePackages.node2nix.version
"1.10.0"

nix-repl> node2nix
«derivation /nix/store/ipkkiavdmj2m8m9zxdkjsd8kfxqdj27c-node2nix-1.10.0.drv»

nix-repl> :b node2nix
[62 copied (334.6 MiB), 79.6 MiB DL]

this derivation produced the following outputs:
  out -> /nix/store/nz0fw6vhdfhv45nzlxahp6383p3m9lyj-node2nix-1.10.0

But:

$ /nix/store/nz0fw6vhdfhv45nzlxahp6383p3m9lyj-node2nix-1.10/bin/node2nix --version
node2nix 1.9.0
$ /nix/store/nz0fw6vhdfhv45nzlxahp6383p3m9lyj-node2nix-1.10/bin/node2nix --help | grep nodejs
  -4, --nodejs-4                Provides all settings to generate expression for usage with Node.js 4.x (default is: Node.js 12.x)
  -6, --nodejs-6                Provides all settings to generate expression for usage with Node.js 6.x (default is: Node.js 12.x)
  -8, --nodejs-8                Provides all settings to generate expression for usage with Node.js 8.x (default is: Node.js 12.x)
  -10, --nodejs-10              Provides all settings to generate expression for usage with Node.js 10.x (default is: Node.js 12.x)
  -12, --nodejs-12              Provides all settings to generate expression for usage with Node.js 12.x (default is: Node.js 12.x)
  -13, --nodejs-13              Provides all settings to generate expression for usage with Node.js 13.x (default is: Node.js 12.x)
  -14, --nodejs-14              Provides all settings to generate expression for usage with Node.js 14.x (default is: Node.js 12.x)
  --pkg-name NAME               Specifies the name of the Node.js package to use from Nixpkgs (defaults to: nodejs)

It doesn't even show the --nodejs-16 option.

@dotlambda
Copy link
Member

I'm wondering why node2nix even on master of nixpkgs (a5774e7) is still 1.9.0 even though it claims to be 1.10.0.

That's because of the override in pkgs/development/node-packages/default.nix.

@dotlambda
Copy link
Member

My suggestion would be something like this where we don't touch packages that aren't inside nodePackages: #170722
Note that it breaks their update scripts however.

Copy link
Contributor

@freezeboy freezeboy left a comment

Choose a reason for hiding this comment

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

Nice ! thank you

@bobby285271 bobby285271 added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Jun 10, 2022
@ParetoOptimalDev
Copy link

I believe this is blocking fixing mongosh and pretty much all nodejs_16x packages.

It seems like it was approved but never merged, so this should be low-hanging fruit if the merge conflict isn't too difficult?

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Aug 19, 2022

I believe this is blocking fixing mongosh and pretty much all nodejs_16x packages.

It seems like it was approved but never merged, so this should be low-hanging fruit if the merge conflict isn't too difficult?

node2nix was already updated in #171195 as I understand it. This PR also regenerates a few of the packages not under pkgs/development/node-packages, but that may have already been done in separate PRs as well

I believe there are still some issues with node2nix and Node.js 16+ that will need to be fixed in svanderburg/node2nix first before some of those packages will work correctly. Mostly svanderburg/node2nix#236 (comment) and svanderburg/node2nix#293 I think

@winterqt
Copy link
Member

I believe this is blocking fixing mongosh and pretty much all nodejs_16x packages.

I don't think this is true -- node2nix still cannot output v2 lockfiles.

@ParetoOptimalDev
Copy link

I believe this is blocking fixing mongosh and pretty much all nodejs_16x packages.

I don't think this is true -- node2nix still cannot output v2 lockfiles.

My mistake, I'm new to node/javascript on NixOS and just got here by attempting to connect to a mongo server using the mongo cli on nixos 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants