Skip to content

[MNG-11642] Add JPMS module support to Maven 4 - #12135

Open
gnodet wants to merge 4 commits into
masterfrom
plume-football
Open

gnodet wants to merge 4 commits into
masterfrom
plume-football

Conversation

@gnodet

@gnodet gnodet commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add explicit module-info.java to 17 modules (11 API + 6 impl) with proper requires, exports, uses, and provides directives
  • Add Automatic-Module-Name manifest entries to all 16 remaining modules (3 impl + 13 compat) via maven-jar-plugin configuration in root POM
  • Use qualified exports for all non-API impl packages — only org.apache.maven.api.* is public API
  • Configure javadoc plugin for JPMS compatibility (legacyMode for aggregate reports, source exclusions, --add-exports for plexus-interpolation)
  • Fix nullable DI collection handling in DefaultModelProcessor and DefaultModelBuilder
  • Resolves Assign automatic module names to core Maven components #11642

Details

Modules with module-info.java (17)

API modules (11): maven-api-annotations, maven-api-di, maven-api-xml, maven-api-metadata, maven-api-model, maven-api-settings, maven-api-toolchain, maven-api-plugin, maven-api-core, maven-api-spi, maven-api-cli

Impl modules (6): maven-di, maven-xml, maven-support, maven-impl (open module), maven-jline, maven-logging

Modules with Automatic-Module-Name only (16)

Impl (3): maven-core, maven-cli, maven-testing — cannot use module-info.java due to split packages with compat modules or complex DI/compat dependencies

Compat (13): All compat modules — split packages among themselves prevent named module status

Qualified exports for non-API packages

