From 8226c7c7a2bffc96d70f297e792f3c0afbce67f0 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Sat, 2 Sep 2023 22:54:21 +0300 Subject: Scratch plan on refinement caused by dependency up/down-grade/drop if any configuration negotiation has been performed --- bpkg/pkg-build-collect.cxx | 143 +++++++++++++++++++++------------------------ bpkg/pkg-build-collect.hxx | 14 ++++- bpkg/pkg-build.cxx | 13 +++++ 3 files changed, 93 insertions(+), 77 deletions(-) (limited to 'bpkg') diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index ec95e7c..47c5344 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -248,11 +248,6 @@ namespace bpkg // assert ((flags & build_repoint) == 0 || (p.flags & build_repoint) == 0); - // We never merge two existing dependent re-evaluations. - // - assert ((flags & build_reevaluate) == 0 || - (p.flags & build_reevaluate) == 0); - // Copy the user-specified options/variables. // if (p.user_selection ()) @@ -324,14 +319,19 @@ namespace bpkg if (!hold_version || (p.hold_version && *p.hold_version > *hold_version)) hold_version = p.hold_version; - // Copy state flags. + // Copy state flags and upgrade dependent repointments and re-evaluations + // to the full builds. But in contrast to the repointed dependents we may + // merge two dependent re-evaluations. // - flags |= p.flags; + flags |= (p.flags & ~build_reevaluate); - // Upgrade dependent repointments and re-evaluations to the full builds. - // if (*action == build) - flags &= ~(build_repoint | build_reevaluate); + { + flags &= ~build_repoint; + + if ((p.flags & build_reevaluate) == 0) + flags &= ~build_reevaluate; + } // Note that we don't copy the build_package::system flag. If it was // set from the command line ("strong system") then we will also have @@ -1422,32 +1422,6 @@ namespace bpkg } else { - // Treat the replacement of the existing dependent that is participating - // in the configuration negotiation also as a version replacement. This - // way we will not be treating the dependent as an existing on the - // re-collection (see query_existing_dependents() for details). - // - // Note: an existing dependent may not be configured as system. - // - if (pkg.selected != nullptr && - (pkg.selected->version != pkg.available_version () || - pkg.system)) - { - for (const postponed_configuration& cfg: postponed_cfgs) - { - auto i (cfg.dependents.find (pk)); - - if (i != cfg.dependents.end () && i->second.existing) - { - l5 ([&]{trace << "existing dependent " << *pkg.selected << pkg.db - << " needs to be replaced with " - << pkg.available_name_version_db ();}); - - replace_ver (pkg); - } - } - } - // This is the first time we are adding this package name to the map. // l4 ([&]{trace << "add " << pkg.available_name_version_db ();}); @@ -1646,6 +1620,12 @@ namespace bpkg << ed.db << " due to dependency " << pkg.available_name_version_db ();}); + collect_existing_dependent (options, + ed, + {pk}, + replaced_vers, + postponed_cfgs); + // 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 @@ -4950,13 +4930,6 @@ namespace bpkg if (ed.reevaluated) continue; - const package_key& pk (d.first); - packages& ds (ed.dependencies); - - pair, - lazy_shared_ptr> rp ( - find_available_fragment (o, pk.db, ed.selected)); - // Note that a re-evaluated package doesn't necessarily needs to // be reconfigured and thus we don't add the // build_package::adjust_reconfigure flag here. @@ -4967,38 +4940,13 @@ namespace bpkg // build_package::adjust_reconfigure flag will be added normally // by collect_order_dependents(). // - build_package p { - build_package::build, - pk.db, - move (ed.selected), - move (rp.first), - move (rp.second), - nullopt, // Dependencies. - nullopt, // Dependencies alternatives. - nullopt, // Package skeleton. - nullopt, // Postponed dependency alternatives. - false, // Recursive collection. - nullopt, // Hold package. - nullopt, // Hold version. - {}, // Constraints. - false, // System. - false, // Keep output directory. - false, // Disfigure (from-scratch reconf). - false, // Configure-only. - nullopt, // Checkout root. - false, // Checkout purge. - strings (), // Configuration variables. - set ( // Required by (dependency). - make_move_iterator (ds.begin ()), - make_move_iterator (ds.end ())), - false, // Required by dependents. - build_package::build_reevaluate}; - - // Note: not recursive. - // - collect_build (o, move (p), replaced_vers, postponed_cfgs); + collect_existing_dependent (o, + ed, + move (ed.dependencies), + replaced_vers, + postponed_cfgs); - build_package* b (entered_build (pk)); + build_package* b (entered_build (d.first)); assert (b != nullptr); // Re-evaluate up to the earliest position. @@ -6706,7 +6654,7 @@ namespace bpkg strings (), // Configuration variables. {dpt}, // Required by (dependent). true, // Required by dependents. - 0}; + 0}; // State flags. // Add constraints, if present. // @@ -6728,6 +6676,51 @@ namespace bpkg } void build_packages:: + collect_existing_dependent (const pkg_build_options& o, + const existing_dependent& ed, + postponed_configuration::packages&& ds, + replaced_versions& replaced_vers, + postponed_configurations& postponed_cfgs) + { + assert (ed.dependency); // May not be a deviated existing dependent. + + pair, + lazy_shared_ptr> rp ( + find_available_fragment (o, ed.db, ed.selected)); + + build_package p { + build_package::build, + ed.db, + ed.selected, + move (rp.first), + move (rp.second), + nullopt, // Dependencies. + nullopt, // Dependencies alternatives. + nullopt, // Package skeleton. + nullopt, // Postponed dependency alternatives. + false, // Recursive collection. + nullopt, // Hold package. + nullopt, // Hold version. + {}, // Constraints. + false, // System. + false, // Keep output directory. + false, // Disfigure (from-scratch reconf). + false, // Configure-only. + nullopt, // Checkout root. + false, // Checkout purge. + strings (), // Configuration variables. + set ( // Required by (dependency). + make_move_iterator (ds.begin ()), + make_move_iterator (ds.end ())), + false, // Required by dependents. + build_package::build_reevaluate}; + + // Note: not recursive. + // + collect_build (o, move (p), replaced_vers, postponed_cfgs); + } + + void build_packages:: collect_deviated_dependent (const pkg_build_options& o, const existing_dependent& ed, package_key orig_dep, diff --git a/bpkg/pkg-build-collect.hxx b/bpkg/pkg-build-collect.hxx index 3621732..1ce6962 100644 --- a/bpkg/pkg-build-collect.hxx +++ b/bpkg/pkg-build-collect.hxx @@ -1090,8 +1090,6 @@ namespace bpkg // instead. Add entry to replaced_vers and throw replace_version if the // existing version needs to be replaced but the new version cannot be // re-collected recursively in-place (see replaced_versions for details). - // Also add an entry and throw if the existing dependent needs to be - // replaced. // // Optionally, pass the function which verifies the chosen package // version. It is called before replace_version is potentially thrown or @@ -1470,6 +1468,18 @@ namespace bpkg replaced_versions&, postponed_configurations&); + // Non-recursively collect an existing non-deviated dependent previously + // returned by the query_existing_dependents() function call for the + // subsequent re-evaluation. + // + void + collect_existing_dependent ( + const pkg_build_options&, + const existing_dependent&, + postponed_configuration::packages&& dependencies, + replaced_versions&, + postponed_configurations&); + // Non-recursively collect the deviated existing dependent previously // returned by the query_existing_dependents() function call and add it to // the postponed package recollections list. diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 1554f01..e2dea9d 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -3867,6 +3867,11 @@ namespace bpkg // grade order where any subsequent entry does not affect the decision of // the previous ones. // + // Note that we also need to rebuild the plan from scratch on adding a new + // up/down-grade/drop if any dependency configuration negotiation has been + // performed, since any package replacement may affect the already + // negotiated configurations. + // // Package managers are an easy, already solved problem, right? // build_packages pkgs; @@ -4916,8 +4921,16 @@ namespace bpkg refine = need_refinement (); + // If no further refinement is necessary, then perform the + // diagnostics run. Otherwise, if any dependency configuration + // negotiation has been performed during the current plan refinement + // iteration, then rebuild the plan from scratch (see above for + // details). + // if (!refine) need_refinement (true /* diag */); + else if (!postponed_cfgs.empty ()) + scratch_exe = true; } // Note that we prevent building multiple instances of the same -- cgit v1.1