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.

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

No way exists to ignore a host JDK #12451

Closed
AustinSchuh opened this issue Nov 11, 2020 · 10 comments
Closed

No way exists to ignore a host JDK #12451

AustinSchuh opened this issue Nov 11, 2020 · 10 comments
Labels
area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: feature request

Comments

@AustinSchuh
Copy link
Contributor

We have developers who accidentally (or on purpose) install different JDK versions. I want to be able to tell Bazel to never ever look at those.

In src/main/cpp/blaze_util_linux.cc

string GetSystemJavabase() {
  // if JAVA_HOME is defined, then use it as default.
  string javahome = GetPathEnv("JAVA_HOME");
  if (!javahome.empty()) {
    string javac = blaze_util::JoinPath(javahome, "bin/javac");
    if (access(javac.c_str(), X_OK) == 0) {
      return javahome;
    }   
    BAZEL_LOG(WARNING)
        << "Ignoring JAVA_HOME, because it must point to a JDK, not a JRE.";
  }

  // which javac
  string javac_dir = Which("javac");
  if (javac_dir.empty()) {
    return ""; 
  }

  // Resolve all symlinks.
  char resolved_path[PATH_MAX];
  if (realpath(javac_dir.c_str(), resolved_path) == NULL) {
    return ""; 
  }
  javac_dir = resolved_path;

  // dirname dirname
  return blaze_util::Dirname(blaze_util::Dirname(javac_dir));
}

This tries pretty hard to find a JDK. I'd like an environmental variable, reserved value (/dev/null?) or flag or something which I can pass in to return "", ie no JDK found. (I'm also happy to implement it if there is consensus.)

@comius
Copy link
Contributor

comius commented Nov 20, 2020

If you prevent one JDK selection, which JDK should be selected instead?

If you know absolute path to the preferred JDK, you can put local_java_repository(name = 'local_jdk', java_home = '...') into the workspace.

You can also use JDK from a remote repository for execution, by setting build --javabase=//tools/jdk:remote_jdk11 in .bazelrc.

@comius comius added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository and removed untriaged labels Nov 20, 2020
@AustinSchuh
Copy link
Contributor Author

AustinSchuh commented Nov 20, 2020

We have the following in our WORKSPACE file.

http_archive_architecture(
    name = "openjdk_linux_archive",
    build_file_content = """
java_runtime(
    name = 'jdk',
    srcs = glob(['**']),
    visibility = ['//visibility:public'],
)
""",
    sha256 = "ddb0fd4526089cf1ce2db36282c282263f587a9e8be373fa02f511a12923cc48",
    strip_prefix = "zulu11.31.11-ca-jdk11.0.3-linux_x64",
    urls = [
            "https://artifactory.bluerivertech.com/artifactory/dev-bazel-local/dependencies/zulu11.31.11-ca-jdk11.0.3-linux_x64.tar.gz",
            "https://mirror.bazel.build/openjdk/azul-zulu11.31.11-ca-jdk11.0.3/zulu11.31.11-ca-jdk11.0.3-linux_x64.tar.gz",
    ],
)

Followed by the following in our .bazelrc:

# Use a hermetic Java.
build --javabase=@openjdk_linux_archive//:jdk --host_javabase=@openjdk_linux_archive//:jdk

The goal being to have Bazel start up with the embedded JDK, and then use the hermetic JDK from there.

I've got a local patch which replaces that function with return ""; and everything works great, but that's obviously not upstream-able.

@comius
Copy link
Contributor

comius commented Nov 21, 2020

If I understand correctly your configuration is already using embedded JDK for bazel and hermetic JDK from there, even if you don't patch the function.

The call to that function is made in startup_options.cc:486. If a patched version was executed it would result in BAZEL_DIE call.

Or is there another loophole?

@AustinSchuh
Copy link
Contributor Author

If the patched version is used, we follow the if statement on 461 with an embedded jdk.

