Skip to content

Commit

Permalink
DepsLog: Use DiskInterface instance.
Browse files Browse the repository at this point in the history
  • Loading branch information
@digit-google
digit-google committed Oct 6, 2024
1 parent a3f5c70 commit 947ed1e
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 62 deletions.
53 changes: 34 additions & 19 deletions src/build_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,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));
Expand Down Expand Up @@ -2330,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.
Expand Down Expand Up @@ -2553,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);

Expand Down Expand Up @@ -2589,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));

Expand Down Expand Up @@ -2620,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", "");
Expand All @@ -2630,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);

Expand Down Expand Up @@ -2659,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));

Expand Down Expand Up @@ -2730,7 +2736,7 @@ TEST_F(BuildWithDepsLogTest, TestInputMtimeRaceCondition) {
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));

Expand Down Expand Up @@ -2813,7 +2819,7 @@ TEST_F(BuildWithDepsLogTest, TestInputMtimeRaceConditionWithDepFile) {
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));

Expand Down Expand Up @@ -2954,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);

Expand All @@ -2986,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));

Expand Down Expand Up @@ -3014,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);

Expand All @@ -3039,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);
Expand Down Expand Up @@ -3091,7 +3102,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.OpenForWrite(deps_log_file_.path(), &err));
ASSERT_EQ("", err);

Expand All @@ -3114,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);
Expand All @@ -3137,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);
Expand All @@ -3161,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);

Expand All @@ -3188,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);
Expand Down Expand Up @@ -4258,6 +4271,8 @@ TEST_F(BuildWithDepsLogTest, ValidationThroughDepfile) {

string err;

SystemDiskInterface disk_interface;

{
fs_.Create("in", "");
fs_.Create("in2", "");
Expand All @@ -4268,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);

Expand Down Expand Up @@ -4303,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);
Expand Down
19 changes: 11 additions & 8 deletions src/deps_log.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -155,7 +158,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;
Expand All @@ -180,7 +183,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;
Expand Down Expand Up @@ -329,13 +332,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;

Expand Down Expand Up @@ -365,12 +368,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;
}
Expand Down Expand Up @@ -437,7 +440,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;
}
Expand Down
20 changes: 15 additions & 5 deletions src/deps_log.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
#ifndef NINJA_DEPS_LOG_H_
#define NINJA_DEPS_LOG_H_

#include <stdio.h>

#include <memory>
#include <string>
#include <vector>

#include <stdio.h>

#include "disk_interface.h"
#include "load_status.h"
#include "timestamp.h"

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -114,8 +122,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.
Expand Down
Loading

0 comments on commit 947ed1e

Please sign in to comment.