Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve incremental build: make ninja handle dynamic outputs #1953

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ add_library(libninja OBJECT
src/clparser.cc
src/dyndep.cc
src/dyndep_parser.cc
src/dynout_parser.cc
src/debug_flags.cc
src/deps_log.cc
src/disk_interface.cc
Expand Down
1 change: 1 addition & 0 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ def has_re2c():
'disk_interface',
'dyndep',
'dyndep_parser',
'dynout_parser',
'edit_distance',
'eval_env',
'graph',
Expand Down
6 changes: 6 additions & 0 deletions doc/manual.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,12 @@ keys.
stored as `.ninja_deps` in the `builddir`, see <<ref_toplevel,the
discussion of `builddir`>>.

`dynout`:: path to an optional _dynout file_ that contains the list
of outputs generated by the rule. The dynout file syntax except one
path per line. This is to make Ninja aware of dynamic outputs, so
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is expeccts one path per line? Are there any escaping mechanisms? I guess this means that paths with embedded newlines are not supported (not that I expect they are anywhere else either, but it'd be good to know before reviewing the parsing code)?

the command is re-run if the some dynamic outputs are missing, and
dynamic outputs are clean when using `-t clean` tool.

`msvc_deps_prefix`:: _(Available since Ninja 1.5.)_ defines the string
which should be stripped from msvc's /showIncludes output. Only
needed when `deps = msvc` and no English Visual Studio version is used.
Expand Down
21 changes: 19 additions & 2 deletions src/build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "depfile_parser.h"
#include "deps_log.h"
#include "disk_interface.h"
#include "dynout_parser.h"
#include "graph.h"
#include "metrics.h"
#include "state.h"
Expand Down Expand Up @@ -519,6 +520,7 @@ void Builder::Cleanup() {
for (vector<Edge*>::iterator e = active_edges.begin();
e != active_edges.end(); ++e) {
string depfile = (*e)->GetUnescapedDepfile();
string dynout = (*e)->GetUnescapedDynout();
for (vector<Node*>::iterator o = (*e)->outputs_.begin();
o != (*e)->outputs_.end(); ++o) {
// Only delete this output if it was actually modified. This is
Expand All @@ -537,6 +539,8 @@ void Builder::Cleanup() {
}
if (!depfile.empty())
disk_interface_->RemoveFile(depfile);
if (!dynout.empty())
disk_interface_->RemoveFile(dynout);
}
}
}
Expand Down Expand Up @@ -716,6 +720,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
// extraction itself can fail, which makes the command fail from a
// build perspective.
vector<Node*> deps_nodes;
int outputs_count = 0;
string deps_type = edge->GetBinding("deps");
const string deps_prefix = edge->GetBinding("msvc_deps_prefix");
if (!deps_type.empty()) {
Expand All @@ -730,6 +735,18 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
}
}

string dynout = edge->GetUnescapedDynout();
if (!dynout.empty()) {
string extract_err;
if (!DynoutParser::Parse(state_, disk_interface_, edge, dynout, &deps_nodes, &outputs_count, &extract_err)
&& result->success()) {
if (!result->output.empty())
result->output.append("\n");
result->output.append(extract_err);
result->status = ExitFailure;
}
}

int64_t start_time_millis, end_time_millis;
RunningEdgeMap::iterator it = running_edges_.find(edge);
start_time_millis = it->second;
Expand Down Expand Up @@ -813,14 +830,14 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
}
}

