Return ArtifactDigest from RepositoryConfig::RepositoryKey

...to prevent additional conversions to ArtifactDigest from plain strings.

8 files changed, 19 insertions, 17 deletions

diff --git a/src/buildtool/common/TARGETS b/src/buildtool/common/TARGETS
index e6d4e126..aff9861d 100644
--- a/src/buildtool/common/TARGETS
+++ b/src/buildtool/common/TARGETS
@@ -156,6 +156,7 @@
     , ["src/buildtool/multithreading", "atomic_value"]
     , ["src/buildtool/storage", "storage"]
     , ["src/buildtool/crypto", "hash_function"]
+    , ["src/buildtool/common", "common"]
     ]
   , "stage": ["src", "buildtool", "common"]
   , "private-deps": [["src/utils/automata", "dfa_minimizer"]]
diff --git a/src/buildtool/common/repository_config.cpp b/src/buildtool/common/repository_config.cpp
index 52694d0b..804bec2e 100644
--- a/src/buildtool/common/repository_config.cpp
+++ b/src/buildtool/common/repository_config.cpp
@@ -36,18 +36,16 @@ auto RepositoryConfig::RepositoryInfo::BaseContentDescription() const
 
 auto RepositoryConfig::RepositoryKey(Storage const& storage,
                                      std::string const& repo) const noexcept
