User-accessible JavaInfo constructor doesn't allow changing module_flags_info #20033

timothyg-stripe · 2023-11-02T20:03:57Z

Description of the feature request:

The user-accessible JavaInfo constructor runs _javainfo_init, which does not allow customizing module_flags_info. This means that it's impossible to replicate some of the semantics of java_info_for_compilation that would be useful for third party rulesets (like rules_scala), like:

It's impossible to make add_exports and add_opens attributes for a third-party rule
It's impossible to merge in runtime_deps' add_exports and add_opens, since _javainfo_init only merges the module flags for deps and exports.

A few ways to resolve this are:

The proper fix (IMO):
1. Add some options to the JavaInfo constructor, like add_exports and add_opens (for "direct" rule attributes).
2. Change the behavior of JavaInfo constructor so that add_exports and add_opens are merged for runtime_deps in addition to just deps & exports. This is IMO the right thing to do anyway, and reduces the delta between JavaInfo and java_info_for_compilation, though it changes some user-visible behavior and probably requires an --[no]incompatible_ flag.
Provide a factory function for JavaInfo that simply wraps a module_flags_info. Then, users can call java_common.merge([JavaInfo(…), java_common.wrap_module_flags_info(…)]).
Loosen java_common.merge input validation to allow JavaInfo-lookalikes rather than strictly JavaInfo objects. Then we can create a fake JavaInfo with module_flags_info set correctly, and call java_common.merge to legitimize it.
Expose _java_common_internal.wrap_java_info to users so that they can legitimize their JavaInfo lookalikes.

I'm happy to provide a patch. Please let me know which approach seems the most reasonable.

cc @hvadehra @cushon

Which category does this issue belong to?

Java Rules, Rules API

What underlying problem are you trying to solve with this feature?

No response

Which operating system are you running Bazel on?

No response

What is the output of `bazel info release`?

No response

If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.

No response

What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

The text was updated successfully, but these errors were encountered:

timothyg-stripe · 2023-11-02T22:29:21Z

I've made two PRs that implement approach 1 above:

Add add_exports & add_opens parameters to JavaInfo constructor #20036. This solves most of the issue.
Add --incompatible_java_info_merge_runtime_module_flags #20037, which changes the behavior of JavaInfo constructor to be more like java_info_for_compilation.

iancha1992 · 2023-11-02T23:14:34Z

@timothyg-stripe awesome! can you actually resubmit this one, timothyg-stripe#1 against the bazelbuild repo instead of your own, so that we can assign and review it?

timothyg-stripe · 2023-11-03T00:23:27Z

@iancha1992 Done – I refrained from doing that at first since there's a merge conflict between the two PRs, but I can fix up the conflict later when one of the two PRs gets merged.

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 #20033. Closes #20036. PiperOrigin-RevId: 580472097 Change-Id: I159e3410c5480ac683fd9af85bfd1d83ac0e6d8a

Change the behavior of `JavaInfo` constructor to be more like `java_info_for_compilation`, by merging in the `add_exports` and `add_opens` flags for `runtime_deps` in addition to just `deps` and `exports`. Guard it under an `--incompatible_` flag which defaults to false, but I'm hoping to make it default to true in 8.x. Second half of #20033 Closes #20037. PiperOrigin-RevId: 582982387 Change-Id: Ibff680f71efed82f20da7d9ee83f0bfa7e5f5697

timothyg-stripe · 2023-11-17T04:21:25Z

@iancha1992 Can we backport #20036 and #20037 to 7.0?

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

Change the behavior of `JavaInfo` constructor to be more like `java_info_for_compilation`, by merging in the `add_exports` and `add_opens` flags for `runtime_deps` in addition to just `deps` and `exports`. Guard it under an `--incompatible_` flag which defaults to false, but I'm hoping to make it default to true in 8.x. Second half of bazelbuild#20033 Closes bazelbuild#20037. PiperOrigin-RevId: 582982387 Change-Id: Ibff680f71efed82f20da7d9ee83f0bfa7e5f5697

keertk · 2023-11-18T15:42:53Z

Hi @timothyg-stripe would you mind just sending a PR against the release-7.0.0 branch with all the changes you need? Since we're cutting it very close to the final RC, this would save us the back and forth.
For now, I'm going to close the cherry-pick PRs we've opened.

keertk · 2023-11-18T15:50:54Z

@bazel-io fork 7.0.0

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

