Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not export rlocation from test setup #24256

Closed
wants to merge 1 commit into from
Closed

Do not export rlocation from test setup #24256

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions tools/test/test-setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ function rlocation() {
fi
}

export -f rlocation
export -f is_absolute
# If RUNFILES_MANIFEST_ONLY is set to 1 and the manifest file does exist,
# then test programs should use manifest file to find runfiles.
if [[ "${RUNFILES_MANIFEST_ONLY:-}" == "1" && -e "${RUNFILES_MANIFEST_FILE:-}" ]]; then
Expand Down Expand Up @@ -236,6 +234,15 @@ else
TEST_PATH="$(rlocation $TEST_WORKSPACE/$EXE)"
fi

# Redefine rlocation to notify users of its removal - it used to be exported.
# TODO: Remove this before Bazel 9.
function rlocation() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be compatible with /bin/sh which caused a test failure at https://buildkite.com/bazel/google-bazel-presubmit/builds/87576#0193afe3-eb2a-49df-b687-13d1be1ce0f7

I can change #!/bin/sh to #!/usr/bin/env bash to fix it in pywrap_template.txt, but maybe we should try to make it compatible?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

function rlocation() {
  caller 0 | {
    read LINE SUB FILE
    echo >&2 "ERROR: rlocation is no longer implicitly provided by Bazel's test setup, but called from $SUB in line $LINE of $FILE. Please use https://github.com/bazelbuild/rules_shell/blob/main/shell/runfiles/runfiles.bash instead."
    exit 1
  }
}

seems to work

read LINE SUB FILE < <(caller 0);
>&2 echo "ERROR: rlocation is no longer implicitly provided by Bazel's test setup, but called from $SUB in line $LINE of $FILE. Please use https://github.com/bazelbuild/rules_shell/blob/main/shell/runfiles/runfiles.bash instead."
exit 1
}
export -f rlocation

# TODO(jsharpe): Use --test_env=TEST_SHORT_EXEC_PATH=true to activate this code
# path to workaround a bug with long executable paths when executing remote
# tests on Windows.
Expand Down
Loading