Fix: API tool reports fictitious changes to execution environment

The API tool only checks the Bundle-RequiredExecutionEnvironment header when comparing the baseline with the workspace bundle. If the EE of the baseline bundle is described via the Require-Capability header, an empty string is compared to the workspace EE, which is treated as an incompatibility. This also adds a simple test case where the execution environment of two bundles is checked. One bundle uses the Require-Capability, the other one the Bundle-RequiredExecutionEnvironment header. Resolves eclipse-pde#1301
ptziegler · Jul 7, 2024 · be7a44b · be7a44b
1 parent 1b1b92a
commit be7a44b
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 39 deletions.
diff --git a/.../src/org/eclipse/pde/api/tools/builder/tests/compatibility/ProjectTypeContainerTests.java b/.../src/org/eclipse/pde/api/tools/builder/tests/compatibility/ProjectTypeContainerTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2018 IBM Corporation and others.
+ * Copyright (c) 2009, 2024 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -13,6 +13,8 @@
  *******************************************************************************/
 package org.eclipse.pde.api.tools.builder.tests.compatibility;
 
+import static org.junit.Assert.assertArrayEquals;
+
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -66,13 +68,13 @@ protected int getDefaultProblemId() {
 	}
 
 	/**
-	 * Returns the type container associated with the "bundle.a" project in the
+	 * Returns the type container associated with the given project in the
 	 * workspace.
 	 */
