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

Add version to JavaRuntimeInfo. #17775

Closed
wants to merge 1 commit into from

Conversation

benjaminp
Copy link
Collaborator

  1. As suggested in Use @argument files in the Java binary wrapper script #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

  2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes Bazel@HEAD on JDK 17: GoogleTestSecurityManager WARNING: System::setSecurityManager will be removed in a future release #14502.

  3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

@benjaminp benjaminp requested a review from lberki as a code owner March 14, 2023 22:00
@benjaminp benjaminp force-pushed the java-runtime-version branch 5 times, most recently from 8e73161 to 30cd9fe Compare March 14, 2023 22:51
@ShreeM01 ShreeM01 added team-Rules-Java Issues for Java rules awaiting-user-response Awaiting a response from the author labels Mar 14, 2023
@benjaminp benjaminp force-pushed the java-runtime-version branch from 30cd9fe to e3960e7 Compare March 14, 2023 23:12
@sgowroji sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Mar 15, 2023
@lberki lberki requested review from hvadehra and removed request for lberki March 15, 2023 07:50
Copy link
Member

@hvadehra hvadehra left a comment

Choose a reason for hiding this comment

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

Hi, thanks a lot for this! LGTM for the most part,

tools/jdk/local_java_repository.bzl Outdated Show resolved Hide resolved
tools/jdk/jdk_build_file.bzl Outdated Show resolved Hide resolved
@benjaminp benjaminp force-pushed the java-runtime-version branch from e3960e7 to bfdf72a Compare March 21, 2023 15:25
@benjaminp benjaminp requested a review from a team as a code owner March 21, 2023 15:25
@benjaminp benjaminp requested review from aiuto and hvadehra and removed request for a team and aiuto March 21, 2023 15:25
@@ -83,6 +84,10 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("java_home", STRING))
.add(attr("output_licenses", LICENSE))
/* <!-- #BLAZE_RULE(java_runtime).ATTRIBUTE(version) -->
The major version of the Java runtime.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this documentation mention the version number here is the same as the 'feature element of the version number' documented in https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Runtime.Version.html#feature() ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup—I didn't realize that "major" terminology had been deprecated.

@benjaminp benjaminp force-pushed the java-runtime-version branch 2 times, most recently from 681bca6 to b003a37 Compare March 21, 2023 15:58
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)
@benjaminp benjaminp force-pushed the java-runtime-version branch from b003a37 to 2d1a30a Compare March 21, 2023 16:07
@hvadehra hvadehra added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Mar 21, 2023
@benjaminp benjaminp deleted the java-runtime-version branch March 23, 2023 15:03
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Mar 23, 2023
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this pull request Mar 24, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes bazelbuild#17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this pull request Mar 28, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes bazelbuild#17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
hvadehra pushed a commit that referenced this pull request Mar 29, 2023
1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

(cherry picked from commit 7556e11)
hvadehra added a commit that referenced this pull request Mar 31, 2023
As suggested in Use @argument files in the Java binary wrapper script #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes Bazel@HEAD on JDK 17: GoogleTestSecurityManager WARNING: System::setSecurityManager will be removed in a future release #14502.

To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
hvadehra pushed a commit that referenced this pull request Apr 3, 2023
1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

(cherry picked from commit 7556e11)
meteorcloudy pushed a commit that referenced this pull request Apr 3, 2023
* Add version to JavaRuntimeInfo.

1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes #17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

(cherry picked from commit 7556e11)

* Use string.replace() instead of string.format() to avoid issues with select{} in the template string

* Remove test and fix formatting

* Fix starlark_repository_test

Change the grep pattern to exactly match what the test is looking for. The old pattern would match BUILD file content from the jdk_build_file.bzl template

---------

Co-authored-by: Benjamin Peterson <[email protected]>
fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version.

2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502.

3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

Closes bazelbuild#17775.

PiperOrigin-RevId: 518860040
Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules
Projects
None yet
6 participants