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

Repackaged the library with native (vs automatic) JPMS module support #2025

Merged
merged 4 commits into from
Nov 1, 2023

Conversation

jhy
Copy link
Owner

@jhy jhy commented Nov 1, 2023

Adds an explicit module-info.java file. As a native module, tools requiring modules such as jlink will operate correctly without warnings.

This makes jsoup a multi-release jar so it's still compatible with java 8. The module-info.java file is:

module org.jsoup {
    exports org.jsoup;
    exports org.jsoup.helper;
    exports org.jsoup.nodes;
    exports org.jsoup.parser;
    exports org.jsoup.safety;
    exports org.jsoup.select;

    requires transitive java.xml; // for org.w3c.dom out of W3CDom
}

Nullability annotations
I looked at having also a requires static jsr305 entry as well. That's a build time dependency for our javax nonnull annotations. However as that's an automodule, we get the compile warning Required filename-based automodules detected: [jsr305-3.0.2.jar]. Please don't publish this project to a public artifact repository!. So I'm keeping it out. Everything still works and downstream consumers will still get nullability warnings in their IDE. However jsoup developers will see edit-time errors / warnings on the import and use of these annotations. Perhaps now is a good time to implement #1992.

Java 8 tests
This change requires a minimum Java version of 9, and the min CI builder is changed to 11 (LTS). We should add a CI action to test a built jar under Java 8. Tracked in #2026.

Fixes #1943
Fixes #1466

@jhy jhy mentioned this pull request Nov 1, 2023
#2026 tracks adding a test action (not compile) for JDK 8.
(That actually exists)
@jhy jhy added the improvement label Nov 1, 2023
@jhy jhy added this to the 1.17.1 milestone Nov 1, 2023
@jhy jhy self-assigned this Nov 1, 2023
@jhy jhy merged commit a8564c3 into master Nov 1, 2023
12 checks passed
@jhy jhy deleted the module-support branch November 1, 2023 05:11
@cowwoc
Copy link

cowwoc commented Nov 1, 2023

Regarding the null annotations, most IDEs just look at the class name, ignoring the package name. I created local copies of these classes in my own library and it solved the problem.

Another option is to use moditect-maven-plugin to add module-info.java to libraries that are missing them and re-publish them under your groupId. Here is an example of what I did for the p6spy library:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
	<modelVersion>4.0.0</modelVersion>
	<parent>
		<groupId>redacted</groupId>
		<artifactId>root</artifactId>
		<version>1.0-SNAPSHOT</version>
	</parent>
	<artifactId>add-module-info</artifactId>
	<description>Adds module-info.java to dependencies that are missing it</description>

	<build>
		<plugins>
			<plugin>
				<groupId>org.moditect</groupId>
				<artifactId>moditect-maven-plugin</artifactId>
				<executions>
					<execution>
						<id>add-modules</id>
						<phase>prepare-package</phase>
						<goals>
							<goal>add-module-info</goal>
						</goals>
						<configuration>
							<outputDirectory>${project.build.directory}/modules</outputDirectory>
							<modules>
								<module>
									<artifact>
										<groupId>p6spy</groupId>
										<artifactId>p6spy</artifactId>
										<version>${p6spy.version}</version>
									</artifact>
									<moduleInfoSource>
										module p6spy
										{
										requires java.sql;
										requires java.management;
										requires org.slf4j;

										uses com.p6spy.engine.logging.LoggingEventListener;
										uses com.p6spy.engine.spy.JdbcEventListenerFactory;
										uses com.p6spy.engine.event.JdbcEventListener;
										provides java.sql.Driver with com.p6spy.engine.spy.P6SpyDriver;

										exports com.p6spy.engine.spy;
										exports com.p6spy.engine.spy.option;
										exports com.p6spy.engine.logging;
										exports com.p6spy.engine.event;
										}
									</moduleInfoSource>
								</module>
							</modules>
							<overwriteExistingFiles>true</overwriteExistingFiles>
						</configuration>
					</execution>
				</executions>
			</plugin>
			<plugin>
				<groupId>org.apache.maven.plugins</groupId>
				<artifactId>maven-dependency-plugin</artifactId>
				<executions>
					<execution>
						<id>copy-sources</id>
						<phase>generate-sources</phase>
						<goals>
							<goal>copy</goal>
						</goals>
						<configuration>
							<artifactItems>
								<artifactItem>
									<groupId>p6spy</groupId>
									<artifactId>p6spy</artifactId>
									<version>${p6spy.version}</version>
									<classifier>sources</classifier>
								</artifactItem>
							</artifactItems>
							<outputDirectory>${project.build.directory}/sources</outputDirectory>
						</configuration>
					</execution>
				</executions>
			</plugin>
			<plugin>
				<groupId>org.apache.maven.plugins</groupId>
				<artifactId>maven-jar-plugin</artifactId>
				<configuration>
					<skipIfEmpty>true</skipIfEmpty>
				</configuration>
			</plugin>
			<plugin>
				<groupId>org.apache.maven.plugins</groupId>
				<artifactId>maven-install-plugin</artifactId>
				<executions>
					<execution>
						<id>default-install</id>
						<phase>install</phase>
						<configuration>
							<skip>true</skip>
						</configuration>
					</execution>
					<execution>
						<id>install-p6spy-with-modules</id>
						<phase>install</phase>
						<goals>
							<goal>install-file</goal>
						</goals>
						<configuration>
							<groupId>${project.groupId}</groupId>
							<artifactId>p6spy</artifactId>
							<version>${p6spy.version}</version>
							<packaging>jar</packaging>
							<file>${project.build.directory}/modules/p6spy-${p6spy.version}.jar</file>
							<sources>${project.build.directory}/sources/p6spy-${p6spy.version}-sources.jar</sources>
						</configuration>
					</execution>
				</executions>
			</plugin>
		</plugins>
	</build>
</project>

I then replaced all references from the original p6spy artifact to the new one. All the errors/warnings went away.

@jhy
Copy link
Owner Author

jhy commented Nov 4, 2023

Thanks @cowwoc, that's helpful. I'm tracking the nullability assertion change in #2028. I recall that when we chose the javax package vs just local names it was to ensure Kotlin understood the assertions. But in practice I don't know that that does much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a java module version of jsoup-xxx.jar Provide module-info.java for Java9+ users
2 participants