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

--release javacopt does not link against specified version #6652

Closed
marcohu opened this issue Nov 11, 2018 · 13 comments
Closed

--release javacopt does not link against specified version #6652

marcohu opened this issue Nov 11, 2018 · 13 comments
Assignees
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: bug

Comments

@marcohu
Copy link
Contributor

marcohu commented Nov 11, 2018

I was under the impression that the Java 9+ --release compiler flag will link against the provided platform version. But as of version 19.0 this does not seem to be the case with Bazel.

The following code compiles fine with the --release 7 javacopt even though it uses a Java 9 API method:

Main.java

import java.io.*;

public class Main
{
    public static void main(String... args) throws IOException
    {
        try (InputStream in = new FileInputStream(""))
        {
            in.readAllBytes(); // only available since Java 9
        }
    }
}

BUILD

java_library(
    name = "java",
    srcs = glob(["*.java"]),
)
$ bazel build --javacopt="--release 7" /
--host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java9 /
--java_toolchain=@bazel_tools//tools/jdk:toolchain_java9 /
//test/...

Is this intentional or am I doing something wrong? Is there a best practice to compile against older Java versions with Bazel?

I would expect that the compilation fails with some message like

[javac] release/test/src/main/java/Main.java:9: error: cannot find symbol
[javac]             in.readAllBytes();
[javac]               ^
[javac]   symbol:   method readAllBytes()
[javac]   location: variable in of type InputStream
[javac] 1 error

(this is what Ant shows when trying to compile the same)

There is another oddity, not sure if it is related. Bazel shows the following warning upon building:

warning: could not find a JDK 8 bootclasspath in external/local_jdk, falling back to --release

I am using Bazel 0.19.0 on Debian Linux. $PATH and $JAVA_HOME point to Oracle 9.0.4

@cushon
Copy link
Contributor

cushon commented Nov 12, 2018

--release isn't yet well-supported by Bazel (in part because of issues like JDK-8206937).

