diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 63803008e1..15ef5d594c 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -619,12 +619,12 @@ rule cc command = cl /showIncludes -c $in /Fo$out ---- -3. (since Ninja 1.11) `deps = msvc_source_dependencies` specifies that the tool - outputs header dependencies in the form produced by Visual Studio's - compiler's `/sourceDependencies` flag. This flag is supported since Visual - Studio 16.7. Ninja will process `depfile` as a JSON file specifying header - dependencies immediately after the compilation finishes, then delete the - `.json` file (which is only used as a temporary). +3. (since Ninja 1.11, Windows only) `deps = msvc_source_dependencies` specifies + that the tool outputs header dependencies in the form produced by Visual + Studio's compiler's `/sourceDependencies` flag. This flag is supported since + Visual Studio 16.7. Ninja will process `depfile` as a JSON file specifying + header dependencies immediately after the compilation finishes, then delete + the `.json` file (which is only used as a temporary). + ---- rule cc diff --git a/src/build.cc b/src/build.cc index bc2dde3419..63721f7c5a 100644 --- a/src/build.cc +++ b/src/build.cc @@ -1074,9 +1074,8 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, } else if (deps_type == "gcc" || deps_type == "msvc_source_dependencies") { string depfile = result->edge->GetUnescapedDepfile(); if (depfile.empty()) { - *err = string("edge with deps="); - err->append(deps_type); - err->append("but no depfile makes no sense"); + *err = string("edge with deps=") + deps_type + + string(" but no depfile makes no sense"); return false; } @@ -1114,12 +1113,12 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, *err = string("msvc_source_dependencies is only supported on Windows"); return false; #else - set includes; + std::set includes; if (!ParseCLSourceDependencies(content, err, includes)) return false; - for (set::iterator i = includes.begin(); i != includes.end(); - ++i) { + for (std::set::iterator i = includes.begin(); + i != includes.end(); ++i) { // Like above, ~0 is assuming that with MSVC-parsed headers, it's ok to // always make all backslashes. deps_nodes->push_back(state_->GetNode(*i, ~0u)); diff --git a/src/build_test.cc b/src/build_test.cc index 12c338354d..2c592889b7 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1838,6 +1838,24 @@ TEST_F(BuildTest, DepsGccWithEmptyDepfileErrorsOut) { ASSERT_EQ(1u, command_runner_.commands_ran_.size()); } +TEST_F(BuildTest, DepsMsvcSourceDependenciesWithEmptyDepfileErrorsOut) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule cc\n" +" command = cc\n" +" deps = msvc_source_dependencies\n" +"build out: cc\n")); + Dirty("out"); + + string err; + EXPECT_TRUE(builder_.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_FALSE(builder_.AlreadyUpToDate()); + + EXPECT_FALSE(builder_.Build(&err)); + ASSERT_EQ("subcommand failed", err); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); +} + TEST_F(BuildTest, StatusFormatElapsed) { status_.BuildStarted(); // Before any task is done, the elapsed time must be zero. diff --git a/src/clsourcedependencies_parser.cc b/src/clsourcedependencies_parser.cc index bed9499630..f925ef43b6 100644 --- a/src/clsourcedependencies_parser.cc +++ b/src/clsourcedependencies_parser.cc @@ -1,3 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); #ifdef _WIN32 +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + #if NINJA_ENABLE_CL_SOURCE_DEPENDENCIES #include "clsourcedependencies_parser.h" @@ -8,80 +22,69 @@ #include "metrics.h" #include "util.h" -using namespace rapidjson; - bool ParseCLSourceDependencies(const StringPiece content, std::string* err, std::set& includes) { METRIC_RECORD("CLSourceDependenciesParser::Parse"); - Document doc; + rapidjson::Document doc; if (doc.Parse(content.str_).HasParseError()) { - *err = string("sourceDependencies parse error"); + *err = std::string("sourceDependencies parse error"); return false; } if (!doc.IsObject()) { - *err = string("sourceDependencies root must be an object"); + *err = std::string("sourceDependencies root must be an object"); return false; } if (!doc.HasMember("Version")) { - *err = string("sourceDependencies missing version"); + *err = std::string("sourceDependencies missing version"); return false; } - const Value& version = doc["Version"]; + const rapidjson::Value& version = doc["Version"]; if (!version.IsString()) { - *err = string("sourceDependencies version must be a string"); + *err = std::string("sourceDependencies version must be a string"); return false; } const char* version_str = version.GetString(); if (strncmp(version_str, "1.", 2)) { - *err = string("expected sourceDependencies version 1.x but found ") + + *err = std::string("expected sourceDependencies version 1.x but found ") + version_str; return false; } if (!doc.HasMember("Data")) { - *err = string("sourceDependencies missing data"); + *err = std::string("sourceDependencies missing data"); return false; } - const Value& data = doc["Data"]; + const rapidjson::Value& data = doc["Data"]; if (!data.IsObject()) { - *err = string("sourceDependencies data must be an object"); + *err = std::string("sourceDependencies data must be an object"); return false; } if (!data.HasMember("Includes")) { - *err = string("sourceDependencies data missing includes"); + *err = std::string("sourceDependencies data missing includes"); return false; } - const Value& includes_val = data["Includes"]; + const rapidjson::Value& includes_val = data["Includes"]; if (!includes_val.IsArray()) { - *err = string("sourceDependencies includes must be an array"); + *err = std::string("sourceDependencies includes must be an array"); return false; } - for (Value::ConstValueIterator i = includes_val.Begin(); + for (rapidjson::Value::ConstValueIterator i = includes_val.Begin(); i != includes_val.End(); ++i) { if (!i->IsString()) { - *err = string("sourceDependencies include path must be a string"); + *err = std::string("sourceDependencies include path must be a string"); return false; } - const char* include_c_str = i->GetString(); - // JSON is encoded in UTF-8, but Ninja on Windows uses the ANSI APIs. - // Attempt to convert to the active code page. This could fail if the user - // has characters outside the code page in their paths, but there's not much - // we can do about that without more fundamental changes to Ninja. - string include_str; - if (!TryConvertUtf8ToAnsi(include_c_str, include_str)) { - *err = string("could not convert include path from UTF-8 to ANSI"); - return false; - } + std::string include_str = std::string(i->GetString()); if (!CLParser::IsSystemInclude(include_str)) includes.insert(include_str); } diff --git a/src/clsourcedependencies_parser.h b/src/clsourcedependencies_parser.h index b9cc528725..4d0696e674 100644 --- a/src/clsourcedependencies_parser.h +++ b/src/clsourcedependencies_parser.h @@ -1,3 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); #ifdef _WIN32 +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + #if NINJA_ENABLE_CL_SOURCE_DEPENDENCIES #ifndef NINJA_CLSOURCEDEPENDENCIESPARSER_H_ diff --git a/src/clsourcedependencies_parser_test.cc b/src/clsourcedependencies_parser_test.cc index eabfd144b8..1eee2e4af7 100644 --- a/src/clsourcedependencies_parser_test.cc +++ b/src/clsourcedependencies_parser_test.cc @@ -1,15 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); #ifdef _WIN32 +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + #if NINJA_ENABLE_CL_SOURCE_DEPENDENCIES #include "clsourcedependencies_parser.h" #include "test.h" -// Needed for GetACP() -#include - TEST(ParseCLSourceDependencies, ParseInvalidJSON) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies(StringPiece("this is not JSON"), &err, includes)); @@ -17,16 +28,16 @@ TEST(ParseCLSourceDependencies, ParseInvalidJSON) { } TEST(ParseCLSourceDependencies, ParseMissingVersion) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies(StringPiece("{}"), &err, includes)); ASSERT_EQ("sourceDependencies missing version", err); } TEST(ParseCLSourceDependencies, ParseVersionWrongType) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies(StringPiece("{\"Version\":1.0}"), &err, includes)); @@ -34,8 +45,8 @@ TEST(ParseCLSourceDependencies, ParseVersionWrongType) { } TEST(ParseCLSourceDependencies, ParseWrongVersion) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies(StringPiece("{\"Version\":\"2.0\"}"), &err, includes)); @@ -43,8 +54,8 @@ TEST(ParseCLSourceDependencies, ParseWrongVersion) { } TEST(ParseCLSourceDependencies, ParseMissingData) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies(StringPiece("{\"Version\":\"1.0\"}"), &err, includes)); @@ -52,8 +63,8 @@ TEST(ParseCLSourceDependencies, ParseMissingData) { } TEST(ParseCLSourceDependencies, ParseDataWrongType) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies( StringPiece("{\"Version\":\"1.0\",\"Data\":true}"), &err, includes)); @@ -61,8 +72,8 @@ TEST(ParseCLSourceDependencies, ParseDataWrongType) { } TEST(ParseCLSourceDependencies, ParseMissingIncludes) { - string err; - set includes; + std::string err; + std::set includes; ASSERT_FALSE(ParseCLSourceDependencies( StringPiece("{\"Version\":\"1.0\",\"Data\":{}}"), &err, includes)); @@ -70,8 +81,8 @@ TEST(ParseCLSourceDependencies, ParseMissingIncludes) { } TEST(ParseCLSourceDependencies, ParseIncludesWrongType) { - string err; - set includes; + std::string err; + std::set includes; StringPiece content("{\"Version\":\"1.0\",\"Data\":{\"Includes\":{}}}"); ASSERT_FALSE(ParseCLSourceDependencies(content, &err, includes)); @@ -79,8 +90,8 @@ TEST(ParseCLSourceDependencies, ParseIncludesWrongType) { } TEST(ParseCLSourceDependencies, ParseBadSingleInclude) { - string err; - set includes; + std::string err; + std::set includes; StringPiece content("{\"Version\":\"1.0\",\"Data\":{\"Includes\":[23]}}"); ASSERT_FALSE(ParseCLSourceDependencies(content, &err, includes)); @@ -88,8 +99,8 @@ TEST(ParseCLSourceDependencies, ParseBadSingleInclude) { } TEST(ParseCLSourceDependencies, ParseSimple) { - string err; - set includes; + std::string err; + std::set includes; StringPiece content( "{\"Version\":\"1.0\",\"Data\":{\"Includes\":[\"c:\\\\test.cpp\"]}}"); @@ -98,25 +109,6 @@ TEST(ParseCLSourceDependencies, ParseSimple) { ASSERT_EQ("c:\\test.cpp", *includes.begin()); } -TEST(ParseCLSourceDependencies, ParseNonAscii) { - // This test is only designed for Windows-1252 (English default). - if (GetACP() != 1252) - return; - - string content( - "{\"Version\":\"1.0\",\"Data\":{\"Includes\":[\"feelix\"]}}"); - content[39] = '\xC3'; - content[40] = '\xA9'; - - set includes; - string err; - ASSERT_TRUE(ParseCLSourceDependencies(content, &err, includes)); - - const char expected[] = { '\x66', '\xE9', '\x6C', '\x69', '\x78', '\x00' }; - ASSERT_EQ(1u, includes.size()); - ASSERT_EQ(expected, *includes.begin()); -} - TEST(ParseCLSourceDependencies, ParseReal) { StringPiece content( "{ \"Version\": \"1.0\", \"Data\": { \"Source\": " @@ -373,8 +365,8 @@ TEST(ParseCLSourceDependencies, ParseReal) { "\"c:\\\\constants.h\", \"c:\\\\test.h\" ], " "\"Modules\": [] }}"); - string err; - set includes; + std::string err; + std::set includes; ASSERT_TRUE(ParseCLSourceDependencies(content, &err, includes)); ASSERT_EQ(2u, includes.size()); diff --git a/src/util.cc b/src/util.cc index 3ec7ad0ed9..4df2bb23c1 100644 --- a/src/util.cc +++ b/src/util.cc @@ -449,70 +449,6 @@ void Win32Fatal(const char* function, const char* hint) { Fatal("%s: %s", function, GetLastErrorString().c_str()); } } - -/// In tests, codepage can be manually specified to override the ANSI codepage. -bool TryConvertUtf8ToAnsi(const char* utf8, string& ansi, - unsigned int codepage) { - // Avoid any issues with null pointers later on. - if (utf8 == NULL) - return false; - - // This size and all the sizes below do not count a null terminator. This lets - // us pass sizes directly to Windows and plays nicely with std::string. - int utf8Size = 0; - bool onlyAscii = true; - while (utf8[utf8Size] != 0) - onlyAscii &= (unsigned char)utf8[utf8Size++] <= 127; - - // Bail out early if the string is entirely ASCII, which encodes the same in - // UTF-8 and all ANSI code pages. This also handles the empty string case. - if (onlyAscii) { - ansi.assign(utf8); - return true; - } - - // First measure how big the UTF-16 buffer needs to be. - int wideSize = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, utf8, - utf8Size, NULL, 0); - if (wideSize == 0) - return false; - - // Then do the actual conversion to UTF-16. - wstring wideString(wideSize, L'\0'); - if (MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, utf8, utf8Size, - &wideString[0], wideSize) == 0) - return false; - - // The docs say that we can't pass any flags for these code pages. We'd like - // to pass WC_NO_BEST_FIT_CHARS to avoid dumb conversions like dropping accent - // marks or converting the pi symbol to p, but if we're not allowed to we'll - // just hope for the best. - if (codepage == 0) - codepage = GetACP(); - unsigned long flags = WC_NO_BEST_FIT_CHARS; - if (codepage == 50220 || codepage == 50221 || codepage == 50222 || - codepage == 50225 || codepage == 50227 || codepage == 50229 || - (57002 <= codepage && codepage <= 57011) || codepage == 65000 || - codepage == 42) - flags = 0; - - // Again, first measure how big the ANSI buffer needs to be. If we used any - // default characters it means that there were characters that don't exist in - // the target code page, so we can bail out early. - int usedDefaultChar = 1; - int ansiSize = WideCharToMultiByte(codepage, flags, wideString.data(), - wideSize, NULL, 0, NULL, &usedDefaultChar); - if (ansiSize == 0 || usedDefaultChar) - return false; - - // Finally convert to the specified code page. - ansi.resize(ansiSize); - if (WideCharToMultiByte(codepage, flags, wideString.data(), wideSize, - &ansi[0], ansiSize, NULL, NULL) == 0) - return false; - - return true; -} #endif bool islatinalpha(int c) { diff --git a/src/util.h b/src/util.h index 43d2ee64b1..6a4a7a9f84 100644 --- a/src/util.h +++ b/src/util.h @@ -120,12 +120,6 @@ string GetLastErrorString(); /// Calls Fatal() with a function name and GetLastErrorString. NORETURN void Win32Fatal(const char* function, const char* hint = NULL); - -/// Converts a UTF-8 encoded string into the current ANSI code page. Returns -/// true iff the conversion was successful. For testing, codepage can be -/// manually specified to override the ANSI codepage. -bool TryConvertUtf8ToAnsi(const char* utf8, string& ansi, - unsigned int codepage = 0); #endif #endif // NINJA_UTIL_H_ diff --git a/src/util_test.cc b/src/util_test.cc index ba0d8ea244..b43788d30b 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -432,82 +432,3 @@ TEST(ElideMiddle, ElideInTheMiddle) { EXPECT_EQ("012...789", elided); EXPECT_EQ("01234567...23456789", ElideMiddle(input, 19)); } - -#ifdef _WIN32 -TEST(TryConvertUtf8ToAnsi, ConvertNull) { - string ansi; - EXPECT_FALSE(TryConvertUtf8ToAnsi(NULL, ansi)); -} - -TEST(TryConvertUtf8ToAnsi, ConvertEmpty) { - string ansi; - EXPECT_TRUE(TryConvertUtf8ToAnsi("", ansi)); - EXPECT_EQ("", ansi); -} - -TEST(TryConvertUtf8ToAnsi, ConvertAscii) { - string ansi; - EXPECT_TRUE(TryConvertUtf8ToAnsi("Entirely ASCII", ansi)); - EXPECT_EQ("Entirely ASCII", ansi); -} - -TEST(TryConvertUtf8ToAnsi, ConvertInvalidUtf8) { - // 0xF9 can never appear in a UTF-8 string because it indicates the beginning - // of a code point larger than the Unicode limit of U+10FFFF. - const char utf8[] = { '\xF9', '\x00' }; - string ansi; - EXPECT_FALSE(TryConvertUtf8ToAnsi(utf8, ansi)); -} - -// Constants for UTF-8 conversion. - -// This is the shaka emoji. -const char SHAKA[] = { '\xF0', '\x9F', '\xA4', '\x99', '\x00' }; - -// This is felix with a forward accent on the e. -const char ACCENTED_E[] = { '\x66', '\xC3', '\xA9', '\x6C', - '\x69', '\x78', '\x00' }; - -// This is 3=lambda where lambda is the lowercase lambda symbol. -const char LAMBDA_SYMBOL[] = { '\x33', '\x3D', '\xCE', '\xBB', '\x00' }; - -// These tests check code page Windows-1252 (English default). - -TEST(TryConvertUtf8ToAnsi, ConvertShaka1252) { - string ansi; - EXPECT_FALSE(TryConvertUtf8ToAnsi(SHAKA, ansi, 1252)); -} - -TEST(TryConvertUtf8ToAnsi, ConvertAccentedE1252) { - string ansi; - EXPECT_TRUE(TryConvertUtf8ToAnsi(ACCENTED_E, ansi, 1252)); - - const char expected[] = { '\x66', '\xE9', '\x6C', '\x69', '\x78', '\x00' }; - EXPECT_EQ(expected, ansi); -} - -TEST(TryConvertUtf8ToAnsi, ConvertLambdaSymbol1252) { - string ansi; - EXPECT_FALSE(TryConvertUtf8ToAnsi(LAMBDA_SYMBOL, ansi, 1252)); -} - -// These tests check code page Windows-1253 (Greek default). - -TEST(TryConvertUtf8ToAnsi, ConvertShaka1253) { - string ansi; - EXPECT_FALSE(TryConvertUtf8ToAnsi(SHAKA, ansi, 1253)); -} - -TEST(TryConvertUtf8ToAnsi, ConvertAccentedE1253) { - string ansi; - EXPECT_FALSE(TryConvertUtf8ToAnsi(ACCENTED_E, ansi, 1253)); -} - -TEST(TryConvertUtf8ToAnsi, ConvertLambdaSymbol1253) { - string ansi; - EXPECT_TRUE(TryConvertUtf8ToAnsi(LAMBDA_SYMBOL, ansi, 1253)); - - const char expected[] = { '\x33', '\x3D', '\xEB', '\x00' }; - EXPECT_EQ(expected, ansi); -} -#endif