Merge pull request #1534 from mathstuf/remove-depslog-restriction
manifest_parser: remove multi-output depslog restriction
diff --git a/src/build.cc b/src/build.cc
index 75671a6..cd8df4e 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -1033,14 +1033,16 @@
}
if (!deps_type.empty() && !config_.dry_run) {
- assert(edge->outputs_.size() == 1 && "should have been rejected by parser");
- Node* out = edge->outputs_[0];
- TimeStamp deps_mtime = disk_interface_->Stat(out->path(), err);
- if (deps_mtime == -1)
- return false;
- if (!scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes)) {
- *err = string("Error writing to deps log: ") + strerror(errno);
- return false;
+ assert(edge->outputs_.size() >= 1 && "should have been rejected by parser");
+ for (std::vector<Node*>::const_iterator o = edge->outputs_.begin();
+ o != edge->outputs_.end(); ++o) {
+ TimeStamp deps_mtime = disk_interface_->Stat((*o)->path(), err);
+ if (deps_mtime == -1)
+ return false;
+ if (!scan_.deps_log()->RecordDeps(*o, deps_mtime, deps_nodes)) {
+ *err = std::string("Error writing to deps log: ") + strerror(errno);
+ return false;
+ }
}
}
return true;
diff --git a/src/build_test.cc b/src/build_test.cc
index ddf8574..426e825 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -488,6 +488,11 @@
status_(config_) {
}
+ BuildTest(DepsLog* log) : config_(MakeConfig()), command_runner_(&fs_),
+ builder_(&state_, config_, NULL, log, &fs_),
+ status_(config_) {
+ }
+
virtual void SetUp() {
StateTestWithBuiltinRules::SetUp();
@@ -582,6 +587,8 @@
edge->rule().name() == "cat_rsp" ||
edge->rule().name() == "cat_rsp_out" ||
edge->rule().name() == "cc" ||
+ edge->rule().name() == "cp_multi_msvc" ||
+ edge->rule().name() == "cp_multi_gcc" ||
edge->rule().name() == "touch" ||
edge->rule().name() == "touch-interrupt" ||
edge->rule().name() == "touch-fail-tick2") {
@@ -643,6 +650,14 @@
return true;
}
+ if (edge->rule().name() == "cp_multi_msvc") {
+ const std::string prefix = edge->GetBinding("msvc_deps_prefix");
+ for (std::vector<Node*>::iterator in = edge->inputs_.begin();
+ in != edge->inputs_.end(); ++in) {
+ result->output += prefix + (*in)->path() + '\n';
+ }
+ }
+
if (edge->rule().name() == "fail" ||
(edge->rule().name() == "touch-fail-tick2" && fs_->now_ == 2))
result->status = ExitFailure;
@@ -1855,6 +1870,214 @@
EXPECT_EQ("subcommand failed", err);
}
+struct BuildWithQueryDepsLogTest : public BuildTest {
+ BuildWithQueryDepsLogTest() : BuildTest(&log_) {
+ }
+
+ ~BuildWithQueryDepsLogTest() {
+ log_.Close();
+ }
+
+ virtual void SetUp() {
+ BuildTest::SetUp();
+
+ temp_dir_.CreateAndEnter("BuildWithQueryDepsLogTest");
+
+ std::string err;
+ ASSERT_TRUE(log_.OpenForWrite("ninja_deps", &err));
+ ASSERT_EQ("", err);
+ }
+
+ ScopedTempDir temp_dir_;
+
+ DepsLog log_;
+};
+
+/// Test a MSVC-style deps log with multiple outputs.
+TEST_F(BuildWithQueryDepsLogTest, TwoOutputsDepFileMSVC) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cp_multi_msvc\n"
+" command = echo 'using $in' && for file in $out; do cp $in $$file; done\n"
+" deps = msvc\n"
+" msvc_deps_prefix = using \n"
+"build out1 out2: cp_multi_msvc in1\n"));
+
+ std::string err;
+ EXPECT_TRUE(builder_.AddTarget("out1", &err));
+ ASSERT_EQ("", err);
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+ ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+ EXPECT_EQ("echo 'using in1' && for file in out1 out2; do cp in1 $file; done", command_runner_.commands_ran_[0]);
+
+ Node* out1_node = state_.LookupNode("out1");
+ DepsLog::Deps* out1_deps = log_.GetDeps(out1_node);
+ EXPECT_EQ(1, out1_deps->node_count);
+ EXPECT_EQ("in1", out1_deps->nodes[0]->path());
+
+ Node* out2_node = state_.LookupNode("out2");
+ DepsLog::Deps* out2_deps = log_.GetDeps(out2_node);
+ EXPECT_EQ(1, out2_deps->node_count);
+ EXPECT_EQ("in1", out2_deps->nodes[0]->path());
+}
+
+/// Test a GCC-style deps log with multiple outputs.
+TEST_F(BuildWithQueryDepsLogTest, TwoOutputsDepFileGCCOneLine) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cp_multi_gcc\n"
+" command = echo '$out: $in' > in.d && for file in $out; do cp in1 $$file; done\n"
+" deps = gcc\n"
+" depfile = in.d\n"
+"build out1 out2: cp_multi_gcc in1 in2\n"));
+
+ std::string err;
+ EXPECT_TRUE(builder_.AddTarget("out1", &err));
+ ASSERT_EQ("", err);
+ fs_.Create("in.d", "out1 out2: in1 in2");
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+ ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+ EXPECT_EQ("echo 'out1 out2: in1 in2' > in.d && for file in out1 out2; do cp in1 $file; done", command_runner_.commands_ran_[0]);
+
+ Node* out1_node = state_.LookupNode("out1");
+ DepsLog::Deps* out1_deps = log_.GetDeps(out1_node);
+ EXPECT_EQ(2, out1_deps->node_count);
+ EXPECT_EQ("in1", out1_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out1_deps->nodes[1]->path());
+
+ Node* out2_node = state_.LookupNode("out2");
+ DepsLog::Deps* out2_deps = log_.GetDeps(out2_node);
+ EXPECT_EQ(2, out2_deps->node_count);
+ EXPECT_EQ("in1", out2_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out2_deps->nodes[1]->path());
+}
+
+/// Test a GCC-style deps log with multiple outputs using a line per input.
+TEST_F(BuildWithQueryDepsLogTest, TwoOutputsDepFileGCCMultiLineInput) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cp_multi_gcc\n"
+" command = echo '$out: in1\\n$out: in2' > in.d && for file in $out; do cp in1 $$file; done\n"
+" deps = gcc\n"
+" depfile = in.d\n"
+"build out1 out2: cp_multi_gcc in1 in2\n"));
+
+ std::string err;
+ EXPECT_TRUE(builder_.AddTarget("out1", &err));
+ ASSERT_EQ("", err);
+ fs_.Create("in.d", "out1 out2: in1\nout1 out2: in2");
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+ ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+ EXPECT_EQ("echo 'out1 out2: in1\\nout1 out2: in2' > in.d && for file in out1 out2; do cp in1 $file; done", command_runner_.commands_ran_[0]);
+
+ Node* out1_node = state_.LookupNode("out1");
+ DepsLog::Deps* out1_deps = log_.GetDeps(out1_node);
+ EXPECT_EQ(2, out1_deps->node_count);
+ EXPECT_EQ("in1", out1_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out1_deps->nodes[1]->path());
+
+ Node* out2_node = state_.LookupNode("out2");
+ DepsLog::Deps* out2_deps = log_.GetDeps(out2_node);
+ EXPECT_EQ(2, out2_deps->node_count);
+ EXPECT_EQ("in1", out2_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out2_deps->nodes[1]->path());
+}
+
+/// Test a GCC-style deps log with multiple outputs using a line per output.
+TEST_F(BuildWithQueryDepsLogTest, TwoOutputsDepFileGCCMultiLineOutput) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cp_multi_gcc\n"
+" command = echo 'out1: $in\\nout2: $in' > in.d && for file in $out; do cp in1 $$file; done\n"
+" deps = gcc\n"
+" depfile = in.d\n"
+"build out1 out2: cp_multi_gcc in1 in2\n"));
+
+ std::string err;
+ EXPECT_TRUE(builder_.AddTarget("out1", &err));
+ ASSERT_EQ("", err);
+ fs_.Create("in.d", "out1: in1 in2\nout2: in1 in2");
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+ ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+ EXPECT_EQ("echo 'out1: in1 in2\\nout2: in1 in2' > in.d && for file in out1 out2; do cp in1 $file; done", command_runner_.commands_ran_[0]);
+
+ Node* out1_node = state_.LookupNode("out1");
+ DepsLog::Deps* out1_deps = log_.GetDeps(out1_node);
+ EXPECT_EQ(2, out1_deps->node_count);
+ EXPECT_EQ("in1", out1_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out1_deps->nodes[1]->path());
+
+ Node* out2_node = state_.LookupNode("out2");
+ DepsLog::Deps* out2_deps = log_.GetDeps(out2_node);
+ EXPECT_EQ(2, out2_deps->node_count);
+ EXPECT_EQ("in1", out2_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out2_deps->nodes[1]->path());
+}
+
+/// Test a GCC-style deps log with multiple outputs mentioning only the main output.
+TEST_F(BuildWithQueryDepsLogTest, TwoOutputsDepFileGCCOnlyMainOutput) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cp_multi_gcc\n"
+" command = echo 'out1: $in' > in.d && for file in $out; do cp in1 $$file; done\n"
+" deps = gcc\n"
+" depfile = in.d\n"
+"build out1 out2: cp_multi_gcc in1 in2\n"));
+
+ std::string err;
+ EXPECT_TRUE(builder_.AddTarget("out1", &err));
+ ASSERT_EQ("", err);
+ fs_.Create("in.d", "out1: in1 in2");
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+ ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+ EXPECT_EQ("echo 'out1: in1 in2' > in.d && for file in out1 out2; do cp in1 $file; done", command_runner_.commands_ran_[0]);
+
+ Node* out1_node = state_.LookupNode("out1");
+ DepsLog::Deps* out1_deps = log_.GetDeps(out1_node);
+ EXPECT_EQ(2, out1_deps->node_count);
+ EXPECT_EQ("in1", out1_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out1_deps->nodes[1]->path());
+
+ Node* out2_node = state_.LookupNode("out2");
+ DepsLog::Deps* out2_deps = log_.GetDeps(out2_node);
+ EXPECT_EQ(2, out2_deps->node_count);
+ EXPECT_EQ("in1", out2_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out2_deps->nodes[1]->path());
+}
+
+/// Test a GCC-style deps log with multiple outputs mentioning only the secondary output.
+TEST_F(BuildWithQueryDepsLogTest, TwoOutputsDepFileGCCOnlySecondaryOutput) {
+ // Note: This ends up short-circuiting the node creation due to the primary
+ // output not being present, but it should still work.
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cp_multi_gcc\n"
+" command = echo 'out2: $in' > in.d && for file in $out; do cp in1 $$file; done\n"
+" deps = gcc\n"
+" depfile = in.d\n"
+"build out1 out2: cp_multi_gcc in1 in2\n"));
+
+ std::string err;
+ EXPECT_TRUE(builder_.AddTarget("out1", &err));
+ ASSERT_EQ("", err);
+ fs_.Create("in.d", "out2: in1 in2");
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+ ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+ EXPECT_EQ("echo 'out2: in1 in2' > in.d && for file in out1 out2; do cp in1 $file; done", command_runner_.commands_ran_[0]);
+
+ Node* out1_node = state_.LookupNode("out1");
+ DepsLog::Deps* out1_deps = log_.GetDeps(out1_node);
+ EXPECT_EQ(2, out1_deps->node_count);
+ EXPECT_EQ("in1", out1_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out1_deps->nodes[1]->path());
+
+ Node* out2_node = state_.LookupNode("out2");
+ DepsLog::Deps* out2_deps = log_.GetDeps(out2_node);
+ EXPECT_EQ(2, out2_deps->node_count);
+ EXPECT_EQ("in1", out2_deps->nodes[0]->path());
+ EXPECT_EQ("in2", out2_deps->nodes[1]->path());
+}
+
/// Tests of builds involving deps logs necessarily must span
/// multiple builds. We reuse methods on BuildTest but not the
/// builder_ it sets up, because we want pristine objects for
diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc
index 6faeac6..90d4a8a 100644
--- a/src/depfile_parser.cc
+++ b/src/depfile_parser.cc
@@ -16,6 +16,8 @@
#include "depfile_parser.h"
#include "util.h"
+#include <algorithm>
+
DepfileParser::DepfileParser(DepfileParserOptions options)
: options_(options)
{
@@ -48,10 +50,8 @@
char* in = &(*content)[0];
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;
+ bool poisoned_input = false;
while (in < end) {
bool have_newline = false;
// out: current output point (typically same as in, but can fall behind
@@ -294,41 +294,32 @@
}
if (len > 0) {
- if (is_dependency) {
- if (have_secondary_target_on_this_rule) {
- if (!have_newline_since_primary_target) {
- *err = "depfile has multiple output paths";
+ StringPiece piece = StringPiece(filename, len);
+ // If we've seen this as an input before, skip it.
+ std::vector<StringPiece>::iterator pos = std::find(ins_.begin(), ins_.end(), piece);
+ if (pos == ins_.end()) {
+ if (is_dependency) {
+ if (poisoned_input) {
+ *err = "inputs may not also have inputs";
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;
}
+ // New input.
+ ins_.push_back(piece);
+ } else {
+ // Check for a new output.
+ if (std::find(outs_.begin(), outs_.end(), piece) == outs_.end())
+ outs_.push_back(piece);
}
- ins_.push_back(StringPiece(filename, len));
- } else if (!out_.str_) {
- out_ = StringPiece(filename, len);
- } else if (out_ != StringPiece(filename, len)) {
- have_secondary_target_on_this_rule = true;
+ } else if (!is_dependency) {
+ // We've passed an input on the left side; reject new inputs.
+ poisoned_input = 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 (have_target) {
- have_newline_since_primary_target = true;
- }
+ poisoned_input = false;
}
}
if (!have_target) {
diff --git a/src/depfile_parser.h b/src/depfile_parser.h
index be20374..11b1228 100644
--- a/src/depfile_parser.h
+++ b/src/depfile_parser.h
@@ -21,17 +21,8 @@
#include "string_piece.h"
-enum DepfileDistinctTargetLinesAction {
- kDepfileDistinctTargetLinesActionWarn,
- kDepfileDistinctTargetLinesActionError,
-};
-
struct DepfileParserOptions {
- DepfileParserOptions()
- : depfile_distinct_target_lines_action_(
- kDepfileDistinctTargetLinesActionWarn) {}
- DepfileDistinctTargetLinesAction
- depfile_distinct_target_lines_action_;
+ DepfileParserOptions() {}
};
/// Parser for the dependency information emitted by gcc's -M flags.
@@ -44,7 +35,7 @@
/// pointers within it.
bool Parse(string* content, string* err);
- StringPiece out_;
+ std::vector<StringPiece> outs_;
vector<StringPiece> ins_;
DepfileParserOptions options_;
};
diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc
index 735a0c3..b32b942 100644
--- a/src/depfile_parser.in.cc
+++ b/src/depfile_parser.in.cc
@@ -15,6 +15,8 @@
#include "depfile_parser.h"
#include "util.h"
+#include <algorithm>
+
DepfileParser::DepfileParser(DepfileParserOptions options)
: options_(options)
{
@@ -47,10 +49,8 @@
char* in = &(*content)[0];
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;
+ bool poisoned_input = false;
while (in < end) {
bool have_newline = false;
// out: current output point (typically same as in, but can fall behind
@@ -146,41 +146,32 @@
}
if (len > 0) {
- if (is_dependency) {
- if (have_secondary_target_on_this_rule) {
- if (!have_newline_since_primary_target) {
- *err = "depfile has multiple output paths";
+ StringPiece piece = StringPiece(filename, len);
+ // If we've seen this as an input before, skip it.
+ std::vector<StringPiece>::iterator pos = std::find(ins_.begin(), ins_.end(), piece);
+ if (pos == ins_.end()) {
+ if (is_dependency) {
+ if (poisoned_input) {
+ *err = "inputs may not also have inputs";
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;
}
+ // New input.
+ ins_.push_back(piece);
+ } else {
+ // Check for a new output.
+ if (std::find(outs_.begin(), outs_.end(), piece) == outs_.end())
+ outs_.push_back(piece);
}
- ins_.push_back(StringPiece(filename, len));
- } else if (!out_.str_) {
- out_ = StringPiece(filename, len);
- } else if (out_ != StringPiece(filename, len)) {
- have_secondary_target_on_this_rule = true;
+ } else if (!is_dependency) {
+ // We've passed an input on the left side; reject new inputs.
+ poisoned_input = 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 (have_target) {
- have_newline_since_primary_target = true;
- }
+ poisoned_input = false;
}
}
if (!have_target) {
diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc
index 19224f3..bf1a0bc 100644
--- a/src/depfile_parser_test.cc
+++ b/src/depfile_parser_test.cc
@@ -34,7 +34,8 @@
"build/ninja.o: ninja.cc ninja.h eval_env.h manifest_parser.h\n",
&err));
ASSERT_EQ("", err);
- EXPECT_EQ("build/ninja.o", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ EXPECT_EQ("build/ninja.o", parser_.outs_[0].AsString());
EXPECT_EQ(4u, parser_.ins_.size());
}
@@ -54,7 +55,8 @@
" bar.h baz.h\n",
&err));
ASSERT_EQ("", err);
- EXPECT_EQ("foo.o", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ EXPECT_EQ("foo.o", parser_.outs_[0].AsString());
EXPECT_EQ(2u, parser_.ins_.size());
}
@@ -65,7 +67,8 @@
" bar.h baz.h\r\n",
&err));
ASSERT_EQ("", err);
- EXPECT_EQ("foo.o", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ EXPECT_EQ("foo.o", parser_.outs_[0].AsString());
EXPECT_EQ(2u, parser_.ins_.size());
}
@@ -79,8 +82,9 @@
" Project\\Thing\\Bar.tlb \\\n",
&err));
ASSERT_EQ("", err);
+ ASSERT_EQ(1u, parser_.outs_.size());
EXPECT_EQ("Project\\Dir\\Build\\Release8\\Foo\\Foo.res",
- parser_.out_.AsString());
+ parser_.outs_[0].AsString());
EXPECT_EQ(4u, parser_.ins_.size());
}
@@ -90,8 +94,9 @@
"a\\ bc\\ def: a\\ b c d",
&err));
ASSERT_EQ("", err);
+ ASSERT_EQ(1u, parser_.outs_.size());
EXPECT_EQ("a bc def",
- parser_.out_.AsString());
+ parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("a b",
parser_.ins_[0].AsString());
@@ -111,8 +116,9 @@
"a\\ b\\#c.h: \\\\\\\\\\ \\\\\\\\ \\\\share\\info\\\\#1",
&err));
ASSERT_EQ("", err);
+ ASSERT_EQ(1u, parser_.outs_.size());
EXPECT_EQ("a b#c.h",
- parser_.out_.AsString());
+ parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("\\\\ ",
parser_.ins_[0].AsString());
@@ -130,8 +136,9 @@
"\\!\\@\\#$$\\%\\^\\&\\[\\]\\\\:",
&err));
ASSERT_EQ("", err);
+ ASSERT_EQ(1u, parser_.outs_.size());
EXPECT_EQ("\\!\\@#$\\%\\^\\&\\[\\]\\\\",
- parser_.out_.AsString());
+ parser_.outs_[0].AsString());
ASSERT_EQ(0u, parser_.ins_.size());
}
@@ -147,8 +154,9 @@
" a[1]b@2%c",
&err));
ASSERT_EQ("", err);
+ ASSERT_EQ(1u, parser_.outs_.size());
EXPECT_EQ("C:/Program Files (x86)/Microsoft crtdefs.h",
- parser_.out_.AsString());
+ parser_.outs_[0].AsString());
ASSERT_EQ(5u, parser_.ins_.size());
EXPECT_EQ("en@quot.header~",
parser_.ins_[0].AsString());
@@ -166,18 +174,25 @@
// check that multiple duplicate targets are properly unified
string err;
EXPECT_TRUE(Parse("foo foo: x y z", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].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, RejectMultipleDifferentOutputs) {
- // check that multiple different outputs are rejected by the parser
+TEST_F(DepfileParserTest, MultipleDifferentOutputs) {
+ // check that multiple different outputs are accepted by the parser
string err;
- EXPECT_FALSE(Parse("foo bar: x y z", &err));
- ASSERT_EQ("depfile has multiple output paths", err);
+ EXPECT_TRUE(Parse("foo bar: x y z", &err));
+ ASSERT_EQ(2u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
+ ASSERT_EQ("bar", parser_.outs_[1].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, MultipleEmptyRules) {
@@ -185,7 +200,8 @@
EXPECT_TRUE(Parse("foo: x\n"
"foo: \n"
"foo:\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(1u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
}
@@ -196,7 +212,8 @@
"foo: y\n"
"foo \\\n"
"foo: z\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -209,7 +226,8 @@
"foo: y\r\n"
"foo \\\r\n"
"foo: z\r\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -222,7 +240,8 @@
" y\n"
"foo \\\n"
"foo: z\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -235,7 +254,8 @@
" y\r\n"
"foo \\\r\n"
"foo: z\r\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -247,7 +267,8 @@
EXPECT_TRUE(Parse(" foo: x\n"
" foo: y\n"
" foo: z\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -259,7 +280,8 @@
EXPECT_TRUE(Parse(" foo: x\r\n"
" foo: y\r\n"
" foo: z\r\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -272,7 +294,8 @@
"x:\n"
"y:\n"
"z:\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
ASSERT_EQ(3u, parser_.ins_.size());
EXPECT_EQ("x", parser_.ins_[0].AsString());
EXPECT_EQ("y", parser_.ins_[1].AsString());
@@ -287,25 +310,34 @@
"y:\n"
"foo: z\n"
"z:\n", &err));
- ASSERT_EQ("foo", parser_.out_.AsString());
+ ASSERT_EQ(1u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].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
+TEST_F(DepfileParserTest, MultipleRulesDifferentOutputs) {
+ // check that multiple different outputs are accepted 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;
- 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);
+ EXPECT_TRUE(Parse("foo: x y\n"
+ "bar: y z\n", &err));
+ ASSERT_EQ(2u, parser_.outs_.size());
+ ASSERT_EQ("foo", parser_.outs_[0].AsString());
+ ASSERT_EQ("bar", parser_.outs_[1].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, BuggyMP) {
+ std::string err;
+ EXPECT_FALSE(Parse("foo: x y z\n"
+ "x: alsoin\n"
+ "y:\n"
+ "z:\n", &err));
+ ASSERT_EQ("inputs may not also have inputs", err);
}
diff --git a/src/graph.cc b/src/graph.cc
index e24a954..28a9653 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -14,6 +14,7 @@
#include "graph.h"
+#include <algorithm>
#include <assert.h>
#include <stdio.h>
@@ -511,6 +512,17 @@
return true;
}
+struct matches {
+ matches(std::vector<StringPiece>::iterator i) : i_(i) {}
+
+ bool operator()(const Node* node) const {
+ StringPiece opath = StringPiece(node->path());
+ return *i_ == opath;
+ }
+
+ std::vector<StringPiece>::iterator i_;
+};
+
bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path,
string* err) {
METRIC_RECORD("depfile load");
@@ -541,9 +553,15 @@
return false;
}
+ if (depfile.outs_.empty()) {
+ *err = path + ": no outputs declared";
+ return false;
+ }
+
uint64_t unused;
- if (!CanonicalizePath(const_cast<char*>(depfile.out_.str_),
- &depfile.out_.len_, &unused, err)) {
+ std::vector<StringPiece>::iterator primary_out = depfile.outs_.begin();
+ if (!CanonicalizePath(const_cast<char*>(primary_out->str_),
+ &primary_out->len_, &unused, err)) {
*err = path + ": " + *err;
return false;
}
@@ -552,12 +570,22 @@
// mark the edge as dirty.
Node* first_output = edge->outputs_[0];
StringPiece opath = StringPiece(first_output->path());
- if (opath != depfile.out_) {
+ if (opath != *primary_out) {
EXPLAIN("expected depfile '%s' to mention '%s', got '%s'", path.c_str(),
- first_output->path().c_str(), depfile.out_.AsString().c_str());
+ first_output->path().c_str(), primary_out->AsString().c_str());
return false;
}
+ // Ensure that all mentioned outputs are outputs of the edge.
+ for (std::vector<StringPiece>::iterator o = depfile.outs_.begin();
+ o != depfile.outs_.end(); ++o) {
+ matches m(o);
+ if (std::find_if(edge->outputs_.begin(), edge->outputs_.end(), m) == edge->outputs_.end()) {
+ *err = path + ": depfile mentions '" + o->AsString() + "' as an output, but no such output was declared";
+ return false;
+ }
+ }
+
// Preallocate space in edge->inputs_ to be filled in below.
vector<Node*>::iterator implicit_dep =
PreallocateSpace(edge, depfile.ins_.size());
diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index e28be2f..bb53dc2 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -379,14 +379,6 @@
}
}
- // Multiple outputs aren't (yet?) supported with depslog.
- string deps_type = edge->GetBinding("deps");
- if (!deps_type.empty() && edge->outputs_.size() > 1) {
- return lexer_.Error("multiple outputs aren't (yet?) supported by depslog; "
- "bring this up on the mailing list if it affects you",
- err);
- }
-
// Lookup, validate, and save any dyndep binding. It will be used later
// to load generated dependency information dynamically, but it must
// be one of our manifest-specified inputs.
diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc
index f2b7467..f4aee2d 100644
--- a/src/manifest_parser_test.cc
+++ b/src/manifest_parser_test.cc
@@ -858,11 +858,10 @@
State local_state;
ManifestParser parser(&local_state, NULL);
string err;
- EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n deps = gcc\n"
+ EXPECT_TRUE(parser.ParseTest("rule cc\n command = foo\n deps = gcc\n"
"build a.o b.o: cc c.cc\n",
&err));
- EXPECT_EQ("input:5: multiple outputs aren't (yet?) supported by depslog; "
- "bring this up on the mailing list if it affects you\n", err);
+ EXPECT_EQ("", err);
}
TEST_F(ParserTest, SubNinja) {
diff --git a/src/ninja.cc b/src/ninja.cc
index 0c90e74..bf97b4a 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -74,10 +74,6 @@
/// 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
@@ -1035,7 +1031,6 @@
printf("warning flags:\n"
" dupbuild={err,warn} multiple build lines for one target\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") {
@@ -1050,11 +1045,9 @@
} 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;
+ } else if (name == "depfilemulti=err" ||
+ name == "depfilemulti=warn") {
+ Warning("deprecated warning 'depfilemulti'");
return true;
} else {
const char* suggestion =
@@ -1338,11 +1331,6 @@
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
