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 Java tests to use Java toolchain resolution. #12695

Closed
wants to merge 10 commits into from

Conversation

comius
Copy link
Contributor

@comius comius commented Dec 14, 2020

The flag is flipped for tests, old configuration options javabase and java_toolchain are marked deprecated. All the tests are fixed to execute using new toolchain resolution.

Changed local_java_repository to generate java_runtime with java_home parameter set (instead of srcs). This behaves better, because java_stub_template.txt then does not need runfiles to execute produced deploy.jar. Kept the same version with srcs in the jdk.WORKSPACE.

Fixes #4592.
Flips #7849.

@google-cla google-cla bot added the cla: yes label Dec 14, 2020
@comius comius changed the title Flip toolchain resolution flag. Flip Java toolchain resolution flag. Dec 14, 2020
@comius comius requested a review from katre December 15, 2020 12:14
@comius comius marked this pull request as ready for review December 15, 2020 12:14
@comius comius requested a review from lberki as a code owner December 15, 2020 12:14
@comius comius requested review from cushon and removed request for lberki December 15, 2020 12:14
Copy link
Member

@katre katre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With all of this code deleted, can users actually unset the flag? If someone needs to do "bazel build --noincompatible_use_toolchain_resolution_for_java_rules", will that work, or will it fail?

@comius
Copy link
Contributor Author

comius commented Dec 15, 2020

With all of this code deleted, can users actually unset the flag? If someone needs to do "bazel build --noincompatible_use_toolchain_resolution_for_java_rules", will that work, or will it fail?

I will fail, I should remove the flag as well.

@katre
Copy link
Member

katre commented Dec 15, 2020

That's the opposite of what I would prefer: we should only flip the flag in this release, wait a bazel release, and then remove the code.

@comius comius requested a review from katre December 15, 2020 14:02
@comius
Copy link
Contributor Author

comius commented Dec 15, 2020

That's the opposite of what I would prefer: we should only flip the flag in this release, wait a bazel release, and then remove the code.

Ok. I reverted the removal and kept everything else.

Copy link
Member

@katre katre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very excited to see this!

@comius comius changed the title Flip Java toolchain resolution flag. Fix Java tests to use Java toolchain resolution. Dec 16, 2020
@comius
Copy link
Contributor Author

comius commented Dec 16, 2020

I will delay flipping the flag a bit, because RBE would potentially break on downstream projects. I need to prepare couple of fixes in both bazel and bazel_toolchains.

@bazel-io bazel-io closed this in f5b6abc Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert Java rules to use Toolchain Resolution
2 participants