just-mr maps: Properly check for missing values in map chain

The root async map in a chain of calls should always be checked
for missing value, which can happen if, e.g., a cycle happens or
a thread gets killed by the system.

Properly handle this by checking explicitly if a value has been
posted. If not, check for cycles where it makes sense (for
example, in the resolving of symlinks), otherwise report any
pending map keys not yet processed.

This is done for all just-mr commands working with async maps.

7 files changed, 45 insertions, 44 deletions

diff --git a/src/other_tools/just_mr/TARGETS b/src/other_tools/just_mr/TARGETS
index 15478898..5d03675a 100644
--- a/src/other_tools/just_mr/TARGETS
+++ b/src/other_tools/just_mr/TARGETS
@@ -122,6 +122,7 @@
     , ["src/buildtool/common/remote", "retry_config"]
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/main", "retry"]
+    , ["src/buildtool/multithreading", "async_map_utils"]
     , ["src/buildtool/multithreading", "task_system"]
     , "exit_codes"
     , ["src/other_tools/just_mr/progress_reporting", "progress"]
@@ -159,6 +160,7 @@
     [ ["@", "fmt", "", "fmt"]
     , ["@", "json", "", "json"]
     , ["src/buildtool/logging", "logging"]
+    , ["src/buildtool/multithreading", "async_map_utils"]
     , ["src/buildtool/multithreading", "task_system"]
     , ["src/buildtool/storage", "config"]
     , ["src/other_tools/git_operations", "git_repo_remote"]
@@ -189,6 +191,7 @@
     , ["src/buildtool/common/remote", "retry_config"]
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/main", "retry"]
+    , ["src/buildtool/multithreading", "async_map_utils"]
     , ["src/buildtool/multithreading", "task_system"]
     , ["src/buildtool/storage", "fs_utils"]
     , "exit_codes"
diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp
index d236b7e8..c1d65010 100644
--- a/src/other_tools/just_mr/fetch.cpp
+++ b/src/other_tools/just_mr/fetch.cpp
@@ -31,6 +31,7 @@
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
 #include "src/buildtool/main/retry.hpp"
+#include "src/buildtool/multithreading/async_map_utils.hpp"
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/buildtool/serve_api/remote/config.hpp"
 #include "src/buildtool/serve_api/remote/serve_api.hpp"
@@ -542,12 +543,15 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
 
     // do the fetch
     bool failed_archives{false};
+    bool has_value_archives{false};
     {
         TaskSystem ts{common_args.jobs};
         archive_fetch_map.ConsumeAfterKeysReady(
             &ts,
             archives_to_fetch,
-            []([[maybe_unused]] auto const& values) {},
+            [&has_value_archives]([[maybe_unused]] auto const& values) {
+                has_value_archives = true;
+            },
             [&failed_archives, &multi_repository_tool_name](auto const& msg,
                                                             bool fatal) {
                 Logger::Log(fatal ? LogLevel::Error : LogLevel::Warning,
@@ -558,12 +562,15 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
             });
     }
     bool failed_git_trees{false};
+    bool has_value_trees{false};
     {
         TaskSystem ts{common_args.jobs};
         git_tree_fetch_map.ConsumeAfterKeysReady(
             &ts,
             git_trees_to_fetch,
-            []([[maybe_unused]] auto const& values) {},
+            [&has_value_trees]([[maybe_unused]] auto const& values) {
+                has_value_trees = true;
+            },
             [&failed_git_trees, &multi_repository_tool_name](auto const& msg,
                                                              bool fatal) {
                 Logger::Log(fatal ? LogLevel::Error : LogLevel::Warning,
@@ -582,6 +589,13 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config,
     if (failed_archives or failed_git_trees) {
         return kExitFetchError;
     }
+    if (not has_value_archives or not has_value_trees) {
+        DetectAndReportPending(
+            "fetch archives", archive_fetch_map, kArchiveContentPrinter);
+        DetectAndReportPending(
+            "fetch trees", git_tree_fetch_map, kGitTreeInfoPrinter);
+        return kExitFetchError;
+    }
     // report success
     Logger::Log(LogLevel::Info, "Fetch completed");
     return kExitSuccess;
diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp
index ba5e39b4..ba22936d 100644
--- a/src/other_tools/just_mr/setup.cpp
+++ b/src/other_tools/just_mr/setup.cpp
@@ -34,6 +34,7 @@
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
 #include "src/buildtool/main/retry.hpp"
+#include "src/buildtool/multithreading/async_map_utils.hpp"
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/buildtool/serve_api/remote/config.hpp"
 #include "src/buildtool/serve_api/remote/serve_api.hpp"
@@ -344,12 +345,15 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config,
 
     // Populate workspace_root and TAKE_OVER fields
     bool failed{false};
+    bool has_value{false};
+
     {
         TaskSystem ts{common_args.jobs};
         repos_to_setup_map.ConsumeAfterKeysReady(
             &ts,
             setup_repos->to_setup,
             [&failed,
+             &has_value,
              &mr_config,
              repos,
              setup_repos,
@@ -357,6 +361,7 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config,
              interactive,
              multi_repo_tool_name](auto const& values) noexcept {
                 try {
+                    has_value = true;
                     // set the initial setup repositories
                     nlohmann::json mr_repos{};
                     for (auto const& repo : setup_repos->to_setup) {
@@ -438,6 +443,18 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config,
     if (failed) {
         return std::nullopt;
     }
+    if (not has_value) {
+        // check for cycles in maps where cycles can occur and have meaning
+        if (auto error = DetectAndReportCycle("resolving symlinks",
+                                              resolve_symlinks_map,
+                                              kGitObjectToResolvePrinter)) {
+            Logger::Log(LogLevel::Error, *error);
+            return std::nullopt;
+        }
+        DetectAndReportPending(
+            "setup", repos_to_setup_map, kReposToSetupPrinter);
+        return std::nullopt;
+    }
     // if successful, return the output config
     return StorageUtils::AddToCAS(storage, mr_config.dump(2));
 }
diff --git a/src/other_tools/just_mr/update.cpp b/src/other_tools/just_mr/update.cpp
index df9de506..1c1626eb 100644
--- a/src/other_tools/just_mr/update.cpp
+++ b/src/other_tools/just_mr/update.cpp
@@ -25,6 +25,7 @@
 #include "nlohmann/json.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
+#include "src/buildtool/multithreading/async_map_utils.hpp"
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/other_tools/git_operations/git_repo_remote.hpp"
 #include "src/other_tools/just_mr/exit_codes.hpp"
@@ -240,16 +241,19 @@ auto MultiRepoUpdate(std::shared_ptr<Configuration> const& config,
 
     // do the update
     bool failed{false};
+    bool has_value{false};
     {
         TaskSystem ts{common_args.jobs};
         git_update_map.ConsumeAfterKeysReady(
             &ts,
             repos_to_update,
             [&failed,
+             &has_value,
              &mr_config,
              repos_to_update_names = update_args.repos_to_update,
              multi_repo_tool_name](auto const& values) noexcept {
                 try {
+                    has_value = true;
                     for (auto const& repo_name : repos_to_update_names) {
                         auto i = static_cast<std::size_t>(
                             &repo_name -
@@ -287,6 +291,11 @@ auto MultiRepoUpdate(std::shared_ptr<Configuration> const& config,
     if (failed) {
         return kExitUpdateError;
     }
+    if (not has_value) {
+        DetectAndReportPending(
+            "update", git_update_map, kRepoDescriptionPrinter);
+        return kExitUpdateError;
+    }
     // report success
     Logger::Log(LogLevel::Info, "Update completed");
     // print mr_config to stdout
diff --git a/src/other_tools/root_maps/TARGETS b/src/other_tools/root_maps/TARGETS
index d4d1e4ba..49e1ee60 100644
--- a/src/other_tools/root_maps/TARGETS
+++ b/src/other_tools/root_maps/TARGETS
@@ -88,7 +88,6 @@
     , ["src/buildtool/execution_api/local", "config"]
     , ["src/buildtool/file_system", "file_root"]
     , ["src/buildtool/file_system", "git_repo"]
-    , ["src/buildtool/multithreading", "async_map_utils"]
     , ["src/buildtool/multithreading", "task_system"]
     , ["src/buildtool/storage", "fs_utils"]
     , ["src/other_tools/git_operations", "git_repo_remote"]
@@ -122,7 +121,6 @@
     , ["src/buildtool/file_system", "file_storage"]
     , ["src/buildtool/file_system", "git_repo"]
     , ["src/buildtool/file_system/symlinks_map", "pragma_special"]
-    , ["src/buildtool/multithreading", "async_map_utils"]
     , ["src/buildtool/multithreading", "task_system"]
     , ["src/buildtool/storage", "fs_utils"]
     , ["src/other_tools/git_operations", "git_repo_remote"]
diff --git a/src/other_tools/root_maps/content_git_map.cpp b/src/other_tools/root_maps/content_git_map.cpp
index f46e3940..d39a70e5 100644
--- a/src/other_tools/root_maps/content_git_map.cpp
+++ b/src/other_tools/root_maps/content_git_map.cpp
@@ -18,7 +18,6 @@
 #include "src/buildtool/file_system/file_root.hpp"
 #include "src/buildtool/file_system/file_storage.hpp"
 #include "src/buildtool/file_system/symlinks_map/pragma_special.hpp"
-#include "src/buildtool/multithreading/async_map_utils.hpp"
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/buildtool/storage/fs_utils.hpp"
 #include "src/other_tools/root_maps/root_utils.hpp"
@@ -234,25 +233,6 @@ void ResolveContentTree(
                  ts,
                  ws_setter,
                  logger](auto const& hashes) {
-                    if (not hashes[0]) {
-                        // check for cycles
-                        if (auto error = DetectAndReportCycle(
-                                fmt::format("resolving Git tree {}", tree_hash),
-                                *resolve_symlinks_map,
-                                kGitObjectToResolvePrinter)) {
-                            (*logger)(fmt::format("Failed to resolve symlinks "
-                                                  "in tree {}:\n{}",
-                                                  tree_hash,
-                                                  *error),
-                                      /*fatal=*/true);
-                            return;
-                        }
-                        (*logger)(fmt::format("Unknown error in resolving "
-                                              "symlinks in tree {}",
-                                              tree_hash),
-                                  /*fatal=*/true);
-                        return;
-                    }
                     auto const& resolved_tree_id = hashes[0]->id;
                     // keep tree alive in Git cache via a tagged commit
                     GitOpKey op_key = {
diff --git a/src/other_tools/root_maps/fpath_git_map.cpp b/src/other_tools/root_maps/fpath_git_map.cpp
index 37b0c543..dbda05ce 100644
--- a/src/other_tools/root_maps/fpath_git_map.cpp
+++ b/src/other_tools/root_maps/fpath_git_map.cpp
@@ -20,7 +20,6 @@
 #include "src/buildtool/execution_api/local/config.hpp"
 #include "src/buildtool/file_system/file_root.hpp"
 #include "src/buildtool/file_system/git_repo.hpp"
-#include "src/buildtool/multithreading/async_map_utils.hpp"
 #include "src/buildtool/multithreading/task_system.hpp"
 #include "src/buildtool/storage/fs_utils.hpp"
 #include "src/other_tools/git_operations/git_repo_remote.hpp"
@@ -151,25 +150,6 @@ void ResolveFilePathTree(
                  ts,
                  ws_setter,
                  logger](auto const& hashes) {
-                    if (not hashes[0]) {
-                        // check for cycles
-                        if (auto error = DetectAndReportCycle(
-                                fmt::format("resolving Git tree {}", tree_hash),
-                                *resolve_symlinks_map,
-                                kGitObjectToResolvePrinter)) {
-                            (*logger)(fmt::format("Failed to resolve symlinks "
-                                                  "in tree {}:\n{}",
-                                                  tree_hash,
-                                                  *error),
-                                      /*fatal=*/true);
-                            return;
-                        }
-                        (*logger)(fmt::format("Unknown error in resolving "
-                                              "symlinks in tree {}",
-                                              tree_hash),
-                                  /*fatal=*/true);
-                        return;
-                    }
                     auto const& resolved_tree_id = hashes[0]->id;
                     // keep tree alive in Git cache via a tagged commit
                     GitOpKey op_key = {