desugar doesn't work with Bazel 0.17.1 and --host_javabase=<jdk8>

[cushon]

original context: #5978 (comment).

cc @lberki @jin

Setup:

• build //src:bazel_nojdk @ 7bb9e0f (0.17.1)
• clone [email protected]:cushon/bazel_issue_5978.git

$ JAVA_HOME=$JAVA8_HOME ANDROID_HOME=$HOME/Android/Sdk/ \
  bazel_nojdk build --config=jdk8   //:sample
...
bazel_issue_5978/BUILD.bazel:1:1: Desugaring sample_resources.jar for Android failed (Exit 1)
Unrecognized option: --add-opens=java.base/java.lang.invoke=ALL-UNNAMED
Error: Could not create the Java Virtual Machine.

It works for me without --config jdk8 if I use a Bazel binary that includes the bundled JDK, or explicitly configure a JDK 9 --host_javabase:

JAVA_HOME=$JAVA8_HOME ANDROID_HOME=$HOME/Android/Sdk/ \
  bazel_nojdk build \
  --host_javabase=:absolute_javabase --define=ABSOLUTE_JAVABASE=$JAVA9_HOME \
  --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk9 \
  --java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk9 \
  //:sample

The problem is that the --add-opens=java.base/java.lang.invoke=ALL-UNNAMED JVM flag added to desugar in d02f876 is incompatible with a JDK 8 `--host_javabase.

That issue was later fixed by 2e677fb, but the fix wasn't included in 0.17.1.

@steeve which installer are you using for Bazel? Is it one of the homebrew ones? If you have the option of using the binary from the releases page: https://github.com/bazelbuild/bazel/releases/tag/0.17.1, it should work around the problem. The other work-around is to install JDK 9 and use it as the --host_javabase, like in the example above.

[steeve]

@cushon I'm using the one from the binary installer.

Also, using JDK9 is not an option when building Android binaries as it does not support it yet (JDK8 only). Although I think Bazel is supposed to handle it automatically? See #5978 (comment)

[cushon]

When I do not specify the build "works", but WARNING: An illegal reflective access operation has occurred

The warnings are annoying but harmless. We should suppress them. (Related: #6151.)

Ultimately this won't work because Android doesn't support JDK >8.

Can you provide more context about what doesn't work? The --host_javabase is only used for tools that run during the build (javac, etc.). You can use a JDK 9 --host_javabase and still build Android code that needs to be compatible with Java 8 or earlier.

[cushon]

I'm using the one from the binary installer.

As in https://github.com/bazelbuild/bazel/releases/download/0.17.1/bazel-0.17.1-installer-darwin-x86_64.sh? Then I can't reproduce the problem you reported without --config=jdk8.

Can you share the output of bazel info?

[steeve]

@cushon never mind you were right, I thought the warning were a sign of the previous bug (which would cause our downstream gradle-based build to fail). But I tested it right now and it does work without --config jdk8.

Yes I'm using https://github.com/bazelbuild/bazel/releases/download/0.17.1/bazel-0.17.1-installer-darwin-x86_64.sh since bazelbuild/homebrew-tap#4 is not merged yet. I wanted to check if there was any issue before people at the office start upgrading.

Just in case:

bazel-bin: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/execroot/__main__/bazel-out/darwin-fastbuild/bin
bazel-genfiles: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/execroot/__main__/bazel-out/darwin-fastbuild/genfiles
bazel-testlogs: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/execroot/__main__/bazel-out/darwin-fastbuild/testlogs
character-encoding: file.encoding = ISO-8859-1, defaultCharset = ISO-8859-1
command_log: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/command.log
committed-heap-size: 1905MB
execution_root: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/execroot/__main__
gc-count: 75
gc-time: 17115ms
install_base: /var/tmp/_bazel_steeve/install/79d9f22886570ca416e6ffca28ab7725
java-home: /private/var/tmp/_bazel_steeve/install/79d9f22886570ca416e6ffca28ab7725/_embedded_binaries/embedded_tools/jdk
java-runtime: OpenJDK Runtime Environment (build 9.0.7.1+1) by Azul Systems, Inc.
java-vm: OpenJDK 64-Bit Server VM (build 9.0.7.1+1, mixed mode) by Azul Systems, Inc.
max-heap-size: 3817MB
output_base: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b
output_path: /private/var/tmp/_bazel_steeve/309ac22c92b055a3e3f78825fe63406b/execroot/__main__/bazel-out
package_path: %workspace%
release: release 0.17.1
repository_cache: /var/tmp/_bazel_steeve/cache/repos/v1
server_pid: 81972
used-heap-size: 778MB

[cushon]

Thanks for confirming!

I'm still interested in the use-case for --config jdk8. The regression is concerning either way and needs to be fixed, but you should be able to do Android builds with the default javabase and java_toolchain flags. If you ran into problems with the defaults that were fixed by using --config jdk8 I'd like to make sure there are bugs tracking those.

[steeve]

Yes, --config jdk8 was a workaround for #5978, and I can confirm I can do successful builds without it on 0.17.1 (with the annoying warning though, but that's tracked at #6151).

[jin]

@cushon thanks for driving this!

So let me summarize the issues so far:

1. Building Android on 0.17.1 without manually forcing JDK8 for --host_javabase: this is now working fine, and no changes are required.
2. Unsafe warning spam from protobuf: tracked at Illegal reflective access by com.google.protobuf.UnsafeUtil when running Bazel 0.17.1 #6151.
3. Forcing --config jdk8 doesn't work on 0.17.1, because my fix at 2e677fb didn't make it into 0.17.1.

Did I miss anything else?

[cushon]

I think that's right.

Note that (3) also affects anyone using Bazel without an embedded JDK together with a locally installed JDK 8, since it will default to using the local JDK 8 as the --host_javabase.

JAVA_HOME=$JAVA8_HOME ANDROID_HOME=$HOME/Android/Sdk/ bazel_nojdk build //:sample
...
Unrecognized option: --add-opens=java.base/java.lang.invoke=ALL-UNNAMED
Error: Could not create the Java Virtual Machine.

I think the mitigation is:

• prefer the binaries with an embedded JDK, and don't override the --host_javabase
• when using the binaries without an embedded JDK, use a JDK 9 --host_javabase

And if there ends up being a 0.17.2 it should include 2e677fb.

[jin]

This sounds like it warrants a point release, since it's a regression introduced in 0.17.

[jin]

(cc @ahumesky)

[cushon]

I agree, given the emphasis on preserving support for --host_javabase=<jdk8> in 0.17 (see #6035).

[steeve]

I think we can close this because #6164 fixed it