Print configuration information in prompt

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<const available_package> ap,
                     strings cvs,
                     bool df,
                     const vector<config_variable>* css,
                     optional<dir_path> src_root,
                     optional<dir_path> 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.<name>** 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<const available_package>,
                       strings config_vars,
                       bool disfigure,
                       const vector<config_variable>* config_srcs,
@@ -65,7 +65,7 @@ namespace bpkg
 
 
     package_key key;
-    reference_wrapper<const available_package> available;
+    shared_ptr<const available_package> 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
     {
       "<sum>",
 
-      "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<available_package>& override = nullptr)
     {
-      assert (!skeleton && available != nullptr);
+      const shared_ptr<available_package>& ap (override != nullptr
+                                               ? override
+                                               : available);
+
+      assert (!skeleton && ap != nullptr);
 
       optional<dir_path> src_root (external_dir ());
 
@@ -1071,7 +1076,7 @@ namespace bpkg
       skeleton = package_skeleton (
         options,
         db,
-        *available,
+        ap,
         config_vars, // @@ Maybe make optional<strings> 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<dir_path> 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<dir_path> out_root (
-            src_root && !p.disfigure
-            ? dir_path (pdb.config) /= p.name ().string ()
-            : optional<dir_path> ());
+          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<available_package> 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<dir_path> src_root (p.external_dir ());
+        if (!p.skeleton)
+          p.init_skeleton (o, find_available (o, pdb, sp));
 
-        optional<dir_path> out_root (
-          src_root && !p.disfigure
-          ? dir_path (pdb.config) /= p.name ().string ()
-          : optional<dir_path> ());
+        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
     :