New configuration logic, iteration 1

6 files changed, 184 insertions, 103 deletions

diff --git a/build2/config/module b/build2/config/module
index 5f3c826..6012442 100644
--- a/build2/config/module
+++ b/build2/config/module
@@ -5,21 +5,35 @@
 #ifndef BUILD2_CONFIG_MODULE
 #define BUILD2_CONFIG_MODULE
 
+#include <butl/prefix-map>
+
 #include <build2/types>
 #include <build2/utility>
 
 #include <build2/module>
+#include <build2/variable>
 
 namespace build2
 {
   namespace config
   {
+    struct module: module_base
+    {
+      // A sorted list of config.* variables and flags (currently unused) that
+      // are used (as opposed to just specified) in this configuration.
+      // Populated by the config utility functions (required(), optional())
+      //
+      butl::prefix_map<variable_cref, uint64_t, '.'> vars;
+
+      static const string name;
+    };
+
     extern "C" void
-    config_boot (scope&, const location&, unique_ptr<module>&);
+    config_boot (scope&, const location&, unique_ptr<module_base>&);
 
     extern "C" bool
     config_init (
-    scope&, scope&, const location&, unique_ptr<module>&, bool, bool);
+      scope&, scope&, const location&, unique_ptr<module_base>&, bool, bool);
   }
 }
 
diff --git a/build2/config/module.cxx b/build2/config/module.cxx
index 6d2bd70..16eabe9 100644
--- a/build2/config/module.cxx
+++ b/build2/config/module.cxx
@@ -9,6 +9,7 @@
 #include <build2/file>
 #include <build2/rule>
 #include <build2/scope>
+#include <build2/context>
 #include <build2/diagnostics>
 
 #include <build2/config/operation>
