Store HashInfo as a field in ArtifactDigest

...and adjust ArtifactDigestFactory accordingly.

4 files changed, 21 insertions, 39 deletions

diff --git a/src/buildtool/common/TARGETS b/src/buildtool/common/TARGETS
index e8554984..6e768cee 100644
--- a/src/buildtool/common/TARGETS
+++ b/src/buildtool/common/TARGETS
@@ -68,11 +68,7 @@
     , ["src/utils/cpp", "expected"]
     ]
   , "private-deps":
-    [ "bazel_digest_factory"
-    , "bazel_types"
-    , ["@", "gsl", "", "gsl"]
-    , ["src/buildtool/compatibility", "compatibility"]
-    ]
+    ["bazel_digest_factory", "bazel_types", ["@", "gsl", "", "gsl"]]
   , "stage": ["src", "buildtool", "common"]
   }
 , "common":
@@ -93,7 +89,6 @@
     , ["src/buildtool/logging", "log_level"]
     , ["src/buildtool/logging", "logging"]
     , ["src/utils/cpp", "hash_combine"]
-    , ["src/utils/cpp", "gsl"]
     , ["@", "json", "", "json"]
     ]
   , "stage": ["src", "buildtool", "common"]
diff --git a/src/buildtool/common/artifact_digest.hpp b/src/buildtool/common/artifact_digest.hpp
index d633b433..f02b161d 100644
--- a/src/buildtool/common/artifact_digest.hpp
+++ b/src/buildtool/common/artifact_digest.hpp
@@ -16,44 +16,43 @@
 #define INCLUDED_SRC_COMMON_ARTIFACT_DIGEST_HPP
 
 #include <cstddef>
-#include <optional>
 #include <string>
 #include <utility>  // std::move
 
-#include "gsl/gsl"
 #include "src/buildtool/crypto/hash_info.hpp"
-#include "src/utils/cpp/gsl.hpp"
 #include "src/utils/cpp/hash_combine.hpp"
 
 // Provides getter for size with convenient non-protobuf type. Contains an
 // unprefixed hex string as hash.
 class ArtifactDigest final {
+    friend class ArtifactDigestFactory;
+
   public:
     ArtifactDigest() noexcept = default;
 
-    explicit ArtifactDigest(HashInfo const& hash_info,
-                            std::size_t size) noexcept
-        : size_{size}, hash_{hash_info.Hash()}, is_tree_{hash_info.IsTree()} {}
+    explicit ArtifactDigest(HashInfo hash_info, std::size_t size) noexcept
+        : hash_info_{std::move(hash_info)}, size_{size} {}
 
     [[nodiscard]] auto hash() const& noexcept -> std::string const& {
-        return hash_;
+        return hash_info_.Hash();
     }
 
     [[nodiscard]] auto hash() && noexcept -> std::string {
-        return std::move(hash_);
+        return std::move(hash_info_).Hash();
     }
 
     [[nodiscard]] auto size() const noexcept -> std::size_t { return size_; }
-    [[nodiscard]] auto IsTree() const noexcept -> bool { return is_tree_; }
+    [[nodiscard]] auto IsTree() const noexcept -> bool {
+        return hash_info_.IsTree();
+    }
 
     [[nodiscard]] auto operator==(ArtifactDigest const& other) const -> bool {
-        return hash_ == other.hash_ and is_tree_ == other.is_tree_;
+        return hash_info_ == other.hash_info_;
     }
 
   private:
-    std::size_t size_{};
-    std::string hash_{};
-    bool is_tree_{};
+    HashInfo hash_info_{};
+    std::size_t size_ = 0;
 };
 
 namespace std {
diff --git a/src/buildtool/common/artifact_digest_factory.cpp b/src/buildtool/common/artifact_digest_factory.cpp
index 0f95f444..dd9da28d 100644
--- a/src/buildtool/common/artifact_digest_factory.cpp
+++ b/src/buildtool/common/artifact_digest_factory.cpp
@@ -17,7 +17,6 @@
 #include "gsl/gsl"
 #include "src/buildtool/common/bazel_digest_factory.hpp"
 #include "src/buildtool/common/bazel_types.hpp"
-#include "src/buildtool/compatibility/compatibility.hpp"
 
 auto ArtifactDigestFactory::Create(HashFunction::Type hash_type,
                                    std::string hash,
@@ -30,7 +29,7 @@ auto ArtifactDigestFactory::Create(HashFunction::Type hash_type,
     if (not hash_info) {
         return unexpected{std::move(hash_info).error()};
     }
-    return ArtifactDigest{*hash_info, size};
+    return ArtifactDigest{*std::move(hash_info), size};
 }
 
 auto ArtifactDigestFactory::FromBazel(HashFunction::Type hash_type,
@@ -40,23 +39,12 @@ auto ArtifactDigestFactory::FromBazel(HashFunction::Type hash_type,
     if (not hash_info) {
         return unexpected{std::move(hash_info).error()};
     }
-    return ArtifactDigest{*hash_info,
+    return ArtifactDigest{*std::move(hash_info),
                           static_cast<std::size_t>(digest.size_bytes())};
 }
 
 auto ArtifactDigestFactory::ToBazel(ArtifactDigest const& digest)
     -> bazel_re::Digest {
-    static constexpr std::size_t kGitSHA1Length = 40;
-    // TODO(denisov): The type of the bazel digest produced here doesn't have to
-    // be the same as Compatibility::IsCompatible returns, since we're
-    // computing hashes with a predefined type in many places. Once
-    // ArtifactDigest gets HashInfo as a field, it will be taken from there
-    // directly, but for now it is 'guessed' based on the length of the hash.
-    auto const type = digest.hash().size() == kGitSHA1Length
-                          ? HashFunction::Type::GitSHA1
-                          : HashFunction::Type::PlainSHA256;
-    auto const hash_info =
-        HashInfo::Create(type, digest.hash(), digest.IsTree());
-    return BazelDigestFactory::Create(*hash_info,
-                                      gsl::narrow<std::int64_t>(digest.size()));
+    return BazelDigestFactory::Create(digest.hash_info_,
+                                      gsl::narrow<std::int64_t>(digest.size_));
 }
diff --git a/src/buildtool/common/artifact_digest_factory.hpp b/src/buildtool/common/artifact_digest_factory.hpp
index 8e28ae83..51425eab 100644
--- a/src/buildtool/common/artifact_digest_factory.hpp
+++ b/src/buildtool/common/artifact_digest_factory.hpp
@@ -74,9 +74,9 @@ class ArtifactDigestFactory final {
     [[nodiscard]] static auto HashDataAs(HashFunction hash_function,
                                          std::string const& content) noexcept
         -> ArtifactDigest {
-        auto const hash_info =
+        auto hash_info =
             HashInfo::HashData(hash_function, content, IsTreeObject(kType));
-        return ArtifactDigest{hash_info, content.size()};
+        return ArtifactDigest{std::move(hash_info), content.size()};
     }
 
     /// \brief Hash file using hash function and return a valid ArtifactDigest
@@ -89,12 +89,12 @@ class ArtifactDigestFactory final {
         HashFunction hash_function,
         std::filesystem::path const& path) noexcept
         -> std::optional<ArtifactDigest> {
-        auto const hash_info =
+        auto hash_info =
             HashInfo::HashFile(hash_function, path, IsTreeObject(kType));
         if (not hash_info) {
             return std::nullopt;
         }
-        return ArtifactDigest{hash_info->first,
+        return ArtifactDigest{std::move(hash_info->first),
                               static_cast<std::size_t>(hash_info->second)};
     }
 };