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 add_exports & add_opens parameters to JavaInfo constructor #20036

Closed

Conversation

timothyg-stripe
Copy link
Contributor

@timothyg-stripe timothyg-stripe commented Nov 2, 2023

There is currently no non-hacky way for third-party rule implementations to add add_exports and add_opens to a JavaInfo, though hacky ways exist (like using a macro to generate a java_library with add_exports / add_opens and add it as a dependency, or creating an empty source file and compiling it using java_common.compile).

Having an official way to create JavaInfos with add_exports and add_opens helps third-party JVM rules better support JDK 9+ (and especially 17+, which requires --add-opens flags to access JDK internals through reflection).

Addresses half of #20033.

@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Rules-Java Issues for Java rules labels Nov 2, 2023
@timothyg-stripe
Copy link
Contributor Author

cc @cushon @hvadehra

@hvadehra hvadehra requested review from hvadehra and cushon November 3, 2023 10:23
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.

LGTM, I'd like @cushon to review as well, in case there are any concerns with this that I'm not aware of.

@cushon
Copy link
Contributor

cushon commented Nov 4, 2023

LGTM, I'd like @cushon to review as well, in case there are any concerns with this that I'm not aware of.

@hvadehra can we put this behind the same allowlist as java_library.add_opens and java_library.add_exports internally?

Otherwise it seems fine, there isn't a principled reason this can be configured with java_common.compile but not directly.

@copybara-service copybara-service bot closed this in d2783a3 Nov 8, 2023
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Nov 8, 2023
@timothyg-stripe timothyg-stripe deleted the java_info_add_flags branch November 8, 2023 17:10
@iancha1992
Copy link
Member

@bazel-io fork 7.0.0

iancha1992 pushed a commit to iancha1992/bazel that referenced this pull request Nov 17, 2023
There is currently no non-hacky way for third-party rule implementations to add `add_exports` and `add_opens` to a `JavaInfo`, though hacky ways exist (like using a macro to generate a `java_library` with `add_exports` / `add_opens` and add it as a dependency).

Having an official way to create `JavaInfo`s with `add_exports` and `add_opens` helps third-party JVM rules better support JDK 9+ (and especially 17+, which requires `--add-opens` flags to access JDK internals through reflection).

Addresses half of bazelbuild#20033.

Closes bazelbuild#20036.

PiperOrigin-RevId: 580472097
Change-Id: I159e3410c5480ac683fd9af85bfd1d83ac0e6d8a
timothyg-stripe added a commit to timothyg-stripe/bazel that referenced this pull request Nov 18, 2023
There is currently no non-hacky way for third-party rule implementations to add `add_exports` and `add_opens` to a `JavaInfo`, though hacky ways exist (like using a macro to generate a `java_library` with `add_exports` / `add_opens` and add it as a dependency).

Having an official way to create `JavaInfo`s with `add_exports` and `add_opens` helps third-party JVM rules better support JDK 9+ (and especially 17+, which requires `--add-opens` flags to access JDK internals through reflection).

Addresses half of bazelbuild#20033.

Closes bazelbuild#20036.

PiperOrigin-RevId: 580472097
Change-Id: I159e3410c5480ac683fd9af85bfd1d83ac0e6d8a
meteorcloudy pushed a commit that referenced this pull request Nov 20, 2023
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
Development

Successfully merging this pull request may close these issues.

4 participants