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

Default java toolchain changed to target java 8 and broke toolchain_vanilla #9415

Closed
davido opened this issue Sep 22, 2019 · 2 comments
Closed
Labels
P1 I'll work on this now. (Assignee required) team-Rules-Java Issues for Java rules type: bug

Comments

@davido
Copy link
Contributor

davido commented Sep 22, 2019

It seems, that 6ef6d87 broke toolchain_vanila. Before this change there was a possibility to use newer Java versions and produce newer byte code as documented in Bazel source code https://github.com/bazelbuild/bazel/blob/master/tools/jdk/BUILD#L363-L382:

# The 'vanilla' toolchain is an unsupported alternative to the default.
#
# It does not provider any of the following features:
#   * Error Prone
#   * Strict Java Deps
#   * Header Compilation
#   * Reduced Classpath Optimization
#
# It uses the version of javac from the `--host_javabase` instead of the
# embedded javac, which may not be source- or bug-compatible with the embedded
# javac.
#
# However it does allow using a wider range of `--host_javabase`s, including
# versions newer than the current embedded JDK.
default_java_toolchain(
    name = "toolchain_vanilla",
    forcibly_disable_header_compilation = True,
    javabuilder = [":vanillajavabuilder"],
    jvm_opts = [],
)

For example this used to work in Gerrit and produced byte code major version: 57 (Java 13), but after aforementioned commit produced byte code major version 52 (Java 8):

  $ bazel build --define=ABSOLUTE_JAVABASE=/usr/local/bin/jdk-13   \
    --javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
     java/com/google/gerrit/common:server
  
  $ javap -v -cp bazel-bin/java/com/google/gerrit/common/libserver.jar \
    com/google/gerrit/common/FileUtil | grep major
  major version: 52

If I revert 6ef6d87, or apply this PR then it works as expected again:

$ bazel build --define=ABSOLUTE_JAVABASE=/use/local/bin/jdk-13  \
    --javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla  \
    --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
     java/com/google/gerrit/common:server
  
  $ javap -v -cp bazel-bin/java/com/google/gerrit/common/libserver.jar \
    com/google/gerrit/common/FileUtil | grep major
  major version: 57

Note, to make Bazel backwards compatible, and also produce byte code major version 52, even if built with Java 11 and newer java version as Host Java version, we can pass in Gerrit source and target language level to 8 in error_prone_warnings_toolchain definition instead:

default_java_toolchain(
    name = "error_prone_warnings_toolchain",
    bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath.jar"],
    jvm_opts = JDK9_JVM_OPTS,
    source_version = "8",
    target_version = "8",
    package_configuration = [
        ":error_prone",
    ],
    visibility = ["//visibility:public"],
)

That way running bazel build :release without any options even on Java 11 would produce Java major byte code 52 (Java 8) as expected:

  $ java -version
  openjdk version "11.0.4" 2019-07-16

  $ bazel build java/com/google/gerrit/common:server

  $ javap -v -cp bazel-bin/java/com/google/gerrit/common/libserver.jar \
    com/google/gerrit/common/FileUtil | grep major
  major version: 52

Update: I forked toolchain_vanilla from @bazel_tools//tools/jdk:toolchain_vanilla to @gerrit//tools:toolchain_vanilla to unblock gerrit project for now: [1].

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

//CC @iirina @cushon

davido added a commit to davido/bazel that referenced this issue Sep 22, 2019
Closes bazelbuild#9415.

The main feature of toolchain_vanilla definition is to support newer
Java language versions that the embedded JDK, e.g.: at the time of this
CL, toolchain_java could be used to support building with JDK 13 and
produce byte code major version 57, using the combination of absolute
javabase and toolchain_vanilla:

  $ bazel build --define=ABSOLUTE_JAVABASE=/use/local/bin/jdk-13 \
    --javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    :gerrit

Unfortunately, 6ef6d87 sets source and
target language levels to Java 8 to restore backwards compatibility,
to fix a regression, that was introduced during bump of remote JDK
version to Java 11, in bad5a2b.

This change restores the neutrality of toolchain_vanilla declaration by
unsetting source and target language level versions. So that the bazel
invocation in the example above produces byte code major version 57
(Java 13), and not 52 (Java 8) as it is the case before this change.

