Merge pull request #1937 from bradking/dyndep-out-depfile-in

dyndep: reconcile dyndep-specified outputs with depfile-specified inputs

5 files changed, 86 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<Node*>::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_; }
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"