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 required --add-opens server JVM args also with non-embedded JDK #16706

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Nov 9, 2022

Since the Bazel server requires JDK 11 or higher to run, the --add-opens server JVM arg for java.lang can now be added unconditionally, which ensures support with JDK 17+.

Also removes the additional opens for java.nio, which was only needed to silence a protobuf warning that has since been fixed upstream.

Fixes #16705
Fixes #15831

@fmeum fmeum marked this pull request as ready for review November 9, 2022 08:25
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 9, 2022

@cushon Could you take a look? This seems too simple of a fix, so I may be missing something.

@sgowroji sgowroji added team-Rules-Java Issues for Java rules awaiting-review PR is awaiting review from an assigned reviewer labels Nov 9, 2022
src/main/cpp/blaze.cc Outdated Show resolved Hide resolved
@cushon
Copy link
Contributor

cushon commented Nov 9, 2022

This seems too simple of a fix, so I may be missing something.

Makes sense to me: I think this was a workaround for not having a better way to tell if the server javabase was JDK > 8, and now that we don't need JDK 8 passing the flags unconditionally SGTM.

Since the Bazel server requires JDK 11 or higher to run, the
`--add-opens` server JVM arg for `java.lang` can now be added
unconditionally, which ensures support with JDK 17+.

Also removes the additional opens for `java.nio`, which was only needed
to silence a protobuf warning that has since been fixed upstream.
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 9, 2022

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Nov 9, 2022
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 17, 2022

@cushon Friendly ping, is this in the process of being merged?

@cushon cushon 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 Nov 17, 2022
@cushon
Copy link
Contributor

cushon commented Nov 17, 2022

@cushon Friendly ping, is this in the process of being merged?

@fmeum I think it is now, sorry for the delay.

@fmeum fmeum deleted the 16705-embedded-opens branch November 18, 2022 04:21
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 18, 2022
fmeum added a commit to fmeum/bazel that referenced this pull request Nov 18, 2022
Since the Bazel server requires JDK 11 or higher to run, the `--add-opens` server JVM arg for `java.lang` can now be added unconditionally, which ensures support with JDK 17+.

Also removes the additional opens for `java.nio`, which was only needed to silence a protobuf warning that has since been fixed upstream.

Fixes bazelbuild#16705
Fixes bazelbuild#15831

Closes bazelbuild#16706.

PiperOrigin-RevId: 489372772
Change-Id: I880e2689f59b2d4420b1e2e0517697d7fb03abbc
meteorcloudy pushed a commit that referenced this pull request Nov 18, 2022
…#16787)

Since the Bazel server requires JDK 11 or higher to run, the `--add-opens` server JVM arg for `java.lang` can now be added unconditionally, which ensures support with JDK 17+.

Also removes the additional opens for `java.nio`, which was only needed to silence a protobuf warning that has since been fixed upstream.

Fixes #16705
Fixes #15831

Closes #16706.

PiperOrigin-RevId: 489372772
Change-Id: I880e2689f59b2d4420b1e2e0517697d7fb03abbc
@meteorcloudy meteorcloudy removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 1, 2022
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
5 participants