From 85390ce88465e18c1c5d2f8d7f6ed21f3e8e8678 Mon Sep 17 00:00:00 2001 From: Conrad Burchert Date: Wed, 12 Feb 2025 19:51:50 +0100 Subject: [PATCH] Bazel: Pass nodejs binary path as an environment variable. (#1518) The current way to derive the binary path relies on a specific name for the nodejs repository. This blocks migrating to bzlmod, as bzlmod prefixes repository directories with the module name that created them. By asking bazel for the path instead we always get the correct path, so we can work with both bzlmod and WORKSPACE based dependencies at the same time. The repository @nodejs, used in the build label, refers to nodejs for the host platform and is generated by the following macro (wasn't too obvious to me): https://github.com/bazel-contrib/rules_nodejs/blob/d19d695275c5fd0d5224ddc7826d9f7f4b8186df/nodejs/repositories.bzl#L452 This is some work towards solving #1509. --- bazel/emscripten_toolchain/BUILD.bazel | 1 + bazel/emscripten_toolchain/default_config | 13 +++---------- bazel/emscripten_toolchain/toolchain.bzl | 6 ++++++ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bazel/emscripten_toolchain/BUILD.bazel b/bazel/emscripten_toolchain/BUILD.bazel index cdbaa8c..a989450 100644 --- a/bazel/emscripten_toolchain/BUILD.bazel +++ b/bazel/emscripten_toolchain/BUILD.bazel @@ -62,6 +62,7 @@ emscripten_cc_toolchain_config_rule( cpu = "wasm", em_config = "@emscripten_cache//:emscripten_config", emscripten_binaries = "@emsdk//:compiler_files", + nodejs_bin = "@nodejs//:node", script_extension = select({ "@bazel_tools//src/conditions:host_windows": "bat", "//conditions:default": "sh", diff --git a/bazel/emscripten_toolchain/default_config b/bazel/emscripten_toolchain/default_config index 648a8fe..8107fe4 100644 --- a/bazel/emscripten_toolchain/default_config +++ b/bazel/emscripten_toolchain/default_config @@ -5,19 +5,12 @@ ROOT_DIR = os.environ["ROOT_DIR"] EMSCRIPTEN_ROOT = os.environ["EMSCRIPTEN"] BINARYEN_ROOT = os.path.join(ROOT_DIR, os.environ["EM_BIN_PATH"]) LLVM_ROOT = os.path.join(BINARYEN_ROOT, "bin") +NODE_JS = os.path.join(ROOT_DIR, os.environ["NODE_JS_PATH"]) FROZEN_CACHE = True -system = platform.system() - -machine = "arm64" if platform.machine() in ('arm64', 'aarch64') else "amd64" -nodejs_binary = "bin/nodejs/node.exe" if(system =="Windows") else "bin/node" -NODE_JS = ROOT_DIR + "/external/nodejs_{}_{}/{}".format(system.lower(), machine, nodejs_binary) - - # This works around an issue with Bazel RBE where the symlinks in node_modules/.bin # are uploaded as the linked files, which means the cli.js cannot load its # dependencies from the expected locations. # See https://github.com/emscripten-core/emscripten/pull/16640 for more -if system != "Windows": - CLOSURE_COMPILER = [NODE_JS, os.path.join(EMSCRIPTEN_ROOT, "node_modules", - "google-closure-compiler", "cli.js")] +CLOSURE_COMPILER = [NODE_JS, os.path.join(EMSCRIPTEN_ROOT, "node_modules", + "google-closure-compiler", "cli.js")] diff --git a/bazel/emscripten_toolchain/toolchain.bzl b/bazel/emscripten_toolchain/toolchain.bzl index 8c724d2..c8cec07 100644 --- a/bazel/emscripten_toolchain/toolchain.bzl +++ b/bazel/emscripten_toolchain/toolchain.bzl @@ -71,6 +71,7 @@ def _impl(ctx): cc_target_os = "emscripten" emscripten_dir = ctx.attr.emscripten_binaries.label.workspace_root + nodejs_path = ctx.file.nodejs_bin.path builtin_sysroot = emscripten_dir + "/emscripten/cache/sysroot" @@ -1060,6 +1061,10 @@ def _impl(ctx): key = "EM_CONFIG_PATH", value = ctx.file.em_config.path, ), + env_entry( + key = "NODE_JS_PATH", + value = nodejs_path, + ), ], ), # Use llvm backend. Off by default, enabled via --features=llvm_backend @@ -1134,6 +1139,7 @@ emscripten_cc_toolchain_config_rule = rule( "cpu": attr.string(mandatory = True, values = ["asmjs", "wasm"]), "em_config": attr.label(mandatory = True, allow_single_file = True), "emscripten_binaries": attr.label(mandatory = True, cfg = "exec"), + "nodejs_bin": attr.label(mandatory = True, allow_single_file = True), "script_extension": attr.string(mandatory = True, values = ["sh", "bat"]), }, provides = [CcToolchainConfigInfo],