This commit will fix issue#2653.

src/build_test.cc

@@ -655,7 +655,7 @@ bool FakeCommandRunner::StartCommand(Edge* edge) {
     // Don't do anything.
   } else if (edge->rule().name() == "cp") {
     assert(!edge->inputs_.empty());
-    assert(edge->outputs_.size() == 1);
+    assert(edge->outputs_.size() >= 1);
     string content;
     string err;
     if (fs_->ReadFile(edge->inputs_[0]->path(), &content, &err) ==
@@ -4405,3 +4405,64 @@ TEST_F(BuildTest, ValidationWithCircularDependency) {
   EXPECT_FALSE(builder_.AddTarget("out", &err));
   EXPECT_EQ("dependency cycle: validate -> validate_in -> validate", err);
 }
+
+// Test case: Late dyndep merges two initially separate dependency graphs into a
+// single unified graph.
+// https://github.com/ninja-build/ninja/issues/2653
+TEST_F(BuildTest, LateDynDepPropagateDirty) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+    "rule cp\n"
+    "  command = cp $in $out\n"
+    "build b1: cp input_b\n"
+    "build b2: cp b1\n"
+    "build b3: cp b2\n"
+    "build b4: cp b3\n"
+    "build b5: cp b4\n"
+    "build a1: cp input_a\n"
+    "build a2: cp a1\n"
+    "build a3: cp a2 || dd\n"
+    "  dyndep = dd\n"
+    "build a4: cp a3\n"
+    "build a5: cp a4\n"));
+
+  fs_.Create("input_b", "");
+  fs_.Create("b1", "");
+  fs_.Create("b2", "");
+  fs_.Create("b3", "");
+  fs_.Create("b4", "");
+  fs_.Create("b5", "");
+
+  fs_.Create("input_a", "");
+  fs_.Create("a1", "");
+  fs_.Create("a2", "");
+  fs_.Create("a3", "");
+  fs_.Create("a4", "");
+  fs_.Create("a5", "");
+  fs_.Create("dd",
+             "ninja_dyndep_version = 1\n"
+             "build  a3 | input_b: dyndep |\n");
+
+  string err;
+
+  fs_.Tick();
+  fs_.Create("input_a", "");
+
+  command_runner_.commands_ran_.clear();
+  state_.Reset();
+
+  EXPECT_TRUE(builder_.AddTarget("b5", &err));
+  EXPECT_EQ("", err);
+  err.clear();
+  EXPECT_TRUE(builder_.AddTarget("a5", &err));
+  EXPECT_EQ("", err);
+  err.clear();
+
+  EXPECT_TRUE(GetNode("a5")->dirty());
+  EXPECT_TRUE(GetNode("b5")->dirty());
+
+  EXPECT_EQ(builder_.Build(&err), ExitSuccess);
+  EXPECT_EQ("", err);
+
+  EXPECT_EQ(10u, command_runner_.commands_ran_.size());
+}
+

src/graph.cc

@@ -79,9 +79,47 @@ bool DependencyScan::RecomputeDirty(Node* initial_node,
   return true;
 }
 
