Merge pull request #97 from dmlloyd/circ

Fix circularity detection and add a test
quarkusio · Nov 15, 2018 · fd43033 · fd43033
2 parents b5aedde + 67b98cc
commit fd43033
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 1 deletion.
diff --git a/builder/src/main/java/org/jboss/builder/BuildChainBuilder.java b/builder/src/main/java/org/jboss/builder/BuildChainBuilder.java
@@ -233,7 +233,7 @@ private void cycleCheck(Set<BuildStepBuilder> builders, Set<BuildStepBuilder> vi
     }
 
     private void cycleCheck(BuildStepBuilder builder, Set<BuildStepBuilder> visited, Set<BuildStepBuilder> checked, final Map<BuildStepBuilder, Set<BuildStepBuilder>> dependencies) throws ChainBuildException {
-        if (checked.add(builder)) {
+        if (! checked.contains(builder)) {
             if (! visited.add(builder)) {
                 throw new ChainBuildException("Cycle detected: " + visited);
             }
@@ -243,6 +243,7 @@ private void cycleCheck(BuildStepBuilder builder, Set<BuildStepBuilder> visited,
                 visited.remove(builder);
             }
         }
+        checked.add(builder);
     }
 
     private void addOne(final Map<ItemId, List<Produce>> allProduces, final Set<BuildStepBuilder> included, final ArrayDeque<BuildStepBuilder> toAdd, final ItemId idToAdd, Set<BuildStepBuilder> dependencies) throws ChainBuildException {

diff --git a/builder/src/test/java/org/jboss/builder/BasicTests.java b/builder/src/test/java/org/jboss/builder/BasicTests.java
@@ -125,4 +125,35 @@ public void execute(final BuildContext context) {
         assertNotNull(result.consume(DummyItem.class));
         assertFalse(ran.get());
     }
+
+    @Test
+    public void testCircular() {
+        final BuildChainBuilder builder = BuildChain.builder();
+        builder.addFinal(DummyItem.class);
+        BuildStepBuilder stepBuilder = builder.addBuildStep(new BuildStep() {
+            public void execute(final BuildContext context) {
+                context.consume(DummyItem2.class);
+                context.produce(new DummyItem());
+            }
+        });
+        stepBuilder.produces(DummyItem.class);
+        stepBuilder.consumes(DummyItem2.class);
+        stepBuilder.build();
+        stepBuilder = builder.addBuildStep(new BuildStep() {
+            public void execute(final BuildContext context) {
+                context.consume(DummyItem.class);
+                context.produce(new DummyItem2());
+            }
+        });
+        stepBuilder.produces(DummyItem2.class);
+        stepBuilder.consumes(DummyItem.class);
+        stepBuilder.build();
+        try {
+            builder.build();
+            fail("Expected exception");
+        } catch (ChainBuildException expected) {
+            // ok
+        }
+    }
+
 }