Do not follow generic requirements when computing the classpath

OSGi uses a generic Provide-Capability/Require-Capability model that
allows to declare requirements beyond the classic Import-Package and
Require-Bunlde. A common example is a bundle that provides a DS
component and needs an extender to actually construct the component and
inject the services. Another is a logging API that needs a provider.

These are especially needed because in such scenario a provider/consumer
do not know each other and should not depend directly (only loosely
coupled). Currently such providers are still pulled in even though they
are not contribute to the compilation classpath as they are completely
invisible at runtime - there is a wiring but no classloader access
granted.

This now only follows build time requirements to avoid the situation.

Author: laeubi

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/MinimalState.java

@@ -271,7 +271,7 @@ public StateDelta resolveState(String[] symbolicNames) {
 		// now we need to check all bundles if there is any fragment. If a
 		// fragment should be resolved its host and all already attached
 		// fragments must be re-resolved again
-		for (BundleDescription bundleDescription : bundles) {
+		for (BundleDescription bundleDescription : bundles.toArray(BundleDescription[]::new)) {
 			HostSpecification host = bundleDescription.getHost();
 			if (host != null) {
 				BundleRequirement requirement = host.getRequirement();

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/RequiredPluginsClasspathContainer.java

@@ -683,8 +683,7 @@ private void addImplicitDependencies(BundleDescription desc, Set<BundleDescripti
 	 */
 	private void addTransitiveDependenciesWithForbiddenAccess(Set<BundleDescription> added,
 			List<IClasspathEntry> entries) throws CoreException {
-		Set<BundleDescription> closure = DependencyManager.findRequirementsClosure(added,
-				INCLUDE_OPTIONAL_DEPENDENCIES);
+		Collection<BundleDescription> closure = ClasspathComputer.collectBuildRelevantDependencies(added);
 		String systemBundleBSN = TargetPlatformHelper.getPDEState().getSystemBundle();
 		Iterator<BundleDescription> transitiveDeps = closure.stream()
 				.filter(desc -> !desc.getSymbolicName().equals(systemBundleBSN))

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/classpath/ClasspathResolutionTest2.java

@@ -29,6 +29,8 @@
 import org.eclipse.jdt.core.IClasspathEntry;
 import org.eclipse.jdt.core.IJavaModelMarker;
 import org.eclipse.jdt.core.compiler.IProblem;
+import org.eclipse.osgi.service.resolver.BundleDescription;
+import org.eclipse.osgi.service.resolver.ResolverError;
 import org.eclipse.pde.core.plugin.IPluginModelBase;
 import org.eclipse.pde.internal.core.ClasspathComputer;
 import org.eclipse.pde.internal.core.PDECore;
@@ -180,8 +182,12 @@ public class ClasspathResolutionTest2 {
 	 */
 	static final List<String> TRANSITIVE_FORBIDDEN_BUNDLES = List.of("C", "D", "E", "F", "H");
 
-	/** All test bundles other than A. */
-	static final List<String> ALL_TEST_BUNDLES = List.of("B", "C", "D", "E", "F", "G", "H");
+	static final List<String> ALL_TEST_BUNDLES = List.of("B", "C", "D", "E", "F", "G", "H", "cyclic-capabilities/Af",
+			"cyclic-capabilities/cyclic.capabilities.provider");
+
+	private static IProject projectConsumer;
+
+	private static IClasspathEntry[] consumerClasspathEntries;
 
 	@BeforeClass
 	public static void setupBeforeClass() throws Exception {
@@ -195,6 +201,8 @@ public static void setupBeforeClass() throws Exception {
 		projectA = ProjectUtils.importTestProject("tests/projects/A");
 		projectAe = ProjectUtils.importTestProject("tests/projects/Ae");
 		projectAd = ProjectUtils.importTestProject("tests/projects/Ad");
+		projectConsumer = ProjectUtils
+				.importTestProject("tests/projects/cyclic-capabilities/cyclic.capabilities.consumer");
 
 		// Trigger a full workspace build. The workspace builder respects
 		// project build order, so dependency bundles (B-H, X) are built
@@ -214,6 +222,7 @@ public static void setupBeforeClass() throws Exception {
 		projectA.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor());
 		projectAe.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor());
 		projectAd.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor());
+		projectConsumer.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor());
 		TestUtils.waitForJobs("ClasspathResolutionTest2.rebuild", 200, 30_000);
 
 		// Compute PDE classpath entries (only plugin dependencies, no
@@ -225,6 +234,10 @@ public static void setupBeforeClass() throws Exception {
 		IPluginModelBase modelAd = PDECore.getDefault().getModelManager().findModel(projectAd);
 		assertNotNull("PDE model for project Ad must be available", modelAd);
 		classpathEntriesAd = ClasspathComputer.computeClasspathEntries(modelAd, projectAd);
+
+		IPluginModelBase consumerModel = PDECore.getDefault().getModelManager().findModel(projectConsumer);
+		consumerClasspathEntries = ClasspathComputer.computeClasspathEntries(consumerModel,
+				projectConsumer);
 	}
 
 	// =========================================================================
@@ -566,9 +579,9 @@ record ForbiddenRef(int line, String qualifiedType, String project) {
 				// A has forbiddenReference=warning → WARNING
 				// Ae has forbiddenReference=error → ERROR
 				assertThat(m.getAttribute(IMarker.SEVERITY, -1))
-						.as("Line %d in project %s: must be %s " + "severity (forbiddenReference=%s in "
-								+ "project settings)",
-								ref.line, project.getName(), severityLabel, severityLabel.toLowerCase())
+				.as("Line %d in project %s: must be %s " + "severity (forbiddenReference=%s in "
+						+ "project settings)",
+						ref.line, project.getName(), severityLabel, severityLabel.toLowerCase())
 				.isEqualTo(expectedSeverity);
 
 				// Problem ID must be ForbiddenReference because all
@@ -675,8 +688,8 @@ public void testDiscouragedMarkersOnProjectAd() throws Exception {
 			.as("Discouraged marker must be on line 44 " + "(x.internal.MyObject)").isEqualTo(44);
 
 			assertThat(m.getAttribute(IMarker.SEVERITY, -1))
-					.as("Discouraged access must be WARNING " + "(discouragedReference=warning in "
-							+ "project settings)")
+			.as("Discouraged access must be WARNING " + "(discouragedReference=warning in "
+					+ "project settings)")
 			.isEqualTo(IMarker.SEVERITY_WARNING);
 
 			// DiscouragedReference vs ForbiddenReference: K_DISCOURAGED
@@ -714,7 +727,13 @@ public void testDiscouragedMarkersOnProjectAd() throws Exception {
 	 */
 	@Test
 	public void testDependencyBundlesBuildClean() throws Exception {
-		for (String bundleName : ALL_TEST_BUNDLES) {
+		for (String projectPath : ALL_TEST_BUNDLES) {
+			String bundleName;
+			if (projectPath.contains("/")) {
+				bundleName = projectPath.split("/")[1];
+			} else {
+				bundleName = projectPath;
+			}
 			IProject project = getProject(bundleName);
 			IMarker[] markers = project.findMarkers(IMarker.PROBLEM, true, IResource.DEPTH_INFINITE);
 
@@ -746,6 +765,40 @@ public void testClasspathComputationIsDeterministic() throws Exception {
 		.containsExactlyInAnyOrderElementsOf(originalNames);
 	}
 
+	// =========================================================================
+	// Section 7: Cyclic capability resolution for fragments
+	// =========================================================================
+
+	/**
+	 * A fragment that uses {@code Require-Capability} to depend on a bundle
+	 * that itself {@code Require-Bundle}s the fragment's host must not end up
+	 * with the capability provider on its classpath — that would create a
+	 * cyclic dependency. Only the host bundle ({@code Af}) should appear.
+	 * <p>
+	 * <b>Test bundle dependency graph:</b>
+	 * <pre>
+	 * Af:                          simple host bundle
+	 * cyclic.capabilities.provider: Provide-Capability: some.test.capability
+	 *                               Require-Bundle: Af
+	 * cyclic.capabilities.consumer: Require-Capability: some.test.capability
+	 *                               Fragment-Host: Af
+	 * </pre>
+	 */
+	@Test
+	public void testRequiredPluginsViaCapabilityForFragment() throws Exception {
+		List<String> entryNames = Arrays.stream(consumerClasspathEntries)
+				.map(e -> e.getPath().lastSegment()).toList();
+		IPluginModelBase consumerModel = PDECore.getDefault().getModelManager().findModel(projectConsumer);
+		assertThat(consumerModel).isNotNull();
+		BundleDescription bundleDescription = consumerModel.getBundleDescription();
+		ResolverError[] resolverErrors = bundleDescription.getContainingState().getResolverErrors(bundleDescription);
+		assertThat(resolverErrors).isEmpty();
+		assertThat(entryNames).as("Fragment host Af must be on the consumer's classpath").contains("Af");
+		assertThat(entryNames)
+		.as("cyclic.capabilities.provider must NOT be on the classpath (would create cycle)")
+		.noneMatch(name -> name.contains("cyclic.capabilities.provider"));
+	}
+
 	// =========================================================================
 	// Utility methods
 	// =========================================================================

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/.classpath

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17"/>
+	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
+	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="output" path="bin"/>
+</classpath>

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/.project

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>Af</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.eclipse.jdt.core.javabuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.ManifestBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.SchemaBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.pde.PluginNature</nature>
+		<nature>org.eclipse.jdt.core.javanature</nature>
+	</natures>
+</projectDescription>

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/.settings/org.eclipse.core.resources.prefs

@@ -0,0 +1,2 @@
+eclipse.preferences.version=1
+encoding/<project>=UTF-8

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/.settings/org.eclipse.jdt.core.prefs

@@ -0,0 +1,9 @@
+eclipse.preferences.version=1
+org.eclipse.jdt.core.compiler.codegen.targetPlatform=17
+org.eclipse.jdt.core.compiler.compliance=17
+org.eclipse.jdt.core.compiler.problem.assertIdentifier=error
+org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=disabled
+org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
+org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=warning
+org.eclipse.jdt.core.compiler.release=enabled
+org.eclipse.jdt.core.compiler.source=17

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/META-INF/MANIFEST.MF

@@ -0,0 +1,8 @@
+Manifest-Version: 1.0
+Bundle-ManifestVersion: 2
+Bundle-Name: Af
+Bundle-SymbolicName: Af
+Bundle-Version: 1.0.0.qualifier
+Export-Package: a.f
+Automatic-Module-Name: Af
+Bundle-RequiredExecutionEnvironment: JavaSE-17

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/build.properties

@@ -0,0 +1,4 @@
+source.. = src/
+output.. = bin/
+bin.includes = META-INF/,\
+               .

ui/org.eclipse.pde.ui.tests/tests/projects/cyclic-capabilities/Af/src/a/f/ServiceInterface.java

@@ -0,0 +1,5 @@
+package a.f;
+
+public interface ServiceInterface {
+
+}