diff --git a/shell/runfiles/runfiles.bash b/shell/runfiles/runfiles.bash index b6bbdfe..efb9276 100644 --- a/shell/runfiles/runfiles.bash +++ b/shell/runfiles/runfiles.bash @@ -87,16 +87,36 @@ # You can skip steps 1 and 2 when setting "use_bash_launcher" attribute in sh_binary or sh_test. # +# The runfiles path of this library, which is also where the initialization snippet sources it from. +# Exported since it is read by the exported functions below. +export _RLOCATION_RUNFILES_LIB_PATH=bazel_tools/tools/bash/runfiles/runfiles.bash + 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/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + elif [[ -f "$0.runfiles/$_RLOCATION_RUNFILES_LIB_PATH" ]]; then export RUNFILES_DIR="$0.runfiles" fi fi +# _RLOCATION_USE_RUNFILES_DIR is set to 1 if the runfiles directory can be used to look up runfiles +# and to the empty string otherwise. It is exported for subprocesses that use the functions below +# without sourcing this library, but never read back from the environment: it is always determined +# here, so that it cannot contradict the RUNFILES_DIR it belongs to. +# +# Whether a runfiles directory is populated is a property of how the action or test is executed +# rather than of the build: with --noenable_runfiles, Bazel leaves behind a directory that contains +# only the MANIFEST file and the subdirectory of the main repository, but the sandbox and remote +# execution materialize it in full regardless. The presence of this library in the directory +# distinguishes the two cases as it is just an ordinary runfile. +if [[ -f "${RUNFILES_DIR:-/dev/null}/$_RLOCATION_RUNFILES_LIB_PATH" ]]; then + export _RLOCATION_USE_RUNFILES_DIR=1 +else + export _RLOCATION_USE_RUNFILES_DIR= +fi + case "$(uname -s | tr '[:upper:]' '[:lower:]')" in msys*|mingw*|cygwin*) # matches an absolute Windows path @@ -229,14 +249,18 @@ function runfiles_export_envvars() { export RUNFILES_MANIFEST_FILE= fi elif [[ ! -d "${RUNFILES_DIR:-/dev/null}" ]]; then + # Only pass on a runfiles directory that has been fully materialized so that + # subprocesses that aren't aware of this issue don't blindly use it. if [[ "$RUNFILES_MANIFEST_FILE" == */MANIFEST \ - && -d "${RUNFILES_MANIFEST_FILE%/MANIFEST}" ]]; then + && -f "${RUNFILES_MANIFEST_FILE%/MANIFEST}/$_RLOCATION_RUNFILES_LIB_PATH" ]]; then export RUNFILES_DIR="${RUNFILES_MANIFEST_FILE%/MANIFEST}" export JAVA_RUNFILES="$RUNFILES_DIR" + export _RLOCATION_USE_RUNFILES_DIR=1 elif [[ "$RUNFILES_MANIFEST_FILE" == *_manifest \ - && -d "${RUNFILES_MANIFEST_FILE%_manifest}" ]]; then + && -f "${RUNFILES_MANIFEST_FILE%_manifest}/$_RLOCATION_RUNFILES_LIB_PATH" ]]; then export RUNFILES_DIR="${RUNFILES_MANIFEST_FILE%_manifest}" export JAVA_RUNFILES="$RUNFILES_DIR" + export _RLOCATION_USE_RUNFILES_DIR=1 else export RUNFILES_DIR= fi @@ -267,43 +291,29 @@ function runfiles_current_repository() { fi local rlocation_path= + # Replace \ with / since the manifest uses / as the path separator even on Windows. + local normalized_caller_path + normalized_caller_path="$(echo "$caller_path" | sed 's|\\\\*|/|g')" - # If the runfiles manifest exists, search for an entry with target the caller's path. - if [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then - # Escape $caller_path for use in the grep regex below. Also replace \ with / since the manifest - # uses / as the path separator even on Windows. - local -r normalized_caller_path="$(echo "$caller_path" | sed 's|\\\\*|/|g')" + # The caller's path was returned by rlocation, so only the source of truth that rlocation uses can + # contain it: a path looked up in the runfiles directory lies in that directory and is never the + # target of a manifest entry, which is the location of the original file. + if [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" && -z "$_RLOCATION_USE_RUNFILES_DIR" ]]; then + # Search for an entry whose target is the caller's path. local -r escaped_caller_path="$(__runfiles_escape_grep "$normalized_caller_path")" rlocation_path=$(__runfiles_maybe_grep -m1 "^[^ ]* ${escaped_caller_path}$" "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 1) if [[ -z "$rlocation_path" ]]; then if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - 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)" - fi - # The binary may also be run directly from bazel-bin or bazel-out. - local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '(^|/)(bazel-out/[^/]+/bin|bazel-bin)/external/[^/]+/' | tail -1 | awk -F/ '{print $(NF-1)}') - if [[ -n "$repository" ]]; then - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in repository ($repository) (parsed exec path)" - fi - echo "$repository" - else - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in the main repository (parsed exec path)" - fi - echo "" + echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) is not the target of an entry in the runfiles manifest ($RUNFILES_MANIFEST_FILE)" fi - return 1 else if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) is the target of ($rlocation_path) in the runfiles manifest" fi fi - fi - - # If the runfiles directory exists, check if the caller's path is of the form - # $RUNFILES_DIR/rlocation_path and if so, set $rlocation_path. - if [[ -z "$rlocation_path" && -d "${RUNFILES_DIR:-/dev/null}" ]]; then - normalized_caller_path="$(echo "$caller_path" | sed 's|\\\\*|/|g')" + elif [[ -d "${RUNFILES_DIR:-/dev/null}" ]]; then + # Check whether the caller's path is of the form $RUNFILES_DIR/rlocation_path. + local normalized_dir normalized_dir="$(echo "${RUNFILES_DIR%[\/]}" | sed 's|\\\\*|/|g')" if [[ -n "${_RLOCATION_GREP_CASE_INSENSITIVE_ARGS}" ]]; then # When comparing file paths insensitively, also normalize the case of the prefixes. @@ -318,37 +328,38 @@ function runfiles_current_repository() { if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) does not lie under the runfiles directory ($normalized_dir)" fi - # The only shell script that is not executed from the runfiles directory (if it is populated) - # is the sh_binary entrypoint. Parse its path under the execroot, using the last match to - # allow for nested execroots (e.g. in Bazel integration tests). The binary may also be run - # directly from bazel-bin. - local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '(^|/)(bazel-out/[^/]+/bin|bazel-bin)/external/[^/]+/' | tail -1 | awk -F/ '{print $(NF-1)}') - if [[ -n "$repository" ]]; then - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in repository ($repository) (parsed exec path)" - fi - echo "$repository" - else - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in the main repository (parsed exec path)" - fi - echo "" - fi - return 0 else if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($caller_path) has path ($rlocation_path) relative to the runfiles directory ($RUNFILES_DIR)" fi fi - fi - - if [[ -z "$rlocation_path" ]]; then + else if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then 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" fi return 1 fi + if [[ -z "$rlocation_path" ]]; then + # The only shell script that is not among the runfiles is the entry point of a binary, which is + # executed from the execroot. Parse its path under the execroot, using the last match to allow + # for nested execroots (e.g. in Bazel integration tests). The binary may also be run directly + # from bazel-bin. + local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '(^|/)(bazel-out/[^/]+/bin|bazel-bin)/external/[^/]+/' | tail -1 | awk -F/ '{print $(NF-1)}') + if [[ -n "$repository" ]]; then + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in repository ($repository) (parsed exec path)" + fi + echo "$repository" + else + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) lies in the main repository (parsed exec path)" + fi + echo "" + fi + return 0 + fi + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($caller_path) corresponds to rlocation path ($rlocation_path)" fi @@ -374,11 +385,11 @@ function runfiles_rlocation_checked() { # FIXME: If the runfiles lookup fails, the exit code of this function is 0 if # and only if the runfiles manifest exists. In particular, the exit code # behavior is not consistent across platforms. - # The manifest takes precedence over the runfiles directory: whether the directory is populated - # is a property of the execution of the action or test, which is not known at analysis time, so - # the directory may exist but contain the stale contents of a previous execution. If the manifest - # exists, it is always authoritative. - if [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + # Exactly one of the runfiles directory and the manifest is consulted, so that the paths returned + # by this function are consistent with those that runfiles_current_repository resolves. The + # directory is preferred if it is usable since looking a path up in it does not require scanning + # the manifest, which can be very large. + if [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" && -z "$_RLOCATION_USE_RUNFILES_DIR" ]]; then if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: rlocation($1): looking in RUNFILES_MANIFEST_FILE ($RUNFILES_MANIFEST_FILE)" fi diff --git a/tests/runfiles/runfiles_test.bash b/tests/runfiles/runfiles_test.bash index fd60eee..43c1ae6 100755 --- a/tests/runfiles/runfiles_test.bash +++ b/tests/runfiles/runfiles_test.bash @@ -73,6 +73,13 @@ function find_runfiles_lib() { fi } +# Marks the given runfiles directory as fully materialized by placing the runfiles library in it at +# the path the initialization snippet looks it up at. +function materialize_runfiles_dir() { + mkdir -p "$1/bazel_tools/tools/bash/runfiles" + cp "$runfiles_lib_path" "$1/bazel_tools/tools/bash/runfiles/runfiles.bash" +} + function test_rlocation_call_requires_no_envvars() { export RUNFILES_DIR=mock/runfiles export RUNFILES_MANIFEST_FILE= @@ -216,6 +223,7 @@ function test_manifest_based_envvars() { export RUNFILES_DIR= export RUNFILES_MANIFEST_FILE=$tmpdir/foo.runfiles_manifest mkdir -p $tmpdir/foo.runfiles + materialize_runfiles_dir "$tmpdir/foo.runfiles" source "$runfiles_lib_path" runfiles_export_envvars @@ -574,20 +582,50 @@ EOF # Both envvars are set at the same time e.g. on Windows with --enable_runfiles, # where the runfiles directory contains the MANIFEST file that -# runfiles_export_envvars promotes to RUNFILES_MANIFEST_FILE. The manifest maps -# rlocation paths to the locations of the *original* files, so a caller that had -# been looked up in the runfiles directory could never be found in it. -function test_current_repository_directory_and_manifest_based() { +# runfiles_export_envvars promotes to RUNFILES_MANIFEST_FILE. rlocation and +# runfiles_current_repository then have to agree on which of the two they use: +# the manifest maps rlocation paths to the locations of the original files, so a +# path that rlocation resolved against the runfiles directory can never be the +# target of a manifest entry. +function test_current_repository_materialized_directory_and_manifest_based() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + + export RUNFILES_DIR="${tmpdir}/mock/runfiles" + export RUNFILES_MANIFEST_FILE="$RUNFILES_DIR/MANIFEST" + mkdir -p "$RUNFILES_DIR" + write_current_repository_lib "$RUNFILES_DIR/protobuf+3.19.2/foo/lib.sh" repo_of_other + write_current_repository_lib "$RUNFILES_DIR/_main/bar/lib.sh" repo_of_main + # Copies that must never be sourced: the runfiles directory is materialized, so the manifest is + # not consulted at all. + write_current_repository_lib "$tmpdir/protobuf+3.19.2/foo/lib.sh" repo_of_other unused + write_current_repository_lib "$tmpdir/_main/bar/lib.sh" repo_of_main unused + cat > "$RUNFILES_MANIFEST_FILE" << EOF +protobuf+3.19.2/foo/lib.sh $tmpdir/protobuf+3.19.2/foo/lib.sh +_main/bar/lib.sh $tmpdir/_main/bar/lib.sh +EOF + materialize_runfiles_dir "$RUNFILES_DIR" + source "$runfiles_lib_path" + + [[ "$(rlocation "protobuf+3.19.2/foo/lib.sh" "" || echo failed)" == "$RUNFILES_DIR/protobuf+3.19.2/foo/lib.sh" ]] || fail + source "$(rlocation "protobuf+3.19.2/foo/lib.sh" "")" || fail + [[ "$(repo_of_other || echo failed)" == "protobuf+3.19.2" ]] || fail + + [[ "$(rlocation "_main/bar/lib.sh" "" || echo failed)" == "$RUNFILES_DIR/_main/bar/lib.sh" ]] || fail + source "$(rlocation "_main/bar/lib.sh" "")" || fail + [[ "$(repo_of_main || echo failed)" == "" ]] || fail +} + +# With --noenable_runfiles, Bazel leaves behind a runfiles directory that only contains the MANIFEST +# file and the subdirectory of the main repository, which both functions have to recognize as not +# usable. +function test_current_repository_unmaterialized_directory_and_manifest_based() { local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" export RUNFILES_DIR="${tmpdir}/mock/runfiles" export RUNFILES_MANIFEST_FILE="$RUNFILES_DIR/MANIFEST" + mkdir -p "$RUNFILES_DIR/_main" write_current_repository_lib "$tmpdir/protobuf+3.19.2/foo/lib.sh" repo_of_other write_current_repository_lib "$tmpdir/_main/bar/lib.sh" repo_of_main - # The runfiles directory may hold stale contents, so the manifest wins. - mkdir -p "$RUNFILES_DIR" - write_current_repository_lib "$RUNFILES_DIR/protobuf+3.19.2/foo/lib.sh" repo_of_other stale - write_current_repository_lib "$RUNFILES_DIR/_main/bar/lib.sh" repo_of_main stale cat > "$RUNFILES_MANIFEST_FILE" << EOF protobuf+3.19.2/foo/lib.sh $tmpdir/protobuf+3.19.2/foo/lib.sh _main/bar/lib.sh $tmpdir/_main/bar/lib.sh @@ -603,6 +641,70 @@ EOF [[ "$(repo_of_main || echo failed)" == "" ]] || fail } +# A materialized runfiles directory is preferred over the manifest, which requires scanning a +# potentially very large file. +function test_rlocation_prefers_materialized_directory() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + + export RUNFILES_DIR="${tmpdir}/foo.runfiles" + export RUNFILES_MANIFEST_FILE="$RUNFILES_DIR/MANIFEST" + mkdir -p "$RUNFILES_DIR/_main/pkg" "$tmpdir/original/pkg" + touch "$RUNFILES_DIR/_main/pkg/file" "$tmpdir/original/pkg/file" + cat > "$RUNFILES_MANIFEST_FILE" << EOF +_main/pkg/file $tmpdir/original/pkg/file +EOF + materialize_runfiles_dir "$RUNFILES_DIR" + source "$runfiles_lib_path" + + [[ "$(rlocation _main/pkg/file || echo failed)" == "$RUNFILES_DIR/_main/pkg/file" ]] || fail + + # A runfile that is missing from the materialized directory is not looked up in the manifest. + rm "$RUNFILES_DIR/_main/pkg/file" + [[ "$(rlocation _main/pkg/file || echo failed)" == failed ]] || fail +} + +# A subprocess may use the exported functions without sourcing the library itself, in which case it +# does not learn which of the two the parent picked. It doesn't have to: it inherits RUNFILES_DIR +# and RUNFILES_MANIFEST_FILE, either of which is usable on its own if set. +function test_exported_functions_in_subprocess() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + + export RUNFILES_DIR="$tmpdir/foo.runfiles" + export RUNFILES_MANIFEST_FILE= + mkdir -p "$RUNFILES_DIR/_main/pkg" "$tmpdir/original/pkg" + touch "$RUNFILES_DIR/_main/pkg/file" "$tmpdir/original/pkg/file" + materialize_runfiles_dir "$RUNFILES_DIR" + source "$runfiles_lib_path" + + [[ "$(bash -c 'set -euo pipefail; rlocation _main/pkg/file')" == "$RUNFILES_DIR/_main/pkg/file" ]] || fail + + # With both set, the subprocess is free to pick the manifest even though this process picked the + # directory, as long as what it resolves to exists. + export RUNFILES_MANIFEST_FILE="$RUNFILES_DIR/MANIFEST" + cat > "$RUNFILES_MANIFEST_FILE" << EOF +_main/pkg/file $tmpdir/original/pkg/file +EOF + local -r resolved="$(bash -c 'set -euo pipefail; rlocation _main/pkg/file')" + [[ -f "$resolved" ]] || fail "subprocess resolved to $resolved" +} + +# A runfiles directory that has not been fully materialized should not be passed on to subprocesses, +# which may blindly trust the value they are given. +function test_manifest_based_envvars_unmaterialized_directory() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + echo "a b" > $tmpdir/foo.runfiles_manifest + + export RUNFILES_DIR= + export RUNFILES_MANIFEST_FILE=$tmpdir/foo.runfiles_manifest + # What --noenable_runfiles leaves behind. + mkdir -p $tmpdir/foo.runfiles/_main + source "$runfiles_lib_path" + + runfiles_export_envvars + [[ -z "${RUNFILES_DIR:-}" ]] || fail + [[ "${RUNFILES_MANIFEST_FILE:-}" == "$tmpdir/foo.runfiles_manifest" ]] || fail +} + function test_directory_based_envvars() { export RUNFILES_DIR=mock/runfiles export RUNFILES_MANIFEST_FILE=