Java 9 should be supported

[dfabulich]

java_toolchain requires a bootstrap classpath, including rt.jar, but Java 9 no longer has rt.jar or anything like that.

java_toolchain has to be rethought for Java 9 support

[lberki]

Sadly, this will have to wait until we get proper support for Java 9, which means quite a bit of work internally.

/cc @cushon

[davido]

@lberki can you quantify "quite a bit of work" in term of time frame? Google is usually years behind bumping its internal Java tool chain to very recent JDK after the day it was released. Final Java 9 release was previously scheduled to July 2017 and was postponed to September 2017. My expectation would be that the day, Java 9 final is released, Bazel supports it. Your comment sounds like, it could happen, that Bazel users would have to wait for months (or years?) for that to happen.

[lberki]

It will happen and definitely not within years, but also probably not sooner than months. It's just another thing we have to fit into our schedule and unfortunately, it's not amenable to being done by non-Googler contributors since we need to make sure that it integrates well with our internal repository. We can shuffle around priorities if there is good reason but my best understanding is that Java 9 is not a hard blocker for anyone, at least for the time being.

[iirina]

Hi all,

I posted a question about the interest in java 9 on bazel-discuss. It also contains an update on when we might support it. Can you comment there about your expectations? Thanks!

[shs96c]

I'd echo @davido's comment above, and @dfabulich's experience: bazel must, at the very least, be able to run on a system with java 9 installed, and that needs to happen ASAP.

Compiling to java 9 is less urgent. Early Q1 2018 might be okay, but end of Q1 is too long --- I want to start taking advantage of new language features RSN. I guess I could specify a java_toolchain to make this work as a hack, but I'd need to be able to do that on a per-clone basis (so I can work on my Mac and have CI on a linux box)

Actually taking advantage of java modules would be a killer feature, but given the slow adoption of java 8, it's the least pressing feature.

[cushon]

bazel must, at the very least, be able to run on a system with java 9 installed

@shs96c does this mean supporting blaze build --host_javabase=<jdk9> ..., or something else?

[dfabulich]

Something else. If you export JAVA_HOME=, Bazel refuses to start, even when you install a version of Bazel that embeds its own JVM.

-Dan

On Sep 25, 2017, at 6:46 PM, Liam Miller-Cushon notifications@github.com wrote:

bazel must, at the very least, be able to run on a system with java 9 installed

@shs96c https://github.com/shs96c does this mean supporting blaze build --host_javabase= ..., or something else?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazel/issues/3410#issuecomment-332061970, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF3lhlicrZcj9wSmx8sGJyFL9jMzhR1ks5smFeHgaJpZM4Ob8zC.

[davido]

I think this diff should fix Bazel start:

diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 27c3d7184..fdfb3c8e2 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -460,17 +460,8 @@ string StartupOptions::GetJvm() {
     }
     exit(1);
   }
-  // If the full JDK is installed
-  string jdk_rt_jar = blaze_util::JoinPath(GetHostJavabase(), "jre/lib/rt.jar");
-  // If just the JRE is installed
-  string jre_rt_jar = blaze_util::JoinPath(GetHostJavabase(), "lib/rt.jar");
-  if (blaze_util::CanReadFile(jdk_rt_jar) ||
-      blaze_util::CanReadFile(jre_rt_jar)) {
-    return java_program;
-  }
-  fprintf(stderr, "Problem with java installation: "
-      "couldn't find/access rt.jar in %s\n", GetHostJavabase().c_str());
-  exit(1);
+  // TODO(davido): Find out a better way to check whether full JDK installed.
+  return java_program;
 }

The next failure if I try to use JDK9 with custom built bazel to compile one single java source file is:

$ /home/davido/projects/bazel/bazel-bin/src/bazel build :printy_lib --verbose_failures
INFO: Found 1 target...
ERROR: missing input file '@local_jdk//:jre/lib/rt.jar'.
ERROR: Process terminated by signal 15.

I think, that this is defined here: https://github.com/bazelbuild/bazel/blob/master/tools/jdk/BUILD#L110-L116.

BOOTCLASS_JARS = [
    "rt.jar",
    "resources.jar",
    "jsse.jar",
    "jce.jar",
    "charsets.jar",
]

If I remove them:

diff --git a/tools/jdk/BUILD b/tools/jdk/BUILD
index 5c8baac96..af80081ea 100644
--- a/tools/jdk/BUILD
+++ b/tools/jdk/BUILD
@@ -107,13 +107,7 @@ filegroup(
     srcs = ["//tools/jdk:VanillaJavaBuilder_deploy.jar"],
 )

