From 620186a31a28aaa15ab14c774fd56b14cc8d392d Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 13 Jun 2022 15:15:11 +0300 Subject: Print configuration information in prompt --- bpkg/package-configuration.cxx | 2 +- bpkg/package-skeleton.cxx | 12 +-- bpkg/package-skeleton.hxx | 8 +- bpkg/pkg-build.cli | 11 +-- bpkg/pkg-build.cxx | 180 ++++++++++++++++++++++++++++++----------- bpkg/pkg-configure.cxx | 2 +- tests/pkg-build.testscript | 72 ++++++++++++++++- 7 files changed, 221 insertions(+), 66 deletions(-) diff --git a/bpkg/package-configuration.cxx b/bpkg/package-configuration.cxx index 9375a7c..b6c153f 100644 --- a/bpkg/package-configuration.cxx +++ b/bpkg/package-configuration.cxx @@ -121,7 +121,7 @@ namespace bpkg pos.first--; pos.second--; // Convert to 0-base. const dependency_alternative& da ( - dept.available.get ().dependencies[pos.first][pos.second]); + dept.available->dependencies[pos.first][pos.second]); assert (da.require || da.prefer); diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index 0b5c05d..49d375a 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -169,14 +169,14 @@ namespace bpkg package_skeleton:: package_skeleton (const common_options& co, database& db, - const available_package& ap, + shared_ptr ap, strings cvs, bool df, const vector* css, optional src_root, optional out_root) - : key (db, ap.id.name), - available (ap), + : key (db, ap->id.name), + available (move (ap)), co_ (&co), db_ (&db), var_prefix_ ("config." + key.name.variable ()), @@ -186,7 +186,7 @@ namespace bpkg { // Should not be created for stubs. // - assert (ap.bootstrap_build); + assert (available != nullptr && available->bootstrap_build); // We are only interested in old user configuration variables. // @@ -365,7 +365,7 @@ namespace bpkg // cannot just iterate over all the config.** values set on the // root scope. Our options seem to be either iterating over the variable // pool or forcing the config module with config.config.module=true and - // then using its saved variables map. Since the amout of stuff we load + // then using its saved variables map. Since the amount of stuff we load // is quite limited, there shouldn't be too many variables in the pool. // So let's go with the simpler approach for now. // @@ -1995,7 +1995,7 @@ namespace bpkg // if (!skl.created_) { - const available_package& ap (skl.available); + const available_package& ap (*skl.available); // Note that we create the skeleton directories in the skeletons/ // subdirectory of the configuration temporary directory to make sure diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index bd9f8f0..031c88d 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -48,15 +48,15 @@ namespace bpkg // (because sometimes we may need to omit it) so most likely it will be // passed as a separate arguments (likely a file path). // - // Note that the options, database, available_package, and config_srcs are - // expected to outlive this object. + // Note that the options, database, and config_srcs are expected to + // outlive this object. // // Note also that this creates an "unloaded" skeleton and is therefore // relatively cheap. // package_skeleton (const common_options& co, database&, - const available_package&, + shared_ptr, strings config_vars, bool disfigure, const vector* config_srcs, @@ -65,7 +65,7 @@ namespace bpkg package_key key; - reference_wrapper available; + shared_ptr available; const package_name& name () const {return key.name;} // @@ TMP: get rid (use key.name). diff --git a/bpkg/pkg-build.cli b/bpkg/pkg-build.cli index b1415fe..493ffbe 100644 --- a/bpkg/pkg-build.cli +++ b/bpkg/pkg-build.cli @@ -379,11 +379,12 @@ namespace bpkg { "", - "Hash the names and versions of all the packages that would be built. - If the resulting checksum matches the specified, then exit without - building anything (potentially with a special error code specified - with the \cb{--noop-exit} option). Otherwise, proceed to build as - normal. In both cases, print the resulting checksum to \cb{stdout}." + "Hash the names, versions, and configurations of all the packages that + would be built. If the resulting checksum matches the specified, then + exit without building anything (potentially with a special error code + specified with the \cb{--noop-exit} option). Otherwise, proceed to + build as normal. In both cases, print the resulting checksum to + \cb{stdout}." } uint16_t --no-private-config diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 7a3c2f4..9e3ad26 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -1057,9 +1057,14 @@ namespace bpkg // Initialize the skeleton of a being built package. // package_skeleton& - init_skeleton (const common_options& options) + init_skeleton (const common_options& options, + const shared_ptr& override = nullptr) { - assert (!skeleton && available != nullptr); + const shared_ptr& ap (override != nullptr + ? override + : available); + + assert (!skeleton && ap != nullptr); optional src_root (external_dir ()); @@ -1071,7 +1076,7 @@ namespace bpkg skeleton = package_skeleton ( options, db, - *available, + ap, config_vars, // @@ Maybe make optional and move? disfigure, (selected != nullptr ? &selected->config_variables : nullptr), @@ -4482,8 +4487,8 @@ namespace bpkg // // But if we merged clusters not yet negotiated, or, worse, // being in the middle of negotiation, then we need to get this - // merged cluster into the fully negotiated state. The we do it - // is by throwing merge_configuration (see below). + // merged cluster into the fully negotiated state. The way we do + // it is by throwing merge_configuration (see below). // // When we are back here after throwing merge_configuration, // then all the clusters have been pre-merged and our call to @@ -12033,6 +12038,11 @@ namespace bpkg // While at it, detect if we have any dependents that the user may want to // update. // + // For the packages being printed also print the configuration specified + // by the user, dependents, and via the reflect clauses. For that we will + // use the package skeletons, initializing them if required. Note that the + // freshly-initialized skeletons will be reused during the plan execution. + // bool update_dependents (false); // We need the plan and to ask for the user's confirmation only if some @@ -12050,6 +12060,11 @@ namespace bpkg o.plan_specified () || o.rebuild_checksum_specified ()) { + // Start the transaction since we may query available packages for + // skeleton initializations. + // + transaction t (mdb); + bool first (true); // First entry in the plan. for (build_package& p: reverse_iterate (pkgs)) @@ -12068,12 +12083,12 @@ namespace bpkg } else { - package_skeleton* cfg (nullptr); // Print configuration variables. - - // @@ TODO set cfg + // Print configuration variables. // - // @@ Maybe use pointer to skeleton as indication since it will have - // to be init'ed differently in different situations. + // The idea here is to only print configuration for those packages + // for which we call pkg_configure*() in execute_plan(). + // + package_skeleton* cfg (nullptr); string cause; if (*p.action == build_package::adjust) @@ -12110,14 +12125,64 @@ namespace bpkg const string& s (pdb.string); if (!s.empty ()) act += ' ' + s; + + // This is an adjustment and so there is no available package + // specified for the build package object and thus the skeleton + // cannot be present. + // + assert (p.available == nullptr && !p.skeleton); + + // We shouldn't be printing configurations for plain unholds. + // + if (p.reconfigure ()) + { + // Since there is no available package specified we need to find + // it (or create a transient one). + // + // @@ TMP Don't create skeletons for system packages for now. + // + // Notes on system packages: + // + // - The available package while specified for the + // package build object may refer to some unrelated + // version (normally the latest available in the + // user-added repositories) to the user-specified + // version. Actually the user may specify a version + // that is not present in any repository. + // + // - The user can specify wildcard version and so there + // is no version which we could take the manifest from + // for the skeleton creation. + // + // - The available package can be a stub and we don't + // create skeletons for stubs since they don't have + // bootstrap-build in manifests. + // + if (!p.system) + cfg = &p.init_skeleton (o, find_available (o, pdb, sp)); + } } else { + assert (p.available != nullptr); // This is a package build. + // Even if we already have this package selected, we have to // make sure it is configured and updated. // if (sp == nullptr) + { act = p.system ? "configure" : "new"; + + // For a new non-system package the skeleton must already be + // initialized. + // + // @@ TMP Don't create skeletons for system packages for now. + // + assert (p.system || p.skeleton.has_value ()); + + if (!p.system) + cfg = &*p.skeleton; + } else if (sp->version == p.available_version ()) { // If this package is already configured and is not part of the @@ -12139,6 +12204,16 @@ namespace bpkg ? "reconfigure" : "reconfigure/update") : "update"); + + if (p.reconfigure ()) + { + // Initialize the skeleton if it is not initialized yet. + // + // @@ TMP Don't create skeletons for system packages for now. + // + if (!p.system) + cfg = &(p.skeleton ? *p.skeleton : p.init_skeleton (o)); + } } else { @@ -12148,6 +12223,16 @@ namespace bpkg ? "upgrade" : "downgrade"; + // For a non-system package up/downgrade the skeleton must + // already be initialized. + // + // @@ TMP Don't create skeletons for system packages for now. + // + assert (p.system || p.skeleton.has_value ()); + + if (!p.system) + cfg = &*p.skeleton; + need_prompt = true; } @@ -12189,7 +12274,7 @@ namespace bpkg ostringstream os; cfg->print_config (os, o.print_only () ? " " : " "); act += '\n'; - act += os.string (); + act += os.str (); } } @@ -12218,6 +12303,8 @@ namespace bpkg if (o.rebuild_checksum_specified ()) csum.append (act); } + + t.commit (); } if (o.rebuild_checksum_specified ()) @@ -12820,6 +12907,9 @@ namespace bpkg // source code one, or just available, in which case it is a system // one. Note that a system package gets selected as being configured. // + // NOTE: remember to update the preparation of the plan to be presented + // to the user if changing anything here. + // assert (sp != nullptr || p.system); database& pdb (p.db); @@ -12854,6 +12944,8 @@ namespace bpkg } else if (ap != nullptr) { + 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. @@ -12890,16 +12982,19 @@ namespace bpkg } else { - assert (sp != nullptr && !p.skeleton); // See above. - - // @@ TODO: factor to init_skeleton. + assert (sp != nullptr); // See above. - optional src_root (p.external_dir ()); + // 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 to be presented to + // the user. This only happens after all the refinements and so this + // plan execution is not simulated. + // + assert (!p.skeleton || !simulate); - optional out_root ( - src_root && !p.disfigure - ? dir_path (pdb.config) /= p.name ().string () - : optional ()); + if (!p.skeleton) + p.init_skeleton (o); pkg_configure (o, pdb, @@ -12907,14 +13002,7 @@ namespace bpkg sp, ap->dependencies, nullptr /* alternatives */, - package_skeleton (o, - pdb, - *ap, - move (p.config_vars), - p.disfigure, - &sp->config_variables, - move (src_root), - move (out_root)), + move (*p.skeleton), prereqs (), simulate, fdb); @@ -12922,24 +13010,29 @@ namespace bpkg } else // Dependent. { + assert (*p.action == build_package::adjust); + // Must be in the unpacked state since it was disfigured on the first // pass (see above). // assert (sp->state == package_state::unpacked); - // Note that here we don't care about the repository fragment the - // package comes from and only need its manifest information. + // Initialize the skeleton if it is not initialized yet. // - shared_ptr dap (find_available (o, pdb, sp)); - - // @@ TODO: factor to init_skeleton. + // 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)); - optional src_root (p.external_dir ()); + if (!p.skeleton) + p.init_skeleton (o, find_available (o, pdb, sp)); - optional out_root ( - src_root && !p.disfigure - ? dir_path (pdb.config) /= p.name ().string () - : optional ()); + 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 @@ -12947,22 +13040,15 @@ namespace bpkg // information in the database? // // I believe this now works for external packages via package - // skeleton (wich extracts user configruation). + // skeleton (which extracts user configuration). // pkg_configure (o, pdb, t, sp, - dap->dependencies, + deps, nullptr /* alternatives */, - package_skeleton (o, - pdb, - *dap, - move (p.config_vars), - p.disfigure, - &sp->config_variables, - move (src_root), - move (out_root)), + move (*p.skeleton), prereqs (), simulate, fdb); diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 34fe269..32cace3 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -650,7 +650,7 @@ namespace bpkg nullptr /* alternatives */, package_skeleton (o, db, - *ap, + ap, move (vars), false /* disfigure */, &p->config_variables, diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 81179a9..8d3945f 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -3500,7 +3500,7 @@ test.options += --no-progress { +$clone_cfg - test.arguments += --yes + test.arguments += --yes --plan 'build plan:' : ambiguity : @@ -3527,6 +3527,10 @@ test.options += --no-progress $clone_cfg; $* fox ?libbaz 2>>~%EOE%; + build plan: + new libbaz/1.1.0 + new fox/1.0.0 + config.fox.backend=libbaz (set by fox) fetched libbaz/1.1.0 unpacked libbaz/1.1.0 fetched fox/1.0.0 @@ -3548,6 +3552,10 @@ test.options += --no-progress $clone_cfg; $* fox libbaz 2>>~%EOE%; + build plan: + new libbaz/1.1.0 + new fox/1.0.0 + config.fox.backend=libbaz (set by fox) fetched libbaz/1.1.0 unpacked libbaz/1.1.0 fetched fox/1.0.0 @@ -3572,6 +3580,9 @@ test.options += --no-progress $* libbaz 2>!; $* fox 2>>~%EOE%; + build plan: + new fox/1.0.0 + config.fox.backend=libbaz (set by fox) fetched fox/1.0.0 unpacked fox/1.0.0 configured fox/1.0.0 @@ -3591,6 +3602,10 @@ test.options += --no-progress $pkg_fetch libbaz/1.0.0; $* fox 2>>~%EOE%; + build plan: + update libbaz/1.0.0 (required by fox) + new fox/1.0.0 + config.fox.backend=libbaz (set by fox) unpacked libbaz/1.0.0 fetched fox/1.0.0 unpacked fox/1.0.0 @@ -3615,6 +3630,12 @@ test.options += --no-progress $clone_cfg; $* fox foo 2>>~%EOE%; + build plan: + new libbaz/1.1.0 (required by foo) + new libbar/1.0.0 (required by foo, fox) + new fox/1.0.0 + config.fox.backend=libbar (set by fox) + new foo/1.0.0 fetched libbaz/1.1.0 unpacked libbaz/1.1.0 fetched libbar/1.0.0 @@ -3643,6 +3664,13 @@ test.options += --no-progress $clone_cfg; $* baz fox bar 2>>~%EOE%; + build plan: + new libbaz/1.1.0 (required by baz) + new baz/1.0.0 + new libbar/1.0.0 (required by bar, fox) + new fox/1.0.0 + config.fox.backend=libbar (set by fox) + new bar/1.0.0 fetched libbaz/1.1.0 unpacked libbaz/1.1.0 fetched baz/1.0.0 @@ -3684,6 +3712,10 @@ test.options += --no-progress $clone_cfg; $* fox ?libbaz/1.0.0 2>>~%EOE%; + build plan: + new libbaz/1.0.0 + new fox/1.0.0 + config.fox.backend=libbaz (set by fox) fetched libbaz/1.0.0 unpacked libbaz/1.0.0 fetched fox/1.0.0 @@ -3697,6 +3729,9 @@ test.options += --no-progress EOE $* ?libbaz 2>>~%EOE%; + build plan: + upgrade libbaz/1.1.0 + reconfigure fox (dependent of libbaz) disfigured fox/1.0.0 disfigured libbaz/1.0.0 fetched libbaz/1.1.0 @@ -3739,6 +3774,10 @@ test.options += --no-progress $* box libbox 2>!; $* box +{ config.box.extras=true } ?libbiz 2>>~%EOE%; + build plan: + reconfigure/update box/1.0.0 + config.box.extras=true (user configuration) + config.box.backend=libbox (set by box) disfigured box/1.0.0 configured box/1.0.0 %info: .+box-1.0.0.+ is up to date% @@ -3746,6 +3785,11 @@ test.options += --no-progress EOE $* box +{ config.box.extras=false } libbiz 2>>~%EOE%; + build plan: + reconfigure/update box/1.0.0 + config.box.extras=false (user configuration) + config.box.backend=libbox (set by box) + new libbiz/1.0.0 disfigured box/1.0.0 fetched libbiz/1.0.0 unpacked libbiz/1.0.0 @@ -3785,6 +3829,12 @@ test.options += --no-progress $* box libbox 2>!; $* box +{ config.box.extras=true } ?libbox/0.1.0 2>>~%EOE%; + build plan: + new libbiz/1.0.0 (required by box) + drop libbox/1.0.0 (unused) + reconfigure/update box/1.0.0 + config.box.extras=true (user configuration) + config.box.backend=libbiz (set by box) disfigured box/1.0.0 disfigured libbox/1.0.0 fetched libbiz/1.0.0 @@ -3820,6 +3870,12 @@ test.options += --no-progress $* box libbox 2>!; $* box +{ config.box.extras=true } libbox/0.1.0 2>>~%EOE%; + build plan: + new libbiz/1.0.0 (required by box) + downgrade libbox/0.1.0 + reconfigure/update box/1.0.0 + config.box.extras=true (user configuration) + config.box.backend=libbiz (set by box) disfigured box/1.0.0 disfigured libbox/1.0.0 fetched libbiz/1.0.0 @@ -3857,6 +3913,11 @@ test.options += --no-progress $clone_cfg; $* box ?libbiz/0.1.0 2>>~%EOE%; + build plan: + new libbox/1.0.0 (required by box) + new libbaz/1.1.0 (required by box) + new box/1.0.0 + config.box.backend=libbox (set by box) fetched libbox/1.0.0 unpacked libbox/1.0.0 fetched libbaz/1.1.0 @@ -3885,6 +3946,10 @@ test.options += --no-progress # libbox as a prerequisite of box. # $* libbiz ?libbaz/1.0.0 2>>~%EOE%; + build plan: + new libbiz/1.0.0 + downgrade libbaz/1.0.0 + reconfigure box (dependent of libbaz) disfigured box/1.0.0 disfigured libbaz/1.1.0 fetched libbiz/1.0.0 @@ -3917,6 +3982,9 @@ test.options += --no-progress # Make sure the decision is hold for downgraded dependency either. # $* ?libbox/0.1.1 2>>~%EOE%; + build plan: + downgrade libbox/0.1.1 + reconfigure box (dependent of libbox) disfigured box/1.0.0 disfigured libbox/1.0.0 fetched libbox/0.1.1 @@ -4740,7 +4808,7 @@ test.options += --no-progress { +$clone_root_cfg && $rep_add $rep/t11a && $rep_fetch - test.arguments += --yes --verbose 5 --build-option --quiet + test.arguments += --yes --plan 'build plan:' --verbose 5 --build-option --quiet : initial-collection : -- cgit v1.1