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

Upgrading to node 16.x LTS (and pkgs.nix upgrade to 21.11 or later) #37

Merged
merged 4 commits into from
Apr 25, 2022

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Apr 18, 2022

Description

Node 14 is already EOL. We should be upgrading to node 16.x LTS. This introduces some major features that we want in NodeJS:

This includes some other changes:

  • Updating pkgs.nix to 21.11 (or later/master) which has nodejs-16_x but nodejs is still 14.19.1 (we may need to go to master instead
  • Using node2nix at 1.10.0 which supports 16.x, currently 1.9.0 does not support 16.x
  • Updating to pkg to 5.6.0, which requires an update to our pkgBuilds in utils.nix. As it would no longer be using 3.1 and no longer using 14.17.0 and upgrade to v16
  • Fix typescript version to 4.5+ so that we can make use of the latest ecmascript features
  • Upgrade @types/node to 16.x as well

Issues Fixed

Tasks

  • 1. Update @types/node from 14.14.35 to 16.11.7
  • 2. Update pkg from 5.3.0 to 5.6.0
  • 3. Update typescript from 4.1.3 to 4.5.2
  • 4. Update pkgs.nix to NixOS/nixpkgs@a5774e7
  • 5. Update tsconfig target from ES2020 to ES2021
  • 6. Update pkgBuilds from 3.1 to 3.3 with node version 16.14.2
  • 7. Update eslint ecma version from 2020 to 2021
  • 8. Update @typescript-eslint/eslint-plugin and @typescript-eslint/parser from 4.12.0 to 5.0.0
  • 9. Update node-gyp-build from 4.2.3 to 4.4.0
  • [ ] 10. Remove leveldown patch for pkg, and replace it with --no-dict leveldown option as specified here: Fix warnings when packaging with leveldown. vercel/pkg#1273 - this did not work, and also the package.json patch did not work, this is kept the way it is
  • 11. Updated typedoc to support TS 4.5.x and regenerated the docs

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai CMCDragonkai changed the title WIP: Upgrading to 21.11 (or more importantly to node 16.x LTS) WIP: Upgrading to node 16.x LTS (and pkgs.nix upgrade to 21.11 or later) Apr 18, 2022
@ghost
Copy link

ghost commented Apr 18, 2022

CodeSee Review Map:

Review these changes using an interactive CodeSee Map

Review in an interactive map

View more CodeSee Maps

Legend

CodeSee Map Legend

@CMCDragonkai
Copy link
Member Author

Note that the current pkgs.nix hash matches what we are using in our platforms. However it currently still hasn't fully updated to node2nix 1.10.0. Initial nix-shell currently works, however, we would want to make sure we are using 1.10.0 so we can try out the node 16.x and to do testing. The pkg however has been updated to 5.4.1.

@CMCDragonkai
Copy link
Member Author

@tegefaulkes relevant for our soon to be release.

@CMCDragonkai
Copy link
Member Author

Will need to test master of nixpkgs, not 21.11.

@CMCDragonkai
Copy link
Member Author

Node 16 has Promise.any and AggregateError too.

This should be ideally used in our ICE of direct connection and signalling.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/any

@tegefaulkes @emmacasolin

@emmacasolin
Copy link
Contributor

emmacasolin commented Apr 22, 2022

With the following changes I've got Promise.any working. All of the existing tests are also still passing.

  1. In package.json @types/node has been updated from 14.14.35 to 16.11.7 - this is the only version of Node 16 supported by @types/node https://www.npmjs.com/package/@types/node?activeTab=versions
  2. In package.json pkg has been updated from 5.3.0 to 5.4.1
  3. In package.json typescript has been updated from 4.1.3 to 4.5.2 - this looks to be the oldest stable version of 4.5 https://github.com/microsoft/TypeScript/tags
  4. In pkgs.nix rev is pointing to NixOS/nixpkgs@b03f03e which has node2nix 1.10.0
  5. In tsconfig.json target has been updated from ES2020 to ES2021
  6. In utils.nix pkgBuilds has been updated from 3.1 to 3.3 with node version 16.14.2 - this is the only supported node version I could find here https://github.com/vercel/pkg-fetch/tags
  7. In .eslintrc parserOptions.ecmaVersion has been updated from 2020 to 2021
  8. In package.json @typescript-eslint/eslint-plugin and @typescript-eslint/parser have been updated from 4.12.0 to 5.0.0 (4.12.0 does not support typescript 4.5+)
  9. In package.json node-gyp-build has been updated from 4.2.3 to 4.4.0 (needed to be updated in order to successfully build releases)

@CMCDragonkai
Copy link
Member Author

Make sure that the version used by PKG fetch is the same version of nodejs used in nix-shell. Or at the very least a compatible version. The hashes would have to be updated.

@CMCDragonkai
Copy link
Member Author

And test that AbortSignal exists and AggregateError exists.

@emmacasolin
Copy link
Contributor

Make sure that the version used by PKG fetch is the same version of nodejs used in nix-shell. Or at the very least a compatible version. The hashes would have to be updated.

And test that AbortSignal exists and AggregateError exists.

They're both using 16.14.2, and I've updated the hashes as well. AbortSignal and AggregateError also both exist, as well as Promise.any.

@emmacasolin
Copy link
Contributor

With all of the changes from this comment #37 (comment) both building the releases

nix-build ./release.nix --attr application
nix-build ./release.nix --attr docker

And installing into Nix user profile

nix-env -f ./release.nix --install --attr application

Work without issues.

However, I'm getting errors from attempting to build the package

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ nix-build -E '(import ./pkgs.nix).callPackage ./default.nix {}'
error: value is a function while a set was expected, at (string):1:1

And installing into Docker

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ loaded="$(docker load --input "$(nix-build ./release.nix --attr docker)")"

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ image="$(cut -d' ' -f3 <<< "$loaded")"

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ docker run -it "$image"
docker: Error response from daemon: OCI runtime create failed: container_linux.go:380: starting container process caused: exec: "/bin/typescript-demo-lib": stat /bin/typescript-demo-lib: no such file or directory: unknown.

@CMCDragonkai
Copy link
Member Author

After doing this:

nix-env -f ./release.nix --install --attr application

Are you able to run the executable as typescript-demo-lib?

Note that you should try just doing nix-build ./release.nix --attr application and then try ./results/bin/typescript-demo-lib as well. What is in the result symlink? Use tree ./result.

I read somewhere that node2nix has a problem producing the final executable right now, and that could be causing the docker image problem.

@CMCDragonkai
Copy link
Member Author

Btw nix-build -E '(import ./pkgs.nix).callPackage ./default.nix {}' is no longer correct. I hadn't updated it yet.

It now needs to be nix-build -E '(import ./pkgs.nix {}).callPackage ./default.nix {}'.

@CMCDragonkai
Copy link
Member Author

Can you update the README.md with the new command.

@CMCDragonkai
Copy link
Member Author

If the bin executable is missing. See if this issue is the culprit: NixOS/nixpkgs#145432.

Furthermore, remember to update the task list for this PR with your changes.

@CMCDragonkai
Copy link
Member Author

Remember to push up your changes so I can run it too.

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Apr 22, 2022

And if the bin executable is truely missing, then you can try to debug why inside the default.nix. See all the cp and ln -s commands. Those are all the shell commands that run to get the final output correct in $out/bin. You can trace backwards by injecting ls at various places like we do with console.log to see what is happening inside during the build.

      # copy the dist
      cp -r ${utils.node2nixDev}/lib/node_modules/${utils.node2nixDev.packageName}/dist $out/lib/node_module>
      # copy over the production dependencies
      if [ -d "${utils.node2nixProd}/lib/node_modules" ]; then
        cp -r ${utils.node2nixProd}/lib/node_modules $out/lib/node_modules/${utils.node2nixDev.packageName}/
      fi
      # create symlink to the deployed executable folder, if applicable
      if [ -d "${utils.node2nixDev}/lib/node_modules/.bin" ]; then
        cp -r ${utils.node2nixDev}/lib/node_modules/.bin $out/lib/node_modules/
        ln -s $out/lib/node_modules/.bin $out/bin # <--------- this is what should be producing the final bin link
      fi

@emmacasolin
Copy link
Contributor

Running nix-build ./release.nix --attr application produces a result symlink with just lib/node_modules/@matrixai/typescript-demo-lib/...

@CMCDragonkai
Copy link
Member Author

Exactly, can you use tree ./result to show the full output?

So the bin executable isn't being produced. This is the thing that people have been mentioning for node2nix.

Can you debug default.nix #37 (comment) to find out why the bin executable isn't being created.

@CMCDragonkai
Copy link
Member Author

For reference, this is what the current master shows:

»» ~/Projects/TypeScript-Demo-Lib
 ♖ ll result/                                                                        (master) pts/7 17:09:43
total 9.0K
lrwxrwxrwx 2 root root 91 Jan  1  1970 bin -> /nix/store/1z01yfni6z6v6283w203i6pnkiqlw5gq-typescript-demo-lib-1.2.0/lib/node_modules/.bin/
dr-xr-xr-x 3 root root  3 Jan  1  1970 lib/

Actually tree produces too much data. But the bin -> ... is what we are looking for. So you might want to check the pointed-to symlink to see if lib/node_modules/.bin exists..

@CMCDragonkai
Copy link
Member Author

Then you post an issue upstream on node2nix (the issue I linked was on nixpkgs).

@emmacasolin
Copy link
Contributor

Digging into this a bit more, it looks like running any of nix-build -E '(import ./pkgs.nix {}).callPackage ./default.nix {}', nix-build ./release.nix --attr application, or nix-env -f ./release.nix --install --attr application creates just the lib subdirectory

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ ll result/
total 9
dr-xr-xr-x 3 root root 3 Jan  1  1970 lib

But running either of the docker-related commands completely breaks the symlink:

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ tree ./result
./result [error opening dir]

0 directories, 0 files

[nix-shell:~/Projects/TypeScript-Demo-Lib]$ ll result/
ls: cannot access 'result/': Not a directory

So I don't think the problem is only with ./default.nix

@CMCDragonkai
Copy link
Member Author

And what is in side ./result/lib/node_modules/.bin?

@emmacasolin
Copy link
Contributor

Nothing, it doesn't exist

@CMCDragonkai
Copy link
Member Author

Ok let's see what's inside the result tomorrow. But you may need to go into node2nix generated nix files and see what's going on with the generated package instructions.

@emmacasolin
Copy link
Contributor

Looks like the problem might be that pkgs.nix hash I'm using isn't using node2nix 1.10.0 properly. Even though it says it's using 1.10.0, when running node2nix --version it says 1.9.0. So I'll have to find a different hash to use.

@emmacasolin
Copy link
Contributor

Looks like the problem might be that pkgs.nix hash I'm using isn't using node2nix 1.10.0 properly. Even though it says it's using 1.10.0, when running node2nix --version it says 1.9.0. So I'll have to find a different hash to use.

So unfortunately it looks like even master is still using 1.9.0 https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/node-packages/default.nix#L261

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Apr 25, 2022

I've compared the old 1.9.0 to 1.11.0. I suspect that something changed with npm install behaviour coupled with changes to node2nix node-env.nix as per above has led to the .bin directory no longer being created.

Reported this upstream: svanderburg/node2nix#293

While upstream figures out why. The workaround right now is to directly refer to these executables under our package and just create those symlinks in default.nix. We do this already in our default.nix, we just need to change:

    # create symlink to the deployed executable folder, if applicable
    if [ -d "${utils.node2nixDev}/lib/node_modules/.bin" ]; then
      cp -v -r ${utils.node2nixDev}/lib/node_modules/.bin $out/lib/node_modules/
      ln -v -s $out/lib/node_modules/.bin $out/bin
    fi

To instead create symlinks for every file in: $out/lib/node_modules/${utils.node2nixDev.packageName}/dist/bin to $out/bin.

Or extract this information from package.json bin.

Or continue figuring out how to make npm install do what it used to do based on this information: https://docs.npmjs.com/cli/v8/configuring-npm/folders#more-information

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Apr 25, 2022

The closest thing I can get to is replacing npm install in node2nixDev with:

npm_config_prefix="$out" npm install --global

However this doesn't work, it has a weird error about not finding the package.json.

Instead npm config_prefix="$out/lib" npm install --global seems to work.

However this doesn't produce the links the place we want to have it. Partly because npm install doesn't just setup the .bin but also wants to setup symlinks for node_modules, and because the name is the same, it results in some problems.

I think our node2nixDev might be need to be slightly changed. It may need to use --install-links as part of npm install. Or we need to override the entire installPhase.

Another alternative is to use:

npm_config_prefix="$out" ${nodejs}/bin/npm install --global ${utils.node2nixDev}/lib/node_modules/${utils.node2nixDev.packageName}

In the default.nix, which will just make the default.nix the global output, with symlinks pointing into the nix store.

»» ~/Projects/tmp/smallproject
 ♖ tree ./result                                       (master) pts/3 22:47:26
./result
├── bin
│   └── dosomething -> ../lib/node_modules/somerandompackage/src/bin/dosomething.js
└── lib
    └── node_modules
        └── somerandompackage -> ../../../9g0xha6s7287jbfq9dma1cz9fnfbn1ai-somerandompackage-1.2.0/lib/node_modules/somerandompackage

@CMCDragonkai
Copy link
Member Author

Given that there's no elegant solution atm, I've decided to go with some duct tape until upstream fixes the problem.

In the default.nix, at the very end it is now using jq to parse out the package.json bin properties, and automatically generates the symlinks to the bin paths, while also making those bin paths executable with chmod a+x.

This works well enough for now, as the previous system was already kind of weird, relying on npm install to build symlinks and then mapping them with further symlinks.

Future solutions may involve redoing the node2nix derivations to be more direct to our infrastructure.

@CMCDragonkai
Copy link
Member Author

Docker builds has a problem, but nothing to do with this repo, it appears nodejs itself doesn't work inside docker containers.

However it's good idea for others to try @tegefaulkes to see if it's just my laptop right now.

Reported upstream: NixOS/nixpkgs#170279

@CMCDragonkai
Copy link
Member Author

@emmacasolin 5.6.0 of pkg is still trying to load v14 nodejs base binaries.

> pkg@5.6.0
> Fetching base Node.js binaries to PKG_CACHE_PATH
  fetched-v14.19.1-linux-x64          [] 0%> Not found in remote cache:
  {"tag":"v3.3","name":"node-v14.19.1-linux-x64"}

I wonder why this is the case.

Btw if you want ot make sure to get the right hash, use nix-prefetch-url on those URLs. Don't just use the ones provided.

@CMCDragonkai
Copy link
Member Author

Easy fix, just had to specify the node range in the release.nix. The builds appears to be working, but we would need to run them to be sure.

* Updated pkgs.nix to a5774e76bb8c3145eac524be62375c937143b80c
* Updated node2nix to 1.11.0 and loading directly from GitHub
* Updated pkg to 5.6.0 and using pkg-fetch 3.3 base binaries
* Updated to TypeScript 4.5+ and eslint to ^5.0.0
* Updated @types/node to 16.11.7
* Updated node-gyp-build to 4.4.0
* Updated typedoc to 0.22.15, the .nojekyll generation is automatic
* Added dist/**/*.json to pkg assets
* Changed to target ES2021 as node 16 supports it
* Bin executable duct-tape in default.nix because node2nix no longer generates bin executables
@CMCDragonkai
Copy link
Member Author

Everything seems to be working now, EXCEPT the docker builds... will wait and see.

@CMCDragonkai
Copy link
Member Author

I've tested the linux executable on Ubuntu so it works.

Going to merge this as CI/CD will be able to test on docker and macos builds and windows builds.

The docker problem might be just my computer. Let's see.

@CMCDragonkai CMCDragonkai changed the title WIP: Upgrading to node 16.x LTS (and pkgs.nix upgrade to 21.11 or later) Upgrading to node 16.x LTS (and pkgs.nix upgrade to 21.11 or later) Apr 25, 2022
@CMCDragonkai CMCDragonkai merged commit 3f889d0 into master Apr 25, 2022
@CMCDragonkai CMCDragonkai deleted the 21.11 branch April 25, 2022 17:37
@tegefaulkes
Copy link
Contributor

I can't seem to build or nix-shell this. I get the error

> nix-shell
error: AWS error fetching 'ymcglmwrkg8kdxpp0wnd29y2m17js9ah.narinfo': The AWS Access Key Id you provided does not exist in our records.

SO i'll have to check my AWS access. But I don't have this problem with the js-polykey repo.

@CMCDragonkai
Copy link
Member Author

It could be that our NixOS has changed something with respect to aws access. That's also about a key id, not the AWS authentication key.

Unfortunate since you'll need to update your NixOS to be the same as all of us, but there's a problem with the current NixOS version we are on and running nodejs inside a docker container.

@tegefaulkes
Copy link
Contributor

Trying to run the docker images give me

> sudo docker run -it typescript-demo-lib-1.3.0:x02my25yw5hip2cpcrs37iadzhqx8p8c                                                                                              
/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node[1]: ../src/node_platform.cc:61:std::unique_ptr<long unsigned int> node::WorkerThreadsTaskRunner::DelayedTaskScheduler::Start(): Assertion `(0) == (uv_thread_create(t.get(), start_thread, this))' failed.
 1: 0xa64dd8 node::Abort() [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 2: 0xa64e67  [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 3: 0xae5f75 node::WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int) [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 4: 0xae60a2 node::NodePlatform::NodePlatform(int, v8::TracingController*) [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 5: 0xa24d2f node::V8Platform::Initialize(int) [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 6: 0xa2304b node::InitializeOncePerProcess(int, char**, node::InitializationSettingsFlags, node::ProcessFlags::Flags) [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 7: 0xa231d9 node::InitializeOncePerProcess(int, char**) [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 8: 0xa23238 node::Start(int, char**) [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]
 9: 0x7f4c495421d7  [/nix/store/ayrsyv7npr0lcbann4k9lxr19x813f0z-glibc-2.34-115/lib/libc.so.6]
10: 0x7f4c49542297 __libc_start_main [/nix/store/ayrsyv7npr0lcbann4k9lxr19x813f0z-glibc-2.34-115/lib/libc.so.6]
11: 0x989ce1 _start [/nix/store/zl4bvsqfxyx5vn9bbhnrmbmpfvzqj4gd-nodejs-16.14.2/bin/node]

@CMCDragonkai
Copy link
Member Author

I've tested the same image on Ubuntu VM. It works on Ubuntu no problem, and on CI/CD.

So it looks like this is a NixOS problem.

image

@CMCDragonkai
Copy link
Member Author

For now since it works in CI/CD and in ubuntu, we should conclude that our docker image is correctly built. But NixOS docker runtime has some misconfiguration/bug. So we will continue as is, until upstream figures out what's going on in our NixOS docker runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants