Add support for optional configuration for cli module

2 files changed, 101 insertions, 47 deletions

diff --git a/build/cli/module.cxx b/build/cli/module.cxx
index 081980b..7ce4235 100644
--- a/build/cli/module.cxx
+++ b/build/cli/module.cxx
@@ -34,7 +34,7 @@ namespace build
               const location& loc,
               std::unique_ptr<module>&,
               bool first,
-              bool)
+              bool optional)
     {
       tracer trace ("cli::init");
       level5 ([&]{trace << "for " << base.out_path ();});
@@ -61,30 +61,6 @@ namespace build
         tts.insert<cli_cxx> ();
       }
 
-      // Register our rules.
-      //
-      {
-        auto& rs (base.rules);
-
-        rs.insert<cli_cxx> (perform_id, update_id, "cli", compile_);
-        rs.insert<cli_cxx> (perform_id, clean_id, "cli", compile_);
-
-        rs.insert<cxx::hxx> (perform_id, update_id, "cli", compile_);
-        rs.insert<cxx::hxx> (perform_id, clean_id, "cli", compile_);
-
-        rs.insert<cxx::cxx> (perform_id, update_id, "cli", compile_);
-        rs.insert<cxx::cxx> (perform_id, clean_id, "cli", compile_);
-
-        rs.insert<cxx::ixx> (perform_id, update_id, "cli", compile_);
-        rs.insert<cxx::ixx> (perform_id, clean_id, "cli", compile_);
-
-        // Other rules (e.g., cxx::compile) may need to have the group
-        // members resolved. Looks like a general pattern: groups should
-        // resolve on configure(update).
-        //
-        rs.insert<cli_cxx> (configure_id, update_id, "cli", compile_);
-      }
-
       // Enter module variables.
       //
       if (first)
@@ -97,19 +73,27 @@ namespace build
 
       // Configure.
       //
+      // The plan is as follows: try to configure the module. If this
+      // fails with the default values and the module is optional,
+      // leave it unconfigured.
+      //
+      bool r (true);
+
+      // We will only honor optional if the user didn't specify any cli
+      // configuration explicitly.
+      //
+      optional = optional && !config::specified (root, "config.cli");
 
       // config.cli
       //
       if (first)
       {
-        auto p (config::required (root, "config.cli", "cli"));
-
-        // If we actually set a new value, test it by trying to execute.
+        // Return version or empty string if unable to execute (e.g.,
+        // the cli executable is not found).
         //
-        if (p.second)
+        auto test = [optional] (const char* cli) -> string
         {
-          const string& cli (as<string> (p.first));
-          const char* args[] = {cli.c_str (), "--version", nullptr};
+          const char* args[] = {cli, "--version", nullptr};
 
           if (verb >= 2)
             print_process (args);
@@ -124,33 +108,75 @@ namespace build
 
             // The version should be the last word on the first line.
             //
-            string l;
-            getline (is, l);
-            auto p (l.rfind (' '));
+            getline (is, ver);
+            auto p (ver.rfind (' '));
             if (p != string::npos)
-              ver = string (l, p + 1);
+              ver = string (ver, p + 1);
 
             is.close (); // Don't block the other end.
 
             if (!pr.wait ())
-              throw failed ();
+              return string (); // Not found.
 
             if (ver.empty ())
               fail << "unexpected output from " << cli;
+
+            return ver;
           }
           catch (const process_error& e)
           {
-            error << "unable to execute " << cli << ": " << e.what ();
+            if (!optional)
+              error << "unable to execute " << cli << ": " << e.what ();
 
             if (e.child ())
               exit (1);
 
             throw failed ();
           }
+        };
 
-          if (verb >= 2)
-            text << cli << " " << ver;
+        string ver;
+        const char* cli ("cli"); // Default.
+
+        if (optional)
+        {
+          // Test the default value before setting any config.cli.* values
+          // so that if we fail to configure, nothing will be written to
+          // config.build.
+          //
+          ver = test (cli);
+
+          if (ver.empty ())
+          {
+            r = false;
+
+            if (verb >= 2)
+              text << cli << " not found, leaving cli module unconfigured";
+          }
+          else
+          {
+            auto p (config::required (root, "config.cli", cli));
+            assert (p.second && as<string> (p.first) == cli);
+          }
+        }
+        else
+        {
+          auto p (config::required (root, "config.cli", cli));
+
+          // If we actually set a new value, test it by trying to execute.
+          //
+          if (p.second)
+          {
+            cli = as<string> (p.first).c_str ();
+            ver = test (cli);
+
+            if (ver.empty ())
+              throw failed ();
+          }
         }
+
+        if (!ver.empty () && verb >= 2)
+          text << cli << " " << ver;
       }
 
       // config.cli.options
@@ -159,10 +185,38 @@ namespace build
       // cli.* variables. See the cxx module for more information on
       // this merging semantics and some of its tricky aspects.
       //
-      if (const value& v = config::optional (root, "config.cli.options"))
-        base.assign ("cli.options") += as<strings> (v);
+      if (r)
+      {
+        if (const value& v = config::optional (root, "config.cli.options"))
+          base.assign ("cli.options") += as<strings> (v);
+      }
+
+      // Register our rules.
+      //
+      if (r)
+      {
+        auto& rs (base.rules);
+
+        rs.insert<cli_cxx> (perform_id, update_id, "cli", compile_);
+        rs.insert<cli_cxx> (perform_id, clean_id, "cli", compile_);
+
+        rs.insert<cxx::hxx> (perform_id, update_id, "cli", compile_);
+        rs.insert<cxx::hxx> (perform_id, clean_id, "cli", compile_);
+
+        rs.insert<cxx::cxx> (perform_id, update_id, "cli", compile_);
+        rs.insert<cxx::cxx> (perform_id, clean_id, "cli", compile_);
+
+        rs.insert<cxx::ixx> (perform_id, update_id, "cli", compile_);
+        rs.insert<cxx::ixx> (perform_id, clean_id, "cli", compile_);
+
+        // Other rules (e.g., cxx::compile) may need to have the group
+        // members resolved. Looks like a general pattern: groups should
+        // resolve on configure(update).
+        //
+        rs.insert<cli_cxx> (configure_id, update_id, "cli", compile_);
+      }
 
-      return true;
+      return r;
     }
   }
 }
diff --git a/build/variable b/build/variable
index 061004c..5d0b175 100644
--- a/build/variable
+++ b/build/variable
@@ -10,7 +10,7 @@
 #include <cstddef>       // nullptr_t
 #include <utility>       // pair, make_pair()
 #include <iterator>
-#include <functional>    // hash, reference_wrapper
+#include <functional>    // hash
 #include <type_traits>   // conditional, is_reference, remove_reference, etc.
 #include <unordered_set>
 
@@ -93,10 +93,10 @@ namespace build
     }
 
     value&
-    operator= (std::reference_wrapper<const value> v)
-    {
-      return *this = v.get ();
-    }
+    operator= (reference_wrapper<value> v) {return *this = v.get ();}
+
+    value&
+    operator= (reference_wrapper<const value> v) {return *this = v.get ();}
 
     value&
     append (value, const variable&); // Aka operator+=().