From 47eeba4a4a7302662f4c39ebcc8e15a7876f8227 Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Mon, 9 Oct 2023 15:30:31 +0200 Subject: just serve: Fix inconsistencies - add missing serve_api TARGETS file - rename service client to align with server naming scheme - fix inconsistencies in comments between implementation and protocol --- src/buildtool/serve_api/TARGETS | 1 + src/buildtool/serve_api/remote/TARGETS | 13 +++--- src/buildtool/serve_api/remote/serve_api.cpp | 4 +- src/buildtool/serve_api/remote/serve_api.hpp | 6 +-- .../remote/serve_target_level_cache_client.cpp | 51 ---------------------- .../remote/serve_target_level_cache_client.hpp | 46 ------------------- .../serve_api/remote/source_tree_client.cpp | 50 +++++++++++++++++++++ .../serve_api/remote/source_tree_client.hpp | 46 +++++++++++++++++++ src/buildtool/serve_api/serve_service/TARGETS | 1 - .../serve_api/serve_service/source_tree.cpp | 12 ++--- .../serve_api/serve_service/source_tree.hpp | 14 ++---- 11 files changed, 117 insertions(+), 127 deletions(-) create mode 100644 src/buildtool/serve_api/TARGETS delete mode 100644 src/buildtool/serve_api/remote/serve_target_level_cache_client.cpp delete mode 100644 src/buildtool/serve_api/remote/serve_target_level_cache_client.hpp create mode 100644 src/buildtool/serve_api/remote/source_tree_client.cpp create mode 100644 src/buildtool/serve_api/remote/source_tree_client.hpp (limited to 'src') diff --git a/src/buildtool/serve_api/TARGETS b/src/buildtool/serve_api/TARGETS new file mode 100644 index 00000000..0967ef42 --- /dev/null +++ b/src/buildtool/serve_api/TARGETS @@ -0,0 +1 @@ +{} diff --git a/src/buildtool/serve_api/remote/TARGETS b/src/buildtool/serve_api/remote/TARGETS index 45114f57..9b038440 100644 --- a/src/buildtool/serve_api/remote/TARGETS +++ b/src/buildtool/serve_api/remote/TARGETS @@ -5,11 +5,11 @@ , "deps": [["src/buildtool/common/remote", "remote_common"]] , "stage": ["src", "buildtool", "serve_api", "remote"] } -, "serve_target_level_cache_client": +, "source_tree_client": { "type": ["@", "rules", "CC", "library"] - , "name": ["serve_target_level_cache_client"] - , "hdrs": ["serve_target_level_cache_client.hpp"] - , "srcs": ["serve_target_level_cache_client.cpp"] + , "name": ["source_tree_client"] + , "hdrs": ["source_tree_client.hpp"] + , "srcs": ["source_tree_client.cpp"] , "deps": [ ["src/buildtool/common/remote", "port"] , ["src/buildtool/logging", "logging"] @@ -23,10 +23,7 @@ , "name": ["serve_api"] , "hdrs": ["serve_api.hpp"] , "srcs": ["serve_api.cpp"] - , "deps": - [ ["src/buildtool/common/remote", "port"] - , "serve_target_level_cache_client" - ] + , "deps": [["src/buildtool/common/remote", "port"], "source_tree_client"] , "stage": ["src", "buildtool", "serve_api", "remote"] } } diff --git a/src/buildtool/serve_api/remote/serve_api.cpp b/src/buildtool/serve_api/remote/serve_api.cpp index 7b57d7d0..0b0493d7 100644 --- a/src/buildtool/serve_api/remote/serve_api.cpp +++ b/src/buildtool/serve_api/remote/serve_api.cpp @@ -15,7 +15,7 @@ #include "src/buildtool/serve_api/remote/serve_api.hpp" ServeApi::ServeApi(std::string const& host, Port port) noexcept - : tlc_{std::make_unique(host, port)} {} + : stc_{std::make_unique(host, port)} {} // implement move constructor in cpp, where all members are complete types ServeApi::ServeApi(ServeApi&& other) noexcept = default; @@ -27,5 +27,5 @@ auto ServeApi::RetrieveTreeFromCommit(std::string const& commit, std::string const& subdir, bool sync_tree) -> std::optional { - return tlc_->ServeCommitTree(commit, subdir, sync_tree); + return stc_->ServeCommitTree(commit, subdir, sync_tree); } diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp index d3fc29d1..d206b416 100644 --- a/src/buildtool/serve_api/remote/serve_api.hpp +++ b/src/buildtool/serve_api/remote/serve_api.hpp @@ -20,7 +20,7 @@ #include #include "src/buildtool/common/remote/port.hpp" -#include "src/buildtool/serve_api/remote/serve_target_level_cache_client.hpp" +#include "src/buildtool/serve_api/remote/source_tree_client.hpp" class ServeApi final { public: @@ -42,8 +42,8 @@ class ServeApi final { -> std::optional; private: - // target-level cache service client - std::unique_ptr tlc_; + // source tree service client + std::unique_ptr stc_; }; #endif // INCLUDED_SRC_BUILDTOOL_SERVE_API_REMOTE_SERVE_API_HPP diff --git a/src/buildtool/serve_api/remote/serve_target_level_cache_client.cpp b/src/buildtool/serve_api/remote/serve_target_level_cache_client.cpp deleted file mode 100644 index b414b7ac..00000000 --- a/src/buildtool/serve_api/remote/serve_target_level_cache_client.cpp +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/buildtool/serve_api/remote/serve_target_level_cache_client.hpp" - -#include "src/buildtool/common/remote/client_common.hpp" - -ServeTargetLevelCacheClient::ServeTargetLevelCacheClient( - std::string const& server, - Port port) noexcept { - stub_ = justbuild::just_serve::SourceTree::NewStub( - CreateChannelWithCredentials(server, port)); -} - -auto ServeTargetLevelCacheClient::ServeCommitTree(std::string const& commit_id, - std::string const& subdir, - bool sync_tree) - -> std::optional { - justbuild::just_serve::ServeCommitTreeRequest request{}; - request.set_commit(commit_id); - request.set_subdir(subdir); - request.set_sync_tree(sync_tree); - - grpc::ClientContext context; - justbuild::just_serve::ServeCommitTreeResponse response; - grpc::Status status = stub_->ServeCommitTree(&context, request, &response); - - if (not status.ok()) { - LogStatus(&logger_, LogLevel::Debug, status); - return std::nullopt; - } - if (response.status() != - ::justbuild::just_serve::ServeCommitTreeResponse::OK) { - logger_.Emit(LogLevel::Debug, - "ServeCommitTree response returned with {}", - static_cast(response.status())); - return std::nullopt; - } - return response.tree(); -} diff --git a/src/buildtool/serve_api/remote/serve_target_level_cache_client.hpp b/src/buildtool/serve_api/remote/serve_target_level_cache_client.hpp deleted file mode 100644 index 551759cb..00000000 --- a/src/buildtool/serve_api/remote/serve_target_level_cache_client.hpp +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef INCLUDED_SRC_BUILDTOOL_SERVE_API_REMOTE_SERVE_TLC_CLIENT_HPP -#define INCLUDED_SRC_BUILDTOOL_SERVE_API_REMOTE_SERVE_TLC_CLIENT_HPP - -#include -#include - -#include "justbuild/just_serve/just_serve.grpc.pb.h" -#include "src/buildtool/common/remote/port.hpp" -#include "src/buildtool/logging/logger.hpp" - -/// Implements client side for service defined in: -/// src/buildtool/serve_api/serve_service/just_serve.proto -class ServeTargetLevelCacheClient { - public: - ServeTargetLevelCacheClient(std::string const& server, Port port) noexcept; - - /// \brief Retrieve the Git tree of a given commit, if known by the remote. - /// \param[in] commit_id Hash of the Git commit to look up. - /// \param[in] subdir Relative path of the tree inside commit. - /// \param[in] sync_tree Sync tree to the remote-execution end point - /// \returns The hash of the tree if commit found, nullopt otherwise. - [[nodiscard]] auto ServeCommitTree(std::string const& commit_id, - std::string const& subdir, - bool sync_tree) - -> std::optional; - - private: - std::unique_ptr stub_; - Logger logger_{"RemoteTLCClient"}; -}; - -#endif // INCLUDED_SRC_BUILDTOOL_SERVE_API_REMOTE_SERVE_TLC_CLIENT_HPP diff --git a/src/buildtool/serve_api/remote/source_tree_client.cpp b/src/buildtool/serve_api/remote/source_tree_client.cpp new file mode 100644 index 00000000..2026d80d --- /dev/null +++ b/src/buildtool/serve_api/remote/source_tree_client.cpp @@ -0,0 +1,50 @@ +// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/buildtool/serve_api/remote/source_tree_client.hpp" + +#include "src/buildtool/common/remote/client_common.hpp" + +SourceTreeClient::SourceTreeClient(std::string const& server, + Port port) noexcept { + stub_ = justbuild::just_serve::SourceTree::NewStub( + CreateChannelWithCredentials(server, port)); +} + +auto SourceTreeClient::ServeCommitTree(std::string const& commit_id, + std::string const& subdir, + bool sync_tree) + -> std::optional { + justbuild::just_serve::ServeCommitTreeRequest request{}; + request.set_commit(commit_id); + request.set_subdir(subdir); + request.set_sync_tree(sync_tree); + + grpc::ClientContext context; + justbuild::just_serve::ServeCommitTreeResponse response; + grpc::Status status = stub_->ServeCommitTree(&context, request, &response); + + if (not status.ok()) { + LogStatus(&logger_, LogLevel::Debug, status); + return std::nullopt; + } + if (response.status() != + ::justbuild::just_serve::ServeCommitTreeResponse::OK) { + logger_.Emit(LogLevel::Debug, + "ServeCommitTree response returned with {}", + static_cast(response.status())); + return std::nullopt; + } + return response.tree(); +} diff --git a/src/buildtool/serve_api/remote/source_tree_client.hpp b/src/buildtool/serve_api/remote/source_tree_client.hpp new file mode 100644 index 00000000..1b557163 --- /dev/null +++ b/src/buildtool/serve_api/remote/source_tree_client.hpp @@ -0,0 +1,46 @@ +// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef INCLUDED_SRC_BUILDTOOL_SERVE_API_SOURCE_TREE_CLIENT_HPP +#define INCLUDED_SRC_BUILDTOOL_SERVE_API_SOURCE_TREE_CLIENT_HPP + +#include +#include + +#include "justbuild/just_serve/just_serve.grpc.pb.h" +#include "src/buildtool/common/remote/port.hpp" +#include "src/buildtool/logging/logger.hpp" + +/// Implements client side for SourceTree service defined in: +/// src/buildtool/serve_api/serve_service/just_serve.proto +class SourceTreeClient { + public: + SourceTreeClient(std::string const& server, Port port) noexcept; + + /// \brief Retrieve the Git tree of a given commit, if known by the remote. + /// \param[in] commit_id Hash of the Git commit to look up. + /// \param[in] subdir Relative path of the tree inside commit. + /// \param[in] sync_tree Sync tree to the remote-execution end point + /// \returns The hash of the tree if commit found, nullopt otherwise. + [[nodiscard]] auto ServeCommitTree(std::string const& commit_id, + std::string const& subdir, + bool sync_tree) + -> std::optional; + + private: + std::unique_ptr stub_; + Logger logger_{"RemoteSourceTreeClient"}; +}; + +#endif // INCLUDED_SRC_BUILDTOOL_SERVE_API_SOURCE_TREE_CLIENT_HPP diff --git a/src/buildtool/serve_api/serve_service/TARGETS b/src/buildtool/serve_api/serve_service/TARGETS index f3d78414..bc8edfd0 100644 --- a/src/buildtool/serve_api/serve_service/TARGETS +++ b/src/buildtool/serve_api/serve_service/TARGETS @@ -3,7 +3,6 @@ , "name": ["just_serve_proto"] , "service": ["yes"] , "srcs": ["just_serve.proto"] - , "deps": [["@", "bazel_remote_apis", "", "remote_execution_proto"]] , "stage": ["justbuild", "just_serve"] } , "source_tree": diff --git a/src/buildtool/serve_api/serve_service/source_tree.cpp b/src/buildtool/serve_api/serve_service/source_tree.cpp index cb3f6220..c8315325 100644 --- a/src/buildtool/serve_api/serve_service/source_tree.cpp +++ b/src/buildtool/serve_api/serve_service/source_tree.cpp @@ -24,7 +24,7 @@ #include "src/buildtool/serve_api/remote/config.hpp" #include "src/buildtool/storage/config.hpp" -auto SourceTreeService::GetTreeFromCommit( +auto SourceTreeService::GetSubtreeFromCommit( std::filesystem::path const& repo_path, std::string const& commit, std::string const& subdir, @@ -36,8 +36,8 @@ auto SourceTreeService::GetTreeFromCommit( [logger, repo_path, commit](auto const& msg, bool fatal) { if (fatal) { auto err = fmt::format( - "ServeCommitTree: While retrieving tree of commit " - "{} from repository {}:\n{}", + "ServeCommitTree: While retrieving subtree of " + "commit {} from repository {}:\n{}", commit, repo_path.string(), msg); @@ -63,7 +63,7 @@ auto SourceTreeService::ServeCommitTree( auto const& commit{request->commit()}; auto const& subdir{request->subdir()}; // try in local build root Git cache - if (auto tree_id = GetTreeFromCommit( + if (auto tree_id = GetSubtreeFromCommit( StorageConfig::GitRoot(), commit, subdir, logger_)) { if (request->sync_tree()) { // sync tree with remote CAS @@ -95,8 +95,8 @@ auto SourceTreeService::ServeCommitTree( } // try given extra repositories, in order for (auto const& path : RemoteServeConfig::KnownRepositories()) { - if (auto tree_id = GetTreeFromCommit(path, commit, subdir, logger_)) { - + if (auto tree_id = + GetSubtreeFromCommit(path, commit, subdir, logger_)) { if (request->sync_tree()) { // sync tree with remote CAS auto digest = ArtifactDigest{*tree_id, 0, /*is_tree=*/true}; diff --git a/src/buildtool/serve_api/serve_service/source_tree.hpp b/src/buildtool/serve_api/serve_service/source_tree.hpp index 089844a1..a74de455 100644 --- a/src/buildtool/serve_api/serve_service/source_tree.hpp +++ b/src/buildtool/serve_api/serve_service/source_tree.hpp @@ -33,15 +33,9 @@ class SourceTreeService final : public justbuild::just_serve::SourceTree::Service { public: - // Retrieve the tree of a commit. + // Retrieve the Git-subtree identifier from a given Git commit. // - // This request interrogates the service whether it knows a given Git - // commit. If requested commit is found, it provides the commit's - // Git-tree identifier. - // - // Errors: - // - // * `NOT_FOUND`: The requested commit could not be found. + // There are no method-specific errors. auto ServeCommitTree( ::grpc::ServerContext* context, const ::justbuild::just_serve::ServeCommitTreeRequest* request, @@ -58,11 +52,11 @@ class SourceTreeService final gsl::not_null const local_api_{ CreateExecutionApi(std::nullopt)}; - [[nodiscard]] static auto GetTreeFromCommit( + [[nodiscard]] static auto GetSubtreeFromCommit( std::filesystem::path const& repo_path, std::string const& commit, std::string const& subdir, std::shared_ptr const& logger) -> std::optional; }; -#endif +#endif // INCLUDED_SRC_BUILDTOOL_SERVE_API_SERVE_SERVICE_SOURCE_TREE_HPP -- cgit v1.2.3