From 714621dba1b8b2d8cd6080b7bc82955b44054734 Mon Sep 17 00:00:00 2001 From: ikifof Date: Fri, 27 Apr 2018 18:34:55 -0700 Subject: Adding a way to clean dead build artifacts that have an entry in the build log, but are no longer produced by the current manifest. For now adding a dedicated "-t cleandead" option, since it should be run after reading the log; ideally it should be part of the build config and done before to start looking for dirty targets so that an incremental build would produce the same end result as a clean build from scratch. But since I am not 100% sure to understand the comment in the NinjaMain::isPathDead(), I opted to make it a tool for now to avoid impacting users who want to keep those files. The option name "cleandead" was selected insteadof something like "reap" to keep the "clean" prefix. --- src/clean.cc | 13 +++++++++ src/clean.h | 5 ++++ src/clean_test.cc | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/ninja.cc | 11 ++++++-- 4 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/clean.cc b/src/clean.cc index d1f221d..ec6e7d7 100644 --- a/src/clean.cc +++ b/src/clean.cc @@ -124,6 +124,19 @@ int Cleaner::CleanAll(bool generator) { return status_; } +int Cleaner::CleanDead(const BuildLog::Entries& entries) { + Reset(); + PrintHeader(); + for (BuildLog::Entries::const_iterator i = entries.begin(); i != entries.end(); ++i) { + Node* n = state_->LookupNode(i->first); + if (!n || !n->in_edge()) { + Remove(i->first.AsString()); + } + } + PrintFooter(); + return status_; +} + void Cleaner::DoCleanTarget(Node* target) { if (Edge* e = target->in_edge()) { // Do not try to remove phony targets diff --git a/src/clean.h b/src/clean.h index d044fb1..4c02ff6 100644 --- a/src/clean.h +++ b/src/clean.h @@ -20,6 +20,7 @@ #include "build.h" #include "dyndep.h" +#include "build_log.h" using namespace std; @@ -58,6 +59,10 @@ struct Cleaner { /// Clean the file produced by the given @a rules. /// @return non-zero if an error occurs. int CleanRules(int rule_count, char* rules[]); + /// Clean the files produced by previous builds that are no longer in the + /// manifest. + /// @return non-zero if an error occurs. + int CleanDead(const BuildLog::Entries& entries); /// @return the number of file cleaned. int cleaned_files_count() const { diff --git a/src/clean_test.cc b/src/clean_test.cc index 45187f4..d068f3c 100644 --- a/src/clean_test.cc +++ b/src/clean_test.cc @@ -15,8 +15,17 @@ #include "clean.h" #include "build.h" +#include "util.h" #include "test.h" +#ifndef _WIN32 +#include +#endif + +namespace { + +const char kTestFilename[] = "CleanTest-tempfile"; + struct CleanTest : public StateTestWithBuiltinRules { VirtualFileSystem fs_; BuildConfig config_; @@ -454,3 +463,76 @@ TEST_F(CleanTest, CleanDepFileAndRspFileWithSpaces) { EXPECT_EQ(0, fs_.Stat("out 1.d", &err)); EXPECT_EQ(0, fs_.Stat("out 2.rsp", &err)); } + +struct CleanDeadTest : public CleanTest, public BuildLogUser{ + virtual void SetUp() { + // In case a crashing test left a stale file behind. + unlink(kTestFilename); + CleanTest::SetUp(); + } + virtual void TearDown() { + unlink(kTestFilename); + } + virtual bool IsPathDead(StringPiece) const { return false; } +}; + +TEST_F(CleanDeadTest, CleanDead) { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, +"rule cat\n" +" command = cat $in > $out\n" +"build out1: cat in\n" +"build out2: cat in\n" +)); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out2: cat in\n" +)); + fs_.Create("in", ""); + fs_.Create("out1", ""); + fs_.Create("out2", ""); + + BuildLog log1; + string err; + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); + ASSERT_EQ("", err); + log1.RecordCommand(state.edges_[0], 15, 18); + log1.RecordCommand(state.edges_[1], 20, 25); + log1.Close(); + + BuildLog log2; + EXPECT_TRUE(log2.Load(kTestFilename, &err)); + ASSERT_EQ("", err); + ASSERT_EQ(2u, log2.entries().size()); + ASSERT_TRUE(log2.LookupByOutput("out1")); + ASSERT_TRUE(log2.LookupByOutput("out2")); + + // First use the manifest that describe how to build out1. + Cleaner cleaner1(&state, config_, &fs_); + EXPECT_EQ(0, cleaner1.CleanDead(log2.entries())); + EXPECT_EQ(0, cleaner1.cleaned_files_count()); + EXPECT_EQ(0u, fs_.files_removed_.size()); + EXPECT_NE(0, fs_.Stat("in", &err)); + EXPECT_NE(0, fs_.Stat("out1", &err)); + EXPECT_NE(0, fs_.Stat("out2", &err)); + + // Then use the manifest that does not build out1 anymore. + Cleaner cleaner2(&state_, config_, &fs_); + EXPECT_EQ(0, cleaner2.CleanDead(log2.entries())); + EXPECT_EQ(1, cleaner2.cleaned_files_count()); + EXPECT_EQ(1u, fs_.files_removed_.size()); + EXPECT_EQ("out1", *(fs_.files_removed_.begin())); + EXPECT_NE(0, fs_.Stat("in", &err)); + EXPECT_EQ(0, fs_.Stat("out1", &err)); + EXPECT_NE(0, fs_.Stat("out2", &err)); + + // Nothing to do now. + EXPECT_EQ(0, cleaner2.CleanDead(log2.entries())); + EXPECT_EQ(0, cleaner2.cleaned_files_count()); + EXPECT_EQ(1u, fs_.files_removed_.size()); + EXPECT_EQ("out1", *(fs_.files_removed_.begin())); + EXPECT_NE(0, fs_.Stat("in", &err)); + EXPECT_EQ(0, fs_.Stat("out1", &err)); + EXPECT_NE(0, fs_.Stat("out2", &err)); + log2.Close(); +} +} // anonymous namespace diff --git a/src/ninja.cc b/src/ninja.cc index a093cd1..3b9fab5 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -123,6 +123,7 @@ struct NinjaMain : public BuildLogUser { int ToolTargets(const Options* options, int argc, char* argv[]); int ToolCommands(const Options* options, int argc, char* argv[]); int ToolClean(const Options* options, int argc, char* argv[]); + int ToolCleanDead(const Options* options, int argc, char* argv[]); int ToolCompilationDatabase(const Options* options, int argc, char* argv[]); int ToolRecompact(const Options* options, int argc, char* argv[]); int ToolUrtle(const Options* options, int argc, char** argv); @@ -153,9 +154,6 @@ struct NinjaMain : public BuildLogUser { void DumpMetrics(); virtual bool IsPathDead(StringPiece s) const { - Node* n = state_.LookupNode(s); - if (!n || !n->in_edge()) - return false; // Just checking n isn't enough: If an old output is both in the build log // and in the deps log, it will have a Node object in state_. (It will also // have an in edge if one of its inputs is another output that's in the deps @@ -719,6 +717,11 @@ int NinjaMain::ToolClean(const Options* options, int argc, char* argv[]) { } } +int NinjaMain::ToolCleanDead(const Options* options, int argc, char* argv[]) { + Cleaner cleaner(&state_, config_, &disk_interface_); + return cleaner.CleanDead(build_log_.entries()); +} + void EncodeJSONString(const char *str) { while (*str) { if (*str == '"' || *str == '\\') @@ -893,6 +896,8 @@ const Tool* ChooseTool(const string& tool_name) { Tool::RUN_AFTER_LOAD, &NinjaMain::ToolRecompact }, { "rules", "list all rules", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolRules }, + { "cleandead", "clean built files that are no longer produced by the manifest", + Tool::RUN_AFTER_LOGS, &NinjaMain::ToolCleanDead }, { "urtle", NULL, Tool::RUN_AFTER_FLAGS, &NinjaMain::ToolUrtle }, { NULL, NULL, Tool::RUN_AFTER_FLAGS, NULL } -- cgit v0.12