search

emscripten-core / emsdk

Emscripten SDK
http://emscripten.org
Other
2.92k stars 660 forks source link

bazel: bump rules_nodejs and migrate to rules_js #1388

Open linzhiq opened 1 month ago

linzhiq commented 1 month ago

Bazel's Node.js dependency comes from rules_nodejs. Previously, bazel/deps.bzl was using rules_nodejs 5.8.0, released in 2022 and only supported Node.js toolchains up to 18.12.1.

This PR bumps rules_nodejs to latest 6.1.1. It also replaces build_bazel_rules_nodejs with rules_js, since npm_install that bazel/emscripten_deps.bzl used was deprecated. The README of rules_nodejs now recommends migrating to rules_js for everything other than the Node.js toolchain: (https://github.com/bazelbuild/rules_nodejs/commit/371e8cab15f9db444217703144b2eb71d0e13235)

Impetus

Our repo builds with Bazel and uses Emscripten and Node.js. Tried to upgrade Node.js 18 to Node.js 20 and saw that emsdk didn't support rules_nodejs 6+ in the same workspace.

walkingeyerobot commented 1 month ago

I'm generally in favor of upgrading here but this causes the bazel CI tests to fail.

mmorel-35 commented 1 month ago

@keith, Can you provide help on that ? It will help create a true bzlmod compatibility to emsdk which is requested in bcr issues

keith commented 1 month ago

I think we probably need to fix the CI here first

allsey87 commented 3 weeks ago

@walkingeyerobot I would also like to upgrade rules_nodejs and nodejs since embuilder is complaining about the current version used in the Bazel workspace and this might be the source of the errors that I am seeing in #1405

embuilder: warning: node version appears too old (seeing "v16.6.2", expected "v16.20.0") [-Wversion-check]

Should I take over this PR or perhaps open a new one? I don't think I can push commits here unless I am a "maintainer" to this repository, right?

walkingeyerobot commented 2 weeks ago

You're welcome to open a new one; I am unsure how to transfer ownership of a PR, so new one seems easiest.