Merge pull request #217 from PetrWolf/master

Response files

14 files changed, 310 insertions, 8 deletions

diff --git a/src/build.cc b/src/build.cc
index c90824d..d3e88ee 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -352,8 +352,8 @@ void Plan::CleanNode(BuildLog* build_log, Node* node) {
       for (vector<Node*>::iterator ni = begin; ni != end; ++ni)
         if ((*ni)->mtime() > most_recent_input)
           most_recent_input = (*ni)->mtime();
-      string command = (*ei)->EvaluateCommand();
-
+      string command = (*ei)->EvaluateCommand(true);
+      
       // Now, recompute the dirty state of each output.
       bool all_outputs_clean = true;
       for (vector<Node*>::iterator ni = (*ei)->outputs_.begin();
@@ -630,6 +630,13 @@ bool Builder::StartEdge(Edge* edge, string* err) {
     if (!disk_interface_->MakeDirs((*i)->path()))
       return false;
   }
+  
+  // Create response file, if needed
+  // XXX: this may also block; do we care?
+  if (edge->HasRspFile()) {
+    if (!disk_interface_->WriteFile(edge->GetRspFile(), edge->GetRspFileContent())) 
+      return false;
+  }
 
   // start command computing and run it
   if (!command_runner_->StartCommand(edge)) {
@@ -687,6 +694,10 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
       }
     }
 
+    // delete the response file on success (if exists)
+    if (edge->HasRspFile()) 
+      disk_interface_->RemoveFile(edge->GetRspFile());
+
     plan_.EdgeFinished(edge);
   }
 
