diff --git a/src/build_log.cc b/src/build_log.cc index 073d2fe81e..eb93121503 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -21,13 +21,13 @@ #endif #include "build_log.h" -#include "disk_interface.h" -#include #include #include #include +#include + #ifndef _WIN32 #include #include @@ -72,8 +72,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(); @@ -137,7 +137,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; } @@ -211,7 +211,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; @@ -241,7 +241,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; @@ -345,8 +345,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; @@ -376,12 +376,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; } @@ -389,15 +389,13 @@ bool BuildLog::Recompact(const string& path, const BuildLogUser& user, return true; } -bool BuildLog::Restat(const StringPiece path, - const DiskInterface& disk_interface, - const int output_count, char** outputs, - std::string* const err) { +bool BuildLog::Restat(const StringPiece path, 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; @@ -417,7 +415,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; @@ -433,12 +431,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; } diff --git a/src/build_log.h b/src/build_log.h index 1223c2a16a..8e322d47d5 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -15,9 +15,12 @@ #ifndef NINJA_BUILD_LOG_H_ #define NINJA_BUILD_LOG_H_ -#include #include +#include +#include + +#include "disk_interface.h" #include "hash_map.h" #include "load_status.h" #include "timestamp.h" @@ -41,7 +44,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 +93,25 @@ 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::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_ diff --git a/src/build_log_perftest.cc b/src/build_log_perftest.cc index 5a936198fb..66ba096480 100644 --- a/src/build_log_perftest.cc +++ b/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; diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 630b1f1a92..30b3047be0 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -36,12 +36,14 @@ const char kTestFilename[] = "BuildLogTest-tempfile"; struct BuildLogTest : public StateTestWithBuiltinRules, public BuildLogUser { virtual void SetUp() { // In case a crashing test left a stale file behind. - unlink(kTestFilename); - } - virtual void TearDown() { - unlink(kTestFilename); + disk_interface_.RemoveFile(kTestFilename); } + virtual void TearDown() { disk_interface_.RemoveFile(kTestFilename); } virtual bool IsPathDead(StringPiece s) const { return false; } + + BuildLog CreateBuildLog() { return BuildLog(disk_interface_); } + + SystemDiskInterface disk_interface_; }; TEST_F(BuildLogTest, WriteRead) { @@ -49,7 +51,7 @@ TEST_F(BuildLogTest, WriteRead) { "build out: cat mid\n" "build mid: cat in\n"); - BuildLog log1; + BuildLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); @@ -57,7 +59,7 @@ TEST_F(BuildLogTest, WriteRead) { log1.RecordCommand(state_.edges_[1], 20, 25); log1.Close(); - BuildLog log2; + BuildLog log2(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &err)); ASSERT_EQ("", err); @@ -76,7 +78,7 @@ TEST_F(BuildLogTest, FirstWriteAddsSignature) { const char kExpectedVersion[] = "# ninja log vX\n"; const size_t kVersionPos = strlen(kExpectedVersion) - 2; // Points at 'X'. - BuildLog log; + BuildLog log(disk_interface_); string contents, err; EXPECT_TRUE(log.OpenForWrite(kTestFilename, *this, &err)); @@ -103,7 +105,7 @@ TEST_F(BuildLogTest, FirstWriteAddsSignature) { } TEST_F(BuildLogTest, DoubleEntry) { - FILE* f = fopen(kTestFilename, "wb"); + FILE* f = disk_interface_.OpenFile(kTestFilename, "wb"); fprintf(f, "# ninja log v7\n"); fprintf(f, "0\t1\t2\tout\t%" PRIx64 "\n", BuildLog::LogEntry::HashCommand("command abc")); @@ -112,7 +114,7 @@ TEST_F(BuildLogTest, DoubleEntry) { fclose(f); string err; - BuildLog log; + BuildLog log(disk_interface_); EXPECT_TRUE(log.Load(kTestFilename, &err)); ASSERT_EQ("", err); @@ -127,7 +129,7 @@ TEST_F(BuildLogTest, Truncate) { "build mid: cat in\n"); { - BuildLog log1; + BuildLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); @@ -147,7 +149,7 @@ TEST_F(BuildLogTest, Truncate) { // For all possible truncations of the input file, assert that we don't // crash when parsing. for (off_t size = statbuf.st_size; size > 0; --size) { - BuildLog log2; + BuildLog log2(disk_interface_); string err; EXPECT_TRUE(log2.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); @@ -157,33 +159,33 @@ TEST_F(BuildLogTest, Truncate) { ASSERT_TRUE(Truncate(kTestFilename, size, &err)); - BuildLog log3; + BuildLog log3(disk_interface_); err.clear(); ASSERT_TRUE(log3.Load(kTestFilename, &err) == LOAD_SUCCESS || !err.empty()); } } TEST_F(BuildLogTest, ObsoleteOldVersion) { - FILE* f = fopen(kTestFilename, "wb"); + FILE* f = disk_interface_.OpenFile(kTestFilename, "wb"); fprintf(f, "# ninja log v3\n"); fprintf(f, "123 456 0 out command\n"); fclose(f); string err; - BuildLog log; + BuildLog log(disk_interface_); EXPECT_TRUE(log.Load(kTestFilename, &err)); ASSERT_NE(err.find("version"), string::npos); } TEST_F(BuildLogTest, SpacesInOutput) { - FILE* f = fopen(kTestFilename, "wb"); + FILE* f = disk_interface_.OpenFile(kTestFilename, "wb"); fprintf(f, "# ninja log v7\n"); fprintf(f, "123\t456\t456\tout with space\t%" PRIx64 "\n", BuildLog::LogEntry::HashCommand("command")); fclose(f); string err; - BuildLog log; + BuildLog log(disk_interface_); EXPECT_TRUE(log.Load(kTestFilename, &err)); ASSERT_EQ("", err); @@ -199,7 +201,7 @@ TEST_F(BuildLogTest, DuplicateVersionHeader) { // Old versions of ninja accidentally wrote multiple version headers to the // build log on Windows. This shouldn't crash, and the second version header // should be ignored. - FILE* f = fopen(kTestFilename, "wb"); + FILE* f = disk_interface_.OpenFile(kTestFilename, "wb"); fprintf(f, "# ninja log v7\n"); fprintf(f, "123\t456\t456\tout\t%" PRIx64 "\n", BuildLog::LogEntry::HashCommand("command")); @@ -209,7 +211,7 @@ TEST_F(BuildLogTest, DuplicateVersionHeader) { fclose(f); string err; - BuildLog log; + BuildLog log(disk_interface_); EXPECT_TRUE(log.Load(kTestFilename, &err)); ASSERT_EQ("", err); @@ -228,49 +230,42 @@ TEST_F(BuildLogTest, DuplicateVersionHeader) { ASSERT_NO_FATAL_FAILURE(AssertHash("command2", e->command_hash)); } -struct TestDiskInterface : public DiskInterface { - virtual TimeStamp Stat(const string& path, string* err) const { - return 4; - } - virtual bool WriteFile(const string& path, const string& contents) { - assert(false); - return true; - } - virtual bool MakeDir(const string& path) { - assert(false); - return false; - } - virtual Status ReadFile(const string& path, string* contents, string* err) { - assert(false); - return NotFound; - } - virtual int RemoveFile(const string& path) { - assert(false); - return 0; +struct TestDiskInterface : public RealDiskInterface { + void EnableMockTimestamps() { enable_mock_ = true; } + + TimeStamp Stat(const string& path, string* err) const override { + if (enable_mock_) + return 4; + else + return this->RealDiskInterface::Stat(path, err); } + + private: + bool enable_mock_ = false; }; TEST_F(BuildLogTest, Restat) { - FILE* f = fopen(kTestFilename, "wb"); + FILE* f = disk_interface_.OpenFile(kTestFilename, "wb"); fprintf(f, "# ninja log v7\n" "1\t2\t3\tout\tcommand\n"); fclose(f); std::string err; - BuildLog log; + TestDiskInterface testDiskInterface; + BuildLog log(testDiskInterface); EXPECT_TRUE(log.Load(kTestFilename, &err)); ASSERT_EQ("", err); BuildLog::LogEntry* e = log.LookupByOutput("out"); ASSERT_EQ(3, e->mtime); - TestDiskInterface testDiskInterface; char out2[] = { 'o', 'u', 't', '2', 0 }; char* filter2[] = { out2 }; - EXPECT_TRUE(log.Restat(kTestFilename, testDiskInterface, 1, filter2, &err)); + testDiskInterface.EnableMockTimestamps(); + EXPECT_TRUE(log.Restat(kTestFilename, 1, filter2, &err)); ASSERT_EQ("", err); e = log.LookupByOutput("out"); ASSERT_EQ(3, e->mtime); // unchanged, since the filter doesn't match - EXPECT_TRUE(log.Restat(kTestFilename, testDiskInterface, 0, NULL, &err)); + EXPECT_TRUE(log.Restat(kTestFilename, 0, NULL, &err)); ASSERT_EQ("", err); e = log.LookupByOutput("out"); ASSERT_EQ(4, e->mtime); @@ -279,7 +274,7 @@ TEST_F(BuildLogTest, Restat) { TEST_F(BuildLogTest, VeryLongInputLine) { // Ninja's build log buffer is currently 256kB. Lines longer than that are // silently ignored, but don't affect parsing of other lines. - FILE* f = fopen(kTestFilename, "wb"); + FILE* f = disk_interface_.OpenFile(kTestFilename, "wb"); fprintf(f, "# ninja log v7\n"); fprintf(f, "123\t456\t456\tout\tcommand start"); for (size_t i = 0; i < (512 << 10) / strlen(" more_command"); ++i) @@ -290,7 +285,7 @@ TEST_F(BuildLogTest, VeryLongInputLine) { fclose(f); string err; - BuildLog log; + BuildLog log(disk_interface_); EXPECT_TRUE(log.Load(kTestFilename, &err)); ASSERT_EQ("", err); @@ -309,7 +304,7 @@ TEST_F(BuildLogTest, MultiTargetEdge) { AssertParse(&state_, "build out out.d: cat\n"); - BuildLog log; + BuildLog log(disk_interface_); log.RecordCommand(state_.edges_[0], 21, 22); ASSERT_EQ(2u, log.entries().size()); @@ -334,7 +329,7 @@ TEST_F(BuildLogRecompactTest, Recompact) { "build out: cat in\n" "build out2: cat in\n"); - BuildLog log1; + BuildLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); @@ -346,7 +341,7 @@ TEST_F(BuildLogRecompactTest, Recompact) { log1.Close(); // Load... - BuildLog log2; + BuildLog log2(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &err)); ASSERT_EQ("", err); ASSERT_EQ(2u, log2.entries().size()); @@ -357,7 +352,7 @@ TEST_F(BuildLogRecompactTest, Recompact) { log2.Close(); // "out2" is dead, it should've been removed. - BuildLog log3; + BuildLog log3(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &err)); ASSERT_EQ("", err); ASSERT_EQ(1u, log2.entries().size()); diff --git a/src/build_test.cc b/src/build_test.cc index 7675aceecf..91d55ba625 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -205,9 +205,9 @@ void PlanTest::TestPoolWithDepthOne(const char* test_case) { GetNode("out1")->MarkDirty(); GetNode("out2")->MarkDirty(); string err; - EXPECT_TRUE(plan_.AddTarget(GetNode("out1"), &err)); + EXPECT_TRUE(plan_.AddTarget(GetNode("out1"), &err)) << err; ASSERT_EQ("", err); - EXPECT_TRUE(plan_.AddTarget(GetNode("out2"), &err)); + EXPECT_TRUE(plan_.AddTarget(GetNode("out2"), &err)) << err; ASSERT_EQ("", err); plan_.PrepareQueue(); ASSERT_TRUE(plan_.more_to_do()); @@ -490,7 +490,9 @@ TEST_F(PlanTest, PriorityWithoutBuildLog) { GetNode("b0")->MarkDirty(); GetNode("c0")->MarkDirty(); GetNode("out")->MarkDirty(); - BuildLog log; + + SystemDiskInterface disk_interface; + BuildLog log(disk_interface); PrepareForTarget("out", &log); EXPECT_EQ(GetNode("out")->in_edge()->critical_path_weight(), 1); @@ -594,7 +596,8 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest, AssertParse(pstate, manifest); string err; - BuildLog build_log, *pbuild_log = NULL; + SystemDiskInterface disk_interface; + BuildLog build_log(disk_interface), *pbuild_log = NULL; if (log_path) { ASSERT_TRUE(build_log.Load(log_path, &err)); ASSERT_TRUE(build_log.OpenForWrite(log_path, *this, &err)); @@ -602,7 +605,7 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest, pbuild_log = &build_log; } - DepsLog deps_log, *pdeps_log = NULL; + DepsLog deps_log(disk_interface), *pdeps_log = NULL; if (deps_path) { ASSERT_TRUE(deps_log.Load(deps_path, pstate, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_path, &err)); @@ -1540,7 +1543,8 @@ struct BuildWithLogTest : public BuildTest { builder_.SetBuildLog(&build_log_); } - BuildLog build_log_; + SystemDiskInterface disk_interface_; + BuildLog build_log_{ disk_interface_ }; }; TEST_F(BuildWithLogTest, ImplicitGeneratedOutOfDate) { @@ -2326,7 +2330,8 @@ struct BuildWithQueryDepsLogTest : public BuildTest { ScopedTempDir temp_dir_; ScopedFilePath deps_log_file_; - DepsLog log_; + SystemDiskInterface disk_interface_; + DepsLog log_{ disk_interface_ }; }; /// Test a MSVC-style deps log with multiple outputs. @@ -2549,13 +2554,15 @@ TEST_F(BuildWithDepsLogTest, Straightforward) { " deps = gcc\n" " depfile = in1.d\n"; + SystemDiskInterface disk_interface; + { State state; ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); // Run the build once, everything should be ok. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -2585,7 +2592,7 @@ TEST_F(BuildWithDepsLogTest, Straightforward) { fs_.Create("in2", ""); // Run the build again. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); @@ -2616,6 +2623,9 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { "build out: cat in1\n" " deps = gcc\n" " depfile = in1.d\n"; + + SystemDiskInterface disk_interface; + { // Run an ordinary build that gathers dependencies. fs_.Create("in1", ""); @@ -2626,7 +2636,7 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); // Run the build once, everything should be ok. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -2655,7 +2665,7 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); @@ -2720,11 +2730,13 @@ TEST_F(BuildWithDepsLogTest, TestInputMtimeRaceCondition) { ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - BuildLog build_log; - ASSERT_TRUE(build_log.Load(build_log_file_.path(), &err)); - ASSERT_TRUE(build_log.OpenForWrite(build_log_file_.path(), *this, &err)); + SystemDiskInterface disk_interface; + BuildLog build_log(disk_interface); + ASSERT_TRUE(build_log.Load(build_log_file_.path(), &err)) << err; + ASSERT_TRUE(build_log.OpenForWrite(build_log_file_.path(), *this, &err)) + << err; - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); @@ -2802,11 +2814,12 @@ TEST_F(BuildWithDepsLogTest, TestInputMtimeRaceConditionWithDepFile) { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - BuildLog build_log; + SystemDiskInterface disk_interface; + BuildLog build_log(disk_interface); ASSERT_TRUE(build_log.Load(build_log_file_.path(), &err)); ASSERT_TRUE(build_log.OpenForWrite(build_log_file_.path(), *this, &err)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); @@ -2947,13 +2960,16 @@ TEST_F(BuildWithDepsLogTest, RestatDepfileDependencyDepsLog) { "build out: cat in1\n" " deps = gcc\n" " depfile = in1.d\n"; + + SystemDiskInterface disk_interface; + { State state; ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); // Run the build once, everything should be ok. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -2979,7 +2995,7 @@ TEST_F(BuildWithDepsLogTest, RestatDepfileDependencyDepsLog) { fs_.Create("header.in", ""); // Run the build again. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); @@ -3007,12 +3023,14 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { fs_.Create("foo.c", ""); + SystemDiskInterface disk_interface; + { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); // Run the build once, everything should be ok. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -3032,7 +3050,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -3077,13 +3095,14 @@ TEST_F(BuildWithDepsLogTest, DiscoveredDepDuringBuildChanged) { fs_.Create("in1", ""); fs_.Tick(); - BuildLog build_log; + SystemDiskInterface disk_interface; + BuildLog build_log(disk_interface); { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -3106,7 +3125,7 @@ TEST_F(BuildWithDepsLogTest, DiscoveredDepDuringBuildChanged) { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -3129,7 +3148,7 @@ TEST_F(BuildWithDepsLogTest, DiscoveredDepDuringBuildChanged) { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -3153,12 +3172,14 @@ TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { fs_.Create("x/y/z/foo.c", ""); + SystemDiskInterface disk_interface; + { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); // Run the build once, everything should be ok. - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -3180,7 +3201,7 @@ TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -4250,6 +4271,8 @@ TEST_F(BuildWithDepsLogTest, ValidationThroughDepfile) { string err; + SystemDiskInterface disk_interface; + { fs_.Create("in", ""); fs_.Create("in2", ""); @@ -4260,7 +4283,7 @@ TEST_F(BuildWithDepsLogTest, ValidationThroughDepfile) { ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); @@ -4295,7 +4318,7 @@ TEST_F(BuildWithDepsLogTest, ValidationThroughDepfile) { ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - DepsLog deps_log; + DepsLog deps_log(disk_interface); ASSERT_TRUE(deps_log.Load(deps_log_file_.path(), &state, &err)); ASSERT_TRUE(deps_log.OpenForWrite(deps_log_file_.path(), &err)); ASSERT_EQ("", err); diff --git a/src/clean_test.cc b/src/clean_test.cc index e99909c0d0..3a33cdcd64 100644 --- a/src/clean_test.cc +++ b/src/clean_test.cc @@ -13,10 +13,11 @@ // limitations under the License. #include "clean.h" -#include "build.h" -#include "util.h" +#include "build.h" +#include "disk_interface.h" #include "test.h" +#include "util.h" #ifndef _WIN32 #include @@ -469,13 +470,13 @@ TEST_F(CleanTest, CleanDepFileAndRspFileWithSpaces) { struct CleanDeadTest : public CleanTest, public BuildLogUser{ virtual void SetUp() { // In case a crashing test left a stale file behind. - unlink(kTestFilename); + disk_interface_.RemoveFile(kTestFilename); CleanTest::SetUp(); } - virtual void TearDown() { - unlink(kTestFilename); - } + virtual void TearDown() { disk_interface_.RemoveFile(kTestFilename); } virtual bool IsPathDead(StringPiece) const { return false; } + + SystemDiskInterface disk_interface_; }; TEST_F(CleanDeadTest, CleanDead) { @@ -493,7 +494,7 @@ TEST_F(CleanDeadTest, CleanDead) { fs_.Create("out1", ""); fs_.Create("out2", ""); - BuildLog log1; + BuildLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); @@ -501,7 +502,7 @@ TEST_F(CleanDeadTest, CleanDead) { log1.RecordCommand(state.edges_[1], 20, 25); log1.Close(); - BuildLog log2; + BuildLog log2(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &err)); ASSERT_EQ("", err); ASSERT_EQ(2u, log2.entries().size()); @@ -556,7 +557,7 @@ TEST_F(CleanDeadTest, CleanDeadPreservesInputs) { fs_.Create("out1", ""); fs_.Create("out2", ""); - BuildLog log1; + BuildLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); @@ -564,7 +565,7 @@ TEST_F(CleanDeadTest, CleanDeadPreservesInputs) { log1.RecordCommand(state.edges_[1], 20, 25); log1.Close(); - BuildLog log2; + BuildLog log2(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &err)); ASSERT_EQ("", err); ASSERT_EQ(2u, log2.entries().size()); diff --git a/src/deps_log.cc b/src/deps_log.cc index 364d54b896..5669272276 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -44,6 +44,9 @@ static const int32_t kCurrentVersion = 4; // internal buffers having to have this size. static constexpr size_t kMaxRecordSize = (1 << 19) - 1; +DepsLog::DepsLog(DiskInterface& disk_interface) + : disk_interface_(&disk_interface) {} + DepsLog::~DepsLog() { Close(); } @@ -154,7 +157,7 @@ void DepsLog::Close() { LoadStatus DepsLog::Load(const string& path, State* state, string* err) { METRIC_RECORD(".ninja_deps load"); char buf[kMaxRecordSize + 1]; - FILE* f = fopen(path.c_str(), "rb"); + FILE* f = disk_interface_->OpenFile(path, "rb"); if (!f) { if (errno == ENOENT) return LOAD_NOT_FOUND; @@ -179,7 +182,7 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) { else *err = "bad deps log signature or version; starting over"; fclose(f); - unlink(path.c_str()); + disk_interface_->RemoveFile(path); // Don't report this as a failure. An empty deps log will cause // us to rebuild the outputs anyway. return LOAD_SUCCESS; @@ -327,13 +330,13 @@ bool DepsLog::Recompact(const string& path, string* err) { METRIC_RECORD(".ninja_deps recompact"); Close(); - string temp_path = path + ".recompact"; + std::string temp_path = path + ".recompact"; // OpenForWrite() opens for append. Make sure it's not appending to a // left-over file from a previous recompaction attempt that crashed somehow. - unlink(temp_path.c_str()); + disk_interface_->RemoveFile(temp_path); - DepsLog new_log; + DepsLog new_log(*disk_interface_); if (!new_log.OpenForWrite(temp_path, err)) return false; @@ -363,12 +366,12 @@ bool DepsLog::Recompact(const string& path, string* err) { deps_.swap(new_log.deps_); nodes_.swap(new_log.nodes_); - 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; } @@ -435,7 +438,7 @@ bool DepsLog::OpenForWriteIfNeeded() { if (file_path_.empty()) { return true; } - file_ = fopen(file_path_.c_str(), "ab"); + file_ = disk_interface_->OpenFile(file_path_, "ab"); if (!file_) { return false; } diff --git a/src/deps_log.h b/src/deps_log.h index cb82c25908..c12d440b07 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -15,11 +15,13 @@ #ifndef NINJA_DEPS_LOG_H_ #define NINJA_DEPS_LOG_H_ +#include + +#include #include #include -#include - +#include "disk_interface.h" #include "load_status.h" #include "timestamp.h" @@ -66,7 +68,13 @@ struct State; /// wins, allowing updates to just be appended to the file. A separate /// repacking step can run occasionally to remove dead records. struct DepsLog { - DepsLog() : needs_recompaction_(false), file_(NULL) {} + /// No default constructor. + DepsLog() = delete; + + /// Constructor takes a DiskInterface reference. + DepsLog(DiskInterface& disk_interface); + + /// Destructor ~DepsLog(); // Writing (build-time) interface. @@ -115,8 +123,10 @@ struct DepsLog { /// be set. bool OpenForWriteIfNeeded(); - bool needs_recompaction_; - FILE* file_; + DiskInterface* disk_interface_ = nullptr; + + bool needs_recompaction_ = false; + FILE* file_ = nullptr; std::string file_path_; /// Maps id -> Node. diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 4819287363..de86100091 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -33,14 +33,16 @@ const char kTestFilename[] = "DepsLogTest-tempfile"; struct DepsLogTest : public testing::Test { virtual void SetUp() { // In case a crashing test left a stale file behind. - unlink(kTestFilename); + disk_interface_.RemoveFile(kTestFilename); } - virtual void TearDown() { unlink(kTestFilename); } + virtual void TearDown() { disk_interface_.RemoveFile(kTestFilename); } + + SystemDiskInterface disk_interface_; }; TEST_F(DepsLogTest, WriteRead) { State state1; - DepsLog log1; + DepsLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -67,7 +69,7 @@ TEST_F(DepsLogTest, WriteRead) { log1.Close(); State state2; - DepsLog log2; + DepsLog log2(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); ASSERT_EQ("", err); @@ -92,7 +94,7 @@ TEST_F(DepsLogTest, LotsOfDeps) { const int kNumDeps = 100000; // More than 64k. State state1; - DepsLog log1; + DepsLog log1(disk_interface_); string err; EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -113,7 +115,7 @@ TEST_F(DepsLogTest, LotsOfDeps) { log1.Close(); State state2; - DepsLog log2; + DepsLog log2(disk_interface_); EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); ASSERT_EQ("", err); @@ -127,7 +129,7 @@ TEST_F(DepsLogTest, DoubleEntry) { int file_size; { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -151,7 +153,7 @@ TEST_F(DepsLogTest, DoubleEntry) { // Now reload the file, and read the same deps. { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -189,7 +191,7 @@ TEST_F(DepsLogTest, Recompact) { { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); - DepsLog log; + DepsLog log(disk_interface_); string err; ASSERT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -221,7 +223,7 @@ TEST_F(DepsLogTest, Recompact) { { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); - DepsLog log; + DepsLog log(disk_interface_); string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -251,7 +253,7 @@ TEST_F(DepsLogTest, Recompact) { { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); - DepsLog log; + DepsLog log(disk_interface_); string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -305,7 +307,7 @@ TEST_F(DepsLogTest, Recompact) { { State state; // Intentionally not parsing kManifest here. - DepsLog log; + DepsLog log(disk_interface_); string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -361,7 +363,7 @@ TEST_F(DepsLogTest, InvalidHeader) { }; for (size_t i = 0; i < sizeof(kInvalidHeaders) / sizeof(kInvalidHeaders[0]); ++i) { - FILE* deps_log = fopen(kTestFilename, "wb"); + FILE* deps_log = disk_interface_.OpenFile(kTestFilename, "wb"); ASSERT_TRUE(deps_log != NULL); ASSERT_EQ( strlen(kInvalidHeaders[i]), @@ -369,7 +371,7 @@ TEST_F(DepsLogTest, InvalidHeader) { ASSERT_EQ(0 ,fclose(deps_log)); string err; - DepsLog log; + DepsLog log(disk_interface_); State state; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); EXPECT_EQ("bad deps log signature or version; starting over", err); @@ -381,7 +383,7 @@ TEST_F(DepsLogTest, Truncated) { // Create a file with some entries. { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -418,7 +420,7 @@ TEST_F(DepsLogTest, Truncated) { ASSERT_TRUE(Truncate(kTestFilename, size, &err)); State state; - DepsLog log; + DepsLog log(disk_interface_); EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); if (!err.empty()) { // At some point the log will be so short as to be unparsable. @@ -445,7 +447,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { // Create a file with some entries. { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -479,7 +481,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { // Load the file again, add an entry. { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); ASSERT_EQ("premature end of file; recovering", err); @@ -504,7 +506,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { // entry doesn't break things. { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -516,7 +518,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { TEST_F(DepsLogTest, ReverseDepsNodes) { State state; - DepsLog log; + DepsLog log(disk_interface_); string err; EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -545,7 +547,7 @@ TEST_F(DepsLogTest, MalformedDepsLog) { std::string err; { State state; - DepsLog log; + DepsLog log(disk_interface_); EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); @@ -620,7 +622,7 @@ TEST_F(DepsLogTest, MalformedDepsLog) { ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno); { State state; - DepsLog log; + DepsLog log(disk_interface_); err.clear(); ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err)); ASSERT_EQ("bad deps log signature or version; starting over", err); @@ -631,7 +633,7 @@ TEST_F(DepsLogTest, MalformedDepsLog) { ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno); { State state; - DepsLog log; + DepsLog log(disk_interface_); err.clear(); ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err)); ASSERT_EQ("bad deps log signature or version; starting over", err); @@ -642,7 +644,7 @@ TEST_F(DepsLogTest, MalformedDepsLog) { ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno); { State state; - DepsLog log; + DepsLog log(disk_interface_); err.clear(); ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err)); ASSERT_EQ("", err); @@ -657,7 +659,7 @@ TEST_F(DepsLogTest, MalformedDepsLog) { ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno); { State state; - DepsLog log; + DepsLog log(disk_interface_); err.clear(); ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err)); ASSERT_EQ("premature end of file; recovering", err); @@ -669,7 +671,7 @@ TEST_F(DepsLogTest, MalformedDepsLog) { ASSERT_TRUE(write_bad_log_file(bad_contents)) << strerror(errno); { State state; - DepsLog log; + DepsLog log(disk_interface_); err.clear(); ASSERT_EQ(LOAD_SUCCESS, log.Load(kBadLogFile, &state, &err)); ASSERT_EQ("premature end of file; recovering", err); diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 20894eed6e..86d4fb559a 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -14,18 +14,20 @@ #include "disk_interface.h" -#include - +#include #include #include #include #include #include +#include + #ifdef _WIN32 #include // _mkdir #include +#include #include #else #include @@ -38,32 +40,49 @@ using namespace std; namespace { -string DirName(const string& path) { +std::string DirName(const std::string& path) { #ifdef _WIN32 - static const char kPathSeparators[] = "\\/"; -#else - static const char kPathSeparators[] = "/"; -#endif - static const char* const kEnd = kPathSeparators + sizeof(kPathSeparators) - 1; - - string::size_type slash_pos = path.find_last_of(kPathSeparators); - if (slash_pos == string::npos) - return string(); // Nothing to do. - while (slash_pos > 0 && - std::find(kPathSeparators, kEnd, path[slash_pos - 1]) != kEnd) - --slash_pos; - return path.substr(0, slash_pos); + auto is_sep = [](char ch) -> bool { return ch == '/' || ch == '\\'; }; +#else // !_WIN32 + auto is_sep = [](char ch) -> bool { return ch == '/'; }; +#endif // !_WIN32 + size_t pos = path.size(); + while (pos > 0 && !is_sep(path[pos - 1])) // skip non-separators. + --pos; + while (pos > 0 && is_sep(path[pos - 1])) // skip separators. + --pos; + return path.substr(0, pos); } int MakeDir(const string& path) { #ifdef _WIN32 - return _mkdir(path.c_str()); + return _wmkdir(UTF8ToWin32Unicode(path).c_str()); #else return mkdir(path.c_str(), 0777); #endif } #ifdef _WIN32 +std::wstring Win32DirName(const std::wstring& path) { + auto is_sep = [](wchar_t ch) { return ch == L'/' || ch == L'\\'; }; + + size_t pos = path.size(); + while (pos > 0 && !is_sep(path[pos - 1])) + --pos; + + while (pos > 0 && is_sep(path[pos - 1])) + --pos; + + return path.substr(0, pos); +} + +std::wstring ToLowercase(const std::wstring& path) { + std::wstring result; + result.resize(path.size()); + std::transform(path.begin(), path.end(), result.begin(), std::towlower); + return result; +} + TimeStamp TimeStampFromFileTime(const FILETIME& filetime) { // FILETIME is in 100-nanosecond increments since the Windows epoch. // We don't much care about epoch correctness but we do want the @@ -74,13 +93,14 @@ TimeStamp TimeStampFromFileTime(const FILETIME& filetime) { return (TimeStamp)mtime - 12622770400LL * (1000000000LL / 100); } -TimeStamp StatSingleFile(const string& path, string* err) { +TimeStamp StatSingleFile(const std::wstring& path, std::string* err) { WIN32_FILE_ATTRIBUTE_DATA attrs; - if (!GetFileAttributesExA(path.c_str(), GetFileExInfoStandard, &attrs)) { + if (!GetFileAttributesExW(path.c_str(), GetFileExInfoStandard, &attrs)) { DWORD win_err = GetLastError(); if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND) return 0; - *err = "GetFileAttributesEx(" + path + "): " + GetLastErrorString(); + *err = "GetFileAttributesEx(" + Win32UnicodeToUTF8(path) + + "): " + GetLastErrorString(); return -1; } return TimeStampFromFileTime(attrs.ftLastWriteTime); @@ -96,8 +116,9 @@ bool IsWindows7OrLater() { &version_info, VER_MAJORVERSION | VER_MINORVERSION, comparison); } -bool StatAllFilesInDir(const string& dir, map* stamps, - string* err) { +bool StatAllFilesInDir(const std::wstring& dir, + std::map* stamps, + std::string* err) { // FindExInfoBasic is 30% faster than FindExInfoStandard. static bool can_use_basic_info = IsWindows7OrLater(); // This is not in earlier SDKs. @@ -105,8 +126,8 @@ bool StatAllFilesInDir(const string& dir, map* stamps, static_cast(1); FINDEX_INFO_LEVELS level = can_use_basic_info ? kFindExInfoBasic : FindExInfoStandard; - WIN32_FIND_DATAA ffd; - HANDLE find_handle = FindFirstFileExA((dir + "\\*").c_str(), level, &ffd, + WIN32_FIND_DATAW ffd; + HANDLE find_handle = FindFirstFileExW((dir + L"\\*").c_str(), level, &ffd, FindExSearchNameMatch, NULL, 0); if (find_handle == INVALID_HANDLE_VALUE) { @@ -114,20 +135,20 @@ bool StatAllFilesInDir(const string& dir, map* stamps, if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND || win_err == ERROR_DIRECTORY) return true; - *err = "FindFirstFileExA(" + dir + "): " + GetLastErrorString(); + *err = "FindFirstFileExW(" + Win32UnicodeToUTF8(dir) + + "): " + GetLastErrorString(); return false; } do { - string lowername = ffd.cFileName; - if (lowername == "..") { + std::wstring lowername = ToLowercase(std::wstring(ffd.cFileName)); + if (lowername == L"..") { // Seems to just copy the timestamp for ".." from ".", which is wrong. // This is the case at least on NTFS under Windows 7. continue; } - transform(lowername.begin(), lowername.end(), lowername.begin(), ::tolower); stamps->insert(make_pair(lowername, TimeStampFromFileTime(ffd.ftLastWriteTime))); - } while (FindNextFileA(find_handle, &ffd)); + } while (FindNextFileW(find_handle, &ffd)); FindClose(find_handle); return true; } @@ -157,10 +178,10 @@ bool DiskInterface::MakeDirs(const string& path) { return MakeDir(dir); } -// RealDiskInterface ----------------------------------------------------------- -RealDiskInterface::RealDiskInterface() +// SystemDiskInterface +// ----------------------------------------------------------- +SystemDiskInterface::SystemDiskInterface() { #ifdef _WIN32 -: use_cache_(false), long_paths_enabled_(false) { // Probe ntdll.dll for RtlAreLongPathsEnabled, and call it if it exists. HINSTANCE ntdll_lib = ::GetModuleHandleW(L"ntdll"); if (ntdll_lib) { @@ -171,49 +192,21 @@ RealDiskInterface::RealDiskInterface() long_paths_enabled_ = (*func_ptr)(); } } +#endif // _WIN32 } -#else -{} -#endif -TimeStamp RealDiskInterface::Stat(const string& path, string* err) const { +TimeStamp SystemDiskInterface::Stat(const string& path, string* err) const { METRIC_RECORD("node stat"); #ifdef _WIN32 // MSDN: "Naming Files, Paths, and Namespaces" // http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx - if (!path.empty() && !AreLongPathsEnabled() && path[0] != '\\' && + if (!path.empty() && !long_paths_enabled_ && path[0] != '\\' && path.size() > MAX_PATH) { - ostringstream err_stream; - err_stream << "Stat(" << path << "): Filename longer than " << MAX_PATH - << " characters"; - *err = err_stream.str(); + *err = + "Stat(" + path + ": Filename longer than " + std::to_string(MAX_PATH); return -1; } - if (!use_cache_) - return StatSingleFile(path, err); - - string dir = DirName(path); - string base(path.substr(dir.size() ? dir.size() + 1 : 0)); - if (base == "..") { - // StatAllFilesInDir does not report any information for base = "..". - base = "."; - dir = path; - } - - string dir_lowercase = dir; - transform(dir.begin(), dir.end(), dir_lowercase.begin(), ::tolower); - transform(base.begin(), base.end(), base.begin(), ::tolower); - - Cache::iterator ci = cache_.find(dir_lowercase); - if (ci == cache_.end()) { - ci = cache_.insert(make_pair(dir_lowercase, DirCache())).first; - if (!StatAllFilesInDir(dir.empty() ? "." : dir, &ci->second, err)) { - cache_.erase(ci); - return -1; - } - } - DirCache::iterator di = ci->second.find(base); - return di != ci->second.end() ? di->second : 0; + return StatSingleFile(UTF8ToWin32Unicode(path), err); #else #ifdef __USE_LARGEFILE64 struct stat64 st; @@ -245,8 +238,13 @@ TimeStamp RealDiskInterface::Stat(const string& path, string* err) const { #endif } -bool RealDiskInterface::WriteFile(const string& path, const string& contents) { +bool SystemDiskInterface::WriteFile(const string& path, + const string& contents) { +#ifdef _WIN32 + FILE* fp = _wfopen(UTF8ToWin32Unicode(path).c_str(), L"wb"); +#else // !_WIN32 FILE* fp = fopen(path.c_str(), "wb"); +#endif // !_WIN32 if (fp == NULL) { Error("WriteFile(%s): Unable to create file. %s", path.c_str(), strerror(errno)); @@ -269,7 +267,7 @@ bool RealDiskInterface::WriteFile(const string& path, const string& contents) { return true; } -bool RealDiskInterface::MakeDir(const string& path) { +bool SystemDiskInterface::MakeDir(const string& path) { if (::MakeDir(path) < 0) { if (errno == EEXIST) { return true; @@ -280,9 +278,9 @@ bool RealDiskInterface::MakeDir(const string& path) { return true; } -FileReader::Status RealDiskInterface::ReadFile(const string& path, - string* contents, - string* err) { +FileReader::Status SystemDiskInterface::ReadFile(const string& path, + string* contents, + string* err) { switch (::ReadFile(path, contents, err)) { case 0: return Okay; case -ENOENT: return NotFound; @@ -290,9 +288,10 @@ FileReader::Status RealDiskInterface::ReadFile(const string& path, } } -int RealDiskInterface::RemoveFile(const string& path) { +int SystemDiskInterface::RemoveFile(const string& path) { #ifdef _WIN32 - DWORD attributes = GetFileAttributesA(path.c_str()); + std::wstring native_path = UTF8ToWin32Unicode(path); + DWORD attributes = GetFileAttributesW(native_path.c_str()); if (attributes == INVALID_FILE_ATTRIBUTES) { DWORD win_err = GetLastError(); if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND) { @@ -303,7 +302,8 @@ int RealDiskInterface::RemoveFile(const string& path) { // On Windows Ninja should behave the same: // https://github.com/ninja-build/ninja/issues/1886 // Skip error checking. If this fails, accept whatever happens below. - SetFileAttributesA(path.c_str(), attributes & ~FILE_ATTRIBUTE_READONLY); + SetFileAttributesW(native_path.c_str(), + attributes & ~FILE_ATTRIBUTE_READONLY); } if (attributes & FILE_ATTRIBUTE_DIRECTORY) { // remove() deletes both files and directories. On Windows we have to @@ -311,7 +311,7 @@ int RealDiskInterface::RemoveFile(const string& path) { // used on a directory) // This fixes the behavior of ninja -t clean in some cases // https://github.com/ninja-build/ninja/issues/828 - if (!RemoveDirectoryA(path.c_str())) { + if (!RemoveDirectoryW(native_path.c_str())) { DWORD win_err = GetLastError(); if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND) { return 1; @@ -321,7 +321,7 @@ int RealDiskInterface::RemoveFile(const string& path) { return -1; } } else { - if (!DeleteFileA(path.c_str())) { + if (!DeleteFileW(native_path.c_str())) { DWORD win_err = GetLastError(); if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND) { return 1; @@ -345,16 +345,121 @@ int RealDiskInterface::RemoveFile(const string& path) { return 0; } +FILE* SystemDiskInterface::OpenFile(const std::string& path, const char* mode) { +#ifdef _WIN32 + std::wstring wide_path = UTF8ToWin32Unicode(path); + std::wstring wide_mode = UTF8ToWin32Unicode(mode); + return _wfopen(wide_path.c_str(), wide_mode.c_str()); +#else // !_WIN32 + return fopen(path.c_str(), mode); +#endif // !_WIN32 +} + +bool SystemDiskInterface::RenameFile(const std::string& from, + const std::string& to) { +#ifdef _WIN32 + std::wstring wide_from = UTF8ToWin32Unicode(from); + std::wstring wide_to = UTF8ToWin32Unicode(to); + return !_wrename(wide_from.c_str(), wide_to.c_str()); +#else // !_WIN32 + return !rename(from.c_str(), to.c_str()); +#endif // !_WIN32 +} + +#ifdef _WIN32 +bool SystemDiskInterface::AreLongPathsEnabled(void) const { + return long_paths_enabled_; +} +#endif + +TimeStamp NullDiskInterface::Stat(const std::string& path, + std::string* err) const { + assert(false); + return -1; +} + +bool NullDiskInterface::WriteFile(const std::string& path, + const std::string& contents) { + assert(false); + return true; +} + +bool NullDiskInterface::MakeDir(const std::string& path) { + assert(false); + errno = EINVAL; + return false; +} +FileReader::Status NullDiskInterface::ReadFile(const std::string& path, + std::string* contents, + std::string* err) { + assert(false); + return NotFound; +} +int NullDiskInterface::RemoveFile(const std::string& path) { + assert(false); + return 0; +} + +FILE* NullDiskInterface::OpenFile(const std::string& path, const char* mode) { + assert(false); + (void)path; + (void)mode; + return nullptr; +} + +bool NullDiskInterface::RenameFile(const std::string& from, + const std::string& to) { + assert(false); + (void)from; + (void)to; + return false; +} + void RealDiskInterface::AllowStatCache(bool allow) { #ifdef _WIN32 use_cache_ = allow; if (!use_cache_) cache_.clear(); +#else + (void)allow; #endif } #ifdef _WIN32 -bool RealDiskInterface::AreLongPathsEnabled(void) const { - return long_paths_enabled_; +TimeStamp RealDiskInterface::Stat(const string& path, string* err) const { + METRIC_RECORD("node stat"); + // MSDN: "Naming Files, Paths, and Namespaces" + // http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx + if (!path.empty() && !long_paths_enabled_ && path[0] != '\\' && + path.size() > MAX_PATH) { + *err = + "Stat(" + path + ": Filename longer than " + std::to_string(MAX_PATH); + return -1; + } + std::wstring native_path = UTF8ToWin32Unicode(path); + if (!use_cache_) + return StatSingleFile(native_path, err); + + std::wstring dir = Win32DirName(native_path); + std::wstring base(native_path.substr(dir.size() ? dir.size() + 1 : 0)); + if (base == L"..") { + // StatAllFilesInDir does not report any information for base = "..". + base = L"."; + dir = native_path; + } + + std::wstring dir_lowercase = ToLowercase(dir); + base = ToLowercase(base); + + Cache::iterator ci = cache_.find(dir_lowercase); + if (ci == cache_.end()) { + ci = cache_.insert(std::make_pair(dir_lowercase, DirCache())).first; + if (!StatAllFilesInDir(dir.empty() ? L"." : dir, &ci->second, err)) { + cache_.erase(ci); + return -1; + } + } + DirCache::iterator di = ci->second.find(base); + return di != ci->second.end() ? di->second : 0; } -#endif +#endif // _WIN32 diff --git a/src/disk_interface.h b/src/disk_interface.h index 74200b8f5b..2966586dab 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -41,7 +41,7 @@ struct FileReader { /// Interface for accessing the disk. /// /// Abstract so it can be mocked out for tests. The real implementation -/// is RealDiskInterface. +/// is SystemDiskInterface. struct DiskInterface: public FileReader { /// stat() a file, returning the mtime, or 0 if missing and -1 on /// other errors. @@ -62,44 +62,82 @@ struct DiskInterface: public FileReader { /// -1 if an error occurs. virtual int RemoveFile(const std::string& path) = 0; + /// Open new C standard i/o FILE instance for |path|. + virtual FILE* OpenFile(const std::string& path, const char* mode) = 0; + + /// Rename a file. Return true on success, or false/errno on failure. + /// NOTE: On Win32, this fails with EEXIST if the destination file already + /// exists (contrary to the Win32 documentation which states that the error + /// should be EACCES). On Posix, an existing file is always replaced + /// (unless trying to rename a file to an existing directory, or a + /// directory to a file or a non-empty one directory). + virtual bool RenameFile(const std::string& from, const std::string& to) = 0; + /// Create all the parent directories for path; like mkdir -p /// `basename path`. bool MakeDirs(const std::string& path); }; /// Implementation of DiskInterface that actually hits the disk. -struct RealDiskInterface : public DiskInterface { - RealDiskInterface(); - virtual ~RealDiskInterface() {} - virtual TimeStamp Stat(const std::string& path, std::string* err) const; - virtual bool MakeDir(const std::string& path); - virtual bool WriteFile(const std::string& path, const std::string& contents); - virtual Status ReadFile(const std::string& path, std::string* contents, - std::string* err); - virtual int RemoveFile(const std::string& path); - - /// Whether stat information can be cached. Only has an effect on Windows. - void AllowStatCache(bool allow); +struct SystemDiskInterface : public DiskInterface { + SystemDiskInterface(); + virtual ~SystemDiskInterface() {} + TimeStamp Stat(const std::string& path, std::string* err) const override; + bool MakeDir(const std::string& path) override; + bool WriteFile(const std::string& path, const std::string& contents) override; + Status ReadFile(const std::string& path, std::string* contents, + std::string* err) override; + int RemoveFile(const std::string& path) override; + FILE* OpenFile(const std::string& path, const char* mode) override; + bool RenameFile(const std::string& from, const std::string& to) override; #ifdef _WIN32 /// Whether long paths are enabled. Only has an effect on Windows. bool AreLongPathsEnabled() const; #endif + protected: +#ifdef _WIN32 + /// Whether long paths are enabled. Auto-detected by constructor. + bool long_paths_enabled_ = false; +#endif +}; + +/// A null DiskInterface implementation that is only useful for tests. +/// All functions do nothing except returning an error. Tests can sub-class +/// this one to add their own method overrides. +struct NullDiskInterface : public DiskInterface { + TimeStamp Stat(const std::string& path, std::string* err) const override; + bool WriteFile(const std::string& path, const std::string& contents) override; + bool MakeDir(const std::string& path) override; + Status ReadFile(const std::string& path, std::string* contents, + std::string* err) override; + int RemoveFile(const std::string& path) override; + FILE* OpenFile(const std::string& path, const char* mode) override; + bool RenameFile(const std::string& from, const std::string& to) override; +}; + +/// Implementation of SystemDiskInterface that speeds up Stat() calls by using +/// a small cache. Note: the cache is never flushed / updated!! +struct RealDiskInterface : public SystemDiskInterface { + /// Whether stat information can be cached. Only has an effect on Windows. + void AllowStatCache(bool allow); + +#ifdef _WIN32 + TimeStamp Stat(const std::string& path, std::string* err) const override; +#endif // _WIN32 + private: #ifdef _WIN32 /// Whether stat information can be cached. - bool use_cache_; + bool use_cache_ = false; - /// Whether long paths are enabled. - bool long_paths_enabled_; - - typedef std::map DirCache; + typedef std::map DirCache; // TODO: Neither a map nor a hashmap seems ideal here. If the statcache // works out, come up with a better data structure. - typedef std::map Cache; + typedef std::map Cache; mutable Cache cache_; -#endif +#endif // _WIN32 }; #endif // NINJA_DISK_INTERFACE_H_ diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index b09ed046e8..38c37318bb 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -171,7 +171,8 @@ TEST_F(DiskInterfaceTest, StatCache) { EXPECT_GT(disk_.Stat("subdir/subsubdir", &err), 1); EXPECT_EQ("", err); -#ifndef _MSC_VER // TODO: Investigate why. Also see https://github.com/ninja-build/ninja/pull/1423 +#ifndef _WIN32 // TODO: Investigate why. Also see + // https://github.com/ninja-build/ninja/pull/1423 EXPECT_EQ(disk_.Stat("subdir", &err), disk_.Stat("subdir/.", &err)); EXPECT_EQ("", err); @@ -257,28 +258,163 @@ TEST_F(DiskInterfaceTest, RemoveDirectory) { EXPECT_EQ(1, disk_.RemoveFile("does not exist")); } -struct StatTest : public StateTestWithBuiltinRules, - public DiskInterface { - StatTest() : scan_(&state_, NULL, NULL, this, NULL, NULL) {} +TEST_F(DiskInterfaceTest, OpenFile) { + // Scoped FILE pointer, ensure instance is closed + // when test exits, even in case of failure. + struct ScopedFILE { + ScopedFILE(FILE* f) : f_(f) {} + ~ScopedFILE() { Close(); } - // DiskInterface implementation. - virtual TimeStamp Stat(const string& path, string* err) const; - virtual bool WriteFile(const string& path, const string& contents) { - assert(false); - return true; + void Close() { + if (f_) { + fclose(f_); + f_ = nullptr; + } + } + + FILE* get() const { return f_; } + + explicit operator bool() const { return !!f_; } + + FILE* f_; + }; + + const char kFileName[] = "file-to-open"; + std::string kContent = "something to write to a file\n"; + + // disk_.WriteFile() opens the FILE instance in binary mode, which + // will not convert the final \n into \r\n on Windows. However, + // disk_.OpenFile() can write in text mode which will do the + // translation on this platform. + ASSERT_TRUE(disk_.WriteFile(kFileName, kContent)); + std::string kExpected = "something to write to a file\n"; +#ifdef _WIN32 + std::string kExpectedText = "something to write to a file\r\n"; +#else + std::string kExpectedText = "something to write to a file\n"; +#endif + + std::string contents; + std::string err; + ASSERT_EQ(FileReader::Okay, disk_.ReadFile(kFileName, &contents, &err)) + << err; + ASSERT_EQ(contents, kExpected); + + // Read a file. + { + ScopedFILE f(disk_.OpenFile(kFileName, "rb")); + ASSERT_TRUE(f); + ASSERT_EQ(fseek(f.get(), 0, SEEK_END), 0) << strerror(errno); + long file_size_long = ftell(f.get()); + ASSERT_GE(file_size_long, 0) << strerror(errno); + + size_t file_size = static_cast(file_size_long); + ASSERT_EQ(file_size, kExpected.size()); + ASSERT_EQ(fseek(f.get(), 0, SEEK_SET), 0) << strerror(errno); + + contents.clear(); + contents.resize(file_size); + ASSERT_EQ(fread(const_cast(contents.data()), file_size, 1, f.get()), + 1) + << strerror(errno); + ASSERT_EQ(contents, kExpected); } - virtual bool MakeDir(const string& path) { - assert(false); - return false; + + // Write a file opened file disk_.OpenFile() in binary mode, then verify its + // content. + const char kFileToWrite[] = "file-to-write"; + { + ScopedFILE f(disk_.OpenFile(kFileToWrite, "wb")); + ASSERT_TRUE(f) << strerror(errno); + ASSERT_EQ(fwrite(kContent.data(), kContent.size(), 1, f.get()), 1); } - virtual Status ReadFile(const string& path, string* contents, string* err) { - assert(false); - return NotFound; + + contents.clear(); + ASSERT_EQ(FileReader::Okay, disk_.ReadFile(kFileToWrite, &contents, &err)) + << err; + ASSERT_EQ(contents, kContent); + + // Write a file opened file disk_.OpenFile() in text mode, then verify its + // content. + { + ScopedFILE f(disk_.OpenFile(kFileToWrite, "wt")); + ASSERT_TRUE(f) << strerror(errno); + ASSERT_EQ(fwrite(kContent.data(), kContent.size(), 1, f.get()), 1); } - virtual int RemoveFile(const string& path) { - assert(false); - return 0; + + contents.clear(); + ASSERT_EQ(FileReader::Okay, disk_.ReadFile(kFileToWrite, &contents, &err)) + << err; + ASSERT_EQ(contents, kExpectedText); + + // Append to the same file, in text mode too. + { + ScopedFILE f(disk_.OpenFile(kFileToWrite, "at")); + ASSERT_TRUE(f) << strerror(errno); + ASSERT_EQ(fwrite(kContent.data(), kContent.size(), 1, f.get()), 1); } + std::string expected = kExpectedText + kExpectedText; + contents.clear(); + ASSERT_EQ(FileReader::Okay, disk_.ReadFile(kFileToWrite, &contents, &err)) + << err; + ASSERT_EQ(contents, expected); +} + +TEST_F(DiskInterfaceTest, RenameFile) { + // Rename a simple file. + std::string kFileA = "a-file"; + std::string kFileB = "b-file"; + + // NOTE: Do not put a newline in this string to avoid dealing + // with \r\n conversions on Win32. + std::string kContent = "something something"; + + ASSERT_TRUE(disk_.WriteFile(kFileA, kContent)); + std::string err; + TimeStamp stamp_a = disk_.Stat(kFileA, &err); + ASSERT_GT(stamp_a, 0); + + ASSERT_TRUE(disk_.RenameFile(kFileA, kFileB)); + EXPECT_EQ(0, disk_.Stat(kFileA, &err)); + EXPECT_EQ(err, ""); + + TimeStamp stamp_b = disk_.Stat(kFileB, &err); + ASSERT_GT(stamp_b, 0) << err; + + // Due to limited granularity on certain filesystems, use >= instead of > + // in the comparison below. + ASSERT_GE(stamp_b, stamp_a); + + std::string contents; + ASSERT_EQ(disk_.ReadFile(kFileB, &contents, &err), FileReader::Okay) << err; + ASSERT_EQ(contents, kContent); + + // Now write something else to the first file, and rename + // the second one to the first. This should work on Posix, and + // fail with EEXIST on Win32! + ASSERT_TRUE(disk_.WriteFile(kFileA, "something else entirely")); + stamp_a = disk_.Stat(kFileA, &err); + ASSERT_GT(stamp_a, 0) << err; + ASSERT_GE(stamp_a, stamp_b) << err; // see comment above. + +#ifdef _WIN32 + ASSERT_FALSE(disk_.RenameFile(kFileB, kFileA)); + EXPECT_EQ(errno, EEXIST); +#else // !_WIN32 + ASSERT_TRUE(disk_.RenameFile(kFileB, kFileA)) << strerror(errno); + EXPECT_EQ(0, disk_.Stat(kFileB, &err)) << err; + + contents.clear(); + ASSERT_EQ(disk_.ReadFile(kFileA, &contents, &err), FileReader::Okay) << err; + ASSERT_EQ(contents, kContent); +#endif // !_WIN32 +} + +struct StatTest : public StateTestWithBuiltinRules, public NullDiskInterface { + StatTest() : scan_(&state_, NULL, NULL, this, NULL, NULL) {} + + // DiskInterface implementation. + TimeStamp Stat(const string& path, string* err) const override; DependencyScan scan_; map mtimes_; diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 081e364ac3..3d41b254b5 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -28,19 +28,23 @@ using namespace std; namespace { -bool InternalGetFullPathName(const StringPiece& file_name, char* buffer, - size_t buffer_length, string *err) { - DWORD result_size = GetFullPathNameA(file_name.AsString().c_str(), - buffer_length, buffer, NULL); - if (result_size == 0) { - *err = "GetFullPathNameA(" + file_name.AsString() + "): " + - GetLastErrorString(); - return false; - } else if (result_size > buffer_length) { - *err = "path too long"; - return false; +std::wstring InternalGetFullPathName(StringPiece file_name, std::string* err) { + std::wstring result; + std::wstring wide_filename = UTF8ToWin32Unicode(file_name.AsString()); + DWORD len = GetFullPathNameW(wide_filename.c_str(), 0, NULL, NULL); + if (!len) { + *err = "GetFullPathNameW(" + file_name.AsString() + + "): " + GetLastErrorString(); + } else { + // The value of len is the buffer wide character size including the + // terminating L'\0'. However, the second call may return a final + // length that is smaller than |len - 1| for some reason. + result.resize(static_cast(len) - 1); + DWORD final_len = GetFullPathNameW( + wide_filename.c_str(), len, const_cast(result.data()), NULL); + result.resize(static_cast(final_len)); } - return true; + return result; } bool IsPathSeparator(char c) { @@ -76,19 +80,19 @@ bool SameDrive(StringPiece a, StringPiece b, string* err) { return true; } - char a_absolute[_MAX_PATH]; - char b_absolute[_MAX_PATH]; - if (!InternalGetFullPathName(a, a_absolute, sizeof(a_absolute), err)) { + std::wstring a_absolute = InternalGetFullPathName(a, err); + if (a_absolute.empty()) return false; - } - if (!InternalGetFullPathName(b, b_absolute, sizeof(b_absolute), err)) { + + std::wstring b_absolute = InternalGetFullPathName(b, err); + if (b_absolute.empty()) return false; - } - char a_drive[_MAX_DIR]; - char b_drive[_MAX_DIR]; - _splitpath(a_absolute, a_drive, NULL, NULL, NULL); - _splitpath(b_absolute, b_drive, NULL, NULL, NULL); - return _stricmp(a_drive, b_drive) == 0; + + wchar_t a_drive[_MAX_DIR]; + wchar_t b_drive[_MAX_DIR]; + _wsplitpath(a_absolute.data(), a_drive, NULL, NULL, NULL); + _wsplitpath(b_absolute.data(), b_drive, NULL, NULL, NULL); + return _wcsicmp(a_drive, b_drive) == 0; } // Check path |s| is FullPath style returned by GetFullPathName. @@ -137,23 +141,23 @@ IncludesNormalize::IncludesNormalize(const string& relative_to) { string IncludesNormalize::AbsPath(StringPiece s, string* err) { if (IsFullPathName(s)) { - string result = s.AsString(); - for (size_t i = 0; i < result.size(); ++i) { - if (result[i] == '\\') { - result[i] = '/'; - } + std::string result = s.AsString(); + for (char& c : result) { + if (c == '\\') + c = '/'; } return result; } - char result[_MAX_PATH]; - if (!InternalGetFullPathName(s, result, sizeof(result), err)) { + std::wstring result = InternalGetFullPathName(s, err); + if (result.empty()) return ""; + + for (wchar_t& c : result) { + if (c == L'\\') + c = L'/'; } - for (char* c = result; *c; ++c) - if (*c == '\\') - *c = '/'; - return result; + return Win32UnicodeToUTF8(result); } string IncludesNormalize::Relativize( @@ -183,17 +187,11 @@ string IncludesNormalize::Relativize( bool IncludesNormalize::Normalize(const string& input, string* result, string* err) const { - char copy[_MAX_PATH + 1]; - size_t len = input.size(); - if (len > _MAX_PATH) { - *err = "path too long"; - return false; - } - strncpy(copy, input.c_str(), input.size() + 1); + std::string copy = input; uint64_t slash_bits; - CanonicalizePath(copy, &len, &slash_bits); - StringPiece partially_fixed(copy, len); - string abs_input = AbsPath(partially_fixed, err); + CanonicalizePath(©, &slash_bits); + StringPiece partially_fixed(copy); + std::string abs_input = AbsPath(partially_fixed, err); if (!err->empty()) return false; diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index 12965f9fb1..4cd9791a21 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -109,10 +109,8 @@ TEST(IncludesNormalize, LongInvalidPath) { // Too long, won't be canonicalized. Ensure doesn't crash. string result, err; IncludesNormalize normalizer("."); - EXPECT_FALSE( - normalizer.Normalize(kLongInputString, &result, &err)); - EXPECT_EQ("path too long", err); - + EXPECT_TRUE(normalizer.Normalize(kLongInputString, &result, &err)); + EXPECT_EQ("", err); // Construct max size path having cwd prefix. // kExactlyMaxPath = "$cwd\\a\\aaaa...aaaa\0"; @@ -153,17 +151,18 @@ TEST(IncludesNormalize, ShortRelativeButTooLongAbsolutePath) { // Construct max size path having cwd prefix. // kExactlyMaxPath = "aaaa\\aaaa...aaaa\0"; - char kExactlyMaxPath[_MAX_PATH + 1]; - for (int i = 0; i < _MAX_PATH; ++i) { - if (i < _MAX_PATH - 1 && i % 10 == 4) + const size_t kMaxPathSize = _MAX_PATH * 2; + char kExactlyMaxPath[kMaxPathSize + 1]; + for (int i = 0; i < kMaxPathSize; ++i) { + if (i < kMaxPathSize - 1 && i % 10 == 4) kExactlyMaxPath[i] = '\\'; else kExactlyMaxPath[i] = 'a'; } - kExactlyMaxPath[_MAX_PATH] = '\0'; - EXPECT_EQ(strlen(kExactlyMaxPath), _MAX_PATH); + kExactlyMaxPath[kMaxPathSize] = '\0'; + EXPECT_EQ(strlen(kExactlyMaxPath), kMaxPathSize); - // Make sure a path that's exactly _MAX_PATH long fails with a proper error. - EXPECT_FALSE(normalizer.Normalize(kExactlyMaxPath, &result, &err)); - EXPECT_TRUE(err.find("GetFullPathName") != string::npos); + // Make sure a path that's larger than _MAX_PATH long does not fail. + EXPECT_TRUE(normalizer.Normalize(kExactlyMaxPath, &result, &err)); + EXPECT_EQ("", err); } diff --git a/src/minidump-win32.cc b/src/minidump-win32.cc index 9aea7678b9..c786cd3a92 100644 --- a/src/minidump-win32.cc +++ b/src/minidump-win32.cc @@ -12,10 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifdef _MSC_VER +#ifdef _WIN32 #include -#include + +// Must appear after +#include #include "util.h" @@ -33,18 +35,18 @@ typedef BOOL (WINAPI *MiniDumpWriteDumpFunc) ( /// Creates a windows minidump in temp folder. void CreateWin32MiniDump(_EXCEPTION_POINTERS* pep) { - char temp_path[MAX_PATH]; - GetTempPathA(sizeof(temp_path), temp_path); - char temp_file[MAX_PATH]; - sprintf(temp_file, "%s\\ninja_crash_dump_%lu.dmp", - temp_path, GetCurrentProcessId()); + wchar_t temp_path[MAX_PATH]; + GetTempPathW(sizeof(temp_path) / sizeof(temp_path[0]), temp_path); + wchar_t temp_file[MAX_PATH]; + _snwprintf(temp_file, MAX_PATH, L"%s\\ninja_crash_dump_%lu.dmp", temp_path, + GetCurrentProcessId()); // Delete any previous minidump of the same name. - DeleteFileA(temp_file); + DeleteFileW(temp_file); // Load DbgHelp.dll dynamically, as library is not present on all // Windows versions. - HMODULE dbghelp = LoadLibraryA("dbghelp.dll"); + HMODULE dbghelp = LoadLibraryW(L"dbghelp.dll"); if (dbghelp == NULL) { Error("failed to create minidump: LoadLibrary('dbghelp.dll'): %s", GetLastErrorString().c_str()); @@ -59,11 +61,11 @@ void CreateWin32MiniDump(_EXCEPTION_POINTERS* pep) { return; } - HANDLE hFile = CreateFileA(temp_file, GENERIC_READ | GENERIC_WRITE, 0, NULL, + HANDLE hFile = CreateFileW(temp_file, GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); if (hFile == NULL) { - Error("failed to create minidump: CreateFileA(%s): %s", - temp_file, GetLastErrorString().c_str()); + Error("failed to create minidump: CreateFileW(%s): %s", + Win32UnicodeToUTF8(temp_file).c_str(), GetLastErrorString().c_str()); return; } @@ -86,4 +88,4 @@ void CreateWin32MiniDump(_EXCEPTION_POINTERS* pep) { Warning("minidump created: %s", temp_file); } -#endif // _MSC_VER +#endif // _WIN32 diff --git a/src/missing_deps_test.cc b/src/missing_deps_test.cc index dae377b49d..bf487dc292 100644 --- a/src/missing_deps_test.cc +++ b/src/missing_deps_test.cc @@ -88,7 +88,8 @@ struct MissingDependencyScannerTest : public testing::Test { MissingDependencyTestDelegate delegate_; Rule generator_rule_; Rule compile_rule_; - DepsLog deps_log_; + SystemDiskInterface disk_interface_; + DepsLog deps_log_{ disk_interface_ }; State state_; VirtualFileSystem filesystem_; MissingDependencyScanner scanner_; diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index 1148ae52a5..a5b8eaeb48 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -46,7 +46,7 @@ int CLWrapper::Run(const string& command, string* output) { // Must be inheritable so subprocesses can dup to children. HANDLE nul = - CreateFileA("NUL", GENERIC_READ, + CreateFileW(L"NUL", GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &security_attributes, OPEN_EXISTING, 0, NULL); if (nul == INVALID_HANDLE_VALUE) @@ -60,16 +60,20 @@ int CLWrapper::Run(const string& command, string* output) { Win32Fatal("SetHandleInformation"); PROCESS_INFORMATION process_info = {}; - STARTUPINFOA startup_info = {}; - startup_info.cb = sizeof(STARTUPINFOA); + STARTUPINFOW startup_info = {}; + startup_info.cb = sizeof(STARTUPINFOW); startup_info.hStdInput = nul; startup_info.hStdError = ::GetStdHandle(STD_ERROR_HANDLE); startup_info.hStdOutput = stdout_write; startup_info.dwFlags |= STARTF_USESTDHANDLES; - if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL, - /* inherit handles */ TRUE, 0, - env_block_, NULL, + // NOTE: CreateProcessW() modifies the input command string. + // env_block_ contains an ANSI environment block, so + // CREATE_UNICODE_ENVIRONMENT is _not_ used here in dwCreationFlags. + std::wstring command_wide = UTF8ToWin32Unicode(command); + if (!CreateProcessW(NULL, const_cast(command_wide.data()), NULL, + NULL, + /* inherit handles */ TRUE, 0, env_block_, NULL, &startup_info, &process_info)) { Win32Fatal("CreateProcess"); } diff --git a/src/ninja.cc b/src/ninja.cc index bf8c3f60c0..ee867e79e3 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -87,9 +87,10 @@ struct Options { /// The Ninja main() loads up a series of data structures; various tools need /// to poke into these, so store them as fields on an object. struct NinjaMain : public BuildLogUser { - NinjaMain(const char* ninja_command, const BuildConfig& config) : - ninja_command_(ninja_command), config_(config), - start_time_millis_(GetTimeMillis()) {} + NinjaMain(const char* ninja_command, const BuildConfig& config) + : ninja_command_(ninja_command), config_(config), + build_log_(disk_interface_), deps_log_(disk_interface_), + start_time_millis_(GetTimeMillis()) {} /// Command line used to run Ninja. const char* ninja_command_; @@ -1133,7 +1134,7 @@ int NinjaMain::ToolRestat(const Options* options, int argc, char* argv[]) { err.clear(); } - bool success = build_log_.Restat(log_path, disk_interface_, argc, argv, &err); + bool success = build_log_.Restat(log_path, argc, argv, &err); if (!success) { Error("failed recompaction: %s", err.c_str()); return EXIT_FAILURE; diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index ff3baaca7f..6050adfffd 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -39,15 +39,13 @@ Subprocess::~Subprocess() { } HANDLE Subprocess::SetupPipe(HANDLE ioport) { - char pipe_name[100]; - snprintf(pipe_name, sizeof(pipe_name), - "\\\\.\\pipe\\ninja_pid%lu_sp%p", GetCurrentProcessId(), this); - - pipe_ = ::CreateNamedPipeA(pipe_name, - PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED, - PIPE_TYPE_BYTE, - PIPE_UNLIMITED_INSTANCES, - 0, 0, INFINITE, NULL); + wchar_t pipe_name[100]; + _snwprintf(pipe_name, sizeof(pipe_name) / sizeof(pipe_name[0]), + L"\\\\.\\pipe\\ninja_pid%lu_sp%p", GetCurrentProcessId(), this); + + pipe_ = ::CreateNamedPipeW( + pipe_name, PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED, PIPE_TYPE_BYTE, + PIPE_UNLIMITED_INSTANCES, 0, 0, INFINITE, NULL); if (pipe_ == INVALID_HANDLE_VALUE) Win32Fatal("CreateNamedPipe"); @@ -62,7 +60,7 @@ HANDLE Subprocess::SetupPipe(HANDLE ioport) { // Get the write end of the pipe as a handle inheritable across processes. HANDLE output_write_handle = - CreateFileA(pipe_name, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); + CreateFileW(pipe_name, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); HANDLE output_write_child; if (!DuplicateHandle(GetCurrentProcess(), output_write_handle, GetCurrentProcess(), &output_write_child, @@ -83,15 +81,14 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { security_attributes.bInheritHandle = TRUE; // Must be inheritable so subprocesses can dup to children. HANDLE nul = - CreateFileA("NUL", GENERIC_READ, + CreateFileW(L"NUL", GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &security_attributes, OPEN_EXISTING, 0, NULL); if (nul == INVALID_HANDLE_VALUE) Fatal("couldn't open nul"); - STARTUPINFOA startup_info; - memset(&startup_info, 0, sizeof(startup_info)); - startup_info.cb = sizeof(STARTUPINFO); + STARTUPINFOW startup_info = {}; + startup_info.cb = sizeof(STARTUPINFOW); if (!use_console_) { startup_info.dwFlags = STARTF_USESTDHANDLES; startup_info.hStdInput = nul; @@ -107,11 +104,13 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { // Ninja handles ctrl-c, except for subprocesses in console pools. DWORD process_flags = use_console_ ? 0 : CREATE_NEW_PROCESS_GROUP; + // NOTE: CreateProcessW() modifies the input string. // Do not prepend 'cmd /c' on Windows, this breaks command // lines greater than 8,191 chars. - if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL, - /* inherit handles */ TRUE, process_flags, - NULL, NULL, + std::wstring wide_command = UTF8ToWin32Unicode(command); + if (!CreateProcessW(NULL, const_cast(wide_command.data()), NULL, + NULL, + /* inherit handles */ TRUE, process_flags, NULL, NULL, &startup_info, &process_info)) { DWORD error = GetLastError(); if (error == ERROR_FILE_NOT_FOUND) { diff --git a/src/test.cc b/src/test.cc index 4d063da96e..8cb51bd510 100644 --- a/src/test.cc +++ b/src/test.cc @@ -26,6 +26,7 @@ #include #include #else +#include #include #endif @@ -80,6 +81,43 @@ string GetSystemTempDir() { #endif } +#ifdef _WIN32 + +/// An implementation of fmemopen() that writes the content of the buffer +/// to a temporary file then returns an open handle to it. The file itself +/// is deleted on fclose(). +FILE* fmemopen(void* buf, size_t size, const char* mode) { + std::string template_path = GetSystemTempDir() + "/VirtualFileSystem.XXXXXX"; + int err = _mktemp_s(const_cast(template_path.data()), + template_path.size() + 1); + if (err < 0) { + perror("_mktemp_s"); + return nullptr; + } + std::wstring wide_path = UTF8ToWin32Unicode(template_path); + HANDLE handle = + CreateFileW(wide_path.c_str(), DELETE | GENERIC_READ | GENERIC_WRITE, 0, + nullptr, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, nullptr); + if (handle == INVALID_HANDLE_VALUE) { + errno = EINVAL; + return nullptr; + } + FILE* fp = _fdopen(_open_osfhandle((intptr_t)handle, 0), "w+b"); + if (!fp) { + ::CloseHandle(handle); + return nullptr; + } + if (buf && size && fwrite(buf, size, 1, fp) != 1) { + fclose(fp); + return nullptr; + } + rewind(fp); + return fp; +} + +#endif // _WIN32 + } // anonymous namespace StateTestWithBuiltinRules::StateTestWithBuiltinRules() { @@ -195,6 +233,26 @@ int VirtualFileSystem::RemoveFile(const string& path) { } } +FILE* VirtualFileSystem::OpenFile(const std::string& path, const char* mode) { + auto it = files_.find(path); + if (it == files_.end()) { + errno = ENOENT; + return nullptr; + } + // This implementation only supports reading. + for (auto ch : StringPiece(mode)) { + if (ch == 'w' || ch == 'a') { + assert(false && + "VirtualFileSystem::OpenFile() does not support write or append " + "mode"); + errno = EINVAL; + return nullptr; + } + } + const std::string& data = it->second.contents; + return fmemopen(const_cast(data.data()), data.size(), mode); +} + void ScopedTempDir::CreateAndEnter(const string& name) { // First change into the system temp dir and save it for cleanup. start_dir_ = GetSystemTempDir(); diff --git a/src/test.h b/src/test.h index a4b9e19f75..f75f42b2c5 100644 --- a/src/test.h +++ b/src/test.h @@ -48,7 +48,7 @@ void VerifyGraph(const State& state); /// An implementation of DiskInterface that uses an in-memory representation /// of disk state. It also logs file accesses and directory creations /// so it can be used by tests to verify disk access patterns. -struct VirtualFileSystem : public DiskInterface { +struct VirtualFileSystem : public NullDiskInterface { VirtualFileSystem() : now_(1) {} /// "Create" a file with contents. @@ -61,12 +61,16 @@ struct VirtualFileSystem : public DiskInterface { } // DiskInterface - virtual TimeStamp Stat(const std::string& path, std::string* err) const; - virtual bool WriteFile(const std::string& path, const std::string& contents); - virtual bool MakeDir(const std::string& path); - virtual Status ReadFile(const std::string& path, std::string* contents, - std::string* err); - virtual int RemoveFile(const std::string& path); + TimeStamp Stat(const std::string& path, std::string* err) const override; + bool WriteFile(const std::string& path, const std::string& contents) override; + bool MakeDir(const std::string& path) override; + Status ReadFile(const std::string& path, std::string* contents, + std::string* err) override; + int RemoveFile(const std::string& path) override; + + // NOTE: This implementation does not support write or append mode! + // It will assert() in debug builds, and return NULL/EINVAL otherwise. + FILE* OpenFile(const std::string& path, const char* mode) override; /// An entry for a single in-memory file. struct Entry { diff --git a/src/util.cc b/src/util.cc index 70421bf119..ee3049b9f8 100644 --- a/src/util.cc +++ b/src/util.cc @@ -416,8 +416,10 @@ int ReadFile(const string& path, string* contents, string* err) { // This makes a ninja run on a set of 1500 manifest files about 4% faster // than using the generic fopen code below. err->clear(); - HANDLE f = ::CreateFileA(path.c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, - OPEN_EXISTING, FILE_FLAG_SEQUENTIAL_SCAN, NULL); + std::wstring native_path = UTF8ToWin32Unicode(path); + HANDLE f = + ::CreateFileW(native_path.c_str(), GENERIC_READ, FILE_SHARE_READ, NULL, + OPEN_EXISTING, FILE_FLAG_SEQUENTIAL_SCAN, NULL); if (f == INVALID_HANDLE_VALUE) { err->assign(GetLastErrorString()); return -ENOENT; @@ -950,3 +952,38 @@ bool Truncate(const string& path, size_t size, string* err) { } return true; } + +#ifdef _WIN32 +std::wstring UTF8ToWin32Unicode(const std::string& path) { + std::wstring result; + if (!path.empty()) { + int path_len_int = static_cast(path.size()); + int len = + MultiByteToWideChar(CP_UTF8, 0, path.c_str(), path_len_int, nullptr, 0); + if (len <= 0) + Win32Fatal("Invalid file path", path.c_str()); + + result.resize(static_cast(len)); + MultiByteToWideChar(CP_UTF8, 0, path.c_str(), path_len_int, + const_cast(result.data()), len); + } + return result; +} + +std::string Win32UnicodeToUTF8(const std::wstring& path) { + std::string result; + if (!path.empty()) { + int path_len_int = static_cast(path.size()); + int len = WideCharToMultiByte(CP_UTF8, 0, path.c_str(), path_len_int, + nullptr, 0, nullptr, nullptr); + if (len <= 0) + Win32Fatal("Invalid native file path"); + + result.resize(static_cast(len)); + (void)WideCharToMultiByte(CP_UTF8, 0, path.c_str(), path_len_int, + const_cast(result.data()), len, nullptr, + nullptr); + } + return result; +} +#endif // _WIN32 diff --git a/src/util.h b/src/util.h index b38578c326..cd89c87f16 100644 --- a/src/util.h +++ b/src/util.h @@ -122,6 +122,12 @@ bool Truncate(const std::string& path, size_t size, std::string* err); /// Convert the value returned by GetLastError() into a string. std::string GetLastErrorString(); +/// Convert UTF-8 path string to Win32 native path. +std::wstring UTF8ToWin32Unicode(const std::string& path); + +/// Convert Win32 native path to UTF-8 path string. +std::string Win32UnicodeToUTF8(const std::wstring& path); + /// Calls Fatal() with a function name and GetLastErrorString. NORETURN void Win32Fatal(const char* function, const char* hint = NULL); #endif diff --git a/src/util_test.cc b/src/util_test.cc index 9b620e895c..dc2678eb5a 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -502,3 +502,37 @@ TEST(StripAnsiEscapeCodes, StripColors) { EXPECT_EQ("affixmgr.cxx:286:15: warning: using the result... [-Wparentheses]", stripped); } + +#ifdef _WIN32 +TEST(UTF8ToWin32Unicode, UTF8ToWin32Unicode) { + static const struct { + const char* input; + const wchar_t* expected; + } kData[] = { + { "foo", L"foo" }, + { "foo/bar", L"foo/bar" }, + { "\x62\xc3\xa9\x62\xc3\xa9", L"\u0062\u00e9\u0062\u00e9" }, // bébé + }; + for (const auto& data : kData) { + std::string input = data.input; + std::wstring expected = data.expected; + EXPECT_EQ(expected, UTF8ToWin32Unicode(input)) << input; + } +} + +TEST(Win32UnicodeToUTF8, Win32UnicodeToUTF8) { + static const struct { + const wchar_t* input; + const char* expected; + } kData[] = { + { L"foo", "foo" }, + { L"foo/bar", "foo/bar" }, + { L"\u0062\u00e9\u0062\u00e9", "\x62\xc3\xa9\x62\xc3\xa9" }, // bébé + }; + for (const auto& data : kData) { + std::wstring input = data.input; + std::string expected = data.expected; + EXPECT_EQ(expected, Win32UnicodeToUTF8(input)) << input; + } +} +#endif // _WIN32