BuildLog: Use DiskInterface instance.

Make the BuildLog class take a DiskInterface
reference in its constructor, to ensure that all
i/o operations use the same interface.

+ Adjust call sites accordingly. For tests, always
  use a SystemDiskInterface instead of the VirtualFileSystem
  instance when the latter is available, as this is
  exactly what the previous code was doing.

src/build_log.cc

@@ -21,7 +21,6 @@
 #endif
 
 #include "build_log.h"
-#include "disk_interface.h"
 
 #include <cassert>
 #include <errno.h>
@@ -121,8 +120,8 @@ BuildLog::LogEntry::LogEntry(const string& output, uint64_t command_hash,
     start_time(start_time), end_time(end_time), mtime(mtime)
 {}
 
-BuildLog::BuildLog()
-  : log_file_(NULL), needs_recompaction_(false) {}
+BuildLog::BuildLog(DiskInterface& disk_interface)
+    : disk_interface_(&disk_interface) {}
 
 BuildLog::~BuildLog() {
   Close();
@@ -186,7 +185,7 @@ bool BuildLog::OpenForWriteIfNeeded() {
   if (log_file_ || log_file_path_.empty()) {
     return true;
   }
-  log_file_ = fopen(log_file_path_.c_str(), "ab");
+  log_file_ = disk_interface_->OpenFile(log_file_path_, "ab");
   if (!log_file_) {
     return false;
   }
@@ -260,7 +259,7 @@ struct LineReader {
 
 LoadStatus BuildLog::Load(const string& path, string* err) {
   METRIC_RECORD(".ninja_log load");
-  FILE* file = fopen(path.c_str(), "r");
+  FILE* file = disk_interface_->OpenFile(path, "r");
   if (!file) {
     if (errno == ENOENT)
       return LOAD_NOT_FOUND;
@@ -290,7 +289,7 @@ LoadStatus BuildLog::Load(const string& path, string* err) {
       }
       if (invalid_log_version) {
         fclose(file);
-        unlink(path.c_str());
+        disk_interface_->RemoveFile(path);
         // Don't report this as a failure. A missing build log will cause
         // us to rebuild the outputs anyway.
         return LOAD_NOT_FOUND;
@@ -394,8 +393,8 @@ bool BuildLog::Recompact(const string& path, const BuildLogUser& user,
   METRIC_RECORD(".ninja_log recompact");
 
   Close();
-  string temp_path = path + ".recompact";
-  FILE* f = fopen(temp_path.c_str(), "wb");
+  std::string temp_path = path + ".recompact";
+  FILE* f = disk_interface_->OpenFile(temp_path, "wb");
   if (!f) {
     *err = strerror(errno);
     return false;
@@ -425,12 +424,12 @@ bool BuildLog::Recompact(const string& path, const BuildLogUser& user,
     entries_.erase(dead_outputs[i]);
 
   fclose(f);
-  if (unlink(path.c_str()) < 0) {
+  if (disk_interface_->RemoveFile(path) < 0) {
     *err = strerror(errno);
     return false;
   }
 
-  if (rename(temp_path.c_str(), path.c_str()) < 0) {
+  if (!disk_interface_->RenameFile(temp_path, path)) {
     *err = strerror(errno);
     return false;
   }
@@ -439,14 +438,13 @@ bool BuildLog::Recompact(const string& path, const BuildLogUser& user,
 }
 
 bool BuildLog::Restat(const StringPiece path,
-                      const DiskInterface& disk_interface,
                       const int output_count, char** outputs,
                       std::string* const err) {
   METRIC_RECORD(".ninja_log restat");
 
   Close();
   std::string temp_path = path.AsString() + ".restat";
-  FILE* f = fopen(temp_path.c_str(), "wb");
+  FILE* f = disk_interface_->OpenFile(temp_path, "wb");
   if (!f) {
     *err = strerror(errno);
     return false;
@@ -466,7 +464,7 @@ bool BuildLog::Restat(const StringPiece path,
       }
     }
     if (!skip) {
-      const TimeStamp mtime = disk_interface.Stat(i->second->output, err);
+      const TimeStamp mtime = disk_interface_->Stat(i->second->output, err);
       if (mtime == -1) {
         fclose(f);
         return false;
@@ -482,12 +480,12 @@ bool BuildLog::Restat(const StringPiece path,
   }
 
   fclose(f);
-  if (unlink(path.str_) < 0) {
+  if (disk_interface_->RemoveFile(path.str_) < 0) {
     *err = strerror(errno);
     return false;
   }
 
-  if (rename(temp_path.c_str(), path.str_) < 0) {
+  if (!disk_interface_->RenameFile(temp_path, path.AsString())) {
     *err = strerror(errno);
     return false;
   }

src/build_log.h

@@ -15,9 +15,11 @@
 #ifndef NINJA_BUILD_LOG_H_
 #define NINJA_BUILD_LOG_H_
 
+#include <memory>
 #include <string>
 #include <stdio.h>
 
+#include "disk_interface.h"
 #include "hash_map.h"
 #include "load_status.h"
 #include "timestamp.h"
@@ -41,7 +43,10 @@ struct BuildLogUser {
 /// 2) timing information, perhaps for generating reports
 /// 3) restat information
 struct BuildLog {
-  BuildLog();
+  /// Constructor takes a reference to an existing DiskInterface instance.
+  BuildLog(DiskInterface& disk_interface);
+
+  /// Destructor.
   ~BuildLog();
 
   /// Prepares writing to the log file without actually opening it - that will
@@ -87,21 +92,24 @@ struct BuildLog {
                  std::string* err);
 
   /// Restat all outputs in the log
-  bool Restat(StringPiece path, const DiskInterface& disk_interface,
-              int output_count, char** outputs, std::string* err);
+  bool Restat(StringPiece path, int output_count, char** outputs, std::string* err);
 
   typedef ExternalStringHashMap<LogEntry*>::Type Entries;
   const Entries& entries() const { return entries_; }
 
  private:
+  /// Default destructor is private and never implemented.
+  BuildLog() = delete;
+
   /// Should be called before using log_file_. When false is returned, errno
   /// will be set.
   bool OpenForWriteIfNeeded();
 
+  DiskInterface* disk_interface_ = nullptr;
   Entries entries_;
-  FILE* log_file_;
+  FILE* log_file_ = nullptr;
   std::string log_file_path_;
-  bool needs_recompaction_;
+  bool needs_recompaction_ = false;
 };
 
 #endif // NINJA_BUILD_LOG_H_

src/build_log_perftest.cc

@@ -35,7 +35,8 @@ struct NoDeadPaths : public BuildLogUser {
 };
 
 bool WriteTestData(string* err) {
-  BuildLog log;
+  SystemDiskInterface disk_interface;
+  BuildLog log(disk_interface);
 
   NoDeadPaths no_dead_paths;
   if (!log.OpenForWrite(kTestFilename, no_dead_paths, err))
@@ -109,9 +110,10 @@ int main() {
     return 1;
   }
 
+  SystemDiskInterface disk_interface;
   {
     // Read once to warm up disk cache.
-    BuildLog log;
+    BuildLog log(disk_interface);
     if (log.Load(kTestFilename, &err) == LOAD_ERROR) {
       fprintf(stderr, "Failed to read test data: %s\n", err.c_str());
       return 1;
@@ -120,7 +122,7 @@ int main() {
   const int kNumRepetitions = 5;
   for (int i = 0; i < kNumRepetitions; ++i) {
     int64_t start = GetTimeMillis();
-    BuildLog log;
+    BuildLog log(disk_interface);
     if (log.Load(kTestFilename, &err) == LOAD_ERROR) {
       fprintf(stderr, "Failed to read test data: %s\n", err.c_str());
       return 1;