fix: fetch node toolchain based on exec platform, not target platform #2565
Conversation
Currently rules_nodejs can't be used when cross-compiling to a different platform, if that platform does not have a node toolchain available. This means if you want to cross-compile a project that includes JS files, you need to do it in multiple steps: - build the JS files with the default host platform - build the other products with a target platform specified, copying the generated JS files in The fact that it's currently set to target makes me wonder if this use case was just not considered at the time, or whether there's some other workflow that changing to exec will break. Thoughts?
dae
requested review from
alexeagle,
gregmagolan,
mattem and
soldair
as code owners
|
Ok, judging by the test failures, I gather this is working as intended at the moment - perhaps for building native node libraries? Presumably I can accomplish the same result by registering a custom toolchain in my workspace file, so I'm guessing that's the recommended way to handle cross compilation? (please feel free to close this) |
|
If "register your own toolchain" is the correct answer here, would you be happy to accept the following patch? Currently the toolchain type is marked as private, so I can't register an exec-bound toolchain in my workspace. diff --git a/toolchains/node/BUILD.bazel b/toolchains/node/BUILD.bazel
index 9bfab568..20ffc3ef 100644
--- a/toolchains/node/BUILD.bazel
+++ b/toolchains/node/BUILD.bazel
@@ -70,7 +70,10 @@ filegroup(
)
# node toolchain type
-toolchain_type(name = "toolchain_type")
+toolchain_type(
+ name = "toolchain_type",
+ visibility = ["//visibility:public"],
+)
# Allow targets to use a toolchains attribute, such as sh_binary and genrule
# This way they can reference the NODE_PATH make variable.If this is not desirable, is there another solution? |
|
Sorry for the delay!
Yes! I think you ought to be able to pass your own toolchain, regardless whether we get this PR green. I don't really understand the toolchain exec-bound vs target-bound. Maybe we can find someone else who does? Or else I'll need to make some time to read up on this. |
Enables rules_nodejs to be used in cross-compilation contexts, where
the target architecture is not supported by rules_nodejs.
rules_nodejs's default toolchains are limited via target_compatible_with,
which means they will not work when cross-compiling to Android or iOS
with --platforms for example. With this change, users can define their
own toolchain that is limited by execution platform instead of target
platform.
For example, if you're targeting an unsupported platform, but building
on a Mac, you can place the following in the local WORKSPACE:
register_toolchains("//ts/node_toolchain")
And the following in ts/node_toolchain/BUILD.bazel:
toolchain(
name = "node_toolchain",
exec_compatible_with = [
"@platforms//os:osx",
"@platforms//cpu:x86_64",
],
toolchain = "@nodejs_darwin_amd64_config//:toolchain",
toolchain_type = "@build_bazel_rules_nodejs//toolchains/node:toolchain_type",
)
Closes bazel-contrib#2565
|
From digging through the sources a bit more, my guess is the current target-bound behaviour is intended to support being able to create nodejs_binary()s that will run on a different architecture to the host machine. #2591 makes it fairly easy to support cross compilation, so I'm happy to close this one. |
…2591) Enables rules_nodejs to be used in cross-compilation contexts, where the target architecture is not supported by rules_nodejs. rules_nodejs's default toolchains are limited via target_compatible_with, which means they will not work when cross-compiling to Android or iOS with --platforms for example. With this change, users can define their own toolchain that is limited by execution platform instead of target platform. For example, if you're targeting an unsupported platform, but building on a Mac, you can place the following in the local WORKSPACE: register_toolchains("//ts/node_toolchain") And the following in ts/node_toolchain/BUILD.bazel: toolchain( name = "node_toolchain", exec_compatible_with = [ "@platforms//os:osx", "@platforms//cpu:x86_64", ], toolchain = "@nodejs_darwin_amd64_config//:toolchain", toolchain_type = "@build_bazel_rules_nodejs//toolchains/node:toolchain_type", ) Closes #2565
Currently rules_nodejs can't be used when cross-compiling to a different
platform, if that platform does not have a node toolchain available.
This means if you want to cross-compile a project that includes JS
files, you need to do it in multiple steps:
the generated JS files in
The fact that it's currently set to target makes me wonder if this
use case was just not considered at the time, or whether there's some
other workflow that changing to exec will break. Thoughts?
PR Checklist
Please check if your PR fulfills the following requirements: