Invent reused-only dependency alternative selection mode

2 files changed, 157 insertions, 52 deletions

diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index b055ab2..f396431 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -3307,10 +3307,17 @@ namespace bpkg
           //
           optional<prebuilds> builds;
 
+          // If some dependency of the alternative cannot be resolved because
+          // there is no version available which can satisfy all the being
+          // built dependents, then this member contains all the dependency
+          // builds (which otherwise would be contained in the builds member).
+          //
+          optional<prebuilds> unsatisfactory;
+
           // True if dependencies can all be resolved (builds is present) and
           // are all reused (see above).
           //
-          bool reused;
+          bool reused = false;
 
           // True if some of the dependencies cannot be resolved (builds is
           // nullopt) and the dependent package prerequisites collection needs
@@ -3318,18 +3325,24 @@ namespace bpkg
           // pre-entered constraint from repositories available to the
           // dependent package.
           //
-          bool repo_postpone;
+          bool repo_postpone = false;
 
           // Create precollect result containing dependency builds.
           //
           precollect_result (prebuilds&& bs, bool r)
-              : builds (move (bs)), reused (r), repo_postpone (false) {}
+              : builds (move (bs)), reused (r) {}
+
+          // Create precollect result containing unsatisfactory dependency
+          // builds.
+          //
+          precollect_result (bool r, prebuilds&& bs)
+              : unsatisfactory (move (bs)), reused (r) {}
 
           // Create precollect result without builds (some dependency can't be
-          // satisfied, etc).
+          // resolved, etc).
           //
           explicit
-          precollect_result (bool p): reused (false), repo_postpone (p) {}
+          precollect_result (bool p): repo_postpone (p) {}
         };
 
         auto precollect = [&options,
@@ -3387,8 +3400,7 @@ namespace bpkg
             // failing due to inability for dependency versions collected by
             // two dependents to satisfy each other constraints (for an
             // example see the
-            // pkg-build/dependency/apply-constraints/resolve-conflict{1,2}
-            // tests).
+            // pkg-build/dependency/apply-constraints/resolve-conflict/ tests).
 
             // Points to the desired dependency version constraint, if
             // specified, and is NULL otherwise. Can be used as boolean flag.
@@ -3958,25 +3970,51 @@ namespace bpkg
               }
             }
 
-            // If the dependency package of a different version is already
-            // being built, then we also need to make sure that we will be
-            // able to choose one of them (either existing or new) which
-            // satisfies all the dependents.
-            //
-            // Note that collect_build() also performs this check but
-            // postponing it till then can end up in failing instead of
-            // selecting some other dependency alternative.
-            //
+            bool ru (i != map_.end () || dsp != nullptr);
+
+            if (!ru)
+              reused = false;
+
+            r.push_back (prebuild {d,
+                                   *ddb,
+                                   move (dsp),
+                                   move (dap),
+                                   move (rp.second),
+                                   system,
+                                   specified,
+                                   force,
+                                   ru});
+          }
+
+          // Now, as we have pre-collected the dependency builds, go through
+          // them and check that for those dependencies which are already
+          // being built we will be able to choose one of them (either
+          // existing or new) which satisfies all the dependents. If that's
+          // not the case, then issue the diagnostics, if requested, and
+          // return the unsatisfactory dependency builds.
+          //
+          // Note that collect_build() also performs this check but postponing
+          // it till then can end up in failing instead of selecting some
+          // other dependency alternative.
+          //
+          for (const prebuild& b: r)
+          {
+            const shared_ptr<available_package>& dap (b.available);
+
             assert (dap != nullptr); // Otherwise we would fail earlier.
 
+            const dependency& d (b.dependency);
+
+            auto i (map_.find (b.db, d.name));
+
             if (i != map_.end () && d.constraint)
             {
               const build_package& bp (i->second.package);
 
               if (bp.action && *bp.action == build_package::build)
               {
-                const version& v1 (system
-                                   ? *dap->system_version (*ddb)
+                const version& v1 (b.system
+                                   ? *dap->system_version (b.db)
                                    : dap->version);
 
                 const version& v2 (bp.available_version ());
@@ -4008,33 +4046,18 @@ namespace bpkg
                             info << "available "
                                  << bp.available_name_version () <<
                             info << "available "
-                                 << package_string (n, v1, system) <<
+                                 << package_string (n, v1, b.system) <<
                             info << "explicitly specify " << n << " version "
                                  << "to manually satisfy both constraints";
                         }
 
-                        return precollect_result (false /* postpone */);
+                        return precollect_result (reused, move (r));
                       }
                     }
                   }
                 }
               }
             }
-
-            bool ru (i != map_.end () || dsp != nullptr);
-
-            if (!ru)
-              reused = false;
-
-            r.push_back (prebuild {d,
-                                   *ddb,
-                                   move (dsp),
-                                   move (dap),
-                                   move (rp.second),
-                                   system,
-                                   specified,
-                                   force,
-                                   ru});
           }
 
           return precollect_result (move (r), reused);
@@ -4798,6 +4821,26 @@ namespace bpkg
           //
           size_t alts_num (0);
 
