ExecutionService: Use LocalApi directly

8 files changed, 57 insertions, 43 deletions

diff --git a/src/buildtool/execution_api/execution_service/TARGETS b/src/buildtool/execution_api/execution_service/TARGETS
index a92bb2bd..dde3c03b 100644
--- a/src/buildtool/execution_api/execution_service/TARGETS
+++ b/src/buildtool/execution_api/execution_service/TARGETS
@@ -16,6 +16,7 @@
     , ["src/buildtool/common", "bazel_types"]
     , ["src/buildtool/execution_api/common", "common"]
     , ["src/buildtool/execution_api/local", "context"]
+    , ["src/buildtool/execution_api/local", "local_api"]
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/storage", "config"]
     , ["src/buildtool/storage", "storage"]
@@ -27,7 +28,6 @@
     , ["src/buildtool/common", "common"]
     , ["src/buildtool/common", "protocol_traits"]
     , ["src/buildtool/crypto", "hash_function"]
-    , ["src/buildtool/execution_api/local", "local_api"]
     , ["src/buildtool/file_system", "file_system_manager"]
     , ["src/buildtool/file_system", "object_type"]
     , ["src/buildtool/logging", "log_level"]
@@ -100,7 +100,6 @@
   , "stage": ["src", "buildtool", "execution_api", "execution_service"]
   , "deps":
     [ ["@", "gsl", "", "gsl"]
-    , ["src/buildtool/execution_api/common", "api_bundle"]
     , ["src/buildtool/execution_api/local", "context"]
     , ["src/buildtool/execution_api/remote", "context"]
     ]
@@ -118,6 +117,7 @@
     , ["src/buildtool/common", "protocol_traits"]
     , ["src/buildtool/common/remote", "port"]
     , ["src/buildtool/crypto", "hash_function"]
+    , ["src/buildtool/execution_api/local", "local_api"]
     , ["src/buildtool/file_system", "atomic"]
     , ["src/buildtool/logging", "log_level"]
     , ["src/buildtool/logging", "logging"]
diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp
index 6f309021..dd086456 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.cpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.cpp
@@ -37,8 +37,10 @@
 #include "src/buildtool/common/artifact_digest_factory.hpp"
 #include "src/buildtool/common/protocol_traits.hpp"
 #include "src/buildtool/crypto/hash_function.hpp"
+#include "src/buildtool/execution_api/common/execution_response.hpp"
 #include "src/buildtool/execution_api/execution_service/operation_cache.hpp"
 #include "src/buildtool/execution_api/local/local_cas_reader.hpp"
+#include "src/buildtool/execution_api/local/local_response.hpp"
 #include "src/buildtool/file_system/file_system_manager.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
 #include "src/buildtool/logging/log_level.hpp"
@@ -65,8 +67,8 @@ void UpdateTimeStamp(
 }
 
 [[nodiscard]] auto ToBazelActionResult(
-    IExecutionResponse::ArtifactInfos const& artifacts,
-    IExecutionResponse::DirSymlinks const& dir_symlinks,
+    LocalResponse::ArtifactInfos const& artifacts,
+    LocalResponse::DirSymlinks const& dir_symlinks,
     Storage const& storage) noexcept
     -> expected<bazel_re::ActionResult, std::string>;
 
