Skip to content

Commit

Permalink
Fix Bash rlocation failure with stricter Bash options (#16773)
Browse files Browse the repository at this point in the history
When run in the context of the bazel-skylib unit test setup, the new repo mapping logic in the Bash runfiles library failed due to a non-matching grep in a pipe. Fix this by using a wrapper around grep throughout that does not exit with exit code 1 if there is no match.

Fixes bazelbuild/bazel-skylib#411 (comment)

Closes #16755.

PiperOrigin-RevId: 488749744
Change-Id: I087b03d9e95ba27a409c551bdc27d0027919a0fe

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
Wyverald and fmeum authored Nov 15, 2022
1 parent b3704e8 commit 0703058
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions tools/bash/runfiles/runfiles.bash
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ msys*|mingw*|cygwin*)
;;
esac

# Does not exit with a non-zero exit code if no match is found.
function __runfiles_maybe_grep() {
grep "$@" || test $? = 1;
}
export -f __runfiles_maybe_grep

# 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
Expand Down Expand Up @@ -145,7 +151,7 @@ function rlocation() {
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
echo >&2 "INFO[runfiles.bash]: rlocation($1): looking up canonical name for ($target_repo_apparent_name) from ($source_repo) in ($RUNFILES_REPO_MAPPING)"
fi
local -r target_repo=$(grep -m1 "^$source_repo,$target_repo_apparent_name," "$RUNFILES_REPO_MAPPING" | cut -d , -f 3)
local -r target_repo=$(__runfiles_maybe_grep -m1 "^$source_repo,$target_repo_apparent_name," "$RUNFILES_REPO_MAPPING" | cut -d , -f 3)
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
echo >&2 "INFO[runfiles.bash]: rlocation($1): canonical name of target repo is ($target_repo)"
fi
Expand Down Expand Up @@ -225,7 +231,7 @@ function runfiles_current_repository() {
# uses / as the path separator even on Windows.
local -r normalized_caller_path="$(echo "$caller_path" | sed 's|\\\\*|/|g')"
local -r escaped_caller_path="$(echo "$normalized_caller_path" | sed 's/[^-A-Za-z0-9_/]/\\&/g')"
rlocation_path=$(grep -m1 "^[^ ]* ${escaped_caller_path}$" "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 1)
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 "INFO[runfiles.bash]: runfiles_current_repository($idx): ($normalized_caller_path) is not the target of an entry in the runfiles manifest ($RUNFILES_MANIFEST_FILE)"
Expand Down Expand Up @@ -254,7 +260,7 @@ function runfiles_current_repository() {
# 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).
local -r repository=$(echo "$normalized_caller_path" | grep -E -o '/execroot/[^/]+/bazel-out/[^/]+/bin/external/[^/]+/' | tail -1 | rev | cut -d / -f 2 | rev)
local -r repository=$(echo "$normalized_caller_path" | __runfiles_maybe_grep -E -o '/execroot/[^/]+/bazel-out/[^/]+/bin/external/[^/]+/' | tail -1 | rev | cut -d / -f 2 | rev)
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)"
Expand Down Expand Up @@ -305,7 +311,7 @@ function runfiles_rlocation_checked() {
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
echo >&2 "INFO[runfiles.bash]: rlocation($1): looking in RUNFILES_MANIFEST_FILE ($RUNFILES_MANIFEST_FILE)"
fi
local -r result=$(grep -m1 "^$1 " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-)
local -r result=$(__runfiles_maybe_grep -m1 "^$1 " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-)
if [[ -z "$result" ]]; then
# If path references a runfile that lies under a directory that itself
# is a runfile, then only the directory is listed in the manifest. Look
Expand All @@ -318,7 +324,7 @@ function runfiles_rlocation_checked() {
new_prefix="${prefix%/*}"
[[ "$new_prefix" == "$prefix" ]] && break
prefix="$new_prefix"
prefix_result=$(grep -m1 "^$prefix " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-)
prefix_result=$(__runfiles_maybe_grep -m1 "^$prefix " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-)
[[ -z "$prefix_result" ]] && continue
local -r candidate="${prefix_result}${1#"${prefix}"}"
if [[ -e "$candidate" ]]; then
Expand Down

0 comments on commit 0703058

Please sign in to comment.