Add a test for collection of dead deps entries.

2 files changed, 52 insertions, 1 deletions

diff --git a/src/deps_log.cc b/src/deps_log.cc
index 0a3f7e5..157b65c 100644
--- a/src/deps_log.cc
+++ b/src/deps_log.cc
@@ -329,6 +329,12 @@ bool DepsLog::Recompact(const string& path, string* err) {
     Edge* e = n->in_edge();
     // FIXME: move this condition to a helper: (also used in src/ninja.cc)
     if (!e || e->GetBinding("deps").empty()) {
+      // Skip entries that don't have in-edges or whose edges don't have a
+      // "deps" attribute. They were in the deps log from previous builds, but
+      // the the files they were for were removed from the build and their deps
+      // entries are no longer needed.
+      // (Without the check for "deps", a chain of two or more nodes that each
+      // had deps wouldn't be collected in a single recompaction.)
       continue;
     }
 
diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc
index 3304edb..e8e5138 100644
--- a/src/deps_log_test.cc
+++ b/src/deps_log_test.cc
@@ -224,6 +224,7 @@ TEST_F(DepsLogTest, Recompact) {
 
   // Now reload the file, verify the new deps have replaced the old, then
   // recompact.
+  int file_size_3;
   {
     State state;
     ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest));
@@ -267,9 +268,53 @@ TEST_F(DepsLogTest, Recompact) {
     // The file should have shrunk a bit for the smaller deps.
     struct stat st;
     ASSERT_EQ(0, stat(kTestFilename, &st));
-    int file_size_3 = (int)st.st_size;
+    file_size_3 = (int)st.st_size;
     ASSERT_LT(file_size_3, file_size_2);
   }
+
+  // Now reload the file and recompact with an empty manifest. The previous
+  // entries should be removed.
+  {
+    State state;
+    // Intentionally not parsing kManifest here.
+    DepsLog log;
+    string err;
+    ASSERT_TRUE(log.Load(kTestFilename, &state, &err));
+
+    Node* out = state.GetNode("out.o");
+    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");
+    deps = log.GetDeps(other_out);
+    ASSERT_TRUE(deps);
+    ASSERT_EQ(1, deps->mtime);
+    ASSERT_EQ(2, deps->node_count);
+    ASSERT_EQ("foo.h", deps->nodes[0]->path());
+    ASSERT_EQ("baz.h", deps->nodes[1]->path());
+
+    ASSERT_TRUE(log.Recompact(kTestFilename, &err));
+
+    // The previous entries should have been removed.
+    deps = log.GetDeps(out);
+    ASSERT_FALSE(deps);
+
+    deps = log.GetDeps(other_out);
+    ASSERT_FALSE(deps);
+
+    // The .h files pulled in via deps should no longer have ids either.
+    ASSERT_EQ(-1, state.LookupNode("foo.h")->id());
+    ASSERT_EQ(-1, state.LookupNode("baz.h")->id());
+
+    // The file should have shrunk more.
+    struct stat st;
+    ASSERT_EQ(0, stat(kTestFilename, &st));
+    int file_size_4 = (int)st.st_size;
+    ASSERT_LT(file_size_4, file_size_3);
+  }
 }
 
 // Verify that invalid file headers cause a new build.