summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/buildtool/execution_api/local/local_action.cpp10
-rw-r--r--src/buildtool/system/system_command.hpp38
-rw-r--r--src/other_tools/git_operations/git_repo_remote.cpp23
-rw-r--r--src/other_tools/ops_maps/git_tree_fetch_map.cpp9
-rw-r--r--test/buildtool/system/system_command.test.cpp34
5 files changed, 49 insertions, 65 deletions
diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 1d964533..d9900a61 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -145,17 +145,17 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept
std::copy(cmdline_.begin(), cmdline_.end(), std::back_inserter(cmdline));
SystemCommand system{"LocalExecution"};
- auto const command_output =
+ auto const exit_code =
system.Execute(cmdline, env_vars_, build_root, *exec_path);
- if (command_output.has_value()) {
+ if (exit_code.has_value()) {
Output result{};
- result.action.set_exit_code(command_output->return_value);
+ result.action.set_exit_code(*exit_code);
if (gsl::owner<bazel_re::Digest*> digest_ptr =
- DigestFromOwnedFile(command_output->stdout_file)) {
+ DigestFromOwnedFile(*exec_path / "stdout")) {
result.action.set_allocated_stdout_digest(digest_ptr);
}
if (gsl::owner<bazel_re::Digest*> digest_ptr =
- DigestFromOwnedFile(command_output->stderr_file)) {
+ DigestFromOwnedFile(*exec_path / "stderr")) {
result.action.set_allocated_stderr_digest(digest_ptr);
}
diff --git a/src/buildtool/system/system_command.hpp b/src/buildtool/system/system_command.hpp
index b9145301..5777404f 100644
--- a/src/buildtool/system/system_command.hpp
+++ b/src/buildtool/system/system_command.hpp
@@ -39,31 +39,26 @@
/// commands. This class is not thread-safe.
class SystemCommand {
public:
- struct ExecOutput {
- int return_value{};
- std::filesystem::path stdout_file{};
- std::filesystem::path stderr_file{};
- };
-
/// \brief Create execution system with name.
explicit SystemCommand(std::string name) : logger_{std::move(name)} {}
/// \brief Execute command and arguments.
+ /// Stdout and stderr can be read from files named 'stdout' and 'stderr'
+ /// created in `outdir`. Those files must not exist before the execution.
/// \param argv argv vector with the command to execute
/// \param env Environment variables set for execution.
/// \param cwd Working directory for execution.
- /// \param tmpdir Temporary directory for storing stdout/stderr files.
- /// \returns std::nullopt if there was an error in the execution setup
- /// outside running the command itself, SystemCommand::ExecOutput otherwise.
+ /// \param outdir Directory for storing stdout/stderr files.
+ /// \returns The command's exit code, or std::nullopt on execution error.
[[nodiscard]] auto Execute(std::vector<std::string> argv,
std::map<std::string, std::string> env,
std::filesystem::path const& cwd,
- std::filesystem::path const& tmpdir) noexcept
- -> std::optional<ExecOutput> {
- if (not FileSystemManager::IsDirectory(tmpdir)) {
+ std::filesystem::path const& outdir) noexcept
+ -> std::optional<int> {
+ if (not FileSystemManager::IsDirectory(outdir)) {
logger_.Emit(LogLevel::Error,
- "Temporary directory does not exist {}",
- tmpdir.string());
+ "Output directory does not exist {}",
+ outdir.string());
return std::nullopt;
}
@@ -82,7 +77,7 @@ class SystemCommand {
return name_value.first + "=" + name_value.second;
});
std::vector<char*> envp = UnwrapStrings(&env_string);
- return ExecuteCommand(cmd.data(), envp.data(), cwd, tmpdir);
+ return ExecuteCommand(cmd.data(), envp.data(), cwd, outdir);
}
private:
@@ -104,24 +99,21 @@ class SystemCommand {
/// \param cmd Command arguments as char pointer array.
/// \param envp Environment variables as char pointer array.
/// \param cwd Working directory for execution.
- /// \param tmpdir Temporary directory for storing stdout/stderr files.
+ /// \param outdir Directory for storing stdout/stderr files.
/// \returns ExecOutput if command was successfully submitted to the system.
/// \returns std::nullopt on internal failure.
[[nodiscard]] auto ExecuteCommand(
char* const* cmd,
char* const* envp,
std::filesystem::path const& cwd,
- std::filesystem::path const& tmpdir) noexcept
- -> std::optional<ExecOutput> {
- auto stdout_file = tmpdir / "stdout";
- auto stderr_file = tmpdir / "stderr";
+ std::filesystem::path const& outdir) noexcept -> std::optional<int> {
+ auto stdout_file = outdir / "stdout";
+ auto stderr_file = outdir / "stderr";
if (auto const out = OpenFile(stdout_file)) {
if (auto const err = OpenFile(stderr_file)) {
if (auto retval = ForkAndExecute(
cmd, envp, cwd, fileno(out.get()), fileno(err.get()))) {
- return ExecOutput{.return_value = *retval,
- .stdout_file = std::move(stdout_file),
- .stderr_file = std::move(stderr_file)};
+ return *retval;
}
}
else {
diff --git a/src/other_tools/git_operations/git_repo_remote.cpp b/src/other_tools/git_operations/git_repo_remote.cpp
index f3964e74..706eedae 100644
--- a/src/other_tools/git_operations/git_repo_remote.cpp
+++ b/src/other_tools/git_operations/git_repo_remote.cpp
@@ -459,13 +459,13 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo(
}
// set up the system command
SystemCommand system{repo_url};
- auto const command_output =
+ auto const exit_code =
system.Execute(cmdline,
env,
GetGitPath(), // which path is not actually relevant
tmp_path);
- if (not command_output) {
+ if (not exit_code) {
(*logger)(fmt::format("exec() on command failed."),
/*fatal=*/true);
return std::nullopt;
@@ -473,15 +473,14 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo(
// output file can be read anyway
std::string out_str{};
- auto cmd_out = FileSystemManager::ReadFile(command_output->stdout_file);
+ auto cmd_out = FileSystemManager::ReadFile(tmp_path / "stdout");
if (cmd_out) {
out_str = *cmd_out;
}
// check for command failure
- if (command_output->return_value != 0) {
+ if (*exit_code != 0) {
std::string err_str{};
- auto cmd_err =
- FileSystemManager::ReadFile(command_output->stderr_file);
+ auto cmd_err = FileSystemManager::ReadFile(tmp_path / "stderr");
if (cmd_err) {
err_str = *cmd_err;
@@ -618,22 +617,20 @@ auto GitRepoRemote::FetchViaTmpRepo(std::string const& repo_url,
}
// run command
SystemCommand system{repo_url};
- auto const command_output =
+ auto const exit_code =
system.Execute(cmdline, env, GetGitPath(), tmp_path);
- if (not command_output) {
+ if (not exit_code) {
(*logger)(fmt::format("exec() on command failed."),
/*fatal=*/true);
return false;
}
- if (command_output->return_value != 0) {
+ if (*exit_code != 0) {
std::string out_str{};
std::string err_str{};
- auto cmd_out =
- FileSystemManager::ReadFile(command_output->stdout_file);
- auto cmd_err =
- FileSystemManager::ReadFile(command_output->stderr_file);
+ auto cmd_out = FileSystemManager::ReadFile(tmp_path / "stdout");
+ auto cmd_err = FileSystemManager::ReadFile(tmp_path / "stderr");
if (cmd_out) {
out_str = *cmd_out;
}
diff --git a/src/other_tools/ops_maps/git_tree_fetch_map.cpp b/src/other_tools/ops_maps/git_tree_fetch_map.cpp
index 423808cc..74214349 100644
--- a/src/other_tools/ops_maps/git_tree_fetch_map.cpp
+++ b/src/other_tools/ops_maps/git_tree_fetch_map.cpp
@@ -289,9 +289,9 @@ auto CreateGitTreeFetchMap(
env[k] = std::string(v);
}
}
- auto const command_output = system.Execute(
+ auto const exit_code = system.Execute(
cmdline, env, tmp_dir->GetPath(), out_dir->GetPath());
- if (not command_output) {
+ if (not exit_code) {
(*logger)(fmt::format("Failed to execute command:\n{}",
nlohmann::json(cmdline).dump()),
/*fatal=*/true);
@@ -315,7 +315,6 @@ auto CreateGitTreeFetchMap(
critical_git_op_map,
just_git_cas = op_result.git_cas,
cmdline,
- command_output,
key,
git_bin,
launcher,
@@ -361,9 +360,9 @@ auto CreateGitTreeFetchMap(
std::string out_str{};
std::string err_str{};
auto cmd_out = FileSystemManager::ReadFile(
- command_output->stdout_file);
+ out_dir->GetPath() / "stdout");
auto cmd_err = FileSystemManager::ReadFile(
- command_output->stderr_file);
+ out_dir->GetPath() / "stderr");
if (cmd_out) {
out_str = *cmd_out;
}
diff --git a/test/buildtool/system/system_command.test.cpp b/test/buildtool/system/system_command.test.cpp
index 292f437f..e14a7657 100644
--- a/test/buildtool/system/system_command.test.cpp
+++ b/test/buildtool/system/system_command.test.cpp
@@ -53,9 +53,9 @@ TEST_CASE("SystemCommand", "[filesystem]") {
auto output = system.Execute(
{"echo"}, {}, FileSystemManager::GetCurrentDirectory(), tmpdir);
REQUIRE(output.has_value());
- CHECK(output->return_value == 0);
- CHECK(*FileSystemManager::ReadFile(output->stdout_file) == "\n");
- CHECK(FileSystemManager::ReadFile(output->stderr_file)->empty());
+ CHECK(*output == 0);
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stdout") == "\n");
+ CHECK(FileSystemManager::ReadFile(tmpdir / "stderr")->empty());
}
SECTION(
@@ -68,10 +68,10 @@ TEST_CASE("SystemCommand", "[filesystem]") {
FileSystemManager::GetCurrentDirectory(),
tmpdir);
REQUIRE(output.has_value());
- CHECK(output->return_value == 0);
- CHECK(*FileSystemManager::ReadFile(output->stdout_file) ==
+ CHECK(*output == 0);
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stdout") ==
"${MY_MESSAGE}\n");
- CHECK(FileSystemManager::ReadFile(output->stderr_file)->empty());
+ CHECK(FileSystemManager::ReadFile(tmpdir / "stderr")->empty());
tmpdir = testdir / "simple_env1";
REQUIRE(FileSystemManager::CreateDirectoryExclusive(tmpdir));
@@ -81,11 +81,9 @@ TEST_CASE("SystemCommand", "[filesystem]") {
FileSystemManager::GetCurrentDirectory(),
tmpdir);
REQUIRE(output_wrapped.has_value());
- CHECK(output_wrapped->return_value == 0);
- CHECK(*FileSystemManager::ReadFile(output_wrapped->stdout_file) ==
- "hello\n");
- CHECK(
- FileSystemManager::ReadFile(output_wrapped->stderr_file)->empty());
+ CHECK(*output_wrapped == 0);
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stdout") == "hello\n");
+ CHECK(FileSystemManager::ReadFile(tmpdir / "stderr")->empty());
}
SECTION("executable, producing std output, std error and return value") {
@@ -99,10 +97,10 @@ TEST_CASE("SystemCommand", "[filesystem]") {
FileSystemManager::GetCurrentDirectory(),
tmpdir);
REQUIRE(output.has_value());
- CHECK(output->return_value == 5);
- CHECK(*FileSystemManager::ReadFile(output->stdout_file) ==
+ CHECK(*output == 5);
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stdout") ==
"this is stdout\n");
- CHECK(*FileSystemManager::ReadFile(output->stderr_file) ==
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stderr") ==
"this is stderr\n");
}
@@ -121,10 +119,8 @@ TEST_CASE("SystemCommand", "[filesystem]") {
FileSystemManager::GetCurrentDirectory(),
tmpdir);
REQUIRE(output.has_value());
- CHECK(output->return_value == 5);
- CHECK(*FileSystemManager::ReadFile(output->stdout_file) ==
- stdout + '\n');
- CHECK(*FileSystemManager::ReadFile(output->stderr_file) ==
- stderr + '\n');
+ CHECK(*output == 5);
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stdout") == stdout + '\n');
+ CHECK(*FileSystemManager::ReadFile(tmpdir / "stderr") == stderr + '\n');
}
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-07-03 17:53:56 +0000