Use RemoteExecutionConfig instances stored in ApiBundle

...wherever an ApiBundle is already being passed.

13 files changed, 72 insertions, 43 deletions

diff --git a/src/buildtool/main/install_cas.cpp b/src/buildtool/main/install_cas.cpp
index 2e1b6917..dcf696ab 100644
--- a/src/buildtool/main/install_cas.cpp
+++ b/src/buildtool/main/install_cas.cpp
@@ -30,17 +30,19 @@
 namespace {
 
 [[nodiscard]] auto InvalidSizeString(std::string const& size_str,
-                                     std::string const& hash) noexcept -> bool {
+                                     std::string const& hash,
+                                     bool has_remote) noexcept -> bool {
     static auto const kEmptyHash = HashFunction::ComputeBlobHash("");
     return Compatibility::IsCompatible() and          // native mode is fine
            (size_str == "0" or size_str.empty()) and  // not "0" or "" is fine
            kEmptyHash.HexString() != hash and         // empty hash is fine
-           RemoteExecutionConfig::RemoteAddress();    // local is fine
+           has_remote;                                // local is fine
 }
 
 }  // namespace
 
-[[nodiscard]] auto ObjectInfoFromLiberalString(std::string const& s) noexcept
+[[nodiscard]] auto ObjectInfoFromLiberalString(std::string const& s,
+                                               bool has_remote) noexcept
     -> Artifact::ObjectInfo {
     std::istringstream iss(s);
     std::string id{};
@@ -56,7 +58,7 @@ namespace {
     if (not iss.eof()) {
         std::getline(iss, type, ']');
     }
-    if (InvalidSizeString(size_str, id)) {
+    if (InvalidSizeString(size_str, id, has_remote)) {
         Logger::Log(
             LogLevel::Warning,
             "{} size in object-id is not supported in compatiblity mode.",
@@ -73,7 +75,8 @@ namespace {
 #ifndef BOOTSTRAP_BUILD_TOOL
 auto FetchAndInstallArtifacts(ApiBundle const& apis,
                               FetchArguments const& clargs) -> bool {
-    auto object_info = ObjectInfoFromLiberalString(clargs.object_id);
+    auto object_info = ObjectInfoFromLiberalString(
+        clargs.object_id, apis.remote_config.RemoteAddress().has_value());
 
     if (clargs.remember) {
         if (not apis.remote->ParallelRetrieveToCas(
diff --git a/src/buildtool/main/install_cas.hpp b/src/buildtool/main/install_cas.hpp
index 3bfdde86..9ca0a638 100644
--- a/src/buildtool/main/install_cas.hpp
+++ b/src/buildtool/main/install_cas.hpp
@@ -24,7 +24,8 @@
 #include "src/buildtool/execution_api/common/api_bundle.hpp"
 #endif
 
-[[nodiscard]] auto ObjectInfoFromLiberalString(std::string const& s) noexcept
+[[nodiscard]] auto ObjectInfoFromLiberalString(std::string const& s,
+                                               bool has_remote) noexcept
     -> Artifact::ObjectInfo;
 
 #ifndef BOOTSTRAP_BUILD_TOOL
diff --git a/src/buildtool/serve_api/remote/TARGETS b/src/buildtool/serve_api/remote/TARGETS
index a1f65ba0..bd0f8646 100644
--- a/src/buildtool/serve_api/remote/TARGETS
+++ b/src/buildtool/serve_api/remote/TARGETS
@@ -85,6 +85,7 @@
     [ ["@", "gsl", "", "gsl"]
     , ["src/buildtool/auth", "auth"]
     , ["src/buildtool/common/remote", "port"]
+    , ["src/buildtool/execution_api/remote", "config"]
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/common/remote", "client_common"]
     , ["src/buildtool/common/remote", "remote_common"]
@@ -93,7 +94,6 @@
   , "stage": ["src", "buildtool", "serve_api", "remote"]
   , "private-deps":
     [ ["@", "json", "", "json"]
-    , ["src/buildtool/execution_api/remote", "config"]
     , ["src/buildtool/logging", "log_level"]
     , ["src/buildtool/serve_api/remote", "config"]
     ]
diff --git a/src/buildtool/serve_api/remote/configuration_client.cpp b/src/buildtool/serve_api/remote/configuration_client.cpp
index f4276aee..46ef4cfc 100644
--- a/src/buildtool/serve_api/remote/configuration_client.cpp
+++ b/src/buildtool/serve_api/remote/configuration_client.cpp
@@ -20,12 +20,11 @@
 #include <optional>
 
 #include "nlohmann/json.hpp"
-#include "src/buildtool/execution_api/remote/config.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/serve_api/remote/config.hpp"
 
 auto ConfigurationClient::CheckServeRemoteExecution() const noexcept -> bool {
-    auto client_remote_address = RemoteExecutionConfig::RemoteAddress();
+    auto const client_remote_address = remote_config_.RemoteAddress();
     if (!client_remote_address) {
         logger_.Emit(LogLevel::Error,
                      "Internal error: the remote execution endpoint should "
diff --git a/src/buildtool/serve_api/remote/configuration_client.hpp b/src/buildtool/serve_api/remote/configuration_client.hpp
index 3d0eb7ff..3ae6d1e6 100644
--- a/src/buildtool/serve_api/remote/configuration_client.hpp
+++ b/src/buildtool/serve_api/remote/configuration_client.hpp
@@ -27,6 +27,7 @@
 #include "src/buildtool/common/remote/client_common.hpp"
 #include "src/buildtool/common/remote/port.hpp"
 #include "src/buildtool/common/remote/remote_common.hpp"
+#include "src/buildtool/execution_api/remote/config.hpp"
 #include "src/buildtool/logging/logger.hpp"
 
 /// Implements client side for Configuration service defined in:
@@ -35,12 +36,15 @@ class ConfigurationClient {
   public:
     explicit ConfigurationClient(
         ServerAddress address,
-        gsl::not_null<Auth const*> const& auth) noexcept
+        gsl::not_null<Auth const*> const& auth,
+        gsl::not_null<RemoteExecutionConfig const*> const&
+            remote_config) noexcept
         : client_serve_address_{std::move(address)},
           stub_{justbuild::just_serve::Configuration::NewStub(
               CreateChannelWithCredentials(client_serve_address_.host,
                                            client_serve_address_.port,
-                                           auth))} {}
+                                           auth))},
+          remote_config_{*remote_config} {}
 
     [[nodiscard]] auto CheckServeRemoteExecution() const noexcept -> bool;
 
@@ -49,6 +53,7 @@ class ConfigurationClient {
   private:
     ServerAddress const client_serve_address_;
     std::unique_ptr<justbuild::just_serve::Configuration::Stub> stub_;
+    RemoteExecutionConfig const& remote_config_;
     Logger logger_{"RemoteConfigurationClient"};
 };
 
diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp
index fd455037..54d06eed 100644
--- a/src/buildtool/serve_api/remote/serve_api.hpp
+++ b/src/buildtool/serve_api/remote/serve_api.hpp
@@ -46,7 +46,7 @@ class ServeApi final {
                       gsl::not_null<ApiBundle const*> const& apis) noexcept
         : stc_{address, &apis->auth},
           tc_{address, storage, apis},
-          cc_{address, &apis->auth} {}
+          cc_{address, &apis->auth, &apis->remote_config} {}
 
     ~ServeApi() noexcept = default;
     ServeApi(ServeApi const&) = delete;
diff --git a/src/buildtool/serve_api/remote/target_client.cpp b/src/buildtool/serve_api/remote/target_client.cpp
index fd886254..d36b518e 100644
--- a/src/buildtool/serve_api/remote/target_client.cpp
+++ b/src/buildtool/serve_api/remote/target_client.cpp
@@ -60,7 +60,7 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key,
     request.mutable_target_cache_key_id()->CopyFrom(key_dgst);
 
     // add execution properties to request
-    for (auto const& [k, v] : RemoteExecutionConfig::PlatformProperties()) {
+    for (auto const& [k, v] : apis_.remote_config.PlatformProperties()) {
         auto* prop = request.add_execution_properties();
         prop->set_name(k);
         prop->set_value(v);
@@ -71,7 +71,7 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key,
     auto dispatch_list = nlohmann::json::array();
     try {
         for (auto const& [props, endpoint] :
-             RemoteExecutionConfig::DispatchList()) {
+             apis_.remote_config.DispatchList()) {
             auto entry = nlohmann::json::array();
             entry.push_back(nlohmann::json(props));
             entry.push_back(endpoint.ToJson());
diff --git a/src/buildtool/serve_api/serve_service/TARGETS b/src/buildtool/serve_api/serve_service/TARGETS
index 6239a0e7..8e1aa21a 100644
--- a/src/buildtool/serve_api/serve_service/TARGETS
+++ b/src/buildtool/serve_api/serve_service/TARGETS
@@ -127,11 +127,10 @@
   , "hdrs": ["configuration.hpp"]
   , "srcs": ["configuration.cpp"]
   , "proto": ["just_serve_proto"]
+  , "deps":
+    [["@", "gsl", "", "gsl"], ["src/buildtool/execution_api/remote", "config"]]
   , "stage": ["src", "buildtool", "serve_api", "serve_service"]
-  , "private-deps":
-    [ ["src/buildtool/compatibility", "compatibility"]
-    , ["src/buildtool/execution_api/remote", "config"]
-    ]
+  , "private-deps": [["src/buildtool/compatibility", "compatibility"]]
   }
 , "target_utils":
   { "type": ["@", "rules", "CC", "library"]
diff --git a/src/buildtool/serve_api/serve_service/configuration.cpp b/src/buildtool/serve_api/serve_service/configuration.cpp
index 4a38cc83..427ee075 100644
--- a/src/buildtool/serve_api/serve_service/configuration.cpp
+++ b/src/buildtool/serve_api/serve_service/configuration.cpp
@@ -16,17 +16,14 @@
 
 #include "src/buildtool/serve_api/serve_service/configuration.hpp"
 
-#include <optional>
-
 #include "src/buildtool/compatibility/compatibility.hpp"
-#include "src/buildtool/execution_api/remote/config.hpp"
 
 auto ConfigurationService::RemoteExecutionEndpoint(
     ::grpc::ServerContext* /*context*/,
     const ::justbuild::just_serve::RemoteExecutionEndpointRequest* /*request*/,
     ::justbuild::just_serve::RemoteExecutionEndpointResponse* response)
     -> ::grpc::Status {
-    auto address = RemoteExecutionConfig::RemoteAddress();
+    auto address = remote_config_.RemoteAddress();
     response->set_address(address ? address->ToJson().dump() : std::string());
     return ::grpc::Status::OK;
 }
diff --git a/src/buildtool/serve_api/serve_service/configuration.hpp b/src/buildtool/serve_api/serve_service/configuration.hpp
index 8a33b5b2..b374148a 100644
--- a/src/buildtool/serve_api/serve_service/configuration.hpp
+++ b/src/buildtool/serve_api/serve_service/configuration.hpp
@@ -15,13 +15,20 @@
 #ifndef INCLUDED_SRC_BUILD_SERVE_API_SERVE_SERVICE_CONFIGURATION_HPP
 #define INCLUDED_SRC_BUILD_SERVE_API_SERVE_SERVICE_CONFIGURATION_HPP
 
+#include "gsl/gsl"
 #include "justbuild/just_serve/just_serve.grpc.pb.h"
+#include "src/buildtool/execution_api/remote/config.hpp"
 
 // This service can be used by the client to double-check the server
 // configuration.
 class ConfigurationService final
     : public justbuild::just_serve::Configuration::Service {
   public:
+    explicit ConfigurationService(
+        gsl::not_null<RemoteExecutionConfig const*> const&
+            remote_config) noexcept
+        : remote_config_{*remote_config} {};
+
     // Returns the address of the associated remote endpoint, if set,
     // or an empty string signaling that the serve endpoint acts also
     // as a remote execution endpoint.
@@ -42,6 +49,9 @@ class ConfigurationService final
         const ::justbuild::just_serve::CompatibilityRequest* request,
         ::justbuild::just_serve::CompatibilityResponse* response)
         -> ::grpc::Status override;
+
+  private:
+    RemoteExecutionConfig const& remote_config_;
 };
 
 #endif  // INCLUDED_SRC_BUILD_SERVE_API_SERVE_SERVICE_CONFIGURATION_HPP
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 cfdb849a..ef8fe0bc 100644
--- a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp
+++ b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp
@@ -115,7 +115,7 @@ auto ServeServerImpl::Run(RemoteServeConfig const& serve_config,
                      &local_exec_config,
                      &apis,
                      serve ? &*serve : nullptr};
-    ConfigurationService cs{};
+    ConfigurationService cs{&apis.remote_config};
 
     grpc::ServerBuilder builder;
 
diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp
index 707ed067..cbd62bda 100644
--- a/src/buildtool/serve_api/serve_service/target.cpp
+++ b/src/buildtool/serve_api/serve_service/target.cpp
@@ -138,7 +138,7 @@ auto TargetService::ServeTarget(
     }
 
     // start filling in the backend description
-    auto const address = RemoteExecutionConfig::RemoteAddress();
+    auto const address = apis_.remote_config.RemoteAddress();
     // read in the execution properties.
     // Important: we will need to pass these platform properties also to the
     // executor (via the graph_traverser) in order for the build to be properly
@@ -480,7 +480,7 @@ auto TargetService::ServeTarget(
                                    &local_exec_config_,
                                    &repository_config,
                                    &apis_.auth,
-                                   &RemoteExecutionConfig::Instance()};
+                                   &apis_.remote_config};
         GraphTraverser const traverser{
             std::move(traverser_args),
             &repository_config,
diff --git a/test/buildtool/main/install_cas.test.cpp b/test/buildtool/main/install_cas.test.cpp
index 4d6ef0b8..05de9c0d 100644
--- a/test/buildtool/main/install_cas.test.cpp
+++ b/test/buildtool/main/install_cas.test.cpp
@@ -22,32 +22,47 @@ TEST_CASE("ObjectInfoFromLiberalString", "[artifact]") {
     auto expected_as_tree = *Artifact::ObjectInfo::FromString(
         "[5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:0:t]");
 
+    // Check (default) file hashes
     CHECK(ObjectInfoFromLiberalString(
-              "[5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f]") == expected);
+              "[5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f]",
+              /*has_remote=*/false) == expected);
     CHECK(ObjectInfoFromLiberalString(
-              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f]") == expected);
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f]",
+              /*has_remote=*/false) == expected);
     CHECK(ObjectInfoFromLiberalString(
-              "[5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f") == expected);
+              "[5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f",
+              /*has_remote=*/false) == expected);
     CHECK(ObjectInfoFromLiberalString(
-              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f") == expected);
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:f",
+              /*has_remote=*/false) == expected);
     CHECK(ObjectInfoFromLiberalString(
-              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:file") == expected);
-    CHECK(ObjectInfoFromLiberalString("5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689"
-                                      ":11:notavalidletter") == expected);
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:file",
+              /*has_remote=*/false) == expected);
+    CHECK(ObjectInfoFromLiberalString(
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:11:notavalidletter",
+              /*has_remote=*/false) == expected);
 
     // Without size, which is not honored in equality
-    CHECK(ObjectInfoFromLiberalString(
-              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689") == expected);
-    CHECK(ObjectInfoFromLiberalString(
-              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:") == expected);
+    CHECK(
+        ObjectInfoFromLiberalString("5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689",
+                                    /*has_remote=*/false) == expected);
+    CHECK(
+        ObjectInfoFromLiberalString("5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:",
+                                    /*has_remote=*/false) == expected);
+
     // Syntactically invalid size should be ignored
     CHECK(ObjectInfoFromLiberalString(
-              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:xyz") == expected);
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:xyz",
+              /*has_remote=*/false) == expected);
 
-    CHECK(ObjectInfoFromLiberalString("5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689"
-                                      "::t") == expected_as_tree);
-    CHECK(ObjectInfoFromLiberalString("5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689"
-                                      "::tree") == expected_as_tree);
-    CHECK(ObjectInfoFromLiberalString("5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689"
-                                      ":xyz:t") == expected_as_tree);
+    // Check tree hashes
+    CHECK(ObjectInfoFromLiberalString(
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689::t",
+              /*has_remote=*/false) == expected_as_tree);
+    CHECK(ObjectInfoFromLiberalString(
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689::tree",
+              /*has_remote=*/false) == expected_as_tree);
+    CHECK(ObjectInfoFromLiberalString(
+              "5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689:xyz:t",
+              /*has_remote=*/false) == expected_as_tree);
 }