From 998621da0315cce966118d1a3486435c379b2320 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 13 Aug 2026 12:15:40 +0200 Subject: [PATCH] Resolve relative manifest targets like the runfiles directory does The target of a runfiles manifest entry is an absolute path, except for unresolved symlinks (`ctx.actions.declare_symlink`), whose target Bazel copies into the manifest verbatim: `SourceManifestAction` writes `metadata.getUnresolvedSymlinkTarget()`, which may be relative. In a runfiles directory, `SymlinkTreeHelper#createRunfilesSymlinks` creates such an entry textually, so the file system resolves it relative to the directory containing the symlink. A manifest-based lookup returned that raw target instead, i.e. a path relative to the current working directory. `rlocation` thus either failed or, worse, returned an unrelated file that happened to exist relative to the working directory. Interpret a relative target as an rlocation path relative to the directory of its entry and look that up in the manifest again, which is what resolving the symlink in a materialized runfiles directory amounts to. Targets that escape the runfiles root cannot be resolved this way and are reported as missing, as are cycles, which are broken after 32 levels. Claude-Session: https://claude.ai/code/session_013o72rMrpgYKD9wYYQ2Nr98 --- shell/runfiles/runfiles.bash | 123 +++++++++++++++++++++++++----- tests/runfiles/runfiles_test.bash | 106 +++++++++++++++++++++++++ 2 files changed, 212 insertions(+), 17 deletions(-) diff --git a/shell/runfiles/runfiles.bash b/shell/runfiles/runfiles.bash index b6bbdfe..161b8c5 100644 --- a/shell/runfiles/runfiles.bash +++ b/shell/runfiles/runfiles.bash @@ -128,6 +128,90 @@ function __runfiles_escape_grep() { } export -f __runfiles_escape_grep +# Lexically resolves the "." and ".." segments in the given rlocation path. +# Fails if the path is empty or would escape the runfiles root. +function __runfiles_normalize_rlocation_path() { + local rest="$1" + local normalized= + local segment + while [[ -n "$rest" ]]; do + if [[ "$rest" == */* ]]; then + segment="${rest%%/*}" + rest="${rest#*/}" + else + segment="$rest" + rest= + fi + case "$segment" in + "" | ".") + ;; + "..") + if [[ "$normalized" == */* ]]; then + normalized="${normalized%/*}" + elif [[ -n "$normalized" ]]; then + normalized= + else + return 1 + fi + ;; + *) + normalized="${normalized:+$normalized/}$segment" + ;; + esac + done + [[ -n "$normalized" ]] || return 1 + echo "$normalized" +} +export -f __runfiles_normalize_rlocation_path + +# Resolves the target of a runfiles manifest entry to a path in the local file system. +# +# Bazel stores the target of an unresolved symlink (ctx.actions.declare_symlink) in the manifest +# verbatim, so unlike all other targets it may be a relative path. In a materialized runfiles +# directory the entry is a symlink with that very target, which the file system resolves relative to +# the directory containing the symlink. A relative target thus has to be interpreted as an rlocation +# path relative to the directory of the entry and looked up in the manifest again. +# +# Arguments: +# $1: the rlocation path of the manifest entry +# $2: the target of the manifest entry +# $3: the path to append to the target, if any +# $4: the current lookup depth, used to break out of cycles of unresolved symlinks +# Prints the resolved path if it exists and the empty string otherwise. +function __runfiles_resolve_manifest_target() { + if [[ "$2" =~ $_RLOCATION_ISABS_PATTERN || "$2" == /* ]]; then + local -r resolved="$2$3" + if [[ -e "$resolved" ]]; then + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: rlocation($1$3): found in manifest as ($resolved)" + fi + echo "$resolved" + else + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: rlocation($1$3): found in manifest as ($resolved), but file does not exist" + fi + echo "" + fi + return 0 + fi + + local entry_dir="${1%/*}" + [[ "$entry_dir" == "$1" ]] && entry_dir= + local target_rlocation_path + target_rlocation_path=$(__runfiles_normalize_rlocation_path "${entry_dir:+$entry_dir/}$2$3") || { + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "ERROR[runfiles.bash]: rlocation($1$3): unresolved symlink target ($2) points outside the runfiles tree" + fi + echo "" + return 0 + } + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: rlocation($1$3): unresolved symlink target ($2) resolves to ($target_rlocation_path)" + fi + runfiles_rlocation_checked "$target_rlocation_path" "$(($4 + 1))" +} +export -f __runfiles_resolve_manifest_target + # Prints to stdout the runtime location of a data-dependency. # The optional second argument can be used to specify the canonical name of the # repository whose repository mapping should be used to resolve the repository @@ -374,6 +458,16 @@ 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 optional second argument is the current lookup depth, which only differs from zero while + # following the target of an unresolved symlink. + local -r depth="${2:-0}" + if [[ "$depth" -gt 32 ]]; then + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "ERROR[runfiles.bash]: rlocation($1): too many levels of symbolic links" + fi + echo "" + return 0 + fi # 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 @@ -438,11 +532,16 @@ function runfiles_rlocation_checked() { prefix_result="${prefix_result//\\b/\\}" fi [[ -z "$prefix_result" ]] && continue - local -r candidate="${prefix_result}${1#"${prefix}"}" - if [[ -e "$candidate" ]]; then - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($candidate) via prefix ($prefix)" - fi + if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then + echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest via prefix ($prefix)" + fi + local candidate + # The trailing marker prevents command substitution from stripping a newline that is part + # of the resolved path. + candidate=$(__runfiles_resolve_manifest_target \ + "$prefix" "$prefix_result" "${1#"${prefix}"}" "$depth"; echo -n x) + candidate="${candidate%$'\n'x}" + if [[ -n "$candidate" ]]; then echo "$candidate" return 0 fi @@ -457,7 +556,7 @@ function runfiles_rlocation_checked() { # better to return no path rather than a potentially different, # non-empty path. if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($candidate) via prefix ($prefix), but file does not exist" + echo >&2 "INFO[runfiles.bash]: rlocation($1): prefix ($prefix) did not resolve, not retrying with a shorter one" fi break done @@ -470,17 +569,7 @@ function runfiles_rlocation_checked() { result="${result//\\n/$'\n'}" result="${result//\\b/\\}" fi - if [[ -e "$result" ]]; then - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($result)" - fi - echo "$result" - else - if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then - echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($result), but file does not exist" - fi - echo "" - fi + __runfiles_resolve_manifest_target "$1" "$result" "" "$depth" fi elif [[ -e "${RUNFILES_DIR:-/dev/null}/$1" ]]; then if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then diff --git a/tests/runfiles/runfiles_test.bash b/tests/runfiles/runfiles_test.bash index fd60eee..fd11d97 100755 --- a/tests/runfiles/runfiles_test.bash +++ b/tests/runfiles/runfiles_test.bash @@ -209,6 +209,112 @@ EOF [[ -z "$(rlocation "dir with spaces/nested/file" || echo failed)" ]] || fail } +# Writes a runfiles layout containing unresolved symlinks with relative targets, both as a manifest +# pointing at the original files and as a materialized runfiles directory. +function write_relative_symlink_target_layout() { + local -r tmpdir="$1" + local -r dir="$tmpdir/foo.runfiles" + + mkdir -p "$tmpdir/original/dir/deeply/nested" + echo file > "$tmpdir/original/file" + echo nested_file > "$tmpdir/original/dir/deeply/nested/file" + # Lies next to the runfiles directory and is thus only reachable through a relative target that + # leaves the runfiles tree. + echo outside > "$tmpdir/outside" + + cat > "$tmpdir/foo.runfiles_manifest" << EOF +_main/pkg/file $tmpdir/original/file +_main/pkg/dir $tmpdir/original/dir +_main/pkg/link ../pkg/file +_main/pkg/nested/link ../../pkg/link +_main/pkg/dir_link ./dir +_main/pkg/dangling ../pkg/missing +_main/pkg/escaping ../../../outside +_main/pkg/loop_a loop_b +_main/pkg/loop_b loop_a + _main/pkg/link\swith\sspaces ../pkg/file +EOF + + mkdir -p "$dir/_main/pkg/nested" + ln -s "$tmpdir/original/file" "$dir/_main/pkg/file" + ln -s "$tmpdir/original/dir" "$dir/_main/pkg/dir" + ln -s ../pkg/file "$dir/_main/pkg/link" + ln -s ../../pkg/link "$dir/_main/pkg/nested/link" + ln -s ./dir "$dir/_main/pkg/dir_link" + ln -s ../pkg/missing "$dir/_main/pkg/dangling" + ln -s ../../../outside "$dir/_main/pkg/escaping" + ln -s loop_b "$dir/_main/pkg/loop_a" + ln -s loop_a "$dir/_main/pkg/loop_b" + ln -s ../pkg/file "$dir/_main/pkg/link with spaces" +} + +# Asserts that the given rlocation path resolves to a file with the given contents. Only the +# contents are compared since the path itself necessarily differs between the two lookup modes: a +# materialized runfiles directory resolves to the entry in that directory, not to the file it +# points at. +function assert_rlocation_contents() { + local -r resolved="$(rlocation "$1" || echo failed)" + [[ -f "$resolved" ]] || fail "$1 did not resolve to a file, got: $resolved" + [[ "$(cat "$resolved")" == "$2" ]] || fail "$1 resolved to $resolved with unexpected contents" +} + +# Asserts that the given rlocation path does not resolve, which rlocation reports as an empty result +# when it uses the manifest and as a non-zero exit code when it uses the runfiles directory (see the +# FIXME on runfiles_rlocation_checked). +function assert_no_rlocation() { + local -r resolved="$(rlocation "$1" || echo failed)" + [[ -z "$resolved" || "$resolved" == failed ]] || fail "$1 unexpectedly resolved to $resolved" +} + +# The lookups whose outcome must not depend on whether the manifest or the runfiles directory backs +# them: resolving a relative target against the manifest has to arrive at the same file that the +# file system arrives at when resolving the corresponding symlink in the runfiles directory. +function assert_relative_symlink_target_lookups() { + assert_rlocation_contents _main/pkg/link file + assert_rlocation_contents _main/pkg/nested/link file + assert_rlocation_contents "_main/pkg/link with spaces" file + # A relative target that resolves to a directory runfile also resolves paths underneath it. + [[ -d "$(rlocation _main/pkg/dir_link || echo failed)" ]] || fail + assert_rlocation_contents _main/pkg/dir_link/deeply/nested/file nested_file + # A target that doesn't resolve to an existing file behaves like a missing runfile. + assert_no_rlocation _main/pkg/dangling + assert_no_rlocation _main/pkg/dir_link/does/not/exist + # A cycle terminates instead of looping forever. + assert_no_rlocation _main/pkg/loop_a +} + +function test_manifest_based_relative_symlink_targets() { + local -r tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + write_relative_symlink_target_layout "$tmpdir" + + export RUNFILES_DIR= + export RUNFILES_MANIFEST_FILE=$tmpdir/foo.runfiles_manifest + source "$runfiles_lib_path" + + assert_relative_symlink_target_lookups + # The one lookup the manifest cannot reproduce: a relative target that leaves the runfiles tree + # can only be resolved against a materialized runfiles directory, whose existence the manifest + # does not imply. Bazel does not generate such a runfile. + assert_no_rlocation _main/pkg/escaping +} + +function test_directory_based_relative_symlink_targets() { + # MSYS2 may materialize symlinks as copies, which does not preserve relative targets. + if is_windows; then + return 0 + fi + + local -r tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" + write_relative_symlink_target_layout "$tmpdir" + + export RUNFILES_DIR="$tmpdir/foo.runfiles" + export RUNFILES_MANIFEST_FILE= + source "$runfiles_lib_path" + + assert_relative_symlink_target_lookups + assert_rlocation_contents _main/pkg/escaping outside +} + function test_manifest_based_envvars() { local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)" echo "a b" > $tmpdir/foo.runfiles_manifest