Merge pull request quarkusio#45120 from aloubyansky/fix-stackoverflow…

…-dev-mode-conditional-deps Fix an endless loop resolving conditional dependencies on regular artifacts
bschuhmann · Dec 16, 2024 · 81c3cf1 · 81c3cf1
2 parents 109cb5d + 6985f01
commit 81c3cf1
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 12 deletions.
diff --git a/.../io/quarkus/bootstrap/resolver/test/DevUiStyleConditionalDevModeDependenciesTestCase.java b/.../io/quarkus/bootstrap/resolver/test/DevUiStyleConditionalDevModeDependenciesTestCase.java
@@ -0,0 +1,67 @@
+package io.quarkus.bootstrap.resolver.test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Collection;
+
+import io.quarkus.bootstrap.app.QuarkusBootstrap;
+import io.quarkus.bootstrap.resolver.CollectDependenciesBase;
+import io.quarkus.bootstrap.resolver.TsArtifact;
+import io.quarkus.bootstrap.resolver.TsQuarkusExt;
+import io.quarkus.bootstrap.resolver.maven.IncubatingApplicationModelResolver;
+import io.quarkus.maven.dependency.ArtifactCoords;
+import io.quarkus.maven.dependency.DependencyFlags;
+import io.quarkus.maven.dependency.ResolvedDependency;
+
+public class DevUiStyleConditionalDevModeDependenciesTestCase extends CollectDependenciesBase {
+
+    @Override
+    protected QuarkusBootstrap.Mode getBootstrapMode() {
+        return QuarkusBootstrap.Mode.DEV;
+    }
+
+    @Override
+    protected void setupDependencies() {
+
+        final TsArtifact extLibDev = TsArtifact.jar("ext-lib-dev");
+
+        final TsQuarkusExt extA = new TsQuarkusExt("ext-a");
+        extA.setConditionalDevDeps(extLibDev);
+        extLibDev.addDependency(extA.getRuntime());
+
+        install(extA, false);
+        installAsDep(extA.getRuntime(),
+                DependencyFlags.DIRECT
+                        | DependencyFlags.RUNTIME_EXTENSION_ARTIFACT
+                        | DependencyFlags.TOP_LEVEL_RUNTIME_EXTENSION_ARTIFACT);
+        install(extLibDev, true);
+        addCollectedDeploymentDep(extA.getDeployment());
+    }
+
+    @Override
+    protected void assertBuildDependencies(Collection<ResolvedDependency> buildDeps) {
+        if (!IncubatingApplicationModelResolver.isIncubatingEnabled(null)) {
+            return;
+        }
+        for (var d : buildDeps) {
+            switch (d.getArtifactId()) {
+                case "ext-a":
+                    assertThat(d.getDependencies()).containsExactlyInAnyOrder(
+                            ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-lib-dev", TsArtifact.DEFAULT_VERSION));
+                    break;
+                case "ext-a-deployment":
+                    assertThat(d.getDependencies()).containsExactlyInAnyOrder(
+                            ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID,
+                                    d.getArtifactId().substring(0, d.getArtifactId().length() - "-deployment".length()),
+                                    TsArtifact.DEFAULT_VERSION));
+                    break;
+                case "ext-lib-dev":
+                    assertThat(d.getDependencies()).containsExactlyInAnyOrder(
+                            ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-a", TsArtifact.DEFAULT_VERSION));
+                    break;
+                default:
+                    throw new RuntimeException("unexpected dependency " + d.toCompactCoords());
+            }
+        }
+    }
+}
diff --git a/.../src/main/java/io/quarkus/bootstrap/resolver/maven/ApplicationDependencyTreeResolver.java b/.../src/main/java/io/quarkus/bootstrap/resolver/maven/ApplicationDependencyTreeResolver.java
@@ -234,12 +234,10 @@ private void enableConditionalDeps() {
                 final Iterator<ConditionalDependency> i = unsatisfiedConditionalDeps.iterator();
                 while (i.hasNext()) {
                     final ConditionalDependency cd = i.next();
-                    final boolean satisfied = cd.isSatisfied();
-                    if (!satisfied) {
-                        continue;
+                    if (cd.isSatisfied()) {
+                        i.remove();
+                        cd.activate();
                     }
-                    i.remove();
-                    cd.activate();
                 }
                 if (totalConditionsToProcess == unsatisfiedConditionalDeps.size()) {
                     // none of the dependencies was satisfied
@@ -465,6 +463,8 @@ private void visitRuntimeDependency(DependencyNode node) {
                     if (isWalkingFlagOn(COLLECT_TOP_EXTENSION_RUNTIME_NODES)) {
                         dep.setFlags(DependencyFlags.TOP_LEVEL_RUNTIME_EXTENSION_ARTIFACT);
                     }
+                    managedDeps.add(new Dependency(extDep.info.deploymentArtifact, JavaScopes.COMPILE));
+                    collectConditionalDependencies(extDep);
                 }
                 if (isWalkingFlagOn(COLLECT_RELOADABLE_MODULES)) {
                     if (module != null) {
@@ -519,13 +519,7 @@ private ExtensionDependency getExtensionDependencyOrNull(DependencyNode node, Ar
         return null;
     }
 
-    private void visitExtensionDependency(ExtensionDependency extDep)
-            throws BootstrapDependencyProcessingException {
-
-        managedDeps.add(new Dependency(extDep.info.deploymentArtifact, JavaScopes.COMPILE));
-
-        collectConditionalDependencies(extDep);
-
+    private void visitExtensionDependency(ExtensionDependency extDep) {
         if (clearWalkingFlag(COLLECT_TOP_EXTENSION_RUNTIME_NODES)) {
             currentTopLevelExtension = extDep;
         } else if (currentTopLevelExtension != null) {