Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

rearrange handling of builtin bindings to make rules simpler

Now, a 'build' block can override any special binding like 'command'
or 'description' if it needs to.
  • Loading branch information...
commit 13dd08c1a03e5a8f4299816fbd3af1b6cb6d9642 1 parent 3249938
@martine authored
View
38 src/build.cc
@@ -256,9 +256,9 @@ void BuildStatus::PrintStatus(Edge* edge) {
bool force_full_command = config_.verbosity == BuildConfig::VERBOSE;
- string to_print = edge->GetDescription();
+ string to_print = edge->GetBinding("description");
if (to_print.empty() || force_full_command)
- to_print = edge->EvaluateCommand();
+ to_print = edge->GetBinding("command");
#ifdef _WIN32
CONSOLE_SCREEN_BUFFER_INFO csbi;
@@ -612,7 +612,7 @@ void Builder::Cleanup() {
for (vector<Edge*>::iterator i = active_edges.begin();
i != active_edges.end(); ++i) {
- bool has_depfile = !(*i)->rule_->depfile().empty();
+ string depfile = (*i)->GetBinding("depfile");
for (vector<Node*>::iterator ni = (*i)->outputs_.begin();
ni != (*i)->outputs_.end(); ++ni) {
// Only delete this output if it was actually modified. This is
@@ -622,12 +622,13 @@ void Builder::Cleanup() {
// need to rebuild an output because of a modified header file
// mentioned in a depfile, and the command touches its depfile
// but is interrupted before it touches its output file.)
- if (has_depfile ||
- (*ni)->mtime() != disk_interface_->Stat((*ni)->path()))
+ if (!depfile.empty() ||
+ (*ni)->mtime() != disk_interface_->Stat((*ni)->path())) {
disk_interface_->RemoveFile((*ni)->path());
+ }
}
- if (has_depfile)
- disk_interface_->RemoveFile((*i)->EvaluateDepFile());
+ if (!depfile.empty())
+ disk_interface_->RemoveFile(depfile);
}
}
}
@@ -771,11 +772,11 @@ bool Builder::StartEdge(Edge* edge, string* err) {
// Create response file, if needed
// XXX: this may also block; do we care?
- if (edge->HasRspFile()) {
- if (!disk_interface_->WriteFile(edge->GetRspFile(),
- edge->GetRspFileContent())) {
+ string rspfile = edge->GetBinding("rspfile");
+ if (!rspfile.empty()) {
+ string content = edge->GetBinding("rspfile_content");
+ if (!disk_interface_->WriteFile(rspfile, content))
return false;
- }
}
// start command computing and run it
@@ -792,7 +793,7 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
TimeStamp restat_mtime = 0;
if (success) {
- if (edge->rule().restat() && !config_.dry_run) {
+ if (edge->GetBindingBool("restat") && !config_.dry_run) {
bool node_cleaned = false;
for (vector<Node*>::iterator i = edge->outputs_.begin();
@@ -817,9 +818,9 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
restat_mtime = input_mtime;
}
- if (restat_mtime != 0 && !edge->rule().depfile().empty()) {
- TimeStamp depfile_mtime =
- disk_interface_->Stat(edge->EvaluateDepFile());
+ 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;
}
@@ -830,9 +831,10 @@ void Builder::FinishEdge(Edge* edge, bool success, const string& output) {
}
}
- // delete the response file on success (if exists)
- if (edge->HasRspFile())
- disk_interface_->RemoveFile(edge->GetRspFile());
+ // Delete the response file on success (if exists)
+ string rspfile = edge->GetBinding("rspfile");
+ if (!rspfile.empty())
+ disk_interface_->RemoveFile(rspfile);
plan_.EdgeFinished(edge);
}
View
6 src/clean.cc
@@ -83,11 +83,11 @@ bool Cleaner::IsAlreadyRemoved(const string& path) {
}
void Cleaner::RemoveEdgeFiles(Edge* edge) {
- string depfile = edge->EvaluateDepFile();
+ string depfile = edge->GetBinding("depfile");
if (!depfile.empty())
Remove(depfile);
- string rspfile = edge->GetRspFile();
+ string rspfile = edge->GetBinding("rspfile");
if (!rspfile.empty())
Remove(rspfile);
}
@@ -117,7 +117,7 @@ int Cleaner::CleanAll(bool generator) {
if ((*e)->is_phony())
continue;
// Do not remove generator's files unless generator specified.
- if (!generator && (*e)->rule().generator())
+ if (!generator && (*e)->GetBindingBool("generator"))
continue;
for (vector<Node*>::iterator out_node = (*e)->outputs_.begin();
out_node != (*e)->outputs_.end(); ++out_node) {
View
16 src/eval_env.cc
@@ -27,6 +27,22 @@ void BindingEnv::AddBinding(const string& key, const string& val) {
bindings_[key] = val;
}
+string BindingEnv::LookupWithFallback(const string& var,
+ const EvalString* eval,
+ Env* env) {
+ map<string, string>::iterator i = bindings_.find(var);
+ if (i != bindings_.end())
+ return i->second;
+
+ if (eval)
+ return eval->Evaluate(env);
+
+ if (parent_)
+ return parent_->LookupVariable(var);
+
+ return "";
+}
+
string EvalString::Evaluate(Env* env) const {
string result;
for (TokenList::const_iterator i = parsed_.begin(); i != parsed_.end(); ++i) {
View
12 src/eval_env.h
@@ -22,6 +22,8 @@ using namespace std;
#include "string_piece.h"
+struct EvalString;
+
/// An interface for a scope for variable (e.g. "$foo") lookups.
struct Env {
virtual ~Env() {}
@@ -33,10 +35,20 @@ struct Env {
struct BindingEnv : public Env {
BindingEnv() : parent_(NULL) {}
explicit BindingEnv(Env* parent) : parent_(parent) {}
+
virtual ~BindingEnv() {}
virtual string LookupVariable(const string& var);
+
void AddBinding(const string& key, const string& val);
+ /// This is tricky. Edges want lookup scope to go in this order:
+ /// 1) value set on edge itself (edge_->env_)
+ /// 2) value set on rule, with expansion in the edge's scope
+ /// 3) value set on enclosing scope of edge (edge_->env_->parent_)
+ /// This function takes as parameters the necessary info to do (2).
+ string LookupWithFallback(const string& var, const EvalString* eval,
+ Env* env);
+
private:
map<string, string> bindings_;
Env* parent_;
View
81 src/graph.cc
@@ -32,16 +32,40 @@ bool Node::Stat(DiskInterface* disk_interface) {
return mtime_ > 0;
}
+void Rule::AddBinding(const string& key, const EvalString& val) {
+ bindings_[key] = val;
+}
+
+const EvalString* Rule::GetBinding(const string& key) const {
+ map<string, EvalString>::const_iterator i = bindings_.find(key);
+ if (i == bindings_.end())
+ return NULL;
+ return &i->second;
+}
+
+// static
+bool Rule::IsReservedBinding(const string& var) {
+ return var == "command" ||
+ var == "depfile" ||
+ var == "description" ||
+ var == "generator" ||
+ var == "pool" ||
+ var == "restat" ||
+ var == "rspfile" ||
+ var == "rspfile_content";
+}
+
bool DependencyScan::RecomputeDirty(Edge* edge, string* err) {
bool dirty = false;
edge->outputs_ready_ = true;
- if (!edge->rule_->depfile().empty()) {
- if (!LoadDepFile(edge, err)) {
+ string depfile = edge->GetBinding("depfile");
+ if (!depfile.empty()) {
+ if (!LoadDepFile(edge, depfile, err)) {
if (!err->empty())
return false;
EXPLAIN("Edge targets are dirty because depfile '%s' is missing",
- edge->EvaluateDepFile().c_str());
+ depfile.c_str());
dirty = true;
}
}
@@ -142,7 +166,7 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge,
// build log. Use that mtime instead, so that the file will only be
// considered dirty if an input was modified since the previous run.
TimeStamp most_recent_stamp = most_recent_input->mtime();
- if (edge->rule_->restat() && build_log() &&
+ if (edge->GetBindingBool("restat") && build_log() &&
(entry = build_log()->LookupByOutput(output->path()))) {
if (entry->restat_mtime < most_recent_stamp) {
EXPLAIN("restat of output %s older than most recent input %s "
@@ -162,7 +186,7 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge,
// 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.
- if (!edge->rule_->generator() && build_log()) {
+ if (!edge->GetBindingBool("generator") && build_log()) {
if (entry || (entry = build_log()->LookupByOutput(output->path()))) {
if (BuildLog::LogEntry::HashCommand(command) != entry->command_hash) {
EXPLAIN("command line changed for %s", output->path().c_str());
@@ -212,11 +236,11 @@ string EdgeEnv::LookupVariable(const string& var) {
return MakePathList(edge_->outputs_.begin(),
edge_->outputs_.end(),
' ');
- } else if (edge_->env_) {
- return edge_->env_->LookupVariable(var);
- } else {
- return string();
}
+
+ // See notes on BindingEnv::LookupWithFallback.
+ const EvalString* eval = edge_->rule_->GetBinding(var);
+ return edge_->env_->LookupWithFallback(var, eval, this);
}
string EdgeEnv::MakePathList(vector<Node*>::iterator begin,
@@ -239,40 +263,26 @@ string EdgeEnv::MakePathList(vector<Node*>::iterator begin,
}
string Edge::EvaluateCommand(bool incl_rsp_file) {
- EdgeEnv env(this);
- string command = rule_->command().Evaluate(&env);
- if (incl_rsp_file && HasRspFile())
- command += ";rspfile=" + GetRspFileContent();
+ string command = GetBinding("command");
+ if (incl_rsp_file) {
+ string rspfile_content = GetBinding("rspfile_content");
+ if (!rspfile_content.empty())
+ command += ";rspfile=" + rspfile_content;
+ }
return command;
}
-string Edge::EvaluateDepFile() {
+string Edge::GetBinding(const string& key) {
EdgeEnv env(this);
- return rule_->depfile().Evaluate(&env);
+ return env.LookupVariable(key);
}
-string Edge::GetDescription() {
- EdgeEnv env(this);
- return rule_->description().Evaluate(&env);
-}
-
-bool Edge::HasRspFile() {
- return !rule_->rspfile().empty();
-}
-
-string Edge::GetRspFile() {
- EdgeEnv env(this);
- return rule_->rspfile().Evaluate(&env);
-}
-
-string Edge::GetRspFileContent() {
- EdgeEnv env(this);
- return rule_->rspfile_content().Evaluate(&env);
+bool Edge::GetBindingBool(const string& key) {
+ return !GetBinding(key).empty();
}
-bool DependencyScan::LoadDepFile(Edge* edge, string* err) {
+bool DependencyScan::LoadDepFile(Edge* edge, const string& path, string* err) {
METRIC_RECORD("depfile load");
- string path = edge->EvaluateDepFile();
string content = disk_interface_->ReadFile(path, err);
if (!err->empty())
return false;
@@ -317,8 +327,7 @@ bool DependencyScan::LoadDepFile(Edge* edge, string* err) {
// 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,
- &State::kDefaultPool);
+ Edge* phony_edge = state_->AddEdge(&State::kPhonyRule);
node->set_in_edge(phony_edge);
phony_edge->outputs_.push_back(node);
View
43 src/graph.h
@@ -102,38 +102,23 @@ struct Node {
/// An invokable build command and associated metadata (description, etc.).
struct Rule {
- explicit Rule(const string& name)
- : name_(name), generator_(false), restat_(false) {}
+ explicit Rule(const string& name) : name_(name) {}
const string& name() const { return name_; }
- bool generator() const { return generator_; }
- bool restat() const { return restat_; }
+ typedef map<string, EvalString> Bindings;
+ void AddBinding(const string& key, const EvalString& val);
- const EvalString& command() const { return command_; }
- const EvalString& description() const { return description_; }
- const EvalString& depfile() const { return depfile_; }
- const EvalString& rspfile() const { return rspfile_; }
- const EvalString& rspfile_content() const { return rspfile_content_; }
+ static bool IsReservedBinding(const string& var);
- /// Used by a test.
- void set_command(const EvalString& command) { command_ = command; }
+ const EvalString* GetBinding(const string& key) const;
private:
// Allow the parsers to reach into this object and fill out its fields.
friend struct ManifestParser;
string name_;
-
- bool generator_;
- bool restat_;
-
- EvalString command_;
- EvalString description_;
- EvalString depfile_;
- EvalString pool_;
- EvalString rspfile_;
- EvalString rspfile_content_;
+ map<string, EvalString> bindings_;
};
struct BuildLog;
@@ -153,17 +138,9 @@ struct Edge {
/// If incl_rsp_file is enabled, the string will also contain the
/// full contents of a response file (if applicable)
string EvaluateCommand(bool incl_rsp_file = false);
- string EvaluateDepFile();
- string GetDescription();
-
- /// Does the edge use a response file?
- bool HasRspFile();
-
- /// Get the path to the response file
- string GetRspFile();
- /// Get the contents of the response file
- string GetRspFileContent();
+ string GetBinding(const string& key);
+ bool GetBindingBool(const string& key);
void Dump(const char* prefix="") const;
@@ -171,7 +148,7 @@ struct Edge {
Pool* pool_;
vector<Node*> inputs_;
vector<Node*> outputs_;
- Env* env_;
+ BindingEnv* env_;
bool outputs_ready_;
const Rule& rule() const { return *rule_; }
@@ -220,7 +197,7 @@ struct DependencyScan {
bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input,
const string& command, Node* output);
- bool LoadDepFile(Edge* edge, string* err);
+ bool LoadDepFile(Edge* edge, const string& path, string* err);
BuildLog* build_log() const {
return build_log_;
View
35 src/graph_test.cc
@@ -187,3 +187,38 @@ TEST_F(GraphTest, DepfileRemoved) {
ASSERT_EQ("", err);
EXPECT_TRUE(GetNode("out.o")->dirty());
}
+
+// Check that rule-level variables are in scope for eval.
+TEST_F(GraphTest, RuleVariablesInScope) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule r\n"
+" depfile = x\n"
+" command = depfile is $depfile\n"
+"build out: r in\n"));
+ Edge* edge = GetNode("out")->in_edge();
+ EXPECT_EQ("depfile is x", edge->EvaluateCommand());
+}
+
+// Check that build statements can override rule builtins like depfile.
+TEST_F(GraphTest, DepfileOverride) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule r\n"
+" depfile = x\n"
+" command = unused\n"
+"build out: r in\n"
+" depfile = y\n"));
+ Edge* edge = GetNode("out")->in_edge();
+ EXPECT_EQ("y", edge->GetBinding("depfile"));
+}
+
+// Check that overridden values show up in expansion of rule-level bindings.
+TEST_F(GraphTest, DepfileOverrideParent) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule r\n"
+" depfile = x\n"
+" command = depfile is $depfile\n"
+"build out: r in\n"
+" depfile = y\n"));
+ Edge* edge = GetNode("out")->in_edge();
+ EXPECT_EQ("depfile is y", edge->GetBinding("command"));
+}
View
76 src/manifest_parser.cc
@@ -154,22 +154,8 @@ bool ManifestParser::ParseRule(string* err) {
if (!ParseLet(&key, &value, err))
return false;
- if (key == "command") {
- rule->command_ = value;
- } else if (key == "depfile") {
- rule->depfile_ = value;
- } else if (key == "description") {
- rule->description_ = value;
- } else if (key == "generator") {
- rule->generator_ = true;
- } else if (key == "restat") {
- rule->restat_ = true;
- } else if (key == "rspfile") {
- rule->rspfile_ = value;
- } else if (key == "rspfile_content") {
- rule->rspfile_content_ = value;
- } else if (key == "pool") {
- rule->pool_ = value;
+ if (Rule::IsReservedBinding(key)) {
+ rule->AddBinding(key, value);
} else {
// Die on other keyvals for now; revisit if we want to add a
// scope here.
@@ -177,12 +163,13 @@ bool ManifestParser::ParseRule(string* err) {
}
}
- if (rule->rspfile_.empty() != rule->rspfile_content_.empty()) {
- return lexer_.Error("rspfile and rspfile_content need to be both specified",
- err);
+ if (rule->bindings_["rspfile"].empty() !=
+ rule->bindings_["rspfile_content"].empty()) {
+ return lexer_.Error("rspfile and rspfile_content need to be "
+ "both specified", err);
}
- if (rule->command_.empty())
+ if (rule->bindings_["command"].empty())
return lexer_.Error("expected 'command =' line", err);
state_->AddRule(rule);
@@ -296,42 +283,29 @@ bool ManifestParser::ParseEdge(string* err) {
if (!ExpectToken(Lexer::NEWLINE, err))
return false;
- // Default to using outer env.
- BindingEnv* env = env_;
- Pool* pool = NULL;
+ // XXX scoped_ptr to handle error case.
+ BindingEnv* env = new BindingEnv(env_);
- // But create and fill a nested env if there are variables in scope.
- if (lexer_.PeekToken(Lexer::INDENT)) {
- // XXX scoped_ptr to handle error case.
- env = new BindingEnv(env_);
- do {
- string key;
- EvalString val;
- if (!ParseLet(&key, &val, err))
- return false;
- if (key == "pool") {
- string pool_name = val.Evaluate(env_);
- pool = state_->LookupPool(pool_name);
- if (pool == NULL)
- return lexer_.Error("undefined pool '" + pool_name + "'", err);
- } else {
- env->AddBinding(key, val.Evaluate(env_));
- }
- } while (lexer_.PeekToken(Lexer::INDENT));
- }
+ while (lexer_.PeekToken(Lexer::INDENT)) {
+ string key;
+ EvalString val;
+ if (!ParseLet(&key, &val, err))
+ return false;
- if (pool == NULL) {
- if (!rule->pool_.empty()) {
- pool = state_->LookupPool(rule->pool_.Evaluate(env_));
- if (pool == NULL)
- return lexer_.Error("cannot resolve pool for this edge.", err);
- } else {
- pool = &State::kDefaultPool;
- }
+ env->AddBinding(key, val.Evaluate(env_));
}
- Edge* edge = state_->AddEdge(rule, pool);
+ Edge* edge = state_->AddEdge(rule);
edge->env_ = env;
+
+ string pool_name = edge->GetBinding("pool");
+ if (!pool_name.empty()) {
+ Pool* pool = state_->LookupPool(pool_name);
+ if (pool == NULL)
+ return lexer_.Error("unknown pool name", err);
+ edge->pool_ = pool;
+ }
+
for (vector<EvalString>::iterator i = ins.begin(); i != ins.end(); ++i) {
string path = i->Evaluate(env);
string path_err;
View
20 src/manifest_parser_test.cc
@@ -61,7 +61,8 @@ TEST_F(ParserTest, Rules) {
ASSERT_EQ(3u, state.rules_.size());
const Rule* rule = state.rules_.begin()->second;
EXPECT_EQ("cat", rule->name());
- EXPECT_EQ("[cat ][$in][ > ][$out]", rule->command().Serialize());
+ EXPECT_EQ("[cat ][$in][ > ][$out]",
+ rule->GetBinding("command")->Serialize());
}
TEST_F(ParserTest, RuleAttributes) {
@@ -92,8 +93,9 @@ TEST_F(ParserTest, IgnoreIndentedComments) {
ASSERT_EQ(2u, state.rules_.size());
const Rule* rule = state.rules_.begin()->second;
EXPECT_EQ("cat", rule->name());
- EXPECT_TRUE(rule->restat());
- EXPECT_FALSE(rule->generator());
+ Edge* edge = state.GetNode("result")->in_edge();
+ EXPECT_TRUE(edge->GetBindingBool("restat"));
+ EXPECT_FALSE(edge->GetBindingBool("generator"));
}
TEST_F(ParserTest, IgnoreIndentedBlankLines) {
@@ -124,9 +126,10 @@ TEST_F(ParserTest, ResponseFiles) {
ASSERT_EQ(2u, state.rules_.size());
const Rule* rule = state.rules_.begin()->second;
EXPECT_EQ("cat_rsp", rule->name());
- EXPECT_EQ("[cat ][$rspfile][ > ][$out]", rule->command().Serialize());
- EXPECT_EQ("[$rspfile]", rule->rspfile().Serialize());
- EXPECT_EQ("[$in]", rule->rspfile_content().Serialize());
+ EXPECT_EQ("[cat ][$rspfile][ > ][$out]",
+ rule->GetBinding("command")->Serialize());
+ EXPECT_EQ("[$rspfile]", rule->GetBinding("rspfile")->Serialize());
+ EXPECT_EQ("[$in]", rule->GetBinding("rspfile_content")->Serialize());
}
TEST_F(ParserTest, InNewline) {
@@ -140,7 +143,8 @@ TEST_F(ParserTest, InNewline) {
ASSERT_EQ(2u, state.rules_.size());
const Rule* rule = state.rules_.begin()->second;
EXPECT_EQ("cat_rsp", rule->name());
- EXPECT_EQ("[cat ][$in_newline][ > ][$out]", rule->command().Serialize());
+ EXPECT_EQ("[cat ][$in_newline][ > ][$out]",
+ rule->GetBinding("command")->Serialize());
Edge* edge = state.edges_[0];
EXPECT_EQ("cat in\nin2 > out", edge->EvaluateCommand());
@@ -200,7 +204,7 @@ TEST_F(ParserTest, Continuation) {
ASSERT_EQ(2u, state.rules_.size());
const Rule* rule = state.rules_.begin()->second;
EXPECT_EQ("link", rule->name());
- EXPECT_EQ("[foo bar baz]", rule->command().Serialize());
+ EXPECT_EQ("[foo bar baz]", rule->GetBinding("command")->Serialize());
}
TEST_F(ParserTest, Backslash) {
View
4 src/state.cc
@@ -91,10 +91,10 @@ Pool* State::LookupPool(const string& pool_name) {
return i->second;
}
-Edge* State::AddEdge(const Rule* rule, Pool* pool) {
+Edge* State::AddEdge(const Rule* rule) {
Edge* edge = new Edge();
edge->rule_ = rule;
- edge->pool_ = pool;
+ edge->pool_ = &State::kDefaultPool;
edge->env_ = &bindings_;
edges_.push_back(edge);
return edge;
View
2  src/state.h
@@ -92,7 +92,7 @@ struct State {
void AddPool(Pool* pool);
Pool* LookupPool(const string& pool_name);
- Edge* AddEdge(const Rule* rule, Pool* pool);
+ Edge* AddEdge(const Rule* rule);
Node* GetNode(StringPiece path);
Node* LookupNode(StringPiece path);
View
4 src/state_test.cc
@@ -29,10 +29,10 @@ TEST(State, Basic) {
command.AddSpecial("out");
Rule* rule = new Rule("cat");
- rule->set_command(command);
+ rule->AddBinding("command", command);
state.AddRule(rule);
- Edge* edge = state.AddEdge(rule, &State::kDefaultPool);
+ Edge* edge = state.AddEdge(rule);
state.AddIn(edge, "in1");
state.AddIn(edge, "in2");
state.AddOut(edge, "out");
Please sign in to comment.
Something went wrong with that request. Please try again.