Save default/hinted ar/ld/rc values as commented out

This way, when we, for example, change the C++ compiler (which hinted these
values), they will be automatically adjusted as well.

7 files changed, 75 insertions, 34 deletions

diff --git a/build2/bin/guess b/build2/bin/guess
index 10f337a..540e644 100644
--- a/build2/bin/guess
+++ b/build2/bin/guess
@@ -39,12 +39,11 @@ namespace build2
       string ranlib_checksum;
     };
 
-    // The ranlib path can be empty, in which case no ranlib guessing will be
-    // attemplated and the returned ranlib_* members will be left empty as
-    // well.
+    // The ranlib path can be NULL, in which case no ranlib guessing will be
+    // attemplated and the returned ranlib_* members will be left empty.
     //
     ar_info
-    guess_ar (const path& ar, const path& ranlib);
+    guess_ar (const path& ar, const path* ranlib);
 
     // ld information.
     //
diff --git a/build2/bin/guess.cxx b/build2/bin/guess.cxx
index 8a91015..de91db3 100644
--- a/build2/bin/guess.cxx
+++ b/build2/bin/guess.cxx
@@ -23,7 +23,7 @@ namespace build2
     };
 
     ar_info
-    guess_ar (const path& ar, const path& rl)
+    guess_ar (const path& ar, const path* rl)
     {
       tracer trace ("bin::guess_ar");
 
@@ -106,7 +106,7 @@ namespace build2
 
       // Now repeat pretty much the same steps for ranlib if requested.
       //
-      if (!rl.empty ())
+      if (rl != nullptr)
       {
         // Binutils, LLVM, and FreeBSD.
         //
@@ -133,7 +133,7 @@ namespace build2
           };
 
           sha256 cs;
-          rlr = run<guess_result> (rl, "--version", f, false, false, &cs);
+          rlr = run<guess_result> (*rl, "--version", f, false, false, &cs);
 
           if (!rlr.empty ())
             rlr.checksum = cs.string ();
@@ -153,7 +153,7 @@ namespace build2
           // Redirect STDERR to STDOUT and ignore exit status.
           //
           sha256 cs;
-          rlr = run<guess_result> (rl, f, false, true, &cs);
+          rlr = run<guess_result> (*rl, f, false, true, &cs);
 
           if (!rlr.empty ())
           {
@@ -163,7 +163,7 @@ namespace build2
         }
 
         if (rlr.empty ())
-          fail << "unable to guess " << rl << " signature";
+          fail << "unable to guess " << *rl << " signature";
       }
 
       return ar_info {
diff --git a/build2/bin/module.cxx b/build2/bin/module.cxx
index 6f9a938..26dc414 100644
--- a/build2/bin/module.cxx
+++ b/build2/bin/module.cxx
@@ -245,7 +245,7 @@ namespace build2
               v = l.value;
           }
 
-          // For ease of use enter it as bin.pattern (it can come from
+          // For ease of use enter it as bin.pattern (since it can come from
           // different places).
           //
           if (v != nullptr)
@@ -415,20 +415,31 @@ namespace build2
         const string& tsys (cast<string> (r["bin.target.system"]));
         const char* ar_d (tsys == "win32-msvc" ? "lib" : "ar");
 
-        auto p (config::required (r,
+        // Don't save the default value to config.build so that if the user
+        // changes, say, the C++ compiler (which hinted the pattern), then
+        // ar will automatically change as well.
+        //
+        auto ap (config::required (r,
                                   "config.bin.ar",
-                                  path (apply (pattern, ar_d))));
-        auto& v (config::optional (r, "config.bin.ranlib"));
+                                  path (apply (pattern, ar_d)),
+                                  false,
+                                  config::save_commented));
 
-        const path& ar (cast<path> (p.first));
-        const path& ranlib (v ? cast<path> (v) : path ());
+        auto rp (config::required (r,
+                                   "config.bin.ranlib",
+                                   nullptr,
+                                   false,
+                                   config::save_commented));
+
+        const path& ar (cast<path> (ap.first));
+        const path* ranlib (cast_null<path> (rp.first));
 
         ar_info ari (guess_ar (ar, ranlib));
 
         // If this is a new value (e.g., we are configuring), then print the
         // report at verbosity level 2 and up (-v).
         //
-        if (verb >= (p.second ? 2 : 3))
+        if (verb >= (ap.second || rp.second ? 2 : 3))
         {
           diag_record dr (text);
 
@@ -438,10 +449,10 @@ namespace build2
              << "  signature  " << ari.ar_signature << '\n'
              << "  checksum   " << ari.ar_checksum;
 
-          if (!ranlib.empty ())
+          if (ranlib != nullptr)
           {
             dr << '\n'
-               << "  ranlib     " << ranlib << '\n'
+               << "  ranlib     " << *ranlib << '\n'
                << "  id         " << ari.ranlib_id << '\n'
                << "  signature  " << ari.ranlib_signature << '\n'
                << "  checksum   " << ari.ranlib_checksum;
@@ -452,7 +463,7 @@ namespace build2
         r.assign<string> ("bin.ar.signature") = move (ari.ar_signature);
         r.assign<string> ("bin.ar.checksum") = move (ari.ar_checksum);
 
-        if (!ranlib.empty ())
+        if (ranlib != nullptr)
         {
           r.assign<string> ("bin.ranlib.id") = move (ari.ranlib_id);
           r.assign<string> ("bin.ranlib.signature") =
@@ -504,7 +515,9 @@ namespace build2
 
         auto p (config::required (r,
                                   "config.bin.ld",
-                                  path (apply (r["bin.pattern"], ld_d))));
+                                  path (apply (r["bin.pattern"], ld_d)),
+                                  false,
+                                  config::save_commented));
 
         const path& ld (cast<path> (p.first));
         ld_info ldi (guess_ld (ld));
@@ -581,7 +594,9 @@ namespace build2
 
         auto p (config::required (r,
                                   "config.bin.rc",
-                                  path (apply (r["bin.pattern"], rc_d))));
+                                  path (apply (r["bin.pattern"], rc_d)),
+                                  false,
+                                  config::save_commented));
 
         const path& rc (cast<path> (p.first));
         rc_info rci (guess_rc (rc));
