Skip to content

Commit

Permalink
Do not fail or print errors when Shellzelisk cannot find a requested ?
Browse files Browse the repository at this point in the history
?Bazel binary, if tools/bazel exists.

Instead, we should do our best to populate the $BAZEL_REAL env var with some meaningful path, but then delegate to tools/bazel without aborting or printing anything, in case we couldn't find a requested binary or make sense of the contents of .bazelversion.

This is in response to #10356 (comment) and hopefully fixes the use case reported by the affected users.

@laurentlb This is the cherry-pick I mentioned. I will import it, if you LGTM this PR.
@Helcaraxan @davido FYI.

Closes #10664.

PiperOrigin-RevId: 291904662
  • Loading branch information
philwo authored and laurentlb committed Jan 29, 2020
1 parent f7b1ade commit 0e146e7
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 15 deletions.
30 changes: 17 additions & 13 deletions scripts/packages/bazel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,6 @@ function get_bazel_version() {

get_bazel_version

if [[ -z $bazel_version ]]; then
color "31" "ERROR: No installed Bazel version found, cannot continue."
(echo ""
echo "Bazel binaries have to be installed in ${wrapper_dir}, but none were found.") 2>&1
exit 1
fi

BAZEL_REAL="${wrapper_dir}/bazel-${bazel_version}-${os_arch_suffix}"

# Try without the architecture suffix.
Expand All @@ -156,6 +149,23 @@ if [[ ! -x ${BAZEL_REAL} && -x ${bazel_real_path} ]]; then
fi
fi

# If the repository contains a checked-in executable called tools/bazel, we
# assume that they know what they're doing and have their own way of versioning
# Bazel. Thus, we don't have to print our helpful messages or error out in case
# we couldn't find a binary.
readonly wrapper="${workspace_dir}/tools/bazel"
if [[ -x "$wrapper" && -f "$wrapper" ]]; then
export BAZEL_REAL
exec -a "$0" "${wrapper}" "$@"
fi

if [[ -z $bazel_version ]]; then
color "31" "ERROR: No installed Bazel version found, cannot continue."
(echo ""
echo "Bazel binaries have to be installed in ${wrapper_dir}, but none were found.") 2>&1
exit 1
fi

if [[ ! -x $BAZEL_REAL ]]; then
color "31" "ERROR: The project you're trying to build requires Bazel ${bazel_version} (${reason}), but it wasn't found in ${wrapper_dir}."

Expand Down Expand Up @@ -190,10 +200,4 @@ if [[ ! -x $BAZEL_REAL ]]; then
exit 1
fi

readonly wrapper="${workspace_dir}/tools/bazel"
if [[ -x "$wrapper" && -f "$wrapper" ]]; then
export BAZEL_REAL
exec -a "$0" "${wrapper}" "$@"
fi

exec -a "$0" "${BAZEL_REAL}" "$@"
38 changes: 36 additions & 2 deletions src/test/shell/bazel/bazel_wrapper_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,43 @@ exit 0
EOF
chmod +x tools/bazel

USE_BAZEL_VERSION="0.29.1" ../bin/bazel build //src:bazel &> "$TEST_log"
# Due to https://github.com/bazelbuild/bazel/issues/10356, we have to ignore
# the .bazelversion file in case a tools/bazel executable is present.
# Even if the requested Bazel version doesn't exist, the wrapper still has to
# add the $BAZEL_REAL environment variable, so this will point to a
# non-existing file in that case. It's up to the owner of the repo to decide
# what to make of it - e.g. print an error message, fallback to something else
# or completely ignore the $BAZEL_REAL variable.
USE_BAZEL_VERSION="3.0.0" ../bin/bazel build //src:bazel &> "$TEST_log"
expect_log "Hello from the wrapper tools/bazel!"
expect_log "BAZEL_REAL = .*/bin/bazel-3.0.0"
expect_log "My args: build //src:bazel"
}

test_gracefully_handles_bogus_bazelversion() {
setup_mock

mkdir tools
cat > tools/bazel <<'EOF'
#!/bin/bash
set -euo pipefail
echo "Hello from the wrapper tools/bazel!"
echo "My args: $@"
exit 0
EOF
chmod +x tools/bazel

# Create a .bazelversion file that looks completely different than what we
# actually support. The wrapper is supposed to not crash and still call the
# tools/bazel executable. The content of the $BAZEL_REAL variable can be
# completely bogus, of course.
cat > .bazelversion <<'EOF'
mirrors: [http://mirror.example/bazel-5.0.0, http://github.com/example/]
# The above is our internal mirror. The syntax is only supported since
# Bazelisk 42.0.
EOF
../bin/bazel build //src:bazel &> "$TEST_log"
expect_log "Hello from the wrapper tools/bazel!"
expect_log "BAZEL_REAL = .*/bin/bazel-0.29.1"
expect_log "My args: build //src:bazel"
}

Expand Down

0 comments on commit 0e146e7

Please sign in to comment.