From a02132dac456875f395614bbc4cc2931a9ac409f Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Oct 2018 06:52:52 -0400 Subject: Re-generate depfile parser with re2cc 1.0.1 --- src/depfile_parser.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 345fa15..24e374f 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -1,4 +1,4 @@ -/* Generated by re2c 0.16 */ +/* Generated by re2c 1.1.1 */ // Copyright 2011 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -108,8 +108,7 @@ yy5: break; } yy6: - ++in; - yych = *in; + yych = *++in; if (yybm[0+yych] & 128) { goto yy6; } -- cgit v0.12 From 94f4153da9dd2ad0c166568f273e9ba2f4928554 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Oct 2018 09:01:59 -0400 Subject: Re-arrange depfile parser token processing logic Re-arrange the existing logic to support later addition of post-token code even for empty tokens. --- src/depfile_parser.cc | 21 ++++++++++----------- src/depfile_parser.in.cc | 21 ++++++++++----------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 24e374f..2ad2a00 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -175,22 +175,21 @@ yy15: } int len = (int)(out - filename); - const bool is_target = parsing_targets; + const bool is_dependency = !parsing_targets; if (len > 0 && filename[len - 1] == ':') { len--; // Strip off trailing colon, if any. parsing_targets = false; } - if (len == 0) - continue; - - if (!is_target) { - ins_.push_back(StringPiece(filename, len)); - } else if (!out_.str_) { - out_ = StringPiece(filename, len); - } else if (out_ != StringPiece(filename, len)) { - *err = "depfile has multiple output paths"; - return false; + if (len > 0) { + if (is_dependency) { + ins_.push_back(StringPiece(filename, len)); + } else if (!out_.str_) { + out_ = StringPiece(filename, len); + } else if (out_ != StringPiece(filename, len)) { + *err = "depfile has multiple output paths"; + return false; + } } } if (parsing_targets) { diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index 464efda..4df8ce2 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -94,22 +94,21 @@ bool DepfileParser::Parse(string* content, string* err) { } int len = (int)(out - filename); - const bool is_target = parsing_targets; + const bool is_dependency = !parsing_targets; if (len > 0 && filename[len - 1] == ':') { len--; // Strip off trailing colon, if any. parsing_targets = false; } - if (len == 0) - continue; - - if (!is_target) { - ins_.push_back(StringPiece(filename, len)); - } else if (!out_.str_) { - out_ = StringPiece(filename, len); - } else if (out_ != StringPiece(filename, len)) { - *err = "depfile has multiple output paths"; - return false; + if (len > 0) { + if (is_dependency) { + ins_.push_back(StringPiece(filename, len)); + } else if (!out_.str_) { + out_ = StringPiece(filename, len); + } else if (out_ != StringPiece(filename, len)) { + *err = "depfile has multiple output paths"; + return false; + } } } if (parsing_targets) { -- cgit v0.12 From 6d6dfd17e83bbde57bee61a0956a73b12088a9d1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 19 Oct 2015 14:16:29 -0400 Subject: Fix depfile parser test case line continuation Escape newlines for line continuation in the SpecialChars test case. Otherwise the test does not cover valid Makefile syntax. The missing line continuation was tolerated by our parser but is never (not) produced by a real compiler. --- src/depfile_parser_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index 824073f..e3eec07 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -119,10 +119,10 @@ TEST_F(DepfileParserTest, SpecialChars) { // https://github.com/google/libcxx/tree/master/test/iterators/stream.iterators/istreambuf.iterator/ string err; EXPECT_TRUE(Parse( -"C:/Program\\ Files\\ (x86)/Microsoft\\ crtdefs.h: \n" -" en@quot.header~ t+t-x!=1 \n" -" openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif\n" -" Fu\303\244ball\n" +"C:/Program\\ Files\\ (x86)/Microsoft\\ crtdefs.h: \\\n" +" en@quot.header~ t+t-x!=1 \\\n" +" openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif\\\n" +" Fu\303\244ball\\\n" " a\\[1\\]b@2%c", &err)); ASSERT_EQ("", err); -- cgit v0.12 From 4a4f9d40e178a9a9e88f4cd502d2be49bf7938d8 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 19 Oct 2015 14:25:03 -0400 Subject: Fix depfile parser handling of multiple rules Currently we handle Makefile rules of the form: out: in1 in2 in3 and the form: out: in1 \ in2 \ in3 Teach the depfile parser to handle the additional form: out: in1 out: in2 out: in3 This is also valid Makefile syntax and is the depfile format generated by the Intel Compiler for Windows. Note that the `gcc -MP` option adds empty phony rules to the generated Makefile fragment: out: in1 in2 in3 in1: in2: in3: Previously we tolerated these because they were treated as inputs, which was accidentally correct. Instead we must now tolerate these by ignoring targets for which no dependencies are specified. --- src/depfile_parser.cc | 104 +++++++++++++++++++++++++++----------- src/depfile_parser.in.cc | 31 ++++++++++-- src/depfile_parser_test.cc | 123 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 225 insertions(+), 33 deletions(-) diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 2ad2a00..7724eed 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -35,8 +35,11 @@ bool DepfileParser::Parse(string* content, string* err) { // parsing_targets: whether we are parsing targets or dependencies. char* in = &(*content)[0]; char* end = in + content->size(); + bool have_target = false; + bool have_secondary_target_on_this_rule = false; bool parsing_targets = true; while (in < end) { + bool have_newline = false; // out: current output point (typically same as in, but can fall behind // as we de-escape backslashes). char* out = in; @@ -45,6 +48,7 @@ bool DepfileParser::Parse(string* content, string* err) { for (;;) { // start: beginning of the current parsed span. const char* start = in; + char* yymarker = NULL; { unsigned char yych; @@ -84,17 +88,25 @@ bool DepfileParser::Parse(string* content, string* err) { }; yych = *in; if (yybm[0+yych] & 128) { - goto yy6; - } - if (yych <= '$') { - if (yych <= 0x00) goto yy2; - if (yych <= '#') goto yy4; goto yy9; + } + if (yych <= '\r') { + if (yych <= '\t') { + if (yych >= 0x01) goto yy4; + } else { + if (yych <= '\n') goto yy6; + if (yych <= '\f') goto yy4; + goto yy8; + } } else { - if (yych == '\\') goto yy10; - goto yy4; + if (yych <= '$') { + if (yych <= '#') goto yy4; + goto yy12; + } else { + if (yych == '\\') goto yy13; + goto yy4; + } } -yy2: ++in; { break; @@ -108,9 +120,20 @@ yy5: break; } yy6: + ++in; + { + // A newline ends the current file name and the current rule. + have_newline = true; + break; + } +yy8: + yych = *++in; + if (yych == '\n') goto yy6; + goto yy5; +yy9: yych = *++in; if (yybm[0+yych] & 128) { - goto yy6; + goto yy9; } { // Got a span of plain text. @@ -121,41 +144,41 @@ yy6: out += len; continue; } -yy9: +yy12: yych = *++in; - if (yych == '$') goto yy11; + if (yych == '$') goto yy14; goto yy5; -yy10: - yych = *++in; +yy13: + yych = *(yymarker = ++in); if (yych <= '"') { if (yych <= '\f') { if (yych <= 0x00) goto yy5; - if (yych == '\n') goto yy5; - goto yy13; + if (yych == '\n') goto yy18; + goto yy16; } else { - if (yych <= '\r') goto yy5; - if (yych == ' ') goto yy15; - goto yy13; + if (yych <= '\r') goto yy20; + if (yych == ' ') goto yy22; + goto yy16; } } else { if (yych <= 'Z') { - if (yych <= '#') goto yy15; - if (yych == '*') goto yy15; - goto yy13; + if (yych <= '#') goto yy22; + if (yych == '*') goto yy22; + goto yy16; } else { - if (yych <= ']') goto yy15; - if (yych == '|') goto yy15; - goto yy13; + if (yych <= ']') goto yy22; + if (yych == '|') goto yy22; + goto yy16; } } -yy11: +yy14: ++in; { // De-escape dollar character. *out++ = '$'; continue; } -yy13: +yy16: ++in; { // Let backslash before other characters through verbatim. @@ -163,7 +186,18 @@ yy13: *out++ = yych; continue; } -yy15: +yy18: + ++in; + { + // A line continuation ends the current file name. + break; + } +yy20: + yych = *++in; + if (yych == '\n') goto yy18; + in = yymarker; + goto yy5; +yy22: ++in; { // De-escape backslashed character. @@ -179,20 +213,30 @@ yy15: if (len > 0 && filename[len - 1] == ':') { len--; // Strip off trailing colon, if any. parsing_targets = false; + have_target = true; } if (len > 0) { if (is_dependency) { + if (have_secondary_target_on_this_rule) { + *err = "depfile has multiple output paths"; + return false; + } ins_.push_back(StringPiece(filename, len)); } else if (!out_.str_) { out_ = StringPiece(filename, len); } else if (out_ != StringPiece(filename, len)) { - *err = "depfile has multiple output paths"; - return false; + have_secondary_target_on_this_rule = true; } } + + if (have_newline) { + // A newline ends a rule so the next filename will be a new target. + parsing_targets = true; + have_secondary_target_on_this_rule = false; + } } - if (parsing_targets) { + if (!have_target) { *err = "expected ':' in depfile"; return false; } diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index 4df8ce2..d299ee2 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -34,8 +34,11 @@ bool DepfileParser::Parse(string* content, string* err) { // parsing_targets: whether we are parsing targets or dependencies. char* in = &(*content)[0]; char* end = in + content->size(); + bool have_target = false; + bool have_secondary_target_on_this_rule = false; bool parsing_targets = true; while (in < end) { + bool have_newline = false; // out: current output point (typically same as in, but can fall behind // as we de-escape backslashes). char* out = in; @@ -44,10 +47,12 @@ bool DepfileParser::Parse(string* content, string* err) { for (;;) { // start: beginning of the current parsed span. const char* start = in; + char* yymarker = NULL; /*!re2c re2c:define:YYCTYPE = "unsigned char"; re2c:define:YYCURSOR = in; re2c:define:YYLIMIT = end; + re2c:define:YYMARKER = yymarker; re2c:yyfill:enable = 0; @@ -56,6 +61,7 @@ bool DepfileParser::Parse(string* content, string* err) { nul = "\000"; escape = [ \\#*[|\]]; + newline = '\r'?'\n'; '\\' escape { // De-escape backslashed character. @@ -85,6 +91,15 @@ bool DepfileParser::Parse(string* content, string* err) { nul { break; } + '\\' newline { + // A line continuation ends the current file name. + break; + } + newline { + // A newline ends the current file name and the current rule. + have_newline = true; + break; + } [^] { // For any other character (e.g. whitespace), swallow it here, // allowing the outer logic to loop around again. @@ -98,20 +113,30 @@ bool DepfileParser::Parse(string* content, string* err) { if (len > 0 && filename[len - 1] == ':') { len--; // Strip off trailing colon, if any. parsing_targets = false; + have_target = true; } if (len > 0) { if (is_dependency) { + if (have_secondary_target_on_this_rule) { + *err = "depfile has multiple output paths"; + return false; + } ins_.push_back(StringPiece(filename, len)); } else if (!out_.str_) { out_ = StringPiece(filename, len); } else if (out_ != StringPiece(filename, len)) { - *err = "depfile has multiple output paths"; - return false; + have_secondary_target_on_this_rule = true; } } + + if (have_newline) { + // A newline ends a rule so the next filename will be a new target. + parsing_targets = true; + have_secondary_target_on_this_rule = false; + } } - if (parsing_targets) { + if (!have_target) { *err = "expected ':' in depfile"; return false; } diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index e3eec07..70e4029 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -158,3 +158,126 @@ TEST_F(DepfileParserTest, RejectMultipleDifferentOutputs) { EXPECT_FALSE(Parse("foo bar: x y z", &err)); ASSERT_EQ("depfile has multiple output paths", err); } + +TEST_F(DepfileParserTest, MultipleEmptyRules) { + string err; + EXPECT_TRUE(Parse("foo: x\n" + "foo: \n" + "foo:\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(1u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); +} + +TEST_F(DepfileParserTest, UnifyMultipleRulesLF) { + string err; + EXPECT_TRUE(Parse("foo: x\n" + "foo: y\n" + "foo \\\n" + "foo: z\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, UnifyMultipleRulesCRLF) { + string err; + EXPECT_TRUE(Parse("foo: x\r\n" + "foo: y\r\n" + "foo \\\r\n" + "foo: z\r\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, UnifyMixedRulesLF) { + string err; + EXPECT_TRUE(Parse("foo: x\\\n" + " y\n" + "foo \\\n" + "foo: z\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, UnifyMixedRulesCRLF) { + string err; + EXPECT_TRUE(Parse("foo: x\\\r\n" + " y\r\n" + "foo \\\r\n" + "foo: z\r\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, IndentedRulesLF) { + string err; + EXPECT_TRUE(Parse(" foo: x\n" + " foo: y\n" + " foo: z\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, IndentedRulesCRLF) { + string err; + EXPECT_TRUE(Parse(" foo: x\r\n" + " foo: y\r\n" + " foo: z\r\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, TolerateMP) { + string err; + EXPECT_TRUE(Parse("foo: x y z\n" + "x:\n" + "y:\n" + "z:\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, MultipleRulesTolerateMP) { + string err; + EXPECT_TRUE(Parse("foo: x\n" + "x:\n" + "foo: y\n" + "y:\n" + "foo: z\n" + "z:\n", &err)); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); + EXPECT_EQ("x", parser_.ins_[0].AsString()); + EXPECT_EQ("y", parser_.ins_[1].AsString()); + EXPECT_EQ("z", parser_.ins_[2].AsString()); +} + +TEST_F(DepfileParserTest, MultipleRulesRejectDifferentOutputs) { + // check that multiple different outputs are rejected by the parser + // when spread across multiple rules + string err; + EXPECT_FALSE(Parse("foo: x y\n" + "bar: y z\n", &err)); + ASSERT_EQ("depfile has multiple output paths", err); +} -- cgit v0.12 From 8a9edb110354d5468ab42685cfece6a073146f27 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 8 Oct 2018 09:14:58 -0400 Subject: Restore depfile toleration of multiple output paths on distinct lines Prior to introduction of depfile parser handling of multiple rules, ninja silently accepted a depfile of the form: out: in1 in2 in3 other: otherIn1 otherIn2 otherIn3 and incorrectly treated `other` and `otherIn*` as additional inputs to `out`. Now we prefer to reject this just as we already do for a depfile specifying multiple outputs on one line. However, this can break existing cases where such a depfile was silently tolerated. Add a `-w depfilemulti={err,warn}` option to control this behavior, and make it just a warning by default. --- src/build.cc | 5 +++-- src/build.h | 2 ++ src/depfile_parser.cc | 30 ++++++++++++++++++++++++++++-- src/depfile_parser.h | 17 +++++++++++++++++ src/depfile_parser.in.cc | 30 ++++++++++++++++++++++++++++-- src/depfile_parser_test.cc | 13 ++++++++++--- src/disk_interface_test.cc | 2 +- src/graph.cc | 4 +++- src/graph.h | 13 +++++++++---- src/graph_test.cc | 2 +- src/ninja.cc | 19 ++++++++++++++++++- 11 files changed, 120 insertions(+), 17 deletions(-) diff --git a/src/build.cc b/src/build.cc index ed219fd..b392803 100644 --- a/src/build.cc +++ b/src/build.cc @@ -557,7 +557,8 @@ Builder::Builder(State* state, const BuildConfig& config, BuildLog* build_log, DepsLog* deps_log, DiskInterface* disk_interface) : state_(state), config_(config), disk_interface_(disk_interface), - scan_(state, build_log, deps_log, disk_interface) { + scan_(state, build_log, deps_log, disk_interface, + &config_.depfile_parser_options) { status_ = new BuildStatus(config); } @@ -906,7 +907,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, if (content.empty()) return true; - DepfileParser deps; + DepfileParser deps(config_.depfile_parser_options); if (!deps.Parse(&content, err)) return false; diff --git a/src/build.h b/src/build.h index 9b90e8a..a42b8d4 100644 --- a/src/build.h +++ b/src/build.h @@ -23,6 +23,7 @@ #include #include +#include "depfile_parser.h" #include "graph.h" // XXX needed for DependencyScan; should rearrange. #include "exit_status.h" #include "line_printer.h" @@ -151,6 +152,7 @@ struct BuildConfig { /// The maximum load average we must not exceed. A negative value /// means that we do not have any limit. double max_load_average; + DepfileParserOptions depfile_parser_options; }; /// Builder wraps the build process: starting commands, updating status. diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 7724eed..405289f 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -14,6 +14,12 @@ // limitations under the License. #include "depfile_parser.h" +#include "util.h" + +DepfileParser::DepfileParser(DepfileParserOptions options) + : options_(options) +{ +} // A note on backslashes in Makefiles, from reading the docs: // Backslash-newline is the line continuation character. @@ -37,6 +43,8 @@ bool DepfileParser::Parse(string* content, string* err) { char* end = in + content->size(); bool have_target = false; bool have_secondary_target_on_this_rule = false; + bool have_newline_since_primary_target = false; + bool warned_distinct_target_lines = false; bool parsing_targets = true; while (in < end) { bool have_newline = false; @@ -219,8 +227,23 @@ yy22: if (len > 0) { if (is_dependency) { if (have_secondary_target_on_this_rule) { - *err = "depfile has multiple output paths"; - return false; + if (!have_newline_since_primary_target) { + *err = "depfile has multiple output paths"; + return false; + } else if (options_.depfile_distinct_target_lines_action_ == + kDepfileDistinctTargetLinesActionError) { + *err = + "depfile has multiple output paths (on separate lines)" + " [-w depfilemulti=err]"; + return false; + } else { + if (!warned_distinct_target_lines) { + warned_distinct_target_lines = true; + Warning("depfile has multiple output paths (on separate lines); " + "continuing anyway [-w depfilemulti=warn]"); + } + continue; + } } ins_.push_back(StringPiece(filename, len)); } else if (!out_.str_) { @@ -234,6 +257,9 @@ yy22: // A newline ends a rule so the next filename will be a new target. parsing_targets = true; have_secondary_target_on_this_rule = false; + if (have_target) { + have_newline_since_primary_target = true; + } } } if (!have_target) { diff --git a/src/depfile_parser.h b/src/depfile_parser.h index 1e6ebb5..be20374 100644 --- a/src/depfile_parser.h +++ b/src/depfile_parser.h @@ -21,8 +21,24 @@ using namespace std; #include "string_piece.h" +enum DepfileDistinctTargetLinesAction { + kDepfileDistinctTargetLinesActionWarn, + kDepfileDistinctTargetLinesActionError, +}; + +struct DepfileParserOptions { + DepfileParserOptions() + : depfile_distinct_target_lines_action_( + kDepfileDistinctTargetLinesActionWarn) {} + DepfileDistinctTargetLinesAction + depfile_distinct_target_lines_action_; +}; + /// Parser for the dependency information emitted by gcc's -M flags. struct DepfileParser { + explicit DepfileParser(DepfileParserOptions options = + DepfileParserOptions()); + /// Parse an input file. Input must be NUL-terminated. /// Warning: may mutate the content in-place and parsed StringPieces are /// pointers within it. @@ -30,6 +46,7 @@ struct DepfileParser { StringPiece out_; vector ins_; + DepfileParserOptions options_; }; #endif // NINJA_DEPFILE_PARSER_H_ diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index d299ee2..f8c94b3 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -13,6 +13,12 @@ // limitations under the License. #include "depfile_parser.h" +#include "util.h" + +DepfileParser::DepfileParser(DepfileParserOptions options) + : options_(options) +{ +} // A note on backslashes in Makefiles, from reading the docs: // Backslash-newline is the line continuation character. @@ -36,6 +42,8 @@ bool DepfileParser::Parse(string* content, string* err) { char* end = in + content->size(); bool have_target = false; bool have_secondary_target_on_this_rule = false; + bool have_newline_since_primary_target = false; + bool warned_distinct_target_lines = false; bool parsing_targets = true; while (in < end) { bool have_newline = false; @@ -119,8 +127,23 @@ bool DepfileParser::Parse(string* content, string* err) { if (len > 0) { if (is_dependency) { if (have_secondary_target_on_this_rule) { - *err = "depfile has multiple output paths"; - return false; + if (!have_newline_since_primary_target) { + *err = "depfile has multiple output paths"; + return false; + } else if (options_.depfile_distinct_target_lines_action_ == + kDepfileDistinctTargetLinesActionError) { + *err = + "depfile has multiple output paths (on separate lines)" + " [-w depfilemulti=err]"; + return false; + } else { + if (!warned_distinct_target_lines) { + warned_distinct_target_lines = true; + Warning("depfile has multiple output paths (on separate lines); " + "continuing anyway [-w depfilemulti=warn]"); + } + continue; + } } ins_.push_back(StringPiece(filename, len)); } else if (!out_.str_) { @@ -134,6 +157,9 @@ bool DepfileParser::Parse(string* content, string* err) { // A newline ends a rule so the next filename will be a new target. parsing_targets = true; have_secondary_target_on_this_rule = false; + if (have_target) { + have_newline_since_primary_target = true; + } } } if (!have_target) { diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index 70e4029..52fe7cd 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -276,8 +276,15 @@ TEST_F(DepfileParserTest, MultipleRulesTolerateMP) { TEST_F(DepfileParserTest, MultipleRulesRejectDifferentOutputs) { // check that multiple different outputs are rejected by the parser // when spread across multiple rules + DepfileParserOptions parser_opts; + parser_opts.depfile_distinct_target_lines_action_ = + kDepfileDistinctTargetLinesActionError; + DepfileParser parser(parser_opts); string err; - EXPECT_FALSE(Parse("foo: x y\n" - "bar: y z\n", &err)); - ASSERT_EQ("depfile has multiple output paths", err); + string input = + "foo: x y\n" + "bar: y z\n"; + EXPECT_FALSE(parser.Parse(&input, &err)); + ASSERT_EQ("depfile has multiple output paths (on separate lines)" + " [-w depfilemulti=err]", err); } diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 2de4f28..bac515d 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -213,7 +213,7 @@ TEST_F(DiskInterfaceTest, RemoveFile) { struct StatTest : public StateTestWithBuiltinRules, public DiskInterface { - StatTest() : scan_(&state_, NULL, NULL, this) {} + StatTest() : scan_(&state_, NULL, NULL, this, NULL) {} // DiskInterface implementation. virtual TimeStamp Stat(const string& path, string* err) const; diff --git a/src/graph.cc b/src/graph.cc index b41c247..9c2f784 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -491,7 +491,9 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return false; } - DepfileParser depfile; + DepfileParser depfile(depfile_parser_options_ + ? *depfile_parser_options_ + : DepfileParserOptions()); string depfile_err; if (!depfile.Parse(&content, &depfile_err)) { *err = path + ": " + depfile_err; diff --git a/src/graph.h b/src/graph.h index a8f0641..d58fecd 100644 --- a/src/graph.h +++ b/src/graph.h @@ -24,6 +24,7 @@ using namespace std; #include "util.h" struct BuildLog; +struct DepfileParserOptions; struct DiskInterface; struct DepsLog; struct Edge; @@ -209,8 +210,10 @@ struct Edge { /// "depfile" attribute in build files. struct ImplicitDepLoader { ImplicitDepLoader(State* state, DepsLog* deps_log, - DiskInterface* disk_interface) - : state_(state), disk_interface_(disk_interface), deps_log_(deps_log) {} + DiskInterface* disk_interface, + DepfileParserOptions const* depfile_parser_options) + : state_(state), disk_interface_(disk_interface), deps_log_(deps_log), + depfile_parser_options_(depfile_parser_options) {} /// Load implicit dependencies for \a edge. /// @return false on error (without filling \a err if info is just missing @@ -242,6 +245,7 @@ struct ImplicitDepLoader { State* state_; DiskInterface* disk_interface_; DepsLog* deps_log_; + DepfileParserOptions const* depfile_parser_options_; }; @@ -249,10 +253,11 @@ struct ImplicitDepLoader { /// and updating the dirty/outputs_ready state of all the nodes and edges. struct DependencyScan { DependencyScan(State* state, BuildLog* build_log, DepsLog* deps_log, - DiskInterface* disk_interface) + DiskInterface* disk_interface, + DepfileParserOptions const* depfile_parser_options) : build_log_(build_log), disk_interface_(disk_interface), - dep_loader_(state, deps_log, disk_interface) {} + dep_loader_(state, deps_log, disk_interface, depfile_parser_options) {} /// Update the |dirty_| state of the given node by inspecting its input edge. /// Examine inputs, outputs, and command lines to judge whether an edge diff --git a/src/graph_test.cc b/src/graph_test.cc index 422bc9a..4a66831 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -18,7 +18,7 @@ #include "test.h" struct GraphTest : public StateTestWithBuiltinRules { - GraphTest() : scan_(&state_, NULL, NULL, &fs_) {} + GraphTest() : scan_(&state_, NULL, NULL, &fs_, NULL) {} VirtualFileSystem fs_; DependencyScan scan_; diff --git a/src/ninja.cc b/src/ninja.cc index a3d73ad..b608426 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -73,6 +73,10 @@ struct Options { /// Whether phony cycles should warn or print an error. bool phony_cycle_should_err; + + /// Whether a depfile with multiple targets on separate lines should + /// warn or print an error. + bool depfile_distinct_target_lines_should_err; }; /// The Ninja main() loads up a series of data structures; various tools need @@ -912,7 +916,9 @@ bool WarningEnable(const string& name, Options* options) { if (name == "list") { printf("warning flags:\n" " dupbuild={err,warn} multiple build lines for one target\n" -" phonycycle={err,warn} phony build statement references itself\n"); +" phonycycle={err,warn} phony build statement references itself\n" +" depfilemulti={err,warn} depfile has multiple output paths on separate lines\n" + ); return false; } else if (name == "dupbuild=err") { options->dupe_edges_should_err = true; @@ -926,6 +932,12 @@ bool WarningEnable(const string& name, Options* options) { } else if (name == "phonycycle=warn") { options->phony_cycle_should_err = false; return true; + } else if (name == "depfilemulti=err") { + options->depfile_distinct_target_lines_should_err = true; + return true; + } else if (name == "depfilemulti=warn") { + options->depfile_distinct_target_lines_should_err = false; + return true; } else { const char* suggestion = SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn", @@ -1200,6 +1212,11 @@ NORETURN void real_main(int argc, char** argv) { if (exit_code >= 0) exit(exit_code); + if (options.depfile_distinct_target_lines_should_err) { + config.depfile_parser_options.depfile_distinct_target_lines_action_ = + kDepfileDistinctTargetLinesActionError; + } + if (options.working_dir) { // The formatting of this string, complete with funny quotes, is // so Emacs can properly identify that the cwd has changed for -- cgit v0.12