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

Problem with maven compilation. #370

Open
BorisYellnikov opened this issue Mar 31, 2023 · 13 comments
Open

Problem with maven compilation. #370

BorisYellnikov opened this issue Mar 31, 2023 · 13 comments

Comments

@BorisYellnikov
Copy link

When i compile my project from Eclipse everything is OK, but if i ran maven i obtain this error:

E:\programy\aikrepo\aik-cms>mvn compile
[INFO] Scanning for projects...
[INFO]
[INFO] ---------------------------< aikweb:aikweb >----------------------------
[INFO] Building aikweb 0.0.1-SNAPSHOT
[INFO] from pom.xml
[INFO] --------------------------------[ war ]---------------------------------
[INFO]
[INFO] --- resources:3.3.0:resources (default-resources) @ aikweb ---
[INFO] Copying 23 resources
[INFO]
[INFO] --- compiler:3.10.1:compile (default-compile) @ aikweb ---
[WARNING] Can't extract module name from rewrite-servlet-6.0.0-SNAPSHOT.jar: Provider class org.ocpsoft.common.logging.JDKLogAdapterFactory not in JAR file rewrite-servlet-6.0.0-SNAPSHOT.jar
[INFO] Required filename-based automodules detected: [weld-servlet-shaded-5.1.0.Final.jar, log4j-1.2.17.jar, commons-io-2.4.jar, xercesImpl-2.11.0.jar, qrgen-1.4.jar, commons-codec-1.9.jar, commons-validator-1.4.0.jar, json-lib-2.4-jdk15.jar, commons-lang-2.5.jar, javax.mail-1.5.0.jar, activation-1.1.jar, zt-zip-1.15.jar, jakarta.faces-4.1.0-SNAPSHOT.jar, rewrite-integration-faces-6.0.0-SNAPSHOT.jar, commons-dbcp2-2.9.0.jar, primefaces-12.0.0-jakarta.jar]. Please don't publish this project to a public artifact repository!
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 1438 source files to E:\programy\aikrepo\aik-cms\target\classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /E:/programy/aikrepo/aik-cms/src/main/java/module-info.java:[24,21] module not found: rewrite.servlet
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 7.988 s
[INFO] Finished at: 2023-03-31T18:18:38+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project aikweb: Compilation failure
[ERROR] /E:/programy/aikrepo/aik-cms/src/main/java/module-info.java:[24,21] module not found: rewrite.servlet

If i remove my module-info.java file then it is ok and maven compiles my project.

