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

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

Open
Silic0nS0ldier opened this issue Oct 15, 2024 · 0 comments

Comments

@Silic0nS0ldier
Copy link

Silic0nS0ldier commented Oct 15, 2024

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.
  1. :*_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

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

No branches or pull requests

1 participant