diff --git a/src/build_log.cc b/src/build_log.cc
index c9267d2..4b93931 100644
--- a/src/build_log.cc
+++ b/src/build_log.cc
@@ -73,7 +73,7 @@ bool BuildLog::OpenForWrite(const string& path, string* err) {
 
 void BuildLog::RecordCommand(Edge* edge, int start_time, int end_time,
                              TimeStamp restat_mtime) {
-  const string command = edge->EvaluateCommand();
+  string command = edge->EvaluateCommand(true);
   for (vector<Node*>::iterator out = edge->outputs_.begin();
        out != edge->outputs_.end(); ++out) {
     const string& path = (*out)->path();
diff --git a/src/build_test.cc b/src/build_test.cc
index 7c3a729..78bc918 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -237,7 +237,9 @@ bool BuildTest::CanRunMore() {
 bool BuildTest::StartCommand(Edge* edge) {
   assert(!last_command_);
   commands_ran_.push_back(edge->EvaluateCommand());
-  if (edge->rule().name() == "cat" || edge->rule_->name() == "cc" ||
+  if (edge->rule().name() == "cat"  ||
+      edge->rule().name() == "cat_rsp" ||
+      edge->rule().name() == "cc" ||
       edge->rule().name() == "touch") {
     for (vector<Node*>::iterator out = edge->outputs_.begin();
          out != edge->outputs_.end(); ++out) {
@@ -813,3 +815,129 @@ TEST_F(BuildDryRun, AllCommandsShown) {
   ASSERT_EQ(3u, commands_ran_.size());
 }
 
+// Test that RSP files are created when & where appropriate and deleted after
+// succesful execution.
+TEST_F(BuildTest, RspFileSuccess)
+{
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+    "rule cat_rsp\n"
+    "  command = cat $rspfile > $out\n"
+    "  rspfile = $rspfile\n"
+    "  rspfile_content = $long_command\n"
+    "build out1: cat in\n"
+    "build out2: cat_rsp in\n"
+    "  rspfile = out2.rsp\n"
+    "  long_command = Some very long command\n"));
+
+  fs_.Create("out1", now_, "");
+  fs_.Create("out2", now_, "");
+  fs_.Create("out3", now_, "");
+    
+  now_++;
+
+  fs_.Create("in", now_, "");
+
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("out1", &err));
+  ASSERT_EQ("", err);
+  EXPECT_TRUE(builder_.AddTarget("out2", &err));
+  ASSERT_EQ("", err);
+
+  size_t files_created = fs_.files_created_.size();
+  size_t files_removed = fs_.files_removed_.size();
+
+  EXPECT_TRUE(builder_.Build(&err));
+  ASSERT_EQ(2, commands_ran_.size()); // cat + cat_rsp
+    
+  // The RSP file was created
+  ASSERT_EQ(files_created + 1, fs_.files_created_.size());
+  ASSERT_EQ(1, fs_.files_created_.count("out2.rsp"));
+    
+  // The RSP file was removed
+  ASSERT_EQ(files_removed + 1, fs_.files_removed_.size());
+  ASSERT_EQ(1, fs_.files_removed_.count("out2.rsp"));
+}
+
+// Test that RSP file is created but not removed for commands, which fail
+TEST_F(BuildTest, RspFileFailure) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+    "rule fail\n"
+    "  command = fail\n"
+    "  rspfile = $rspfile\n"
+    "  rspfile_content = $long_command\n"
+    "build out: fail in\n"
+    "  rspfile = out.rsp\n"
+    "  long_command = Another very long command\n"));
+
+  fs_.Create("out", now_, "");
+  now_++;
+  fs_.Create("in", now_, "");
+
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("out", &err));
+  ASSERT_EQ("", err);
+
+  size_t files_created = fs_.files_created_.size();
+  size_t files_removed = fs_.files_removed_.size();
+
+  EXPECT_FALSE(builder_.Build(&err));
+  ASSERT_EQ("subcommand failed", err);
+  ASSERT_EQ(1, commands_ran_.size());
+
+  // The RSP file was created
+  ASSERT_EQ(files_created + 1, fs_.files_created_.size());
+  ASSERT_EQ(1, fs_.files_created_.count("out.rsp"));
+
+  // The RSP file was NOT removed
+  ASSERT_EQ(files_removed, fs_.files_removed_.size());
+  ASSERT_EQ(0, fs_.files_removed_.count("out.rsp"));
+
+  // The RSP file contains what it should
+  ASSERT_EQ("Another very long command", fs_.files_["out.rsp"].contents);
+}
+
+// Test that contens of the RSP file behaves like a regular part of 
+// command line, i.e. triggers a rebuild if changed
+TEST_F(BuildWithLogTest, RspFileCmdLineChange) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+    "rule cat_rsp\n"
+    "  command = cat $rspfile > $out\n"
+    "  rspfile = $rspfile\n"
+    "  rspfile_content = $long_command\n"
+    "build out: cat_rsp in\n"
+    "  rspfile = out.rsp\n"
+    "  long_command = Original very long command\n"));
+
+  fs_.Create("out", now_, "");
+  now_++;
+  fs_.Create("in", now_, "");
+
+  string err;
+  EXPECT_TRUE(builder_.AddTarget("out", &err));
+  ASSERT_EQ("", err);
+
+  // 1. Build for the 1st time (-> populate log)
+  EXPECT_TRUE(builder_.Build(&err));
+  ASSERT_EQ(1, commands_ran_.size());
+
+  // 2. Build again (no change)
+  commands_ran_.clear();
+  state_.Reset();
+  EXPECT_TRUE(builder_.AddTarget("out", &err));
+  EXPECT_EQ("", err);
+  ASSERT_TRUE(builder_.AlreadyUpToDate());
+
+  // 3. Alter the entry in the logfile 
+  // (to simulate a change in the command line between 2 builds)
+  BuildLog::LogEntry * log_entry = build_log_.LookupByOutput("out");
+  ASSERT_TRUE(NULL != log_entry);
+  ASSERT_EQ("cat out.rsp > out;rspfile=Original very long command", log_entry->command);
+  log_entry->command = "cat out.rsp > out;rspfile=Altered very long command";
+  // Now expect the target to be rebuilt
+  commands_ran_.clear();
+  state_.Reset();
+  EXPECT_TRUE(builder_.AddTarget("out", &err));
+  EXPECT_EQ("", err);
+  EXPECT_TRUE(builder_.Build(&err));
+  EXPECT_EQ(1, commands_ran_.size());
+}
diff --git a/src/clean.cc b/src/clean.cc
index bb912f6..e09ab4e 100644
--- a/src/clean.cc
+++ b/src/clean.cc
@@ -111,8 +111,12 @@ int Cleaner::CleanAll(bool generator) {
          out_node != (*e)->outputs_.end(); ++out_node) {
       Remove((*out_node)->path());
     }
+    // Remove the depfile
     if (!(*e)->rule().depfile().empty())
       Remove((*e)->EvaluateDepFile());
+    // Remove the response file
+    if ((*e)->HasRspFile()) 
+      Remove((*e)->GetRspFile());      
   }
   PrintFooter();
   return status_;
