Store HashFunction by value

Although references give an additional information about ownership, they introduce additional design difficulties.

18 files changed, 51 insertions, 55 deletions

diff --git a/src/buildtool/execution_api/common/api_bundle.cpp b/src/buildtool/execution_api/common/api_bundle.cpp
index fa559f7f..69b628bc 100644
--- a/src/buildtool/execution_api/common/api_bundle.cpp
+++ b/src/buildtool/execution_api/common/api_bundle.cpp
@@ -29,7 +29,7 @@ auto ApiBundle::Create(
     gsl::not_null<LocalContext const*> const& local_context,
     gsl::not_null<RemoteContext const*> const& remote_context,
     RepositoryConfig const* repo_config) -> ApiBundle {
-    auto const& hash_fct = local_context->storage_config->hash_function;
+    HashFunction const hash_fct = local_context->storage_config->hash_function;
     IExecutionApi::Ptr local_api =
         std::make_shared<LocalApi>(local_context, repo_config);
     IExecutionApi::Ptr remote_api = local_api;
@@ -42,7 +42,7 @@ auto ApiBundle::Create(
                                                 remote_context->auth,
                                                 remote_context->retry_config,
                                                 config,
-                                                &hash_fct);
+                                                hash_fct);
     }
     return ApiBundle{.hash_function = hash_fct,
                      .local = std::move(local_api),
@@ -63,7 +63,7 @@ auto ApiBundle::MakeRemote(
                                           authentication,
                                           retry_config,
                                           config,
-                                          &hash_function);
+                                          hash_function);
     }
     return local;
 }
diff --git a/src/buildtool/execution_api/common/api_bundle.hpp b/src/buildtool/execution_api/common/api_bundle.hpp
index 19146f66..a08e7bba 100644
--- a/src/buildtool/execution_api/common/api_bundle.hpp
+++ b/src/buildtool/execution_api/common/api_bundle.hpp
@@ -51,7 +51,7 @@ struct ApiBundle final {
         gsl::not_null<RetryConfig const*> const& retry_config) const
         -> gsl::not_null<IExecutionApi::Ptr>;
 
-    HashFunction const& hash_function;
+    HashFunction const hash_function;
     gsl::not_null<IExecutionApi::Ptr> const local;
     gsl::not_null<IExecutionApi::Ptr> const remote;
 };
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
index af8b7052..2dfe07de 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
@@ -137,14 +137,13 @@ namespace {
 
 }  // namespace
 
