From 1a5b56c180a148f097933300e94267c7dab0df82 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Mon, 23 Nov 2020 21:29:06 -0800 Subject: [PATCH 1/8] Add --{no,}autodetect_server_javabase. We want bazel to fail to start instead of falling back to a host JRE/JDK. We are using a hermetic JDK and the embedded JRE, so there should be no need to use anything from the host. We've debugged enough cases so far where the host installed JDK was buggy and causing random crashes on specific machines. Fixes: #12451 --- site/docs/user-manual.html | 8 ++++++++ src/main/cpp/blaze.cc | 4 +++- src/main/cpp/startup_options.cc | 7 +++++++ src/main/cpp/startup_options.h | 2 ++ src/test/cpp/bazel_startup_options_test.cc | 1 + 5 files changed, 21 insertions(+), 1 deletion(-) diff --git a/site/docs/user-manual.html b/site/docs/user-manual.html index 20933c49f72d45..fd7e683b035863 100644 --- a/site/docs/user-manual.html +++ b/site/docs/user-manual.html @@ -2835,6 +2835,14 @@

--host_jvm_debug

subprocesses of Bazel: applications, tests, tools, etc.)

+

--autodetect_server_javabase

+

+ This option causes Bazel to automatically search for an installed JDK on startup, + and to fall back to the installed JRE if the embedded JRE isn't available. + --explicit_server_javabase can be used to pick an explicit JRE to + run bazel with. +

+

