Fix re-evaluating multiple existing dependents with config clause of common dependency

2 files changed, 138 insertions, 19 deletions

diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx
index 729c145..83bb96a 100644
--- a/bpkg/pkg-build-collect.cxx
+++ b/bpkg/pkg-build-collect.cxx
@@ -1606,7 +1606,9 @@ namespace bpkg
       {
         for (existing_dependent& ed: eds)
         {
-          package_key dpk (ed.db, ed.selected->name);
+          package_key dpt (ed.db, ed.selected->name);
+          package_key dep (pk);
+
           size_t& di (ed.dependency_position.first);
 
           const build_package* bp (&pkg);
@@ -1623,7 +1625,7 @@ namespace bpkg
           // for details).
           //
           {
-            auto pi (postponed_poss.find (dpk));
+            auto pi (postponed_poss.find (dpt));
 
             if (pi != postponed_poss.end () && pi->second.first < di)
             {
@@ -1654,7 +1656,7 @@ namespace bpkg
                   replaced_vers,
                   postponed_cfgs);
 
-                pk = package_key (bp->db, bp->name ());
+                dep = package_key (bp->db, bp->name ());
 
                 // Note that here we side-step the bogus logic (by not setting
                 // the skipped flag) because in this case (replace=true) our
@@ -1676,7 +1678,7 @@ namespace bpkg
           for (const postponed_configuration& cfg: postponed_cfgs)
           {
             if (const pair<size_t, size_t>* p =
-                cfg.existing_dependent_position (pk))
+                cfg.existing_dependent_position (dpt))
             {
               size_t ei (p->first);
 
@@ -1687,7 +1689,7 @@ namespace bpkg
                 postponed_position pp (ed.dependency_position,
                                        false /* replace */);
 
-                auto p (postponed_poss.emplace (move (pk), pp));
+                auto p (postponed_poss.emplace (move (dpt), pp));
                 if (!p.second)
                 {
                   assert (p.first->second > pp);
@@ -1726,7 +1728,23 @@ namespace bpkg
                         << " of existing dependent " << *ed.selected
                         << ed.db;});
 
-          postponed_cfgs.add (move (dpk), ed.dependency_position, move (pk));
+          // Only add this dependent/dependency to the newly created cluster
+          // if this dependency doesn't belong to any cluster yet, which may
+          // not be the case if there are multiple existing dependents with
+          // configuration clause for this dependency.
+          //
+          // To put it another way, if there are multiple such an existing
+          // dependents for this dependency, here we will create the
+          // configuration cluster only for the first one. The remaining
+          // dependents will be added to this dependency's cluster when the
+          // existing dependents of dependencies in this cluster are all
+          // discovered and reevaluated (see collect_build_postponed() for
+          // details).
+          //
+          if (postponed_cfgs.find_dependency (dep) == nullptr)
+            postponed_cfgs.add (move (dpt),
+                                ed.dependency_position,
+                                move (dep));
         }
 
         return;
@@ -1861,12 +1879,12 @@ namespace bpkg
     package_skeleton& skel (*pkg.skeleton);
 
     auto fail_reeval = [&pkg] ()
-      {
-        fail << "unable to re-create dependency information of already "
-        << "configured package " << pkg.available_name_version_db () <<
+    {
+      fail << "unable to re-create dependency information of already "
+           << "configured package " << pkg.available_name_version_db () <<
         info << "likely cause is change in external environment" <<
         info << "consider resetting the build configuration";
-      };
+    };
 
     bool postponed (false);
     bool reevaluated (false);
@@ -2633,14 +2651,14 @@ namespace bpkg
               reused = false;
 
             r.push_back (prebuild {d,
-                  *ddb,
-                  move (dsp),
-                  move (dap),
-                  move (rp.second),
-                  system,
-                  specified,
-                  force,
-                  ru});
+                                   *ddb,
+                                   move (dsp),
+                                   move (dap),
+                                   move (rp.second),
+                                   system,
+                                   specified,
+                                   force,
+                                    ru});
           }
 
           // Now, as we have pre-collected the dependency builds, go through
@@ -5292,7 +5310,7 @@ namespace bpkg
             // Note that in this case we keep the accumulated configuration,
             // if any.
 
-            pc->depth = 0;
+            pc->depth = 0; // Mark as non-negotiated.
 
             // If requested, "replace" the "later" dependent-dependency
             // cluster with an earlier.
diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript
index a891ca3..945402b 100644
--- a/tests/pkg-build.testscript
+++ b/tests/pkg-build.testscript
@@ -6060,6 +6060,107 @@ test.arguments += --sys-no-query
           $pkg_drop libfoo
         }
 
