From 8cc72e88f7fbe3a41719225d32b98a051f1e7107 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Thu, 27 Jun 2024 14:59:37 +0300
Subject: Fix pkg-build crash by preferring deorphaning target versions during
 package collection (GH issue #397)

---
 bpkg/pkg-build-collect.cxx | 42 +++++++++++++++++++++++++-----------------
 bpkg/pkg-build-collect.hxx |  4 +++-
 bpkg/pkg-build.cxx         |  2 ++
 3 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx
index e99557a..c7be96a 100644
--- a/bpkg/pkg-build-collect.cxx
+++ b/bpkg/pkg-build-collect.cxx
@@ -1639,27 +1639,35 @@ namespace bpkg
 
         // Pick with the following preference order: user selection over
         // implicit one, source package over a system one, replacement version
-        // over a non-replacement one, newer version over an older one. So get
-        // the preferred into p1 and the other into p2.
+        // over a non-replacement one, deorphaning target version over the
+        // other one, newer version over an older one. So get the preferred
+        // into p1 and the other into p2.
         //
         {
-          const version& v1 (p1->available_version ());
-          const version& v2 (p2->available_version ());
-
-          int us (p1->user_selection () - p2->user_selection ());
-          int sf (p1->system - p2->system);
-          int rv (replacement_version != nullptr
-                  ? (v1 == *replacement_version) - (v2 == *replacement_version)
-                  : 0);
-
-          if (us < 0              ||
-              (us == 0 && sf > 0) ||
-              (us == 0 &&
-               sf == 0 &&
-               (rv < 0 || (rv == 0 && v2 > v1))))
+          auto prefer_p2 = [p1, p2, replacement_version] ()
           {
+            const version& v1 (p1->available_version ());
+            const version& v2 (p2->available_version ());
+
+            if (int us = p1->user_selection () - p2->user_selection ())
+              return us < 0;
+
+            if (int sf = p1->system - p2->system)
+              return sf > 0;
+
+            if (int rv = replacement_version != nullptr
+                ? (v1 == *replacement_version) - (v2 == *replacement_version)
+                : 0)
+              return rv < 0;
+
+            if (int d = p1->deorphan - p2->deorphan)
+              return d < 0;
+
+            return v2 > v1;
+          };
+
+          if (prefer_p2 ())
             swap (p1, p2);
-          }
         }
 
         // If the versions differ, pick the satisfactory one and if both are
diff --git a/bpkg/pkg-build-collect.hxx b/bpkg/pkg-build-collect.hxx
index 5c8a635..493e0f1 100644
--- a/bpkg/pkg-build-collect.hxx
+++ b/bpkg/pkg-build-collect.hxx
@@ -311,7 +311,9 @@ namespace bpkg
     // If true, then this package is requested to be deorphaned. Can only be
     // true if the package is already selected and is orphaned. Used by the
     // unsatisfied dependency constraints resolution logic (see
-    // try_replace_dependency() in pkg-build.cxx for details).
+    // try_replace_dependency() in pkg-build.cxx for details). Also, the
+    // package deorphaning target versions are normally preferred over the
+    // other versions by collect_build().
     //
     bool deorphan;
 
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index e355783..de8d3ec 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -8023,6 +8023,8 @@ namespace bpkg
 
           const package_location& pl (ap->locations[0]);
 
+          // The package location always belongs to some repository fragment.
+          //
           assert (pl.repository_fragment != nullptr);
 
           if (pl.repository_fragment.object_id () != "") // Special root?
-- 
cgit v1.1