diff --git a/src/Algos.cc b/src/Algos.cc index fa5b644ff..002f8b7c9 100644 --- a/src/Algos.cc +++ b/src/Algos.cc @@ -1,5 +1,6 @@ #include "Algos.hpp" +#include "Command.hpp" #include "Exception.hpp" #include "Logger.hpp" #include "Rustify.hpp" @@ -39,48 +40,23 @@ toMacroName(const std::string_view name) noexcept { } int -execCmd(const std::string_view cmd) noexcept { +execCmd(const Command& cmd) noexcept { logger::debug("Running `", cmd, '`'); - const int status = std::system(cmd.data()); - const int exitCode = status >> 8; - return exitCode; -} - -static std::pair -getCmdOutputImpl(const std::string_view cmd) { - constexpr usize bufferSize = 128; - std::array buffer{}; - std::string output; - - FILE* pipe = popen(cmd.data(), "r"); - if (!pipe) { - throw PoacError("popen() failed!"); - } - - while (fgets(buffer.data(), buffer.size(), pipe) != nullptr) { - output += buffer.data(); - } - - const int status = pclose(pipe); - if (status == -1) { - throw PoacError("pclose() failed!"); - } - const int exitCode = status >> 8; - return { output, exitCode }; + return cmd.output().exitCode; } std::string -getCmdOutput(const std::string_view cmd, const usize retry) { +getCmdOutput(const Command& cmd, const usize retry) { logger::debug("Running `", cmd, '`'); int exitCode = EXIT_SUCCESS; int waitTime = 1; for (usize i = 0; i < retry; ++i) { - const auto result = getCmdOutputImpl(cmd); - if (result.second == EXIT_SUCCESS) { - return result.first; + const auto [output, status] = cmd.output(); + if (status == EXIT_SUCCESS) { + return output; } - exitCode = result.second; + exitCode = status; // Sleep for an exponential backoff. std::this_thread::sleep_for(std::chrono::seconds(waitTime)); @@ -91,9 +67,7 @@ getCmdOutput(const std::string_view cmd, const usize retry) { bool commandExists(const std::string_view cmd) noexcept { - std::string checkCmd = "command -v "; - checkCmd += cmd; - checkCmd += " >/dev/null 2>&1"; + auto checkCmd = Command("command").addArg("-v").addArg(cmd); return execCmd(checkCmd) == EXIT_SUCCESS; } diff --git a/src/Algos.hpp b/src/Algos.hpp index d1fc40b78..25a3cc36c 100644 --- a/src/Algos.hpp +++ b/src/Algos.hpp @@ -1,5 +1,6 @@ #pragma once +#include "Command.hpp" #include "Exception.hpp" #include "Rustify.hpp" @@ -20,8 +21,8 @@ std::string toUpper(std::string_view str) noexcept; std::string toMacroName(std::string_view name) noexcept; -int execCmd(std::string_view cmd) noexcept; -std::string getCmdOutput(std::string_view cmd, usize retry = 3); +int execCmd(const Command& cmd) noexcept; +std::string getCmdOutput(const Command& cmd, usize retry = 3); bool commandExists(std::string_view cmd) noexcept; template diff --git a/src/BuildConfig.cc b/src/BuildConfig.cc index 5b7028ca5..937cc08fa 100644 --- a/src/BuildConfig.cc +++ b/src/BuildConfig.cc @@ -110,10 +110,10 @@ struct BuildConfig { std::string OUT_DIR; std::string CXX = "clang++"; - std::string CXXFLAGS; - std::string DEFINES; - std::string INCLUDES = " -I../../include"; - std::string LIBS; + std::vector CXXFLAGS; + std::vector DEFINES; + std::vector INCLUDES = { "-I../../include" }; + std::vector LIBS; BuildConfig() = default; explicit BuildConfig(const std::string& packageName) @@ -334,21 +334,21 @@ 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; - std::string cmd = CXX; - cmd += ' '; - cmd += CXXFLAGS; - cmd += DEFINES; - cmd += INCLUDES; - cmd += " -c "; - cmd += file; - cmd += " -o "; - cmd += output; + const Command cmd = Command(CXX) + .addArg(" ") + .addArgs(CXXFLAGS) + .addArgs(DEFINES) + .addArgs(INCLUDES) + .addArg(" -c ") + .addArg(file) + .addArg(" -o ") + .addArg(output); oss << indent1 << "{\n"; oss << indent2 << "\"directory\": " << baseDirPath << ",\n"; oss << indent2 << "\"file\": " << std::quoted(file) << ",\n"; oss << indent2 << "\"output\": " << std::quoted(output) << ",\n"; - oss << indent2 << "\"command\": " << std::quoted(cmd) << "\n"; + oss << indent2 << "\"command\": " << std::quoted(cmd.to_string()) << "\n"; oss << indent1 << "},\n"; } @@ -366,18 +366,14 @@ BuildConfig::emitCompdb(const std::string_view baseDir, std::ostream& os) std::string BuildConfig::runMM(const std::string& sourceFile, const bool isTest) const { - std::string command = "cd "; - command += getOutDir(); - command += " && "; - command += CXX; - command += CXXFLAGS; - command += DEFINES; - command += INCLUDES; + Command command = + Command(CXX).addArgs(CXXFLAGS).addArgs(DEFINES).addArgs(INCLUDES); if (isTest) { - command += " -DPOAC_TEST"; + command.addArg("-DPOAC_TEST"); } - command += " -MM "; - command += sourceFile; + command.addArg("-MM"); + command.addArg(sourceFile); + command.setWorkingDirectory(getOutDir()); return getCmdOutput(command); } @@ -430,17 +426,16 @@ 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? - std::string command = CXX; - command += " -E "; - command += CXXFLAGS; - command += DEFINES; - command += INCLUDES; - command += ' '; - command += sourceFile; + Command command(CXX); + command.addArg("-E"); + command.addArgs(CXXFLAGS); + command.addArgs(DEFINES); + command.addArgs(INCLUDES); + command.addArg(sourceFile); const std::string src = getCmdOutput(command); - command += " -DPOAC_TEST"; + command.addArg("-DPOAC_TEST"); const std::string testSrc = getCmdOutput(command); // If the source file contains POAC_TEST, by processing the source @@ -570,42 +565,46 @@ void BuildConfig::installDeps(const bool includeDevDeps) { const std::vector deps = installDependencies(includeDevDeps); for (const DepMetadata& dep : deps) { - INCLUDES += ' ' + dep.includes; - LIBS += ' ' + dep.libs; + INCLUDES.push_back(dep.includes); + LIBS.push_back(dep.libs); } - logger::debug("INCLUDES: ", INCLUDES); - logger::debug("LIBS: ", LIBS); + logger::debug(fmt::format("INCLUDES: {}", fmt::join(INCLUDES, " "))); + logger::debug(fmt::format("LIBS: {}", fmt::join(LIBS, " "))); } void BuildConfig::addDefine( const std::string_view name, const std::string_view value ) { - DEFINES += fmt::format(" -D{}='\"{}\"'", name, value); + DEFINES.push_back(fmt::format("-D{}='\"{}\"'", name, value)); } void BuildConfig::setVariables(const bool isDebug) { this->defineCondVar("CXX", CXX); - CXXFLAGS += " -std=c++" + getPackageEdition().getString(); + CXXFLAGS.push_back("-std=c++" + getPackageEdition().getString()); if (shouldColor()) { - CXXFLAGS += " -fdiagnostics-color"; + CXXFLAGS.push_back("-fdiagnostics-color"); } if (isDebug) { - CXXFLAGS += " -g -O0 -DDEBUG"; + CXXFLAGS.push_back("-g"); + CXXFLAGS.push_back("-O0"); + CXXFLAGS.push_back("-DDEBUG"); } else { - CXXFLAGS += " -O3 -DNDEBUG"; + CXXFLAGS.push_back("-O3"); + CXXFLAGS.push_back("-DNDEBUG"); } const Profile& profile = isDebug ? getDevProfile() : getReleaseProfile(); if (profile.lto) { - CXXFLAGS += " -flto"; + CXXFLAGS.push_back("-flto"); } for (const std::string_view flag : profile.cxxflags) { - CXXFLAGS += ' '; - CXXFLAGS += flag; + CXXFLAGS.emplace_back(flag); } - this->defineSimpleVar("CXXFLAGS", CXXFLAGS); + this->defineSimpleVar( + "CXXFLAGS", fmt::format("{:s}", fmt::join(CXXFLAGS, " ")) + ); const std::string pkgName = toMacroName(this->packageName); const Version& pkgVersion = getPackageVersion(); @@ -646,9 +645,13 @@ BuildConfig::setVariables(const bool isDebug) { addDefine(key, val); } - this->defineSimpleVar("DEFINES", DEFINES); - this->defineSimpleVar("INCLUDES", INCLUDES); - this->defineSimpleVar("LIBS", LIBS); + this->defineSimpleVar( + "DEFINES", fmt::format("{:s}", fmt::join(DEFINES, " ")) + ); + this->defineSimpleVar( + "INCLUDES", fmt::format("{:s}", fmt::join(INCLUDES, " ")) + ); + this->defineSimpleVar("LIBS", fmt::format("{:s}", fmt::join(LIBS, " "))); } void @@ -956,18 +959,16 @@ modeToProfile(const bool isDebug) { return isDebug ? "dev" : "release"; } -std::string +Command getMakeCommand() { - std::string makeCommand; - if (isVerbose()) { - makeCommand = "make"; - } else { - makeCommand = "make -s --no-print-directory Q=@"; + Command makeCommand("make"); + if (!isVerbose()) { + makeCommand.addArg("-s").addArg("--no-print-directory").addArg("Q=@"); } const usize numThreads = getParallelism(); if (numThreads > 1) { - makeCommand += " -j" + std::to_string(numThreads); + makeCommand.addArg("-j" + std::to_string(numThreads)); } return makeCommand; diff --git a/src/BuildConfig.hpp b/src/BuildConfig.hpp index ddc637380..801ae753a 100644 --- a/src/BuildConfig.hpp +++ b/src/BuildConfig.hpp @@ -1,5 +1,7 @@ #pragma once +#include "Command.hpp" + #include #include @@ -16,4 +18,4 @@ std::string emitMakefile(bool isDebug, bool includeDevDeps); std::string emitCompdb(bool isDebug, bool includeDevDeps); std::string_view modeToString(bool isDebug); std::string_view modeToProfile(bool isDebug); -std::string getMakeCommand(); +Command getMakeCommand(); diff --git a/src/Cmd/Build.cc b/src/Cmd/Build.cc index 297f277d6..b63b86507 100644 --- a/src/Cmd/Build.cc +++ b/src/Cmd/Build.cc @@ -35,7 +35,7 @@ buildImpl(std::string& outDir, const bool isDebug) { const auto start = std::chrono::steady_clock::now(); outDir = emitMakefile(isDebug, /*includeDevDeps=*/false); - const std::string makeCommand = getMakeCommand() + " -C " + outDir; + const auto makeCommand = getMakeCommand().addArg("-C").addArg(outDir); const int exitCode = execCmd(makeCommand); const auto end = std::chrono::steady_clock::now(); diff --git a/src/Cmd/Fmt.cc b/src/Cmd/Fmt.cc index b644c428b..7c9e09dcb 100644 --- a/src/Cmd/Fmt.cc +++ b/src/Cmd/Fmt.cc @@ -31,7 +31,7 @@ const Subcmd FMT_CMD = static void collectFormatTargetFiles( const fs::path& manifestDir, const std::vector& excludes, - std::string& clangFormatArgs + std::vector& clangFormatArgs ) { // Read git repository if exists git2::Repository repo = git2::Repository(); @@ -74,7 +74,7 @@ collectFormatTargetFiles( const std::string ext = path.extension().string(); if (SOURCE_FILE_EXTS.contains(ext) || HEADER_FILE_EXTS.contains(ext)) { - clangFormatArgs += ' ' + path.string(); + clangFormatArgs.push_back(path.string()); } } } @@ -114,22 +114,27 @@ fmtMain(const std::span args) { } const std::string_view packageName = getPackageName(); - std::string clangFormatArgs = "--style=file --fallback-style=LLVM -Werror"; + std::vector clangFormatArgs{ + "--style=file", + "--fallback-style=LLVM", + "-Werror", + }; if (isVerbose()) { - clangFormatArgs += " --verbose"; + clangFormatArgs.push_back("--verbose"); } if (isCheck) { - clangFormatArgs += " --dry-run"; + clangFormatArgs.push_back("--dry-run"); } else { - clangFormatArgs += " -i"; + clangFormatArgs.push_back("-i"); logger::info("Formatting", packageName); } const fs::path& manifestDir = getManifestPath().parent_path(); collectFormatTargetFiles(manifestDir, excludes, clangFormatArgs); - const std::string clangFormat = "cd " + manifestDir.string() - + " && ${POAC_FMT:-clang-format} " - + clangFormatArgs; + const Command clangFormat = + Command("${POAC_FMT:-clang-format}", std::move(clangFormatArgs)) + .setWorkingDirectory(manifestDir.string()); + return execCmd(clangFormat); } diff --git a/src/Cmd/Lint.cc b/src/Cmd/Lint.cc index 08997b826..4fb1ed8f5 100644 --- a/src/Cmd/Lint.cc +++ b/src/Cmd/Lint.cc @@ -23,17 +23,16 @@ const Subcmd LINT_CMD = Subcmd{ "lint" } .setMainFn(lintMain); struct LintArgs { - std::string excludes; + std::vector excludes; }; static int -lint(const std::string_view name, const std::string_view cpplintArgs) { +lint(const std::string_view name, const std::vector& cpplintArgs) { logger::info("Linting", name); - std::string cpplintCmd = "cpplint"; - cpplintCmd += cpplintArgs; + Command cpplintCmd("cpplint", cpplintArgs); if (!isVerbose()) { - cpplintCmd += " --quiet"; + cpplintCmd.addArg("--quiet"); } // Read .gitignore if exists @@ -45,12 +44,12 @@ lint(const std::string_view name, const std::string_view cpplintArgs) { continue; } - cpplintCmd += " --exclude="; - cpplintCmd += line; + cpplintCmd.addArg("--exclude=" + line); } } // NOTE: This should come after the `--exclude` options. - cpplintCmd += " --recursive ."; + cpplintCmd.addArg("--recursive"); + cpplintCmd.addArg("."); return execCmd(cpplintCmd); } @@ -69,8 +68,7 @@ lintMain(const std::span args) { return Subcmd::missingArgumentForOpt(*itr); } - lintArgs.excludes += " --exclude="; - lintArgs.excludes += *++itr; + lintArgs.excludes.push_back("--exclude=" + std::string(*++itr)); } else { return LINT_CMD.noSuchArg(*itr); } @@ -84,7 +82,7 @@ lintMain(const std::span args) { return EXIT_FAILURE; } - std::string cpplintArgs = lintArgs.excludes; + std::vector cpplintArgs = lintArgs.excludes; const std::string_view packageName = getPackageName(); if (fs::exists("CPPLINT.cfg")) { logger::debug("Using CPPLINT.cfg for lint ..."); @@ -92,27 +90,28 @@ lintMain(const std::span args) { } if (fs::exists("include")) { - cpplintArgs += " --root=include"; + cpplintArgs.push_back("--root=include"); } else if (fs::exists("src")) { - cpplintArgs += " --root=src"; + cpplintArgs.push_back("--root=src"); } const std::vector& cpplintFilters = getLintCpplintFilters(); if (!cpplintFilters.empty()) { logger::debug("Using Poac manifest file for lint ..."); - cpplintArgs += " --filter="; + std::string filterArg = "--filter="; for (const std::string_view filter : cpplintFilters) { - cpplintArgs += filter; - cpplintArgs += ','; + filterArg += filter; + filterArg += ','; } // Remove last comma - cpplintArgs.pop_back(); + filterArg.pop_back(); + cpplintArgs.push_back(filterArg); return lint(packageName, cpplintArgs); } else { logger::debug("Using default arguments for lint ..."); if (Edition::Cpp11 < getPackageEdition()) { // Disable C++11-related lints - cpplintArgs += " --filter=-build/c++11"; + cpplintArgs.push_back("--filter=-build/c++11"); } return lint(packageName, cpplintArgs); } diff --git a/src/Cmd/Run.cc b/src/Cmd/Run.cc index 832ff739c..a542484c2 100644 --- a/src/Cmd/Run.cc +++ b/src/Cmd/Run.cc @@ -56,9 +56,9 @@ runMain(const std::span args) { } } - std::string runArgs; + std::vector runArgs; for (; itr != args.end(); ++itr) { - runArgs += ' ' + std::string(*itr); + runArgs.push_back(std::string(*itr)); } std::string outDir; @@ -67,6 +67,6 @@ runMain(const std::span args) { } const std::string& projectName = getPackageName(); - const std::string command = outDir + "/" + projectName + runArgs; + const auto command = Command(outDir + "/" + projectName, runArgs); return execCmd(command); } diff --git a/src/Cmd/Test.cc b/src/Cmd/Test.cc index 5e7d815c6..f524a0bdf 100644 --- a/src/Cmd/Test.cc +++ b/src/Cmd/Test.cc @@ -57,7 +57,8 @@ testMain(const std::span args) { const auto start = std::chrono::steady_clock::now(); const std::string outDir = emitMakefile(isDebug, /*includeDevDeps=*/true); - const int exitCode = execCmd(getMakeCommand() + " -C " + outDir + " test"); + const int exitCode = + execCmd(getMakeCommand().addArg("-C").addArg(outDir).addArg("test")); const auto end = std::chrono::steady_clock::now(); const std::chrono::duration elapsed = end - start; diff --git a/src/Cmd/Tidy.cc b/src/Cmd/Tidy.cc index 7b99fc2cc..94cb541e5 100644 --- a/src/Cmd/Tidy.cc +++ b/src/Cmd/Tidy.cc @@ -24,7 +24,7 @@ const Subcmd TIDY_CMD = .setMainFn(tidyMain); static int -tidyImpl(const std::string_view makeCmd) { +tidyImpl(const Command& makeCmd) { const auto start = std::chrono::steady_clock::now(); const int exitCode = execCmd(makeCmd); @@ -87,11 +87,11 @@ tidyMain(const std::span args) { } tidyFlags += '\''; - std::string makeCmd = getMakeCommand(); - makeCmd += " -C "; - makeCmd += outDir.string(); - makeCmd += tidyFlags; - makeCmd += " tidy"; + Command makeCmd(getMakeCommand()); + makeCmd.addArg("-C"); + makeCmd.addArg(outDir.string()); + makeCmd.addArg(tidyFlags); + makeCmd.addArg("tidy"); logger::info("Running", "clang-tidy"); return tidyImpl(makeCmd); diff --git a/src/Command.cc b/src/Command.cc index 066020ad5..bf24c1ced 100644 --- a/src/Command.cc +++ b/src/Command.cc @@ -5,6 +5,8 @@ #include "Rustify.hpp" #include +#include +#include #include #include #include @@ -38,6 +40,13 @@ Command::output() const { args.push_back(arg.data()); } args.push_back(nullptr); + + if (!working_directory.empty()) { + if (chdir(working_directory.c_str()) == -1) { + throw PoacError("chdir() failed"); + } + } + if (execvp(command.data(), args.data()) == -1) { throw PoacError("execvp() failed"); } @@ -45,11 +54,6 @@ Command::output() const { } else { close(pipefd[1]); // parent doesn't write - int status; - if (waitpid(pid, &status, 0) == -1) { - throw PoacError("waitpid() failed"); - } - FILE* stream = fdopen(pipefd[0], "r"); if (stream == nullptr) { close(pipefd[0]); @@ -62,7 +66,26 @@ Command::output() const { fclose(stream); + int status; + if (waitpid(pid, &status, 0) == -1) { + throw PoacError("waitpid() failed"); + } + const int exitCode = WEXITSTATUS(status); return { output, exitCode }; } } + +std::string +Command::to_string() const { + std::string res = command; + for (const std::string& arg : arguments) { + res += ' ' + arg; + } + return res; +} + +std::ostream& +operator<<(std::ostream& os, const Command& cmd) { + return os << cmd.to_string(); +} diff --git a/src/Command.hpp b/src/Command.hpp index cf9dc76e2..74732ac85 100644 --- a/src/Command.hpp +++ b/src/Command.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include struct CommandOutput { @@ -13,22 +14,30 @@ struct CommandOutput { struct Command { std::string command; std::vector arguments; + std::string working_directory; - explicit Command(const std::string_view cmd) : command(cmd) {} + explicit Command(std::string_view cmd) : command(cmd) {} + Command(std::string_view cmd, std::vector args) + : command(cmd), arguments(std::move(args)) {} Command& addArg(const std::string_view arg) { arguments.emplace_back(arg); return *this; } + Command& addArgs(const std::vector& args) { + arguments.insert(arguments.end(), args.begin(), args.end()); + return *this; + } + + Command& setWorkingDirectory(const std::string_view wd) { + working_directory = wd; + return *this; + } + + std::string to_string() const; + CommandOutput output() const; }; -std::ostream& -operator<<(std::ostream& os, const Command& cmd) { - os << cmd.command; - for (const std::string& arg : cmd.arguments) { - os << " " << arg; - } - return os; -} +std::ostream& operator<<(std::ostream& os, const Command& cmd); diff --git a/src/Manifest.cc b/src/Manifest.cc index 0af01225f..1b57ab94a 100644 --- a/src/Manifest.cc +++ b/src/Manifest.cc @@ -598,7 +598,7 @@ GitDependency::install() const { } const fs::path includeDir = installDir / "include"; - std::string includes = "-isystem "; + std::string includes = "-I"; if (fs::exists(includeDir) && fs::is_directory(includeDir) && !fs::is_empty(includeDir)) { @@ -614,8 +614,10 @@ GitDependency::install() const { DepMetadata SystemDependency::install() const { const std::string pkgConfigVer = versionReq.toPkgConfigString(name); - const std::string cflagsCmd = "pkg-config --cflags '" + pkgConfigVer + "'"; - const std::string libsCmd = "pkg-config --libs '" + pkgConfigVer + "'"; + const Command cflagsCmd = + Command("pkg-config").addArg("--cflags").addArg(pkgConfigVer); + const Command libsCmd = + Command("pkg-config").addArg("--libs").addArg(pkgConfigVer); std::string cflags = getCmdOutput(cflagsCmd); cflags.pop_back(); // remove '\n'