Redo bin pattern as PATH-like search paths rather than fallback directory

Also, unlike the fallback directory, the search paths are searched first
rather than last.

7 files changed, 129 insertions, 100 deletions

diff --git a/libbuild2/bin/guess.cxx b/libbuild2/bin/guess.cxx
index 68ef827..633c798 100644
--- a/libbuild2/bin/guess.cxx
+++ b/libbuild2/bin/guess.cxx
@@ -37,37 +37,60 @@ namespace build2
       return v ? *v : semantic_version ();
     }
 
-    ar_info
-    guess_ar (const path& ar, const path* rl, const dir_path& fallback)
+    // Search for a program first in paths if not NULL and then using the
+    // standard path search semantics. Use var to suggest an override if the
+    // search fails.
+    //
+    // Only search in PATH (specifically, omitting the current executable's
+    // directory on Windows).
+    //
+    static process_path
+    search (const path& prog, const char* paths, const char* var)
     {
-      tracer trace ("bin::guess_ar");
-
-      process_path arp, rlp;
-      guess_result arr, rlr;
-
+      if (paths != nullptr)
       {
-        auto df = make_diag_frame (
-          [](const diag_record& dr)
-          {
-            dr << info << "use config.bin.ar to override";
-          });
+        process_path r (
+          try_run_search (prog,
+                          true        /* init */,
+                          dir_path () /* fallback */,
+                          true        /* path_only */,
+                          paths));
 
-        // Only search in PATH (specifically, omitting the current
-        // executable's directory on Windows).
-        //
-        arp = run_search (ar, true, fallback, true /* path_only */);
+        if (!r.empty ())
+        {
+          // Clear the recall path since we found it in custom search paths.
+          // An alternative would have been to also do a search in PATH and if
+          // the two effective paths are the same (which means, this program
+          // is also in PATH), keep the recall. The benefit of this approach
+          // is that we will have tidier command lines without long absolute
+          // paths. The drawback is the extra complexity (we would need to
+          // normalize the paths, etc). Let's keep it simple for now.
+          //
+          r.clear_recall ();
+          return r;
+        }
       }
 
-      if (rl != nullptr)
-      {
-        auto df = make_diag_frame (
-          [](const diag_record& dr)
-          {
-            dr << info << "use config.bin.ranlib to override";
-          });
+      auto df = make_diag_frame (
+        [var](const diag_record& dr)
+        {
+          dr << info << "use " << var << " to override";
+        });
 
-        rlp = run_search (*rl, true, fallback, true /* path_only */);
-      }
+      return run_search (prog, true, dir_path (), true);
+    }
+
+    ar_info
+    guess_ar (const path& ar, const path* rl, const char* paths)
+    {
+      tracer trace ("bin::guess_ar");
+
+      guess_result arr, rlr;
+
+      process_path arp (search (ar, paths, "config.bin.ar"));
+      process_path rlp (rl != nullptr
+                        ? search (*rl, paths, "config.bin.ranlib")
+                        : process_path ());
 
       // Binutils, LLVM, and FreeBSD ar/ranlib all recognize the --version
       // option. While Microsoft's lib.exe doesn't support --version, it only
@@ -258,25 +281,13 @@ namespace build2
     }
 
     ld_info
-    guess_ld (const path& ld, const dir_path& fallback)
+    guess_ld (const path& ld, const char* paths)
     {
       tracer trace ("bin::guess_ld");
 
       guess_result r;
 
-      process_path pp;
-      {
-        auto df = make_diag_frame (
-          [](const diag_record& dr)
-          {
-            dr << info << "use config.bin.ld to override";
-          });
-
-        // Only search in PATH (specifically, omitting the current
-        // executable's directory on Windows).
-        //
-        pp = run_search (ld, true, fallback, true /* path_only */);
-      }
+      process_path pp (search (ld, paths, "config.bin.ld"));
 
       // Binutils ld recognizes the --version option. Microsoft's link.exe
       // doesn't support --version (nor any other way to get the version
@@ -389,25 +400,13 @@ namespace build2
     }
 
     rc_info
