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

Fix remotejdk_11 toolchain registration #109

Closed
wants to merge 1 commit into from

Conversation

joeljeske
Copy link

The registration for JDK11 was defaulting to java_runtime of JDK17, causing issues. Instead, register the remotejdk_11 toolchain explicitly using the proper java_runtime value of JDK11. Oddly, the naming is inconsistent in this file, remote_jdk11 and remotejdk_11. It appears to be inadvertent 🤷

@joeljeske joeljeske requested review from comius and a team as code owners May 11, 2023 14:34
@hvadehra
Copy link
Member

The registration for JDK11 was defaulting to java_runtime of JDK17, causing issues

Could you please elaborate on what issues this was causing? Using the latest runtime (by default) is desired (the inconsistency was fixed in fda9e4a).

Oddly, the naming is inconsistent in this file

Yes, we plan to fix this shortly, but it will be a breaking change, so will need to tread carefully.

@sfc-gh-jmerino
Copy link

I'm finding the same problem here as I updated our repo from rules_java 5.1.0 to 7.0.6 and now the build breaks because it's picking up JDK21 with a source level of 21. We pass the following flags:

build --java_language_version=11
build --tool_java_language_version=11
build --java_runtime_version=remotejdk_11

And here is where things get even more confusing.

If I clone the repository and check out the 7.0.6 tag, I can apply the patch proposed here and things work as before for me.

But if I download the 7.0.6 release tarball, it doesn't work, and it's a completely different codebase that includes the changes referenced by @hvadehra (and I have the same breakage). It seems like something went wrong in the release process?

@hvadehra
Copy link
Member

hvadehra commented Nov 6, 2023

It seems like something went wrong in the release process?

Yes, sorry about that, there was a bug in the release pipeline with releases made on a non-main branch. Will be fixed going forward.

Please see bazelbuild/bazel#19934 (comment) for how the flags affect which java_runtime gets used. If you'd like to use a specific version for javac itself, right now the way to do that is to define a custom java toolchain (with the appropraite java_runtime attribute).

@samsternatretool
Copy link

@joeljeske @hvadehra I'm dealing with the same issue ... seems like you both found workarounds to this PR. Any pointers for me?

@hvadehra
Copy link
Member

@samsternatretool If you would like to use a specific runtime version (although this should not be generally necessary), you can declare a custom java toolchain with the specific java_runtime. For example, for jdk17:

# //my/pkg/BUILD
load("@rules_java//toolchains:default_java_toolchain.bzl", "default_java_toolchain")

default_java_toolchain(
    name = 'my_java17_toolchain',
    java_runtime = "@rules_java//toolchains:remotejdk_17",
    source_version = "17",
    target_version = "17",
)

Then register this in your WORKSPACE / MODULE.bzl.

register_toolchains("//my/pkg:my_java17_toolchain_definition")

(note that order of registration matters, so put this before any other if you want it to win)

@samsternatretool
Copy link

@hvadehra thank you!

@hvadehra
Copy link
Member

@joeljeske I'm closing this PR as I don't think we'll be making these changes and we haven't gotten any more details about what issues are being caused by using the latest runtime.

Please feel free to discuss on #148 or bazelbuild/bazel#19934

@hvadehra hvadehra closed this Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants