You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by tj...@apache.org on 2020/01/14 21:31:18 UTC
svn commit: r1872796 - in /felix/trunk/resolver/src:
main/java/org/apache/felix/resolver/ResolverImpl.java
test/java/org/apache/felix/resolver/test/ResolverTest.java
Author: tjwatson
Date: Tue Jan 14 21:31:18 2020
New Revision: 1872796
URL: http://svn.apache.org/viewvc?rev=1872796&view=rev
Log:
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.
Modified:
felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java
Modified: felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java?rev=1872796&r1=1872795&r2=1872796&view=diff
==============================================================================
--- felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java (original)
+++ felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java Tue Jan 14 21:31:18 2020
@@ -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 Res
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 Res
return uses;
}
- private static void addUsedBlame(
- ArrayMap<Capability, UsedBlames> usedBlames, Capability usedCap,
- List<Requirement> blameReqs, Capability matchingCap)
- {
- // 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
+ private static void addUsedBlames(
+ ArrayMap<Set<Capability>, UsedBlames> usedBlames, Collection<Blame> blames, Capability matchingCap, Map<Resource, Packages> resourcePkgMap)
+ {
+ 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 Res
{
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 Res
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 Res
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 Res
}
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 Res
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 Res
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 Res
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 Res
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 Res
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 Res
*/
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: "
Modified: felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java?rev=1872796&r1=1872795&r2=1872796&view=diff
==============================================================================
--- felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java (original)
+++ felix/trunk/resolver/src/test/java/org/apache/felix/resolver/test/ResolverTest.java Tue Jan 14 21:31:18 2020
@@ -913,6 +913,16 @@ public class ResolverTest
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)
{
@@ -1091,6 +1101,97 @@ public class ResolverTest
return new ResolveContextImpl(Collections.<Resource, Wiring> emptyMap(), candMap,
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)
{