-	protected IApiTypeContainer getTypeContainer() throws CoreException {
+	protected IApiTypeContainer getTypeContainer(String projectName) throws CoreException {
 		IApiBaseline baseline = ApiBaselineManager.getManager().getWorkspaceBaseline();
 		assertNotNull("Missing workspace baseline", baseline); //$NON-NLS-1$
-		IApiComponent component = baseline.getApiComponent(getEnv().getProject("bundle.a")); //$NON-NLS-1$
+		IApiComponent component = baseline.getApiComponent(getEnv().getProject(projectName));
 		assertNotNull("Missing API component", component); //$NON-NLS-1$
 		IApiTypeContainer[] containers = component.getApiTypeContainers();
 		assertEquals("Wrong number of API type containers", 1, containers.length); //$NON-NLS-1$
@@ -154,11 +156,26 @@ protected Set<String> collectAllTypeNames() throws CoreException {
 		return names;
 	}
 
+	/**
+	 * Tests whether the execution environment can be extracted from both the
+	 * {@code Bundle-RequiredExecutionEnvironment} and the
+	 * {@code Require-Capability} header.
+	 */
+	public void testExecutionEnvironment() throws CoreException {
+		IApiTypeContainer bundleA = getTypeContainer("bundle.a"); //$NON-NLS-1$
+		assertArrayEquals("Unable to find BREE for bundle using 'Bundle-RequiredExecutionEvironment'", //$NON-NLS-1$
+				new String[] { "J2SE-1.5" }, bundleA.getApiComponent().getExecutionEnvironments()); //$NON-NLS-1$
+
+		IApiTypeContainer bundleB = getTypeContainer("bundle.b"); //$NON-NLS-1$
+		assertArrayEquals("Unable to find BREE for bundle using 'Require-Capability'", //$NON-NLS-1$
+				new String[] { "JavaSE-17" }, bundleB.getApiComponent().getExecutionEnvironments()); //$NON-NLS-1$
+	}
+
 	/**
 	 * Tests all packages are returned.
 	 */
 	public void testPackageNames() throws CoreException {
-		IApiTypeContainer container = getTypeContainer();
+		IApiTypeContainer container = getTypeContainer("bundle.a"); //$NON-NLS-1$
 		assertEquals("Should be a project type container", IApiTypeContainer.FOLDER, container.getContainerType()); //$NON-NLS-1$
 
 		// build expected list
@@ -184,7 +201,7 @@ public void testPackageNames() throws CoreException {
 	 * Test type lookup.
 	 */
 	public void testFindType() throws CoreException {
-		IApiTypeContainer container = getTypeContainer();
+		IApiTypeContainer container = getTypeContainer("bundle.a"); //$NON-NLS-1$
 		IApiTypeRoot root = container.findTypeRoot("a.classes.fields.AddPrivateField"); //$NON-NLS-1$
 		assertNotNull("Unable to find type 'a.classes.fields.AddPrivateField'", root); //$NON-NLS-1$
 		IApiType structure = root.getStructure();
@@ -195,7 +212,7 @@ public void testFindType() throws CoreException {
 	 * Test that type lookup fails for a type that is not in the project.
 	 */
 	public void testMissingType() throws CoreException {
-		IApiTypeContainer container = getTypeContainer();
+		IApiTypeContainer container = getTypeContainer("bundle.a"); //$NON-NLS-1$
 		IApiTypeRoot root = container.findTypeRoot("some.bogus.Type"); //$NON-NLS-1$
 		assertNull("Should not be able to find type 'some.bogus.Type'", root); //$NON-NLS-1$
 	}
@@ -218,7 +235,7 @@ public void visit(String packageName, IApiTypeRoot typeroot) {
 				typeNames.add(typeroot.getTypeName());
 			}
 		};
-		getTypeContainer().accept(visitor);
+		getTypeContainer("bundle.a").accept(visitor); //$NON-NLS-1$
 
 		// validate type names
 		Set<String> set = collectAllTypeNames();

diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.classpath b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.classpath
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
+	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
+	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="output" path="bin"/>
+</classpath>
diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.project b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.project
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>bundle.b</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>
diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/META-INF/MANIFEST.MF b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/META-INF/MANIFEST.MF
@@ -0,0 +1,7 @@
+Manifest-Version: 1.0
+Bundle-ManifestVersion: 2
+Bundle-Name: API Tools Tests Plug-in B
+Bundle-SymbolicName: bundle.b
+Bundle-Version: 1.0.0
+Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=17))"
+Export-Package: test
diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/build.properties b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/build.properties
@@ -0,0 +1,4 @@
+source.. = src/
+output.. = bin/
+bin.includes = META-INF/,\
+               .
diff --git a/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/src/test/Stub.java b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/src/test/Stub.java
@@ -0,0 +1,2 @@
+package test;
+public class Stub {}
diff --git a/...g.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java b/...g.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2021 IBM Corporation and others.
+ * Copyright (c) 2007, 2024 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -82,7 +82,6 @@
 import org.osgi.framework.Constants;
 import org.osgi.framework.FrameworkUtil;
 import org.osgi.framework.Version;
-import org.osgi.framework.namespace.ExecutionEnvironmentNamespace;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 
@@ -93,8 +92,6 @@
  */
 public class BundleComponent extends Component {
 
-	private static final String[] NO_EXECUTION_ENRIONMENTS = new String[0];
-
 	static final String TMP_API_FILE_PREFIX = "api"; //$NON-NLS-1$
 
 	/**
@@ -164,7 +161,7 @@ public class BundleComponent extends Component {
 	 * @see #getExecutionEnvironments()
 	 * @see #hasDeclaredRequiredEE(Map)
 	 */
-	private volatile boolean fHasDeclaredRequiredEE;
+	private volatile String[] fdeclaredRequiredEE;
 
 	/**
 	 * Constructs a new API component from the specified location in the file
@@ -312,7 +309,7 @@ protected void init() throws CoreException {
 				BundleDescription bundleDescription = getBundleDescription(manifest, fLocation, fBundleId);
 				fSymbolicName = bundleDescription.getSymbolicName();
 				fVersion = bundleDescription.getVersion();
-				fHasDeclaredRequiredEE = hasDeclaredRequiredEE(manifest);
+				fdeclaredRequiredEE = ManifestUtils.getRequiredExecutionEnvironments(bundleDescription).toArray(String[]::new);
 				setName(manifest.get(Constants.BUNDLE_NAME));
 				fBundleDescription = bundleDescription;
 			} catch (BundleException e) {
@@ -889,7 +886,7 @@ private static InputStream loadFixedBundleApiDescription(String bundleAndVersion
 	public String[] getExecutionEnvironments() throws CoreException {
 		// Return the EE from the description only if explicitly specified in the
 		// manifest.
-		return fHasDeclaredRequiredEE ? getBundleDescription().getExecutionEnvironments() : NO_EXECUTION_ENRIONMENTS;
+		return fdeclaredRequiredEE;
 	}
 
 	@Override
@@ -1204,28 +1201,4 @@ protected void baselineDisposed(IApiBaseline baseline) throws CoreException {
 		throw new CoreException(new Status(IStatus.ERROR, ApiPlugin.PLUGIN_ID, ApiPlugin.REPORT_BASELINE_IS_DISPOSED,
 				NLS.bind(Messages.BundleApiComponent_baseline_disposed, getName(), baseline.getName()), disposeSource));
 	}
-
-	/*
-	 * This is a copy of
-	 * org.eclipse.pde.internal.core.MinimalState.hasDeclaredRequiredEE(Map<String,
-	 * String>). PDE ends up adding a synthetic EE to the manifest when that method
-	 * returns false, and that ends up in the bundle description such that we cannot
-	 * determine whether the EE was actually present or was synthesized. So we
-	 * repeat that computation here
-	 */
-	@SuppressWarnings("deprecation")
-	private boolean hasDeclaredRequiredEE(Map<String, String> manifest) {
-		if (manifest.containsKey(Constants.BUNDLE_REQUIREDEXECUTIONENVIRONMENT)) {
-			return true;
-		}
-		try {
-			String capability = manifest.get(Constants.REQUIRE_CAPABILITY);
-			ManifestElement[] header = ManifestElement.parseHeader(Constants.REQUIRE_CAPABILITY, capability);
-			return header != null && Arrays.stream(header).map(ManifestElement::getValue)
-					.anyMatch(ExecutionEnvironmentNamespace.EXECUTION_ENVIRONMENT_NAMESPACE::equals);
-		} catch (BundleException e) {
-			return false; // ignore
-		}
-	}
-
 }