From 13336bebdddb7721347407f64e432627d0d8d6c1 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 23 Mar 2022 20:49:32 +0300 Subject: In pkg-build make sure that reconfiguring dependent doesn't change current dependency selection Also fix the dependency alternative selection so that if the dependency package of a different version is already being built, then make sure that one of them is satisfactory for all the dependents and don't consider this alternative if that's not the case. --- bpkg/pkg-configure.cxx | 261 ++++++++++++++++++++++++++++--------------------- 1 file changed, 152 insertions(+), 109 deletions(-) (limited to 'bpkg/pkg-configure.cxx') diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 0519f7c..1392545 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -49,6 +49,7 @@ namespace bpkg transaction&, const dependencies& deps, package_skeleton&& ps, + const vector* prev_prereqs, bool simulate, const function& fdb) { @@ -58,150 +59,189 @@ namespace bpkg for (size_t di (0); di != deps.size (); ++di) { + // Skip the toolchain build-time dependencies and dependencies without + // enabled alternatives. + // const dependency_alternatives_ex& das (deps[di]); if (das.empty () || toolchain_buildtime_dependency (o, das, ps.name ())) continue; - // Pick the first alternative with dependencies that can all be resolved - // to the configured packages, satisfying the respective constraints. - // - bool satisfied (false); - bool enabled (false); // True if there is an enabled alternative. + small_vector, 2> edas; + for (const dependency_alternative& da: das) { - if (da.enable && !ps.evaluate_enable (*da.enable, di)) - continue; - - enabled = true; - - // Cache the selected packages which correspond to the alternative - // dependencies, pairing them with the respective constraints. If the - // alternative turns out to be fully resolvable, we will add the - // cached packages into the dependent's prerequisites map. - // - small_vector< - pair, - const optional&>, 1> prerequisites; - - dependency_alternative::const_iterator b (da.begin ()); - dependency_alternative::const_iterator i (b); - dependency_alternative::const_iterator e (da.end ()); + if (!da.enable || ps.evaluate_enable (*da.enable, di)) + edas.push_back (da); + } - assert (b != e); + if (edas.empty ()) + continue; - for (; i != e; ++i) + // Pick the first alternative with dependencies that can all be resolved + // to the configured packages, satisfying the respective constraints. + // + // If the list of the former prerequisites is specified, then first try + // to select an alternative in the "recreate dependency decisions" mode, + // filtering out alternatives where dependencies do not all belong to + // this list. If we end up with no alternative selected, then retry in + // the "make dependency decisions" mode and select the alternative + // regardless of the former prerequisites. + // + for (const vector* pps (prev_prereqs);;) + { + bool satisfied (false); + for (const dependency_alternative& da: edas) { - const dependency& d (*i); - const package_name& n (d.name); - - database* ddb (fdb ? fdb (db, n, das.buildtime) : nullptr); + // Cache the selected packages which correspond to the alternative + // dependencies, pairing them with the respective constraints. If + // the alternative turns out to be fully resolvable, we will add the + // cached packages into the dependent's prerequisites map. + // + small_vector< + pair, + const optional&>, 1> prerequisites; - pair, database*> spd ( - ddb != nullptr - ? make_pair (ddb->find (n), ddb) - : find_dependency (db, n, das.buildtime)); + dependency_alternative::const_iterator b (da.begin ()); + dependency_alternative::const_iterator i (b); + dependency_alternative::const_iterator e (da.end ()); - const shared_ptr& dp (spd.first); + assert (b != e); - if (dp == nullptr || - dp->state != package_state::configured || - !satisfies (dp->version, d.constraint)) - break; + for (; i != e; ++i) + { + const dependency& d (*i); + const package_name& n (d.name); + + database* ddb (fdb ? fdb (db, n, das.buildtime) : nullptr); + + pair, database*> spd ( + ddb != nullptr + ? make_pair (ddb->find (n), ddb) + : find_dependency (db, n, das.buildtime)); + + const shared_ptr& dp (spd.first); + + if (dp == nullptr || + dp->state != package_state::configured || + !satisfies (dp->version, d.constraint) || + (pps != nullptr && + find (pps->begin (), pps->end (), dp->name) == pps->end ())) + break; + + // See the package_prerequisites definition for details on + // creating the map keys with the database passed. + // + prerequisites.emplace_back ( + lazy_shared_ptr (*spd.second, dp), + d.constraint); + } - // See the package_prerequisites definition for details on creating - // the map keys with the database passed. + // Try the next alternative if there are unresolved dependencies for + // this alternative. // - prerequisites.emplace_back ( - lazy_shared_ptr (*spd.second, dp), - d.constraint); - } + if (i != e) + continue; - // Try the next alternative if there are unresolved dependencies for - // this alternative. - // - if (i != e) - continue; - - // Now add the selected packages resolved for the alternative into the - // dependent's prerequisites map and skip the remaining alternatives. - // - for (auto& pr: prerequisites) - { - const package_name& pn (pr.first.object_id ()); - const optional& pc (pr.second); - - auto p (prereqs.emplace (pr.first, pc)); - - // Currently we can only capture a single constraint, so if we - // already have a dependency on this package and one constraint is - // not a subset of the other, complain. + // Now add the selected packages resolved for the alternative into + // the dependent's prerequisites map and skip the remaining + // alternatives. // - if (!p.second) + for (auto& pr: prerequisites) { - auto& c (p.first->second); + const package_name& pn (pr.first.object_id ()); + const optional& pc (pr.second); - bool s1 (satisfies (c, pc)); - bool s2 (satisfies (pc, c)); + auto p (prereqs.emplace (pr.first, pc)); - if (!s1 && !s2) - fail << "multiple dependencies on package " << pn << - info << pn << " " << *c << - info << pn << " " << *pc; + // Currently we can only capture a single constraint, so if we + // already have a dependency on this package and one constraint is + // not a subset of the other, complain. + // + if (!p.second) + { + auto& c (p.first->second); - if (s2 && !s1) - c = pc; - } + bool s1 (satisfies (c, pc)); + bool s2 (satisfies (pc, c)); - // If the prerequisite is configured in the linked configuration, - // then add the respective config.import.* variable. - // - if (!simulate) - { - database& pdb (pr.first.database ()); + if (!s1 && !s2) + fail << "multiple dependencies on package " << pn << + info << pn << " " << *c << + info << pn << " " << *pc; + + if (s2 && !s1) + c = pc; + } - if (pdb != db) + // If the prerequisite is configured in the linked configuration, + // then add the respective config.import.* variable. + // + if (!simulate) { - shared_ptr sp (pr.first.load ()); + database& pdb (pr.first.database ()); - if (!sp->system ()) + if (pdb != db) { - // @@ Note that this doesn't work for build2 modules that - // require bootstrap. For their dependents we need to - // specify the import variable as a global override, - // whenever required (configure, update, etc). - // - // This, in particular, means that if we build a package - // that doesn't have direct build2 module dependencies but - // some of its (potentially indirect) dependencies do, then - // we still need to specify the !config.import.* global - // overrides for all of the involved build2 - // modules. Implementation of that feels too hairy at the - // moment, so let's handle all the build2 modules uniformly - // for now. - // - // Also note that such modules are marked with `requires: - // bootstrap` in their manifest. - // - dir_path od (sp->effective_out_root (pdb.config)); - vars.push_back ("config.import." + sp->name.variable () + - "='" + od.representation () + "'"); + shared_ptr sp (pr.first.load ()); + + if (!sp->system ()) + { + // @@ Note that this doesn't work for build2 modules that + // require bootstrap. For their dependents we need to + // specify the import variable as a global override, + // whenever required (configure, update, etc). + // + // This, in particular, means that if we build a package + // that doesn't have direct build2 module dependencies + // but some of its (potentially indirect) dependencies + // do, then we still need to specify the !config.import.* + // global overrides for all of the involved build2 + // modules. Implementation of that feels too hairy at the + // moment, so let's handle all the build2 modules + // uniformly for now. + // + // Also note that such modules are marked with `requires: + // bootstrap` in their manifest. + // + dir_path od (sp->effective_out_root (pdb.config)); + vars.push_back ("config.import." + sp->name.variable () + + "='" + od.representation () + "'"); + } } } } + + // Evaluate the dependency alternative reflect clause, if present. + // + if (da.reflect) + ps.evaluate_reflect (*da.reflect, di); + + satisfied = true; + break; } - // Evaluate the dependency alternative reflect clause, if present. + // Fail if no dependency alternative is selected, unless we are in the + // "recreate dependency decisions" mode. In the latter case fall back + // to the "make dependency decisions" mode and retry. // - if (da.reflect) - ps.evaluate_reflect (*da.reflect, di); + if (!satisfied) + { + if (pps != nullptr) + { + pps = nullptr; + continue; + } + + fail << "unable to satisfy dependency on " << das; + } - satisfied = true; + // The dependency alternative is selected and its dependencies are + // resolved to the selected packages. So proceed to the next depends + // value. + // break; } - - if (enabled && !satisfied) - fail << "unable to satisfy dependency on " << das; } // Add the configuration variables collected from the reflect clauses, if @@ -248,6 +288,7 @@ namespace bpkg const shared_ptr& p, const dependencies& deps, package_skeleton&& ps, + const vector* pps, bool simulate, const function& fdb) { @@ -282,6 +323,7 @@ namespace bpkg t, deps, move (ps), + pps, simulate, fdb)); @@ -523,6 +565,7 @@ namespace bpkg move (vars), move (src_root), move (out_root)), + nullptr /* prerequisites */, false /* simulate */); } -- cgit v1.1