This is my module-info.java
`module aikweb
{

requires jakarta.faces;
requires log4j;
requires java.sql;
requires commons.io;
requires primefaces;
requires weld.servlet.shaded;
requires java.desktop;
requires commons.codec;
requires org.apache.groovy;
requires java.sql.rowset;
requires java.net.http;
requires org.json;
requires rewrite.integration.faces;
requires zt.zip;
requires rewrite.servlet;
requires xercesImpl;
requires json.lib;
requires commons.lang;
requires qrgen;
requires commons.validator;
requires activation;
requires javax.mail;
requires commons.dbcp2;
requires com.google.gson;

}
And POM
4.0.0
aikweb
aikweb
0.0.1-SNAPSHOT
war

<properties>
	 <!-- Jakarta EE API -->
    <jakartaee-api.version>10.0.0</jakartaee-api.version>
    <!-- Rewrite -->
	<rewrite.version>6.0.0-SNAPSHOT</rewrite.version>
	
	<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
	<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
	<maven.compiler.source>19</maven.compiler.source>
	<maven.compiler.target>19</maven.compiler.target>
	<failOnMissingWebXml>false</failOnMissingWebXml>
	
</properties>

<dependencyManagement>
	
	<dependencies>
		<dependency>
			<groupId>org.junit</groupId>
			<artifactId>junit-bom</artifactId>
			<version>5.9.2</version>
			<type>pom</type>
			<scope>import</scope>
		</dependency>
	</dependencies>
	
</dependencyManagement>

<dependencies>
	
	<dependency>
		<groupId>org.junit.jupiter</groupId>
		<artifactId>junit-jupiter</artifactId>
		<scope>test</scope>
	</dependency>
	
	<dependency>
		<groupId>jakarta.platform</groupId>
		<artifactId>jakarta.jakartaee-api</artifactId>
		<version>10.0.0</version>
		<scope>provided</scope>
	</dependency>
	
	<dependency>
		<groupId>org.jboss.weld.servlet</groupId>
		<artifactId>weld-servlet-shaded</artifactId>
		<version>5.1.0.Final</version>
	</dependency>
	
	<dependency>
		<groupId>log4j</groupId>
		<artifactId>log4j</artifactId>
		<version>1.2.17</version>
	</dependency>
	
	<dependency>
		<groupId>org.json</groupId>
		<artifactId>json</artifactId>
		<version>20190722</version>
	</dependency>
	
	<dependency>
		<groupId>commons-io</groupId>
		<artifactId>commons-io</artifactId>
		<version>2.4</version>
	</dependency>
	
	<dependency>
		<groupId>org.apache.commons</groupId>
		<artifactId>commons-lang3</artifactId>
		<version>3.1</version>
	</dependency>

	<dependency>
		<groupId>xerces</groupId>
		<artifactId>xercesImpl</artifactId>
		<version>2.11.0</version>
	</dependency>
	
	<dependency>
		<groupId>net.glxn</groupId>
		<artifactId>qrgen</artifactId>
		<version>1.4</version>
	</dependency>
	
	<dependency>
		<groupId>org.owasp.encoder</groupId>
		<artifactId>encoder</artifactId>
		<version>1.2.3</version>
	</dependency>
	
	<dependency>
		<groupId>commons-codec</groupId>
		<artifactId>commons-codec</artifactId>
		<version>1.9</version>
	</dependency>
	
	<dependency>
		<groupId>commons-validator</groupId>
		<artifactId>commons-validator</artifactId>
		<version>1.4.0</version>
	</dependency>
	
	<dependency>
		<groupId>net.sf.json-lib</groupId>
		<artifactId>json-lib</artifactId>
		<version>2.4</version>
		<classifier>jdk15</classifier>
	</dependency>
	
	<dependency>
		<groupId>jakarta.servlet.jsp.jstl</groupId>
		<artifactId>jakarta.servlet.jsp.jstl-api</artifactId>
		<version>2.0.0</version>
	</dependency>

	<dependency>
		<groupId>com.sun.mail</groupId>
		<artifactId>javax.mail</artifactId>
		<version>1.5.0</version>
	</dependency>
	
	<dependency>
		<groupId>org.zeroturnaround</groupId>
		<artifactId>zt-zip</artifactId>
		<version>1.15</version>
	</dependency>
	
	<dependency>
		<groupId>org.glassfish</groupId>
		<artifactId>jakarta.faces</artifactId>
		<version>4.1.0-SNAPSHOT</version>
	</dependency>
	
	<dependency>
		<groupId>org.apache.groovy</groupId>
		<artifactId>groovy</artifactId>
		<version>4.0.7</version>
	</dependency>
	
	<dependency>
		<groupId>org.ocpsoft.rewrite</groupId>
		<artifactId>rewrite-servlet</artifactId>
		<version>${rewrite.version}</version>
	</dependency>
	
	<dependency>
		<groupId>org.ocpsoft.rewrite</groupId>
		<artifactId>rewrite-integration-faces</artifactId>
		<version>${rewrite.version}</version>
	</dependency>

	<!--	
	<dependency>
		<groupId>org.ocpsoft.rewrite</groupId>
		<artifactId>rewrite-integration-cdi</artifactId>
		<version>${rewrite.version}}</version>
	</dependency>
	-->
	<dependency>
		<groupId>mysql</groupId>
		<artifactId>mysql-connector-java</artifactId>
		<version>8.0.30</version>
	</dependency>
	
	<dependency>
		<groupId>net.sf.ezmorph</groupId>
		<artifactId>ezmorph</artifactId>
		<version>1.0.6</version>
	</dependency>
	
	<!-- https://mvnrepository.com/artifact/org.apache.commons/commons-dbcp2 -->
	<dependency>
		<groupId>org.apache.commons</groupId>
		<artifactId>commons-dbcp2</artifactId>
		<version>2.9.0</version>
	</dependency>
	<!-- https://mvnrepository.com/artifact/commons-pool/commons-pool -->
	<dependency>
		<groupId>commons-pool</groupId>
		<artifactId>commons-pool</artifactId>
		<version>1.6</version>
	</dependency>

	<dependency>
		<groupId>jakarta.el</groupId>
		<artifactId>jakarta.el-api</artifactId>
		<version>4.0.0</version>
	</dependency>
	
	<dependency>
		<groupId>jakarta.enterprise</groupId>
		<artifactId>jakarta.enterprise.cdi-api</artifactId>
		<version>3.0.0</version>
	</dependency>
	
	<dependency>
		<groupId>org.primefaces</groupId>
		<artifactId>primefaces</artifactId>
		<version>12.0.0</version>
		<classifier>jakarta</classifier>
	</dependency>
	
	<!-- https://mvnrepository.com/artifact/com.google.zxing/core -->
	<dependency>
		<groupId>com.google.zxing</groupId>
		<artifactId>core</artifactId>
		<version>3.5.1</version>
	</dependency>
	<!-- https://mvnrepository.com/artifact/com.google.zxing/javase -->
	<dependency>
		<groupId>com.google.zxing</groupId>
		<artifactId>javase</artifactId>
		<version>3.5.1</version>
	</dependency>
	
	<dependency>
		<groupId>com.google.code.gson</groupId>
		<artifactId>gson</artifactId>
		<version>2.10.1</version>
	</dependency>
		
</dependencies>

<build>
	<resources>
		<resource>
			<directory>src/main/java</directory>
			<excludes>
				<exclude>**/*.java</exclude>
			</excludes>
		</resource>
	</resources>
	
	<plugins>
		
		<plugin>
			<artifactId>maven-compiler-plugin</artifactId>
			<version>3.10.1</version>
			<configuration>
				<release>19</release>
			</configuration>
		</plugin>
		
		<plugin>
			<artifactId>maven-war-plugin</artifactId>
			<version>3.3.2</version>
		</plugin>


	</plugins>
</build>

`

