diff options
| -rw-r--r-- | src/buildtool/build_engine/target_map/absent_target_map.cpp | 34 | ||||
| -rw-r--r-- | src/buildtool/build_engine/target_map/export.cpp | 40 | ||||
| -rw-r--r-- | src/buildtool/serve_api/remote/TARGETS | 1 | ||||
| -rw-r--r-- | src/buildtool/serve_api/remote/serve_api.hpp | 2 | ||||
| -rw-r--r-- | src/buildtool/serve_api/remote/target_client.cpp | 128 | ||||
| -rw-r--r-- | src/buildtool/serve_api/remote/target_client.hpp | 20 |
6 files changed, 163 insertions, 62 deletions
diff --git a/src/buildtool/build_engine/target_map/absent_target_map.cpp b/src/buildtool/build_engine/target_map/absent_target_map.cpp index 39e3d3bb..917ddd2a 100644 --- a/src/buildtool/build_engine/target_map/absent_target_map.cpp +++ b/src/buildtool/build_engine/target_map/absent_target_map.cpp @@ -52,7 +52,7 @@ auto BuildMaps::Target::CreateAbsentTargetMap( key.target.GetNamedTarget().name); if (!flexible_vars) { (*logger)(fmt::format("Failed to obtain flexible config variables " - "for absent target \"{}\"", + "for absent target {}", key.target.ToString()), /*fatal=*/true); return; @@ -89,8 +89,36 @@ auto BuildMaps::Target::CreateAbsentTargetMap( key.target.ToString()); exports_progress->TaskTracker().Start( target_cache_key->Id().ToString()); - target_cache_value = - ServeApi::ServeTarget(*target_cache_key, *repo_key); + auto res = ServeApi::ServeTarget(*target_cache_key, *repo_key); + // process response from serve endpoint + if (not res) { + // report target not found + (*logger)(fmt::format("Absent target {} was not found on serve " + "endpoint", + key.target.ToString()), + /*fatal=*/true); + return; + } + if (res->index() == 0) { + (*logger)( + fmt::format("Failure to remotely analyse or build absent " + "target {}\nDetailed log available on the " + "remote-execution endpoint as blob {}", + key.target.ToString(), + std::get<0>(*res)), + /*fatal=*/true); + return; + } + if (res->index() == 1) { + (*logger)(fmt::format("While querying serve endpoint for " + "absent export target {}:\n{}", + key.target.ToString(), + std::get<1>(*res)), + /*fatal=*/true); + return; + } + // index == 2 + target_cache_value = std::get<2>(*res); exports_progress->TaskTracker().Stop( target_cache_key->Id().ToString()); from_just_serve = true; diff --git a/src/buildtool/build_engine/target_map/export.cpp b/src/buildtool/build_engine/target_map/export.cpp index 3ed86dd1..b8b5b1db 100644 --- a/src/buildtool/build_engine/target_map/export.cpp +++ b/src/buildtool/build_engine/target_map/export.cpp @@ -135,11 +135,43 @@ void ExportRule( auto task = fmt::format( "[{},{}]", key.target.ToString(), effective_config.ToString()); exports_progress->TaskTracker().Start(task); - - target_cache_value = - ServeApi::ServeTarget(*target_cache_key, *repo_key); + auto res = ServeApi::ServeTarget(*target_cache_key, *repo_key); + // process response from serve endpoint + if (not res) { + // target not found: log to performance, and continue + Logger::Log(LogLevel::Performance, + "Export target {} not known to serve endpoint", + key.target.ToString()); + } + else { + if (res->index() == 0) { + // target found but failed to analyse/build: this should be + // a fatal error for the local build too + (*logger)( + fmt::format("Failure to remotely analyse or build " + "target {}\nDetailed log available on the " + "remote-execution endpoint as blob {}", + key.target.ToString(), + std::get<0>(*res)), + /*fatal=*/true); + return; + } + if (res->index() == 1) { + // some other failure occurred while querying the serve + // endpoint; log to debug and continue locally + Logger::Log(LogLevel::Debug, + "While querying serve endpoint for export " + "target {}:\n{}", + key.target.ToString(), + std::get<1>(*res)); + } + else { + // index == 2 + target_cache_value = std::get<2>(*res); + from_just_serve = true; + } + } exports_progress->TaskTracker().Stop(task); - from_just_serve = true; } #endif // BOOTSTRAP_BUILD_TOOL diff --git a/src/buildtool/serve_api/remote/TARGETS b/src/buildtool/serve_api/remote/TARGETS index 1e875bbd..e039cfdb 100644 --- a/src/buildtool/serve_api/remote/TARGETS +++ b/src/buildtool/serve_api/remote/TARGETS @@ -58,6 +58,7 @@ , "private-deps": [ ["src/buildtool/common/remote", "client_common"] , ["src/buildtool/common", "bazel_types"] + , ["@", "fmt", "", "fmt"] , ["@", "json", "", "json"] ] } diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp index 219fe7ab..055cf78b 100644 --- a/src/buildtool/serve_api/remote/serve_api.hpp +++ b/src/buildtool/serve_api/remote/serve_api.hpp @@ -111,7 +111,7 @@ class ServeApi final { [[nodiscard]] static auto ServeTarget(const TargetCacheKey& key, const std::string& repo_key) noexcept - -> std::optional<std::pair<TargetCacheEntry, Artifact::ObjectInfo>> { + -> std::optional<serve_target_result_t> { return Instance().tc_->ServeTarget(key, repo_key); } diff --git a/src/buildtool/serve_api/remote/target_client.cpp b/src/buildtool/serve_api/remote/target_client.cpp index bee31249..d64f24c8 100644 --- a/src/buildtool/serve_api/remote/target_client.cpp +++ b/src/buildtool/serve_api/remote/target_client.cpp @@ -17,6 +17,7 @@ #include <exception> #include <utility> +#include "fmt/core.h" #include "nlohmann/json.hpp" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/remote/client_common.hpp" @@ -29,23 +30,22 @@ TargetClient::TargetClient(std::string const& server, Port port) noexcept { auto TargetClient::ServeTarget(const TargetCacheKey& key, const std::string& repo_key) noexcept - -> std::optional<std::pair<TargetCacheEntry, Artifact::ObjectInfo>> { + -> std::optional<serve_target_result_t> { // make sure the blob containing the key is in the remote cas if (!local_api_->RetrieveToCas({key.Id()}, &*remote_api_)) { - logger_.Emit(LogLevel::Performance, - "failed to retrieve to remote cas ObjectInfo {}", - key.Id().ToString()); - return std::nullopt; + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Failed to retrieve to remote cas ObjectInfo {}", + key.Id().ToString())}; } // make sure the repository configuration blob is in the remote cas if (!local_api_->RetrieveToCas( {Artifact::ObjectInfo{.digest = ArtifactDigest{repo_key, 0, false}, .type = ObjectType::File}}, &*remote_api_)) { - logger_.Emit(LogLevel::Performance, - "failed to retrieve to remote cas blob {}", - repo_key); - return std::nullopt; + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Failed to retrieve to remote cas blob {}", repo_key)}; } // add target cache key to request @@ -72,27 +72,27 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key, dispatch_list.push_back(entry); } } catch (std::exception const& ex) { - logger_.Emit(LogLevel::Performance, - "populating dispatch JSON array failed with:\n{}", - ex.what()); - return std::nullopt; + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Populating dispatch JSON array failed with:\n{}", + ex.what())}; } auto dispatch_dgst = Storage::Instance().CAS().StoreBlob(dispatch_list.dump(2)); if (not dispatch_dgst) { - logger_.Emit(LogLevel::Performance, - "failed to store blob {} to local cas", - dispatch_list.dump(2)); - return std::nullopt; + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Failed to store blob {} to local cas", + dispatch_list.dump(2))}; } auto const& dispatch_info = Artifact::ObjectInfo{ .digest = ArtifactDigest{*dispatch_dgst}, .type = ObjectType::File}; if (!local_api_->RetrieveToCas({dispatch_info}, &*remote_api_)) { - logger_.Emit(LogLevel::Performance, - "failed to upload blob {} to remote cas", - dispatch_info.ToString()); - return std::nullopt; + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Failed to upload blob {} to remote cas", + dispatch_info.ToString())}; } request.mutable_dispatch_info()->CopyFrom(*dispatch_dgst); @@ -101,39 +101,67 @@ auto TargetClient::ServeTarget(const TargetCacheKey& key, justbuild::just_serve::ServeTargetResponse response; auto const& status = stub_->ServeTarget(&context, request, &response); if (!status.ok()) { - LogStatus(&logger_, LogLevel::Performance, status); - return std::nullopt; + return serve_target_result_t{std::in_place_index<1>, + StatusString(status)}; } - auto const& target_value_dgst = ArtifactDigest{response.target_value()}; - auto const& obj_info = Artifact::ObjectInfo{.digest = target_value_dgst, - .type = ObjectType::File}; - if (!local_api_->IsAvailable(target_value_dgst)) { - if (!remote_api_->RetrieveToCas({obj_info}, &*local_api_)) { - logger_.Emit(LogLevel::Performance, - "failed to retrieve blob {} from remote cas", - obj_info.ToString()); - return std::nullopt; + // differentiate status codes + switch (status.error_code()) { + case grpc::StatusCode::OK: { + // if log has been set, pass it along as index 0 + if (response.has_log()) { + return serve_target_result_t{ + std::in_place_index<0>, + ArtifactDigest(response.log()).hash()}; + } + // if no log has been set, it must have the target cache value + if (not response.has_target_value()) { + return serve_target_result_t{ + std::in_place_index<1>, + "Serve endpoint failed to set expected response field"}; + } + auto const& target_value_dgst = + ArtifactDigest{response.target_value()}; + auto const& obj_info = Artifact::ObjectInfo{ + .digest = target_value_dgst, .type = ObjectType::File}; + if (!local_api_->IsAvailable(target_value_dgst)) { + if (!remote_api_->RetrieveToCas({obj_info}, &*local_api_)) { + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format( + "Failed to retrieve blob {} from remote cas", + obj_info.ToString())}; + } + } + auto const& target_value_str = + local_api_->RetrieveToMemory(obj_info); + if (!target_value_str) { + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Failed to retrieve blob {} from local cas", + obj_info.ToString())}; + } + try { + auto const& result = TargetCacheEntry::FromJson( + nlohmann::json::parse(*target_value_str)); + // return the target cache value information + return serve_target_result_t{std::in_place_index<2>, + std::make_pair(result, obj_info)}; + } catch (std::exception const& ex) { + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Parsing target cache value failed with:\n{}", + ex.what())}; + } } + case grpc::StatusCode::NOT_FOUND: { + return std::nullopt; // this might allow a local build to continue + } + default:; // fallthrough } - - auto const& target_value_str = local_api_->RetrieveToMemory(obj_info); - if (!target_value_str) { - logger_.Emit(LogLevel::Performance, - "failed to retrieve blob {} from local cas", - obj_info.ToString()); - return std::nullopt; - } - try { - auto const& result = TargetCacheEntry::FromJson( - nlohmann::json::parse(*target_value_str)); - return std::make_pair(result, obj_info); - } catch (std::exception const& ex) { - logger_.Emit(LogLevel::Performance, - "parsing target cache value failed with:\n{}", - ex.what()); - return std::nullopt; - } + return serve_target_result_t{ + std::in_place_index<1>, + fmt::format("Serve endpoint failed with:\n{}", status.error_message())}; } auto TargetClient::ServeTargetVariables(std::string const& target_root_id, diff --git a/src/buildtool/serve_api/remote/target_client.hpp b/src/buildtool/serve_api/remote/target_client.hpp index c9540cb0..948e9337 100644 --- a/src/buildtool/serve_api/remote/target_client.hpp +++ b/src/buildtool/serve_api/remote/target_client.hpp @@ -19,6 +19,7 @@ #include <optional> #include <string> #include <utility> +#include <variant> #include <vector> #include "gsl/gsl" @@ -31,6 +32,16 @@ #include "src/buildtool/storage/target_cache_entry.hpp" #include "src/buildtool/storage/target_cache_key.hpp" +/// \brief Result union for the ServeTarget request. +/// Index 0 will contain the hash of the blob containing the logged +/// analysis/build failure received from the endpoint, as a string. +/// Index 1 will contain any other failure message, as a string. +/// Index 2 will contain the target cache value information on success. +using serve_target_result_t = + std::variant<std::string, + std::string, + std::pair<TargetCacheEntry, Artifact::ObjectInfo>>; + /// Implements client side for Target service defined in: /// src/buildtool/serve_api/serve_service/just_serve.proto class TargetClient { @@ -39,12 +50,13 @@ class TargetClient { /// \brief Retrieve the pair of TargetCacheEntry and ObjectInfo associated /// to the given key. - /// \param[in] key The TargetCacheKey of an export target - /// \param[in] repo_key The RepositoryKey to upload as precondition - /// \returns Pair of cache entry and its object info on success or nullopt. + /// \param[in] key The TargetCacheKey of an export target. + /// \param[in] repo_key The RepositoryKey to upload as precondition. + /// \returns A correspondingly populated result union, or nullopt if remote + /// reported that the target was not found. [[nodiscard]] auto ServeTarget(const TargetCacheKey& key, const std::string& repo_key) noexcept - -> std::optional<std::pair<TargetCacheEntry, Artifact::ObjectInfo>>; + -> std::optional<serve_target_result_t>; /// \brief Retrieve the flexible config variables of an export target. /// \param[in] target_root_id Hash of target-level root tree. |