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;