Skip to content

Commit 3eb4260

Browse files
Alex Eaglealexeagle
authored andcommitted
fix(builtin): when using chdir attribute, don't write to source dir
creating a file in the source dir leaves a mess when not running in an execroot
1 parent 1639ecf commit 3eb4260

File tree

2 files changed

+29
-20
lines changed

2 files changed

+29
-20
lines changed

internal/node/launcher.sh

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ if [ "$NODE_PATCHES" = true ]; then
235235
# Absolute path on Windows, e.g. C:/path/to/thing
236236
[a-zA-Z]:/* ) ;;
237237
# Otherwise it needs to be made relative
238-
* ) node_patches_script="./${node_patches_script}" ;;
238+
* ) node_patches_script="${PWD}/${node_patches_script}" ;;
239239
esac
240240
LAUNCHER_NODE_OPTIONS+=( "--require" "$node_patches_script" )
241241
fi
@@ -286,14 +286,14 @@ if [ "$PATCH_REQUIRE" = true ]; then
286286
# Absolute path on Windows, e.g. C:/path/to/thing
287287
[a-zA-Z]:/* ) ;;
288288
# Otherwise it needs to be made relative
289-
* ) require_patch_script="./${require_patch_script}" ;;
289+
* ) require_patch_script="${PWD}/${require_patch_script}" ;;
290290
esac
291291
LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" )
292292
# Change the entry point to be the loader.js script so we run code before node
293293
MAIN=$(rlocation "TEMPLATED_loader_script")
294294
else
295295
# Entry point is the user-supplied script
296-
MAIN=TEMPLATED_entry_point_execroot_path
296+
MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
297297
# TODO: after we link-all-bins we should not need this extra lookup
298298
if [[ ! -f "$MAIN" ]]; then
299299
if [ "$FROM_EXECROOT" = true ]; then
@@ -304,11 +304,6 @@ else
304304
fi
305305
fi
306306

307-
if [[ -n "$NODE_WORKING_DIR" ]]; then
308-
echo "process.chdir(__dirname)" > "$NODE_WORKING_DIR/__chdir.js"
309-
LAUNCHER_NODE_OPTIONS+=( "--require" "./$NODE_WORKING_DIR/__chdir.js" )
310-
fi
311-
312307
# The EXPECTED_EXIT_CODE lets us write bazel tests which assert that
313308
# a binary fails to run. Otherwise any failure would make such a test
314309
# fail before we could assert that we expected that failure.
@@ -334,6 +329,9 @@ _int() {
334329
}
335330

336331
# Execute the main program
332+
if [[ -n "$NODE_WORKING_DIR" ]]; then
333+
cd "$NODE_WORKING_DIR"
334+
fi
337335
set +e
338336

339337
if [[ -n "${STDOUT_CAPTURE}" ]] && [[ -n "${STDERR_CAPTURE}" ]]; then

third_party/github.com/bazelbuild/bazel/tools/bash/runfiles/runfiles.bash

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,27 +68,38 @@
6868
# cat "$(rlocation my_workspace/path/to/my/data.txt)"
6969
#
7070

71-
if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
72-
if [[ -f "$0.runfiles_manifest" ]]; then
73-
export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
74-
elif [[ -f "$0.runfiles/MANIFEST" ]]; then
75-
export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
76-
elif [[ -f "$0.runfiles/build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel/tools/bash/runfiles/runfiles.bash" ]]; then
77-
export RUNFILES_DIR="$0.runfiles"
78-
fi
79-
fi
80-
8171
case "$(uname -s | tr [:upper:] [:lower:])" in
8272
msys*|mingw*|cygwin*)
8373
# matches an absolute Windows path
8474
export _RLOCATION_ISABS_PATTERN="^[a-zA-Z]:[/\\]"
8575
;;
8676
*)
8777
# matches an absolute Unix path
88-
export _RLOCATION_ISABS_PATTERN="^/[^/].*"
78+
# rules_nodejs modification
79+
# In the upstream this pattern requires a second character which is not a slash
80+
# https://github.com/bazelbuild/bazel/blob/22d376cf41d50bfee129a0a7fa656d66af2dbf14/tools/bash/runfiles/runfiles.bash#L88
81+
# However in integration testing with rules_docker we observe runfiles path starting with two slashes
82+
# This fails on Linux CI (not Mac or Windows) in our //e2e:e2e_nodejs_image:
83+
# ERROR[runfiles.bash]: cannot look up runfile "nodejs_linux_amd64/bin/nodejs/bin/node"
84+
# (RUNFILES_DIR="/app/main.runfiles/e2e_nodejs_image//app//main.runfiles", RUNFILES_MANIFEST_FILE="")
85+
export _RLOCATION_ISABS_PATTERN="^/.*"
8986
;;
9087
esac
9188

89+
if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
90+
if [[ -f "$0.runfiles_manifest" ]]; then
91+
export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
92+
elif [[ -f "$0.runfiles/MANIFEST" ]]; then
93+
export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
94+
elif [[ -f "$0.runfiles/build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel/tools/bash/runfiles/runfiles.bash" ]]; then
95+
if [[ "${0}" =~ $_RLOCATION_ISABS_PATTERN ]]; then
96+
export RUNFILES_DIR="${0}.runfiles"
97+
else
98+
export RUNFILES_DIR="${PWD}/${0}.runfiles"
99+
fi
100+
fi
101+
fi
102+
92103
# --- begin rules_nodejs custom code ---
93104
# normpath() function removes all `/./` and `dir/..` sequences from a path
94105
function normpath() {
@@ -223,4 +234,4 @@ function runfiles_export_envvars() {
223234
fi
224235
fi
225236
}
226-
export -f runfiles_export_envvars
237+
export -f runfiles_export_envvars

0 commit comments

Comments
 (0)