Improve incremental build: make ninja handle dynamic outputs (continuation)

CMakeLists.txt

@@ -131,6 +131,7 @@ add_library(libninja OBJECT
 	src/clparser.cc
 	src/dyndep.cc
 	src/dyndep_parser.cc
+	src/dynout_parser.cc
 	src/debug_flags.cc
 	src/deps_log.cc
 	src/disk_interface.cc
@@ -264,6 +265,7 @@ if(BUILD_TESTING)
     src/deps_log_test.cc
     src/disk_interface_test.cc
     src/dyndep_parser_test.cc
+    src/dynout_parser_test.cc
     src/edit_distance_test.cc
     src/explanations_test.cc
     src/graph_test.cc

configure.py

@@ -538,6 +538,7 @@ def has_re2c() -> bool:
              'disk_interface',
              'dyndep',
              'dyndep_parser',
+             'dynout_parser',
              'edit_distance',
              'eval_env',
              'graph',

doc/manual.asciidoc

@@ -292,6 +292,9 @@ _Available since Ninja 1.2._
 `deps`:: show all dependencies stored in the `.ninja_deps` file. When given a
 target, show just the target's dependencies. _Available since Ninja 1.4._
 
+`dynouts`:: show all dynamic outputs stored in the `.ninja_deps` file. When
+given a target, show just the target's outputs.
+
 `missingdeps`:: given a list of targets, look for targets that depend on
 a generated file, but do not have a properly (possibly transitive) dependency
 on the generator.  Such targets may cause build flakiness on clean builds.
@@ -307,6 +310,9 @@ each of them with a missing include error or equivalent pointing to the
 generated file.
 _Available since Ninja 1.11._
 
+`outputs`:: list all outputs of the build graph. This includes any dynamic
+outputs stored in the deps log.
+
 `recompact`:: recompact the `.ninja_deps` file. _Available since Ninja 1.4._
 
 `restat`:: updates all recorded file modification timestamps in the `.ninja_log`
@@ -900,6 +906,13 @@ keys.
    stored as `.ninja_deps` in the `builddir`, see <<ref_toplevel,the
    discussion of `builddir`>>.
 
+`dynout`:: path to an optional _dynout file_ that contains extra _implicit
+  outputs_ generated by the rule. This allows ninja to dynamically discover
+  output files whose presence is decided during the build, so that for
+  subsequent builds the edge is re-run if some dynamic output is missing, and
+  dynamic outputs are cleaned when using the `-t clean` tool. The dynout file
+  syntax expects one path per line.
+
 `msvc_deps_prefix`:: _(Available since Ninja 1.5.)_ defines the string
   which should be stripped from msvc's /showIncludes output. Only
   needed when `deps = msvc` and no English Visual Studio version is used.

src/build.cc

@@ -34,6 +34,7 @@
 #include "depfile_parser.h"
 #include "deps_log.h"
 #include "disk_interface.h"
+#include "dynout_parser.h"
 #include "explanations.h"
 #include "graph.h"
 #include "metrics.h"
@@ -698,6 +699,7 @@ void Builder::Cleanup() {
     for (vector<Edge*>::iterator e = active_edges.begin();
          e != active_edges.end(); ++e) {
       string depfile = (*e)->GetUnescapedDepfile();
+      string dynout = (*e)->GetUnescapedDynout();
       for (vector<Node*>::iterator o = (*e)->outputs_.begin();
            o != (*e)->outputs_.end(); ++o) {
         // Only delete this output if it was actually modified.  This is
@@ -716,6 +718,8 @@ void Builder::Cleanup() {
       }
       if (!depfile.empty())
         disk_interface_->RemoveFile(depfile);
+      if (!dynout.empty())
+        disk_interface_->RemoveFile(dynout);
     }
   }
 
@@ -949,6 +953,18 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
     }
   }
 
+  int outputs_count = 0;
+  string extract_err;
+  std::string dynout_file = edge->GetUnescapedDynout();
+  if (!ExtractDynouts(edge, dynout_file, &deps_nodes, &outputs_count,
+                      &extract_err) &&
+      result->success()) {
+    if (!result->output.empty())
+      result->output.append("\n");
+    result->output.append(extract_err);
+    result->status = ExitFailure;
+  }
+
   int64_t start_time_millis, end_time_millis;
   RunningEdgeMap::iterator it = running_edges_.find(edge);
   start_time_millis = it->second;
