propagate include normalization failure to caller instead

5 files changed, 30 insertions, 25 deletions

diff --git a/src/build.cc b/src/build.cc
index cdb8e0a..fcde626 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -849,7 +849,8 @@ bool Builder::ExtractDeps(CommandRunner::Result* result,
 #ifdef _WIN32
   if (deps_type == "msvc") {
     CLParser parser;
-    result->output = parser.Parse(result->output, deps_prefix);
+    if (!parser.Parse(result->output, deps_prefix, &result->output, err))
+      return false;
     for (set<string>::iterator i = parser.includes_.begin();
          i != parser.includes_.end(); ++i) {
       // ~0 is assuming that with MSVC-parsed headers, it's ok to always make
diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc
index a52dd38..78b79b0 100644
--- a/src/msvc_helper-win32.cc
+++ b/src/msvc_helper-win32.cc
@@ -82,9 +82,8 @@ bool CLParser::FilterInputFilename(string line) {
       EndsWith(line, ".cpp");
 }
 
-string CLParser::Parse(const string& output, const string& deps_prefix) {
-  string filtered_output;
-
+bool CLParser::Parse(const string& output, const string& deps_prefix,
+                     string* filtered_output, string* err) {
   // Loop over all lines in the output to process them.
   size_t start = 0;
   while (start < output.size()) {
@@ -96,11 +95,8 @@ string CLParser::Parse(const string& output, const string& deps_prefix) {
     string include = FilterShowIncludes(line, deps_prefix);
     if (!include.empty()) {
       string normalized;
-      string err;
-      if (!IncludesNormalize::Normalize(include, NULL, &normalized, &err)) {
-        Fatal("failed to normalize path: %s: %s\n", include.c_str(),
-              err.c_str());
-      }
+      if (!IncludesNormalize::Normalize(include, NULL, &normalized, err))
+        return false;
       if (!IsSystemInclude(normalized))
         includes_.insert(normalized);
     } else if (FilterInputFilename(line)) {
@@ -108,8 +104,8 @@ string CLParser::Parse(const string& output, const string& deps_prefix) {
       // 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");
+      filtered_output->append(line);
+      filtered_output->append("\n");
     }
 
     if (end < output.size() && output[end] == '\r')
@@ -119,7 +115,7 @@ string CLParser::Parse(const string& output, const string& deps_prefix) {
     start = end;
   }
 
-  return filtered_output;
+  return true;
 }
 
 int CLWrapper::Run(const string& command, string* output) {
diff --git a/src/msvc_helper.h b/src/msvc_helper.h
index 5d7dcb0..08c0ad5 100644
--- a/src/msvc_helper.h
+++ b/src/msvc_helper.h
@@ -41,8 +41,9 @@ struct CLParser {
   static bool FilterInputFilename(string line);
 
   /// Parse the full output of cl, returning the output (if any) that
-  /// should printed.
-  string Parse(const string& output, const string& deps_prefix);
+  /// should printed. Returns true on success, or false with err filled.
+  bool Parse(const string& output, const string& deps_prefix,
+             string* filtered_output, string* err);
 
   set<string> includes_;
 };
diff --git a/src/msvc_helper_main-win32.cc b/src/msvc_helper_main-win32.cc
index 58bc797..680aaad 100644
--- a/src/msvc_helper_main-win32.cc
+++ b/src/msvc_helper_main-win32.cc
@@ -127,7 +127,9 @@ int MSVCHelperMain(int argc, char** argv) {
 
   if (output_filename) {
     CLParser parser;
-    output = parser.Parse(output, deps_prefix);
+    string err;
+    if (!parser.Parse(output, deps_prefix, &output, &err))
+      Fatal("%s\n", err.c_str());
     WriteDepFileOrDie(output_filename, parser);
   }
 
diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc
index 29db650..49d27c1 100644
--- a/src/msvc_helper_test.cc
+++ b/src/msvc_helper_test.cc
@@ -46,11 +46,12 @@ TEST(CLParserTest, FilterInputFilename) {
 
 TEST(CLParserTest, ParseSimple) {
   CLParser parser;
-  string output = parser.Parse(
+  string output, err;
+  ASSERT_TRUE(parser.Parse(
       "foo\r\n"
       "Note: inc file prefix:  foo.h\r\n"
       "bar\r\n",
-      "Note: inc file prefix:");
+      "Note: inc file prefix:", &output, &err));
 
   ASSERT_EQ("foo\nbar\n", output);
   ASSERT_EQ(1u, parser.includes_.size());
@@ -59,20 +60,22 @@ TEST(CLParserTest, ParseSimple) {
 
 TEST(CLParserTest, ParseFilenameFilter) {
   CLParser parser;
-  string output = parser.Parse(
+  string output, err;
+  ASSERT_TRUE(parser.Parse(
       "foo.cc\r\n"
       "cl: warning\r\n",
-      "");
+      "", &output, &err));
   ASSERT_EQ("cl: warning\n", output);
 }
 
 TEST(CLParserTest, ParseSystemInclude) {
   CLParser parser;
-  string output = parser.Parse(
+  string output, err;
+  ASSERT_TRUE(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",
-      "");
+      "", &output, &err));
   // We should have dropped the first two includes because they look like
   // system headers.
   ASSERT_EQ("", output);
@@ -82,11 +85,12 @@ TEST(CLParserTest, ParseSystemInclude) {
 
 TEST(CLParserTest, DuplicatedHeader) {
   CLParser parser;
-  string output = parser.Parse(
+  string output, err;
+  ASSERT_TRUE(parser.Parse(
       "Note: including file: foo.h\r\n"
       "Note: including file: bar.h\r\n"
       "Note: including file: foo.h\r\n",
-      "");
+      "", &output, &err));
   // We should have dropped one copy of foo.h.
   ASSERT_EQ("", output);
   ASSERT_EQ(2u, parser.includes_.size());
@@ -94,11 +98,12 @@ TEST(CLParserTest, DuplicatedHeader) {
 
 TEST(CLParserTest, DuplicatedHeaderPathConverted) {
   CLParser parser;
-  string output = parser.Parse(
+  string output, err;
+  ASSERT_TRUE(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",
-      "");
+      "", &output, &err));
   // We should have dropped one copy of foo.h.
   ASSERT_EQ("", output);
   ASSERT_EQ(2u, parser.includes_.size());