micronaut-core icon indicating copy to clipboard operation
micronaut-core copied to clipboard

Add module-info to micronaut-inject and micronaut-core

Open msgilligan opened this issue 4 years ago • 62 comments

Feature description

It has been possible to use Micronaut dependency injection as a Java Module System-compatible standalone dependency injection framework by processing the jars with tools like Badass Jlink Plugin since the 1.x series. As Micronaut and its dependencies have evolved, it is now possible for (at least) micronaut-inject and micronaut-core to be "fully modularized" by adding module-info.java files to their jars.

This can be done while retaining Java 8 compatibility via two different mechanisms:

  1. Add the module-info.class file directly to an otherwise JDK8-compatible jar
  2. Create Multi-Release JARs that completely hide module-info.class from JDK8

The current dependency tree looks like:

+--- project <root>
+--- io.micronaut:micronaut-inject:3.1.1
|    +--- org.slf4j:slf4j-api:1.7.29 -> 2.0.0-alpha5
|    +--- javax.annotation:javax.annotation-api:1.3.2
|    +--- jakarta.inject:jakarta.inject-api:2.0.0 -> 2.0.1
|    +--- jakarta.annotation:jakarta.annotation-api:2.0.0
|    +--- io.micronaut:micronaut-core:3.1.1
|    |    \--- org.slf4j:slf4j-api:1.7.29 -> 2.0.0-alpha5
|    \--- org.yaml:snakeyaml:1.29

Analyzing each 3rd-party dependency:

  • slf4j-api: Is an automatic module that can be updated by applications to 2.0.0-alpha5 or later to get fully-modular
  • :javax.annotation-api - is an optional dependency that can be a requires static
  • jakarta.inject-api - has been updated to fully-modular 2.0.1 via 2860a466b16206a83fc5eefad3bee89049e18fd6
  • jakarta.annotation-api - fully-modular
  • snakeyaml - is an optional dependency that can be a requires static

So it looks like micronaut-inject and micronaut-core are ready to go fully modular.

It would be great to see these two jars released with module-info support in Milestone 3.2.0

I'm not 100% sure my analysis is correct, but I'd like to at least start a discussion here.

msgilligan avatar Oct 21 '21 20:10 msgilligan

@msgilligan so this would be a good area to contribute to the framework since nobody on the Micronaut team that I am aware of has a good understanding of what it takes to make Micronaut modular nor add tests for modularity

Maybe you and @aalmiray could advise

graemerocher avatar Oct 22 '21 07:10 graemerocher

Happy to support here, too.

sormuras avatar Oct 22 '21 17:10 sormuras

I would welcome any advice from @aalmiray, @gunnarmorling, @hansolo, @siordache, or @sormuras -- all of these guys all know much more about modules than I do.

I've had good luck running Micronaut in a modular JavaFX environment after reprocessing with the @siordache 's Badass JLink Plugin, but the applications have been fairly simple and I have not been able to implement much automated testing yet.

(A real benefit for me has been the ability to create reactive services -- in my case using RxJava 3 -- that are portable between a modular JavaFX application and a non-modular Micronaut server)

