From d5a957afddb68c216184dff0e90e16c083be1134 Mon Sep 17 00:00:00 2001 From: Maksim Denisov Date: Thu, 23 Jan 2025 14:33:45 +0100 Subject: BazelNetworkReader: make reading methods that use bazel digest private --- .../remote/bazel/bazel_network_reader.hpp | 12 ++-- .../execution_api/remote/bazel/bazel_response.cpp | 26 +++++--- test/buildtool/execution_api/bazel/TARGETS | 4 +- .../execution_api/bazel/bazel_network.test.cpp | 74 +++++++++++++--------- 4 files changed, 70 insertions(+), 46 deletions(-) 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 0bfa9a84..54be0008 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_network_reader.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_network_reader.hpp @@ -74,9 +74,6 @@ class BazelNetworkReader final { [[nodiscard]] auto IsNativeProtocol() const noexcept -> bool; - [[nodiscard]] auto ReadSingleBlob(bazel_re::Digest const& digest) - const noexcept -> std::optional; - [[nodiscard]] auto ReadSingleBlob(ArtifactDigest const& digest) const noexcept -> std::optional; @@ -84,9 +81,6 @@ class BazelNetworkReader final { std::vector const& digests) const noexcept -> IncrementalReader; - [[nodiscard]] auto ReadIncrementally(std::vector digests) - const noexcept -> IncrementalReader; - private: using DirectoryMap = std::unordered_map; @@ -100,10 +94,16 @@ class BazelNetworkReader final { std::vector&& full_tree) const noexcept -> std::optional; + [[nodiscard]] auto ReadSingleBlob(bazel_re::Digest const& digest) + const noexcept -> std::optional; + [[nodiscard]] auto BatchReadBlobs( std::vector const& blobs) const noexcept -> std::vector; + [[nodiscard]] auto ReadIncrementally(std::vector digests) + const noexcept -> IncrementalReader; + [[nodiscard]] auto Validate(BazelBlob const& blob) const noexcept -> std::optional; }; diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp index 2ea2ce1b..ad418000 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp @@ -14,12 +14,10 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_response.hpp" -#include #include #include #include #include -#include #include #include @@ -66,9 +64,13 @@ auto ProcessDirectoryMessage(HashFunction hash_function, auto BazelResponse::ReadStringBlob(bazel_re::Digest const& id) noexcept -> std::string { - auto reader = network_->CreateReader(); - if (auto blob = reader.ReadSingleBlob(id)) { - return *blob->data; + auto digest = ArtifactDigestFactory::FromBazel( + network_->GetHashFunction().GetType(), id); + if (digest.has_value()) { + auto reader = network_->CreateReader(); + if (auto blob = reader.ReadSingleBlob(*digest)) { + return *blob->data; + } } Logger::Log(LogLevel::Warning, "reading digest {} from action response failed", @@ -221,13 +223,17 @@ auto BazelResponse::Populate() noexcept -> std::optional { } // obtain tree digests for output directories - std::vector tree_digests{}; + std::vector tree_digests{}; tree_digests.reserve( gsl::narrow(action_result.output_directories_size())); - std::transform(action_result.output_directories().begin(), - action_result.output_directories().end(), - std::back_inserter(tree_digests), - [](auto dir) { return dir.tree_digest(); }); + for (auto const& directory : action_result.output_directories()) { + auto digest = ArtifactDigestFactory::FromBazel( + network_->GetHashFunction().GetType(), directory.tree_digest()); + if (not digest) { + return std::move(digest).error(); + } + tree_digests.push_back(*std::move(digest)); + } // collect root digests from trees and store them auto reader = network_->CreateReader(); diff --git a/test/buildtool/execution_api/bazel/TARGETS b/test/buildtool/execution_api/bazel/TARGETS index 805b9797..570a8111 100644 --- a/test/buildtool/execution_api/bazel/TARGETS +++ b/test/buildtool/execution_api/bazel/TARGETS @@ -82,12 +82,14 @@ [ ["@", "catch2", "", "catch2"] , ["@", "grpc", "", "grpc"] , ["@", "gsl", "", "gsl"] - , ["@", "src", "src/buildtool/common", "bazel_digest_factory"] + , ["@", "src", "src/buildtool/common", "artifact_digest_factory"] , ["@", "src", "src/buildtool/common", "bazel_types"] + , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/common", "protocol_traits"] , ["@", "src", "src/buildtool/common/remote", "remote_common"] , ["@", "src", "src/buildtool/common/remote", "retry_config"] , ["@", "src", "src/buildtool/crypto", "hash_function"] + , ["@", "src", "src/buildtool/crypto", "hash_info"] , ["@", "src", "src/buildtool/execution_api/bazel_msg", "bazel_msg"] , [ "@" , "src" diff --git a/test/buildtool/execution_api/bazel/bazel_network.test.cpp b/test/buildtool/execution_api/bazel/bazel_network.test.cpp index 60393100..5444c5d5 100644 --- a/test/buildtool/execution_api/bazel/bazel_network.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_network.test.cpp @@ -26,12 +26,14 @@ #include "catch2/catch_test_macros.hpp" #include "gsl/gsl" -#include "src/buildtool/common/bazel_digest_factory.hpp" +#include "src/buildtool/common/artifact_digest.hpp" +#include "src/buildtool/common/artifact_digest_factory.hpp" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/protocol_traits.hpp" #include "src/buildtool/common/remote/remote_common.hpp" #include "src/buildtool/common/remote/retry_config.hpp" #include "src/buildtool/crypto/hash_function.hpp" +#include "src/buildtool/crypto/hash_info.hpp" #include "src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp" #include "src/buildtool/execution_api/common/artifact_blob_container.hpp" #include "src/buildtool/execution_api/common/content_blob_container.hpp" @@ -70,25 +72,34 @@ TEST_CASE("Bazel network: write/read blobs", "[execution_api]") { std::string content_bar("bar"); std::string content_baz(kLargeSize, 'x'); // single larger blob - BazelBlob foo{BazelDigestFactory::HashDataAs( - hash_function, content_foo), - content_foo, - /*is_exec=*/false}; - BazelBlob bar{BazelDigestFactory::HashDataAs( - hash_function, content_bar), - content_bar, - /*is_exec=*/false}; - BazelBlob baz{BazelDigestFactory::HashDataAs( - hash_function, content_baz), - content_baz, - /*is_exec=*/false}; + ArtifactBlob foo{ArtifactDigestFactory::HashDataAs( + hash_function, content_foo), + content_foo, + /*is_exec=*/false}; + ArtifactBlob bar{ArtifactDigestFactory::HashDataAs( + hash_function, content_bar), + content_bar, + /*is_exec=*/false}; + ArtifactBlob baz{ArtifactDigestFactory::HashDataAs( + hash_function, content_baz), + content_baz, + /*is_exec=*/false}; + BazelBlob bazel_foo{ArtifactDigestFactory::ToBazel(foo.digest), + foo.data, + /*is_exec=*/false}; + BazelBlob bazel_bar{ArtifactDigestFactory::ToBazel(bar.digest), + content_bar, + /*is_exec=*/false}; + BazelBlob bazel_baz{ArtifactDigestFactory::ToBazel(baz.digest), + content_baz, + /*is_exec=*/false}; // Search blobs via digest - REQUIRE(network.UploadBlobs({foo, bar, baz})); + REQUIRE(network.UploadBlobs({bazel_foo, bazel_bar, bazel_baz})); // Read blobs in order auto reader = network.CreateReader(); - std::vector to_read{ + std::vector to_read{ foo.digest, bar.digest, baz.digest, bar.digest, foo.digest}; std::vector blobs{}; for (auto next : reader.ReadIncrementally(to_read)) { @@ -133,25 +144,30 @@ TEST_CASE("Bazel network: read blobs with unknown size", "[execution_api]") { std::string content_foo("foo"); std::string content_bar(kLargeSize, 'x'); // single larger blob - BazelBlob foo{BazelDigestFactory::HashDataAs( - hash_function, content_foo), - content_foo, - /*is_exec=*/false}; - BazelBlob bar{BazelDigestFactory::HashDataAs( - hash_function, content_bar), - content_bar, - /*is_exec=*/false}; + auto const info_foo = + HashInfo::HashData(hash_function, content_foo, /*is_tree=*/false); + auto const info_bar = + HashInfo::HashData(hash_function, content_bar, /*is_tree=*/false); + + ArtifactBlob foo{ArtifactDigest(info_foo, /*size_unknown=*/0), + content_foo, + /*is_exec=*/false}; + ArtifactBlob bar{ArtifactDigest(info_bar, /*size_unknown=*/0), + content_bar, + /*is_exec=*/false}; + BazelBlob bazel_foo{ArtifactDigestFactory::ToBazel(foo.digest), + foo.data, + /*is_exec=*/false}; + BazelBlob bazel_bar{ArtifactDigestFactory::ToBazel(bar.digest), + content_bar, + /*is_exec=*/false}; // Upload blobs - REQUIRE(network.UploadBlobs({foo, bar})); - - // Set size to unknown - foo.digest.set_size_bytes(0); - bar.digest.set_size_bytes(0); + REQUIRE(network.UploadBlobs({bazel_foo, bazel_bar})); // Read blobs auto reader = network.CreateReader(); - std::vector to_read{foo.digest, bar.digest}; + std::vector to_read{foo.digest, bar.digest}; std::vector blobs{}; for (auto next : reader.ReadIncrementally(to_read)) { blobs.insert(blobs.end(), next.begin(), next.end()); -- cgit v1.2.3