From a5b178048746383ac8be231c074b11fbfce6f6c8 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 24 Mar 2021 14:06:19 -0400 Subject: dyndep: add dedicated test for dyndep-discovered implicit dependencies Previously this was covered only as part of more complex tests. --- src/graph_test.cc | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/graph_test.cc b/src/graph_test.cc index 14f6375..6b4bb51 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -511,6 +511,37 @@ TEST_F(GraphTest, DyndepLoadTrivial) { EXPECT_FALSE(edge->GetBindingBool("restat")); } +TEST_F(GraphTest, DyndepLoadImplicit) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build out1: r in || dd\n" +" dyndep = dd\n" +"build out2: r in\n" + ); + fs_.Create("dd", +"ninja_dyndep_version = 1\n" +"build out1: dyndep | out2\n" + ); + + string err; + ASSERT_TRUE(GetNode("dd")->dyndep_pending()); + EXPECT_TRUE(scan_.LoadDyndeps(GetNode("dd"), &err)); + EXPECT_EQ("", err); + EXPECT_FALSE(GetNode("dd")->dyndep_pending()); + + Edge* edge = GetNode("out1")->in_edge(); + ASSERT_EQ(1u, edge->outputs_.size()); + EXPECT_EQ("out1", edge->outputs_[0]->path()); + ASSERT_EQ(3u, edge->inputs_.size()); + EXPECT_EQ("in", edge->inputs_[0]->path()); + EXPECT_EQ("out2", edge->inputs_[1]->path()); + EXPECT_EQ("dd", edge->inputs_[2]->path()); + EXPECT_EQ(1u, edge->implicit_deps_); + EXPECT_EQ(1u, edge->order_only_deps_); + EXPECT_FALSE(edge->GetBindingBool("restat")); +} + TEST_F(GraphTest, DyndepLoadMissingFile) { AssertParse(&state_, "rule r\n" -- cgit v0.12 From fa577b6d5364d103e57784ba5d6efa973b96a5c3 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 24 Mar 2021 15:38:35 -0400 Subject: dyndep: reconcile dyndep-specified outputs with depfile-specified inputs When a path loaded from a depfile does not have a node, we create a new node with a phony edge producing it. If we later load a dyndep file that specifies the same node as an output of a known edge, we previously failed with a "multiple rules generate ..." error. Instead, since the conflicting edge was internally generated, replace the node's input edge with the now-known real edge that produces it. --- src/build_test.cc | 42 ++++++++++++++++++++++++++++++++++++++++++ src/dyndep.cc | 12 +++++++++--- src/graph.cc | 1 + src/graph.h | 5 +++-- 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index a92f7c5..e0c43b1 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -3115,6 +3115,48 @@ TEST_F(BuildTest, DyndepBuildDiscoverImplicitConnection) { EXPECT_EQ("touch out out.imp", command_runner_.commands_ran_[2]); } +TEST_F(BuildTest, DyndepBuildDiscoverOutputAndDepfileInput) { + // Verify that a dyndep file can be built and loaded to discover + // that one edge has an implicit output that is also reported by + // a depfile as an input of another edge. + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule touch\n" +" command = touch $out $out.imp\n" +"rule cp\n" +" command = cp $in $out\n" +"build dd: cp dd-in\n" +"build tmp: touch || dd\n" +" dyndep = dd\n" +"build out: cp tmp\n" +" depfile = out.d\n" +)); + fs_.Create("out.d", "out: tmp.imp\n"); + fs_.Create("dd-in", +"ninja_dyndep_version = 1\n" +"build tmp | tmp.imp: dyndep\n" +); + + string err; + EXPECT_TRUE(builder_.AddTarget("out", &err)); + ASSERT_EQ("", err); + + // Loading the depfile gave tmp.imp a phony input edge. + ASSERT_TRUE(GetNode("tmp.imp")->in_edge()->is_phony()); + + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); + + // Loading the dyndep file gave tmp.imp a real input edge. + ASSERT_FALSE(GetNode("tmp.imp")->in_edge()->is_phony()); + + ASSERT_EQ(3u, command_runner_.commands_ran_.size()); + EXPECT_EQ("cp dd-in dd", command_runner_.commands_ran_[0]); + EXPECT_EQ("touch tmp tmp.imp", command_runner_.commands_ran_[1]); + EXPECT_EQ("cp tmp out", command_runner_.commands_ran_[2]); + EXPECT_EQ(1u, fs_.files_created_.count("tmp.imp")); + EXPECT_TRUE(builder_.AlreadyUpToDate()); +} + TEST_F(BuildTest, DyndepBuildDiscoverNowWantEdge) { // Verify that a dyndep file can be built and loaded to discover // that an edge is actually wanted due to a missing implicit output. diff --git a/src/dyndep.cc b/src/dyndep.cc index b388e9b..dd4ed09 100644 --- a/src/dyndep.cc +++ b/src/dyndep.cc @@ -97,9 +97,15 @@ bool DyndepLoader::UpdateEdge(Edge* edge, Dyndeps const* dyndeps, for (std::vector::const_iterator i = dyndeps->implicit_outputs_.begin(); i != dyndeps->implicit_outputs_.end(); ++i) { - if ((*i)->in_edge() != NULL) { - *err = "multiple rules generate " + (*i)->path(); - return false; + if (Edge* old_in_edge = (*i)->in_edge()) { + // This node already has an edge producing it. Fail with an error + // unless the edge was generated by ImplicitDepLoader, in which + // case we can replace it with the now-known real producer. + if (!old_in_edge->generated_by_dep_loader_) { + *err = "multiple rules generate " + (*i)->path(); + return false; + } + old_in_edge->outputs_.clear(); } (*i)->set_in_edge(edge); } diff --git a/src/graph.cc b/src/graph.cc index 90e8d08..822b7c5 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -654,6 +654,7 @@ void ImplicitDepLoader::CreatePhonyInEdge(Node* node) { return; Edge* phony_edge = state_->AddEdge(&State::kPhonyRule); + phony_edge->generated_by_dep_loader_ = true; node->set_in_edge(phony_edge); phony_edge->outputs_.push_back(node); diff --git a/src/graph.h b/src/graph.h index 6756378..bb4f10c 100644 --- a/src/graph.h +++ b/src/graph.h @@ -147,8 +147,8 @@ struct Edge { Edge() : rule_(NULL), pool_(NULL), dyndep_(NULL), env_(NULL), mark_(VisitNone), id_(0), outputs_ready_(false), deps_loaded_(false), - deps_missing_(false), implicit_deps_(0), order_only_deps_(0), - implicit_outs_(0) {} + deps_missing_(false), generated_by_dep_loader_(false), + implicit_deps_(0), order_only_deps_(0), implicit_outs_(0) {} /// Return true if all inputs' in-edges are ready. bool AllInputsReady() const; @@ -182,6 +182,7 @@ struct Edge { bool outputs_ready_; bool deps_loaded_; bool deps_missing_; + bool generated_by_dep_loader_; const Rule& rule() const { return *rule_; } Pool* pool() const { return pool_; } -- cgit v0.12