diff --git a/build2/config/module b/build2/config/module
index adc9b05..aed85a5 100644
--- a/build2/config/module
+++ b/build2/config/module
@@ -19,9 +19,10 @@ namespace build2
   {
     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())
+      // A sorted list of config.* variables and their "save flags" (see
+      // save_variable()) that are used (as opposed to just being specified)
+      // in this configuration. Populated by the config utility functions
+      // (required(), optional())
       //
       butl::prefix_map<variable_cref, uint64_t, '.'> vars;
 
diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx
index 4a5b39c..c3868ab 100644
--- a/build2/config/operation.cxx
+++ b/build2/config/operation.cxx
@@ -16,6 +16,7 @@
 #include <build2/diagnostics>
 
 #include <build2/config/module>
+#include <build2/config/utility>
 
 using namespace std;
 using namespace butl;
@@ -86,14 +87,16 @@ namespace build2
 
         ofs << "# Created automatically by the config module, but feel " <<
           "free to edit." << endl
-            << "#" << endl;
+            << "#" << endl
+            << endl;
 
         if (auto l = root.vars["amalgamation"])
         {
           const dir_path& d (cast<dir_path> (l));
 
           ofs << "# Base configuration inherited from " << d << endl
-              << "#" << endl;
+              << "#" << endl
+              << endl;
         }
 
         // Separate variables for modules with blank lines.
@@ -135,6 +138,7 @@ namespace build2
         {
           const auto& p (*i);
           const variable& var (p.first);
+          uint64_t sflags (p.second);
 
           pair<lookup, size_t> org (root.find_original (var));
           pair<lookup, size_t> ovr (var.override == nullptr
@@ -249,6 +253,16 @@ namespace build2
           if (next_module (var))
             ofs << endl;
 
+          // Handle the save_commented flag.
+          //
+          if (org.first->extra &&       // Default value.
+              org.first == ovr.first && // Not overriden.
+              (sflags & save_commented) == save_commented)
+          {
+            ofs << '#' << n << " =" << endl;
+            continue;
+          }
+
           if (v)
           {
             storage.clear ();
diff --git a/build2/config/utility b/build2/config/utility
index c47cecf..c1ccc0f 100644
--- a/build2/config/utility
+++ b/build2/config/utility
@@ -22,6 +22,7 @@ namespace build2
     // 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" to the default value on this root scope.
+    // See save_variable() for more information on save_flags.
     //
     // Return the reference to the value as well as the indication of whether
     // the value is "new", that is, it was set to the default value (inherited
@@ -34,25 +35,30 @@ namespace build2
     required (scope& root,
               const variable&,
               const T& default_value,
-              bool override = false);
+              bool override = false,
+              uint64_t save_flags = 0);
 
     template <typename T>
     inline pair<reference_wrapper<const value>, bool>
     required (scope& root,
               const string& name,
               const T& default_value,
-              bool override = false)
+              bool override = false,
+              uint64_t save_flags = 0)
     {
-      return required (root, var_pool.find (name), default_value, override);
+      return required (
+        root, var_pool.find (name), default_value, override, save_flags);
     }
 
     inline pair<reference_wrapper<const value>, bool>
     required (scope& root,
               const string& name,
               const char* default_value,
-              bool override = false)
+              bool override = false,
+              uint64_t save_flags = 0)
     {
-      return required (root, name, string (default_value), override);
+      return required (
+        root, name, string (default_value), override, save_flags);
     }
 
     // As above, but leave the unspecified value as undefined (and return
@@ -116,8 +122,10 @@ namespace build2
     unconfigured (scope& root, const string& ns, bool);
 
     // Enter the variable so that it is saved during configuration. See
-    // config::module.
+    // config::module for details.
     //
+    const uint64_t save_commented = 0x01; // Save default value as commented.
+
     void
     save_variable (scope& root, const variable&, uint64_t flags = 0);
   }
diff --git a/build2/config/utility.txx b/build2/config/utility.txx
index 5ebd261..4e6747b 100644
--- a/build2/config/utility.txx
+++ b/build2/config/utility.txx
@@ -11,13 +11,17 @@ namespace build2
   {
     template <typename T>
     pair<reference_wrapper<const value>, bool>
-    required (scope& root, const variable& var, const T& def_val, bool def_ovr)
+    required (scope& root,
+              const variable& var,
+              const T& def_val,
+              bool def_ovr,
+              uint64_t save_flags)
     {
       // Note: see also the other required() version if changing anything
       // here.
 
       if (current_mif->id == configure_id)
-        save_variable (root, var);
+        save_variable (root, var, save_flags);
 
       pair<lookup, size_t> org (root.find_original (var));