Skip to content

refactor: adopt new NodeJS runtime toolchain#2330

Merged
jbedard merged 3 commits intoaspect-build:3.xfrom
guw:use_runtime_toolchain
Nov 13, 2025
Merged

refactor: adopt new NodeJS runtime toolchain#2330
jbedard merged 3 commits intoaspect-build:3.xfrom
guw:use_runtime_toolchain

Conversation

@guw
Copy link
Copy Markdown
Contributor

@guw guw commented Aug 24, 2025

js_binary should use the new runtime toolchain to avoid execution toolchain being leaked into target environments (eg., js_image_oci)

See bazel-contrib/rules_nodejs#3854

Depends on:

Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: yes
  • Breaking change (forces users to change their own code or config): no
  • Suggested release notes appear below: yes

Switched js_binary (and js_test) to start using the new runtime toolchain type introduced by rules_nodejs to better support cross-platform builds (eg., building arm64 container from amd64).

Test plan

  • Covered by existing test cases

@aspect-workflows
Copy link
Copy Markdown

aspect-workflows bot commented Aug 24, 2025 •
edited
Loading

Bazel 7 (Test)

All tests were cache hits

291 tests (100.0%) were fully cached saving 49s.

Bazel 8 (Test)

All tests were cache hits

254 tests (100.0%) were fully cached saving 35s.

Bazel 7 (Test)

e2e/bzlmod

All tests were cache hits

5 tests (100.0%) were fully cached saving 661ms.

Bazel 7 (Test)

e2e/git_dep_metadata

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Bazel 7 (Test)

e2e/gyp_no_install_script

All tests were cache hits

2 tests (100.0%) were fully cached saving 177ms.

Bazel 7 (Test)

e2e/js_image_oci

All tests were cache hits

1 test (100.0%) was fully cached saving 7s.

Bazel 7 (Test)

e2e/npm_link_package

All tests were cache hits

2 tests (100.0%) were fully cached saving 297ms.

Bazel 7 (Test)

e2e/npm_link_package-esm

All tests were cache hits

2 tests (100.0%) were fully cached saving 212ms.

Bazel 7 (Test)

e2e/npm_link_package-rerooted

All tests were cache hits

2 tests (100.0%) were fully cached saving 196ms.

Bazel 7 (Test)

e2e/npm_translate_lock

All tests were cache hits

3 tests (100.0%) were fully cached saving 725ms.

Bazel 7 (Test)

e2e/npm_translate_lock_disable_hooks

All tests were cache hits

3 tests (100.0%) were fully cached saving 698ms.

Bazel 7 (Test)

e2e/npm_translate_lock_empty

All tests were cache hits

2 tests (100.0%) were fully cached saving 128ms.

Bazel 7 (Test)

e2e/npm_translate_lock_exclude_package_contents

All tests were cache hits

1 test (100.0%) was fully cached saving 34ms.

Bazel 7 (Test)

e2e/npm_translate_lock_link_workspace

All tests were cache hits

2 tests (100.0%) were fully cached saving 171ms.

Bazel 7 (Test)

e2e/npm_translate_lock_multi

All tests were cache hits

2 tests (100.0%) were fully cached saving 164ms.

Bazel 7 (Test)

e2e/npm_translate_lock_partial_clone

All tests were cache hits

1 test (100.0%) was fully cached saving 61ms.

Bazel 7 (Test)

e2e/npm_translate_lock_replace_packages

All tests were cache hits

4 tests (100.0%) were fully cached saving 590ms.

Bazel 7 (Test)

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

1 test (100.0%) was fully cached saving 96ms.

Bazel 7 (Test)

e2e/npm_translate_package_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Bazel 7 (Test)

e2e/npm_translate_yarn_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Bazel 7 (Test)

e2e/package_json_module

All tests were cache hits

1 test (100.0%) was fully cached saving 247ms.

Bazel 7 (Test)

e2e/patch_from_repo

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Bazel 7 (Test)

e2e/pnpm_lockfiles

All tests were cache hits

33 tests (100.0%) were fully cached saving 3s.

Bazel 7 (Test)

e2e/pnpm_repo_install

All tests were cache hits

1 test (100.0%) was fully cached saving 1s.

Bazel 7 (Test)

e2e/pnpm_version

All tests were cache hits

1 test (100.0%) was fully cached saving 81ms.

Bazel 7 (Test)

e2e/pnpm_workspace

All tests were cache hits

15 tests (100.0%) were fully cached saving 3s.

Bazel 7 (Test)

e2e/pnpm_workspace_deps

All tests were cache hits

3 tests (100.0%) were fully cached saving 359ms.

Bazel 7 (Test)

e2e/pnpm_workspace_rerooted

All tests were cache hits

15 tests (100.0%) were fully cached saving 8s.

Bazel 7 (Test)

e2e/repo_mapping

All tests were cache hits

3 tests (100.0%) were fully cached saving 386ms.

Bazel 7 (Test)

e2e/runfiles

All tests were cache hits

1 test (100.0%) was fully cached saving 96ms.