453 std::pair<blaze_util::Path, StartupOptions::JavabaseType>
454 StartupOptions::GetServerJavabaseAndType() const {
455   // 1) Allow overriding the server_javabase via --server_javabase.
456   if (!explicit_server_javabase_.IsEmpty()) {
457     return std::pair<blaze_util::Path, JavabaseType>(explicit_server_javabase_,
458                                                      JavabaseType::EXPLICIT);
459   }
460   if (default_server_javabase_.first.IsEmpty()) {
461     blaze_util::Path bundled_jre_path = GetEmbeddedJavabase();
462     if (!bundled_jre_path.IsEmpty()) {
463       // 2) Use a bundled JVM if we have one.
464       default_server_javabase_ = std::pair<blaze_util::Path, JavabaseType>(
465           bundled_jre_path, JavabaseType::EMBEDDED);
466     } else {
467       // 3) Otherwise fall back to using the default system JVM.
468       blaze_util::Path system_javabase = GetSystemJavabase();
469       if (system_javabase.IsEmpty()) {
470         BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
471             << "Could not find system javabase. Ensure JAVA_HOME is set, or "
472                "javac is on your PATH.";
473       }
474       default_server_javabase_ = std::pair<blaze_util::Path, JavabaseType>(
475           system_javabase, JavabaseType::SYSTEM);
476     }
477   }
478   return default_server_javabase_;
479 }

So we follow 2), the embedded JDK, or die if 3) is taken.

If you look in src/main/cpp/blaze.cc, you'll see:

 460   result.push_back("--default_system_javabase=" + GetSystemJavabase());
 461  

Essentially, we pass the default system javabase in to Bazel regardless of if there is an embedded JDK.

I'm pretty certain one of the bazel versions I was using was built without the embedded JDK. The end result without the patch was that it auto-detected the host JDK instead of failing. This was resulting in random crashes of bazel which were hard to debug. I'd like a way to make bazel explode instead of falling back, catching the BAZEL_DIE call that you pointed out.

Options are things like:
Add a NO_LOCAL_JDK environmental variable and return "".
Make JAVA_HOME == "/dev/null" return "".

Or something else. I'm happy to plumb up whatever interface folks prefer. We have a tools/bazel script which sanitizes the environment and versions bazel, so any of these are an option.

@comius
Copy link
Contributor

comius commented Nov 23, 2020

I'd go for a startup command line option "--autodetect_server_javabase", type boolean with default value true.
When set to false "Bazel would be prevented to start with local JDK". Startup JDK autodetection would be skipped and Bazel would only work if either --server_javabase is set or this is a version of Bazel bundled with embedded JDK.

(Rationale: Ensure stable Bazel invocation on uncontrolled developer workstations)

@lberki, @philwo What do you think?

@lberki
Copy link
Contributor

lberki commented Nov 23, 2020

A startup option is not a great idea in any case, because changing it incurs a server restart, but it doesn't need to be since there are plenty of places to prevent the discovered javabase from getting into the value of DEFAULT_SYSTEM_JAVABASE in the WORKSPACE file by a non-startup command line option.

Other than that, I don't have a very strong opinion; Ivo knows more about JVM/JRE selection so he can make a better decision.

@comius
Copy link
Contributor

comius commented Nov 23, 2020

A startup option is not a great idea in any case, because changing it incurs a server restart.

In this case this isn't a problem. The issue is particularly about not running Bazel using local JDK.

@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) labels Nov 23, 2020
@AustinSchuh
Copy link
Contributor Author

Thanks! I'll put something together.

@AustinSchuh
Copy link
Contributor Author

PR sent! That's the functionality I would like. Happy to adjust as you see fit.

@AustinSchuh
Copy link
Contributor Author

I'm realizing you might be waiting for me to make the tests pass, and I'm waiting for feedback to see if the approach looks reasonable before fixing the last tests. Could I get a quick look?

meisterT pushed a commit that referenced this issue Jan 12, 2021
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

Closes #12542.

PiperOrigin-RevId: 347411720
ulfjack pushed a commit to EngFlow/bazel that referenced this issue Mar 5, 2021
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: bazelbuild#12451

Closes bazelbuild#12542.

PiperOrigin-RevId: 347411720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-java-toolchains javabase, java_toolchain flags, JDK selection, java_toolchain rules, java_tools repository P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Java Issues for Java rules type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants