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.