From 334ff0a070031eac2e08839aa4c97666941bc653 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 13 Jul 2022 22:13:22 +0300 Subject: Invent reused-only dependency alternative selection mode --- bpkg/pkg-build.cxx | 191 +++++++++++++++++++++++++++++++++------------ tests/pkg-build.testscript | 18 ++++- 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 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 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& 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 -- cgit v1.1