-BOOTCLASS_JARS = [
-    "rt.jar",
-    "resources.jar",
-    "jsse.jar",
-    "jce.jar",
-    "charsets.jar",
-]
+BOOTCLASS_JARS = []

 alias(
     name = "bootclasspath",

and rebuild bazel with JDK8, and try again to use JDK8 to build one single java source, I get this failure now:

$ /home/davido/projects/bazel/bazel-bin/src/bazel build :printy_lib --verbose_failures
.................
INFO: Found 1 target...
ERROR: /home/davido/projects/bazel_printy/BUILD:1:1: Building libprinty_lib.jar (1 source file) failed: IOException while borrowing a worker from the pool:

---8<---8<--- Exception details ---8<---8<---
java.io.IOException: Cannot run program "/home/davido/.cache/bazel/_bazel_davido/1d26503054d6b6be1850338cefd090db/execroot/__main__/external/local_jdk/bin/java" (in directory "/home/davido/.cache/bazel/_bazel_davido/1d26503054d6b6be1850338cefd090db/execroot/__main__"): error=2, No such file or directory
    at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1128)
    at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1071)
    at com.google.devtools.build.lib.worker.Worker.createProcess(Worker.java:83)
    at com.google.devtools.build.lib.worker.WorkerFactory.create(WorkerFactory.java:69)
    at com.google.devtools.build.lib.worker.WorkerFactory.create(WorkerFactory.java:30)
    at org.apache.commons.pool2.BaseKeyedPooledObjectFactory.makeObject(BaseKeyedPooledObjectFactory.java:62)
    at org.apache.commons.pool2.impl.GenericKeyedObjectPool.create(GenericKeyedObjectPool.java:1036)
    at org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:356)
    at org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:278)
    at com.google.devtools.build.lib.worker.WorkerPool.borrowObject(WorkerPool.java:41)
    at com.google.devtools.build.lib.worker.WorkerSpawnRunner.execInWorker(WorkerSpawnRunner.java:259)
    at com.google.devtools.build.lib.worker.WorkerSpawnRunner.actuallyExec(WorkerSpawnRunner.java:155)
    at com.google.devtools.build.lib.worker.WorkerSpawnRunner.exec(WorkerSpawnRunner.java:110)
    at com.google.devtools.build.lib.exec.AbstractSpawnStrategy.exec(AbstractSpawnStrategy.java:96)
    at com.google.devtools.build.lib.exec.AbstractSpawnStrategy.exec(AbstractSpawnStrategy.java:64)
    at com.google.devtools.build.lib.analysis.actions.SpawnAction.internalExecute(SpawnAction.java:259)
    at com.google.devtools.build.lib.analysis.actions.SpawnAction.execute(SpawnAction.java:266)
    at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.executeActionTask(SkyframeActionExecutor.java:850)
    at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.prepareScheduleExecuteAndCompleteAction(SkyframeActionExecutor.java:793)
    at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.access$900(SkyframeActionExecutor.java:107)
    at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$ActionRunner.call(SkyframeActionExecutor.java:661)
    at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$ActionRunner.call(SkyframeActionExecutor.java:618)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.executeAction(SkyframeActionExecutor.java:404)
    at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.checkCacheAndExecuteIfNeeded(ActionExecutionFunction.java:440)
    at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.compute(ActionExecutionFunction.java:201)
    at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:338)
    at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:352)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
    at java.base/java.lang.Thread.run(Thread.java:844)
Caused by: java.io.IOException: error=2, No such file or directory
    at java.base/java.lang.ProcessImpl.forkAndExec(Native Method)
    at java.base/java.lang.ProcessImpl.<init>(ProcessImpl.java:339)
    at java.base/java.lang.ProcessImpl.start(ProcessImpl.java:270)
    at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1107)
    ... 30 more
---8<---8<--- End of exception details ---8<---8<---.
Target //:printy_lib failed to build
INFO: Elapsed time: 4.101s, Critical Path: 0.24s

To reproduce, bazel_printy is here: [1]. The CL is here: [2].

• [1] https://github.com/davido/bazel_printy
• [2] https://bazel-review.googlesource.com/17350

[ilovezfs]

What is the status here? I'm not eager to use wrapper scripts on bazel in Homebrew but we will if we have to.

[davido]

As mentioned in my previous comments: it will take years, for Java 9 to be supported in Bazel.

[ilovezfs]

In Homebrew, we're doing this:

iMac-TMP:~ joe$ cat `which bazel`
#!/bin/bash
JAVA_HOME="$(/usr/libexec/java_home --version 1.8+)" exec "/usr/local/Cellar/bazel/0.7.0_1/libexec/bin/bazel" "$@"

iMac-TMP:~ joe$ cat `which bazel-real`
#!/bin/bash
JAVA_HOME="$(/usr/libexec/java_home --version 1.8+)" exec "/usr/local/Cellar/bazel/0.7.0_1/libexec/bin/bazel-real" "$@"
iMac-TMP:~ joe$

It would be nice if this were something bazel could handle internally since I don't think it makes sense to force every downstream to use wrapper scripts just so it doesn't break on systems that have both Java 8 and Java 9 installed.

[cheister]

