From 70d43e48e8b8a71c1f1cc55718b34480f68e1341 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 6 Jan 2013 11:36:17 -0800 Subject: factor MSVC parsing out of CLWrapper into CLParser --- src/msvc_helper-win32.cc | 94 +++++++++++++++----------------- src/msvc_helper.h | 42 ++++++++------- src/msvc_helper_main-win32.cc | 26 ++++----- src/msvc_helper_test.cc | 123 ++++++++++++++++++++---------------------- 4 files changed, 139 insertions(+), 146 deletions(-) diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index fd9b671..be2a5e0 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -39,15 +39,15 @@ string Replace(const string& input, const string& find, const string& replace) { return result; } +} // anonymous namespace + string EscapeForDepfile(const string& path) { // Depfiles don't escape single \. return Replace(path, " ", "\\ "); } -} // anonymous namespace - // static -string CLWrapper::FilterShowIncludes(const string& line) { +string CLParser::FilterShowIncludes(const string& line) { static const char kMagicPrefix[] = "Note: including file: "; const char* in = line.c_str(); const char* end = in + line.size(); @@ -63,14 +63,14 @@ string CLWrapper::FilterShowIncludes(const string& line) { } // static -bool CLWrapper::IsSystemInclude(const string& path) { +bool CLParser::IsSystemInclude(const string& path) { // 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 CLWrapper::FilterInputFilename(const string& line) { +bool CLParser::FilterInputFilename(const string& line) { // TODO: other extensions, like .asm? return EndsWith(line, ".c") || EndsWith(line, ".cc") || @@ -78,7 +78,42 @@ bool CLWrapper::FilterInputFilename(const string& line) { EndsWith(line, ".cpp"); } -int CLWrapper::Run(const string& command, string* extra_output) { +string CLParser::Parse(const string& output) { + string filtered_output; + + // Loop over all lines in the output to process them. + size_t start = 0; + while (start < output.size()) { + size_t end = output.find_first_of("\r\n", start); + if (end == string::npos) + end = output.size(); + string line = output.substr(start, end - start); + + string include = FilterShowIncludes(line); + if (!include.empty()) { + include = IncludesNormalize::Normalize(include, NULL); + if (!IsSystemInclude(include)) + includes_.insert(include); + } else if (FilterInputFilename(line)) { + // Drop it. + // TODO: if we support compiling multiple output files in a single + // cl.exe invocation, we should stash the filename. + } else { + filtered_output.append(line); + filtered_output.append("\n"); + } + + if (end < output.size() && output[end] == '\r') + ++end; + if (end < output.size() && output[end] == '\n') + ++end; + start = end; + } + + return filtered_output; +} + +int CLWrapper::Run(const string& command, string* output) { SECURITY_ATTRIBUTES security_attributes = {}; security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); security_attributes.bInheritHandle = TRUE; @@ -118,8 +153,7 @@ int CLWrapper::Run(const string& command, string* extra_output) { Win32Fatal("CloseHandle"); } - // Read output of the subprocess and parse it. - string output; + // Read all output of the subprocess. DWORD read_len = 1; while (read_len) { char buf[64 << 10]; @@ -128,44 +162,12 @@ int CLWrapper::Run(const string& command, string* extra_output) { GetLastError() != ERROR_BROKEN_PIPE) { Win32Fatal("ReadFile"); } - output.append(buf, read_len); - - // Loop over all lines in the output and process them. - for (;;) { - size_t ofs = output.find_first_of("\r\n"); - if (ofs == string::npos) - break; - string line = output.substr(0, ofs); - - string include = FilterShowIncludes(line); - if (!include.empty()) { - include = IncludesNormalize::Normalize(include, NULL); - if (!IsSystemInclude(include)) - includes_.insert(include); - } else if (FilterInputFilename(line)) { - // Drop it. - // TODO: if we support compiling multiple output files in a single - // cl.exe invocation, we should stash the filename. - } else { - if (extra_output) { - extra_output->append(line); - extra_output->append("\n"); - } else { - printf("%s\n", line.c_str()); - } - } - - if (ofs < output.size() && output[ofs] == '\r') - ++ofs; - if (ofs < output.size() && output[ofs] == '\n') - ++ofs; - output = output.substr(ofs); - } + output->append(buf, read_len); } + // Wait for it to exit and grab its exit code. if (WaitForSingleObject(process_info.hProcess, INFINITE) == WAIT_FAILED) Win32Fatal("WaitForSingleObject"); - DWORD exit_code = 0; if (!GetExitCodeProcess(process_info.hProcess, &exit_code)) Win32Fatal("GetExitCodeProcess"); @@ -178,11 +180,3 @@ int CLWrapper::Run(const string& command, string* extra_output) { return exit_code; } - -vector CLWrapper::GetEscapedResult() { - vector result; - for (set::iterator i = includes_.begin(); i != includes_.end(); ++i) { - result.push_back(EscapeForDepfile(*i)); - } - return result; -} diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 102201b..32ab606 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -17,23 +17,13 @@ #include using namespace std; +string EscapeForDepfile(const string& path); + /// Visual Studio's cl.exe requires some massaging to work with Ninja; /// for example, it emits include information on stderr in a funny -/// format when building with /showIncludes. This class wraps a CL -/// process and parses that output to extract the file list. -struct CLWrapper { - CLWrapper() : env_block_(NULL) {} - - /// Set the environment block (as suitable for CreateProcess) to be used - /// by Run(). - void SetEnvBlock(void* env_block) { env_block_ = env_block; } - - /// Start a process and parse its output. Returns its exit code. - /// Any non-parsed output is buffered into \a extra_output if provided, - /// otherwise it is printed to stdout while the process runs. - /// Crashes (calls Fatal()) on error. - int Run(const string& command, string* extra_output=NULL); - +/// format when building with /showIncludes. This class parses this +/// output. +struct CLParser { /// Parse a line of cl.exe output and extract /showIncludes info. /// If a dependency is extracted, returns a nonempty string. /// Exposed for testing. @@ -50,10 +40,24 @@ struct CLWrapper { /// Exposed for testing. static bool FilterInputFilename(const string& line); - /// Fill a vector with the unique'd headers, escaped for output as a .d - /// file. - vector GetEscapedResult(); + /// Parse the full output of cl, returning the output (if any) that + /// should printed. + string Parse(const string& output); - void* env_block_; set includes_; }; + +/// Wraps a synchronous execution of a CL subprocess. +struct CLWrapper { + CLWrapper() : env_block_(NULL) {} + + /// Set the environment block (as suitable for CreateProcess) to be used + /// by Run(). + void SetEnvBlock(void* env_block) { env_block_ = env_block; } + + /// Start a process and gather its raw output. Returns its exit code. + /// Crashes (calls Fatal()) on error. + int Run(const string& command, string* output); + + void* env_block_; +}; diff --git a/src/msvc_helper_main-win32.cc b/src/msvc_helper_main-win32.cc index 0bbe98b..3192821 100644 --- a/src/msvc_helper_main-win32.cc +++ b/src/msvc_helper_main-win32.cc @@ -44,12 +44,13 @@ void PushPathIntoEnvironment(const string& env_block) { } } -void WriteDepFileOrDie(const char* object_path, CLWrapper* cl) { +void WriteDepFileOrDie(const char* object_path, const CLParser& parse) { string depfile_path = string(object_path) + ".d"; FILE* depfile = fopen(depfile_path.c_str(), "w"); if (!depfile) { unlink(object_path); - Fatal("opening %s: %s", depfile_path.c_str(), GetLastErrorString().c_str()); + Fatal("opening %s: %s", depfile_path.c_str(), + GetLastErrorString().c_str()); } if (fprintf(depfile, "%s: ", object_path) < 0) { unlink(object_path); @@ -57,9 +58,9 @@ void WriteDepFileOrDie(const char* object_path, CLWrapper* cl) { unlink(depfile_path.c_str()); Fatal("writing %s", depfile_path.c_str()); } - vector headers = cl->GetEscapedResult(); - for (vector::iterator i = headers.begin(); i != headers.end(); ++i) { - if (fprintf(depfile, "%s\n", i->c_str()) < 0) { + const set& headers = parse.includes_; + for (set::iterator i = headers.begin(); i != headers.end(); ++i) { + if (fprintf(depfile, "%s\n", EscapeForDepfile(*i).c_str()) < 0) { unlink(object_path); fclose(depfile); unlink(depfile_path.c_str()); @@ -95,11 +96,6 @@ int MSVCHelperMain(int argc, char** argv) { } } - if (!output_filename) { - Usage(); - Fatal("-o required"); - } - string env; if (envfile) { string err; @@ -118,9 +114,15 @@ int MSVCHelperMain(int argc, char** argv) { CLWrapper cl; if (!env.empty()) cl.SetEnvBlock((void*)env.data()); - int exit_code = cl.Run(command); + string output; + int exit_code = cl.Run(command, &output); - WriteDepFileOrDie(output_filename, &cl); + if (output_filename) { + CLParser parser; + output = parser.Parse(output); + WriteDepFileOrDie(output_filename, parser); + } + printf("%s\n", output.c_str()); return exit_code; } diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 7730425..1e1cbde 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -19,100 +19,93 @@ #include "test.h" #include "util.h" -TEST(MSVCHelperTest, ShowIncludes) { - ASSERT_EQ("", CLWrapper::FilterShowIncludes("")); +TEST(CLParserTest, ShowIncludes) { + ASSERT_EQ("", CLParser::FilterShowIncludes("")); - ASSERT_EQ("", CLWrapper::FilterShowIncludes("Sample compiler output")); + ASSERT_EQ("", CLParser::FilterShowIncludes("Sample compiler output")); ASSERT_EQ("c:\\Some Files\\foobar.h", - CLWrapper::FilterShowIncludes("Note: including file: " - "c:\\Some Files\\foobar.h")); + CLParser::FilterShowIncludes("Note: including file: " + "c:\\Some Files\\foobar.h")); ASSERT_EQ("c:\\initspaces.h", - CLWrapper::FilterShowIncludes("Note: including file: " - "c:\\initspaces.h")); + CLParser::FilterShowIncludes("Note: including file: " + "c:\\initspaces.h")); } -TEST(MSVCHelperTest, FilterInputFilename) { - ASSERT_TRUE(CLWrapper::FilterInputFilename("foobar.cc")); - ASSERT_TRUE(CLWrapper::FilterInputFilename("foo bar.cc")); - ASSERT_TRUE(CLWrapper::FilterInputFilename("baz.c")); +TEST(CLParserTest, FilterInputFilename) { + ASSERT_TRUE(CLParser::FilterInputFilename("foobar.cc")); + ASSERT_TRUE(CLParser::FilterInputFilename("foo bar.cc")); + ASSERT_TRUE(CLParser::FilterInputFilename("baz.c")); - ASSERT_FALSE(CLWrapper::FilterInputFilename( + ASSERT_FALSE(CLParser::FilterInputFilename( "src\\cl_helper.cc(166) : fatal error C1075: end " "of file found ...")); } -TEST(MSVCHelperTest, Run) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo foo&& echo Note: including file: foo.h&&echo bar\"", - &output); +TEST(CLParserTest, ParseSimple) { + CLParser parser; + string output = parser.Parse( + "foo\r\n" + "Note: including file: foo.h\r\n" + "bar\r\n"); + ASSERT_EQ("foo\nbar\n", output); - ASSERT_EQ(1u, cl.includes_.size()); - ASSERT_EQ("foo.h", *cl.includes_.begin()); + ASSERT_EQ(1u, parser.includes_.size()); + ASSERT_EQ("foo.h", *parser.includes_.begin()); } -TEST(MSVCHelperTest, RunFilenameFilter) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo foo.cc&& echo cl: warning\"", - &output); +TEST(CLParserTest, ParseFilenameFilter) { + CLParser parser; + string output = parser.Parse( + "foo.cc\r\n" + "cl: warning\r\n"); ASSERT_EQ("cl: warning\n", output); } -TEST(MSVCHelperTest, RunSystemInclude) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo Note: including file: c:\\Program Files\\foo.h&&" - "echo Note: including file: d:\\Microsoft Visual Studio\\bar.h&&" - "echo Note: including file: path.h\"", - &output); +TEST(CLParserTest, ParseSystemInclude) { + CLParser parser; + string output = parser.Parse( + "Note: including file: c:\\Program Files\\foo.h\r\n" + "Note: including file: d:\\Microsoft Visual Studio\\bar.h\r\n" + "Note: including file: path.h\r\n"); // We should have dropped the first two includes because they look like // system headers. ASSERT_EQ("", output); - ASSERT_EQ(1u, cl.includes_.size()); - ASSERT_EQ("path.h", *cl.includes_.begin()); -} - -TEST(MSVCHelperTest, EnvBlock) { - char env_block[] = "foo=bar\0"; - CLWrapper cl; - cl.SetEnvBlock(env_block); - string output; - cl.Run("cmd /c \"echo foo is %foo%", &output); - ASSERT_EQ("foo is bar\n", output); + ASSERT_EQ(1u, parser.includes_.size()); + ASSERT_EQ("path.h", *parser.includes_.begin()); } -TEST(MSVCHelperTest, DuplicatedHeader) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo Note: including file: foo.h&&" - "echo Note: including file: bar.h&&" - "echo Note: including file: foo.h\"", - &output); +TEST(CLParserTest, DuplicatedHeader) { + CLParser parser; + string output = parser.Parse( + "Note: including file: foo.h\r\n" + "Note: including file: bar.h\r\n" + "Note: including file: foo.h\r\n"); // We should have dropped one copy of foo.h. ASSERT_EQ("", output); - ASSERT_EQ(2u, cl.includes_.size()); + ASSERT_EQ(2u, parser.includes_.size()); } -TEST(MSVCHelperTest, DuplicatedHeaderPathConverted) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo Note: including file: sub/foo.h&&" - "echo Note: including file: bar.h&&" - "echo Note: including file: sub\\foo.h\"", - &output); +TEST(CLParserTest, DuplicatedHeaderPathConverted) { + CLParser parser; + string output = parser.Parse( + "Note: including file: sub/foo.h\r\n" + "Note: including file: bar.h\r\n" + "Note: including file: sub\\foo.h\r\n"); // We should have dropped one copy of foo.h. ASSERT_EQ("", output); - ASSERT_EQ(2u, cl.includes_.size()); + ASSERT_EQ(2u, parser.includes_.size()); } -TEST(MSVCHelperTest, SpacesInFilename) { +TEST(CLParserTest, SpacesInFilename) { + ASSERT_EQ("sub\\some\\ sdk\\foo.h", + EscapeForDepfile("sub\\some sdk\\foo.h")); +} + +TEST(MSVCHelperTest, EnvBlock) { + char env_block[] = "foo=bar\0"; CLWrapper cl; + cl.SetEnvBlock(env_block); string output; - cl.Run("cmd /c \"echo Note: including file: sub\\some sdk\\foo.h", - &output); - ASSERT_EQ("", output); - vector headers = cl.GetEscapedResult(); - ASSERT_EQ(1u, headers.size()); - ASSERT_EQ("sub\\some\\ sdk\\foo.h", headers[0]); + cl.Run("cmd /c \"echo foo is %foo%", &output); + ASSERT_EQ("foo is bar\r\n", output); } -- cgit v0.12