Change-Id: I1f471c987487b81bc149d1ad4368eee149004d79
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Sep 23, 2019
The main feature of toolchain_vanilla definition is to support newer
Java language versions that the embedded JDK, e.g.: at the time of this
change, toolchain_java could be used to support building with JDK 13 and
produce byte code major version 57, using the combination of absolute
javabase and toolchain_vanilla:

  $ bazel build --define=ABSOLUTE_JAVABASE=/use/local/bin/jdk-13 \
    --javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
  :gerrit

Unfortunately, [1] sets source and target language levels to Java 8 to
restore backwards compatibility, to fix a regression, that was
introduced during upgrade of remote JDK version to Java 11, in: [2].

This change restores the neutrality of toolchain_vanilla declaration by
unsetting source and target language level versions. So that the bazel
invocation in the example above produces byte code major version 57
(Java 13), and not 52 (Java 8) as it is the case before this change.

Also add a TODO, to remove the workaround, when this issue is fixed:
[3].

[1] bazelbuild/bazel@6ef6d87
[2] bazelbuild/bazel@bad5a2b
[3] bazelbuild/bazel#9415

Bug: Issue 11571
Change-Id: I721067202de744883bc8c74bb4106ad08f19c66d
@iirina iirina added P1 I'll work on this now. (Assignee required) and removed untriaged labels Sep 26, 2019
@dslomov
Copy link
Contributor

dslomov commented Sep 27, 2019

Since this breaks backwards compatibility, should I cherrypick 847df72 into 1.0? (#8573)

@iirina
Copy link
Contributor

iirina commented Sep 27, 2019

@dslomov Yes, please cherry pick 847df72 into 1.0.

dslomov pushed a commit that referenced this issue Sep 30, 2019
Closes #9415.

The main feature of toolchain_vanilla definition is to support newer
Java language versions that the embedded JDK, e.g.: at the time of this
CL, toolchain_java could be used to support building with JDK 13 and
produce byte code major version 57, using the combination of absolute
javabase and toolchain_vanilla:

  $ bazel build --define=ABSOLUTE_JAVABASE=/use/local/bin/jdk-13 \
    --javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    :gerrit

Unfortunately, 6ef6d87 sets source and
target language levels to Java 8 to restore backwards compatibility,
to fix a regression, that was introduced during bump of remote JDK
version to Java 11, in bad5a2b.

This change restores the neutrality of toolchain_vanilla declaration by
unsetting source and target language level versions. So that the bazel
invocation in the example above produces byte code major version 57
(Java 13), and not 52 (Java 8) as it is the case before this change.

Change-Id: I1f471c987487b81bc149d1ad4368eee149004d79

Closes #9416.

Change-Id: I1f471c987487b81bc149d1ad4368eee149004d79
PiperOrigin-RevId: 271342117
dslomov pushed a commit that referenced this issue Oct 2, 2019
Closes #9415.

The main feature of toolchain_vanilla definition is to support newer
Java language versions that the embedded JDK, e.g.: at the time of this
CL, toolchain_java could be used to support building with JDK 13 and
produce byte code major version 57, using the combination of absolute
javabase and toolchain_vanilla:

  $ bazel build --define=ABSOLUTE_JAVABASE=/use/local/bin/jdk-13 \
    --javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
    --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
    :gerrit

Unfortunately, 6ef6d87 sets source and
target language levels to Java 8 to restore backwards compatibility,
to fix a regression, that was introduced during bump of remote JDK
version to Java 11, in bad5a2b.

This change restores the neutrality of toolchain_vanilla declaration by
unsetting source and target language level versions. So that the bazel
invocation in the example above produces byte code major version 57
(Java 13), and not 52 (Java 8) as it is the case before this change.

Change-Id: I1f471c987487b81bc149d1ad4368eee149004d79

Closes #9416.

Change-Id: I1f471c987487b81bc149d1ad4368eee149004d79
PiperOrigin-RevId: 271342117
qtprojectorg pushed a commit to qtqa/gerrit that referenced this issue Nov 11, 2019
This reverts commit aee1f80.

Reason for revert: The upstream issue: [1] was fixed in 1.0
Bazel release and gerrit switched to using Bazel 1.1 recently.

[1] bazelbuild/bazel#9415

Change-Id: I6f81b187e0265ff797589479b7bff98a487c561a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants