Merge pull request #1780 from jhasse/delay-open

Delay actually opening log files until the first write, fix #1724

4 files changed, 89 insertions, 47 deletions

diff --git a/src/build_log.cc b/src/build_log.cc
index 98543b6..e5f179c 100644
--- a/src/build_log.cc
+++ b/src/build_log.cc
@@ -23,6 +23,7 @@
 #include "build_log.h"
 #include "disk_interface.h"
 
+#include <cassert>
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
@@ -132,25 +133,9 @@ bool BuildLog::OpenForWrite(const string& path, const BuildLogUser& user,
       return false;
   }
 
-  log_file_ = fopen(path.c_str(), "ab");
-  if (!log_file_) {
-    *err = strerror(errno);
-    return false;
-  }
-  setvbuf(log_file_, NULL, _IOLBF, BUFSIZ);
-  SetCloseOnExec(fileno(log_file_));
-
-  // Opening a file in append mode doesn't set the file pointer to the file's
-  // end on Windows. Do that explicitly.
-  fseek(log_file_, 0, SEEK_END);
-
-  if (ftell(log_file_) == 0) {
-    if (fprintf(log_file_, kFileSignature, kCurrentVersion) < 0) {
-      *err = strerror(errno);
-      return false;
-    }
-  }
-
+  assert(!log_file_);
+  log_file_path_ = path;  // we don't actually open the file right now, but will
+                          // do so on the first write attempt
   return true;
 }
 
