Bug 578681 - DependencyManager: Ignore requirements of ignored fragments

Change-Id: Idf258dc1c2c076966494dd03172b885e3b43da8f
Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
Reviewed-on: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/190688
Tested-by: PDE Bot <pde-bot@eclipse.org>
diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java
index 971cd1d..7d9d63c 100644
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java
@@ -169,7 +169,7 @@
 		}
 
 		Set<BundleDescription> closure = new HashSet<>(bundles.size() * 4 / 3 + 1);
-		Queue<BundleDescription> pending = new ArrayDeque<>();
+		Queue<BundleDescription> pending = new ArrayDeque<>(bundles.size());
 
 		// initialize with given bundles
 		for (BundleDescription bundle : bundles) {
@@ -185,15 +185,6 @@
 				continue;
 			}
 
-			List<BundleWire> requiredWires = wiring.getRequiredWires(null);
-			for (BundleWire wire : requiredWires) {
-				BundleRevision provider = getProvider(wire);
-				if (provider instanceof BundleDescription && (includeOptional || !isOptional(wire.getRequirement()))) {
-					BundleDescription requiredBundle = (BundleDescription) provider;
-					addNewRequiredBundle(requiredBundle, closure, pending);
-				}
-			}
-
 			if (includeAllFragments || includeNonTestFragments) {
 				// A fragment's host is already required by a wire
 				for (BundleDescription fragment : bundle.getFragments()) {
@@ -202,6 +193,21 @@
 					}
 				}
 			}
+
+			List<BundleWire> requiredWires = wiring.getRequiredWires(null);
+			for (BundleWire wire : requiredWires) {
+				BundleRevision declaringBundle = wire.getRequirement().getRevision();
+				if (declaringBundle != bundle && !closure.contains(declaringBundle)) {
+					// Requirement is declared by an attached fragment, which is
+					// not included into the closure.
+					continue;
+				}
+				BundleRevision provider = getProvider(wire);
+				if (provider instanceof BundleDescription && (includeOptional || !isOptional(wire.getRequirement()))) {
+					BundleDescription requiredBundle = (BundleDescription) provider;
+					addNewRequiredBundle(requiredBundle, closure, pending);
+				}
+			}
 		}
 		return closure;
 	}
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java
index 696a3d9..a0f64dc 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java
@@ -182,10 +182,26 @@
 						entry(EXPORT_PACKAGE, "bundle.a.pack" + version("1.0.0"))),
 
 				bundle("bundle.fragment", "1.0.0", //
-						entry(FRAGMENT_HOST, "bundle.a")));
+						entry(FRAGMENT_HOST, "bundle.a")),
+
+				bundle("bundle.fragment.with.dependencies", "1.0.0", //
+						entry(FRAGMENT_HOST, "bundle.a"), //
+						entry(REQUIRE_BUNDLE, "bundle.b"), //
+						entry(IMPORT_PACKAGE, "bundle.c.pack")),
+
+				bundle("bundle.b", "1.0.0"), //
+
+				bundle("bundle.c", "1.0.0", //
+						entry(EXPORT_PACKAGE, "bundle.c.pack")),
+
+				bundle("bundle.d", "1.0.0", //
+						entry(EXPORT_PACKAGE, "bundle.d.pack")));
 
 		BundleDescription bundleA = bundleDescription("bundle.a", "1.0.0");
+		BundleDescription bundleB = bundleDescription("bundle.b", "1.0.0");
+		BundleDescription bundleC = bundleDescription("bundle.c", "1.0.0");
 		BundleDescription bundleFragment = bundleDescription("bundle.fragment", "1.0.0");
+		BundleDescription bundleFragmentWithDeps = bundleDescription("bundle.fragment.with.dependencies", "1.0.0");
 
 		Set<BundleDescription> bundles = Set.of(bundleA);
 
@@ -193,7 +209,8 @@
 		assertThat(noFragmentsClosure).isEqualTo(Set.of(bundleA));
 
 		Set<BundleDescription> allFragmentsClosure = findRequirementsClosure(bundles, INCLUDE_ALL_FRAGMENTS);
-		assertThat(allFragmentsClosure).isEqualTo(Set.of(bundleA, bundleFragment));
+		assertThat(allFragmentsClosure)
+				.isEqualTo(Set.of(bundleA, bundleFragment, bundleFragmentWithDeps, bundleB, bundleC));
 	}
 
 	@Test
@@ -228,7 +245,7 @@
 
 		Set<BundleDescription> nonTestFragmentsClosure = findRequirementsClosure(bundles, INCLUDE_NON_TEST_FRAGMENTS);
 		assertThat(nonTestFragmentsClosure)
-				.isEqualTo(Set.of(bundleA, bundleFragment, otherFragmentWithTestName, testFragmentWithoutTestAttr));
+		.isEqualTo(Set.of(bundleA, bundleFragment, otherFragmentWithTestName, testFragmentWithoutTestAttr));
 	}
 
 	@Test