Make header cache case-sensitive on Windows (GH issue #390)

2 files changed, 29 insertions, 4 deletions

diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index cebd244..99c3b90 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -3148,7 +3148,7 @@ namespace build2
         }
 
         hk.file = move (fp);
-        hk.hash = hash<path> () (hk.file);
+        hk.hash = hash<string> () (hk.file.string ());
 
         slock l (hc.header_map_mutex);
         auto i (hc.header_map.find (hk));
@@ -3201,7 +3201,7 @@ namespace build2
         // path has changed (header has been remapped).
         //
         if (!e || r.second)
-          hk.hash = hash<path> () (hk.file);
+          hk.hash = hash<string> () (hk.file.string ());
 
         const file* f;
         {
@@ -3214,6 +3214,8 @@ namespace build2
         {
           //cache_cls.fetch_add (1, memory_order_relaxed);
 
+          // @@ TMP cleanup.
+          //
 #if 0
           assert (r.first == f);
 #else
diff --git a/libbuild2/cc/module.hxx b/libbuild2/cc/module.hxx
index 4213516..2ef07d6 100644
--- a/libbuild2/cc/module.hxx
+++ b/libbuild2/cc/module.hxx
@@ -92,13 +92,36 @@ namespace build2
       //
       struct header_key
       {
-        path   file;
+        // We used to use path comparison/hash which are case-insensitive on
+        // Windows. While this sounds right on the surface, the catch is that
+        // our target names are always case-sensitive, even on Windows. So
+        // what can happen is that the same header spelled in different case
+        // ends up with distinct targets but trying to occupy the same cache
+        // entry. See GH issue #390 for details.
+        //
+        // The conceptually correct way to fix this would be to actualize the
+        // header name. But that would be prohibitively expensive, especially
+        // on Windows. Plus the header may be generated and thus not yet
+        // exist.
+        //
+        // On the other hand, we can already end up with different targets
+        // mapped to the same filesystem entry in other situations (h{} vs
+        // hxx{} is the canonical example). And we are ok with that provided
+        // they have noop recipes. So it feels like the simplest solution here
+        // is to go along by having case-sensitive cache entries. This means
+        // that auto-generated headers will have to be included using the same
+        // spelling, but that seems like a sensible restriction (doing
+        // otherwise won't be portable).
+        //
+        path file;
         size_t hash;
 
         friend bool
         operator== (const header_key& x, const header_key& y)
         {
-          return x.file == y.file; // Note: hash was already compared.
+          // Note: hash was already compared.
+          //
+          return x.file.string () == y.file.string ();
         }
       };