fix: enforce plugin-required dependencies and log incompatibilities (#…

…20601) (#20604) The Flow Maven Plugin uses a class loader that combines project and plugin dependencies to ensure class scanning happens on runtime artifacts. However, plugin execution may fail if the project defines dependency versions incompatible with those used by the plugin. This change enforces the use of plugin-defined versions for certain dependencies not directly used by Flow at runtime. Additionally, it logs potential incompatibilities for other dependencies if the build fails. Fixes vaadin/mpr-demo#23 Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin · Dec 3, 2024 · ef0e8a6 · ef0e8a6
1 parent ddfb795
commit ef0e8a6
Show file tree

Hide file tree

Showing 12 changed files with 463 additions and 24 deletions.
diff --git a/...flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/BuildDevBundleMojo.java b/...flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/BuildDevBundleMojo.java
@@ -17,6 +17,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -210,15 +211,37 @@ public void execute() throws MojoExecutionException, MojoFailureException {
         try {
             org.apache.maven.plugin.Mojo task = reflector.createMojo(this);
             findExecuteMethod(task.getClass()).invoke(task);
+            reflector.logIncompatibilities(getLog()::debug);
         } catch (MojoExecutionException | MojoFailureException e) {
+            logTroubleshootingHints(reflector, e);
             throw e;
         } catch (Exception e) {
+            logTroubleshootingHints(reflector, e);
             throw new MojoFailureException(e.getMessage(), e);
         } finally {
             Thread.currentThread().setContextClassLoader(tccl);
         }
     }
 
+    private void logTroubleshootingHints(Reflector reflector, Throwable ex) {
+        reflector.logIncompatibilities(getLog()::warn);
+        if (ex instanceof InvocationTargetException) {
+            ex = ex.getCause();
+        }
+        StringBuilder errorMessage = new StringBuilder(ex.getMessage());
+        Throwable cause = ex.getCause();
+        while (cause != null) {
+            if (cause.getMessage() != null) {
+                errorMessage.append(" ").append(cause.getMessage());
+            }
+            cause = cause.getCause();
+        }
+        getLog().error(
+                "The build process encountered an error: " + errorMessage);
+        logError(
+                "To diagnose the issue, please re-run Maven with the -X option to enable detailed debug logging and identify the root cause.");
+    }
+
     public void executeInternal() throws MojoFailureException {
         long start = System.nanoTime();
 

diff --git a/...-plugins/flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/Reflector.java b/...-plugins/flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/Reflector.java
@@ -28,6 +28,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -52,8 +53,13 @@ public final class Reflector {
     private static final Set<String> DEPENDENCIES_GROUP_EXCLUSIONS = Set.of(
             "org.apache.maven", "org.codehaus.plexus", "org.slf4j",
             "org.eclipse.sisu");
+    // Dependency required by the plugin but not provided by Flow at runtime
+    private static final Set<String> REQUIRED_PLUGIN_DEPENDENCIES = Set.of(
+            "org.reflections:reflections:jar",
+            "org.zeroturnaround:zt-exec:jar");
 
     private final URLClassLoader isolatedClassLoader;
+    private List<String> dependenciesIncompatibility;
     private Object classFinder;
 
     /**
@@ -66,9 +72,11 @@ public Reflector(URLClassLoader isolatedClassLoader) {
         this.isolatedClassLoader = isolatedClassLoader;
     }
 
-    private Reflector(URLClassLoader isolatedClassLoader, Object classFinder) {
+    private Reflector(URLClassLoader isolatedClassLoader, Object classFinder,
+            List<String> dependenciesIncompatibility) {
         this.isolatedClassLoader = isolatedClassLoader;
         this.classFinder = classFinder;
+        this.dependenciesIncompatibility = dependenciesIncompatibility;
     }
 
     /**
@@ -91,6 +99,7 @@ private Reflector(URLClassLoader isolatedClassLoader, Object classFinder) {
      *             it is not possible to make a copy for it due to class
      *             definition incompatibilities.
      */
+    @SuppressWarnings("unchecked")
     static Reflector adapt(Object reflector) {
         if (reflector instanceof Reflector sameClassLoader) {
             return sameClassLoader;
@@ -103,9 +112,13 @@ static Reflector adapt(Object reflector) {
                                 findField(reflectorClass,
                                         "isolatedClassLoader"),
                                 URLClassLoader.class);
+                List<String> dependenciesIncompatibility = (List<String>) ReflectTools
+                        .getJavaFieldValue(reflector, findField(reflectorClass,
+                                "dependenciesIncompatibility"));
                 Object classFinder = ReflectTools.getJavaFieldValue(reflector,
                         findField(reflectorClass, "classFinder"));
-                return new Reflector(classLoader, classFinder);
+                return new Reflector(classLoader, classFinder,
+                        dependenciesIncompatibility);
             } catch (Exception e) {
                 throw new IllegalArgumentException(
                         "Object of type " + reflector.getClass().getName()
@@ -198,9 +211,27 @@ public Mojo createMojo(BuildDevBundleMojo sourceMojo) throws Exception {
      */
     public static Reflector of(MavenProject project,
             MojoExecution mojoExecution) {
+        List<String> dependenciesIncompatibility = new ArrayList<>();
         URLClassLoader classLoader = createIsolatedClassLoader(project,
-                mojoExecution);
-        return new Reflector(classLoader);
+                mojoExecution, dependenciesIncompatibility);
+        Reflector reflector = new Reflector(classLoader);
+        reflector.dependenciesIncompatibility = dependenciesIncompatibility;
+        return reflector;
+    }
+
+    void logIncompatibilities(Consumer<String> logger) {
+        if (dependenciesIncompatibility != null) {
+            logger.accept(
+                    """
+                            Found dependencies defined with different versions in project and Vaadin maven plugin.
+                            Project dependencies are used, but plugin execution could fail if the versions are incompatible.
+                            In case of build failure please analyze the project dependencies and update versions or configure exclusions for potential offending transitive dependencies.
+                            You can use 'mvn dependency:tree -Dincludes=groupId:artifactId' to detect where the dependency is defined in the project.
+
+                            """
+                            + String.join(System.lineSeparator(),
+                                    dependenciesIncompatibility));
+        }
     }
 
     private synchronized Object getOrCreateClassFinder() throws Exception {
@@ -215,7 +246,8 @@ private synchronized Object getOrCreateClassFinder() throws Exception {
     }
 
     private static URLClassLoader createIsolatedClassLoader(
-            MavenProject project, MojoExecution mojoExecution) {
+            MavenProject project, MojoExecution mojoExecution,
+            List<String> dependenciesIncompatibility) {
         List<URL> urls = new ArrayList<>();
         String outputDirectory = project.getBuild().getOutputDirectory();
         if (outputDirectory != null) {
@@ -246,17 +278,61 @@ private static URLClassLoader createIsolatedClassLoader(
                                         && artifact.getFile().getPath().matches(
                                                 INCLUDE_FROM_COMPILE_DEPS_REGEX))))
                 .collect(Collectors.toMap(keyMapper, Function.identity())));
+
         if (mojoExecution != null) {
-            mojoExecution.getMojoDescriptor().getPluginDescriptor()
-                    .getArtifacts().stream()
+
+            List<Artifact> pluginDependencies = mojoExecution
+                    .getMojoDescriptor().getPluginDescriptor().getArtifacts()
+                    .stream()
                     // Exclude all maven artifacts to prevent class loading
                     // clash with maven.api class realm
                     .filter(artifact -> !DEPENDENCIES_GROUP_EXCLUSIONS
                             .contains(artifact.getGroupId()))
-                    .filter(artifact -> !projectDependencies
-                            .containsKey(keyMapper.apply(artifact)))
-                    .forEach(artifact -> projectDependencies
-                            .put(keyMapper.apply(artifact), artifact));
+                    .toList();
+
+            // Exclude project artifact that are also defined as mandatory
+            // plugin dependencies. The version provided by the plugin will be
+            // used to prevent failures during maven build.
+            pluginDependencies.stream().map(keyMapper)
+                    .filter(REQUIRED_PLUGIN_DEPENDENCIES::contains)
+                    .forEach(projectDependencies::remove);
+
+            // Preserve required plugin dependency that are not provided by Flow
+            // -1: dependency defined on both plugin and project, with different
+            // version
+            // 0: dependency defined on both plugin and project, with same
+            // version
+            // 1: dependency defined by the plugin only
+            Map<Integer, List<Artifact>> potentialDuplicates = pluginDependencies
+                    .stream().collect(Collectors.groupingBy(pluginArtifact -> {
+                        Artifact projectArtifact = projectDependencies
+                                .get(keyMapper.apply(pluginArtifact));
+                        if (projectArtifact == null) {
+                            return 1;
+                        } else if (projectArtifact.getId()
+                                .equals(pluginArtifact.getId())) {
+                            return 0;
+                        }
+                        return -1;
+                    }));
+            // Log potential plugin and project dependency versions
+            // incompatibilities.
+            if (potentialDuplicates.containsKey(-1)) {
+                potentialDuplicates.get(-1).stream().map(pluginArtifact -> {
+                    String key = keyMapper.apply(pluginArtifact);
+                    return String.format(
+                            "%s: project version [%s], plugin version [%s]",
+                            key, projectDependencies.get(key).getBaseVersion(),
+                            pluginArtifact.getBaseVersion());
+                }).forEach(dependenciesIncompatibility::add);
+            }
+
+            // Add dependencies defined only by the plugin
+            if (potentialDuplicates.containsKey(1)) {
+                potentialDuplicates.get(1)
+                        .forEach(artifact -> projectDependencies
+                                .put(keyMapper.apply(artifact), artifact));
+            }
         }
 
         projectDependencies.values().stream()

diff --git a/flow-plugins/flow-maven-plugin/src/it/offending-dependency-project/invoker.properties b/flow-plugins/flow-maven-plugin/src/it/offending-dependency-project/invoker.properties
@@ -0,0 +1,18 @@
+#
+# Copyright 2000-2024 Vaadin Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+#
+
+invoker.goals=clean package
+invoker.buildResult=failure
diff --git a/flow-plugins/flow-maven-plugin/src/it/offending-dependency-project/pom.xml b/flow-plugins/flow-maven-plugin/src/it/offending-dependency-project/pom.xml
@@ -0,0 +1,68 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <groupId>com.vaadin.test.maven</groupId>
+    <artifactId>offending-dependency-project</artifactId>
+    <version>1.0</version>
+    <packaging>jar</packaging>
+
+    <description>
+        Tests that project dependencies does not override plugin required dependency.
+    </description>
+
+    <properties>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <maven.compiler.release>17</maven.compiler.release>
+        <maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
+        <maven.compiler.target>${maven.compiler.release}</maven.compiler.target>
+        <maven.test.skip>true</maven.test.skip>
+
+        <flow.version>@project.version@</flow.version>
+        <maven.version>3.9.9</maven.version>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>com.vaadin</groupId>
+            <artifactId>flow-server</artifactId>
+            <version>${flow.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>com.vaadin</groupId>
+            <artifactId>flow-client</artifactId>
+            <version>${flow.version}</version>
+        </dependency>
+        <!-- commons-io 2.6 is incompatible with Flow plugin -->
+        <dependency>
+            <groupId>commons-io</groupId>
+            <artifactId>commons-io</artifactId>
+            <version>2.6</version>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <version>3.13.0</version>
+            </plugin>
+            <plugin>
+                <groupId>com.vaadin</groupId>
+                <artifactId>flow-maven-plugin</artifactId>
+                <version>${flow.version}</version>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>prepare-frontend</goal>
+                            <goal>build-frontend</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+
+</project>
diff --git a/...c/it/offending-dependency-project/src/main/java/com/vaadin/test/ProjectFlowExtension.java b/...c/it/offending-dependency-project/src/main/java/com/vaadin/test/ProjectFlowExtension.java
@@ -0,0 +1,20 @@
+package com.vaadin.test;
+
+import java.util.List;
+
+import com.vaadin.flow.server.frontend.Options;
+import com.vaadin.flow.server.frontend.TypeScriptBootstrapModifier;
+import com.vaadin.flow.server.frontend.scanner.FrontendDependenciesScanner;
+
+/**
+ * Hello world!
+ */
+public class ProjectFlowExtension implements TypeScriptBootstrapModifier {
+
+    @Override
+    public void modify(List<String> bootstrapTypeScript, Options options,
+            FrontendDependenciesScanner frontendDependenciesScanner) {
+        System.out.println("ProjectFlowExtension");
+        bootstrapTypeScript.add("(window as any).testProject=1;");
+    }
+}
diff --git a/flow-plugins/flow-maven-plugin/src/it/offending-dependency-project/verify.bsh b/flow-plugins/flow-maven-plugin/src/it/offending-dependency-project/verify.bsh
@@ -0,0 +1,15 @@
+import java.nio.file.*;
+
+flowTsx = basedir.toPath().resolve("build.log");
+if ( !Files.exists(flowTsx, new LinkOption[0]) )
+{
+    throw new RuntimeException("build.log not found");
+}
+
+lines = Files.readString(flowTsx);
+if (
+    !lines.contains("Found dependencies defined with different versions in project and Vaadin maven plugin") &&
+    !lines.matches("^commons-io:commons-io.*\\[2\\.6\\],.*")
+    ) {
+    throw new RuntimeException("Offending commons-io 2.6 dependency not detected");
+}
diff --git a/flow-plugins/flow-maven-plugin/src/it/plugin-pinned-deps-project/invoker.properties b/flow-plugins/flow-maven-plugin/src/it/plugin-pinned-deps-project/invoker.properties
@@ -0,0 +1,17 @@
+#
+# Copyright 2000-2024 Vaadin Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+#
+
+invoker.goals=clean package