-    guess_rc (const path& rc, const dir_path& fallback)
+    guess_rc (const path& rc, const char* paths)
     {
       tracer trace ("bin::guess_rc");
 
       guess_result r;
 
-      process_path pp;
-      {
-        auto df = make_diag_frame (
-          [](const diag_record& dr)
-          {
-            dr << info << "use config.bin.rc to override";
-          });
-
-        // Only search in PATH (specifically, omitting the current
-        // executable's directory on Windows).
-        //
-        pp = run_search (rc, true, fallback, true /* path_only */);
-      }
+      process_path pp (search (rc, paths, "config.bin.rc"));
 
       // Binutils windres recognizes the --version option.
       //
diff --git a/libbuild2/bin/guess.hxx b/libbuild2/bin/guess.hxx
index 0e04ba5..b4239c2 100644
--- a/libbuild2/bin/guess.hxx
+++ b/libbuild2/bin/guess.hxx
@@ -46,7 +46,7 @@ namespace build2
     // attemplated and the returned ranlib_* members will be left empty.
     //
     ar_info
-    guess_ar (const path& ar, const path* ranlib, const dir_path& fallback);
+    guess_ar (const path& ar, const path* ranlib, const char* paths);
 
     // ld information.
     //
@@ -77,7 +77,7 @@ namespace build2
     };
 
     ld_info
-    guess_ld (const path& ld, const dir_path& fallback);
+    guess_ld (const path& ld, const char* paths);
 
     // rc information.
     //
@@ -101,7 +101,7 @@ namespace build2
     };
 
     rc_info
-    guess_rc (const path& rc, const dir_path& fallback);
+    guess_rc (const path& rc, const char* paths);
   }
 }
 
diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx
index d56e0a5..7f7fd02 100644
--- a/libbuild2/bin/init.cxx
+++ b/libbuild2/bin/init.cxx
@@ -37,6 +37,30 @@ namespace build2
     static const strings liba_lib {"static", "shared"};
     static const strings libs_lib {"shared", "static"};
 
+    struct pattern_paths
+    {
+      const char* pattern = nullptr;
+      const char* paths   = nullptr;
+    };
+
+    static inline pattern_paths
+    lookup_pattern (scope& rs)
+    {
+      pattern_paths r;
+
+      // Theoretically, we could have both the pattern and the search paths,
+      // for example, the pattern can come first followed by the paths.
+      //
+      if (const string* v = cast_null<string> (rs["bin.pattern"]))
+      {
+        (path::traits_type::is_separator (v->back ())
+         ? r.paths
+         : r.pattern) = v->c_str ();
+      }
+
+      return r;
+    }
+
     bool
     vars_init (scope& rs,
                scope&,
@@ -370,7 +394,9 @@ namespace build2
                 (!path::traits_type::is_separator (s.back ()) &&
                  s.find ('*') == string::npos))
             {
-              fail << "missing '*' in binutils pattern '" << s << "'";
+              fail << "missing '*' or trailing '"
+                   << path::traits_type::directory_separator
+                   << "' in binutils pattern '" << s << "'";
             }
 
             rs.assign<string> ("bin.pattern") = s;
@@ -602,12 +628,9 @@ namespace build2
         const string& tsys (cast<string> (rs["bin.target.system"]));
         const char* ar_d (tsys == "win32-msvc" ? "lib" : "ar");
 
-        // This can be either a pattern or a fallback search directory.
+        // This can be either a pattern or search path(s).
         //
-        const string* pat (cast_null<string> (rs["bin.pattern"]));
-
-        bool fb (pat != nullptr &&
-                 path::traits_type::is_separator (pat->back ()));
+        pattern_paths pat (lookup_pattern (rs));
 
         // Don't save the default value to config.build so that if the user
         // changes, say, the C++ compiler (which hinted the pattern), then
