Vert.x forces users to import log4j bom about vertx-dependencies

[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)