use DiskInterface to create the build directory

Fixes issue #392 (didn't handle creating nested build dirs right).

Moves MakeDir out of util.h; all code should go through
DiskInterface to simplify testing.  Moves ownership of the
DiskInterface into the client of the Builder, which also allows
removing some code that reached inside the object as well as
a minor leak.

7 files changed, 31 insertions, 34 deletions

diff --git a/src/build.cc b/src/build.cc
index a1c94e4..6d1318c 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -552,9 +552,9 @@ struct DryRunCommandRunner : public CommandRunner {
   queue<Edge*> finished_;
 };
 
-Builder::Builder(State* state, const BuildConfig& config)
-    : state_(state), config_(config) {
-  disk_interface_ = new RealDiskInterface;
+Builder::Builder(State* state, const BuildConfig& config,
+                 DiskInterface* disk_interface)
+    : state_(state), config_(config), disk_interface_(disk_interface) {
   status_ = new BuildStatus(config);
   log_ = state->build_log_;
 }
diff --git a/src/build.h b/src/build.h
index 5e26bbb..986e8a9 100644
--- a/src/build.h
+++ b/src/build.h
@@ -119,7 +119,8 @@ struct BuildConfig {
 
 /// Builder wraps the build process: starting commands, updating status.
 struct Builder {
-  Builder(State* state, const BuildConfig& config);
+  Builder(State* state, const BuildConfig& config,
+          DiskInterface* disk_interface);
   ~Builder();
 
   /// Clean up after interrupted commands by deleting output files.
@@ -144,12 +145,13 @@ struct Builder {
   State* state_;
   const BuildConfig& config_;
   Plan plan_;
-  DiskInterface* disk_interface_;
   auto_ptr<CommandRunner> command_runner_;
   BuildStatus* status_;
   BuildLog* log_;
 
  private:
+  DiskInterface* disk_interface_;
+
   // Unimplemented copy ctor and operator= ensure we don't copy the auto_ptr.
   Builder(const Builder &other);        // DO NOT IMPLEMENT
   void operator=(const Builder &other); // DO NOT IMPLEMENT
diff --git a/src/build_test.cc b/src/build_test.cc
index a3f345b..574ffb4 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -178,9 +178,8 @@ TEST_F(PlanTest, DependencyCycle) {
 
 struct BuildTest : public StateTestWithBuiltinRules,
                    public CommandRunner {
-  BuildTest() : config_(MakeConfig()), builder_(&state_, config_), now_(1),
-                last_command_(NULL), status_(config_) {
-    builder_.disk_interface_ = &fs_;
+  BuildTest() : config_(MakeConfig()), builder_(&state_, config_, &fs_),
+                now_(1), last_command_(NULL), status_(config_) {
     builder_.command_runner_.reset(this);
     AssertParse(&state_,
 "build cat1: cat in1\n"
@@ -212,11 +211,10 @@ struct BuildTest : public StateTestWithBuiltinRules,
   }
 
   BuildConfig config_;
+  VirtualFileSystem fs_;
   Builder builder_;
   int now_;
 
-  VirtualFileSystem fs_;
-
   vector<string> commands_ran_;
   Edge* last_command_;
   BuildStatus status_;
diff --git a/src/disk_interface.cc b/src/disk_interface.cc
index 74f33c4..515ff59 100644
--- a/src/disk_interface.cc
+++ b/src/disk_interface.cc
@@ -18,9 +18,11 @@
 #include <stdio.h>
 #include <string.h>
 #include <sys/stat.h>
+#include <sys/types.h>
 
 #ifdef _WIN32
 #include <windows.h>
+#include <direct.h>  // _mkdir
 #endif
 
 #include "util.h"
@@ -42,6 +44,14 @@ string DirName(const string& path) {
   return path.substr(0, slash_pos);
 }
 
+int MakeDir(const string& path) {
+#ifdef _WIN32
+  return _mkdir(path.c_str());
+#else
+  return mkdir(path.c_str(), 0777);
+#endif
+}
+
 }  // namespace
 
 // DiskInterface ---------------------------------------------------------------
diff --git a/src/ninja.cc b/src/ninja.cc
index 778eb53..d212579 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -32,6 +32,7 @@
 #include "build.h"
 #include "build_log.h"
 #include "clean.h"
+#include "disk_interface.h"
 #include "edit_distance.h"
 #include "explain.h"
 #include "graph.h"
@@ -66,6 +67,8 @@ struct Globals {
   BuildConfig config;
   /// Loaded state (rules, nodes). This is a pointer so it can be reset.
   State* state;
+  /// Functions for interacting with the disk.
+  RealDiskInterface disk_interface;
 };
 
 /// Print usage information.
@@ -122,16 +125,16 @@ struct RealFileReader : public ManifestParser::FileReader {
 
 /// Rebuild the build manifest, if necessary.
 /// Returns true if the manifest was rebuilt.
-bool RebuildManifest(State* state, const BuildConfig& config,
-                     const char* input_file, string* err) {
+bool RebuildManifest(Globals* globals, const char* input_file, string* err) {
   string path = input_file;
   if (!CanonicalizePath(&path, err))
     return false;
-  Node* node = state->LookupNode(path);
+  Node* node = globals->state->LookupNode(path);
   if (!node)
     return false;
 
-  Builder manifest_builder(state, config);
+  Builder manifest_builder(globals->state, globals->config,
+                           &globals->disk_interface);
   if (!manifest_builder.AddTarget(node, err))
     return false;
 
@@ -579,7 +582,7 @@ int RunBuild(Globals* globals, int argc, char** argv) {
     return 1;
   }
 
-  Builder builder(globals->state, globals->config);
+  Builder builder(globals->state, globals->config, &globals->disk_interface);
   for (size_t i = 0; i < targets.size(); ++i) {
     if (!builder.AddTarget(targets[i], &err)) {
       if (!err.empty()) {
@@ -745,12 +748,12 @@ reload:
   const char* kLogPath = ".ninja_log";
   string log_path = kLogPath;
   if (!build_dir.empty()) {
-    if (MakeDir(build_dir) < 0 && errno != EEXIST) {
+    log_path = build_dir + "/" + kLogPath;
+    if (globals.disk_interface.MakeDirs(log_path) < 0 && errno != EEXIST) {
       Error("creating build directory %s: %s",
             build_dir.c_str(), strerror(errno));
       return 1;
     }
-    log_path = build_dir + "/" + kLogPath;
   }
 
   if (!build_log.Load(log_path, &err)) {
@@ -765,7 +768,7 @@ reload:
 
   if (!rebuilt_manifest) { // Don't get caught in an infinite loop by a rebuild
                            // target that is never up to date.
-    if (RebuildManifest(globals.state, globals.config, input_file, &err)) {
+    if (RebuildManifest(&globals, input_file, &err)) {
       rebuilt_manifest = true;
       globals.ResetState();
       goto reload;
diff --git a/src/util.cc b/src/util.cc
index be2347c..7c2f895 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -34,10 +34,6 @@
 
 #include <vector>
 
-#ifdef _WIN32
-#include <direct.h>  // _mkdir
-#endif
-
 #if defined(__APPLE__) || defined(__FreeBSD__)
 #include <sys/sysctl.h>
 #elif defined(linux)
@@ -163,14 +159,6 @@ bool CanonicalizePath(char* path, int* len, string* err) {
   return true;
 }
 
-int MakeDir(const string& path) {
-#ifdef _WIN32
-  return _mkdir(path.c_str());
-#else
-  return mkdir(path.c_str(), 0777);
-#endif
-}
-
 int ReadFile(const string& path, string* contents, string* err) {
   FILE* f = fopen(path.c_str(), "r");
   if (!f) {
diff --git a/src/util.h b/src/util.h
index fc701cd..bfcbfa8 100644
--- a/src/util.h
+++ b/src/util.h
@@ -41,10 +41,6 @@ bool CanonicalizePath(string* path, string* err);
 
 bool CanonicalizePath(char* path, int* len, string* err);
 
-/// Create a directory (mode 0777 on Unix).
-/// Portability abstraction.
-int MakeDir(const string& path);
-
 /// Read a file to a string.
 /// Returns -errno and fills in \a err on error.
 int ReadFile(const string& path, string* contents, string* err);