Repository: bazelbuild/bazel
Issue: Only use JAVA_HOME/@local_jdk as the default --javabase
Labels: type: feature request

### Description of the problem / feature request:

Since 849df36c5ad31ebe8791c4228321c38c6d0ae56c, bazel will always pick up a JDK from `which javac` or $JAVA_HOME in preference to an embedded one for `@local_jdk//`. I want to always use the embedded one instead. A startup option to do this would be fine.

### Feature requests: what underlying problem are you trying to solve with this feature?

I version the bazel binary in my repository. I want this to fix the version of the JDK being used too, like it did before.

### What operating system are you running Bazel on?

Debian Jessie amd64

### What's the output of `bazel info release`?

release 0.15.0-peloton-201807111305+9c52124

This is 0.15.0 with a few local patches that aren't relevant.

###  Have you found anything relevant by searching the web?

@cushon changed this.

Issue comments:
To confirm, is the request to support using the embedded JDK as the `--javabase`?

If the goal is to have `--javabase` versioned with your respository, have you considered versioning a separate JDK?
Not quite. What I'm really looking for is a way to completely eliminate uses of any JDKs found via `which javac` or `$JAVA_HOME` or any other form of looking around on the system. Experimenting with `--javabase=@embedded_jdk//:jdk --host_javabase=@embedded_jdk//:jdk` just now, builds still fail on a machine with a weird (broken?) JDK exposed via `which javac` because that JDK is missing jar files in places bazel expects them.

I would also like a way to use the embedded JDK as the alternative so I have less stuff to version, but I wouldn't mind versioning a separate one via a bazel package too much.

As far as I can see, there are no user-accessible flags to change `@local_jdk`. src/main/cpp/blaze.cc always passes its own value for `--default_system_javabase`, and then everything else just uses that value. If there was some way to do that, I think it would be enough for my use case. I guess I could make my tools/bazel wrapper script download a fixed JDK, extract it, and set JAVA_HOME to point to it, but that seems like an awful lot of infrastructure to add to emulate something that used to just work. It's also a super trivial 1-line patch to make `StartupOptions::GetSystemJavabase()` return an empty string, and seems like it wouldn't be much harder to control that via a flag.
> > To confirm, is the request to support using the embedded JDK as the --javabase?
>
> Not quite. What I'm really looking for is a way to completely eliminate uses of any JDKs found via which javac or $JAVA_HOME or any other form of looking around on the system.

To try to recap, there are three JDKs:
1) the JDK used to run Bazel itself. This is the `startup --host_javabase` (i.e. the LHS host_javabase)
2) the JDK used to run host tools that run during the build, e.g. javac. This is the `build --host_javabase` (i.e. the RHS host_javabase)
3) the JDK that the build is targeting, that is used to run any `java_binary` and `java_tests` that a build produces. This is the `--javabase`.

(1) and (2) now default to the embedded JDK (which is not the local JDK). They should be unaffected by `JAVA_HOME` and the presence of `javac` on the path.

(3) is the one that depends on `JAVA_HOME` / `javac`. You can set it explicitly using `--javabase`. `--javabase=@embedded_jdk//:jdk` doesn't work because Bazel doesn't support using JDK 9 as a target `--javabase` yet.

If you obtain a separate JDK 8, create a `java_runtime` rule and set the `java_runtime.java_home` attribute to the root of that JDK 8, and then set `--javabase` to the label of the `java_runtime` rule, that should guarantee that Bazel's behaviour is unaffected by `JAVA_HOME` etc.

Does that match what you're seeing?

> I would also like a way to use the embedded JDK as the alternative so I have less stuff to version

This makes sense, but it isn't really the intended use-case for the embedded JDK. There's some discussion about shipping a minimal JDK that contains only what's need for the host_javabase use-case, which probably isn't what's wanted fro a target `--javabase`. It would be less fragile to explicitly choose the JDK version to target instead of using whatever version Bazel happens to be currently using.
@philwo @buchgr FYI

