From 14a686ab8814204dfa79dab3d713f838b2b62bab Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 11 Jun 2026 18:30:39 +0200 Subject: [PATCH 1/2] Find all elementary dependency cycles using Johnson's algorithm The previous DFS with parent pointers only reported cycles reachable along its particular DFS tree, so overlapping cycles in dense graphs were missed. The detector now enumerates every elementary cycle exactly once via Johnson's algorithm (per-vertex strongly connected component plus blocked-set circuit search), which also makes the old cycle normalization and deduplication unnecessary. A safety cap of 1000 cycles guards against exponential blow-up and is reported in the console when hit. Also removes the unused jakarta.annotation import from the debugtools manifest, which failed to resolve against the 2026-06 target platform and broke the build of this bundle. --- .../META-INF/MANIFEST.MF | 3 +- .../DetectCyclicDependenciesHandler.java | 242 +++++++++++------- 2 files changed, 145 insertions(+), 100 deletions(-) diff --git a/com.vogella.ide.debugtools/META-INF/MANIFEST.MF b/com.vogella.ide.debugtools/META-INF/MANIFEST.MF index 88dbe39..9558425 100644 --- a/com.vogella.ide.debugtools/META-INF/MANIFEST.MF +++ b/com.vogella.ide.debugtools/META-INF/MANIFEST.MF @@ -13,7 +13,6 @@ Require-Bundle: org.eclipse.jface, org.eclipse.ui.console;bundle-version="3.15.0", org.eclipse.ui;bundle-version="3.207.400" Bundle-RequiredExecutionEnvironment: JavaSE-25 -Import-Package: jakarta.annotation;version="[2.1.0,3.0.0)", - jakarta.inject;version="[2.0.0,3.0.0)", +Import-Package: jakarta.inject;version="[2.0.0,3.0.0)", org.osgi.framework;version="[1.10.0,2.0.0)" Automatic-Module-Name: com.vogella.ide.debugtools diff --git a/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java b/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java index 0afd1e1..b145890 100644 --- a/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java +++ b/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java @@ -20,6 +20,7 @@ /** * Eclipse e4 Handler to detect cyclic dependencies between plug-ins in the workspace. * Detects cycles from both Require-Bundle and Import-Package dependencies. + * Uses Johnson's algorithm to enumerate all elementary cycles, each reported exactly once. */ public class DetectCyclicDependenciesHandler { @@ -63,9 +64,13 @@ public void execute(@Named(IServiceConstants.ACTIVE_SHELL) Shell shell) { out.println("\nCycle " + (i + 1) + ":"); out.println(generateAsciiArt(cycleInfo)); } - + + if (detector.isLimitReached()) { + out.println("\nNote: more cycles exist; output was truncated at " + cycles.size() + " cycles."); + } + out.println("================================================="); - + // Show a dialog, but refer them to the console for the big ASCII art MessageDialog.openWarning(shell, "Cyclic Dependencies Detected", dialogMessage.toString()); @@ -157,8 +162,8 @@ private void showConsoleView(IConsole myConsole) { } } - // --- Nested Helper Classes (CycleInfo, CyclicDependencyDetector) remain unchanged --- - + // --- Nested Helper Classes --- + private static class CycleInfo { List cycle; Map edgeTypes; @@ -178,43 +183,32 @@ String getEdgeType(String from, String to) { } private static class CyclicDependencyDetector { - private Map> dependencyGraph; - private Set visited; - private Set recursionStack; + + private static final int MAX_CYCLES = 1000; + + // from -> (to -> dependency type of that edge) + private Map> dependencyGraph; + private Map> reverseGraph; private List cycles; - private Map parent; - private Map parentEdge; - - private static class DependencyEdge { - String target; - String type; - - DependencyEdge(String target, String type) { - this.target = target; - this.type = type; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - DependencyEdge that = (DependencyEdge) o; - return Objects.equals(target, that.target) && Objects.equals(type, that.type); - } - - @Override - public int hashCode() { - return Objects.hash(target, type); - } - } - + private boolean limitReached; + + // state of Johnson's circuit search + private Deque path; + private Set blocked; + private Map> blockedBy; + public List detectCycles() throws CoreException { dependencyGraph = new HashMap<>(); cycles = new ArrayList<>(); buildDependencyGraph(); + buildReverseGraph(); findAllCycles(); return cycles; } + + public boolean isLimitReached() { + return limitReached; + } private void buildDependencyGraph() throws CoreException { IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot(); @@ -242,7 +236,7 @@ private void buildDependencyGraph() throws CoreException { for (Map.Entry entry : workspaceModels.entrySet()) { String pluginId = entry.getKey(); IPluginModelBase model = entry.getValue(); - Set dependencies = new HashSet<>(); + Map dependencies = new HashMap<>(); BundleDescription bundleDesc = model.getBundleDescription(); if (bundleDesc != null) { BundleSpecification[] requiredBundles = bundleDesc.getRequiredBundles(); @@ -250,7 +244,7 @@ private void buildDependencyGraph() throws CoreException { for (BundleSpecification spec : requiredBundles) { String depId = spec.getName(); if (workspaceModels.containsKey(depId)) { - dependencies.add(new DependencyEdge(depId, "Require-Bundle")); + dependencies.putIfAbsent(depId, "Require-Bundle"); } } } @@ -260,7 +254,7 @@ private void buildDependencyGraph() throws CoreException { String packageName = importSpec.getName(); String providingBundle = packageToBundle.get(packageName); if (providingBundle != null && !providingBundle.equals(pluginId)) { - dependencies.add(new DependencyEdge(providingBundle, "Import-Package: " + packageName)); + dependencies.putIfAbsent(providingBundle, "Import-Package: " + packageName); } } } @@ -268,84 +262,136 @@ private void buildDependencyGraph() throws CoreException { dependencyGraph.put(pluginId, dependencies); } } + + private void buildReverseGraph() { + reverseGraph = new HashMap<>(); + for (Map.Entry> entry : dependencyGraph.entrySet()) { + for (String target : entry.getValue().keySet()) { + reverseGraph.computeIfAbsent(target, k -> new HashSet<>()).add(entry.getKey()); + } + } + } + /** + * Johnson's algorithm: for each vertex (in sorted order) enumerate all + * cycles through it within its strongly connected component, then + * remove it from the graph. Every elementary cycle is found exactly + * once, rooted at its lexicographically smallest plug-in. + */ private void findAllCycles() { - visited = new HashSet<>(); - for (String plugin : dependencyGraph.keySet()) { - if (!visited.contains(plugin)) { - recursionStack = new HashSet<>(); - parent = new HashMap<>(); - parentEdge = new HashMap<>(); - detectCycleFromNode(plugin); + List vertices = new ArrayList<>(dependencyGraph.keySet()); + Collections.sort(vertices); + Set removed = new HashSet<>(); + for (String start : vertices) { + if (limitReached) { + return; } + Map edges = dependencyGraph.get(start); + if (edges.containsKey(start)) { + CycleInfo selfCycle = new CycleInfo(List.of(start, start)); + selfCycle.addEdge(start, start, edges.get(start)); + addCycle(selfCycle); + } + Set component = strongComponentOf(start, removed); + if (component.size() > 1) { + path = new ArrayDeque<>(); + blocked = new HashSet<>(); + blockedBy = new HashMap<>(); + circuit(start, start, component); + } + removed.add(start); } } - - private void detectCycleFromNode(String node) { - visited.add(node); - recursionStack.add(node); - Set dependencies = dependencyGraph.get(node); - if (dependencies != null) { - for (DependencyEdge edge : dependencies) { - String dep = edge.target; - if (!visited.contains(dep)) { - parent.put(dep, node); - parentEdge.put(dep, edge); - detectCycleFromNode(dep); - } else if (recursionStack.contains(dep)) { - CycleInfo cycle = extractCycle(node, dep, edge); - if (!isDuplicateCycle(cycle)) { - cycles.add(cycle); - } + + /** + * Strongly connected component containing start, ignoring removed + * vertices: the intersection of the vertices reachable from start and + * the vertices from which start is reachable. + */ + private Set strongComponentOf(String start, Set removed) { + Set forward = new HashSet<>(); + collectReachable(start, removed, forward, false); + Set backward = new HashSet<>(); + collectReachable(start, removed, backward, true); + forward.retainAll(backward); + return forward; + } + + private void collectReachable(String start, Set removed, Set seen, boolean reverse) { + Deque work = new ArrayDeque<>(); + seen.add(start); + work.push(start); + while (!work.isEmpty()) { + String node = work.pop(); + Collection targets = reverse + ? reverseGraph.getOrDefault(node, Collections.emptySet()) + : dependencyGraph.getOrDefault(node, Collections.emptyMap()).keySet(); + for (String next : targets) { + if (!removed.contains(next) && seen.add(next)) { + work.push(next); } } } - recursionStack.remove(node); } - - private CycleInfo extractCycle(String current, String cycleStart, DependencyEdge finalEdge) { - LinkedList path = new LinkedList<>(); - path.addFirst(current); - String node = current; - while (!node.equals(cycleStart)) { - node = parent.get(node); - path.addFirst(node); + + private boolean circuit(String node, String start, Set component) { + boolean foundCycle = false; + path.addLast(node); + blocked.add(node); + for (String next : dependencyGraph.get(node).keySet()) { + if (!component.contains(next) || limitReached) { + continue; + } + if (next.equals(start)) { + recordCycle(); + foundCycle = true; + } else if (!blocked.contains(next) && circuit(next, start, component)) { + foundCycle = true; + } } - List cycleList = new ArrayList<>(path); - cycleList.add(cycleStart); - CycleInfo cycleInfo = new CycleInfo(cycleList); - for (int i = 0; i < path.size() - 1; i++) { - String from = path.get(i); - String to = path.get(i + 1); - DependencyEdge edge = parentEdge.get(to); - cycleInfo.addEdge(from, to, edge.type); + if (foundCycle) { + unblock(node); + } else { + for (String next : dependencyGraph.get(node).keySet()) { + if (component.contains(next)) { + blockedBy.computeIfAbsent(next, k -> new HashSet<>()).add(node); + } + } } - cycleInfo.addEdge(current, cycleStart, finalEdge.type); - return cycleInfo; + path.removeLast(); + return foundCycle; } - - private boolean isDuplicateCycle(CycleInfo newCycleInfo) { - List normalized = normalizeCycle(newCycleInfo.cycle); - for (CycleInfo existingCycleInfo : cycles) { - List normalizedExisting = normalizeCycle(existingCycleInfo.cycle); - if (normalized.equals(normalizedExisting)) return true; + + private void unblock(String node) { + blocked.remove(node); + Set dependents = blockedBy.remove(node); + if (dependents != null) { + for (String dependent : dependents) { + if (blocked.contains(dependent)) { + unblock(dependent); + } + } } - return false; } - - private List normalizeCycle(List cycle) { - if (cycle.size() <= 1) return new ArrayList<>(cycle); - List temp = new ArrayList<>(cycle.subList(0, cycle.size() - 1)); - int minIndex = 0; - for (int i = 1; i < temp.size(); i++) { - if (temp.get(i).compareTo(temp.get(minIndex)) < 0) minIndex = i; + + private void recordCycle() { + List cycleList = new ArrayList<>(path); + cycleList.add(cycleList.get(0)); + CycleInfo cycleInfo = new CycleInfo(cycleList); + for (int i = 0; i < cycleList.size() - 1; i++) { + String from = cycleList.get(i); + String to = cycleList.get(i + 1); + cycleInfo.addEdge(from, to, dependencyGraph.get(from).get(to)); } - List normalized = new ArrayList<>(); - for (int i = 0; i < temp.size(); i++) { - normalized.add(temp.get((minIndex + i) % temp.size())); + addCycle(cycleInfo); + } + + private void addCycle(CycleInfo cycleInfo) { + if (cycles.size() >= MAX_CYCLES) { + limitReached = true; + return; } - normalized.add(normalized.get(0)); - return normalized; + cycles.add(cycleInfo); } } } \ No newline at end of file From 098818b004971add0871b84e8b33e9beb39fb8d9 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 12 Jun 2026 19:35:08 +0200 Subject: [PATCH 2/2] Avoid recording self-cycles twice in cycle detection A plug-in with a dependency on itself that is also part of a larger strongly connected component was reported both by the self-loop check in findAllCycles() and by the trivial path in circuit(). Skip the trivial path so each self-cycle is only recorded once. --- .../handlers/DetectCyclicDependenciesHandler.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java b/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java index b145890..80cffcd 100644 --- a/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java +++ b/com.vogella.ide.debugtools/src/com/vogella/ide/debugtools/handlers/DetectCyclicDependenciesHandler.java @@ -343,8 +343,11 @@ private boolean circuit(String node, String start, Set component) { continue; } if (next.equals(start)) { - recordCycle(); - foundCycle = true; + // path.size() == 1 is the self-loop, already recorded in findAllCycles() + if (path.size() > 1) { + recordCycle(); + foundCycle = true; + } } else if (!blocked.contains(next) && circuit(next, start, component)) { foundCycle = true; }