Skip to content

Commit

Permalink
Fix NINJA_STATUS %r on dumb terminals
Browse files Browse the repository at this point in the history
PR ninja-build#999 made dumb terminals only output when edges finish.  PrintStatus
is called after finished_edges_ is incremented, which means the
calculation for running edges will always return 1 less than the real
number of running processes.  This happens on smart terminals too, but
ninja will immediately print the status for the next edge with
starting_edges_ incremented, so the incorrect value is never visible.

Pass a boolean specifying whether the status is being printed on an edge
finishing, and if so count the edge that just finished as being running.
  • Loading branch information
colincross committed Apr 28, 2016
1 parent f1b5d2b commit 323f72d
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 11 deletions.
19 changes: 12 additions & 7 deletions src/build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void BuildStatus::BuildEdgeStarted(Edge* edge) {
++started_edges_;

if (edge->use_console() || printer_.is_smart_terminal())
PrintStatus(edge);
PrintStatus(edge, false);

if (edge->use_console())
printer_.SetConsoleLocked(true);
Expand Down Expand Up @@ -129,7 +129,7 @@ void BuildStatus::BuildEdgeFinished(Edge* edge,
return;

if (!edge->use_console())
PrintStatus(edge);
PrintStatus(edge, true);

// Print the command that is spewing before printing its output.
if (!success) {
Expand Down Expand Up @@ -170,7 +170,7 @@ void BuildStatus::BuildFinished() {
}

string BuildStatus::FormatProgressStatus(
const char* progress_status_format) const {
const char* progress_status_format, bool finished) const {
string out;
char buf[32];
int percent;
Expand All @@ -195,10 +195,15 @@ string BuildStatus::FormatProgressStatus(
break;

// Running edges.
case 'r':
snprintf(buf, sizeof(buf), "%d", started_edges_ - finished_edges_);
case 'r': {
int running_edges = started_edges_ - finished_edges_;
// count the edge that just finished as a running edge
if (finished)
running_edges++;
snprintf(buf, sizeof(buf), "%d", running_edges);
out += buf;
break;
}

// Unstarted edges.
case 'u':
Expand Down Expand Up @@ -252,7 +257,7 @@ string BuildStatus::FormatProgressStatus(
return out;
}

void BuildStatus::PrintStatus(Edge* edge) {
void BuildStatus::PrintStatus(Edge* edge, bool finished) {
if (config_.verbosity == BuildConfig::QUIET)
return;

Expand All @@ -262,7 +267,7 @@ void BuildStatus::PrintStatus(Edge* edge) {
if (to_print.empty() || force_full_command)
to_print = edge->GetBinding("command");

to_print = FormatProgressStatus(progress_status_format_) + to_print;
to_print = FormatProgressStatus(progress_status_format_, finished) + to_print;

printer_.Print(to_print,
force_full_command ? LinePrinter::FULL : LinePrinter::ELIDE);
Expand Down
6 changes: 4 additions & 2 deletions src/build.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,12 @@ struct BuildStatus {
/// See the user manual for more information about the available
/// placeholders.
/// @param progress_status_format The format of the progress status.
string FormatProgressStatus(const char* progress_status_format) const;
/// @param finished True if the edge being printed just finished
string FormatProgressStatus(const char* progress_status_format,
bool finished) const;

private:
void PrintStatus(Edge* edge);
void PrintStatus(Edge* edge, bool finished);

const BuildConfig& config_;

Expand Down
4 changes: 2 additions & 2 deletions src/build_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,7 @@ TEST_F(BuildWithLogTest, RestatTest) {
ASSERT_EQ("", err);
EXPECT_TRUE(builder_.Build(&err));
ASSERT_EQ("", err);
EXPECT_EQ("[3/3]", builder_.status_->FormatProgressStatus("[%s/%t]"));
EXPECT_EQ("[3/3]", builder_.status_->FormatProgressStatus("[%s/%t]", false));
command_runner_.commands_ran_.clear();
state_.Reset();

Expand Down Expand Up @@ -1726,7 +1726,7 @@ TEST_F(BuildTest, DepsGccWithEmptyDepfileErrorsOut) {

TEST_F(BuildTest, StatusFormatReplacePlaceholder) {
EXPECT_EQ("[%/s0/t0/r0/u0/f0]",
status_.FormatProgressStatus("[%%/s%s/t%t/r%r/u%u/f%f]"));
status_.FormatProgressStatus("[%%/s%s/t%t/r%r/u%u/f%f]", false));
}

TEST_F(BuildTest, FailedDepsParse) {
Expand Down

0 comments on commit 323f72d

Please sign in to comment.