Get rid of no longer used shadow dependents logic

1 files changed, 20 insertions, 295 deletions

diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 9a92df7..fa97cad 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -1347,14 +1347,13 @@ namespace bpkg
     //
     package_configurations dependency_configurations;
 
-    // Shadow dependencies and clusters.
+    // Shadow clusters.
     //
     // See the collect lambda in collect_build_prerequisites() for details.
     //
     using positions = small_vector<pair<size_t, size_t>, 1>;
     using shadow_dependents_map = map<package_key, positions>;
 
-    shadow_dependents_map shadow_dependents;
     shadow_dependents_map shadow_cluster;
 
     // Absent -- not negotiated yet, false -- being negotiated, true -- has
@@ -1548,61 +1547,6 @@ namespace bpkg
       }
       else
         depth = c.depth;
-
-      // Merge shadow dependents.
-      //
-      // This is only necessary if all the merged non-shadow configurations
-      // have already been negotiated (otherwise we throw and replace the
-      // shadow with newly merged non-shadow).
-      //
-      for (auto& d: c.shadow_dependents)
-      {
-        auto i (shadow_dependents.find (d.first));
-
-        if (i != shadow_dependents.end ())
-        {
-          const positions& sps (d.second);
-          positions& dps (i->second);
-
-          for (const auto& sp: sps)
-          {
-            if (find (dps.begin (), dps.end (), sp) == dps.end ())
-              dps.push_back (sp);
-          }
-        }
-        else
-          shadow_dependents.emplace (d.first, move (d.second));
-      }
-    }
-
-    void
-    add_shadow (package_key dependent, pair<size_t, size_t> pos)
-    {
-      auto i (shadow_dependents.find (dependent));
-
-      if (i != shadow_dependents.end ())
-      {
-        positions& ps (i->second);
-        if (find (ps.begin (), ps.end (), pos) == ps.end ())
-          ps.push_back (pos);
-      }
-      else
-        shadow_dependents.emplace (move (dependent), positions ({pos}));
-    }
-
-    bool
-    contains_shadow_dependent (package_key dependent,
-                               pair<size_t, size_t> pos) const
-    {
-      auto i (shadow_dependents.find (dependent));
-
-      if (i != shadow_dependents.end ())
-      {
-        const positions& ps (i->second);
-        return find (ps.begin (), ps.end (), pos) != ps.end ();
-      }
-      else
-        return false;
     }
 
     void
@@ -2079,9 +2023,6 @@ namespace bpkg
   {
     size_t               depth;
     package_key          dependent;
-#if 0 // @@ TMP
-    pair<size_t, size_t> position;
-#endif
   };
 
   struct merge_configuration
