Vert.x forces users to import log4j bom

[tsegismont]

In order to make sure logging frameworks are not transitive dependencies of vert.x, we have forced log4j and others versions and scope in 1c6a087

This prevents users from simply adding log4j core or logback to their project dependencies in order to get logging to work (see forum or StackOverflow)

The workaround is to add the log4j bom before the vertx stack depchain in the user project. We do the same in the Vert.x starter:

https://github.com/vert-x3/vertx-starter/blob/03615431ea8dab854cf7893bf294fd6811711850/pom.xml#L20-L44

[tsegismont]

In order to fix this, we could use the Maven enforcer plugin in the Vert.x parent, configured with a rule that checks the scope/optional attributes of the logging framework dependencies.

[tsegismont]

There is the banned dependencies built-in rule in the Maven enforcer project which does almost what we need: it can be configured to authorize some dependencies only if they have a certain scope.

For example, this config allows log4j and slf4j only if they have test scope.

                  <excludes>
                    <exclude>org.slf4j</exclude>
                    <exclude>org.apache.logging.log4j</exclude>
                  </excludes>
                  <includes>
                    <include>org.slf4j:*:*:jar:test</include>
                    <include>org.apache.logging.log4j:*:*:jar:test</include>
                  </includes>

But this is not exactly what we need, because we want to authorize logging libraries if they are declared optional.

I made this patch to the project:

Index: enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependencies.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependencies.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependencies.java
--- a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependencies.java	(revision 7c543f03b31c7009eb65401b1ed8b2bc80ea97a0)
+++ b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependencies.java	(date 1707469829111)
@@ -33,6 +33,11 @@
 @Named("bannedDependencies")
 public final class BannedDependencies extends BannedDependenciesBase {
 
+    /**
+     * Whether a banned dependency can be permitted if it is declared optional.
+     */
+    private boolean permitOptionalDependencies = true;
+
     @Inject
     BannedDependencies(MavenSession session, ResolverUtil resolverUtil) {
         super(session, resolverUtil);
@@ -41,7 +46,8 @@
     @Override
     protected boolean validate(Artifact artifact) {
         return !ArtifactUtils.matchDependencyArtifact(artifact, getExcludes())
-                || ArtifactUtils.matchDependencyArtifact(artifact, getIncludes());
+                || ArtifactUtils.matchDependencyArtifact(artifact, getIncludes())
+                || (permitOptionalDependencies && artifact.isOptional());
     }
 
     @Override
@@ -52,7 +58,23 @@
     @Override
     public String toString() {
         return String.format(
-                "BannedDependencies[message=%s, excludes=%s, includes=%s, searchTransitive=%b]",
-                getMessage(), getExcludes(), getIncludes(), isSearchTransitive());
+                "BannedDependencies[message=%s, excludes=%s, includes=%s, searchTransitive=%b, permitOptionalDependencies=%b]",
+                getMessage(), getExcludes(), getIncludes(), isSearchTransitive(), isPermitOptionalDependencies());
+    }
+
+    /**
+     * @return {@code true} if a banned dependency can be permitted if it is declared optional
+     */
+    public boolean isPermitOptionalDependencies() {
+        return permitOptionalDependencies;
+    }
+
+    /**
+     * Set whether a banned dependency can be permitted if it is declared optional.
+     *
+     * @param permitOptionalDependencies {@code true} to permit, otherwise {@code false}
+     */
+    public void setPermitOptionalDependencies(boolean permitOptionalDependencies) {
+        this.permitOptionalDependencies = permitOptionalDependencies;
     }
 }

Using a snapshot build of the enforcer Maven plugin, with this configuration:

          <execution>
            <id>enforce-banned-dependencies</id>
            <goals>
              <goal>enforce</goal>
            </goals>
            <configuration>
              <skip>${skipBannedLoggingDependencyRule}</skip>
              <rules>
                <bannedDependencies>
                  <message>No logging dependencies unless explicitly declared optional</message>
                  <excludes>
                    <exclude>org.slf4j</exclude>
                    <exclude>org.apache.logging.log4j</exclude>
                  </excludes>
                  <includes>
                    <include>org.slf4j:*:*:jar:test</include>
                    <include>org.apache.logging.log4j:*:*:jar:test</include>
                  </includes>
                  <permitOptionalDependencies>true</permitOptionalDependencies>
                </bannedDependencies>
              </rules>
              <fail>true</fail>
            </configuration>
          </execution>

I was able to confirm optional logging dependencies are permitted, others are banned (build fails).

But some projects require a logging dependency (if, for example, the implementation relies on a 3rd-party library that only works with slf4j). In this case, we'd only have to add a property to the module POM file:

diff --git a/pom.xml b/pom.xml
index 4a4a9fa..e39070a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>io.vertx</groupId>
     <artifactId>vertx-ext-parent</artifactId>
-    <version>38</version>
+    <version>39-SNAPSHOT</version>
   </parent>
 
   <artifactId>vertx-cassandra-client</artifactId>
@@ -22,6 +22,8 @@
     <logback.version>1.3.12</logback.version>
 
     <jar.manifest>${project.basedir}/src/main/resources/META-INF/MANIFEST.MF</jar.manifest>
+
+    <skipBannedLoggingDependencyRule>true</skipBannedLoggingDependencyRule>
   </properties>
 
   <dependencyManagement>

I've tested these changes with the vertx aggregator project and then the dependency convergence test in vertx-stack. The builds and test pass succesfully.

[tsegismont]

@vietj any comments before I start discussing the patch with the Maven enforcer plugin committers?

[tsegismont]

Things to consider before moving forward with this:

• remove relationship between vertx-stack-depchain and vertx-dependencies, which implies (at least):
  • removing import at the top level vertx-stack project
  • adding missing modules in vertx-stack-depchain (e.g. Vert.x Lang Kotlin)
  • adding project version property to all entries of ``vertx-stack-depchain`
• understanding why scopes where added to vertx-dependencies in 1c6a087
  • my assumption is to enforce a rule for logging dependencies, as the commit message indicates
• make sure tests pass in some projects (e.g Vert.x core)