From 8884066b31b2ce6501e22824825bea554ba202be Mon Sep 17 00:00:00 2001
From: John Drouhard <john@jmdtech.org>
Date: Mon, 22 Mar 2021 12:07:12 -0500
Subject: [PATCH] Check the output file's mtime against the most recent input
 time before checking the build log

This is a followup fix for #1753. Fixes #1932

`build_log()` will always be valid (even for new builds). We should be
checking for the output being older than the input before we check the
build log for additional possible conditions that would make the output
dirty.
---
 src/build_test.cc | 18 ++++++++++++++++++
 src/graph.cc      | 32 +++++++++++++++++++++++---------
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/src/build_test.cc b/src/build_test.cc
index 474dd196a0..9a94168430 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -1288,6 +1288,24 @@ struct BuildWithLogTest : public BuildTest {
   BuildLog build_log_;
 };
 
+TEST_F(BuildWithLogTest, ImplicitGeneratedOutOfDate) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule generator\n"
+"  command = touch out.imp\n"
+"  generator = 1\n"
+"build out.imp:  generator | in\n"));
+  fs_.Create("out.imp", "");
+  fs_.Tick();
+  fs_.Create("in", "");
+
+  string err;
+
+  EXPECT_TRUE(builder_.AddTarget("out.imp", &err));
+  EXPECT_FALSE(builder_.AlreadyUpToDate());
+
+  EXPECT_TRUE(GetNode("out.imp")->dirty());
+}
+
 TEST_F(BuildWithLogTest, NotInLogButOnDisk) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "rule cc\n"
diff --git a/src/graph.cc b/src/graph.cc
index 9e8eda69de..c6022b7027 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -249,6 +249,28 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge,
   }
 
   const Node* most_recent_input = edge->most_recent_input_;
+  bool used_log = false;
+
+  // Dirty if the output is older than the input.
+  if (most_recent_input && output->mtime() < most_recent_input->mtime()) {
+    TimeStamp output_mtime = output->mtime();
+
+    // if this output exists in the build log, use the mtime from there instead. That time
+    // reflects the most recent input file's mtime at the time the output began
+    if (build_log() && (entry = build_log()->LookupByOutput(output->path()))) {
+      output_mtime = entry->mtime;
+      used_log = true;
+    }
+
+    if (output_mtime < most_recent_input->mtime()) {
+      EXPLAIN("%soutput %s older than most recent input %s "
+              "(%" PRId64 " vs %" PRId64 ")",
+              used_log ? "logged mtime of " : "", output->path().c_str(),
+              most_recent_input->path().c_str(),
+              output_mtime, most_recent_input->mtime());
+      return true;
+    }
+  }
 
   if (build_log()) {
     bool generator = edge->GetBindingBool("generator");
@@ -261,7 +283,7 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge,
         EXPLAIN("command line changed for %s", output->path().c_str());
         return true;
       }
-      if (most_recent_input && entry->mtime < most_recent_input->mtime()) {
+      if (!used_log && most_recent_input && entry->mtime < most_recent_input->mtime()) {
         // May also be dirty due to the mtime in the log being older than the
         // mtime of the most recent input.  This can occur even when the mtime
         // on disk is newer if a previous run wrote to the output file but
@@ -276,14 +298,6 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge,
       EXPLAIN("command line not found in log for %s", output->path().c_str());
       return true;
     }
-  } else if (most_recent_input &&
-             output->mtime() < most_recent_input->mtime()) {
-      EXPLAIN(
-          "output %s older than most recent input %s "
-          "(%" PRId64 " vs %" PRId64 ")",
-          output->path().c_str(), most_recent_input->path().c_str(),
-          output->mtime(), most_recent_input->mtime());
-    return true;
   }
 
   return false;