+namespace {
+class propagateDirty {
+ public:
+  propagateDirty(std::vector<Node*>* topLevelNodes)
+      : topLevelNodes_(topLevelNodes) {}
+
+  void process(Edge* edge);
+
+ private:
+  std::vector<Node*>* topLevelNodes_;
+};
+
+void propagateDirty::process(Edge* const edge) {
+  for (Node* const out : edge->outputs_) {
+    if (out->clean()) {
+      out->MarkDirty();
+      edge->outputs_ready_ = false;
+      if (out->out_edges().empty()) {
+        // add target to top level
+        topLevelNodes_->push_back(out);
+        continue;
+      }
+      for (Edge* outEdge : out->out_edges()) {
+        if (outEdge)
+          process(outEdge);
+      }
+    }
+  }
+}
+}  // namespace
+
 bool DependencyScan::RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
                                         std::vector<Node*>* validation_nodes,
                                         string* err) {
+  bool dummy(false);
+  return RecomputeNodeDirty(node, stack, validation_nodes, dummy, err);
+}
+
+bool DependencyScan::RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
+                                        std::vector<Node*>* validation_nodes, bool& dyndepLoadedPath,
+                                        string* err) {
   Edge* edge = node->in_edge();
   if (!edge) {
     // If we already visited this leaf node then we are done.
@@ -112,6 +150,7 @@ bool DependencyScan::RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
   bool dirty = false;
   edge->outputs_ready_ = true;
   edge->deps_missing_ = false;
+  bool dyndepLoaded = false;
 
   if (!edge->deps_loaded_) {
     // This is our first encounter with this edge.
@@ -126,14 +165,17 @@ bool DependencyScan::RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
     //   Later during the build the dyndep file will become ready and be
     //   loaded to update this edge before it can possibly be scheduled.
     if (edge->dyndep_ && edge->dyndep_->dyndep_pending()) {
-      if (!RecomputeNodeDirty(edge->dyndep_, stack, validation_nodes, err))
+      if (!RecomputeNodeDirty(edge->dyndep_, stack, validation_nodes,
+                              dyndepLoadedPath, err))
         return false;
 
       if (!edge->dyndep_->in_edge() ||
           edge->dyndep_->in_edge()->outputs_ready()) {
         // The dyndep file is ready, so load it now.
         if (!LoadDyndeps(edge->dyndep_, err))
           return false;
+        else
+          dyndepLoaded = true;
       }
     }
   }
@@ -165,32 +207,64 @@ bool DependencyScan::RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
   validation_nodes->insert(validation_nodes->end(),
       edge->validations_.begin(), edge->validations_.end());
 
+  if (dyndepLoaded)
+    dyndepLoadedPath = true;
   // Visit all inputs; we're dirty if any of the inputs are dirty.
   Node* most_recent_input = NULL;
-  for (vector<Node*>::iterator i = edge->inputs_.begin();
-       i != edge->inputs_.end(); ++i) {
+  for (std::size_t index_i = 0; index_i < edge->inputs_.size();
+       index_i++) {
     // Visit this input.
-    if (!RecomputeNodeDirty(*i, stack, validation_nodes, err))
+    const auto i = std::next(edge->inputs_.begin(), index_i);
+
+    bool dyndepLoadedPathLocal = false;
+    if (!RecomputeNodeDirty(*i, stack, validation_nodes, dyndepLoadedPathLocal,
+                            err)) {
+      if (dyndepLoadedPathLocal)
+        dyndepLoadedPath = true;
       return false;
+    }
+
+    if (dyndepLoadedPathLocal)
+      dyndepLoadedPath = true;
 
     // If an input is not ready, neither are our outputs.
     if (Edge* in_edge = (*i)->in_edge()) {
       if (!in_edge->outputs_ready_)
         edge->outputs_ready_ = false;
     }
 
+    bool finish = false;
     if (!edge->is_order_only(i - edge->inputs_.begin())) {
       // If a regular input is dirty (or missing), we're dirty.
       // Otherwise consider mtime.
       if ((*i)->dirty()) {
         explanations_.Record(node, "%s is dirty", (*i)->path().c_str());
         dirty = true;
+        finish = true;
       } else {
         if (!most_recent_input || (*i)->mtime() > most_recent_input->mtime()) {
           most_recent_input = *i;
         }
       }
     }
+
+    if (dyndepLoadedPathLocal && !finish) {
+      // recheck input, without current one. Dyndep may have added additional
+      // dependencies
+      for (std::size_t index = 0; index < index_i; ++index) {
+        if (edge->is_order_only(index))
+          continue;
+        auto input = std::next(edge->inputs_.begin(), index);
+        if ((*input)->dirty()) {
+          explanations_.Record(node, "%s is dirty", (*input)->path().c_str());
+          dirty = true;
+          break;
+        } else if (!most_recent_input ||
+                   (*input)->mtime() > most_recent_input->mtime()) {
+          most_recent_input = *input;
+        }
+      }
+    }
   }
 
   // We may also be dirty due to output state: missing outputs, out of
@@ -202,8 +276,16 @@ bool DependencyScan::RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
   // Finally, visit each output and update their dirty state if necessary.
   for (vector<Node*>::iterator o = edge->outputs_.begin();
        o != edge->outputs_.end(); ++o) {
-    if (dirty)
+    if (dirty) {
       (*o)->MarkDirty();
+      if (dyndepLoaded) {
+        propagateDirty propagateDirtyDyndep(validation_nodes);
+        for (Edge* outedges : (*o)->out_edges())
+          propagateDirtyDyndep.process(outedges);
+      }
+    } else {
+      (*o)->MarkClean();
+    }
   }
 
   // If an edge is dirty, its outputs are normally not ready.  (It's

src/graph.h

@@ -37,6 +37,12 @@ struct Node;
 struct Pool;
 struct State;
 
+enum class DirtyState {
+  SPARE = 0,
+  DIRTY = 1,
+  CLEAN = 2,
+};
+
 /// Information about a node in the dependency graph: the file, whether
 /// it's dirty, mtime, etc.
 struct Node {
@@ -60,7 +66,7 @@ struct Node {
   void ResetState() {
     mtime_ = -1;
     exists_ = ExistenceStatusUnknown;
-    dirty_ = false;
+    dirty_ = DirtyState::SPARE;
   }
 
   /// Mark the Node as already-stat()ed and missing.
@@ -90,9 +96,18 @@ struct Node {
 
   TimeStamp mtime() const { return mtime_; }
 
-  bool dirty() const { return dirty_; }
-  void set_dirty(bool dirty) { dirty_ = dirty; }
-  void MarkDirty() { dirty_ = true; }
+  bool dirty() const { return dirty_ == DirtyState::DIRTY; }
+  void set_dirty(bool dirty) {
+    if (dirty) {
+      dirty_ = DirtyState::DIRTY;
+    } else if (dirty_ == DirtyState::DIRTY) {
+      dirty_ = DirtyState::CLEAN;
+    }
+  }
+  void MarkDirty() { dirty_ = DirtyState::DIRTY; }
+
+  bool clean() const { return dirty_ == DirtyState::CLEAN; }
+  void MarkClean() { dirty_ = DirtyState::CLEAN; }
 
   bool dyndep_pending() const { return dyndep_pending_; }
   void set_dyndep_pending(bool pending) { dyndep_pending_ = pending; }
@@ -144,7 +159,7 @@ struct Node {
   /// Dirty is true when the underlying file is out-of-date.
   /// But note that Edge::outputs_ready_ is also used in judging which
   /// edges to build.
-  bool dirty_ = false;
+  DirtyState dirty_ = DirtyState::SPARE;
 
   /// Store whether dyndep information is expected from this node but
   /// has not yet been loaded.
@@ -374,6 +389,9 @@ struct DependencyScan {
  private:
   bool RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
                           std::vector<Node*>* validation_nodes, std::string* err);
+  bool RecomputeNodeDirty(Node* node, std::vector<Node*>* stack,
+                          std::vector<Node*>* validation_nodes,
+                          bool& dyndepLoaded, std::string* err);
   bool VerifyDAG(Node* node, std::vector<Node*>* stack, std::string* err);
 
   /// Recompute whether a given single output should be marked dirty.

src/graph_test.cc

@@ -1140,4 +1140,78 @@ TEST_F(GraphTest, EdgeQueuePriority) {
   EXPECT_TRUE(queue.empty());
 }
 
+// https://github.com/ninja-build/ninja/issues/2641
+// https://github.com/ninja-build/ninja/issues/2653
+TEST_F(GraphTest, LateDynDep) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, R"ninja(
+rule cp
+  command = cp $in $out
+rule touch
+  command = touch $out
+
+build other3: cp other2
+build other2: cp other
+build copy: touch other3 || orderOnly
+build orderOnly: cp stamp
+build stamp: cp in || dd
+  dyndep = dd
+)ninja"));
+
+  fs_.Create("orderOnly", "");
+  fs_.Create("other", "");
+  fs_.Create("other2", "");
+  fs_.Create("other3", "");
+  fs_.Create("copy", "");
+  fs_.Create("stamp", "");
+  fs_.Create("dd",
+             "ninja_dyndep_version = 1\n"
+             "build  stamp | other: dyndep\n");
+  fs_.Tick();
+  fs_.Create("in", "");
+
+  string err;
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("copy"), NULL, &err));
+  ASSERT_EQ("", err);
+
+  EXPECT_TRUE(GetNode("copy")->dirty());
+  EXPECT_TRUE(GetNode("orderOnly")->dirty());
+  EXPECT_TRUE(GetNode("stamp")->dirty());
+  EXPECT_TRUE(GetNode("other")->dirty());
+  EXPECT_TRUE(GetNode("other2")->dirty());
+  EXPECT_TRUE(GetNode("other3")->dirty());
+}
+
+// https://github.com/ninja-build/ninja/issues/2641
+TEST_F(GraphTest, LateDynDep2) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, R"ninja(
+rule cp
+  command = cp $in $out
+rule touch
+  command = touch $out
+
+build copy: touch other || orderOnly
+build orderOnly: cp stamp
+build stamp: cp in || dd
+  dyndep = dd
+)ninja"));
+
+  fs_.Create("orderOnly", "");
+  fs_.Create("other", "");
+  fs_.Create("copy", "");
+  fs_.Create("stamp", "");
+  fs_.Create("dd",
+             "ninja_dyndep_version = 1\n"
+             "build  stamp | other: dyndep\n");
+  fs_.Tick();
+  fs_.Create("in", "");
+
+  string err;
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("copy"), NULL, &err));
+  ASSERT_EQ("", err);
+
+  EXPECT_TRUE(GetNode("copy")->dirty());
+  EXPECT_TRUE(GetNode("orderOnly")->dirty());
+  EXPECT_TRUE(GetNode("stamp")->dirty());
+  EXPECT_TRUE(GetNode("other")->dirty());
+}