From 48350481c47f544a432f34ec5e479bd48d2d77c5 Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Tue, 3 Dec 2024 15:51:23 +0100 Subject: [PATCH] Propagate supbrocess' exit codes to the ninja exit code --- src/build.cc | 21 +++++++++++---------- src/build.h | 2 +- src/exit_status.h | 14 +++++++++++--- src/ninja.cc | 24 ++++++++++++------------ src/subprocess-posix.cc | 14 +++++++++----- src/subprocess-win32.cc | 6 +++--- src/subprocess_test.cc | 17 +++++++++++++++-- 7 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/build.cc b/src/build.cc index d256d940b0..4e98a98cde 100644 --- a/src/build.cc +++ b/src/build.cc @@ -23,6 +23,7 @@ #include #include #include +#include "exit_status.h" #if defined(__SVR4) && defined(__sun) #include @@ -39,7 +40,6 @@ #include "metrics.h" #include "state.h" #include "status.h" -#include "subprocess.h" #include "util.h" using namespace std; @@ -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(); @@ -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(); @@ -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; @@ -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()) { @@ -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) { diff --git a/src/build.h b/src/build.h index ba39e7728a..8f126d395c 100644 --- a/src/build.h +++ b/src/build.h @@ -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); diff --git a/src/exit_status.h b/src/exit_status.h index a714ece791..3b723efede 100644 --- a/src/exit_status.h +++ b/src/exit_status.h @@ -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_ diff --git a/src/ninja.cc b/src/ninja.cc index f681bfec11..119e7d4467 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -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" @@ -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(); @@ -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 @@ -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 targets; if (!CollectTargetsFromArgs(argc, argv, &targets, &err)) { status->Error("%s", err.c_str()); - return 1; + return ExitFailure; } disk_interface_.AllowStatCache(g_experimental_statcache); @@ -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. @@ -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 @@ -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); diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 8e785406c9..311751c7ae 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -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 @@ -23,6 +24,7 @@ #include #include #include +#include #if defined(USE_PPOLL) #include @@ -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(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(status | 0x80); } bool Subprocess::Done() const { diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index ff3baaca7f..4cb8472141 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -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 @@ -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(exit_code); } bool Subprocess::Done() const { diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index 073fe86931..34cb393e08 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -14,6 +14,7 @@ #include "subprocess.h" +#include "exit_status.h" #include "test.h" #ifndef _WIN32 @@ -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()); } @@ -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 "