From 07d2416af7bf059456f2cda2537309b521757b9c Mon Sep 17 00:00:00 2001 From: Maksim Denisov Date: Thu, 29 Aug 2024 15:51:14 +0200 Subject: Replace bazel_re::Digest in BazelMsgFactory (trees) ...with ArtifactDigest. --- src/buildtool/execution_api/bazel_msg/TARGETS | 1 + .../execution_api/bazel_msg/bazel_msg_factory.cpp | 44 +++++++++++----------- .../execution_api/bazel_msg/bazel_msg_factory.hpp | 9 +++-- src/buildtool/execution_api/common/common_api.cpp | 9 ++--- src/buildtool/execution_api/local/local_api.hpp | 9 ++--- .../execution_api/remote/bazel/bazel_api.cpp | 5 ++- test/buildtool/execution_api/bazel/TARGETS | 1 - .../execution_api/bazel/bazel_msg_factory.test.cpp | 9 ++--- 8 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/buildtool/execution_api/bazel_msg/TARGETS b/src/buildtool/execution_api/bazel_msg/TARGETS index ddb40899..f5e06ab1 100644 --- a/src/buildtool/execution_api/bazel_msg/TARGETS +++ b/src/buildtool/execution_api/bazel_msg/TARGETS @@ -25,6 +25,7 @@ , "directory_tree" , ["src/buildtool/common", "common"] , ["src/buildtool/execution_engine/dag", "dag"] + , ["src/buildtool/execution_api/common", "artifact_blob_container"] , ["src/buildtool/common", "bazel_types"] , ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp index 27e07c12..532910fb 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp @@ -32,11 +32,6 @@ #include "src/utils/cpp/hex_string.hpp" namespace { -struct DirectoryNodeBundle final { - bazel_re::DirectoryNode const message; - BazelBlob const bazel_blob; -}; - /// \brief Serialize protobuf message to string. template [[nodiscard]] auto SerializeMessage(T const& message) noexcept @@ -122,7 +117,7 @@ template /// instances at once [[nodiscard]] auto CreateSymlinkNodesFromDigests( std::vector const& symlink_names, - std::vector const& symlink_digests, + std::vector const& symlink_digests, BazelMsgFactory::LinkDigestResolveFunc const& resolve_links) -> std::vector { std::vector symlink_targets; @@ -137,6 +132,11 @@ template return symlink_nodes; } +struct DirectoryNodeBundle final { + bazel_re::DirectoryNode message; + ArtifactBlob blob; +}; + /// \brief Create bundle for protobuf message DirectoryNode from Directory. [[nodiscard]] auto CreateDirectoryNodeBundle(std::string const& dir_name, bazel_re::Directory const& dir) @@ -153,7 +153,7 @@ template return DirectoryNodeBundle{ .message = CreateDirectoryNode(dir_name, digest), - .bazel_blob = BazelBlob{ + .blob = ArtifactBlob{ std::move(digest), std::move(*content), /*is_exec=*/false}}; } @@ -239,18 +239,18 @@ template std::vector file_nodes{}; std::vector dir_nodes{}; std::vector symlink_names{}; - std::vector symlink_digests{}; + std::vector symlink_digests{}; try { for (auto const& [name, node] : *tree) { if (std::holds_alternative(node)) { auto const& dir = std::get(node); - auto const dir_bundle = DirectoryTreeToBundle( + auto dir_bundle = DirectoryTreeToBundle( name, dir, resolve_links, process_blob, parent / name); if (not dir_bundle) { return std::nullopt; } - dir_nodes.emplace_back(dir_bundle->message); - if (not process_blob(BazelBlob{dir_bundle->bazel_blob})) { + dir_nodes.emplace_back(std::move(dir_bundle->message)); + if (not process_blob(std::move(dir_bundle->blob))) { return std::nullopt; } } @@ -295,19 +295,21 @@ auto BazelMsgFactory::CreateDirectoryDigestFromTree( DirectoryTreePtr const& tree, LinkDigestResolveFunc const& resolve_links, BlobProcessFunc const& process_blob) noexcept - -> std::optional { - if (auto bundle = - DirectoryTreeToBundle("", tree, resolve_links, process_blob)) { - try { - if (not process_blob(BazelBlob{bundle->bazel_blob})) { - return std::nullopt; - } - } catch (...) { + -> std::optional { + auto bundle = DirectoryTreeToBundle("", tree, resolve_links, process_blob); + if (not bundle) { + return std::nullopt; + } + + auto const digest = bundle->blob.digest; + try { + if (not process_blob(std::move(bundle->blob))) { return std::nullopt; } - return bundle->bazel_blob.digest; + } catch (...) { + return std::nullopt; } - return std::nullopt; + return digest; } auto BazelMsgFactory::CreateDirectoryDigestFromLocalTree( diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp index 0e209153..564c346b 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp @@ -31,6 +31,7 @@ #include "src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp" #include "src/buildtool/execution_api/bazel_msg/bazel_common.hpp" #include "src/buildtool/execution_api/bazel_msg/directory_tree.hpp" +#include "src/buildtool/execution_api/common/artifact_blob_container.hpp" #include "src/buildtool/execution_engine/dag/dag.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" @@ -41,10 +42,10 @@ class BazelMsgFactory { public: /// \brief Store or otherwise process a blob. Returns success flag. - using BlobProcessFunc = std::function; + using BlobProcessFunc = std::function; using LinkDigestResolveFunc = - std::function const&, - std::vector*)>; + std::function const&, + gsl::not_null*> const&)>; using FileStoreFunc = std::function< std::optional(std::filesystem::path const&, bool)>; using SymlinkStoreFunc = @@ -63,7 +64,7 @@ class BazelMsgFactory { DirectoryTreePtr const& tree, LinkDigestResolveFunc const& resolve_links, BlobProcessFunc const& process_blob) noexcept - -> std::optional; + -> std::optional; /// \brief Create Directory digest from local file root. /// Recursively traverse entire root and store files and directories. diff --git a/src/buildtool/execution_api/common/common_api.cpp b/src/buildtool/execution_api/common/common_api.cpp index 74e52c19..319be597 100644 --- a/src/buildtool/execution_api/common/common_api.cpp +++ b/src/buildtool/execution_api/common/common_api.cpp @@ -87,7 +87,7 @@ auto CommonUploadBlobTree(BlobTreePtr const& blob_tree, // Create digest list from blobs for batch availability check. auto missing_blobs_info = GetMissingArtifactsInfo( api, blob_tree->begin(), blob_tree->end(), [](BlobTreePtr const& node) { - return ArtifactDigest{node->Blob().digest}; + return node->Blob().digest; }); if (not missing_blobs_info) { Logger::Log(LogLevel::Error, @@ -133,11 +133,10 @@ auto CommonUploadTreeCompatible( ArtifactBlobContainer blobs{}; // Store and upload blobs, taking into account the maximum transfer size. auto digest = BazelMsgFactory::CreateDirectoryDigestFromTree( - build_root, resolve_links, [&blobs, &api](BazelBlob&& blob) { + build_root, resolve_links, [&blobs, &api](ArtifactBlob&& blob) { return UpdateContainerAndUpload( &blobs, - std::move(ArtifactBlob{ - ArtifactDigest{blob.digest}, blob.data, blob.is_exec}), + std::move(blob), /*exception_is_fatal=*/false, [&api](ArtifactBlobContainer&& container) -> bool { return api.Upload(std::move(container), @@ -159,7 +158,7 @@ auto CommonUploadTreeCompatible( Logger::Log(LogLevel::Debug, "failed to upload blobs for build root."); return std::nullopt; } - return ArtifactDigest{*digest}; + return digest; } auto CommonUploadTreeNative(IExecutionApi const& api, diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp index f1515418..6fd7456a 100644 --- a/src/buildtool/execution_api/local/local_api.hpp +++ b/src/buildtool/execution_api/local/local_api.hpp @@ -289,13 +289,12 @@ class LocalApi final : public IExecutionApi { *this, *build_root, [&cas = local_context_.storage->CAS()]( - std::vector const& digests, - std::vector* targets) { + std::vector const& digests, + gsl::not_null*> const& targets) { targets->reserve(digests.size()); for (auto const& digest : digests) { - auto p = - cas.BlobPath(static_cast(digest), - /*is_executable=*/false); + auto p = cas.BlobPath(digest, + /*is_executable=*/false); auto content = FileSystemManager::ReadFile(*p); targets->emplace_back(*content); } diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp index b76c3877..2f8d5d08 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp @@ -531,8 +531,9 @@ auto BazelApi::CreateAction( return CommonUploadTreeCompatible( *this, *build_root, - [&network = network_](std::vector const& digests, - std::vector* targets) { + [&network = network_]( + std::vector const& digests, + gsl::not_null*> const& targets) { auto reader = network->CreateReader(); targets->reserve(digests.size()); for (auto blobs : reader.ReadIncrementally(digests)) { diff --git a/test/buildtool/execution_api/bazel/TARGETS b/test/buildtool/execution_api/bazel/TARGETS index b0f8e2eb..18a9e012 100644 --- a/test/buildtool/execution_api/bazel/TARGETS +++ b/test/buildtool/execution_api/bazel/TARGETS @@ -103,7 +103,6 @@ , "src/buildtool/execution_api/common" , "artifact_blob_container" ] - , ["@", "src", "src/buildtool/execution_api/bazel_msg", "bazel_msg"] , ["@", "src", "src/buildtool/file_system", "file_system_manager"] ] , "stage": ["test", "buildtool", "execution_api", "bazel"] diff --git a/test/buildtool/execution_api/bazel/bazel_msg_factory.test.cpp b/test/buildtool/execution_api/bazel/bazel_msg_factory.test.cpp index 8fca672b..a775ef4d 100644 --- a/test/buildtool/execution_api/bazel/bazel_msg_factory.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_msg_factory.test.cpp @@ -24,7 +24,6 @@ #include "src/buildtool/common/artifact_digest.hpp" #include "src/buildtool/compatibility/compatibility.hpp" #include "src/buildtool/crypto/hash_function.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/file_system/file_system_manager.hpp" #include "src/buildtool/file_system/object_type.hpp" @@ -105,16 +104,16 @@ TEST_CASE("Bazel internals: MessageFactory", "[execution_api]") { // a mapping between digests and content is needed; usually via a concrete // API one gets this content either locally or from the network - std::unordered_map fake_cas{ + std::unordered_map fake_cas{ {file1_blob->digest, file1}, {file2_blob->digest, file2}, {link_blob->digest, link}}; // create blobs via tree - BazelBlobContainer blobs{}; + ArtifactBlobContainer blobs{}; REQUIRE(BazelMsgFactory::CreateDirectoryDigestFromTree( *tree, - [&fake_cas](std::vector const& digests, + [&fake_cas](std::vector const& digests, std::vector* targets) { targets->reserve(digests.size()); for (auto const& digest : digests) { @@ -133,7 +132,7 @@ TEST_CASE("Bazel internals: MessageFactory", "[execution_api]") { } } }, - [&blobs](BazelBlob&& blob) { + [&blobs](ArtifactBlob&& blob) { blobs.Emplace(std::move(blob)); return true; })); -- cgit v1.2.3