From 1eaacc8ccf26363ae3a820675721164873a31f3d Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 30 Apr 2013 09:11:09 -0700 Subject: Keep a DepsLog's data valid after a call to DepsLog::Recompact(). Previously, a DepsLog would become invalid after Recompact() was called, due to Recompact() making all node ids refer to a temporary DepsLog object constructed in Recompact(). --- src/deps_log.cc | 13 ++++++++----- src/deps_log_test.cc | 29 +++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index fd7d107..7dbd5b2 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -250,12 +250,11 @@ bool DepsLog::Recompact(const string& path, string* err) { if (!new_log.OpenForWrite(temp_path, err)) return false; - // Clear all known ids so that new ones can be reassigned. - for (vector::iterator i = nodes_.begin(); - i != nodes_.end(); ++i) { + // Clear all known ids so that new ones can be reassigned. The new indices + // will refer to the ordering in new_log, not in the current log. + for (vector::iterator i = nodes_.begin(); i != nodes_.end(); ++i) (*i)->set_id(-1); - } - + // Write out all deps again. for (int old_id = 0; old_id < (int)deps_.size(); ++old_id) { Deps* deps = deps_[old_id]; @@ -270,6 +269,10 @@ bool DepsLog::Recompact(const string& path, string* err) { new_log.Close(); + // All nodes now have ids that refer to new_log, so steal its data. + deps_.swap(new_log.deps_); + nodes_.swap(new_log.nodes_); + if (unlink(path.c_str()) < 0) { *err = strerror(errno); return false; diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 31f0414..0591736 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -188,18 +188,43 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - DepsLog::Deps* deps = log.GetDeps(state.GetNode("out.o")); + 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 in-memory deps graph should still be valid after recompaction. + 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()); + ASSERT_EQ(out, log.nodes()[out->id()]); + + 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_EQ(other_out, log.nodes()[other_out->id()]); + + // 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; - // The file should have shrunk a bit for the smaller deps. ASSERT_LT(file_size_3, file_size_2); } } -- cgit v0.12