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 full JPMS module descriptor #232

Open
A248 opened this issue Jun 27, 2021 · 13 comments
Open

Add full JPMS module descriptor #232

A248 opened this issue Jun 27, 2021 · 13 comments
Labels
for: team-attention An issue we need to discuss as a team to make progress type: enhancement A general enhancement

Comments

@A248
Copy link

A248 commented Jun 27, 2021

Feature Request

Add a full module descriptor for improved use with JPMS. See also reactive-streams/reactive-streams-jvm#531

Is your feature request related to a problem? Please describe

r2dbc-spi doesn't have a full module descriptor. At the moment, it only has a stable automatic module name.

A full module descriptor is a requirement for some tools, like JLink. The module system in general has better support for full modules – for example, compiling requires transitive r2dbc.spi currently produces a warning about automatic modules.

Describe the solution you'd like

A module descriptor is added to the R2DBC-Spi jar, either at the root of the jar, or in a multi-release directory.

Describe alternatives you've considered

Continue to use the Automatic-Module-Name manifest entry. This has the drawbacks mentioned previously.

Teachability, Documentation, Adoption, Migration Strategy

Users don't have to do anything. The module name would stay the same, but instead of being an automatic module, R2DBC-Spi would become a full JPMS module.

The feature would ideally be mentioned in the release notes.

@mp911de mp911de added type: enhancement A general enhancement for: team-attention An issue we need to discuss as a team to make progress labels Jun 27, 2021
@Michael-A-McMahon
Copy link
Contributor

Adding module-info.java would mean that R2DBC is no longer compatible with JDK 8.

Oracle R2DBC is already modular, and requires JDK 11 or newer, so it wouldn't be effected by this change.

So what about the other drivers and frameworks that consume R2DBC? Are we all ready to leave JDK 8 behind?

@lukaseder
Copy link
Contributor

So what about the other drivers and frameworks that consume R2DBC?

The jOOQ Open Source Edition has JDK 11 as a baseline already, so jOOQ could live with that.

@A248
Copy link
Author

A248 commented Nov 11, 2021

It isn't true that adding a module descriptor necessitates dropping Java 8 compatibility. There are a few ways to create a modular jar which is compatible with JDK 8.

@Michael-A-McMahon
Copy link
Contributor

Oh, I see, you're right!

Creating a multi-release jar seems like a nice approach:
https://openjdk.java.net/jeps/238#Modular-multi-release-JAR-files

@bowbahdoe
Copy link

Ran into this today where I can't use JOOQ w/ jlink on account of this.

@Sineaggi
Copy link

Unfortunately r2dbc-spi has a dependency on the jsr305 annotations jar.

@bowbahdoe
Copy link

bowbahdoe commented Sep 13, 2023

Thats an annoying thing for a library that has its own nullability annotations anyways.

Just writing this down

  • I want to use Jooq and deploy my app with jlink
  • Jooq has a module descriptor, but r2dbc-spi does not
  • r2dbc-spi cannot provide a stable module descriptor because it depends on the jsr305 jar, which does not have an automatic module name, explicit module descriptor, or anyone able to publish new versions.
  • Its only usage of jsr305 is to annotate its own nullability annotations.

The paths forward are

  • Drop the jsr305 annotations. Don't know what tool wants @Nonnull(when = When.MAYBE), but I'd guess the biggest breakage would be kotlin, which has sanctioned support for jsr305.
  • Replace jsr305 annotations with jetbrains, checkerframework, or whatever other one kotlin supports
  • Put a pin in this for half a decade or more until jspecify does their thing
  • Put a pin in this for a full decade or two until valhalla puts nullness into the language + a decade more before someone wants to bump the minimum java version.

This isn't my first time dealing with this BS - I even republished jsr305 under a GAV i own - but god is it frustrating.

@lukaseder
Copy link
Contributor

  • Drop the jsr305 annotations. Don't know what tool wants @Nonnull(when = When.MAYBE), but I'd guess the biggest breakage would be kotlin, which has sanctioned support for jsr305.

jOOQ itself has a org.jetbrains.annotations module dependency for this and other related reasons, assuming the folks who benefit the most from such annotations are indeed kotlin users, and kotlin = JetBrains.

@mp911de
Copy link
Member

mp911de commented Sep 13, 2023

jsr305 isn't required, however, it appears in the pom as optional dependency. We define an automatic module name, however, we're reluctant to introduce a module descriptor as doing so causes a lot of downstream effects and we're not convinced that introducing module descriptors provides more benefit than hassle for our community.

Can you help me understand what trouble the jsr305 dependency causes for you? I've been under the impression, that absent annotation libraries are just fine for Java runtime and the compiler (except for silly warnings during the build).

@bowbahdoe
Copy link

bowbahdoe commented Sep 13, 2023

we're reluctant to introduce a module descriptor as doing so causes a lot of downstream effects and we're not convinced that introducing module descriptors provides more benefit than hassle for our community.

Can you talk about the downstream effects you anticipate? I've heard of some people putting a module-info.class at the top level while also supporting java 8, and that causes issues

I.E. they did this

a/
   A.class [Java 8]
module-info.class [Java 9+]

Instead of

a/
   A.class [Java 8]
META-INF/
  MANIFEST.MF [set Multi-Release: true]
  versions/
    9/
        module-info.class [Java 9]

or

a/
   A.class [Java 9]
module-info.class [Java 9]

And that tripped up tooling that just scanned all class files.

Can you help me understand what trouble the jsr305 dependency causes for you?

This issue better job of explaining than I can google/guava#2960

@mp911de
Copy link
Member

mp911de commented Sep 13, 2023

We've encountered various behavioral changes in tools once modules are used. Another aspect here is that the minority of our maintainers use modules regularly so there's not sufficient proficiency to work through issues coming from the module system. We have a similar case with OSGi metadata as well that some folks would like to see a smoother integration while maintainers can't afford to maintain these things.

Arguably, the sentiment would be a different one if the maintainer team would be different.

@A248
Copy link
Author

A248 commented Sep 14, 2023

I'm going to clarify matters here, because it's important not to lose track of context. R2DBC-Spi is a service-provider interface designed as a low-level building block for nonblocking database APIs. It defines an API. It has no implementation. It has no runtime dependencies.

Adding a module descriptor breaks no contracts. It merely enhances module metadata for use with the Java Platform Module System added in 2017. R2DBC came later, in 2019, with its first stable release in 2022

But old software is broken and fragile, there are lots of "downstream effects" due to not properly implementing support for a feature introduced 6 years ago. A class file under META-INF/versions is somehow supposed to be a valid Java 8 class. Tools are broken, outdated IDEs are buggy. Plus, adding a module creates "hassle" for the community.

R2DBC was released after JPMS without JPMS. The maintainers don't use modules -- except for the fully-modularized JDK itself -- so we shouldn't expect them to know anything about the module system or its benefits. Once created, the module-info file might be easy to maintain, but someone who has barely worked with the module system, or has only experienced the ill effects of interactions with broken legacy software, will never know that.


I'm sorry if I sound cynical; I decided that a dose of hard reality (at least, according to my perspective) was what this discussion needed.

@mp911de
Copy link
Member

mp911de commented Sep 14, 2023

I think this pretty much sums it up and your summary reflects the current state properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants