aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2019-06-06 12:28:13 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2019-06-06 12:28:13 +0200
commit679dee2d851a67250e4a24a61da249565d478b9d (patch)
tree2968743bb74704ea7ca740670533c680b1344ace
parent952c3b41399247615fe3a3c6e5109199aacf73b5 (diff)
Redo header path normalization/realization logic
We now try to use the normalized path (which preserves symlinks) if possible and fall back to realized otherwise.
-rw-r--r--build2/cc/compile-rule.cxx86
-rw-r--r--build2/types.hxx3
-rw-r--r--tests/cc/modules/headers.testscript6
3 files changed, 72 insertions, 23 deletions
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index dd187ba..443ec35 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -2349,29 +2349,79 @@ namespace build2
else
{
// We used to just normalize the path but that could result in an
- // invalid path (e.g., on CentOS 7 with Clang 3.4) because of the
- // symlinks. So now we realize (i.e., realpath(3)) it instead. Unless
- // it comes from the depdb, in which case we've already done that.
- // This is also where we handle src-out remap (again, not needed if
- // cached).
+ // invalid path (e.g., for some system/compiler headers on CentOS 7
+ // with Clang 3.4) because of the symlinks (if a directory component
+ // is a symlink, then any following `..` are resolved relative to the
+ // target; see path::normalize() for background).
+ //
+ // Initially, to fix this, we realized (i.e., realpath(3)) it instead.
+ // But that turned out also not to be quite right since now we have
+ // all the symlinks resolved: conceptually it feels correct to keep
+ // the original header names since that's how the user chose to
+ // arrange things and practically this is how the compilers see/report
+ // them (e.g., the GCC module mapper).
+ //
+ // So now we have a pretty elaborate scheme where we try to use the
+ // normalized path if possible and fallback to realized. Normalized
+ // paths will work for situations where `..` does not cross symlink
+ // boundaries, which is the sane case. And for the insane case we only
+ // really care about out-of-project files (i.e., system/compiler
+ // headers). In other words, if you have the insane case inside your
+ // project, then you are on your own.
+ //
+ // All of this is unless the path comes from the depdb, in which case
+ // we've already done that. This is also where we handle src-out remap
+ // (again, not needed if cached).
//
if (!cache)
{
- // While we can reasonably expect this path to exit, things do go
- // south from time to time (like compiling under wine with file
- // wlantypes.h included as WlanTypes.h).
+ // Interestingly, on most paltforms and with most compilers (Clang
+ // on Linux being a notable exception) most system/compiler headers
+ // are already normalized.
//
- try
+ path_abnormality a (f.abnormalities ());
+ if (a != path_abnormality::none)
{
- f.realize ();
- }
- catch (const invalid_path&)
- {
- fail << "invalid header path '" << f << "'";
- }
- catch (const system_error& e)
- {
- fail << "invalid header path '" << f << "': " << e;
+ // While we can reasonably expect this path to exit, things do go
+ // south from time to time (like compiling under wine with file
+ // wlantypes.h included as WlanTypes.h).
+ //
+ try
+ {
+ // If we have any parent components, then we have to verify the
+ // normalized path matches realized.
+ //
+ path r;
+ if ((a & path_abnormality::parent) == path_abnormality::parent)
+ {
+ r = f;
+ r.realize ();
+ }
+
+ try
+ {
+ f.normalize ();
+
+ // Note that we might still need to resolve symlinks in the
+ // normalized path.
+ //
+ if (!r.empty () && f != r && path (f).realize () != r)
+ f = move (r);
+ }
+ catch (const invalid_path&)
+ {
+ assert (!r.empty ()); // Shouldn't have failed if no `..`.
+ f = move (r); // Fallback to realize.
+ }
+ }
+ catch (const invalid_path&)
+ {
+ fail << "invalid header path '" << f.string () << "'";
+ }
+ catch (const system_error& e)
+ {
+ fail << "invalid header path '" << f.string () << "': " << e;
+ }
}
if (!so_map.empty ())
diff --git a/build2/types.hxx b/build2/types.hxx
index c759b7f..15b7df6 100644
--- a/build2/types.hxx
+++ b/build2/types.hxx
@@ -225,9 +225,10 @@ namespace build2
//
using butl::path;
using butl::dir_path;
+ using butl::path_cast;
using butl::basic_path;
using butl::invalid_path;
- using butl::path_cast;
+ using butl::path_abnormality;
using butl::path_map;
using butl::dir_path_map;
diff --git a/tests/cc/modules/headers.testscript b/tests/cc/modules/headers.testscript
index 4fb9a42..d4eb919 100644
--- a/tests/cc/modules/headers.testscript
+++ b/tests/cc/modules/headers.testscript
@@ -21,8 +21,7 @@
: import
:
-#ln -s ../core.hxx ./; @@ why isn't working?
-cp ../core.hxx ./;
+ln -s ../core.hxx ./;
cat <<EOI >=driver.cxx;
#define CORE_IN 1
import "core.hxx";
@@ -37,8 +36,7 @@ $* test clean <<EOI
: include-translation
:
-#ln -s ../core.hxx ./; @@ why isn't working?
-cp ../core.hxx ./;
+ln -s ../core.hxx ./;
cat <<EOI >=driver.cxx;
#define CORE_IN 1
#include "core.hxx"