Skip to content

Report all dependency cycles through the edited plug-in #2406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vogella merged 1 commit into from
Aug 6, 2026
Merged

Report all dependency cycles through the edited plug-in #2406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 123 additions & 78 deletions ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/builders/DependencyLoopFinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2012 IBM Corporation and others.
* Copyright (c) 2000, 2026 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -13,7 +13,15 @@
*******************************************************************************/
package org.eclipse.pde.internal.core.builders;

import java.util.Vector;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.eclipse.osgi.util.NLS;
import org.eclipse.pde.core.plugin.IPlugin;
Expand All @@ -25,6 +33,13 @@

public class DependencyLoopFinder {

/**
* Upper bound on the number of reported loops. The search runs
* synchronously on the UI thread and the number of distinct cycles through
* a plug-in can grow exponentially with the size of the tangle it sits in.
*/
private static final int MAX_LOOPS = 100;

public static DependencyLoop[] findLoops(IPlugin root) {
return findLoops(root, null);
}
Expand All @@ -34,94 +49,128 @@ public static DependencyLoop[] findLoops(IPlugin root, IPlugin[] candidates) {
}

public static DependencyLoop[] findLoops(IPlugin root, IPlugin[] candidates, boolean onlyCandidates) {
Vector<DependencyLoop> loops = new Vector<>();

Vector<IPlugin> path = new Vector<>();
findLoops(loops, path, root, candidates, onlyCandidates, new Vector<>());
return loops.toArray(new DependencyLoop[loops.size()]);
LoopSearch search = new LoopSearch(root.getId());
List<IPlugin> rootDependencies = new ArrayList<>();
if (!onlyCandidates) {
rootDependencies.addAll(search.dependenciesOf(root));
}
if (candidates != null) {
rootDependencies.addAll(Arrays.asList(candidates));
}
search.restrictToLoopMembers(rootDependencies);
search.collectLoops(root, rootDependencies);
return search.loops();
}

private static void findLoops(Vector<DependencyLoop> loops, Vector<IPlugin> path, IPlugin subroot, IPlugin[] candidates, boolean onlyCandidates, Vector<String> exploredPlugins) {
if (!path.isEmpty()) {
// test the path so far
// is the subroot the same as root - if yes, that's it

IPlugin root = path.elementAt(0);
if (isEquivalent(root, subroot)) {
// our loop!!
DependencyLoop loop = new DependencyLoop();
loop.setMembers(path.toArray(new IPlugin[path.size()]));
int no = loops.size() + 1;
loop.setName(NLS.bind(PDECoreMessages.Builders_DependencyLoopFinder_loopName, ("" + no))); //$NON-NLS-1$
loops.add(loop);
return;
/**
* Enumerates the cycles that pass through one root plug-in.
*/
private static final class LoopSearch {

private final String rootId;
private final List<DependencyLoop> loops = new ArrayList<>();
private final List<IPlugin> path = new ArrayList<>();
private final Map<String, List<IPlugin>> dependencies = new HashMap<>();
private Set<String> loopMembers = Set.of();

LoopSearch(String rootId) {
this.rootId = rootId;
}

/**
* Narrows the search to the plug-ins that can actually sit on a cycle
* through the root, that is those reachable from the root that also
* lead back to it. Unlike a "this plug-in yielded no loop" blacklist,
* this property does not depend on the path taken to reach a plug-in,
* so pruning by it cannot hide a cycle.
*/
void restrictToLoopMembers(List<IPlugin> rootDependencies) {
Map<String, List<String>> dependents = new HashMap<>();
Set<String> reached = new HashSet<>();
Deque<IPlugin> pending = new ArrayDeque<>();
for (IPlugin dependency : rootDependencies) {
dependents.computeIfAbsent(dependency.getId(), id -> new ArrayList<>()).add(rootId);
if (reached.add(dependency.getId())) {
pending.add(dependency);
}
}
// is the subroot the same as any other node?
// if yes, abort - local loop that is not ours
for (int i = 1; i < path.size(); i++) {
IPlugin node = path.elementAt(i);
if (isEquivalent(subroot, node)) {
// local loop
return;
while (!pending.isEmpty()) {
IPlugin plugin = pending.remove();
for (IPlugin dependency : dependenciesOf(plugin)) {
dependents.computeIfAbsent(dependency.getId(), id -> new ArrayList<>()).add(plugin.getId());
if (reached.add(dependency.getId())) {
pending.add(dependency);
}
}
}
// walking the collected edges backwards from the root yields the
// plug-ins that lead back to it
loopMembers = new HashSet<>();
Deque<String> backwards = new ArrayDeque<>();
backwards.add(rootId);
while (!backwards.isEmpty()) {
for (String dependent : dependents.getOrDefault(backwards.remove(), List.of())) {
if (!dependent.equals(rootId) && loopMembers.add(dependent)) {
backwards.add(dependent);
}
}
}
}
@SuppressWarnings("unchecked")
Vector<IPlugin> newPath = !path.isEmpty() ? ((Vector<IPlugin>) path.clone()) : path;
newPath.add(subroot);

if (!onlyCandidates) {
IPluginImport[] iimports = subroot.getImports();
for (IPluginImport iimport : iimports) {
String id = iimport.getId();
//Be paranoid
if (id == null) {
continue;
void collectLoops(IPlugin plugin, List<IPlugin> pluginDependencies) {
path.add(plugin);
for (IPlugin dependency : pluginDependencies) {
if (loops.size() >= MAX_LOOPS) {
break;
}
if (!exploredPlugins.contains(id)) {
// is plugin in list of non loop yielding plugins
//Commenting linear lookup - was very slow
//when called from here. We will use
//model manager instead because it
//has a hash table lookup that is much faster.
//IPlugin child = PDECore.getDefault().findPlugin(id);
IPlugin child = findPlugin(id);
if (child != null) {
// number of loops before traversing plugin
int oldLoopSize = loops.size();

findLoops(loops, newPath, child, null, false, exploredPlugins);

// number of loops after traversing plugin
int newLoopsSize = loops.size();

if (oldLoopSize == newLoopsSize) {// no change in number of loops
// no loops from going to this node, skip next time
exploredPlugins.add(id);
}
}
String id = dependency.getId();
if (rootId.equals(id)) {
addLoop();
} else if (loopMembers.contains(id) && !isOnPath(id)) {
collectLoops(dependency, dependenciesOf(dependency));
}

}

path.remove(path.size() - 1);
}
if (candidates != null) {
for (IPlugin candidate : candidates) {
// number of loops before traversing plugin
int oldLoopSize = loops.size();

findLoops(loops, newPath, candidate, null, false, exploredPlugins);
/**
* Returns the plug-ins required by the given one, resolved through the
* registry. Cached, as the search visits a plug-in once per path
* leading to it.
*/
List<IPlugin> dependenciesOf(IPlugin plugin) {
return dependencies.computeIfAbsent(plugin.getId(), id -> {
List<IPlugin> resolved = new ArrayList<>();
for (IPluginImport iimport : plugin.getImports()) {
String importedId = iimport.getId();
//Be paranoid
if (importedId == null) {
continue;
}
IPlugin imported = findPlugin(importedId);
if (imported != null) {
resolved.add(imported);
}
}
return resolved;
});
}

// number of loops after traversing plugin
int newLoopsSize = loops.size();
private boolean isOnPath(String id) {
return path.stream().anyMatch(plugin -> id.equals(plugin.getId()));
}

if (oldLoopSize == newLoopsSize) { // no change in number of loops
// no loops from going to this node, skip next time
exploredPlugins.add(candidate.getId());
}
}
private void addLoop() {
DependencyLoop loop = new DependencyLoop();
loop.setMembers(path.toArray(new IPlugin[path.size()]));
int no = loops.size() + 1;
loop.setName(NLS.bind(PDECoreMessages.Builders_DependencyLoopFinder_loopName, ("" + no))); //$NON-NLS-1$
loops.add(loop);
}

DependencyLoop[] loops() {
return loops.toArray(new DependencyLoop[loops.size()]);
}
}

private static IPlugin findPlugin(String id) {
Expand All @@ -131,8 +180,4 @@ private static IPlugin findPlugin(String id) {
}
return (IPlugin) childModel.getPluginBase();
}

private static boolean isEquivalent(IPlugin left, IPlugin right) {
return left.getId().equals(right.getId());
}
}
28 changes: 28 additions & 0 deletions ...tests/src/org/eclipse/pde/core/tests/internal/core/builders/DependencyLoopFinderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,34 @@ public void testTwoSeparateCyclesThroughRoot() throws Exception {
assertEquals(List.of("loop.r -> loop.a", "loop.r -> loop.b"), loopSignatures("loop.r"));
}

/**
* A cycle that is only reachable through a second dependency of the root
* must be reported too.
*
* <pre>
* r -> a, r -> d
* a -> b, a -> r
* b -> a
* d -> b
* </pre>
*
* Two cycles pass through {@code r}: {@code r -> a -> r} and
* {@code r -> d -> b -> a -> r}. Reaching {@code b} from {@code a} ends in
* a cycle that does not touch {@code r}, which must not stop the search
* from reaching {@code b} again through {@code d}.
*/
@Test
public void testCycleReachableOnlyViaSecondImportPathIsFound() throws Exception {
setTargetPlatform( //
bundle("loop.r", "1.0.0", entry(REQUIRE_BUNDLE, "loop.a,loop.d")), //
bundle("loop.a", "1.0.0", entry(REQUIRE_BUNDLE, "loop.b,loop.r")), //
bundle("loop.b", "1.0.0", entry(REQUIRE_BUNDLE, "loop.a")), //
bundle("loop.d", "1.0.0", entry(REQUIRE_BUNDLE, "loop.b")));

assertEquals(List.of("loop.r -> loop.a", "loop.r -> loop.d -> loop.b -> loop.a"),
loopSignatures("loop.r"));
}

/**
* The reported cycles must not depend on the order in which the root
* declares its dependencies.
Expand Down
Loading