hgschmie commented on PR #227:
URL:
https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-1709311461
This is the slightly longer (and a bit more winded) explanation of what that
patch does:
consider a project that spits out a module. This can be a defined module or
an automatic module; it does not actually matter. What is important is that the
maven tool chain considers it something where it will use the module path.
let's consider this dependency graph: `project --> automatic module with
manifest entry --> automatic module with file name based module id` (If that is
too abstract for you, you can call those `jdbi3-guice -> guice-5.1.0 ->
javax.inject-1`).
- jdbi3-guice is a defined module (it contains a module-info.java file)
- guice-5.1.0 is an automatic module that contains an entry in
`META-INF/MANIFEST.MF`
- javax.inject-1 is an automatic module that has no JPMS information, so it
is treated as a filename based module.
To run code that uses these three modules, it needs to do `--module-path
jdbi3-guice.jar:guice-5.1.0.jar:javax.inject-1.jar`. This will work fine. The
automatic modules open all their packages to the module path; there are no
overlaps and the jdbi3-guice module can consume all the pieces that it needs.
When the maven toolchain is trying to set up javadoc generation, it will
inspect all three modules and come to the following conclusion:
- jdbi3-guice --> `ModuleNameSource.MODULEDESCRIPTOR`
- guice-5.1.0 --> `ModuleNameSource.MANIFEST`
- javax.inject-1 --> `ModuleNameSource.FILENAME`
(this is buried deep in the plexus-java component)
Now, the javadoc plugin sees this and creates module and classpath:
```
--add-modules ALL-MODULE-PATH
--module-path
'jdbi/guice/target/jdbi3-guice-3.41.0.jar:m2repo/guice-5.1.0.jar'
--patch-module org.jdbi.v3.guice='m2repo/javax.inject-1.jar'
```
because it treats the `ModuleNameSource.MANIFEST` *different* from
`ModuleNameSource.FILENAME`
So, everything cool? No. What happens now is that a class in the main module
(`InternalBindingProvider`) implements a class in the guice module
(`com.google.inject.Provider`) which in turn *extends* a class from
javax.inject-1 (`javax.inject.Provider`). But the javax.inject-1 jar has *only
been patched into the jdbi3-guice component*. So javadoc can not find it when
the guice class says "I extend that" and that leads to:
```
InternalImportBindingBuilder.java:88: error: cannot access Provider
static final class InternalBindingProvider<T> implements Provider<T> {
^
class file for javax.inject.Provider not found
1 error
```
so the right fix is one of two things:
- move all dependencies of the main artifact onto the module path. This
sounds nice in theory but reality is that this only works for defined modules
(those that have a module-info.java). Many of the other ones (especially the
ones that simply tack an automatic module name into the jar after compile time)
have dependencies with overlapping packages (e.g. jsr305 as the biggest
offender) and short of inspecting every jar and keeping a list of exports, this
option leads to more problems than actual solutions. It is, however, *the right
solution* but, unless the rest of the tooling starts to differentiate between
defined modules and automatic modules, not a viable solution.
- patch *all* the automatic modules that are dependencies for the main
artifact into the main artifact. IAW, don't differentiate between the two types
of automatic modules (manifest and filename based).
And that is what this patch does: It removes the differentiation between
manifest and filename based dependencies.
A more complete fix would do this:
- plot the complete dependency graph
- start from the root. Any dependency that is a defined module "infects" its
full downstream tree. All of those dependencies need to go onto the module path
- look at the remainder of the graph. Anything here needs to be patched into
the main module.
E.g.
```
artifact --+--- defined module a --+-- defined module b-- +-- automatic
module 1 --+ automatic module 2
|
+--- automatic module 4 --+-- automatic module 5
|
+--- automatic module 6 --+-- defined module c -+--
automatic module 5
```
- defined module a goes onto the module path. As does defined module b,
automatic module 1 and automatic module 2
- defined module c goes onto the module path. This infects automatic module
5.
- automatic module 4 gets patched into the main module. its dependency has
been forced onto the module path
- automatic module 6 gets patched into the main module.
So the result is
```
--module-path defined module a:defined module b:defined module c:automatic
module 1:automatic module 2:automatic module 5
-- patch-module main.module=automatic module 4:automatic module 6
```
Or in the case above, the chain `main artifact --> guice (automatic module)
-> javax.inject (automatic module)` leads to both guice and javax.inject being
patched into main artifact.
If guice ever wises up and provides a module descriptor, it would be this
scenario: `main artifact --> guice (defined module) -> javax.inject (automatic
module)`
Which would force guice onto the module path and its dependency
(javax.inject) as well. That works as well as patching both. What does not work
is move one of them onto the module path and patch the other one into the main
module.
I *would* implement that, except that AbstractJavadocMojo.java is a brittle
6,100 lines (!) abomination littered with special cases, exceptions and
dependencies on other components of unclear function. Also most mojos in the
plugin share functionality here, all the way to the aggregation jar target. I
did not dare to touch this and so I did the minimal invasive patch.
No, seriously, this thing needs to be refactored before any serious work can
be done. Or better yet, thrown away and start over (maven-javadoc-next-plugin
anyone?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
