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 2017/03/28 21:23:01 UTC
svn commit: r1789240 - in
/felix/trunk/resolver/src/main/java/org/apache/felix/resolver:
Candidates.java ResolverImpl.java
Author: tjwatson
Date: Tue Mar 28 21:23:01 2017
New Revision: 1789240
URL: http://svn.apache.org/viewvc?rev=1789240&view=rev
Log:
FELIX-5601 - issues resolving with substitutable exports
Original fix had issues with assuming the Candidates::m_dependenMap
got updated as the CandidateSelectors moved on to the next candidate.
Need an additional check to make sure the current candidate is the
one that could be substituted.
This also exposed an issue with the solution for the testcase
that had long blame requirement chain. In this case we have
historically started at the direct requirement for the conflicting
capability and worked our way back to the root requirement for
the blame chain.
This worked for the most part when we didn't pay attention to
substitutable capabilities when permuting, but I suspect there
were still cases where this would eliminate. This fix now
traverses the blame chain from both directions to ensure
we can find a solution when one exists
Modified:
felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java
felix/trunk/resolver/src/main/java/org/apache/felix/resolver/ResolverImpl.java
Modified: felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java
URL: http://svn.apache.org/viewvc/felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java?rev=1789240&r1=1789239&r2=1789240&view=diff
==============================================================================
--- felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java (original)
+++ felix/trunk/resolver/src/main/java/org/apache/felix/resolver/Candidates.java Tue Mar 28 21:23:01 2017
@@ -1225,7 +1225,8 @@ class Candidates
// See ResolverImpl::checkPackageSpaceConsistency
// Check if the current candidate is substitutable by the req;
- // This check is necessary here because
+ // This check is necessary here because of the way we traverse used blames
+ // allows multiple requirements to be permuted in one Candidates
if (req.equals(m_subtitutableMap.get(current)))
{
// this is a substitute req,
@@ -1235,14 +1236,20 @@ class Candidates
{
for (Requirement dependent : dependents)
{
- CandidateSelector dependentSelector = m_candidateMap.get(dependent);
- // If the dependent selector only has one capability left then we know it is
- // using the current candidate and has no options left
- if (dependentSelector != null && dependentSelector.getRemainingCandidateCount() <= 1)
+ CandidateSelector dependentSelector = m_candidateMap.get(
+ dependent);
+ // If the dependent selector only has one capability left then check if
+ // the current candidate is the selector's current candidate.
+ if (dependentSelector != null
+ && dependentSelector.getRemainingCandidateCount() <= 1)
{
- // return false since we do not want to allow this requirement
- // to substitute the capability
- return false;
+ if (current.equals(
+ dependentSelector.getCurrentCandidate()))
+ {
+ // return false since we do not want to allow this requirement
+ // to substitute the capability
+ return false;
+ }
}
}
}
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=1789240&r1=1789239&r2=1789240&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 Mar 28 21:23:01 2017
@@ -23,6 +23,8 @@ 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.util.ArrayMap;
import org.apache.felix.resolver.util.CandidateSelector;
import org.apache.felix.resolver.util.OpenHashMap;
@@ -1337,8 +1339,9 @@ public class ResolverImpl implements Res
// When several reqs are permuted at the same time this reduces the number of solutions tried.
// See the method Candidates::canRemoveCandidate for a case where substitutions must be checked
// because of this code that may permute multiple reqs in on candidates permutation.
+ AtomicReference<Candidates> permRef1 = new AtomicReference<Candidates>();
+ AtomicReference<Candidates> permRef2 = new AtomicReference<Candidates>();
Set<Requirement> mutated = null;
- Candidates permutation = null;
// Check if there are any uses conflicts with exported packages.
for (Entry<String, Blame> entry : pkgs.m_exportedPkgs.fast())
@@ -1354,54 +1357,11 @@ public class ResolverImpl implements Res
{
if (!isCompatible(exportBlame, usedBlames.m_cap, resourcePkgMap))
{
- for (Blame usedBlame : usedBlames.m_blames)
- {
- if (session.checkMultiple(usedBlames, usedBlame, allCandidates))
- {
- // Continue to the next usedBlame, if possible we
- // removed the conflicting candidates.
- continue;
- }
- // Create a candidate permutation that eliminates all candidates
- // that conflict with existing selected candidates.
- permutation = (permutation != null)
- ? permutation
- : allCandidates.copy();
- if (rethrow == null)
- {
- rethrow = new UseConstraintError(
- session.getContext(), allCandidates, resource, pkgName, usedBlame);
- }
- mutated = (mutated != null)
- ? mutated
- : new HashSet<Requirement>();
-
- for (int reqIdx = usedBlame.m_reqs.size() - 1; reqIdx >= 0; reqIdx--)
- {
- Requirement req = usedBlame.m_reqs.get(reqIdx);
- // Sanity check for multiple.
- if (Util.isMultiple(req))
- {
- continue;
- }
- // If we've already permutated this requirement in another
- // uses constraint, don't permutate it again just continue
- // with the next uses constraint.
- if (mutated.contains(req))
- {
- break;
- }
-
- // See if we can permutate the candidates for blamed
- // requirement; there may be no candidates if the resource
- // associated with the requirement is already resolved.
- if (permutation.canRemoveCandidate(req)) {
- permutation.removeFirstCandidate(req);
- mutated.add(req);
- break;
- }
- }
- }
+ mutated = (mutated != null)
+ ? mutated
+ : new HashSet<Requirement>();
+ rethrow = permuteUsedBlames(session, rethrow, allCandidates, resource,
+ pkgName, null, usedBlames, permRef1, permRef2, mutated);
}
}
@@ -1409,7 +1369,8 @@ public class ResolverImpl implements Res
{
if (!mutated.isEmpty())
{
- session.addPermutation(PermutationType.USES, permutation);
+ session.addPermutation(PermutationType.USES, permRef1.get());
+ session.addPermutation(PermutationType.USES, permRef2.get());
}
if (m_logger.isDebugEnabled())
{
@@ -1451,60 +1412,13 @@ public class ResolverImpl implements Res
{
if (!isCompatible(requirementBlames, usedBlames.m_cap, resourcePkgMap))
{
+ mutated = (mutated != null)
+ ? mutated
+ : new HashSet<Requirement>();
// Split packages, need to think how to get a good message for split packages (sigh)
// For now we just use the first requirement that brings in the package that conflicts
Blame requirementBlame = requirementBlames.get(0);
- for (Blame usedBlame : usedBlames.m_blames)
- {
- if (session.checkMultiple(usedBlames, usedBlame, allCandidates))
- {
- // Continue to the next usedBlame, if possible we
- // removed the conflicting candidates.
- continue;
- }
- // Create a candidate permutation that eliminates all candidates
- // that conflict with existing selected candidates.
- permutation = (permutation != null)
- ? permutation
- : allCandidates.copy();
- if (rethrow == null)
- {
- rethrow = new UseConstraintError(
- session.getContext(), allCandidates,
- resource, pkgName,
- requirementBlame, usedBlame);
- }
-
- mutated = (mutated != null)
- ? mutated
- : new HashSet<Requirement>();
-
- for (int reqIdx = usedBlame.m_reqs.size() - 1; reqIdx >= 0; reqIdx--)
- {
- Requirement req = usedBlame.m_reqs.get(reqIdx);
- // Sanity check for multiple.
- if (Util.isMultiple(req))
- {
- continue;
- }
- // If we've already permutated this requirement in another
- // uses constraint, don't permutate it again just continue
- // with the next uses constraint.
- if (mutated.contains(req))
- {
- break;
- }
-
- // See if we can permutate the candidates for blamed
- // requirement; there may be no candidates if the resource
- // associated with the requirement is already resolved.
- if (permutation.canRemoveCandidate(req)) {
- permutation.removeFirstCandidate(req);
- mutated.add(req);
- break;
- }
- }
- }
+ rethrow = permuteUsedBlames(session, rethrow, allCandidates, resource, pkgName, requirementBlame, usedBlames, permRef1, permRef2, mutated);
}
// If there was a uses conflict, then we should add a uses
@@ -1518,7 +1432,8 @@ public class ResolverImpl implements Res
// Add uses permutation if we m_mutated any candidates.
if (!mutated.isEmpty())
{
- session.addPermutation(PermutationType.USES, permutation);
+ session.addPermutation(PermutationType.USES, permRef1.get());
+ session.addPermutation(PermutationType.USES, permRef2.get());
}
// Try to permutate the candidate for the original
@@ -1584,6 +1499,99 @@ public class ResolverImpl implements Res
return null;
}
+ private ResolutionError permuteUsedBlames(ResolveSession session,
+ ResolutionError rethrow, Candidates allCandidates, Resource resource,
+ String pkgName, Blame requirementBlame, UsedBlames usedBlames,
+ AtomicReference<Candidates> permRef1, AtomicReference<Candidates> permRef2,
+ Set<Requirement> mutated)
+ {
+ for (Blame usedBlame : usedBlames.m_blames)
+ {
+ if (session.checkMultiple(usedBlames, usedBlame, allCandidates))
+ {
+ // Continue to the next usedBlame, if possible we
+ // removed the conflicting candidates.
+ continue;
+ }
+
+ if (rethrow == null)
+ {
+ if (requirementBlame == null)
+ {
+ rethrow = new UseConstraintError(session.getContext(), allCandidates,
+ resource, pkgName, usedBlame);
+ }
+ else
+ {
+ rethrow = new UseConstraintError(session.getContext(), allCandidates,
+ resource, pkgName, requirementBlame, usedBlame);
+ }
+ }
+
+ // Create a candidate permutation that eliminates all candidates
+ // that conflict with existing selected candidates going from direct requirement -> root
+ Candidates perm1 = permRef1.get();
+ if (perm1 == null)
+ {
+ perm1 = allCandidates.copy();
+ permRef1.set(perm1);
+ }
+ for (int reqIdx = usedBlame.m_reqs.size() - 1; reqIdx >= 0; reqIdx--)
+ {
+ Requirement req = usedBlame.m_reqs.get(reqIdx);
+ if (permuteUsedBlameRequirement(req, mutated, perm1))
+ {
+ break;
+ }
+ }
+ // Create a candidate permutation that eliminates all candidates
+ // that conflict with existing selected candidates going from root -> direct requirement
+ Candidates perm2 = permRef2.get();
+ if (perm2 == null)
+ {
+ perm2 = allCandidates.copy();
+ permRef2.set(perm2);
+ }
+ for (int reqIdx = 0; reqIdx < usedBlame.m_reqs.size(); reqIdx++)
+ {
+ Requirement req = usedBlame.m_reqs.get(reqIdx);
+ if (permuteUsedBlameRequirement(req, mutated, perm2))
+ {
+ break;
+ }
+ }
+ }
+ return rethrow;
+ }
+
+ private boolean permuteUsedBlameRequirement(Requirement req, Set<Requirement> mutated,
+ Candidates permutation)
+ {
+ // Sanity check for multiple.
+ if (Util.isMultiple(req))
+ {
+ return false;
+ }
+ // If we've already permutated this requirement in another
+ // uses constraint, don't permutate it again just continue
+ // with the next uses constraint.
+ if (mutated.contains(req))
+ {
+ return true;
+ }
+
+ // See if we can permutate the candidates for blamed
+ // requirement; there may be no candidates if the resource
+ // associated with the requirement is already resolved.
+ if (permutation.canRemoveCandidate(req))
+ {
+ permutation.removeFirstCandidate(req);
+ mutated.add(req);
+ return true;
+ }
+ return false;
+ }
+
private static OpenHashMap<String, Blame> calculateExportedPackages(
ResolveSession session,
Candidates allCandidates,