Drag configuration sources into package skeleton

3 files changed, 166 insertions, 65 deletions

diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index e0d85ed..385a4fd 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -59,6 +59,7 @@ namespace bpkg
       db_ = v.db_;
       available_ = v.available_;
       config_vars_ = move (v.config_vars_);
+      config_srcs_ = v.config_srcs_;
       src_root_ = move (v.src_root_);
       out_root_ = move (v.out_root_);
       created_ = v.created_;
@@ -82,6 +83,7 @@ namespace bpkg
         db_ (v.db_),
         available_ (v.available_),
         config_vars_ (v.config_vars_),
+        config_srcs_ (v.config_srcs_),
         src_root_ (v.src_root_),
         out_root_ (v.out_root_),
         created_ (v.created_),
@@ -99,14 +101,22 @@ namespace bpkg
                     database& db,
                     const available_package& ap,
                     strings cvs,
+                    /*const vector<config_variable>* css,*/ // @@ TMP
                     optional<dir_path> src_root,
                     optional<dir_path> out_root)
-      : co_ (&co), db_ (&db), available_ (&ap), config_vars_ (move (cvs))
+      : co_ (&co),
+        db_ (&db),
+        available_ (&ap),
+        config_vars_ (move (cvs)),
+        config_srcs_ (/*css*/ nullptr) // @@ TMP
   {
     // Should not be created for stubs.
     //
     assert (available_->bootstrap_build);
 
+    if (config_srcs_ != nullptr && config_srcs_->empty ())
+      config_srcs_ = nullptr;
+
     if (src_root)
     {
       src_root_ = move (*src_root);
@@ -561,61 +571,140 @@ namespace bpkg
     }
   }
 