+          // If true, then only reused alternatives will be considered for the
+          // selection.
+          //
+          // The idea here is that we don't want to bloat the configuration by
+          // silently configuring a new dependency package as the alternative
+          // for an already used but not satisfactory for all the dependents
+          // dependency. Think of silently configuring Qt6 just because the
+          // configured version of Qt5 is not satisfactory for all the
+          // dependents. The user must have a choice if to either configure
+          // this new dependency by specifying it explicitly or, for example,
+          // to upgrade dependents so that the existing dependency is
+          // satisfactory for all of them.
+          //
+          // Note that if there are multiple alternatives with all their
+          // dependencies resolved/satisfied, then only reused alternatives
+          // are considered anyway. Thus, this flag only affects the single
+          // alternative case.
+          //
+          bool reused_only (false);
+
           for (size_t i (0); i != edas.size (); ++i)
           {
             const dependency_alternative& da (edas[i].first);
@@ -4829,6 +4872,12 @@ namespace bpkg
                 break;
               }
 
+              // If this alternative is reused but is not satisfactory, then
+              // switch to the reused-only mode.
+              //
+              if (r.reused && r.unsatisfactory)
+                reused_only = true;
+
               continue;
             }
 
@@ -4937,24 +4986,31 @@ namespace bpkg
           if (postponed)
             break;
 
-          // Select the single satisfactory alternative (regardless of its
-          // dependencies reuse).
+          // Select the single satisfactory alternative if it is reused or we
+          // are not in the reused-only mode.
           //
           if (!selected && alts_num == 1)
           {
-            assert (first_alt && first_alt->second.builds);
+            assert (first_alt);
+
+            precollect_result& r (first_alt->second);
 
-            const auto& eda (edas[first_alt->first]);
-            const dependency_alternative& da (eda.first);
-            size_t dai (eda.second);
+            assert (r.builds);
 
-            if (!collect (da, dai, move (*first_alt->second.builds)))
+            if (r.reused || !reused_only)
             {
-              postpone (nullptr); // Already inserted into postponed_cfgs.
-              break;
-            }
+              const auto& eda (edas[first_alt->first]);
+              const dependency_alternative& da (eda.first);
+              size_t dai (eda.second);
+
+              if (!collect (da, dai, move (*r.builds)))
+              {
+                postpone (nullptr); // Already inserted into postponed_cfgs.
+                break;
+              }
 
-            select (da, dai);
+              select (da, dai);
+            }
           }
 
           // If an alternative is selected, then we are done.
@@ -4992,11 +5048,11 @@ namespace bpkg
             throw failed ();
           }
 
-          // Issue diagnostics and fail if there are multiple alternatives
-          // with non-reused dependencies, unless the failure needs to be
-          // postponed.
+          // Issue diagnostics and fail if there are multiple non-reused
+          // alternatives or there is a single non-reused alternative in the
+          // reused-only mode, unless the failure needs to be postponed.
           //
-          assert (alts_num > 1);
+          assert (alts_num > (!reused_only ? 1 : 0));
 
           if (postponed_alts != nullptr)
           {
@@ -5016,10 +5072,11 @@ namespace bpkg
           }
 
           diag_record dr (fail);
+
           dr << "unable to select dependency alternative for package "
              << pkg.available_name_version_db () <<
             info << "explicitly specify dependency packages to manually "
-             << "select the alternative";
+                 << "select the alternative";
 
           for (const auto& da: edas)
           {
@@ -5042,6 +5099,38 @@ namespace bpkg
               }
             }
           }
+
+          // If there is only a single alternative (while we are in the
+          // reused-only mode), then also print the reused unsatisfactory
+          // alternatives and the reasons why they are not satisfactory.
+          //
+          if (alts_num == 1)
+          {
+            assert (reused_only);
+
+            for (const auto& da: edas)
+            {
+              precollect_result r (
+                precollect (da.first, das.buildtime, nullptr /* prereqs */));
+
+              if (r.reused && r.unsatisfactory)
+              {
+                // Print the alternative.
+                //
+                dr << info << "unsatisfactory alternative:";
+
+                for (const prebuild& b: *r.unsatisfactory)
+                  dr << ' ' << b.dependency.name;
+
+                // Print the reason.
+                //
+                precollect (da.first,
+                            das.buildtime,
+                            nullptr /* prereqs */,
+                            &dr);
+              }
+            }
+          }
         }
 
         if (postponed)
diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript
index 0e6eafe..3533b8d 100644
--- a/tests/pkg-build.testscript
+++ b/tests/pkg-build.testscript
@@ -3991,7 +3991,23 @@ test.options += --no-progress
 
             $* box libbox 2>!;
 
-            $* box +{ config.box.extras=true } libbox/0.1.0 2>>~%EOE%;
+            # While at it, test the reused-only alternative selection mode.
+            #
+            $* box +{ config.box.extras=true } libbox/0.1.0 2>>EOE != 0;
+              error: unable to select dependency alternative for package box/1.0.0
+                info: explicitly specify dependency packages to manually select the alternative
+                info: alternative: libbiz
+                info: unsatisfactory alternative: libbox
+                error: unable to satisfy constraints on package libbox
+                info: command line depends on (libbox == 0.1.0)
+                info: box depends on (libbox >= 0.1.1)
+                info: available libbox/0.1.0
+                info: available libbox/1.0.0
+                info: explicitly specify libbox version to manually satisfy both constraints
+              info: while satisfying box/1.0.0
+              EOE
+
+            $* box +{ config.box.extras=true } libbox/0.1.0 ?libbiz 2>>~%EOE%;
               build plan:
                 new libbiz/1.0.0 (required by box)
                 downgrade libbox/0.1.0