Skip to content

Commit

Permalink
feat: allow running NPM tools from execroot (#2297)
Browse files Browse the repository at this point in the history
NPM tools are currently run out of external/{md}/node_modules. If the
tool loads user-provided files in the execroot, which is the common
case, then this can lead to bad interactions.

Two examples of bad interactions are:
  1. A tool that loads React and the user code too,
     hooks becoming unusable.
  2. A tool that calls into a bundler like Webpack,
     potentially bundling duplicate packages.

Signed-off-by: Duarte Nunes <duarte@hey.com>
  • Loading branch information
duarten authored and Alex Eagle committed Dec 18, 2020
1 parent a549411 commit 2a4ba8f
Showing 1 changed file with 33 additions and 26 deletions.
59 changes: 33 additions & 26 deletions internal/node/launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ EXIT_CODE_CAPTURE=""
RUN_LINKER=true
NODE_PATCHES=true
PATCH_REQUIRE=false
FROM_EXECROOT=false
for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
case "$ARG" in
# Supply custom linker arguments for first-party dependencies
Expand All @@ -202,6 +203,8 @@ for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
# It also enables the --bazel_patch_module_resolver flag, as either the linker or require() patch
# is needed for resolving third-party node modules.
--nobazel_run_linker) RUN_LINKER=false PATCH_REQUIRE=true ;;
# If running an NPM package, run it from execroot instead of from external
--bazel_run_from_execroot) FROM_EXECROOT=true ;;
# Let users pass through arguments to node itself
--node_options=*) USER_NODE_OPTIONS+=( "${ARG#--node_options=}" ) ;;
# Remaining argv is collected to pass to the program
Expand Down Expand Up @@ -235,29 +238,6 @@ if [ "$NODE_PATCHES" = true ]; then
LAUNCHER_NODE_OPTIONS+=( "--require" "$node_patches_script" )
fi

if [ "$PATCH_REQUIRE" = true ]; then
require_patch_script=${BAZEL_NODE_PATCH_REQUIRE}
# See comment above
case "${require_patch_script}" in
# Absolute path on unix
/* ) ;;
# Absolute path on Windows, e.g. C:/path/to/thing
[a-zA-Z]:/* ) ;;
# Otherwise it needs to be made relative
* ) require_patch_script="./${require_patch_script}" ;;
esac
LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" )
# Change the entry point to be the loader.js script so we run code before node
MAIN=$(rlocation "TEMPLATED_loader_script")
else
# Entry point is the user-supplied script
MAIN=TEMPLATED_entry_point_execroot_path
# TODO: after we link-all-bins we should not need this extra lookup
if [[ ! -f "$MAIN" ]]; then
MAIN=TEMPLATED_entry_point_manifest_path
fi
fi

# Tell the node_patches_script that programs should not escape the execroot
# Bazel always sets the PWD to execroot/my_wksp so we go up one directory.
export BAZEL_PATCH_ROOT=$(dirname $PWD)
Expand All @@ -278,14 +258,14 @@ if [[ "$PWD" == *"/bazel-out/"* ]]; then
echo "No 'bazel-out' folder found in path '${PWD}'!"
exit 1
fi
readonly execroot=${PWD:0:${index}}
export BAZEL_PATCH_GUARDS="${execroot}/node_modules"
EXECROOT=${PWD:0:${index}}
else
# We are in execroot or in some other context all together such as a nodejs_image or a manually
# run nodejs_binary. If this is execroot then linker node_modules is in the PWD. If this another
# context then it is safe to assume the node_modules are there and guard that directory if it exists.
export BAZEL_PATCH_GUARDS="${PWD}/node_modules"
EXECROOT=${PWD}
fi
export BAZEL_PATCH_GUARDS="${EXECROOT}/node_modules"
if [[ -n "${BAZEL_NODE_MODULES_ROOT:-}" ]]; then
if [[ "${BAZEL_NODE_MODULES_ROOT}" != "${BAZEL_WORKSPACE}/node_modules" ]]; then
# If BAZEL_NODE_MODULES_ROOT is set and it is not , add it to the list of bazel patch guards
Expand All @@ -295,6 +275,33 @@ if [[ -n "${BAZEL_NODE_MODULES_ROOT:-}" ]]; then
fi
fi

if [ "$PATCH_REQUIRE" = true ]; then
require_patch_script=${BAZEL_NODE_PATCH_REQUIRE}
# See comment above
case "${require_patch_script}" in
# Absolute path on unix
/* ) ;;
# Absolute path on Windows, e.g. C:/path/to/thing
[a-zA-Z]:/* ) ;;
# Otherwise it needs to be made relative
* ) require_patch_script="./${require_patch_script}" ;;
esac
LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" )
# Change the entry point to be the loader.js script so we run code before node
MAIN=$(rlocation "TEMPLATED_loader_script")
else
# Entry point is the user-supplied script
MAIN=TEMPLATED_entry_point_execroot_path
# TODO: after we link-all-bins we should not need this extra lookup
if [[ ! -f "$MAIN" ]]; then
if [ "$FROM_EXECROOT" = true ]; then
MAIN="$EXECROOT/$MAIN"
else
MAIN=TEMPLATED_entry_point_manifest_path
fi
fi
fi

# The EXPECTED_EXIT_CODE lets us write bazel tests which assert that
# a binary fails to run. Otherwise any failure would make such a test
# fail before we could assert that we expected that failure.
Expand Down

0 comments on commit 2a4ba8f

Please sign in to comment.