The recommended way to target Java 7 is by setting --javacopt='-source 7 -target 7' and a JDK 7 --javabase (e.g. --define=ABSOLUTE_JAVABASE=${JAVA7_HOME?} --javabase=@bazel_tools//tools/jdk:absolute_javabase).

The platform APIs that the compilation uses are chosen based on --javabase, so e.g. InputStream#readAllBytes won't be available with a JDK 7 --javabase.

@cushon cushon added the team-Rules-Java Issues for Java rules label Nov 12, 2018
@marcohu
Copy link
Contributor Author

marcohu commented Nov 12, 2018

Is there an ETA when --release will be supported?

My actual requirement is a bit more involved than with the mentioned example. I ultimately need to be able to link different projects against different Java releases. The proposed --javabase approach is therefore no option.

Ideally, java_library, java_binary and java_common.compile would provide a distinct release attribute to set the desired link version. I'm not sure why this requires additional JDK API support, it already works just fine with other build tools.

@lberki
Copy link
Contributor

lberki commented Nov 12, 2018

What happens if e.g. you have a java_library that says --release 7 and another that says --release 8 in the dependencies of a single _java_binary?

If it's okay to do built the two Java binaries that require different Java releases within different Bazel invocations, you can always have different --javabase arguments for them on the command line. If not, I don't think we have a solution for you at the moment. :( Maybe /cc @katre or @gregestren have a smart idea?

@cushon
Copy link
Contributor

cushon commented Nov 12, 2018

What happens if e.g. you have a java_library that says --release 7 and another that says --release 8 in the dependencies of a single java_binary?

If a --release 8 library depends on a --release 7 library that usually Just Works, Java has a good binary compatibility story in that direction.

This might be something we could model with constraints and fulfills, and then somehow tie that to the --release flag.

I've thought about doing something like that for the existing -source/-target/-bootclasspath configuration, it might improve on the way we currently handle mixed server/android/appengine builds.

I'm not sure why this requires additional JDK API support, it already works just fine with other build tools.

To clarify, that's not a blocker for supprting it in Bazel, it's just that we might have more motivation to add the support if Bazel itself was able to use the flag. As it is we have no choice to continue supporting -source/-target/-bootclasspath because --release doesn't work for all the same use-cases.

@marcohu
Copy link
Contributor Author

marcohu commented Nov 13, 2018

I have it working with Bazel 0.12 by the means of a custom java_comon.compile rule which luckily already accepts a toolchain argument, but I was running into problems with the toolchain under Bazel 0.19 as -Xbootclasspath is no longer supported. --release seemed like the perfect solution, but sadly doesn't (fully) work yet.

@lberki Not sure using multiple Bazel invocations could work. The Java 8 + 9 projects might have Java 7 project dependences, but invocation with a different javabase seems to recompile these dependences as well.

Anyway, thanks much for your help and keep up the good work! I hope Java 9 support will get better over time.

@jin
Copy link
Member

jin commented Jan 14, 2019

@lberki could you add a priority to this issue, please?

@lberki lberki added the P2 We'll consider working on this in future. (Assignee optional) label Feb 4, 2019
@comius comius added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 21, 2020
@srdo-humio
Copy link

srdo-humio commented Mar 8, 2022

Hi.

I've been trying to set up Bazel (5.0.0) to use JDK 17 for compilation and execution, but to generate code for Java 11.
Ideally this would be possible by passing --release to javac.
I hit the following issues:

  • Defining a custom toolchain using default_java_toolchain with DEFAULT_TOOLCHAIN_CONFIGURATION and remotejdk_17, the --patch_modules commands from JDK9_JVM_OPTS are incompatible with --release. I get the impression they will be removed in the future toolchain_vanilla works with locally defined jdk 11, but toolchain_java11 breaks the jdk compiler #14533 (comment).
  • I tried instead using VANILLA_TOOLCHAIN_CONFIGURATION, but hit the issue that it is seemingly not possible to get Bazel to leave -source/-target off the invocation of javac. I removed source_version and target_version from the _BASE_TOOLCHAIN_CONFIGURATION, but still get the following error from javac
Exception in thread "main" java.lang.IllegalArgumentException: error: option --source cannot be used together with --release

so the flags are being set to a default value somewhere.

After messing around with this a bit, I got --release 11 working by setting up a toolchain similar to the default remotejdk_17, except it leaves out the --patch_modules flags.
If those flags are removed in a future release, --release might work for Bazel out of the box, so good news there :)

Regarding VanillaJavaBuilder, I think the limitation is that there's no good way to ensure -source/-target aren't being passed to javac, it seems like they always get set to a default value.
BazelJavaBuilder solves this by postprocessing the javac options, to ensure that if you set --release, the other flags are removed

if (release != null) {
normalized.add("--release");
normalized.add(release);
} else {
if (source != null) {
normalized.add("-source");
normalized.add(source);
}
if (target != null) {
normalized.add("-target");
normalized.add(target);
}

Would it make sense to use this postprocessing in VanillaJavaBuilder as well? It seems like a good strategy, since there doesn't appear to be another way to remove the default -source/-target values from the arguments to javac.

@cushon
Copy link
Contributor

cushon commented Mar 8, 2022

I get the impression they will be removed in the future

I'm going to try to remove those --patch-module flags once JDK 11.0.15 is released, which should be April 19, 2022

it is seemingly not possible to get Bazel to leave -source/-target off the invocation of javac

This should be possible. Did you try setting source_version = None in the toolchain? Note that _BASE_TOOLCHAIN_CONFIGURATION is coming from the released java_tools archive, not the version of that file in the repo where you're developing Bazel. There's some related context about making changes to it in bazelbuild/java_tools#43

Would it make sense to use this postprocessing in VanillaJavaBuilder as well?

It could and we're already doing some pre-processing of the args there:

JavacOptions.removeBazelSpecificFlags(optionsParser.getJavacOpts()),

In general though VanillaJavaBuilder is supposed to be as close as possible to stock javac behaviour, and it's javac that doesn't allow combining -source and --release, so I think it'd be better to make it possible to get Bazel to leave out -source/-target to allow specifying just --release

@srdo-humio
Copy link

I'm going to try to remove those --patch-module flags once JDK 11.0.15 is released, which should be April 19, 2022

Sounds great :)

This should be possible. Did you try setting source_version = None in the toolchain

I didn't, but trying it out it does not appear to work. The following toolchain fails when compiling Java code because Bazel still passes --source to javac alongside --release, with the exception I mentioned earlier:

load("@bazel_tools//tools/jdk:default_java_toolchain.bzl", "default_java_toolchain", "VANILLA_TOOLCHAIN_CONFIGURATION")
toolchain(
    name = "demo_toolchain",
    toolchain = "demo_toolchain_impl",
    toolchain_type = "@bazel_tools//tools/jdk:toolchain_type",
    visibility = ["//visibility:public"]
)

default_java_toolchain(
    toolchain_definition = False, # We define the toolchain above.
    name = "demo_toolchain_impl",
    configuration = VANILLA_TOOLCHAIN_CONFIGURATION,
    java_runtime = "@bazel_tools//tools/jdk:remotejdk_17",
    javacopts = [
        "--release 11", # Compile for, and target the APIs of, this Java release
        ],
    source_version = None,
    target_version = None,
    bootclasspath = None,
)

Here are the relevant parts of the exception:

Exception in thread "main" java.lang.IllegalArgumentException: error: option --source cannot be used together with --release
...
	at com.google.devtools.build.buildjar.VanillaJavaBuilder.run(VanillaJavaBuilder.java:175)
	at com.google.devtools.build.buildjar.VanillaJavaBuilder.runPersistentWorker(VanillaJavaBuilder.java:106)
	at com.google.devtools.build.buildjar.VanillaJavaBuilder.main(VanillaJavaBuilder.java:87)

I have also tried copy-pasting the relevant bits from default_java_toolchain.bzl into my own project to define a toolchain where source_version and target_version are set to None in the _BASE_TOOLCHAIN_CONFIGURATION, with the same result.

For reference, this configuration works for me, which should be the same as the default, except it uses BASE_JDK9_JVM_OPTS instead of JDK9_JVM_OPTS to avoid the --patch-modules flags:

load("@bazel_tools//tools/jdk:default_java_toolchain.bzl", "default_java_toolchain", "BASE_JDK9_JVM_OPTS", "DEFAULT_TOOLCHAIN_CONFIGURATION")
toolchain(
    name = "demo_toolchain",
    toolchain = "demo_toolchain_impl",
    toolchain_type = "@bazel_tools//tools/jdk:toolchain_type",
    visibility = ["//visibility:public"]
)

default_java_toolchain(
    toolchain_definition = False, # We define the toolchain above.
    name = "demo_toolchain_impl",
    configuration = DEFAULT_TOOLCHAIN_CONFIGURATION,
    java_runtime = "@bazel_tools//tools/jdk:remotejdk_17",
    jvm_opts = [
        # Compact strings make JavaBuilder slightly slower.
        "-XX:-CompactStrings",
    ] + BASE_JDK9_JVM_OPTS,
    javacopts = [
        "--release 11", # Compile for, and target the APIs of, this Java release
        ]
)

There's some related context about making changes to it in bazelbuild/java_tools#43

Thank you, this is good to know. I am not building Bazel locally to try this out, instead I copied code from default_java_toolchain.bzl into my own project, so I should be using the code from the java_tools archive.

I think it'd be better to make it possible to get Bazel to leave out -source/-target

I agree, if that's feasible. I see the Java version mentioned in defaults in a bunch of places (e.g. java_language_version, source_version, target_version, bootclasspath), so processing the final javac args seems like it might be easier to get (and keep) working though.

@guw
Copy link
Contributor

guw commented Mar 19, 2022

@srdo-humio Do you mind sharing the setup you have working? I'm in a similar need.

@srdo-humio
Copy link

@guw It's the bit I posted above.

load("@bazel_tools//tools/jdk:default_java_toolchain.bzl", "default_java_toolchain", "BASE_JDK9_JVM_OPTS", "DEFAULT_TOOLCHAIN_CONFIGURATION")
toolchain(
    name = "demo_toolchain",
    toolchain = "demo_toolchain_impl",
    toolchain_type = "@bazel_tools//tools/jdk:toolchain_type",
    visibility = ["//visibility:public"]
)

default_java_toolchain(
    toolchain_definition = False, # We define the toolchain above.
    name = "demo_toolchain_impl",
    configuration = DEFAULT_TOOLCHAIN_CONFIGURATION,
    java_runtime = "@bazel_tools//tools/jdk:remotejdk_17",
    jvm_opts = [
        # Compact strings make JavaBuilder slightly slower.
        "-XX:-CompactStrings",
    ] + BASE_JDK9_JVM_OPTS,
    javacopts = [
        "--release 11", # Compile for, and target the APIs of, this Java release
        ]
)

Put that in a bazel file somewhere, then register that toolchain in your WORKSPACE, e.g.

register_toolchains("//your-toolchain-package:demo_toolchain")

@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jun 13, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Development

No branches or pull requests

8 participants