From 6e003d1b2e322eb9e95d223fcbeb274a3d0363b8 Mon Sep 17 00:00:00 2001 From: Patrick Ziegler Date: Fri, 14 Jun 2024 17:26:24 +0200 Subject: [PATCH] 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 https://github.com/eclipse-pde/eclipse.pde/issues/1301 --- .../ProjectTypeContainerTests.java | 33 +++++++++++++----- .../test-builder/baseline/bundle.b/.classpath | 7 ++++ .../test-builder/baseline/bundle.b/.project | 28 +++++++++++++++ .../baseline/bundle.b/META-INF/MANIFEST.MF | 7 ++++ .../baseline/bundle.b/build.properties | 4 +++ .../baseline/bundle.b/src/test/Stub.java | 2 ++ .../tools/internal/model/BundleComponent.java | 34 +++---------------- 7 files changed, 77 insertions(+), 38 deletions(-) create mode 100644 apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.classpath create mode 100644 apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.project create mode 100644 apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/META-INF/MANIFEST.MF create mode 100644 apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/build.properties create mode 100644 apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/src/test/Stub.java diff --git a/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/ProjectTypeContainerTests.java b/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/ProjectTypeContainerTests.java index 0d21ef95b1..51ef88628d 100644 --- a/apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/ProjectTypeContainerTests.java +++ b/apitools/org.eclipse.pde.api.tools.tests/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 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 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 new file mode 100644 index 0000000000..1fa3e6803d --- /dev/null +++ b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.classpath @@ -0,0 +1,7 @@ + + + + + + + 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 new file mode 100644 index 0000000000..d78376aff6 --- /dev/null +++ b/apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.project @@ -0,0 +1,28 @@ + + + bundle.b + + + + + + org.eclipse.jdt.core.javabuilder + + + + + org.eclipse.pde.ManifestBuilder + + + + + org.eclipse.pde.SchemaBuilder + + + + + + org.eclipse.pde.PluginNature + org.eclipse.jdt.core.javanature + + 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 new file mode 100644 index 0000000000..350b29fadf --- /dev/null +++ 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 new file mode 100644 index 0000000000..34d2e4d2da --- /dev/null +++ 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 new file mode 100644 index 0000000000..9eeba5557e --- /dev/null +++ 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 {} \ No newline at end of file diff --git a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java index 56a0b06467..3a7fd1c423 100644 --- a/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java +++ b/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java @@ -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; @@ -91,10 +90,9 @@ * * @since 1.0.0 */ +@SuppressWarnings("restriction") // org.eclipse.osgi.internal.framework.FilterImpl 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 +162,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 +310,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 +887,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 +1202,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). 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 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 - } - } - }