@@ -174,6 +159,9 @@ bool BuildLog::RecordCommand(Edge* edge, int start_time, int end_time,
     log_entry->end_time = end_time;
     log_entry->mtime = mtime;
 
+    if (!OpenForWriteIfNeeded()) {
+      return false;
+    }
     if (log_file_) {
       if (!WriteEntry(log_file_, *log_entry))
         return false;
@@ -186,11 +174,36 @@ bool BuildLog::RecordCommand(Edge* edge, int start_time, int end_time,
 }
 
 void BuildLog::Close() {
+  OpenForWriteIfNeeded();  // create the file even if nothing has been recorded
   if (log_file_)
     fclose(log_file_);
   log_file_ = NULL;
 }
 
+bool BuildLog::OpenForWriteIfNeeded() {
+  if (log_file_path_.empty()) {
+    return true;
+  }
+  log_file_ = fopen(log_file_path_.c_str(), "ab");
+  if (!log_file_) {
+    return false;
+  }
+  setvbuf(log_file_, NULL, _IOLBF, BUFSIZ);
+  SetCloseOnExec(fileno(log_file_));
+
+  // Opening a file in append mode doesn't set the file pointer to the file's
+  // end on Windows. Do that explicitly.
+  fseek(log_file_, 0, SEEK_END);
+
+  if (ftell(log_file_) == 0) {
+    if (fprintf(log_file_, kFileSignature, kCurrentVersion) < 0) {
+      return false;
+    }
+  }
+  log_file_path_.clear();
+  return true;
+}
+
 struct LineReader {
   explicit LineReader(FILE* file)
     : file_(file), buf_end_(buf_), line_start_(buf_), line_end_(NULL) {
diff --git a/src/build_log.h b/src/build_log.h
index ebe0530..6d060d1 100644
--- a/src/build_log.h
+++ b/src/build_log.h
@@ -45,7 +45,10 @@ struct BuildLog {
   BuildLog();
   ~BuildLog();
 
+  /// Prepares writing to the log file without actually opening it - that will
+  /// happen when/if it's needed
   bool OpenForWrite(const string& path, const BuildLogUser& user, string* err);
+
   bool RecordCommand(Edge* edge, int start_time, int end_time,
                      TimeStamp mtime = 0);
   void Close();
@@ -91,8 +94,13 @@ struct BuildLog {
   const Entries& entries() const { return entries_; }
 
  private:
+  /// Should be called before using log_file_. When false is returned, errno
+  /// will be set.
+  bool OpenForWriteIfNeeded();
+
   Entries entries_;
   FILE* log_file_;
+  std::string log_file_path_;
   bool needs_recompaction_;
 };
 
diff --git a/src/deps_log.cc b/src/deps_log.cc
index 59a1956..1fb65ae 100644
--- a/src/deps_log.cc
+++ b/src/deps_log.cc
@@ -49,34 +49,9 @@ bool DepsLog::OpenForWrite(const string& path, string* err) {
       return false;
   }
 
-  file_ = fopen(path.c_str(), "ab");
-  if (!file_) {
-    *err = strerror(errno);
-    return false;
-  }
-  // Set the buffer size to this and flush the file buffer after every record
-  // to make sure records aren't written partially.
-  setvbuf(file_, NULL, _IOFBF, kMaxRecordSize + 1);
-  SetCloseOnExec(fileno(file_));
-
-  // Opening a file in append mode doesn't set the file pointer to the file's
-  // end on Windows. Do that explicitly.
-  fseek(file_, 0, SEEK_END);
-
-  if (ftell(file_) == 0) {
-    if (fwrite(kFileSignature, sizeof(kFileSignature) - 1, 1, file_) < 1) {
-      *err = strerror(errno);
-      return false;
-    }
-    if (fwrite(&kCurrentVersion, 4, 1, file_) < 1) {
-      *err = strerror(errno);
-      return false;
-    }
-  }
-  if (fflush(file_) != 0) {
-    *err = strerror(errno);
-    return false;
-  }
+  assert(!file_);
+  file_path_ = path;  // we don't actually open the file right now, but will do
+                      // so on the first write attempt
   return true;
 }
 
@@ -132,6 +107,10 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime,
     errno = ERANGE;
     return false;
   }
+
+  if (!OpenForWriteIfNeeded()) {
+    return false;
+  }
   size |= 0x80000000;  // Deps record: set high bit.
   if (fwrite(&size, 4, 1, file_) < 1)
     return false;
@@ -162,6 +141,7 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime,
 }
 
 void DepsLog::Close() {
+  OpenForWriteIfNeeded();  // create the file even if nothing has been recorded
   if (file_)
     fclose(file_);
   file_ = NULL;
@@ -396,6 +376,10 @@ bool DepsLog::RecordId(Node* node) {
     errno = ERANGE;
     return false;
   }
+
+  if (!OpenForWriteIfNeeded()) {
+    return false;
+  }
   if (fwrite(&size, 4, 1, file_) < 1)
     return false;
   if (fwrite(node->path().data(), path_size, 1, file_) < 1) {
@@ -416,3 +400,35 @@ bool DepsLog::RecordId(Node* node) {
 
   return true;
 }
+
+bool DepsLog::OpenForWriteIfNeeded() {
+  if (file_path_.empty()) {
+    return true;
+  }
+  file_ = fopen(file_path_.c_str(), "ab");
+  if (!file_) {
+    return false;
+  }
+  // Set the buffer size to this and flush the file buffer after every record
+  // to make sure records aren't written partially.
+  setvbuf(file_, NULL, _IOFBF, kMaxRecordSize + 1);
+  SetCloseOnExec(fileno(file_));
+
+  // Opening a file in append mode doesn't set the file pointer to the file's
+  // end on Windows. Do that explicitly.
+  fseek(file_, 0, SEEK_END);
+
+  if (ftell(file_) == 0) {
+    if (fwrite(kFileSignature, sizeof(kFileSignature) - 1, 1, file_) < 1) {
+      return false;
+    }
+    if (fwrite(&kCurrentVersion, 4, 1, file_) < 1) {
+      return false;
+    }
+  }
+  if (fflush(file_) != 0) {
+    return false;
+  }
+  file_path_.clear();
+  return true;
+}
diff --git a/src/deps_log.h b/src/deps_log.h
index e7974a1..c4ada8b 100644
--- a/src/deps_log.h
+++ b/src/deps_log.h
@@ -110,8 +110,13 @@ struct DepsLog {
   // Write a node name record, assigning it an id.
   bool RecordId(Node* node);
 
+  /// Should be called before using file_. When false is returned, errno will
+  /// be set.
+  bool OpenForWriteIfNeeded();
+
   bool needs_recompaction_;
   FILE* file_;
+  std::string file_path_;
 
   /// Maps id -> Node.
   vector<Node*> nodes_;