diff options
author | Evan Martin <martine@danga.com> | 2013-04-09 04:43:49 (GMT) |
---|---|---|
committer | Evan Martin <martine@danga.com> | 2013-04-09 04:48:10 (GMT) |
commit | 0a2fc151976277d8c0319cdc4ee3b1932b673d91 (patch) | |
tree | 81c2f0d5319397870c1cd70e95051487b67d58d7 /src | |
parent | c78e1eea9a385340a1d47d5402f5a7c41f99c95d (diff) | |
download | Ninja-0a2fc151976277d8c0319cdc4ee3b1932b673d91.zip Ninja-0a2fc151976277d8c0319cdc4ee3b1932b673d91.tar.gz Ninja-0a2fc151976277d8c0319cdc4ee3b1932b673d91.tar.bz2 |
add a straightforward deps log test, fix the other one
The first test I wrote was wrong; write a simpler test that exercises
the "no failures" code paths, then fix the second test and the bugs it
exposed.
Diffstat (limited to 'src')
-rw-r--r-- | src/build.cc | 23 | ||||
-rw-r--r-- | src/build.h | 3 | ||||
-rw-r--r-- | src/build_test.cc | 91 |
3 files changed, 100 insertions, 17 deletions
diff --git a/src/build.cc b/src/build.cc index 5e874df..7de4b16 100644 --- a/src/build.cc +++ b/src/build.cc @@ -801,10 +801,12 @@ void Builder::FinishCommand(CommandRunner::Result* result) { // can fail, which makes the command fail from a build perspective. vector<Node*> deps_nodes; + TimeStamp deps_mtime = 0; string deps_type = edge->GetBinding("deps"); if (result->success() && !deps_type.empty()) { string extract_err; - if (!ExtractDeps(result, deps_type, &deps_nodes, &extract_err)) { + if (!ExtractDeps(result, deps_type, &deps_nodes, &deps_mtime, + &extract_err)) { if (!result->output.empty()) result->output.append("\n"); result->output.append(extract_err); @@ -875,10 +877,12 @@ void Builder::FinishCommand(CommandRunner::Result* result) { if (!deps_type.empty()) { assert(edge->outputs_.size() == 1 && "should have been rejected by parser"); Node* out = edge->outputs_[0]; - TimeStamp mtime = disk_interface_->Stat(out->path()); - // XXX we could reuse the restat logic to avoid a second stat, - // but in practice we only care about the single output. - scan_.deps_log()->RecordDeps(out, mtime, deps_nodes); + if (!deps_mtime) { + // On Windows there's no separate file to compare against; just reuse + // the output's mtime. + deps_mtime = disk_interface_->Stat(out->path()); + } + scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes); } } @@ -886,6 +890,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { bool Builder::ExtractDeps(CommandRunner::Result* result, const string& deps_type, vector<Node*>* deps_nodes, + TimeStamp* deps_mtime, string* err) { #ifdef _WIN32 if (deps_type == "msvc") { @@ -900,7 +905,13 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, if (deps_type == "gcc") { string depfile = result->edge->GetBinding("depfile"); if (depfile.empty()) { - *err = string("edge with deps=gcc but no depfile makes no sense\n"); + *err = string("edge with deps=gcc but no depfile makes no sense"); + return false; + } + + *deps_mtime = disk_interface_->Stat(depfile); + if (*deps_mtime <= 0) { + *err = string("unable to read depfile"); return false; } diff --git a/src/build.h b/src/build.h index fb5fd10..d5f01cd 100644 --- a/src/build.h +++ b/src/build.h @@ -177,7 +177,8 @@ struct Builder { private: bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, - vector<Node*>* deps_nodes, string* err); + vector<Node*>* deps_nodes, TimeStamp* deps_mtime, + string* err); DiskInterface* disk_interface_; DependencyScan scan_; diff --git a/src/build_test.cc b/src/build_test.cc index 7df742f..2a0fa0f 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1388,7 +1388,7 @@ TEST_F(BuildTest, FailedDepsParse) { // These deps will fail to parse, as they should only have one // path to the left of the colon. - fs_.Create("in1.d", "XXX YYY"); + fs_.Create("in1.d", "AAA BBB"); EXPECT_FALSE(builder_.Build(&err)); EXPECT_EQ("subcommand failed", err); @@ -1417,10 +1417,8 @@ struct BuildWithDepsLogTest : public BuildTest { void* builder_; }; -TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { - // Don't make use of the class's built-in Builder etc. so that we - // can construct objects with the DepsLog in place. - +/// Run a straightforwad build where the deps log is used. +TEST_F(BuildWithDepsLogTest, Straightforward) { string err; // Note: in1 was created by the superclass SetUp(). const char* manifest = @@ -1447,6 +1445,8 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { // The deps file should have been removed. EXPECT_EQ(0, fs_.Stat("in1.d")); + // Recreate it for the next step. + fs_.Create("in1.d", "out: in2"); deps_log.Close(); builder.command_runner_.release(); } @@ -1456,12 +1456,9 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - // Pretend that the build aborted before the deps were - // removed, leaving behind an obsolete .d file, but after - // the output was written. - fs_.Create("in1.d", "XXX"); + // Touch the file only mentioned in the deps. fs_.Tick(); - fs_.Create("out", ""); + fs_.Create("in2", ""); // Run the build again. DepsLog deps_log; @@ -1476,6 +1473,80 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { EXPECT_TRUE(builder.Build(&err)); EXPECT_EQ("", err); + // We should have rebuilt the output due to in2 being + // out of date. + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + + builder.command_runner_.release(); + } +} + +/// Verify that obsolete deps still cause a rebuild. +TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { + string err; + // Note: in1 was created by the superclass SetUp(). + const char* manifest = + "build out: cat in1\n" + " deps = gcc\n" + " depfile = in1.d\n"; + { + // Create the obsolete deps, then run a build to incorporate them. + // The idea is that the inputs/outputs are newer than the logged + // deps. + fs_.Create("in1.d", "out: "); + fs_.Tick(); + + fs_.Create("in1", ""); + + State state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + // Run the build once, everything should be ok. + DepsLog deps_log; + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + fs_.Create("out", ""); + // The deps file should have been removed. + EXPECT_EQ(0, fs_.Stat("in1.d")); + deps_log.Close(); + builder.command_runner_.release(); + } + + // Now we should be in a situation where in1/out2 both have recent + // timestamps but the deps are old. Verify we rebuild. + fs_.Tick(); + + { + State state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + + // Recreate the deps here just to prove the old recorded deps are + // the problem. + fs_.Create("in1.d", "out: "); + + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + // We should have rebuilt the output due to the deps being // out of date. EXPECT_EQ(1u, command_runner_.commands_ran_.size()); |