Report all dependency cycles through the edited plug-in - #2406
Merged
Conversation
vogella
force-pushed
the
dependency-loop-finder-fix
branch
2 times, most recently
from
August 5, 2026 13:29
e56238d to
7a87906
Compare
DependencyLoopFinder kept a shared list of plug-ins that had been visited without yielding a loop and skipped them on every later path. Whether a plug-in yields a loop depends on the path taken to reach it: a branch that ends in a cycle not passing through the root adds the plug-ins it visited to that list, so a cycle reachable only through another dependency of the root is never reported. Which cycles get lost depends on the order of the Require-Bundle entries. Replace the list with a prune that does not depend on the path: only plug-ins that are reachable from the root and lead back to it can sit on a cycle through the root, and that property is a plain graph reachability question. Plug-ins outside that set are skipped, which also keeps the common case of a plug-in without any cycle cheap, and the remaining search enumerates the cycles without dropping any. The number of reported loops is capped, as the search runs on the UI thread. Resolved dependencies are cached for the duration of one search, since the search visits a plug-in once per path leading to it.
vogella
force-pushed
the
dependency-loop-finder-fix
branch
from
August 6, 2026 04:40
7a87906 to
916ecb9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DependencyLoopFinder kept a shared list of plug-ins that had been visited without yielding a loop and skipped them on every later path. Whether a plug-in yields a loop depends on the path taken to reach it, so a branch ending in a cycle that does not pass through the root would blacklist the plug-ins it visited, and a cycle reachable only through another dependency of the root was never reported. Which cycles got lost depended on the order of the Require-Bundle entries, which matches the reports of the action missing cycles.
The list is replaced by a prune that does not depend on the path: only plug-ins that are reachable from the root and lead back to it can sit on a cycle through it. That is plain graph reachability, so pruning by it cannot hide a cycle, and it keeps the common case of a plug-in without any cycle cheap, which matters because the search runs on the UI thread. The number of reported loops is capped for the same reason.
The added regression test fails against the current implementation and passes with this change.