@davido Can you look at https://github.com/bazelbuild/bazel/pull/4045 it should solve some of @ilovezfs issue.

[kamalmarhubi]

Can we separate out the issues of bazel compiling Java 9 source from bazel running under jvm 9? This seems to cover both, but it seems like it should be possible to run bazel with embedded jdk under jvm 9. I propose reopening #3789 to track the running under jvm 9 issue.

[hhclam]

I have gotten a bazel build that works for both JDK8 and JDK9. I created the fork is here: https://github.com/hhclam/bazel/commits/java9. The fork has only one commit with very little code change: https://github.com/hhclam/bazel/commit/64212c8026df9fc6361d5af8414acc373e221955.

It's fair to say that it is tricky but Bazel is mostly capable to run in both JDKs. If you build bazel from source with a JDK8 with the fork, e.g.: bazel build src:bazel

The resultant bazel binary should work with both JDK8 and JDK9. For JDK9 specifically you have to run Bazel with: bazel build //something --host_java_toolchain=@bazel_tools//tools/jdk:jdk9 --java_toolchain=@bazel_tools//tools/jdk:jdk9

Worker process works with this toolchain. However Turbine (java header compilation) is disabled. The neat things like strict deps and error prone are still intact.

The major non-code change in this commit is to define a new toolchain called "jdk9" which does not have -Xbootclasspath/p and does not use the javac that is bundled with Bazel. It could work with the Bazel-bundled javac since it is a build from OpenJDK 9 but it's confusing. I went even further to stripping the javac (com.sun) classes from the deploy jar to provide a new JavaBuilder without javac and JDK internal classes.

The minor code change is to suppress warnings from jigsaw. The major code change is to accept --patch-module, --add-modules and --add-exports for "javacopts" for java_* and "misc" for java_toolchain. This is needed because a lot of existing code uses JDK internals.

This patch should work with a simple Java application. I was able to build a large-ish swagger application with this patch. The real pain being jigsaw complaining about javax.annotation and jsr305. This affects a lot of generated code including grpc (which uses javax.annotation.Generated). The solution is to create your own java_toolchain (make a copy from the jdk9 toolchain) and then add --patch-module and --add-modules to patch the javax.annotation module with the jsr305 jar.

[davido]

@hhclam Thanks, this is very promising!

I started to use your Bazel patch to build Gerrit Code Review, on Java 9, but it didn't really worked. So far I discovered two problems:

• Error Prone reported false positive
• Auto-Value refused to work (even after upgrading to latest relase 1.5.2)

For the former problem:

