summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--.clang-tidy2
-rw-r--r--src/buildtool/execution_api/local/local_action.cpp39
-rw-r--r--src/buildtool/execution_api/local/local_api.hpp4
-rw-r--r--src/buildtool/execution_api/remote/bazel/bazel_action.cpp33
-rw-r--r--src/buildtool/execution_api/remote/bazel/bazel_api.cpp4
-rw-r--r--test/buildtool/execution_engine/executor/executor.test.cpp38
6 files changed, 81 insertions, 39 deletions
diff --git a/.clang-tidy b/.clang-tidy
index 67f0b1b1..ca95fb99 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -2,7 +2,7 @@ FormatStyle: Google
Checks: >-
*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-darwin-*,-fuchsia-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-zircon-*
WarningsAsErrors: >-
- bugprone-*,-bugprone-easily-swappable-parameters,-bugprone-unchecked-optional-access,-bugprone-unhandled-exception-at-new,-bugprone-empty-catch,-bugprone-exception-escape,
+ bugprone-*,-bugprone-easily-swappable-parameters,-bugprone-unchecked-optional-access,-bugprone-empty-catch,-bugprone-exception-escape,
clang-analyzer-*,-clang-analyzer-cplusplus.NewDeleteLeaks,-clang-analyzer-cplusplus.StringChecker,
clang-diagnostic-*,-clang-diagnostic-unused-command-line-argument,
hicpp-*,
diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 10359e92..e2ee373c 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -104,16 +104,33 @@ auto LocalAction::Execute(Logger const* logger) noexcept
action->hash());
}
+ auto create_response = [](Logger const* logger,
+ std::string const& action_hash,
+ auto&&... args) -> IExecutionResponse::Ptr {
+ try {
+ return IExecutionResponse::Ptr{new LocalResponse{
+ action_hash, std::forward<decltype(args)>(args)...}};
+ } catch (...) {
+ if (logger != nullptr) {
+ logger->Emit(LogLevel::Error,
+ "failed to create a response for {}",
+ action_hash);
+ }
+ }
+ return nullptr;
+ };
+
if (do_cache) {
if (auto result =
local_context_.storage->ActionCache().CachedResult(*action)) {
if (result->exit_code() == 0 and
ActionResultContainsExpectedOutputs(
*result, output_files_, output_dirs_)) {
- return IExecutionResponse::Ptr{
- new LocalResponse{action->hash(),
- {std::move(*result), /*is_cached=*/true},
- local_context_.storage}};
+ return create_response(
+ logger,
+ action->hash(),
+ LocalAction::Output{*std::move(result), /*is_cached=*/true},
+ local_context_.storage);
}
}
}
@@ -135,13 +152,15 @@ auto LocalAction::Execute(Logger const* logger) noexcept
}
output->is_cached = true;
- return IExecutionResponse::Ptr{
- new LocalResponse{action_cached->hash(),
- std::move(*output),
- local_context_.storage}};
+ return create_response(logger,
+ action_cached->hash(),
+ *std::move(output),
+ local_context_.storage);
}
- return IExecutionResponse::Ptr{new LocalResponse{
- action->hash(), std::move(*output), local_context_.storage}};
+ return create_response(logger,
+ action->hash(),
+ *std::move(output),
+ local_context_.storage);
}
}
diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp
index 4fc6b16e..e564d72f 100644
--- a/src/buildtool/execution_api/local/local_api.hpp
+++ b/src/buildtool/execution_api/local/local_api.hpp
@@ -19,6 +19,7 @@
#include <iterator>
#include <map>
#include <memory>
+#include <new> // std::nothrow
#include <optional>
#include <sstream>
#include <string>
@@ -67,7 +68,8 @@ class LocalApi final : public IExecutionApi {
std::map<std::string, std::string> const& env_vars,
std::map<std::string, std::string> const& properties) const noexcept
-> IExecutionAction::Ptr final {
- return IExecutionAction::Ptr{new LocalAction{&local_context_,
+ return IExecutionAction::Ptr{new (std::nothrow)
+ LocalAction{&local_context_,
root_digest,
command,
cwd,
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp
index 15ca949f..f805c6fc 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp
@@ -69,6 +69,22 @@ auto BazelAction::Execute(Logger const* logger) noexcept
action->hash());
}
+ auto create_response = [](Logger const* logger,
+ std::string const& action_hash,
+ auto&&... args) -> IExecutionResponse::Ptr {
+ try {
+ return IExecutionResponse::Ptr{new BazelResponse{
+ action_hash, std::forward<decltype(args)>(args)...}};
+ } catch (...) {
+ if (logger != nullptr) {
+ logger->Emit(LogLevel::Error,
+ "failed to create a response for {}",
+ action_hash);
+ }
+ }
+ return nullptr;
+ };
+
if (do_cache) {
if (auto result =
network_->GetCachedActionResult(*action, output_files_)) {
@@ -77,8 +93,11 @@ auto BazelAction::Execute(Logger const* logger) noexcept
*result, output_files_, output_dirs_)
) {
- return IExecutionResponse::Ptr{new BazelResponse{
- action->hash(), network_, {*result, true}}};
+ return create_response(
+ logger,
+ action->hash(),
+ network_,
+ BazelExecutionClient::ExecutionOutput{*result, true});
}
}
}
@@ -101,11 +120,13 @@ auto BazelAction::Execute(Logger const* logger) noexcept
}
output->cached_result = true;
- return IExecutionResponse::Ptr{new BazelResponse{
- action_cached->hash(), network_, std::move(*output)}};
+ return create_response(logger,
+ action_cached->hash(),
+ network_,
+ *std::move(output));
}
- return IExecutionResponse::Ptr{new BazelResponse{
- action->hash(), network_, std::move(*output)}};
+ return create_response(
+ logger, action->hash(), network_, *std::move(output));
}
}
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
index 8dbd026e..1b2933af 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
@@ -19,6 +19,7 @@
#include <cstdint>
#include <iterator>
#include <mutex>
+#include <new>
#include <sstream>
#include <unordered_map>
#include <unordered_set>
@@ -218,7 +219,8 @@ auto BazelApi::CreateAction(
std::map<std::string, std::string> const& env_vars,
std::map<std::string, std::string> const& properties) const noexcept
-> IExecutionAction::Ptr {
- return std::unique_ptr<BazelAction>{new BazelAction{network_,
+ return std::unique_ptr<BazelAction>{new (std::nothrow)
+ BazelAction{network_,
root_digest,
command,
cwd,
diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp
index 46ad743e..7eeee78f 100644
--- a/test/buildtool/execution_engine/executor/executor.test.cpp
+++ b/test/buildtool/execution_engine/executor/executor.test.cpp
@@ -17,6 +17,7 @@
#include <algorithm>
#include <filesystem>
#include <map>
+#include <memory>
#include <optional>
#include <string>
#include <unordered_map>
@@ -77,9 +78,10 @@ class TestResponse;
/// \brief Mockup Response, stores only config and action result
class TestResponse : public IExecutionResponse {
- friend class TestAction;
-
public:
+ explicit TestResponse(TestApiConfig config) noexcept
+ : config_{std::move(config)} {}
+
[[nodiscard]] auto Status() const noexcept -> StatusCode final {
return StatusCode::Success;
}
@@ -116,9 +118,6 @@ class TestResponse : public IExecutionResponse {
ArtifactInfos artifacts_;
bool populated_ = false;
- explicit TestResponse(TestApiConfig config) noexcept
- : config_{std::move(config)} {}
-
void Populate() noexcept {
if (populated_) {
return;
@@ -145,23 +144,22 @@ class TestResponse : public IExecutionResponse {
/// \brief Mockup Action, stores only config
class TestAction : public IExecutionAction {
- friend class TestApi;
-
public:
+ explicit TestAction(TestApiConfig config) noexcept
+ : config_{std::move(config)} {}
+
auto Execute(Logger const* /*unused*/) noexcept
-> IExecutionResponse::Ptr final {
if (config_.execution.failed) {
return nullptr;
}
- return IExecutionResponse::Ptr{new TestResponse{config_}};
+ return std::make_unique<TestResponse>(config_);
}
void SetCacheFlag(CacheFlag /*unused*/) noexcept final {}
void SetTimeout(std::chrono::milliseconds /*unused*/) noexcept final {}
private:
TestApiConfig config_{};
- explicit TestAction(TestApiConfig config) noexcept
- : config_{std::move(config)} {}
};
/// \brief Mockup Api, use config to create action and handle artifact upload
@@ -179,7 +177,7 @@ class TestApi : public IExecutionApi {
std::map<std::string, std::string> const& /*unused*/,
std::map<std::string, std::string> const& /*unused*/) const noexcept
-> IExecutionAction::Ptr final {
- return IExecutionAction::Ptr{new TestAction(config_)};
+ return std::make_unique<TestAction>(config_);
}
[[nodiscard]] auto RetrieveToPaths(
std::vector<Artifact::ObjectInfo> const& /*unused*/,
@@ -322,7 +320,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") {
.exec_config = &remote_config};
SECTION("Processing succeeds for valid config") {
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -340,7 +338,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") {
SECTION("Processing fails if uploading local artifact failed") {
config.artifacts[NamedDigest("local.cpp").hash()].uploads = false;
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -358,7 +356,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") {
SECTION("Processing fails if known artifact is not available") {
config.artifacts[NamedDigest("known.cpp").hash()].available = false;
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -405,7 +403,7 @@ TEST_CASE("Executor: Process action", "[executor]") {
.exec_config = &remote_config};
SECTION("Processing succeeds for valid config") {
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -426,7 +424,7 @@ TEST_CASE("Executor: Process action", "[executor]") {
SECTION("Processing succeeds even if result was is not cached") {
config.response.cached = false;
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -447,7 +445,7 @@ TEST_CASE("Executor: Process action", "[executor]") {
SECTION("Processing succeeds even if output is not available in CAS") {
config.artifacts[NamedDigest("output2.exe").hash()].available = false;
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -471,7 +469,7 @@ TEST_CASE("Executor: Process action", "[executor]") {
SECTION("Processing fails if execution failed") {
config.execution.failed = true;
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -492,7 +490,7 @@ TEST_CASE("Executor: Process action", "[executor]") {
SECTION("Processing fails if exit code is non-zero") {
config.response.exit_code = 1;
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
@@ -516,7 +514,7 @@ TEST_CASE("Executor: Process action", "[executor]") {
SECTION("Processing fails if any output is missing") {
config.execution.outputs = {"output1.exe" /*, "output2.exe"*/};
- auto api = TestApi::Ptr{new TestApi{config}};
+ auto api = std::make_shared<TestApi>(config);
Statistics stats{};
Progress progress{};
auto const apis = CreateTestApiBundle(&hash_function, api);
generated by cgit v1.2.3 (git 2.39.1) at 2025-07-03 21:29:13 +0000