Ensure that edge outputs are >= their newest input
Otherwise they'll be dirty again during the next build. So they either
need to update their timestamps, or be marked with restat so that they
won't be considered dirty again.
Test: ninja_tests (run by build-prebuilts.sh)
Change-Id: Ie004cca58735f6576e1e2ea7a126dca6992e6eaf
diff --git a/src/build.cc b/src/build.cc
index aad4a3f..5a7221d 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -602,12 +602,23 @@
bool restat = edge->IsRestat();
vector<Node*> nodes_cleaned;
+ TimeStamp newest_input = 0;
+ for (vector<Node*>::iterator i = edge->inputs_.begin();
+ i != edge->inputs_.end() - edge->order_only_deps_; ++i) {
+ TimeStamp input_mtime = (*i)->mtime();
+ if (input_mtime == -1)
+ return false;
+ if (input_mtime > newest_input)
+ newest_input = input_mtime;
+ }
+
for (vector<Node*>::iterator o = edge->outputs_.begin();
o != edge->outputs_.end(); ++o) {
bool is_dir = false;
- TimeStamp new_mtime = disk_interface_->LStat((*o)->path(), &is_dir, err);
- if (new_mtime == -1)
+ TimeStamp old_mtime = (*o)->mtime();
+ if (!(*o)->LStat(disk_interface_, &is_dir, err))
return false;
+ TimeStamp new_mtime = (*o)->mtime();
if (config_.uses_phony_outputs) {
if (new_mtime == 0) {
if (!result->output.empty())
@@ -617,6 +628,14 @@
if (config_.missing_output_file_should_err) {
result->status = ExitFailure;
}
+ } else if (!restat && new_mtime < newest_input) {
+ if (!result->output.empty())
+ result->output.append("\n");
+ result->output.append("ninja: Missing `restat`? An output file is older than the most recent input: ");
+ result->output.append((*o)->path());
+ if (config_.old_output_should_err) {
+ result->status = ExitFailure;
+ }
}
if (is_dir) {
if (!result->output.empty())
@@ -630,8 +649,9 @@
}
if (new_mtime > output_mtime)
output_mtime = new_mtime;
- if ((*o)->mtime() == new_mtime && restat) {
+ if (old_mtime == new_mtime && restat) {
nodes_cleaned.push_back(*o);
+ continue;
}
}
@@ -647,22 +667,9 @@
return false;
}
- TimeStamp restat_mtime = 0;
// If any output was cleaned, find the most recent mtime of any
// (existing) non-order-only input or the depfile.
- for (vector<Node*>::iterator i = edge->inputs_.begin();
- i != edge->inputs_.end() - edge->order_only_deps_; ++i) {
- TimeStamp input_mtime;
- if ((*i)->in_edge() != nullptr) {
- input_mtime = disk_interface_->LStat((*i)->path(), nullptr, err);
- } else {
- input_mtime = disk_interface_->Stat((*i)->path(), err);
- }
- if (input_mtime == -1)
- return false;
- if (input_mtime > restat_mtime)
- restat_mtime = input_mtime;
- }
+ TimeStamp restat_mtime = newest_input;
string depfile = edge->GetUnescapedDepfile();
if (restat_mtime != 0 && deps_type.empty() && !depfile.empty()) {
diff --git a/src/build.h b/src/build.h
index c1b234d..fa11442 100644
--- a/src/build.h
+++ b/src/build.h
@@ -146,7 +146,8 @@
missing_depfile_should_err(false),
uses_phony_outputs(false),
output_directory_should_err(false),
- missing_output_file_should_err(false) {}
+ missing_output_file_should_err(false),
+ old_output_should_err(false) {}
enum Verbosity {
NORMAL,
@@ -179,6 +180,10 @@
/// Whether a missing output file should warn or print an error.
bool missing_output_file_should_err;
+
+ /// Whether an output with an older timestamp than the inputs should
+ /// warn or print an error.
+ bool old_output_should_err;
};
/// Builder wraps the build process: starting commands, updating status.
diff --git a/src/build_test.cc b/src/build_test.cc
index 2becbc7..406b291 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -2493,6 +2493,140 @@
builder.command_runner_.release();
}
+TEST_F(BuildWithLogTest, OldOutputFileIgnored) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule true\n command = true\n"
+"build out: true in\n"));
+
+ fs_.Create("in", "");
+ fs_.Tick();
+ fs_.Create("out", "");
+
+ string err;
+ EXPECT_TRUE(builder_.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+
+ fs_.Tick();
+ fs_.Create("in", "");
+
+ state_.Reset();
+ EXPECT_TRUE(builder_.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder_.Build(&err));
+ EXPECT_EQ("", err);
+
+ EXPECT_EQ("", status_.last_output_);
+}
+
+TEST_F(BuildWithLogTest, OldOutputFileWarning) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule true\n command = true\n"
+"build out: true in\n"));
+
+ config_.uses_phony_outputs = true;
+
+ Builder builder(&state_, config_, &build_log_, NULL, &fs_, &status_, 0);
+ builder.command_runner_.reset(&command_runner_);
+
+ fs_.Create("in", "");
+ fs_.Tick();
+ fs_.Create("out", "");
+
+ string err;
+ EXPECT_TRUE(builder.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder.Build(&err));
+ EXPECT_EQ("", err);
+
+ fs_.Tick();
+ fs_.Create("in", "");
+
+ command_runner_.commands_ran_.clear();
+ state_.Reset();
+ EXPECT_TRUE(builder.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder.Build(&err));
+ EXPECT_EQ("", err);
+
+ EXPECT_EQ("ninja: Missing `restat`? An output file is older than the most recent input: out", status_.last_output_);
+
+ builder.command_runner_.release();
+}
+
+TEST_F(BuildWithLogTest, OldOutputFileError) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule true\n command = true\n"
+"build out: true in\n"));
+
+ config_.uses_phony_outputs = true;
+ config_.old_output_should_err = true;
+
+ Builder builder(&state_, config_, &build_log_, NULL, &fs_, &status_, 0);
+ builder.command_runner_.reset(&command_runner_);
+
+ fs_.Create("in", "");
+ fs_.Tick();
+ fs_.Create("out", "");
+
+ string err;
+ EXPECT_TRUE(builder.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder.Build(&err));
+ EXPECT_EQ("", err);
+
+ fs_.Tick();
+ fs_.Create("in", "");
+
+ command_runner_.commands_ran_.clear();
+ state_.Reset();
+ EXPECT_TRUE(builder.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_FALSE(builder.Build(&err));
+ EXPECT_EQ("subcommand failed", err);
+
+ EXPECT_EQ("ninja: Missing `restat`? An output file is older than the most recent input: out", status_.last_output_);
+
+ builder.command_runner_.release();
+}
+
+TEST_F(BuildWithLogTest, OutputFileUpdated) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule touch\n command = touch ${out}\n"
+"build out: touch in\n"));
+
+ config_.uses_phony_outputs = true;
+ config_.old_output_should_err = true;
+
+ Builder builder(&state_, config_, &build_log_, NULL, &fs_, &status_, 0);
+ builder.command_runner_.reset(&command_runner_);
+
+ fs_.Create("in", "");
+ fs_.Tick();
+ fs_.Create("out", "");
+
+ string err;
+ EXPECT_TRUE(builder.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder.Build(&err));
+ EXPECT_EQ("", err);
+
+ fs_.Tick();
+ fs_.Create("in", "");
+
+ command_runner_.commands_ran_.clear();
+ state_.Reset();
+ EXPECT_TRUE(builder.AddTarget("out", &err));
+ EXPECT_EQ("", err);
+ EXPECT_TRUE(builder.Build(&err));
+ EXPECT_EQ("", err);
+
+ EXPECT_EQ("", status_.last_output_);
+
+ builder.command_runner_.release();
+}
+
TEST_F(BuildWithDepsLogTest, MissingDepfileWarning) {
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
"build out1: cat in1\n"
diff --git a/src/graph.cc b/src/graph.cc
index 78998ff..b1cdf77 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -47,6 +47,12 @@
}
}
+bool Node::LStat(DiskInterface* disk_interface, bool* is_dir, string* err) {
+ assert(in_edge() != nullptr);
+ assert(!in_edge()->IsPhonyOutput());
+ return (mtime_ = disk_interface->LStat(path_.str(), is_dir, err)) != -1;
+}
+
bool DependencyScan::RecomputeNodesDirty(const std::vector<Node*>& initial_nodes,
std::string* err) {
METRIC_RECORD("dep scan");
diff --git a/src/graph.h b/src/graph.h
index b8637bf..07bd377 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -111,8 +111,12 @@
}
/// Return false on error.
+ /// Uses stat() or lstat() as appropriate.
bool Stat(DiskInterface* disk_interface, string* err);
+ /// Only use when lstat() is desired (output files)
+ bool LStat(DiskInterface* disk_interface, bool* is_dir, string* err);
+
/// Return false on error.
bool StatIfNecessary(DiskInterface* disk_interface, string* err) {
if (status_known())
diff --git a/src/ninja.cc b/src/ninja.cc
index ace3236..628af7c 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -993,7 +993,8 @@
" usesphonyoutputs={yes,no} whether the generate uses 'phony_output's so \n"
" that the following warnings work\n"
" outputdir={err,warn} how to treat outputs that are directories\n"
-" missingoutfile={err,warn} how to treat missing output files\n");
+" missingoutfile={err,warn} how to treat missing output files\n"
+" oldoutput={err,warn} how to treat output files older than their inputs\n");
return false;
} else if (name == "dupbuild=err") {
options->dupe_edges_should_err = true;
@@ -1031,6 +1032,12 @@
} else if (name == "missingoutfile=warn") {
config->missing_output_file_should_err = false;
return true;
+ } else if (name == "oldoutput=err") {
+ config->old_output_should_err = true;
+ return true;
+ } else if (name == "oldoutput=warn") {
+ config->old_output_should_err = false;
+ return true;
} else {
const char* suggestion =
SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn",
@@ -1038,7 +1045,8 @@
"missingdepfile=err", "missingdepfile=warn",
"usesphonyoutputs=yes", "usesphonyoutputs=no",
"outputdir=err", "outputdir=warn",
- "missingoutfile=err", "missingoutfile=warn", NULL);
+ "missingoutfile=err", "missingoutfile=warn",
+ "oldoutput=err", "oldoutput=warn", NULL);
if (suggestion) {
Error("unknown warning flag '%s', did you mean '%s'?",
name.c_str(), suggestion);