ERROR: /home/davido/projects/gerrit2/java/com/google/gerrit/common/BUILD:32:1: Building java/com/google/gerrit/common/libserver.jar (57 source files) failed (Exit 1)
java/com/google/gerrit/common/RawInputUtil.java:34: error: [CheckReturnValue] Ignored return value of method that is annotated with @CheckReturnValue
    Preconditions.checkNotNull(bytes);
                              ^
    (see http://errorprone.info/bugpattern/CheckReturnValue)
  Did you mean to remove this line?
[...]
ERROR: /home/davido/projects/gerrit2/java/com/google/gerrit/util/cli/BUILD:1:1: Building java/com/google/gerrit/util/cli/libcli.jar (6 source files) failed (Exit 1)
java/com/google/gerrit/util/cli/CmdLineParser.java:229: error: [CheckReturnValue] Ignored return value of method that is annotated with @CheckReturnValue
        map.put(ent.getKey(), val);
               ^
    (see http://errorprone.info/bugpattern/CheckReturnValue)
  Did you mean to remove this line?
Target //java/com/google/gerrit/server:server failed to build

I filed this issue: https://github.com/google/error-prone/issues/850, and was able to turn off EP/Bazel integration with javacopt -XepDisableAllChecks" option for now.

For the latter auto-value integration problem: NoClassDefFoundError: javax/annotation/Generated, I haven't found a workaround so far, and filed this issue upstream: https://github.com/google/auto/issues/560. The only way I managed to move forward is to include javax.annotation:javax.annotation-api:1.3.1 as java_plugin dependency:

java_plugin(
    name = "auto-annotation-plugin",
    processor_class = "com.google.auto.value.processor.AutoAnnotationProcessor",
    deps = [
        ":javax-annotation",
        "@auto_value//jar",
    ],
)

but then annotation processor was failing with a NPE on an innocent @AutoAnnotation line of code.

I extracted this reproducer from gerrit and verified, that the reproducer can be built with Java 9, with Maven's compiler-plugin 3.7.0. So that this seems to be a Bazel related issue.

[hhclam]

@davido The issue you are hitting is not Bazel specific but caused by jigsaw. However there's no easy way to solve it in Bazel.

This commit is getting a bit hacky by patching the offending module for all java invocation https://github.com/hhclam/bazel/commit/776d7b6a5ce20d3ec503c3d03d6b4ae67f89c72c

I tested your example that this Bazel build passed. However you'll probably hit another issue with the javax.annotation package. This thread has a lot of relavent information: https://github.com/google/dagger/issues/880

I think to summarize this is: a lot of 3rd party libraries have issue with javax.annotation (potentially other packages as well) due to jigsaw and Bazel provides no way to workaround that easily.

[davido]

@hhclam At least auto-value specific problem can be easily avoided by adding dependency on javax.annotation-api to java_plugin: [1].

The next issue, as you noticed, is gerrit -> rules_closure -> dager related: [2]. Gerrit has two UIs: GWT and JS, so I disabled JS UI for now: [3] and was able to build GWT UI based gerrit with Java 9. This is already not bad, besides that, trying to initialize with Java 9 built gerrit site is failing with NPE: [4].

• [1] http://paste.openstack.org/show/628312
• [2] http://paste.openstack.org/show/628314
• [3] http://paste.openstack.org/show/628315
• [4] http://paste.openstack.org/show/628316

[hhclam]

The NPE issue is with auto-value I believe. I don't know what version of gerrit you're building as the latest revision seems to being using it's own Nullable. You might find more helpful information here: https://github.com/google/auto/issues/283. This is another issue with JDK9 caused by jigsaw and javax.annotation.

[davido]

It worth noting, that almost all tests are failing in gerrit, on java 9 with the same NPE above: [1].

• [1] http://paste.openstack.org/show/628328

[davido]

The NPE issue is with auto-value I believe.

@hhclam correct. Auto-Value is broken on Java 9. That's because auto-value doesn't respect Nullable annotation on Java 9: https://github.com/google/auto/issues/562 and probably related https://github.com/google/auto/issues/546, the source Nullable annotation is lost on Java 9:

[...]
  @Nullable
  public abstract String rule();

Thus, the generated code: [1] lost the Nullable, and does this instead on Java 9:

@Override
    public SubmitRuleOptions.Builder rule(String rule) {
      if (rule == null) {
        throw new NullPointerException("Null rule");
      }
      this.rule = rule;
      return this;
    }

While auto-value on Java 8 was generating sane code:

 @Nullable
  @Override
  public String rule() {
    return rule;
  }

So that this valid code in gerrit:

public static Builder builder() {
    return new AutoValue_SubmitRuleOptions.Builder()
        .allowClosed(false)
        .skipFilters(false)
        .rule(null);
  }

Is ending up with this NPE:

Type                           [lucene/?]: 
Exception in thread "main" java.lang.ExceptionInInitializerError
    at com.google.gerrit.server.index.change.ChangeSchemaDefinitions.<clinit>(ChangeSchemaDefinitions.java:25)
    at com.google.gerrit.server.index.IndexModule.<clinit>(IndexModule.java:78)
    at com.google.gerrit.pgm.init.InitIndex.run(InitIndex.java:69)
    at com.google.gerrit.pgm.init.SitePathInitializer.run(SitePathInitializer.java:93)
    at com.google.gerrit.pgm.init.BaseInit.run(BaseInit.java:138)
    at com.google.gerrit.pgm.util.AbstractProgram.main(AbstractProgram.java:61)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:564)
    at com.google.gerrit.launcher.GerritLauncher.invokeProgram(GerritLauncher.java:204)
    at com.google.gerrit.launcher.GerritLauncher.mainImpl(GerritLauncher.java:109)
    at com.google.gerrit.launcher.GerritLauncher.main(GerritLauncher.java:64)
    at Main.main(Main.java:24)
Caused by: java.lang.NullPointerException: Null rule
    at com.google.gerrit.server.project.AutoValue_SubmitRuleOptions$Builder.rule(AutoValue_SubmitRuleOptions.java:102)
    at com.google.gerrit.server.project.SubmitRuleOptions.builder(SubmitRuleOptions.java:32)

• [1] http://paste.openstack.org/show/628332/

[hhclam]

To summarize the findings:

1. Bazel can run with Java 9 and build with JDK 9 (without the need of bundled javac) with some small tweaks to java_toolchain. However Java header compilation does not work.
2. Bazel needs some new features or new attributes to java_library / java_binary to allow patching of modules to workaround jigsaw issues with existing code.
3. A lot of existing Java libraries don't work well with Java 9. This is another hurdle for Java 9 support.

[cushon]

However Java header compilation does not work.

Can you share details about this part? It should work, Bazel might just need to be updated to the latest version.

[hhclam]

@cushon This is the only error: Fatal Error: Unable to find package java.lang in classpath or bootclasspath

[cushon]

Unable to find package java.lang in classpath or bootclasspath

Thanks. That indicates java_toolchain.bootclasspath is not configured correctly. That attribute needs to be re-thought for Java 9 in light of --release (see JEP-247).

[hhclam]

Thanks for the quick response. Ideally if Bazel can pick the correct toolchain with some repository rule magic just like the internal cc_configure() to pick the correct java_toolchain that will be great.

[davido]

Bazel needs some new features or new attributes to java_library / java_binary to allow patching of modules to workaround jigsaw issues with existing code.

I wouldn't call it "to workaround jigsaw issues", may be "to reflect the jigsaw module concept"?

Trying to build JGit, that gerrit depends on, is failing on Java 9 with:

$ bazel build org.eclipse.jgit:jgit --host_java_toolchain=@bazel_tools//tools/jdk:jdk9 --java_toolchain=@bazel_tools//tools/jdk:jdk9

org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkEncryption.java:69: error: package javax.xml.bind is not visible
import javax.xml.bind.DatatypeConverter;
                ^
  (package javax.xml.bind is declared in module java.xml.bind, which is not in the module graph)

So that there probably should be modules attribute (or similar) for java_library:

java_library(
    name = "jgit",
    srcs = SRCS,
    resource_strip_prefix = "org.eclipse.jgit/resources",
    resources = RESOURCES,
    deps = [
        ":insecure_cipher_factory",
        "//lib:javaewah",
        "//lib:jsch",
        "//lib:slf4j-api",
    ],
    modules = [
        "java.xml.bind",
    ],
)

I wonder if there should be a design proposal for all possible extensions to java build rules to reflect Jigsaw module concept? Any volunteers to write one?

[cushon]

So that there probably should be modules attribute (or similar) for java_library:

Can you use javacopts to set --add-modules=java.xml?

[davido]

Can you use javacopts to set --add-modules=java.xml?

Unfortunately, BazelJavaBuilder doesn't seem to accept it:

$ bazel build org.eclipse.jgit:jgit --javacopt="--add-modules=java.xml.bind"  --host_java_toolchain=@bazel_tools//tools/jdk:jdk9 --java_toolchain=@bazel_tools//tools/jdk:jdk9
INFO: Analysed target //org.eclipse.jgit:jgit (0 packages loaded).
INFO: Found 1 target...
ERROR: /home/davido/projects/jgit/org.eclipse.jgit/BUILD:27:1: Building org.eclipse.jgit/libinsecure_cipher_factory.jar (1 source file) failed (Exit 1)
BazelJavaBuilder threw exception: unknown option : '--add-modules=java.xml.bind'
Target //org.eclipse.jgit:jgit failed to build

If I omit -- to bypass bazel CLI parsing, then compiler rejects it:

$ bazel build org.eclipse.jgit:jgit --javacopt="add-modules=java.xml.bind" --host_java_toolchain=@bazel_tools//tools/jdk:jdk9 --java_toolchain=@bazel_tools//tools/jdk:jdk9
INFO: Analysed target //org.eclipse.jgit:jgit (0 packages loaded).
INFO: Found 1 target...
ERROR: /home/davido/projects/jgit/org.eclipse.jgit/BUILD:27:1: Building org.eclipse.jgit/libinsecure_cipher_factory.jar (1 source file) failed (Exit 1)
java.lang.IllegalArgumentException: invalid flag: add-modules=java.xml.bind
    at jdk.compiler/com.sun.tools.javac.main.Arguments.error(Arguments.java:917)
    at jdk.compiler/com.sun.tools.javac.main.Arguments.doProcessArgs(Arguments.java:415)
    at jdk.compiler/com.sun.tools.javac.main.Arguments.processArgs(Arguments.java:367)
    at jdk.compiler/com.sun.tools.javac.main.Arguments.init(Arguments.java:246)
    at jdk.compiler/com.sun.tools.javac.api.JavacTool.getTask(JavacTool.java:185)
    at com.google.devtools.build.buildjar.javac.BlazeJavacMain.compile(BlazeJavacMain.java:93)
    at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder$1.invokeJavac(SimpleJavaLibraryBuilder.java:105)
    at com.google.devtools.build.buildjar.ReducedClasspathJavaLibraryBuilder.compileSources(ReducedClasspathJavaLibraryBuilder.java:54)
    at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.compileJavaLibrary(SimpleJavaLibraryBuilder.java:108)
    at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.run(SimpleJavaLibraryBuilder.java:116)
    at com.google.devtools.build.buildjar.BazelJavaBuilder.processRequest(BazelJavaBuilder.java:105)
    at com.google.devtools.build.buildjar.BazelJavaBuilder.runPersistentWorker(BazelJavaBuilder.java:67)
    at com.google.devtools.build.buildjar.BazelJavaBuilder.main(BazelJavaBuilder.java:45)
Target //org.eclipse.jgit:jgit failed to build

[cushon]

Ah, right:

https://github.com/bazelbuild/bazel/blob/c1914135477581163560c00a3a728d37ad0ea846/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java#L108

[davido]

Thanks for the pointer, this restriction should be re-considered and should be easily fixable for Java 9?

[davido]

Bazel needs some new features or new attributes to java_library / java_binary to allow patching of modules to workaround jigsaw issues with existing code.

I wouldn't call it "to workaround jigsaw issues", may be "to reflect the jigsaw module concept"?

If we want that jigsaw modules are first class citizens in Bazel, we should consider to add java_module rule: [1]. Among other, one challenge here is allow to provide the content for module-info.java in flexible ways, to not end up needing to put module-info.java file at the root of the module’s source-file hierarchy:

The source code for a module declaration is, by convention, placed in a file
named module-info.java at the root of the module’s source-file hierarchy.

As this would be a contradiction to Bazel concept to have the whole sources in flattened tree:

java/com/google/gerrit/api
java/com/google/gerrit/http
java/com/google/gerrit/server
java/com/google/gerrit/ssh
[...]

[1] http://openjdk.java.net/projects/jigsaw/spec/sotms/#module-declarations

[ittaiz]

David, When I started thinking about jigsaw and Bazel I actually had some hopes of leveraging the targets granularity, explicitness and visibility declarations to automatically generate java modules. Wdyt about that approach (as opposed to a new rules)? On Fri, 8 Dec 2017 at 13:14 David Ostrovsky notifications@github.com wrote:

Bazel needs some new features or new attributes to java_library / java_binary to allow patching of modules to workaround jigsaw issues with existing code.

I wouldn't call it "to workaround jigsaw issues", may be "to reflect the jigsaw module concept"?

If we want that modules are first class citizens in Bazel, we should consider to add java_module rule: [1]. Among other, one challenge here is allow to provide the content for module-info.java in flexible ways, to not end up needing to put module-info.java file in root of the module:

`` The source code for a module declaration is, by convention, placed in a file named module-info.java at the root of the module’s source-file hierarchy.

As this would be a contradiction to Bazel concept to have the whole sources in flattened tree:

java/com/google/gerrit/api
java/com/google/gerrit/http
java/com/google/gerrit/ssh
java/com/google/gerrit/server

[1]
http://openjdk.java.net/projects/jigsaw/spec/sotms/#module-declarations

—
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/3410#issuecomment-350238822>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF6sf0VBSfPPUuWCbMjtc_iKhncjaks5s-Rn4gaJpZM4Ob8zC>
.

[davido]

Wdyt about that approach (as opposed to a new rules)?

That's exactly the question here.

See @cushon 's previous comment. Do we really want to abuse javacopts in java_library and produce such mess in BUILD file like this: https://github.com/gradle/gradle-java-modules/blob/master/src/main/java/org/gradle/java/JigsawPlugin.java#L124-L135?

[hhclam]

@davido The fork added --add-modules support so you should be able to use it in javacopts in java_library. https://github.com/hhclam/bazel/commit/64212c8026df9fc6361d5af8414acc373e221955#diff-1246b7472359b0d744101200bb63d70cR211 If it doesn't work then there might be something I missed. This I think is a only a short term solution.

I think @ittaiz has a good point. If java_library can be translated automatically to a module then Bazel can hide a lot of the ugliness under JavaBuilder. And java_library can map to a (module, package) pair directly. The exception being multiple java_library rules building the same (module, package) pair for legacy code. I think having module and package attributes in java_library will be a good starting point.

[davido]

If it doesn't work then there might be something I missed.

I cherry-picked your change on Bazel@HEAD and added this Error Prone upgrade CL on top of it. It doesn't work. I tried both: passing from the command line and from java_library rule and got the same error. I can debug it later today.

I think having module and package attributes in java_library will be a good starting point.

I doubt this would be sufficient. For example, I could imagine to provide content of module description. Or to point to a file that is located in my current package java/foo/bar/baz/BUILD, say java/foo/bar/baz/module-info.java and should be re-located at the root of the module’s source-file hierarchy in the final module artifact, see my previous comment, why. Of course this and other funny stuff cannot be done by vanilla java_library. Another option would be to provide Skylark macro java_module that handles all this.

[davido]

@hhclam I looked deeper into your patch. OptionsParser.java part of it doesn't seem to be correct. For reasons, @cushon pointed out, the current code assumes that javacopts options must not start with "--".

As we are talking about a temporary hack anyway, I moved the Jigsaw options that you introduced to a different place, and assumed, that they are starting with single dash now:

• -patch-module
• -add-exports
• -add-modules

Now, when I provide something like: javacopts = ["-add-modules=java.xml.bind"], e.g.:

java_library(
    name = "jgit",
    srcs = SRCS,
    resource_strip_prefix = "org.eclipse.jgit/resources",
    resources = RESOURCES,
    deps = [
        ":insecure_cipher_factory",
        "//lib:javaewah",
        "//lib:jsch",
        "//lib:slf4j-api",
    ],
    javacopts = ["-add-modules=java.xml.bind"],
)

I prepend the missing dash to recognized Jigsaw options and now it works as expected. Here is my patch applied on top of your fork: [1].

BTW, I fixed auto-value issue, that is still using legacy Generated annotation and switched to using the one that presents on Java 9: [2].

• [1] http://paste.openstack.org/show/628484
• [2] https://github.com/google/auto/pull/563

[davido]

With some tweaks, passing these JVM options:

diff --git a/javatests/com/google/gerrit/acceptance/tests.bzl b/javatests/com/google/gerrit/acceptance/tests.bzl
index 4b3b802ded..4ec8b2e5e0 100644
--- a/javatests/com/google/gerrit/acceptance/tests.bzl
+++ b/javatests/com/google/gerrit/acceptance/tests.bzl
@@ -16,6 +16,9 @@ def acceptance_tests(
       'slow',
     ],
     size = "large",
-    jvm_flags = vm_args,
+    jvm_flags = vm_args + [
+      "--add-modules java.activation",
+      "--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED"
+    ],
     **kwargs
   )

