GitRepo: Fix wrong handling of fatal logging

...in creating Git tree from filesystem directory.

2 files changed, 24 insertions, 15 deletions

diff --git a/src/buildtool/file_system/git_repo.cpp b/src/buildtool/file_system/git_repo.cpp
index 2e42db4a..b3ac2a36 100644
--- a/src/buildtool/file_system/git_repo.cpp
+++ b/src/buildtool/file_system/git_repo.cpp
@@ -2097,12 +2097,9 @@ auto GitRepo::CreateShallowTree(tree_entries_t const& entries) noexcept
         for (auto const& entry : std::filesystem::directory_iterator{dir}) {
             if (auto type = FileSystemManager::Type(entry.path(),
                                                     /*allow_upwards=*/true)) {
-                if (not read_and_store_entry(entry.path().filename(), *type)) {
-                    std::invoke(*logger,
-                                fmt::format("Could not read and store to ODB "
-                                            "subdir entry {}",
-                                            entry.path().string()),
-                                /*fatal=*/true);
+                if (not read_and_store_entry(
+                        entry.path().filename(), *type, logger)) {
+                    // logging with fatal already handled
                     return false;
                 }
             }
@@ -2133,8 +2130,10 @@ auto GitRepo::CreateTreeFromDirectory(std::filesystem::path const& dir,
     return std::nullopt;
 #else
     tree_entries_t entries{};
-    auto dir_read_and_store = [this, &entries, dir, logger](auto name,
-                                                            auto type) {
+    StoreDirEntryFunc dir_read_and_store =
+        [this, &entries, dir](std::filesystem::path const& name,
+                              ObjectType type,
+                              anon_logger_ptr const& logger) -> bool {
         try {
             const auto full_name = dir / name;
             if (IsTreeObject(type)) {
@@ -2145,10 +2144,7 @@ auto GitRepo::CreateTreeFromDirectory(std::filesystem::path const& dir,
                                                              ObjectType::Tree);
                     return true;
                 }
-                std::invoke(
-                    *logger,
-                    fmt::format("Failed creating tree {}", full_name.string()),
-                    /*fatal=*/true);
+                // logging with fatal already handled
                 return false;
             }
             // for non-tree entries, read content and write it as a blob to the
@@ -2179,8 +2175,16 @@ auto GitRepo::CreateTreeFromDirectory(std::filesystem::path const& dir,
     };
 
     if (ReadDirectory(dir, dir_read_and_store, logger)) {
-        return CreateTree(entries);
+        if (auto tree = CreateTree(entries)) {
+            return tree;
+        }
+        std::invoke(
+            *logger,
+            fmt::format("failed to create tree from entries of directory {}",
+                        dir.string()),
+            /*fatal=*/true);
     }
+    // logging with fatal already handled
     return std::nullopt;
 #endif  // BOOTSTRAP_BUILD_TOOL
 }
diff --git a/src/buildtool/file_system/git_repo.hpp b/src/buildtool/file_system/git_repo.hpp
index c559cbba..3fcbbf83 100644
--- a/src/buildtool/file_system/git_repo.hpp
+++ b/src/buildtool/file_system/git_repo.hpp
@@ -361,13 +361,17 @@ class GitRepo {
     /// \brief Open real repository at given location.
     explicit GitRepo(std::filesystem::path const& repo_path) noexcept;
 
-    using StoreDirEntryFunc =
-        std::function<bool(std::filesystem::path const&, ObjectType type)>;
+    /// \brief Function type handling directory entries read from filesystem.
+    /// \returns Success flag. Must guarantee that the logger is called exactly
+    /// once with fatal if returning false.
+    using StoreDirEntryFunc = std::function<
+        bool(std::filesystem::path const&, ObjectType, anon_logger_ptr const&)>;
 
     /// \brief Helper function to read the entries of a filesystem subdirectory
     /// and store them to the ODB. It is a modified version of the same-named
     /// function from FileSystemManager which accepts a subdir and a specific
     /// logger instead of the default.
+    /// It guarantees the logger is called exactly once with fatal if failure.
     [[nodiscard]] static auto ReadDirectory(
         std::filesystem::path const& dir,
         StoreDirEntryFunc const& read_and_store_entry,
@@ -376,6 +380,7 @@ class GitRepo {
     /// \brief Create a tree from the content of a directory by recursively
     /// adding its entries to the object database.
     /// \return The raw id of the tree.
+    /// It guarantees the logger is called exactly once with fatal if failure.
     [[nodiscard]] auto CreateTreeFromDirectory(
         std::filesystem::path const& dir,
         anon_logger_ptr const& logger) noexcept -> std::optional<std::string>;