Skip to content

Commit

Permalink
Add an opt-in flag to make duplicate edges an error (-w dupbuild=err).
Browse files Browse the repository at this point in the history
This is step 1 on ninja-build#931.  Duplicated edges will become an error by default in
the future.
  • Loading branch information
nico committed Mar 24, 2015
1 parent 6bac2fb commit 3b3876c
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 9 deletions.
14 changes: 10 additions & 4 deletions src/manifest_parser.cc
Expand Up @@ -24,8 +24,10 @@
#include "util.h"
#include "version.h"

ManifestParser::ManifestParser(State* state, FileReader* file_reader)
: state_(state), file_reader_(file_reader), quiet_(false) {
ManifestParser::ManifestParser(State* state, FileReader* file_reader,
bool dupe_edge_should_err)
: state_(state), file_reader_(file_reader),
dupe_edge_should_err_(dupe_edge_should_err), quiet_(false) {
env_ = &state->bindings_;
}

Expand Down Expand Up @@ -329,10 +331,14 @@ bool ManifestParser::ParseEdge(string* err) {
if (!CanonicalizePath(&path, &slash_bits, &path_err))
return lexer_.Error(path_err, err);
if (!state_->AddOut(edge, path, slash_bits)) {
if (!quiet_) {
if (dupe_edge_should_err_) {
lexer_.Error("multiple rules generate " + path + " [-w dupbuild=err]",
err);
return false;
} else if (!quiet_) {
Warning("multiple rules generate %s. "
"builds involving this target will not be correct; "
"continuing anyway",
"continuing anyway [-w dupbuild=warn]",
path.c_str());
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/manifest_parser.h
Expand Up @@ -32,10 +32,11 @@ struct ManifestParser {
virtual bool ReadFile(const string& path, string* content, string* err) = 0;
};

ManifestParser(State* state, FileReader* file_reader);
ManifestParser(State* state, FileReader* file_reader,
bool dupe_edge_should_err = false);

/// Load and parse a file.
bool Load(const string& filename, string* err, Lexer* parent=NULL);
bool Load(const string& filename, string* err, Lexer* parent = NULL);

/// Parse a text string of input. Used by tests.
bool ParseTest(const string& input, string* err) {
Expand Down Expand Up @@ -65,6 +66,7 @@ struct ManifestParser {
BindingEnv* env_;
FileReader* file_reader_;
Lexer lexer_;
bool dupe_edge_should_err_;
bool quiet_;
};

Expand Down
13 changes: 13 additions & 0 deletions src/manifest_parser_test.cc
Expand Up @@ -364,6 +364,19 @@ TEST_F(ParserTest, NoDeadPointerFromDuplicateEdge) {
// That's all the checking that this test needs.
}

TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputsError) {
const char kInput[] =
"rule cat\n"
" command = cat $in > $out\n"
"build out1 out2: cat in1\n"
"build out1: cat in2\n"
"build final: cat out1\n";
ManifestParser parser(&state, this, /*dupe_edges_should_err=*/true);
string err;
EXPECT_FALSE(parser.ParseTest(kInput, &err));
EXPECT_EQ("input:5: multiple rules generate out1 [-w dupbuild=err]\n", err);
}

TEST_F(ParserTest, ReservedWords) {
ASSERT_NO_FATAL_FAILURE(AssertParse(
"rule build\n"
Expand Down
41 changes: 38 additions & 3 deletions src/ninja.cc
Expand Up @@ -64,6 +64,9 @@ struct Options {

/// Tool to run rather than building.
const Tool* tool;

/// Whether duplicate rules for one target should warn or print an error.
bool dupe_edges_should_err;
};

/// The Ninja main() loads up a series of data structures; various tools need
Expand Down Expand Up @@ -199,7 +202,8 @@ void Usage(const BuildConfig& config) {
"\n"
" -d MODE enable debugging (use -d list to list modes)\n"
" -t TOOL run a subtool (use -t list to list subtools)\n"
" terminates toplevel options; further flags are passed to the tool\n",
" terminates toplevel options; further flags are passed to the tool\n"
" -w FLAG adjust warnings (use -w list to list warnings)\n",
kNinjaVersion, config.parallelism);
}

Expand Down Expand Up @@ -792,6 +796,32 @@ bool DebugEnable(const string& name) {
}
}

/// Set a warning flag. Returns false if Ninja should exit instead of
/// continuing.
bool WarningEnable(const string& name, Options* options) {
if (name == "list") {
printf("warning flags:\n"
" dupbuild=[err|warn] multiple build lines for one target\n");
return false;
} else if (name == "dupbuild=err") {
options->dupe_edges_should_err = true;
return true;
} else if (name == "dupbuild=warn") {
options->dupe_edges_should_err = false;
return true;
} else {
const char* suggestion =
SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn", NULL);
if (suggestion) {
Error("unknown warning flag '%s', did you mean '%s'?",
name.c_str(), suggestion);
} else {
Error("unknown warning flag '%s'", name.c_str());
}
return false;
}
}

bool NinjaMain::OpenBuildLog(bool recompact_only) {
string log_path = ".ninja_log";
if (!build_dir_.empty())
Expand Down Expand Up @@ -962,7 +992,7 @@ int ReadFlags(int* argc, char*** argv,

int opt;
while (!options->tool &&
(opt = getopt_long(*argc, *argv, "d:f:j:k:l:nt:vC:h", kLongOptions,
(opt = getopt_long(*argc, *argv, "d:f:j:k:l:nt:vw:C:h", kLongOptions,
NULL)) != -1) {
switch (opt) {
case 'd':
Expand Down Expand Up @@ -1011,6 +1041,10 @@ int ReadFlags(int* argc, char*** argv,
case 'v':
config->verbosity = BuildConfig::VERBOSE;
break;
case 'w':
if (!WarningEnable(optarg, options))
return 1;
break;
case 'C':
options->working_dir = optarg;
break;
Expand Down Expand Up @@ -1067,7 +1101,8 @@ int real_main(int argc, char** argv) {
NinjaMain ninja(ninja_command, config);

RealFileReader file_reader;
ManifestParser parser(&ninja.state_, &file_reader);
ManifestParser parser(&ninja.state_, &file_reader,
options.dupe_edges_should_err);
string err;
if (!parser.Load(options.input_file, &err)) {
Error("%s", err.c_str());
Expand Down

0 comments on commit 3b3876c

Please sign in to comment.