What can I do?

@blutorange
Copy link

We are getting the same error when we try to run the Javadoc tool on our module project. The error message is a little bit cryptic, but the cause of the error is that the current rewrite-servlet-3.5.1.Final.jar pretends that is has the org.ocpsoft.common.logging.JDKLogAdapterFactory class while the JAR does not actually contain that class.

I don't think you can really do much (other than modifying the JAR itself or perhaps manually putting rewrite-servlet on the --classpath). Due to this issue, rewrite-servlet is currently not compatible with the module system. The fix would be straightforward though, I think> That class needs to be removed from the WEB-INF/services/org.ocpsoft.rewrite.spi.LogAdapterFactory file and if necessary, added to the module that actually contains the class.

image

@lincolnthree
Copy link
Member

lincolnthree commented Aug 4, 2023

Hey, sorry, I was away on Vacation.

Out of curiosity, do you have ocpsoft-logging in your module
https://mvnrepository.com/artifact/org.ocpsoft.logging/logging-api/1.0.5.Final

This is where the class is located:
https://github.com/ocpsoft/logging/blob/master/api/src/main/java/org/ocpsoft/logging/JDKLogAdapterFactory.java

The logging-api JAR needs to be on the same module/classpath as Rewrite Servlet. (I would also guess any other ocpsoft/rewrite JAR would also need to be in the same module/classpath.

Let me know if that helps? I don't think moving the JDKLogAdapterFactory into rewrite-servlet.jar is the right solution.

@blutorange
Copy link

Unfortunately, I don't think that's enough. If one uses the module system, when a module (rewrite-servlet) declares a service provider implementation (org.ocpsoft.common.logging.JDKLogAdapterFactory), that implementation class must exist in the current module. Even if it exists in some other package in the module path, that still results in an error

For example, https://docs.oracle.com/javase/specs/jls/se11/html/jls-7.html#jls-7.7.4

Every service provider must be declared in the current module, or a compile-time error occurs.

@blutorange
Copy link

Let me know if that helps? I don't think moving the JDKLogAdapterFactory into rewrite-servlet.jar is the right solution.

What about the opposite? It should be enough to move the META-INF/services declaration into the ocpsoft-logging module.

@lincolnthree
Copy link
Member

lincolnthree commented Aug 6, 2023

What about the opposite? It should be enough to move the META-INF/services declaration into the ocpsoft-logging module.

That would essentially force all dependents who use ocpsoft-logging to be defaulted to the JDKLoggingAdapterFactory, which is not necessarily correct.

The solution (as I understand it) is to make sure that rewrite-servlet.jar and ocpsoft-logging.jar are both in the same module. Is this not possible for some reason?

Per the docs you linked:
The service may be declared in the current module or in another module. If the service is not declared in the current module, then the service must be accessible to code in the current module ([§6.6](https://docs.oracle.com/javase/specs/jls/se11/html/jls-6.html#jls-6.6)), or a compile-time error occurs.

@blutorange
Copy link

blutorange commented Aug 6, 2023

The service may be declared in the current module or in another module. If the service is not declared in the current module, then the service must be accessible to code in the current module

Note the difference between service and service provider. The service is org.ocpsoft.rewrite.spi.LogAdapterFactory, the service provider is JDKLoggingAdapterFactory. The service can be declared in another module, but the implementation must be in the same module as the META-INF/services (or the module-info.java) file that declares the service provider.

The solution (as I understand it) is to make sure that rewrite-servlet.jar and ocpsoft-logging.jar are both in the same module. Is this not possible for some reason?

Unless I'm completely mistaken: A "module" here refers to a JPMS (Java Platform Module System) module, not a Maven project or runtime environment. It's not enough to load all modules to the module path, the service provider must be declared within the same JPMS module. JPMS preserves the boundaries between modules even at runtime. E.g. ocpsoft-logging is a separate module, as is rewrite-servlet. In other words, making sure that rewrite-servlet.jar and ocpsoft-logging.jar are both in the same module would mean creating a new custom JAR with the contents of both modules.

That would essentially force all dependents who use ocpsoft-logging to be defaulted to the JDKLoggingAdapterFactory, which is not necessarily correct.

So if I understand the intention correctly, it should default to JDKLoggingAdapterFactory when using rewrite-servlet, but not when using only ocpsoft-logging?

It would require some refactoring, but another solution I can think of is to extract JDKLoggingAdapterFactory into a separate package (e.g. ocpsoft-logging-jdk) that contains the META-INF/services entry, then rewrite-servlet could include that new package as a dependency.

Or perhaps, as a workaround, it's possible to add a dummy implementation of org.ocpsoft.rewrite.spi.LogAdapterFactory to the rewrite-servlet module that just delegates to JDKLoggingAdapterFactory?

@lincolnthree
Copy link
Member

lincolnthree commented Aug 7, 2023

Unless I'm completely mistaken: A "module" here refers to a JPMS (Java Platform Module System) module, not a Maven project or runtime environment.

Yeah, that's right. In most module systems I've used in the past, developers have control over which artifacts (JARs and Classes) are actually loaded into a specific module. This can generally be done via the Module System configuration files.

It sounds like your JPMS is setting each ocpsoft Maven artifact JAR to be in its own module. Which, while possibly reasonable by default, is probably not right in this case.

The JPMS system should be configured such that all ocpsoft JARs are in the same Classpath, e.g. using the Unnamed module. Other module systems I've used have allowed this type of configuration as well:
https://www.baeldung.com/java-9-modularity#adding-modules-to-the-unnamed-module (all ocpsoft JARs should be added to the unnamed module).

Ideally rewrite would have module-info.java files to make it modular, but unless someone is volunteering for that, the unnamed module is my recommendation. I haven't found any great tutorials on how to set that up, but I believe that's the answer we're looking for in the short term.

PS. It also sounds like it's fine for service implementations to be in separate jars from their loader configuration, but in the JDK9 module system, those implementations need to be exported via module-info.java and marked as explicitly "provided" (e.g. ocpsoft-logging). Then "used" in the dependent module (in this case rewrite-servlet.jar). (See https://www.baeldung.com/java-9-modularity). So this is part of the work that would need to be done to add module descriptors to Rewrite.

@lincolnthree
Copy link
Member

@blutorange
Copy link

blutorange commented Aug 7, 2023

PS. It also sounds like it's fine for service implementations to be in separate jars from their loader configuration, but in the JDK9 module system, those implementations need to be exported via module-info.java and marked as explicitly "provided" (e.g. ocpsoft-logging). Then "used" in the dependent module (in this case rewrite-servlet.jar). (See https://www.baeldung.com/java-9-modularity). So this is part of the work that would need to be done to add module descriptors to Rewrite.

I've been working with JPMS a bit, so let me try to explain. First of all, provides and uses are not optional, but are always required in a proper module.

provides service with implementation (which is the replacement for WEB-INF/services in the modular world) in a module says that the module is able to offer an implementation for the service. This implementation class must be in the same module that has the provides . In other words, the module that has the module-info.java with the following line must contain the implementing class

provides org.ocpsoft.rewrite.spi.LogAdapterFactory with JDKLoggingAdapterFactory

The uses service says that a module (which is usually in a different module or that's rather pointless) says that the module wishes to make use all available implementation of service, without having to care where these implementations come from.

The module must use ServiceLoader to find the implementations of a service it wants to use. The ServiceLoader returns an iterable over all implementations that were found. I'd assume that LogAdapterFactory is not loaded by the user, but by some OCPSoft code, though?

It sounds like your JPMS is setting each ocpsoft Maven artifact JAR to be in its own module. Which, while possibly reasonable by default, is probably not right in this case.

That's how JPMS is designed (to ensure encapsulation), there's no other (proper) way. The unnamed module only exists as a workaround for backwards compatibility, but should not be used when solving the problem "properly".

Ideally rewrite would have module-info.java files to make it modular, but unless someone is volunteering for that, the unnamed module is my recommendation.

To clarify: the unnamed module doesn't exist, there are multiple unnamed modules for different JARs. Essentially that just means placing rewrite-servlet on the class path, not the module path. When running a modular application, everything on the class path is put in an unnamed module. Essentially this means not using the modular system.

But don't get me wrong: That's completely reasonable for now, it takes time to make changes in a large project.

My only worry with this workaround, though, is that it's a bit cumbersome, as it means having to tinker with Java options, which can't be set by library authors, but need to be set by end users who, in one way or another, happen to include rewrite-servlet as a dependency.

PS: This is not (yet) a problem for us, as (a) we can't switch to modules for some time to come due to many dependencies that are not yet compatible with the module system, (b) we only noticed this when we attempted to create JavaDocs in a modular environment, an attempt which we had to abort for various other reasons.

@lincolnthree
Copy link
Member

lincolnthree commented Aug 7, 2023

I've been working with JPMS a bit, so let me try to explain. First of all, provides and uses are not optional, but are always required in a proper module.

Yeah, my experience is with OSGI and JBoss Modules. I have not used JPMS so I'm just reading the docs here right now. Thanks. Our understanding of provided & uses are the same.

provides service with implementation (which is the replacement for WEB-INF/services in the modular world) in a module says that the module is able to offer an implementation for the service. This implementation class must be in the same module that has the provides . In other words, the module that has the module-info.java with the following line must contain the implementing class

provides org.ocpsoft.rewrite.spi.LogAdapterFactory with JDKLoggingAdapterFactory

Right. So in JPMS does that mean the service-loader's META-INF/services/XYZ file ALSO needs to be in the module that provides the service? I think that's what you were getting at earlier, but... that seems limiting in a problematic way. (Or at least one that requires significant code-refactoring.)

The uses service says that a module (which is usually in a different module or that's rather pointless) says that the module wishes to make use all available implementation of service, without having to care where these implementations come from.

Right.

The module must use ServiceLoader to find the implementations of a service it wants to use. The ServiceLoader returns an iterable over all implementations that were found. I'd assume that LogAdapterFactory is not loaded by the user, but by some OCPSoft code, though?

Yes: https://github.com/ocpsoft/logging/blob/master/api/src/main/java/org/ocpsoft/logging/Logger.java#L326 (And the service being loaded is both implemented and declared (in META-INF/services) in the same JAR, here.

That's how JPMS is designed (to ensure encapsulation), there's no other (proper) way. The unnamed module only exists as a workaround for backwards compatibility, but should not be used when solving the problem "properly".

Yes. I agree. This is the same in other systems. And we are definitely talking about backwards compatibility here :)

To clarify: the unnamed module doesn't exist, there are multiple unnamed modules for different JARs. Essentially that just means placing rewrite-servlet on the class path, not the module path. When running a modular application, everything on the class path is put in an unnamed module. Essentially this means not using the modular system.

Right. It's basically "default" for any artifact without a module-info.java file, and is available to all other modules by default, but my understanding is they felt that naming would be confusing. Is that right?

But don't get me wrong: That's completely reasonable for now, it takes time to make changes in a large project.

Cool. I would like to make sure this gets fixed though, so definitely welcome the fix once we land on it. That said, I have a new idea about what is wrong. See below.

My only worry with this workaround, though, is that it's a bit cumbersome, as it means having to tinker with Java options, which can't be set by library authors, but need to be set by end users who, in one way or another, happen to include rewrite-servlet as a dependency.

Agreed, it is not ideal.

OK. Now on to my new realization about what's wrong here. The org.ocpsoft.logging package SHOULD be bundled in rewrite-servlet.jar. That's an UBER Jar made with Maven assembly that is supposed to be an all-inclusive (except for extensions like *-integration-cdi, and etc. (https://github.com/ocpsoft/rewrite/blob/main/rewrite-servlet/pom.xml#L164)

I am concerned that if what you're saying is true, that Rewrite simply isn't compatible with the JDK module system, since if I'm understanding you correctly, services must be both 'provided' and explicitly 'used' by other modules in order to show up to the ServiceLoader.

If that's the case, then none of the Rewrite integration/extension modules will ever work in the JDK module system, since they are supposed to be "add-ons" that can be simply added to the classpath and they self-register their extensions. (via service providers.) The JDK module paradigm would completely break the rewrite extension model. I feel like there's something we're missing here.

@blutorange
Copy link

blutorange commented Aug 7, 2023

Great, I was almost done with my answer the my notebook decided to crash...

I would like to make sure this gets fixed though, so definitely welcome the fix once we land on it.

Glad we're on the same page, I very much support it if you want to fix it : )

Right. So in JPMS does that mean the service-loader's META-INF/services/XYZ file ALSO needs to be in the module that provides the service? I think that's what you were getting at earlier, but... that seems limiting in a problematic way. (Or at least one that requires significant code-refactoring.)

Yes, you only need the provides ... in the module-info.java, the META-INF/services file is neither required nor useful for a module. See also the sections Deploying service providers as modules and Deploying service providers on the class path in the ServiceLoader JavaDocs.

The only restriction here is that you can't have a service provider in module X (x.jar), but the provides ... or META-INF/services in module Y (y.jar). Which is what the error message from the beginning of this issue was trying to say

On a side note, I can't count the times I had to debug into ServiceLoader because it just wouldn't load my providers. The implementation that looks in the class path for META-INF/services checks if the provider is in a named module, and if so, skips it.

Provider class org.ocpsoft.common.logging.JDKLogAdapterFactory not in JAR file rewrite-servlet-6.0.0-SNAPSHOT.jar

OK. Now on to my new realization about what's wrong here. The org.ocpsoft.logging package SHOULD be bundled in rewrite-servlet.jar.

Yes that should work. If that's how it was supposed to be anyways, that's probably the best (and proper) solution for now.

In the long term run, though, I'm not sure if uber JARs are a good idea for the module system. I don't have much experience with modular uber JARs. It should be possible in principle, but see also this Jigsaw spec issue regarding MultiModuleExecutableJAR.
I tried once and decided it wasn't a good idea: (a) Things will probably break when anybody tries to use both the uber JAR and one of the individual JARs, (b) you need to merge the individual module-info.java descriptors, (c) it's now one JAR with one module name, so all references, if any, to the individual modules need to be replaced.

Either way, that only applies once you add proper module-info.java descriptors. A uber JAR with all classes and all merged META-INF/services files should work.

I am concerned that if what you're saying is true, that Rewrite simply isn't compatible with the JDK module system, since if I'm understanding you correctly, services must be both 'provided' and explicitly 'used' by other modules in order to show up to the ServiceLoader.

I think it should work, if I understand the use case correctly. Perhaps I didn't explain it well enough. It might be easier to illustrate with an example:

  • Module api contains the interface LoggingProvider { ... }
    • Its module-info.java doesn't need anything special other than the general exports ocpsoft.api
  • Module core looks for all available implementation of the logging provider, using ServiceLoader.load(LoggingProvider.class)
    • For that to work, it has a uses LoggingProvider in its module-info.java
  • Module logging-jdk includes an implementation class JdkLogger implements LoggingProvider { ... }
    • Its module-info.java has a provides LoggingProvider with JdkLogger
  • Module logging-log4j includes an implementation class Log4JLogger implements LoggingProvider { ... }
    • Its module-info.java has a provides LoggingProvider with Log4JLogger

Now with that setup, a consumer can include core and logging-jdk as a dependency, which gets picked up automatically. If they replace logging-jdk with logging-log4j, that gets picked up instead.

simply isn't compatible with the JDK module system

Not sure if it's relevant for this project, but another important unnegotiable requirement to keep in mind is: There must be no overlapping packages, i.e. there can't be any two modules that have a class in the same package ("sub" packages count as different packages). Otherwise it's just not compatible with the module system.

@lincolnthree
Copy link
Member

lincolnthree commented Aug 9, 2023

Hey @blutorange, thanks for this update/in-depth reply. It totally sucks when that happens and you lose everything you typed :(

Glad we're on the same page, I very much support it if you want to fix it : )

Yes, I do!

A Uber JAR with all classes and all merged META-INF/services files should work.

Hmmm.... does it? Assuming the current packaging structure, and the following scenario:

  • The rewrite-servlet Uber-JAR contains and exports the org.ocpsoft.rewrite.spi package.
  • The rewrite-integration-faces module depends on org.ocpsoft.rewrite.spi and provides several ServiceLoader configs such as org.ocpsoft.rewrite.spi.ConfigurationProvider
  • The rewrite-servlet Uber-JAR requests these implementations of org.ocpsoft.rewrite.spi.ConfigurationProvider via the ServiceLoader, and should be provided with those from the rewrite-integration-faces module (and anything else providing this service).

If this works (and once the logging JAR situation is resolved), then I think we can add a module-info.java file to the uber-jar. And also to each config/integration module. Is that a reasonable understanding/assumption?

========================================

Alternately... Let's say we add module-info files to everything. Then you wouldn't need to use the Uber-JAR in modular projects, just in projects with a flat classpath -- but you'd still need to make sure you include the right chain of dependencies --- we could figure out what's needed there. I believe it's just rewrite-servlet-api and rewrite-servlet-impl at a minimum + optional jars.)

What's the real work in doing that? Just crafting these files? What else would be required?

========================================

Related, a couple folks have started a similar update for the OCPsoft PrettyTime library, here:
ocpsoft/prettytime#262

They are recommending the Moditect Maven Plugin, what are your thoughts about using this plugin for the Rewrite side of things? Could we possibly auto-generate the module-info.java files for all rewrite modules by adding this config to the Rewrite parent POM? I'm guessing it would work for the providers, but not for the consumers, right?

@blutorange
Copy link

blutorange commented Aug 9, 2023

Hmmm.... does it? Assuming the current packaging structure, and the following scenario:

Yeah, that sounds reasonable to me. Basically rewrite-servlet both exports the service and simultaneously also uses the service, with provider implementations provided by a different module. I have a modular project with the same setup, that works perfectly fine.

I think we can add a module-info.java file to the uber-jar. And also to each config/integration module. Is that a reasonable understanding/assumption?

Yes again, that sounds reasonable, but two points to keep in mind:

  • No two modules must share the same package. From a cursory glance at the various packages that seems to be the case though.
  • All your dependencies must at least be compatible with the module system. Ideally they should also have a module-info.java, but at least they must have MANIFEST.MF with an Automatic-Module-Name (or the maven-compiler-plugin will try to persuade you not to publish the module) Also, you must not have any two dependencies that contain the same package.

Regarding the possibility of the uber JAR, there's one question I think that needs to be answered: What about the the case where both rewrite-servlet and one of its constituents such as org.ocpsoft.common:common-api end up being included as a dependency?

I could imagine some project that includes rewrite-servlet itself that also includes some other library that only depends on common-api. When everything is on the class path, it doesn't matter, as long as the JVM finds the class files in some JAR it's happy. But in a modular environment, this will result in an error (as different modules contain the same package).

So either we can say that such a scenario is unlikely to occur and/or is unsupported; or the uber JAR approach won't work. Packages such as org.ocpsoft.common:common-api are currently published as Maven artifacts. Is that only for internal use? If you take the uber JAR approach, then you might want to consider not publishing these individual packages anymore.


What's the real work in doing that? Just crafting these files? What else would be required?

Writing the module-info.java shouldn't take long. The real work, I'd say, is testing all of this and making sure it works as expected (again: no duplicate packages, all dependencies must be compatible). Probably with some test Maven project where you include the various dependencies. And I'm almost certain, given the number of individual project, that you will find something that won't quite work and needs some refactoring or adjustment.

And of course, as mentioned in that prettytime thread, if you want to support Java 8, you need create a multi release JAR (which too shouldn't be too hard to setup, but needs to be tested as well).


They are recommending the Moditect Maven Plugin, what are your thoughts about using this plugin for the Rewrite side of things? Could we possibly auto-generate the module-info.java files for all rewrite modules by adding this config to the Rewrite parent POM? I'm guessing it would work for the providers, but not for the consumers, right?

I know about that plugin, but I haven't used it much in my projects personally. Each tool adds additional complexity (and bugs). And the projects that I set up as modules weren't large enough to be worth the effort.

One thing you definitely can't do is completely auto-generate the module-info.,java just from the information in a normal Maven pom.xml. It will require at least some amount of hand-crafting, at least for more complex projects (e.g. it couldn't possibly know what you want to export and open).

If you take a look at the usage section of the moditect plugin, you'll see it has a long <moduleInfo> section in its configuration dedicated to "fine-tuning" the generated module-info.java, with exports, requires, opens and uses directives as plain text inside the pom. Imho, that can make it harder to understand and maintain, when instead of having a single module-info.java file, that file is partly auto-generated from various sources and partly augmented by handwritten declarations.

In the end, that plugin is just a different way of creating a module-info.java. If it makes it easier, then by all means go for it. But I'd probably start with writing the module-info.java "by hand", if not only to get better feeling for the module system.

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

No branches or pull requests

3 participants