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 JDK 8 tools.jar contents to bootclasspath #183

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Apr 3, 2024

Work towards bazelbuild/bazel#21771

@fmeum fmeum requested review from comius and a team as code owners April 3, 2024 17:17
@fmeum
Copy link
Contributor Author

fmeum commented Apr 3, 2024

@cushon Could you review this?

CC @rbeasley

@cushon
Copy link
Collaborator

cushon commented Apr 3, 2024

Thanks, this LGTM.

I have one observation, I am not requesting a change: inside Google I've done some work to restrict the set of JDK APIs that are available by default to only include the [root modules] for JDK 9 and newer, and to also omit internal JDK APIs from the bootclasspath jar.

For Bazel I could see some value in discouraging accidentally using internal / non-root / non-default APIs, including tools.jar. But it would need to be possible to use those APIs for builds that really wanted them, and I don't have great ideas for making that optional; we already have a lot of configuration knobs.

@fmeum
Copy link
Contributor Author

fmeum commented Apr 3, 2024

In terms of knobs, as it's essentially a portability issue, maybe this could become a new errorprone check that could then be configured on the toolchain/package/target level as usual?

@copybara-service copybara-service bot closed this in f195f33 Apr 5, 2024
@fmeum fmeum deleted the patch-2 branch April 6, 2024 19:15
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.

None yet

2 participants