Change the behavior of `JavaInfo` constructor to be more like `java_info_for_compilation`, by merging in the `add_exports` and `add_opens` flags for `runtime_deps` in addition to just `deps` and `exports`. Guard it under an `--incompatible_` flag which defaults to false, but I'm hoping to make it default to true in 8.x. Second half of bazelbuild#20033 Closes bazelbuild#20037. PiperOrigin-RevId: 582982387 Change-Id: Ibff680f71efed82f20da7d9ee83f0bfa7e5f5697

timothyg-stripe · 2023-11-18T16:18:06Z

@keertk Done: #20260

iancha1992 · 2023-12-01T19:20:20Z

A fix for this issue has been included in Bazel 7.0.0 RC5. Please test out the release candidate and report any issues as soon as possible. Thanks!

timothyg-stripe added type: feature request untriaged labels Nov 2, 2023

timothyg-stripe assigned iancha1992, Pavank1992 and sgowroji Nov 2, 2023

iancha1992 unassigned sgowroji, Pavank1992 and iancha1992 Nov 2, 2023

iancha1992 added the team-Rules-Java Issues for Java rules label Nov 2, 2023

This was referenced Nov 2, 2023

Add add_exports & add_opens parameters to JavaInfo constructor #20036

Closed

Add --incompatible_java_info_merge_runtime_module_flags timothyg-stripe/bazel#1

Closed

This was referenced Nov 2, 2023

Add add_opens and add_exports support bazelbuild/rules_scala#1523

Open

Add --incompatible_java_info_merge_runtime_module_flags #20037

Closed

timothyg-stripe mentioned this issue Nov 7, 2023

Add add_exports & add_opens attributes to java_import #20035

Closed

timothyg-stripe closed this as completed Nov 16, 2023

iancha1992 mentioned this issue Nov 17, 2023

[7.0.0] Add add_exports & add_opens parameters to JavaInfo constructor #20244

Closed

keertk mentioned this issue Nov 18, 2023

[7.0.0] Add --incompatible_java_info_merge_runtime_module_flags #20243

Closed

keertk added this to the 7.0.0 release blockers milestone Nov 18, 2023

keertk mentioned this issue Nov 18, 2023

[7.0.0] Add add_exports & add_opens attributes to java_import #20250

Closed

keertk removed this from the 7.0.0 release blockers milestone Nov 18, 2023

bazel-io mentioned this issue Nov 18, 2023

[7.0.0] User-accessible JavaInfo constructor doesn't allow changing module_flags_info #20258

Closed

timothyg-stripe mentioned this issue Mar 7, 2024

Add support for add_exports/add_opens bazelbuild/rules_scala#1551

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User-accessible JavaInfo constructor doesn't allow changing module_flags_info #20033

User-accessible JavaInfo constructor doesn't allow changing module_flags_info #20033

timothyg-stripe commented Nov 2, 2023 •

edited

Loading

timothyg-stripe commented Nov 2, 2023 •

edited

Loading

iancha1992 commented Nov 2, 2023 •

edited

Loading

timothyg-stripe commented Nov 3, 2023

timothyg-stripe commented Nov 17, 2023 •

edited

Loading

keertk commented Nov 18, 2023 •

edited

Loading

keertk commented Nov 18, 2023

timothyg-stripe commented Nov 18, 2023

iancha1992 commented Dec 1, 2023

User-accessible JavaInfo constructor doesn't allow changing module_flags_info #20033

User-accessible JavaInfo constructor doesn't allow changing module_flags_info #20033

Comments

timothyg-stripe commented Nov 2, 2023 • edited Loading

Description of the feature request:

Which category does this issue belong to?

What underlying problem are you trying to solve with this feature?

Which operating system are you running Bazel on?

What is the output of bazel info release?

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

Have you found anything relevant by searching the web?

Any other information, logs, or outputs that you want to share?

timothyg-stripe commented Nov 2, 2023 • edited Loading

iancha1992 commented Nov 2, 2023 • edited Loading

timothyg-stripe commented Nov 3, 2023

timothyg-stripe commented Nov 17, 2023 • edited Loading

keertk commented Nov 18, 2023 • edited Loading

keertk commented Nov 18, 2023

timothyg-stripe commented Nov 18, 2023

iancha1992 commented Dec 1, 2023

timothyg-stripe commented Nov 2, 2023 •

edited

Loading

What is the output of `bazel info release`?

If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.

What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?

timothyg-stripe commented Nov 2, 2023 •

edited

Loading

iancha1992 commented Nov 2, 2023 •

edited

Loading

timothyg-stripe commented Nov 17, 2023 •

edited

Loading

keertk commented Nov 18, 2023 •

edited

Loading