-    -> std::optional<std::string> {
+    -> std::optional<ArtifactDigest> {
     auto const unique = DeduplicateRepo(repo, storage.GetHashFunction());
     if (auto const* data = Data(unique)) {
         // compute key only once (thread-safe)
         return data->key.SetOnceAndGet(
-            [this, &storage, &unique]() -> std::optional<std::string> {
+            [this, &storage, &unique]() -> std::optional<ArtifactDigest> {
                 if (auto graph = BuildGraphForRepository(
                         unique, storage.GetHashFunction())) {
                     auto const& cas = storage.CAS();
-                    if (auto digest = cas.StoreBlob(graph->dump(2))) {
-                        return digest->hash();
-                    }
+                    return cas.StoreBlob(graph->dump(2));
                 }
                 return std::nullopt;
             });
diff --git a/src/buildtool/common/repository_config.hpp b/src/buildtool/common/repository_config.hpp
index 34918d51..3319a710 100644
--- a/src/buildtool/common/repository_config.hpp
+++ b/src/buildtool/common/repository_config.hpp
@@ -25,6 +25,7 @@
 
 #include "gsl/gsl"
 #include "nlohmann/json.hpp"
+#include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/crypto/hash_function.hpp"
 #include "src/buildtool/file_system/file_root.hpp"
 #include "src/buildtool/file_system/git_cas.hpp"
@@ -140,7 +141,7 @@ class RepositoryConfig {
     // std::nullopt otherwise.
     [[nodiscard]] auto RepositoryKey(Storage const& storage,
                                      std::string const& repo) const noexcept
-        -> std::optional<std::string>;
+        -> std::optional<ArtifactDigest>;
 
     // used for testing
     void Reset() {
@@ -159,7 +160,7 @@ class RepositoryConfig {
         // Base description if content-fixed
         std::optional<nlohmann::json> base_desc{};
         // Cache key if content-fixed
-        AtomicValue<std::optional<std::string>> key{};
+        AtomicValue<std::optional<ArtifactDigest>> key{};
     };
 
     std::unordered_map<std::string, RepositoryData> repos_;
diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp
index f3861eb7..23c87982 100644
--- a/src/buildtool/serve_api/remote/serve_api.hpp
+++ b/src/buildtool/serve_api/remote/serve_api.hpp
@@ -139,8 +139,8 @@ class ServeApi final {
     }
 
     [[nodiscard]] auto ServeTarget(const TargetCacheKey& key,
-                                   const std::string& repo_key) const noexcept
-        -> std::optional<serve_target_result_t> {
+                                   const ArtifactDigest& repo_key)
+        const noexcept -> std::optional<serve_target_result_t> {
         return tc_.ServeTarget(key, repo_key);
     }
 
diff --git a/src/buildtool/serve_api/remote/target_client.cpp b/src/buildtool/serve_api/remote/target_client.cpp
index 1404fe6e..32d7377f 100644
--- a/src/buildtool/serve_api/remote/target_client.cpp
+++ b/src/buildtool/serve_api/remote/target_client.cpp
@@ -56,7 +56,7 @@ TargetClient::TargetClient(
 }
 
 auto TargetClient::ServeTarget(const TargetCacheKey& key,
-                               const std::string& repo_key) const noexcept
+                               const ArtifactDigest& repo_key) const noexcept
     -> std::optional<serve_target_result_t> {
     // make sure the blob containing the key is in the remote cas
     if (not apis_.local->RetrieveToCas({key.Id()}, *apis_.remote)) {
@@ -67,12 +67,13 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key,
     }
     // make sure the repository configuration blob is in the remote cas
     if (not apis_.local->RetrieveToCas(
-            {Artifact::ObjectInfo{.digest = ArtifactDigest{repo_key, 0, false},
+            {Artifact::ObjectInfo{.digest = repo_key,
                                   .type = ObjectType::File}},
             *apis_.remote)) {
         return serve_target_result_t{
             std::in_place_index<1>,
-            fmt::format("Failed to retrieve to remote cas blob {}", repo_key)};
+            fmt::format("Failed to retrieve to remote cas blob {}",
+                        repo_key.hash())};
     }
 
     // add target cache key to request
diff --git a/src/buildtool/serve_api/remote/target_client.hpp b/src/buildtool/serve_api/remote/target_client.hpp
index e6ca07ff..7e0fb726 100644
--- a/src/buildtool/serve_api/remote/target_client.hpp
+++ b/src/buildtool/serve_api/remote/target_client.hpp
@@ -68,8 +68,8 @@ class TargetClient {
     /// \returns A correspondingly populated result union, or nullopt if remote
     /// reported that the target was not found.
     [[nodiscard]] auto ServeTarget(const TargetCacheKey& key,
-                                   const std::string& repo_key) const noexcept
-        -> std::optional<serve_target_result_t>;
+                                   const ArtifactDigest& repo_key)
+        const noexcept -> std::optional<serve_target_result_t>;
 
     /// \brief Retrieve the flexible config variables of an export target.
     /// \param[in] target_root_id Hash of target-level root tree.
diff --git a/src/buildtool/storage/target_cache.hpp b/src/buildtool/storage/target_cache.hpp
index 6feff6a2..81750bf0 100644
--- a/src/buildtool/storage/target_cache.hpp
+++ b/src/buildtool/storage/target_cache.hpp
@@ -27,6 +27,7 @@
 #include "src/buildtool/build_engine/base_maps/entity_name_data.hpp"
 #include "src/buildtool/build_engine/expression/configuration.hpp"
 #include "src/buildtool/common/artifact.hpp"
+#include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/file_system/file_storage.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
 #include "src/buildtool/logging/logger.hpp"
@@ -92,7 +93,7 @@ class TargetCache {
     /// Doesn't create a TargetCacheEntry in the TargetCache.
     /// \return TargetCacheKey on success.
     [[nodiscard]] auto ComputeKey(
-        std::string const& repo_key,
+        ArtifactDigest const& repo_key,
         BuildMaps::Base::NamedTarget const& target_name,
         Configuration const& effective_config) const noexcept
         -> std::optional<TargetCacheKey>;
diff --git a/src/buildtool/storage/target_cache.tpp b/src/buildtool/storage/target_cache.tpp
index 4026238f..be1b9560 100644
--- a/src/buildtool/storage/target_cache.tpp
+++ b/src/buildtool/storage/target_cache.tpp
@@ -43,14 +43,14 @@ auto TargetCache<kDoGlobalUplink>::Store(
 
 template <bool kDoGlobalUplink>
 auto TargetCache<kDoGlobalUplink>::ComputeKey(
-    std::string const& repo_key,
+    ArtifactDigest const& repo_key,
     BuildMaps::Base::NamedTarget const& target_name,
     Configuration const& effective_config) const noexcept
     -> std::optional<TargetCacheKey> {
     try {
         // target's repository is content-fixed, we can compute a cache key
         auto target_desc = nlohmann::json{
-            {"repo_key", repo_key},
+            {"repo_key", repo_key.hash()},
             {"target_name",
              nlohmann::json{target_name.module, target_name.name}.dump()},
             {"effective_config", effective_config.ToString()}};