API rework

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 (<cfg-var>), if any, should be
      specified before packages (<pkg-spec>) 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<build_package> 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_package> 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<build2::context> 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<build2::context> 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<build2::context>
+  pkg_configure_context (
+    const common_options& o,
+    strings&& cmd_vars,
+    const function<build2::context::var_override_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<context> 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<selected_package>& p,
                  configure_prerequisites_result&& cpr,
-                 bool disfigured,
+#ifndef BPKG_OUTPROC_CONFIGURE
+                 const unique_ptr<build2::context>& pctx,
+                 const build2::variable_overrides& ovrs,
+#else
+                 const unique_ptr<build2::context>&,
+                 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<build2::context> 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<selected_package>
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 <libbpkg/package-name.hxx>
 
 #include <libbuild2/context.hxx>
+#include <libbuild2/variable.hxx> // variable_overrides
 
 #include <bpkg/types.hxx>
 #include <bpkg/forward.hxx> // 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<selected_package>&,
                  configure_prerequisites_result&&,
-                 bool disfigured,
+                 const unique_ptr<build2::context>&,
+                 const build2::variable_overrides&,
                  bool simulate);
 
   // Note: loads selected packages.