Skip to content

Commit

Permalink
Improve error handling for missing/invalid server JVM.
Browse files Browse the repository at this point in the history
Tell the user if the error was caused by a bad --server_javabase option, and
return a standard exit code.

PiperOrigin-RevId: 264957762
  • Loading branch information
tetromino authored and copybara-github committed Aug 23, 2019
1 parent e749677 commit 166efd6
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 21 deletions.
83 changes: 66 additions & 17 deletions src/main/cpp/startup_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -492,16 +492,19 @@ string StartupOptions::GetEmbeddedJavabase() const {
return "";
}

string StartupOptions::GetServerJavabase() const {
std::pair<string, StartupOptions::JavabaseType>
StartupOptions::GetServerJavabaseAndType() const {
// 1) Allow overriding the server_javabase via --server_javabase.
if (!explicit_server_javabase_.empty()) {
return explicit_server_javabase_;
return std::pair<string, JavabaseType>(explicit_server_javabase_,
JavabaseType::EXPLICIT);
}
if (default_server_javabase_.empty()) {
if (default_server_javabase_.first.empty()) {
string bundled_jre_path = GetEmbeddedJavabase();
if (!bundled_jre_path.empty()) {
// 2) Use a bundled JVM if we have one.
default_server_javabase_ = bundled_jre_path;
default_server_javabase_ = std::pair<string, JavabaseType>(
bundled_jre_path, JavabaseType::EMBEDDED);
} else {
// 3) Otherwise fall back to using the default system JVM.
string system_javabase = GetSystemJavabase();
Expand All @@ -510,19 +513,67 @@ string StartupOptions::GetServerJavabase() const {
<< "Could not find system javabase. Ensure JAVA_HOME is set, or "
"javac is on your PATH.";
}
default_server_javabase_ = system_javabase;
default_server_javabase_ = std::pair<string, JavabaseType>(
system_javabase, JavabaseType::SYSTEM);
}
}
return default_server_javabase_;
}

string StartupOptions::GetServerJavabase() const {
return GetServerJavabaseAndType().first;
}

string StartupOptions::GetExplicitServerJavabase() const {
return explicit_server_javabase_;
}

string StartupOptions::GetJvm() const {
string java_program =
blaze_util::JoinPath(GetServerJavabase(), GetJavaBinaryUnderJavabase());
auto javabase_and_type = GetServerJavabaseAndType();
blaze_exit_code::ExitCode sanity_check_code =
SanityCheckJavabase(javabase_and_type.first, javabase_and_type.second);
if (sanity_check_code != blaze_exit_code::SUCCESS) {
exit(sanity_check_code);
}
return blaze_util::JoinPath(javabase_and_type.first,
GetJavaBinaryUnderJavabase());
}

// Prints an appropriate error message and returns an appropriate error exit
// code for a server javabase which failed sanity checks.
static blaze_exit_code::ExitCode BadServerJavabaseError(
StartupOptions::JavabaseType javabase_type,
const std::map<string, string> &option_sources) {
switch (javabase_type) {
case StartupOptions::JavabaseType::EXPLICIT: {
auto source = option_sources.find("server_javabase");
string rc_file;
if (source != option_sources.end() && !source->second.empty()) {
rc_file = source->second;
}
BAZEL_LOG(ERROR)
<< " The java path was specified by a '--server_javabase' option " +
(rc_file.empty() ? "on the command line" : "in " + rc_file);
return blaze_exit_code::BAD_ARGV;
}
case StartupOptions::JavabaseType::EMBEDDED:
BAZEL_LOG(ERROR) << " Internal error: embedded JDK fails sanity check.";
return blaze_exit_code::INTERNAL_ERROR;
case StartupOptions::JavabaseType::SYSTEM:
return blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR;
default:
BAZEL_LOG(ERROR)
<< " Internal error: server javabase type was not initialized.";
// Fall through.
}
return blaze_exit_code::INTERNAL_ERROR;
}

blaze_exit_code::ExitCode StartupOptions::SanityCheckJavabase(
const std::string &javabase,
StartupOptions::JavabaseType javabase_type) const {
std::string java_program =
blaze_util::JoinPath(javabase, GetJavaBinaryUnderJavabase());
if (!blaze_util::CanExecuteFile(java_program)) {
if (!blaze_util::PathExists(java_program)) {
BAZEL_LOG(ERROR) << "Couldn't find java at '" << java_program << "'.";
Expand All @@ -531,27 +582,25 @@ string StartupOptions::GetJvm() const {
<< "' exists but is not executable: "
<< blaze_util::GetLastErrorString();
}
exit(1);
return BadServerJavabaseError(javabase_type, option_sources);
}
// If the full JDK is installed
string jdk_rt_jar =
blaze_util::JoinPath(GetServerJavabase(), "jre/lib/rt.jar");
string jdk_rt_jar = blaze_util::JoinPath(javabase, "jre/lib/rt.jar");
// If just the JRE is installed
string jre_rt_jar = blaze_util::JoinPath(GetServerJavabase(), "lib/rt.jar");
string jre_rt_jar = blaze_util::JoinPath(javabase, "lib/rt.jar");
// rt.jar does not exist in java 9+ so check for java instead
string jre_java = blaze_util::JoinPath(GetServerJavabase(), "bin/java");
string jre_java_exe =
blaze_util::JoinPath(GetServerJavabase(), "bin/java.exe");
string jre_java = blaze_util::JoinPath(javabase, "bin/java");
string jre_java_exe = blaze_util::JoinPath(javabase, "bin/java.exe");
if (blaze_util::CanReadFile(jdk_rt_jar) ||
blaze_util::CanReadFile(jre_rt_jar) ||
blaze_util::CanReadFile(jre_java) ||
blaze_util::CanReadFile(jre_java_exe)) {
return java_program;
return blaze_exit_code::SUCCESS;
}
BAZEL_LOG(ERROR) << "Problem with java installation: couldn't find/access "
"rt.jar or java in "
<< GetServerJavabase();
exit(1);
<< javabase;
return BadServerJavabaseError(javabase_type, option_sources);
}

string StartupOptions::GetExe(const string &jvm, const string &jar_path) const {
Expand Down
34 changes: 30 additions & 4 deletions src/main/cpp/startup_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ struct RcStartupFlag {
// which displays the defaults). The actual defaults are defined
// in the constructor.
//
// Note that this class is not thread-safe.
//
// TODO(bazel-team): The encapsulation is not quite right -- there are some
// places in blaze.cc where some of these fields are explicitly modified. Their
// names also don't conform to the style guide.
Expand Down Expand Up @@ -188,8 +190,23 @@ class StartupOptions {
// Returns the embedded JDK, or an empty string.
std::string GetEmbeddedJavabase() const;

// Returns the GetHostJavabase. This should be called after parsing
// the --server_javabase option.
// The source of truth for the server javabase.
enum class JavabaseType {
UNKNOWN,
// An explicit --server_javabase startup option.
EXPLICIT,
// The embedded JDK.
EMBEDDED,
// The default system JVM.
SYSTEM
};

// Returns the server javabase and its source of truth. This should be called
// after parsing the --server_javabase option.
std::pair<std::string, JavabaseType> GetServerJavabaseAndType() const;

// Returns the server javabase. This should be called after parsing the
// --server_javabase option.
std::string GetServerJavabase() const;

// Returns the explicit value of the --server_javabase startup option or the
Expand Down Expand Up @@ -256,6 +273,14 @@ class StartupOptions {
const char *arg, const char *next_arg, const std::string &rcfile,
const char **value, bool *is_processed, std::string *error) = 0;

// Checks whether the given javabase contains a java executable and runtime.
// On success, returns blaze_exit_code::SUCCESS. On error, prints an error
// message and returns an appropriate exit code with which the client should
// terminate.
blaze_exit_code::ExitCode SanityCheckJavabase(
const std::string &javabase,
StartupOptions::JavabaseType javabase_type) const;

// Returns the absolute path to the user's local JDK install, to be used as
// the default target javabase and as a fall-back host_javabase. This is not
// the embedded JDK.
Expand Down Expand Up @@ -304,8 +329,9 @@ class StartupOptions {
// The server javabase as provided on the commandline.
std::string explicit_server_javabase_;

// The server javabase to be used (computed lazily).
mutable std::string default_server_javabase_;
// The default server javabase to be used and its source of truth (computed
// lazily). Not guarded by a mutex - StartupOptions is not thread-safe.
mutable std::pair<std::string, JavabaseType> default_server_javabase_;

// Startup flags that don't expect a value, e.g. "master_bazelrc".
// Valid uses are "--master_bazelrc" are "--nomaster_bazelrc".
Expand Down
1 change: 1 addition & 0 deletions src/test/shell/integration/bazel_javabase_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ function test_fallback_depot_javabase() {
fi
if ! bazel --batch --server_javabase=$path version >& $TEST_log; then
expect_log "Couldn't find java at"
expect_log "--server_javabase"
expect_not_log "Problem with java installation"
fi
}
Expand Down

0 comments on commit 166efd6

Please sign in to comment.