Bazel 7 (Test)

e2e/stamped_package_json

All tests were cache hits

1 test (100.0%) was fully cached saving 44ms.

Bazel 7 (Test)

e2e/vendored_node

All tests were cache hits

1 test (100.0%) was fully cached saving 78ms.

Bazel 7 (Test)

e2e/vendored_tarfile

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Bazel 7 (Test)

e2e/verify_patches

All tests were cache hits

2 tests (100.0%) were fully cached saving 109ms.

Bazel 7 (Test)

e2e/worker

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.

Buildifier      Format

@guw guw mentioned this pull request Aug 25, 2025
Merged
7 tasks
@keith keith mentioned this pull request Aug 26, 2025
Closed
@keith
Copy link
Copy Markdown
Contributor

keith commented Aug 26, 2025

nice, verified this fixes this case too #2328

@guw guw force-pushed the use_runtime_toolchain branch from 1390f08 to 3eb24e8 Compare October 27, 2025 08:59
@guw
Copy link
Copy Markdown
Contributor Author

guw commented Oct 27, 2025

I am left with the following issue when building:

>  bazel build --toolchain_resolution_debug='@@rules_nodejs~//nodejs:toolchain_type' //...

INFO: ToolchainResolution: Performing resolution of @@rules_nodejs~//nodejs:toolchain_type for target platform @@platforms//host:host
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_amd64//:toolchain; mismatching values: linux, x86_64
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_arm64//:toolchain; mismatching values: linux
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_s390x//:toolchain; mismatching values: linux, s390x
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_ppc64le//:toolchain; mismatching values: linux, ppc
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_darwin_amd64//:toolchain; mismatching values: x86_64
      ToolchainResolution:   Toolchain @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain is compatible with target plaform, searching for execution platforms:
      ToolchainResolution:     Compatible execution platform @@platforms//host:host
      ToolchainResolution:   All execution platforms have been assigned a @@rules_nodejs~//nodejs:toolchain_type toolchain, stopping
      ToolchainResolution: Recap of selected @@rules_nodejs~//nodejs:toolchain_type toolchains for target platform @@platforms//host:host:
      ToolchainResolution:   Selected @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain to run on execution platform @@platforms//host:host
INFO: ToolchainResolution: Performing resolution of @@rules_nodejs~//nodejs:toolchain_type for target platform @@platforms//host:host
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_amd64//:toolchain; mismatching values: linux, x86_64
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_arm64//:toolchain; mismatching values: linux
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_s390x//:toolchain; mismatching values: linux, s390x
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_ppc64le//:toolchain; mismatching values: linux, ppc
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_darwin_amd64//:toolchain; mismatching values: x86_64
      ToolchainResolution:   Toolchain @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain is compatible with target plaform, searching for execution platforms:
      ToolchainResolution:     Compatible execution platform @@platforms//host:host
      ToolchainResolution:   All execution platforms have been assigned a @@rules_nodejs~//nodejs:toolchain_type toolchain, stopping
      ToolchainResolution: Recap of selected @@rules_nodejs~//nodejs:toolchain_type toolchains for target platform @@platforms//host:host:
      ToolchainResolution:   Selected @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain to run on execution platform @@platforms//host:host
INFO: ToolchainResolution: Performing resolution of @@rules_nodejs~//nodejs:toolchain_type for target platform //js/private/test/image:linux_amd64
      ToolchainResolution:   Toolchain @@rules_nodejs~~node~nodejs_linux_amd64//:toolchain is compatible with target plaform, searching for execution platforms:
      ToolchainResolution:     Incompatible execution platform @@platforms//host:host; mismatching values: linux, x86_64
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_arm64//:toolchain; mismatching values: aarch64
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_s390x//:toolchain; mismatching values: s390x
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_linux_ppc64le//:toolchain; mismatching values: ppc
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_darwin_amd64//:toolchain; mismatching values: osx
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain; mismatching values: osx, aarch64
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_windows_amd64//:toolchain; mismatching values: windows
      ToolchainResolution:   Rejected toolchain @@rules_nodejs~~node~nodejs_windows_arm64//:toolchain; mismatching values: windows, aarch64
      ToolchainResolution: No @@rules_nodejs~//nodejs:toolchain_type toolchain found for target platform //js/private/test/image:linux_amd64.
INFO: ToolchainResolution: Target platform @@platforms//host:host: Selected execution platform @@platforms//host:host, type @@rules_nodejs~//nodejs:toolchain_type -> toolchain @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain
INFO: ToolchainResolution: Target platform @@platforms//host:host: Selected execution platform @@platforms//host:host, type @@rules_nodejs~//nodejs:toolchain_type -> toolchain @@rules_nodejs~~node~nodejs_darwin_arm64//:toolchain
ERROR: /Users/gwagenknecht/Development/aspect-build/rules_js/js/private/test/image/BUILD.bazel:114:20: While resolving toolchains for target //js/private/test/image:js_image_layer_untransitioned (3f5e2da): No matching toolchains found for types @@rules_nodejs~//nodejs:toolchain_type.
To debug, rerun with --toolchain_resolution_debug='@@rules_nodejs~//nodejs:toolchain_type'
If platforms or toolchains are a new concept for you, we'd encourage reading https://bazel.build/concepts/platforms-intro.
ERROR: Analysis of target '//js/private/test/image:transition_js_image_layer' failed; build aborted: Analysis failed