+        : multiple-dependents
+        :
+        {
+          $clone_cfg;
+
+          $* libfoo/0.1.0 foo/1.0.0 fox/1.0.0 2>!;
+
+          $pkg_status -r >>EOO;
+            !libfoo configured !0.1.0 available 1.0.0
+            !foo configured !1.0.0
+              !libfoo configured !0.1.0 available 1.0.0
+            !fox configured !1.0.0
+              !libfoo configured !0.1.0 available 1.0.0
+            EOO
+
+          $* libfoo 2>>~%EOE%;
+            %.*
+            trace: pkg_build: refine package collection/plan execution from scratch
+            %.*
+            trace: collect_build: add libfoo/1.0.0
+            %.*
+            trace: collect_build_prerequisites: cfg-postpone dependency libfoo/1.0.0 of existing dependent foo/1.0.0
+            trace: postponed_configurations::add: create {foo^ | libfoo->{foo/1,1}}
+            trace: collect_build_prerequisites: cfg-postpone dependency libfoo/1.0.0 of existing dependent fox/1.0.0
+            trace: collect_build_postponed (0): begin
+            trace: collect_build_postponed (1): begin {foo^ | libfoo->{foo/1,1}}
+            %.*
+            trace: collect_build_postponed (1): re-evaluate existing dependents for {foo^ | libfoo->{foo/1,1}}
+            %.*
+            trace: collect_build: add foo/1.0.0
+            trace: collect_build_prerequisites: reeval foo/1.0.0
+            %.*
+            trace: collect_build: pick libfoo/1.0.0 over libfoo/0.1.0
+            trace: postponed_configurations::add: add {foo^ 1,1: libfoo} to {foo^ | libfoo->{foo/1,1}}
+            trace: collect_build_prerequisites: re-evaluating dependent foo/1.0.0 results in {foo^ | libfoo->{foo/1,1}}
+            trace: collect_build_prerequisites: re-evaluated foo/1.0.0
+            %.*
+            trace: collect_build: add fox/1.0.0
+            trace: collect_build_prerequisites: reeval fox/1.0.0
+            %.*
+            trace: collect_build: pick libfoo/1.0.0 over libfoo/0.1.0
+            trace: postponed_configurations::add: add {fox^ 1,1: libfoo} to {foo^ | libfoo->{foo/1,1}}
+            trace: collect_build_prerequisites: re-evaluating dependent fox/1.0.0 results in {foo^ fox^ | libfoo->{foo/1,1 fox/1,1}}
+            trace: collect_build_prerequisites: re-evaluated fox/1.0.0
+            trace: collect_build_postponed (1): cfg-negotiate begin {foo^ fox^ | libfoo->{foo/1,1 fox/1,1}}
+            %.*
+            trace: collect_build_postponed (1): recursively collect cfg-negotiated dependencies
+            trace: collect_build_prerequisites: begin libfoo/1.0.0
+            trace: collect_build_prerequisites: end libfoo/1.0.0
+            trace: collect_build_postponed (1): recursively collect cfg-negotiated dependents
+            trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent foo/1.0.0
+            trace: collect_build_prerequisites: resume foo/1.0.0
+            trace: collect_build_prerequisites: end foo/1.0.0
+            trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent fox/1.0.0
+            trace: collect_build_prerequisites: resume fox/1.0.0
+            trace: collect_build_prerequisites: end fox/1.0.0
+            trace: collect_build_postponed (1): cfg-negotiate end {foo^ fox^ | libfoo->{foo/1,1 fox/1,1}}!
+            trace: collect_build_postponed (1): end {foo^ | libfoo->{foo/1,1}}
+            trace: collect_build_postponed (0): end
+            %.*
+            trace: execute_plan: simulate: yes
+            %.*
+            build plan:
+              upgrade libfoo/1.0.0
+                config.libfoo.extras=true (set by foo)
+              reconfigure fox/1.0.0 (dependent of libfoo)
+              reconfigure foo/1.0.0 (dependent of libfoo)
+            %.*
+            disfigured foo/1.0.0
+            %.*
+            disfigured fox/1.0.0
+            %.*
+            disfigured libfoo/0.1.0
+            %.*
+            fetched libfoo/1.0.0
+            %.*
+            unpacked libfoo/1.0.0
+            %.*
+            configured libfoo/1.0.0
+            %.*
+            configured foo/1.0.0
+            %.*
+            updated libfoo/1.0.0
+            %.*
+            updated fox/1.0.0
+            %.*
+            updated foo/1.0.0
+            %.*
+            EOE
+
+          $pkg_status -r >>EOO;
+            !libfoo configured 1.0.0
+            !foo configured !1.0.0
+              !libfoo configured 1.0.0
+            !fox configured !1.0.0
+              !libfoo configured 1.0.0
+            EOO
+
+          $pkg_drop libfoo foo fox
+        }
+
         : postpone-existing
         :
         {