Skip to content

Commit

Permalink
Fix: API tool reports fictitious changes to execution environment
Browse files Browse the repository at this point in the history
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 #1301
  • Loading branch information
ptziegler authored and HannesWell committed Jul 7, 2024
1 parent 1b1b92a commit be7a44b
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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$
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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$
}
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
source.. = src/
output.. = bin/
bin.includes = META-INF/,\
.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
package test;
public class Stub {}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;

Expand All @@ -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$

/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

}

0 comments on commit be7a44b

Please sign in to comment.