-  pair<strings, vector<optional<config_source>>> package_skeleton::
+  pair<strings, vector<config_variable>> package_skeleton::
   collect_config () &&
   {
-    // Note that if the reflect variables list is not empty, then it also
-    // contains the user-specified configuration variables, which all come
-    // first (see above).
+    assert (db_ != nullptr); // Must be called only once.
+
+    strings vars;
+    vector<config_variable> srcs;
+
+    // Check whether the user-specified configuration variable is a project
+    // variables (i.e., its name start with config.<project>).
     //
-    size_t nc (config_vars_.size ());
+    // Note that some user-specified variables may have qualifications
+    // (global, scope, etc) but there is no reason to expect any project
+    // configuration variables to use such qualifications (since they can only
+    // apply to one project). So we ignore all qualified variables.
+    //
+    auto prj_var = [this, p = optional<string> ()] (const string& v) mutable
+    {
+      if (!p)
+        p = "config." + name ().variable ();
 
-    strings vars (reflect_vars_.empty ()
-                  ? move (config_vars_)
-                  : move (reflect_vars_));
+      size_t n (p->size ());
 
-    vector<optional<config_source>> sources;
+      return v.compare (0, n, *p) == 0 && strchr (".=+ \t", v[n]) != nullptr;
+    };
 
-    if (!vars.empty ())
+    if (!reflect_vars_.empty ())
     {
-      sources.reserve (vars.size ());
+      assert (config_srcs_ == nullptr); // Should have been merged.
 
-      // Assign the user source to only those user-specified configuration
-      // variables which are project variables (i.e., names start with
-      // config.<project>). Assign the reflect source to all variables that
-      // follow the user-specified configuration variables (which can only be
-      // project variables).
-      //
-      // Note that some user-specified variables may have qualifications
-      // (global, scope, etc) but there is no reason to expect any project
-      // configuration variables to use such qualifications (since they can
-      // only apply to one project). So we skip all qualified variables.
+      vars = move (reflect_vars_);
+
+      // Note that if the reflect variables list is not empty, then it also
+      // contains the user-specified configuration variables, which all come
+      // first (see above).
       //
-      string p ("config." + name ().variable ());
-      size_t n (p.size ());
+      size_t nc (config_vars_.size ());
 
-      for (const string& v: vars)
+      if (!vars.empty ())
       {
-        if (sources.size () < nc)
+        srcs.reserve (vars.size ()); // At most that many.
+
+        // Assign the user source only to user-specified configuration
+        // variables which are project variables (i.e., names start with
+        // config.<project>). Assign the reflect source to all variables that
+        // follow the user-specified configuration variables (which can only
+        // be project variables).
+        //
+        for (size_t j (0); j != vars.size (); ++j)
         {
-          if (v.compare (0, n, p) == 0 && strchr (".=+ \t", v[n]) != nullptr)
-            sources.push_back (config_source::user);
+          const string& v (vars[j]);
+
+          config_source s;
+
+          if (j < nc)
+          {
+            if (prj_var (v))
+              s = config_source::user;
+            else
+              continue;
+          }
+          else
+            s = config_source::reflect;
+
+          size_t p (v.find_first_of ("=+ \t"));
+          assert (p != string::npos);
+
+          string n (v, 0, p);
+
+          // Check for a duplicate.
+          //
+          auto i (find_if (srcs.begin (), srcs.end (),
+                           [&n] (const config_variable& cv)
+                           {
+                             return cv.name == n;
+                           }));
+
+          if (i != srcs.end ())
+            assert (i->source == s); // See evaluate_reflect() for details.
           else
-            sources.push_back (nullopt);
+            srcs.push_back (config_variable {move (n), s});
         }
-        else
-          sources.push_back (config_source::reflect);
       }
     }
+    else
+    {
+      vars = move (config_vars_);
 
-    assert (db_ != nullptr);
+      // If we don't have any reflect variables, then we don't really need to
+      // load user variables from config.build (or equivalent) and add them to
+      // config_vars since there is nothing for them to possibly override. So
+      // all we need to do is to add user variables from config_vars.
+      //
+      if (!vars.empty () || config_srcs_ != nullptr)
+      {
+        srcs.reserve ((config_srcs_ != nullptr ? config_srcs_->size () : 0)
+                      + vars.size ()); // At most that many.
 
-    ctx_ = nullptr; // In case we only had conditions.
+        if (config_srcs_ != nullptr)
+          for (const config_variable& v: *config_srcs_)
+            if (v.source == config_source::user)
+              srcs.push_back (v);
+
+        for (const string& v: vars)
+        {
+          // Similar logic to the above case.
+          //
+          if (prj_var (v))
+          {
+            size_t p (v.find_first_of ("=+ \t"));
+            assert (p != string::npos);
+
+            string n (v, 0, p);
 
+            // Check for a duplicate.
+            //
+            auto i (find_if (srcs.begin (), srcs.end (),
+                             [&n] (const config_variable& cv)
+                             {
+                               return cv.name == n;
+                             }));
+
+            if (i == srcs.end ())
+              srcs.push_back (config_variable {move (n), config_source::user});
+          }
+        }
+      }
+    }
+
+    ctx_ = nullptr; // In case we only had conditions.
     db_ = nullptr;
     available_ = nullptr;
 
-    return make_pair (move (vars), move (sources));
+    return make_pair (move (vars), move (srcs));
   }
 
   build2::scope& package_skeleton::
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index aaa9b8c..3e0da50 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -29,8 +29,23 @@ namespace bpkg
     // If the package is not external, then none of the root directories
     // should be specified.
     //
-    // Note that the options, database, and available_package are expected to
-    // outlive this object.
+    // Note that the options, database, available_package, and config_srcs are
+    // expected to outlive this object.
+    //
+    // The config_vars argument contains configuration variables specified by
+    // the user in this bpkg execution. Optional config_srcs is used to
+    // extract (from config.build or equivalent) configuration variables
+    // specified by the user in previous bpkg executions. It should be NULL if
+    // this is the first build of the package or if it is being disfigured.
+    // The extracted variables are merged with config_vars and the combined
+    // result is returned by collect_config() below.
+    //
+    // @@ TODO: speaking of the "config.build or equivalent" part, the
+    //    equivalent is likely to be extracted configuration (probably saved
+    //    to file in tmp somewhere) that we will load with config.config.load.
+    //    It doesn't seem like a good idea to pass it as part of config_vars
+    //    (because sometimes we may need to omit it) so most likely it will be
+    //    passed as a separate arguments (likely a file path).
     //
     // Note also that this creates an "unloaded" skeleton and is therefore
     // relatively cheap.
