diff --git a/shell/runfiles/runfiles.bash b/shell/runfiles/runfiles.bash index 3dfe3e2..b6bbdfe 100644 --- a/shell/runfiles/runfiles.bash +++ b/shell/runfiles/runfiles.bash @@ -374,12 +374,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. - if [[ -e "${RUNFILES_DIR:-/dev/null}/$1" ]]; then - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: rlocation($1): found under RUNFILES_DIR ($RUNFILES_DIR), return" - fi - echo "${RUNFILES_DIR}/$1" - elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + # 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 if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "INFO[runfiles.bash]: rlocation($1): looking in RUNFILES_MANIFEST_FILE ($RUNFILES_MANIFEST_FILE)" fi @@ -483,6 +482,11 @@ function runfiles_rlocation_checked() { echo "" fi fi + elif [[ -e "${RUNFILES_DIR:-/dev/null}/$1" ]]; then + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: rlocation($1): found under RUNFILES_DIR ($RUNFILES_DIR), return" + fi + echo "${RUNFILES_DIR}/$1" else if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then echo >&2 "ERROR[runfiles.bash]: cannot look up runfile \"$1\" " \ diff --git a/tests/runfiles/runfiles_test.bash b/tests/runfiles/runfiles_test.bash index f738b3d..fd60eee 100755 --- a/tests/runfiles/runfiles_test.bash +++ b/tests/runfiles/runfiles_test.bash @@ -512,6 +512,97 @@ EOF [[ "$(rlocation "repo2+/foo/runfile" "my_module++ext+repo1" || echo failed)" == "$tmpdir/repo2+/runfile" ]] || fail } +# Writes a script that reports the repository it lies in via the given function. +# The function must not be defined by more than one script in a single test, as +# runfiles_current_repository resolves the path of the script that defines it. +function write_current_repository_lib() { + local -r path="$1" + local -r func="$2" + local -r result="${3:-}" + mkdir -p "$(dirname "$path")" + if [[ -n "$result" ]]; then + # A stale copy that must never be sourced. + cat > "$path" << EOF +function $func() { + echo "$result" +} +EOF + else + cat > "$path" << EOF +function $func() { + runfiles_current_repository 1 +} +EOF + fi +} + +function test_current_repository_directory_based() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + + export RUNFILES_DIR="${tmpdir}/mock/runfiles" + export RUNFILES_MANIFEST_FILE= + 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 + source "$runfiles_lib_path" + + source "$(rlocation "protobuf+3.19.2/foo/lib.sh" "")" || fail + [[ "$(repo_of_other || echo failed)" == "protobuf+3.19.2" ]] || fail + + source "$(rlocation "_main/bar/lib.sh" "")" || fail + [[ "$(repo_of_main || echo failed)" == "" ]] || fail +} + +function test_current_repository_manifest_based() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + + export RUNFILES_DIR= + export RUNFILES_MANIFEST_FILE="$tmpdir/foo.runfiles_manifest" + 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 + 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 + source "$runfiles_lib_path" + + source "$(rlocation "protobuf+3.19.2/foo/lib.sh" "")" || fail + [[ "$(repo_of_other || echo failed)" == "protobuf+3.19.2" ]] || fail + + source "$(rlocation "_main/bar/lib.sh" "")" || fail + [[ "$(repo_of_main || echo failed)" == "" ]] || fail +} + +# 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() { + local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + + export RUNFILES_DIR="${tmpdir}/mock/runfiles" + export RUNFILES_MANIFEST_FILE="$RUNFILES_DIR/MANIFEST" + 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 +EOF + source "$runfiles_lib_path" + + [[ "$(rlocation "protobuf+3.19.2/foo/lib.sh" "" || echo failed)" == "$tmpdir/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)" == "$tmpdir/_main/bar/lib.sh" ]] || fail + source "$(rlocation "_main/bar/lib.sh" "")" || fail + [[ "$(repo_of_main || echo failed)" == "" ]] || fail +} + function test_directory_based_envvars() { export RUNFILES_DIR=mock/runfiles export RUNFILES_MANIFEST_FILE=