Document how to generate module files using moditect

[leventov]

Working on this PR: #51, I'm having trouble generating module-info.java for the new module (primitive-collections-base). It would be nice if this repository contained some dev documentation about how to initially generate & update module-infos for new and existing modules in this project.

[cowtowncoder]

Hmmh. That would be nice... I learned it by trial and error, but not sure I ever figured out a reliable mechanism even. Moditect plug-in has (at least) 2 targets; one that is enabled (use existing module-info.java, bake it in) is easy, but one used for first-time generation is trickier.
I think I added plugin settings at some point, under specific profile (so it needs to be invoked by setting maven profile on command line). This is done in pom jackson-base (which is actually in jackson-bom repo):

  <profiles>
    <profile>
      <id>moditect</id>
      <properties>
        <!-- Not only do we need JDK 9+, must target later JDK too -->
        <java.version>1.9</java.version>
      </properties>
      <build>
	<plugins>
	  <plugin>
	    <groupId>org.moditect</groupId>
	    <artifactId>moditect-maven-plugin</artifactId>
	    <executions>
	      <execution>
		<id>generate-module-info</id>
		<phase>generate-sources</phase>
		<goals>
		  <goal>generate-module-info</goal>
		</goals>
		<configuration>
		  <modules>
		    <module>
		      <artifact>
			<groupId>${moditect.sourceGroup}</groupId>
			<artifactId>${moditect.sourceArtifact}</artifactId>
			<version>${moditect.sourceVersion}</version>
		      </artifact>
		    </module>
		  </modules>
		</configuration>
	      </execution>
	    </executions>
	  </plugin>
	</plugins>
      </build>
    </profile>
  </profiles>

So. I think it's something like

mvn -P moditect package

but for some reason this requires that version has already been locally installed (mvn clean install).
Further, I think you need to run this from sub-directory (like guava etc), not from main level (since parent pom does not produce jar or bundle).

I'll check if I can make this work from home: and if so, it'd be great if you could double-check it, before I write it down. What I do remember is that Moditect plug-in is quite finicky, unfortunately.

[leventov]

A few findings:

1. I had to edit jackson-databind's module-info.java and remove the line requires java.base; from it. Otherwise, I got error like

Error reading module: .../.m2/repository/com/fasterxml/jackson/core/jackson-databind/3.0.0-SNAPSHOT/jackson-databind-3.0.0-SNAPSHOT.jar: Dependence upon java.base already declared

When generating module-infos in jackson-datatypes-collections.

2. In order to do the first mvn clean install step in jackson-datatypes-collections, I commented out the corresponding sections in modules where module-infos didn't exist yet:

<!--            <plugin>-->
<!--                <groupId>org.moditect</groupId>-->
<!--                <artifactId>moditect-maven-plugin</artifactId>-->
<!--            </plugin>-->

Because otherwise, there is a chicken-and-egg problem with mvn clean install as it wants src/moditect/module-info.java to exist already. Maybe there is a "smarter" Maven-way to disable plugin/goal/phase temporarily, but commenting out (and then uncommenting, after mvn clean install succeeds) worked just fine.

3. After that, going to a module directory (like primitive-collections-base) and running mvn -P moditect package still failed, but before the failure, it created a module-info.java in target/moditect/..., which I then was able to copy to src/moditect/module-info.java.

[leventov]

Another issue is that generated module-info doesn't look quite right to me (at least, it differs from module-infos generated previously), e. g. it has requires transitive directives: https://github.com/FasterXML/jackson-datatypes-collections/pull/51/files#diff-ec0e4a55788bc9b83c3cb05dc8017bf0

[cowtowncoder]

@GedMarc any idea why that recently added java.base dependency would be problematic here.

@leventov requires transitive is sort of matter of taste, I think: it just means that a dependency is exposed automatically to whoever relies on module. If not, they will separately need to declare it.
You can manually change that to follow what other modules are doing, I think: initial generation does NOT necessarily produce 100% as-is optimal output. At least that was my take.

Oh, one other thing: ideally we would get module-info in 2.10 branch. It may differ slightly from 3.0 (master), but I think you can check out diffs between 2.10 / 3.0 of other collection modules like Guava.

[GedMarc]

@cowtowncoder java.base is kind of the root of the entire module structure, so shouldn't really be placed anywhere ever. Should maybe raise that as an actual issue on moditect, I know @lukehutch worked on it for a bit? I think perhaps he would know better on this. Any alterations to this module is forcibly done on the command line --add-opens etc - even in jlink/jpackage artefacts

In terms of requires transitive (forcing the module dependency down the module chain breaking clean separation), from what I remember, this is done whenever a class of a dependent module is exposed in a public api method. In these cases the dependency must be transitive.
requires static comes to play when supporting multiple versions of an API with different module names.

(Also why I never auto generate the module-info's, I've come to always code from bottom up adding only what is referenced. that method limit of 64k is extremely painful and libraries adding to that complexity size for me now is a bit erggg)

[cowtowncoder]

@GedMarc ok I am asking because you suggested that it should be added in jackson-databind:

FasterXML/jackson-databind#2485

which seemed odd but (I thought) harmless. I can easily remove it, just want to make sure it should NOT be there, even if Moditect plug-in might suggest it (which I assume is where it came from)

And yes, I think as a rule of thumb if API exposes a type, that type should be transitive dependency.

[GedMarc]

omg oo sorry! covers face
I suppose lessons learnt, i got burnt on it trying to do the data.time exposures. I'm not having any issues though on my side using the current databind and annotations - then I have no idea where that error is coming from. It isn't really causing any hassles though
We'll see on the jre 11 tests soon soon *tm

[lukehutch]

You can see how ClassGraph uses ModiTect here:

https://github.com/classgraph/classgraph/blob/master/pom.xml

The reason for this usage was to build for JDK 7, but allow for proper requires on JDK 9+.

Although fair warning, even putting module-info.class into META-INF/versions/9 can cause older Spring and JBoss installations to crash, since they don't know how to parse the newer classfile format. (They depend on an old version of ASM, and it scans the whole classpath, even META-INF, then throws exceptions on classfile versions it doesn't recognize.)

I don't think you ever need to add requires java.base. No idea why that causes issues.

[cowtowncoder]

Will be interesting to see what kinds of issues users bump into with 2.10.0. I am hoping devs running on very old containers would use older Jackson versions, but one challenge is that due to CVEs reported there has been surprisingly strong push to newer versions.
And 2.10.0 is -- for better and worse -- the version where this torrent of CVEs will stop. So demand for upgrades should be high.

But so far I haven't really heard of issues with odd module-info.class.

I did remove that base reference so it won't be in 2.10.1, fwtw.

[leventov]

@cowtowncoder could you please also remove requires java.base in databind's master branch?

[cowtowncoder]

Done. Odd that merge didn't do it...