From b48bd809a4760bc6e8718adc38837fe62bce1c20 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Wed, 23 Aug 2023 20:37:00 +0300
Subject: Fix pkg-build not to issue 'unable to reconfigure dependent' instead
 of 'unable to up/downgrade package' error message

---
 bpkg/pkg-build.cxx         | 61 ++++++++++++++++++++++++++--------------------
 bpkg/pkg-configure.cxx     |  8 ++++--
 tests/pkg-build.testscript | 41 +++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 28 deletions(-)

diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index fa3d762..79f89bb 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -6374,6 +6374,38 @@ namespace bpkg
           return &i->second;
         };
 
+        // In the simulation mode unconstrain all the unsatisfactory
+        // dependencies, if any, while configuring the dependent (see
+        // build_packages::collect_order_dependents() for details).
+        //
+        // Note: must be called at most once.
+        //
+        auto unconstrain_deps = [simulate,
+                                 &p,
+                                 deps = vector<package_key> ()] () mutable
+        {
+          if (simulate)
+          {
+            unsatisfied_dependent* ud (
+              simulate->find_dependent (package_key (p.db, p.name ())));
+
+            if (ud != nullptr)
+            {
+              assert (deps.empty ());
+
+              deps.reserve (ud->dependencies.size ());
+
+              for (const auto& d: ud->dependencies)
+              {
+                const build_package& p (*d.first);
+                deps.emplace_back (p.db, p.name ());
+              }
+            }
+          }
+
+          return !deps.empty () ? &deps : nullptr;
+        };
+
         if (ap != nullptr)
         {
           assert (*p.action == build_package::build);
@@ -6434,7 +6466,8 @@ namespace bpkg
                                                prereqs (),
                                                simulate,
                                                fdb,
-                                               configured_state);
+                                               configured_state,
+                                               unconstrain_deps ());
           }
         }
         else // Existing dependent.
@@ -6469,28 +6502,6 @@ namespace bpkg
 
           const dependencies& deps (p.skeleton->available->dependencies);
 
-          // In the simulation mode unconstrain all the unsatisfactory
-          // dependencies, if any, while configuring the dependent.
-          //
-          vector<package_key> unconstrain_deps;
-
-          if (simulate)
-          {
-            unsatisfied_dependent* ud (
-              simulate->find_dependent (package_key (pdb, p.name ())));
-
-            if (ud != nullptr)
-            {
-              unconstrain_deps.reserve (ud->dependencies.size ());
-
-              for (const auto& d: ud->dependencies)
-              {
-                const build_package& p (*d.first);
-                unconstrain_deps.emplace_back (p.db, p.name ());
-              }
-            }
-          }
-
           // @@ Note that on reconfiguration the dependent looses the
           //    potential configuration variables specified by the user on
           //    some previous build, which can be quite surprising. Should we
@@ -6509,9 +6520,7 @@ namespace bpkg
                                              simulate,
                                              fdb,
                                              configured_state,
-                                             (!unconstrain_deps.empty ()
-                                              ? &unconstrain_deps
-                                              : nullptr));
+                                             unconstrain_deps ());
         }
 
         t.commit ();
diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx
index 53c104e..c145b28 100644
--- a/bpkg/pkg-configure.cxx
+++ b/bpkg/pkg-configure.cxx
@@ -422,9 +422,13 @@ namespace bpkg
 
         // In the reconfiguration mode ban the usage of the selected
         // alternative dependency configuration variables in the subsequent
-        // enable and reflect clauses.
+        // enable and reflect clauses, unless we are also unconstraining
+        // dependencies (which indicates it's a relaxed mode that precedes
+        // a drop or failure with better diagnostics).
         //
-        if (alts == nullptr && !manual && (da.prefer || da.require))
+        if (alts == nullptr && !manual  &&
+            unconstrain_deps == nullptr &&
+            (da.prefer || da.require))
         {
           for (const dependency& d: da)
             banned_var_prefixes.push_back (
diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript
index 48a886f..214a50e 100644
--- a/tests/pkg-build.testscript
+++ b/tests/pkg-build.testscript
@@ -5432,6 +5432,47 @@ test.arguments += --sys-no-query
 
         $pkg_drop tpx
       }
+
+      : fail-change-unsatisfactory-alternative
+      :
+      {
+        $clone_cfg;
+
+        $* tpx ?libfoo 2>!;
+
+        $pkg_status -r >>EOO;
+          !tpx configured 1.0.0
+            libfoo configured 2.0.0
+          EOO
+
+        cat cfg/tpx-1.0.0/build/config.build >>~%EOO%;
+          %.*
+          config.tpx.libfoo_protocol = 2
+          %.*
+          EOO
+
+        $* tax 2>>EOE != 0;
+          error: unable to downgrade package libfoo/2.0.0 to 1.0.0
+            info: because package tpx depends on (libfoo >= 2.0.0)
+            info: package libfoo/1.0.0 required by tax
+            info: explicitly request up/downgrade of package tpx
+            info: or explicitly specify package libfoo version to manually satisfy these constraints
+          EOE
+
+        # @@ Note that the above advises doesn't really work here since the
+        #    tpx package is not re-collected recursively. We should probably
+        #    invent the package-specific --rebuild option to re-collect a
+        #    configured package.
+        #
+        $* tax tpx ?libfoo/1.0.0 2>>EOE != 0;
+          error: unable to downgrade package libfoo/2.0.0 to 1.0.0
+            info: because package tpx depends on (libfoo >= 2.0.0)
+            info: explicitly request up/downgrade of package tpx
+            info: or explicitly specify package libfoo version to manually satisfy these constraints
+          EOE
+
+        $pkg_drop tpx
+      }
     }
   }
 
-- 
cgit v1.1