Fix restat builds with edges generating headers depended on through deps files
authorNico Weber <thakis@chromium.org>
Mon, 3 Jun 2013 23:37:57 +0000 (16:37 -0700)
committerNico Weber <thakis@chromium.org>
Mon, 3 Jun 2013 23:37:57 +0000 (16:37 -0700)
in deps mode.

`ImplicitDepLoader::LoadDepFile()` already adds a depfile edge from every file
mentioned in a depfile to the depfile's output.

`ImplicitDepLoader::LoadDepsFromLog()` didn't do this yet, so add it. Else,
if a restat rule clears a generated .h file, this wouldn't propagate to cc files
depending on that .h file through a depfile.

Fixues issue #590.

src/build_test.cc
src/graph.cc

index 90c328a..ed9ade3 100644 (file)
@@ -1602,3 +1602,67 @@ TEST_F(BuildWithDepsLogTest, DepsIgnoredInDryRun) {
 
   builder.command_runner_.release();
 }
+
+/// Check that a restat rule generating a header cancels compilations correctly.
+TEST_F(BuildWithDepsLogTest, RestatDepfileDependency) {
+  string err;
+  // Note: in1 was created by the superclass SetUp().
+  const char* manifest =
+      "rule true\n"
+      "  command = true\n"  // Would be "write if out-of-date" in reality.
+      "  restat = 1\n"
+      "build header.h: true header.in\n"
+      "build out: cat in1\n"
+      "  deps = gcc\n"
+      "  depfile = in1.d\n";
+  {
+    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);
+    fs_.Create("in1.d", "out: header.h");
+    EXPECT_TRUE(builder.Build(&err));
+    EXPECT_EQ("", err);
+
+    deps_log.Close();
+    builder.command_runner_.release();
+  }
+
+  {
+    State state;
+    ASSERT_NO_FATAL_FAILURE(AddCatRule(&state));
+    ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest));
+
+    // Touch the input of the restat rule.
+    fs_.Tick();
+    fs_.Create("header.in", "");
+
+    // Run the build again.
+    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);
+    EXPECT_TRUE(builder.Build(&err));
+    EXPECT_EQ("", err);
+
+    // Rule "true" should have run again, but the build of "out" should have
+    // been cancelled due to restat propagating through the depfile header.
+    EXPECT_EQ(1u, command_runner_.commands_ran_.size());
+
+    builder.command_runner_.release();
+  }
+}
index b245e52..7a57753 100644 (file)
@@ -418,6 +418,7 @@ bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, TimeStamp* deps_mtime,
       PreallocateSpace(edge, deps->node_count);
   for (int i = 0; i < deps->node_count; ++i, ++implicit_dep) {
     *implicit_dep = deps->nodes[i];
+    deps->nodes[i]->AddOutEdge(edge);
     CreatePhonyInEdge(*implicit_dep);
   }
   return true;