almost all Gerrit tests are pasing now on Java 9. One exception: DateTime.format() method seems to behaive differently on Java 9. The same problem was reported in Gson project here: [1].

I created this reproducer and was able to verify, that this problem exists on both Bazel and Maven (tests are passing on Java 8, but failing on Java 9):

• [1] http://paste.openstack.org/show/628522

[cheister]

This is because in Java 9 they changed the default locale data to use Unicode Common Locale Data Repository (CLDR) https://docs.oracle.com/javase/9/intl/internationalization-enhancements-jdk-9.htm#JSINT-GUID-9DCDB41C-A989-4220-8140-DBFB844A0FCA If you pass -Djava.locale.providers=COMPAT,CLDR,SPI to the jvm then you get the JDK 8 behavior.

[davido]

@cheister Great, thank you!

With this diff:

diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 2eaf4f50d4..454eba8180 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -54,4 +54,7 @@ junit_tests(
         "//lib/jgit/org.eclipse.jgit:jgit",
         "//lib/jgit/org.eclipse.jgit.junit:junit",
     ],
+    # Enforce JDK 8 compatibility, see
+    # https://docs.oracle.com/javase/9/intl/internationalization-enhancements-jdk-9.htm#JSINT-GUID-AF5AECA7-07C1-4E7D-BC10-BC7E73DC6C7F
+    jvm_flags = ["-Djava.locale.providers=COMPAT,CLDR,SPI"],
 )