@@ -617,7 +640,7 @@ namespace build2
           config::required (
             rs,
             "config.bin.ar",
-            path (apply_pattern (ar_d, fb ? nullptr : pat)),
+            path (apply_pattern (ar_d, pat.pattern)),
             false,
             config::save_commented));
 
@@ -632,8 +655,7 @@ namespace build2
         const path& ar (cast<path> (ap.first));
         const path* ranlib (cast_null<path> (rp.first));
 
-        ar_info ari (
-          guess_ar (ar, ranlib, fb ? dir_path (*pat) : dir_path ()));
+        ar_info ari (guess_ar (ar, ranlib, pat.paths));
 
         // If this is a new value (e.g., we are configuring), then print the
         // report at verbosity level 2 and up (-v).
@@ -762,23 +784,20 @@ namespace build2
         const string& tsys (cast<string> (rs["bin.target.system"]));
         const char* ld_d (tsys == "win32-msvc" ? "link" : "ld");
 
-        // This can be either a pattern or a fallback search directory.
+        // This can be either a pattern or search path(s).
         //
-        const string* pat (cast_null<string> (rs["bin.pattern"]));
-
-        bool fb (pat != nullptr &&
-                 path::traits_type::is_separator (pat->back ()));
+        pattern_paths pat (lookup_pattern (rs));
 
         auto p (
           config::required (
             rs,
             "config.bin.ld",
-            path (apply_pattern (ld_d, fb ? nullptr : pat)),
+            path (apply_pattern (ld_d, pat.pattern)),
             false,
             config::save_commented));
 
         const path& ld (cast<path> (p.first));
-        ld_info ldi (guess_ld (ld, fb ? dir_path (*pat) : dir_path ()));
+        ld_info ldi (guess_ld (ld, pat.paths));
 
         // If this is a new value (e.g., we are configuring), then print the
         // report at verbosity level 2 and up (-v).
@@ -875,23 +894,20 @@ namespace build2
         const string& tsys (cast<string> (rs["bin.target.system"]));
         const char* rc_d (tsys == "win32-msvc" ? "rc" : "windres");
 
-        // This can be either a pattern or a fallback search directory.
+        // This can be either a pattern or search path(s).
         //
-        const string* pat (cast_null<string> (rs["bin.pattern"]));
-
-        bool fb (pat != nullptr &&
-                 path::traits_type::is_separator (pat->back ()));
+        pattern_paths pat (lookup_pattern (rs));
 
         auto p (
           config::required (
             rs,
             "config.bin.rc",
-            path (apply_pattern (rc_d, fb ? nullptr : pat)),
+            path (apply_pattern (rc_d, pat.pattern)),
             false,
             config::save_commented));
 
         const path& rc (cast<path> (p.first));
-        rc_info rci (guess_rc (rc, fb ? dir_path (*pat) : dir_path ()));
+        rc_info rci (guess_rc (rc, pat.paths));
 
         // If this is a new value (e.g., we are configuring), then print the
         // report at verbosity level 2 and up (-v).
diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx
index 1952796..e218539 100644
--- a/libbuild2/cc/guess.cxx
+++ b/libbuild2/cc/guess.cxx
@@ -2059,7 +2059,7 @@ namespace build2
       // still want to try the target in case we could not pre-guess (think
       // x86_64-w64-mingw32-c++).
       //
-      // BTW, for GCC we also get gcc-{ar,ranlib} (but not -ld) which add
+      // BTW, for GCC we also get gcc-{ar,ranlib} (but not gcc-ld) which add
       // support for the LTO plugin though it seems more recent GNU binutils
       // (2.25) are able to load the plugin when needed automatically. So it
       // doesn't seem we should bother trying to support this on our end (one
@@ -2068,9 +2068,8 @@ namespace build2
       // It's also normal for native (i.e., non-cross-compiler) builds of GCC
       // and Clang to not have binutils installed in the same directory and
       // instead relying on the system ones. In this case, if the compiler is
