Refactor resolver to simplify/unify entry points; no longer differentiate
between a single root resolve and multiple root resolves. (FELIX-2950)


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1155562 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleRevisionDependencies.java b/framework/src/main/java/org/apache/felix/framework/BundleRevisionDependencies.java
index f2ea240..9948ad0 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleRevisionDependencies.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleRevisionDependencies.java
@@ -163,8 +163,8 @@
         List<BundleRevision> revisions = bundle.adapt(BundleRevisions.class).getRevisions();
         for (BundleRevision revision : revisions)
         {
-// TODO: OSGi R4.3 - This is sort of a hack. We need to special case fragments,
-//       since their dependents are their hosts.
+            // We need to special case fragments,
+            // since their dependents are their hosts.
             if (Util.isFragment(revision))
             {
                 BundleWiring wiring = revision.getWiring();
diff --git a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
index fa858bd..acc1df4 100644
--- a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
+++ b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
@@ -542,8 +542,12 @@
         {
             try
             {
-                result = ((BundleRevisionImpl)
-                    extBundle.adapt(BundleRevision.class)).getResourceLocal(path);
+                BundleRevisionImpl bri =
+                    (BundleRevisionImpl) extBundle.adapt(BundleRevision.class);
+                if (bri != null)
+                {
+                    result = bri.getResourceLocal(path);
+                }
             }
             catch (Exception ex)
             {
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index f3d9dc8..e59f2e2 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -676,7 +676,9 @@
                 // state to be set to RESOLVED.
                 try
                 {
-                    m_resolver.resolve(adapt(BundleRevision.class));
+                    m_resolver.resolve(
+                        Collections.singleton(adapt(BundleRevision.class)),
+                        Collections.EMPTY_SET);
                 }
                 catch (ResolveException ex)
                 {
@@ -3800,7 +3802,7 @@
                 }
                 try
                 {
-                    m_resolver.resolve(revisions);
+                    m_resolver.resolve(Collections.EMPTY_SET, revisions);
                     if (result)
                     {
                         for (BundleRevision br : revisions)
@@ -3836,7 +3838,7 @@
     {
         try
         {
-            m_resolver.resolve(revision);
+            m_resolver.resolve(Collections.singleton(revision), Collections.EMPTY_SET);
         }
         catch (ResolveException ex)
         {
diff --git a/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java b/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java
index 2d01f27..769e023 100644
--- a/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java
+++ b/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java
@@ -92,195 +92,10 @@
         return m_resolverState.getCandidates(req, obeyMandatory);
     }
 
-    void resolve(BundleRevision rootRevision) throws ResolveException, BundleException
-    {
-        // Although there is a race condition to check the bundle state
-        // then lock it, we do this because we don't want to acquire the
-        // a lock just to check if the revision is resolved, which itself
-        // is a safe read. If the revision isn't resolved, we end up double
-        // check the resolved status later.
-        if (rootRevision.getWiring() == null)
-        {
-            // Acquire global lock.
-            boolean locked = m_felix.acquireGlobalLock();
-            if (!locked)
-            {
-                throw new ResolveException(
-                    "Unable to acquire global lock for resolve.", rootRevision, null);
-            }
-
-            // Make sure we are not already resolving, which can be
-            // the case if a resolver hook does something bad.
-            if (m_isResolving)
-            {
-                m_felix.releaseGlobalLock();
-                throw new IllegalStateException("Nested resolve operations not allowed.");
-            }
-            m_isResolving = true;
-
-            Map<BundleRevision, List<ResolverWire>> wireMap = null;
-            try
-            {
-                // Extensions are resolved differently.
-                BundleImpl bundle = (BundleImpl) rootRevision.getBundle();
-                if (bundle.isExtension())
-                {
-                    return;
-                }
-
-                // Get resolver hook factories.
-                Set<ServiceReference<ResolverHookFactory>> hookRefs =
-                    m_felix.getHooks(ResolverHookFactory.class);
-                if (!hookRefs.isEmpty())
-                {
-                    // Create triggers list.
-                    List<BundleRevision> triggers = new ArrayList<BundleRevision>(1);
-                    triggers.add(rootRevision);
-                    triggers = Collections.unmodifiableList(triggers);
-
-                    // Create resolver hook objects by calling begin() on factory.
-                    for (ServiceReference<ResolverHookFactory> ref : hookRefs)
-                    {
-                        try
-                        {
-                            ResolverHookFactory rhf = m_felix.getService(m_felix, ref);
-                            if (rhf != null)
-                            {
-                                ResolverHook hook =
-                                    Felix.m_secureAction
-                                        .invokeResolverHookFactory(rhf, triggers);
-                                if (hook != null)
-                                {
-                                    m_hooks.add(hook);
-                                }
-                            }
-                        }
-                        catch (Throwable ex)
-                        {
-                            throw new BundleException(
-                                "Resolver hook exception: " + ex.getMessage(),
-                                BundleException.REJECTED_BY_HOOK,
-                                ex);
-                        }
-                    }
-
-                    // Ask hooks to indicate which revisions should not be resolved.
-                    m_whitelist =
-                        new ShrinkableCollection<BundleRevision>(
-                            m_resolverState.getUnresolvedRevisions());
-                    int originalSize = m_whitelist.size();
-                    for (ResolverHook hook : m_hooks)
-                    {
-                        try
-                        {
-                            Felix.m_secureAction
-                                .invokeResolverHookResolvable(hook, m_whitelist);
-                        }
-                        catch (Throwable ex)
-                        {
-                            throw new BundleException(
-                                "Resolver hook exception: " + ex.getMessage(),
-                                BundleException.REJECTED_BY_HOOK,
-                                ex);
-                        }
-                    }
-                    // If nothing was removed, then just null the whitelist
-                    // as an optimization.
-                    if (m_whitelist.size() == originalSize)
-                    {
-                        m_whitelist = null;
-                    }
-
-                    // Check to make sure the target revision is allowed to resolve.
-                    if ((m_whitelist != null) && !m_whitelist.contains(rootRevision))
-                    {
-                        throw new ResolveException(
-                            "Resolver hook prevented resolution.", rootRevision, null);
-                    }
-                }
-
-                // Catch any resolve exception to rethrow later because
-                // we may need to call end() on resolver hooks.
-                ResolveException rethrow = null;
-                try
-                {
-                    // Resolve the revision.
-                    wireMap = m_resolver.resolve(
-                        m_resolverState, rootRevision, m_resolverState.getFragments());
-                }
-                catch (ResolveException ex)
-                {
-                    rethrow = ex;
-                }
-
-                // If we have resolver hooks, we must call end() on them.
-                if (!hookRefs.isEmpty())
-                {
-                    // Verify that all resolver hook service references are still valid
-                    // Call end() on resolver hooks.
-                    for (ResolverHook hook : m_hooks)
-                    {
-// TODO: OSGi R4.3/RESOLVER HOOK - We likely need to put these hooks into a map
-//       to their svc ref since we aren't supposed to call end() on unregistered
-//       but currently we call end() on all.
-                        try
-                        {
-                            Felix.m_secureAction.invokeResolverHookEnd(hook);
-                        }
-                        catch (Throwable th)
-                        {
-                            m_logger.log(
-                                Logger.LOG_WARNING, "Resolver hook exception.", th);
-                        }
-                    }
-                    // Verify that all hook service references are still valid
-                    // and unget all resolver hook factories.
-                    boolean invalid = false;
-                    for (ServiceReference<ResolverHookFactory> ref : hookRefs)
-                    {
-                        if (ref.getBundle() == null)
-                        {
-                            invalid = true;
-                        }
-                        m_felix.ungetService(m_felix, ref);
-                    }
-                    if (invalid)
-                    {
-                        throw new BundleException(
-                            "Resolver hook service unregistered during resolve.",
-                            BundleException.REJECTED_BY_HOOK);
-                    }
-                }
-
-                // If the resolve failed, rethrow the exception.
-                if (rethrow != null)
-                {
-                    throw rethrow;
-                }
-
-                // Otherwise, mark all revisions as resolved.
-                markResolvedRevisions(wireMap);
-            }
-            finally
-            {
-                // Clear resolving flag.
-                m_isResolving = false;
-                // Clear whitelist.
-                m_whitelist = null;
-                // Always clear any hooks.
-                m_hooks.clear();
-                // Always release the global lock.
-                m_felix.releaseGlobalLock();
-            }
-
-            fireResolvedEvents(wireMap);
-        }
-    }
-
-// TODO: OSGi R4.3 - Isn't this method just a generalization of the above method?
-//       Can't we combine them and perhaps simplify the various resolve() methods
-//       here and in Felix.java too?
-    void resolve(Set<BundleRevision> revisions) throws ResolveException, BundleException
+    void resolve(
+        Set<BundleRevision> mandatoryRevisions,
+        Set<BundleRevision> optionalRevisions)
+        throws ResolveException, BundleException
     {
         // Acquire global lock.
         boolean locked = m_felix.acquireGlobalLock();
@@ -303,10 +118,21 @@
         try
         {
             // Make our own copy of revisions.
-            revisions = new HashSet<BundleRevision>(revisions);
+            mandatoryRevisions = (mandatoryRevisions.isEmpty())
+                ? mandatoryRevisions : new HashSet<BundleRevision>(mandatoryRevisions);
+            optionalRevisions = (optionalRevisions.isEmpty())
+                ? optionalRevisions : new HashSet<BundleRevision>(optionalRevisions);
 
             // Extensions are resolved differently.
-            for (Iterator<BundleRevision> it = revisions.iterator(); it.hasNext(); )
+            for (Iterator<BundleRevision> it = mandatoryRevisions.iterator(); it.hasNext(); )
+            {
+                BundleImpl bundle = (BundleImpl) it.next().getBundle();
+                if (bundle.isExtension())
+                {
+                    it.remove();
+                }
+            }
+            for (Iterator<BundleRevision> it = optionalRevisions.iterator(); it.hasNext(); )
             {
                 BundleImpl bundle = (BundleImpl) it.next().getBundle();
                 if (bundle.isExtension())
@@ -321,7 +147,18 @@
             if (!hookRefs.isEmpty())
             {
                 // Create triggers list.
-                Collection<BundleRevision> triggers = Collections.unmodifiableSet(revisions);
+                Set<BundleRevision> triggers;
+                if (!mandatoryRevisions.isEmpty() && !optionalRevisions.isEmpty())
+                {
+                    triggers = new HashSet<BundleRevision>(mandatoryRevisions);
+                    triggers.addAll(optionalRevisions);
+                }
+                else
+                {
+                    triggers = (mandatoryRevisions.isEmpty())
+                        ? optionalRevisions : mandatoryRevisions;
+                }
+                triggers = Collections.unmodifiableSet(triggers);
 
                 // Create resolver hook objects by calling begin() on factory.
                 for (ServiceReference<ResolverHookFactory> ref : hookRefs)
@@ -379,8 +216,9 @@
                 // Check to make sure the target revision is allowed to resolve.
                 if (m_whitelist != null)
                 {
-                    revisions.retainAll(m_whitelist);
-                    if (revisions.isEmpty())
+                    mandatoryRevisions.retainAll(m_whitelist);
+                    optionalRevisions.retainAll(m_whitelist);
+                    if (mandatoryRevisions.isEmpty() && optionalRevisions.isEmpty())
                     {
                         throw new ResolveException(
                             "Resolver hook prevented resolution.", null, null);
@@ -395,7 +233,10 @@
             {
                 // Resolve the revision.
                 wireMap = m_resolver.resolve(
-                    m_resolverState, revisions, m_resolverState.getFragments());
+                    m_resolverState,
+                    mandatoryRevisions,
+                    optionalRevisions,
+                    m_resolverState.getFragments());
             }
             catch (ResolveException ex)
             {
diff --git a/framework/src/main/java/org/apache/felix/framework/resolver/Candidates.java b/framework/src/main/java/org/apache/felix/framework/resolver/Candidates.java
index 298cbb0..e291772 100644
--- a/framework/src/main/java/org/apache/felix/framework/resolver/Candidates.java
+++ b/framework/src/main/java/org/apache/felix/framework/resolver/Candidates.java
@@ -46,6 +46,10 @@
 
 class Candidates
 {
+    public static final int MANDATORY = 0;
+    public static final int OPTIONAL = 1;
+    public static final int ON_DEMAND = 2;
+
     // Set of all involved bundle revisions.
     private final Set<BundleRevision> m_involvedRevisions;
     // Maps a capability to requirements that match it.
@@ -106,12 +110,77 @@
     }
 
     /**
-     * Populates additional candidates for the specified module.
+     * Populates candidates for the specified revision. How a revision is
+     * resolved depends on its resolution type as follows:
+     * <ul>
+     *   <li><tt>MANDATORY</tt> - must resolve and failure to do so throws
+     *       an exception.</li>
+     *   <li><tt>OPTIONAL</tt> - attempt to resolve, but no exception is thrown
+     *       if the resolve fails.</li>
+     *   <li><tt>ON_DEMAND</tt> - only resolve on demand; this only applies to
+     *       fragments and will only resolve a fragment if its host is already
+     *       selected as a candidate.</li>
+     * </ul>
      * @param state the resolver state used for populating the candidates.
-     * @param revision the module whose candidates should be populated.
+     * @param revision the revision whose candidates should be populated.
+     * @param resolution indicates the resolution type.
+     */
+    public final void populate(
+        ResolverState state, BundleRevision revision, int resolution)
+    {
+        // Get the current result cache value, to make sure the revision
+        // hasn't already been populated.
+        Object cacheValue = m_populateResultCache.get(revision);
+        // Has been unsuccessfully populated.
+        if (cacheValue instanceof ResolveException)
+        {
+            return;
+        }
+        // Has been successfully populated.
+        else if (cacheValue instanceof Boolean)
+        {
+            return;
+        }
+
+        // We will always attempt to populate fragments, since this is necessary
+        // for ondemand attaching of fragment. However, we'll only attempt to
+        // populate optional non-fragment revisions if they aren't already
+        // resolved.
+        boolean isFragment = Util.isFragment(revision);
+        if (!isFragment && (revision.getWiring() != null))
+        {
+            return;
+        }
+
+        // Always attempt to populate mandatory or optional revisions.
+        // However, for on-demand fragments only populate if their host
+        // is already populated.
+        if ((resolution != ON_DEMAND)
+            || (isFragment && populateFragment(state, revision)))
+        {
+            try
+            {
+                // Try to populate candidates for the optional revision.
+                populateRevision(state, revision);
+            }
+            catch (ResolveException ex)
+            {
+                // Only throw an exception if resolution is mandatory.
+                if (resolution == MANDATORY)
+                {
+                    throw ex;
+                }
+            }
+        }
+    }
+
+    /**
+     * Populates candidates for the specified revision.
+     * @param state the resolver state used for populating the candidates.
+     * @param revision the revision whose candidates should be populated.
      */
 // TODO: FELIX3 - Modify to not be recursive.
-    public final void populate(ResolverState state, BundleRevision revision)
+    private void populateRevision(ResolverState state, BundleRevision revision)
     {
         // Determine if we've already calculated this revision's candidates.
         // The result cache will have one of three values:
@@ -249,7 +318,7 @@
                 {
                     try
                     {
-                        populate(state, candCap.getRevision());
+                        populateRevision(state, candCap.getRevision());
                     }
                     catch (ResolveException ex)
                     {
@@ -343,120 +412,63 @@
         }
     }
 
-// TODO: OSGi R4.3 - Related to resolve() method clean up, can this just
-//       become the normal case? Currently, it just swallows the resolve
-//       exception, which would have to change.
-    public final boolean populate(
-        ResolverState state, BundleRevision revision, boolean isGreedyAttach)
+    private boolean populateFragment(ResolverState state, BundleRevision revision)
+        throws ResolveException
     {
-        // Get the current result cache value, to make sure the revision
-        // hasn't already been populated.
-        Object cacheValue = m_populateResultCache.get(revision);
-        // Has been unsuccessfully populated.
-        if (cacheValue instanceof ResolveException)
+        // Create a modifiable list of the revision's requirements.
+        List<BundleRequirement> remainingReqs =
+            new ArrayList(revision.getDeclaredRequirements(null));
+        // Find the host requirement.
+        BundleRequirement hostReq = null;
+        for (Iterator<BundleRequirement> it = remainingReqs.iterator();
+            it.hasNext(); )
         {
-            return false;
-        }
-        // Has been successfully populated.
-        else if (cacheValue instanceof Boolean)
-        {
-            return true;
-        }
-
-        // We will always attempt to populate fragments, since this is necessary
-        // for greedy attaching of fragment. However, we'll only attempt to
-        // populate optional non-fragment revisions if they aren't already
-        // resolved.
-        boolean isFragment = Util.isFragment(revision);
-        if (!isFragment && (revision.getWiring() != null))
-        {
-            return false;
-        }
-
-        try
-        {
-            // If the optional revision is a fragment and this is a greedy attach,
-            // then only populate the fragment if it has a candidate host in the
-            // set of already populated revisions. We do this to avoid resolving
-            // unneeded fragments and hosts. If the fragment has a host, we'll
-            // prepopulate the result cache here to avoid having to do the host
-            // lookup again in populate().
-            if (isGreedyAttach && isFragment)
+            BundleRequirement r = it.next();
+            if (r.getNamespace().equals(BundleRevision.HOST_NAMESPACE))
             {
-                // Create a modifiable list of the revision's requirements.
-                List<BundleRequirement> remainingReqs =
-                    new ArrayList(revision.getDeclaredRequirements(null));
-
-                // Find the host requirement.
-                BundleRequirement hostReq = null;
-                for (Iterator<BundleRequirement> it = remainingReqs.iterator();
-                    it.hasNext(); )
-                {
-                    BundleRequirement r = it.next();
-                    if (r.getNamespace().equals(BundleRevision.HOST_NAMESPACE))
-                    {
-                        hostReq = r;
-                        it.remove();
-                        break;
-                    }
-                }
-
-                // Get candidates hosts and keep any that have been populated.
-                SortedSet<BundleCapability> hosts =
-                    state.getCandidates((BundleRequirementImpl) hostReq, false);
-                for (Iterator<BundleCapability> it = hosts.iterator(); it.hasNext(); )
-                {
-                    BundleCapability host = it.next();
-                    if (!isPopulated(host.getRevision()))
-                    {
-                        it.remove();
-                    }
-                }
-
-                // If there aren't any populated hosts, then we can just
-                // return since this fragment isn't needed.
-                if (hosts.isEmpty())
-                {
-                    return false;
-                }
-
-                // If there are populates host candidates, then finish up
-                // some other checks and prepopulate the result cache with
-                // the work we've done so far.
-
-                // Verify that any required execution environment is satisfied.
-                state.checkExecutionEnvironment(revision);
-
-                // Verify that any native libraries match the current platform.
-                state.checkNativeLibraries(revision);
-
-                // Record cycle count, but start at -1 since it will
-                // be incremented again in populate().
-                Integer cycleCount = new Integer(-1);
-
-                // Create a local map for populating candidates first, just in case
-                // the revision is not resolvable.
-                Map<BundleRequirement, SortedSet<BundleCapability>> localCandidateMap =
-                    new HashMap<BundleRequirement, SortedSet<BundleCapability>>();
-
-                // Add the discovered host candidates to the local candidate map.
-                localCandidateMap.put(hostReq, hosts);
-
-                // Add these value to the result cache so we know we are
-                // in the middle of populating candidates for the current
-                // revision.
-                m_populateResultCache.put(revision,
-                    new Object[] { cycleCount, localCandidateMap, remainingReqs });
+                hostReq = r;
+                it.remove();
+                break;
             }
-
-            // Try to populate candidates for the optional revision.
-            populate(state, revision);
         }
-        catch (ResolveException ex)
+        // Get candidates hosts and keep any that have been populated.
+        SortedSet<BundleCapability> hosts =
+            state.getCandidates((BundleRequirementImpl) hostReq, false);
+        for (Iterator<BundleCapability> it = hosts.iterator(); it.hasNext(); )
+        {
+            BundleCapability host = it.next();
+            if (!isPopulated(host.getRevision()))
+            {
+                it.remove();
+            }
+        }
+        // If there aren't any populated hosts, then we can just
+        // return since this fragment isn't needed.
+        if (hosts.isEmpty())
         {
             return false;
         }
-
+        // If there are populates host candidates, then finish up
+        // some other checks and prepopulate the result cache with
+        // the work we've done so far.
+        // Verify that any required execution environment is satisfied.
+        state.checkExecutionEnvironment(revision);
+        // Verify that any native libraries match the current platform.
+        state.checkNativeLibraries(revision);
+        // Record cycle count, but start at -1 since it will
+        // be incremented again in populate().
+        Integer cycleCount = new Integer(-1);
+        // Create a local map for populating candidates first, just in case
+        // the revision is not resolvable.
+        Map<BundleRequirement, SortedSet<BundleCapability>> localCandidateMap =
+            new HashMap<BundleRequirement, SortedSet<BundleCapability>>();
+        // Add the discovered host candidates to the local candidate map.
+        localCandidateMap.put(hostReq, hosts);
+        // Add these value to the result cache so we know we are
+        // in the middle of populating candidates for the current
+        // revision.
+        m_populateResultCache.put(revision,
+            new Object[] { cycleCount, localCandidateMap, remainingReqs });
         return true;
     }
 
@@ -490,7 +502,7 @@
             {
                 try
                 {
-                    populate(state, candCap.getRevision());
+                    populateRevision(state, candCap.getRevision());
                 }
                 catch (ResolveException ex)
                 {
diff --git a/framework/src/main/java/org/apache/felix/framework/resolver/Resolver.java b/framework/src/main/java/org/apache/felix/framework/resolver/Resolver.java
index 4791991..eabfdd1 100644
--- a/framework/src/main/java/org/apache/felix/framework/resolver/Resolver.java
+++ b/framework/src/main/java/org/apache/felix/framework/resolver/Resolver.java
@@ -29,12 +29,13 @@
 public interface Resolver
 {
     Map<BundleRevision, List<ResolverWire>> resolve(
-        ResolverState state, BundleRevision revision, Set<BundleRevision> optional);
-    Map<BundleRevision, List<ResolverWire>> resolve(
-        ResolverState state, Set<BundleRevision> revisions, Set<BundleRevision> optional);
+        ResolverState state,
+        Set<BundleRevision> mandatoryRevisions,
+        Set<BundleRevision> optionalRevisions,
+        Set<BundleRevision> ondemandFragments);
     Map<BundleRevision, List<ResolverWire>> resolve(
         ResolverState state, BundleRevision revision, String pkgName,
-        Set<BundleRevision> fragments);
+        Set<BundleRevision> ondemandFragments);
 
     public static interface ResolverState
     {
diff --git a/framework/src/main/java/org/apache/felix/framework/resolver/ResolverImpl.java b/framework/src/main/java/org/apache/felix/framework/resolver/ResolverImpl.java
index 46d6c27..68c9c72 100644
--- a/framework/src/main/java/org/apache/felix/framework/resolver/ResolverImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/resolver/ResolverImpl.java
@@ -58,144 +58,15 @@
     }
 
     public Map<BundleRevision, List<ResolverWire>> resolve(
-        ResolverState state, BundleRevision revision, Set<BundleRevision> optional)
+        ResolverState state,
+        Set<BundleRevision> mandatoryRevisions,
+        Set<BundleRevision> optionalRevisions,
+        Set<BundleRevision> ondemandFragments)
     {
-        Map<BundleRevision, List<ResolverWire>> wireMap = new HashMap<BundleRevision, List<ResolverWire>>();
-        Map<BundleRevision, Packages> revisionPkgMap = new HashMap<BundleRevision, Packages>();
-
-        if (revision.getWiring() == null)
-        {
-            boolean retryFragments;
-            do
-            {
-                retryFragments = false;
-
-                try
-                {
-                    // Populate revision's candidates.
-                    Candidates allCandidates = new Candidates();
-                    allCandidates.populate(state, revision);
-
-                    // Try to populate optional fragments.
-                    for (BundleRevision br : optional)
-                    {
-                        allCandidates.populate(state, br, true);
-                    }
-
-                    // Merge any fragments into hosts.
-                    allCandidates.prepare(getResolvedSingletons(state));
-                    // Make sure revision is still valid, since it could
-                    // fail due to fragment and/or singleton selection.
-// TODO: OSGi R4.3 - Could this be merged back into Candidates?
-                    if (!allCandidates.isPopulated(revision))
-                    {
-                        throw allCandidates.getResolveException(revision);
-                    }
-
-                    // Record the initial candidate permutation.
-                    m_usesPermutations.add(allCandidates);
-
-                    ResolveException rethrow = null;
-
-                    // If the requested revision is a fragment, then
-                    // ultimately we will verify the host.
-                    List<BundleRequirement> hostReqs =
-                        revision.getDeclaredRequirements(BundleRevision.HOST_NAMESPACE);
-
-                    BundleRevision target = revision;
-
-                    do
-                    {
-                        rethrow = null;
-
-                        revisionPkgMap.clear();
-                        m_packageSourcesCache.clear();
-
-                        allCandidates = (m_usesPermutations.size() > 0)
-                            ? m_usesPermutations.remove(0)
-                            : m_importPermutations.remove(0);
-//allCandidates.dump();
-
-                        // If we are resolving a fragment, then we
-                        // actually want to verify its host.
-                        if (!hostReqs.isEmpty())
-                        {
-                            target = allCandidates.getCandidates(hostReqs.get(0))
-                                .iterator().next().getRevision();
-                        }
-
-                        calculatePackageSpaces(
-                            allCandidates.getWrappedHost(target), allCandidates, revisionPkgMap,
-                            new HashMap(), new HashSet());
-//System.out.println("+++ PACKAGE SPACES START +++");
-//dumpRevisionPkgMap(revisionPkgMap);
-//System.out.println("+++ PACKAGE SPACES END +++");
-
-                        try
-                        {
-                            checkPackageSpaceConsistency(
-                                false, allCandidates.getWrappedHost(target),
-                                allCandidates, revisionPkgMap, new HashMap());
-                        }
-                        catch (ResolveException ex)
-                        {
-                            rethrow = ex;
-                        }
-                    }
-                    while ((rethrow != null)
-                        && ((m_usesPermutations.size() > 0) || (m_importPermutations.size() > 0)));
-
-                    // If there is a resolve exception, then determine if an
-                    // optionally resolved revision is to blame (typically a fragment).
-                    // If so, then remove the optionally resolved resolved and try
-                    // again; otherwise, rethrow the resolve exception.
-                    if (rethrow != null)
-                    {
-                        BundleRevision faultyRevision =
-                            getActualBundleRevision(rethrow.getRevision());
-                        if (rethrow.getRequirement() instanceof HostedRequirement)
-                        {
-                            faultyRevision =
-                                ((HostedRequirement) rethrow.getRequirement())
-                                    .getOriginalRequirement().getRevision();
-                        }
-                        if (optional.remove(faultyRevision))
-                        {
-                            retryFragments = true;
-                        }
-                        else
-                        {
-                            throw rethrow;
-                        }
-                    }
-                    // If there is no exception to rethrow, then this was a clean
-                    // resolve, so populate the wire map.
-                    else
-                    {
-                        wireMap =
-                            populateWireMap(
-                                allCandidates.getWrappedHost(target),
-                                revisionPkgMap, wireMap, allCandidates);
-                    }
-                }
-                finally
-                {
-                    // Always clear the state.
-                    m_usesPermutations.clear();
-                    m_importPermutations.clear();
-                }
-            }
-            while (retryFragments);
-        }
-
-        return wireMap;
-    }
-
-    public Map<BundleRevision, List<ResolverWire>> resolve(
-        ResolverState state, Set<BundleRevision> revisions, Set<BundleRevision> optional)
-    {
-        Map<BundleRevision, List<ResolverWire>> wireMap = new HashMap<BundleRevision, List<ResolverWire>>();
-        Map<BundleRevision, Packages> revisionPkgMap = new HashMap<BundleRevision, Packages>();
+        Map<BundleRevision, List<ResolverWire>> wireMap =
+            new HashMap<BundleRevision, List<ResolverWire>>();
+        Map<BundleRevision, Packages> revisionPkgMap =
+            new HashMap<BundleRevision, Packages>();
 
         boolean retry;
         do
@@ -207,33 +78,70 @@
                 // Create object to hold all candidates.
                 Candidates allCandidates = new Candidates();
 
-                // Populate revisions.
-                for (Iterator<BundleRevision> it = revisions.iterator(); it.hasNext(); )
+                // Populate mandatory revisions; since these are mandatory
+                // revisions, failure throws a resolve exception.
+                for (Iterator<BundleRevision> it = mandatoryRevisions.iterator();
+                    it.hasNext(); )
                 {
                     BundleRevision br = it.next();
-                    if ((!Util.isFragment(br) && br.getWiring() != null)
-                        || !allCandidates.populate(state, br, false))
+                    if (Util.isFragment(br) || (br.getWiring() == null))
+                    {
+                        allCandidates.populate(state, br, Candidates.MANDATORY);
+                    }
+                    else
                     {
                         it.remove();
                     }
                 }
 
-                // Try to populate optional fragments.
-                for (BundleRevision br : optional)
+                // Populate optional revisions; since these are optional
+                // revisions, failure does not throw a resolve exception.
+                for (BundleRevision br : optionalRevisions)
                 {
-                    allCandidates.populate(state, br, true);
+                    boolean isFragment = Util.isFragment(br);
+                    if (isFragment || (br.getWiring() == null))
+                    {
+                        allCandidates.populate(state, br, Candidates.OPTIONAL);
+                    }
+                }
+
+                // Populate ondemand fragments; since these are optional
+                // revisions, failure does not throw a resolve exception.
+                for (BundleRevision br : ondemandFragments)
+                {
+                    boolean isFragment = Util.isFragment(br);
+                    if (isFragment)
+                    {
+                        allCandidates.populate(state, br, Candidates.ON_DEMAND);
+                    }
                 }
 
                 // Merge any fragments into hosts.
                 allCandidates.prepare(getResolvedSingletons(state));
 
-                // Prune failed revisions.
+                // Make sure mandatory revisions are still resolved,
+                // since they could fail due to fragment and/or singleton
+                // selection.
 // TODO: OSGi R4.3 - Could this be merged back into Candidates?
-                for (Iterator<BundleRevision> it = revisions.iterator(); it.hasNext(); )
+                for (BundleRevision br : mandatoryRevisions)
                 {
-                    if (!allCandidates.isPopulated(it.next()))
+                    if (!allCandidates.isPopulated(br))
                     {
-                        it.remove();
+                        throw allCandidates.getResolveException(br);
+                    }
+                }
+
+                // Create a combined list of populated revisions; for
+                // optional revisions. We do not need to consider ondemand
+                // fragments, since they will only be pulled in if their
+                // host is already present.
+                Set<BundleRevision> allRevisions =
+                    new HashSet<BundleRevision>(mandatoryRevisions);
+                for (BundleRevision br : optionalRevisions)
+                {
+                    if (allCandidates.isPopulated(br))
+                    {
+                        allRevisions.add(br);
                     }
                 }
 
@@ -242,15 +150,19 @@
 
                 ResolveException rethrow = null;
 
-                // If the requested revision is a fragment, then
-                // ultimately we will verify the host., so store
-                // any host requirements
+                // If a populated revision is a fragment, then its host
+                // must ultimately be verified, so store its host requirement
+                // to use for package space calculation.
                 Map<BundleRevision, List<BundleRequirement>> hostReqs =
                     new HashMap<BundleRevision, List<BundleRequirement>>();
-                for (BundleRevision br : revisions)
+                for (BundleRevision br : allRevisions)
                 {
-                    hostReqs.put(
-                        br, br.getDeclaredRequirements(BundleRevision.HOST_NAMESPACE));
+                    if (Util.isFragment(br))
+                    {
+                        hostReqs.put(
+                            br,
+                            br.getDeclaredRequirements(BundleRevision.HOST_NAMESPACE));
+                    }
                 }
 
                 do
@@ -265,14 +177,14 @@
                         : m_importPermutations.remove(0);
 //allCandidates.dump();
 
-                    for (BundleRevision br : revisions)
+                    for (BundleRevision br : allRevisions)
                     {
                         BundleRevision target = br;
 
-                        // If we are resolving a fragment, then we
-                        // actually want to verify its host.
+                        // If we are resolving a fragment, then get its
+                        // host candidate and verify it instead.
                         List<BundleRequirement> hostReq = hostReqs.get(br);
-                        if (!hostReq.isEmpty())
+                        if (hostReq != null)
                         {
                             target = allCandidates.getCandidates(hostReq.get(0))
                                 .iterator().next().getRevision();
@@ -314,11 +226,11 @@
                             ((HostedRequirement) rethrow.getRequirement())
                                 .getOriginalRequirement().getRevision();
                     }
-                    if (revisions.remove(faultyRevision))
+                    if (optionalRevisions.remove(faultyRevision))
                     {
                         retry = true;
                     }
-                    else if (optional.remove(faultyRevision))
+                    else if (ondemandFragments.remove(faultyRevision))
                     {
                         retry = true;
                     }
@@ -331,14 +243,14 @@
                 // resolve, so populate the wire map.
                 else
                 {
-                    for (BundleRevision br : revisions)
+                    for (BundleRevision br : allRevisions)
                     {
                         BundleRevision target = br;
 
                         // If we are resolving a fragment, then we
                         // actually want to populate its host's wires.
                         List<BundleRequirement> hostReq = hostReqs.get(br);
-                        if (!hostReq.isEmpty())
+                        if (hostReq != null)
                         {
                             target = allCandidates.getCandidates(hostReq.get(0))
                                 .iterator().next().getRevision();
@@ -368,7 +280,7 @@
 
     public Map<BundleRevision, List<ResolverWire>> resolve(
         ResolverState state, BundleRevision revision, String pkgName,
-        Set<BundleRevision> optional)
+        Set<BundleRevision> ondemandFragments)
     {
         // We can only create a dynamic import if the following
         // conditions are met:
@@ -386,21 +298,25 @@
             Map<BundleRevision, List<ResolverWire>> wireMap = new HashMap<BundleRevision, List<ResolverWire>>();
             Map<BundleRevision, Packages> revisionPkgMap = new HashMap<BundleRevision, Packages>();
 
-            boolean retryFragments;
+            boolean retry;
             do
             {
-                retryFragments = false;
+                retry = false;
 
                 try
                 {
                     // Try to populate optional fragments.
-                    for (BundleRevision br : optional)
+                    for (BundleRevision br : ondemandFragments)
                     {
-                        allCandidates.populate(state, br, true);
+                        if (Util.isFragment(br))
+                        {
+                            allCandidates.populate(state, br, Candidates.ON_DEMAND);
+                        }
                     }
 
                     // Merge any fragments into hosts.
                     allCandidates.prepare(getResolvedSingletons(state));
+
                     // Make sure revision is still valid, since it could
                     // fail due to fragment and/or singleton selection.
 // TODO: OSGi R4.3 - Could this be merged back into Candidates?
@@ -466,9 +382,9 @@
                                 ((HostedRequirement) rethrow.getRequirement())
                                     .getOriginalRequirement().getRevision();
                         }
-                        if (optional.remove(faultyRevision))
+                        if (ondemandFragments.remove(faultyRevision))
                         {
-                            retryFragments = true;
+                            retry = true;
                         }
                         else
                         {
@@ -491,7 +407,7 @@
                     m_importPermutations.clear();
                 }
             }
-            while (retryFragments);
+            while (retry);
         }
 
         return null;