@@ -20,12 +21,14 @@ namespace build2
 {
   namespace config
   {
+    const string module::name ("config");
+
     //@@ Same as in operation.cxx
     //
     static const path config_file ("build/config.build");
 
     extern "C" void
-    config_boot (scope& root, const location&, unique_ptr<module>&)
+    config_boot (scope& root, const location&, unique_ptr<module_base>&)
     {
       tracer trace ("config::boot");
 
@@ -54,7 +57,7 @@ namespace build2
     config_init (scope& root,
                  scope&,
                  const location& l,
-                 unique_ptr<module>&,
+                 unique_ptr<module_base>& mod,
                  bool first,
                  bool)
     {
@@ -68,6 +71,11 @@ namespace build2
 
       l5 ([&]{trace << "for " << root.out_path ();});
 
+      // Only create the module if we are configuring.
+      //
+      if (current_mif->id == configure_id)
+        mod.reset (new module);
+
       // Register alias and fallback rule for the configure meta-operation.
       //
       {
diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx
index e75f8d0..d1a01ea 100644
--- a/build2/config/operation.cxx
+++ b/build2/config/operation.cxx
@@ -15,6 +15,8 @@
 #include <build2/algorithm>
 #include <build2/diagnostics>
 
+#include <build2/config/module>
+
 using namespace std;
 using namespace butl;
 
@@ -51,8 +53,6 @@ namespace build2
 
         ofs.exceptions (ofstream::failbit | ofstream::badbit);
 
-        //@@ TODO: quote path
-        //
         ofs << "# Created automatically by the config module." << endl
             << "#" << endl
             << "src_root = " << src_root << endl;
@@ -72,6 +72,8 @@ namespace build2
       if (verb)
         text << (verb >= 2 ? "config::save " : "save ") << f;
 
+      const module& mod (*root.modules.lookup<const module> (module::name));
+
       try
       {
         ofstream ofs (f.string ());
@@ -80,8 +82,8 @@ namespace build2
 
         ofs.exceptions (ofstream::failbit | ofstream::badbit);
 
-        ofs << "# Created automatically by the config module, but" << endl
-            << "# feel free to edit." << endl
+        ofs << "# Created automatically by the config module, but feel " <<
+          "free to edit." << endl
             << "#" << endl;
 
         if (auto l = root.vars["amalgamation"])
@@ -92,28 +94,33 @@ namespace build2
               << "#" << endl;
         }
 
-        // Save all the variables in the config namespace that are set
-        // on the project's root scope.
+        // Save config variables.
         //
         names storage;
 
-        for (auto p (root.vars.find_namespace ("config"));
-             p.first != p.second;
-             ++p.first)
+        for (const auto& p: mod.vars)
         {
-          const variable& var (p.first->first);
-          const value& val (p.first->second);
-          const string& n (var.name);
+          const variable& var (p.first);
 
-          // Skip special variables.
+          lookup l (root[var]);
+
+          // We only write values that are set on our root scope or are global
+          // overrides. Anything in-between is inherited. We might also not
+          // have any value at all (see unconfigured()).
           //
-          if (n == "config.loaded" ||
-              n == "config.configured")
+          if (!l.defined () ||
+              !(l.belongs (root) || l.belongs (*global_scope)))
             continue;
 
-          // We will only write config.*.configured if it is false
-          // (true is implied by its absence).
+          const value& val (*l);
+
+          // We will only write config.*.configured if it is false (true is
+          // implied by its absence).
+          //
+          // @@ Do we still need this?
           //
+          const string& n (var.name);
+
           if (n.size () > 11 &&
               n.compare (n.size () - 11, 11, ".configured") == 0)
           {
@@ -121,24 +128,13 @@ namespace build2
               continue;
           }
 
-          // Warn the user if the value that we are saving differs
-          // from the one they specified on the command line.
-          //
-          auto l ((*global_scope)[var]);
-          if (l.defined () && *l != val)
-          {
-            warn << "variable " << var.name << " configured value "
-                 << "differs from command line value" <<
-              info << "reconfigure the project to use command line value";
-          }
-
           if (val)
           {
             storage.clear ();
             ofs << var.name << " = " << reverse (val, storage) << endl;
           }
           else
-            ofs << var.name << " = #[null]" << endl; // @@ TODO: [null]
+            ofs << var.name << " = [null]" << endl;
         }
       }
       catch (const ofstream::failure&)
@@ -148,13 +144,19 @@ namespace build2
     }
 
     static void
-    configure_project (action a, scope& root)
+    configure_project (action a, scope& root, set<scope*>& projects)
     {
       tracer trace ("configure_project");
 
       const dir_path& out_root (root.out_path ());
       const dir_path& src_root (root.src_path ());
 
+      if (!projects.insert (&root).second)
+      {
+        l5 ([&]{trace << "skipping already configured " << out_root;});
+        return;
+      }
+
       // Make sure the directories exist.
       //
       if (out_root != src_root)
@@ -200,7 +202,7 @@ namespace build2
           if (nroot.out_path () != out_nroot) // This subproject not loaded.
             continue;
 
-          configure_project (a, nroot);
+          configure_project (a, nroot, projects);
         }
       }
     }
@@ -219,6 +221,8 @@ namespace build2
       // callbacks here since the meta operation is configure and we
       // know what we are doing.
       //
+      set<scope*> projects;
+
       for (void* v: ts)
       {
         target& t (*static_cast<target*> (v));
@@ -243,11 +247,12 @@ namespace build2
           match (action (configure_id, id), t);
         }
 
-        configure_project (a, *rs);
+        configure_project (a, *rs, projects);
       }
     }
 
     meta_operation_info configure {
+      configure_id,
       "configure",
       "configure",
       "configuring",
@@ -299,15 +304,21 @@ namespace build2
     disfigure_match (action, action_targets&) {}
 
     static bool
-    disfigure_project (action a, scope& root)
+    disfigure_project (action a, scope& root, set<scope*>& projects)
     {
       tracer trace ("disfigure_project");
 
-      bool m (false); // Keep track of whether we actually did anything.
-
       const dir_path& out_root (root.out_path ());
       const dir_path& src_root (root.src_path ());
 
+      if (!projects.insert (&root).second)
+      {
+        l5 ([&]{trace << "skipping already disfigured " << out_root;});
+        return true;
+      }
+
+      bool m (false); // Keep track of whether we actually did anything.
+
       // Disfigure subprojects. Since we don't load buildfiles during
       // disfigure, we do it for all known subprojects.
       //
@@ -338,7 +349,7 @@ namespace build2
             bootstrap_src (nroot);
           }
 
-          m = disfigure_project (a, nroot) || m;
+          m = disfigure_project (a, nroot, projects) || m;
 
           // We use mkdir_p() to create the out_root of a subproject
           // which means there could be empty parent directories left
@@ -408,11 +419,13 @@ namespace build2
     {
       tracer trace ("disfigure_execute");
 
+      set<scope*> projects;
+
       for (void* v: ts)
       {
         scope& root (*static_cast<scope*> (v));
 
-        if (!disfigure_project (a, root))
+        if (!disfigure_project (a, root, projects))
         {
           // Create a dir{$out_root/} target to signify the project's
           // root in diagnostics. Not very clean but seems harmless.
@@ -428,6 +441,7 @@ namespace build2
     }
 
     meta_operation_info disfigure {
+      disfigure_id,
       "disfigure",
       "disfigure",
       "disfiguring",
diff --git a/build2/config/utility b/build2/config/utility
index 9218106..451df30 100644
--- a/build2/config/utility
+++ b/build2/config/utility
@@ -19,12 +19,14 @@ namespace build2
   {
     // Set, if necessary, a required config.* variable.
     //
-    // If override is true and the variable doesn't come from this root
-    // scope or from the command line, then its value is "overridden"
-    // for this root scope.
+    // If override is true and the variable doesn't come from this root scope
+    // or from the command line (i.e., it is inherited from the amalgamtion),
+    // then its value is "overridden" for this root scope.
     //
-    // Return the reference to the value as well as the indication of
-    // whether the variable has actually been set.
+    // Return the reference to the value as well as the indication of whether
+    // the value is "new", that is, it was either set (including override) or
+    // it came from the command line and was not inherited. This is usually
+    // used to test the new value.
     //
     template <typename T>
     pair<reference_wrapper<const value>, bool>
@@ -68,19 +70,6 @@ namespace build2
       return optional (root, var_pool.find (var));
     }
 
-    // As above but assumes the value is dir_path and makes it
-    // absolute if the value specified on the command line is
-    // relative.
-    //
-    const value&
-    optional_absolute (scope& root, const variable&);
-
-    inline const value&
-    optional_absolute (scope& root, const string& var)
-    {
-      return optional_absolute (root, var_pool.find (var));
-    }
-
     // Check whether there are any variables specified from the config
     // namespace. The idea is that we can check if there are any, say,
     // config.install.* values. If there are none, then we can assume
@@ -90,10 +79,25 @@ namespace build2
     //
     // Note that this function detects and ignores the special
     // config.*.configured variable which may be used by a module to
-    // "remember" that it is unconfigured.
+    // "remember" that it is unconfigured (e.g., in order to avoid re-
+    // running the tests, etc).
     //
     bool
     specified (scope& root, const string& ns);
+
+    //
+    //
+    bool
+    unconfigured (scope& root, const string& ns);
+
+    void
+    unconfigured (scope& root, const string& ns, bool);
+
+    // Enter the variable so that it is saved during configuration. See
+    // config::module.
+    //
+    void
+    save_variable (scope& root, const variable&, uint64_t flags = 0);
   }
 }
 
diff --git a/build2/config/utility.cxx b/build2/config/utility.cxx
index e96a896..1dbf3d3 100644
--- a/build2/config/utility.cxx
+++ b/build2/config/utility.cxx
@@ -6,53 +6,37 @@
 
 #include <build2/context>
 
+#include <build2/config/module>
+
 using namespace std;
 
 namespace build2
 {
   namespace config
   {
-    const value&
-    optional (scope& root, const variable& var)
+    void
+    save_variable (scope& root, const variable& var, uint64_t flags)
     {
-      auto l (root[var]);
-
-      return l.defined ()
-        ? l.belongs (*global_scope) ? (root.assign (var) = *l) : *l
-        : root.assign (var); // NULL
+      if (current_mif->id == configure_id)
+      {
+        // The project might not be using the config module. But then how
+        // could we be configuring it? Good question.
+        //
+        if (module* mod = root.modules.lookup<module> (module::name))
+          mod->vars.emplace (var, flags);
+      }
     }
 
     const value&
-    optional_absolute (scope& root, const variable& var)
+    optional (scope& root, const variable& var)
     {
-      auto l (root[var]);
+      if (current_mif->id == configure_id)
+        save_variable (root, var);
 
-      if (!l.defined ())
-        return root.assign (var); // NULL
-
-      if (!l.belongs (*global_scope)) // Value from (some) root scope.
-        return *l;
-
-      // Make the command-line value absolute. This is necessary to avoid
-      // a warning issued by the config module about global/root scope
-      // value mismatch.
-      //
-      // @@ CMDVAR
-      //
-      value& v (const_cast<value&> (*l));
-
-      if (v && !v.empty ())
-      {
-        dir_path& d (cast<dir_path> (v));
-
-        if (d.relative ())
-        {
-          d = work / d;
-          d.normalize ();
-        }
-      }
-
-      return root.assign (var) = v;
+      auto l (root[var]);
+      return l.defined ()
+        ? *l
+        : root.assign (var); // NULL.
     }
 
     bool
@@ -60,6 +44,12 @@ namespace build2
     {
       // Search all outer scopes for any value in this namespace.
       //
+      // What about "pure" overrides, i.e., those without any original values?
+      // Well, they will also be found since their names have the original
+      // variable as a prefix. But do they apply? Yes, since we haven't found
+      // any original values, they will be "visible"; see find_override() for
+      // details.
+      //
       for (scope* s (&r); s != nullptr; s = s->parent_scope ())
       {
         for (auto p (s->vars.find_namespace (ns));
@@ -78,5 +68,32 @@ namespace build2
 
       return false;
     }
+
+    bool
+    unconfigured (scope& root, const string& ns)
+    {
+      // Note: not overridable.
+      //
+      const variable& var (var_pool.insert<bool> (ns + ".configured"));
+
+      if (current_mif->id == configure_id)
+        save_variable (root, var);
+
+      auto l (root[var]); // Include inherited values.
+      return l && !cast<bool> (l);
+    }
+
+    void
+    unconfigured (scope& root, const string& ns, bool v)
+    {
+      // Note: not overridable.
+      //
+      const variable& var (var_pool.insert<bool> (ns + ".configured"));
+
+      if (current_mif->id == configure_id)
+        save_variable (root, var);
+
+      root.assign (var) = !v;
+    }
   }
 }
diff --git a/build2/config/utility.txx b/build2/config/utility.txx
index d3c57a5..f49be75 100644
--- a/build2/config/utility.txx
+++ b/build2/config/utility.txx
@@ -3,6 +3,7 @@
 // license   : MIT; see accompanying LICENSE file
 
 #include <build2/scope>
+#include <build2/context>
 
 namespace build2
 {
@@ -10,20 +11,43 @@ namespace build2
   {
     template <typename T>
     pair<reference_wrapper<const value>, bool>
-    required (scope& root, const variable& var, const T& def_value, bool ovr)
+    required (scope& root, const variable& var, const T& def_val, bool def_ovr)
     {
-      using result = pair<reference_wrapper<const value>, bool>;
+      if (current_mif->id == configure_id)
+        save_variable (root, var);
 
-      if (auto l = root[var])
+      pair<lookup, size_t> org (root.find_original (var));
+      lookup l (org.first);
+      bool n (false);
+
+      // The interaction with command line overrides can get tricky. For
+      // example, the override to defaul value could make (non-recursive)
+      // command line override in the outer scope no longer apply. So what we
+      // are going to do is first ignore overrides and perform the normal
+      // logic on the original. Then we apply the overrides on the result.
+      //
+      if (!l.defined () || (def_ovr && !l.belongs (root)))
       {
-        if (l.belongs (*global_scope))
-          return result (root.assign (var) = *l, true);
+        l = lookup ((root.assign (var) = def_val), root);
+        org = make_pair (l, 1); // Lookup depth is 1 since in root.vars.
+        n = true;
+      }
+
+      if (var.override != nullptr)
+      {
+        pair<lookup, size_t> ovr (root.find_override (var, move (org)));
+
+        if (l != ovr.first) // Overriden?
+        {
+          l = move (ovr.first);
 
-        if (!ovr || l.belongs (root))
-          return result (*l, false);
+          // Overriden and not inherited (same logic as in save_config()).
+          //
+          n = l.belongs (root) || l.belongs (*global_scope);
+        }
       }
 
-      return result (root.assign (var) = def_value, true);
+      return pair<reference_wrapper<const value>, bool> (*l, n);
     }
   }
 }