diff --git a/src/Algos.cc b/src/Algos.cc index 3f9cc1736..9490d9f9b 100644 --- a/src/Algos.cc +++ b/src/Algos.cc @@ -69,11 +69,12 @@ getCmdOutput(const Command& cmd, const usize retry) { int exitCode = EXIT_SUCCESS; int waitTime = 1; for (usize i = 0; i < retry; ++i) { - const auto [output, status] = cmd.output(); - if (status == EXIT_SUCCESS) { - return output; + const auto [curExitCode, stdout, stderr] = cmd.output(); + static_cast(stderr); + if (curExitCode == EXIT_SUCCESS) { + return stdout; } - exitCode = status; + exitCode = curExitCode; // Sleep for an exponential backoff. std::this_thread::sleep_for(std::chrono::seconds(waitTime)); @@ -86,7 +87,7 @@ bool commandExists(const std::string_view cmd) noexcept { const int exitCode = Command("which") .addArg(cmd) - .setStdoutConfig(Command::StdioConfig::Null) + .setStdoutConfig(Command::IOConfig::Null) .spawn() .wait(); return exitCode == EXIT_SUCCESS; diff --git a/src/BuildConfig.cc b/src/BuildConfig.cc index 9e609c29c..de57b5612 100644 --- a/src/BuildConfig.cc +++ b/src/BuildConfig.cc @@ -37,20 +37,10 @@ #include #include -static constinit const std::string_view TEST_OUT_DIR = "tests"; -static constinit const std::string_view PATH_FROM_OUT_DIR = "../../"; +namespace { -static std::vector -listSourceFilePaths(const std::string_view directory) { - std::vector sourceFilePaths; - for (const auto& entry : fs::recursive_directory_iterator(directory)) { - if (!SOURCE_FILE_EXTS.contains(entry.path().extension())) { - continue; - } - sourceFilePaths.emplace_back(entry.path()); - } - return sourceFilePaths; -} +constinit const std::string_view TEST_OUT_DIR = "tests"; +constinit const std::string_view PATH_FROM_OUT_DIR = "../../"; enum class VarType : std::uint8_t { Recursive, // = @@ -87,12 +77,6 @@ struct Variable { VarType type = VarType::Simple; }; -std::ostream& -operator<<(std::ostream& os, const Variable& var) { - os << var.type << ' ' << var.value; - return os; -} - struct Target { std::vector commands; std::optional sourceFile; @@ -110,9 +94,9 @@ struct BuildConfig { std::optional> phony; std::optional> all; - // NOLINTBEGIN(readability-identifier-naming) std::string outDir; - std::string CXX; + std::string cxx; + // NOLINTBEGIN(readability-identifier-naming) std::vector CXXFLAGS; std::vector DEFINES; std::vector INCLUDES = { "-I../../include" }; @@ -122,22 +106,23 @@ struct BuildConfig { explicit BuildConfig(const std::string& packageName) : packageName{ packageName }, buildOutDir{ packageName + ".d" } { if (const char* cxx = std::getenv("CXX")) { - CXX = cxx; + this->cxx = cxx; } else { const std::string output = Command("make") .addArg("--print-data-base") .addArg("--question") .addArg("-f") .addArg("/dev/null") + .setStderrConfig(Command::IOConfig::Null) .output() - .output; + .stdout; std::istringstream iss(output); std::string line; while (std::getline(iss, line)) { if (line.starts_with("CXX = ")) { constexpr usize offset = 6; - CXX = line.substr(offset); + this->cxx = line.substr(offset); return; } } @@ -229,7 +214,7 @@ struct BuildConfig { ) const; }; -static void +void emitDep(std::ostream& os, usize& offset, const std::string_view dep) { constexpr usize maxLineLen = 80; if (offset + dep.size() + 2 > maxLineLen) { // 2 for space and \. @@ -241,7 +226,7 @@ emitDep(std::ostream& os, usize& offset, const std::string_view dep) { offset += dep.size() + 1; // space } -static void +void emitTarget( std::ostream& os, const std::string_view target, const std::unordered_set& dependsOn, @@ -302,6 +287,8 @@ BuildConfig::emitVariable(std::ostream& os, const std::string& varName) const { os << '\n'; } +} // namespace + void BuildConfig::emitMakefile(std::ostream& os) const { const std::vector sortedVars = topoSort(variables, varDeps); @@ -370,7 +357,7 @@ BuildConfig::emitCompdb(const std::string_view baseDir, std::ostream& os) const std::string file = targetInfo.sourceFile.value(); // The output is the target. const std::string output = target; - const Command cmd = Command(CXX) + const Command cmd = Command(cxx) .addArgs(CXXFLAGS) .addArgs(DEFINES) .addArgs(INCLUDES) @@ -402,7 +389,7 @@ BuildConfig::emitCompdb(const std::string_view baseDir, std::ostream& os) std::string BuildConfig::runMM(const std::string& sourceFile, const bool isTest) const { Command command = - Command(CXX).addArgs(CXXFLAGS).addArgs(DEFINES).addArgs(INCLUDES); + Command(cxx).addArgs(CXXFLAGS).addArgs(DEFINES).addArgs(INCLUDES); if (isTest) { command.addArg("-DPOAC_TEST"); } @@ -461,7 +448,7 @@ BuildConfig::containsTestCode(const std::string& sourceFile) const { while (std::getline(ifs, line)) { if (line.find("POAC_TEST") != std::string::npos) { // TODO: Can't we somehow elegantly make the compiler command sharable? - Command command(CXX); + Command command(cxx); command.addArg("-E"); command.addArgs(CXXFLAGS); command.addArgs(DEFINES); @@ -623,7 +610,7 @@ BuildConfig::addDefine( void BuildConfig::setVariables(const bool isDebug) { - this->defineSimpleVar("CXX", CXX); + this->defineSimpleVar("CXX", cxx); CXXFLAGS.push_back("-std=c++" + getPackageEdition().getString()); if (shouldColor()) { @@ -812,6 +799,18 @@ processTestSrc( } } +static std::vector +listSourceFilePaths(const std::string_view directory) { + std::vector sourceFilePaths; + for (const auto& entry : fs::recursive_directory_iterator(directory)) { + if (!SOURCE_FILE_EXTS.contains(entry.path().extension())) { + continue; + } + sourceFilePaths.emplace_back(entry.path()); + } + return sourceFilePaths; +} + static void configureBuild(BuildConfig& config, const bool isDebug) { if (!fs::exists("src")) { diff --git a/src/Command.cc b/src/Command.cc index 86e17e8bf..586118be8 100644 --- a/src/Command.cc +++ b/src/Command.cc @@ -18,11 +18,21 @@ int Child::wait() const { int status{}; if (waitpid(pid, &status, 0) == -1) { - close(stdoutfd); + if (stdoutfd != -1) { + close(stdoutfd); + } + if (stderrfd != -1) { + close(stderrfd); + } throw PoacError("waitpid() failed"); } - close(stdoutfd); + if (stdoutfd != -1) { + close(stdoutfd); + } + if (stderrfd != -1) { + close(stderrfd); + } const int exitCode = WEXITSTATUS(status); return exitCode; @@ -30,38 +40,112 @@ Child::wait() const { CommandOutput Child::waitWithOutput() const { - constexpr std::size_t bufferSize = 128; - std::array buffer{}; - std::string output; + std::string stdoutOutput; + std::string stderrOutput; - FILE* stream = fdopen(stdoutfd, "r"); - if (stream == nullptr) { - close(stdoutfd); - throw PoacError("fdopen() failed"); - } + int maxfd = -1; + fd_set readfds; - while (fgets(buffer.data(), buffer.size(), stream) != nullptr) { - output += buffer.data(); + // Determine the maximum file descriptor + if (stdoutfd > maxfd) { + maxfd = stdoutfd; } + if (stderrfd > maxfd) { + maxfd = stderrfd; + } + + bool stdoutEOF = (stdoutfd == -1); + bool stderrEOF = (stderrfd == -1); + + while (!stdoutEOF || !stderrEOF) { + FD_ZERO(&readfds); + if (!stdoutEOF) { + FD_SET(stdoutfd, &readfds); + } + if (!stderrEOF) { + FD_SET(stderrfd, &readfds); + } - fclose(stream); + int ret = select(maxfd + 1, &readfds, nullptr, nullptr, nullptr); + if (ret == -1) { + if (stdoutfd != -1) { + close(stdoutfd); + } + if (stderrfd != -1) { + close(stderrfd); + } + throw PoacError("select() failed"); + } + + // Read from stdout if available + if (!stdoutEOF && FD_ISSET(stdoutfd, &readfds)) { + char buffer[128]; + ssize_t count = read(stdoutfd, buffer, sizeof(buffer) - 1); + if (count == -1) { + if (stdoutfd != -1) { + close(stdoutfd); + } + if (stderrfd != -1) { + close(stderrfd); + } + throw PoacError("read() failed on stdout"); + } else if (count == 0) { + stdoutEOF = true; + close(stdoutfd); + } else { + buffer[count] = '\0'; + stdoutOutput += buffer; + } + } + + // Read from stderr if available + if (!stderrEOF && FD_ISSET(stderrfd, &readfds)) { + char buffer[128]; + ssize_t count = read(stderrfd, buffer, sizeof(buffer) - 1); + if (count == -1) { + if (stdoutfd != -1) { + close(stdoutfd); + } + if (stderrfd != -1) { + close(stderrfd); + } + throw PoacError("read() failed on stderr"); + } else if (count == 0) { + stderrEOF = true; + close(stderrfd); + } else { + buffer[count] = '\0'; + stderrOutput += buffer; + } + } + } int status{}; if (waitpid(pid, &status, 0) == -1) { throw PoacError("waitpid() failed"); } - const int exitCode = WEXITSTATUS(status); - return { .output = output, .exitCode = exitCode }; + int exitCode = WEXITSTATUS(status); + return { .exitCode = exitCode, + .stdout = stdoutOutput, + .stderr = stderrOutput }; } Child Command::spawn() const { int stdoutPipe[2]; + int stderrPipe[2]; - if (stdoutConfig == StdioConfig::Piped) { + // Set up stdout pipe if needed + if (stdoutConfig == IOConfig::Piped) { if (pipe(stdoutPipe) == -1) { - throw PoacError("pipe() failed"); + throw PoacError("pipe() failed for stdout"); + } + } + // Set up stderr pipe if needed + if (stderrConfig == IOConfig::Piped) { + if (pipe(stderrPipe) == -1) { + throw PoacError("pipe() failed for stderr"); } } @@ -69,22 +153,34 @@ Command::spawn() const { if (pid == -1) { throw PoacError("fork() failed"); } else if (pid == 0) { - if (stdoutConfig == StdioConfig::Piped) { - close(stdoutPipe[0]); // child doesn't read + // Child process - // redirect stdout to pipe - dup2(stdoutPipe[1], 1); + // Redirect stdout + if (stdoutConfig == IOConfig::Piped) { + close(stdoutPipe[0]); // Child doesn't read from stdout pipe + dup2(stdoutPipe[1], STDOUT_FILENO); close(stdoutPipe[1]); - } else if (stdoutConfig == StdioConfig::Null) { - const int nullfd = open("/dev/null", O_WRONLY); - dup2(nullfd, 1); + } else if (stdoutConfig == IOConfig::Null) { + int nullfd = open("/dev/null", O_WRONLY); + dup2(nullfd, STDOUT_FILENO); + close(nullfd); + } + + // Redirect stderr + if (stderrConfig == IOConfig::Piped) { + close(stderrPipe[0]); // Child doesn't read from stderr pipe + dup2(stderrPipe[1], STDERR_FILENO); + close(stderrPipe[1]); + } else if (stderrConfig == IOConfig::Null) { + int nullfd = open("/dev/null", O_WRONLY); + dup2(nullfd, STDERR_FILENO); close(nullfd); } std::vector args; - args.push_back(const_cast(command).data()); - for (std::string& arg : const_cast&>(arguments)) { - args.push_back(arg.data()); + args.push_back(const_cast(command.c_str())); + for (const std::string& arg : arguments) { + args.push_back(const_cast(arg.c_str())); } args.push_back(nullptr); @@ -94,18 +190,25 @@ Command::spawn() const { } } - if (execvp(command.data(), args.data()) == -1) { + // Execute the command + if (execvp(command.c_str(), args.data()) == -1) { throw PoacError("execvp() failed"); } unreachable(); } else { - if (stdoutConfig == StdioConfig::Piped) { - close(stdoutPipe[1]); // parent doesn't write + // Parent process - return { pid, stdoutPipe[0] }; - } else { - return { pid, /* stdin */ 0 }; + // Close unused pipe ends + if (stdoutConfig == IOConfig::Piped) { + close(stdoutPipe[1]); // Parent doesn't write to stdout pipe + } + if (stderrConfig == IOConfig::Piped) { + close(stderrPipe[1]); // Parent doesn't write to stderr pipe } + + // Return the Child object with appropriate file descriptors + return { pid, stdoutConfig == IOConfig::Piped ? stdoutPipe[0] : -1, + stderrConfig == IOConfig::Piped ? stderrPipe[0] : -1 }; } } @@ -114,7 +217,8 @@ Command::spawn() const { CommandOutput Command::output() const { Command cmd = *this; - cmd.setStdoutConfig(StdioConfig::Piped); + cmd.setStdoutConfig(IOConfig::Piped); + cmd.setStderrConfig(IOConfig::Piped); return cmd.spawn().waitWithOutput(); } diff --git a/src/Command.hpp b/src/Command.hpp index 160b560e5..8beaed226 100644 --- a/src/Command.hpp +++ b/src/Command.hpp @@ -9,16 +9,19 @@ #include struct CommandOutput { - std::string output; int exitCode; + std::string stdout; + std::string stderr; }; class Child { private: pid_t pid; int stdoutfd; + int stderrfd; - Child(pid_t pid, int stdoutfd) noexcept : pid(pid), stdoutfd(stdoutfd) {} + Child(pid_t pid, int stdoutfd, int stderrfd) noexcept + : pid(pid), stdoutfd(stdoutfd), stderrfd(stderrfd) {} friend struct Command; @@ -28,7 +31,7 @@ class Child { }; struct Command { - enum class StdioConfig : std::uint8_t { + enum class IOConfig : std::uint8_t { Null, Inherit, Piped, @@ -37,7 +40,8 @@ struct Command { std::string command; std::vector arguments; std::string workingDirectory; - StdioConfig stdoutConfig = StdioConfig::Inherit; + IOConfig stdoutConfig = IOConfig::Inherit; + IOConfig stderrConfig = IOConfig::Inherit; explicit Command(std::string_view cmd) : command(cmd) {} Command(std::string_view cmd, std::vector args) @@ -53,11 +57,16 @@ struct Command { return *this; } - Command& setStdoutConfig(StdioConfig config) noexcept { + Command& setStdoutConfig(IOConfig config) noexcept { stdoutConfig = config; return *this; } + Command& setStderrConfig(IOConfig config) noexcept { + stderrConfig = config; + return *this; + } + Command& setWorkingDirectory(const std::string_view dir) { workingDirectory = dir; return *this;