From 74d42005e25e4a3c8356d44010b82a206b8aa7e6 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 10 Apr 2023 11:18:10 +0200 Subject: 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). --- libbuild2/cc/common.hxx | 12 ++++++++---- libbuild2/cc/compile-rule.cxx | 13 +++++++------ libbuild2/cc/link-rule.cxx | 19 +++++++++---------- libbuild2/cc/module.cxx | 41 +++++++++++++++++++++++++++++------------ 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]; } } -- cgit v1.1