Skip to content

Commit

Permalink
fix(builtin): when using chdir attribute, don't write to source dir
Browse files Browse the repository at this point in the history
creating a file in the source dir leaves a mess when not running in an execroot
  • Loading branch information
Alex Eagle authored and alexeagle committed Jan 25, 2021
1 parent 1639ecf commit 3eb4260
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 20 deletions.
14 changes: 6 additions & 8 deletions internal/node/launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ if [ "$NODE_PATCHES" = true ]; then
# Absolute path on Windows, e.g. C:/path/to/thing
[a-zA-Z]:/* ) ;;
# Otherwise it needs to be made relative
* ) node_patches_script="./${node_patches_script}" ;;
* ) node_patches_script="${PWD}/${node_patches_script}" ;;
esac
LAUNCHER_NODE_OPTIONS+=( "--require" "$node_patches_script" )
fi
Expand Down Expand Up @@ -286,14 +286,14 @@ if [ "$PATCH_REQUIRE" = true ]; then
# 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}" ;;
* ) require_patch_script="${PWD}/${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
MAIN="${PWD}/"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
Expand All @@ -304,11 +304,6 @@ else
fi
fi

if [[ -n "$NODE_WORKING_DIR" ]]; then
echo "process.chdir(__dirname)" > "$NODE_WORKING_DIR/__chdir.js"
LAUNCHER_NODE_OPTIONS+=( "--require" "./$NODE_WORKING_DIR/__chdir.js" )
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 All @@ -334,6 +329,9 @@ _int() {
}

# Execute the main program
if [[ -n "$NODE_WORKING_DIR" ]]; then
cd "$NODE_WORKING_DIR"
fi
set +e

if [[ -n "${STDOUT_CAPTURE}" ]] && [[ -n "${STDERR_CAPTURE}" ]]; then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,27 +68,38 @@
# cat "$(rlocation my_workspace/path/to/my/data.txt)"
#

if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
if [[ -f "$0.runfiles_manifest" ]]; then
export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
elif [[ -f "$0.runfiles/MANIFEST" ]]; then
export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
elif [[ -f "$0.runfiles/build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel/tools/bash/runfiles/runfiles.bash" ]]; then
export RUNFILES_DIR="$0.runfiles"
fi
fi

case "$(uname -s | tr [:upper:] [:lower:])" in
msys*|mingw*|cygwin*)
# matches an absolute Windows path
export _RLOCATION_ISABS_PATTERN="^[a-zA-Z]:[/\\]"
;;
*)
# matches an absolute Unix path
export _RLOCATION_ISABS_PATTERN="^/[^/].*"
# rules_nodejs modification
# In the upstream this pattern requires a second character which is not a slash
# https://github.com/bazelbuild/bazel/blob/22d376cf41d50bfee129a0a7fa656d66af2dbf14/tools/bash/runfiles/runfiles.bash#L88
# However in integration testing with rules_docker we observe runfiles path starting with two slashes
# This fails on Linux CI (not Mac or Windows) in our //e2e:e2e_nodejs_image:
# ERROR[runfiles.bash]: cannot look up runfile "nodejs_linux_amd64/bin/nodejs/bin/node"
# (RUNFILES_DIR="/app/main.runfiles/e2e_nodejs_image//app//main.runfiles", RUNFILES_MANIFEST_FILE="")
export _RLOCATION_ISABS_PATTERN="^/.*"
;;
esac

if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
if [[ -f "$0.runfiles_manifest" ]]; then
export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
elif [[ -f "$0.runfiles/MANIFEST" ]]; then
export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
elif [[ -f "$0.runfiles/build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel/tools/bash/runfiles/runfiles.bash" ]]; then
if [[ "${0}" =~ $_RLOCATION_ISABS_PATTERN ]]; then
export RUNFILES_DIR="${0}.runfiles"
else
export RUNFILES_DIR="${PWD}/${0}.runfiles"
fi
fi
fi

# --- begin rules_nodejs custom code ---
# normpath() function removes all `/./` and `dir/..` sequences from a path
function normpath() {
Expand Down Expand Up @@ -223,4 +234,4 @@ function runfiles_export_envvars() {
fi
fi
}
export -f runfiles_export_envvars
export -f runfiles_export_envvars

0 comments on commit 3eb4260

Please sign in to comment.