-      // specified with the absolute path, the pattern will be the fallback
-      // search directory (though it feels like it should be checked first
-      // rather than last).
+      // specified with the absolute path, the pattern will be the search
+      // path.
       //
       if (r.bin_pattern.empty ())
       {
@@ -2103,7 +2102,7 @@ namespace build2
       }
 
       // If we could not derive the pattern, then see if we can come up with a
-      // fallback search directory.
+      // search path.
       //
       if (r.bin_pattern.empty ())
       {
@@ -2152,7 +2151,7 @@ namespace build2
         }
       }
 
-      return path (apply_pattern (s, &pat));
+      return path (apply_pattern (s, pat));
     }
   }
 }
diff --git a/libbuild2/cc/guess.hxx b/libbuild2/cc/guess.hxx
index a8784fc..bf6a5e6 100644
--- a/libbuild2/cc/guess.hxx
+++ b/libbuild2/cc/guess.hxx
@@ -158,9 +158,9 @@ namespace build2
     //
     // The bin_pattern is the binutils program pattern that could sometimes be
     // derived for some toolchains. For example, i686-w64-mingw32-*. If the
-    // pattern could not be derived, then it could contain a fallback search
-    // directory, in which case it will end with a directory separator but
-    // will not contain '*'.
+    // pattern could not be derived, then it could alternatively contain
+    // search paths (similar to the PATH environment variable), in which case
+    // it will end with a directory separator but will not contain '*'.
     //
     struct compiler_info
     {
diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx
index 16bbd85..d6a32ab 100644
--- a/libbuild2/utility.cxx
+++ b/libbuild2/utility.cxx
@@ -186,9 +186,10 @@ namespace build2
   try_run_search (const path& f,
                   bool init,
                   const dir_path& fallback,
-                  bool path_only)
+                  bool path_only,
+                  const char* paths)
   {
-    return process::try_path_search (f, init, fallback, path_only);
+    return process::try_path_search (f, init, fallback, path_only, paths);
   }
 
   process
@@ -477,17 +478,18 @@ namespace build2
   }
 
   string
-  apply_pattern (const char* s, const string* p)
+  apply_pattern (const char* stem, const char* pat)
   {
-    if (p == nullptr || p->empty ())
-      return s;
+    if (pat == nullptr || *pat == '\0')
+      return stem;
 
-    size_t i (p->find ('*'));
-    assert (i != string::npos);
+    size_t n (string::traits_type::length (pat));
+    const char* p (string::traits_type::find (pat, n, '*'));
+    assert (p != nullptr);
 
-    string r (*p, 0, i++);
-    r.append (s);
-    r.append (*p, i, p->size () - i);
+    string r (pat, p++ - pat);
+    r.append (stem);
+    r.append (p, n - (p - pat));
     return r;
   }
 
diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx
index 8256fa4..c362b34 100644
--- a/libbuild2/utility.hxx
+++ b/libbuild2/utility.hxx
@@ -225,7 +225,8 @@ namespace build2
   try_run_search (const path&,
                   bool init = false,
                   const dir_path& fallback = dir_path (),
-                  bool path_only = false);
+                  bool path_only = false,
+                  const char* paths = nullptr);
 
   // Wait for process termination. Issue diagnostics and throw failed in case
   // of abnormal termination. If the process has terminated normally but with
@@ -674,7 +675,19 @@ namespace build2
   // i.e., contains a single '*' character.
   //
   LIBBUILD2_SYMEXPORT string
-  apply_pattern (const char* stem, const string* pattern);
+  apply_pattern (const char* stem, const char* pattern);
+
+  inline string
+  apply_pattern (const char* s, const string* p)
+  {
+    return apply_pattern (s, p != nullptr ? p->c_str () : nullptr);
+  }
+
+  inline string
+  apply_pattern (const char* s, const string& p)
+  {
+    return apply_pattern (s, p.c_str ());
+  }
 }
 
 #include <libbuild2/utility.ixx>