From 668ffb5d3258945e35f8f6e62d0395aacc219e23 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Wed, 23 Aug 2023 16:40:17 +0300
Subject: Fix configuration negotiation to throw postpone_dependency for
 collected dependency which belongs to not (being) negotiated cluster

---
 bpkg/pkg-build-collect.cxx | 78 ++++++++++++++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 26 deletions(-)

(limited to 'bpkg')

diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx
index a52bba5..de944a4 100644
--- a/bpkg/pkg-build-collect.cxx
+++ b/bpkg/pkg-build-collect.cxx
@@ -3164,35 +3164,60 @@ namespace bpkg
                     i->second.with_config = true;
                 }
 
-                // Prematurely collected before we saw any config clauses.
+                // Throw postpone_dependency if the dependency is prematurely
+                // collected before we saw any config clauses.
                 //
-                if (p->recursive_collection &&
-                    postponed_cfgs.find_dependency (dpk) == nullptr)
+                // Note that we don't throw if the dependency already belongs
+                // to some (being) negotiated cluster since we will
+                // up-negotiate its configuration (at the end of the loop)
+                // instead.
+                //
+                if (p->recursive_collection)
                 {
-                  if (reeval)
-                  {
-                    l5 ([&]{trace << "cannot re-evaluate existing dependent "
-                                  << pkg.available_name_version_db ()
-                                  << " due to dependencys "
-                                  << p->available_name_version_db ()
-                                  << " (collected prematurely), "
-                                  << "throwing postpone_dependency";});
-                  }
-                  else
+                  const postponed_configuration* pcfg (
+                    postponed_cfgs.find_dependency (dpk));
+
+                  // Note that it may well happen that a dependency was added
+                  // to a not-yet (being) negotiated cluster at the initial
+                  // collection phase, then the dependency was recursively
+                  // collected via some different dependent without
+                  // configuration clause during the postponed collection
+                  // phase (which was not prevented since its entry was
+                  // removed from postponed_deps at the end of the initial
+                  // collection phase), and now we are trying to collect this
+                  // dependency via some third dependent with the
+                  // configuration clause during the postponed collection
+                  // phase. If that's the case, we will throw
+                  // postpone_dependency and this is the reason why we check
+                  // if the cluster is (being) negotiated.
+                  //
+                  if (!(pcfg != nullptr && pcfg->negotiated))
                   {
-                    l5 ([&]{trace << "cannot cfg-postpone dependency "
-                                  << p->available_name_version_db ()
-                                  << " of dependent "
-                                  << pkg.available_name_version_db ()
-                                  << " (collected prematurely), "
-                                  << "throwing postpone_dependency";});
-                  }
+                    if (reeval)
+                    {
+                      l5 ([&]{trace << "cannot re-evaluate existing dependent "
+                                    << pkg.available_name_version_db ()
+                                    << " due to dependency "
+                                    << p->available_name_version_db ()
+                                    << " (collected prematurely), "
+                                    << "throwing postpone_dependency";});
+                    }
+                    else
+                    {
+                      l5 ([&]{trace << "cannot cfg-postpone dependency "
+                                    << p->available_name_version_db ()
+                                    << " of dependent "
+                                    << pkg.available_name_version_db ()
+                                    << " (collected prematurely), "
+                                    << "throwing postpone_dependency";});
+                    }
 
-                  // Don't print the "while satisfying..." chain.
-                  //
-                  dep_chain.clear ();
+                    // Don't print the "while satisfying..." chain.
+                    //
+                    dep_chain.clear ();
 
-                  throw postpone_dependency (move (dpk));
+                    throw postpone_dependency (move (dpk));
+                  }
                 }
 
                 if (!reeval)
@@ -3484,8 +3509,9 @@ namespace bpkg
               // To recap, r.second values mean:
               //
               //   absent  -- shadow cluster-based merge is/being negotiated
-              //   false   -- some non or being negotiated
-              //   true    -- all have been negotiated
+              //   false   -- some non or being negotiated clusters are merged
+              //   true    -- no clusters are merged or all merged have been
+              //              negotiated
               //
               if (r.second && !*r.second)
               {
-- 
cgit v1.1