Only org.apache.maven.api.* packages are Maven 4's public API. All implementation packages use qualified exports restricted to internal consumers:

  • maven-impl: All 14 org.apache.maven.impl.* exports qualified to maven-core, maven-cli, maven-testing, maven-compat, maven-embedder. Removed unused org.apache.maven.impl.model.profile. Only org.apache.maven.api.services.model remains unqualified (it's an API package).
  • maven-di: org.apache.maven.di.impl qualified to internal consumers (kept org.apache.maven.di unqualified as DI API)
  • maven-logging: org.apache.maven.slf4j qualified to maven-cling, maven-embedder, maven-core
  • maven-jline: org.apache.maven.jline qualified to maven-logging, maven-cling, maven-embedder
  • maven-xml, maven-support: left unqualified (widely used by 5-15+ modules; qualifying would be verbose and fragile)

Since all consumers currently compile in classpath mode (no module-info.java), the qualified exports serve as documentation of intent and will be enforced when those modules gain module descriptors.

Build infrastructure changes

  • Root POM: maven-jar-plugin in <pluginManagement> sets Automatic-Module-Name from ${javaModuleName} property
  • Root POM: maven-javadoc-plugin configured with sourceFileExcludes for module-info.java to prevent javadoc tool from switching to modular mode
  • Root POM (reporting profile): legacyMode=true for aggregate javadoc — forces classpath mode to avoid "named and unnamed modules" conflict
  • maven-compat, maven-model-builder: --add-exports=org.codehaus.plexus.interpolation/org.codehaus.plexus.interpolation.util for javadoc — plexus-interpolation 1.29 does not export the .util package in its module descriptor
  • maven-impl, maven-di, maven-jline: annotationProcessorPaths for DiIndexProcessor — required when compiling with module-info.java since the processor is no longer on the classpath
  • maven-impl: useModulePath=false in surefire to preserve existing test behavior on the classpath

Bug fixes included

  • DefaultModelProcessor / DefaultModelBuilder: DI framework injects null for @Nullable collections when no providers exist; added null-safe defaults (Map.of() / List.of())
  • LookupInvoker: Reordered ProjectBuildLogAppender creation to occur before terminal initialization
  • maven-jline: Removed requires static org.jline.terminal.ffm — the FFM terminal provider is discovered via ServiceLoader at runtime, and the requires static directive fails compilation on Java 17 CI where the module doesn't exist

Notable design decisions

  • maven-impl is an open module because Maven's DI framework needs reflective access to instantiate components
  • maven-cli uses Automatic-Module-Name instead of module-info.java due to split packages with compat modules it depends on (e.g., org.apache.maven.settings in both maven-settings and maven-core)
  • API modules use requires transitive for inter-API dependencies since API types leak across module boundaries
  • maven-api-model declares uses ModelObjectProcessor for ServiceLoader support
  • maven-impl declares uses RootDetector for ServiceLoader support

Test plan

  • mvn verify -B — all modules compile and tests pass
  • CI: initial-build, all 9 integration-tests, all 9 full-build (including mvn site -Preporting) pass
  • Verified jar --describe-module shows correct module descriptors for all 17 module-info modules
  • Verified Automatic-Module-Name manifest entries present in all 16 remaining module JARs

🤖 Generated with Claude Code

@desruisseaux

Copy link
Copy Markdown
Contributor

Thanks. There are a lot of export, for example in this implementation module. Are all these exported packages public API? If not, are there some exports that we could omit, or restrict to qualified exports?

@gnodet

gnodet commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

Claude Code on behalf of Guillaume Nodet

Good point. None of the org.apache.maven.impl.* packages are public API — only org.apache.maven.api.* is. I've pushed a commit (252ab5e) that addresses this:

maven-impl:

  • exports org.apache.maven.api.services.model — kept unqualified (it's an API package)
  • exports org.apache.maven.impl.model.profile — removed entirely (not used by any external module)
  • All 14 other org.apache.maven.impl.* exports — now qualified to org.apache.maven.core, org.apache.maven.cling, org.apache.maven.testing, org.apache.maven.compat, org.apache.maven.embedder

Other impl modules:

  • maven-di: qualified org.apache.maven.di.impl to the same 5 internal consumers (kept org.apache.maven.di unqualified as it's the DI API)
  • maven-logging: qualified org.apache.maven.slf4j to maven-cling, maven-embedder, maven-core
  • maven-jline: qualified org.apache.maven.jline to maven-logging, maven-cling, maven-embedder

Left unchanged:

  • API modules (11): all exports are org.apache.maven.api.* — they are the public API
  • maven-xml: single package, used by 5+ modules
  • maven-support: 6 generated-model packages used by 15+ modules — qualifying would be very verbose and fragile

Since all consumers currently use Automatic-Module-Name (no module-info.java), they compile in classpath mode where JPMS boundaries aren't enforced. The qualified exports serve as documentation of intent and will be enforced if/when those modules gain their own module-info.java.

@gnodet
gnodet marked this pull request as ready for review June 17, 2026 13:31
Add explicit module-info.java to 17 modules (11 API + 6 impl) and
Automatic-Module-Name manifest entries to all 17 remaining modules.

Only org.apache.maven.api.* packages are public API. All implementation
packages use qualified exports restricted to internal consumers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet gnodet added mvn4 enhancement New feature or request labels Jun 22, 2026
@gnodet gnodet added this to the 4.1.0 milestone Jun 24, 2026

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review notes — the JPMS module support looks correctly implemented across all 34 modules. All 17 module-info.java module names are consistent with their POM javaModuleName properties, the qualified exports in maven-impl properly restrict internal packages, and the build infrastructure changes (javadoc exclusions, legacyMode, annotationProcessorPaths, useModulePath=false) are appropriate.

Minor items to fix in the PR description

  1. PR body mentions maven-executor — Under "Modules with Automatic-Module-Name only", the description lists "Impl (4): maven-core, maven-cli, maven-executor, maven-testing". There are actually 3 impl modules in this category — maven-executor does not exist in the project's module structure.

  2. Unrelated fix not in diff — The PR body mentions "AbstractUpgradeGoal: Fixed domtrip API change children()childElements()" under "Unrelated fix", but this change does not appear in the diff. It may have been dropped during a rebase.

Neither of these affects the code — only the PR description text.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — well-structured JPMS migration adding module-info.java to 17 modules and Automatic-Module-Name to 16 remaining modules. All CI checks pass across all platforms (Linux, macOS, Windows) and Java versions (17, 21, 25).

Strengths:

  • Excellent consistency: all 33 javaModuleName POM properties exactly match their corresponding module-info.java module names. No split packages found.
  • The null-safety fixes in DefaultModelProcessor and DefaultModelBuilder are correct and necessary.
  • Qualified exports in maven-impl properly restrict all 14 org.apache.maven.impl.* packages to the 5 internal consumers.
  • Build infrastructure changes are all well-motivated (annotationProcessorPaths, useModulePath=false for surefire, sourceFileExcludes for javadoc, legacyMode=true for aggregate javadoc).

Minor observations (non-blocking):

  • The open module declaration for maven-impl grants deep reflective access; targeted opens directives might reduce the surface area but may not be practical given the number of DI consumers.
  • The woodstox-core and stax2-api dependencies in maven-support are transitively available but explicitly needed for JPMS requires — a comment noting this reasoning would help future maintainers.

No issues found that would block this PR.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-executed JPMS modularization with correct module descriptors, appropriate qualified exports for internal APIs, and sound design decisions for handling split-package constraints. All 23 CI checks pass green.

✅ Strengths

  • Module name consistency verified: all 17 modules with module-info.java have their javaModuleName POM property matching the module declaration exactly. All 16 Automatic-Module-Name entries are consistent.
  • Split-package analysis is well-founded: 13 compat modules and 3 impl modules cannot use module-info.java due to 20+ split packages between impl/compat boundaries. Using Automatic-Module-Name for these is correct.
  • Qualified exports in maven-impl are consistent — all 14 implementation packages restricted to the same 5 internal consumers (maven-core, maven-cling, maven-testing, maven-compat, maven-embedder).
  • The open module declaration for maven-impl is justified for DI framework reflective access.
  • API modules appropriately use requires transitive for inter-API dependencies where types leak across module boundaries.
  • Null-safety fixes in DefaultModelProcessor and DefaultModelBuilder are correct defensive changes.
  • Javadoc plugin configuration is well thought out (sourceFileExcludes for module-info.java, legacyMode=true for aggregate reports).

Minor observations (non-blocking)

  • org.apache.maven.api.di exports org.apache.maven.di.tool (non-API-namespaced package in an API module) — reasonable for co-located annotation processor, but a comment explaining why could help.
  • New explicit deps on woodstox-core/stax2-api in maven-support — previously transitive, now required for JPMS requires directives. Correct but widens declared surface.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI failure traced to maven-xml/module-info.java: the requires static plexus.xml directive fails compilation because plexus-xml is declared with scope=test in impl/maven-xml/pom.xml — it is therefore absent from the main compile module path.

The compiler error is:

module not found: plexus.xml

The fix is either to make plexus-xml a compile-scoped dependency, or to remove the requires static plexus.xml directive if it is not actually needed at compile time (the requires static org.eclipse.sisu.plexus on the adjacent line may already cover what you need, or neither directive may be necessary if these jars are only used by test code).

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Comment thread impl/maven-xml/src/main/java/module-info.java Outdated
plexus-xml 4.2.0 uses Automatic-Module-Name 'org.codehaus.plexus.util.xml',
not 'plexus.xml'. Moreover, plexus-xml is test-scoped in maven-xml and no
main sources use classes from it, so the requires directive is unnecessary.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review after new commits.

✅ Prior finding addressed

The CHANGES_REQUESTED finding from 2026-09-19 — requires static plexus.xml in impl/maven-xml/src/main/java/module-info.java was build-breaking because plexus-xml is test-scoped and therefore absent from the main compile module path — is fixed. The new module-info.java correctly uses requires static org.eclipse.sisu.plexus instead, and the Woodstox/StAX2 dependencies required for com.ctc.wstx and org.codehaus.stax2 are now declared as explicit compile-scope entries in the POM.

Review of new changes

The full JPMS modularization across 33 modules reviewed:

  • API modules (11) — correct requires transitive between inter-dependent API modules; java.compiler in maven-api-core justified by JavaPathType.java's use of javax.tools.*; maven-api-model correctly declares uses ModelObjectProcessor.
  • maven-implopen module for DI reflective access; 14 implementation packages qualified to 5 internal consumers; maven-api-spi and maven-api via requires transitive; annotationProcessorPaths for DiIndexProcessor; useModulePath=false for surefire — all correct.
  • maven-di — Qualified org.apache.maven.di.impl to the right 5 consumers; public org.apache.maven.di unqualified as the DI API; annotationProcessorPaths for self-annotation processing — correct.
  • maven-jlineopens org.apache.maven.jline to org.apache.maven.di for reflective instantiation of @Named/@Singleton beans; annotationProcessorPaths — correct. No requires static org.jline.terminal.ffm (correctly removed from prior version).
  • maven-logging — No DI annotations in the module source, no annotationProcessorPaths needed; provides org.slf4j.spi.SLF4JServiceProvider via ServiceLoader — correct.
  • maven-xmlrequires static org.eclipse.sisu.plexus is compile-optional for Plexus XML compatibility; explicit woodstox-core and stax2-api compile-scope deps in POM — correct.
  • maven-support — Promoted woodstox-core/stax2-api to compile scope (was transitive) — necessary and correct for JPMS requires.
  • Compat modules (13)Automatic-Module-Name only; correct decision given split packages preventing named module status.
  • Null-safety fixesDefaultModelProcessor and DefaultModelBuilder null-guard on @Nullable collections — correct.
  • Root POMmaven-jar-plugin Automatic-Module-Name via ${javaModuleName} property; sourceFileExcludes for module-info.java in javadoc; legacyMode=true in reporting profile — all appropriate.

No issues found.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewing after 897f0e0.

Previous finding addressed ✅ — requires static plexus.xml correctly removed. The commit message explains both the wrong module name and the test-scope-only usage; the fix is clean.

New issue found ❌ — same class of bug as before, different dependency: requires plexus.sec.dispatcher in maven-impl/module-info.java (line 39). The Automatic-Module-Name in plexus-sec-dispatcher-4.2.0.jar (the version declared in the root POM) is org.codehaus.plexus.components.secdispatcher, not plexus.sec.dispatcher. This will produce the same module not found compilation error on CI.

Fix: change requires plexus.sec.dispatcherrequires org.codehaus.plexus.components.secdispatcher.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Comment thread impl/maven-impl/src/main/java/module-info.java Outdated
The Automatic-Module-Name in plexus-sec-dispatcher-4.2.0.jar is
org.codehaus.plexus.components.secdispatcher, not plexus.sec.dispatcher.
Using the wrong name causes a 'module not found' compile error.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review after 3bff5fa.

✅ Prior finding addressed

The CHANGES_REQUESTED from 897f0e0requires plexus.sec.dispatcher in maven-impl/module-info.java — is fixed. The new commit corrects it to requires org.codehaus.plexus.components.secdispatcher, which matches the actual Automatic-Module-Name entry in plexus-sec-dispatcher-4.2.0.jar (verified from the JAR manifest).

✅ All prior findings resolved

  • 897f0e0: requires static plexus.xml removed (wrong module name + test-scope-only dep) ✅
  • 3bff5fa: requires plexus.sec.dispatcherrequires org.codehaus.plexus.components.secdispatcher

CI is pending on the new push — but this is a single-line mechanical fix with the correct module name verified against the JAR. No other issues found in the diff.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

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

Labels

enhancement New feature or request mvn4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assign automatic module names to core Maven components

3 participants