diff options
Diffstat (limited to 'src')
5 files changed, 58 insertions, 63 deletions
diff --git a/src/buildtool/execution_api/common/execution_response.hpp b/src/buildtool/execution_api/common/execution_response.hpp index 3ad4e2ba..57e1f087 100644 --- a/src/buildtool/execution_api/common/execution_response.hpp +++ b/src/buildtool/execution_api/common/execution_response.hpp @@ -56,13 +56,12 @@ class IExecutionResponse { [[nodiscard]] virtual auto StdOut() noexcept -> std::string = 0; - [[nodiscard]] virtual auto ActionDigest() const noexcept -> std::string = 0; + [[nodiscard]] virtual auto ActionDigest() const noexcept + -> std::string const& = 0; - [[nodiscard]] virtual auto Artifacts() noexcept -> ArtifactInfos = 0; - - // Artifacts plus extra useful info - [[nodiscard]] virtual auto ArtifactsWithDirSymlinks() noexcept - -> std::pair<ArtifactInfos, DirSymlinks> = 0; + [[nodiscard]] virtual auto Artifacts() noexcept -> ArtifactInfos const& = 0; + [[nodiscard]] virtual auto DirectorySymlinks() noexcept + -> DirSymlinks const& = 0; }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_REMOTE_EXECUTION_RESPONSE_HPP diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp index 132a812f..c99cfa04 100644 --- a/src/buildtool/execution_api/execution_service/execution_server.cpp +++ b/src/buildtool/execution_api/execution_service/execution_server.cpp @@ -281,15 +281,16 @@ static auto CreateTreeDigestFromDirectoryDigest( static auto AddOutputPaths(::bazel_re::ExecuteResponse* response, IExecutionResponse::Ptr const& execution, Storage const& storage) noexcept -> bool { - auto const& artifacts_plus = execution->ArtifactsWithDirSymlinks(); - auto const& size = static_cast<int>(artifacts_plus.first.size()); + auto const& artifacts = execution->Artifacts(); + auto const& dir_symlinks = execution->DirectorySymlinks(); + auto const size = static_cast<int>(artifacts.size()); response->mutable_result()->mutable_output_files()->Reserve(size); response->mutable_result()->mutable_output_file_symlinks()->Reserve(size); response->mutable_result()->mutable_output_directory_symlinks()->Reserve( size); response->mutable_result()->mutable_output_directories()->Reserve(size); - for (auto const& [path, info] : artifacts_plus.first) { + for (auto const& [path, info] : artifacts) { auto dgst = static_cast<::bazel_re::Digest>(info.digest); if (info.type == ObjectType::Tree) { @@ -326,7 +327,7 @@ static auto AddOutputPaths(::bazel_re::ExecuteResponse* response, return false; } *(out_link.mutable_target()) = *content; - if (artifacts_plus.second.contains(path)) { + if (dir_symlinks.contains(path)) { // directory symlink response->mutable_result() ->mutable_output_directory_symlinks() diff --git a/src/buildtool/execution_api/local/local_response.hpp b/src/buildtool/execution_api/local/local_response.hpp index 4871097a..21dd5952 100644 --- a/src/buildtool/execution_api/local/local_response.hpp +++ b/src/buildtool/execution_api/local/local_response.hpp @@ -67,31 +67,27 @@ class LocalResponse final : public IExecutionResponse { } auto IsCached() const noexcept -> bool final { return output_.is_cached; }; - auto ActionDigest() const noexcept -> std::string final { + auto ActionDigest() const noexcept -> std::string const& final { return action_id_; } - auto Artifacts() noexcept -> ArtifactInfos final { - return ArtifactsWithDirSymlinks().first; + auto Artifacts() noexcept -> ArtifactInfos const& final { + Populate(); + return artifacts_; } - auto ArtifactsWithDirSymlinks() noexcept - -> std::pair<ArtifactInfos, DirSymlinks> final { - // make sure to populate only once - populated_ = Populate(); - if (not populated_) { - return {}; - } - return std::pair(artifacts_, dir_symlinks_); - }; + auto DirectorySymlinks() noexcept -> DirSymlinks const& final { + Populate(); + return dir_symlinks_; + } private: std::string action_id_{}; LocalAction::Output output_{}; Storage const& storage_; - ArtifactInfos artifacts_{}; - DirSymlinks dir_symlinks_{}; - bool populated_{false}; + ArtifactInfos artifacts_; + DirSymlinks dir_symlinks_; + bool populated_ = false; explicit LocalResponse( std::string action_id, @@ -101,10 +97,13 @@ class LocalResponse final : public IExecutionResponse { output_{std::move(output)}, storage_{*storage} {} - [[nodiscard]] auto Populate() noexcept -> bool { + void Populate() noexcept { + // Initialized only once lazily if (populated_) { - return true; + return; } + populated_ = true; + ArtifactInfos artifacts{}; auto const& action_result = output_.action; artifacts.reserve( @@ -116,7 +115,7 @@ class LocalResponse final : public IExecutionResponse { static_cast<std::size_t>(action_result.output_directories_size())); DirSymlinks dir_symlinks{}; - dir_symlinks_.reserve(static_cast<std::size_t>( + dir_symlinks.reserve(static_cast<std::size_t>( action_result.output_directory_symlinks_size())); // collect files and store them @@ -129,7 +128,7 @@ class LocalResponse final : public IExecutionResponse { .type = file.is_executable() ? ObjectType::Executable : ObjectType::File}); } catch (...) { - return false; + return; } } @@ -143,7 +142,7 @@ class LocalResponse final : public IExecutionResponse { storage_.GetHashFunction(), link.target()), .type = ObjectType::Symlink}); } catch (...) { - return false; + return; } } for (auto const& link : action_result.output_directory_symlinks()) { @@ -156,7 +155,7 @@ class LocalResponse final : public IExecutionResponse { .type = ObjectType::Symlink}); dir_symlinks.emplace(link.path()); // add it to set } catch (...) { - return false; + return; } } @@ -169,12 +168,11 @@ class LocalResponse final : public IExecutionResponse { .digest = ArtifactDigest{dir.tree_digest()}, .type = ObjectType::Tree}); } catch (...) { - return false; + return; } } artifacts_ = std::move(artifacts); dir_symlinks_ = std::move(dir_symlinks); - return true; } }; diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp index ae9f00e3..914836fe 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp @@ -50,24 +50,23 @@ auto BazelResponse::ReadStringBlob(bazel_re::Digest const& id) noexcept return std::string{}; } -auto BazelResponse::Artifacts() noexcept -> ArtifactInfos { - return ArtifactsWithDirSymlinks().first; +auto BazelResponse::Artifacts() noexcept -> ArtifactInfos const& { + Populate(); + return artifacts_; } -auto BazelResponse::ArtifactsWithDirSymlinks() noexcept - -> std::pair<ArtifactInfos, DirSymlinks> { - // make sure to populate only once - populated_ = Populate(); - if (not populated_) { - return {}; - } - return std::pair(artifacts_, dir_symlinks_); +auto BazelResponse::DirectorySymlinks() noexcept -> DirSymlinks const& { + Populate(); + return dir_symlinks_; } -auto BazelResponse::Populate() noexcept -> bool { +void BazelResponse::Populate() noexcept { + // Initialized only once lazily if (populated_) { - return true; + return; } + populated_ = true; + ArtifactInfos artifacts{}; auto const& action_result = output_.action_result; artifacts.reserve( @@ -91,7 +90,7 @@ auto BazelResponse::Populate() noexcept -> bool { ? ObjectType::Executable : ObjectType::File}); } catch (...) { - return false; + return; } } @@ -105,7 +104,7 @@ auto BazelResponse::Populate() noexcept -> bool { network_->GetHashFunction(), link.target()), .type = ObjectType::Symlink}); } catch (...) { - return false; + return; } } for (auto const& link : action_result.output_directory_symlinks()) { @@ -118,7 +117,7 @@ auto BazelResponse::Populate() noexcept -> bool { .type = ObjectType::Symlink}); dir_symlinks.emplace(link.path()); // add it to set } catch (...) { - return false; + return; } } @@ -133,12 +132,12 @@ auto BazelResponse::Populate() noexcept -> bool { .digest = ArtifactDigest{tree.tree_digest()}, .type = ObjectType::Tree}); } catch (...) { - return false; + return; } } artifacts_ = std::move(artifacts); dir_symlinks_ = std::move(dir_symlinks); - return true; + return; } // obtain tree digests for output directories @@ -159,7 +158,7 @@ auto BazelResponse::Populate() noexcept -> bool { auto tree = BazelMsgFactory::MessageFromString<bazel_re::Tree>( *tree_blob.data); if (not tree) { - return false; + return; } // The server does not store the Directory messages it just @@ -168,21 +167,20 @@ auto BazelResponse::Populate() noexcept -> bool { // have to upload them manually. auto root_digest = UploadTreeMessageDirectories(*tree); if (not root_digest) { - return false; + return; } artifacts.emplace( action_result.output_directories(pos).path(), Artifact::ObjectInfo{.digest = *root_digest, .type = ObjectType::Tree}); } catch (...) { - return false; + return; } ++pos; } } artifacts_ = std::move(artifacts); dir_symlinks_ = std::move(dir_symlinks); - return true; } auto BazelResponse::UploadTreeMessageDirectories( diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp index 30a70d7f..6e37d344 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp @@ -15,6 +15,7 @@ #ifndef INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BAZEL_RESPONSE_HPP #define INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BAZEL_RESPONSE_HPP +#include <optional> #include <string> #include <utility> // std::move #include <vector> @@ -53,22 +54,20 @@ class BazelResponse final : public IExecutionResponse { return output_.cached_result; }; - auto ActionDigest() const noexcept -> std::string final { + auto ActionDigest() const noexcept -> std::string const& final { return action_id_; } - auto Artifacts() noexcept -> ArtifactInfos final; - - auto ArtifactsWithDirSymlinks() noexcept - -> std::pair<ArtifactInfos, DirSymlinks> final; + auto Artifacts() noexcept -> ArtifactInfos const& final; + auto DirectorySymlinks() noexcept -> DirSymlinks const& final; private: std::string action_id_{}; std::shared_ptr<BazelNetwork> const network_{}; BazelExecutionClient::ExecutionOutput output_{}; - ArtifactInfos artifacts_{}; - DirSymlinks dir_symlinks_{}; - bool populated_{false}; + ArtifactInfos artifacts_; + DirSymlinks dir_symlinks_; + bool populated_ = false; explicit BazelResponse(std::string action_id, std::shared_ptr<BazelNetwork> network, @@ -85,7 +84,7 @@ class BazelResponse final : public IExecutionResponse { return id.size_bytes() != 0; } - [[nodiscard]] auto Populate() noexcept -> bool; + void Populate() noexcept; [[nodiscard]] auto UploadTreeMessageDirectories( bazel_re::Tree const& tree) const -> std::optional<ArtifactDigest>; |