if (!deps_type.empty() && !config_.dry_run) {
if ((!deps_type.empty() || !dynout.empty()) && !config_.dry_run) {
assert(!edge->outputs_.empty() && "should have been rejected by parser");
for (std::vector<Node*>::const_iterator o = edge->outputs_.begin();
o != edge->outputs_.end(); ++o) {
TimeStamp deps_mtime = disk_interface_->Stat((*o)->path(), err);
if (deps_mtime == -1)
return false;
if (!scan_.deps_log()->RecordDeps(*o, deps_mtime, deps_nodes)) {
if (!scan_.deps_log()->RecordDeps(*o, deps_mtime, deps_nodes, outputs_count)) {
*err = std::string("Error writing to deps log: ") + strerror(errno);
return false;
}
Expand Down
182 changes: 182 additions & 0 deletions src/build_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,18 @@ bool FakeCommandRunner::StartCommand(Edge* edge) {
if (fs_->ReadFile(edge->inputs_[0]->path(), &content, &err) ==
DiskInterface::Okay)
fs_->WriteFile(edge->outputs_[0]->path(), content);
} else if (edge->rule().name() == "cp-plus-bis") {
assert(!edge->inputs_.empty());
assert(edge->outputs_.size() >= 1);
string content;
string err;
if (fs_->ReadFile(edge->inputs_[0]->path(), &content, &err) ==
DiskInterface::Okay) {
fs_->Tick();
fs_->WriteFile(edge->outputs_[0]->path() + ".bis", content);
fs_->Tick();
fs_->WriteFile(edge->outputs_[0]->path(), content);
}
} else if (edge->rule().name() == "touch-implicit-dep-out") {
string dep = edge->GetBinding("test_dependency");
fs_->Create(dep, "");
Expand Down Expand Up @@ -3526,3 +3538,173 @@ TEST_F(BuildTest, DyndepTwoLevelDiscoveredDirty) {
EXPECT_EQ("touch tmp", command_runner_.commands_ran_[3]);
EXPECT_EQ("touch out", command_runner_.commands_ran_[4]);
}

TEST_F(BuildTest, RebuildMissingDynamicOutputs) {
string err;
BuildLog build_log;

{
FakeCommandRunner command_runner(&fs_);
State state;
DepsLog deps_log;
ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err));
ASSERT_EQ("", err);
Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0);
builder.command_runner_.reset(&command_runner);

ASSERT_NO_FATAL_FAILURE(
AssertParse(&state,
"rule cp-plus-bis\n"
" command = cp $in $out && cp $in $out.bis\n"
" dynout = $out.dynout\n"
"build out: cp-plus-bis in\n"));

fs_.Tick();
fs_.Create("in", "");
fs_.Create("out.dynout", "out.bis\n");

EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_TRUE(builder.Build(&err));
ASSERT_EQ("", err);
ASSERT_EQ(1u, command_runner.commands_ran_.size());
ASSERT_GT(fs_.Stat("out", &err), 0);
ASSERT_GT(fs_.Stat("out.bis", &err), 0);
// Make sure the dynout file has been removed after its
// information has been extracted in the deps log.
ASSERT_EQ(fs_.Stat("out.dynout", &err), 0);

// all clean, no rebuild.
command_runner.commands_ran_.clear();
state.Reset();
EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_EQ("", err);
EXPECT_TRUE(builder.AlreadyUpToDate());

// Test dynamic output are rebuilt if they are deleted.
fs_.RemoveFile("out.bis");
command_runner.commands_ran_.clear();
state.Reset();

fs_.Create("out.dynout", "out.bis\n");
EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_TRUE(builder.Build(&err));
ASSERT_EQ("", err);
ASSERT_EQ(1u, command_runner.commands_ran_.size());

builder.command_runner_.release();
deps_log.Close();
}

// Create a new state to make sure outputs are reset
{
FakeCommandRunner command_runner(&fs_);
State state;
DepsLog deps_log;
ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err));
ASSERT_EQ("", err);
deps_log.Load("ninja_deps", &state, &err);
ASSERT_EQ("", err);
Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0);
builder.command_runner_.reset(&command_runner);

ASSERT_NO_FATAL_FAILURE(
AssertParse(&state,
"rule cp-plus-bis\n"
" command = cp $in $out && cp $in $out.bis\n"
" dynout = $out.dynout\n"
"build out: cp-plus-bis in\n"));

// all clean, no rebuild.
command_runner.commands_ran_.clear();
state.Reset();
EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_EQ("", err);
EXPECT_TRUE(builder.AlreadyUpToDate());

// Test dynamic output are rebuilt if they are deleted
// after having been rebuilt in order to make sure
// when dynamic output information was already exist they
// keep being valid for the next build.
fs_.RemoveFile("out.bis");
command_runner.commands_ran_.clear();
state.Reset();

// Recreate the dynout file because it is not created by the edge
fs_.Create("out.dynout", "out.bis\n");
EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_TRUE(builder.Build(&err));
ASSERT_EQ("", err);
ASSERT_EQ(1u, command_runner.commands_ran_.size());

builder.command_runner_.release();
deps_log.Close();
}

RealDiskInterface disk_interface;
disk_interface.RemoveFile("ninja_deps");
}

TEST_F(BuildTest, RebuildMissingDynamicOutputsWithRestat) {
string err;

FakeCommandRunner command_runner(&fs_);
BuildLog build_log;
State state;
DepsLog deps_log;
ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err));
ASSERT_EQ("", err);
Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0);
builder.command_runner_.reset(&command_runner);

ASSERT_NO_FATAL_FAILURE(
AssertParse(&state,
"rule cp-plus-bis\n"
" command = cp $in $out && cp $in $out.bis\n"
" dynout = $out.dynout\n"
" restat = 1\n"
"build out: cp-plus-bis in\n"));

