Skip to content

Commit 58ce112

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Fix Bash runfiles calling repository lookup for directly run scripts
When an `sh_binary` is run directly from `bazel-bin`, its path won't be contained in the manifest and will not match a `bazel-out` regex. Instead, make the path absolute and then match after the `bazel-bin` to extract the binaries repository name. Also improve the error message when `runfiles_current_repository` can find neither the runfiles manifest nor the runfiles directory. Work towards fixing #19796 (comment) Closes #19810. PiperOrigin-RevId: 574208158 Change-Id: I45d6b503e8f34e13177d57ca64c202640306cfb8
1 parent bbc68d2 commit 58ce112

2 files changed

Lines changed: 102 additions & 5 deletions

File tree

‎src/test/shell/bazel/bazel_rules_test.sh‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ case "$(uname -s | tr [:upper:] [:lower:])" in
5151
msys*)
5252
# As of 2019-01-15, Bazel on Windows only supports MSYS Bash.
5353
declare -r is_windows=true
54+
declare -r exe_suffix=.exe
5455
;;
5556
*)
5657
declare -r is_windows=false
58+
declare -r exe_suffix=
5759
;;
5860
esac
5961

@@ -937,6 +939,13 @@ EOF
937939
chmod +x other_repo/pkg/test.sh
938940
}
939941

