From 1aebd4750048f8c82a4f45e8670eac6bfdbb9ffb Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 19 Jul 2023 22:50:22 +0300 Subject: Fix re-evaluating multiple existing dependents with config clause of common dependency --- bpkg/pkg-build-collect.cxx | 56 ++++++++++++++++--------- tests/pkg-build.testscript | 101 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 19 deletions(-) diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index 729c145..83bb96a 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -1606,7 +1606,9 @@ namespace bpkg { for (existing_dependent& ed: eds) { - package_key dpk (ed.db, ed.selected->name); + package_key dpt (ed.db, ed.selected->name); + package_key dep (pk); + size_t& di (ed.dependency_position.first); const build_package* bp (&pkg); @@ -1623,7 +1625,7 @@ namespace bpkg // for details). // { - auto pi (postponed_poss.find (dpk)); + auto pi (postponed_poss.find (dpt)); if (pi != postponed_poss.end () && pi->second.first < di) { @@ -1654,7 +1656,7 @@ namespace bpkg replaced_vers, postponed_cfgs); - pk = package_key (bp->db, bp->name ()); + dep = package_key (bp->db, bp->name ()); // Note that here we side-step the bogus logic (by not setting // the skipped flag) because in this case (replace=true) our @@ -1676,7 +1678,7 @@ namespace bpkg for (const postponed_configuration& cfg: postponed_cfgs) { if (const pair* p = - cfg.existing_dependent_position (pk)) + cfg.existing_dependent_position (dpt)) { size_t ei (p->first); @@ -1687,7 +1689,7 @@ namespace bpkg postponed_position pp (ed.dependency_position, false /* replace */); - auto p (postponed_poss.emplace (move (pk), pp)); + auto p (postponed_poss.emplace (move (dpt), pp)); if (!p.second) { assert (p.first->second > pp); @@ -1726,7 +1728,23 @@ namespace bpkg << " of existing dependent " << *ed.selected << ed.db;}); - postponed_cfgs.add (move (dpk), ed.dependency_position, move (pk)); + // Only add this dependent/dependency to the newly created cluster + // if this dependency doesn't belong to any cluster yet, which may + // not be the case if there are multiple existing dependents with + // configuration clause for this dependency. + // + // To put it another way, if there are multiple such an existing + // dependents for this dependency, here we will create the + // configuration cluster only for the first one. The remaining + // dependents will be added to this dependency's cluster when the + // existing dependents of dependencies in this cluster are all + // discovered and reevaluated (see collect_build_postponed() for + // details). + // + if (postponed_cfgs.find_dependency (dep) == nullptr) + postponed_cfgs.add (move (dpt), + ed.dependency_position, + move (dep)); } return; @@ -1861,12 +1879,12 @@ namespace bpkg package_skeleton& skel (*pkg.skeleton); auto fail_reeval = [&pkg] () - { - fail << "unable to re-create dependency information of already " - << "configured package " << pkg.available_name_version_db () << + { + fail << "unable to re-create dependency information of already " + << "configured package " << pkg.available_name_version_db () << info << "likely cause is change in external environment" << info << "consider resetting the build configuration"; - }; + }; bool postponed (false); bool reevaluated (false); @@ -2633,14 +2651,14 @@ namespace bpkg reused = false; r.push_back (prebuild {d, - *ddb, - move (dsp), - move (dap), - move (rp.second), - system, - specified, - force, - ru}); + *ddb, + move (dsp), + move (dap), + move (rp.second), + system, + specified, + force, + ru}); } // Now, as we have pre-collected the dependency builds, go through @@ -5292,7 +5310,7 @@ namespace bpkg // Note that in this case we keep the accumulated configuration, // if any. - pc->depth = 0; + pc->depth = 0; // Mark as non-negotiated. // If requested, "replace" the "later" dependent-dependency // cluster with an earlier. diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index a891ca3..945402b 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -6060,6 +6060,107 @@ test.arguments += --sys-no-query $pkg_drop libfoo } + : multiple-dependents + : + { + $clone_cfg; + + $* libfoo/0.1.0 foo/1.0.0 fox/1.0.0 2>!; + + $pkg_status -r >>EOO; + !libfoo configured !0.1.0 available 1.0.0 + !foo configured !1.0.0 + !libfoo configured !0.1.0 available 1.0.0 + !fox configured !1.0.0 + !libfoo configured !0.1.0 available 1.0.0 + EOO + + $* libfoo 2>>~%EOE%; + %.* + trace: pkg_build: refine package collection/plan execution from scratch + %.* + trace: collect_build: add libfoo/1.0.0 + %.* + trace: collect_build_prerequisites: cfg-postpone dependency libfoo/1.0.0 of existing dependent foo/1.0.0 + trace: postponed_configurations::add: create {foo^ | libfoo->{foo/1,1}} + trace: collect_build_prerequisites: cfg-postpone dependency libfoo/1.0.0 of existing dependent fox/1.0.0 + trace: collect_build_postponed (0): begin + trace: collect_build_postponed (1): begin {foo^ | libfoo->{foo/1,1}} + %.* + trace: collect_build_postponed (1): re-evaluate existing dependents for {foo^ | libfoo->{foo/1,1}} + %.* + trace: collect_build: add foo/1.0.0 + trace: collect_build_prerequisites: reeval foo/1.0.0 + %.* + trace: collect_build: pick libfoo/1.0.0 over libfoo/0.1.0 + trace: postponed_configurations::add: add {foo^ 1,1: libfoo} to {foo^ | libfoo->{foo/1,1}} + trace: collect_build_prerequisites: re-evaluating dependent foo/1.0.0 results in {foo^ | libfoo->{foo/1,1}} + trace: collect_build_prerequisites: re-evaluated foo/1.0.0 + %.* + trace: collect_build: add fox/1.0.0 + trace: collect_build_prerequisites: reeval fox/1.0.0 + %.* + trace: collect_build: pick libfoo/1.0.0 over libfoo/0.1.0 + trace: postponed_configurations::add: add {fox^ 1,1: libfoo} to {foo^ | libfoo->{foo/1,1}} + trace: collect_build_prerequisites: re-evaluating dependent fox/1.0.0 results in {foo^ fox^ | libfoo->{foo/1,1 fox/1,1}} + trace: collect_build_prerequisites: re-evaluated fox/1.0.0 + trace: collect_build_postponed (1): cfg-negotiate begin {foo^ fox^ | libfoo->{foo/1,1 fox/1,1}} + %.* + trace: collect_build_postponed (1): recursively collect cfg-negotiated dependencies + trace: collect_build_prerequisites: begin libfoo/1.0.0 + trace: collect_build_prerequisites: end libfoo/1.0.0 + trace: collect_build_postponed (1): recursively collect cfg-negotiated dependents + trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent foo/1.0.0 + trace: collect_build_prerequisites: resume foo/1.0.0 + trace: collect_build_prerequisites: end foo/1.0.0 + trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent fox/1.0.0 + trace: collect_build_prerequisites: resume fox/1.0.0 + trace: collect_build_prerequisites: end fox/1.0.0 + trace: collect_build_postponed (1): cfg-negotiate end {foo^ fox^ | libfoo->{foo/1,1 fox/1,1}}! + trace: collect_build_postponed (1): end {foo^ | libfoo->{foo/1,1}} + trace: collect_build_postponed (0): end + %.* + trace: execute_plan: simulate: yes + %.* + build plan: + upgrade libfoo/1.0.0 + config.libfoo.extras=true (set by foo) + reconfigure fox/1.0.0 (dependent of libfoo) + reconfigure foo/1.0.0 (dependent of libfoo) + %.* + disfigured foo/1.0.0 + %.* + disfigured fox/1.0.0 + %.* + disfigured libfoo/0.1.0 + %.* + fetched libfoo/1.0.0 + %.* + unpacked libfoo/1.0.0 + %.* + configured libfoo/1.0.0 + %.* + configured foo/1.0.0 + %.* + updated libfoo/1.0.0 + %.* + updated fox/1.0.0 + %.* + updated foo/1.0.0 + %.* + EOE + + $pkg_status -r >>EOO; + !libfoo configured 1.0.0 + !foo configured !1.0.0 + !libfoo configured 1.0.0 + !fox configured !1.0.0 + !libfoo configured 1.0.0 + EOO + + $pkg_drop libfoo foo fox + } + : postpone-existing : { -- cgit v1.1