--batch

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index f42a4f887aaf7a..a3fd334f24248d 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -457,7 +457,9 @@ static vector GetServerExeArgs(const blaze_util::Path &jvm_path, startup_options.output_base.AsCommandLineArgument()); result.push_back("--workspace_directory=" + blaze_util::ConvertPath(workspace)); - result.push_back("--default_system_javabase=" + GetSystemJavabase()); + if (startup_options.autodetect_server_javabase) { + result.push_back("--default_system_javabase=" + GetSystemJavabase()); + } if (!startup_options.server_jvm_out.IsEmpty()) { result.push_back("--server_jvm_out=" + diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index 5ee43bac80a98a..0190dccef33ea3 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -71,6 +71,7 @@ StartupOptions::StartupOptions(const string &product_name, ignore_all_rc_files(false), block_for_lock(true), host_jvm_debug(false), + autodetect_server_javabase(true), batch(false), batch_cpu_scheduling(false), io_nice_level(-1), @@ -135,6 +136,8 @@ StartupOptions::StartupOptions(const string &product_name, RegisterNullaryStartupFlag("fatal_event_bus_exceptions", &fatal_event_bus_exceptions); RegisterNullaryStartupFlag("host_jvm_debug", &host_jvm_debug); + RegisterNullaryStartupFlag("autodetect_server_javabase", + &autodetect_server_javabase); RegisterNullaryStartupFlag("idle_server_tasks", &idle_server_tasks); RegisterNullaryStartupFlag("incompatible_enable_execution_transition", &incompatible_enable_execution_transition); @@ -481,6 +484,10 @@ StartupOptions::GetServerJavabaseAndType() const { // 2) Use a bundled JVM if we have one. default_server_javabase_ = std::pair( bundled_jre_path, JavabaseType::EMBEDDED); + } else if (!autodetect_server_javabase) { + BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) + << "Could not find embedded or explicit server javabase, and " + "--noautodetect_server_javabase is set."; } else { // 3) Otherwise fall back to using the default system JVM. blaze_util::Path system_javabase = GetSystemJavabase(); diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index 41aac66a687093..dc78702ec0e7b4 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -165,6 +165,8 @@ class StartupOptions { bool host_jvm_debug; + bool autodetect_server_javabase; + std::string host_jvm_profile; std::vector host_jvm_args; diff --git a/src/test/cpp/bazel_startup_options_test.cc b/src/test/cpp/bazel_startup_options_test.cc index 91f1c14a8a9ec1..742ddba87d1ae1 100644 --- a/src/test/cpp/bazel_startup_options_test.cc +++ b/src/test/cpp/bazel_startup_options_test.cc @@ -100,6 +100,7 @@ TEST_F(BazelStartupOptionsTest, ValidStartupFlags) { ExpectValidNullaryOption(options, "fatal_event_bus_exceptions"); ExpectValidNullaryOption(options, "home_rc"); ExpectValidNullaryOption(options, "host_jvm_debug"); + ExpectValidNullaryOption(options, "autodetect_server_javabase"); ExpectValidNullaryOption(options, "ignore_all_rc_files"); ExpectValidNullaryOption(options, "incompatible_enable_execution_transition"); ExpectValidNullaryOption(options, "master_bazelrc"); From 3c00cb3d4b26237c970a067c724ef218f61c7eeb Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sat, 5 Dec 2020 22:30:23 -0800 Subject: [PATCH 2/8] Document --autodetect_server_javabase --- .../build/lib/runtime/BlazeServerStartupOptions.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java index a9f976e6592052..390de3858e096c 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java @@ -493,4 +493,14 @@ public String getTypeDescription() { + "extended attribute is checked on all source files and output files, meaning " + "that it causes a significant number of invocations of the getxattr() system call.") public String unixDigestHashAttributeName; + + @Option( + name = "autodetect_server_javabase", + defaultValue = "true", // NOTE: only for documentation, value never passed to the server. + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOSES_INCREMENTAL_STATE}, + help = + "When --noautodetect_server_javabase is passed, Bazel does not fall back to the local " + + "JDK for running the bazel server and instead exits.") + public boolean autodetectServerJavabase; } From 2f889a9da9a7de0d976b6d0dfebd17928b62ae91 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sat, 5 Dec 2020 22:44:48 -0800 Subject: [PATCH 3/8] Add test for --autodetect_server_javabase with embedded jdk --- src/test/shell/integration/startup_options_test.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 71dd2fd7a3f500..8ed0166f1b3eec 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -75,4 +75,10 @@ function test_command_args_are_not_parsed_as_startup_args() { expect_not_log "Error: Unable to read .bazelrc file" } +# Test that normal bazel works with and without --autodetect_server_javabase +function test_autodetect_server_javabase() { + bazel --autodetect_server_javabase version &> $TEST_log || fail "Should pass" + bazel --noautodetect_server_javabase version &> $TEST_log || fail "Should pass" +} + run_suite "${PRODUCT_NAME} startup options test" From dc9f3103e9b00ad34fb48cdab6a62af05cf77211 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sat, 5 Dec 2020 23:01:23 -0800 Subject: [PATCH 4/8] Add shell tests to confirm bazel starts correctly. --- src/test/shell/bin/bazel | 7 ++ src/test/shell/integration/BUILD | 12 ++++ .../integration/nojdk_startup_options_test.sh | 69 +++++++++++++++++++ .../shell/integration/startup_options_test.sh | 1 + 4 files changed, 89 insertions(+) create mode 100755 src/test/shell/integration/nojdk_startup_options_test.sh diff --git a/src/test/shell/bin/bazel b/src/test/shell/bin/bazel index aba1ed9635d1a8..de4191c81b019f 100755 --- a/src/test/shell/bin/bazel +++ b/src/test/shell/bin/bazel @@ -18,6 +18,13 @@ # affect all our integration tests. # bazel_bin=$(rlocation io_bazel/src/bazel) + +# One test uses bazel_nojdk instead. If bazel_bin is empty, try that. +if [[ -z "${bazel_bin}" ]]; +then + bazel_bin=$(rlocation io_bazel/src/bazel_nojdk) +fi + # unset runfiles environment so that the "inner bazel" # has to detect its own runfiles unset RUNFILES_DIR diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD index 5bb5dbdf6729cf..b8d7d776a58c0f 100644 --- a/src/test/shell/integration/BUILD +++ b/src/test/shell/integration/BUILD @@ -185,6 +185,18 @@ sh_test( ], ) +sh_test( + name = "nojdk_startup_options_test", + size = "medium", + srcs = ["nojdk_startup_options_test.sh"], + data = [ + "//src:bazel_nojdk", + "//src/test/shell:bin/bazel", + "//src/test/shell/bazel:test-deps-wo-bazel", + "@bazel_tools//tools/bash/runfiles", + ], +) + sh_test( name = "run_test", size = "medium", diff --git a/src/test/shell/integration/nojdk_startup_options_test.sh b/src/test/shell/integration/nojdk_startup_options_test.sh new file mode 100755 index 00000000000000..02190ecde72324 --- /dev/null +++ b/src/test/shell/integration/nojdk_startup_options_test.sh @@ -0,0 +1,69 @@ +#!/bin/bash +# +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Test of Bazel's startup option handling cof the nojdk version. + +# --- begin runfiles.bash initialization --- +set -euo pipefail +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +case "$(uname -s | tr [:upper:] [:lower:])" in +msys*|mingw*|cygwin*) + declare -r is_windows=true + ;; +*) + declare -r is_windows=false + ;; +esac + +if "$is_windows"; then + export MSYS_NO_PATHCONV=1 + export MSYS2_ARG_CONV_EXCL="*" +fi + +# Test that nojdk bazel works with --autodetect_server_javabase +function test_autodetect_server_javabase() { + bazel --autodetect_server_javabase version &> $TEST_log || fail "Should pass" +} + +# Test that nojdk bazel fails with --noautodetect_server_javabase +function test_noautodetect_server_javabase() { + bazel --noautodetect_server_javabase version &> $TEST_log && fail "Should fail" + expect_log "FATAL: Could not find embedded or explicit server javabase, and --noautodetect_server_javabase is set." +} + +run_suite "${PRODUCT_NAME} startup options test" diff --git a/src/test/shell/integration/startup_options_test.sh b/src/test/shell/integration/startup_options_test.sh index 8ed0166f1b3eec..63f1624866d41d 100755 --- a/src/test/shell/integration/startup_options_test.sh +++ b/src/test/shell/integration/startup_options_test.sh @@ -76,6 +76,7 @@ function test_command_args_are_not_parsed_as_startup_args() { } # Test that normal bazel works with and without --autodetect_server_javabase +# because it has an embedded JRE. function test_autodetect_server_javabase() { bazel --autodetect_server_javabase version &> $TEST_log || fail "Should pass" bazel --noautodetect_server_javabase version &> $TEST_log || fail "Should pass" From c62dd890261e11fd6b178ca166741aa05ff5d448 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sat, 5 Dec 2020 23:18:51 -0800 Subject: [PATCH 5/8] Fix test failure FATAL: The install_base directory '/var/lib/buildkite-agent/bazeltest/install_base' contains a different Bazel version (found aa41a738151267f7716fdef640010dd7 but this binary is 8dc389309eace6214919196718842380). Remove it or specify a different --install_base. --- src/test/shell/integration/nojdk_startup_options_test.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/shell/integration/nojdk_startup_options_test.sh b/src/test/shell/integration/nojdk_startup_options_test.sh index 02190ecde72324..ed31fdd2579b8d 100755 --- a/src/test/shell/integration/nojdk_startup_options_test.sh +++ b/src/test/shell/integration/nojdk_startup_options_test.sh @@ -38,6 +38,10 @@ else fi # --- end runfiles.bash initialization --- +# We don't want to use the cached, extracted bazel. It will have a different +# sha1 and fail the test. The 2 version commands below are cheap. +unset TEST_INSTALL_BASE + source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } From 1e057628371de7975aebdc40039210cbc23d5525 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sat, 5 Dec 2020 23:39:09 -0800 Subject: [PATCH 6/8] Use BAZEL_SUFFIX instead --- site/docs/user-manual.html | 2 +- src/test/shell/BUILD | 1 + src/test/shell/bin/bazel | 7 ------- src/test/shell/bin/bazel_nojdk | 21 +++++++++++++++++++ src/test/shell/integration/BUILD | 14 ++++++++++--- .../integration/nojdk_startup_options_test.sh | 5 +---- 6 files changed, 35 insertions(+), 15 deletions(-) create mode 100755 src/test/shell/bin/bazel_nojdk diff --git a/site/docs/user-manual.html b/site/docs/user-manual.html index fd7e683b035863..d98598de0fb425 100644 --- a/site/docs/user-manual.html +++ b/site/docs/user-manual.html @@ -2838,7 +2838,7 @@

--host_jvm_debug

--autodetect_server_javabase

This option causes Bazel to automatically search for an installed JDK on startup, - and to fall back to the installed JRE if the embedded JRE isn't available. + and to fall back to the installed JRE if the embedded JRE isn't available. --explicit_server_javabase can be used to pick an explicit JRE to run bazel with.

diff --git a/src/test/shell/BUILD b/src/test/shell/BUILD index 5f6735ba01b863..c6b2693d4d0939 100644 --- a/src/test/shell/BUILD +++ b/src/test/shell/BUILD @@ -5,6 +5,7 @@ package(default_visibility = ["//visibility:private"]) exports_files([ "bin/bazel", "bin/bazel_jdk_minimal", + "bin/bazel_nojdk", "testenv.sh", "integration_test_setup.sh", "sandboxing_test_utils.sh", diff --git a/src/test/shell/bin/bazel b/src/test/shell/bin/bazel index de4191c81b019f..aba1ed9635d1a8 100755 --- a/src/test/shell/bin/bazel +++ b/src/test/shell/bin/bazel @@ -18,13 +18,6 @@ # affect all our integration tests. # bazel_bin=$(rlocation io_bazel/src/bazel) - -# One test uses bazel_nojdk instead. If bazel_bin is empty, try that. -if [[ -z "${bazel_bin}" ]]; -then - bazel_bin=$(rlocation io_bazel/src/bazel_nojdk) -fi - # unset runfiles environment so that the "inner bazel" # has to detect its own runfiles unset RUNFILES_DIR diff --git a/src/test/shell/bin/bazel_nojdk b/src/test/shell/bin/bazel_nojdk new file mode 100755 index 00000000000000..6db26634d57b3b --- /dev/null +++ b/src/test/shell/bin/bazel_nojdk @@ -0,0 +1,21 @@ +#!/bin/bash +# +# Copyright 2018 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Wrapper script to run bazel in the tests. Any change to this file will +# affect all our integration tests. +# +exec $(rlocation io_bazel/src/bazel_nojdk) \ + --bazelrc=$TEST_TMPDIR/bazelrc "$@" diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD index b8d7d776a58c0f..373b288dbf8b14 100644 --- a/src/test/shell/integration/BUILD +++ b/src/test/shell/integration/BUILD @@ -26,6 +26,16 @@ filegroup( ], ) +filegroup( + name = "test-deps-nojdk", + testonly = 1, + srcs = [ + "//src:bazel-bin_nojdk", + "//src/test/shell:bin/bazel_nojdk", + "//src/test/shell/bazel:test-deps-wo-bazel", + ], +) + sh_test( name = "progress_reporting_test", size = "large", @@ -190,9 +200,7 @@ sh_test( size = "medium", srcs = ["nojdk_startup_options_test.sh"], data = [ - "//src:bazel_nojdk", - "//src/test/shell:bin/bazel", - "//src/test/shell/bazel:test-deps-wo-bazel", + ":test-deps-nojdk", "@bazel_tools//tools/bash/runfiles", ], ) diff --git a/src/test/shell/integration/nojdk_startup_options_test.sh b/src/test/shell/integration/nojdk_startup_options_test.sh index ed31fdd2579b8d..5cba7b6bfdad08 100755 --- a/src/test/shell/integration/nojdk_startup_options_test.sh +++ b/src/test/shell/integration/nojdk_startup_options_test.sh @@ -38,10 +38,7 @@ else fi # --- end runfiles.bash initialization --- -# We don't want to use the cached, extracted bazel. It will have a different -# sha1 and fail the test. The 2 version commands below are cheap. -unset TEST_INSTALL_BASE - +export BAZEL_SUFFIX="_nojdk" source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } From a035c2c3ed4995660fcdc8416a5c9f7b2aab9a89 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sun, 6 Dec 2020 00:01:24 -0800 Subject: [PATCH 7/8] Unset TEST_INSTALL_BASE --- src/test/shell/integration/nojdk_startup_options_test.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/shell/integration/nojdk_startup_options_test.sh b/src/test/shell/integration/nojdk_startup_options_test.sh index 5cba7b6bfdad08..3332d85865199f 100755 --- a/src/test/shell/integration/nojdk_startup_options_test.sh +++ b/src/test/shell/integration/nojdk_startup_options_test.sh @@ -38,6 +38,10 @@ else fi # --- end runfiles.bash initialization --- +# We don't want to use the cached, extracted bazel. It will have a different +# sha1 and fail the test. The 2 version commands below are cheap. +unset TEST_INSTALL_BASE + export BAZEL_SUFFIX="_nojdk" source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } From 9a42e7b2a40537252e9f88ede4f1d0004e08b075 Mon Sep 17 00:00:00 2001 From: Austin Schuh Date: Sun, 6 Dec 2020 00:17:04 -0800 Subject: [PATCH 8/8] Skip windows --- src/test/shell/integration/BUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD index 373b288dbf8b14..7aea04a20340f6 100644 --- a/src/test/shell/integration/BUILD +++ b/src/test/shell/integration/BUILD @@ -203,6 +203,8 @@ sh_test( ":test-deps-nojdk", "@bazel_tools//tools/bash/runfiles", ], + # Windows doesn't support sandboxing, which BAZEL_SUFFIX needs. + tags = ["no_windows"], ) sh_test(