From c7a59eb0b1d7f8710aab8d06d2adc6d1df80bfa1 Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Sun, 8 Sep 2024 10:58:36 +0200
Subject: [PATCH] Fail build for `static transitive` modules
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This adds a Groovy script that fails the build
if any optional JPMS module has the `transitive` qualifier.

We remove the `transitive` modifier from all optional dependencies.

Closes #2929.

Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
---
 log4j-api/pom.xml      |  4 ++-
 log4j-core/pom.xml     |  1 +
 log4j-parent/pom.xml   | 63 ++++++++++++++++++++++++++++++++++++++++++
 log4j-to-jul/pom.xml   |  4 +++
 log4j-to-slf4j/pom.xml |  4 +++
 5 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/log4j-api/pom.xml b/log4j-api/pom.xml
index 5423c4f99fb..4b0525466df 100644
--- a/log4j-api/pom.xml
+++ b/log4j-api/pom.xml
@@ -54,7 +54,9 @@
       <!-- Used in StringBuilders through reflection -->
       java.sql;static=true,
       <!-- Used in ProcessIdUtil through reflection -->
-      java.management;static=true
+      java.management;static=true,
+      <!-- Remove `transitive` for optional dependencies -->
+      org.jspecify;transitive=false
     </bnd-extra-module-options>
 
   </properties>
diff --git a/log4j-core/pom.xml b/log4j-core/pom.xml
index 7976c800b5e..1e35ebce729 100644
--- a/log4j-core/pom.xml
+++ b/log4j-core/pom.xml
@@ -98,6 +98,7 @@
       java.naming;transitive=false,
       org.apache.commons.csv;transitive=false,
       org.fusesource.jansi;transitive=false,
+      org.jspecify;transitive=false,
       org.zeromq.jeromq;transitive=false,
       <!-- A module descriptor is only available in version 1.2.16+, hence it is not detected -->
       com.conversantmedia.disruptor;substitute="disruptor";transitive=false;static=true,
diff --git a/log4j-parent/pom.xml b/log4j-parent/pom.xml
index 5b4dc64cbcf..9753e45d30f 100644
--- a/log4j-parent/pom.xml
+++ b/log4j-parent/pom.xml
@@ -53,6 +53,7 @@
     <asciidoctor-maven-plugin.version>2.2.4</asciidoctor-maven-plugin.version>
     <docker-maven-plugin.version>0.45.0</docker-maven-plugin.version>
     <exam-maven-plugin.version>4.13.5</exam-maven-plugin.version>
+    <gmavenplus-plugin.version>3.0.2</gmavenplus-plugin.version>
     <maven-taglib-plugin.version>2.4</maven-taglib-plugin.version>
     <!-- `surefire.version` property used in `apache.org:apache`: -->
     <surefire.version>3.5.0</surefire.version>
@@ -995,6 +996,68 @@
         </executions>
       </plugin>
 
+      <plugin>
+        <groupId>org.codehaus.gmavenplus</groupId>
+        <artifactId>gmavenplus-plugin</artifactId>
+        <version>${gmavenplus-plugin.version}</version>
+        <dependencies>
+          <dependency>
+            <groupId>org.codehaus.groovy</groupId>
+            <artifactId>groovy-ant</artifactId>
+            <version>${groovy.version}</version>
+            <scope>runtime</scope>
+          </dependency>
+          <dependency>
+            <groupId>org.codehaus.groovy</groupId>
+            <artifactId>groovy</artifactId>
+            <version>${groovy.version}</version>
+            <scope>runtime</scope>
+          </dependency>
+        </dependencies>
+        <executions>
+          <execution>
+            <id>ban-static-transitive</id>
+            <goals>
+              <goal>execute</goal>
+            </goals>
+            <phase>verify</phase>
+            <configuration>
+              <continueExecuting>false</continueExecuting>
+              <scripts>
+                <script><![CDATA[
+                  import java.io.StringWriter
+                  import java.util.spi.ToolProvider
+
+                  if ("jar" != project.packaging) {
+                      log.info("Skipping module descriptor check, since the project type is not `jar`.")
+                      return
+                  }
+                  String jarFile = project.build.directory + "/" + project.build.finalName + ".jar";
+                  if (!new File(jarFile).exists()) {
+                      log.info("Skipping module descriptor check, since `" + jarFile + "` is missing.")
+                      return
+                  }
+                  StringWriter out = new StringWriter()
+                  StringWriter err = new StringWriter()
+                  ToolProvider jar = ToolProvider.findFirst("jar").orElseThrow()
+                  int result = jar.run(new PrintWriter(out), new PrintWriter(err), "-d", "-f", jarFile)
+                  if (result != 0) {
+                      throw new RuntimeException("Failed to decompile the module descriptor in `" + jarFile + "`:\n" + err)
+                  }
+                  log.debug("Module descriptor: " + out)
+                  for (String line : out.toString().split("\r?\n", -1)) {
+                      if (line.contains("static") && line.contains("transitive")) {
+                          throw new RuntimeException("The `static` and `transitive` modifiers should not be use together: " + line)
+                      }
+                  }
+                  log.info("Successfully verified module descriptor in `" + jarFile + "`.")
+              ]]></script>
+              </scripts>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+
     </plugins>
   </build>
 
diff --git a/log4j-to-jul/pom.xml b/log4j-to-jul/pom.xml
index 25caed1639f..54899fad282 100644
--- a/log4j-to-jul/pom.xml
+++ b/log4j-to-jul/pom.xml
@@ -34,6 +34,10 @@
       <!-- Annotations only -->
       org.jspecify.*;resolution:=optional
     </bnd-extra-package-options>
+    <bnd-extra-module-options>
+      <!-- Remove `transitive` for optional dependencies -->
+      org.jspecify;transitive=false
+    </bnd-extra-module-options>
   </properties>
 
   <dependencies>
diff --git a/log4j-to-slf4j/pom.xml b/log4j-to-slf4j/pom.xml
index 45907097dc5..50732f24c96 100644
--- a/log4j-to-slf4j/pom.xml
+++ b/log4j-to-slf4j/pom.xml
@@ -45,6 +45,10 @@
       <!-- This bridge also support SLF4J 2.x -->
       org.slf4j.*;version="${slf4j.support.range}"
     </bnd-extra-package-options>
+    <bnd-extra-module-options>
+      <!-- Remove `transitive` for optional dependencies -->
+      org.jspecify;transitive=false
+    </bnd-extra-module-options>
 
     <slf4j2.version>2.0.16</slf4j2.version>
   </properties>