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.

Sign up for GitHub

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

Jump to bottom

Move JSpecify from provided to compile scope #3228

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

Move JSpecify from provided to compile scope #3228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions log4j-api-test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
15 changes: 9 additions & 6 deletions log4j-api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1

We have always had a rule that Log4j-API will have no required dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know this is against current policy, but can that policy be reevaluated?

The rationale is that keeping nullability annotations in the artifacts is mostly useful to log4j-api users. Log4j Core calls usually don't appear in user code, but Log4j API classes are more used.

While the implementation can certainly do null-checks, we might mark the format parameters of log calls as @NonNull: it doesn't make sense to log something, when the format string is null. Also the Level parameter should be marked @NonNull as well as many of the parameters and return types of ThreadContext.

</dependency>

</dependencies>
<build>
<plugins>
Expand Down
1 change: 1 addition & 0 deletions log4j-core-test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@
<artifactId>jspecify</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.sun.mail</groupId>
<artifactId>javax.mail</artifactId>
Expand Down
33 changes: 27 additions & 6 deletions log4j-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -108,114 +108,135 @@
</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>
<artifactId>javax.jms-api</artifactId>
<scope>provided</scope>
<optional>true</optional>
</dependency>

<!-- Required for SMTPAppender -->
<dependency>
<groupId>javax.mail</groupId>
<artifactId>javax.mail-api</artifactId>
<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>
Expand Down
13 changes: 12 additions & 1 deletion log4j-fuzz-test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
Loading