Skip to content

Commit

Permalink
fix: fix nodejs_binary cross-platform RBE issue bazel-contrib#1305
Browse files Browse the repository at this point in the history
* node_launcher.sh now picks the node binary at runtime instead of it being a templated arg evaluated at build time
* node_binary now adds the correct node tool_files to the nodejs_binary runfiles

Fixes bazel-contrib#1305
  • Loading branch information
gregmagolan committed Nov 1, 2019
1 parent e7d15c7 commit ccf3f0b
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 118 deletions.
40 changes: 27 additions & 13 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ def _to_execroot_path(ctx, file):

return ("<ERROR> _to_execroot_path not yet implemented for " + file.path)

def _strip_external(path):
return path[len("external/"):] if path.startswith("external/") or path.startswith("external\\") else path

def _nodejs_binary_impl(ctx):
node_modules = depset(ctx.files.node_modules)

Expand Down Expand Up @@ -173,18 +176,11 @@ def _nodejs_binary_impl(ctx):
if hasattr(ctx.attr, "expected_exit_code"):
expected_exit_code = ctx.attr.expected_exit_code

node_tool_info = ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo

# Make a copy so we don't try to mutate a frozen object
node_tool_files = node_tool_info.tool_files[:]
if node_tool_info.target_tool_path == "":
# If tool_path is empty and tool_target is None then there is no local
# node tool, we will just print a nice error message if the user
# attempts to do bazel run
fail("The node toolchain was not properly configured so %s cannot be executed. Make sure that target_tool_path or target_tool is set." % ctx.attr.name)

node_tool_files.append(ctx.file._link_modules_script)
node_tool_files.append(ctx.file._bazel_require_script)
node_tool_files = [
ctx.file._link_modules_script,
ctx.file._bazel_require_script,
]
node_tool_files.extend(ctx.files._node)

if not ctx.outputs.templated_args_file:
templated_args = ctx.attr.templated_args
Expand Down Expand Up @@ -221,7 +217,9 @@ def _nodejs_binary_impl(ctx):
"TEMPLATED_expected_exit_code": str(expected_exit_code),
"TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
"TEMPLATED_loader_path": script_path,
"TEMPLATED_node": node_tool_info.target_tool_path,
"TEMPLATED_node_darwin": _strip_external(ctx.file._node_darwin.path),
"TEMPLATED_node_linux": _strip_external(ctx.file._node_linux.path),
"TEMPLATED_node_windows": _strip_external(ctx.file._node_windows.path),
"TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
"TEMPLATED_script_path": _to_execroot_path(ctx, ctx.file.entry_point),
}
Expand Down Expand Up @@ -466,6 +464,22 @@ jasmine_node_test(
default = Label("//internal/node:node_loader.js"),
allow_single_file = True,
),
"_node": attr.label(
default = Label("@nodejs//:node_bin"),
allow_single_file = True,
),
"_node_darwin": attr.label(
default = Label("@nodejs_darwin_amd64//:node_bin"),
allow_single_file = True,
),
"_node_linux": attr.label(
default = Label("@nodejs_linux_amd64//:node_bin"),
allow_single_file = True,
),
"_node_windows": attr.label(
default = Label("@nodejs_windows_amd64//:node_bin"),
allow_single_file = True,
),
"_repository_args": attr.label(
default = Label("@nodejs//:bin/node_repo_args.sh"),
allow_single_file = True,
Expand Down
19 changes: 18 additions & 1 deletion internal/node/node_launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,24 @@ TEMPLATED_env_vars
# This redirects to stderr so it doesn't interfere with Bazel's worker protocol
# find . -name thingImLookingFor 1>&2

readonly node=$(rlocation "TEMPLATED_node")
# Check environment
unameOut="$(uname -s)"
case "${unameOut}" in
Linux*) machine=linux;;
Darwin*) machine=darwin;;
CYGWIN*) machine=windows;;
MINGW*) machine=windows;;
MSYS_NT*) machine=windows;;
*) machine=linux;;
esac

case "${machine}" in
linux) readonly node=$(rlocation "TEMPLATED_node_linux");;
darwin) readonly node=$(rlocation "TEMPLATED_node_darwin");;
windows) readonly node=$(rlocation "TEMPLATED_node_windows");;
*) readonly node=$(rlocation "TEMPLATED_node_linux");;
esac

readonly repository_args=$(rlocation "TEMPLATED_repository_args")
MAIN=$(rlocation "TEMPLATED_loader_path")
readonly link_modules_script=$(rlocation "TEMPLATED_link_modules_script")
Expand Down
3 changes: 0 additions & 3 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@ load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test", "n
load("//internal/js_library:js_library.bzl", "js_library")
load("//third_party/github.com/bazelbuild/bazel-skylib:rules/copy_file.bzl", "copy_file")
load("//third_party/github.com/bazelbuild/bazel-skylib:rules/write_file.bzl", "write_file")
load(":nodejs_toolchain_test.bzl", "nodejs_binary_test_suite")

nodejs_binary_test_suite()

# You can have a nodejs_binary with no node_modules attribute
# and no fine grained deps
Expand Down
101 changes: 0 additions & 101 deletions internal/node/test/nodejs_toolchain_test.bzl

This file was deleted.

0 comments on commit ccf3f0b

Please sign in to comment.