@@ -4475,7 +4416,6 @@ namespace bpkg
               return false; // Cases (1) or (2).
             else
             {
-#if 1
               // Case (3).
               //
               // There is just one complication:
@@ -4521,20 +4461,20 @@ namespace bpkg
                 // clusters that will have no corresponding try-catch frames
                 // (because we may unwind them in the process).
                 //
-                // So the approach we will use is the "shadow" idea but for
-                // merging clusters rather than up-negotiating a
-                // configuration. Specifically, we throw merge_configuration
-                // to the outer try/catch. At the catch site we make the newly
-                // merged cluster a shadow of the restored cluster and retry
-                // the same steps similar to retry_configuration. As we redo
-                // these steps, we consult the shadow cluster and if the
-                // dependent/dependency entry is there, then instead of adding
-                // it to another (new/existing) cluster that would later be
-                // merged into this non-shadow cluster, we add it directly to
-                // the non-shadow cluster (potentially merging other cluster
-                // which it feels like by definition should all be already
-                // fully negotiated). The end result is that once we reach
-                // this point again, there will be nothing to merge.
+                // So the approach we will use is the "shadow" idea for
+                // merging clusters. Specifically, we throw
+                // merge_configuration to the outer try/catch. At the catch
+                // site we make the newly merged cluster a shadow of the
+                // restored cluster and retry the same steps similar to
+                // retry_configuration. As we redo these steps, we consult the
+                // shadow cluster and if the dependent/dependency entry is
+                // there, then instead of adding it to another (new/existing)
+                // cluster that would later be merged into this non-shadow
+                // cluster, we add it directly to the non-shadow cluster
+                // (potentially merging other cluster which it feels like by
+                // definition should all be already fully negotiated). The end
+                // result is that once we reach this point again, there will
+                // be nothing to merge.
                 //
                 // The shadow check is part of postponed_configs::add().
                 //
@@ -4633,7 +4573,7 @@ namespace bpkg
                 //
                 dep_chain.clear ();
 
-                throw retry_configuration {cfg.depth, move (pk)/*, dp*/}; // @@ TMP
+                throw retry_configuration {cfg.depth, move (pk)};
               }
 
               l5 ([&]{trace << "configuration for cfg-postponed "
@@ -4717,209 +4657,6 @@ namespace bpkg
               }
 
               return true;
-#else
-              //@@ TMP: get rid of shadow_dependents (including in
-              //        retry_configuration) if pans out.
-
-              // Case (3).
-              //
-              // If this is the first time we are here (see below), then we
-              // up-negotiate the configuration and throw retry_configuration
-              // to the try/catch frame corresponding to the negotiation of
-              // the outermost merged cluster in order to retry the same steps
-              // (potentially refining the configuration as we go along) and
-              // likely (but not necessarily) ending up here again, at which
-              // point we do not need to re-do the up-negotiation and can just
-              // merge this dependent/dependency into the cluster. To
-              // distinguish these "first time"/"second time" cases the
-              // cluster contains the shadow dependents map (see the catch
-              // site for details).
-              //
-              // Note: skip this test if the dependent was added via the
-              // shadow cluster logic (see below).
-              //
-              if (r.second && !cfg.contains_shadow_dependent (pk, dp))
-              {
-                // The "first time" case.
-                //
-
-                // @@ TODO: if configuration has not changed, then we don't
-                //    need to throw. Do it at the end (there is no harm in
-                //    throwing even if not needed and it will change test
-                //    trace).
-                //
-                //    Also, we may have to throw multiple times if the
-                //    configuration gets refined... In a sense, semantically,
-                //    we should act like a one more iteration of the initial
-                //    negotiation loop with the exception acting like a
-                //    request to restart the refinement process from the
-                //    beginning.
-                //
-                //    It feels like the unchanged case semantics should be
-                //    like falling through to the "second time" case. Though
-                //    I wonder if there could be some missing steps, like
-                //    collecting dependencies? Doesn't feel like it since
-                //    we are re-using existing configuration which presumably
-                //    means all this has already been done.
-                //
-                //    Hm, won't we need to "reset" dependency package
-                //    skeletons (which have by now potentially seen
-                //    evaluate_*() calls) in order to negotiate. And if the
-                //    configuration hasn't changed, then how will we restore
-                //    them? Maybe we should make copies (that are "reset" to
-                //    the initial state) and use that?
-                //
-
-                l5 ([&]{trace << "cfg-postponing dependent "
-                              << pkg.available_name_version_db ()
-                              << " involves negotiated configurations and "
-                              << "results in " << cfg
-                              << ", throwing retry_configuration";});
-
-                // up_negotiate (...);
-
-                // Don't print the "while satisfying..." chain.
-                //
-                dep_chain.clear ();
-
-                throw retry_configuration {cfg.depth, move (pk), dp};
-              }
-              else
-              {
-                // The "second time" case.
-                //
-                // There is just one complication:
-                //
-                // If all the merged clusters are already negotiated, then all
-                // is good: all the dependencies in cfg_deps have been
-                // collected recursively as part of the configuration
-                // negotiation (because everything in this cluster is already
-                // negotiated) and we can return true (no need to postpone any
-                // further steps).
-                //
-                // But if we do have clusters not yet negotiated, or, worse,
-                // being in the middle of negotiation, then we need to get
-                // this merged cluster into the fully negotiated state. The
-                // we do it is by throwing merge_configuration (see below).
-                //
-                // When we are back here after throwing merge_configuration,
-                // then all the clusters have been pre-merged and our call to
-                // add() shouldn't have added any new cluster. In this case
-                // the cluster can either be already negotiated or being
-                // negotiated and we can proceed as in the "everything is
-                // negotiated case" above (we just need to get the the
-                // dependencies that we care about into the recursively
-                // collected state).
-                //
-                l5 ([&]{trace << "dependent "
-                              << pkg.available_name_version_db ()
-                              << " is a shadow dependent for " << cfg;});
-
-                if (!r.second || // Shadow cluster was/is being negotiated.
-                    *r.second)   // Everything already negotiated.
-                {
-                  // The everything already negotiated (sort of) case.
-                  //
-                  l5 ([&]{trace << "configuration for cfg-postponed "
-                                << "dependencies of dependent "
-                                << pkg.available_name_version_db ()
-                                << " is negotiated";});
-
-                  // Note that we may still add extra dependencies to this
-                  // cluster which we still need to configure and recursively
-                  // collect before indicating to the caller (returning true)
-                  // that we are done with this depends value and the
-                  // dependent is not postponed.
-                  //
-                  // @@ TODO call verify_sensible(), etc. DO NOW.
-                  //
-                  for (const package_key& p: cfg_deps)
-                  {
-                    build_package* b (entered_build (p));
-                    assert (b != nullptr);
-
-                    if (!b->recursive_collection)
-                    {
-                      l5 ([&]{trace << "collecting cfg-postponed dependency "
-                                    << b->available_name_version_db ()
-                                    << " of dependent "
-                                    << pkg.available_name_version_db ();});
-
-                      collect_build_prerequisites (options,
-                                                   *b,
-                                                   fdb,
-                                                   rpt_depts,
-                                                   apc,
-                                                   initial_collection,
-                                                   replaced_vers,
-                                                   dep_chain,
-                                                   postponed_repo,
-                                                   postponed_alts,
-                                                   0 /* max_alt_index */,
-                                                   postponed_deps,
-                                                   postponed_cfgs,
-                                                   postponed_poss,
-                                                   true /* force_configured */);
-                    }
-                    else
-                      l5 ([&]{trace << "dependency "
-                                    << b->available_name_version_db ()
-                                    << " of dependent "
-                                    << pkg.available_name_version_db ()
-                                    << " is already (being) recursively "
-                                    << "collected, skipping";});
-                  }
-
-                  return true;
-                }
-                else
-                {
-                  // The partially negotiated case.
-                  //
-                  // Handling this in a straightforward way is not easy due to
-                  // the being negotiated cases -- we have code up the stack
-                  // that is in the middle of the negotiation logic.
-                  //
-                  // Another idea is to again throw to the outer try/catch
-                  // frame (thus unwinding all the being negotiated code) and
-                  // complete the work there. The problem with this approach
-                  // is that without restoring the state we may end up with
-                  // unrelated clusters that will have no corresponding
-                  // try-catch frames (because we may unwind them in the
-                  // process).
-                  //
-                  // So the approach we will use is the "shadow" idea but for
-                  // merging clusters rather than up-negotiating a
-                  // configuration. Specifically, we throw merge_configuration
-                  // to the outer try/catch. At the catch site we make the
-                  // newly merged cluster a shadow of the restored cluster and
-                  // retry the same steps similar to retry_configuration. As
-                  // we redo these steps, we consult the shadow cluster and if
-                  // the dependent/dependency entry is there, then instead of
-                  // adding it to another (new/existing) cluster that would
-                  // later be merged into this non-shadow cluster, we add it
-                  // directly to the non-shadow cluster (potentially merging
-                  // other cluster which it feels like by definition should
-                  // all be already fully negotiated). The end result is that
-                  // once we reach this point again, there will be nothing to
-                  // merge.
-                  //
-                  // The shadow check is part of postponed_configs::add().
-                  //
-                  l5 ([&]{trace << "cfg-postponing dependent "
-                                << pkg.available_name_version_db ()
-                                << " involves non-negotiated configurations "
-                                << "and results in " << cfg << ", throwing "
-                                << "merge_configuration";});
-
-                  // Don't print the "while satisfying..." chain.
-                  //
-                  dep_chain.clear ();
-
-                  throw merge_configuration {cfg.depth};
-                }
-              }
-#endif
             }
           }
 
