Fix inconsistent /usr/local/{include,lib} search

In particular, we were adding -L/usr/local/lib which means it is considered
before built-in directories (/usr/lib, etc) but in our own library search
code we were considering it after (because we were storing it at the end of
sys_lib_dirs).

Now in both sys_{hdr,lib}_dirs we store /usr/local/{include,lib} after mode
and before built-in directories. Note that as part of this fix we now pass
-isystem /usr/local/include instead of -idirafter (which is consistent with
the -L behavior and also the customarily expected semantics).

4 files changed, 53 insertions, 32 deletions

diff --git a/libbuild2/cc/common.hxx b/libbuild2/cc/common.hxx
index c17db08..9090c9c 100644
--- a/libbuild2/cc/common.hxx
+++ b/libbuild2/cc/common.hxx
@@ -198,18 +198,22 @@ namespace build2
       build2::cc::importable_headers* importable_headers;
 
       // The order of sys_*_dirs is the mode entries first, followed by the
-      // compiler built-in entries, and finished off with any extra entries
-      // (e.g., fallback directories such as /usr/local/*).
+      // extra entries (e.g., /usr/local/*), followed by the compiler built-in
+      // entries.
+      //
+      // Note that even if we wanted to, we wouldn't be able to support extra
+      // trailing (after built-in) directories since we would need a portable
+      // equivalent of -idirafter for both headers and libraries.
       //
       const dir_paths& sys_lib_dirs; // x.sys_lib_dirs
       const dir_paths& sys_hdr_dirs; // x.sys_hdr_dirs
       const dir_paths* sys_mod_dirs; // compiler_info::sys_mod_dirs
 
-      size_t sys_lib_dirs_mode; // Number of leading mode entries (0 if none).
+      size_t sys_lib_dirs_mode;  // Number of mode entries (0 if none).
       size_t sys_hdr_dirs_mode;
       size_t sys_mod_dirs_mode;
 
-      size_t sys_lib_dirs_extra; // First trailing extra entry (size if none).
+      size_t sys_lib_dirs_extra; // Number of extra entries (0 if none).
       size_t sys_hdr_dirs_extra;
 
       // Note that x_obj is patched in by the x.objx module. So it stays NULL
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index 7a357d9..b63b3cf 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -295,24 +295,25 @@ namespace build2
     void compile_rule::
     append_sys_hdr_options (T& args) const
     {
-      assert (sys_hdr_dirs_extra <= sys_hdr_dirs.size ());
+      assert (sys_hdr_dirs_mode + sys_hdr_dirs_extra <= sys_hdr_dirs.size ());
 
       // Note that the mode options are added as part of cmode.
       //
       auto b (sys_hdr_dirs.begin () + sys_hdr_dirs_mode);
-      auto m (sys_hdr_dirs.begin () + sys_hdr_dirs_extra);
-      auto e (sys_hdr_dirs.end ());
+      auto x (b + sys_hdr_dirs_extra);
 
+      // Add extras.
+      //
       // Note: starting from 16.10, MSVC gained /external:I option though it
       // doesn't seem to affect the order, only "system-ness".
       //
       append_option_values (
         args,
-        cclass == compiler_class::gcc  ? "-idirafter" :
+        cclass == compiler_class::gcc  ? "-isystem" :
         cclass == compiler_class::msvc ? (isystem (*this)
                                           ? "/external:I"
                                           : "/I") : "-I",
-        m, e,
+        b, x,
         [] (const dir_path& d) {return d.string ().c_str ();});
 
       // For MSVC if we have no INCLUDE environment variable set, then we
@@ -328,7 +329,7 @@ namespace build2
         {
           append_option_values (
             args, "/I",
-            b, m,
+            x, sys_hdr_dirs.end (),
             [] (const dir_path& d) {return d.string ().c_str ();});
         }
       }
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 490e0b4..76fd3c5 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -3365,25 +3365,24 @@ namespace build2
 
         // Extra system library dirs (last).
         //
-        assert (sys_lib_dirs_extra <= sys_lib_dirs.size ());
+        assert (sys_lib_dirs_mode + sys_lib_dirs_extra <= sys_lib_dirs.size ());
+
+        // Note that the mode options are added as part of cmode.
+        //
+        auto b (sys_lib_dirs.begin () + sys_lib_dirs_mode);
+        auto x (b + sys_lib_dirs_extra);
 
         if (tsys == "win32-msvc")
         {
           // If we have no LIB environment variable set, then we add all of
           // them. But we want extras to come first.
           //
-          // Note that the mode options are added as part of cmode.
-          //
-          auto b (sys_lib_dirs.begin () + sys_lib_dirs_mode);
-          auto m (sys_lib_dirs.begin () + sys_lib_dirs_extra);
-          auto e (sys_lib_dirs.end ());
-
-          for (auto i (m); i != e; ++i)
+          for (auto i (b); i != x; ++i)
             sargs1.push_back ("/LIBPATH:" + i->string ());
 
           if (!getenv ("LIB"))
           {
-            for (auto i (b); i != m; ++i)
+            for (auto i (x), e (sys_lib_dirs.end ()); i != e; ++i)
               sargs1.push_back ("/LIBPATH:" + i->string ());
           }
 
@@ -3394,7 +3393,7 @@ namespace build2
           append_option_values (
             args,
             "-L",
-            sys_lib_dirs.begin () + sys_lib_dirs_extra, sys_lib_dirs.end (),
+            b, x,
             [] (const dir_path& d) {return d.string ().c_str ();});
         }
       }