-BazelApi::BazelApi(
-    std::string const& instance_name,
-    std::string const& host,
-    Port port,
-    gsl::not_null<Auth const*> const& auth,
-    gsl::not_null<RetryConfig const*> const& retry_config,
-    ExecutionConfiguration const& exec_config,
-    gsl::not_null<HashFunction const*> const& hash_function) noexcept {
+BazelApi::BazelApi(std::string const& instance_name,
+                   std::string const& host,
+                   Port port,
+                   gsl::not_null<Auth const*> const& auth,
+                   gsl::not_null<RetryConfig const*> const& retry_config,
+                   ExecutionConfiguration const& exec_config,
+                   HashFunction hash_function) noexcept {
     network_ = std::make_shared<BazelNetwork>(instance_name,
                                               host,
                                               port,
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp
index 2e43d0eb..53445385 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp
@@ -49,7 +49,7 @@ class BazelApi final : public IExecutionApi {
              gsl::not_null<Auth const*> const& auth,
              gsl::not_null<RetryConfig const*> const& retry_config,
              ExecutionConfiguration const& exec_config,
-             gsl::not_null<HashFunction const*> const& hash_function) noexcept;
+             HashFunction hash_function) noexcept;
     BazelApi(BazelApi const&) = delete;
     BazelApi(BazelApi&& other) noexcept;
     auto operator=(BazelApi const&) -> BazelApi& = delete;
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
index d2a84c04..e2b6f244 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
@@ -29,7 +29,7 @@ BazelNetwork::BazelNetwork(
     gsl::not_null<Auth const*> const& auth,
     gsl::not_null<RetryConfig const*> const& retry_config,
     ExecutionConfiguration const& exec_config,
-    gsl::not_null<HashFunction const*> const& hash_function) noexcept
+    HashFunction hash_function) noexcept
     : instance_name_{std::move(instance_name)},
       capabilities_{std::make_unique<BazelCapabilitiesClient>(host,
                                                               port,
@@ -46,7 +46,7 @@ BazelNetwork::BazelNetwork(
                                                    auth,
                                                    retry_config)},
       exec_config_{exec_config},
-      hash_function_{*hash_function} {}
+      hash_function_{hash_function} {}
 
 auto BazelNetwork::IsAvailable(ArtifactDigest const& digest) const noexcept
     -> bool {
@@ -157,7 +157,7 @@ auto BazelNetwork::ExecuteBazelActionSync(
 }
 
 auto BazelNetwork::CreateReader() const noexcept -> BazelNetworkReader {
-    return BazelNetworkReader{instance_name_, cas_.get(), &hash_function_};
+    return BazelNetworkReader{instance_name_, cas_.get(), hash_function_};
 }
 
 auto BazelNetwork::GetCachedActionResult(
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp
index 64fe852f..95faa76e 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp
@@ -39,14 +39,13 @@
 /// \brief Contains all network clients and is responsible for all network IO.
 class BazelNetwork {
   public:
-    explicit BazelNetwork(
-        std::string instance_name,
-        std::string const& host,
-        Port port,
-        gsl::not_null<Auth const*> const& auth,
-        gsl::not_null<RetryConfig const*> const& retry_config,
-        ExecutionConfiguration const& exec_config,
-        gsl::not_null<HashFunction const*> const& hash_function) noexcept;
+    explicit BazelNetwork(std::string instance_name,
+                          std::string const& host,
+                          Port port,
+                          gsl::not_null<Auth const*> const& auth,
+                          gsl::not_null<RetryConfig const*> const& retry_config,
+                          ExecutionConfiguration const& exec_config,
+                          HashFunction hash_function) noexcept;
 
     /// \brief Check if digest exists in CAS
     /// \param[in]  digest  The digest to look up
@@ -99,7 +98,7 @@ class BazelNetwork {
     std::unique_ptr<BazelAcClient> ac_;
     std::unique_ptr<BazelExecutionClient> exec_;
     ExecutionConfiguration exec_config_{};
-    HashFunction const& hash_function_;
+    HashFunction hash_function_;
 
     [[nodiscard]] auto DoUploadBlobs(
         std::unordered_set<ArtifactBlob> blobs) noexcept -> bool;
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network_reader.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network_reader.cpp
index 48cdd414..ed637535 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network_reader.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network_reader.cpp
@@ -34,10 +34,10 @@
 BazelNetworkReader::BazelNetworkReader(
     std::string instance_name,
     gsl::not_null<BazelCasClient const*> const& cas,
-    gsl::not_null<HashFunction const*> const& hash_function) noexcept
+    HashFunction hash_function) noexcept
     : instance_name_{std::move(instance_name)},
       cas_{*cas},
-      hash_function_{*hash_function} {}
+      hash_function_{hash_function} {}
 
 BazelNetworkReader::BazelNetworkReader(
     BazelNetworkReader&& other,
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network_reader.hpp b/src/buildtool/execution_api/remote/bazel/bazel_network_reader.hpp
index e1a75df9..c0ad1cd9 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network_reader.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network_reader.hpp
@@ -39,10 +39,9 @@ class BazelNetworkReader final {
   public:
     using DumpCallback = std::function<bool(std::string const&)>;
 
-    explicit BazelNetworkReader(
-        std::string instance_name,
-        gsl::not_null<BazelCasClient const*> const& cas,
-        gsl::not_null<HashFunction const*> const& hash_function) noexcept;
+    explicit BazelNetworkReader(std::string instance_name,
+                                gsl::not_null<BazelCasClient const*> const& cas,
+                                HashFunction hash_function) noexcept;
 
     BazelNetworkReader(
         BazelNetworkReader&& other,
@@ -84,7 +83,7 @@ class BazelNetworkReader final {
 
     std::string const instance_name_;
     BazelCasClient const& cas_;
-    HashFunction const& hash_function_;
+    HashFunction hash_function_;
     std::optional<DirectoryMap> auxiliary_map_;
 
     [[nodiscard]] auto MakeAuxiliaryMap(
diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp
index 8519639f..3926a330 100644
--- a/src/buildtool/execution_engine/executor/executor.hpp
+++ b/src/buildtool/execution_engine/executor/executor.hpp
@@ -84,7 +84,7 @@ class ExecutorImpl {
         IExecutionApi const& api,
         ExecutionProperties const& merged_properties,
         gsl::not_null<RemoteContext const*> const& remote_context,
-        gsl::not_null<HashFunction const*> const& hash_function,
+        HashFunction hash_function,
         std::chrono::milliseconds const& timeout,
         IExecutionAction::CacheFlag cache_flag,
         gsl::not_null<Statistics*> const& stats,
@@ -752,8 +752,7 @@ class ExecutorImpl {
     [[nodiscard]] static auto GetAlternativeEndpoint(
         const ExecutionProperties& properties,
         const gsl::not_null<RemoteContext const*>& remote_context,
-        const gsl::not_null<HashFunction const*>& hash_function)
-        -> std::unique_ptr<BazelApi> {
+        HashFunction hash_function) -> std::unique_ptr<BazelApi> {
         for (auto const& [pred, endpoint] :
              remote_context->exec_config->dispatch) {
             bool match = true;
@@ -819,7 +818,7 @@ class Executor {
                     context_.remote_context->exec_config->platform_properties,
                     action->ExecutionProperties()),
                 context_.remote_context,
-                &context_.apis->hash_function,
+                context_.apis->hash_function,
                 Impl::ScaleTime(timeout_, action->TimeoutScale()),
                 action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput,
                 context_.statistics,
@@ -842,7 +841,7 @@ class Executor {
                 context_.remote_context->exec_config->platform_properties,
                 action->ExecutionProperties()),
             context_.remote_context,
-            &context_.apis->hash_function,
+            context_.apis->hash_function,
             Impl::ScaleTime(timeout_, action->TimeoutScale()),
             action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput,
             context_.statistics,
@@ -916,7 +915,7 @@ class Rebuilder {
                 context_.remote_context->exec_config->platform_properties,
                 action->ExecutionProperties()),
             context_.remote_context,
-            &context_.apis->hash_function,
+            context_.apis->hash_function,
             Impl::ScaleTime(timeout_, action->TimeoutScale()),
             CF::PretendCached,
             context_.statistics,
@@ -935,7 +934,7 @@ class Rebuilder {
                 context_.remote_context->exec_config->platform_properties,
                 action->ExecutionProperties()),
             context_.remote_context,
-            &context_.apis->hash_function,
+            context_.apis->hash_function,
             Impl::ScaleTime(timeout_, action->TimeoutScale()),
             CF::FromCacheOnly,
             context_.statistics,
diff --git a/src/buildtool/file_system/object_cas.hpp b/src/buildtool/file_system/object_cas.hpp
index ad1da349..5d193b21 100644
--- a/src/buildtool/file_system/object_cas.hpp
+++ b/src/buildtool/file_system/object_cas.hpp
@@ -54,13 +54,13 @@ class ObjectCAS {
     /// \param store_path   The path to use for storing blobs.
     /// \param exists       (optional) Function for checking blob existence.
     explicit ObjectCAS(
-        gsl::not_null<HashFunction const*> const& hash_function,
+        HashFunction hash_function,
         std::filesystem::path const& store_path,
         std::optional<gsl::not_null<ExistsFunc>> exists = std::nullopt)
         : file_store_{store_path},
           exists_{exists.has_value() ? std::move(exists)->get()
                                      : kDefaultExists},
-          hash_function_{*hash_function} {}
+          hash_function_{hash_function} {}
 
     ObjectCAS(ObjectCAS const&) = delete;
     ObjectCAS(ObjectCAS&&) = delete;
@@ -116,7 +116,7 @@ class ObjectCAS {
     FileStorage<kStorageType, StoreMode::FirstWins, /*kSetEpochTime=*/true>
         file_store_;
     gsl::not_null<ExistsFunc> exists_;
-    HashFunction const& hash_function_;
+    HashFunction hash_function_;
 
     /// Default callback for checking blob existence.
     static inline ExistsFunc const kDefaultExists = [](auto const& /*digest*/,
diff --git a/src/buildtool/serve_api/remote/TARGETS b/src/buildtool/serve_api/remote/TARGETS
index be1dae39..a225d91e 100644
--- a/src/buildtool/serve_api/remote/TARGETS
+++ b/src/buildtool/serve_api/remote/TARGETS
@@ -49,6 +49,7 @@
     , ["@", "gsl", "", "gsl"]
     , ["src/buildtool/common", "common"]
     , ["src/buildtool/common/remote", "remote_common"]
+    , ["src/buildtool/crypto", "hash_function"]
     , ["src/buildtool/execution_api/common", "api_bundle"]
     , ["src/buildtool/execution_api/local", "context"]
     , ["src/buildtool/execution_api/remote", "context"]
@@ -61,7 +62,6 @@
     [ ["@", "fmt", "", "fmt"]
     , ["src/buildtool/common", "config"]
     , ["src/buildtool/common", "protocol_traits"]
-    , ["src/buildtool/crypto", "hash_function"]
     , ["src/buildtool/execution_api/common", "common"]
     , ["src/buildtool/execution_api/serve", "mr_git_api"]
     , ["src/buildtool/execution_api/utils", "rehash_utils"]
diff --git a/src/buildtool/serve_api/remote/serve_api.cpp b/src/buildtool/serve_api/remote/serve_api.cpp
index 6eeb85a0..107f4439 100644
--- a/src/buildtool/serve_api/remote/serve_api.cpp
+++ b/src/buildtool/serve_api/remote/serve_api.cpp
@@ -20,7 +20,6 @@
 #include "src/buildtool/common/artifact.hpp"
 #include "src/buildtool/common/protocol_traits.hpp"
 #include "src/buildtool/common/repository_config.hpp"
-#include "src/buildtool/crypto/hash_function.hpp"
 #include "src/buildtool/execution_api/common/execution_api.hpp"
 #include "src/buildtool/execution_api/serve/mr_git_api.hpp"
 #include "src/buildtool/execution_api/utils/rehash_utils.hpp"
diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp
index dc1c9cb9..ce089668 100644
--- a/src/buildtool/serve_api/remote/serve_api.hpp
+++ b/src/buildtool/serve_api/remote/serve_api.hpp
@@ -31,6 +31,7 @@ class ServeApi final {};
 #include "gsl/gsl"
 #include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/common/remote/remote_common.hpp"
+#include "src/buildtool/crypto/hash_function.hpp"
 #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"
@@ -51,7 +52,7 @@ class ServeApi final {
                       gsl::not_null<RemoteContext const*> const& remote_context,
                       gsl::not_null<ApiBundle const*> const& apis) noexcept
         : stc_{address,
-               &local_context->storage_config->hash_function,
+               local_context->storage_config->hash_function,
                remote_context},
           tc_{address, local_context->storage, remote_context, apis},
           cc_{address, remote_context},
diff --git a/src/buildtool/serve_api/remote/source_tree_client.cpp b/src/buildtool/serve_api/remote/source_tree_client.cpp
index bdf3fa57..e24f2db8 100644
--- a/src/buildtool/serve_api/remote/source_tree_client.cpp
+++ b/src/buildtool/serve_api/remote/source_tree_client.cpp
@@ -68,9 +68,9 @@ auto PragmaSpecialToSymlinksResolve(
 
 SourceTreeClient::SourceTreeClient(
     ServerAddress const& address,
-    gsl::not_null<HashFunction const*> const& hash_function,
+    HashFunction hash_function,
     gsl::not_null<RemoteContext const*> const& remote_context) noexcept
-    : hash_function_{*hash_function} {
+    : hash_function_{hash_function} {
     stub_ =
         justbuild::just_serve::SourceTree::NewStub(CreateChannelWithCredentials(
             address.host, address.port, remote_context->auth));
diff --git a/src/buildtool/serve_api/remote/source_tree_client.hpp b/src/buildtool/serve_api/remote/source_tree_client.hpp
index f6a49566..0d31c803 100644
--- a/src/buildtool/serve_api/remote/source_tree_client.hpp
+++ b/src/buildtool/serve_api/remote/source_tree_client.hpp
@@ -37,7 +37,7 @@ class SourceTreeClient {
   public:
     explicit SourceTreeClient(
         ServerAddress const& address,
-        gsl::not_null<HashFunction const*> const& hash_function,
+        HashFunction hash_function,
         gsl::not_null<RemoteContext const*> const& remote_context) noexcept;
 
     struct TreeResult {
@@ -141,7 +141,7 @@ class SourceTreeClient {
         const noexcept -> expected<ArtifactDigest, GitLookupError>;
 
   private:
-    HashFunction const& hash_function_;  // hash function of the remote
+    HashFunction hash_function_;  // hash function of the remote
     std::unique_ptr<justbuild::just_serve::SourceTree::Stub> stub_;
     Logger logger_{"RemoteSourceTreeClient"};
 };
diff --git a/src/buildtool/storage/local_cas.hpp b/src/buildtool/storage/local_cas.hpp
index e4da10f3..de63a008 100644
--- a/src/buildtool/storage/local_cas.hpp
+++ b/src/buildtool/storage/local_cas.hpp
@@ -53,13 +53,13 @@ class LocalCAS {
     explicit LocalCAS(
         GenerationConfig const& config,
         gsl::not_null<Uplinker<kDoGlobalUplink> const*> const& uplinker)
-        : cas_file_{&config.storage_config->hash_function,
+        : cas_file_{config.storage_config->hash_function,
                     config.cas_f,
                     MakeUplinker<ObjectType::File>(config, uplinker)},
-          cas_exec_{&config.storage_config->hash_function,
+          cas_exec_{config.storage_config->hash_function,
                     config.cas_x,
                     MakeUplinker<ObjectType::Executable>(config, uplinker)},
-          cas_tree_{&config.storage_config->hash_function,
+          cas_tree_{config.storage_config->hash_function,
                     config.cas_t,
                     MakeUplinker<ObjectType::Tree>(config, uplinker)},
           cas_file_large_{this, config, uplinker},
@@ -285,7 +285,7 @@ class LocalCAS {
     ObjectCAS<ObjectType::Tree> cas_tree_;
     LargeObjectCAS<kDoGlobalUplink, ObjectType::File> cas_file_large_;
     LargeObjectCAS<kDoGlobalUplink, ObjectType::Tree> cas_tree_large_;
-    HashFunction const& hash_function_;
+    HashFunction hash_function_;
 
     /// \brief Provides uplink via "exists callback" for physical object CAS.
     template <ObjectType kType>
diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp
index 8ac53212..03813b13 100644
--- a/src/other_tools/just_mr/fetch.cpp
+++ b/src/other_tools/just_mr/fetch.cpp
@@ -425,7 +425,7 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
                                                 &*auth_config,
                                                 &*retry_config,
                                                 config,
-                                                &hash_fct);
+                                                hash_fct);
     }
     bool const has_remote_api = remote_api != nullptr;
 
diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp
index 2d6e61ad..427c3288 100644
--- a/src/other_tools/just_mr/setup.cpp
+++ b/src/other_tools/just_mr/setup.cpp
@@ -233,7 +233,7 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config,
                                                 &*auth_config,
                                                 &*retry_config,
                                                 config,
-                                                &hash_fct);
+                                                hash_fct);
     }
     bool const has_remote_api = remote_api != nullptr;