There's some discussion happening about making it possible to configure host JDKs using remote repositories or similar, and it would probably be possible to use the same approach for target JDKs. So if you want the ability to version the host and target JDKs and avoid implicitly depending on the local JDK that'd be another way to approach it.

This original feature request (of using the embedded JDK as a target JDK) is not something we're likely to pursue for the reason I mentioned above: we want to make the embedded JDK image as a small as possible to reduce the installation size.
@cushon
Are there tickets for these discussions (depending on an external versioned
jdk for host/target)?
On Thu, 26 Jul 2018 at 17:30 Liam Miller-Cushon <notifications@github.com>
wrote:

> @philwo <https://github.com/philwo> @buchgr <https://github.com/buchgr>
> FYI
>
> There's some discussion happening about making it possible to configure
> host JDKs using remote repositories or similar, and it would probably be
> possible to use the same approach for target JDKs. So if you want the
> ability to version the host and target JDKs and avoid implicitly depending
> on the local JDK that'd be another way to approach it.
>
> This original feature request (of using the embedded JDK as a target JDK)
> is not something we're likely to pursue for the reason I mentioned above:
> we want to make the embedded JDK image as a small as possible to reduce the
> installation size.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/5594#issuecomment-408137102>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABUIFxQaUNJTbTzOh8lWcW80q_ySjISJks5uKeCsgaJpZM4VPYjF>
> .
>

@ittaiz https://groups.google.com/d/topic/bazel-sig-jvm/noXlM1HOvVY/discussion
Even if I set --javabase and --host_javabase, builds still don't work on a machine where `which javac` finds a broken JDK. Is there some other flag that also needs to be set to completely avoid the system-installed JDK from affecting builds?
Specifically, with this WORKSPACE:
```python
new_http_archive(
    name = "zulu_jdk8",
    build_file = "zulu_jdk8.BUILD",
    sha256 = "65a1b180ad46c5a1cc2f9ffff29b85d682c0e937b7b964b557f2a3b9084de5b9",
    strip_prefix = "zulu8.30.0.1-jdk8.0.172-linux_x64",
    url = "https://cdn.azul.com/zulu/bin/zulu8.30.0.1-jdk8.0.172-linux_x64.tar.gz",
)
```
And this BUILD:
```python
genrule(
    name = "gen_main",
    outs = ["Main.java"],
    cmd = "echo \"class Main { public static void main(String[] args) {} }\" > $@",
)

java_binary(
    name = "main",
    srcs = ["Main.java"],
    main_class = "Main",
)
```

