From 730ea18be65aeb5c3a32373f443831ceae553a1f Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 28 Jul 2026 19:55:58 +0200 Subject: [PATCH] Detect dependency cycles formed through Import-Package The imports of a plug-in read from the state were computed while the model was loaded, which happens before the state is resolved: PDEState creates the models in its constructor and PluginModelManager resolves the state afterwards, once the workspace bundles have been added. At that point BundleDescription.getResolvedImports() is still empty, so the bundles behind the Import-Package header were dropped and the empty result was kept for the lifetime of the model. getRequiredBundles() does not need a resolved state, which is why only Require-Bundle edges survived. Compute those imports on demand instead. A result computed from an unresolved state is provisional and recomputed once the state has been resolved, after which it is kept, so early callers still see the previous result. Only models read from a BundleDescription are affected, and those are the read-only target models, so a model being edited cannot lose its imports. As a result the MANIFEST.MF editor's "Look for cycles in the dependency graph" action reports cycles that close through package wiring instead of silently ignoring them. --- .../pde/internal/core/plugin/PluginBase.java | 39 ++++++++++++- .../builders/DependencyLoopFinderTest.java | 55 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java index dae75c55be1..0d5dbbfcf4a 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginBase.java @@ -21,11 +21,13 @@ import org.eclipse.osgi.service.resolver.BundleDescription; import org.eclipse.osgi.service.resolver.BundleSpecification; import org.eclipse.osgi.service.resolver.ExportPackageDescription; +import org.eclipse.osgi.service.resolver.State; import org.eclipse.pde.core.IModelChangedEvent; import org.eclipse.pde.core.plugin.IMatchRules; import org.eclipse.pde.core.plugin.IPluginBase; import org.eclipse.pde.core.plugin.IPluginImport; import org.eclipse.pde.core.plugin.IPluginLibrary; +import org.eclipse.pde.core.plugin.IPluginModelBase; import org.eclipse.pde.internal.core.ICoreConstants; import org.eclipse.pde.internal.core.PDECoreMessages; import org.eclipse.pde.internal.core.PDEState; @@ -48,6 +50,9 @@ public abstract class PluginBase extends AbstractExtensions implements IPluginBa private boolean fHasBundleStructure; private String fBundleSourceEntry; private boolean fExportsExternalAnnotations; + private boolean fImportsFromState; + private boolean fImportsLoaded; + private boolean fImportsResolved; public PluginBase(boolean readOnly) { super(readOnly); @@ -90,9 +95,38 @@ public IPluginLibrary[] getLibraries() { @Override public IPluginImport[] getImports() { + ensureImportsLoaded(); return fImports.toArray(new IPluginImport[fImports.size()]); } + /** + * Computes the imports of a plug-in read from a {@link BundleDescription} + * on demand. The bundles behind the Import-Package header are only known + * once the state is resolved, which happens after the models have been + * created, so reading them while loading the model would silently drop + * them. A result computed from an unresolved state is provisional and + * recomputed once the state has been resolved; after that it is kept. + */ + private synchronized void ensureImportsLoaded() { + if (!fImportsFromState || fImportsResolved) { + return; + } + IPluginModelBase model = getPluginModel(); + BundleDescription description = model != null ? model.getBundleDescription() : null; + if (description == null) { + return; + } + State state = description.getContainingState(); + boolean resolved = state == null || state.isResolved(); + if (fImportsLoaded && !resolved) { + return; + } + fImports = new ArrayList<>(); + loadImports(description); + fImportsLoaded = true; + fImportsResolved = resolved; + } + @Override public IPluginBase getPluginBase() { return this; @@ -122,7 +156,7 @@ void load(BundleDescription bundleDesc, PDEState state) { fBundleSourceEntry = state.getBundleSourceEntry(bundleDesc.getBundleId()); fExportsExternalAnnotations = state.exportsExternalAnnotations(bundleDesc.getBundleId()); loadRuntime(bundleDesc, state); - loadImports(bundleDesc); + fImportsFromState = true; } @Override @@ -296,6 +330,9 @@ public void remove(IPluginImport[] iimports) throws CoreException { public void reset() { fLibraries = new ArrayList<>(); fImports = new ArrayList<>(); + fImportsFromState = false; + fImportsLoaded = false; + fImportsResolved = false; fProviderName = null; fSchemaVersion = null; fVersion = ""; //$NON-NLS-1$ diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/core/builders/DependencyLoopFinderTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/core/builders/DependencyLoopFinderTest.java index 810878cf8df..93edd380dea 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/core/builders/DependencyLoopFinderTest.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/core/builders/DependencyLoopFinderTest.java @@ -15,8 +15,11 @@ import static java.util.Map.entry; import static org.eclipse.pde.ui.tests.util.TargetPlatformUtil.bundle; +import static org.eclipse.pde.ui.tests.util.TargetPlatformUtil.version; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.osgi.framework.Constants.EXPORT_PACKAGE; +import static org.osgi.framework.Constants.IMPORT_PACKAGE; import static org.osgi.framework.Constants.REQUIRE_BUNDLE; import java.io.IOException; @@ -115,6 +118,58 @@ public void testTwoSeparateCyclesThroughRoot() throws Exception { assertEquals(List.of("loop.r -> loop.a", "loop.r -> loop.b"), loopSignatures("loop.r")); } + /** + * Bundles wired to each other through Export-Package/Import-Package form a + * cycle just like bundles wired through Require-Bundle. + */ + @Test + public void testImportPackageCycle() throws Exception { + // a and b depend on each other only via package wiring (no Require-Bundle) + setTargetPlatform( // + bundle("loop.a", "1.0.0", // + entry(EXPORT_PACKAGE, "loop.a.pack"), // + entry(IMPORT_PACKAGE, "loop.b.pack")), // + bundle("loop.b", "1.0.0", // + entry(EXPORT_PACKAGE, "loop.b.pack"), // + entry(IMPORT_PACKAGE, "loop.a.pack"))); + + assertEquals(List.of("loop.a -> loop.b"), loopSignatures("loop.a")); + } + + /** + * A cycle that closes through a mix of both dependency kinds is reported + * too: {@code a} requires {@code b}, and {@code b} imports a package that + * {@code a} exports. + */ + @Test + public void testCycleMixingRequireBundleAndImportPackage() throws Exception { + setTargetPlatform( // + bundle("loop.a", "1.0.0", // + entry(REQUIRE_BUNDLE, "loop.b"), // + entry(EXPORT_PACKAGE, "loop.a.pack")), // + bundle("loop.b", "1.0.0", entry(IMPORT_PACKAGE, "loop.a.pack"))); + + assertEquals(List.of("loop.a -> loop.b"), loopSignatures("loop.a")); + } + + /** + * A package import creates a dependency only to the exporter it is wired + * to, not to every bundle exporting that package. + */ + @Test + public void testImportPackageWiredElsewhereIsNoCycle() throws Exception { + // a is wired to b's 2.0.0 export; c exports the same package at 1.0.0 + // and requires a, so an edge a -> c would close a cycle + setTargetPlatform( // + bundle("loop.a", "1.0.0", entry(IMPORT_PACKAGE, "loop.shared.pack" + version("2.0.0"))), // + bundle("loop.b", "1.0.0", entry(EXPORT_PACKAGE, "loop.shared.pack" + version("2.0.0"))), // + bundle("loop.c", "1.0.0", // + entry(EXPORT_PACKAGE, "loop.shared.pack" + version("1.0.0")), // + entry(REQUIRE_BUNDLE, "loop.a"))); + + assertEquals(List.of(), loopSignatures("loop.a")); + } + /** * A cycle that is only reachable through a second dependency of the root * must be reported too.