Bug 559118 - Add testcase and fix reexport uses constraint check
This is a testcase that reproduces the issue found in bug 558895. When
an exporter of a split package requires multiple bundles that also
export the split package and that bundle does a reexport on one of the
parts it can cause resolution issues for uses constraints.
The actual problem is with reexport itself. The reexporting bundle does
not have to also export the package to cause the issue. The problem is
that each part of the package pulled in from the require-reexport is
checked in isolation with the using bundles wire to the same package
name. This is incorrect because that is only a subset of the actual
used package from the perspective of the exported package that is using
the split package.
The fix is to record all the package parts for the split used package
from the wiring of the bundle exporting the package that uses the split
package. That way during the compatibility check we can accurately use
the set of sources for the split package that the exporting bundle is
using.
Change-Id: I5d6194adabc7c04fe990d663ad1dd6bb77f2ac39
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
index bb48c72..161841e 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java
@@ -3571,6 +3571,73 @@
stopError.printStackTrace();
}
+ @Test
+ public void testUsesWithRequireReexport() throws BundleException, IOException {
+ DummyContainerAdaptor adaptor = createDummyAdaptor();
+ ModuleContainer container = adaptor.getContainer();
+
+ // install the system.bundle
+ Module systemBundle = installDummyModule("system.bundle.MF", Constants.SYSTEM_BUNDLE_LOCATION, Constants.SYSTEM_BUNDLE_SYMBOLICNAME, null, null, container);
+ ResolutionReport report = container.resolve(Arrays.asList(systemBundle), true);
+ Assert.assertNull("Failed to resolve system.bundle.", report.getResolutionException());
+
+ // install part 1 (ui.workbench)
+ Map<String, String> split1Manifest = new HashMap<String, String>();
+ split1Manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ split1Manifest.put(Constants.BUNDLE_SYMBOLICNAME, "split1");
+ split1Manifest.put(Constants.EXPORT_PACKAGE, "split.pkg");
+ Module moduleSplit1 = installDummyModule(split1Manifest, "split1", container);
+
+ // install part 2 (e4.ui.ide)
+ Map<String, String> split2Manifest = new HashMap<String, String>();
+ split2Manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ split2Manifest.put(Constants.BUNDLE_SYMBOLICNAME, "split2");
+ split2Manifest.put(Constants.EXPORT_PACKAGE, "split.pkg");
+ Module moduleSplit2 = installDummyModule(split2Manifest, "split2", container);
+
+ // install part 3 which requires part 1 and 2, reexports 1 and 2 (ui.ide)
+ Map<String, String> split3Manifest = new HashMap<String, String>();
+ split3Manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ split3Manifest.put(Constants.BUNDLE_SYMBOLICNAME, "split3");
+ split3Manifest.put(Constants.EXPORT_PACKAGE, "split.pkg");
+ // the reexport here are not necessary; but cause issues for the resolver
+ split3Manifest.put(Constants.REQUIRE_BUNDLE, "split1; visibility:=reexport, split2; visibility:=reexport");
+ Module moduleSplit3 = installDummyModule(split3Manifest, "split3", container);
+
+ // install reexporter of part1 (ui)
+ Map<String, String> reexporterPart1Manifest = new HashMap<String, String>();
+ reexporterPart1Manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ reexporterPart1Manifest.put(Constants.BUNDLE_SYMBOLICNAME, "reexport1");
+ reexporterPart1Manifest.put(Constants.REQUIRE_BUNDLE, "split1; visibility:=reexport");
+ Module moduleReexport1 = installDummyModule(reexporterPart1Manifest, "reexport1", container);
+
+ // install reexporter of split3
+ Map<String, String> reexporterSplit3Manifest = new HashMap<String, String>();
+ reexporterSplit3Manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ reexporterSplit3Manifest.put(Constants.BUNDLE_SYMBOLICNAME, "reexportSplit3");
+ reexporterSplit3Manifest.put(Constants.REQUIRE_BUNDLE, "split3; visibility:=reexport");
+ Module moduleReexportSplit3 = installDummyModule(reexporterSplit3Manifest, "reexportSplit3", container);
+
+ // install test export that requires reexportSplit3 (should get access to all 3 parts)
+ Map<String, String> testExporterUses = new HashMap<String, String>();
+ testExporterUses.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ testExporterUses.put(Constants.BUNDLE_SYMBOLICNAME, "test.exporter");
+ testExporterUses.put(Constants.REQUIRE_BUNDLE, "reexportSplit3");
+ testExporterUses.put(Constants.EXPORT_PACKAGE, "export.pkg; uses:=split.pkg");
+ Module testExporter = installDummyModule(testExporterUses, "test.exporter", container);
+
+ // install test requirer that requires the exporter and reexport1 (should get access to only part 1)
+ // part 1 is a subset of what the exporter has access to so it should resolve
+ Map<String, String> testRequireUses = new HashMap<String, String>();
+ testRequireUses.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ testRequireUses.put(Constants.BUNDLE_SYMBOLICNAME, "test.requirer");
+ testRequireUses.put(Constants.REQUIRE_BUNDLE, "test.exporter, reexport1");
+ Module testRequirer = installDummyModule(testRequireUses, "test.requirer", container);
+
+ report = container.resolve(Arrays.asList(moduleSplit1, moduleSplit2, moduleSplit3, moduleReexport1, moduleReexportSplit3, testExporter, testRequirer), true);
+ Assert.assertNull("Failed to resolve", report.getResolutionException());
+ }
+
private static void assertWires(List<ModuleWire> required, List<ModuleWire>... provided) {
for (ModuleWire requiredWire : required) {
for (List<ModuleWire> providedList : provided) {
diff --git a/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java b/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java
index e9a781e..b06d432 100755
--- a/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java
+++ b/bundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java
@@ -22,7 +22,6 @@
import java.util.*;
import java.util.Map.Entry;
import java.util.concurrent.*;
-import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.felix.resolver.reason.ReasonException;
@@ -1094,26 +1093,30 @@
continue;
}
- ArrayMap<Capability, UsedBlames> usedPkgBlames = currentPkgs.m_usedPkgs.getOrCompute(usedPkgName);
+ ArrayMap<Set<Capability>, UsedBlames> usedPkgBlames = currentPkgs.m_usedPkgs.getOrCompute(usedPkgName);
+ List<Blame> newBlames = new ArrayList<Blame>();
for (Blame blame : candSourceBlames)
{
+ List<Requirement> newBlameReqs;
if (blame.m_reqs != null)
{
- List<Requirement> blameReqs2 = new ArrayList<Requirement>(blameReqs.size() + 1);
- blameReqs2.addAll(blameReqs);
+ newBlameReqs = new ArrayList<Requirement>(blameReqs.size() + 1);
+ newBlameReqs.addAll(blameReqs);
// Only add the last requirement in blame chain because
// that is the requirement wired to the blamed capability
- blameReqs2.add(blame.m_reqs.get(blame.m_reqs.size() - 1));
- addUsedBlame(usedPkgBlames, blame.m_cap, blameReqs2, matchingCap);
- mergeUses(session, current, currentPkgs, blame.m_cap, blameReqs2, matchingCap,
- resourcePkgMap, cycleMap);
+ newBlameReqs.add(blame.m_reqs.get(blame.m_reqs.size() - 1));
}
else
{
- addUsedBlame(usedPkgBlames, blame.m_cap, blameReqs, matchingCap);
- mergeUses(session, current, currentPkgs, blame.m_cap, blameReqs, matchingCap,
- resourcePkgMap, cycleMap);
+ newBlameReqs = blameReqs;
}
+ newBlames.add(new Blame(blame.m_cap, newBlameReqs));
+ }
+ addUsedBlames(usedPkgBlames, newBlames, matchingCap, resourcePkgMap);
+ for (Blame newBlame : newBlames)
+ {
+ mergeUses(session, current, currentPkgs, newBlame.m_cap, newBlame.m_reqs, matchingCap,
+ resourcePkgMap, cycleMap);
}
}
}
@@ -1276,18 +1279,31 @@
return uses;
}
- private static void addUsedBlame(
- ArrayMap<Capability, UsedBlames> usedBlames, Capability usedCap,
- List<Requirement> blameReqs, Capability matchingCap)
+ private static void addUsedBlames(
+ ArrayMap<Set<Capability>, UsedBlames> usedBlames, Collection<Blame> blames, Capability matchingCap, Map<Resource, Packages> resourcePkgMap)
{
- // Create a new Blame based off the used capability and the
- // blame chain requirements.
- Blame newBlame = new Blame(usedCap, blameReqs);
- // Find UsedBlame that uses the same capablity as the new blame.
- UsedBlames addToBlame = usedBlames.getOrCompute(usedCap);
- // Add the new Blame and record the matching capability cause
+ Set<Capability> usedCaps;
+ if (blames.size() == 1)
+ {
+ usedCaps = getPackageSources(blames.iterator().next().m_cap, resourcePkgMap);
+ }
+ else
+ {
+ usedCaps = new HashSet<Capability>();
+ for (Blame blame : blames)
+ {
+ usedCaps.addAll(getPackageSources(blame.m_cap, resourcePkgMap));
+ }
+ }
+
+ // Find UsedBlame that uses the same capability as the new blame.
+ UsedBlames addToBlame = usedBlames.getOrCompute(usedCaps);
+ // Add the new Blames and record the matching capability cause
// in case the root requirement has multiple cardinality.
- addToBlame.addBlame(newBlame, matchingCap);
+ for (Blame blame : blames)
+ {
+ addToBlame.addBlame(blame, matchingCap);
+ }
}
private ResolutionError checkPackageSpaceConsistency(
@@ -1368,14 +1384,14 @@
{
String pkgName = entry.getKey();
Blame exportBlame = entry.getValue();
- ArrayMap<Capability, UsedBlames> pkgBlames = pkgs.m_usedPkgs.get(pkgName);
+ ArrayMap<Set<Capability>, UsedBlames> pkgBlames = pkgs.m_usedPkgs.get(pkgName);
if (pkgBlames == null)
{
continue;
}
for (UsedBlames usedBlames : pkgBlames.values())
{
- if (!isCompatible(exportBlame, usedBlames.m_cap, resourcePkgMap))
+ if (!isCompatible(exportBlame, usedBlames.m_caps, resourcePkgMap))
{
mutated = (mutated != null)
? mutated
@@ -1421,7 +1437,7 @@
for (Entry<String, List<Blame>> entry : allImportRequirePkgs.fast())
{
String pkgName = entry.getKey();
- ArrayMap<Capability, UsedBlames> pkgBlames = pkgs.m_usedPkgs.get(pkgName);
+ ArrayMap<Set<Capability>, UsedBlames> pkgBlames = pkgs.m_usedPkgs.get(pkgName);
if (pkgBlames == null)
{
continue;
@@ -1430,7 +1446,7 @@
for (UsedBlames usedBlames : pkgBlames.values())
{
- if (!isCompatible(requirementBlames, usedBlames.m_cap, resourcePkgMap))
+ if (!isCompatible(requirementBlames, usedBlames.m_caps, resourcePkgMap))
{
mutated = (mutated != null)
? mutated
@@ -1686,21 +1702,20 @@
}
private static boolean isCompatible(
- Blame currentBlame, Capability candCap,
+ Blame currentBlame, Set<Capability> candSources,
Map<Resource, Packages> resourcePkgMap)
{
- if (currentBlame.m_cap.equals(candCap))
+ if (candSources.contains(currentBlame.m_cap))
{
return true;
}
- Set<Capability> candSources = getPackageSources(candCap, resourcePkgMap);
Set<Capability> currentSources = getPackageSources(currentBlame.m_cap, resourcePkgMap);
return currentSources.containsAll(candSources)
|| candSources.containsAll(currentSources);
}
private static boolean isCompatible(
- List<Blame> currentBlames, Capability candCap,
+ List<Blame> currentBlames, Set<Capability> candSources,
Map<Resource, Packages> resourcePkgMap)
{
int size = currentBlames.size();
@@ -1709,7 +1724,7 @@
case 0:
return true;
case 1:
- return isCompatible(currentBlames.get(0), candCap, resourcePkgMap);
+ return isCompatible(currentBlames.get(0), candSources, resourcePkgMap);
default:
Set<Capability> currentSources = new HashSet<Capability>(currentBlames.size());
for (Blame currentBlame : currentBlames)
@@ -1717,7 +1732,6 @@
Set<Capability> blameSources = getPackageSources(currentBlame.m_cap, resourcePkgMap);
currentSources.addAll(blameSources);
}
- Set<Capability> candSources = getPackageSources(candCap, resourcePkgMap);
return currentSources.containsAll(candSources)
|| candSources.containsAll(currentSources);
}
@@ -2073,7 +2087,7 @@
System.out.println(" " + entry.getKey() + " - " + entry.getValue());
}
System.out.println(" USED");
- for (Entry<String, ArrayMap<Capability, UsedBlames>> entry : packages.m_usedPkgs.entrySet())
+ for (Entry<String, ArrayMap<Set<Capability>, UsedBlames>> entry : packages.m_usedPkgs.entrySet())
{
System.out.println(" " + entry.getKey() + " - " + entry.getValue().values());
}
@@ -2097,7 +2111,7 @@
public final OpenHashMap<String, Blame> m_substitePkgs;
public final OpenHashMap<String, List<Blame>> m_importedPkgs;
public final OpenHashMap<String, List<Blame>> m_requiredPkgs;
- public final OpenHashMap<String, ArrayMap<Capability, UsedBlames>> m_usedPkgs;
+ public final OpenHashMap<String, ArrayMap<Set<Capability>, UsedBlames>> m_usedPkgs;
public final OpenHashMap<Capability, Set<Capability>> m_sources;
@SuppressWarnings("serial")
@@ -2118,12 +2132,12 @@
return new ArrayList<Blame>();
}
};
- m_usedPkgs = new OpenHashMap<String, ArrayMap<Capability, UsedBlames>>(128) {
+ m_usedPkgs = new OpenHashMap<String, ArrayMap<Set<Capability>, UsedBlames>>(128) {
@Override
- protected ArrayMap<Capability, UsedBlames> compute(String s) {
- return new ArrayMap<Capability, UsedBlames>() {
+ protected ArrayMap<Set<Capability>, UsedBlames> compute(String s) {
+ return new ArrayMap<Set<Capability>, UsedBlames>() {
@Override
- protected UsedBlames compute(Capability key) {
+ protected UsedBlames compute(Set<Capability> key) {
return new UsedBlames(key);
}
};
@@ -2177,18 +2191,18 @@
*/
private static class UsedBlames
{
- public final Capability m_cap;
+ public final Set<Capability> m_caps;
public final List<Blame> m_blames = new ArrayList<ResolverImpl.Blame>();
private Map<Requirement, Set<Capability>> m_rootCauses;
- public UsedBlames(Capability cap)
+ public UsedBlames(Set<Capability> caps)
{
- m_cap = cap;
+ m_caps = caps;
}
public void addBlame(Blame blame, Capability matchingRootCause)
{
- if (!m_cap.equals(blame.m_cap))
+ if (!m_caps.contains(blame.m_cap))
{
throw new IllegalArgumentException(
"Attempt to add a blame with a different used capability: "
