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

Modular Java #16502

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Modular Java #16502

wants to merge 6 commits into from

Conversation

sgammon
Copy link
Contributor

@sgammon sgammon commented Apr 15, 2024

Summary

This changeset attempts to discover the minimal set of changes necessary to add module-info.java definitions to Protocol Buffers. Fixes and closes #16133.

This is a draft PR for now. I'm looking for feedback and to setup an integration testsuite downstream with popular Protobuf-based projects to make sure things work as expected.

Note

Re-filing after a bad rebase on #16178.

PR Tree

This PR is applied on top of the following:

Relevant PRs + issues upstream

JAR Structure

JARs issued as part of the Protobuf Java release now have Multi-Release: true instead of Automatic-Module-Name within their MANIFEST.MF, making them eligible to be considered as Multi-Release JARs (or, MRJARs). Such JARs can include a module-info.class without interfering with bytecode targeting at JDK8 or earlier, where modules are not supported yet. These JARs are thus called "modular MRJARs."

For example, after building Protobuf Java with bazel build //java:release, vim bazel-bin/java/core/amended_lite_mvn-project.jar shows:

JAR structure:

META-INF/MANIFEST.MF
META-INF/
META-INF/versions/
META-INF/versions/9/
META-INF/versions/9/module-info.class
# ...

In the MANIFEST.MF:
Screenshot 2024-03-14 at 5 25 16 PM

Modular Java downstream

For projects that build with Protobuf Java on the classpath, no change is experienced by the end-user; for projects which build on modulepath:

module my.module {
  requires com.google.protobuf;
  requires com.google.protobuf.kotlin;  // optional
  requires com.google.protobuf.util;  // optional
}

Open module

Protobuf, Protobuf Util, and Protobuf Kotlin are expressed as open modules. This is the case because Protobuf Java is often used reflectively by library consumers. By default, all packages provided by Modular Java protobuf artifacts are open as a result.

Exports + Lite Variants

Protobuf Java Core, Protobuf Java Util, and Protobuf Kotlin all reside in one package path each, which is extremely lucky and convenient for Modular Java because there is no split package problem.

Kotlin Lite/Kotlin and Java Core/Java Lite are meant to be used exclusively within the classpath, but there is no enforcement possible for this case from the compiler or VM, at least not in a consistent way across environments. Building on the modulepath solves this because Kotlin Lite/Kotlin and Java Core/Java Lite share a module coordinate and export an identical package path.

Thus, only one of (Kotlin/Kotlin Lite) and (Java Core/Java Lite) can be used at a time when building on the modulepath. This guarantee is enforced by the compiler and by the JVM at runtime.

Changelog

  • feat(java): support for modular java
  • fix: transition to rules_kotlin

@sgammon sgammon requested review from a team as code owners April 15, 2024 04:51
@sgammon sgammon requested review from haberman and googleberg and removed request for a team April 15, 2024 04:51
Fixes and closes protocolbuffers#16169 by pinning the
build to Bazel `6.3.2`.

Signed-off-by: Sam Gammon <[email protected]>
Fixes and closes protocolbuffers#16170 by adding missing
test-gen mappings and exclusions.

- fix: missing test-gen mappings
- fix: missing test exclusions

Signed-off-by: Sam Gammon <[email protected]>
@vietj
Copy link

vietj commented Jul 14, 2024

is this PR under consideration for being merged in the project ?

@vietj vietj mentioned this pull request Aug 8, 2024
26 tasks
@googleberg
Copy link
Member

@sgammon Thank you for the proposal.
There's a few too many things going on in this PR.
Any updates to library dependencies should be done in a separate PR.
Changes to use the more modern rules_kotlin repo can be in a separate PR.
We don't want to maintain extensive infrastructure for OSGI if we can help it. The more you can cut down the amount of special logic and artifacts to support this, the better. We'd especially like to avoid extensive shell scripts. Is it possible to get all of the METADATA modifications done by the OsgiWrapper process?

@sgammon
Copy link
Contributor Author

sgammon commented Sep 14, 2024

@googleberg I'm happy to try and make this a smaller PR, yes. I'll need to review my code in order to answer your other comments with context.

Generally speaking I try to keep PRs with Google's software as small as possible, though, so if something is in this PR, I am probably under the impression that it is needed. If you can see routes toward making things smaller that I don't see, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a module-info.java
5 participants