Commit e953738c authored by Brett Wilson's avatar Brett Wilson Committed by Commit Bot

Clarify GN nonfatal unused build arg messages.

Changes the messaging for nonfatal "unused build arg" messages to be
"WARNING" instead of "ERROR" when they are nonfatal and clarify that
the build has continued. Previously it would print "ERROR" and
continue anyway which was confusing.

Add an Err object move constructor and de-inline the assignment
operator (previously implicit).

Bug: 500696
Change-Id: Id6056b7a4d33466485f9e138a890e211f447817f
Reviewed-on: https://chromium-review.googlesource.com/762086
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: default avatarWill Harris <wfh@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516320}
parent e99f912d
...@@ -248,7 +248,7 @@ bool Args::VerifyAllOverridesUsed(Err* err) const { ...@@ -248,7 +248,7 @@ bool Args::VerifyAllOverridesUsed(Err* err) const {
std::string err_help( std::string err_help(
"The variable \"" + name + "\" was set as a build argument\n" "The variable \"" + name + "\" was set as a build argument\n"
"but never appeared in a declare_args() block in any buildfile.\n\n" "but never appeared in a declare_args() block in any buildfile.\n\n"
"To view all possible args, run \"gn args --list <builddir>\""); "To view all possible args, run \"gn args --list <out_dir>\"");
// Use all declare_args for a spelling suggestion. // Use all declare_args for a spelling suggestion.
std::vector<base::StringPiece> candidates; std::vector<base::StringPiece> candidates;
......
...@@ -150,18 +150,26 @@ Err::~Err() { ...@@ -150,18 +150,26 @@ Err::~Err() {
} }
void Err::PrintToStdout() const { void Err::PrintToStdout() const {
InternalPrintToStdout(false); InternalPrintToStdout(false, true);
}
void Err::PrintNonfatalToStdout() const {
InternalPrintToStdout(false, false);
} }
void Err::AppendSubErr(const Err& err) { void Err::AppendSubErr(const Err& err) {
sub_errs_.push_back(err); sub_errs_.push_back(err);
} }
void Err::InternalPrintToStdout(bool is_sub_err) const { void Err::InternalPrintToStdout(bool is_sub_err, bool is_fatal) const {
DCHECK(has_error_); DCHECK(has_error_);
if (!is_sub_err) if (!is_sub_err) {
OutputString("ERROR ", DECORATION_RED); if (is_fatal)
OutputString("ERROR ", DECORATION_RED);
else
OutputString("WARNING ", DECORATION_RED);
}
// File name and location. // File name and location.
const InputFile* input_file = location_.file(); const InputFile* input_file = location_.file();
...@@ -191,5 +199,5 @@ void Err::InternalPrintToStdout(bool is_sub_err) const { ...@@ -191,5 +199,5 @@ void Err::InternalPrintToStdout(bool is_sub_err) const {
// Sub errors. // Sub errors.
for (const auto& sub_err : sub_errs_) for (const auto& sub_err : sub_errs_)
sub_err.InternalPrintToStdout(true); sub_err.InternalPrintToStdout(true, is_fatal);
} }
...@@ -24,7 +24,7 @@ class Value; ...@@ -24,7 +24,7 @@ class Value;
// below. They can provide additional context. // below. They can provide additional context.
class Err { class Err {
public: public:
typedef std::vector<LocationRange> RangeList; using RangeList = std::vector<LocationRange>;
// Indicates no error. // Indicates no error.
Err(); Err();
...@@ -70,8 +70,20 @@ class Err { ...@@ -70,8 +70,20 @@ class Err {
void PrintToStdout() const; void PrintToStdout() const;
// Prints to standard out but uses a "WARNING" messaging instead of the
// normal "ERROR" messaging. This is a property of the printing system rather
// than of the Err class because there is no expectation that code calling a
// function that take an Err check that the error is nonfatal and continue.
// Generally all Err objects with has_error() set are fatal.
//
// In some very specific cases code will detect a condition and print a
// nonfatal error to the screen instead of returning it. In these cases, that
// code can decide at printing time whether it will continue (and use this
// method) or not (and use PrintToStdout()).
void PrintNonfatalToStdout() const;
private: private:
void InternalPrintToStdout(bool is_sub_err) const; void InternalPrintToStdout(bool is_sub_err, bool is_fatal) const;
bool has_error_; bool has_error_;
Location location_; Location location_;
......
...@@ -371,13 +371,14 @@ bool Setup::RunPostMessageLoop() { ...@@ -371,13 +371,14 @@ bool Setup::RunPostMessageLoop() {
} }
if (!build_settings_.build_args().VerifyAllOverridesUsed(&err)) { if (!build_settings_.build_args().VerifyAllOverridesUsed(&err)) {
// TODO(brettw) implement a system to have a different marker for
// warnings. Until we have a better system, print the error but don't
// return failure unless requested on the command line.
err.PrintToStdout();
if (base::CommandLine::ForCurrentProcess()->HasSwitch( if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kFailOnUnusedArgs)) switches::kFailOnUnusedArgs)) {
err.PrintToStdout();
return false; return false;
}
err.PrintNonfatalToStdout();
OutputString("\nThe build continued as if that argument was "
"unspecified.\n\n");
return true; return true;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment