LocalTreeMap: Prevent tree objects from being stored

... to align with the original idea of caching a flat list
of blob objects, without the need to recursively traverse
any trees. Consequently, we cannot create any map entry in
places where we do not have all sub-tree entries at hand
(e.g., LocalAPI, BazelAPI, BazelResponse).

5 files changed, 61 insertions, 99 deletions

diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 0c15e788..af0f2476 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -248,20 +248,9 @@ auto LocalAction::CollectOutputDir(std::filesystem::path const& exec_path,
             [this](auto path, auto is_exec) {
                 return storage_->StoreBlob</*kOwner=*/true>(path, is_exec);
             },
-            [this](auto bytes, auto dir) -> std::optional<bazel_re::Digest> {
-                auto digest = storage_->StoreBlob(bytes);
-                if (digest and not tree_map_->HasTree(*digest)) {
-                    auto tree = tree_map_->CreateTree();
-                    if (not BazelMsgFactory::ReadObjectInfosFromDirectory(
-                            dir,
-                            [&tree](auto path, auto info) {
-                                return tree.AddInfo(path, info);
-                            }) or
-                        not tree_map_->AddTree(*digest, std::move(tree))) {
-                        return std::nullopt;
-                    }
-                }
-                return digest;
+            [this](auto bytes,
+                   auto /*dir*/) -> std::optional<bazel_re::Digest> {
+                return storage_->StoreBlob(bytes);
             });
     }
     else {
@@ -271,27 +260,8 @@ auto LocalAction::CollectOutputDir(std::filesystem::path const& exec_path,
                 return storage_->StoreBlob</*kOwner=*/true>(path, is_exec);
             },
             [this](auto bytes,
-                   auto entries) -> std::optional<bazel_re::Digest> {
-                auto digest = storage_->StoreTree(bytes);
-                if (digest and not tree_map_->HasTree(*digest)) {
-                    auto tree = tree_map_->CreateTree();
-                    for (auto const& [raw_id, es] : entries) {
-                        auto id = ToHexString(raw_id);
-                        for (auto const& entry : es) {
-                            auto info = Artifact::ObjectInfo{
-                                ArtifactDigest{
-                                    id, 0, entry.type == ObjectType::Tree},
-                                entry.type};
-                            if (not tree.AddInfo(entry.name, info)) {
-                                return std::nullopt;
-                            }
-                        }
-                    }
-                    if (not tree_map_->AddTree(*digest, std::move(tree))) {
-                        return std::nullopt;
-                    }
-                }
-                return digest;
+                   auto /*entries*/) -> std::optional<bazel_re::Digest> {
+                return storage_->StoreTree(bytes);
             });
     }
     if (digest) {
diff --git a/src/buildtool/execution_api/local/local_action.hpp b/src/buildtool/execution_api/local/local_action.hpp
index 43569ea3..315897e9 100644
--- a/src/buildtool/execution_api/local/local_action.hpp
+++ b/src/buildtool/execution_api/local/local_action.hpp
@@ -37,7 +37,6 @@ class LocalAction final : public IExecutionAction {
   private:
     Logger logger_{"LocalExecution"};
     std::shared_ptr<LocalStorage> storage_;
-    std::shared_ptr<LocalTreeMap> tree_map_;
     ArtifactDigest root_digest_{};
     std::vector<std::string> cmdline_{};
     std::vector<std::string> output_files_{};
@@ -48,7 +47,6 @@ class LocalAction final : public IExecutionAction {
     CacheFlag cache_flag_{CacheFlag::CacheOutput};
 
     LocalAction(std::shared_ptr<LocalStorage> storage,
-                std::shared_ptr<LocalTreeMap> tree_map,
                 ArtifactDigest root_digest,
                 std::vector<std::string> command,
                 std::vector<std::string> output_files,
@@ -56,7 +54,6 @@ class LocalAction final : public IExecutionAction {
                 std::map<std::string, std::string> env_vars,
                 std::map<std::string, std::string> const& properties) noexcept
         : storage_{std::move(storage)},
-          tree_map_{std::move(tree_map)},
           root_digest_{std::move(root_digest)},
           cmdline_{std::move(command)},
           output_files_{std::move(output_files)},
diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp
index 38b98601..13bcb7e3 100644
--- a/src/buildtool/execution_api/local/local_api.hpp
+++ b/src/buildtool/execution_api/local/local_api.hpp
@@ -10,7 +10,6 @@
 #include "src/buildtool/compatibility/native_support.hpp"
 #include "src/buildtool/execution_api/bazel_msg/bazel_blob.hpp"
 #include "src/buildtool/execution_api/common/execution_api.hpp"
-#include "src/buildtool/execution_api/common/local_tree_map.hpp"
 #include "src/buildtool/execution_api/local/local_action.hpp"
 #include "src/buildtool/execution_api/local/local_storage.hpp"
 
@@ -26,7 +25,6 @@ class LocalApi final : public IExecutionApi {
         std::map<std::string, std::string> const& properties) noexcept
         -> IExecutionAction::Ptr final {
         return IExecutionAction::Ptr{new LocalAction{storage_,
-                                                     tree_map_,
                                                      root_digest,
                                                      command,
                                                      output_files,
@@ -202,11 +200,9 @@ class LocalApi final : public IExecutionApi {
         std::vector<DependencyGraph::NamedArtifactNodePtr> const&
             artifacts) noexcept -> std::optional<ArtifactDigest> final {
         BlobContainer blobs{};
-        auto tree = tree_map_->CreateTree();
         auto digest = BazelMsgFactory::CreateDirectoryDigestFromTree(
             artifacts,
-            [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); },
-            [&tree](auto path, auto info) { return tree.AddInfo(path, info); });
+            [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); });
         if (not digest) {
             Logger::Log(LogLevel::Debug, "failed to create digest for tree.");
             return std::nullopt;
@@ -217,10 +213,7 @@ class LocalApi final : public IExecutionApi {
             return std::nullopt;
         }
 
-        if (tree_map_->AddTree(*digest, std::move(tree))) {
-            return ArtifactDigest{*digest};
-        }
-        return std::nullopt;
+        return ArtifactDigest{*digest};
     }
 
     [[nodiscard]] auto IsAvailable(ArtifactDigest const& digest) const noexcept
@@ -247,9 +240,7 @@ class LocalApi final : public IExecutionApi {
     }
 
   private:
-    std::shared_ptr<LocalTreeMap> tree_map_{std::make_shared<LocalTreeMap>()};
-    std::shared_ptr<LocalStorage> storage_{
-        std::make_shared<LocalStorage>(tree_map_)};
+    std::shared_ptr<LocalStorage> storage_{std::make_shared<LocalStorage>()};
 };
 
 #endif  // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_LOCAL_LOCAL_API_HPP
diff --git a/src/buildtool/execution_api/local/local_storage.cpp b/src/buildtool/execution_api/local/local_storage.cpp
index e6d9571c..483f664f 100644
--- a/src/buildtool/execution_api/local/local_storage.cpp
+++ b/src/buildtool/execution_api/local/local_storage.cpp
@@ -114,69 +114,77 @@ auto LocalStorage::ReadObjectInfosRecursively(
     std::filesystem::path const& parent,
     bazel_re::Digest const& digest) const noexcept -> bool {
     // read from in-memory tree map
-    if (tree_map_) {
-        auto const* tree = tree_map_->GetTree(digest);
-        if (tree != nullptr) {
-            return std::all_of(
-                tree->begin(),
-                tree->end(),
-                // NOLINTNEXTLINE(misc-no-recursion)
-                [this, &store_info, &parent](auto const& entry) {
-                    try {
-                        auto const& [path, info] = entry;
-                        return IsTreeObject(info->type)
-                                   ? ReadObjectInfosRecursively(store_info,
-                                                                parent / path,
-                                                                info->digest)
-                                   : store_info(parent / path, *info);
-                    } catch (...) {  // satisfy clang-tidy, store_info() could
-                        return false;
-                    }
-                });
-        }
-        Logger::Log(
-            LogLevel::Debug, "tree {} not found in tree map", digest.hash());
+    auto const* tree = tree_map_.GetTree(digest);
+    if (tree != nullptr) {
+        return std::all_of(
+            tree->begin(),
+            tree->end(),
+            [&store_info, &parent](auto const& entry) {
+                try {
+                    // LocalTree (from tree_map_) is flat, no recursion needed
+                    auto const& [path, info] = entry;
+                    return store_info(parent / path, *info);
+                } catch (...) {  // satisfy clang-tidy, store_info() could
+                    return false;
+                }
+            });
     }
+    Logger::Log(
+        LogLevel::Debug, "tree {} not found in tree map", digest.hash());
 
     // fallback read from CAS and cache it in in-memory tree map
     if (Compatibility::IsCompatible()) {
         if (auto dir = ReadDirectory(this, digest)) {
-            auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree())
-                                  : std::nullopt;
+            auto tree = tree_map_.CreateTree();
             return BazelMsgFactory::ReadObjectInfosFromDirectory(
                        *dir,
                        [this, &store_info, &parent, &tree](auto path,
                                                            auto info) {
-                           return (not tree or tree->AddInfo(path, info)) and
-                                  (IsTreeObject(info.type)
-                                       ? ReadObjectInfosRecursively(
-                                             store_info,
-                                             parent / path,
-                                             info.digest)
-                                       : store_info(parent / path, info));
+                           if (IsTreeObject(info.type)) {
+                               // LocalTree (from tree_map_) is flat, so
+                               // recursively traverse subtrees and add blobs.
+                               auto tree_store_info =
+                                   [&store_info, &tree, &parent](auto path,
+                                                                 auto info) {
+                                       auto tree_path =
+                                           path.lexically_relative(parent);
+                                       return tree.AddInfo(tree_path, info) and
+                                              store_info(path, info);
+                                   };
+                               return ReadObjectInfosRecursively(
+                                   tree_store_info, parent / path, info.digest);
+                           }
+                           return tree.AddInfo(path, info) and
+                                  store_info(parent / path, info);
                        }) and
-                   (not tree_map_ or
-                    tree_map_->AddTree(digest, std::move(*tree)));
+                   tree_map_.AddTree(digest, std::move(tree));
         }
     }
     else {
         if (auto entries = ReadGitTree(this, digest)) {
-            auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree())
-                                  : std::nullopt;
+            auto tree = tree_map_.CreateTree();
             return BazelMsgFactory::ReadObjectInfosFromGitTree(
                        *entries,
                        [this, &store_info, &parent, &tree](auto path,
                                                            auto info) {
-                           return (not tree or tree->AddInfo(path, info)) and
-                                  (IsTreeObject(info.type)
-                                       ? ReadObjectInfosRecursively(
-                                             store_info,
-                                             parent / path,
-                                             info.digest)
-                                       : store_info(parent / path, info));
+                           if (IsTreeObject(info.type)) {
+                               // LocalTree (from tree_map_) is flat, so
+                               // recursively traverse subtrees and add blobs.
+                               auto tree_store_info =
+                                   [&store_info, &tree, &parent](auto path,
+                                                                 auto info) {
+                                       auto tree_path =
+                                           path.lexically_relative(parent);
+                                       return tree.AddInfo(tree_path, info) and
+                                              store_info(path, info);
+                                   };
+                               return ReadObjectInfosRecursively(
+                                   tree_store_info, parent / path, info.digest);
+                           }
+                           return tree.AddInfo(path, info) and
+                                  store_info(parent / path, info);
                        }) and
-                   (not tree_map_ or
-                    tree_map_->AddTree(digest, std::move(*tree)));
+                   tree_map_.AddTree(digest, std::move(tree));
         }
     }
     return false;
diff --git a/src/buildtool/execution_api/local/local_storage.hpp b/src/buildtool/execution_api/local/local_storage.hpp
index 4ffe2881..f9594218 100644
--- a/src/buildtool/execution_api/local/local_storage.hpp
+++ b/src/buildtool/execution_api/local/local_storage.hpp
@@ -12,10 +12,6 @@
 
 class LocalStorage {
   public:
-    explicit LocalStorage(
-        std::shared_ptr<LocalTreeMap> tree_map = nullptr) noexcept
-        : tree_map_{std::move(tree_map)} {}
-
     /// \brief Store blob from file path with x-bit determined from file system.
     template <bool kOwner = false>
     [[nodiscard]] auto StoreBlob(std::filesystem::path const& file_path)
@@ -89,7 +85,7 @@ class LocalStorage {
     LocalCAS<ObjectType::Executable> cas_exec_{};
     LocalCAS<ObjectType::Tree> cas_tree_{};
     LocalAC ac_{&cas_file_};
-    std::shared_ptr<LocalTreeMap> tree_map_;
+    mutable LocalTreeMap tree_map_;
 
     /// \brief Try to sync blob between file CAS and executable CAS.
     /// \param digest        Blob digest.