content_git_map: Reorder logic for setting up absent roots

If we set up the root for an archive repository as absent, we
should first check if the serve endpoint can set it up for us, and
only then try to provide it from locally available means.

1 files changed, 113 insertions, 88 deletions

diff --git a/src/other_tools/root_maps/content_git_map.cpp b/src/other_tools/root_maps/content_git_map.cpp
index 3bbc21c5..46326b91 100644
--- a/src/other_tools/root_maps/content_git_map.cpp
+++ b/src/other_tools/root_maps/content_git_map.cpp
@@ -70,37 +70,66 @@ void EnsureRootAsAbsent(
         }
         if (not *has_tree) {
             // try to see if serve endpoint has the information to prepare the
-            // root itself
-            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
-                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),
-                              /*fatal=*/true);
-                    return;
+            // root itself; this is redundant if root is not already cached
+            if (is_cache_hit) {
+                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
+                    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),
+                                  /*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 remote_api) {
+                        (*logger)(
+                            fmt::format(
+                                "Missing or incompatible remote-execution "
+                                "endpoint needed to sync workspace root {} "
+                                "with the serve endpoint.",
+                                tree_id),
+                            /*fatal=*/true);
+                        return;
+                    }
+                    // the tree is known locally, so we can upload it to remote
+                    // CAS for the serve endpoint to retrieve it and set up the
+                    // root
+                    if (not EnsureAbsentRootOnServe(
+                            tree_id,
+                            StorageConfig::GitRoot(),
+                            *remote_api,
+                            logger,
+                            /*no_sync_is_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;
-                }
+                // the tree is known locally, so we can upload it to remote CAS
+                // for the serve endpoint to retrieve it and set up the root
                 if (not remote_api) {
                     (*logger)(
                         fmt::format("Missing or incompatible remote-execution "
@@ -110,8 +139,9 @@ void EnsureRootAsAbsent(
                         /*fatal=*/true);
                     return;
                 }
-                // the tree is known locally, so we can upload it to remote CAS
-                // for the serve endpoint to retrieve it and set up the root
+                // the tree is known locally, so we can upload it to remote
+                // CAS for the serve endpoint to retrieve it and set up the
+                // root
                 if (not EnsureAbsentRootOnServe(tree_id,
                                                 StorageConfig::GitRoot(),
                                                 *remote_api,
@@ -135,10 +165,9 @@ void EnsureRootAsAbsent(
                   /*is_cache_hit=*/is_cache_hit));
 }
 
-/// \brief Called to get the resolved root (with respect to symlinks) from an
-/// unresolved tree.
-/// It guarantees the logger is called exactly once with fatal on failure, and
-/// the setter on success.
+/// \brief Called to get the resolved root (with respect to symlinks) from
+/// an unresolved tree. It guarantees the logger is called exactly once with
+/// fatal on failure, and the setter on success.
 void ResolveContentTree(
     ArchiveRepoInfo const& key,
     std::string const& tree_hash,
@@ -158,15 +187,14 @@ void ResolveContentTree(
             // read resolved tree id
             auto resolved_tree_id = FileSystemManager::ReadFile(tree_id_file);
             if (not resolved_tree_id) {
-                (*logger)(
-                    fmt::format("Failed to read resolved tree id from file {}",
-                                tree_id_file.string()),
-                    /*fatal=*/true);
+                (*logger)(fmt::format("Failed to read resolved tree id "
+                                      "from file {}",
+                                      tree_id_file.string()),
+                          /*fatal=*/true);
                 return;
             }
             // set the workspace root
             if (is_absent) {
-                // try all the available means to generate & set the absent root
                 EnsureRootAsAbsent(*resolved_tree_id,
                                    key,
                                    serve_api_exists,
@@ -232,8 +260,6 @@ void ResolveContentTree(
                     }
                     // set the workspace root
                     if (is_absent) {
-                        // try all the available means to generate & set the
-                        // absent root
                         EnsureRootAsAbsent(resolved_tree.id,
                                            key,
                                            serve_api_exists,
@@ -264,7 +290,6 @@ void ResolveContentTree(
     else {
         // set the workspace root as-is
         if (is_absent) {
-            // try all the available means to generate & set the absent root
             EnsureRootAsAbsent(tree_hash,
                                key,
                                serve_api_exists,
@@ -284,8 +309,8 @@ void ResolveContentTree(
 }
 
 /// \brief Called to store the file association and then set the root.
-/// It guarantees the logger is called exactly once with fatal on failure, and
-/// the setter on success.
+/// It guarantees the logger is called exactly once with fatal on failure,
+/// and the setter on success.
 void WriteIdFileAndSetWSRoot(
     ArchiveRepoInfo const& key,
     std::string const& archive_tree_id,
@@ -342,10 +367,9 @@ void WriteIdFileAndSetWSRoot(
                        logger);
 }
 
-/// \brief Called when archive is in local CAS. Performs the import-to-git and
-/// follow-up processing.
-/// It guarantees the logger is called exactly once with fatal on failure, and
-/// the setter on success.
+/// \brief Called when archive is in local CAS. Performs the import-to-git
+/// and follow-up processing. It guarantees the logger is called exactly
+/// once with fatal on failure, and the setter on success.
 void ExtractAndImportToGit(
     ArchiveRepoInfo const& key,
     std::filesystem::path const& content_cas_path,
@@ -370,11 +394,11 @@ void ExtractAndImportToGit(
     auto res =
         ExtractArchive(content_cas_path, key.repo_type, tmp_dir->GetPath());
     if (res != std::nullopt) {
-        (*logger)(
-            fmt::format("Failed to extract archive {} from CAS with error:\n{}",
-                        content_cas_path.string(),
-                        *res),
-            /*fatal=*/true);
+        (*logger)(fmt::format("Failed to extract archive {} from CAS with "
+                              "error:\n{}",
+                              content_cas_path.string(),
+                              *res),
+                  /*fatal=*/true);
         return;
     }
     // import to git
@@ -548,6 +572,42 @@ auto CreateContentGitMap(
         else {
             // separate logic between absent and present roots
             if (key.absent and not fetch_absent) {
+                // request the resolved subdir tree from the serve endpoint, if
+                // given
+                if (serve_api_exists) {
+                    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,
+                                 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;
+                    }
+                }
+                // if serve endpoint cannot set up the root, we might still be
+                // able to set up the absent root with local information, and if
+                // a serve endpoint exists we can upload it the root ourselves;
+
                 // check if content already in CAS
                 auto const& cas = Storage::Instance().CAS();
                 auto digest = ArtifactDigest(key.archive.content, 0, false);
@@ -694,41 +754,6 @@ auto CreateContentGitMap(
                             // done
                             return;
                         }
-                        // request the resolved subdir tree from the serve
-                        // endpoint, if given
-                        if (serve_api_exists) {
-                            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,
-                                         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;
-                            }
-                        }
                         // report not being able to set up this root as absent
                         (*logger)(fmt::format("Cannot create workspace root as "
                                               "absent for content {}.",