Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Commit 02d51b3

Browse files
committedMay 29, 2013
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.)
1 parent af7c76c commit 02d51b3

5 files changed

+20
-17
lines changed
 

‎src/includes_normalize-win32.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,6 @@ string IncludesNormalize::Normalize(const string& input,
110110
}
111111
StringPiece partially_fixed(copy, len);
112112
if (!SameDrive(partially_fixed, relative_to))
113-
return ToLower(partially_fixed.AsString());
114-
return ToLower(Relativize(partially_fixed, relative_to));
113+
return partially_fixed.AsString();
114+
return Relativize(partially_fixed, relative_to);
115115
}

‎src/includes_normalize_test.cc

+10-10
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ TEST(IncludesNormalize, WithRelative) {
5252

5353
TEST(IncludesNormalize, Case) {
5454
EXPECT_EQ("b", IncludesNormalize::Normalize("Abc\\..\\b", NULL));
55-
EXPECT_EQ("bdef", IncludesNormalize::Normalize("Abc\\..\\BdEf", NULL));
56-
EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\.\\b", NULL));
57-
EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\./b", NULL));
58-
EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\.\\B", NULL));
59-
EXPECT_EQ("a\\b", IncludesNormalize::Normalize("A\\./B", NULL));
55+
EXPECT_EQ("BdEf", IncludesNormalize::Normalize("Abc\\..\\BdEf", NULL));
56+
EXPECT_EQ("A\\b", IncludesNormalize::Normalize("A\\.\\b", NULL));
57+
EXPECT_EQ("a\\b", IncludesNormalize::Normalize("a\\./b", NULL));
58+
EXPECT_EQ("A\\B", IncludesNormalize::Normalize("A\\.\\B", NULL));
59+
EXPECT_EQ("A\\B", IncludesNormalize::Normalize("A\\./B", NULL));
6060
}
6161

6262
TEST(IncludesNormalize, Join) {
@@ -91,12 +91,12 @@ TEST(IncludesNormalize, DifferentDrive) {
9191
EXPECT_EQ("stuff.h",
9292
IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "p:\\vs08"));
9393
EXPECT_EQ("stuff.h",
94-
IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "p:\\vs08"));
95-
EXPECT_EQ("p:\\vs08\\stuff.h",
96-
IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "c:\\vs08"));
97-
EXPECT_EQ("p:\\vs08\\stuff.h",
98-
IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "D:\\stuff/things"));
94+
IncludesNormalize::Normalize("P:\\Vs08\\stuff.h", "p:\\vs08"));
9995
EXPECT_EQ("p:\\vs08\\stuff.h",
96+
IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "c:\\vs08"));
97+
EXPECT_EQ("P:\\vs08\\stufF.h",
98+
IncludesNormalize::Normalize("P:\\vs08\\stufF.h", "D:\\stuff/things"));
99+
EXPECT_EQ("P:\\vs08\\stuff.h",
100100
IncludesNormalize::Normalize("P:/vs08\\stuff.h", "D:\\stuff/things"));
101101
// TODO: this fails; fix it.
102102
//EXPECT_EQ("P:\\wee\\stuff.h",

‎src/msvc_helper-win32.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "msvc_helper.h"
1616

17+
#include <algorithm>
1718
#include <stdio.h>
1819
#include <string.h>
1920
#include <windows.h>
@@ -63,14 +64,16 @@ string CLParser::FilterShowIncludes(const string& line) {
6364
}
6465

6566
// static
66-
bool CLParser::IsSystemInclude(const string& path) {
67+
bool CLParser::IsSystemInclude(string path) {
68+
transform(path.begin(), path.end(), path.begin(), ::tolower);
6769
// TODO: this is a heuristic, perhaps there's a better way?
6870
return (path.find("program files") != string::npos ||
6971
path.find("microsoft visual studio") != string::npos);
7072
}
7173

7274
// static
73-
bool CLParser::FilterInputFilename(const string& line) {
75+
bool CLParser::FilterInputFilename(string line) {
76+
transform(line.begin(), line.end(), line.begin(), ::tolower);
7477
// TODO: other extensions, like .asm?
7578
return EndsWith(line, ".c") ||
7679
EndsWith(line, ".cc") ||

‎src/msvc_helper.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,14 @@ struct CLParser {
3030
static string FilterShowIncludes(const string& line);
3131

3232
/// Return true if a mentioned include file is a system path.
33-
/// Expects the path to already by normalized (including lower case).
3433
/// Filtering these out reduces dependency information considerably.
35-
static bool IsSystemInclude(const string& path);
34+
static bool IsSystemInclude(string path);
3635

3736
/// Parse a line of cl.exe output and return true if it looks like
3837
/// it's printing an input filename. This is a heuristic but it appears
3938
/// to be the best we can do.
4039
/// Exposed for testing.
41-
static bool FilterInputFilename(const string& line);
40+
static bool FilterInputFilename(string line);
4241

4342
/// Parse the full output of cl, returning the output (if any) that
4443
/// should printed.

‎src/msvc_helper_test.cc

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ TEST(CLParserTest, FilterInputFilename) {
3535
ASSERT_TRUE(CLParser::FilterInputFilename("foobar.cc"));
3636
ASSERT_TRUE(CLParser::FilterInputFilename("foo bar.cc"));
3737
ASSERT_TRUE(CLParser::FilterInputFilename("baz.c"));
38+
ASSERT_TRUE(CLParser::FilterInputFilename("FOOBAR.CC"));
3839

3940
ASSERT_FALSE(CLParser::FilterInputFilename(
4041
"src\\cl_helper.cc(166) : fatal error C1075: end "

0 commit comments

Comments
 (0)
This repository has been archived.