diff options
author | Nico Weber <thakis@chromium.org> | 2013-05-29 14:37:06 (GMT) |
---|---|---|
committer | Nico Weber <thakis@chromium.org> | 2013-05-29 15:11:17 (GMT) |
commit | 02d51b3f3d3058411a06bb08d5b773f60848fec4 (patch) | |
tree | 3ee24b8d01c1e76240c682977046db81d918bd56 | |
parent | af7c76cf64539ea51ae86c2dbc77cc45ccc79a46 (diff) | |
download | Ninja-02d51b3f3d3058411a06bb08d5b773f60848fec4.zip Ninja-02d51b3f3d3058411a06bb08d5b773f60848fec4.tar.gz Ninja-02d51b3f3d3058411a06bb08d5b773f60848fec4.tar.bz2 |
Stop `-t msvc -o` from lowercasing paths from /showIncludes output.
/showIncludes prints absolute paths. If a source file says
`#include <WiNdOwS.h>`, /showIncludes will use that spelling in
its output for the basename and use the on-disk cache for the
directory names in the rest for its output.
This makes the .d files created by `-t msvc -o` consistent with the
.d files written by gcc and clang.
Before this change, `-t msvc -o` would convert this output to
lower case. This is a problem if a build step produces a header file
with mixed case, such as "RuntimeFeatures.h". Due to the lowercasing,
the .d file would contain "runtimefeatures.h", while the build step
will create "RuntimeFeatures.h". Due to the case difference, ninja
would not realize that regeneration of the .h file would require
a rebuild of all source files having the header in the .d file.
(On the next build, ninja would rebuild them since stat()ing is not
case-sensitive on Windows.) One possible fix for this is to make sure
that generators always write generated header files in lower case too,
but on Mac gcc doesn't do lower-casing and .d files end up with the
case-as-written, so generators would have to be different on Mac and
Windows, which is undesirable.
If case-insensitve path comparisons are useful, they should be done
somewhere else (e.g. in CanonicalizePath()) where they affect both
paths read from .d files and paths read from .ninja files. This should
then be controlled by a top-level variable.
This patch changes behavior, but it only has an effect on generated
header files, which aren't common, and it only affects -t msvc, which
is still marked as experimental. (cmake doesn't use it yet.)
(If a file has both `#include <windows.h>` and `<Windows.h>`, this
will now take 2 stat() calls instead of just one, but that should
have a negligible cost.)
-rw-r--r-- | src/includes_normalize-win32.cc | 4 | ||||
-rw-r--r-- | src/includes_normalize_test.cc | 20 | ||||
-rw-r--r-- | src/msvc_helper-win32.cc | 7 | ||||
-rw-r--r-- | src/msvc_helper.h | 5 | ||||
-rw-r--r-- | src/msvc_helper_test.cc | 1 |
5 files changed, 20 insertions, 17 deletions
diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 4bc8756..05ce75d 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -110,6 +110,6 @@ string IncludesNormalize::Normalize(const string& input, } StringPiece partially_fixed(copy, len); if (!SameDrive(partially_fixed, relative_to)) - return ToLower(partially_fixed.AsString()); - return ToLower(Relativize(partially_fixed, relative_to)); + return partially_fixed.AsString(); + return Relativize(partially_fixed, relative_to); } diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index 29e6755..1713d5d 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -52,11 +52,11 @@ TEST(IncludesNormalize, WithRelative) { TEST(IncludesNormalize, Case) { EXPECT_EQ("b", IncludesNormalize::Normalize("Abc\\..\\b", NULL)); - EXPECT_EQ("bdef", IncludesNormalize::Normalize("Abc\\..\\BdEf", NULL)); - EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\.\\b", NULL)); - EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\./b", NULL)); - EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\.\\B", NULL)); - EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\./B", NULL)); + EXPECT_EQ("BdEf", IncludesNormalize::Normalize("Abc\\..\\BdEf", NULL)); + EXPECT_EQ("A\\b", IncludesNormalize::Normalize("A\\.\\b", NULL)); + EXPECT_EQ("a\\b", IncludesNormalize::Normalize("a\\./b", NULL)); + EXPECT_EQ("A\\B", IncludesNormalize::Normalize("A\\.\\B", NULL)); + EXPECT_EQ("A\\B", IncludesNormalize::Normalize("A\\./B", NULL)); } TEST(IncludesNormalize, Join) { @@ -91,12 +91,12 @@ TEST(IncludesNormalize, DifferentDrive) { EXPECT_EQ("stuff.h", IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "p:\\vs08")); EXPECT_EQ("stuff.h", - IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "p:\\vs08")); - EXPECT_EQ("p:\\vs08\\stuff.h", - IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "c:\\vs08")); - EXPECT_EQ("p:\\vs08\\stuff.h", - IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "D:\\stuff/things")); + IncludesNormalize::Normalize("P:\\Vs08\\stuff.h", "p:\\vs08")); EXPECT_EQ("p:\\vs08\\stuff.h", + IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "c:\\vs08")); + EXPECT_EQ("P:\\vs08\\stufF.h", + IncludesNormalize::Normalize("P:\\vs08\\stufF.h", "D:\\stuff/things")); + EXPECT_EQ("P:\\vs08\\stuff.h", IncludesNormalize::Normalize("P:/vs08\\stuff.h", "D:\\stuff/things")); // TODO: this fails; fix it. //EXPECT_EQ("P:\\wee\\stuff.h", diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index be2a5e0..7c45029 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -14,6 +14,7 @@ #include "msvc_helper.h" +#include <algorithm> #include <stdio.h> #include <string.h> #include <windows.h> @@ -63,14 +64,16 @@ string CLParser::FilterShowIncludes(const string& line) { } // static -bool CLParser::IsSystemInclude(const string& path) { +bool CLParser::IsSystemInclude(string path) { + transform(path.begin(), path.end(), path.begin(), ::tolower); // TODO: this is a heuristic, perhaps there's a better way? return (path.find("program files") != string::npos || path.find("microsoft visual studio") != string::npos); } // static -bool CLParser::FilterInputFilename(const string& line) { +bool CLParser::FilterInputFilename(string line) { + transform(line.begin(), line.end(), line.begin(), ::tolower); // TODO: other extensions, like .asm? return EndsWith(line, ".c") || EndsWith(line, ".cc") || diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 32ab606..e207485 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -30,15 +30,14 @@ struct CLParser { static string FilterShowIncludes(const string& line); /// Return true if a mentioned include file is a system path. - /// Expects the path to already by normalized (including lower case). /// Filtering these out reduces dependency information considerably. - static bool IsSystemInclude(const string& path); + static bool IsSystemInclude(string path); /// Parse a line of cl.exe output and return true if it looks like /// it's printing an input filename. This is a heuristic but it appears /// to be the best we can do. /// Exposed for testing. - static bool FilterInputFilename(const string& line); + static bool FilterInputFilename(string line); /// Parse the full output of cl, returning the output (if any) that /// should printed. diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 1e1cbde..02f2863 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -35,6 +35,7 @@ TEST(CLParserTest, FilterInputFilename) { ASSERT_TRUE(CLParser::FilterInputFilename("foobar.cc")); ASSERT_TRUE(CLParser::FilterInputFilename("foo bar.cc")); ASSERT_TRUE(CLParser::FilterInputFilename("baz.c")); + ASSERT_TRUE(CLParser::FilterInputFilename("FOOBAR.CC")); ASSERT_FALSE(CLParser::FilterInputFilename( "src\\cl_helper.cc(166) : fatal error C1075: end " |