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

Calling DepsLog/BuildLog::OpenForWrite will now only save the file path.
The actual opening of the file (moved to OpenForWriteIfNeeded) happens
right before the first write attempt.

This is needed so that the files aren't held open when the generator
runs (i.e. RebuildManifest) as it may call tools like recompact which
won't be able to open the file on Windows.

The disadvantage is that now the error reporting happens at a later time
and will be reported as a failed write, not a failed open.

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) {

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_;
 };
 

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;
+}

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_;