diff --git a/src/build.cc b/src/build.cc index 7ad08117be..f30b8f3a38 100644 --- a/src/build.cc +++ b/src/build.cc @@ -825,6 +825,17 @@ bool Builder::StartEdge(Edge* edge, string* err) { if (edge->is_phony()) return true; + // Late validation: verify that dyndep‑only inputs are present. + for (const auto input : edge->inputs_) { + if (input->missing() && input->generated_by_dyndep_loader()) { + string referenced; + referenced = ", needed by dyndep '" + edge->GetUnescapedDyndep() + "',"; + *err = "'" + input->path() + "'" + referenced + + " missing and no known rule to make it"; + return false; + } + } + int64_t start_time_millis = GetTimeMillis() - start_time_millis_; running_edges_.insert(make_pair(edge, start_time_millis)); @@ -943,6 +954,13 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { } } } + + // Refresh output file status to support later dyndep existence checks. + // https://github.com/ninja-build/ninja/issues/2573 + for (auto o : edge->outputs_) { + o->MarkExists(); + } + if (node_cleaned) { record_mtime = edge->command_start_time_; } @@ -998,7 +1016,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, // all backslashes (as some of the slashes will certainly be backslashes // anyway). This could be fixed if necessary with some additional // complexity in IncludesNormalize::Relativize. - deps_nodes->push_back(state_->GetNode(*i, ~0u)); + deps_nodes->push_back(state_->FindOrCreateDepfileNode(*i, ~0u)); } } else if (deps_type == "gcc") { string depfile = result->edge->GetUnescapedDepfile(); @@ -1031,7 +1049,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, i != deps.ins_.end(); ++i) { uint64_t slash_bits; CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits); - deps_nodes->push_back(state_->GetNode(*i, slash_bits)); + deps_nodes->push_back(state_->FindOrCreateDepfileNode(*i, slash_bits)); } if (!g_keep_depfile) { diff --git a/src/build_test.cc b/src/build_test.cc index 5ba87faa1e..f23c3f57a6 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1044,6 +1044,7 @@ TEST_F(BuildTest, DepFileOK) { Edge* edge = state_.edges_.back(); fs_.Create("foo.c", ""); + state_.FindOrCreateDepfileNode("bar.h",0); // create node 'bar.h' GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. fs_.Create("foo.o.d", "foo.o: blah.h bar.h\n"); EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); @@ -1231,7 +1232,8 @@ TEST_F(BuildTest, DepFileCanonicalize) { "build gen/stuff\\things/foo.o: cc x\\y/z\\foo.c\n")); fs_.Create("x/y/z/foo.c", ""); - GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. + state_.FindOrCreateDyndepNode("bar.h",0); // create node 'bar.h' + GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. // Note, different slashes from manifest. fs_.Create("gen/stuff\\things/foo.o.d", "gen\\stuff\\things\\foo.o: blah.h bar.h\n"); @@ -1969,7 +1971,7 @@ TEST_F(BuildWithLogTest, RestatInputChangesDueToRule) { // mtime EXPECT_TRUE(builder_.AddTarget("out1", &err)); ASSERT_EQ("", err); - EXPECT_TRUE(!state_.GetNode("out1", 0)->dirty()); + EXPECT_TRUE(!state_.GetNode("out1")->dirty()); EXPECT_EQ(builder_.Build(&err), ExitSuccess); ASSERT_EQ("", err); EXPECT_EQ(size_t(1), command_runner_.commands_ran_.size()); @@ -3068,7 +3070,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Edge* edge = state.edges_.back(); - state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. + state.FindOrCreateDepfileNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); @@ -3214,7 +3216,7 @@ TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { builder.command_runner_.reset(&command_runner_); SafeRelease protect(&builder); - state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. + state.FindOrCreateDyndepNode("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); @@ -3365,6 +3367,61 @@ TEST_F(BuildTest, DyndepMissingAndNoRule) { EXPECT_EQ("loading 'dd': No such file or directory", err); } +TEST_F(BuildTest, DyndepMissingInput) { + // Check that the build system detects when a dyndep-provided input is missing + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, + R"ninja( +rule touch + command = touch $out $out.imp +build out: touch || dd + dyndep = dd +)ninja")); + + fs_.Create("dd", R"ninja( +ninja_dyndep_version = 1 +build out | out.imp: dyndep | tmp.imp missing.imp +)ninja"); + + fs_.Create("tmp.imp", ""); + + string err; + EXPECT_TRUE(builder_.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_EQ(builder_.Build(&err), ExitFailure); + EXPECT_EQ("'missing.imp', needed by dyndep 'dd', missing and no known rule to make it", err); + EXPECT_TRUE(command_runner_.commands_ran_.empty()); +} + +TEST_F(BuildTest, DyndepMissingInputUsedInDepFile) { + // The missing input cannot be detected because the dyndep‑introduced node is + // also marked as coming from a depfile. + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, + R"ninja( +rule touch + command = touch $out $out.imp +build out2: touch out1 || dd + dyndep = dd +build out1: touch + depfile = out2.d +)ninja")); + + fs_.Create("dd", R"ninja( +ninja_dyndep_version = 1 +build out2 | out2.imp: dyndep | tmp.imp missing.imp +)ninja"); + fs_.Create("out2.d", "out1: missing.imp\n"); + fs_.Create("tmp.imp", ""); + + string err; + EXPECT_TRUE(builder_.AddTarget("out2", &err)); + ASSERT_EQ("", err); + EXPECT_EQ(builder_.Build(&err), ExitSuccess); + EXPECT_EQ("", err); + ASSERT_EQ(2u, command_runner_.commands_ran_.size()); + EXPECT_EQ("touch out1 out1.imp", command_runner_.commands_ran_[0]); + EXPECT_EQ("touch out2 out2.imp", command_runner_.commands_ran_[1]); +} + TEST_F(BuildTest, DyndepReadyImplicitConnection) { // Verify that a dyndep file can be loaded immediately to discover // that one edge has an implicit output that is also an implicit diff --git a/src/deps_log.cc b/src/deps_log.cc index a42b5eb42a..1cf9750d57 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -252,7 +252,7 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) { // have a correct slash_bits that GetNode will look up), or it is an // implicit dependency from a .d which does not affect the build command // (and so need not have its slashes maintained). - Node* node = state->GetNode(subpath, 0); + Node* node = state->FindOrCreateDepfileNode(subpath, 0); // Check that the expected index matches the actual index. This can only // happen if two ninja processes write to the same deps log concurrently. diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 62192b099e..0d61755216 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -47,16 +47,16 @@ TEST_F(DepsLogTest, WriteRead) { { vector deps; - deps.push_back(state1.GetNode("foo.h", 0)); - deps.push_back(state1.GetNode("bar.h", 0)); - log1.RecordDeps(state1.GetNode("out.o", 0), 1, deps); + deps.push_back(state1.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state1.FindOrCreateDepfileNode("bar.h", 0)); + log1.RecordDeps(state1.FindOrCreateDepfileNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state1.GetNode("foo.h", 0)); - deps.push_back(state1.GetNode("bar2.h", 0)); - log1.RecordDeps(state1.GetNode("out2.o", 0), 2, deps); + deps.push_back(state1.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state1.FindOrCreateDepfileNode("bar2.h", 0)); + log1.RecordDeps(state1.FindOrCreateDepfileNode("out2.o", 0), 2, deps); - DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o", 0)); + DepsLog::Deps* log_deps = log1.GetDeps(state1.FindOrCreateDepfileNode("out.o", 0)); ASSERT_TRUE(log_deps); ASSERT_EQ(1, log_deps->mtime); ASSERT_EQ(2, log_deps->node_count); @@ -80,7 +80,7 @@ TEST_F(DepsLogTest, WriteRead) { } // Spot-check the entries in log2. - DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out2.o", 0)); + DepsLog::Deps* log_deps = log2.GetDeps(state2.FindOrCreateDepfileNode("out2.o", 0)); ASSERT_TRUE(log_deps); ASSERT_EQ(2, log_deps->mtime); ASSERT_EQ(2, log_deps->node_count); @@ -102,11 +102,11 @@ TEST_F(DepsLogTest, LotsOfDeps) { for (int i = 0; i < kNumDeps; ++i) { char buf[32]; sprintf(buf, "file%d.h", i); - deps.push_back(state1.GetNode(buf, 0)); + deps.push_back(state1.FindOrCreateDepfileNode(buf, 0)); } - log1.RecordDeps(state1.GetNode("out.o", 0), 1, deps); + log1.RecordDeps(state1.FindOrCreateDepfileNode("out.o", 0), 1, deps); - DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o", 0)); + DepsLog::Deps* log_deps = log1.GetDeps(state1.FindOrCreateDepfileNode("out.o", 0)); ASSERT_EQ(kNumDeps, log_deps->node_count); } @@ -117,7 +117,7 @@ TEST_F(DepsLogTest, LotsOfDeps) { EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); ASSERT_EQ("", err); - DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out.o", 0)); + DepsLog::Deps* log_deps = log2.GetDeps(state2.FindOrCreateDepfileNode("out.o", 0)); ASSERT_EQ(kNumDeps, log_deps->node_count); } @@ -133,9 +133,9 @@ TEST_F(DepsLogTest, DoubleEntry) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); log.Close(); #ifdef __USE_LARGEFILE64 struct stat64 st; @@ -159,9 +159,9 @@ TEST_F(DepsLogTest, DoubleEntry) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); log.Close(); #ifdef __USE_LARGEFILE64 struct stat64 st; @@ -195,14 +195,14 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("baz.h", 0)); - log.RecordDeps(state.GetNode("other_out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("baz.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("other_out.o", 0), 1, deps); log.Close(); #ifdef __USE_LARGEFILE64 @@ -229,8 +229,8 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); log.Close(); #ifdef __USE_LARGEFILE64 @@ -255,14 +255,14 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - Node* out = state.GetNode("out.o", 0); + Node* out = state.FindOrCreateDepfileNode("out.o", 0); DepsLog::Deps* deps = log.GetDeps(out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); ASSERT_EQ(1, deps->node_count); ASSERT_EQ("foo.h", deps->nodes[0]->path()); - Node* other_out = state.GetNode("other_out.o", 0); + Node* other_out = state.FindOrCreateDepfileNode("other_out.o", 0); deps = log.GetDeps(other_out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); @@ -309,14 +309,14 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - Node* out = state.GetNode("out.o", 0); + Node* out = state.FindOrCreateDepfileNode("out.o", 0); DepsLog::Deps* deps = log.GetDeps(out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); ASSERT_EQ(1, deps->node_count); ASSERT_EQ("foo.h", deps->nodes[0]->path()); - Node* other_out = state.GetNode("other_out.o", 0); + Node* other_out = state.FindOrCreateDepfileNode("other_out.o", 0); deps = log.GetDeps(other_out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); @@ -387,14 +387,14 @@ TEST_F(DepsLogTest, Truncated) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar2.h", 0)); - log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar2.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out2.o", 0), 2, deps); log.Close(); } @@ -451,14 +451,14 @@ TEST_F(DepsLogTest, TruncatedRecovery) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar2.h", 0)); - log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar2.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out2.o", 0), 2, deps); log.Close(); } @@ -486,16 +486,16 @@ TEST_F(DepsLogTest, TruncatedRecovery) { err.clear(); // The truncated entry should've been discarded. - EXPECT_EQ(NULL, log.GetDeps(state.GetNode("out2.o", 0))); + EXPECT_EQ(NULL, log.GetDeps(state.FindOrCreateDepfileNode("out2.o", 0))); EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); // Add a new entry. vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar2.h", 0)); - log.RecordDeps(state.GetNode("out2.o", 0), 3, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar2.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out2.o", 0), 3, deps); log.Close(); } @@ -509,7 +509,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); // The truncated entry should exist. - DepsLog::Deps* deps = log.GetDeps(state.GetNode("out2.o", 0)); + DepsLog::Deps* deps = log.GetDeps(state.FindOrCreateDepfileNode("out2.o", 0)); ASSERT_TRUE(deps); } } @@ -522,23 +522,23 @@ TEST_F(DepsLogTest, ReverseDepsNodes) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar.h", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h", 0)); - deps.push_back(state.GetNode("bar2.h", 0)); - log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.h", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar2.h", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out2.o", 0), 2, deps); log.Close(); - Node* rev_deps = log.GetFirstReverseDepsNode(state.GetNode("foo.h", 0)); - EXPECT_TRUE(rev_deps == state.GetNode("out.o", 0) || - rev_deps == state.GetNode("out2.o", 0)); + Node* rev_deps = log.GetFirstReverseDepsNode(state.FindOrCreateDepfileNode("foo.h", 0)); + EXPECT_TRUE(rev_deps == state.FindOrCreateDepfileNode("out.o", 0) || + rev_deps == state.FindOrCreateDepfileNode("out2.o", 0)); - rev_deps = log.GetFirstReverseDepsNode(state.GetNode("bar.h", 0)); - EXPECT_TRUE(rev_deps == state.GetNode("out.o", 0)); + rev_deps = log.GetFirstReverseDepsNode(state.FindOrCreateDepfileNode("bar.h", 0)); + EXPECT_TRUE(rev_deps == state.FindOrCreateDepfileNode("out.o", 0)); } TEST_F(DepsLogTest, MalformedDepsLog) { @@ -551,9 +551,9 @@ TEST_F(DepsLogTest, MalformedDepsLog) { // First, create a valid log file. std::vector deps; - deps.push_back(state.GetNode("foo.hh", 0)); - deps.push_back(state.GetNode("bar.hpp", 0)); - log.RecordDeps(state.GetNode("out.o", 0), 1, deps); + deps.push_back(state.FindOrCreateDepfileNode("foo.hh", 0)); + deps.push_back(state.FindOrCreateDepfileNode("bar.hpp", 0)); + log.RecordDeps(state.FindOrCreateDepfileNode("out.o", 0), 1, deps); log.Close(); } diff --git a/src/dyndep_parser.cc b/src/dyndep_parser.cc index f35f48d92d..f6bc298061 100644 --- a/src/dyndep_parser.cc +++ b/src/dyndep_parser.cc @@ -201,7 +201,7 @@ bool DyndepParser::ParseEdge(string* err) { return lexer_.Error("empty path", err); uint64_t slash_bits; CanonicalizePath(&path, &slash_bits); - Node* n = state_->GetNode(path, slash_bits); + Node* n = state_->FindOrCreateDyndepNode(path, slash_bits); dyndeps->implicit_inputs_.push_back(n); } @@ -212,7 +212,7 @@ bool DyndepParser::ParseEdge(string* err) { return lexer_.Error("empty path", err); uint64_t slash_bits; CanonicalizePath(&path, &slash_bits); - Node* n = state_->GetNode(path, slash_bits); + Node* n = state_->FindOrCreateDyndepNode(path, slash_bits); dyndeps->implicit_outputs_.push_back(n); } diff --git a/src/graph.cc b/src/graph.cc index 227c615822..cb0e033bf9 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -718,7 +718,7 @@ bool ImplicitDepLoader::ProcessDepfileDeps( i != depfile_ins->end(); ++i, ++implicit_dep) { uint64_t slash_bits; CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits); - Node* node = state_->GetNode(*i, slash_bits); + Node* node = state_->FindOrCreateDepfileNode(*i, slash_bits); *implicit_dep = node; node->AddOutEdge(edge); } diff --git a/src/graph.h b/src/graph.h index 4812caf0c4..9ddefad25c 100644 --- a/src/graph.h +++ b/src/graph.h @@ -40,8 +40,10 @@ struct State; /// Information about a node in the dependency graph: the file, whether /// it's dirty, mtime, etc. struct Node { - Node(const std::string& path, uint64_t slash_bits) - : path_(path), slash_bits_(slash_bits) {} + enum class Loader : char { Manifest, Depfile, Dyndep }; + + Node(const std::string& path, uint64_t slash_bits, Loader loader) + : path_(path), slash_bits_(slash_bits), loader_(loader) {} /// Return false on error. bool Stat(DiskInterface* disk_interface, std::string* err); @@ -75,6 +77,14 @@ struct Node { return exists_ == ExistenceStatusExists; } + void MarkExists() { + exists_ = ExistenceStatusExists; + } + + bool missing() const { + return exists_ == ExistenceStatusMissing; + } + bool status_known() const { return exists_ != ExistenceStatusUnknown; } @@ -102,10 +112,22 @@ struct Node { /// 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_; } + bool generated_by_dep_loader() const { + return Loader::Manifest != loader_; + } + + bool generated_by_dyndep_loader() const { + return Loader::Dyndep == loader_; + } + + void set_generated_by_manifest() { + loader_ = Loader::Manifest; + } - void set_generated_by_dep_loader(bool value) { - generated_by_dep_loader_ = value; + /// A depfile overrides a dyndep-generated value. + void set_generated_by_depfile() { + if (loader_ == Loader::Dyndep) + loader_ = Loader::Depfile; } int id() const { return id_; } @@ -150,12 +172,18 @@ struct Node { /// has not yet been loaded. bool dyndep_pending_ = false; - /// 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; + /// Tracks how this node was loaded: from a depfile, deps log, dyndep file, or + /// the manifest. A depfile overrides dyndep, the manifest always takes + /// precedence. Depfile and deps log share the same status and are not tracked + /// separately. + /// + /// If node is loaded from depfile or deps log and do not have a producing + /// edge, the build should not abort if it is missing (as for regular source + /// inputs or inputs loaded from dyndep). + /// + /// There is no implicit default, callers must explicitly set an initial + /// value. + Loader loader_; /// A dense integer id for the node, assigned and used by DepsLog. int id_ = -1; @@ -224,7 +252,6 @@ struct Edge { bool outputs_ready_ = false; bool deps_loaded_ = false; bool deps_missing_ = false; - bool generated_by_dep_loader_ = false; TimeStamp command_start_time_ = 0; const Rule& rule() const { return *rule_; } diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 9505753fe4..cc6c713a86 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -403,7 +403,7 @@ bool ManifestParser::ParseEdge(string* err) { if (!dyndep.empty()) { uint64_t slash_bits; CanonicalizePath(&dyndep, &slash_bits); - edge->dyndep_ = state_->GetNode(dyndep, slash_bits); + edge->dyndep_ = state_->FindOrCreateManifestNode(dyndep, slash_bits); edge->dyndep_->set_dyndep_pending(true); vector::iterator dgi = std::find(edge->inputs_.begin(), edge->inputs_.end(), edge->dyndep_); diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 03ce0b1b80..eeac196b1b 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -86,7 +86,7 @@ TEST_F(ParserTest, IgnoreIndentedComments) { ASSERT_EQ(2u, state.bindings_.GetRules().size()); const auto& rule = state.bindings_.GetRules().begin()->second; EXPECT_EQ("cat", rule->name()); - Edge* edge = state.GetNode("result", 0)->in_edge(); + Edge* edge = state.GetNode("result")->in_edge(); EXPECT_TRUE(edge->GetBindingBool("restat")); EXPECT_FALSE(edge->GetBindingBool("generator")); } @@ -1072,7 +1072,7 @@ TEST_F(ParserTest, DyndepNotSpecified) { "rule cat\n" " command = cat $in > $out\n" "build result: cat in\n")); - Edge* edge = state.GetNode("result", 0)->in_edge(); + Edge* edge = state.GetNode("result")->in_edge(); ASSERT_FALSE(edge->dyndep_); } @@ -1095,7 +1095,7 @@ TEST_F(ParserTest, DyndepExplicitInput) { " command = cat $in > $out\n" "build result: cat in\n" " dyndep = in\n")); - Edge* edge = state.GetNode("result", 0)->in_edge(); + Edge* edge = state.GetNode("result")->in_edge(); ASSERT_TRUE(edge->dyndep_); EXPECT_TRUE(edge->dyndep_->dyndep_pending()); EXPECT_EQ(edge->dyndep_->path(), "in"); @@ -1107,7 +1107,7 @@ TEST_F(ParserTest, DyndepImplicitInput) { " command = cat $in > $out\n" "build result: cat in | dd\n" " dyndep = dd\n")); - Edge* edge = state.GetNode("result", 0)->in_edge(); + Edge* edge = state.GetNode("result")->in_edge(); ASSERT_TRUE(edge->dyndep_); EXPECT_TRUE(edge->dyndep_->dyndep_pending()); EXPECT_EQ(edge->dyndep_->path(), "dd"); @@ -1119,7 +1119,7 @@ TEST_F(ParserTest, DyndepOrderOnlyInput) { " command = cat $in > $out\n" "build result: cat in || dd\n" " dyndep = dd\n")); - Edge* edge = state.GetNode("result", 0)->in_edge(); + Edge* edge = state.GetNode("result")->in_edge(); ASSERT_TRUE(edge->dyndep_); EXPECT_TRUE(edge->dyndep_->dyndep_pending()); EXPECT_EQ(edge->dyndep_->path(), "dd"); @@ -1131,7 +1131,7 @@ TEST_F(ParserTest, DyndepRuleInput) { " command = cat $in > $out\n" " dyndep = $in\n" "build result: cat in\n")); - Edge* edge = state.GetNode("result", 0)->in_edge(); + Edge* edge = state.GetNode("result")->in_edge(); ASSERT_TRUE(edge->dyndep_); EXPECT_TRUE(edge->dyndep_->dyndep_pending()); EXPECT_EQ(edge->dyndep_->path(), "in"); diff --git a/src/missing_deps.cc b/src/missing_deps.cc index 6f3bd0e623..f80dfdf900 100644 --- a/src/missing_deps.cc +++ b/src/missing_deps.cc @@ -53,7 +53,7 @@ bool NodeStoringImplicitDepLoader::ProcessDepfileDeps( i != depfile_ins->end(); ++i) { uint64_t slash_bits; CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits); - Node* node = state_->GetNode(*i, slash_bits); + Node* node = state_->FindOrCreateDepfileNode(*i, slash_bits); dep_nodes_output_->push_back(node); } return true; diff --git a/src/state.cc b/src/state.cc index a8a1482d39..8c89755fc5 100644 --- a/src/state.cc +++ b/src/state.cc @@ -92,15 +92,39 @@ Edge* State::AddEdge(const Rule* rule) { return edge; } -Node* State::GetNode(StringPiece path, uint64_t slash_bits) { +Node* State::GetNode(StringPiece path) const { + Node* node = LookupNode(path); + assert(node); + return node; +} + +Node* State::FindOrCreateNode(StringPiece path, uint64_t slash_bits, + Node::Loader Loader) { Node* node = LookupNode(path); if (node) return node; - node = new Node(path.AsString(), slash_bits); + + node = new Node(path.AsString(), slash_bits, Loader); paths_[node->path()] = node; return node; } +Node* State::FindOrCreateManifestNode(StringPiece path, uint64_t slash_bits) { + auto node = FindOrCreateNode(path, slash_bits, Node::Loader::Manifest); + node->set_generated_by_manifest(); // Manifest wins + return node; +} + +Node* State::FindOrCreateDepfileNode(StringPiece path, uint64_t slash_bits) { + auto node = FindOrCreateNode(path, slash_bits, Node::Loader::Depfile); + node->set_generated_by_depfile(); + return node; +} + +Node* State::FindOrCreateDyndepNode(StringPiece path, uint64_t slash_bits) { + return FindOrCreateNode(path, slash_bits, Node::Loader::Dyndep); +} + Node* State::LookupNode(StringPiece path) const { Paths::const_iterator i = paths_.find(path); if (i != paths_.end()) @@ -126,15 +150,14 @@ 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); + Node* node = FindOrCreateManifestNode(path, slash_bits); edge->inputs_.push_back(node); node->AddOutEdge(edge); } bool State::AddOut(Edge* edge, StringPiece path, uint64_t slash_bits, std::string* err) { - Node* node = GetNode(path, slash_bits); + Node* node = FindOrCreateManifestNode(path, slash_bits); if (Edge* other = node->in_edge()) { if (other == edge) { *err = path.AsString() + " is defined as an output multiple times"; @@ -145,15 +168,13 @@ bool State::AddOut(Edge* edge, StringPiece path, uint64_t slash_bits, } edge->outputs_.push_back(node); node->set_in_edge(edge); - node->set_generated_by_dep_loader(false); return true; } void State::AddValidation(Edge* edge, StringPiece path, uint64_t slash_bits) { - Node* node = GetNode(path, slash_bits); + Node* node = FindOrCreateManifestNode(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 13e0e818d4..107a0f7557 100644 --- a/src/state.h +++ b/src/state.h @@ -103,13 +103,37 @@ struct State { Edge* AddEdge(const Rule* rule); - Node* GetNode(StringPiece path, uint64_t slash_bits); + /// For use in unit tests only; the node must already exist. + /// Returns nullptr if no such node is found. + Node* GetNode(StringPiece path) const; + + // Convenience helpers for creating or retrieving nodes with a specific + // loader. + + /// Returns a node loaded by Manifest. Creates the node if it does not exist. + Node* FindOrCreateManifestNode(StringPiece path, uint64_t slash_bits); + + /// Creates the node loaded by Depfile if it does not exist. + /// If the node already exists and its loader is Dyndep, it is updated to + /// Depfile. Nodes already loaded by Manifest or Depfile remain unchanged. + Node* FindOrCreateDepfileNode(StringPiece path, uint64_t slash_bits); + + /// Creates the node loaded by Depfile if it does not exist. + /// If the node already exists, its loader remains unchanged. + Node* FindOrCreateDyndepNode(StringPiece path, uint64_t slash_bits); + + private: + /// Creates the node with the given arguments if it does not already exist. + /// If the node exists, returns it unchanged. + Node* FindOrCreateNode(StringPiece path, uint64_t slash_bits, + Node::Loader Loader); + + public: 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. + /// Add input / output / validation nodes to a given edge. This also ensures + /// that the loader is explicitly marked as originating from the input manifest. void AddIn(Edge* edge, StringPiece path, uint64_t slash_bits); bool AddOut(Edge* edge, StringPiece path, uint64_t slash_bits, std::string* err); void AddValidation(Edge* edge, StringPiece path, uint64_t slash_bits); diff --git a/src/state_test.cc b/src/state_test.cc index 9c1af1c95c..731362d4af 100644 --- a/src/state_test.cc +++ b/src/state_test.cc @@ -40,9 +40,9 @@ TEST(State, Basic) { EXPECT_EQ("cat in1 in2 > out", edge->EvaluateCommand()); - EXPECT_FALSE(state.GetNode("in1", 0)->dirty()); - EXPECT_FALSE(state.GetNode("in2", 0)->dirty()); - EXPECT_FALSE(state.GetNode("out", 0)->dirty()); + EXPECT_FALSE(state.GetNode("in1")->dirty()); + EXPECT_FALSE(state.GetNode("in2")->dirty()); + EXPECT_FALSE(state.GetNode("out")->dirty()); } } // namespace diff --git a/src/test.cc b/src/test.cc index aab6996e3d..0d5db58e5f 100644 --- a/src/test.cc +++ b/src/test.cc @@ -92,9 +92,9 @@ void StateTestWithBuiltinRules::AddCatRule(State* state) { " command = cat $in > $out\n"); } -Node* StateTestWithBuiltinRules::GetNode(const string& path) { +Node* StateTestWithBuiltinRules::GetNode(const string& path) const { EXPECT_FALSE(strpbrk(path.c_str(), "/\\")); - return state_.GetNode(path, 0); + return state_.GetNode(path); } void AssertParse(State* state, const char* input, diff --git a/src/test.h b/src/test.h index 993e55a27a..b7fb66dfd8 100644 --- a/src/test.h +++ b/src/test.h @@ -35,7 +35,7 @@ struct StateTestWithBuiltinRules : public testing::Test { void AddCatRule(State* state); /// Short way to get a Node by its path from state_. - Node* GetNode(const std::string& path); + Node* GetNode(const std::string& path) const; State state_; };