My proposal would be to do the following:

  1. Using the appropriate Gradle plugin (I need to research which one this would be) add the module-info directly to the JAR (option 1 above) -- As far as I know this will only create problems for a 2-3 year-old Android toolchain, but that issue has been resolved for a few years now.

  2. Develop and review the module-info.java files that specify which functionality is exported and which functionality is private. (The generated "merged-module" from BA JLink Plugin I've been using essentially exports everything)

  3. Implement a minimal set of unit tests. (or more if others are able to help here)

  4. Update the Micronaut documentation to say (something like) modular support is "incubating" and limited to the application of standalone D.I.

  5. Collect feedback from testing of the "incubating" version.

  6. Work to add more unit tests over time.

  7. Eventually move towards making the other components of the Micronaut stack modular.

This is based on the following assumptions:

A. The risk from adding module-info to a JDK 8 JAR is minimal or non-existent (this practice seems to be more and more common, perhaps even a best practice)

B. The presence of module-info in Micronaut apps using JDK 9+ will not by itself cause non-modular apps to be run as modular.

C. The Micronaut team is comfortable communicating that the modular support is "incubating" (or whatever terminology is most appropriate)

How does this approach sound to everyone?

msgilligan avatar Oct 22 '21 17:10 msgilligan

Update: It looks like the addMainModuleInfo task of ModiTect Gradle Plugin should do what we need.

msgilligan avatar Oct 22 '21 20:10 msgilligan

Another option is the badass-jar-plugin, which is a lightweight plugin dedicated to this task. It allows both options (adding the module-info.class file directly to the JAR or creating a Multi-Release JAR).

siordache avatar Oct 22 '21 20:10 siordache

Thanks, @siordache! Yes, it looks like the badass-jar-plugin is simpler and a better fit for our relatively minimal requirements.

msgilligan avatar Oct 22 '21 20:10 msgilligan

I'll wait for more feedback, but I'm leaning towards creating a draft pull request for adding a module-info to micronaut-core using badass-jar-plugin and then getting specific feedback on that PR.

@graemerocher do you know of any tools in the Micronaut ecosystem that might choke on a module-info in the root of a JAR?

msgilligan avatar Oct 22 '21 20:10 msgilligan

Do not put module-info.class at the root of the JAR. Instead use /META-INF/versions/<number>. I suggest number to be 11.

aalmiray avatar Oct 22 '21 20:10 aalmiray

Do not put module-info.class at the root of the JAR. Instead use /META-INF/versions/<number>. I suggest number to be 11.

Is there a specific reason? @sormuras (in a separate conversation) recommended option 1 (root of jar.) I'm happy to do it either way and the badass-jar-plugin can easily be configured for option 1 or option 2.

msgilligan avatar Oct 22 '21 20:10 msgilligan

Do not put module-info.class at the root of the JAR. Instead use /META-INF/versions/<number>. I suggest number to be 11.

Is there a specific reason? @sormuras (in a separate conversation) recommended option 1 (root of jar.) I'm happy to do it either way and the badass-jar-plugin can easily be configured for option 1 or option 2.

If you put it at the root then it becomes a class visible to everyone, just like any other class is. If you put it at /META-INF/versions/<number> then it becomes invisible to most tools (as many are unaware of JPMS and MultiResource JARs). However the JVM will figure it out correctly when running the application, same as the compiler.

aalmiray avatar Oct 22 '21 20:10 aalmiray

All JUnit's modules have their module-info.class in each JAR file's root for years, and we didn't receive a single complain by our users. Tools and frameworks seem to have no troubles with that approach. Those who have (or had) scanned entire JAR files anyway, and fail(ed) when loading module-info.class from every location, including /META-INF/versions/<number>.

Taking jar's CLI into account, I prefer storing a module descriptor in the root of the .jar archive. If you don't, you need to specify a release version number in order let jar describe a module:

jar --describe-module --file ...jar

releases: 9

No root module descriptor, specify --release

jar --describe-module --file ...jar --release 9

releases: 9

MODULE-NAME jar:file:///....jar!/META-INF/versions/9/module-info.class
exports P
exports ...
requires java.base mandated
requires ...
...

This reads just simpler:

jar --describe-module --file ...

MODULE-NAME jar:file:///...jar!/module-info.class
exports ...
requires java.base mandated
...

sormuras avatar Oct 23 '21 08:10 sormuras

Note that in terms of micronaut-inject dependencies we see the following:

  • jakarta.inject-api - module-info in the root
  • jakarta.annotation-api - module-info in the root
  • slf4j-api - MR JAR (2.0.0-alpha)

Update: I just discovered that jakarta.inject-api is also available as an MR jar. The release notes for the MR JAR say:

This is a 2.0.1 service release with a multi-release jar that adds the module-info class to META-INF/versions/9/module-info.class using the https://github.com/moditect/moditect plugin.

This is an experimental release to allow users to choose between this format of a modularized api jar and the 2.0.1 release that has the module-info.class in the root of the jar as is the convention adopted for EE10. Unless there is user feedback that this format has sufficient advantages over the 2.0.1 release format, this is not a format we intend to support in the future.

msgilligan avatar Oct 25 '21 16:10 msgilligan

On Friday I looked at @siordache 's badass-jar-plugin and I opened https://github.com/beryx/badass-jar-plugin/issues/2 to suggest that it be made even more lightweight on Gradle 6.4+ (by using the built-in module support in Gradle) and this morning I see there is a 2.0.0-rc1 version that implements my suggestion!

So it looks like the 2.0.0 version of badass-jar-plugin is what we should use (given that Micronaut is using Gradle 7.2)

msgilligan avatar Oct 25 '21 16:10 msgilligan

Update: I've been testing badass-jar-plugin and it is working well for my use case.

I would like to try using it to make a PR.

I understand that Micronaut 4 will use/require JDK 11, which will eventually make use of the plugin unnecessary. This is great news! Not sure if I should wait for a 4.x branch or try to start with the plugin...

msgilligan avatar Jan 07 '22 17:01 msgilligan

4.x is now being worked on. Is this on the radar for the new version or is the idea dropped?

nbrugger-tgm avatar Mar 14 '23 06:03 nbrugger-tgm

idea hasn't been dropped certainly can be added now in 4.x but would like to see community contribution because none of the team use the module system

graemerocher avatar Mar 14 '23 07:03 graemerocher

Thanks for the answer, I am trying at the moment to get a working draft together - but i have a few questions:

  • Which submodules should be modularized (annotation processors for example shouldn't matter right?)
    • micronaut-aop for example seems like it is included in builds using compile scope reference but contains classes that are needed during runtime (like InterceptorChain) from my point of view - possibly wrong
    • in the title of this issue micronaut-inject is stated but i don't see the point in this - correct me if i am wrong - as far as i understand the annotation processor is more like a javac extension that doesn't benefits from the JPMS (most noticeable advantage being a custom minimal java runtime)
    • My idea for now would be just modules that are included in the runtime scope is that fine?
  • Are there specific rrules/requirements regarding 3rd party gradle plugins like badass-jar-plugin ?
  • Just to confirm: Java 8 does not need to be supported using "multiversion jar" right? minimal version for 4.x is Java 11
  • Plugin tasks for things like dockerBuildModularJre are out of the scope here, right?

Edit:

  • With JPMS/modular support you normally define packages which you export (publi api) and which packages you do not (internal/private implementation). In your modules you use package private. Is it ok for me to move internal classes to a internal package that is hidden with module-info.java (and be forced to make them publick as drawback) or leave is and expose them in the module-info?

nbrugger-tgm avatar Mar 23 '23 16:03 nbrugger-tgm

Opened draft PR: #9002

nbrugger-tgm avatar Mar 24 '23 08:03 nbrugger-tgm

@aalmiray

I now experimented a bit with modularizing micronaut runtime components and discovered a few topics that are problematic for the adoption of JPMS. While I could still implement it, it would cause changes and I would need to make a decision, which I don't feel entitled to.

These are the most problematic points that I would like to be decided by the micronaut team, or at least somebody who is working on micronaut for longer.

  1. Kotlin std commons (kotlin-stdlib-commons) is not a java module, but is referenced in code that should be modularized inject and therefore inject-java while it is possible to use them using a 3rd party plugin extra-java-module-info it is either non-modular (AutomaticModule) or unstable and possibly causes runtime issues when using modules. (Why) Is it necessary that kotlin stdlib(commons) is included in java apps, or was it just a "doesn't hurt us for now" thing? What would be the best solution. Edit: only affects stdlib-commons not stdlib itself 1.1. Keep as is using the Gradle plugin and "faking" it as a module. (risky since it is not just annotations and simple code) 1.2. Separate kotlin things into inject-kotlin (don't know if possible, since I guess it would be done already if it was)
  2. javax.inject is still used and is not modular (and will likely never be) similar problem as above but here using the extraJavaModuleInfo plugin is not risky but still a decision I don't want to make 2.1. Drop javax support in micronaut 4.x in favor of jakarta (which is modular) 2.2. Offer javax support as a new dependency like micronaut-javax-support which is not modular 2.3 use the plugin and its fine 2.4 apparently jakarta released the javax classes (with the javax package) under their name but modularized. Strange but works - no other changes needed : https://github.com/javax-inject/javax-inject/issues/33#issuecomment-1489250744
  3. core-processor has a "shared package" with other modules like io.micronaut.aop which is not just forbidden but just impossible with JPMS since every package needs to belong to exactly one module. renaming the root package of core-processorfrom io.micronautto io.micronaut.core.processor has 2 huge issues (which i am currently stuck on). The renaming causes >450 changes (basically every visitor, processor, writer etc). And also micronaut internally relies on package private visibility so the access to the classes would need several changes. 3.1. Creating a strict separation between processor code, runtime code and shared code. This way the duplicated packages could continue to exist because they never "meet" → there is no artifact that includes runtime and processor code on the runtime side and on the processor side it doesn't matter since they won't be modular (AFAIK).
    Pros: cleaner module architecture in the end, slimmer runtime dependencies
    Cons: requires quite a lot of change also to the Gradle module structure and refactorings to clean the accesses 3.2. Renaming the package and making it a proper module. Pros: easy to implement, Cons: breaking changes for AP devs and 400+ file PR

I hope these issues are not a "won't happen" for JPMS support.

Sorry for creating such a "mega thread" but I tried to communicate everything I found (still quite new to OSS) and not make it more work for you than it would be to do it yourself, thank you.

TL;DR it is problematic - there are design issues that need decisions from the core team.

nbrugger-tgm avatar Mar 29 '23 20:03 nbrugger-tgm

Ad 1. "Kotlin" does ship as module(s) - not?

kotlin.reflect=https://repo.maven.apache.org/maven2/org/jetbrains/kotlin/kotlin-reflect/1.8.20-RC2/kotlin-reflect-1.8.20-RC2.jar kotlin.stdlib=https://repo.maven.apache.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.8.20-RC2/kotlin-stdlib-1.8.20-RC2.jar kotlin.stdlib.jdk7=https://repo.maven.apache.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.8.20-RC2/kotlin-stdlib-jdk7-1.8.20-RC2.jar kotlin.stdlib.jdk8=https://repo.maven.apache.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk8/1.8.20-RC2/kotlin-stdlib-jdk8-1.8.20-RC2.jar

sormuras avatar Mar 30 '23 06:03 sormuras

Thanks, updated my question. stdlib-common has no module descriptor also in version 1.8.20-RC2, but you are right kotlin-stdlib was wrong

nbrugger-tgm avatar Mar 30 '23 07:03 nbrugger-tgm

  • core-processor doesn't need to be modularised. It is only used with the annotation processors for compilation and is not public API
  • Micronaut still supports javax.inject but it is no longer a runtime dependency and jakarta.inject is used by default

graemerocher avatar Mar 30 '23 10:03 graemerocher

  • Kotlin is also an optional dependency, so again not sure it needs to be described in any module metadata

graemerocher avatar Mar 30 '23 10:03 graemerocher

Also to expand on the core-processor point, all modules that depend on this module do not need to be modularised because they are compilation time only (ie inject-groovy, inject-kotlin,inject-java etc.)

graemerocher avatar Mar 30 '23 10:03 graemerocher

Also to expand on the core-processor point, all modules that depend on this module do not need to be modularised because they are compilation time only (ie inject-groovy, inject-kotlin,inject-java etc.)

Maybe I am misunderstanding, but the :context module has a compileOnly dependency to :core-processor and since :context is a runtime module (public api) JPMS complains that context reads the package io.micronaut.inject.annotation from both io.micronaut.core_processor and micronaut.inject.

error: module micronaut.context reads package io.micronaut.inject.annotation from both io.micronaut.core_processor and micronaut.inject

And since :inject is a transitive runtime dependency (api) of :context it should be module as well

So while :core-processor is not a runtime requirement, it is still a dependency (class/module-path) in a modularized artifact.

:context

    compileOnly project(':core-processor')

For example the class AsyncTypeElementVisitor is an annotation-processor (or an lazy loaded extension for one) but resides in :context. So if I understood right this "AsyncTypeElementVisitor" should be in inject-java/groovy/kotlin and not in context

Maybe there is a misunderstanding how JPMS deals with runtime/compile time dependencies:

  • You are right that there is no use in modularizing annotation processors and their dependencies. And JPMS is (equally to me) not concerned about them
  • BUT compile time dependencies (compileOnly in Gradle) are the same to JPMS as runtime dependencies (implementation and api in Gradle) since the JPMS is mostly concerned with compilation. And when you compile the project, both runtime and compile time artifacts are on the class-path (or module-path respectively)
  • When you have a compile time dependency (such as slf4j-api and :core-processor) they have to be modules or automatic modules to be included in the compilation. And you define that they are compile time modules in the module descriptor with requires static instead of requires so that JPMS knows if it needs to include them when running the app (or building a custom JVM image)

nbrugger-tgm avatar Mar 30 '23 12:03 nbrugger-tgm

as expressed by compileOnly it is a dependency only needed for compilation, if JPMS is not able to handle that case then 🤷‍♂️

graemerocher avatar Mar 30 '23 14:03 graemerocher

Ok for everyone tracking the issue: modularizing micronaut-core is only possible with internal breaking changes that would require downsteam projects (such as micronaut-openapi) to update and be incompatible with micronaut 3.x.

And the micronaut team seems to not want this kind of breaking changes so as far as I see it there won't be modular support for micronaut

For more detailed reasoning read the last message in my PR linked to this issue. Maybe I overlooked something and my reasoning is wrong. https://github.com/micronaut-projects/micronaut-core/pull/9002#discussion_r1154535688

nbrugger-tgm avatar Apr 22 '23 11:04 nbrugger-tgm

@nbrugger-tgm so no support for JLink either? I'm using the latest versions of Micronaut and I can't package my application because of the errors described here: GitHub

palexdev avatar Jun 08 '23 12:06 palexdev

When the badass-jlink thing does not work jlink / JPMS will not work with micronaut unless the micronaut team is willing to do this breaking change. The base problem is that 2 artifacts use the same package which is strictly forbidden by JPMS

nbrugger-tgm avatar Jun 08 '23 16:06 nbrugger-tgm

When the badass-jlink thing does not work jlink / JPMS will not work with micronaut unless the micronaut team is willing to do this breaking change. The base problem is that 2 artifacts use the same package which is strictly forbidden by JPMS

Although it does not seem to be the issue in my case: io.micronaut.context.exceptions.NoSuchBeanException

My next try is to add the bean definitions programmatically to the merged module. When building a shadowJar we need to specify mergeServiceFiles() too, I believe that's the issue but I may be wrong

Anyhow, if I can't make jlink work no matter what, what other alternatives do I have?

palexdev avatar Jun 08 '23 16:06 palexdev