Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2020-01-13 15:53:33 +0000
committerThomas Watson2020-01-14 22:32:15 +0000
commitb61cf1d96b3e7ef8da2e506f0393d91e422dbcfd (patch)
tree7bbfc8eb49ce1693969e2609cfdfa691556861c5
parent4d9b3a851bc877af780577fa4f096976399c121b (diff)
downloadrt.equinox.framework-I20200116-0330.tar.gz
rt.equinox.framework-I20200116-0330.tar.xz
rt.equinox.framework-I20200116-0330.zip
Bug 559118 - Add testcase and fix reexport uses constraint checkI20200116-1800I20200116-0930I20200116-0330I20200116-0310I20200115-1800
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>
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java67
-rwxr-xr-xbundles/org.eclipse.osgi/felix/src/org/apache/felix/resolver/ResolverImpl.java96
2 files changed, 122 insertions, 41 deletions
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 bb48c72b4..161841eaf 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 @@ public class TestModuleContainer extends AbstractTest {
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 e9a781e5e..b06d43227 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.security.*;
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
{
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
}
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
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 class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
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 @@ public class ResolverImpl implements Resolver
*/
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: "

Back to the top