diff --git a/src/build.cc b/src/build.cc index 76ff93af03..6903e45306 100644 --- a/src/build.cc +++ b/src/build.cc @@ -95,15 +95,20 @@ bool Plan::AddTarget(const Node* target, string* err) { bool Plan::AddSubTarget(const Node* node, const Node* dependent, string* err, set* dyndep_walk) { Edge* edge = node->in_edge(); - if (!edge) { // Leaf node. - if (node->dirty()) { - string referenced; - if (dependent) - referenced = ", needed by '" + dependent->path() + "',"; - *err = "'" + node->path() + "'" + referenced + " missing " - "and no known rule to make it"; - } - return false; + if (!edge) { + // Leaf node, this can be either a regular input from the manifest + // (e.g. a source file), or an implicit input from a depfile or dyndep + // file. In the first case, a dirty flag means the file is missing, + // and the build should stop. In the second, do not do anything here + // since there is no producing edge to add to the plan. + if (node->dirty() && !node->generated_by_dep_loader()) { + string referenced; + if (dependent) + referenced = ", needed by '" + dependent->path() + "',"; + *err = "'" + node->path() + "'" + referenced + + " missing and no known rule to make it"; + } + return false; } if (edge->outputs_ready()) diff --git a/src/build_test.cc b/src/build_test.cc index d32ad3e4c6..5ed8245e25 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -986,9 +986,19 @@ TEST_F(BuildTest, DepFileOK) { ASSERT_EQ(1u, fs_.files_read_.size()); EXPECT_EQ("foo.o.d", fs_.files_read_[0]); - // Expect three new edges: one generating foo.o, and two more from - // loading the depfile. - ASSERT_EQ(orig_edges + 3, (int)state_.edges_.size()); + // Expect one new edge generating foo.o. Loading the depfile should have + // added nodes, but not phony edges to the graph. + ASSERT_EQ(orig_edges + 1, (int)state_.edges_.size()); + + // Verify that nodes for blah.h and bar.h were added and that they + // are marked as generated by a dep loader. + ASSERT_FALSE(state_.LookupNode("foo.o")->generated_by_dep_loader()); + ASSERT_FALSE(state_.LookupNode("foo.c")->generated_by_dep_loader()); + ASSERT_TRUE(state_.LookupNode("blah.h")); + ASSERT_TRUE(state_.LookupNode("blah.h")->generated_by_dep_loader()); + ASSERT_TRUE(state_.LookupNode("bar.h")); + ASSERT_TRUE(state_.LookupNode("bar.h")->generated_by_dep_loader()); + // Expect our edge to now have three inputs: foo.c and two headers. ASSERT_EQ(3u, edge->inputs_.size()); @@ -1154,7 +1164,6 @@ TEST_F(BuildTest, DepFileCanonicalize) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" "build gen/stuff\\things/foo.o: cc x\\y/z\\foo.c\n")); - Edge* edge = state_.edges_.back(); fs_.Create("x/y/z/foo.c", ""); GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. @@ -1167,10 +1176,10 @@ TEST_F(BuildTest, DepFileCanonicalize) { // The depfile path does not get Canonicalize as it seems unnecessary. EXPECT_EQ("gen/stuff\\things/foo.o.d", fs_.files_read_[0]); - // Expect three new edges: one generating foo.o, and two more from - // loading the depfile. - ASSERT_EQ(orig_edges + 3, (int)state_.edges_.size()); + // Expect one new edge enerating foo.o. + ASSERT_EQ(orig_edges + 1, (int)state_.edges_.size()); // Expect our edge to now have three inputs: foo.c and two headers. + Edge* edge = state_.edges_.back(); ASSERT_EQ(3u, edge->inputs_.size()); // Expect the command line we generate to only use the original input, and @@ -2968,9 +2977,9 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); - // Expect three new edges: one generating fo o.o, and two more from - // loading the depfile. - ASSERT_EQ(3u, state.edges_.size()); + // Expect one new edge generating fo o.o, loading the depfile should + // note generate new edges. + ASSERT_EQ(1u, state.edges_.size()); // Expect our edge to now have three inputs: foo.c and two headers. ASSERT_EQ(3u, edge->inputs_.size()); @@ -3110,16 +3119,14 @@ TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { Builder builder(&state, config_, NULL, &deps_log, &fs_, &status_, 0); builder.command_runner_.reset(&command_runner_); - Edge* edge = state.edges_.back(); - state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. EXPECT_TRUE(builder.AddTarget("a/b/c/d/e/fo o.o", &err)); ASSERT_EQ("", err); - // Expect three new edges: one generating fo o.o, and two more from - // loading the depfile. - ASSERT_EQ(3u, state.edges_.size()); + // Expect one new edge generating fo o.o. + ASSERT_EQ(1u, state.edges_.size()); // Expect our edge to now have three inputs: foo.c and two headers. + Edge* edge = state.edges_.back(); ASSERT_EQ(3u, edge->inputs_.size()); // Expect the command line we generate to only use the original input. @@ -3675,8 +3682,8 @@ TEST_F(BuildTest, DyndepBuildDiscoverOutputAndDepfileInput) { 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()); + // Loading the depfile did not give tmp.imp a phony input edge. + ASSERT_FALSE(GetNode("tmp.imp")->in_edge()); EXPECT_TRUE(builder_.Build(&err)); EXPECT_EQ("", err); diff --git a/src/dyndep.cc b/src/dyndep.cc index dd4ed099a9..a0d699d5db 100644 --- a/src/dyndep.cc +++ b/src/dyndep.cc @@ -97,15 +97,10 @@ 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 (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(); + if ((*i)->in_edge()) { + // This node already has an edge producing it. + *err = "multiple rules generate " + (*i)->path(); + return false; } (*i)->set_in_edge(edge); } diff --git a/src/graph.cc b/src/graph.cc index 199294d481..62f13ec100 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -728,7 +728,6 @@ bool ImplicitDepLoader::ProcessDepfileDeps( Node* node = state_->GetNode(*i, slash_bits); *implicit_dep = node; node->AddOutEdge(edge); - CreatePhonyInEdge(node); } return true; @@ -756,7 +755,6 @@ bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, string* err) { Node* node = deps->nodes[i]; *implicit_dep = node; node->AddOutEdge(edge); - CreatePhonyInEdge(node); } return true; } @@ -768,21 +766,3 @@ vector::iterator ImplicitDepLoader::PreallocateSpace(Edge* edge, edge->implicit_deps_ += count; return edge->inputs_.end() - edge->order_only_deps_ - count; } - -void ImplicitDepLoader::CreatePhonyInEdge(Node* node) { - if (node->in_edge()) - 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); - - // RecomputeDirty might not be called for phony_edge if a previous call - // to RecomputeDirty had caused the file to be stat'ed. Because previous - // invocations of RecomputeDirty would have seen this node without an - // input edge (and therefore ready), we have to set outputs_ready_ to true - // to avoid a potential stuck build. If we do call RecomputeDirty for - // this node, it will simply set outputs_ready_ to the correct value. - phony_edge->outputs_ready_ = true; -} diff --git a/src/graph.h b/src/graph.h index d07a9b7639..5c8ca2c610 100644 --- a/src/graph.h +++ b/src/graph.h @@ -104,6 +104,14 @@ struct Node { Edge* in_edge() const { return in_edge_; } void set_in_edge(Edge* edge) { in_edge_ = edge; } + /// Indicates whether this node was generated from a depfile or dyndep file, + /// instead of being a regular input or output from the Ninja manifest. + bool generated_by_dep_loader() const { return generated_by_dep_loader_; } + + void set_generated_by_dep_loader(bool value) { + generated_by_dep_loader_ = value; + } + int id() const { return id_; } void set_id(int id) { id_ = id; } @@ -146,6 +154,13 @@ struct Node { /// has not yet been loaded. bool dyndep_pending_; + /// Set to true when this node comes from a depfile, a dyndep file or the + /// deps log. If it does not have a producing edge, the build should not + /// abort if it is missing (as for regular source inputs). By default + /// all nodes have this flag set to true, since the deps and build logs + /// can be loaded before the manifest. + bool generated_by_dep_loader_ = true; + /// The Edge that produces this Node, or NULL when there is no /// known edge to produce it. Edge* in_edge_; @@ -297,11 +312,6 @@ struct ImplicitDepLoader { /// an iterator pointing at the first new space. std::vector::iterator PreallocateSpace(Edge* edge, int count); - /// If we don't have a edge that generates this input already, - /// create one; this makes us not abort if the input is missing, - /// but instead will rebuild in that circumstance. - void CreatePhonyInEdge(Node* node); - State* state_; DiskInterface* disk_interface_; DepsLog* deps_log_; diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 8db6eb3009..103c365ebf 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -14,8 +14,10 @@ #include "manifest_parser.h" +#include #include #include + #include #include "graph.h" @@ -416,6 +418,7 @@ bool ManifestParser::ParseEdge(string* err) { if (dgi == edge->inputs_.end()) { return lexer_.Error("dyndep '" + dyndep + "' is not an input", err); } + assert(!edge->dyndep_->generated_by_dep_loader()); } return true; diff --git a/src/state.cc b/src/state.cc index 556b0d8802..0a68f2163a 100644 --- a/src/state.cc +++ b/src/state.cc @@ -128,6 +128,7 @@ Node* State::SpellcheckNode(const string& path) { void State::AddIn(Edge* edge, StringPiece path, uint64_t slash_bits) { Node* node = GetNode(path, slash_bits); + node->set_generated_by_dep_loader(false); edge->inputs_.push_back(node); node->AddOutEdge(edge); } @@ -138,6 +139,7 @@ bool State::AddOut(Edge* edge, StringPiece path, uint64_t slash_bits) { return false; edge->outputs_.push_back(node); node->set_in_edge(edge); + node->set_generated_by_dep_loader(false); return true; } @@ -145,6 +147,7 @@ void State::AddValidation(Edge* edge, StringPiece path, uint64_t slash_bits) { Node* node = GetNode(path, slash_bits); edge->validations_.push_back(node); node->AddValidationOutEdge(edge); + node->set_generated_by_dep_loader(false); } bool State::AddDefault(StringPiece path, string* err) { diff --git a/src/state.h b/src/state.h index 878ac6d991..886b78f765 100644 --- a/src/state.h +++ b/src/state.h @@ -105,6 +105,9 @@ struct State { Node* LookupNode(StringPiece path) const; Node* SpellcheckNode(const std::string& path); + /// Add input / output / validation nodes to a given edge. This also + /// ensures that the generated_by_dep_loader() flag for all these nodes + /// is set to false, to indicate that they come from the input manifest. void AddIn(Edge* edge, StringPiece path, uint64_t slash_bits); bool AddOut(Edge* edge, StringPiece path, uint64_t slash_bits); void AddValidation(Edge* edge, StringPiece path, uint64_t slash_bits);