@@ -6513,8 +6250,8 @@ namespace bpkg
 
               pc = &postponed_cfgs[ci];
 
-              // Note that in this case we keep the accumulated configuration
-              // and shadow dependents, if any.
+              // Note that in this case we keep the accumulated configuration,
+              // if any.
 
               pc->depth = 0;
 
@@ -6597,17 +6334,6 @@ namespace bpkg
                     v.confirmed = false;
 
               pc->dependency_configurations = move (cfgs);
-
-#if 0
-              // @@ TMP drop if pans out.
-
-              // Record the dependent/dependencies/position that triggered
-              // this so that we don't repeat this up-negotiate/throw/catch
-              // dance when we retry collect_build_postponed(). @@ Hm. could
-              // this not be re-done via "unchanged configuration"?
-              //
-              pc->add_shadow (move (e.dependent), e.position);
-#endif
             }
             catch (merge_configuration& e)
             {
@@ -6632,11 +6358,10 @@ namespace bpkg
 
               assert (!pc->negotiated);
 
-              // Drop any accumulated configuration and shadow dependents
-              // (which could be carried over from retry_configuration logic).
+              // Drop any accumulated configuration (which could be carried
+              // over from retry_configuration logic).
               //
               pc->dependency_configurations.clear ();
-              pc->shadow_dependents.clear ();
 
               l5 ([&]{trace << "cfg-negotiation of " << *pc << " failed due "
                             << "to non-negotiated clusters, force-merging "