Skip to content

Commit

Permalink
Propagate supbrocess' exit codes to the ninja exit code
Browse files Browse the repository at this point in the history
  • Loading branch information
Felixoid committed Dec 9, 2024
1 parent d344479 commit 4835048
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 36 deletions.
21 changes: 11 additions & 10 deletions src/build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <climits>
#include <functional>
#include <unordered_set>
#include "exit_status.h"

#if defined(__SVR4) && defined(__sun)
#include <sys/termios.h>
Expand All @@ -39,7 +40,6 @@
#include "metrics.h"
#include "state.h"
#include "status.h"
#include "subprocess.h"
#include "util.h"

using namespace std;
Expand Down Expand Up @@ -687,7 +687,7 @@ bool Builder::AlreadyUpToDate() const {
return !plan_.more_to_do();
}

bool Builder::Build(string* err) {
ExitStatus Builder::Build(string* err) {
assert(!AlreadyUpToDate());
plan_.PrepareQueue();

Expand All @@ -711,6 +711,7 @@ bool Builder::Build(string* err) {
// command runner.
// Second, we attempt to wait for / reap the next finished command.
while (plan_.more_to_do()) {
CommandRunner::Result result;
// See if we can start any more commands.
if (failures_allowed) {
size_t capacity = command_runner_->CanRunMore();
Expand All @@ -726,14 +727,14 @@ bool Builder::Build(string* err) {
if (!StartEdge(edge, err)) {
Cleanup();
status_->BuildFinished();
return false;
return ExitFailure;
}

if (edge->is_phony()) {
if (!plan_.EdgeFinished(edge, Plan::kEdgeSucceeded, err)) {
Cleanup();
status_->BuildFinished();
return false;
return ExitFailure;
}
} else {
++pending_commands;
Expand All @@ -754,20 +755,20 @@ bool Builder::Build(string* err) {

// See if we can reap any finished commands.
if (pending_commands) {
CommandRunner::Result result;
if (!command_runner_->WaitForCommand(&result) ||
result.status == ExitInterrupted) {
Cleanup();
status_->BuildFinished();
*err = "interrupted by user";
return false;
return result.status;
}

--pending_commands;
if (!FinishCommand(&result, err)) {
bool command_finished = FinishCommand(&result, err);
if (!command_finished) {
Cleanup();
status_->BuildFinished();
return false;
return result.status;
}

if (!result.success()) {
Expand All @@ -791,11 +792,11 @@ bool Builder::Build(string* err) {
else
*err = "stuck [this is a bug]";

return false;
return result.status;
}

status_->BuildFinished();
return true;
return ExitSuccess;
}

bool Builder::StartEdge(Edge* edge, string* err) {
Expand Down
2 changes: 1 addition & 1 deletion src/build.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ struct Builder {

/// Run the build. Returns false on error.
/// It is an error to call this function when AlreadyUpToDate() is true.
bool Build(std::string* err);
ExitStatus Build(std::string* err);

bool StartEdge(Edge* edge, std::string* err);

Expand Down
14 changes: 11 additions & 3 deletions src/exit_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,18 @@
#ifndef NINJA_EXIT_STATUS_H_
#define NINJA_EXIT_STATUS_H_

enum ExitStatus {
ExitSuccess,
// The underlying type of the ExitStatus enum, used to represent a platform-specific
// process exit code.
#ifdef _WIN32
#define EXIT_STATUS_TYPE unsigned long
#else // !_WIN32
#define EXIT_STATUS_TYPE int
#endif // !_WIN32

enum ExitStatus : EXIT_STATUS_TYPE {
ExitSuccess=0,
ExitFailure,
ExitInterrupted
ExitInterrupted=130
};

#endif // NINJA_EXIT_STATUS_H_
24 changes: 12 additions & 12 deletions src/ninja.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
#include "clean.h"
#include "command_collector.h"
#include "debug_flags.h"
#include "depfile_parser.h"
#include "disk_interface.h"
#include "exit_status.h"
#include "graph.h"
#include "graphviz.h"
#include "json.h"
Expand Down Expand Up @@ -165,7 +165,7 @@ struct NinjaMain : public BuildLogUser {

/// Build the targets listed on the command line.
/// @return an exit code.
int RunBuild(int argc, char** argv, Status* status);
ExitStatus RunBuild(int argc, char** argv, Status* status);

/// Dump the output requested by '-d stats'.
void DumpMetrics();
Expand Down Expand Up @@ -281,7 +281,7 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* err,
if (builder.AlreadyUpToDate())
return false; // Not an error, but we didn't rebuild.

if (!builder.Build(err))
if (builder.Build(err) != ExitSuccess)
return false;

// The manifest was only rebuilt if it is now dirty (it may have been cleaned
Expand Down Expand Up @@ -1541,12 +1541,12 @@ bool NinjaMain::EnsureBuildDirExists() {
return true;
}

int NinjaMain::RunBuild(int argc, char** argv, Status* status) {
ExitStatus NinjaMain::RunBuild(int argc, char** argv, Status* status) {
string err;
vector<Node*> targets;
if (!CollectTargetsFromArgs(argc, argv, &targets, &err)) {
status->Error("%s", err.c_str());
return 1;
return ExitFailure;
}

disk_interface_.AllowStatCache(g_experimental_statcache);
Expand All @@ -1557,7 +1557,7 @@ int NinjaMain::RunBuild(int argc, char** argv, Status* status) {
if (!builder.AddTarget(targets[i], &err)) {
if (!err.empty()) {
status->Error("%s", err.c_str());
return 1;
return ExitFailure;
} else {
// Added a target that is already up-to-date; not really
// an error.
Expand All @@ -1572,18 +1572,18 @@ int NinjaMain::RunBuild(int argc, char** argv, Status* status) {
if (config_.verbosity != BuildConfig::NO_STATUS_UPDATE) {
status->Info("no work to do.");
}
return 0;
return ExitSuccess;
}

if (!builder.Build(&err)) {
ExitStatus exit_status = builder.Build(&err);
if (exit_status != ExitSuccess) {
status->Info("build stopped: %s.", err.c_str());
if (err.find("interrupted by user") != string::npos) {
return 130;
return ExitInterrupted;
}
return 1;
}

return 0;
return exit_status;
}

#ifdef _MSC_VER
Expand Down Expand Up @@ -1801,7 +1801,7 @@ NORETURN void real_main(int argc, char** argv) {

ninja.ParsePreviousElapsedTimes();

int result = ninja.RunBuild(argc, argv, status);
ExitStatus result = ninja.RunBuild(argc, argv, status);
if (g_metrics)
ninja.DumpMetrics();
exit(result);
Expand Down
14 changes: 9 additions & 5 deletions src/subprocess-posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "exit_status.h"
#include "subprocess.h"

#include <sys/select.h>
Expand All @@ -23,6 +24,7 @@
#include <string.h>
#include <sys/wait.h>
#include <spawn.h>
#include <cmath>

#if defined(USE_PPOLL)
#include <poll.h>
Expand Down Expand Up @@ -165,15 +167,17 @@ ExitStatus Subprocess::Finish() {
#endif

if (WIFEXITED(status)) {
int exit = WEXITSTATUS(status);
if (exit == 0)
return ExitSuccess;
} else if (WIFSIGNALED(status)) {
// propagate the status transparently
return static_cast<ExitStatus>(WEXITSTATUS(status));
}
if (WIFSIGNALED(status)) {
// Overwrite interrupts to exit code 2
if (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGTERM
|| WTERMSIG(status) == SIGHUP)
return ExitInterrupted;
}
return ExitFailure;
// At this point, we exit with any other signal+128
return static_cast<ExitStatus>(status | 0x80);
}

bool Subprocess::Done() const {
Expand Down
6 changes: 3 additions & 3 deletions src/subprocess-win32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "exit_status.h"
#include "subprocess.h"

#include <assert.h>
Expand Down Expand Up @@ -198,9 +199,8 @@ ExitStatus Subprocess::Finish() {
CloseHandle(child_);
child_ = NULL;

return exit_code == 0 ? ExitSuccess :
exit_code == CONTROL_C_EXIT ? ExitInterrupted :
ExitFailure;
return exit_code == CONTROL_C_EXIT ? ExitInterrupted :
static_cast<ExitStatus>(exit_code);
}

bool Subprocess::Done() const {
Expand Down
17 changes: 15 additions & 2 deletions src/subprocess_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "subprocess.h"

#include "exit_status.h"
#include "test.h"

#ifndef _WIN32
Expand Down Expand Up @@ -50,7 +51,13 @@ TEST_F(SubprocessTest, BadCommandStderr) {
subprocs_.DoWork();
}

EXPECT_EQ(ExitFailure, subproc->Finish());
ExitStatus exit = subproc->Finish();
#ifdef _POSIX_VERSION
EXPECT_EQ(127, exit);
#endif
#ifdef _WIN32
EXPECT_EQ(ExitFailure, exit);
#endif
EXPECT_NE("", subproc->GetOutput());
}

Expand All @@ -64,7 +71,13 @@ TEST_F(SubprocessTest, NoSuchCommand) {
subprocs_.DoWork();
}

EXPECT_EQ(ExitFailure, subproc->Finish());
ExitStatus exit = subproc->Finish();
#ifdef _POSIX_VERSION
EXPECT_EQ(127, exit);
#endif
#ifdef _WIN32
EXPECT_EQ(ExitFailure, exit);
#endif
EXPECT_NE("", subproc->GetOutput());
#ifdef _WIN32
ASSERT_EQ("CreateProcess failed: The system cannot find the file "
Expand Down

0 comments on commit 4835048

Please sign in to comment.