Skip to content

Commit

Permalink
fix(builtin): remove unnecessary loader script
Browse files Browse the repository at this point in the history
  • Loading branch information
kormide committed Jun 28, 2022
1 parent 12a768b commit 2de3b8a
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 87 deletions.
2 changes: 0 additions & 2 deletions internal/node/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package(default_visibility = ["//visibility:public"])
exports_files([
"node_patches.cjs",
"require_patch.cjs",
"loader.cjs",
])

bzl_library(
Expand All @@ -40,7 +39,6 @@ exports_files([
"node.bzl", # Exported to be consumed for generating stardoc.
"node_repositories.bzl", # Exported to be consumed for generating stardoc.
"launcher.sh",
"loader.cjs",
])

filegroup(
Expand Down
39 changes: 19 additions & 20 deletions internal/node/launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,11 @@ for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
--bazel_capture_exit_code=*) EXIT_CODE_CAPTURE="${PWD}/${ARG#--bazel_capture_exit_code=}" ;;
# Outputs nothing on success
--bazel_silent_on_success=*) SILENT_ON_SUCCESS=true ;;
# Disable the node_loader.cjs monkey patches for require()
# Disable the require_patch.cjs monkey patches for require()
# Note that this means you need an explicit runfiles helper library
# This flag is now a no-op since the default is also false
--nobazel_patch_module_resolver) PATCH_REQUIRE=false ;;
# Enable the node_loader.cjs monkey patches for require()
# Enable the require_patch.cjs monkey patches for require()
--bazel_patch_module_resolver) PATCH_REQUIRE=true ;;
# Disable the --require node-patches (undocumented and unused; only here as an escape value)
--nobazel_node_patches) NODE_PATCHES=false ;;
Expand Down Expand Up @@ -331,26 +331,24 @@ if [ "$PATCH_REQUIRE" = true ]; then
* ) require_patch_script="${PWD}/${require_patch_script}" ;;
esac
LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" )
# Change the entry point to be the loader.cjs script so we run code before node
MAIN=$(rlocation "TEMPLATED_loader_script")
else
# Entry point is the user-supplied script
MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
# TODO: after we link-all-bins we should not need this extra lookup
if [[ ! -e "$MAIN" ]]; then
if [ "$FROM_EXECROOT" = true ]; then
MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path
else
MAIN=TEMPLATED_entry_point_manifest_path
fi
fi
# Always set up source-map-support using our vendored copy, just like the require_patch_script
register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js)
LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" )
if [[ -n "TEMPLATED_entry_point_main" ]]; then
MAIN="${MAIN}/"TEMPLATED_entry_point_main
fi

# Entry point is the user-supplied script
MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
# TODO: after we link-all-bins we should not need this extra lookup
if [[ ! -e "$MAIN" ]]; then
if [ "$FROM_EXECROOT" = true ]; then
MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path
else
MAIN=TEMPLATED_entry_point_manifest_path
fi
fi
# Always set up source-map-support using our vendored copy, just like the require_patch_script
register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js)
LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" )
if [[ -n "TEMPLATED_entry_point_main" ]]; then
MAIN="${MAIN}/"TEMPLATED_entry_point_main
fi

if [ "${SILENT_ON_SUCCESS:-}" = true ]; then
if [[ -z "${STDOUT_CAPTURE}" ]]; then
Expand Down Expand Up @@ -484,3 +482,4 @@ if [[ -n "${EXIT_CODE_CAPTURE}" ]]; then
else
exit ${RESULT}
fi

39 changes: 0 additions & 39 deletions internal/node/loader.cjs

This file was deleted.

25 changes: 0 additions & 25 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,6 @@ def _get_entry_point_file(ctx):
return ctx.attr.entry_point[DirectoryFilePathInfo].directory
fail("entry_point must either be a file, or provide DirectoryFilePathInfo")

def _write_loader_script(ctx):
substitutions = {}
substitutions["TEMPLATED_entry_point_path"] = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx)))
if DirectoryFilePathInfo in ctx.attr.entry_point:
substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[DirectoryFilePathInfo].path
else:
substitutions["TEMPLATED_entry_point_main"] = ""

