From e826ff4361bced296a8d7af57228755d22a31f15 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Sat, 26 Aug 2023 14:21:54 +0300 Subject: Fix configuration negotiation for cases when existing dependent collection is postponed --- bpkg/pkg-build-collect.cxx | 51 +++++++++++++++++++++++++++++++++----- bpkg/pkg-build.cxx | 62 +++++++++++++++++++++++++++++----------------- 2 files changed, 84 insertions(+), 29 deletions(-) (limited to 'bpkg') diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index de944a4..30a9e90 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -4736,8 +4736,8 @@ namespace bpkg assert (!pcfg->negotiated); // Re-evaluate existing dependents for dependencies in this - // configuration cluster. Omit dependents which are already being built - // or dropped. + // configuration cluster. Omit dependents which are already being built, + // dropped, or postponed. // // Note that the existing dependent can be re-evaluated to an earlier // position than the position of the dependency which has introduced @@ -4754,6 +4754,15 @@ namespace bpkg // collected in the same way and at the same time as the new dependents // of the clusters they belong to. // + // Note that some of the postponed existing dependents may already be in + // the cluster. Thus, collect the postponed existing dependents to omit + // them from the configuration negotiation and from the subsequent + // recursive collection. Note that we will up-negotiate the + // configuration these dependents apply to their dependencies after + // these dependents will be collected via their own dependents with the + // configuration clauses. + // + set postponed_existing_dependents; { // Map existing dependents to the dependencies they apply a // configuration to. Also, collect the information which is required @@ -4825,6 +4834,7 @@ namespace bpkg // pi->second.wout_config = true; + postponed_existing_dependents.insert (pk); continue; } @@ -4978,6 +4988,16 @@ namespace bpkg i (b), e (pcfg->dependents.end ()); i != e; ) { + if (postponed_existing_dependents.find (i->first) != + postponed_existing_dependents.end ()) + { + l5 ([&]{trace << "skip dep-postponed existing dependent " + << i->first;}); + + ++i; + continue; + } + // Resolve package skeletons for the dependent and its dependencies. // // For the dependent, the skeleton should be already there (since we @@ -5119,7 +5139,20 @@ namespace bpkg // if (!b->recursive_collection) { - assert (b->skeleton); // Should have been init'ed above. + // Note that due to the existing dependents postponement some of the + // dependencies may have no dependent configuration applied to them + // at this time. In this case such dependencies may have no skeleton + // yet and thus we initialize it. Note that we will still apply the + // empty configuration to such dependencies and collect them + // recursively, since the negotiation machinery relies on the fact + // that the dependencies of a negotiated cluster are (being) + // recursively collected. When the time comes and such a dependency + // is collected via its (currently postponed) existing dependent, + // then its configuration will be up-negotiated (likely involving + // throwing the retry_configuration exception). + // + if (!b->skeleton) + b->init_skeleton (o /* options */); package_skeleton& ps (*b->skeleton); @@ -5202,6 +5235,13 @@ namespace bpkg for (const auto& p: dependents) { + if (postponed_existing_dependents.find (p) != + postponed_existing_dependents.end ()) + { + l5 ([&]{trace << "skip dep-postponed existing dependent " << p;}); + continue; + } + // Select the dependency alternative for which configuration has been // negotiated and collect this dependent starting from the next // depends value. @@ -5955,9 +5995,8 @@ namespace bpkg } // While the assumption is that we shouldn't leave any non-negotiated - // clusters, we can potentially miss some corner cases in the above "skip - // configuration" logic. Let's thus trace the non-negotiated clusters - // before the assertion. + // clusters, let's verify that for good measure. Let's thus trace the + // non-negotiated clusters before the assertion. // #ifndef NDEBUG for (const postponed_configuration& cfg: postponed_cfgs) diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 79f89bb..02ced21 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -4350,11 +4350,6 @@ namespace bpkg } else { - // Wouldn't be here otherwise. - // - assert (postponed_deps.find (package_key {ddb, d.name}) == - postponed_deps.end ()); - shared_ptr sp ( ddb.find (d.name)); @@ -4366,7 +4361,7 @@ namespace bpkg // Marking upgraded dependencies as "required by command line" // may seem redundant as they should already be pre-entered as // such (see above). But remember dependencies upgraded with - // -i|-r? Note that the required_by data member should never be + // -i|-r? Note that the required_by data member should never be // empty, as it is used in prompts/diagnostics. // build_package p { @@ -4396,25 +4391,46 @@ namespace bpkg ? build_package::build_replace : uint16_t (0))}; - build_package_refs dep_chain; + package_key pk {ddb, d.name}; - // Note: recursive. + // Similar to the user-selected packages, collect non- + // recursively the dependencies for which recursive collection + // is postponed (see above for details). // - pkgs.collect_build (o, - move (p), - replaced_vers, - postponed_cfgs, - &dep_chain, - true /* initial_collection */, - find_prereq_database, - add_priv_cfg, - &rpt_depts, - &postponed_repo, - &postponed_alts, - &postponed_recs, - &postponed_edeps, - &postponed_deps, - &unacceptable_alts); + auto i (postponed_deps.find (pk)); + if (i == postponed_deps.end ()) + { + build_package_refs dep_chain; + + // Note: recursive. + // + pkgs.collect_build (o, + move (p), + replaced_vers, + postponed_cfgs, + &dep_chain, + true /* initial_collection */, + find_prereq_database, + add_priv_cfg, + &rpt_depts, + &postponed_repo, + &postponed_alts, + &postponed_recs, + &postponed_edeps, + &postponed_deps, + &unacceptable_alts); + } + else + { + i->second.wout_config = true; + + l5 ([&]{trace << "dep-postpone user-specified dependency " + << pk;}); + + // Note: not recursive. + // + pkgs.collect_build (o, move (p), replaced_vers, postponed_cfgs); + } } } -- cgit v1.1