summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNico Weber <nicolasweber@gmx.de>2016-04-19 15:22:53 (GMT)
committerNico Weber <nicolasweber@gmx.de>2016-04-19 15:22:53 (GMT)
commit144a0138e62b636c04d9c08deaf8b1eb603be530 (patch)
tree38bdbd22f795136c6c61009528908947e341ac1d
parent78f548880e549c701bd77760e4b3f3a4ee147641 (diff)
parente8d855f2622693a4b616cba96084cd7a58cb0611 (diff)
downloadNinja-144a0138e62b636c04d9c08deaf8b1eb603be530.zip
Ninja-144a0138e62b636c04d9c08deaf8b1eb603be530.tar.gz
Ninja-144a0138e62b636c04d9c08deaf8b1eb603be530.tar.bz2
Merge pull request #1059 from bradking/avoid-double-scheduling-edge
Avoid double-scheduling build edges in another case
-rw-r--r--src/build.cc17
-rw-r--r--src/build_test.cc23
2 files changed, 33 insertions, 7 deletions
diff --git a/src/build.cc b/src/build.cc
index 2df5c1a..f6f022e 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -366,19 +366,22 @@ Edge* Plan::FindWork() {
}
void Plan::ScheduleWork(Edge* edge) {
+ set<Edge*>::iterator e = ready_.lower_bound(edge);
+ if (e != ready_.end() && !ready_.key_comp()(edge, *e)) {
+ // This edge has already been scheduled. We can get here again if an edge
+ // and one of its dependencies share an order-only input, or if a node
+ // duplicates an out edge (see https://github.com/ninja-build/ninja/pull/519).
+ // Avoid scheduling the work again.
+ return;
+ }
+
Pool* pool = edge->pool();
if (pool->ShouldDelayEdge()) {
- // The graph is not completely clean. Some Nodes have duplicate Out edges.
- // We need to explicitly ignore these here, otherwise their work will get
- // scheduled twice (see https://github.com/ninja-build/ninja/pull/519)
- if (ready_.count(edge)) {
- return;
- }
pool->DelayEdge(edge);
pool->RetrieveReadyEdges(&ready_);
} else {
pool->EdgeScheduled(*edge);
- ready_.insert(edge);
+ ready_.insert(e, edge);
}
}
diff --git a/src/build_test.cc b/src/build_test.cc
index 7c6060d..26bf7ef 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -875,6 +875,29 @@ TEST_F(BuildTest, DepFileParseError) {
EXPECT_EQ("foo.o.d: expected ':' in depfile", err);
}
+TEST_F(BuildTest, EncounterReadyTwice) {
+ string err;
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule touch\n"
+" command = touch $out\n"
+"build c: touch\n"
+"build b: touch || c\n"
+"build a: touch | b || c\n"));
+
+ vector<Edge*> c_out = GetNode("c")->out_edges();
+ ASSERT_EQ(2u, c_out.size());
+ EXPECT_EQ("b", c_out[0]->outputs_[0]->path());
+ EXPECT_EQ("a", c_out[1]->outputs_[0]->path());
+
+ fs_.Create("b", "");
+ EXPECT_TRUE(builder_.AddTarget("a", &err));
+ ASSERT_EQ("", err);
+
+ EXPECT_TRUE(builder_.Build(&err));
+ ASSERT_EQ("", err);
+ ASSERT_EQ(2u, command_runner_.commands_ran_.size());
+}
+
TEST_F(BuildTest, OrderOnlyDeps) {
string err;
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,