942+
# Runs the given command with all runfiles environment variables removed from
943+
# the environment. Also enables debug logging for the runfiles library.
944+
function clean_runfiles_run() {
945+
env -u RUNFILES_DIR -u RUNFILES_MANIFEST_FILE -u RUNFILES_MANIFEST_ONLY \
946+
RUNFILES_LIB_DEBUG=1 "$@"
947+
}
948+
940949
function test_bash_runfiles_current_repository_binary_enable_runfiles() {
941950
setup_bash_runfiles_current_repository
942951

@@ -953,6 +962,26 @@ function test_bash_runfiles_current_repository_binary_enable_runfiles() {
953962
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
954963
}
955964

965+
function test_bash_runfiles_current_repository_binary_enable_runfiles_direct_run() {
966+
setup_bash_runfiles_current_repository
967+
968+
bazel build --enable_bzlmod --enable_runfiles //pkg:binary \
969+
&>"$TEST_log" || fail "Build should succeed"
970+
clean_runfiles_run bazel-bin/pkg/binary$exe_suffix &>"$TEST_log" \
971+
|| fail "Direct run should succeed"
972+
expect_log "in pkg/binary.sh: ''"
973+
expect_log "in pkg/library.sh: ''"
974+
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
975+
976+
bazel run --enable_bzlmod --enable_runfiles @other_repo//pkg:binary \
977+
&>"$TEST_log" || fail "Run should succeed"
978+
clean_runfiles_run bazel-bin/external/other_repo/pkg/binary$exe_suffix \
979+
&>"$TEST_log" || fail "Direct run should succeed"
980+
expect_log "in external/other_repo/pkg/binary.sh: 'other_repo'"
981+
expect_log "in pkg/library.sh: ''"
982+
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
983+
}
984+
956985
function test_bash_runfiles_current_repository_test_enable_runfiles() {
957986
setup_bash_runfiles_current_repository
958987

@@ -985,6 +1014,26 @@ function test_bash_runfiles_current_repository_binary_noenable_runfiles() {
9851014
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
9861015
}
9871016

1017+
function test_bash_runfiles_current_repository_binary_noenable_runfiles_direct_run() {
1018+
setup_bash_runfiles_current_repository
1019+
1020+
bazel build --enable_bzlmod --noenable_runfiles //pkg:binary \
1021+
&>"$TEST_log" || fail "Build should succeed"
1022+
clean_runfiles_run bazel-bin/pkg/binary$exe_suffix &>"$TEST_log" \
1023+
|| fail "Direct run should succeed"
1024+
expect_log "in pkg/binary.sh: ''"
1025+
expect_log "in pkg/library.sh: ''"
1026+
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
1027+
1028+
bazel run --enable_bzlmod --noenable_runfiles @other_repo//pkg:binary \
1029+
&>"$TEST_log" || fail "Run should succeed"
1030+
clean_runfiles_run bazel-bin/external/other_repo/pkg/binary$exe_suffix \
1031+
&>"$TEST_log" || fail "Direct run should succeed"
1032+
expect_log "in external/other_repo/pkg/binary.sh: 'other_repo'"
1033+
expect_log "in pkg/library.sh: ''"
1034+
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
1035+
}
1036+
9881037
function test_bash_runfiles_current_repository_test_noenable_runfiles() {
9891038
setup_bash_runfiles_current_repository
9901039

@@ -1017,6 +1066,26 @@ function test_bash_runfiles_current_repository_binary_nobuild_runfile_links() {
10171066
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
10181067
}
10191068

1069+
function test_bash_runfiles_current_repository_binary_nobuild_runfile_links_direct_run() {
1070+
setup_bash_runfiles_current_repository
1071+
1072+
bazel build --enable_bzlmod --nobuild_runfile_links //pkg:binary \
1073+
&>"$TEST_log" || fail "Build should succeed"
1074+
clean_runfiles_run bazel-bin/pkg/binary$exe_suffix &>"$TEST_log" \
1075+
|| fail "Direct run should succeed"
1076+
expect_log "in pkg/binary.sh: ''"
1077+
expect_log "in pkg/library.sh: ''"
1078+
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
1079+
1080+
bazel run --enable_bzlmod --nobuild_runfile_links @other_repo//pkg:binary \
1081+
&>"$TEST_log" || fail "Run should succeed"
1082+
clean_runfiles_run bazel-bin/external/other_repo/pkg/binary$exe_suffix \
1083+
&>"$TEST_log" || fail "Direct run should succeed"
1084+
expect_log "in external/other_repo/pkg/binary.sh: 'other_repo'"
1085+
expect_log "in pkg/library.sh: ''"
1086+
expect_log "in external/other_repo/pkg/library2.sh: 'other_repo'"
1087+
}
1088+
10201089
function test_bash_runfiles_current_repository_test_nobuild_runfile_links() {
10211090
setup_bash_runfiles_current_repository
10221091

‎tools/bash/runfiles/runfiles.bash‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,14 @@ export -f runfiles_export_envvars
227227
# its return value is ignored if passed to rlocation.
228228
function runfiles_current_repository() {
229229
local -r idx=${1:-1}
230-
local -r caller_path="${BASH_SOURCE[$idx]}"
230+
local -r raw_caller_path="${BASH_SOURCE[$idx]}"
231+
# Make the caller path absolute if needed to handle the case where the script is run directly
232+
# from bazel-bin, with working directory a subdirectory of bazel-bin.
233+
if [[ "$raw_caller_path" =~ $_RLOCATION_ISABS_PATTERN ]]; then
234+
local -r caller_path="$raw_caller_path"
235+
else
236+
local -r caller_path="$(cd $(dirname "$raw_caller_path"); pwd)/$(basename "$raw_caller_path")"
237+
fi
231238
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
232239
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): caller's path is ($caller_path)"
233240
fi
@@ -245,6 +252,19 @@ function runfiles_current_repository() {
245252
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
246253
echo >&2 "ERROR[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) is not the target of an entry in the runfiles manifest ($RUNFILES_MANIFEST_FILE)"
247254
fi
255+
# The binary may also be run directly from bazel-bin or bazel-out.
256+
local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '(^|/)(bazel-out/[^/]+/bin|bazel-bin)/external/[^/]+/' | tail -1 | rev | cut -d / -f 2 | rev)
257+
if [[ -n "$repository" ]]; then
258+
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
259+
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in repository ($repository) (parsed exec path)"
260+
fi
261+
echo "$repository"
262+
else
263+
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
264+
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in the main repository (parsed exec path)"
265+
fi
266+
echo ""
267+
fi
248268
return 1
249269
else
250270
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
@@ -273,16 +293,17 @@ function runfiles_current_repository() {
273293
fi
274294
# The only shell script that is not executed from the runfiles directory (if it is populated)
275295
# is the sh_binary entrypoint. Parse its path under the execroot, using the last match to
276-
# allow for nested execroots (e.g. in Bazel integration tests).
277-
local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '(^|/)bazel-out/[^/]+/bin/external/[^/]+/' | tail -1 | rev | cut -d / -f 2 | rev)
296+
# allow for nested execroots (e.g. in Bazel integration tests). The binary may also be run
297+
# directly from bazel-bin.
298+
local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '(^|/)(bazel-out/[^/]+/bin|bazel-bin)/external/[^/]+/' | tail -1 | rev | cut -d / -f 2 | rev)
278299
if [[ -n "$repository" ]]; then
279300
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
280-
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in repository ($repository)"
301+
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in repository ($repository) (parsed exec path)"
281302
fi
282303
echo "$repository"
283304
else
284305
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
285-
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in the main repository"
306+
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in the main repository (parsed exec path)"
286307
fi
287308
echo ""
288309
fi
@@ -294,6 +315,13 @@ function runfiles_current_repository() {
294315
fi
295316
fi
296317

318+
if [[ -z "$rlocation_path" ]]; then
319+
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
320+
echo >&2 "ERROR[runfiles.bash]: runfiles_current_repository($idx): cannot determine repository for ($caller_path) since neither the runfiles directory (${RUNFILES_DIR:-}) nor the runfiles manifest (${RUNFILES_MANIFEST_FILE:-}) exist"
321+
fi
322+
return 1
323+
fi
324+
297325
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
298326
echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($caller_path) corresponds to rlocation path ($rlocation_path)"
299327
fi

0 commit comments

Comments
 (0)