This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch feature/2.x/jspecify in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 51cf323fbcad28de27350777ec9b079760e0e449 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Fri Nov 22 10:38:13 2024 +0100 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 aef8a1b5d6..a30086d2b5 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 4b0525466d..8f38096247 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 8c8cbc19bb..7e13dee664 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 f5693dc2e9..6d0565627a 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 ba7cca9ce0..752f684c76 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 3855d8f441..dc66ff62c6 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 54899fad28..3d683e2319 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 68f305133a..1120f84601 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 babb336a73..942280c8ad 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>