@guw guw force-pushed the use_runtime_toolchain branch from 3eb24e8 to f997478 Compare October 27, 2025 10:29
@guw guw marked this pull request as ready for review October 27, 2025 10:30
guw
guw commented Oct 27, 2025
View reviewed changes
fmeum
fmeum reviewed Oct 27, 2025
View reviewed changes
@guw guw force-pushed the use_runtime_toolchain branch from f997478 to 6e15cc8 Compare October 27, 2025 13:05
@guw guw force-pushed the use_runtime_toolchain branch 2 times, most recently from a9d9687 to dfeafdb Compare November 5, 2025 22:53
@guw guw requested a review from fmeum November 5, 2025 22:54
@guw guw force-pushed the use_runtime_toolchain branch 2 times, most recently from 7bdc6bd to 6fc0597 Compare November 6, 2025 01:57
@guw
Copy link
Copy Markdown
Contributor Author

guw commented Nov 6, 2025 •
edited
Loading

@alexeagle We are down to one failing test. This looks like a side effect caused by rules_nodejs update, i.e. unrelated to the changes in this PR.

@guw guw force-pushed the use_runtime_toolchain branch from 6fc0597 to 60c74e5 Compare November 6, 2025 02:24
@jbedard
Copy link
Copy Markdown
Member

jbedard commented Nov 12, 2025

@guw would you be able to rebase this and target the 3.x branch? I'm uncertain if we want to force a rules_nodejs upgrade in rules_js 2.x, especially since we still have code to support rules_nodejs v5 in rules_js.

In the 3.x branch I upgraded the minimum rules_nodejs to 6.6 already but some oci tests are failing on mac atm. Maybe this change will fix that?

@guw
Adopt new NodeJS runtime toolchain
9ea18f2 
js_binary should use the new runtime toolchain to avoid execution toolchain being leaked into target environments (eg.,  js_image_oci)

See bazel-contrib/rules_nodejs#3854
@guw guw changed the base branch from main to 3.x November 12, 2025 20:38
@guw guw force-pushed the use_runtime_toolchain branch from 60c74e5 to 9ea18f2 Compare November 12, 2025 20:39
@guw
Copy link
Copy Markdown
Contributor Author

guw commented Nov 12, 2025

@jbedard here you go

@jbedard
Copy link
Copy Markdown
Member

jbedard commented Nov 12, 2025

Great, thanks! 👍

Any ideas about the e2e/vendored_node test failure? Then I think you just need to bazel run @@//:update within the docs/ dir...

@jbedard
Copy link
Copy Markdown
Member

jbedard commented Nov 12, 2025

Do you know what the e2e/vendored_node test failure is?

@guw
Copy link
Copy Markdown
Contributor Author

guw commented Nov 12, 2025

Unfortunately not. But reading through the code, it seems to expect a specific NodeJS version, which is only setup via WORKSPACE file but not via MODULE.bazel. Thus, I suspect this test is broken in bzlmod.

@jbedard
Copy link
Copy Markdown
Member

jbedard commented Nov 12, 2025

That test is green on the 3.x branch though. Your change seems to have effected the register_toolchains() call in e2e/vendored_node/MODULE.bazel?

@guw
Copy link
Copy Markdown
Contributor Author

guw commented Nov 12, 2025

Yes. The tests expects a very specific NodeJS version. I do believe - however - that it should be fetched from somewhere else because of the "vendored" name.

@jbedard
Copy link
Copy Markdown
Member

jbedard commented Nov 12, 2025

I think previously the register_toolchains registered all in //toolchains/...

Why does your PR change that?

@guw
Copy link
Copy Markdown
Contributor Author

guw commented Nov 13, 2025

@jbedard I think I know what's going on. The toolchain registrations are only for compile/transpile (aka. build) actions. For tests the runtime toolchain is being used. However, the test does not register a runtime toolchain. I'll try to fix that.

@guw guw force-pushed the use_runtime_toolchain branch from e7fd609 to 844f9d0 Compare November 13, 2025 10:11
jbedard
jbedard approved these changes Nov 13, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jbedard jbedard left a comment

Choose a reason for hiding this comment

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

Great 👍

@jbedard jbedard changed the title Adopt new NodeJS runtime toolchain refactor: adopt new NodeJS runtime toolchain Nov 13, 2025
@jbedard jbedard merged commit a3aa85b into aspect-build:3.x Nov 13, 2025
164 of 170 checks passed
@guw guw deleted the use_runtime_toolchain branch November 14, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbedard jbedard jbedard approved these changes

+1 more reviewer

@fmeum fmeum fmeum approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@guw @keith @jbedard @fmeum