[FR][Bug]: Split `@rules_nodejs//nodejs:toolchain_type` to appropriately cover all use cases

[Silic0nS0ldier]

What is the current behavior?

Currently @rules_nodejs//nodejs:toolchain_type is covering 2 separate use cases.

1. Direct usage within a rule.
2. Runtime for executable outputs.

When execution and target platforms are the same (as is usually the case) scenarios 1 and 2 can be satisfied by the same toolchain implementation.

However when the execution and target platforms are different, problems arise. For example;

# From /___/external/rules_nodejs~~node~nodejs_toolchains/BUILD.bazel
toolchain(
    name = "linux_amd64_toolchain",
    exec_compatible_with = ["@platforms//os:linux", "@platforms//cpu:x86_64"],
    toolchain = "@nodejs_linux_amd64//:toolchain",
    toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)
toolchain(
    name = "linux_amd64_toolchain_target",
    target_compatible_with = ["@platforms//os:linux", "@platforms//cpu:x86_64"],
    toolchain = "@nodejs_linux_amd64//:toolchain",
    toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)
toolchain(
    name = "darwin_arm64_toolchain",
    exec_compatible_with = ["@platforms//os:macos", "@platforms//cpu:arm64"],
    toolchain = "@nodejs_darwin_arm64//:toolchain",
    toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)
toolchain(
    name = "darwin_arm64_toolchain_target",
    target_compatible_with = ["@platforms//os:macos", "@platforms//cpu:arm64"],
    toolchain = "@nodejs_darwin_arm64//:toolchain",
    toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)

Suppose I am on macOS and building for Linux.

• :linux_amd64_toolchain will be skipped;
  • 🔴 exec_compatible_with
  • 🟢 target_compatible_with (no constaints)
• :linux_amd64_toolchain_target can be selected;
  • 🟢 exec_compatible_with (no constaints)
  • 🟢 target_compatible_with
• :darwin_arm64_toolchain can be selected;
  • 🟢 exec_compatible_with
  • 🟢 target_compatible_with (no constaints)
• darwin_arm64_toolchain_target will be skipped;
  • 🟢 exec_compatible_with (no constaints)
  • 🔴 target_compatible_with

That both :linux_amd64_toolchain_target and :darwin_arm64_toolchain represents a contradiction.

Describe the feature

Starting off with an example. Rules Java has 3 toolchain types, 2 of which are of interest.

1. @bazel_tools//tools/jdk:toolchain_type
   The compilation toolchain.
   This is always run within a rule (think cfg = "exec") and is used to produce platform-neutral output.
   Implementations use exec_compatible_with only.
2. @bazel_tools//tools/jdk:runtime_toolchain_type
   The runtime toolchain.
   This is used for outputs (e.g. from java_binary), which indirectly be used in other rules (as an implementation detail that it not directly executed).
   Implementations use target_compatible_with only.

So in Java a rule like java_binary would accept both toolchains. @bazel_tools//tools/jdk:toolchain_type for compilation and @bazel_tools//tools/jdk:runtime_toolchain_type so the output has a Java runtime to use.

This split makes it possible to cross-compile (e.g. build for Linux on macOS).

---

JS is not a compiled language however the Rules JS toolchains are setup to support such a scenario (a subset at least). The problem, as hinted in "What is the current behavior?", is that such cross-compilation scenarios do not work. Attempts to do so lead to one of 2 outcomes.

1. :*_toolchain is resolved.

• 🟢 Actions using node from the toolchain runs correctly.
• 🔴 node in the output cannot run on the target platform.

2. :*_toolchain_target is resolved.

• 🔴 Actions using node from the toolchain run cannot run.
• 🟡 node in the output runs correctly on the target platform, provided dependent actions do not depend on the resolved node.

Implementing a split like what Rules Java has would address this issue, although it still leaves once use case uncovered.

NodeJS has a handful workflows that require the execution and target platforms to be identical.

• Snapshot generation (--build-snapshot and --build-snapshot-config)
  Produces a snapshot blob that can be later loaded with --snapshot-blob. Requires NodeJS version, architecture, platform, V8 flags and CPU features match what was used to generate the snapshot.
• Single executable applications (--experimental-sea-config)
  This is a multi-step workflow, of which --experimental-sea-config incurs the same requirements as "snapshot generation" when useSnapshot or useCodeCache are enabled.

---

So, here is my proposal.

1. Introduce a new toolchain type @rules_nodejs//nodejs:runtime_toolchain_type with the same usage semantics as @bazel_tools//tools/jdk:runtime_toolchain_type. Add implementations for this new type to nodejs_toolchains generated external repository.
2. [[BREAKING CHANGE]] Remove :*_toolchain_target toolchain implementations, bringing usage semantics in line with @bazel_tools//tools/jdk:toolchain_type (ignoring that JS doesn't need to be compiled).
3. Introduce a new toolchain type to cover cases where execution and target platforms need to be the same. Add implementations for this new type to nodejs_toolchains generated external repository.

It may make sense to tweak the plan outlined according to what the bulk of current @rules_nodejs//nodejs:toolchain_type usage is for (I suspect right now executable outputs are the primary use case).

How existing toolchain types are supposed to be used isn't always obvious at a glance, same goes for those in Rules Java and especially the generic toolchain_type name. May be worthwhile workshopping the names.

Related

• No way to use toolchains correctly in executable rule output bazelbuild/bazel#19645
• [Bug]: js_image_layer is broken with bzlmod aspect-build/rules_js#1530
• fix: register Node.js toolchains in correct order #3750

[thesayyn]

AFAIK, exec_group feature is going to fix some of these issues, which i am waiting for to be on Bazel LTS to migrate rules_nodejs toolchains.

[ashi009]

for now the temporary fix would be not registering the *_target toolchains

[guw]

I think #3855 could be temporary fix, though. It works for our cross-platform rules_oci image building.