diff --git a/libbuild2/cc/module.cxx b/libbuild2/cc/module.cxx
index aa9a526..b337754 100644
--- a/libbuild2/cc/module.cxx
+++ b/libbuild2/cc/module.cxx
@@ -649,8 +649,8 @@ namespace build2
       sys_hdr_dirs_mode = hdr_dirs.second;
       sys_mod_dirs_mode = mod_dirs ? mod_dirs->second : 0;
 
-      sys_lib_dirs_extra = lib_dirs.first.size ();
-      sys_hdr_dirs_extra = hdr_dirs.first.size ();
+      sys_lib_dirs_extra = 0;
+      sys_hdr_dirs_extra = 0;
 
 #ifndef _WIN32
       // Add /usr/local/{include,lib}. We definitely shouldn't do this if we
@@ -666,11 +666,11 @@ namespace build2
       // on the next invocation.
       //
       {
-        auto& is (hdr_dirs.first);
+        auto& hs (hdr_dirs.first);
         auto& ls (lib_dirs.first);
 
-        bool ui  (find (is.begin (), is.end (), usr_inc)     != is.end ());
-        bool uli (find (is.begin (), is.end (), usr_loc_inc) != is.end ());
+        bool ui  (find (hs.begin (), hs.end (), usr_inc)     != hs.end ());
+        bool uli (find (hs.begin (), hs.end (), usr_loc_inc) != hs.end ());
 
 #ifdef __APPLE__
         // On Mac OS starting from 10.14 there is no longer /usr/include.
@@ -695,7 +695,7 @@ namespace build2
         //
         if (!ui && !uli)
         {
-          for (const dir_path& d: is)
+          for (const dir_path& d: hs)
           {
             if (path_match (d, a_usr_inc))
             {
@@ -709,18 +709,29 @@ namespace build2
         {
           bool ull (find (ls.begin (), ls.end (), usr_loc_lib) != ls.end ());
 
-          // Many platforms don't search in /usr/local/lib by default (but do
-          // for headers in /usr/local/include). So add it as the last option.
+          // Many platforms don't search in /usr/local/lib by default but do
+          // for headers in /usr/local/include.
+          //
+          // Note that customarily /usr/local/include is searched before
+          // /usr/include so we add /usr/local/lib before built-in entries
+          // (there isn't really a way to add it after since all we can do is
+          // specify it with -L).
           //
           if (!ull && exists (usr_loc_lib, true /* ignore_error */))
-            ls.push_back (usr_loc_lib);
+          {
+            ls.insert (ls.begin () + sys_lib_dirs_mode, usr_loc_lib);
+            ++sys_lib_dirs_extra;
+          }
 
           // FreeBSD is at least consistent: it searches in neither. Quoting
           // its wiki: "FreeBSD can't even find libraries that it installed."
           // So let's help it a bit.
           //
           if (!uli && exists (usr_loc_inc, true /* ignore_error */))
-            is.push_back (usr_loc_inc);
+          {
+            hs.insert (hs.begin () + sys_hdr_dirs_mode, usr_loc_inc);
+            ++sys_hdr_dirs_extra;
+          }
         }
       }
 #endif
@@ -824,8 +835,11 @@ namespace build2
           dr << "\n  hdr dirs";
           for (size_t i (0); i != incs.size (); ++i)
           {
-            if (i == sys_hdr_dirs_extra)
+            if ((sys_hdr_dirs_mode  != 0 && i == sys_hdr_dirs_mode) ||
+                (sys_hdr_dirs_extra != 0 &&
+                 i == sys_hdr_dirs_extra + sys_hdr_dirs_mode))
               dr << "\n    --";
+
             dr << "\n    " << incs[i];
           }
         }
@@ -835,8 +849,11 @@ namespace build2
           dr << "\n  lib dirs";
           for (size_t i (0); i != libs.size (); ++i)
           {
-            if (i == sys_lib_dirs_extra)
+            if ((sys_lib_dirs_mode  != 0 && i == sys_lib_dirs_mode) ||
+                (sys_lib_dirs_extra != 0 &&
+                 i == sys_lib_dirs_extra + sys_lib_dirs_mode))
               dr << "\n    --";
+
             dr << "\n    " << libs[i];
           }
         }