FELIX-6212 : Add testcase and fix reexport uses constraint check
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.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1872796 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java b/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
index e9a781e..b06d432 100644
--- a/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
+++ b/resolver/src/main/java/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: "
diff --git a/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java b/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java
index a4517f4..99abd02 100644
--- a/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java
+++ b/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java
@@ -913,6 +913,16 @@
resolver.resolve(rci);
}
+ @Test
+ public void testScenario19() throws Exception
+ {
+ ResolveContext rci = populateScenario19();
+ ResolverImpl resolver = new ResolverImpl(new Logger(Logger.LOG_DEBUG), 1);
+ Map<Resource, List<Wire>> result = resolver.resolve(rci);
+
+ assertEquals("Wrong number of resolved bundles", 9, result.size());
+ }
+
private ResolveContext populateScenario17(boolean realSubstitute,
boolean felixResolveContext, boolean existingWirings)
{
@@ -1092,6 +1102,97 @@
mandatory, Collections.<Resource> emptyList());
}
+ private ResolveContext populateScenario19()
+ {
+ Map<Requirement, List<Capability>> candMap = new HashMap<Requirement, List<Capability>>();
+
+ ResourceImpl split1 = new ResourceImpl("split1");
+ Capability split1_bundle = addCap(split1, BundleNamespace.BUNDLE_NAMESPACE, "splil1");
+ Capability split1_pkg = addCap(split1, PackageNamespace.PACKAGE_NAMESPACE,
+ "split.pkg");
+
+ ResourceImpl split2 = new ResourceImpl("split2");
+ Capability split2_bundle = addCap(split2, BundleNamespace.BUNDLE_NAMESPACE, "splil2");
+ Capability split2_pkg = addCap(split2, PackageNamespace.PACKAGE_NAMESPACE,
+ "split.pkg");
+
+ ResourceImpl split3 = new ResourceImpl("split3");
+ Capability split3_bundle = addCap(split3, BundleNamespace.BUNDLE_NAMESPACE, "splil3");
+ Capability split3_pkg = addCap(split3, PackageNamespace.PACKAGE_NAMESPACE,
+ "split.pkg");
+
+ ResourceImpl split4 = new ResourceImpl("split4");
+ Capability split4_bundle = addCap(split4, BundleNamespace.BUNDLE_NAMESPACE,
+ "splil4");
+ Capability split4_pkg = addCap(split4, PackageNamespace.PACKAGE_NAMESPACE,
+ "split.pkg");
+
+ GenericRequirement split3_split1Req = (GenericRequirement) addReq(split3,
+ BundleNamespace.BUNDLE_NAMESPACE, "split1");
+ split3_split1Req.addDirective(BundleNamespace.REQUIREMENT_VISIBILITY_DIRECTIVE,
+ BundleNamespace.VISIBILITY_REEXPORT);
+ GenericRequirement split3_split2Req = (GenericRequirement) addReq(split3,
+ BundleNamespace.BUNDLE_NAMESPACE, "split2");
+ split3_split2Req.addDirective(BundleNamespace.REQUIREMENT_VISIBILITY_DIRECTIVE,
+ BundleNamespace.VISIBILITY_REEXPORT);
+
+ ResourceImpl reexportSplit1 = new ResourceImpl("reexportSplit1");
+ Capability reexportSplit1_bundle = addCap(reexportSplit1, BundleNamespace.BUNDLE_NAMESPACE, "reexportSplit1");
+ GenericRequirement reexportSplit1_split1Req = (GenericRequirement) addReq(
+ reexportSplit1, BundleNamespace.BUNDLE_NAMESPACE, "split1");
+ reexportSplit1_split1Req.addDirective(
+ BundleNamespace.REQUIREMENT_VISIBILITY_DIRECTIVE,
+ BundleNamespace.VISIBILITY_REEXPORT);
+
+ ResourceImpl reexportSplit3 = new ResourceImpl("reexportSplit3");
+ Capability reexportSplit3_bundle = addCap(reexportSplit3, BundleNamespace.BUNDLE_NAMESPACE, "reexportSplit3");
+ GenericRequirement reexportSplit3_split3Req = (GenericRequirement) addReq(
+ reexportSplit3, BundleNamespace.BUNDLE_NAMESPACE, "split3");
+ reexportSplit3_split3Req.addDirective(
+ BundleNamespace.REQUIREMENT_VISIBILITY_DIRECTIVE,
+ BundleNamespace.VISIBILITY_REEXPORT);
+
+ ResourceImpl exportUsesSplit3 = new ResourceImpl("exportUsesSplit3");
+ Capability exportUsesSplit3_bundle = addCap(exportUsesSplit3, BundleNamespace.BUNDLE_NAMESPACE, "exportUsesSplit3");
+ GenericCapability exportUsesSplit_pkg = (GenericCapability) addCap(
+ exportUsesSplit3, PackageNamespace.PACKAGE_NAMESPACE, "export.pkg");
+ exportUsesSplit_pkg.addDirective(Namespace.CAPABILITY_USES_DIRECTIVE,
+ "split.pkg");
+ GenericRequirement exportUsesSplit3_split3Req = (GenericRequirement) addReq(
+ exportUsesSplit3, BundleNamespace.BUNDLE_NAMESPACE, "reexportSplit3");
+
+ ResourceImpl requireExportAndSplit1 = new ResourceImpl("requireExportAndSplit1");
+ Requirement requireExportAndSplit1_split1Req = addReq(requireExportAndSplit1,
+ BundleNamespace.BUNDLE_NAMESPACE, "reexportSplit1");
+ Requirement requireExportAndSplit1_exportReq = addReq(requireExportAndSplit1,
+ BundleNamespace.BUNDLE_NAMESPACE, "exportUsesSplit3");
+
+ ResourceImpl importExportAndSplit = new ResourceImpl("importExportAndSplit");
+ Requirement importExportAndSplit_importExport = addReq(importExportAndSplit,
+ PackageNamespace.PACKAGE_NAMESPACE, "export.pkg");
+ Requirement importExportAndSplit_importSplit = addReq(importExportAndSplit,
+ PackageNamespace.PACKAGE_NAMESPACE, "split.pkg");
+
+ candMap.put(split3_split1Req, Arrays.asList(split1_bundle));
+ candMap.put(split3_split2Req, Arrays.asList(split2_bundle));
+ candMap.put(reexportSplit1_split1Req, Arrays.asList(split1_bundle));
+ candMap.put(reexportSplit3_split3Req, Arrays.asList(split3_bundle));
+ candMap.put(exportUsesSplit3_split3Req, Arrays.asList(reexportSplit3_bundle));
+ candMap.put(requireExportAndSplit1_split1Req, Arrays.asList(reexportSplit1_bundle));
+ candMap.put(requireExportAndSplit1_exportReq,
+ Arrays.asList(exportUsesSplit3_bundle));
+ candMap.put(importExportAndSplit_importSplit,
+ Arrays.asList(split4_pkg, split3_pkg));
+ candMap.put(importExportAndSplit_importExport,
+ Arrays.asList((Capability) exportUsesSplit_pkg));
+
+ Collection<Resource> mandatory = Arrays.<Resource> asList(split1, split2,
+ split3, split4, reexportSplit1, reexportSplit3, exportUsesSplit3,
+ requireExportAndSplit1, importExportAndSplit);
+ return new ResolveContextImpl(Collections.<Resource, Wiring> emptyMap(), candMap,
+ mandatory, Collections.<Resource> emptyList());
+ }
+
private static String getResourceName(Resource r)
{
return r.getCapabilities(IdentityNamespace.IDENTITY_NAMESPACE).get(0).getAttributes()
