From cc222d3f6ba769fd800b1dfdb1699fbb84516c7b Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 7 Aug 2012 13:58:55 -0700 Subject: 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. --- src/build.cc | 6 +++--- src/build.h | 6 ++++-- src/build_test.cc | 8 +++----- src/disk_interface.cc | 10 ++++++++++ src/ninja.cc | 19 +++++++++++-------- src/util.cc | 12 ------------ src/util.h | 4 ---- 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 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 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 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 #include #include +#include #ifdef _WIN32 #include +#include // _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 -#ifdef _WIN32 -#include // _mkdir -#endif - #if defined(__APPLE__) || defined(__FreeBSD__) #include #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); -- cgit v0.12