fs_.Tick();
fs_.Create("in", "");
fs_.Tick();
fs_.Create("out.dynout", "out.bis\n");
fs_.Tick();

EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_TRUE(builder.Build(&err));
ASSERT_EQ("", err);
ASSERT_EQ(1u, command_runner.commands_ran_.size());
ASSERT_GT(fs_.Stat("out", &err), 0);
ASSERT_GT(fs_.Stat("out.bis", &err), 0);
// Make sure the dynout file has been removed after its
// information has been extracted in the deps log.
ASSERT_EQ(fs_.Stat("out.dynout", &err), 0);

// all clean, no rebuild.
command_runner.commands_ran_.clear();
state.Reset();
EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_EQ("", err);
EXPECT_TRUE(builder.AlreadyUpToDate());

fs_.RemoveFile("out.bis");
command_runner.commands_ran_.clear();
state.Reset();

// Recreate the dynout file because it is not created by the edge
fs_.Create("out.dynout", "out.bis\n");
EXPECT_TRUE(builder.AddTarget("out", &err));
EXPECT_TRUE(builder.Build(&err));
ASSERT_EQ("", err);
ASSERT_EQ(1u, command_runner.commands_ran_.size());

// Make sure the dynout file has been removed after its
// information has been extracted in the deps log.
ASSERT_EQ(fs_.Stat("out.dynout", &err), 0);

builder.command_runner_.release();

deps_log.Close();
RealDiskInterface disk_interface;
disk_interface.RemoveFile("ninja_deps");
}
29 changes: 28 additions & 1 deletion src/clean.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ using namespace std;

Cleaner::Cleaner(State* state,
const BuildConfig& config,
DiskInterface* disk_interface)
DiskInterface* disk_interface,
DepsLog* deps_log)
: state_(state),
config_(config),
dyndep_loader_(state, disk_interface),
cleaned_files_count_(0),
disk_interface_(disk_interface),
deps_log_(deps_log),
status_(0) {
}

Expand Down Expand Up @@ -79,6 +81,10 @@ void Cleaner::RemoveEdgeFiles(Edge* edge) {
if (!depfile.empty())
Remove(depfile);

string dynout = edge->GetUnescapedDynout();
if (!dynout.empty())
Remove(dynout);

string rspfile = edge->GetUnescapedRspfile();
if (!rspfile.empty())
Remove(rspfile);
Expand All @@ -105,6 +111,7 @@ int Cleaner::CleanAll(bool generator) {
Reset();
PrintHeader();
LoadDyndeps();
LoadDynamicOutputs();
for (vector<Edge*>::iterator e = state_->edges_.begin();
e != state_->edges_.end(); ++e) {
// Do not try to remove phony targets
Expand Down Expand Up @@ -164,6 +171,7 @@ int Cleaner::CleanTarget(Node* target) {
Reset();
PrintHeader();
LoadDyndeps();
LoadDynamicOutputs();
DoCleanTarget(target);
PrintFooter();
return status_;
Expand All @@ -187,6 +195,7 @@ int Cleaner::CleanTargets(int target_count, char* targets[]) {
Reset();
PrintHeader();
LoadDyndeps();
LoadDynamicOutputs();
for (int i = 0; i < target_count; ++i) {
string target_name = targets[i];
uint64_t slash_bits;
Expand Down Expand Up @@ -231,6 +240,7 @@ int Cleaner::CleanRule(const Rule* rule) {
Reset();
PrintHeader();
LoadDyndeps();
LoadDynamicOutputs();
DoCleanRule(rule);
PrintFooter();
return status_;
Expand All @@ -256,6 +266,7 @@ int Cleaner::CleanRules(int rule_count, char* rules[]) {
Reset();
PrintHeader();
LoadDyndeps();
LoadDynamicOutputs();
for (int i = 0; i < rule_count; ++i) {
const char* rule_name = rules[i];
const Rule* rule = state_->bindings_.LookupRule(rule_name);
Expand Down Expand Up @@ -291,3 +302,19 @@ void Cleaner::LoadDyndeps() {
}
}
}

void Cleaner::LoadDynamicOutputs() {
std::string err;
// Load dynamic outputs which may exist in the deps log
DepfileParserOptions depfileOptions;
ImplicitDepLoader implicit_dep_loader(state_, deps_log_, disk_interface_,
&depfileOptions);
for (vector<Edge*>::iterator e = state_->edges_.begin();
e != state_->edges_.end(); ++e) {
string dynout = (*e)->GetUnescapedDynout();

if (!dynout.empty()) {
implicit_dep_loader.LoadImplicitOutputs(*e, &err);
}
}
}
Loading