@@ -39,6 +54,7 @@ namespace bpkg
                       database&,
                       const available_package&,
                       strings config_vars,
+                      /*const vector<config_variable>* config_srcs,*/ // @@ TMP
                       optional<dir_path> src_root,
                       optional<dir_path> out_root);
 
@@ -56,16 +72,16 @@ namespace bpkg
     void
     evaluate_reflect (const string&, size_t depends_index);
 
-    // Return the accumulated reflect values together with their sources (the
-    // resulting vectors are parallel).
+    // Return the accumulated configuration variables (first) and project
+    // configuration variable sources (second). Note that the arrays are
+    // not necessarily parallel.
     //
-    // Note that the result is merged with config_vars and should be used
-    // instead rather than in addition to config_vars. The source of
-    // configuration variables which are not the project variables is absent.
+    // Note that the reflect variables are merged with config_vars/config_srcs
+    // and should be used instead rather than in addition to config_vars.
     //
     // Note also that this should be the final call on this object.
     //
-    pair<strings, vector<optional<config_source>>>
+    pair<strings, vector<config_variable>>
     collect_config () &&;
 
     const package_name&
@@ -97,7 +113,10 @@ namespace bpkg
     const common_options* co_;
     database* db_;
     const available_package* available_;
+
     strings config_vars_;
+    const vector<config_variable>* config_srcs_; // NULL if nothing to do or
+                                                 // already done.
 
     dir_path src_root_; // Must be absolute and normalized.
     dir_path out_root_; // If empty, the same as src_root_.
diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx
index ee123d5..49570c2 100644
--- a/bpkg/pkg-configure.cxx
+++ b/bpkg/pkg-configure.cxx
@@ -36,7 +36,7 @@ namespace bpkg
     // package skeleton, excluding those user-specified variables which are
     // not the project variables for the specified package (module
     // configuration variables, etc). Thus, it is not parallel to the
-    // variables member.
+    // config_variables member.
     //
     vector<config_variable> config_sources; // Note: name and source.
   };
@@ -54,9 +54,8 @@ namespace bpkg
                                bool simulate,
                                const function<find_database_function>& fdb)
   {
-    package_prerequisites   prereqs;
-    strings                 vars;
-    vector<config_variable> srcs;
+    package_prerequisites prereqs;
+    strings               vars;
 
     // Alternatives argument must be parallel to the dependencies argument if
     // specified.
@@ -286,34 +285,28 @@ namespace bpkg
       }
     }
 
-    // Add the configuration variables collected from the reflect clauses, if
-    // any.
+    // Add the rest of the configuration variables (user overrides, reflects,
+    // etc) as well as their sources.
     //
+    vector<config_variable> srcs;
+
     if (!simulate)
     {
-      auto rvs (move (ps).collect_config ());
+      pair<strings, vector<config_variable>> rvs (move (ps).collect_config ());
 
-      strings&                         vs (rvs.first);
-      vector<optional<config_source>>& ss (rvs.second);
+      strings& vs (rvs.first);
+      srcs = move (rvs.second);
 
       if (!vs.empty ())
       {
-        vars.reserve (vars.size () + vs.size ());
-
-        for (size_t i (0); i != vs.size (); ++i)
+        if (vars.empty ())
+          vars = move (vs);
+        else
         {
-          string&                        v (vs[i]);
-          const optional<config_source>& s (ss[i]);
-
-          if (s)
-          {
-            size_t p (v.find_first_of ("=+ \t"));
-            assert (p != string::npos);
-
-            srcs.push_back (config_variable {string (v, 0, p), *s});
-          }
+          vars.reserve (vars.size () + vs.size ());
 
-          vars.push_back (move (v));
+          for (string& v: vs)
+            vars.push_back (move (v));
         }
       }
     }