Merge pull request #1964 from rascani/fix-phony-edges

Set output mtime of phony edges to the latest inputs

5 files changed, 232 insertions, 6 deletions

diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc
index a5012b4..2b948b9 100644
--- a/doc/manual.asciidoc
+++ b/doc/manual.asciidoc
@@ -483,6 +483,11 @@ nothing, but phony rules are handled specially in that they aren't
 printed when run, logged (see below), nor do they contribute to the
 command count printed as part of the build process.
 
+When a `phony` target is used as an input to another build rule, the
+other build rule will, semantically, consider the inputs of the
+`phony` rule as its own. Therefore, `phony` rules can be used to group
+inputs, e.g. header files.
+
 `phony` can also be used to create dummy targets for files which
 may not exist at build time.  If a phony build statement is written
 without any dependencies, the target will be considered out of date if
diff --git a/src/build_test.cc b/src/build_test.cc
index e0c43b1..8b6dca2 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -1182,6 +1182,151 @@ TEST_F(BuildTest, PhonySelfReference) {
   EXPECT_TRUE(builder_.AlreadyUpToDate());
 }
 
+// There are 6 different cases for phony rules:
+//
+// 1. output edge does not exist, inputs are not real
+// 2. output edge does not exist, no inputs
+// 3. output edge does not exist, inputs are real, newest mtime is M
+// 4. output edge is real, inputs are not real
+// 5. output edge is real, no inputs
+// 6. output edge is real, inputs are real, newest mtime is M
+//
+// Expected results :
+// 1. Edge is marked as clean, mtime is newest mtime of dependents.
+//     Touching inputs will cause dependents to rebuild.
+// 2. Edge is marked as dirty, causing dependent edges to always rebuild
+// 3. Edge is marked as clean, mtime is newest mtime of dependents.
+//     Touching inputs will cause dependents to rebuild.
+// 4. Edge is marked as clean, mtime is newest mtime of dependents.
+//     Touching inputs will cause dependents to rebuild.
+// 5. Edge is marked as dirty, causing dependent edges to always rebuild
+// 6. Edge is marked as clean, mtime is newest mtime of dependents.
+//     Touching inputs will cause dependents to rebuild.
+void TestPhonyUseCase(BuildTest* t, int i) {
+  State& state_ = t->state_;
+  Builder& builder_ = t->builder_;
+  FakeCommandRunner& command_runner_ = t->command_runner_;
+  VirtualFileSystem& fs_ = t->fs_;
+
+  string err;
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule touch\n"
+" command = touch $out\n"
+"build notreal: phony blank\n"
+"build phony1: phony notreal\n"
+"build phony2: phony\n"
+"build phony3: phony blank\n"
+"build phony4: phony notreal\n"
+"build phony5: phony\n"
+"build phony6: phony blank\n"
+"\n"
+"build test1: touch phony1\n"
+"build test2: touch phony2\n"
+"build test3: touch phony3\n"
+"build test4: touch phony4\n"
+"build test5: touch phony5\n"
+"build test6: touch phony6\n"
+));
+
+  // Set up test.
+  builder_.command_runner_.reset(&command_runner_);
+
+  fs_.Create("blank", "");  // a "real" file
+  EXPECT_TRUE(builder_.AddTarget("test1", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.AddTarget("test2", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.AddTarget("test3", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.AddTarget("test4", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.AddTarget("test5", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.AddTarget("test6", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.Build(&err));
+  ASSERT_EQ("", err);
+
+  string ci;
+  ci += static_cast<char>('0' + i);
+
+  // Tests 1, 3, 4, and 6 should rebuild when the input is updated.
+  if (i != 2 && i != 5) {
+    Node* testNode  = t->GetNode("test" + ci);
+    Node* phonyNode = t->GetNode("phony" + ci);
+    Node* inputNode = t->GetNode("blank");
+
+    state_.Reset();
+    TimeStamp startTime = fs_.now_;
+
+    // Build number 1
+    EXPECT_TRUE(builder_.AddTarget("test" + ci, &err));
+    ASSERT_EQ("", err);
+    if (!builder_.AlreadyUpToDate())
+      EXPECT_TRUE(builder_.Build(&err));
+    ASSERT_EQ("", err);
+
+    // Touch the input file
+    state_.Reset();
+    command_runner_.commands_ran_.clear();
+    fs_.Tick();
+    fs_.Create("blank", "");  // a "real" file
+    EXPECT_TRUE(builder_.AddTarget("test" + ci, &err));
+    ASSERT_EQ("", err);
+
+    // Second build, expect testN edge to be rebuilt
+    // and phonyN node's mtime to be updated.
+    EXPECT_FALSE(builder_.AlreadyUpToDate());
+    EXPECT_TRUE(builder_.Build(&err));
+    ASSERT_EQ("", err);
+    ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+    EXPECT_EQ(string("touch test") + ci, command_runner_.commands_ran_[0]);
+    EXPECT_TRUE(builder_.AlreadyUpToDate());
+
+    TimeStamp inputTime = inputNode->mtime();
+
+    EXPECT_FALSE(phonyNode->exists());
+    EXPECT_FALSE(phonyNode->dirty());
+
+    EXPECT_GT(phonyNode->mtime(), startTime);
+    EXPECT_EQ(phonyNode->mtime(), inputTime);
+    ASSERT_TRUE(testNode->Stat(&fs_, &err));
+    EXPECT_TRUE(testNode->exists());
+    EXPECT_GT(testNode->mtime(), startTime);
+  } else {
+    // Tests 2 and 5: Expect dependents to always rebuild.
+
+    state_.Reset();
+    command_runner_.commands_ran_.clear();
+    fs_.Tick();
+    command_runner_.commands_ran_.clear();
+    EXPECT_TRUE(builder_.AddTarget("test" + ci, &err));
+    ASSERT_EQ("", err);
+    EXPECT_FALSE(builder_.AlreadyUpToDate());
+    EXPECT_TRUE(builder_.Build(&err));
+    ASSERT_EQ("", err);
+    ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+    EXPECT_EQ("touch test" + ci, command_runner_.commands_ran_[0]);
+
+    state_.Reset();
+    command_runner_.commands_ran_.clear();
+    EXPECT_TRUE(builder_.AddTarget("test" + ci, &err));
+    ASSERT_EQ("", err);
+    EXPECT_FALSE(builder_.AlreadyUpToDate());
+    EXPECT_TRUE(builder_.Build(&err));
+    ASSERT_EQ("", err);
+    ASSERT_EQ(1u, command_runner_.commands_ran_.size());
+    EXPECT_EQ("touch test" + ci, command_runner_.commands_ran_[0]);
+  }
+}
+
+TEST_F(BuildTest, PhonyUseCase1) { TestPhonyUseCase(this, 1); }
+TEST_F(BuildTest, PhonyUseCase2) { TestPhonyUseCase(this, 2); }
+TEST_F(BuildTest, PhonyUseCase3) { TestPhonyUseCase(this, 3); }
+TEST_F(BuildTest, PhonyUseCase4) { TestPhonyUseCase(this, 4); }
+TEST_F(BuildTest, PhonyUseCase5) { TestPhonyUseCase(this, 5); }
+TEST_F(BuildTest, PhonyUseCase6) { TestPhonyUseCase(this, 6); }
+
 TEST_F(BuildTest, Fail) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "rule fail\n"
diff --git a/src/graph.cc b/src/graph.cc
index c142f0c..c875d3b 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -31,7 +31,19 @@
 using namespace std;
 
 bool Node::Stat(DiskInterface* disk_interface, string* err) {
-  return (mtime_ = disk_interface->Stat(path_, err)) != -1;
+  METRIC_RECORD("node stat");
+  mtime_ = disk_interface->Stat(path_, err);
+  if (mtime_ == -1) {
+    return false;
+  }
+  exists_ = (mtime_ != 0) ? ExistenceStatusExists : ExistenceStatusMissing;
+  return true;
+}
+
+void Node::UpdatePhonyMtime(TimeStamp mtime) {
+  if (!exists()) {
+    mtime_ = std::max(mtime_, mtime);
+  }
 }
 
 bool DependencyScan::RecomputeDirty(Node* node, string* err) {
@@ -237,6 +249,14 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge,
               output->path().c_str());
       return true;
     }
+
+    // Update the mtime with the newest input. Dependents can thus call mtime()
+    // on the fake node and get the latest mtime of the dependencies
+    if (most_recent_input) {
+      output->UpdatePhonyMtime(most_recent_input->mtime());
+    }
+
+    // Phony edges are clean, nothing to do
     return false;
   }
 
@@ -487,7 +507,7 @@ string Node::PathDecanonicalized(const string& path, uint64_t slash_bits) {
 void Node::Dump(const char* prefix) const {
   printf("%s <%s 0x%p> mtime: %" PRId64 "%s, (:%s), ",
          prefix, path().c_str(), this,
-         mtime(), mtime() ? "" : " (:missing)",
+         mtime(), exists() ? "" : " (:missing)",
          dirty() ? " dirty" : " clean");
   if (in_edge()) {
     in_edge()->Dump("in-edge: ");
diff --git a/src/graph.h b/src/graph.h
index bb4f10c..fac8059 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -15,6 +15,7 @@
 #ifndef NINJA_GRAPH_H_
 #define NINJA_GRAPH_H_
 
+#include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
@@ -40,6 +41,7 @@ struct Node {
       : path_(path),
         slash_bits_(slash_bits),
         mtime_(-1),
+        exists_(ExistenceStatusUnknown),
         dirty_(false),
         dyndep_pending_(false),
         in_edge_(NULL),
@@ -48,6 +50,9 @@ struct Node {
   /// Return false on error.
   bool Stat(DiskInterface* disk_interface, std::string* err);
 
+  /// If the file doesn't exist, set the mtime_ from its dependencies
+  void UpdatePhonyMtime(TimeStamp mtime);
+
   /// Return false on error.
   bool StatIfNecessary(DiskInterface* disk_interface, std::string* err) {
     if (status_known())
@@ -58,20 +63,24 @@ struct Node {
   /// Mark as not-yet-stat()ed and not dirty.
   void ResetState() {
     mtime_ = -1;
+    exists_ = ExistenceStatusUnknown;
     dirty_ = false;
   }
 
   /// Mark the Node as already-stat()ed and missing.
   void MarkMissing() {
-    mtime_ = 0;
+    if (mtime_ == -1) {
+      mtime_ = 0;
+    }
+    exists_ = ExistenceStatusMissing;
   }
 
   bool exists() const {
-    return mtime_ != 0;
+    return exists_ == ExistenceStatusExists;
   }
 
   bool status_known() const {
-    return mtime_ != -1;
+    return exists_ != ExistenceStatusUnknown;
   }
 
   const std::string& path() const { return path_; }
@@ -113,9 +122,19 @@ private:
   /// Possible values of mtime_:
   ///   -1: file hasn't been examined
   ///   0:  we looked, and file doesn't exist
-  ///   >0: actual file's mtime
+  ///   >0: actual file's mtime, or the latest mtime of its dependencies if it doesn't exist
   TimeStamp mtime_;
 
+  enum ExistenceStatus {
+    /// The file hasn't been examined.
+    ExistenceStatusUnknown,
+    /// The file doesn't exist. mtime_ will be the latest mtime of its dependencies.
+    ExistenceStatusMissing,
+    /// The path is an actual file. mtime_ will be the file's mtime.
+    ExistenceStatusExists
+  };
+  ExistenceStatus exists_;
+
   /// Dirty is true when the underlying file is out-of-date.
   /// But note that Edge::outputs_ready_ is also used in judging which
   /// edges to build.
diff --git a/src/graph_test.cc b/src/graph_test.cc
index 6b4bb51..4f0de98 100644
--- a/src/graph_test.cc
+++ b/src/graph_test.cc
@@ -889,3 +889,40 @@ TEST_F(GraphTest, DyndepFileCircular) {
   EXPECT_EQ(1u, edge->implicit_deps_);
   EXPECT_EQ(1u, edge->order_only_deps_);
 }
+
+// Check that phony's dependencies' mtimes are propagated.
+TEST_F(GraphTest, PhonyDepsMtimes) {
+  string err;
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule touch\n"
+" command = touch $out\n"
+"build in_ph: phony in1\n"
+"build out1: touch in_ph\n"
+));
+  fs_.Create("in1", "");
+  fs_.Create("out1", "");
+  Node* out1 = GetNode("out1");
+  Node* in1  = GetNode("in1");
+
+  EXPECT_TRUE(scan_.RecomputeDirty(out1, &err));
+  EXPECT_TRUE(!out1->dirty());
+
+  // Get the mtime of out1
+  ASSERT_TRUE(in1->Stat(&fs_, &err));
+  ASSERT_TRUE(out1->Stat(&fs_, &err));
+  TimeStamp out1Mtime1 = out1->mtime();
+  TimeStamp in1Mtime1 = in1->mtime();
+
+  // Touch in1. This should cause out1 to be dirty
+  state_.Reset();
+  fs_.Tick();
+  fs_.Create("in1", "");
+
+  ASSERT_TRUE(in1->Stat(&fs_, &err));
+  EXPECT_GT(in1->mtime(), in1Mtime1);
+
+  EXPECT_TRUE(scan_.RecomputeDirty(out1, &err));
+  EXPECT_GT(in1->mtime(), in1Mtime1);
+  EXPECT_EQ(out1->mtime(), out1Mtime1);
+  EXPECT_TRUE(out1->dirty());
+}