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 +- 6 files changed, 151 insertions(+), 64 deletions(-) (limited to 'bpkg') 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, -- cgit v1.1