From bfe1679447599a05d953c6c6376cbec09af0401d Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 11 Apr 2023 16:21:20 +0200 Subject: Fix holes in previous commit --- bpkg/pkg-build.cxx | 307 +++++++++++++++++++++++++++---------------------- bpkg/pkg-configure.cxx | 45 ++++++-- bpkg/pkg-configure.hxx | 10 +- 3 files changed, 218 insertions(+), 144 deletions(-) diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index adbcd8f..2484880 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -5362,6 +5362,26 @@ namespace bpkg 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)) { assert (p.action); @@ -5383,12 +5403,6 @@ namespace bpkg // assert (sp != nullptr || p.system); - if (p.system) - { - configure_packages.push_back (configure_package {p, {}}); - continue; - } - database& pdb (p.db); transaction t (pdb, !simulate /* start */); @@ -5410,119 +5424,142 @@ namespace bpkg }; configure_prerequisites_result cpr; - if (ap != nullptr) + if (p.system) { - 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. + // 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 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. + // Note: commits the transaction. // - if (p.dependencies) + sp = pkg_configure_system (ap->id.name, + p.available_version (), + pdb, + t); + } + else + { + if (ap != nullptr) { - assert (p.skeleton); + assert (*p.action == build_package::build); - cpr = pkg_configure_prerequisites (o, - pdb, - t, - *p.dependencies, - &*p.alternatives, - move (*p.skeleton), - nullptr /* prev_prerequisites */, - 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 ()); + + // Must be in the unpacked state since it was disfigured on the + // first pass (see above). + // + assert (sp->state == package_state::unpacked); - // 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. + // 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)); + + 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, - ap->dependencies, + deps, nullptr /* alternatives */, move (*p.skeleton), prereqs (), simulate, - fdb); + fdb, + configured_state); } - } - 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 && !sp->system ()); - // 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, find_available (o, pdb, sp)); - - 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); + t.commit (); } configure_packages.push_back (configure_package {p, move (cpr)}); - - t.commit (); } if (progress) @@ -5536,54 +5573,52 @@ namespace bpkg { build_package& p (cp.pkg); - shared_ptr& sp (p.selected); - const shared_ptr& ap (p.available); + const shared_ptr& sp (p.selected); - // Configure the package. + // 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); - transaction t (pdb, !simulate /* start */); - - // Show how we got here if things go wrong. - // - auto g ( - make_exception_guard ( - [&p] () - { - info << "while configuring " << p.name () << p.db; - })); - // Note that pkg_configure*() commits the transaction. - // - if (p.system) + if (!p.system) { - sp = pkg_configure_system (ap->id.name, - p.available_version (), - pdb, - t); - } - else 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); + const shared_ptr& ap (p.available); + + transaction t (pdb, !simulate /* start */); + + // Show how we got here if things go wrong. + // + auto g ( + make_exception_guard ( + [&p] () + { + info << "while configuring " << p.name () << p.db; + })); + + // Note that pkg_configure() commits the transaction. + // + 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; diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index c440877..59a6af0 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -28,7 +28,8 @@ namespace bpkg package_skeleton&& ps, const vector* prev_prereqs, bool simulate, - const function& fdb) + const function& fdb, + const function& fps) { tracer trace ("pkg_configure_prerequisites"); @@ -146,9 +147,15 @@ namespace bpkg const shared_ptr& dp (spd.first); - if (dp == nullptr || - dp->state != package_state::configured || - !satisfies (dp->version, d.constraint) || + if (dp == nullptr) + break; + + optional> dps; + if (fps != nullptr) + dps = fps (dp); + + if ((dps ? dps->first : dp->state) != package_state::configured || + !satisfies (dp->version, d.constraint) || (pps != nullptr && find (pps->begin (), pps->end (), dp->name) == pps->end ())) break; @@ -223,7 +230,13 @@ namespace bpkg { shared_ptr sp (pr.first.load ()); - if (!sp->system ()) + optional> ps; + if (fps != nullptr) + ps = fps (sp); + + if (ps + ? ps->second != package_substate::system + : !sp->system ()) { // @@ Note that this doesn't work for build2 modules that // require bootstrap. For their dependents we need to @@ -246,7 +259,22 @@ namespace bpkg // in the shared build2 context (but could probably do, // if necessary). // - dir_path od (sp->effective_out_root (pdb.config)); + + dir_path od; + if (ps) + { + // There is no out_root for a would-be configured package. + // So we calculate it like in pkg_configure() below (yeah, + // it's an ugly hack). + // + od = sp->external () + ? pdb.config / dir_path (sp->name.string ()) + : pdb.config / dir_path (sp->name.string () + '-' + + sp->version.string ()); + } + else + od = sp->effective_out_root (pdb.config); + vars.push_back ("config.import." + sp->name.variable () + "='" + od.representation () + '\''); } @@ -338,6 +366,8 @@ namespace bpkg // Calculate package's out_root. // + // Note: see a version of this in pkg_configure_prerequisites(). + // dir_path out_root ( p->external () ? c / dir_path (p->name.string ()) @@ -440,7 +470,8 @@ namespace bpkg move (ps), pps, simulate, - fdb)); + fdb, + nullptr)); pkg_configure (o, db, t, p, move (cpr), disfigured, simulate); } diff --git a/bpkg/pkg-configure.hxx b/bpkg/pkg-configure.hxx index 7587713..119da64 100644 --- a/bpkg/pkg-configure.hxx +++ b/bpkg/pkg-configure.hxx @@ -95,6 +95,13 @@ namespace bpkg vector config_sources; // Note: name and source. }; + // Return the "would be" state for packages that would be configured + // by this stage. + // + using find_package_state_function = + optional> ( + const shared_ptr&); + // Note: loads selected packages. // configure_prerequisites_result @@ -106,7 +113,8 @@ namespace bpkg package_skeleton&&, const vector* prev_prerequisites, bool simulate, - const function&); + const function&, + const function&); void pkg_configure (const common_options&, -- cgit v1.1