From 4d0df41f94bec13a9af5c76b0176596f1ca3ab7b Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 12 Apr 2023 07:34:57 +0200 Subject: API rework --- bpkg/buildfile | 2 + bpkg/pkg-build.cli | 9 ++- bpkg/pkg-build.cxx | 100 ++++++++++++++++++++--- bpkg/pkg-configure.cxx | 209 ++++++++++++++++++++++++++++--------------------- bpkg/pkg-configure.hxx | 11 ++- 5 files changed, 223 insertions(+), 108 deletions(-) diff --git a/bpkg/buildfile b/bpkg/buildfile index 6ab6c11..6bddbb8 100644 --- a/bpkg/buildfile +++ b/bpkg/buildfile @@ -98,6 +98,8 @@ for t: cxx{**.test...} # Build options. # +cxx.poptions += -DBPKG_OUTPROC_CONFIGURE + obj{utility}: cxx.poptions += \ "-DBPKG_EXE_PREFIX=\"$bin.exe.prefix\"" \ "-DBPKG_EXE_SUFFIX=\"$bin.exe.suffix\"" \ diff --git a/bpkg/pkg-build.cli b/bpkg/pkg-build.cli index 28e4fa2..6aee9b0 100644 --- a/bpkg/pkg-build.cli +++ b/bpkg/pkg-build.cli @@ -119,10 +119,11 @@ namespace bpkg Additional configuration variables (), if any, should be specified before packages () and should be separated with \cb{--}. Such variables are effective only when configuring and only for - packages that were explicitly specified on the command line (they can - also be specified to only apply to specific packages using the argument - grouping mechanism discussed below). See \l{bpkg-pkg-configure(1)} for - more information on configuration variables. + packages that were explicitly specified on the command line (unless + global overrides). They can also be specified to only apply to specific + packages using the argument grouping mechanism discussed below. See + \l{bpkg-pkg-configure(1)} for more information on configuration + variables. By default a package that is specified explicitly on the command line is built to \i{hold}: it will not be considered for automatic removal if it diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 61b5916..5d73eb7 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -1245,7 +1245,13 @@ namespace bpkg if (a.find ('=') == string::npos) fail << "unexpected group argument '" << a << "'"; - cvs.push_back (move (trim (a))); + trim (a); + + if (a[0] == '!') + fail << "global override in package-specific configuration " + << "variable '" << a << "'"; + + cvs.push_back (move (a)); } } @@ -5356,12 +5362,23 @@ namespace bpkg struct configure_package { reference_wrapper pkg; - configure_prerequisites_result res; // Unused for system package. + + // These are unused for system packages. + // + configure_prerequisites_result res; + build2::variable_overrides ovrs; }; vector configure_packages; - configure_packages.reserve (build_pkgs.size ()); + // While at it also extract global configuration variable overrides from + // each configure_prerequisites_result::config_variables and merge them + // into configure_global_vars. + // +#ifndef BPKG_OUTPROC_CONFIGURE + strings configure_global_vars; +#endif + // Return the "would be" state of packages that would be configured // by this stage. // @@ -5557,11 +5574,72 @@ namespace bpkg } t.commit (); + + /* + if (!simulate && !cpr.config_variables.empty ()) + { + diag_record dr (text); + + dr << sp->name << pdb << ':'; + + for (const string& cv: cpr.config_variables) + dr << "\n " << cv; + } + */ + + if (!simulate) + { + +#ifndef BPKG_OUTPROC_CONFIGURE + auto& gvs (configure_global_vars); + + for (auto i (cpr.config_variables.begin ()); + i != cpr.config_variables.end (); ) + { + // Each package should have exactly the same set of global + // overrides by construction since we don't allow package- + // specific global overrides. + // + string& v (*i); + + if (v[0] == '!') + { + if (find (gvs.begin (), gvs.end (), v) == gvs.end ()) + gvs.push_back (move (v)); + + i = cpr.config_variables.erase (i); + } + else + ++i; + } +#endif + // Add config.config.disfigure unless already disfigured (see the + // high-level pkg_configure() version for background). + // + if (ap == nullptr || !p.disfigure) + { + cpr.config_variables.push_back ( + "config.config.disfigure='config." + sp->name.variable () + "**'"); + } + } } - configure_packages.push_back (configure_package {p, move (cpr)}); + configure_packages.push_back (configure_package {p, move (cpr), {}}); } + // Reuse the build state to avoid reloading the dependencies over and over + // again. This is a valid optimization since we are configuring in the + // dependency-dependent order. + // + unique_ptr configure_ctx; + +#ifndef BPKG_OUTPROC_CONFIGURE + // @@ TODO: create context. Unless simulating. + + // @@ TODO + //assert (ctx->var_overrides.empty ()); // Global only. +#endif + if (progress) { prog_i = 0; @@ -5569,12 +5647,6 @@ namespace bpkg prog_percent = 100; } - // Reuse the build state to avoid reloading the dependencies over and over - // again. This is a valid optimization since we are configuring in the - // dependency-dependent order. - // - unique_ptr configure_ctx; - for (configure_package& cp: configure_packages) { build_package& p (cp.pkg); @@ -5612,7 +5684,8 @@ namespace bpkg t, sp, move (cp.res), - p.disfigure, + configure_ctx, + cp.ovrs, simulate); } else // Dependent. @@ -5622,7 +5695,8 @@ namespace bpkg t, sp, move (cp.res), - false /* disfigured */, + configure_ctx, + cp.ovrs, simulate); } } @@ -5649,7 +5723,9 @@ namespace bpkg } } +#ifndef BPKG_OUTPROC_CONFIGURE configure_ctx.reset (); // Free. +#endif // Clear the progress if shown. // diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index fbeb647..ded567d 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -356,13 +356,86 @@ namespace bpkg move (srcs)}; } + + unique_ptr + pkg_configure_context ( + const common_options& o, + strings&& cmd_vars, + const function& var_ovr_func) + { + using namespace build2; + + // Initialize the build system. + // + // Note that this takes into account --build-option and default options + // files (which may have global overrides and which end up in + // build2_cmd_vars). + // + if (!build2_sched.started ()) + build2_init (o); + + // Re-tune the scheduler for parallel execution (see build2_init() + // for details). + // + if (!build2_sched.serial ()) + build2_sched.tune (0); + + auto merge_cmd_vars = [&cmd_vars] () -> const strings& + { + if (cmd_vars.empty ()) + return build2_cmd_vars; + + if (!build2_cmd_vars.empty ()) + cmd_vars.insert (cmd_vars.begin (), + build2_cmd_vars.begin (), build2_cmd_vars.end ()); + + return cmd_vars; + }; + + // Shouldn't we shared the module context with package skeleton + // contexts? Maybe we don't have to since we don't build modules in + // them concurrently (in a sence, we didn't share it when we were + // invoking the build system driver). + // + unique_ptr ctx ( + new context (build2_sched, + build2_mutexes, + build2_fcache, + false /* match_only */, + false /* no_external_modules */, + false /* dry_run */, + false /* no_diag_buffer */, + false /* keep_going */, + merge_cmd_vars (), + context::reserves { + 30000 /* targets */, + 1100 /* variables */}, + nullptr /* module_context */, + nullptr /* inherited_mudules_lock */, + var_ovr_func)); + + scope& gs (ctx->global_scope.rw ()); + + ctx->current_mname = "configure"; + ctx->current_oname = string (); // default + gs.assign (ctx->var_build_meta_operation) = "configure"; + + return ctx; + } + void pkg_configure (const common_options& o, database& db, transaction& t, const shared_ptr& p, configure_prerequisites_result&& cpr, - bool disfigured, +#ifndef BPKG_OUTPROC_CONFIGURE + const unique_ptr& pctx, + const build2::variable_overrides& ovrs, +#else + const unique_ptr&, + const build2::variable_overrides&, // Still in cpr.config_variables. +#endif bool simulate) { tracer trace ("pkg_configure"); @@ -394,31 +467,14 @@ namespace bpkg // if (!simulate) { - // Unless this package has been completely disfigured, disfigure all the - // package configuration variables to reset all the old values to - // defaults (all the new user/dependent/reflec values, including old - // user, are returned by collect_config() and specified as overrides). - // Note that this semantics must be consistent with how we load things - // in the package skeleton during configuration negotiation. - // - // Note also that this means we don't really use the dependent and - // reflect sources that we save in the database. But let's keep them - // for the completeness of information (maybe could be useful during - // configuration reset or some such). - // - string dvar; - if (!disfigured) - { - // Note: must be quoted to preserve the pattern. - // - dvar = "config.config.disfigure='config."; - dvar += p->name.variable (); - dvar += "**'"; - } - // Original implementation that runs the standard build system driver. // -#if 1 + // Note that the semantics doesn't match 100%. In particular, in the + // in-process implementation we enter unqualified variable in each + // project instead of the amalgamation (which is probably more accurate, + // since we don't re-configure the amalgamation or some dependencies). + // +#ifdef BPKG_OUTPROC_CONFIGURE // Form the buildspec. // string bspec; @@ -436,11 +492,7 @@ namespace bpkg try { - run_b (o, - verb_b::quiet, - cpr.config_variables, - (!dvar.empty () ? dvar.c_str () : nullptr), - bspec); + run_b (o, verb_b::quiet, cpr.config_variables, bspec); } catch (const failed&) { @@ -462,55 +514,9 @@ namespace bpkg using build2::endf; using build2::location; - if (pctx == nullptr) - { - // Initialize the build system. - // - // Note that this takes into account --build-option and default - // options files (which may have global overrides and which end - // up in build2_cmd_vars). - // - // @@ Uses verb_b::normal, we need verb_b::quiet. Temporarily - // adjust build2::verb then restore? - // - if (!build2_sched.started ()) - build2_init (o); - - // Re-tune the scheduler for parallel execution (see build2_init() - // for details). - // - if (!build2_sched.serial ()) - build2_sched.tune (0); - - // Shouldn't we shared the module context with package skeleton - // contexts? Maybe we don't have to since we don't build modules in - // them concurrently (in a sence, we didn't share it when we were - // invoking the build system driver). - // - pctx.reset (new context (build2_sched, - build2_mutexes, - build2_fcache, - false /* match_only */, - false /* no_external_modules */, - false /* dry_run */, - false /* no_diag_buffer */, - false /* keep_going */, - build2_cmd_vars, - context::reserves { - 30000 /* targets */, - 1100 /* variables */})); - - assert (pctx->var_overrides.empty ()); // Global only. - - context& ctx (*pctx); - scope& gs (ctx.global_scope.rw ()); - - const string mname ("configure"); - - ctx.current_mname = mname; - ctx.current_oname = string (); // default - gs.assign (ctx.var_build_meta_operation) = mname; - } + // @@ Uses verb_b::normal, we need verb_b::quiet. Temporarily + // adjust build2::verb then restore? + // context& ctx (*pctx); @@ -582,17 +588,9 @@ namespace bpkg if (!bf) fail << "no buildfile in " << src_root; - // @@ TODO: var overrides. + // Enter project-wide overrides. // - { - // @@ We have the careful order of when we do stuff -- this is - // all messed up now, right? Maybe we need to pre-enter them - // ahead of time all at once (also that index shit). - - variable_overrides ovrs; - - ctx.enter_project_overrides (rs, out_root, ovrs); - } + ctx.enter_project_overrides (rs, out_root, ovrs); // The goal here is to be more or less semantically equivalent to // configuring several projects at once. Except that here we have @@ -692,7 +690,40 @@ namespace bpkg fdb, nullptr)); - pkg_configure (o, db, t, p, move (cpr), disfigured, simulate); + if (!simulate) + { + // Unless this package has been completely disfigured, disfigure all the + // package configuration variables to reset all the old values to + // defaults (all the new user/dependent/reflec values, including old + // user, are returned by collect_config() and specified as overrides). + // Note that this semantics must be consistent with how we load things + // in the package skeleton during configuration negotiation. + // + // Note also that this means we don't really use the dependent and + // reflect sources that we save in the database. But let's keep them + // for the completeness of information (maybe could be useful during + // configuration reset or some such). + // + if (!disfigured) + { + // Note: must be quoted to preserve the pattern. + // + cpr.config_variables.push_back ( + "config.config.disfigure='config." + p->name.variable () + "**'"); + } + } + + unique_ptr ctx; + build2::variable_overrides ovrs; + +#ifndef BPKG_OUTPROC_CONFIGURE + + // @@ TODO: create context unless simulating, populate ovrs from + // cpr via callback. + // +#endif + + pkg_configure (o, db, t, p, move (cpr), ctx, ovrs, simulate); } shared_ptr diff --git a/bpkg/pkg-configure.hxx b/bpkg/pkg-configure.hxx index 63506a9..255371c 100644 --- a/bpkg/pkg-configure.hxx +++ b/bpkg/pkg-configure.hxx @@ -8,6 +8,7 @@ #include #include +#include // variable_overrides #include #include // transaction, selected_package @@ -105,8 +106,11 @@ namespace bpkg // Configure the package, update its state, and commit the transaction. // - // The build2::context argument allows re-using the build state in - // subsequent pkg_configure() calls. @@ TODO + // This is a lower-level version meant for sharing the same build context + // to configure multiple packages (in the dependency order). + // + // Note: variable_overrides must include config.config.disfigure, if + // required. // void pkg_configure (const common_options&, @@ -114,7 +118,8 @@ namespace bpkg transaction&, const shared_ptr&, configure_prerequisites_result&&, - bool disfigured, + const unique_ptr&, + const build2::variable_overrides&, bool simulate); // Note: loads selected packages. -- cgit v1.1