@@ -121,6 +125,8 @@ int Cleaner::CleanAll(bool generator) {
 void Cleaner::DoCleanTarget(Node* target) {
   if (target->in_edge()) {
     Remove(target->path());
+    if (target->in_edge()->HasRspFile())
+      Remove(target->in_edge()->GetRspFile());
     for (vector<Node*>::iterator n = target->in_edge()->inputs_.begin();
          n != target->in_edge()->inputs_.end();
          ++n) {
@@ -181,6 +187,8 @@ void Cleaner::DoCleanRule(const Rule* rule) {
       for (vector<Node*>::iterator out_node = (*e)->outputs_.begin();
            out_node != (*e)->outputs_.end(); ++out_node) {
         Remove((*out_node)->path());
+        if ((*e)->HasRspFile()) 
+          Remove((*e)->GetRspFile());
       }
     }
   }
diff --git a/src/clean_test.cc b/src/clean_test.cc
index 9cd4a95..fbbe6a1 100644
--- a/src/clean_test.cc
+++ b/src/clean_test.cc
@@ -249,6 +249,67 @@ TEST_F(CleanTest, CleanDepFile) {
   EXPECT_EQ(2u, fs_.files_removed_.size());
 }
 
+TEST_F(CleanTest, CleanRspFile) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cc\n"
+"  command = cc $in > $out\n"
+"  rspfile = $rspfile\n"
+"  rspfile_content=$in\n"
+"build out1: cc in1\n"
+"  rspfile = cc1.rsp\n"
+"  rspfile_content=$in\n"));
+  fs_.Create("out1", 1, "");
+  fs_.Create("cc1.rsp", 1, "");
+
+  Cleaner cleaner(&state_, config_, &fs_);
+  EXPECT_EQ(0, cleaner.CleanAll());
+  EXPECT_EQ(2, cleaner.cleaned_files_count());
+  EXPECT_EQ(2u, fs_.files_removed_.size());
+}
+
+TEST_F(CleanTest, CleanRsp) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cat_rsp \n"
+"  command = cat $rspfile > $out\n"
+"  rspfile = $rspfile\n"
+"  rspfile_content = $in\n"
+"build in1: cat src1\n"
+"build out1: cat in1\n"
+"build in2: cat_rsp src2\n"
+"  rspfile=in2.rsp\n"
+"  rspfile_content=$in\n"
+"build out2: cat_rsp in2\n"
+"  rspfile=out2.rsp\n"
+"  rspfile_content=$in\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2.rsp", 1, "");
+  fs_.Create("out2.rsp", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  Cleaner cleaner(&state_, config_, &fs_);
+  ASSERT_EQ(0, cleaner.cleaned_files_count());
+  ASSERT_EQ(0, cleaner.CleanTarget("out1"));
+  EXPECT_EQ(2, cleaner.cleaned_files_count()); 
+  ASSERT_EQ(0, cleaner.CleanTarget("in2"));
+  EXPECT_EQ(2, cleaner.cleaned_files_count());
+  ASSERT_EQ(0, cleaner.CleanRule("cat_rsp"));
+  EXPECT_EQ(2, cleaner.cleaned_files_count());
+
+  EXPECT_EQ(6u, fs_.files_removed_.size());
+
+  // Check they are removed.
+  EXPECT_EQ(0, fs_.Stat("in1"));
+  EXPECT_EQ(0, fs_.Stat("out1"));
+  EXPECT_EQ(0, fs_.Stat("in2"));
+  EXPECT_EQ(0, fs_.Stat("out2"));
+  EXPECT_EQ(0, fs_.Stat("in2.rsp"));
+  EXPECT_EQ(0, fs_.Stat("out2.rsp"));
+
+  fs_.files_removed_.clear();
+}
+
 TEST_F(CleanTest, CleanFailure) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
                                       "build dir: cat src1\n"));
diff --git a/src/disk_interface.cc b/src/disk_interface.cc
index 789f199..96a1e59 100644
--- a/src/disk_interface.cc
+++ b/src/disk_interface.cc
@@ -103,6 +103,26 @@ TimeStamp RealDiskInterface::Stat(const string& path) {
 #endif
 }
 