all Gerrit tests: [1] are now passing on Java 9 ;-)

[1] http://paste.openstack.org/show/628547

[davido]

To summarize, here is the Gerrit series: [1], with links to PRs, to fix transtitve dependencies, to support Java 9:

• auto-value
• dagger 2
• rules_closure

Stil unsolved, Java 9 adjustements are way too intrusive and are not backward compatible. Trying to run the tests with Java 8 is failing with:

Unrecognized option: --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

Or:

$ bazel test javatests/com/google/gerrit/acceptance/api/change:api_change
Unrecognized option: --add-modules
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

We should find a way to conditionally specify jvm_flags and javacopts based on used java_toolchan, so that Java 9 specific stuff is only added on JDK9 toolchain.

• [1] https://gerrit-review.googlesource.com/c/gerrit/+/147234

[davido]

We should find a way to conditionally specify jvm_flags and javacopts based on used java_toolchan, so that Java 9 specific stuff is only added on JDK9 toolchain.

Figured out how to do it and thus make Java 9 support backwards compatible. Add config setting based on --java_toolchain option:

config_setting(
    name = "jdk9",
    values = {
        "java_toolchain": "@bazel_tools//tools/jdk:jdk9",
    },
) 

And specify jvm_flags and javacopts options based on it:

 junit_tests(
    name = group,
    deps = deps + [
      '//java/com/google/gerrit/acceptance:lib',
    ],
    tags = labels + [
      'acceptance',
      'slow',
    ],
    size = "large",
    jvm_flags = vm_args + select({
      "//:jdk9": [
        "--add-modules java.activation",
        "--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED",
      ],
      "//conditions:default": [],
    }),
    **kwargs
  )