ctx.actions.expand_template(
template = ctx.file._loader_template,
output = ctx.outputs.loader_script,
substitutions = substitutions,
is_executable = True,
)

# Avoid using non-normalized paths (workspace/../other_workspace/path)
def _to_manifest_path(ctx, file):
if file.short_path.startswith("../"):
Expand Down Expand Up @@ -208,8 +193,6 @@ def _nodejs_binary_impl(ctx, data = [], runfiles = [], expanded_args = []):
node_modules_root = "build_bazel_rules_nodejs/node_modules"
_write_require_patch_script(ctx, data, node_modules_root)

_write_loader_script(ctx)

# Provide the target name as an environment variable avaiable to all actions for the
# runfiles helpers to use.
env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
Expand Down Expand Up @@ -276,7 +259,6 @@ fi
runfiles = runfiles[:]
runfiles.extend(node_tool_files)
runfiles.extend(ctx.files._bash_runfile_helper)
runfiles.append(ctx.outputs.loader_script)
runfiles.append(ctx.outputs.require_patch_script)

# First replace any instances of "$(rlocation " with "$$(rlocation " to preserve
Expand Down Expand Up @@ -331,7 +313,6 @@ if (process.cwd() !== __dirname) {
"TEMPLATED_expected_exit_code": str(expected_exit_code),
"TEMPLATED_lcov_merger_script": _to_manifest_path(ctx, ctx.file._lcov_merger_script),
"TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
"TEMPLATED_loader_script": _to_manifest_path(ctx, ctx.outputs.loader_script),
"TEMPLATED_modules_manifest": _to_manifest_path(ctx, node_modules_manifest),
"TEMPLATED_node_patches_script": _to_manifest_path(ctx, ctx.file._node_patches_script),
"TEMPLATED_require_patch_script": _to_manifest_path(ctx, ctx.outputs.require_patch_script),
Expand Down Expand Up @@ -378,7 +359,6 @@ if (process.cwd() !== __dirname) {
runfiles = ctx.runfiles(
transitive_files = depset(runfiles),
files = node_tool_files + [
ctx.outputs.loader_script,
ctx.outputs.require_patch_script,
] + ctx.files._source_map_support_files +

Expand Down Expand Up @@ -608,10 +588,6 @@ Predefined genrule variables are not supported in this context.
default = Label("//internal/linker:index.js"),
allow_single_file = True,
),
"_loader_template": attr.label(
default = Label("//internal/node:loader.cjs"),
allow_single_file = True,
),
"toolchain": attr.label(),
"_node_args": attr.label(default = "@rules_nodejs//nodejs:default_args"),
"_node_patches_script": attr.label(
Expand Down Expand Up @@ -642,7 +618,6 @@ Predefined genrule variables are not supported in this context.

_NODEJS_EXECUTABLE_OUTPUTS = {
"launcher_sh": "%{name}.sh",
"loader_script": "%{name}_loader.cjs",
"require_patch_script": "%{name}_require_patch.cjs",
}

Expand Down
15 changes: 14 additions & 1 deletion internal/node/test/esm/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "npm_package_bin")

nodejs_binary(
name = "has_deps",
Expand All @@ -12,3 +12,16 @@ nodejs_binary(
name = "no_deps",
entry_point = ":no-deps.mjs",
)

nodejs_binary(
name = "no_deps_linker_disabled",
entry_point = ":no-deps-create-file.mjs",
templated_args = ["--nobazel_run_linker"],
)

npm_package_bin(
name = "run_with_linker_disabled",
outs = ["out.txt"],
args = ["$@"],
tool = ":no_deps_linker_disabled",
)
4 changes: 4 additions & 0 deletions internal/node/test/esm/no-deps-create-file.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import {writeFileSync} from "fs";

const outputFile = process.argv[2];
writeFileSync(outputFile, "foobar\n");

0 comments on commit 2de3b8a

Please sign in to comment.