aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2022-05-19 20:27:12 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2022-06-07 21:14:14 +0300
commitb5428e045ee7049d4e67474497cf9db00eb587ed (patch)
tree2e8ee10fad6bebe9dfd26be7b7c0f3c80e044660
parent56ddb0a7dfcc00cf1ef5ea3777b6bb1e7a1faa34 (diff)
Implement existing dependent re-evaluation
-rw-r--r--bpkg/pkg-build.cxx276
1 files changed, 162 insertions, 114 deletions
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 4d04f7a..8c87f02 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -6,7 +6,7 @@
#include <map>
#include <set>
#include <list>
-#include <limits> // @@ TMP
+#include <limits> // numeric_limits
#include <cstring> // strlen()
#include <iostream> // cout
#include <functional> // ref()
@@ -2633,7 +2633,10 @@ namespace bpkg
//
// @@ TMP This will probably be gone (see below).
//
- bool force_configured = false)
+ bool force_configured = false,
+
+ pair<size_t, size_t> reeval_pos =
+ make_pair(0, 0))
{
// NOTE: don't forget to update collect_build_postponed() if changing
// anything in this function.
@@ -2650,12 +2653,21 @@ namespace bpkg
database& pdb (pkg.db);
config_package cp (pdb, nm);
- // If this package is not yet collected recursively, needs to be
- // reconfigured, and is not yet postponed, then check if it is a
- // dependency of any dependent with configuration clause and postpone
- // the collection if that's the case.
+ bool reeval (reeval_pos.first != 0);
+
+ // The being re-evaluated dependent cannot be recursively collected yet.
+ // Also, we don't expect it being configured as system.
+ //
+ assert (!reeval ||
+ (!pkg.recursive_collection && !pkg.skeleton && !pkg.system));
+
+ // If this package is not being re-evaluated, is not yet collected
+ // recursively, needs to be reconfigured, and is not yet postponed, then
+ // check if it is a dependency of any dependent with configuration
+ // clause and postpone the collection if that's the case.
//
- if (!pkg.recursive_collection &&
+ if (!reeval &&
+ !pkg.recursive_collection &&
pkg.reconfigure () &&
postponed_cfgs.find_dependency (cp) == nullptr)
{
@@ -2737,6 +2749,12 @@ namespace bpkg
sp->state == package_state::configured &&
sp->substate != package_substate::system);
+ // The being re-evaluated dependent must be configured as a source
+ // package and should not be collected recursively (due to upgrade,
+ // etc).
+ //
+ assert (!reeval || (src_conf && !pkg.recollect_recursively (rpt_depts)));
+
if (src_conf)
{
repointed_dependents::const_iterator i (rpt_depts.find (cp));
@@ -2744,7 +2762,9 @@ namespace bpkg
if (i != rpt_depts.end ())
rpt_prereq_flags = &i->second;
- if (!force_configured && !pkg.recollect_recursively (rpt_depts))
+ if (!force_configured &&
+ !reeval &&
+ !pkg.recollect_recursively (rpt_depts))
{
l5 ([&]{trace << "skip configured "
<< pkg.available_name_version_db ();});
@@ -2769,7 +2789,8 @@ namespace bpkg
//
if (!pkg.dependencies)
{
- l5 ([&]{trace << "begin " << pkg.available_name_version_db ();});
+ l5 ([&]{trace << (reeval ? "reeval " : "begin ")
+ << pkg.available_name_version_db ();});
pkg.dependencies = dependencies ();
pkg.alternatives = vector<size_t> ();
@@ -2839,9 +2860,23 @@ namespace bpkg
package_skeleton& skel (*pkg.skeleton);
+ auto fail_reeval = [&pkg] ()
+ {
+ fail << "re-evaluation of configured dependent "
+ << pkg.available_name_version_db ()
+ << " failed due to some environmental changes";
+ };
+
bool postponed (false);
+ bool reevaluated (false);
+
for (size_t di (sdeps.size ()); di != deps.size (); ++di)
{
+ // Fail if we missed re-evaluation position by any reason.
+ //
+ if (reeval && di == reeval_pos.first)
+ fail_reeval ();
+
const dependency_alternatives_ex& das (deps[di]);
// Add an empty alternatives list into the selected dependency list if
@@ -3663,6 +3698,10 @@ namespace bpkg
&postponed_deps,
&postponed_cfgs,
&di,
+ reeval,
+ &reeval_pos,
+ &reevaluated,
+ &fail_reeval,
&trace,
this]
(const dependency_alternative& da,
@@ -3673,6 +3712,11 @@ namespace bpkg
//
pair<size_t, size_t> dp (di + 1, dai + 1);
+ if (reeval &&
+ dp.first == reeval_pos.first &&
+ dp.second != reeval_pos.second)
+ fail_reeval ();
+
postponed_configuration::packages cfg_deps;
for (prebuild& b: bs)
@@ -3819,6 +3863,21 @@ namespace bpkg
nullptr /* postponed_cfgs */,
verify));
+ config_package dcp (b.db, b.available->id.name);
+
+ // Do not collect prerequisites recursively for dependent
+ // re-evaluation. Instead, if the re-evaluation position is
+ // reached, collect the dependency packages to add them to the
+ // existing dependent's cluster
+ //
+ if (reeval)
+ {
+ if (dp == reeval_pos)
+ cfg_deps.push_back (move (dcp));
+
+ continue;
+ }
+
// Do not recursively collect a dependency of a dependent with
// configuration clauses, which could be this or some other
// (indicated by the presence in postponed_deps) dependent. In the
@@ -3832,8 +3891,6 @@ namespace bpkg
bool collect_prereqs (p != nullptr);
{
- config_package dcp (b.db, b.available->id.name);
-
build_package* bp (entered_build (dcp));
assert (bp != nullptr);
@@ -3939,6 +3996,36 @@ namespace bpkg
//
if (!cfg_deps.empty ())
{
+ if (reeval)
+ {
+ reevaluated = true;
+
+ // Note: the dependent may already exist in the cluster with a
+ // subset of dependencies.
+ //
+ // @@ Can we merge clusters as a result?
+ //
+ // @@ It seems that we need to make sure that we are adding into
+ // that specific cluster we were about to negotiate when we
+ // called collect_build_prerequisites() and merge the
+ // intersecting clusters into it (similar to shadow-based
+ // merge).
+ //
+ // @@ What if some of the merged clusters are already
+ // negotiated?
+ //
+ pair<postponed_configuration&, optional<bool>> r (
+ postponed_cfgs.add (cp, true /* existing */, dp, cfg_deps));
+
+ l5 ([&]{trace << "re-evaluating dependent "
+ << pkg.available_name_version_db ()
+ << " results in " << r.first;});
+
+ assert (r.second && !*r.second);
+
+ return false;
+ }
+
// As a first step add this dependent/dependencies to one of the
// new/existing postponed_configuration clusters, which could
// potentially cause some of them to be merged. Here are the
@@ -4166,7 +4253,7 @@ namespace bpkg
// the package to the postpones set (can potentially already be there)
// and saving the enabled alternatives.
//
- auto postpone = [&pkg, &edas, &postponed]
+ auto postpone = [&pkg, &edas, reeval, &postponed]
(postponed_packages* postpones)
{
if (postpones != nullptr)
@@ -4189,10 +4276,19 @@ namespace bpkg
// decisions" mode and select the alternative ignoring the current
// prerequisites.
//
+ // Note though, that if we are re-evaluating an existing dependent
+ // then we fail if we didn't succeed in the "recreate dependency
+ // decisions" mode.
+ //
const package_prerequisites* prereqs (src_conf && !ud
? &sp->prerequisites
: nullptr);
+ // During the dependent re-evaluation we always try to reproduce the
+ // existing setup.
+ //
+ assert (!reeval || prereqs != nullptr);
+
for (;;)
{
// The index and pre-collection result of the first satisfactory
@@ -4228,6 +4324,9 @@ namespace bpkg
{
if (r.repo_postpone)
{
+ if (reeval)
+ fail_reeval ();
+
postpone (nullptr); // Already inserted into postponed_repo.
break;
}
@@ -4257,6 +4356,7 @@ namespace bpkg
//
auto try_select = [postponed_alts, &max_alt_index,
&edas, &pkg,
+ reeval,
&trace,
&postpone, &collect, &select]
(size_t index, precollect_result&& r)
@@ -4270,6 +4370,11 @@ namespace bpkg
//
if (postponed_alts != nullptr && index >= max_alt_index)
{
+ // For a dependent re-evaluation max_alt_index is expected to
+ // be max size_t.
+ //
+ assert (!reeval);
+
l5 ([&]{trace << "alt-postpone dependent "
<< pkg.available_name_version_db ()
<< " since max index is reached: " << index <<
@@ -4298,9 +4403,11 @@ namespace bpkg
select (da, dai);
// Make sure no more true alternatives are selected during
- // this function call.
+ // this function call unless we are re-evaluating a dependent.
//
- max_alt_index = 0;
+ if (!reeval)
+ max_alt_index = 0;
+
return true;
}
else
@@ -4358,12 +4465,16 @@ namespace bpkg
break;
// Fail or postpone the collection if no 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.
+ // unless we are re-evaluating a dependent or are in the "recreate
+ // dependency decisions" mode. In the latter case fail for
+ // re-evaluation and fall back to the "make dependency decisions"
+ // mode and retry otherwise.
//
if (prereqs != nullptr)
{
+ if (reeval)
+ fail_reeval ();
+
prereqs = nullptr;
continue;
}
@@ -4439,9 +4550,19 @@ namespace bpkg
break;
}
+ if (reeval)
+ {
+ if (!reevaluated)
+ fail_reeval ();
+
+ assert (postponed);
+ }
+
dep_chain.pop_back ();
- l5 ([&]{trace << (!postponed ? "end " : "postpone ")
+ l5 ([&]{trace << (!postponed ? "end " :
+ reeval ? "reevaluated " :
+ "postpone ")
<< pkg.available_name_version_db ();});
}
@@ -5021,104 +5142,27 @@ namespace bpkg
build_package* b (entered_build (cp));
assert (b != nullptr);
- // @@ Re-evaluate up-to the earliest position.
-
- // @@ TMP: need proper implementation.
- //
- // @@ TODO: fail if we do not "arrive" at this position.
+ // Re-evaluate up-to the earliest position.
//
- {
- // @@ This emulates collect_build_prerequisites() called for
- // the first time and postponing the first dependency
- // alternative. See collect_build_prerequisites() for
- // details.
- //
- if (postponed_cfgs.find_dependency (cp) == nullptr)
- {
- vector<existing_dependent> eds (
- query_existing_dependents (trace,
- b->db,
- b->name (),
- replaced_vers,
- rpt_depts));
-
- if (!eds.empty ())
- {
- // @@ NOTE: doesn't really repeat the latest
- // implementation in collect_build_prerequisites().
- //
- existing_dependent& ed (eds.front ());
-
- l5 ([&]{trace << "cfg-postpone dependency "
- << b->available_name_version_db ()
- << " of existing dependent " << *ed.selected
- << ed.db;});
-
- postponed_cfgs.add (
- config_package (ed.db, ed.selected->name),
- ed.dependency_position,
- cp);
-
- // @@ Note that collect_build_prerequisites() returns here
- // while we continue. Is that right?
- //
- //return;
- }
- }
-
- b->recursive_collection = true;
-
- b->dependencies = dependencies ();
- b->alternatives = vector<size_t> ();
-
- optional<dir_path> src_root (b->external_dir ());
-
- optional<dir_path> out_root (
- src_root && !b->disfigure
- ? dir_path (b->db.get ().config) /= b->name ().string ()
- : optional<dir_path> ());
-
- const shared_ptr<available_package>& ap (b->available);
-
- b->skeleton =
- package_skeleton (o,
- b->db,
- *ap,
- b->config_vars,
- nullptr /* config_srcs */, // @@ TMP
- move (src_root),
- move (out_root));
+ assert (ed.dependency_position.first != 0);
- const auto& pos (ed.dependency_position);
-
- assert (pos.first != 0 && pos.second != 0);
-
- size_t di (pos.first - 1);
- size_t dai (pos.second - 1);
-
- assert (di < ap->dependencies.size () &&
- dai < ap->dependencies[di].size ());
-
- const dependency_alternative& da (ap->dependencies[di][dai]);
-
- build_package::dependency_alternatives_refs edas;
- edas.push_back (make_pair (ref (da), dai));
-
- b->postponed_dependency_alternatives = move (edas);
+ build_package_refs dep_chain;
- // Note: the dependent may already exist in the cluster
- // with a subset of dependencies.
- //
- // @@ TODO: we actually need to pass packages from da (all
- // dependencies) instead of ds (only postponed
- // dependencies). That's not easy to emulate at the moment
- // so let's do as a part of the proper implementation.
- //
- postponed_cfgs.add (move (cp),
- true /* existing */,
- pos,
- move (ds));
- }
+ collect_build_prerequisites (o,
+ *b,
+ fdb,
+ rpt_depts,
+ apc,
+ false /* initial_collection */,
+ replaced_vers,
+ dep_chain,
+ &postponed_repo,
+ &postponed_alts,
+ numeric_limits<size_t>::max (),
+ postponed_deps,
+ postponed_cfgs,
+ false /* force_configured */,
+ ed.dependency_position);
}
}
}
@@ -5231,9 +5275,13 @@ namespace bpkg
// package's dependency or some such.
//
if (di == deps.size ())
+ {
l5 ([&]{trace << "dependent " << b->available_name_version_db ()
<< " is already recursively collected, skipping";});
+ continue;
+ }
+
l5 ([&]{trace << "select cfg-negotiated dependency alternative "
<< "for dependent "
<< b->available_name_version_db ();});
@@ -6095,7 +6143,7 @@ namespace bpkg
{
bool build;
if (((build = *p->action == build_package::build) &&
- p->recollect_recursively (rpt_depts)) ||
+ (p->system || p->recollect_recursively (rpt_depts))) ||
*p->action == build_package::drop)
{
l5 ([&]{trace << "skip being " << (build ? "built" : "dropped")