From b6a9a1c8adbb444c2489d884f06e5bd39627c3e9 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 17 Dec 2012 09:08:15 -0800 Subject: add DepsLog, a new data structure for dependency information DepsLog is a compact serialization of dependency information. It can be used to replace depfiles for faster loading. --- configure.py | 2 + src/deps_log.cc | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/deps_log.h | 91 +++++++++++++++++++++++++++++++ src/deps_log_test.cc | 63 ++++++++++++++++++++++ src/graph.h | 9 +++- src/state.cc | 5 +- 6 files changed, 316 insertions(+), 3 deletions(-) create mode 100644 src/deps_log.cc create mode 100644 src/deps_log.h create mode 100644 src/deps_log_test.cc diff --git a/configure.py b/configure.py index 10c6994..8f5a497 100755 --- a/configure.py +++ b/configure.py @@ -269,6 +269,7 @@ for name in ['build', 'build_log', 'clean', 'depfile_parser', + 'deps_log', 'disk_interface', 'edit_distance', 'eval_env', @@ -348,6 +349,7 @@ for name in ['build_log_test', 'build_test', 'clean_test', 'depfile_parser_test', + 'deps_log_test', 'disk_interface_test', 'edit_distance_test', 'graph_test', diff --git a/src/deps_log.cc b/src/deps_log.cc new file mode 100644 index 0000000..ca7fd4b --- /dev/null +++ b/src/deps_log.cc @@ -0,0 +1,149 @@ +// Copyright 2012 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "deps_log.h" + +#include +#include +#include +#include + +#include "graph.h" +#include "state.h" +#include "util.h" + +bool DepsLog::OpenForWrite(const string& path, string* err) { + file_ = fopen(path.c_str(), "ab"); + if (!file_) { + *err = strerror(errno); + return false; + } + SetCloseOnExec(fileno(file_)); + + // Opening a file in append mode doesn't set the file pointer to the file's + // end on Windows. Do that explicitly. + fseek(file_, 0, SEEK_END); + + /* XXX + if (ftell(log_file_) == 0) { + if (fprintf(log_file_, kFileSignature, kCurrentVersion) < 0) { + *err = strerror(errno); + return false; + } + } + */ + + return true; +} + +bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, + const vector& nodes) { + // Assign ids to all nodes that are missing one. + if (node->id() < 0) + RecordId(node); + for (vector::const_iterator i = nodes.begin(); + i != nodes.end(); ++i) { + if ((*i)->id() < 0) + RecordId(*i); + } + + uint16_t size = 4 * (1 + 1 + nodes.size()); + size |= 0x8000; // Deps record: set high bit. + fwrite(&size, 2, 1, file_); + int id = node->id(); + fwrite(&id, 4, 1, file_); + int timestamp = node->mtime(); + fwrite(×tamp, 4, 1, file_); + for (vector::const_iterator i = nodes.begin(); + i != nodes.end(); ++i) { + id = node->id(); + fwrite(&id, 4, 1, file_); + } + + return true; +} + +void DepsLog::Close() { + fclose(file_); + file_ = NULL; +} + +bool DepsLog::Load(const string& path, State* state, string* err) { + char buf[32 << 10]; + FILE* f = fopen(path.c_str(), "rb"); + if (!f) { + *err = strerror(errno); + return false; + } + + int id = 0; + for (;;) { + uint16_t size; + if (fread(&size, 2, 1, f) < 1) + break; + bool is_deps = (size >> 15) != 0; + size = size & 0x7FFF; + + if (fread(buf, size, 1, f) < 1) + break; + + if (is_deps) { + assert(size % 4 == 0); + int* deps_data = reinterpret_cast(buf); + int out_id = deps_data[0]; + int mtime = deps_data[1]; + deps_data += 2; + int deps_count = (size / 4) - 2; + + Deps* deps = new Deps; + deps->mtime = mtime; + deps->node_count = deps_count; + deps->nodes = new Node*[deps_count]; + for (int i = 0; i < deps_count; ++i) { + assert(deps_data[i] < (int)nodes_.size()); + assert(nodes_[deps_data[i]]); + deps->nodes[i] = nodes_[deps_data[i]]; + } + + if (out_id >= (int)deps_.size()) + deps_.resize(out_id + 1); + if (deps_[out_id]) + delete deps_[out_id]; + deps_[out_id] = deps; + } else { + StringPiece path(buf, size); + Node* node = state->GetNode(path); + assert(node->id() < 0); + node->set_id(id); + ++id; + } + } + if (ferror(f)) { + *err = strerror(ferror(f)); + return false; + } + fclose(f); + return true; +} + +bool DepsLog::RecordId(Node* node) { + uint16_t size = node->path().size(); + fwrite(&size, 2, 1, file_); + fwrite(node->path().data(), node->path().size(), 1, file_); + + node->set_id(nodes_.size()); + nodes_.push_back(node); + + return true; +} diff --git a/src/deps_log.h b/src/deps_log.h new file mode 100644 index 0000000..45d2cea --- /dev/null +++ b/src/deps_log.h @@ -0,0 +1,91 @@ +// Copyright 2012 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef NINJA_DEPS_LOG_H_ +#define NINJA_DEPS_LOG_H_ + +#include +#include +using namespace std; + +#include + +#include "timestamp.h" + +struct Node; +struct State; + +/// As build commands run they can output extra dependency information +/// (e.g. header dependencies for C source) via a pipe. DepsLog collects +/// that information at build time and reloads it at startup. +/// +/// The on-disk format is based on two primary constraints: +/// - it must be written to as a stream (during the build, which may be +/// interrupted); +/// - it can be read all at once on startup. (Alternative designs, where +/// it contains indexing information, were considered and discarded as +/// too complicated to implement; if the file is small than reading it +/// fully on startup is acceptable.) +/// Here are some stats from the Windows Chrome dependency files, to +/// help guide the design space. The total text in the files sums to +/// 90mb so some compression is warranted to keep load-time fast. +/// There's about 10k files worth of dependencies that reference about +/// 40k total paths totalling 2mb of unique strings. +/// +/// Based on these above, the file is structured as a sequence of records. +/// Each record is either a path string or a dependency list. +/// Numbering the path strings in file order gives them dense integer ids. +/// A dependency list maps an output id to a list of input ids. +/// +/// Concretely, a record is: +/// two bytes record length, high bit indicates record type +/// (implies max record length 32k) +/// path records contain just the string name of the path +/// dependency records are an array of 4-byte integers +/// [output path id, output path mtime, input path id, input path id...] +/// (The mtime is compared against the on-disk output path mtime +/// to verify the stored data is up-to-date.) +/// If two records reference the same output the latter one in the file +/// wins, allowing updates to just be appended to the file. A separate +/// repacking step can run occasionally to remove dead records. +struct DepsLog { + + // Writing (build-time) interface. + bool OpenForWrite(const string& path, string* err); + bool RecordDeps(Node* node, TimeStamp mtime, const vector& nodes); + void Close(); + + // Reading (startup-time) interface. + bool Load(const string& path, State* state, string* err); + + private: + // Write a node name record, assigning it an id. + bool RecordId(Node* node); + + struct Deps { + Deps() : mtime(-1), node_count(0), nodes(NULL) {} + ~Deps() { delete [] nodes; } + int mtime; + int node_count; + Node** nodes; + }; + + FILE* file_; + vector nodes_; + vector deps_; + + friend struct DepsLogTest; +}; + +#endif // NINJA_DEPS_LOG_H_ diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc new file mode 100644 index 0000000..540865b --- /dev/null +++ b/src/deps_log_test.cc @@ -0,0 +1,63 @@ +// Copyright 2012 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "deps_log.h" + +#include "graph.h" +#include "util.h" +#include "test.h" + +namespace { + +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); + } + virtual void TearDown() { + //unlink(kTestFilename); + } +}; + +TEST_F(DepsLogTest, WriteRead) { + State state1; + DepsLog log1; + string err; + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); + ASSERT_EQ("", err); + + vector deps; + deps.push_back(state1.GetNode("foo.h")); + deps.push_back(state1.GetNode("bar.h")); + log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + + deps.clear(); + deps.push_back(state1.GetNode("foo.h")); + deps.push_back(state1.GetNode("bar2.h")); + log1.RecordDeps(state1.GetNode("out2.o"), 2, deps); + + log1.Close(); + + State state2; + DepsLog log2; + EXPECT_TRUE(log1.Load(kTestFilename, &state2, &err)); + ASSERT_EQ("", err); + state2.Dump(); + + state2.GetNode("out2.o")->Dump(); +} + +} // anonymous namespace diff --git a/src/graph.h b/src/graph.h index 8b93e29..4ef05ec 100644 --- a/src/graph.h +++ b/src/graph.h @@ -32,7 +32,8 @@ struct Node { : path_(path), mtime_(-1), dirty_(false), - in_edge_(NULL) {} + in_edge_(NULL), + id_(-1) {} /// Return true if the file exists (mtime_ got a value). bool Stat(DiskInterface* disk_interface); @@ -74,6 +75,9 @@ struct Node { Edge* in_edge() const { return in_edge_; } void set_in_edge(Edge* edge) { in_edge_ = edge; } + int id() const { return id_; } + void set_id(int id) { id_ = id; } + const vector& out_edges() const { return out_edges_; } void AddOutEdge(Edge* edge) { out_edges_.push_back(edge); } @@ -98,6 +102,9 @@ private: /// All Edges that use this Node as an input. vector out_edges_; + + /// A dense integer id for the node, assigned and used by DepsLog. + int id_; }; /// An invokable build command and associated metadata (description, etc.). diff --git a/src/state.cc b/src/state.cc index 9f46fee..d2d5ebe 100644 --- a/src/state.cc +++ b/src/state.cc @@ -202,10 +202,11 @@ void State::Reset() { void State::Dump() { for (Paths::iterator i = paths_.begin(); i != paths_.end(); ++i) { Node* node = i->second; - printf("%s %s\n", + printf("%s %s [id:%d]\n", node->path().c_str(), node->status_known() ? (node->dirty() ? "dirty" : "clean") - : "unknown"); + : "unknown", + node->id()); } if (!pools_.empty()) { printf("resource_pools:\n"); -- cgit v0.12 From 695a8e5704d390c84737292b06a2ec97aab52093 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sat, 29 Dec 2012 10:53:59 -0800 Subject: pass command results via a struct WaitForCommand now passes all command output via a struct. This will allow adding more output in a future change. --- src/build.cc | 74 ++++++++++++++++++++++--------------------------------- src/build.h | 13 ++++++++-- src/build_test.cc | 35 +++++++++++++------------- 3 files changed, 58 insertions(+), 64 deletions(-) diff --git a/src/build.cc b/src/build.cc index ae47a50..451d7ca 100644 --- a/src/build.cc +++ b/src/build.cc @@ -47,7 +47,7 @@ struct DryRunCommandRunner : public CommandRunner { // Overridden from CommandRunner: virtual bool CanRunMore(); virtual bool StartCommand(Edge* edge); - virtual Edge* WaitForCommand(ExitStatus* status, string* /* output */); + virtual bool WaitForCommand(Result* result); private: queue finished_; @@ -62,16 +62,14 @@ bool DryRunCommandRunner::StartCommand(Edge* edge) { return true; } -Edge* DryRunCommandRunner::WaitForCommand(ExitStatus* status, - string* /*output*/) { - if (finished_.empty()) { - *status = ExitFailure; - return NULL; - } - *status = ExitSuccess; - Edge* edge = finished_.front(); +bool DryRunCommandRunner::WaitForCommand(Result* result) { + if (finished_.empty()) + return false; + + result->status = ExitSuccess; + result->edge = finished_.front(); finished_.pop(); - return edge; + return true; } } // namespace @@ -549,7 +547,7 @@ struct RealCommandRunner : public CommandRunner { virtual ~RealCommandRunner() {} virtual bool CanRunMore(); virtual bool StartCommand(Edge* edge); - virtual Edge* WaitForCommand(ExitStatus* status, string* output); + virtual bool WaitForCommand(Result* result); virtual vector GetActiveEdges(); virtual void Abort(); @@ -586,25 +584,23 @@ bool RealCommandRunner::StartCommand(Edge* edge) { return true; } -Edge* RealCommandRunner::WaitForCommand(ExitStatus* status, string* output) { +bool RealCommandRunner::WaitForCommand(Result* result) { Subprocess* subproc; while ((subproc = subprocs_.NextFinished()) == NULL) { bool interrupted = subprocs_.DoWork(); - if (interrupted) { - *status = ExitInterrupted; - return 0; - } + if (interrupted) + return false; } - *status = subproc->Finish(); - *output = subproc->GetOutput(); + result->status = subproc->Finish(); + result->output = subproc->GetOutput(); map::iterator i = subproc_to_edge_.find(subproc); - Edge* edge = i->second; + result->edge = i->second; subproc_to_edge_.erase(i); delete subproc; - return edge; + return true; } Builder::Builder(State* state, const BuildConfig& config, @@ -696,8 +692,6 @@ bool Builder::Build(string* err) { // First, we attempt to start as many commands as allowed by the // command runner. // Second, we attempt to wait for / reap the next finished command. - // If we can do neither of those, the build is stuck, and we report - // an error. while (plan_.more_to_do()) { // See if we can start any more commands. if (failures_allowed && command_runner_->CanRunMore()) { @@ -719,34 +713,24 @@ bool Builder::Build(string* err) { // See if we can reap any finished commands. if (pending_commands) { - ExitStatus status; - string output; - Edge* edge = command_runner_->WaitForCommand(&status, &output); - if (edge && status != ExitInterrupted) { - bool success = (status == ExitSuccess); - --pending_commands; - FinishEdge(edge, success, output); - if (!success) { - if (failures_allowed) - failures_allowed--; - } - - // We made some progress; start the main loop over. - continue; - } - - if (status == ExitInterrupted) { + CommandRunner::Result result; + if (!command_runner_->WaitForCommand(&result) || + result.status == ExitInterrupted) { status_->BuildFinished(); *err = "interrupted by user"; return false; } - } - // If we get here, we can neither enqueue new commands nor are any running. - if (pending_commands) { - status_->BuildFinished(); - *err = "stuck: pending commands but none to wait for? [this is a bug]"; - return false; + bool success = (result.status == ExitSuccess); + --pending_commands; + FinishEdge(result.edge, success, result.output); + if (!success) { + if (failures_allowed) + failures_allowed--; + } + + // We made some progress; start the main loop over. + continue; } // If we get here, we cannot make any more progress. diff --git a/src/build.h b/src/build.h index 5747170..fa73620 100644 --- a/src/build.h +++ b/src/build.h @@ -103,8 +103,17 @@ struct CommandRunner { virtual ~CommandRunner() {} virtual bool CanRunMore() = 0; virtual bool StartCommand(Edge* edge) = 0; - /// Wait for a command to complete. - virtual Edge* WaitForCommand(ExitStatus* status, string* output) = 0; + + /// The result of waiting for a command. + struct Result { + Result() : edge(NULL) {} + Edge* edge; + ExitStatus status; + string output; + }; + /// Wait for a command to complete, or return false if interrupted. + virtual bool WaitForCommand(Result* result) = 0; + virtual vector GetActiveEdges() { return vector(); } virtual void Abort() {} }; diff --git a/src/build_test.cc b/src/build_test.cc index d4468a3..a74beb9 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -383,7 +383,7 @@ struct FakeCommandRunner : public CommandRunner { // CommandRunner impl virtual bool CanRunMore(); virtual bool StartCommand(Edge* edge); - virtual Edge* WaitForCommand(ExitStatus* status, string* output); + virtual bool WaitForCommand(Result* result); virtual vector GetActiveEdges(); virtual void Abort(); @@ -457,24 +457,25 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { return true; } -Edge* FakeCommandRunner::WaitForCommand(ExitStatus* status, - string* /* output */) { - if (Edge* edge = last_command_) { - if (edge->rule().name() == "interrupt" || - edge->rule().name() == "touch-interrupt") { - *status = ExitInterrupted; - return NULL; - } +bool FakeCommandRunner::WaitForCommand(Result* result) { + if (!last_command_) + return false; - if (edge->rule().name() == "fail") - *status = ExitFailure; - else - *status = ExitSuccess; - last_command_ = NULL; - return edge; + Edge* edge = last_command_; + result->edge = edge; + + if (edge->rule().name() == "interrupt" || + edge->rule().name() == "touch-interrupt") { + result->status = ExitInterrupted; + return true; } - *status = ExitFailure; - return NULL; + + if (edge->rule().name() == "fail") + result->status = ExitFailure; + else + result->status = ExitSuccess; + last_command_ = NULL; + return true; } vector FakeCommandRunner::GetActiveEdges() { -- cgit v0.12 From 00ffada47e1f9649ba76f12ff514f9434a182ef8 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sat, 29 Dec 2012 11:24:03 -0800 Subject: factor out implicit dep loading --- src/graph.cc | 5 +++-- src/graph.h | 21 ++++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/graph.cc b/src/graph.cc index 0f99698..cbd93b6 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -61,7 +61,7 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { string depfile = edge->GetBinding("depfile"); if (!depfile.empty()) { - if (!LoadDepFile(edge, depfile, err)) { + if (!dep_loader_.LoadDepFile(edge, depfile, err)) { if (!err->empty()) return false; EXPLAIN("Edge targets are dirty because depfile '%s' is missing", @@ -282,7 +282,8 @@ bool Edge::GetBindingBool(const string& key) { return !GetBinding(key).empty(); } -bool DependencyScan::LoadDepFile(Edge* edge, const string& path, string* err) { +bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, + string* err) { METRIC_RECORD("depfile load"); string content = disk_interface_->ReadFile(path, err); if (!err->empty()) { diff --git a/src/graph.h b/src/graph.h index 4ef05ec..dc1ef89 100644 --- a/src/graph.h +++ b/src/graph.h @@ -185,13 +185,26 @@ struct Edge { }; +/// ImplicitDepLoader loads implicit dependencies, as referenced via the +/// "depfile" attribute in build files. +struct ImplicitDepLoader { + explicit ImplicitDepLoader(State* state, DiskInterface* disk_interface) + : state_(state), disk_interface_(disk_interface) {} + + bool LoadDepFile(Edge* edge, const string& path, string* err); + + State* state_; + DiskInterface* disk_interface_; +}; + /// DependencyScan manages the process of scanning the files in a graph /// and updating the dirty/outputs_ready state of all the nodes and edges. struct DependencyScan { DependencyScan(State* state, BuildLog* build_log, DiskInterface* disk_interface) - : state_(state), build_log_(build_log), - disk_interface_(disk_interface) {} + : build_log_(build_log), + disk_interface_(disk_interface), + dep_loader_(state, disk_interface) {} /// Examine inputs, outputs, and command lines to judge whether an edge /// needs to be re-run, and update outputs_ready_ and each outputs' |dirty_| @@ -204,8 +217,6 @@ struct DependencyScan { bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input, const string& command, Node* output); - bool LoadDepFile(Edge* edge, const string& path, string* err); - BuildLog* build_log() const { return build_log_; } @@ -214,9 +225,9 @@ struct DependencyScan { } private: - State* state_; BuildLog* build_log_; DiskInterface* disk_interface_; + ImplicitDepLoader dep_loader_; }; #endif // NINJA_GRAPH_H_ -- cgit v0.12 From 5504c0cb054ecfa1c8cd04fa141f5831560d13f4 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sat, 29 Dec 2012 11:55:52 -0800 Subject: use DepsLog in loading dependencies WIP --- src/deps_log.cc | 6 +++ src/deps_log.h | 12 ++--- src/graph.cc | 135 +++++++++++++++++++++++++++++++++----------------------- src/graph.h | 23 +++++++--- 4 files changed, 109 insertions(+), 67 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index ca7fd4b..744b031 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -137,6 +137,12 @@ bool DepsLog::Load(const string& path, State* state, string* err) { return true; } +DepsLog::Deps* DepsLog::GetDeps(Node* node) { + if (node->id() < 0) + return NULL; + return deps_[node->id()]; +} + bool DepsLog::RecordId(Node* node) { uint16_t size = node->path().size(); fwrite(&size, 2, 1, file_); diff --git a/src/deps_log.h b/src/deps_log.h index 45d2cea..e11cca3 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -67,12 +67,6 @@ struct DepsLog { void Close(); // Reading (startup-time) interface. - bool Load(const string& path, State* state, string* err); - - private: - // Write a node name record, assigning it an id. - bool RecordId(Node* node); - struct Deps { Deps() : mtime(-1), node_count(0), nodes(NULL) {} ~Deps() { delete [] nodes; } @@ -80,6 +74,12 @@ struct DepsLog { int node_count; Node** nodes; }; + bool Load(const string& path, State* state, string* err); + Deps* GetDeps(Node* node); + + private: + // Write a node name record, assigning it an id. + bool RecordId(Node* node); FILE* file_; vector nodes_; diff --git a/src/graph.cc b/src/graph.cc index cbd93b6..43f4304 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -19,6 +19,7 @@ #include "build_log.h" #include "depfile_parser.h" +#include "deps_log.h" #include "disk_interface.h" #include "explain.h" #include "manifest_parser.h" @@ -282,6 +283,48 @@ bool Edge::GetBindingBool(const string& key) { return !GetBinding(key).empty(); } +void Edge::Dump(const char* prefix) const { + printf("%s[ ", prefix); + for (vector::const_iterator i = inputs_.begin(); + i != inputs_.end() && *i != NULL; ++i) { + printf("%s ", (*i)->path().c_str()); + } + printf("--%s-> ", rule_->name().c_str()); + for (vector::const_iterator i = outputs_.begin(); + i != outputs_.end() && *i != NULL; ++i) { + printf("%s ", (*i)->path().c_str()); + } + if (pool_) { + if (!pool_->name().empty()) { + printf("(in pool '%s')", pool_->name().c_str()); + } + } else { + printf("(null pool?)"); + } + printf("] 0x%p\n", this); +} + +bool Edge::is_phony() const { + return rule_ == &State::kPhonyRule; +} + +void Node::Dump(const char* prefix) const { + printf("%s <%s 0x%p> mtime: %d%s, (:%s), ", + prefix, path().c_str(), this, + mtime(), mtime() ? "" : " (:missing)", + dirty() ? " dirty" : " clean"); + if (in_edge()) { + in_edge()->Dump("in-edge: "); + } else { + printf("no in-edge\n"); + } + printf(" out edges:\n"); + for (vector::const_iterator e = out_edges().begin(); + e != out_edges().end() && *e != NULL; ++e) { + (*e)->Dump(" +- "); + } +} + bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, string* err) { METRIC_RECORD("depfile load"); @@ -311,11 +354,8 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, } // Preallocate space in edge->inputs_ to be filled in below. - edge->inputs_.insert(edge->inputs_.end() - edge->order_only_deps_, - depfile.ins_.size(), 0); - edge->implicit_deps_ += depfile.ins_.size(); vector::iterator implicit_dep = - edge->inputs_.end() - edge->order_only_deps_ - depfile.ins_.size(); + PreallocateSpace(edge, depfile.ins_.size()); // Add all its in-edges. for (vector::iterator i = depfile.ins_.begin(); @@ -326,66 +366,49 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, Node* node = state_->GetNode(*i); *implicit_dep = node; node->AddOutEdge(edge); - - // If we don't have a edge that generates this input already, - // create one; this makes us not abort if the input is missing, - // but instead will rebuild in that circumstance. - if (!node->in_edge()) { - Edge* phony_edge = state_->AddEdge(&State::kPhonyRule); - node->set_in_edge(phony_edge); - phony_edge->outputs_.push_back(node); - - // RecomputeDirty might not be called for phony_edge if a previous call - // to RecomputeDirty had caused the file to be stat'ed. Because previous - // invocations of RecomputeDirty would have seen this node without an - // input edge (and therefore ready), we have to set outputs_ready_ to true - // to avoid a potential stuck build. If we do call RecomputeDirty for - // this node, it will simply set outputs_ready_ to the correct value. - phony_edge->outputs_ready_ = true; - } + CreatePhonyInEdge(node); } return true; } -void Edge::Dump(const char* prefix) const { - printf("%s[ ", prefix); - for (vector::const_iterator i = inputs_.begin(); - i != inputs_.end() && *i != NULL; ++i) { - printf("%s ", (*i)->path().c_str()); - } - printf("--%s-> ", rule_->name().c_str()); - for (vector::const_iterator i = outputs_.begin(); - i != outputs_.end() && *i != NULL; ++i) { - printf("%s ", (*i)->path().c_str()); - } - if (pool_) { - if (!pool_->name().empty()) { - printf("(in pool '%s')", pool_->name().c_str()); - } - } else { - printf("(null pool?)"); +bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, string* err) { + DepsLog::Deps* deps = deps_log_->GetDeps(edge->outputs_[0]); + if (!deps) + return false; + + // XXX mtime + + vector::iterator implicit_dep = + PreallocateSpace(edge, deps->node_count); + for (int i = 0; i < deps->node_count; ++i, ++implicit_dep) { + *implicit_dep = deps->nodes[i]; + CreatePhonyInEdge(*implicit_dep); } - printf("] 0x%p\n", this); + return true; } -bool Edge::is_phony() const { - return rule_ == &State::kPhonyRule; +vector::iterator ImplicitDepLoader::PreallocateSpace(Edge* edge, + int count) { + edge->inputs_.insert(edge->inputs_.end() - edge->order_only_deps_, + (size_t)count, 0); + edge->implicit_deps_ += count; + return edge->inputs_.end() - edge->order_only_deps_ - count; } -void Node::Dump(const char* prefix) const { - printf("%s <%s 0x%p> mtime: %d%s, (:%s), ", - prefix, path().c_str(), this, - mtime(), mtime() ? "" : " (:missing)", - dirty() ? " dirty" : " clean"); - if (in_edge()) { - in_edge()->Dump("in-edge: "); - } else { - printf("no in-edge\n"); - } - printf(" out edges:\n"); - for (vector::const_iterator e = out_edges().begin(); - e != out_edges().end() && *e != NULL; ++e) { - (*e)->Dump(" +- "); - } +void ImplicitDepLoader::CreatePhonyInEdge(Node* node) { + if (node->in_edge()) + return; + + Edge* phony_edge = state_->AddEdge(&State::kPhonyRule); + node->set_in_edge(phony_edge); + phony_edge->outputs_.push_back(node); + + // RecomputeDirty might not be called for phony_edge if a previous call + // to RecomputeDirty had caused the file to be stat'ed. Because previous + // invocations of RecomputeDirty would have seen this node without an + // input edge (and therefore ready), we have to set outputs_ready_ to true + // to avoid a potential stuck build. If we do call RecomputeDirty for + // this node, it will simply set outputs_ready_ to the correct value. + phony_edge->outputs_ready_ = true; } diff --git a/src/graph.h b/src/graph.h index dc1ef89..b27111b 100644 --- a/src/graph.h +++ b/src/graph.h @@ -22,8 +22,13 @@ using namespace std; #include "eval_env.h" #include "timestamp.h" +struct BuildLog; struct DiskInterface; +struct DepsLog; struct Edge; +struct Node; +struct Pool; +struct State; /// Information about a node in the dependency graph: the file, whether /// it's dirty, mtime, etc. @@ -128,11 +133,6 @@ struct Rule { map bindings_; }; -struct BuildLog; -struct Node; -struct State; -struct Pool; - /// An edge in the dependency graph; links between Nodes using Rules. struct Edge { Edge() : rule_(NULL), env_(NULL), outputs_ready_(false), implicit_deps_(0), @@ -192,11 +192,24 @@ struct ImplicitDepLoader { : state_(state), disk_interface_(disk_interface) {} bool LoadDepFile(Edge* edge, const string& path, string* err); + bool LoadDepsFromLog(Edge* edge, string* err); + + private: + /// Preallocate \a count spaces in the input array on \a edge, returning + /// an iterator pointing at the first new space. + vector::iterator PreallocateSpace(Edge* edge, int count); + + /// If we don't have a edge that generates this input already, + /// create one; this makes us not abort if the input is missing, + /// but instead will rebuild in that circumstance. + void CreatePhonyInEdge(Node* node); State* state_; DiskInterface* disk_interface_; + DepsLog* deps_log_; }; + /// DependencyScan manages the process of scanning the files in a graph /// and updating the dirty/outputs_ready state of all the nodes and edges. struct DependencyScan { -- cgit v0.12 From 78ed77667671cde08b5ef045226aae3f848b1b40 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 30 Dec 2012 09:53:57 -0800 Subject: factor out creation of build directory --- src/ninja.cc | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/ninja.cc b/src/ninja.cc index 69646e1..da99020 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -641,20 +641,13 @@ bool DebugEnable(const string& name, Globals* globals) { } } -bool OpenLog(BuildLog* build_log, Globals* globals, - DiskInterface* disk_interface) { - const string build_dir = - globals->state->bindings_.LookupVariable("builddir"); - const char* kLogPath = ".ninja_log"; - string log_path = kLogPath; - if (!build_dir.empty()) { - log_path = build_dir + "/" + kLogPath; - if (!disk_interface->MakeDirs(log_path) && errno != EEXIST) { - Error("creating build directory %s: %s", - build_dir.c_str(), strerror(errno)); - return false; - } - } +/// Open the build log. +/// @return false on error. +bool OpenBuildLog(BuildLog* build_log, const string& build_dir, + Globals* globals, DiskInterface* disk_interface) { + string log_path = ".ninja_log"; + if (!build_dir.empty()) + log_path = build_dir + "/" + log_path; string err; if (!build_log->Load(log_path, &err)) { @@ -872,9 +865,21 @@ reload: if (tool && tool->when == Tool::RUN_AFTER_LOAD) return tool->func(&globals, argc, argv); - BuildLog build_log; RealDiskInterface disk_interface; - if (!OpenLog(&build_log, &globals, &disk_interface)) + + // Create the build dir if it doesn't exist. + const string build_dir = globals.state->bindings_.LookupVariable("builddir"); + if (!build_dir.empty() && !config.dry_run) { + if (!disk_interface.MakeDirs(build_dir + "/.") && + errno != EEXIST) { + Error("creating build directory %s: %s", + build_dir.c_str(), strerror(errno)); + return 1; + } + } + + BuildLog build_log; + if (!OpenBuildLog(&build_log, build_dir, &globals, &disk_interface)) return 1; if (!rebuilt_manifest) { // Don't get caught in an infinite loop by a rebuild -- cgit v0.12 From 7c357182a4f672b9bdb901e5710887613eef20e8 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 30 Dec 2012 10:10:03 -0800 Subject: load deps log at startup --- src/ninja.cc | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/ninja.cc b/src/ninja.cc index da99020..9f4d67b 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -32,6 +32,7 @@ #include "browse.h" #include "build.h" #include "build_log.h" +#include "deps_log.h" #include "clean.h" #include "disk_interface.h" #include "edit_distance.h" @@ -670,6 +671,36 @@ bool OpenBuildLog(BuildLog* build_log, const string& build_dir, return true; } +/// Open the deps log: load it, then open for writing. +/// @return false on error. +bool OpenDepsLog(DepsLog* deps_log, const string& build_dir, + Globals* globals, DiskInterface* disk_interface) { + string path = ".ninja_deps"; + if (!build_dir.empty()) + path = build_dir + "/" + path; + + string err; + if (!deps_log->Load(path, globals->state, &err)) { + Error("loading deps log %s: %s", path.c_str(), err.c_str()); + return false; + } + if (!err.empty()) { + // Hack: Load() can return a warning via err by returning true. + Warning("%s", err.c_str()); + err.clear(); + } + + if (!globals->config->dry_run) { + if (!deps_log->OpenForWrite(path, &err)) { + Error("opening deps log: %s", err.c_str()); + return false; + } + } + + return true; +} + + /// Dump the output requested by '-d stats'. void DumpMetrics(Globals* globals) { g_metrics->Report(); @@ -882,6 +913,10 @@ reload: if (!OpenBuildLog(&build_log, build_dir, &globals, &disk_interface)) return 1; + DepsLog deps_log; + if (!OpenDepsLog(&deps_log, build_dir, &globals, &disk_interface)) + return 1; + if (!rebuilt_manifest) { // Don't get caught in an infinite loop by a rebuild // target that is never up to date. Builder manifest_builder(globals.state, config, &build_log, -- cgit v0.12 From 6e21a236edd859b885f8c1bea8090bbdd76bf15d Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 30 Dec 2012 10:11:13 -0800 Subject: no error if deps log doesn't exist --- src/deps_log.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/deps_log.cc b/src/deps_log.cc index 744b031..23c9820 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -83,6 +83,8 @@ bool DepsLog::Load(const string& path, State* state, string* err) { char buf[32 << 10]; FILE* f = fopen(path.c_str(), "rb"); if (!f) { + if (errno == ENOENT) + return true; *err = strerror(errno); return false; } -- cgit v0.12 From 70fd0f590c3a46a4a9ff9a160f455c5c30f5b90a Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 30 Dec 2012 10:15:49 -0800 Subject: plumb DepsLog load through Builder --- src/build.cc | 5 +++-- src/build.h | 3 ++- src/build_test.cc | 2 +- src/disk_interface_test.cc | 2 +- src/graph.h | 9 +++++---- src/graph_test.cc | 2 +- src/ninja.cc | 5 +++-- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/build.cc b/src/build.cc index 451d7ca..f44cd52 100644 --- a/src/build.cc +++ b/src/build.cc @@ -604,9 +604,10 @@ bool RealCommandRunner::WaitForCommand(Result* result) { } Builder::Builder(State* state, const BuildConfig& config, - BuildLog* log, DiskInterface* disk_interface) + BuildLog* build_log, DepsLog* deps_log, + DiskInterface* disk_interface) : state_(state), config_(config), disk_interface_(disk_interface), - scan_(state, log, disk_interface) { + scan_(state, build_log, deps_log, disk_interface) { status_ = new BuildStatus(config); } diff --git a/src/build.h b/src/build.h index fa73620..36bca57 100644 --- a/src/build.h +++ b/src/build.h @@ -140,7 +140,8 @@ struct BuildConfig { /// Builder wraps the build process: starting commands, updating status. struct Builder { Builder(State* state, const BuildConfig& config, - BuildLog* log, DiskInterface* disk_interface); + BuildLog* build_log, DepsLog* deps_log, + DiskInterface* disk_interface); ~Builder(); /// Clean up after interrupted commands by deleting output files. diff --git a/src/build_test.cc b/src/build_test.cc index a74beb9..3617439 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -394,7 +394,7 @@ struct FakeCommandRunner : public CommandRunner { struct BuildTest : public StateTestWithBuiltinRules { BuildTest() : config_(MakeConfig()), command_runner_(&fs_), - builder_(&state_, config_, NULL, &fs_), + builder_(&state_, config_, NULL, NULL, &fs_), status_(config_) { builder_.command_runner_.reset(&command_runner_); AssertParse(&state_, diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index c2315c7..9b43d0f 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -104,7 +104,7 @@ TEST_F(DiskInterfaceTest, RemoveFile) { struct StatTest : public StateTestWithBuiltinRules, public DiskInterface { - StatTest() : scan_(&state_, NULL, this) {} + StatTest() : scan_(&state_, NULL, NULL, this) {} // DiskInterface implementation. virtual TimeStamp Stat(const string& path); diff --git a/src/graph.h b/src/graph.h index b27111b..d943702 100644 --- a/src/graph.h +++ b/src/graph.h @@ -188,8 +188,9 @@ struct Edge { /// ImplicitDepLoader loads implicit dependencies, as referenced via the /// "depfile" attribute in build files. struct ImplicitDepLoader { - explicit ImplicitDepLoader(State* state, DiskInterface* disk_interface) - : state_(state), disk_interface_(disk_interface) {} + ImplicitDepLoader(State* state, DepsLog* deps_log, + DiskInterface* disk_interface) + : state_(state), disk_interface_(disk_interface), deps_log_(deps_log) {} bool LoadDepFile(Edge* edge, const string& path, string* err); bool LoadDepsFromLog(Edge* edge, string* err); @@ -213,11 +214,11 @@ struct ImplicitDepLoader { /// DependencyScan manages the process of scanning the files in a graph /// and updating the dirty/outputs_ready state of all the nodes and edges. struct DependencyScan { - DependencyScan(State* state, BuildLog* build_log, + DependencyScan(State* state, BuildLog* build_log, DepsLog* deps_log, DiskInterface* disk_interface) : build_log_(build_log), disk_interface_(disk_interface), - dep_loader_(state, disk_interface) {} + dep_loader_(state, deps_log, disk_interface) {} /// Examine inputs, outputs, and command lines to judge whether an edge /// needs to be re-run, and update outputs_ready_ and each outputs' |dirty_| diff --git a/src/graph_test.cc b/src/graph_test.cc index c105684..63d5757 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -17,7 +17,7 @@ #include "test.h" struct GraphTest : public StateTestWithBuiltinRules { - GraphTest() : scan_(&state_, NULL, &fs_) {} + GraphTest() : scan_(&state_, NULL, NULL, &fs_) {} VirtualFileSystem fs_; DependencyScan scan_; diff --git a/src/ninja.cc b/src/ninja.cc index 9f4d67b..8ba1aa6 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -919,7 +919,7 @@ reload: if (!rebuilt_manifest) { // Don't get caught in an infinite loop by a rebuild // target that is never up to date. - Builder manifest_builder(globals.state, config, &build_log, + Builder manifest_builder(globals.state, config, &build_log, &deps_log, &disk_interface); if (RebuildManifest(&manifest_builder, input_file, &err)) { rebuilt_manifest = true; @@ -931,7 +931,8 @@ reload: } } - Builder builder(globals.state, config, &build_log, &disk_interface); + Builder builder(globals.state, config, &build_log, &deps_log, + &disk_interface); int result = RunBuild(&builder, argc, argv); if (g_metrics) DumpMetrics(&globals); -- cgit v0.12 From 2f2bacc39ee47b648d6b0aea4c60cb64c41393df Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 30 Dec 2012 12:48:44 -0800 Subject: expand DepsLog test, fix two bugs it revealed --- src/deps_log.cc | 9 ++++----- src/deps_log.h | 5 +++++ src/deps_log_test.cc | 38 +++++++++++++++++++++++++++----------- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 23c9820..5732b6d 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -63,11 +63,11 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, fwrite(&size, 2, 1, file_); int id = node->id(); fwrite(&id, 4, 1, file_); - int timestamp = node->mtime(); + int timestamp = mtime; fwrite(×tamp, 4, 1, file_); for (vector::const_iterator i = nodes.begin(); i != nodes.end(); ++i) { - id = node->id(); + id = (*i)->id(); fwrite(&id, 4, 1, file_); } @@ -89,7 +89,6 @@ bool DepsLog::Load(const string& path, State* state, string* err) { return false; } - int id = 0; for (;;) { uint16_t size; if (fread(&size, 2, 1, f) < 1) @@ -127,8 +126,8 @@ bool DepsLog::Load(const string& path, State* state, string* err) { StringPiece path(buf, size); Node* node = state->GetNode(path); assert(node->id() < 0); - node->set_id(id); - ++id; + node->set_id(nodes_.size()); + nodes_.push_back(node); } } if (ferror(f)) { diff --git a/src/deps_log.h b/src/deps_log.h index e11cca3..5ddc7bd 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -77,12 +77,17 @@ struct DepsLog { bool Load(const string& path, State* state, string* err); Deps* GetDeps(Node* node); + /// Used for tests. + const vector& nodes() const { return nodes_; } + private: // Write a node name record, assigning it an id. bool RecordId(Node* node); FILE* file_; + /// Maps id -> Node. vector nodes_; + /// Maps id -> deps of that id. vector deps_; friend struct DepsLogTest; diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 540865b..3f47fef 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -39,25 +39,41 @@ TEST_F(DepsLogTest, WriteRead) { EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); - vector deps; - deps.push_back(state1.GetNode("foo.h")); - deps.push_back(state1.GetNode("bar.h")); - log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + { + vector deps; + deps.push_back(state1.GetNode("foo.h")); + deps.push_back(state1.GetNode("bar.h")); + log1.RecordDeps(state1.GetNode("out.o"), 1, deps); - deps.clear(); - deps.push_back(state1.GetNode("foo.h")); - deps.push_back(state1.GetNode("bar2.h")); - log1.RecordDeps(state1.GetNode("out2.o"), 2, deps); + deps.clear(); + deps.push_back(state1.GetNode("foo.h")); + deps.push_back(state1.GetNode("bar2.h")); + log1.RecordDeps(state1.GetNode("out2.o"), 2, deps); + } log1.Close(); State state2; DepsLog log2; - EXPECT_TRUE(log1.Load(kTestFilename, &state2, &err)); + EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); ASSERT_EQ("", err); - state2.Dump(); - state2.GetNode("out2.o")->Dump(); + ASSERT_EQ(log1.nodes().size(), log2.nodes().size()); + for (int i = 0; i < (int)log1.nodes().size(); ++i) { + Node* node1 = log1.nodes()[i]; + Node* node2 = log2.nodes()[i]; + ASSERT_EQ(i, node1->id()); + ASSERT_EQ(node1->id(), node2->id()); + } + + // log1 has no deps entries, as it was only used for writing. + // Manually check the entries in log2. + DepsLog::Deps* deps = log2.GetDeps(state2.GetNode("out.o")); + ASSERT_TRUE(deps); + ASSERT_EQ(1, deps->mtime); + ASSERT_EQ(2, deps->node_count); + ASSERT_EQ("foo.h", deps->nodes[0]->path()); + ASSERT_EQ("bar.h", deps->nodes[1]->path()); } } // anonymous namespace -- cgit v0.12 From c683ca7d0d9281ef8fc7cb2c4ec022ebc861b745 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 30 Dec 2012 12:50:43 -0800 Subject: track deps log load time in metrics --- src/deps_log.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/deps_log.cc b/src/deps_log.cc index 5732b6d..de25f7b 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -20,6 +20,7 @@ #include #include "graph.h" +#include "metrics.h" #include "state.h" #include "util.h" @@ -80,6 +81,7 @@ void DepsLog::Close() { } bool DepsLog::Load(const string& path, State* state, string* err) { + METRIC_RECORD(".ninja_deps load"); char buf[32 << 10]; FILE* f = fopen(path.c_str(), "rb"); if (!f) { -- cgit v0.12 From 85a4db7822d1b433eff8cf9f94f8f1dab3f0b23d Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Fri, 4 Jan 2013 09:39:01 -0800 Subject: add "special=gcc" attribute, use to load depslog --- misc/ninja_syntax.py | 4 +++- src/graph.cc | 44 ++++++++++++++++++++++++++++++++++---------- src/graph.h | 11 ++++++++++- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index ece7eb5..519330e 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -38,7 +38,7 @@ class Writer(object): def rule(self, name, command, description=None, depfile=None, generator=False, pool=None, restat=False, rspfile=None, - rspfile_content=None): + rspfile_content=None, special=None): self._line('rule %s' % name) self.variable('command', command, indent=1) if description: @@ -55,6 +55,8 @@ class Writer(object): self.variable('rspfile', rspfile, indent=1) if rspfile_content: self.variable('rspfile_content', rspfile_content, indent=1) + if special: + self.variable('special', special, indent=1) def build(self, outputs, rule, inputs=None, implicit=None, order_only=None, variables=None): diff --git a/src/graph.cc b/src/graph.cc index 43f4304..2f86785 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -53,22 +53,19 @@ bool Rule::IsReservedBinding(const string& var) { var == "pool" || var == "restat" || var == "rspfile" || - var == "rspfile_content"; + var == "rspfile_content" || + var == "special"; } bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { bool dirty = false; edge->outputs_ready_ = true; - string depfile = edge->GetBinding("depfile"); - if (!depfile.empty()) { - if (!dep_loader_.LoadDepFile(edge, depfile, err)) { - if (!err->empty()) - return false; - EXPLAIN("Edge targets are dirty because depfile '%s' is missing", - depfile.c_str()); - dirty = true; - } + if (!dep_loader_.LoadDeps(edge, err)) { + if (!err->empty()) + return false; + // Failed to load dependency info: rebuild to regenerate it. + dirty = true; } // Visit all inputs; we're dirty if any of the inputs are dirty. @@ -325,6 +322,33 @@ void Node::Dump(const char* prefix) const { } } +bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { + string special = edge->GetBinding("special"); + if (!special.empty() && special == "gcc") { + if (!LoadDepsFromLog(edge, err)) { + if (!err->empty()) + return false; + EXPLAIN("deps for %s are missing", edge->outputs_[0]->path().c_str()); + return false; + } + return true; + } + + string depfile = edge->GetBinding("depfile"); + if (!depfile.empty()) { + if (!LoadDepFile(edge, depfile, err)) { + if (!err->empty()) + return false; + EXPLAIN("depfile '%s' is missing", depfile.c_str()); + return false; + } + return true; + } + + // No deps to load. + return true; +} + bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, string* err) { METRIC_RECORD("depfile load"); diff --git a/src/graph.h b/src/graph.h index d943702..a7b60db 100644 --- a/src/graph.h +++ b/src/graph.h @@ -192,10 +192,19 @@ struct ImplicitDepLoader { DiskInterface* disk_interface) : state_(state), disk_interface_(disk_interface), deps_log_(deps_log) {} + /// Load implicit dependencies for \a edge. + /// @return false on error (without filling \a err if info is just missing). + bool LoadDeps(Edge* edge, string* err); + + private: + /// Load implicit dependencies for \a edge from a depfile attribute. + /// @return false on error (without filling \a err if info is just missing). bool LoadDepFile(Edge* edge, const string& path, string* err); + + /// Load implicit dependencies for \a edge from the DepsLog. + /// @return false on error (without filling \a err if info is just missing). bool LoadDepsFromLog(Edge* edge, string* err); - private: /// Preallocate \a count spaces in the input array on \a edge, returning /// an iterator pointing at the first new space. vector::iterator PreallocateSpace(Edge* edge, int count); -- cgit v0.12 From d9c24381c50506436212ac447a0805f5ff6892ac Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sat, 5 Jan 2013 10:49:48 -0800 Subject: windows: add uint16 casts in depslog --- src/deps_log.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index de25f7b..da6cd93 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -59,7 +59,7 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, RecordId(*i); } - uint16_t size = 4 * (1 + 1 + nodes.size()); + uint16_t size = 4 * (1 + 1 + (uint16_t)nodes.size()); size |= 0x8000; // Deps record: set high bit. fwrite(&size, 2, 1, file_); int id = node->id(); @@ -147,7 +147,7 @@ DepsLog::Deps* DepsLog::GetDeps(Node* node) { } bool DepsLog::RecordId(Node* node) { - uint16_t size = node->path().size(); + uint16_t size = (uint16_t)node->path().size(); fwrite(&size, 2, 1, file_); fwrite(node->path().data(), node->path().size(), 1, file_); -- cgit v0.12 From 70d43e48e8b8a71c1f1cc55718b34480f68e1341 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 6 Jan 2013 11:36:17 -0800 Subject: factor MSVC parsing out of CLWrapper into CLParser --- src/msvc_helper-win32.cc | 94 +++++++++++++++----------------- src/msvc_helper.h | 42 ++++++++------- src/msvc_helper_main-win32.cc | 26 ++++----- src/msvc_helper_test.cc | 123 ++++++++++++++++++++---------------------- 4 files changed, 139 insertions(+), 146 deletions(-) diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index fd9b671..be2a5e0 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -39,15 +39,15 @@ string Replace(const string& input, const string& find, const string& replace) { return result; } +} // anonymous namespace + string EscapeForDepfile(const string& path) { // Depfiles don't escape single \. return Replace(path, " ", "\\ "); } -} // anonymous namespace - // static -string CLWrapper::FilterShowIncludes(const string& line) { +string CLParser::FilterShowIncludes(const string& line) { static const char kMagicPrefix[] = "Note: including file: "; const char* in = line.c_str(); const char* end = in + line.size(); @@ -63,14 +63,14 @@ string CLWrapper::FilterShowIncludes(const string& line) { } // static -bool CLWrapper::IsSystemInclude(const string& path) { +bool CLParser::IsSystemInclude(const string& path) { // TODO: this is a heuristic, perhaps there's a better way? return (path.find("program files") != string::npos || path.find("microsoft visual studio") != string::npos); } // static -bool CLWrapper::FilterInputFilename(const string& line) { +bool CLParser::FilterInputFilename(const string& line) { // TODO: other extensions, like .asm? return EndsWith(line, ".c") || EndsWith(line, ".cc") || @@ -78,7 +78,42 @@ bool CLWrapper::FilterInputFilename(const string& line) { EndsWith(line, ".cpp"); } -int CLWrapper::Run(const string& command, string* extra_output) { +string CLParser::Parse(const string& output) { + string filtered_output; + + // Loop over all lines in the output to process them. + size_t start = 0; + while (start < output.size()) { + size_t end = output.find_first_of("\r\n", start); + if (end == string::npos) + end = output.size(); + string line = output.substr(start, end - start); + + string include = FilterShowIncludes(line); + if (!include.empty()) { + include = IncludesNormalize::Normalize(include, NULL); + if (!IsSystemInclude(include)) + includes_.insert(include); + } else if (FilterInputFilename(line)) { + // Drop it. + // TODO: if we support compiling multiple output files in a single + // cl.exe invocation, we should stash the filename. + } else { + filtered_output.append(line); + filtered_output.append("\n"); + } + + if (end < output.size() && output[end] == '\r') + ++end; + if (end < output.size() && output[end] == '\n') + ++end; + start = end; + } + + return filtered_output; +} + +int CLWrapper::Run(const string& command, string* output) { SECURITY_ATTRIBUTES security_attributes = {}; security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); security_attributes.bInheritHandle = TRUE; @@ -118,8 +153,7 @@ int CLWrapper::Run(const string& command, string* extra_output) { Win32Fatal("CloseHandle"); } - // Read output of the subprocess and parse it. - string output; + // Read all output of the subprocess. DWORD read_len = 1; while (read_len) { char buf[64 << 10]; @@ -128,44 +162,12 @@ int CLWrapper::Run(const string& command, string* extra_output) { GetLastError() != ERROR_BROKEN_PIPE) { Win32Fatal("ReadFile"); } - output.append(buf, read_len); - - // Loop over all lines in the output and process them. - for (;;) { - size_t ofs = output.find_first_of("\r\n"); - if (ofs == string::npos) - break; - string line = output.substr(0, ofs); - - string include = FilterShowIncludes(line); - if (!include.empty()) { - include = IncludesNormalize::Normalize(include, NULL); - if (!IsSystemInclude(include)) - includes_.insert(include); - } else if (FilterInputFilename(line)) { - // Drop it. - // TODO: if we support compiling multiple output files in a single - // cl.exe invocation, we should stash the filename. - } else { - if (extra_output) { - extra_output->append(line); - extra_output->append("\n"); - } else { - printf("%s\n", line.c_str()); - } - } - - if (ofs < output.size() && output[ofs] == '\r') - ++ofs; - if (ofs < output.size() && output[ofs] == '\n') - ++ofs; - output = output.substr(ofs); - } + output->append(buf, read_len); } + // Wait for it to exit and grab its exit code. if (WaitForSingleObject(process_info.hProcess, INFINITE) == WAIT_FAILED) Win32Fatal("WaitForSingleObject"); - DWORD exit_code = 0; if (!GetExitCodeProcess(process_info.hProcess, &exit_code)) Win32Fatal("GetExitCodeProcess"); @@ -178,11 +180,3 @@ int CLWrapper::Run(const string& command, string* extra_output) { return exit_code; } - -vector CLWrapper::GetEscapedResult() { - vector result; - for (set::iterator i = includes_.begin(); i != includes_.end(); ++i) { - result.push_back(EscapeForDepfile(*i)); - } - return result; -} diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 102201b..32ab606 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -17,23 +17,13 @@ #include using namespace std; +string EscapeForDepfile(const string& path); + /// Visual Studio's cl.exe requires some massaging to work with Ninja; /// for example, it emits include information on stderr in a funny -/// format when building with /showIncludes. This class wraps a CL -/// process and parses that output to extract the file list. -struct CLWrapper { - CLWrapper() : env_block_(NULL) {} - - /// Set the environment block (as suitable for CreateProcess) to be used - /// by Run(). - void SetEnvBlock(void* env_block) { env_block_ = env_block; } - - /// Start a process and parse its output. Returns its exit code. - /// Any non-parsed output is buffered into \a extra_output if provided, - /// otherwise it is printed to stdout while the process runs. - /// Crashes (calls Fatal()) on error. - int Run(const string& command, string* extra_output=NULL); - +/// format when building with /showIncludes. This class parses this +/// output. +struct CLParser { /// Parse a line of cl.exe output and extract /showIncludes info. /// If a dependency is extracted, returns a nonempty string. /// Exposed for testing. @@ -50,10 +40,24 @@ struct CLWrapper { /// Exposed for testing. static bool FilterInputFilename(const string& line); - /// Fill a vector with the unique'd headers, escaped for output as a .d - /// file. - vector GetEscapedResult(); + /// Parse the full output of cl, returning the output (if any) that + /// should printed. + string Parse(const string& output); - void* env_block_; set includes_; }; + +/// Wraps a synchronous execution of a CL subprocess. +struct CLWrapper { + CLWrapper() : env_block_(NULL) {} + + /// Set the environment block (as suitable for CreateProcess) to be used + /// by Run(). + void SetEnvBlock(void* env_block) { env_block_ = env_block; } + + /// Start a process and gather its raw output. Returns its exit code. + /// Crashes (calls Fatal()) on error. + int Run(const string& command, string* output); + + void* env_block_; +}; diff --git a/src/msvc_helper_main-win32.cc b/src/msvc_helper_main-win32.cc index 0bbe98b..3192821 100644 --- a/src/msvc_helper_main-win32.cc +++ b/src/msvc_helper_main-win32.cc @@ -44,12 +44,13 @@ void PushPathIntoEnvironment(const string& env_block) { } } -void WriteDepFileOrDie(const char* object_path, CLWrapper* cl) { +void WriteDepFileOrDie(const char* object_path, const CLParser& parse) { string depfile_path = string(object_path) + ".d"; FILE* depfile = fopen(depfile_path.c_str(), "w"); if (!depfile) { unlink(object_path); - Fatal("opening %s: %s", depfile_path.c_str(), GetLastErrorString().c_str()); + Fatal("opening %s: %s", depfile_path.c_str(), + GetLastErrorString().c_str()); } if (fprintf(depfile, "%s: ", object_path) < 0) { unlink(object_path); @@ -57,9 +58,9 @@ void WriteDepFileOrDie(const char* object_path, CLWrapper* cl) { unlink(depfile_path.c_str()); Fatal("writing %s", depfile_path.c_str()); } - vector headers = cl->GetEscapedResult(); - for (vector::iterator i = headers.begin(); i != headers.end(); ++i) { - if (fprintf(depfile, "%s\n", i->c_str()) < 0) { + const set& headers = parse.includes_; + for (set::iterator i = headers.begin(); i != headers.end(); ++i) { + if (fprintf(depfile, "%s\n", EscapeForDepfile(*i).c_str()) < 0) { unlink(object_path); fclose(depfile); unlink(depfile_path.c_str()); @@ -95,11 +96,6 @@ int MSVCHelperMain(int argc, char** argv) { } } - if (!output_filename) { - Usage(); - Fatal("-o required"); - } - string env; if (envfile) { string err; @@ -118,9 +114,15 @@ int MSVCHelperMain(int argc, char** argv) { CLWrapper cl; if (!env.empty()) cl.SetEnvBlock((void*)env.data()); - int exit_code = cl.Run(command); + string output; + int exit_code = cl.Run(command, &output); - WriteDepFileOrDie(output_filename, &cl); + if (output_filename) { + CLParser parser; + output = parser.Parse(output); + WriteDepFileOrDie(output_filename, parser); + } + printf("%s\n", output.c_str()); return exit_code; } diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 7730425..1e1cbde 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -19,100 +19,93 @@ #include "test.h" #include "util.h" -TEST(MSVCHelperTest, ShowIncludes) { - ASSERT_EQ("", CLWrapper::FilterShowIncludes("")); +TEST(CLParserTest, ShowIncludes) { + ASSERT_EQ("", CLParser::FilterShowIncludes("")); - ASSERT_EQ("", CLWrapper::FilterShowIncludes("Sample compiler output")); + ASSERT_EQ("", CLParser::FilterShowIncludes("Sample compiler output")); ASSERT_EQ("c:\\Some Files\\foobar.h", - CLWrapper::FilterShowIncludes("Note: including file: " - "c:\\Some Files\\foobar.h")); + CLParser::FilterShowIncludes("Note: including file: " + "c:\\Some Files\\foobar.h")); ASSERT_EQ("c:\\initspaces.h", - CLWrapper::FilterShowIncludes("Note: including file: " - "c:\\initspaces.h")); + CLParser::FilterShowIncludes("Note: including file: " + "c:\\initspaces.h")); } -TEST(MSVCHelperTest, FilterInputFilename) { - ASSERT_TRUE(CLWrapper::FilterInputFilename("foobar.cc")); - ASSERT_TRUE(CLWrapper::FilterInputFilename("foo bar.cc")); - ASSERT_TRUE(CLWrapper::FilterInputFilename("baz.c")); +TEST(CLParserTest, FilterInputFilename) { + ASSERT_TRUE(CLParser::FilterInputFilename("foobar.cc")); + ASSERT_TRUE(CLParser::FilterInputFilename("foo bar.cc")); + ASSERT_TRUE(CLParser::FilterInputFilename("baz.c")); - ASSERT_FALSE(CLWrapper::FilterInputFilename( + ASSERT_FALSE(CLParser::FilterInputFilename( "src\\cl_helper.cc(166) : fatal error C1075: end " "of file found ...")); } -TEST(MSVCHelperTest, Run) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo foo&& echo Note: including file: foo.h&&echo bar\"", - &output); +TEST(CLParserTest, ParseSimple) { + CLParser parser; + string output = parser.Parse( + "foo\r\n" + "Note: including file: foo.h\r\n" + "bar\r\n"); + ASSERT_EQ("foo\nbar\n", output); - ASSERT_EQ(1u, cl.includes_.size()); - ASSERT_EQ("foo.h", *cl.includes_.begin()); + ASSERT_EQ(1u, parser.includes_.size()); + ASSERT_EQ("foo.h", *parser.includes_.begin()); } -TEST(MSVCHelperTest, RunFilenameFilter) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo foo.cc&& echo cl: warning\"", - &output); +TEST(CLParserTest, ParseFilenameFilter) { + CLParser parser; + string output = parser.Parse( + "foo.cc\r\n" + "cl: warning\r\n"); ASSERT_EQ("cl: warning\n", output); } -TEST(MSVCHelperTest, RunSystemInclude) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo Note: including file: c:\\Program Files\\foo.h&&" - "echo Note: including file: d:\\Microsoft Visual Studio\\bar.h&&" - "echo Note: including file: path.h\"", - &output); +TEST(CLParserTest, ParseSystemInclude) { + CLParser parser; + string output = parser.Parse( + "Note: including file: c:\\Program Files\\foo.h\r\n" + "Note: including file: d:\\Microsoft Visual Studio\\bar.h\r\n" + "Note: including file: path.h\r\n"); // We should have dropped the first two includes because they look like // system headers. ASSERT_EQ("", output); - ASSERT_EQ(1u, cl.includes_.size()); - ASSERT_EQ("path.h", *cl.includes_.begin()); -} - -TEST(MSVCHelperTest, EnvBlock) { - char env_block[] = "foo=bar\0"; - CLWrapper cl; - cl.SetEnvBlock(env_block); - string output; - cl.Run("cmd /c \"echo foo is %foo%", &output); - ASSERT_EQ("foo is bar\n", output); + ASSERT_EQ(1u, parser.includes_.size()); + ASSERT_EQ("path.h", *parser.includes_.begin()); } -TEST(MSVCHelperTest, DuplicatedHeader) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo Note: including file: foo.h&&" - "echo Note: including file: bar.h&&" - "echo Note: including file: foo.h\"", - &output); +TEST(CLParserTest, DuplicatedHeader) { + CLParser parser; + string output = parser.Parse( + "Note: including file: foo.h\r\n" + "Note: including file: bar.h\r\n" + "Note: including file: foo.h\r\n"); // We should have dropped one copy of foo.h. ASSERT_EQ("", output); - ASSERT_EQ(2u, cl.includes_.size()); + ASSERT_EQ(2u, parser.includes_.size()); } -TEST(MSVCHelperTest, DuplicatedHeaderPathConverted) { - CLWrapper cl; - string output; - cl.Run("cmd /c \"echo Note: including file: sub/foo.h&&" - "echo Note: including file: bar.h&&" - "echo Note: including file: sub\\foo.h\"", - &output); +TEST(CLParserTest, DuplicatedHeaderPathConverted) { + CLParser parser; + string output = parser.Parse( + "Note: including file: sub/foo.h\r\n" + "Note: including file: bar.h\r\n" + "Note: including file: sub\\foo.h\r\n"); // We should have dropped one copy of foo.h. ASSERT_EQ("", output); - ASSERT_EQ(2u, cl.includes_.size()); + ASSERT_EQ(2u, parser.includes_.size()); } -TEST(MSVCHelperTest, SpacesInFilename) { +TEST(CLParserTest, SpacesInFilename) { + ASSERT_EQ("sub\\some\\ sdk\\foo.h", + EscapeForDepfile("sub\\some sdk\\foo.h")); +} + +TEST(MSVCHelperTest, EnvBlock) { + char env_block[] = "foo=bar\0"; CLWrapper cl; + cl.SetEnvBlock(env_block); string output; - cl.Run("cmd /c \"echo Note: including file: sub\\some sdk\\foo.h", - &output); - ASSERT_EQ("", output); - vector headers = cl.GetEscapedResult(); - ASSERT_EQ(1u, headers.size()); - ASSERT_EQ("sub\\some\\ sdk\\foo.h", headers[0]); + cl.Run("cmd /c \"echo foo is %foo%", &output); + ASSERT_EQ("foo is bar\r\n", output); } -- cgit v0.12 From befa46192f97e98947e6d8772888e91e8a3bc629 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 6 Jan 2013 11:42:40 -0800 Subject: refactor build-time deps-extraction --- src/build.cc | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/build.h | 3 +++ 2 files changed, 53 insertions(+) diff --git a/src/build.cc b/src/build.cc index f44cd52..eea2d70 100644 --- a/src/build.cc +++ b/src/build.cc @@ -15,6 +15,7 @@ #include "build.h" #include +#include #include #include #include @@ -32,6 +33,7 @@ #endif #include "build_log.h" +#include "depfile_parser.h" #include "disk_interface.h" #include "graph.h" #include "state.h" @@ -723,6 +725,18 @@ bool Builder::Build(string* err) { } bool success = (result.status == ExitSuccess); + + if (success) { + vector deps_nodes; + string extract_err; + if (!ExtractDeps(&result, &deps_nodes, &extract_err)) { + if (!result.output.empty()) + result.output.append("\n"); + result.output.append(extract_err); + success = false; + } + } + --pending_commands; FinishEdge(result.edge, success, result.output); if (!success) { @@ -846,3 +860,39 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) { scan_.build_log()->RecordCommand(edge, start_time, end_time, restat_mtime); } +bool Builder::ExtractDeps(CommandRunner::Result* result, + vector* deps_nodes, + string* err) { +#ifdef _WIN32 +#else + string depfile = result->edge->GetBinding("depfile"); + if (depfile.empty()) + return true; // No dependencies to load. + + string content = disk_interface_->ReadFile(depfile, err); + if (!err->empty()) + return false; + + DepfileParser deps; + if (!deps.Parse(&content, err)) + return false; + + // XXX check depfile matches expected output. + deps_nodes->reserve(deps.ins_.size()); + for (vector::iterator i = deps.ins_.begin(); + i != deps.ins_.end(); ++i) { + if (!CanonicalizePath(const_cast(i->str_), &i->len_, err)) + return false; + deps_nodes->push_back(state_->GetNode(*i)); + } + + /* TODO: unlink the file via diskinterface. + if (unlink(depfile.c_str()) < 0) { + *err = string("unlink: ")) + strerror(errno); + return false; + } + */ +#endif + + return deps_nodes; +} diff --git a/src/build.h b/src/build.h index 36bca57..e71549d 100644 --- a/src/build.h +++ b/src/build.h @@ -175,6 +175,9 @@ struct Builder { BuildStatus* status_; private: + bool ExtractDeps(CommandRunner::Result* result, vector* deps_nodes, + string* err); + DiskInterface* disk_interface_; DependencyScan scan_; -- cgit v0.12 From 1bda3330103dbc3b11c2043318da5fc66991a893 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 6 Jan 2013 15:21:07 -0800 Subject: windows: use CLParser to extract deps during build --- src/build.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/build.cc b/src/build.cc index eea2d70..f648584 100644 --- a/src/build.cc +++ b/src/build.cc @@ -36,6 +36,7 @@ #include "depfile_parser.h" #include "disk_interface.h" #include "graph.h" +#include "msvc_helper.h" #include "state.h" #include "subprocess.h" #include "util.h" @@ -864,6 +865,12 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, vector* deps_nodes, string* err) { #ifdef _WIN32 + CLParser parser; + result->output = parser.Parse(result->output); + for (set::iterator i = parser.includes_.begin(); + i != parser.includes_.end(); ++i) { + deps_nodes->push_back(state_->GetNode(*i)); + } #else string depfile = result->edge->GetBinding("depfile"); if (depfile.empty()) @@ -894,5 +901,5 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, */ #endif - return deps_nodes; + return true; } -- cgit v0.12 From ce65cb987adc7366f6da8f9d4e34c8cdab2d49bc Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 6 Jan 2013 15:26:20 -0800 Subject: use special=anything to trigger loading from depslog --- src/graph.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/graph.cc b/src/graph.cc index 2f86785..bb99bcb 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -324,7 +324,7 @@ void Node::Dump(const char* prefix) const { bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { string special = edge->GetBinding("special"); - if (!special.empty() && special == "gcc") { + if (!special.empty()) { if (!LoadDepsFromLog(edge, err)) { if (!err->empty()) return false; -- cgit v0.12 From 58c7139b9f404e18097d4f3ef6adcd49a01e3d73 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 6 Jan 2013 15:33:46 -0800 Subject: windows: drop use of msvc helper in build --- bootstrap.py | 4 +--- configure.py | 12 +++--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/bootstrap.py b/bootstrap.py index fcf1a20..4d9bc84 100755 --- a/bootstrap.py +++ b/bootstrap.py @@ -143,9 +143,7 @@ Done! Note: to work around Windows file locking, where you can't rebuild an in-use binary, to run ninja after making any changes to build ninja itself -you should run ninja.bootstrap instead. Your build is also configured to -use ninja.bootstrap.exe as the MSVC helper; see the --with-ninja flag of -the --help output of configure.py.""") +you should run ninja.bootstrap instead.""") else: print('Building ninja using itself...') run([sys.executable, 'configure.py'] + conf_args) diff --git a/configure.py b/configure.py index 8f5a497..eddf248 100755 --- a/configure.py +++ b/configure.py @@ -47,9 +47,6 @@ parser.add_option('--with-gtest', metavar='PATH', parser.add_option('--with-python', metavar='EXE', help='use EXE as the Python interpreter', default=os.path.basename(sys.executable)) -parser.add_option('--with-ninja', metavar='NAME', - help="name for ninja binary for -t msvc (MSVC only)", - default="ninja") (options, args) = parser.parse_args() if args: print('ERROR: extra unparsed command-line arguments:', args) @@ -190,14 +187,11 @@ n.variable('ldflags', ' '.join(shell_escape(flag) for flag in ldflags)) n.newline() if platform == 'windows': - compiler = '$cxx' - if options.with_ninja: - compiler = ('%s -t msvc -o $out -- $cxx /showIncludes' % - options.with_ninja) n.rule('cxx', - command='%s $cflags -c $in /Fo$out' % compiler, + command='$cxx /showIncludes $cflags -c $in /Fo$out', depfile='$out.d', - description='CXX $out') + description='CXX $out', + special='msvc') else: n.rule('cxx', command='$cxx -MMD -MT $out -MF $out.d $cflags -c $in -o $out', -- cgit v0.12 From ab218230c4c6c3f0bb2a26215d1ac09e397e6065 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 7 Jan 2013 10:59:27 -0800 Subject: don't write out deps entries if nothing changed Shortcuts a common case. --- src/deps_log.cc | 32 ++++++++++++++++++++++++++++++-- src/deps_log_test.cc | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index da6cd93..0f10b5c 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -50,15 +50,43 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, const vector& nodes) { + // Track whether there's any new data to be recorded. + bool made_change = false; + // Assign ids to all nodes that are missing one. - if (node->id() < 0) + if (node->id() < 0) { RecordId(node); + made_change = true; + } for (vector::const_iterator i = nodes.begin(); i != nodes.end(); ++i) { - if ((*i)->id() < 0) + if ((*i)->id() < 0) { RecordId(*i); + made_change = true; + } + } + + // See if the new data is different than the existing data, if any. + if (!made_change) { + Deps* deps = GetDeps(node); + if (!deps || + deps->mtime != mtime || + deps->node_count != (int)nodes.size()) { + made_change = true; + } else { + for (int i = 0; i < (int)nodes.size(); ++i) { + if (deps->nodes[i] != nodes[i]) { + made_change = true; + break; + } + } + } } + // Don't write anything if there's no new info. + if (!made_change) + return true; + uint16_t size = 4 * (1 + 1 + (uint16_t)nodes.size()); size |= 0x8000; // Deps record: set high bit. fwrite(&size, 2, 1, file_); diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 3f47fef..e411e12 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -76,4 +76,50 @@ TEST_F(DepsLogTest, WriteRead) { ASSERT_EQ("bar.h", deps->nodes[1]->path()); } +// Verify that adding the same deps twice doesn't grow the file. +TEST_F(DepsLogTest, DoubleEntry) { + // Write some deps to the file and grab its size. + int file_size; + { + State state; + DepsLog log; + string err; + EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); + ASSERT_EQ("", err); + + vector deps; + deps.push_back(state.GetNode("foo.h")); + deps.push_back(state.GetNode("bar.h")); + log.RecordDeps(state.GetNode("out.o"), 1, deps); + log.Close(); + + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + file_size = (int)st.st_size; + ASSERT_GT(file_size, 0); + } + + // Now reload the file, and readd the same deps. + { + State state; + DepsLog log; + string err; + EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); + + EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); + ASSERT_EQ("", err); + + vector deps; + deps.push_back(state.GetNode("foo.h")); + deps.push_back(state.GetNode("bar.h")); + log.RecordDeps(state.GetNode("out.o"), 1, deps); + log.Close(); + + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + int file_size_2 = (int)st.st_size; + ASSERT_EQ(file_size, file_size_2); + } +} + } // anonymous namespace -- cgit v0.12 From afd206d99004d551afcfef55ec69ab65c4eb81d4 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 7 Jan 2013 11:04:24 -0800 Subject: record and check depslog file version Future-proofing against some change we may need to make later. --- src/deps_log.cc | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 0f10b5c..5706be4 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -24,6 +24,12 @@ #include "state.h" #include "util.h" +namespace { +const char kFileSignature[] = "# ninja deps v%d\n"; +const int kCurrentVersion = 1; +} // anonymous namespace + + bool DepsLog::OpenForWrite(const string& path, string* err) { file_ = fopen(path.c_str(), "ab"); if (!file_) { @@ -36,14 +42,12 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { // end on Windows. Do that explicitly. fseek(file_, 0, SEEK_END); - /* XXX - if (ftell(log_file_) == 0) { - if (fprintf(log_file_, kFileSignature, kCurrentVersion) < 0) { + if (ftell(file_) == 0) { + if (fprintf(file_, kFileSignature, kCurrentVersion) < 0) { *err = strerror(errno); return false; } } - */ return true; } @@ -119,6 +123,22 @@ bool DepsLog::Load(const string& path, State* state, string* err) { return false; } + if (!fgets(buf, sizeof(buf), f)) { + *err = strerror(errno); + return false; + } + int version = 0; + if (sscanf(buf, kFileSignature, &version) != 1) { + *err = "unable to read file signature"; + return false; + } + if (version != kCurrentVersion) { + *err = "bad deps log version; starting over"; + // Don't report this as a failure. An empty deps log will cause + // us to rebuild the outputs anyway. + return true; + } + for (;;) { uint16_t size; if (fread(&size, 2, 1, f) < 1) -- cgit v0.12 From e801e67a4cfae117098f252d365d67704eba4bfd Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 8 Jan 2013 08:17:12 -0800 Subject: make old deps format migration actually work --- src/deps_log.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 5706be4..081fcf0 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -128,12 +128,11 @@ bool DepsLog::Load(const string& path, State* state, string* err) { return false; } int version = 0; - if (sscanf(buf, kFileSignature, &version) != 1) { - *err = "unable to read file signature"; - return false; - } + sscanf(buf, kFileSignature, &version); if (version != kCurrentVersion) { - *err = "bad deps log version; starting over"; + *err = "bad deps log signature or version; starting over"; + fclose(f); + unlink(path.c_str()); // Don't report this as a failure. An empty deps log will cause // us to rebuild the outputs anyway. return true; -- cgit v0.12 From e280115c03a296078b26da4b10a643cb1a6bda8a Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 8 Jan 2013 08:20:56 -0800 Subject: clarify depslog overview --- src/deps_log.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/deps_log.h b/src/deps_log.h index 5ddc7bd..56f9590 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -27,10 +27,10 @@ struct Node; struct State; /// As build commands run they can output extra dependency information -/// (e.g. header dependencies for C source) via a pipe. DepsLog collects -/// that information at build time and reloads it at startup. +/// (e.g. header dependencies for C source) dynamically. DepsLog collects +/// that information at build time and uses it for subsequent builds. /// -/// The on-disk format is based on two primary constraints: +/// The on-disk format is based on two primary design constraints: /// - it must be written to as a stream (during the build, which may be /// interrupted); /// - it can be read all at once on startup. (Alternative designs, where @@ -43,7 +43,8 @@ struct State; /// There's about 10k files worth of dependencies that reference about /// 40k total paths totalling 2mb of unique strings. /// -/// Based on these above, the file is structured as a sequence of records. +/// Based on these stats, here's the current design. +/// The file is structured as version header followed by a sequence of records. /// Each record is either a path string or a dependency list. /// Numbering the path strings in file order gives them dense integer ids. /// A dependency list maps an output id to a list of input ids. -- cgit v0.12 From ec92fe3da6e792f9e14a490675aebc132ec37ef6 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 8 Jan 2013 08:43:39 -0800 Subject: add recompaction to depslog Not done automatically yet, just an implementation and a test. --- src/deps_log.cc | 63 ++++++++++++++++++++++++++++++++++++++-------- src/deps_log.h | 4 +++ src/deps_log_test.cc | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 081fcf0..5031515 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -54,6 +54,11 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, const vector& nodes) { + return RecordDeps(node, mtime, nodes.size(), (Node**)&nodes.front()); +} + +bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, + int node_count, Node** nodes) { // Track whether there's any new data to be recorded. bool made_change = false; @@ -62,10 +67,9 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, RecordId(node); made_change = true; } - for (vector::const_iterator i = nodes.begin(); - i != nodes.end(); ++i) { - if ((*i)->id() < 0) { - RecordId(*i); + for (int i = 0; i < node_count; ++i) { + if (nodes[i]->id() < 0) { + RecordId(nodes[i]); made_change = true; } } @@ -75,10 +79,10 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, Deps* deps = GetDeps(node); if (!deps || deps->mtime != mtime || - deps->node_count != (int)nodes.size()) { + deps->node_count != node_count) { made_change = true; } else { - for (int i = 0; i < (int)nodes.size(); ++i) { + for (int i = 0; i < node_count; ++i) { if (deps->nodes[i] != nodes[i]) { made_change = true; break; @@ -91,16 +95,15 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, if (!made_change) return true; - uint16_t size = 4 * (1 + 1 + (uint16_t)nodes.size()); + uint16_t size = 4 * (1 + 1 + (uint16_t)node_count); size |= 0x8000; // Deps record: set high bit. fwrite(&size, 2, 1, file_); int id = node->id(); fwrite(&id, 4, 1, file_); int timestamp = mtime; fwrite(×tamp, 4, 1, file_); - for (vector::const_iterator i = nodes.begin(); - i != nodes.end(); ++i) { - id = (*i)->id(); + for (int i = 0; i < node_count; ++i) { + id = nodes[i]->id(); fwrite(&id, 4, 1, file_); } @@ -193,6 +196,46 @@ DepsLog::Deps* DepsLog::GetDeps(Node* node) { return deps_[node->id()]; } +bool DepsLog::Recompact(const string& path, string* err) { + METRIC_RECORD(".ninja_deps recompact"); + printf("Recompacting deps...\n"); + + string temp_path = path + ".recompact"; + DepsLog new_log; + if (!new_log.OpenForWrite(temp_path, err)) + return false; + + // Clear all known ids so that new ones can be reassigned. + for (vector::iterator i = nodes_.begin(); + i != nodes_.end(); ++i) { + (*i)->set_id(-1); + } + + // Write out all deps again. + for (int old_id = 0; old_id < (int)deps_.size(); ++old_id) { + Deps* deps = deps_[old_id]; + if (!new_log.RecordDeps(nodes_[old_id], deps->mtime, + deps->node_count, deps->nodes)) { + new_log.Close(); + return false; + } + } + + new_log.Close(); + + if (unlink(path.c_str()) < 0) { + *err = strerror(errno); + return false; + } + + if (rename(temp_path.c_str(), path.c_str()) < 0) { + *err = strerror(errno); + return false; + } + + return true; +} + bool DepsLog::RecordId(Node* node) { uint16_t size = (uint16_t)node->path().size(); fwrite(&size, 2, 1, file_); diff --git a/src/deps_log.h b/src/deps_log.h index 56f9590..d763a1d 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -65,6 +65,7 @@ struct DepsLog { // Writing (build-time) interface. bool OpenForWrite(const string& path, string* err); bool RecordDeps(Node* node, TimeStamp mtime, const vector& nodes); + bool RecordDeps(Node* node, TimeStamp mtime, int node_count, Node** nodes); void Close(); // Reading (startup-time) interface. @@ -78,6 +79,9 @@ struct DepsLog { bool Load(const string& path, State* state, string* err); Deps* GetDeps(Node* node); + /// Rewrite the known log entries, throwing away old data. + bool Recompact(const string& path, string* err); + /// Used for tests. const vector& nodes() const { return nodes_; } diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index e411e12..2d91c0e 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -122,4 +122,74 @@ TEST_F(DepsLogTest, DoubleEntry) { } } +// Verify that adding the new deps works and can be compacted away. +TEST_F(DepsLogTest, Recompact) { + // Write some deps to the file and grab its size. + int file_size; + { + State state; + DepsLog log; + string err; + ASSERT_TRUE(log.OpenForWrite(kTestFilename, &err)); + ASSERT_EQ("", err); + + vector deps; + deps.push_back(state.GetNode("foo.h")); + deps.push_back(state.GetNode("bar.h")); + log.RecordDeps(state.GetNode("out.o"), 1, deps); + log.Close(); + + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + file_size = (int)st.st_size; + ASSERT_GT(file_size, 0); + } + + // Now reload the file, and add slighly different deps. + int file_size_2; + { + State state; + DepsLog log; + string err; + ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); + + ASSERT_TRUE(log.OpenForWrite(kTestFilename, &err)); + ASSERT_EQ("", err); + + vector deps; + deps.push_back(state.GetNode("foo.h")); + log.RecordDeps(state.GetNode("out.o"), 1, deps); + log.Close(); + + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + file_size_2 = (int)st.st_size; + // The file should grow to record the new deps. + ASSERT_GT(file_size_2, file_size); + } + + // Now reload the file, verify the new deps have replaced the old, then + // recompact. + { + State state; + DepsLog log; + string err; + ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); + + DepsLog::Deps* deps = log.GetDeps(state.GetNode("out.o")); + ASSERT_TRUE(deps); + ASSERT_EQ(1, deps->mtime); + ASSERT_EQ(1, deps->node_count); + ASSERT_EQ("foo.h", deps->nodes[0]->path()); + + ASSERT_TRUE(log.Recompact(kTestFilename, &err)); + + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + int file_size_3 = (int)st.st_size; + // The file should have shrunk a bit for the smaller deps. + ASSERT_LT(file_size_3, file_size_2); + } +} + } // anonymous namespace -- cgit v0.12 From e223be80b6d308d64674ec884df461fe5b8e42e6 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 8 Jan 2013 17:52:44 -0800 Subject: depslog: track dead record count --- src/deps_log.cc | 4 +++- src/deps_log.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 5031515..b1ec009 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -171,8 +171,10 @@ bool DepsLog::Load(const string& path, State* state, string* err) { if (out_id >= (int)deps_.size()) deps_.resize(out_id + 1); - if (deps_[out_id]) + if (deps_[out_id]) { + ++dead_record_count_; delete deps_[out_id]; + } deps_[out_id] = deps; } else { StringPiece path(buf, size); diff --git a/src/deps_log.h b/src/deps_log.h index d763a1d..e32a6fe 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -61,6 +61,7 @@ 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() : dead_record_count_(0) {} // Writing (build-time) interface. bool OpenForWrite(const string& path, string* err); @@ -89,6 +90,10 @@ struct DepsLog { // Write a node name record, assigning it an id. bool RecordId(Node* node); + /// Number of deps record read while loading the file that ended up + /// being unused (due to a latter entry superceding it). + int dead_record_count_; + FILE* file_; /// Maps id -> Node. vector nodes_; -- cgit v0.12 From c3c1b3fda8b415e917b2762526c0c86e099b4300 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 14 Jan 2013 18:00:52 -0800 Subject: remove depfiles files as they're parsed --- src/build.cc | 8 ++++---- src/build_test.cc | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/build.cc b/src/build.cc index f648584..0d819b2 100644 --- a/src/build.cc +++ b/src/build.cc @@ -879,6 +879,8 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, string content = disk_interface_->ReadFile(depfile, err); if (!err->empty()) return false; + if (content.empty()) + return true; DepfileParser deps; if (!deps.Parse(&content, err)) @@ -893,12 +895,10 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, deps_nodes->push_back(state_->GetNode(*i)); } - /* TODO: unlink the file via diskinterface. - if (unlink(depfile.c_str()) < 0) { - *err = string("unlink: ")) + strerror(errno); + if (disk_interface_->RemoveFile(depfile) < 0) { + *err = string("deleting depfile: ") + strerror(errno); return false; } - */ #endif return true; diff --git a/src/build_test.cc b/src/build_test.cc index 3617439..bcd4d2e 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -762,6 +762,9 @@ TEST_F(BuildTest, OrderOnlyDeps) { fs_.Tick(); + // Recreate the depfile, as it should have been deleted by the build. + fs_.Create("foo.o.d", "foo.o: blah.h bar.h\n"); + // implicit dep dirty, expect a rebuild. fs_.Create("blah.h", ""); fs_.Create("bar.h", ""); @@ -774,6 +777,9 @@ TEST_F(BuildTest, OrderOnlyDeps) { fs_.Tick(); + // Recreate the depfile, as it should have been deleted by the build. + fs_.Create("foo.o.d", "foo.o: blah.h bar.h\n"); + // order only dep dirty, no rebuild. fs_.Create("otherfile", ""); command_runner_.commands_ran_.clear(); -- cgit v0.12 From 317ee87f86119556f4a3e4f0150d40c594f3581a Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sat, 16 Feb 2013 14:34:49 -0800 Subject: missing header --- src/deps_log.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/deps_log.cc b/src/deps_log.cc index b1ec009..78eb23f 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include "graph.h" #include "metrics.h" -- cgit v0.12 From a8d7d8163a0e022c838a830e6c093ba428c10f24 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 17 Feb 2013 12:09:54 -0800 Subject: rename "special" to "deps" --- configure.py | 2 +- misc/ninja_syntax.py | 6 +++--- src/graph.cc | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/configure.py b/configure.py index eddf248..eaf67c7 100755 --- a/configure.py +++ b/configure.py @@ -191,7 +191,7 @@ if platform == 'windows': command='$cxx /showIncludes $cflags -c $in /Fo$out', depfile='$out.d', description='CXX $out', - special='msvc') + deps='msvc') else: n.rule('cxx', command='$cxx -MMD -MT $out -MF $out.d $cflags -c $in -o $out', diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index 519330e..d69e3e4 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -38,7 +38,7 @@ class Writer(object): def rule(self, name, command, description=None, depfile=None, generator=False, pool=None, restat=False, rspfile=None, - rspfile_content=None, special=None): + rspfile_content=None, deps=None): self._line('rule %s' % name) self.variable('command', command, indent=1) if description: @@ -55,8 +55,8 @@ class Writer(object): self.variable('rspfile', rspfile, indent=1) if rspfile_content: self.variable('rspfile_content', rspfile_content, indent=1) - if special: - self.variable('special', special, indent=1) + if deps: + self.variable('deps', deps, indent=1) def build(self, outputs, rule, inputs=None, implicit=None, order_only=None, variables=None): diff --git a/src/graph.cc b/src/graph.cc index bb99bcb..b11973f 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -49,12 +49,12 @@ bool Rule::IsReservedBinding(const string& var) { return var == "command" || var == "depfile" || var == "description" || + var == "deps" || var == "generator" || var == "pool" || var == "restat" || var == "rspfile" || - var == "rspfile_content" || - var == "special"; + var == "rspfile_content"; } bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { @@ -323,8 +323,8 @@ void Node::Dump(const char* prefix) const { } bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { - string special = edge->GetBinding("special"); - if (!special.empty()) { + string deps_type = edge->GetBinding("deps"); + if (!deps_type.empty()) { if (!LoadDepsFromLog(edge, err)) { if (!err->empty()) return false; -- cgit v0.12 From 98d937903a4fe43e874a10b7e78c76cec8da48d8 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 17 Feb 2013 12:53:40 -0800 Subject: hook up depslog writing into build process --- src/build.cc | 216 +++++++++++++++++++++++++++++++++-------------------------- src/build.h | 7 +- src/graph.h | 8 +++ 3 files changed, 134 insertions(+), 97 deletions(-) diff --git a/src/build.cc b/src/build.cc index 0d819b2..df208af 100644 --- a/src/build.cc +++ b/src/build.cc @@ -34,6 +34,7 @@ #include "build_log.h" #include "depfile_parser.h" +#include "deps_log.h" #include "disk_interface.h" #include "graph.h" #include "msvc_helper.h" @@ -705,10 +706,11 @@ bool Builder::Build(string* err) { return false; } - if (edge->is_phony()) - FinishEdge(edge, true, ""); - else + if (edge->is_phony()) { + plan_.EdgeFinished(edge); + } else { ++pending_commands; + } // We made some progress; go back to the main loop. continue; @@ -725,22 +727,10 @@ bool Builder::Build(string* err) { return false; } - bool success = (result.status == ExitSuccess); - - if (success) { - vector deps_nodes; - string extract_err; - if (!ExtractDeps(&result, &deps_nodes, &extract_err)) { - if (!result.output.empty()) - result.output.append("\n"); - result.output.append(extract_err); - success = false; - } - } - --pending_commands; - FinishEdge(result.edge, success, result.output); - if (!success) { + FinishCommand(&result); + + if (!result.success()) { if (failures_allowed) failures_allowed--; } @@ -801,105 +791,143 @@ bool Builder::StartEdge(Edge* edge, string* err) { return true; } -void Builder::FinishEdge(Edge* edge, bool success, const string& output) { - METRIC_RECORD("FinishEdge"); - TimeStamp restat_mtime = 0; +void Builder::FinishCommand(CommandRunner::Result* result) { + METRIC_RECORD("FinishCommand"); - if (success) { - if (edge->GetBindingBool("restat") && !config_.dry_run) { - bool node_cleaned = false; - - for (vector::iterator i = edge->outputs_.begin(); - i != edge->outputs_.end(); ++i) { - TimeStamp new_mtime = disk_interface_->Stat((*i)->path()); - if ((*i)->mtime() == new_mtime) { - // The rule command did not change the output. Propagate the clean - // state through the build graph. - // Note that this also applies to nonexistent outputs (mtime == 0). - plan_.CleanNode(&scan_, *i); - node_cleaned = true; - } + Edge* edge = result->edge; + + // First try to extract dependencies from the result, if any. + // This must happen first as it filters the command output and + // can fail, which makes the command fail from a build perspective. + + vector deps_nodes; + if (result->success()) { + string deps_type = edge->GetBinding("deps"); + if (!deps_type.empty()) { + string extract_err; + if (!ExtractDeps(result, deps_type, &deps_nodes, &extract_err)) { + if (!result->output.empty()) + result->output.append("\n"); + result->output.append(extract_err); + result->status = ExitFailure; } + } + } - if (node_cleaned) { - // If any output was cleaned, find the most recent mtime of any - // (existing) non-order-only input or the depfile. - for (vector::iterator i = edge->inputs_.begin(); - i != edge->inputs_.end() - edge->order_only_deps_; ++i) { - TimeStamp input_mtime = disk_interface_->Stat((*i)->path()); - if (input_mtime > restat_mtime) - restat_mtime = input_mtime; - } + int start_time, end_time; + status_->BuildEdgeFinished(edge, result->success(), result->output, + &start_time, &end_time); - string depfile = edge->GetBinding("depfile"); - if (restat_mtime != 0 && !depfile.empty()) { - TimeStamp depfile_mtime = disk_interface_->Stat(depfile); - if (depfile_mtime > restat_mtime) - restat_mtime = depfile_mtime; - } + // The rest of this function only applies to successful commands. + if (!result->success()) + return; - // The total number of edges in the plan may have changed as a result - // of a restat. - status_->PlanHasTotalEdges(plan_.command_edge_count()); + // Restat the edge outputs, if necessary. + TimeStamp restat_mtime = 0; + if (edge->GetBindingBool("restat") && !config_.dry_run) { + bool node_cleaned = false; + + for (vector::iterator i = edge->outputs_.begin(); + i != edge->outputs_.end(); ++i) { + TimeStamp new_mtime = disk_interface_->Stat((*i)->path()); + if ((*i)->mtime() == new_mtime) { + // The rule command did not change the output. Propagate the clean + // state through the build graph. + // Note that this also applies to nonexistent outputs (mtime == 0). + plan_.CleanNode(&scan_, *i); + node_cleaned = true; } } - // Delete the response file on success (if exists) - string rspfile = edge->GetBinding("rspfile"); - if (!rspfile.empty()) - disk_interface_->RemoveFile(rspfile); + if (node_cleaned) { + // If any output was cleaned, find the most recent mtime of any + // (existing) non-order-only input or the depfile. + for (vector::iterator i = edge->inputs_.begin(); + i != edge->inputs_.end() - edge->order_only_deps_; ++i) { + TimeStamp input_mtime = disk_interface_->Stat((*i)->path()); + if (input_mtime > restat_mtime) + restat_mtime = input_mtime; + } + + string depfile = edge->GetBinding("depfile"); + if (restat_mtime != 0 && !depfile.empty()) { + TimeStamp depfile_mtime = disk_interface_->Stat(depfile); + if (depfile_mtime > restat_mtime) + restat_mtime = depfile_mtime; + } - plan_.EdgeFinished(edge); + // The total number of edges in the plan may have changed as a result + // of a restat. + status_->PlanHasTotalEdges(plan_.command_edge_count()); + } } - if (edge->is_phony()) - return; + plan_.EdgeFinished(edge); + + // Delete any left over response file. + string rspfile = edge->GetBinding("rspfile"); + if (!rspfile.empty()) + disk_interface_->RemoveFile(rspfile); + + if (scan_.build_log()) { + scan_.build_log()->RecordCommand(edge, start_time, end_time, + restat_mtime); + } + + if (scan_.deps_log()) { + // XXX figure out multiple outputs. + Node* out = edge->outputs_[0]; + // XXX need to restat for restat_mtime. + scan_.deps_log()->RecordDeps(out, restat_mtime, deps_nodes); + } - int start_time, end_time; - status_->BuildEdgeFinished(edge, success, output, &start_time, &end_time); - if (success && scan_.build_log()) - scan_.build_log()->RecordCommand(edge, start_time, end_time, restat_mtime); } bool Builder::ExtractDeps(CommandRunner::Result* result, + const string& deps_type, vector* deps_nodes, string* err) { #ifdef _WIN32 - CLParser parser; - result->output = parser.Parse(result->output); - for (set::iterator i = parser.includes_.begin(); - i != parser.includes_.end(); ++i) { - deps_nodes->push_back(state_->GetNode(*i)); - } -#else - string depfile = result->edge->GetBinding("depfile"); - if (depfile.empty()) - return true; // No dependencies to load. - - string content = disk_interface_->ReadFile(depfile, err); - if (!err->empty()) - return false; - if (content.empty()) - return true; + if (deps_type == "msvc") { + CLParser parser; + result->output = parser.Parse(result->output); + for (set::iterator i = parser.includes_.begin(); + i != parser.includes_.end(); ++i) { + deps_nodes->push_back(state_->GetNode(*i)); + } + } else +#endif + if (deps_type == "gcc") { + string depfile = result->edge->GetBinding("depfile"); + if (depfile.empty()) + return true; // No dependencies to load. - DepfileParser deps; - if (!deps.Parse(&content, err)) - return false; + string content = disk_interface_->ReadFile(depfile, err); + if (!err->empty()) + return false; + if (content.empty()) + return true; - // XXX check depfile matches expected output. - deps_nodes->reserve(deps.ins_.size()); - for (vector::iterator i = deps.ins_.begin(); - i != deps.ins_.end(); ++i) { - if (!CanonicalizePath(const_cast(i->str_), &i->len_, err)) + DepfileParser deps; + if (!deps.Parse(&content, err)) return false; - deps_nodes->push_back(state_->GetNode(*i)); - } - if (disk_interface_->RemoveFile(depfile) < 0) { - *err = string("deleting depfile: ") + strerror(errno); - return false; + // XXX check depfile matches expected output. + deps_nodes->reserve(deps.ins_.size()); + for (vector::iterator i = deps.ins_.begin(); + i != deps.ins_.end(); ++i) { + if (!CanonicalizePath(const_cast(i->str_), &i->len_, err)) + return false; + deps_nodes->push_back(state_->GetNode(*i)); + } + + if (disk_interface_->RemoveFile(depfile) < 0) { + *err = string("deleting depfile: ") + strerror(errno); + return false; + } + } else { + Fatal("unknown deps type '%s'", deps_type.c_str()); } -#endif return true; } diff --git a/src/build.h b/src/build.h index e71549d..fb5fd10 100644 --- a/src/build.h +++ b/src/build.h @@ -110,6 +110,7 @@ struct CommandRunner { Edge* edge; ExitStatus status; string output; + bool success() const { return status == ExitSuccess; } }; /// Wait for a command to complete, or return false if interrupted. virtual bool WaitForCommand(Result* result) = 0; @@ -161,7 +162,7 @@ struct Builder { bool Build(string* err); bool StartEdge(Edge* edge, string* err); - void FinishEdge(Edge* edge, bool success, const string& output); + void FinishCommand(CommandRunner::Result* result); /// Used for tests. void SetBuildLog(BuildLog* log) { @@ -175,8 +176,8 @@ struct Builder { BuildStatus* status_; private: - bool ExtractDeps(CommandRunner::Result* result, vector* deps_nodes, - string* err); + bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, + vector* deps_nodes, string* err); DiskInterface* disk_interface_; DependencyScan scan_; diff --git a/src/graph.h b/src/graph.h index a7b60db..650a83e 100644 --- a/src/graph.h +++ b/src/graph.h @@ -196,6 +196,10 @@ struct ImplicitDepLoader { /// @return false on error (without filling \a err if info is just missing). bool LoadDeps(Edge* edge, string* err); + DepsLog* deps_log() const { + return deps_log_; + } + private: /// Load implicit dependencies for \a edge from a depfile attribute. /// @return false on error (without filling \a err if info is just missing). @@ -247,6 +251,10 @@ struct DependencyScan { build_log_ = log; } + DepsLog* deps_log() const { + return dep_loader_.deps_log(); + } + private: BuildLog* build_log_; DiskInterface* disk_interface_; -- cgit v0.12 From 5031940a8fa5c0c7a76a4bb7fffdb4526713ce7c Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 17 Feb 2013 13:19:53 -0800 Subject: don't call .front() on an empty vector Fixes a gcc debug-mode assertion. --- src/deps_log.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 78eb23f..454d2e5 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -55,7 +55,8 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, const vector& nodes) { - return RecordDeps(node, mtime, nodes.size(), (Node**)&nodes.front()); + return RecordDeps(node, mtime, nodes.size(), + nodes.empty() ? NULL : (Node**)&nodes.front()); } bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, -- cgit v0.12 From 149e8d8d5d2b043fa53f9f8b74fff893ea1819df Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 17 Feb 2013 15:13:35 -0800 Subject: use logged deps mtime in dirty calculation The idea here is that it's possible for a build to complete (writing its output) but then for Ninja to get interrupted before writing out the updated dependency information. In that case the mtime stored in the deps log (if any) will match the previous output, and we'll know we need to rebuild the output just to get the deps updated. --- src/build.cc | 2 +- src/graph.cc | 20 +++++++++++++++----- src/graph.h | 8 +++++--- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/build.cc b/src/build.cc index df208af..ecde4df 100644 --- a/src/build.cc +++ b/src/build.cc @@ -516,7 +516,7 @@ void Plan::CleanNode(DependencyScan* scan, Node* node) { if (!(*ni)->dirty()) continue; - if (scan->RecomputeOutputDirty(*ei, most_recent_input, + if (scan->RecomputeOutputDirty(*ei, most_recent_input, 0, command, *ni)) { (*ni)->MarkDirty(); all_outputs_clean = false; diff --git a/src/graph.cc b/src/graph.cc index b11973f..5cc491b 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -61,7 +61,8 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { bool dirty = false; edge->outputs_ready_ = true; - if (!dep_loader_.LoadDeps(edge, err)) { + TimeStamp deps_mtime = 0; + if (!dep_loader_.LoadDeps(edge, &deps_mtime, err)) { if (!err->empty()) return false; // Failed to load dependency info: rebuild to regenerate it. @@ -112,7 +113,8 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { for (vector::iterator i = edge->outputs_.begin(); i != edge->outputs_.end(); ++i) { (*i)->StatIfNecessary(disk_interface_); - if (RecomputeOutputDirty(edge, most_recent_input, command, *i)) { + if (RecomputeOutputDirty(edge, most_recent_input, deps_mtime, + command, *i)) { dirty = true; break; } @@ -141,6 +143,7 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { bool DependencyScan::RecomputeOutputDirty(Edge* edge, Node* most_recent_input, + TimeStamp deps_mtime, const string& command, Node* output) { if (edge->is_phony()) { @@ -182,6 +185,12 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge, } } + // Dirty if the output is newer than the deps. + if (deps_mtime && output->mtime() > deps_mtime) { + EXPLAIN("stored deps info out of date for for %s", output->path().c_str()); + return true; + } + // May also be dirty due to the command changing since the last build. // But if this is a generator rule, the command changing does not make us // dirty. @@ -322,10 +331,10 @@ void Node::Dump(const char* prefix) const { } } -bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { +bool ImplicitDepLoader::LoadDeps(Edge* edge, TimeStamp* mtime, string* err) { string deps_type = edge->GetBinding("deps"); if (!deps_type.empty()) { - if (!LoadDepsFromLog(edge, err)) { + if (!LoadDepsFromLog(edge, mtime, err)) { if (!err->empty()) return false; EXPLAIN("deps for %s are missing", edge->outputs_[0]->path().c_str()); @@ -396,7 +405,8 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return true; } -bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, string* err) { +bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, TimeStamp* deps_mtime, + string* err) { DepsLog::Deps* deps = deps_log_->GetDeps(edge->outputs_[0]); if (!deps) return false; diff --git a/src/graph.h b/src/graph.h index 650a83e..428ba01 100644 --- a/src/graph.h +++ b/src/graph.h @@ -192,9 +192,10 @@ struct ImplicitDepLoader { DiskInterface* disk_interface) : state_(state), disk_interface_(disk_interface), deps_log_(deps_log) {} - /// Load implicit dependencies for \a edge. + /// Load implicit dependencies for \a edge. May fill in \a mtime with + /// the timestamp of the loaded information. /// @return false on error (without filling \a err if info is just missing). - bool LoadDeps(Edge* edge, string* err); + bool LoadDeps(Edge* edge, TimeStamp* mtime, string* err); DepsLog* deps_log() const { return deps_log_; @@ -207,7 +208,7 @@ struct ImplicitDepLoader { /// Load implicit dependencies for \a edge from the DepsLog. /// @return false on error (without filling \a err if info is just missing). - bool LoadDepsFromLog(Edge* edge, string* err); + bool LoadDepsFromLog(Edge* edge, TimeStamp* mtime, string* err); /// Preallocate \a count spaces in the input array on \a edge, returning /// an iterator pointing at the first new space. @@ -242,6 +243,7 @@ struct DependencyScan { /// Recompute whether a given single output should be marked dirty. /// Returns true if so. bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input, + TimeStamp deps_mtime, const string& command, Node* output); BuildLog* build_log() const { -- cgit v0.12 From c47f2cedbb8f44182a72343d0c71f5af5196d274 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 19 Feb 2013 10:00:05 -0800 Subject: only write deps log if edge is in deps log mode --- src/build.cc | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/build.cc b/src/build.cc index ecde4df..56f2d9b 100644 --- a/src/build.cc +++ b/src/build.cc @@ -801,16 +801,14 @@ void Builder::FinishCommand(CommandRunner::Result* result) { // can fail, which makes the command fail from a build perspective. vector deps_nodes; - if (result->success()) { - string deps_type = edge->GetBinding("deps"); - if (!deps_type.empty()) { - string extract_err; - if (!ExtractDeps(result, deps_type, &deps_nodes, &extract_err)) { - if (!result->output.empty()) - result->output.append("\n"); - result->output.append(extract_err); - result->status = ExitFailure; - } + string deps_type = edge->GetBinding("deps"); + if (result->success() && !deps_type.empty()) { + string extract_err; + if (!ExtractDeps(result, deps_type, &deps_nodes, &extract_err)) { + if (!result->output.empty()) + result->output.append("\n"); + result->output.append(extract_err); + result->status = ExitFailure; } } @@ -874,7 +872,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { restat_mtime); } - if (scan_.deps_log()) { + if (!deps_type.empty() && scan_.deps_log()) { // XXX figure out multiple outputs. Node* out = edge->outputs_[0]; // XXX need to restat for restat_mtime. -- cgit v0.12 From adc4ee81443dbfae8584456c04ea1ba383da3d01 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Fri, 5 Apr 2013 10:27:22 -0700 Subject: make it an error for now to have multiple outputs with depslog --- src/build.cc | 2 +- src/manifest_parser.cc | 8 ++++++++ src/manifest_parser_test.cc | 12 ++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/build.cc b/src/build.cc index 56f2d9b..0caf6e7 100644 --- a/src/build.cc +++ b/src/build.cc @@ -873,7 +873,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { } if (!deps_type.empty() && scan_.deps_log()) { - // XXX figure out multiple outputs. + assert(edge->outputs_.size() == 1); Node* out = edge->outputs_[0]; // XXX need to restat for restat_mtime. scan_.deps_log()->RecordDeps(out, restat_mtime, deps_nodes); diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 14fca73..a581114 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -329,6 +329,14 @@ bool ManifestParser::ParseEdge(string* err) { edge->implicit_deps_ = implicit; edge->order_only_deps_ = order_only; + // Multiple outputs aren't (yet?) supported with depslog. + string deps_type = edge->GetBinding("deps"); + if (!deps_type.empty() && edge->outputs_.size() > 1) { + return lexer_.Error("multiple outputs aren't (yet?) supported by depslog; " + "bring this up on the mailing list if it affects you", + err); + } + return true; } diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 4ac093f..76d235d 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -71,6 +71,7 @@ TEST_F(ParserTest, RuleAttributes) { "rule cat\n" " command = a\n" " depfile = a\n" +" deps = a\n" " description = a\n" " generator = a\n" " restat = a\n" @@ -599,6 +600,17 @@ TEST_F(ParserTest, MultipleOutputs) { EXPECT_EQ("", err); } +TEST_F(ParserTest, MultipleOutputsWithDeps) { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n deps = gcc\n" + "build a.o b.o: cc c.cc\n", + &err)); + EXPECT_EQ("input:5: multiple outputs aren't (yet?) supported by depslog; " + "bring this up on the mailing list if it affects you\n", err); +} + TEST_F(ParserTest, SubNinja) { files_["test.ninja"] = "var = inner\n" -- cgit v0.12 From 8ec425abe38f468bc4bbb4c95d78fab3b93d2141 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Fri, 5 Apr 2013 14:08:41 -0700 Subject: add a test verifying build failure on bad deps --- src/build_test.cc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/build_test.cc b/src/build_test.cc index bcd4d2e..1907197 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1352,3 +1352,20 @@ TEST_F(BuildTest, StatusFormatReplacePlaceholder) { status_.FormatProgressStatus("[%%/s%s/t%t/r%r/u%u/f%f]")); } +TEST_F(BuildTest, FailedDepsParse) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build bad_deps.o: cat in1\n" +" deps = gcc\n" +" depfile = in1.d\n")); + + string err; + EXPECT_TRUE(builder_.AddTarget("bad_deps.o", &err)); + ASSERT_EQ("", err); + + // These deps will fail to parse, as they should only have one + // path to the left of the colon. + fs_.Create("in1.d", "XXX YYY"); + + EXPECT_FALSE(builder_.Build(&err)); + EXPECT_EQ("subcommand failed", err); +} -- cgit v0.12 From 0397155218f3d311200ec4e25786028f14c53c6a Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 8 Apr 2013 10:20:58 -0700 Subject: add a test for the "deps out of date" case It touched the various remaining XXXes in the code, hooray. --- src/build.cc | 8 +++-- src/build_test.cc | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/deps_log.cc | 6 +++- src/deps_log.h | 3 +- src/graph.cc | 2 +- src/test.cc | 6 +++- src/test.h | 6 ++++ 7 files changed, 120 insertions(+), 7 deletions(-) diff --git a/src/build.cc b/src/build.cc index 0caf6e7..2d3d9b4 100644 --- a/src/build.cc +++ b/src/build.cc @@ -872,11 +872,13 @@ void Builder::FinishCommand(CommandRunner::Result* result) { restat_mtime); } - if (!deps_type.empty() && scan_.deps_log()) { + if (!deps_type.empty()) { assert(edge->outputs_.size() == 1); Node* out = edge->outputs_[0]; - // XXX need to restat for restat_mtime. - scan_.deps_log()->RecordDeps(out, restat_mtime, deps_nodes); + TimeStamp mtime = disk_interface_->Stat(out->path()); + // XXX we could reuse the restat logic to avoid a second stat, + // but in practice we only care about the single output. + scan_.deps_log()->RecordDeps(out, mtime, deps_nodes); } } diff --git a/src/build_test.cc b/src/build_test.cc index 1907197..a227854 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -15,6 +15,7 @@ #include "build.h" #include "build_log.h" +#include "deps_log.h" #include "graph.h" #include "test.h" @@ -396,6 +397,11 @@ struct BuildTest : public StateTestWithBuiltinRules { BuildTest() : config_(MakeConfig()), command_runner_(&fs_), builder_(&state_, config_, NULL, NULL, &fs_), status_(config_) { + } + + virtual void SetUp() { + StateTestWithBuiltinRules::SetUp(); + builder_.command_runner_.reset(&command_runner_); AssertParse(&state_, "build cat1: cat in1\n" @@ -1369,3 +1375,93 @@ TEST_F(BuildTest, FailedDepsParse) { EXPECT_FALSE(builder_.Build(&err)); EXPECT_EQ("subcommand failed", err); } + +/// Tests of builds involving deps logs necessarily must span +/// multiple builds. We reuse methods on BuildTest but not the +/// builder_ it sets up, because we want pristine objects for +/// each build. +struct BuildWithDepsLogTest : public BuildTest { + BuildWithDepsLogTest() {} + + virtual void SetUp() { + BuildTest::SetUp(); + + temp_dir_.CreateAndEnter("BuildWithDepsLogTest"); + } + + virtual void TearDown() { + temp_dir_.Cleanup(); + } + + ScopedTempDir temp_dir_; + + /// Shadow parent class builder_ so we don't accidentally use it. + void* builder_; +}; + +TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { + // Don't make use of the class's built-in Builder etc. so that we + // can construct objects with the DepsLog in place. + + string err; + // Note: in1 was created by the superclass SetUp(). + const char* manifest = + "build out: cat in1\n" + " deps = gcc\n" + " depfile = in1.d\n"; + { + 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; + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + fs_.Create("in1.d", "out: in2"); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + // The deps file should have been removed. + EXPECT_EQ(0, fs_.Stat("in1.d")); + deps_log.Close(); + builder.command_runner_.release(); + } + + { + State state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + // Pretend that the build aborted before the deps were + // removed, leaving behind an obsolete .d file, but after + // the output was written. + fs_.Create("in1.d", "XXX"); + fs_.Tick(); + fs_.Create("out", ""); + + // Run the build again. + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + // We should have rebuilt the output due to the deps being + // out of date. + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + + builder.command_runner_.release(); + } +} diff --git a/src/deps_log.cc b/src/deps_log.cc index 454d2e5..5590a32 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -30,6 +30,9 @@ const char kFileSignature[] = "# ninja deps v%d\n"; const int kCurrentVersion = 1; } // anonymous namespace +DepsLog::~DepsLog() { + Close(); +} bool DepsLog::OpenForWrite(const string& path, string* err) { file_ = fopen(path.c_str(), "ab"); @@ -113,7 +116,8 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, } void DepsLog::Close() { - fclose(file_); + if (file_) + fclose(file_); file_ = NULL; } diff --git a/src/deps_log.h b/src/deps_log.h index e32a6fe..99b006e 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -61,7 +61,8 @@ 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() : dead_record_count_(0) {} + DepsLog() : dead_record_count_(0), file_(NULL) {} + ~DepsLog(); // Writing (build-time) interface. bool OpenForWrite(const string& path, string* err); diff --git a/src/graph.cc b/src/graph.cc index 5cc491b..2614882 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -411,7 +411,7 @@ bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, TimeStamp* deps_mtime, if (!deps) return false; - // XXX mtime + *deps_mtime = deps->mtime; vector::iterator implicit_dep = PreallocateSpace(edge, deps->node_count); diff --git a/src/test.cc b/src/test.cc index c48ca82..45a9226 100644 --- a/src/test.cc +++ b/src/test.cc @@ -74,7 +74,11 @@ string GetSystemTempDir() { } // anonymous namespace StateTestWithBuiltinRules::StateTestWithBuiltinRules() { - AssertParse(&state_, + AddCatRule(&state_); +} + +void StateTestWithBuiltinRules::AddCatRule(State* state) { + AssertParse(state, "rule cat\n" " command = cat $in > $out\n"); } diff --git a/src/test.h b/src/test.h index bff0e5a..9f29e07 100644 --- a/src/test.h +++ b/src/test.h @@ -29,6 +29,12 @@ struct Node; /// builtin "cat" rule. struct StateTestWithBuiltinRules : public testing::Test { StateTestWithBuiltinRules(); + + /// Add a "cat" rule to \a state. Used by some tests; it's + /// otherwise done by the ctor to state_. + void AddCatRule(State* state); + + /// Short way to get a Node by its path from state_. Node* GetNode(const string& path); State state_; -- cgit v0.12 From ba1642091f67f3e71da1fe2768dd54146beb9c63 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 8 Apr 2013 21:08:19 -0700 Subject: Make deps=gcc without depfile an error. When I first played with depslog, I accidentally removed the depfile attribute on my cc edges, which had the effect of ninja silently ignoring all depfiles. Instead, let ninja complain about edges that have deps set to gcc and depfile set to nothing. This is done at edge build time, instead of at mainfest load time, because adding this check to ParseEdge() regressed empty build time for target 'chrome' by 30ms. The check is only useful for generator authors, regular users should never see this. --- src/build.cc | 10 ++++++---- src/build_test.cc | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/build.cc b/src/build.cc index 2d3d9b4..5e874df 100644 --- a/src/build.cc +++ b/src/build.cc @@ -873,7 +873,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { } if (!deps_type.empty()) { - assert(edge->outputs_.size() == 1); + assert(edge->outputs_.size() == 1 && "should have been rejected by parser"); Node* out = edge->outputs_[0]; TimeStamp mtime = disk_interface_->Stat(out->path()); // XXX we could reuse the restat logic to avoid a second stat, @@ -899,8 +899,10 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, #endif if (deps_type == "gcc") { string depfile = result->edge->GetBinding("depfile"); - if (depfile.empty()) - return true; // No dependencies to load. + if (depfile.empty()) { + *err = string("edge with deps=gcc but no depfile makes no sense\n"); + return false; + } string content = disk_interface_->ReadFile(depfile, err); if (!err->empty()) @@ -922,7 +924,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, } if (disk_interface_->RemoveFile(depfile) < 0) { - *err = string("deleting depfile: ") + strerror(errno); + *err = string("deleting depfile: ") + strerror(errno) + string("\n"); return false; } } else { diff --git a/src/build_test.cc b/src/build_test.cc index a227854..7df742f 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1353,6 +1353,24 @@ TEST_F(BuildTest, PhonyWithNoInputs) { ASSERT_EQ(1u, command_runner_.commands_ran_.size()); } +TEST_F(BuildTest, DepsGccWithEmptyDeps) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule cc\n" +" command = cc\n" +" deps = gcc\n" +"build out: cc\n")); + Dirty("out"); + + string err; + EXPECT_TRUE(builder_.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_FALSE(builder_.AlreadyUpToDate()); + + EXPECT_FALSE(builder_.Build(&err)); + ASSERT_EQ("subcommand failed", err); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); +} + TEST_F(BuildTest, StatusFormatReplacePlaceholder) { EXPECT_EQ("[%/s0/t0/r0/u0/f0]", status_.FormatProgressStatus("[%%/s%s/t%t/r%r/u%u/f%f]")); -- cgit v0.12 From c78e1eea9a385340a1d47d5402f5a7c41f99c95d Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 8 Apr 2013 17:18:30 -0700 Subject: Write the depslog version in binary instead of text. This way, it doubles as a byte-order marker. The header is now exactly one line in a hex editor, and it's still relatively easy to look at the version in a text editor. --- src/deps_log.cc | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 5590a32..8946e32 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -25,10 +25,10 @@ #include "state.h" #include "util.h" -namespace { -const char kFileSignature[] = "# ninja deps v%d\n"; +// The version is stored as 4 bytes after the signature and also serves as a +// byte order mark. Signature and version combined are 16 bytes long. +const char kFileSignature[] = "# ninjadeps\n"; const int kCurrentVersion = 1; -} // anonymous namespace DepsLog::~DepsLog() { Close(); @@ -47,7 +47,11 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { fseek(file_, 0, SEEK_END); if (ftell(file_) == 0) { - if (fprintf(file_, kFileSignature, kCurrentVersion) < 0) { + if (fwrite(kFileSignature, sizeof(kFileSignature) - 1, 1, file_) < 1) { + *err = strerror(errno); + return false; + } + if (fwrite(&kCurrentVersion, 4, 1, file_) < 1) { *err = strerror(errno); return false; } @@ -137,7 +141,10 @@ bool DepsLog::Load(const string& path, State* state, string* err) { return false; } int version = 0; - sscanf(buf, kFileSignature, &version); + if (fread(&version, 4, 1, f) < 1) { + *err = strerror(errno); + return false; + } if (version != kCurrentVersion) { *err = "bad deps log signature or version; starting over"; fclose(f); -- cgit v0.12 From 0a2fc151976277d8c0319cdc4ee3b1932b673d91 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 8 Apr 2013 21:43:49 -0700 Subject: add a straightforward deps log test, fix the other one The first test I wrote was wrong; write a simpler test that exercises the "no failures" code paths, then fix the second test and the bugs it exposed. --- src/build.cc | 23 ++++++++++---- src/build.h | 3 +- src/build_test.cc | 91 +++++++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 100 insertions(+), 17 deletions(-) diff --git a/src/build.cc b/src/build.cc index 5e874df..7de4b16 100644 --- a/src/build.cc +++ b/src/build.cc @@ -801,10 +801,12 @@ void Builder::FinishCommand(CommandRunner::Result* result) { // can fail, which makes the command fail from a build perspective. vector deps_nodes; + TimeStamp deps_mtime = 0; string deps_type = edge->GetBinding("deps"); if (result->success() && !deps_type.empty()) { string extract_err; - if (!ExtractDeps(result, deps_type, &deps_nodes, &extract_err)) { + if (!ExtractDeps(result, deps_type, &deps_nodes, &deps_mtime, + &extract_err)) { if (!result->output.empty()) result->output.append("\n"); result->output.append(extract_err); @@ -875,10 +877,12 @@ void Builder::FinishCommand(CommandRunner::Result* result) { if (!deps_type.empty()) { assert(edge->outputs_.size() == 1 && "should have been rejected by parser"); Node* out = edge->outputs_[0]; - TimeStamp mtime = disk_interface_->Stat(out->path()); - // XXX we could reuse the restat logic to avoid a second stat, - // but in practice we only care about the single output. - scan_.deps_log()->RecordDeps(out, mtime, deps_nodes); + if (!deps_mtime) { + // On Windows there's no separate file to compare against; just reuse + // the output's mtime. + deps_mtime = disk_interface_->Stat(out->path()); + } + scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes); } } @@ -886,6 +890,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { bool Builder::ExtractDeps(CommandRunner::Result* result, const string& deps_type, vector* deps_nodes, + TimeStamp* deps_mtime, string* err) { #ifdef _WIN32 if (deps_type == "msvc") { @@ -900,7 +905,13 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, if (deps_type == "gcc") { string depfile = result->edge->GetBinding("depfile"); if (depfile.empty()) { - *err = string("edge with deps=gcc but no depfile makes no sense\n"); + *err = string("edge with deps=gcc but no depfile makes no sense"); + return false; + } + + *deps_mtime = disk_interface_->Stat(depfile); + if (*deps_mtime <= 0) { + *err = string("unable to read depfile"); return false; } diff --git a/src/build.h b/src/build.h index fb5fd10..d5f01cd 100644 --- a/src/build.h +++ b/src/build.h @@ -177,7 +177,8 @@ struct Builder { private: bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, - vector* deps_nodes, string* err); + vector* deps_nodes, TimeStamp* deps_mtime, + string* err); DiskInterface* disk_interface_; DependencyScan scan_; diff --git a/src/build_test.cc b/src/build_test.cc index 7df742f..2a0fa0f 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1388,7 +1388,7 @@ TEST_F(BuildTest, FailedDepsParse) { // These deps will fail to parse, as they should only have one // path to the left of the colon. - fs_.Create("in1.d", "XXX YYY"); + fs_.Create("in1.d", "AAA BBB"); EXPECT_FALSE(builder_.Build(&err)); EXPECT_EQ("subcommand failed", err); @@ -1417,10 +1417,8 @@ struct BuildWithDepsLogTest : public BuildTest { void* builder_; }; -TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { - // Don't make use of the class's built-in Builder etc. so that we - // can construct objects with the DepsLog in place. - +/// Run a straightforwad build where the deps log is used. +TEST_F(BuildWithDepsLogTest, Straightforward) { string err; // Note: in1 was created by the superclass SetUp(). const char* manifest = @@ -1447,6 +1445,8 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { // The deps file should have been removed. EXPECT_EQ(0, fs_.Stat("in1.d")); + // Recreate it for the next step. + fs_.Create("in1.d", "out: in2"); deps_log.Close(); builder.command_runner_.release(); } @@ -1456,12 +1456,9 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); - // Pretend that the build aborted before the deps were - // removed, leaving behind an obsolete .d file, but after - // the output was written. - fs_.Create("in1.d", "XXX"); + // Touch the file only mentioned in the deps. fs_.Tick(); - fs_.Create("out", ""); + fs_.Create("in2", ""); // Run the build again. DepsLog deps_log; @@ -1476,6 +1473,80 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { EXPECT_TRUE(builder.Build(&err)); EXPECT_EQ("", err); + // We should have rebuilt the output due to in2 being + // out of date. + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + + builder.command_runner_.release(); + } +} + +/// Verify that obsolete deps still cause a rebuild. +TEST_F(BuildWithDepsLogTest, ObsoleteDeps) { + string err; + // Note: in1 was created by the superclass SetUp(). + const char* manifest = + "build out: cat in1\n" + " deps = gcc\n" + " depfile = in1.d\n"; + { + // Create the obsolete deps, then run a build to incorporate them. + // The idea is that the inputs/outputs are newer than the logged + // deps. + fs_.Create("in1.d", "out: "); + fs_.Tick(); + + fs_.Create("in1", ""); + + 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; + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + fs_.Create("out", ""); + // The deps file should have been removed. + EXPECT_EQ(0, fs_.Stat("in1.d")); + deps_log.Close(); + builder.command_runner_.release(); + } + + // Now we should be in a situation where in1/out2 both have recent + // timestamps but the deps are old. Verify we rebuild. + fs_.Tick(); + + { + State state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + + // Recreate the deps here just to prove the old recorded deps are + // the problem. + fs_.Create("in1.d", "out: "); + + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + // We should have rebuilt the output due to the deps being // out of date. EXPECT_EQ(1u, command_runner_.commands_ran_.size()); -- cgit v0.12