@@ -1008,21 +1024,23 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
     disk_interface_->RemoveFile(rspfile);
 
   if (scan_.build_log()) {
-    if (!scan_.build_log()->RecordCommand(edge, start_time_millis,
-                                          end_time_millis, record_mtime)) {
+    std::vector<Node*> dynouts(deps_nodes.end() - outputs_count,
+                               deps_nodes.end());
+    if (!scan_.build_log()->RecordCommand(
+            edge, start_time_millis, end_time_millis, record_mtime, dynouts)) {
       *err = string("Error writing to build log: ") + strerror(errno);
       return false;
     }
   }
 
-  if (!deps_type.empty() && !config_.dry_run) {
+  if ((!deps_type.empty() || !dynout_file.empty()) && !config_.dry_run) {
     assert(!edge->outputs_.empty() && "should have been rejected by parser");
     for (std::vector<Node*>::const_iterator o = edge->outputs_.begin();
          o != edge->outputs_.end(); ++o) {
       TimeStamp deps_mtime = disk_interface_->Stat((*o)->path(), err);
       if (deps_mtime == -1)
         return false;
-      if (!scan_.deps_log()->RecordDeps(*o, deps_mtime, deps_nodes)) {
+      if (!scan_.deps_log()->RecordDeps(*o, deps_mtime, deps_nodes, outputs_count)) {
         *err = std::string("Error writing to deps log: ") + strerror(errno);
         return false;
       }
@@ -1109,3 +1127,58 @@ bool Builder::LoadDyndeps(Node* node, string* err) {
 
   return true;
 }
+
+bool Builder::ExtractDynouts(Edge* edge, const std::string& dynout_file,
+                             std::vector<Node*>* nodes, int* outputs_count,
+                             std::string* err) {
+  if (dynout_file.empty()) {
+    return true;
+  }
+
+  // Read depfile content.  Treat a missing depfile as empty.
+  std::string content;
+  switch (disk_interface_->ReadFile(dynout_file, &content, err)) {
+  case DiskInterface::Okay:
+    break;
+  case DiskInterface::NotFound:
+    if (err != NULL) {
+      err->clear();
+    }
+    break;
+  case DiskInterface::OtherError:
+    if (err != NULL) {
+      *err = "loading '" + dynout_file + "': " + *err;
+    }
+    return false;
+  }
+
+  std::vector<StringPiece> output_paths;
+  std::string parse_err;
+  if (!DynoutParser::Parse(content, output_paths, &parse_err)) {
+    if (err != NULL) {
+      *err = parse_err;
+    }
+    return false;
+  }
+
+  int start_size = nodes->size();
+
+  for (const StringPiece &p : output_paths) {
+    uint64_t slash_bits;
+    std::string canonical = p.AsString();
+    CanonicalizePath(&canonical, &slash_bits);
+    Node* new_node = state_->GetNode(canonical, slash_bits);
+    nodes->push_back(new_node);
+  }
+  *outputs_count = (int) nodes->size() - start_size;
+
+  if (!g_keep_dynout) {
+    if (disk_interface_->RemoveFile(dynout_file) < 0) {
+      if (err != NULL) {
+        *err = std::string("deleting dynout: ") + strerror(errno) + std::string("\n");
+      }
+      return false;
+    }
+  }
+  return true;
+}

src/build.h

@@ -233,6 +233,10 @@ struct Builder {
                    const std::string& deps_prefix,
                    std::vector<Node*>* deps_nodes, std::string* err);
 
+  bool ExtractDynouts(Edge* edge, const std::string& dynout_file,
+                      std::vector<Node*>* nodes, int* outputs_count,
+                      std::string* err);
+
   /// Map of running edge to time the edge started running.
   typedef std::map<const Edge*, int> RunningEdgeMap;
   RunningEdgeMap running_edges_;

src/build_log.cc

@@ -142,33 +142,36 @@ bool BuildLog::OpenForWrite(const string& path, const BuildLogUser& user,
 }
 
 bool BuildLog::RecordCommand(Edge* edge, int start_time, int end_time,
-                             TimeStamp mtime) {
+                             TimeStamp mtime,
+                             const std::vector<Node*> &extra_outputs) {
   string command = edge->EvaluateCommand(true);
   uint64_t command_hash = LogEntry::HashCommand(command);
-  for (vector<Node*>::iterator out = edge->outputs_.begin();
-       out != edge->outputs_.end(); ++out) {
-    const string& path = (*out)->path();
-    Entries::iterator i = entries_.find(path);
-    LogEntry* log_entry;
-    if (i != entries_.end()) {
-      log_entry = i->second;
-    } else {
-      log_entry = new LogEntry(path);
-      entries_.insert(Entries::value_type(log_entry->output, log_entry));
-    }
-    log_entry->command_hash = command_hash;
-    log_entry->start_time = start_time;
-    log_entry->end_time = end_time;
-    log_entry->mtime = mtime;
 
-    if (!OpenForWriteIfNeeded()) {
-      return false;
-    }
-    if (log_file_) {
-      if (!WriteEntry(log_file_, *log_entry))
+  for (const vector<Node*> outputs : { edge->outputs_, extra_outputs }) {
+    for (const Node *out : outputs) {
+      const string& path = out->path();
+      Entries::iterator i = entries_.find(path);
+      LogEntry* log_entry;
+      if (i != entries_.end()) {
+        log_entry = i->second;
+      } else {
+        log_entry = new LogEntry(path);
+        entries_.insert(Entries::value_type(log_entry->output, log_entry));
+      }
+      log_entry->command_hash = command_hash;
+      log_entry->start_time = start_time;
+      log_entry->end_time = end_time;
+      log_entry->mtime = mtime;
+
+      if (!OpenForWriteIfNeeded()) {
         return false;
-      if (fflush(log_file_) != 0) {
+      }
+      if (log_file_) {
+        if (!WriteEntry(log_file_, *log_entry))
           return false;
+        if (fflush(log_file_) != 0) {
+            return false;
+        }
       }
     }
   }

src/build_log.h

@@ -25,6 +25,7 @@
 
 struct DiskInterface;
 struct Edge;
+struct Node;
 
 /// Can answer questions about the manifest for the BuildLog.
 struct BuildLogUser {
@@ -49,7 +50,8 @@ struct BuildLog {
   bool OpenForWrite(const std::string& path, const BuildLogUser& user,
                     std::string* err);
   bool RecordCommand(Edge* edge, int start_time, int end_time,
-                     TimeStamp mtime = 0);
+                     TimeStamp mtime = 0,
+                     const std::vector<Node*>& extra_outputs = {});
   void Close();
 
   /// Load the on-disk log.

src/build_log_test.cc

@@ -365,4 +365,29 @@ TEST_F(BuildLogRecompactTest, Recompact) {
   ASSERT_FALSE(log2.LookupByOutput("out2"));
 }
 
+// Make sure the build log can record extra outputs not part of the edge (e.g.
+// from a dynout file).
+TEST_F(BuildLogTest, ExtraOutputs) {
+  AssertParse(&state_,
+"build out: cat mid\n");
+
+  BuildLog log;
+
+  // The build log must handle overlap between the extra outputs and the edge
+  // outputs, and only record each output once
+  Node *out_node = state_.LookupNode("out");
+  ASSERT_TRUE(out_node);
+  Node *second_out = state_.GetNode("out.bis", 0);
+  std::vector<Node*> extra_outputs = { out_node, second_out };
+
+  log.RecordCommand(state_.edges_[0], 15, 18, 0, extra_outputs);
+
+  ASSERT_EQ(2u, log.entries().size());
+  BuildLog::LogEntry* e1 = log.LookupByOutput("out.bis");
+  ASSERT_TRUE(e1);
+  ASSERT_EQ(15, e1->start_time);
+  ASSERT_EQ("out.bis", e1->output);
+}
+
+
 }  // anonymous namespace