Skip to content

Conversation

desruisseaux
Copy link
Contributor

This is a variation of the practice consisting in overwriting the module-info.java of the main code with another module-info.java defined in the test directory. Instead, the module-info.java file formerly defined in the test would be replaced by a module-info-patch.maven file in the same directory. The latter is Maven-specific: the content of module-info-patch.maven looks like module-info.java, but is not standard Java, hence the .maven file suffix. The principles are:

  • Everything that a developer would like to change in a module-info.java file for testing purposes is declared in module-info-patch.maven.
  • Everything that is not in module-info.java is not in module-info-patch.maven neither. In particular, everything that specify paths to JAR files stay in the pom.xml file.
  • All keywords inside the patch-module block of that file map directly to Java compiler or Java launcher options.

The keywords are add-modules, limit-modules, add-reads, add-exports and add-opens. All these keywords map to javac options of the same names. Note that they are options where the values are package or module names, not paths to source or binary files. Options with path values should be handled in the <sources> and <dependencies> elements of the POM instead. Below is an example of a module-info-patch.maven file content for modifying the module-info of a module named my.product.foo:

/*
 * The same comments as in Java are allowed.
 */
patch-module my.product.foo {             // Put here the name of the module to patch.
    add-modules TEST-MODULE-PATH;         // Recommended value in the majority of cases.

    add-reads org.junit.jupiter.api,      // Frequently used dependency for tests.
              my.product.test.fixture;    // Put here any other dependency needed for tests.

    add-exports my.product.foo.internal   // Name of a package which is normally not exported.
             to org.junit.jupiter.api,    // Any module that need access to above package for testing.
                my.product.test.fixture;  // Can export to many modules, as a coma-separated list.

    add-exports my.product.foo.mock       // Another package to export. It may be a package defined in the tests.
             to my.product.biz;           // Another module of this project which may want to reuse test classes.
}

Why this approach

It is not convenient to put these options in the plugin configuration in the POM because these options need to be specified on a module-per-module (in JPMS sense) basis, while the plugin configuration in the POM applies to the whole module source hierarchy. Actually, the options of all modules end up merged together in a single set of options for javac, but in a more verbose form for differentiating each module. For example, if the option add-reads org.junit.jupiter.api is declared inside the my.product.foo module, it becomes --add-reads my.product.foo=org.junit.jupiter.api in the options given to javac. It is easer for developers to read and maintain their code if this verbosity is managed by the compiler plugin, in addition of avoiding the XML verbosity of the POM.

Another demonstration that options declared in plugin configuration are impracticable is that many developers prefer to provide a module-info.java in their test which override the module-info.java of their main code. But this approach has problems: the test module-info.java must duplicate everything that is in the main module-info.java. If they diverge, the tests may not behave as the main code would do. Furthermore, this approach sometime requires hacks in the compiler plugin such as pretending that the tests are the main code and the main code is a patch applied over the tests. For avoiding those issues, javac options such as --add-reads should be used instead, but they are tedious to write and maintain as plugin configuration in the POM.

Impact on other plugins

Other plugins do not need to parse this module-info-patch.maven file. Only the Maven Compiler Plugin needs to do so. The compiler plugin generates a META-INF/maven/module-info-patch.args file which would be consumed by the Surefire plugin for executing the tests. Other plugins would need to care about this file only if they have something to do with tests. Note that Maven 3 already generates a file for JPMS arguments, but with a different content. So this approach is not really new.

Gradual approach proposal

We could take a gradual approach by accepting this pull request with an amendment: if a module-info-patch.maven file is found, print a warning saying that this is an experimental feature. I think that the risk is low because most users will probably not experiment this feature until the Surefire is upgraded for consuming the above-cited META-INF/maven/module-info-patch.args file. But having feedbacks from peoples willing to try on the command-line would be very valuable.

@desruisseaux
Copy link
Contributor Author

Maybe @slawekjaranowski as a reviewer here too? Like #959 the decision about accepting a module-info-patch.maven file is probably of limited impact for now, as developers will probably not use it before other plugins are updated. But the intend is to get feedbacks if some developers are willing to try.

Note: an email has been sent to the developer mailing list about those issues, but got no reaction yet.

@gnodet
Copy link
Contributor

gnodet commented Sep 22, 2025

I'd like to experiment switching JLine's master branch to the new plugins, since i'm migrating it to Maven 4 with JPMS support.

@desruisseaux
Copy link
Contributor Author

Documentation about this feature has been added in #976, in particular in the module-info-patch.md file. Would it help acceptance if I add a commit for printing a message saying that this feature is experimental when module-info-patch.maven is used?

@slachiewicz slachiewicz requested a review from Copilot October 5, 2025 20:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for parsing module-info-patch.maven files that provide a more convenient way to specify module system options like --add-exports and --add-reads for testing purposes. Instead of overwriting module-info.java files in test directories or configuring verbose XML in the POM, developers can use a Maven-specific syntax that maps directly to Java compiler options.

Key changes:

  • Adds a new parser for module-info-patch.maven files with Java-like syntax for module system options
  • Integrates the parser into the test compilation workflow to generate appropriate compiler flags
  • Updates existing code to handle the new patch-based approach while maintaining backward compatibility

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/test/resources/org/apache/maven/plugin/compiler/module-info-patch.maven Example patch file showing the supported syntax and options
src/test/java/org/apache/maven/plugin/compiler/ModuleInfoPatchTest.java Test class validating the parser functionality and output generation
src/main/java/org/apache/maven/plugin/compiler/ToolExecutorForTest.java Integration of patch file parsing into the test compilation process
src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java Updated comment clarifying main code handling
src/main/java/org/apache/maven/plugin/compiler/TestCompilerMojo.java Updated deprecation warnings to reference the new patch file approach
src/main/java/org/apache/maven/plugin/compiler/ModuleInfoPatchException.java Exception class for patch file parsing errors
src/main/java/org/apache/maven/plugin/compiler/ModuleInfoPatch.java Main parser implementation for module-info-patch.maven files

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

desruisseaux and others added 2 commits October 6, 2025 14:11
…` and similar options can be specified more easily.
Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants