From 306ad185e8d8f50923316bdd1b68ef9a3b1e50e6 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 11 Apr 2023 14:47:42 +0200 Subject: Split package configuration into two passes in pkg-build --- bpkg/pkg-build.cxx | 309 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 201 insertions(+), 108 deletions(-) (limited to 'bpkg/pkg-build.cxx') diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index d30555f..2484880 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -5350,14 +5350,37 @@ namespace bpkg return true; }; - if (progress) + // On the first pass collect all the build_package's to be configured and + // calculate their configure_prerequisites_result's. + // + struct configure_package { - prog_i = 0; - prog_n = static_cast (count_if (build_pkgs.begin (), - build_pkgs.end (), - configure_pred)); - prog_percent = 100; - } + reference_wrapper pkg; + configure_prerequisites_result res; // Unused for system package. + }; + vector configure_packages; + + configure_packages.reserve (build_pkgs.size ()); + + // Return the "would be" state of packages that would be configured + // by this stage. + // + function configured_state ( + [&configure_packages] (const shared_ptr& sp) + -> optional> + { + for (const configure_package& cp: configure_packages) + { + const build_package& p (cp.pkg); + + if (p.selected == sp) + return make_pair ( + package_state::configured, + p.system ? package_substate::system : package_substate::none); + } + + return nullopt; + }); for (build_package& p: reverse_iterate (build_pkgs)) { @@ -5369,7 +5392,7 @@ namespace bpkg shared_ptr& sp (p.selected); const shared_ptr& ap (p.available); - // Configure the package. + // Collect the package. // // At this stage the package is either selected, in which case it's a // source code one, or just available, in which case it is a system @@ -5381,7 +5404,6 @@ namespace bpkg assert (sp != nullptr || p.system); database& pdb (p.db); - transaction t (pdb, !simulate /* start */); // Show how we got here if things go wrong, for example selecting a @@ -5401,131 +5423,202 @@ namespace bpkg return i != previous_prerequisites.end () ? &i->second : nullptr; }; - // Note that pkg_configure() commits the transaction. - // + configure_prerequisites_result cpr; if (p.system) { + // We have no choice but to configure system packages on the first + // pass since otherwise there will be no selected package for + // pkg_configure_prerequisites() to find. Luckily they have no + // dependencies and so can be configured in any order. We will print + // their progress/result on the second pass in the proper order. + // + // Note: commits the transaction. + // sp = pkg_configure_system (ap->id.name, p.available_version (), pdb, t); } - else if (ap != nullptr) + else { - assert (*p.action == build_package::build); - - // If the package prerequisites builds are collected, then use the - // resulting package skeleton and the pre-selected dependency - // alternatives. - // - // Note that we may not collect the package prerequisites builds if - // the package is already configured but we still need to reconfigure - // it due, for example, to an upgrade of its dependency. In this case - // we pass to pkg_configure() the newly created package skeleton which - // contains the package configuration variables specified on the - // command line but (naturally) no reflection configuration variables. - // Note, however, that in this case pkg_configure() call will evaluate - // the reflect clauses itself and so the proper reflection variables - // will still end up in the package configuration. - // - // @@ Note that if we ever allow the user to override the alternative - // selection, this will break (and also if the user re-configures - // the package manually). Maybe that a good reason not to allow - // this? Or we could store this information in the database. - // - if (p.dependencies) + if (ap != nullptr) { - assert (p.skeleton); + assert (*p.action == build_package::build); - pkg_configure (o, - pdb, - t, - sp, - *p.dependencies, - &*p.alternatives, - move (*p.skeleton), - nullptr /* previous_prerequisites */, - p.disfigure, - simulate, - fdb); + // If the package prerequisites builds are collected, then use the + // resulting package skeleton and the pre-selected dependency + // alternatives. + // + // Note that we may not collect the package prerequisites builds if + // the package is already configured but we still need to + // reconfigure it due, for example, to an upgrade of its dependency. + // In this case we pass to pkg_configure() the newly created package + // skeleton which contains the package configuration variables + // specified on the command line but (naturally) no reflection + // configuration variables. Note, however, that in this case + // pkg_configure() call will evaluate the reflect clauses itself and + // so the proper reflection variables will still end up in the + // package configuration. + // + // @@ Note that if we ever allow the user to override the + // alternative selection, this will break (and also if the user + // re-configures the package manually). Maybe that a good reason + // not to allow this? Or we could store this information in the + // database. + // + if (p.dependencies) + { + assert (p.skeleton); + + cpr = pkg_configure_prerequisites (o, + pdb, + t, + *p.dependencies, + &*p.alternatives, + move (*p.skeleton), + nullptr /* prev_prerequisites */, + simulate, + fdb, + configured_state); + } + else + { + assert (sp != nullptr); // See above. + + // Note that the skeleton can be present if, for example, this is + // a dependency which configuration has been negotiated but it is + // not collected recursively since it has no buildfile clauses. + // + if (!p.skeleton) + p.init_skeleton (o); + + cpr = pkg_configure_prerequisites (o, + pdb, + t, + ap->dependencies, + nullptr /* alternatives */, + move (*p.skeleton), + prereqs (), + simulate, + fdb, + configured_state); + } } - else + else // Dependent. { - assert (sp != nullptr); // See above. + // This is an adjustment of a dependent which cannot be system + // (otherwise it wouldn't be a dependent) and cannot become system + // (otherwise it would be a build). + // + assert (*p.action == build_package::adjust && !sp->system ()); - // Note that the skeleton can be present if, for example, this is a - // dependency which configuration has been negotiated but it is not - // collected recursively since it has no buildfile clauses. + // Must be in the unpacked state since it was disfigured on the + // first pass (see above). // + assert (sp->state == package_state::unpacked); + + // Initialize the skeleton if it is not initialized yet. + // + // Note that the skeleton can only be present here if it was + // initialized during the preparation of the plan and so this plan + // execution is not simulated (see above for details). + // + // Also note that there is no available package specified for the + // build package object here and so we need to find it (or create a + // transient one). + // + assert (p.available == nullptr && (!p.skeleton || !simulate)); + if (!p.skeleton) - p.init_skeleton (o); + p.init_skeleton (o, find_available (o, pdb, sp)); - pkg_configure (o, - pdb, - t, - sp, - ap->dependencies, - nullptr /* alternatives */, - move (*p.skeleton), - prereqs (), - p.disfigure, - simulate, - fdb); + assert (p.skeleton->available != nullptr); // Can't be system. + + const dependencies& deps (p.skeleton->available->dependencies); + + // @@ Note that on reconfiguration the dependent looses the + // potential configuration variables specified by the user on + // some previous build, which can be quite surprising. Should we + // store this information in the database? + // + // Note: this now works for external packages via package + // skeleton (which extracts user configuration). + // + cpr = pkg_configure_prerequisites (o, + pdb, + t, + deps, + nullptr /* alternatives */, + move (*p.skeleton), + prereqs (), + simulate, + fdb, + configured_state); } + + t.commit (); } - else // Dependent. - { - // This is an adjustment of a dependent which cannot be system - // (otherwise it wouldn't be a dependent) and cannot become system - // (otherwise it would be a build). - // - assert (*p.action == build_package::adjust && - !p.system && - !sp->system ()); - // Must be in the unpacked state since it was disfigured on the first - // pass (see above). - // - assert (sp->state == package_state::unpacked); + configure_packages.push_back (configure_package {p, move (cpr)}); + } - // Initialize the skeleton if it is not initialized yet. - // - // Note that the skeleton can only be present here if it was - // initialized during the preparation of the plan and so this plan - // execution is not simulated (see above for details). - // - // Also note that there is no available package specified for the - // build package object here and so we need to find it (or create a - // transient one). - // - assert (p.available == nullptr && (!p.skeleton || !simulate)); + if (progress) + { + prog_i = 0; + prog_n = configure_packages.size (); + prog_percent = 100; + } - if (!p.skeleton) - p.init_skeleton (o, find_available (o, pdb, sp)); + for (configure_package& cp: configure_packages) + { + build_package& p (cp.pkg); - assert (p.skeleton->available != nullptr); // Can't be system. + const shared_ptr& sp (p.selected); - const dependencies& deps (p.skeleton->available->dependencies); + // Configure the package (system already configured). + // + // NOTE: remember to update the preparation of the plan to be presented + // to the user if changing anything here. + // + database& pdb (p.db); + + if (!p.system) + { + const shared_ptr& ap (p.available); + + transaction t (pdb, !simulate /* start */); - // @@ Note that on reconfiguration the dependent looses the potential - // configuration variables specified by the user on some previous - // build, which can be quite surprising. Should we store this - // information in the database? + // Show how we got here if things go wrong. // - // I believe this now works for external packages via package - // skeleton (which extracts user configuration). + auto g ( + make_exception_guard ( + [&p] () + { + info << "while configuring " << p.name () << p.db; + })); + + // Note that pkg_configure() commits the transaction. // - pkg_configure (o, - pdb, - t, - sp, - deps, - nullptr /* alternatives */, - move (*p.skeleton), - prereqs (), - false /* disfigured */, - simulate, - fdb); + if (ap != nullptr) + { + pkg_configure (o, + pdb, + t, + sp, + move (cp.res), + p.disfigure, + simulate); + } + else // Dependent. + { + pkg_configure (o, + pdb, + t, + sp, + move (cp.res), + false /* disfigured */, + simulate); + } } r = true; -- cgit v1.1