This works (I'm doing this on a machine with a working JDK found via `which javac`):
```console
$ /tmp/bazel-0.15.2-linux-x86_64 build //...
INFO: Build options have changed, discarding analysis cache.
INFO: Analysed 2 targets (16 packages loaded).
INFO: Found 2 targets...
INFO: Elapsed time: 5.201s, Critical Path: 2.85s
INFO: 2 processes: 1 linux-sandbox, 1 worker.
INFO: Build completed successfully, 7 total actions
```
But this fails:
```console
$ JAVA_HOME=/nonexistent /tmp/bazel-0.15.2-linux-x86_64 build //...
WARNING: Running Bazel server needs to be killed, because the startup options are different.
Starting local Bazel server and connecting to it...
.....................
ERROR: /x1/home/brian/.cache/bazel/_bazel_brian/9a14994a41429a580e5cd8b6f1b379ce/external/bazel_tools/tools/jdk/BUILD:181:1: no such package '@local_jdk//': Expected directory at /nonexistent but it does not exist. and referenced by '@bazel_tools//tools/jdk:jdk'
ERROR: Analysis of target '//:main' failed; build aborted: Analysis failed
INFO: Elapsed time: 6.264s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (10 packages loaded)
    currently loading: @bazel_tools//src/tools/launcher ... (2 packages)
```
> Even if I set --javabase and --host_javabase, builds still don't work on a machine where which javac finds a broken JDK.

Are you setting the `--host_javabase` build flag, or startup flag, or both?
I was using the build flag because I didn't realize there was a startup one. What version of JDK is supported for the startup one? If I use a Java 8 one, it breaks because `src/main/cpp/blaze.cc` says `TODO(b/109998449): only assume JDK >= 9 for embedded JDKs`, and then adds a Java 9-only flag for passing to the JVM found via the `--host_javabase` startup flag. If I use a Java 9 one, then compilation via the `java_*` rules breaks because they don't support Java 9 JDKs yet. Neither the `--host_javabase` or `--javabase` build flags seem to make any difference whatsoever because `@local_jdk` fails to load either way.
> If I use a Java 9 one, then compilation via the java_* rules breaks because they don't support Java 9 JDKs yet.

The default `startup --host_javabase` is the embedded JDK, which is a JDK 10 in Bazel at head. It was JDK 9 until recently, and JDK 9 should still work.

JDK 9 isn't supported as a target `--javabase` yet.

If you use `bazel --host_javabase=<jdk 9> build ...` that should work. If it doesn't, what error are you seeing?
Oh, I was testing the wrong bazel binary... `bazel --host_javabase=<jdk 9> build ...` works with 0.6.0, but setting `JAVA_HOME=/dev/null/nonexistent` still breaks it. Same error as before:
```console
ERROR: /x1/home/brian/.cache/bazel/_bazel_brian/efaa4e6bb8c9b0585e183b73359e618d/external/bazel_tools/tools/jdk/BUILD:149:1: no such package '@local_jdk//': Expected directory at /dev/null/nonexistent but it does not exist. and referenced by '@bazel_tools//tools/jdk:bootclasspath'
```
@buchgr @meisterT FYI. I think this is exactly the bug I found when we discussed ~2 weeks ago, which `--{host_,}javabase` flag influences what part of the build: Namely, the `@local_jdk` repo, which is *not* set to the path given by `--javabase` (which would be the right behavior), but is always taken from the JAVA_HOME environment variable (or calculated from `which javac`) and cannot be influenced at all via flags.

Here's the current set of flags and what they do:

- `bazel --host_javabase=... build ...` = Defines what JDK Bazel itself runs under.
- `bazel build --host_javabase=...` = Defines the JDK that Bazel uses to run Java tools that run on your machine as part of the build (e.g. JavaBuilder, SingleJar and friends).
- `bazel build --javabase=...` = Defines what Bazel puts into the launcher script of java_binary and java_test targets as their Java runtime. So, if you run `bazel test --javabase=//my:jdk //:javatest`, then `//my:jdk` would be used to run the test.
- `$JAVA_HOME` = what is made available as `@local_jdk` and used by JavaBuilder as the classpath to build against.

AFAIU the proposed fix is to merge the last two and make `bazel build --javabase=$JAVA_HOME` the default, so that people who have a JDK installed can just go and build stuff, but when you explicitly set `bazel build --javabase=//my:jdk`, then that would also redirect `@local_jdk` to `//my:jdk` and your system JDK (if it exists) would be completely ignored.

(@cushon Please correct me if I'm wrong!)

@philwo that all sounds right to me.

If I'm following, the goal should be that `@local_jdk` is used only as the default value of `--javabase`, and that all other uses of it (e.g. in `@bazel_tools//tools/jdk:bootclasspath` should be migrated to use `current_java_runtime`). By default the host-config `current_java_runtime` will still be `@local_jdk`, but changing `--javabase` will be sufficient to avoid using `JAVA_HOME` anywhere.
That sounds like it would work for my use case. Any ETA on implementing it? I'm currently stuck not being able to roll out a new bazel release with other helpful bugfixes and new features due to this.
cc @buchgr 

> That sounds like it would work for my use case. Any ETA on implementing it?

I'm re-opening this bug to track breaking dependencies on `JAVA_HOME` / `@local_jdk` except as the default value of `--javabase`. I'm hoping that will be ready 'soon', but Bazel team may have other ideas to mitigate this issue.
This is fixed at head, and the fix will be included in the upcoming 0.17 release.
Almost.  https://github.com/bazelbuild/bazel/issues/6341
In case someone else gets confused about how to refer to the java toolchain from genrules etc., please have a look here: https://stackoverflow.com/questions/53066974/how-can-i-use-the-jar-tool-with-bazel-v0-19
copying jdk tools.jar introduced bugs. refer to, https://github.com/bazelbuild/intellij/issues/2164

Pull request review comments:

Patch that resolved the issue:
diff --git a/tools/jdk/BUILD b/tools/jdk/BUILD
index 8598fdfa9a72ed..ff52e25b1f49c6 100644
--- a/tools/jdk/BUILD
+++ b/tools/jdk/BUILD
@@ -176,8 +176,12 @@ genrule(
     cmd = """
 set -eu
 TMPDIR=$$(mktemp -d -t tmp.XXXXXXXX)
-$(JAVABASE)/bin/javac $< -d $$TMPDIR
-$(JAVA) -cp $$TMPDIR DumpPlatformClassPath $@
+$(JAVABASE)/bin/javac -source 8 -target 8 \
+    -cp $(JAVABASE)/lib/tools.jar \
+    -d $$TMPDIR $<
+$(JAVA) -XX:+IgnoreUnrecognizedVMOptions \
+    --add-exports=jdk.compiler/com.sun.tools.javac.platform=ALL-UNNAMED \
+    -cp $$TMPDIR DumpPlatformClassPath $@
 rm -rf $$TMPDIR
 """,
     toolchains = ["@bazel_tools//tools/jdk:current_host_java_runtime"],
diff --git a/tools/jdk/DumpPlatformClassPath.java b/tools/jdk/DumpPlatformClassPath.java
index 1e97fb052dab76..c81ab5d50ca51a 100644
--- a/tools/jdk/DumpPlatformClassPath.java
+++ b/tools/jdk/DumpPlatformClassPath.java
@@ -12,11 +12,17 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import static java.util.Comparator.comparing;
+
+import com.sun.tools.javac.api.JavacTool;
+import com.sun.tools.javac.util.Context;
 import java.io.BufferedOutputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.UncheckedIOException;
 import java.net.URI;
-import java.nio.file.DirectoryStream;
 import java.nio.file.FileSystems;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
@@ -24,11 +30,20 @@
 import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.GregorianCalendar;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
 import java.util.jar.JarOutputStream;
 import java.util.zip.CRC32;
 import java.util.zip.ZipEntry;
+import javax.tools.JavaFileManager;
+import javax.tools.JavaFileObject;
+import javax.tools.JavaFileObject.Kind;
+import javax.tools.StandardLocation;
 
 /**
  * Output a jar file containing all classes on the platform classpath of the current JDK.
@@ -37,42 +52,93 @@
  */
 public class DumpPlatformClassPath {
 
-  public static void main(String[] args) throws IOException {
+  public static void main(String[] args) throws Exception {
     if (args.length != 1) {
       System.err.println("usage: DumpPlatformClassPath <output jar>");
       System.exit(1);
     }
     Path output = Paths.get(args[0]);
-    Path path = Paths.get(System.getProperty("java.home"));
-    if (path.endsWith("jre")) {
-      path = path.getParent();
+
+    Map<String, byte[]> entries = new HashMap<>();
+
+    // JDK 8 bootclasspath handling
+    Path javaHome = Paths.get(System.getProperty("java.home"));
+    if (javaHome.endsWith("jre")) {
+      javaHome = javaHome.getParent();
     }
-    Path rtJar = path.resolve("jre/lib/rt.jar");
-    if (Files.exists(rtJar)) {
-      Files.copy(rtJar, output);
-      return;
+    for (String jar :
+        Arrays.asList("rt.jar", "resources.jar", "jsse.jar", "jce.jar", "charsets.jar")) {
+      Path path = javaHome.resolve("jre/lib").resolve(jar);
+      if (!Files.exists(path)) {
+        continue;
+      }
+      try (JarFile jf = new JarFile(path.toFile())) {
+        jf.stream()
+            .forEachOrdered(
+                entry -> {
+                  try {
+                    entries.put(entry.getName(), toByteArray(jf.getInputStream(entry)));
+                  } catch (IOException e) {
+                    throw new UncheckedIOException(e);
+                  }
+                });
+      }
     }
-    Path modules = FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules");
+
+    if (entries.isEmpty()) {
+      // JDK > 8 bootclasspath handling
+
+      // Set up a compilation with --release 8 to initialize a filemanager
+      Context context = new Context();
+      JavacTool.create()
+          .getTask(null, null, null, Arrays.asList("--release", "8"), null, null, context);
+      JavaFileManager fileManager = context.get(JavaFileManager.class);
+
+      for (JavaFileObject fileObject :
+          fileManager.list(
+              StandardLocation.PLATFORM_CLASS_PATH,
+              "",
+              EnumSet.of(Kind.CLASS),
+              /* recurse= */ true)) {
+        String binaryName =
+            fileManager.inferBinaryName(StandardLocation.PLATFORM_CLASS_PATH, fileObject);
+        entries.put(
+            binaryName.replace('.', '/') + ".class", toByteArray(fileObject.openInputStream()));
+      }
+
+      // Include the jdk.unsupported module for compatibility with JDK 8.
+      // (see: https://bugs.openjdk.java.net/browse/JDK-8206937)
+      // `--release 8` only provides access to supported APIs, which excludes e.g. sun.misc.Unsafe.
+      Path module =
+          FileSystems.getFileSystem(URI.create("jrt:/")).getPath("modules/jdk.unsupported");
+      Files.walkFileTree(
+          module,
+          new SimpleFileVisitor<Path>() {
+            @Override
+            public FileVisitResult visitFile(Path path, BasicFileAttributes attrs)
+                throws IOException {
+              String name = path.getFileName().toString();
+              if (name.endsWith(".class") && !name.equals("module-info.class")) {
+                entries.put(module.relativize(path).toString(), Files.readAllBytes(path));
+              }
+              return super.visitFile(path, attrs);
+            }
+          });
+    }
+
     try (OutputStream os = Files.newOutputStream(output);
         BufferedOutputStream bos = new BufferedOutputStream(os, 65536);
         JarOutputStream jos = new JarOutputStream(bos)) {
-      try (DirectoryStream<Path> modulePaths = Files.newDirectoryStream(modules)) {
-        for (Path modulePath : modulePaths) {
-          Files.walkFileTree(
-              modulePath,
-              new SimpleFileVisitor<Path>() {
-                @Override
-                public FileVisitResult visitFile(Path path, BasicFileAttributes attrs)
-                    throws IOException {
-                  String name = path.getFileName().toString();
-                  if (name.endsWith(".class") && !name.equals("module-info.class")) {
-                    addEntry(jos, modulePath.relativize(path).toString(), Files.readAllBytes(path));
-                  }
-                  return super.visitFile(path, attrs);
+      entries.entrySet().stream()
+          .sorted(comparing(Map.Entry::getKey))
+          .forEachOrdered(
+              entry -> {
+                try {
+                  addEntry(jos, entry.getKey(), entry.getValue());
+                } catch (IOException e) {
+                  throw new UncheckedIOException(e);
                 }
               });
-        }
-      }
     }
   }
 
@@ -91,4 +157,17 @@ private static void addEntry(JarOutputStream jos, String name, byte[] bytes) thr
     jos.putNextEntry(je);
     jos.write(bytes);
   }
+
+  private static byte[] toByteArray(InputStream is) throws IOException {
+    byte[] buffer = new byte[8192];
+    ByteArrayOutputStream boas = new ByteArrayOutputStream();
+    while (true) {
+      int r = is.read(buffer);
+      if (r == -1) {
+        break;
+      }
+      boas.write(buffer, 0, r);
+    }
+    return boas.toByteArray();
+  }
 }


Test patch: