diff options
author | Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com> | 2024-01-24 17:09:01 +0100 |
---|---|---|
committer | Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com> | 2024-01-31 17:15:46 +0100 |
commit | 66df7f4956c6fe1823eb43d31c3cbac7d8716125 (patch) | |
tree | 3f8bf106b3e189eeb4cfa01f97e8f9bb18678406 /src | |
parent | 3dc81b4b5a89f4e37af45ec9954723c79c3017cf (diff) | |
download | justbuild-66df7f4956c6fe1823eb43d31c3cbac7d8716125.tar.gz |
just-mr: Failure in serve api root tree requests should be fatal
The requests to retrieve the tree of a commit, archive, or distdir
also set up those trees in a way that the serve endpoint can later
build against them, besides allowing just-mr to set up roots
locally. Therefore, if the witnessing entity (Git commit, content
blob, or distdir, respectively) is known to the serve endpoint,
then failing to set up the root tree there should result in a
failure also of the just-mr setup on the client side.
Diffstat (limited to 'src')
-rw-r--r-- | src/buildtool/serve_api/remote/serve_api.hpp | 6 | ||||
-rw-r--r-- | src/buildtool/serve_api/remote/source_tree_client.cpp | 31 | ||||
-rw-r--r-- | src/buildtool/serve_api/remote/source_tree_client.hpp | 35 | ||||
-rw-r--r-- | src/other_tools/root_maps/commit_git_map.cpp | 96 | ||||
-rw-r--r-- | src/other_tools/root_maps/content_git_map.cpp | 113 | ||||
-rw-r--r-- | src/other_tools/root_maps/distdir_git_map.cpp | 88 |
6 files changed, 255 insertions, 114 deletions
diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp index 44169223..23b8904b 100644 --- a/src/buildtool/serve_api/remote/serve_api.hpp +++ b/src/buildtool/serve_api/remote/serve_api.hpp @@ -44,7 +44,7 @@ class ServeApi final { [[nodiscard]] static auto RetrieveTreeFromCommit( std::string const& commit, std::string const& subdir = ".", - bool sync_tree = false) -> std::optional<std::string> { + bool sync_tree = false) -> std::variant<bool, std::string> { return Instance().stc_->ServeCommitTree(commit, subdir, sync_tree); } @@ -53,7 +53,7 @@ class ServeApi final { std::string const& archive_type = "archive", std::string const& subdir = ".", std::optional<PragmaSpecial> const& resolve_symlinks = std::nullopt, - bool sync_tree = false) -> std::optional<std::string> { + bool sync_tree = false) -> std::variant<bool, std::string> { return Instance().stc_->ServeArchiveTree( content, archive_type, subdir, resolve_symlinks, sync_tree); } @@ -61,7 +61,7 @@ class ServeApi final { [[nodiscard]] static auto RetrieveTreeFromDistdir( std::shared_ptr<std::unordered_map<std::string, std::string>> const& distfiles, - bool sync_tree = false) -> std::optional<std::string> { + bool sync_tree = false) -> std::variant<bool, std::string> { return Instance().stc_->ServeDistdirTree(distfiles, sync_tree); } diff --git a/src/buildtool/serve_api/remote/source_tree_client.cpp b/src/buildtool/serve_api/remote/source_tree_client.cpp index 073181a0..8267df7f 100644 --- a/src/buildtool/serve_api/remote/source_tree_client.cpp +++ b/src/buildtool/serve_api/remote/source_tree_client.cpp @@ -64,8 +64,7 @@ SourceTreeClient::SourceTreeClient(std::string const& server, auto SourceTreeClient::ServeCommitTree(std::string const& commit_id, std::string const& subdir, - bool sync_tree) - -> std::optional<std::string> { + bool sync_tree) -> result_t { justbuild::just_serve::ServeCommitTreeRequest request{}; request.set_commit(commit_id); request.set_subdir(subdir); @@ -77,16 +76,18 @@ auto SourceTreeClient::ServeCommitTree(std::string const& commit_id, if (not status.ok()) { LogStatus(&logger_, LogLevel::Debug, status); - return std::nullopt; + return true; // fatal failure } if (response.status() != ::justbuild::just_serve::ServeCommitTreeResponse::OK) { logger_.Emit(LogLevel::Debug, "ServeCommitTree response returned with {}", static_cast<int>(response.status())); - return std::nullopt; + return /*fatal = */ ( + response.status() != + ::justbuild::just_serve::ServeCommitTreeResponse::NOT_FOUND); } - return response.tree(); + return response.tree(); // success } auto SourceTreeClient::ServeArchiveTree( @@ -94,7 +95,7 @@ auto SourceTreeClient::ServeArchiveTree( std::string const& archive_type, std::string const& subdir, std::optional<PragmaSpecial> const& resolve_symlinks, - bool sync_tree) -> std::optional<std::string> { + bool sync_tree) -> result_t { justbuild::just_serve::ServeArchiveTreeRequest request{}; request.set_content(content); request.set_archive_type(StringToArchiveType(archive_type)); @@ -109,22 +110,24 @@ auto SourceTreeClient::ServeArchiveTree( if (not status.ok()) { LogStatus(&logger_, LogLevel::Debug, status); - return std::nullopt; + return true; // fatal failure } if (response.status() != ::justbuild::just_serve::ServeArchiveTreeResponse::OK) { logger_.Emit(LogLevel::Debug, "ServeArchiveTree response returned with {}", static_cast<int>(response.status())); - return std::nullopt; + return /*fatal = */ ( + response.status() != + ::justbuild::just_serve::ServeArchiveTreeResponse::NOT_FOUND); } - return response.tree(); + return response.tree(); // success } auto SourceTreeClient::ServeDistdirTree( std::shared_ptr<std::unordered_map<std::string, std::string>> const& distfiles, - bool sync_tree) -> std::optional<std::string> { + bool sync_tree) -> result_t { justbuild::just_serve::ServeDistdirTreeRequest request{}; for (auto const& [k, v] : *distfiles) { auto* distfile = request.add_distfiles(); @@ -139,16 +142,18 @@ auto SourceTreeClient::ServeDistdirTree( if (not status.ok()) { LogStatus(&logger_, LogLevel::Debug, status); - return std::nullopt; + return true; // fatal failure } if (response.status() != ::justbuild::just_serve::ServeDistdirTreeResponse::OK) { logger_.Emit(LogLevel::Debug, "ServeDistdirTree response returned with {}", static_cast<int>(response.status())); - return std::nullopt; + return /*fatal = */ ( + response.status() != + ::justbuild::just_serve::ServeDistdirTreeResponse::NOT_FOUND); } - return response.tree(); + return response.tree(); // success } auto SourceTreeClient::ServeContent(std::string const& content) -> bool { diff --git a/src/buildtool/serve_api/remote/source_tree_client.hpp b/src/buildtool/serve_api/remote/source_tree_client.hpp index 3ea1058b..24b2654a 100644 --- a/src/buildtool/serve_api/remote/source_tree_client.hpp +++ b/src/buildtool/serve_api/remote/source_tree_client.hpp @@ -17,6 +17,7 @@ #include <memory> #include <string> +#include <variant> #include "justbuild/just_serve/just_serve.grpc.pb.h" #include "src/buildtool/common/remote/port.hpp" @@ -29,42 +30,54 @@ 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. + // An error + data union type + using result_t = std::variant<bool, std::string>; + + /// \brief Retrieve the Git tree of a given commit, if known by the + /// endpoint. It is a fatal error if the commit is known to the endpoint but + /// no tree is able to be returned. /// \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 endpoint. - /// \returns The hash of the tree if commit found, nullopt otherwise. + /// \returns An error + data union, where at index 0 we return a fatal flag, + /// with false meaning non-fatal failure (commit or subtree not found), and + /// at index 1 the tree identifier on success. [[nodiscard]] auto ServeCommitTree(std::string const& commit_id, std::string const& subdir, - bool sync_tree) - -> std::optional<std::string>; + bool sync_tree) -> result_t; - /// \brief Retrieve the Git tree of an archive content, if known by remote. + /// \brief Retrieve the Git tree of an archive content, if known by the + /// endpoint. It is a fatal error if the content blob is known to the + /// endpoint but no tree is able to be returned. /// \param[in] content Hash of the archive content to look up. /// \param[in] archive_type Type of archive ("archive"|"zip"). /// \param[in] subdir Relative path of the tree inside archive. /// \param[in] resolve_symlinks Optional enum to state how symlinks in the /// archive should be handled if the tree has to be actually computed. /// \param[in] sync_tree Sync tree to the remote-execution endpoint. - /// \returns The hash of the tree if content blob found, nullopt otherwise. + /// \returns An error + data union, where at index 0 we return a fatal flag, + /// with false meaning non-fatal failure (content blob not found), and at + /// index 1 the tree identifier on success. [[nodiscard]] auto ServeArchiveTree( std::string const& content, std::string const& archive_type, std::string const& subdir, std::optional<PragmaSpecial> const& resolve_symlinks, - bool sync_tree) -> std::optional<std::string>; + bool sync_tree) -> result_t; /// \brief Retrieve the Git tree of a directory of distfiles, if all the - /// content blobs are known by serve remote. + /// content blobs are known by the endpoint. It is a fatal error if all + /// content blobs are known but no tree is able to be returned. /// \param[in] distfiles Mapping from distfile names to content blob ids. /// \param[in] sync_tree Sync tree and all ditfile blobs to the /// remote-execution endpoint. - /// \returns The hash of the resulting tree if all distfile blobs found, - /// nullopt otherwise. + /// \returns An error + data union, where at index 0 we return a fatal flag, + /// with false meaning non-fatal failure (at least one distfile blob + /// missing), and at index 1 the tree identifier on success. [[nodiscard]] auto ServeDistdirTree( std::shared_ptr<std::unordered_map<std::string, std::string>> const& distfiles, - bool sync_tree) -> std::optional<std::string>; + bool sync_tree) -> result_t; /// \brief Make a given content blob available in remote CAS, if known by /// serve remote. diff --git a/src/other_tools/root_maps/commit_git_map.cpp b/src/other_tools/root_maps/commit_git_map.cpp index 63cfa02a..7dc332ea 100644 --- a/src/other_tools/root_maps/commit_git_map.cpp +++ b/src/other_tools/root_maps/commit_git_map.cpp @@ -62,21 +62,34 @@ void EnsureRootAsAbsent( if (not *has_tree) { // try to see if serve endpoint has the information to prepare the // root itself - if (auto served_tree_id = - ServeApi::RetrieveTreeFromCommit(repo_info.hash, - repo_info.subdir, - /*sync_tree = */ false)) { + auto serve_result = + ServeApi::RetrieveTreeFromCommit(repo_info.hash, + repo_info.subdir, + /*sync_tree = */ false); + if (std::holds_alternative<std::string>(serve_result)) { // if serve has set up the tree, it must match what we expect - if (tree_id != *served_tree_id) { + auto const& served_tree_id = + std::get<std::string>(serve_result); + if (tree_id != served_tree_id) { (*logger)(fmt::format("Mismatch in served root tree " "id:\nexpected {}, but got {}", tree_id, - *served_tree_id), + served_tree_id), /*fatal=*/true); return; } } else { + // check if serve failure was due to commit not being found or + // it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)(fmt::format("Serve endpoint failed to set up " + "root from known commit {}", + repo_info.hash), + /*fatal=*/true); + return; + } if (not remote_api) { (*logger)(fmt::format("Missing remote-execution endpoint " "needed to sync workspace root {} " @@ -263,10 +276,11 @@ void EnsureCommit( if (serve_api_exists) { // if root purely absent, request only the subdir tree if (repo_info.absent and not fetch_absent) { - if (auto tree_id = ServeApi::RetrieveTreeFromCommit( - repo_info.hash, - repo_info.subdir, - /*sync_tree = */ false)) { + auto serve_result = + ServeApi::RetrieveTreeFromCommit(repo_info.hash, + repo_info.subdir, + /*sync_tree = */ false); + if (std::holds_alternative<std::string>(serve_result)) { // set the workspace root as absent JustMRProgress::Instance().TaskTracker().Stop( repo_info.origin); @@ -275,23 +289,36 @@ void EnsureCommit( {repo_info.ignore_special ? FileRoot::kGitTreeIgnoreSpecialMarker : FileRoot::kGitTreeMarker, - *tree_id}), + std::get<std::string>(serve_result)}), /*is_cache_hit=*/false)); return; } + // check if serve failure was due to commit not being found or + // it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)(fmt::format("Serve endpoint failed to set up " + "root from known commit {}", + repo_info.hash), + /*fatal=*/true); + return; + } } // otherwise, request (and sync) the whole commit tree, to ensure // we maintain the id file association else { - if (auto root_tree_id = ServeApi::RetrieveTreeFromCommit( - repo_info.hash, - /*subdir = */ ".", - /*sync_tree = */ true)) { + auto serve_result = + ServeApi::RetrieveTreeFromCommit(repo_info.hash, + /*subdir = */ ".", + /*sync_tree = */ true); + if (std::holds_alternative<std::string>(serve_result)) { + auto const& root_tree_id = + std::get<std::string>(serve_result); // verify if we know the tree already locally auto wrapped_logger = std::make_shared<AsyncMapConsumerLogger>( - [logger, tree = *root_tree_id](auto const& msg, - bool fatal) { + [logger, tree = root_tree_id](auto const& msg, + bool fatal) { (*logger)( fmt::format("While verifying presence of " "tree {}:\n{}", @@ -299,8 +326,8 @@ void EnsureCommit( msg), fatal); }); - auto tree_present = git_repo->CheckTreeExists( - *root_tree_id, wrapped_logger); + auto tree_present = + git_repo->CheckTreeExists(root_tree_id, wrapped_logger); if (not tree_present) { return; } @@ -309,7 +336,7 @@ void EnsureCommit( repo_info.origin); // write association to id file, get subdir tree, // and set the workspace root as present - WriteIdFileAndSetWSRoot(*root_tree_id, + WriteIdFileAndSetWSRoot(root_tree_id, repo_info.subdir, repo_info.ignore_special, git_cas, @@ -321,7 +348,7 @@ void EnsureCommit( } // try to get root tree from remote execution endpoint auto root_digest = - ArtifactDigest{*root_tree_id, 0, /*is_tree=*/true}; + ArtifactDigest{root_tree_id, 0, /*is_tree=*/true}; if (remote_api and remote_api.value()->RetrieveToCas( {Artifact::ObjectInfo{.digest = root_digest, @@ -337,7 +364,7 @@ void EnsureCommit( fmt::format("Failed to create tmp " "directory after fetching root " "tree {} for absent commit {}", - *root_tree_id, + root_tree_id, repo_info.hash), /*fatal=*/true); return; @@ -349,18 +376,18 @@ void EnsureCommit( {tmp_dir->GetPath()})) { (*logger)(fmt::format("Failed to copy fetched root " "tree {} to {}", - *root_tree_id, + root_tree_id, tmp_dir->GetPath().string()), /*fatal=*/true); return; } CommitInfo c_info{ - tmp_dir->GetPath(), "tree", *root_tree_id}; + tmp_dir->GetPath(), "tree", root_tree_id}; import_to_git_map->ConsumeAfterKeysReady( ts, {std::move(c_info)}, [tmp_dir, // keep tmp_dir alive - root_tree_id = *root_tree_id, + root_tree_id, subdir = repo_info.subdir, ignore_special = repo_info.ignore_special, git_cas, @@ -411,7 +438,7 @@ void EnsureCommit( (*logger)( fmt::format("While moving root tree {} " "from {} to local git:\n{}", - *root_tree_id, + root_tree_id, tmp_dir->GetPath().string(), msg), fatal); @@ -423,15 +450,20 @@ void EnsureCommit( // remote CAS, so log this attempt and revert to network (*logger)(fmt::format("Tree {} marked as served, but not " "found on remote", - *root_tree_id), + root_tree_id), /*fatal=*/false); } else { - // give warning - (*logger)(fmt::format( - "Root tree for commit {} could not be served", - repo_info.hash), - /*fatal=*/false); + // check if serve failure was due to commit not being found + // or it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)(fmt::format("Serve endpoint failed to set up " + "root from known commit {}", + repo_info.hash), + /*fatal=*/true); + return; + } } } } diff --git a/src/other_tools/root_maps/content_git_map.cpp b/src/other_tools/root_maps/content_git_map.cpp index 5dafef06..5d910015 100644 --- a/src/other_tools/root_maps/content_git_map.cpp +++ b/src/other_tools/root_maps/content_git_map.cpp @@ -71,23 +71,36 @@ void EnsureRootAsAbsent( if (not *has_tree) { // try to see if serve endpoint has the information to prepare the // root itself - if (auto served_tree_id = - ServeApi::RetrieveTreeFromArchive(key.archive.content, - key.repo_type, - key.subdir, - key.pragma_special, - /*sync_tree=*/false)) { + auto serve_result = + ServeApi::RetrieveTreeFromArchive(key.archive.content, + key.repo_type, + key.subdir, + key.pragma_special, + /*sync_tree=*/false); + if (std::holds_alternative<std::string>(serve_result)) { // if serve has set up the tree, it must match what we expect - if (tree_id != *served_tree_id) { + auto const& served_tree_id = + std::get<std::string>(serve_result); + if (tree_id != served_tree_id) { (*logger)(fmt::format("Mismatch in served root tree " "id:\nexpected {}, but got {}", tree_id, - *served_tree_id), + served_tree_id), /*fatal=*/true); return; } } else { + // check if serve failure was due to archive content not being + // found or it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)(fmt::format("Serve endpoint failed to set up " + "root from known archive content {}", + key.archive.content), + /*fatal=*/true); + return; + } if (not is_on_remote and not remote_api) { (*logger)(fmt::format("Missing remote-execution endpoint " "needed to sync workspace root {} " @@ -700,39 +713,58 @@ auto CreateContentGitMap( // if purely absent, request the resolved subdir tree // directly if (key.absent and not fetch_absent) { - if (auto tree_id = - ServeApi::RetrieveTreeFromArchive( - key.archive.content, - key.repo_type, - key.subdir, - key.pragma_special, - /*sync_tree = */ false)) { + auto serve_result = + ServeApi::RetrieveTreeFromArchive( + key.archive.content, + key.repo_type, + key.subdir, + key.pragma_special, + /*sync_tree = */ false); + if (std::holds_alternative<std::string>( + serve_result)) { // set the workspace root as absent JustMRProgress::Instance().TaskTracker().Stop( key.archive.origin); (*setter)(std::pair( nlohmann::json::array( - {FileRoot::kGitTreeMarker, *tree_id}), + {FileRoot::kGitTreeMarker, + std::get<std::string>(serve_result)}), /*is_cache_hit = */ false)); return; } + // check if serve failure was due to archive content + // not being found or it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)( + fmt::format( + "Serve endpoint failed to set up " + "root from known archive content {}", + key.archive.content), + /*fatal=*/true); + return; + } } // otherwise, request (and sync) the whole archive tree, // UNRESOLVED, to ensure we maintain the id file // association else { - if (auto root_tree_id = - ServeApi::RetrieveTreeFromArchive( - key.archive.content, - key.repo_type, - /*subdir = */ ".", - /* resolve_symlinks = */ std::nullopt, - /*sync_tree = */ true)) { + auto serve_result = + ServeApi::RetrieveTreeFromArchive( + key.archive.content, + key.repo_type, + /*subdir = */ ".", + /* resolve_symlinks = */ std::nullopt, + /*sync_tree = */ true); + if (std::holds_alternative<std::string>( + serve_result)) { + auto const& root_tree_id = + std::get<std::string>(serve_result); // verify if we already know the tree locally; // setup wrapped logger auto wrapped_logger = std::make_shared<AsyncMapConsumerLogger>( - [&logger, tree = *root_tree_id]( + [&logger, tree = root_tree_id]( auto const& msg, bool fatal) { (*logger)( fmt::format( @@ -744,7 +776,7 @@ auto CreateContentGitMap( }); auto tree_present = just_git_repo->CheckTreeExists( - *root_tree_id, wrapped_logger); + root_tree_id, wrapped_logger); if (not tree_present) { return; } @@ -756,7 +788,7 @@ auto CreateContentGitMap( // this results in a present root WriteIdFileAndSetWSRoot( key, - *root_tree_id, + root_tree_id, just_git_cas, archive_tree_id_file, /*is_absent=*/false, @@ -773,7 +805,7 @@ auto CreateContentGitMap( // try to get root tree from remote execution // endpoint auto root_digest = ArtifactDigest{ - *root_tree_id, 0, /*is_tree=*/true}; + root_tree_id, 0, /*is_tree=*/true}; if (remote_api and remote_api.value()->RetrieveToCas( {Artifact::ObjectInfo{ @@ -793,7 +825,7 @@ auto CreateContentGitMap( "Failed to create tmp " "directory after fetching root " "tree {} for absent archive {}", - *root_tree_id, + root_tree_id, key.archive.content), true); return; @@ -807,14 +839,14 @@ auto CreateContentGitMap( fmt::format( "Failed to copy fetched root " "tree {} to {}", - *root_tree_id, + root_tree_id, tmp_dir->GetPath().string()), true); return; } CommitInfo c_info{tmp_dir->GetPath(), "tree", - *root_tree_id}; + root_tree_id}; import_to_git_map->ConsumeAfterKeysReady( ts, {std::move(c_info)}, @@ -838,7 +870,7 @@ auto CreateContentGitMap( // present root WriteIdFileAndSetWSRoot( key, - *root_tree_id, + root_tree_id, just_git_cas, archive_tree_id_file, /*is_absent=*/false, @@ -856,7 +888,7 @@ auto CreateContentGitMap( fmt::format( "While moving root tree {} " "from {} to local git:\n{}", - *root_tree_id, + root_tree_id, tmp_dir->GetPath().string(), msg), fatal); @@ -919,11 +951,18 @@ auto CreateContentGitMap( // done return; } - // give warning - (*logger)(fmt::format("Root tree for content {} " - "could not be served", - key.archive.content), - /*fatal=*/false); + // check if serve failure was due to archive content + // not being found or it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)( + fmt::format( + "Serve endpoint failed to set up root " + "from known archive content {}", + key.archive.content), + /*fatal=*/true); + return; + } } } diff --git a/src/other_tools/root_maps/distdir_git_map.cpp b/src/other_tools/root_maps/distdir_git_map.cpp index 7646882b..2ee0265f 100644 --- a/src/other_tools/root_maps/distdir_git_map.cpp +++ b/src/other_tools/root_maps/distdir_git_map.cpp @@ -191,6 +191,7 @@ auto CreateDistdirGitMap( ts, {std::move(op_key)}, [distdir_tree_id = *distdir_tree_id, + content_id = key.content_id, key, serve_api_exists, remote_api, @@ -216,24 +217,45 @@ auto CreateDistdirGitMap( if (not *has_tree) { // try to see if serve endpoint has the // information to prepare the root itself - if (auto served_tree_id = - ServeApi::RetrieveTreeFromDistdir( - key.content_list, - /*sync_tree=*/false)) { + auto serve_result = + ServeApi::RetrieveTreeFromDistdir( + key.content_list, + /*sync_tree=*/false); + if (std::holds_alternative<std::string>( + serve_result)) { // if serve has set up the tree, it must // match what we expect - if (distdir_tree_id != *served_tree_id) { + auto const& served_tree_id = + std::get<std::string>(serve_result); + if (distdir_tree_id != served_tree_id) { (*logger)( fmt::format( "Mismatch in served root tree " "id:\nexpected {}, but got {}", distdir_tree_id, - *served_tree_id), + served_tree_id), /*fatal=*/true); return; } } else { + // check if serve failure was due to distdir + // content not being found or it is + // otherwise fatal + auto const& is_fatal = + std::get<bool>(serve_result); + if (is_fatal) { + (*logger)( + fmt::format( + "Serve endpoint failed to set " + "up root from known distdir " + "content {}", + content_id), + /*fatal=*/true); + return; + } + // at this point we cannot continue without + // the remote api if (not remote_api) { (*logger)( fmt::format( @@ -344,17 +366,20 @@ auto CreateDistdirGitMap( } // try to see if serve endpoint has the information to // prepare the root itself - if (auto served_tree_id = ServeApi::RetrieveTreeFromDistdir( - key.content_list, - /*sync_tree=*/false)) { - // if serve has set up the tree, it must match what we - // expect - if (tree_id != *served_tree_id) { + auto serve_result = + ServeApi::RetrieveTreeFromDistdir(key.content_list, + /*sync_tree=*/false); + if (std::holds_alternative<std::string>(serve_result)) { + // if serve has set up the tree, it must + // match what we expect + auto const& served_tree_id = + std::get<std::string>(serve_result); + if (tree_id != served_tree_id) { (*logger)( fmt::format("Mismatch in served root tree " "id:\nexpected {}, but got {}", tree_id, - *served_tree_id), + served_tree_id), /*fatal=*/true); return; } @@ -365,6 +390,17 @@ auto CreateDistdirGitMap( /*is_cache_hit=*/false)); return; } + // check if serve failure was due to distdir content not + // being found or it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)( + fmt::format("Serve endpoint failed to set up root " + "from known distdir content {}", + key.content_id), + /*fatal=*/true); + return; + } // at this point we cannot continue without the remote api if (not remote_api) { (*logger)( @@ -467,16 +503,19 @@ auto CreateDistdirGitMap( } // now ask serve endpoint if it can set up the root if (serve_api_exists and remote_api) { - if (auto served_tree_id = - ServeApi::RetrieveTreeFromDistdir(key.content_list, - /*sync_tree=*/true)) { + auto serve_result = + ServeApi::RetrieveTreeFromDistdir(key.content_list, + /*sync_tree=*/true); + if (std::holds_alternative<std::string>(serve_result)) { // if serve has set up the tree, it must match what we // expect - if (tree_id != *served_tree_id) { + auto const& served_tree_id = + std::get<std::string>(serve_result); + if (tree_id != served_tree_id) { (*logger)(fmt::format("Mismatch in served root tree " "id:\nexpected {}, but got {}", tree_id, - *served_tree_id), + served_tree_id), /*fatal=*/true); return; } @@ -484,6 +523,19 @@ auto CreateDistdirGitMap( // root, as we will check the remote CAS for the // resulting tree anyway } + else { + // check if serve failure was due to distdir content not + // being found or it is otherwise fatal + auto const& is_fatal = std::get<bool>(serve_result); + if (is_fatal) { + (*logger)( + fmt::format("Serve endpoint failed to set up root " + "from known distdir content {}", + key.content_id), + /*fatal=*/true); + return; + } + } } // check the remote CAS for the tree if (remote_api and |