From f3ed48f053b8938c01a96f61c160e9a9c5f83562 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Tue, 31 Oct 2023 22:12:53 +0300
Subject: Fix pkg-build to order drops after collecting/ordering dependents
 when all reconfigurations are determined

---
 bpkg/pkg-build-collect.cxx | 50 +++++++++++++++++++++++++++-------------------
 bpkg/pkg-build.cxx         | 28 +++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 26 deletions(-)

(limited to 'bpkg')

diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx
index cb7ea9a..a70ef10 100644
--- a/bpkg/pkg-build-collect.cxx
+++ b/bpkg/pkg-build-collect.cxx
@@ -7159,13 +7159,18 @@ namespace bpkg
         // dependent. But first "prune" if the dependent is being dropped or
         // this is a replaced prerequisite of the repointed dependent.
         //
-        // Note that the repointed dependents are always collected and have
-        // all their collected prerequisites ordered (including new and old
-        // ones). See collect_build_prerequisites() and order() for details.
+        // Note that the package drops are not ordered at this stage, since to
+        // order them properly all the package reconfigurations must be
+        // determined.
+        //
+        // Also note that the repointed dependents are always collected and
+        // have all their collected prerequisites ordered (including new and
+        // old ones). See collect_build_prerequisites() and order() for
+        // details.
         //
         bool check (ud != 0 && dc);
 
-        if (i != map_.end () && i->second.position != end ())
+        if (i != map_.end ())
         {
           build_package& dp (i->second.package);
 
@@ -7174,27 +7179,30 @@ namespace bpkg
           if (dp.action && *dp.action == build_package::drop)
             continue;
 
-          repointed_dependents::const_iterator j (
-            rpt_depts.find (package_key {ddb, dn}));
-
-          if (j != rpt_depts.end ())
+          if (i->second.position != end ())
           {
-            const map<package_key, bool>& prereqs_flags (j->second);
+            repointed_dependents::const_iterator j (
+              rpt_depts.find (package_key {ddb, dn}));
 
-            auto k (prereqs_flags.find (package_key {pdb, n}));
+            if (j != rpt_depts.end ())
+            {
+              const map<package_key, bool>& prereqs_flags (j->second);
 
-            if (k != prereqs_flags.end () && !k->second)
-              continue;
-          }
+              auto k (prereqs_flags.find (package_key {pdb, n}));
 
-          // There is one tricky aspect: the dependent could be in the process
-          // of being reconfigured or up/downgraded as well. In this case all
-          // we need to do is detect this situation and skip the test since
-          // all the (new) constraints of this package have been satisfied in
-          // collect_build().
-          //
-          if (check)
-            check = !dp.dependencies;
+              if (k != prereqs_flags.end () && !k->second)
+                continue;
+            }
+
+            // There is one tricky aspect: the dependent could be in the
+            // process of being reconfigured or up/downgraded as well. In this
+            // case all we need to do is detect this situation and skip the
+            // test since all the (new) constraints of this package have been
+            // satisfied in collect_build().
+            //
+            if (check)
+              check = !dp.dependencies;
+          }
         }
 
         if (check)
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 857e66e..ede26bf 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -4553,11 +4553,19 @@ namespace bpkg
         // dependencies between the specified packages).
         //
         // The order of dependency upgrades/downgrades/drops is not really
-        // deterministic. We, however, do them before hold_pkgs so that they
-        // appear (e.g., on the plan) last.
+        // deterministic. We, however, do upgrades/downgrades before hold_pkgs
+        // so that they appear (e.g., on the plan) last. We handle drops
+        // later, though, after collecting/ordering dependents when all the
+        // package reconfigurations are determined.
         //
         for (const dep& d: deps)
-          pkgs.order (d.db, d.name, find_prereq_database, false /* reorder */);
+        {
+          if (d.available != nullptr)
+            pkgs.order (d.db,
+                        d.name,
+                        find_prereq_database,
+                        false /* reorder */);
+        }
 
         for (const build_package& p: reverse_iterate (hold_pkgs))
           pkgs.order (p.db, p.name (), find_prereq_database);
@@ -4593,8 +4601,7 @@ namespace bpkg
         //
         pkgs.collect_order_dependents (rpt_depts, unsatisfied_depts);
 
-        // And, finally, make sure all the packages that we need to unhold
-        // are on the list.
+        // Make sure all the packages that we need to unhold are on the list.
         //
         for (const dependency_package& p: dep_pkgs)
         {
@@ -4623,6 +4630,17 @@ namespace bpkg
           }
         }
 
+        // And, finally, order the package drops.
+        //
+        for (const dep& d: deps)
+        {
+          if (d.available == nullptr)
+            pkgs.order (d.db,
+                        d.name,
+                        find_prereq_database,
+                        false /* reorder */);
+        }
+
         // Make sure all the postponed dependencies of existing dependents
         // have been collected and fail if that's not the case.
         //
-- 
cgit v1.1