From 51cf323fbcad28de27350777ec9b079760e0e449 Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Fri, 22 Nov 2024 10:38:13 +0100
Subject: [PATCH] Move JSpecify from `provided` to `compile` scope

[JSpecify](https://jspecify.dev/) is a nullability annotation library with a
[large list of supporters](https://jspecify.dev/about/).
It is the recommended library to solve
[LOG4J2-1477](https://issues.apache.org/jira/browse/LOG4J2-1477).

The usage of JSpecify as `provided` library has however a drawback: if users don't have JSpecify on their stack, they'll get a Javac Linter warning whenever they use a class with nullability annotations
(see [#3110](https://github.com/apache/logging-log4j2/issues/3110) for example.
Since JSpecify is an annotation with `RUNTIME` retention and is not covered by
[JDK-8342833](https://bugs.openjdk.org/browse/JDK-8342833), the easiest solution provided by this PR is:

- Move JSpecify from the `provided` to the `compile` scope. This will inflate users dependency stack by a whooping 4000 bytes.
- Mark JSpecify as optional for OSGi and JPMS, so users can exclude the dependency if they wish to do it.

In practice, this change should be neutral for users and will allow us to add nullability annotations to important class, such as `Logger` without breaking the build of users that use the [`-Werror` compiler flag](https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html#option-Werror).
---
 log4j-api-test/pom.xml  | 22 ++++++++++++++++++++++
 log4j-api/pom.xml       | 15 +++++++++------
 log4j-core-test/pom.xml |  1 +
 log4j-core/pom.xml      | 33 +++++++++++++++++++++++++++------
 log4j-fuzz-test/pom.xml | 13 ++++++++++++-
 log4j-taglib/pom.xml    | 19 +++++++++++++++++--
 log4j-to-jul/pom.xml    | 22 ++++++++++++++++------
 log4j-to-slf4j/pom.xml  | 28 ++++++++++++++++++++++------
 pom.xml                 | 12 ------------
 9 files changed, 126 insertions(+), 39 deletions(-)

diff --git a/log4j-api-test/pom.xml b/log4j-api-test/pom.xml
index aef8a1b5d64..a30086d2b5a 100644
--- a/log4j-api-test/pom.xml
+++ b/log4j-api-test/pom.xml
@@ -69,102 +69,124 @@
     ~     It is used in StackLocatorUtilTest through a Class.forName
     -->
   <dependencies>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-api</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest</artifactId>
     </dependency>
+
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-api</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.junit-pioneer</groupId>
       <artifactId>junit-pioneer</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.junit.platform</groupId>
       <artifactId>junit-platform-commons</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.apache.maven</groupId>
       <artifactId>maven-core</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.apache.maven</groupId>
       <artifactId>maven-model</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.codehaus.plexus</groupId>
       <artifactId>plexus-utils</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.assertj</groupId>
       <artifactId>assertj-core</artifactId>
     </dependency>
+
     <!-- Required for JSON support -->
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <!-- Required for JSON support -->
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-databind</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-params</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-inline</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.jspecify</groupId>
       <artifactId>jspecify</artifactId>
       <scope>test</scope>
     </dependency>
+
     <!-- Used by ServiceLoaderUtilTest -->
     <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>uk.org.webcompere</groupId>
       <artifactId>system-stubs-core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>uk.org.webcompere</groupId>
       <artifactId>system-stubs-jupiter</artifactId>
       <scope>test</scope>
     </dependency>
+
   </dependencies>
   <build>
     <plugins>
diff --git a/log4j-api/pom.xml b/log4j-api/pom.xml
index 4b0525466df..8f38096247c 100644
--- a/log4j-api/pom.xml
+++ b/log4j-api/pom.xml
@@ -47,7 +47,7 @@
     <bnd-extra-package-options>
       <!-- Not exported by most OSGi system bundles, hence we use the system classloader to load `sun.reflect.Reflection` -->
       !sun.reflect,
-      <!-- Annotations only -->
+      <!-- Annotations only: users are allowed to exclude this dependency -->
       org.jspecify.*;resolution:=optional
     </bnd-extra-package-options>
     <bnd-extra-module-options>
@@ -63,16 +63,19 @@
   <dependencies>
 
     <dependency>
-      <groupId>org.jspecify</groupId>
-      <artifactId>jspecify</artifactId>
+      <groupId>org.osgi</groupId>
+      <artifactId>org.osgi.core</artifactId>
       <scope>provided</scope>
     </dependency>
 
+    <!--
+      ~ Effectively optional, but included due to its size and the compilation warnings its absence causes.
+      -->
     <dependency>
-      <groupId>org.osgi</groupId>
-      <artifactId>org.osgi.core</artifactId>
-      <scope>provided</scope>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
     </dependency>
+
   </dependencies>
   <build>
     <plugins>
diff --git a/log4j-core-test/pom.xml b/log4j-core-test/pom.xml
index 8c8cbc19bbd..7e13dee6640 100644
--- a/log4j-core-test/pom.xml
+++ b/log4j-core-test/pom.xml
@@ -254,6 +254,7 @@
       <artifactId>jspecify</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>com.sun.mail</groupId>
       <artifactId>javax.mail</artifactId>
diff --git a/log4j-core/pom.xml b/log4j-core/pom.xml
index f5693dc2e92..6d0565627ad 100644
--- a/log4j-core/pom.xml
+++ b/log4j-core/pom.xml
@@ -53,7 +53,7 @@
       -->
     <bnd-multi-release>true</bnd-multi-release>
     <bnd-extra-package-options>
-      <!-- Annotations only -->
+      <!-- Annotations only: users are allowed to exclude this dependency -->
       org.jspecify.*;resolution:=optional,
       <!-- External optional dependencies -->
       com.conversantmedia.util.concurrent;resolution:=optional;
@@ -108,12 +108,14 @@
   </properties>
 
   <dependencies>
+
     <dependency>
       <groupId>javax.activation</groupId>
       <artifactId>javax.activation-api</artifactId>
       <scope>provided</scope>
       <optional>true</optional>
     </dependency>
+
     <!-- Used for JMS appenders (needs an implementation of course) -->
     <dependency>
       <groupId>javax.jms</groupId>
@@ -121,6 +123,7 @@
       <scope>provided</scope>
       <optional>true</optional>
     </dependency>
+
     <!-- Required for SMTPAppender -->
     <dependency>
       <groupId>javax.mail</groupId>
@@ -128,94 +131,112 @@
       <scope>provided</scope>
       <optional>true</optional>
     </dependency>
-    <dependency>
-      <groupId>org.jspecify</groupId>
-      <artifactId>jspecify</artifactId>
-      <scope>provided</scope>
-    </dependency>
+
     <!-- Used for OSGi bundle support -->
     <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.core</artifactId>
       <scope>provided</scope>
     </dependency>
+
     <!-- Naturally, all implementations require the log4j-api JAR -->
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-api</artifactId>
     </dependency>
+
     <!-- Used for compressing to formats other than zip and gz -->
     <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-compress</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Used for the CSV layout -->
     <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-csv</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Alternative implementation of BlockingQueue using Conversant Disruptor for AsyncAppender -->
     <dependency>
       <groupId>com.conversantmedia</groupId>
       <artifactId>disruptor</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Required for AsyncLoggers -->
     <dependency>
       <groupId>com.lmax</groupId>
       <artifactId>disruptor</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Required for JSON support -->
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-core</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Required for JSON support -->
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-databind</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Required for XML layout and receiver support -->
     <dependency>
       <groupId>com.fasterxml.jackson.dataformat</groupId>
       <artifactId>jackson-dataformat-xml</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Required for YAML support (including JSON requirements) -->
     <dependency>
       <groupId>com.fasterxml.jackson.dataformat</groupId>
       <artifactId>jackson-dataformat-yaml</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Alternative implementation of BlockingQueue using JCTools for AsyncAppender -->
     <dependency>
       <groupId>org.jctools</groupId>
       <artifactId>jctools-core</artifactId>
       <optional>true</optional>
     </dependency>
+
     <!-- Used for ZeroMQ JeroMQ appender -->
     <dependency>
       <groupId>org.zeromq</groupId>
       <artifactId>jeromq</artifactId>
       <optional>true</optional>
     </dependency>
+
+    <!--
+      ~ Effectively optional, but included due to its size and the compilation warnings its absence causes.
+      -->
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+    </dependency>
+
     <!-- Used for Kafka appender -->
     <dependency>
       <groupId>org.apache.kafka</groupId>
       <artifactId>kafka-clients</artifactId>
       <optional>true</optional>
     </dependency>
+
     <dependency>
       <groupId>com.sun.mail</groupId>
       <artifactId>javax.mail</artifactId>
       <scope>runtime</scope>
       <optional>true</optional>
     </dependency>
+
   </dependencies>
 
   <build>
diff --git a/log4j-fuzz-test/pom.xml b/log4j-fuzz-test/pom.xml
index ba7cca9ce02..752f684c76a 100644
--- a/log4j-fuzz-test/pom.xml
+++ b/log4j-fuzz-test/pom.xml
@@ -41,6 +41,15 @@
     <!-- dependency versions -->
     <json.version>20240303</json.version>
 
+    <bnd-extra-package-options>
+      <!-- Annotations only: users are allowed to exclude this dependency -->
+      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>
@@ -50,10 +59,12 @@
       <artifactId>log4j-core</artifactId>
     </dependency>
 
+    <!--
+      ~ Effectively optional, but included due to its size and the compilation warnings its absence causes.
+      -->
     <dependency>
       <groupId>org.jspecify</groupId>
       <artifactId>jspecify</artifactId>
-      <scope>provided</scope>
     </dependency>
 
     <dependency>
diff --git a/log4j-taglib/pom.xml b/log4j-taglib/pom.xml
index 3855d8f441f..dc66ff62c61 100644
--- a/log4j-taglib/pom.xml
+++ b/log4j-taglib/pom.xml
@@ -32,7 +32,7 @@
       ~ OSGi and JPMS options
       -->
     <bnd-extra-package-options>
-      <!-- Annotations only -->
+      <!-- Annotations only: users are allowed to exclude this dependency -->
       org.jspecify.*;resolution:=optional
     </bnd-extra-package-options>
     <bnd-extra-module-options>
@@ -44,60 +44,75 @@
     </bnd-extra-module-options>
     <Fragment-Host>org.apache.logging.log4j.core</Fragment-Host>
   </properties>
+
   <dependencies>
+
     <dependency>
       <groupId>javax.servlet.jsp</groupId>
       <artifactId>javax.servlet.jsp-api</artifactId>
       <scope>provided</scope>
     </dependency>
+
     <dependency>
       <groupId>javax.servlet</groupId>
       <artifactId>javax.servlet-api</artifactId>
       <scope>provided</scope>
     </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-api</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-web</artifactId>
       <optional>true</optional>
     </dependency>
+
+    <!--
+      ~ Effectively optional, but included due to its size and the compilation warnings its absence causes.
+      -->
     <dependency>
       <groupId>org.jspecify</groupId>
       <artifactId>jspecify</artifactId>
-      <scope>provided</scope>
     </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-core-test</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>commons-logging</groupId>
       <artifactId>commons-logging</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.vintage</groupId>
       <artifactId>junit-vintage-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-test</artifactId>
       <scope>test</scope>
     </dependency>
+
   </dependencies>
 </project>
diff --git a/log4j-to-jul/pom.xml b/log4j-to-jul/pom.xml
index 54899fad282..3d683e2319e 100644
--- a/log4j-to-jul/pom.xml
+++ b/log4j-to-jul/pom.xml
@@ -31,7 +31,7 @@
 
   <properties>
     <bnd-extra-package-options>
-      <!-- Annotations only -->
+      <!-- Annotations only: users are allowed to exclude this dependency -->
       org.jspecify.*;resolution:=optional
     </bnd-extra-package-options>
     <bnd-extra-module-options>
@@ -41,39 +41,49 @@
   </properties>
 
   <dependencies>
-    <dependency>
-      <groupId>org.jspecify</groupId>
-      <artifactId>jspecify</artifactId>
-      <scope>provided</scope>
-    </dependency>
+
     <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.core</artifactId>
       <scope>provided</scope>
     </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-api</artifactId>
     </dependency>
+
+    <!--
+      ~ Effectively optional, but included due to its size and the compilation warnings its absence causes.
+      -->
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>org.assertj</groupId>
       <artifactId>assertj-core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava-testlib</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.vintage</groupId>
       <artifactId>junit-vintage-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
   </dependencies>
 </project>
diff --git a/log4j-to-slf4j/pom.xml b/log4j-to-slf4j/pom.xml
index 68f305133aa..1120f846017 100644
--- a/log4j-to-slf4j/pom.xml
+++ b/log4j-to-slf4j/pom.xml
@@ -40,7 +40,7 @@
       -->
     <slf4j.support.range>[1.7,3)</slf4j.support.range>
     <bnd-extra-package-options>
-      <!-- Annotations only -->
+      <!-- Annotations only: users are allowed to exclude this dependency -->
       org.jspecify.*;resolution:=optional,
       <!-- This bridge also support SLF4J 2.x -->
       org.slf4j.*;version="${slf4j.support.range}"
@@ -64,70 +64,86 @@
   </dependencyManagement>
 
   <dependencies>
+
     <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.core</artifactId>
       <scope>provided</scope>
     </dependency>
-    <dependency>
-      <groupId>org.jspecify</groupId>
-      <artifactId>jspecify</artifactId>
-      <scope>provided</scope>
-    </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-api</artifactId>
     </dependency>
+
+    <!--
+      ~ Effectively optional, but included due to its size and the compilation warnings its absence causes.
+      -->
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
+
     <dependency>
       <groupId>org.assertj</groupId>
       <artifactId>assertj-core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-params</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>ch.qos.logback</groupId>
       <artifactId>logback-classic</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>ch.qos.logback</groupId>
       <artifactId>logback-core</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>ch.qos.logback</groupId>
       <artifactId>logback-core</artifactId>
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-api-test</artifactId>
       <scope>test</scope>
     </dependency>
+
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
+
   </dependencies>
 
   <build>
diff --git a/pom.xml b/pom.xml
index babb336a735..942280c8ad7 100644
--- a/pom.xml
+++ b/pom.xml
@@ -977,18 +977,6 @@
         </property>
       </activation>
 
-      <!-- There are certain Java 8 bugs[1] that cause Mockito failures[2].
-           Adding necessary dependencies to workaround them.
-           [1] https://bugs.openjdk.org/browse/JDK-8152174
-           [2] https://github.com/mockito/mockito/issues/1449 -->
-      <dependencies>
-        <dependency>
-          <groupId>org.jspecify</groupId>
-          <artifactId>jspecify</artifactId>
-          <version>${jspecify.version}</version>
-        </dependency>
-      </dependencies>
-
       <build>
         <plugins>
           <plugin>