+bool RealDiskInterface::WriteFile(const string & path, const string & contents) {
+  FILE * fp = fopen(path.c_str(), "w");
+  if (fp == NULL) {
+    Error("WriteFile(%s): Unable to create file. %s", path.c_str(), strerror(errno));
+    return false;
+  }
+ 
+  if (fwrite(contents.data(), 1, contents.length(), fp) < contents.length())  {
+    Error("WriteFile(%s): Unable to write to the file. %s", path.c_str(), strerror(errno));
+    return false;
+  }
+
+  if (fclose(fp) == EOF) {
+    Error("WriteFile(%s): Unable to close the file. %s", path.c_str(), strerror(errno));
+    return false;
+  }
+
+  return true;
+}
+
 bool RealDiskInterface::MakeDir(const string& path) {
   if (::MakeDir(path) < 0) {
     Error("mkdir(%s): %s", path.c_str(), strerror(errno));
diff --git a/src/disk_interface.h b/src/disk_interface.h
index b0fed3d..c0f5ee5 100644
--- a/src/disk_interface.h
+++ b/src/disk_interface.h
@@ -34,6 +34,10 @@ struct DiskInterface {
   /// Create a directory, returning false on failure.
   virtual bool MakeDir(const string& path) = 0;
 
+  /// Create a file, with the specified name and contents
+  /// Returns true on success, false on failure
+  virtual bool WriteFile(const string & path, const string & contents) = 0;
+
   /// Read a file to a string.  Fill in |err| on error.
   virtual string ReadFile(const string& path, string* err) = 0;
 
@@ -54,6 +58,7 @@ struct RealDiskInterface : public DiskInterface {
   virtual ~RealDiskInterface() {}
   virtual TimeStamp Stat(const string& path);
   virtual bool MakeDir(const string& path);
+  virtual bool WriteFile(const string & path, const string & contents);
   virtual string ReadFile(const string& path, string* err);
   virtual int RemoveFile(const string& path);
 };
diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc
index b8ad5e6..052a94b 100644
--- a/src/disk_interface_test.cc
+++ b/src/disk_interface_test.cc
@@ -102,6 +102,10 @@ struct StatTest : public StateTestWithBuiltinRules,
                   public DiskInterface {
   // DiskInterface implementation.
   virtual TimeStamp Stat(const string& path);
+  virtual bool WriteFile(const string& path, const string & contents) {
+    assert(false);
+    return true;
+  }
   virtual bool MakeDir(const string& path) {
     assert(false);
     return false;
diff --git a/src/graph.cc b/src/graph.cc
index 5320cfa..10aaeb3 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -76,7 +76,7 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface,
   // date outputs, etc.  Visit all outputs and determine whether they're dirty.
   if (!dirty) {
     BuildLog* build_log = state ? state->build_log_ : 0;
-    string command = EvaluateCommand();
+    string command = EvaluateCommand(true);
 
     for (vector<Node*>::iterator i = outputs_.begin();
          i != outputs_.end(); ++i) {
@@ -204,9 +204,12 @@ string EdgeEnv::MakePathList(vector<Node*>::iterator begin,
   return result;
 }
 
-string Edge::EvaluateCommand() {
+string Edge::EvaluateCommand(bool incl_rsp_file) {
   EdgeEnv env(this);
-  return rule_->command().Evaluate(&env);
+  string command = rule_->command().Evaluate(&env);
+  if (incl_rsp_file && HasRspFile()) 
+    command += ";rspfile=" + GetRspFileContent();
+  return command;
 }
 
 string Edge::EvaluateDepFile() {
@@ -219,6 +222,20 @@ string Edge::GetDescription() {
   return rule_->description().Evaluate(&env);
 }
 
+bool Edge::HasRspFile() {
+  return !rule_->rspfile_.empty();
+}
+
+string Edge::GetRspFile() {
+  EdgeEnv env(this);
+  return rule_->rspfile_.Evaluate(&env);
+}
+
+string Edge::GetRspFileContent() {
+  EdgeEnv env(this);
+  return rule_->rspfile_content_.Evaluate(&env);
+}
+
 bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface,
                        string* err) {
   METRIC_RECORD("depfile load");
diff --git a/src/graph.h b/src/graph.h
index c83d790..b66316d 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -121,6 +121,8 @@ struct Rule {
   EvalString command_;
   EvalString description_;
   EvalString depfile_;
+  EvalString rspfile_;
+  EvalString rspfile_content_;
 };
 
 struct BuildLog;
@@ -146,9 +148,22 @@ struct Edge {
   /// Return true if all inputs' in-edges are ready.
   bool AllInputsReady() const;
 
-  string EvaluateCommand();  // XXX move to env, take env ptr
+  /// Expand all variables in a command and return it as a string.
+  /// If incl_rsp_file is enabled, the string will also contain the 
+  /// full contents of a response file (if applicable)
+  string EvaluateCommand(bool incl_rsp_file = false);  // XXX move to env, take env ptr
   string EvaluateDepFile();
   string GetDescription();
+  
+  /// Does the edge use a response file?
+  bool HasRspFile();
+  
+  /// Get the path to the response file
+  string GetRspFile();
+
+  /// Get the contents of the response file
+  string GetRspFileContent();
+
   bool LoadDepFile(State* state, DiskInterface* disk_interface, string* err);
 
   void Dump();
diff --git a/src/parsers.cc b/src/parsers.cc
index 5d347b2..c3844fb 100644
--- a/src/parsers.cc
+++ b/src/parsers.cc
@@ -121,6 +121,10 @@ bool ManifestParser::ParseRule(string* err) {
       rule->generator_ = true;
     } else if (key == "restat") {
       rule->restat_ = true;
+    } else if (key == "rspfile") {
+      rule->rspfile_ = value;
+    } else if (key == "rspfile_content") {
+      rule->rspfile_content_ = value;
     } else {
       // Die on other keyvals for now; revisit if we want to add a
       // scope here.
@@ -128,6 +132,9 @@ bool ManifestParser::ParseRule(string* err) {
     }
   }
 
+  if (rule->rspfile_.empty() != rule->rspfile_content_.empty())
+    return lexer_.Error("rspfile and rspfile_content need to be both specified", err);
+
   if (rule->command_.empty())
     return lexer_.Error("expected 'command =' line", err);
 
diff --git a/src/parsers_test.cc b/src/parsers_test.cc
index 2a30a83..ff04608 100644
--- a/src/parsers_test.cc
+++ b/src/parsers_test.cc
@@ -97,6 +97,24 @@ TEST_F(ParserTest, IgnoreIndentedBlankLines) {
   EXPECT_EQ("1", state.bindings_.LookupVariable("variable"));
 }
 
+TEST_F(ParserTest, ResponseFiles) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(
+"rule cat_rsp\n"
+"  command = cat $rspfile > $out\n"
+"  rspfile = $rspfile\n"
+"  rspfile_content = $in\n"
+"\n"
+"build out: cat_rsp in\n"
+"  rspfile=out.rsp\n"));
+
+  ASSERT_EQ(2u, state.rules_.size());
+  const Rule* rule = state.rules_.begin()->second;
+  EXPECT_EQ("cat_rsp", rule->name());
+  EXPECT_EQ("[cat ][$rspfile][ > ][$out]", rule->command().Serialize());
+  EXPECT_EQ("[$rspfile]", rule->rspfile_.Serialize());
+  EXPECT_EQ("[$in]", rule->rspfile_content_.Serialize());
+}
+
 TEST_F(ParserTest, Variables) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(
 "l = one-letter-test\n"
diff --git a/src/test.cc b/src/test.cc
index 53bd798..2fbb4df 100644
--- a/src/test.cc
+++ b/src/test.cc
@@ -93,6 +93,7 @@ void VirtualFileSystem::Create(const string& path, int time,
                                const string& contents) {
   files_[path].mtime = time;
   files_[path].contents = contents;
+  files_created_.insert(path);
 }
 
 TimeStamp VirtualFileSystem::Stat(const string& path) {
@@ -102,6 +103,11 @@ TimeStamp VirtualFileSystem::Stat(const string& path) {
   return 0;
 }
 
+bool VirtualFileSystem::WriteFile(const string& path, const string& contents) {
+  Create(path, 0, contents);
+  return true;
+}
+
 bool VirtualFileSystem::MakeDir(const string& path) {
   directories_made_.push_back(path);
   return true;  // success
diff --git a/src/test.h b/src/test.h
index d730151..97f7cb1 100644
--- a/src/test.h
+++ b/src/test.h
@@ -45,6 +45,7 @@ struct VirtualFileSystem : public DiskInterface {
 
   // DiskInterface
   virtual TimeStamp Stat(const string& path);
+  virtual bool WriteFile(const string& path, const string& contents);
   virtual bool MakeDir(const string& path);
   virtual string ReadFile(const string& path, string* err);
   virtual int RemoveFile(const string& path);
@@ -60,6 +61,7 @@ struct VirtualFileSystem : public DiskInterface {
   typedef map<string, Entry> FileMap;
   FileMap files_;
   set<string> files_removed_;
+  set<string> files_created_;
 };
 
 struct ScopedTempDir {