[hhclam]

I think: Bazel should pick the java_toolchain with a repository rule based on the version of java from @local_jdk//:java. The repository rule should also set up the config so flags can be added manually with select like @davido is doing.

[davido]

Bazel should pick the java_toolchain with a repository rule based on the version of java from @local_jdk//:java.

By repository rule you mean something that I would need to put in WORKSPACE file of my project? Can't Bazel be smart enough to do it on its own? I'm switching to different Java version system wide, what Bazel needs more to pick up the right builtin java toolchain and setup config option?

Say I switched to Java_x system wide on multiple builtin java toolchain awared Bazel version, then I would expect, that I neither have to provide --host_java_toolchain=java_x --java_toolchain=java_x nor to set up config_setting(name = "java_x", [...] in BUILD file. Bazel should do all this automagically and

[...]
foo = select({
      "//:java_x": [
        "--bar",
        "--baz",
      ],
      "//conditions:default": [],
    }),

should just work.

[hhclam]

What I mean is Bazel can have built-in repository rule similar to cc_configure and be smart to pick the right toolchain so you don't have to. We're talking about the same thing.

[cushon]

@hhclam - thanks for investigating!

There's been some progress. Simple examples should work at head with a JDK 9 --host_javabase (both the startup option and the build option). Java 9 language support (--release 9, etc.) is still work in progress.

• 5c16ab2378befcd8000bf4f95da596ada04ae8c6 adds some magic to select a java_toolchain based on the left-hand-side --host_javabase:

$ bazel --host_javabase=$JAVA9_HOME query --output build //external:java_toolchain
bind(
  name = "java_toolchain",
  actual = "@bazel_tools//tools/jdk:toolchain_jdk9",
)
$ bazel --host_javabase=$JAVA8_HOME query --output build //external:java_toolchain
bind(
  name = "java_toolchain",
  actual = "@bazel_tools//tools/jdk:toolchain_jdk8",
)

• 29a96822a3bc5a81094285a995d905a074ef5714 adds a temporary work-around to avoid breaking java_toolchain.bootclasspath when running on JDK 9

• c2fd539dfedbb4291f7d9fcace54b806bbb3b328 fixes the issue with assuming a jre/ subdirectory exists

[davido]

@cushon Thanks, this is very impressive, indeed!

You haven't mentioned, but it seems that with https://github.com/bazelbuild/bazel/commit/94402893b0a935b69aa9cbb5b025c4a47220ea58 javacopts with '--' like '--add-modules' are supported now?

With this upgrade series (only bump of auto-value and rules_closure: [1]) I'm able to build gerrit: [2] with vanila bazel@HEAD (26b7ea6a37961ce993afe510d0ff8c1b5ecb30bc) and all tests are passing: [3]

  $ /home/davido/projects/bazel/bazel-bin/src/bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build gerrit
  [...]
  Target //:gerrit up-to-date:
  bazel-bin/gerrit.war
INFO: Elapsed time: 240.706s, Critical Path: 186.73s
INFO: Build completed successfully, 348 total actions

One problem/question: We need to conditionally pass some Java 9 specific jvm option, in java tests rules, and switching on java toolchain. I expected, that passing host_javabase as startup option sould be enough for that config option to work?

config_setting(
    name = "jdk9",
    values = {
        "java_toolchain": "@bazel_tools//tools/jdk:toolchain_jdk9",
    },
)

And then say this in java_test invocation:

[...]
  jvm_flags = vm_args + select({
      "//:jdk9": [
        "--add-modules java.activation",
        "--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED",
      ],
      "//conditions:default": [],
    }),
[...]

Unfortunately this doesn't seem to work with --host_javabase only, I also need to pass --java-toolchain:

  $ /home/davido/projects/bazel/bazel-bin/src/bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk test --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 javatests/...
[...]

Needles to say, I would like to avoid passing host_javabase, java_toolchain and defining :jdk9 with config_setting in gerrit build altogether. The fact that I'm using Java 9 should be sufficient to switch. Is this going to be supported?

• [1] https://gerrit-review.googlesource.com/#/c/gerrit/+/147234/
• [2] http://paste.openstack.org/show/629545
• [3] http://paste.openstack.org/show/629548

[hhclam]

@cushon You're are very welcome! This is great progress!

[cushon]

@davido - the default value of --java_toolchain doesn't change, just the target that //external:java_toolchain points to. So there's no way to select on that.

Eventually JDK 9 is going to be the default host_javabase so you won't have to pass an explicit flag. It isn't necessary to set the host_javabase for builds targeting an older version of the platform. Instead, those builds should set --java_toolchain and --javabase, which you also can select on to add different jvm flags. Does that help?

[davido]

Instead, those builds should set --java_toolchain and --javabase, which you also can select on to add different jvm flags. Does that help?

@cushon Yes. Thanks for clarification.

[davido]

@cushon It seems, that 9440289 didn't dix the '--' prefix for javacopts, e.g.: ["--add-modules=java.xml.bind"].

So, trying to build this JGit Java 9 aware CL: [1] , is failing on Bazel@HEAD, with:

  $ bazel --host_javabase=/usr/lib64/jvm/java-9-openjdk build --java_toolchain=@bazel_tools//tools/jdk:toolchain_jdk9 :all
ERROR: /home/davido/projects/jgit/org.eclipse.jgit/BUILD:14:1: Building org.eclipse.jgit/libjgit-class.jar (808 source files) failed (Exit 1)
BazelJavaBuilder threw exception: unknown option : '--add-modules=java.xml.bind'
Target //:all failed to build

It seems to be the last missing feature, to fully support Java 9 build on Bazel@HEAD.

• [1] https://git.eclipse.org/r/113368