Open BorisYellnikov opened 1 year ago
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.
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.
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.
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.
What about the opposite? It should be enough to move the
META-INF/services
declaration into theocpsoft-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.
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
?
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.
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.
I've been working with JPMS a bit, so let me try to explain. First of all,
provides
anduses
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 forWEB-INF/services
in the modular world) in a module says that the module is able to offer animplementation
for theservice
. This implementation class must be in the same module that has the provides . In other words, the module that has themodule-info.java
with the following line must contain the implementing classprovides 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 ofservice
, 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 thatLogAdapterFactory
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.
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:
api
contains the interface LoggingProvider { ... }
module-info.java
doesn't need anything special other than the general exports ocpsoft.api
core
looks for all available implementation of the logging provider, using ServiceLoader.load(LoggingProvider.class)
uses LoggingProvider
in its module-info.java
logging-jdk
includes an implementation class JdkLogger implements LoggingProvider { ... }
module-info.java
has a provides LoggingProvider with JdkLogger
logging-log4j
includes an implementation class Log4JLogger implements LoggingProvider { ... }
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.
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:
rewrite-servlet
Uber-JAR contains and exports
the org.ocpsoft.rewrite.spi
package.rewrite-integration-faces
module depends on org.ocpsoft.rewrite.spi
and provides
several ServiceLoader
configs such as org.ocpsoft.rewrite.spi.ConfigurationProvider
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: https://github.com/ocpsoft/prettytime/issues/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?
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:
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.
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 {
}
And POM
`
What can I do?