@@ -116,13 +118,13 @@ auto ExecutionServiceImpl::ToIExecutionAction(
 }
 
 auto ExecutionServiceImpl::ToBazelExecuteResponse(
-    IExecutionResponse::Ptr const& i_execution_response) const noexcept
+    gsl::not_null<LocalResponse*> const& local_response) const noexcept
     -> expected<::bazel_re::ExecuteResponse, std::string> {
-    auto artifacts = i_execution_response->Artifacts();
+    auto artifacts = local_response->Artifacts();
     if (not artifacts) {
         return unexpected{std::move(artifacts).error()};
     }
-    auto dir_symlinks = i_execution_response->DirectorySymlinks();
+    auto dir_symlinks = local_response->DirectorySymlinks();
     if (not dir_symlinks) {
         return unexpected{std::move(dir_symlinks).error()};
     }
@@ -135,28 +137,26 @@ auto ExecutionServiceImpl::ToBazelExecuteResponse(
 
     auto action_result = *std::move(result);
 
-    action_result.set_exit_code(i_execution_response->ExitCode());
-    if (i_execution_response->HasStdErr()) {
+    action_result.set_exit_code(local_response->ExitCode());
+    if (local_response->HasStdErr()) {
         auto const cas_digest =
-            storage_.CAS().StoreBlob(i_execution_response->StdErr(),
+            storage_.CAS().StoreBlob(local_response->StdErr(),
                                      /*is_executable=*/false);
         if (not cas_digest) {
-            return unexpected{
-                fmt::format("Could not store stderr of action {}",
-                            i_execution_response->ActionDigest())};
+            return unexpected{fmt::format("Could not store stderr of action {}",
+                                          local_response->ActionDigest())};
         }
         (*action_result.mutable_stderr_digest()) =
             ArtifactDigestFactory::ToBazel(*cas_digest);
     }
 
-    if (i_execution_response->HasStdOut()) {
+    if (local_response->HasStdOut()) {
         auto const cas_digest =
-            storage_.CAS().StoreBlob(i_execution_response->StdOut(),
+            storage_.CAS().StoreBlob(local_response->StdOut(),
                                      /*is_executable=*/false);
         if (not cas_digest) {
-            return unexpected{
-                fmt::format("Could not store stdout of action {}",
-                            i_execution_response->ActionDigest())};
+            return unexpected{fmt::format("Could not store stdout of action {}",
+                                          local_response->ActionDigest())};
         }
         (*action_result.mutable_stdout_digest()) =
             ArtifactDigestFactory::ToBazel(*cas_digest);
@@ -164,7 +164,7 @@ auto ExecutionServiceImpl::ToBazelExecuteResponse(
 
     ::bazel_re::ExecuteResponse bazel_response{};
     (*bazel_response.mutable_result()) = std::move(action_result);
-    bazel_response.set_cached_result(i_execution_response->IsCached());
+    bazel_response.set_cached_result(local_response->IsCached());
 
     // we run the action locally, so no communication issues should happen
     bazel_response.mutable_status()->set_code(grpc::StatusCode::OK);
@@ -241,14 +241,19 @@ auto ExecutionServiceImpl::Execute(
         action_digest->hash(),
         std::chrono::duration_cast<std::chrono::seconds>(t1 - t0).count());
 
-    if (i_execution_response == nullptr) {
-        auto const str =
-            fmt::format("Failed to execute action {}", action_digest->hash());
-        logger_.Emit(LogLevel::Error, "{}", str);
-        return ::grpc::Status{grpc::StatusCode::INTERNAL, str};
+    auto* local_response =
+        dynamic_cast<LocalResponse*>(i_execution_response.get());
+    if (local_response == nullptr) {
+        auto error_msg =
+            (i_execution_response == nullptr)
+                ? fmt::format("Failed to execute action {}",
+                              action_digest->hash())
+                : std::string{"Local action did not produce a local response"};
+        logger_.Emit(LogLevel::Error, "{}", error_msg);
+        return ::grpc::Status{grpc::StatusCode::INTERNAL, error_msg};
     }
 
-    auto execute_response = ToBazelExecuteResponse(i_execution_response);
+    auto execute_response = ToBazelExecuteResponse(local_response);
     if (not execute_response) {
         logger_.Emit(LogLevel::Error, "{}", execute_response.error());
         return ::grpc::Status{grpc::StatusCode::INTERNAL,
@@ -381,8 +386,8 @@ namespace {
 }
 
 [[nodiscard]] auto ToBazelActionResult(
-    IExecutionResponse::ArtifactInfos const& artifacts,
-    IExecutionResponse::DirSymlinks const& dir_symlinks,
+    LocalResponse::ArtifactInfos const& artifacts,
+    LocalResponse::DirSymlinks const& dir_symlinks,
     Storage const& storage) noexcept
     -> expected<bazel_re::ActionResult, std::string> {
     bazel_re::ActionResult result{};
diff --git a/src/buildtool/execution_api/execution_service/execution_server.hpp b/src/buildtool/execution_api/execution_service/execution_server.hpp
index 7e1bcfa1..2ae636da 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.hpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.hpp
@@ -26,20 +26,21 @@
 #include "gsl/gsl"
 #include "src/buildtool/common/bazel_types.hpp"
 #include "src/buildtool/execution_api/common/execution_action.hpp"
-#include "src/buildtool/execution_api/common/execution_api.hpp"
-#include "src/buildtool/execution_api/common/execution_response.hpp"
 #include "src/buildtool/execution_api/execution_service/operation_cache.hpp"
 #include "src/buildtool/execution_api/local/context.hpp"
+#include "src/buildtool/execution_api/local/local_api.hpp"
 #include "src/buildtool/logging/logger.hpp"
 #include "src/buildtool/storage/config.hpp"
 #include "src/buildtool/storage/storage.hpp"
 #include "src/utils/cpp/expected.hpp"
 
+class LocalResponse;
+
 class ExecutionServiceImpl final : public bazel_re::Execution::Service {
   public:
     explicit ExecutionServiceImpl(
         gsl::not_null<LocalContext const*> const& local_context,
-        gsl::not_null<IExecutionApi const*> const& local_api,
+        gsl::not_null<LocalApi const*> const& local_api,
         std::optional<std::uint8_t> op_exponent) noexcept
         : storage_config_{*local_context->storage_config},
           storage_{*local_context->storage},
@@ -137,7 +138,7 @@ class ExecutionServiceImpl final : public bazel_re::Execution::Service {
   private:
     StorageConfig const& storage_config_;
     Storage const& storage_;
-    IExecutionApi const& api_;
+    LocalApi const& api_;
     OperationCache op_cache_;
     Logger logger_{"execution-service"};
 
@@ -146,7 +147,7 @@ class ExecutionServiceImpl final : public bazel_re::Execution::Service {
         const noexcept -> std::optional<IExecutionAction::Ptr>;
 
     [[nodiscard]] auto ToBazelExecuteResponse(
-        IExecutionResponse::Ptr const& i_execution_response) const noexcept
+        gsl::not_null<LocalResponse*> const& local_response) const noexcept
         -> expected<::bazel_re::ExecuteResponse, std::string>;
 
     void WriteResponse(
diff --git a/src/buildtool/execution_api/execution_service/server_implementation.cpp b/src/buildtool/execution_api/execution_service/server_implementation.cpp
index fddc36b6..9b822620 100644
--- a/src/buildtool/execution_api/execution_service/server_implementation.cpp
+++ b/src/buildtool/execution_api/execution_service/server_implementation.cpp
@@ -39,6 +39,7 @@
 #include "src/buildtool/execution_api/execution_service/cas_server.hpp"
 #include "src/buildtool/execution_api/execution_service/execution_server.hpp"
 #include "src/buildtool/execution_api/execution_service/operations_server.hpp"
+#include "src/buildtool/execution_api/local/local_api.hpp"
 #include "src/buildtool/file_system/atomic.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
@@ -75,11 +76,11 @@ auto ServerImpl::Create(std::optional<std::string> interface,
 
 auto ServerImpl::Run(gsl::not_null<LocalContext const*> const& local_context,
                      gsl::not_null<RemoteContext const*> const& remote_context,
-                     ApiBundle const& apis,
+                     gsl::not_null<LocalApi const*> const& local_api,
                      std::optional<std::uint8_t> op_exponent) -> bool {
     auto const hash_type =
         local_context->storage_config->hash_function.GetType();
-    ExecutionServiceImpl es{local_context, &*apis.local, op_exponent};
+    ExecutionServiceImpl es{local_context, local_api, op_exponent};
     ActionCacheServiceImpl ac{local_context};
     CASServiceImpl cas{local_context};
     BytestreamServiceImpl b{local_context};
diff --git a/src/buildtool/execution_api/execution_service/server_implementation.hpp b/src/buildtool/execution_api/execution_service/server_implementation.hpp
index 581365ff..4abf744f 100644
--- a/src/buildtool/execution_api/execution_service/server_implementation.hpp
+++ b/src/buildtool/execution_api/execution_service/server_implementation.hpp
@@ -20,10 +20,11 @@
 #include <string>
 
 #include "gsl/gsl"
-#include "src/buildtool/execution_api/common/api_bundle.hpp"
 #include "src/buildtool/execution_api/local/context.hpp"
 #include "src/buildtool/execution_api/remote/context.hpp"
 
+class LocalApi;
+
 class ServerImpl final {
   public:
     [[nodiscard]] static auto Create(
@@ -44,12 +45,11 @@ class ServerImpl final {
     /// \brief Start the execution service.
     /// \param local_context    The LocalContext to be used.
     /// \param remote_context   The RemoteContext to be used.
-    /// \param apis             Apis to be used, only local api is actually
-    ///                         needed.
+    /// \param local_api        The LocalApi used.
     /// \param op_exponent      Log2 threshold for operation cache.
     auto Run(gsl::not_null<LocalContext const*> const& local_context,
              gsl::not_null<RemoteContext const*> const& remote_context,
-             ApiBundle const& apis,
+             gsl::not_null<LocalApi const*> const& local_api,
              std::optional<std::uint8_t> op_exponent) -> bool;
 
   private:
diff --git a/src/buildtool/main/TARGETS b/src/buildtool/main/TARGETS
index 86ea56a9..72ff2df7 100644
--- a/src/buildtool/main/TARGETS
+++ b/src/buildtool/main/TARGETS
@@ -42,11 +42,13 @@
     , ["src/buildtool/computed_roots", "evaluate"]
     , ["src/buildtool/crypto", "hash_function"]
     , ["src/buildtool/execution_api/common", "api_bundle"]
+    , ["src/buildtool/execution_api/common", "common"]
     , [ "src/buildtool/execution_api/execution_service"
       , "server_implementation"
       ]
     , ["src/buildtool/execution_api/local", "config"]
     , ["src/buildtool/execution_api/local", "context"]
+    , ["src/buildtool/execution_api/local", "local_api"]
     , ["src/buildtool/execution_api/remote", "config"]
     , ["src/buildtool/execution_api/remote", "context"]
     , ["src/buildtool/execution_engine/executor", "context"]
diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp
index d13ad2e7..798336b3 100644
--- a/src/buildtool/main/main.cpp
+++ b/src/buildtool/main/main.cpp
@@ -51,6 +51,7 @@
 #include "src/buildtool/common/repository_config.hpp"
 #include "src/buildtool/common/statistics.hpp"
 #include "src/buildtool/crypto/hash_function.hpp"
+#include "src/buildtool/execution_api/common/execution_api.hpp"
 #include "src/buildtool/file_system/file_root.hpp"
 #include "src/buildtool/file_system/file_system_manager.hpp"
 #include "src/buildtool/logging/log_config.hpp"
@@ -86,6 +87,7 @@
 #include "src/buildtool/execution_api/execution_service/server_implementation.hpp"
 #include "src/buildtool/execution_api/local/config.hpp"
 #include "src/buildtool/execution_api/local/context.hpp"
+#include "src/buildtool/execution_api/local/local_api.hpp"
 #include "src/buildtool/execution_api/remote/config.hpp"
 #include "src/buildtool/execution_api/remote/context.hpp"
 #include "src/buildtool/execution_engine/executor/context.hpp"
@@ -827,10 +829,11 @@ auto main(int argc, char* argv[]) -> int {
                                       &remote_context,
                                       /*repo_config=*/nullptr);
 
-                return execution_server->Run(&local_context,
-                                             &remote_context,
-                                             exec_apis,
-                                             arguments.service.op_exponent)
+                return execution_server->Run(
+                           &local_context,
+                           &remote_context,
+                           dynamic_cast<LocalApi const*>(&*exec_apis.local),
+                           arguments.service.op_exponent)
                            ? kExitSuccess
                            : kExitFailure;
             }
diff --git a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp
index f8c391b6..e9905a6e 100644
--- a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp
+++ b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp
@@ -184,7 +184,9 @@ auto ServeServerImpl::Run(
     // the user has not given any remote-execution endpoint
     // so we start a "just-execute instance" on the same process
     [[maybe_unused]] ExecutionServiceImpl es{
-        local_context, &*apis.local, op_exponent};
+        local_context,
+        dynamic_cast<LocalApi const*>(&*apis.local),
+        op_exponent};
     [[maybe_unused]] ActionCacheServiceImpl ac{local_context};
     [[maybe_unused]] CASServiceImpl cas{local_context};
     [[maybe_unused]] BytestreamServiceImpl b{local_context};