GitRepo: Change logic that creates commits to explicitly give directory

In preparation for subsequent changes, specify the directory path
containing the tree content to be committed explicitly. This change
will allow eventually to be able to specify paths that are
different from the root path of the repository in which the commit
is created.

This commit renames and refactors StageAndCommitAllAnnonymous to
allow a directory path to be passed. The just-mr and serve service
logic is updated such that current behaviour is otherwise
unchanged.

5 files changed, 48 insertions, 28 deletions

diff --git a/src/buildtool/file_system/git_repo.cpp b/src/buildtool/file_system/git_repo.cpp
index d378a2ba..11c5a569 100644
--- a/src/buildtool/file_system/git_repo.cpp
+++ b/src/buildtool/file_system/git_repo.cpp
@@ -591,8 +591,9 @@ auto GitRepo::GetGitOdb() const noexcept
     return git_cas_->odb_;
 }
 
-auto GitRepo::StageAndCommitAllAnonymous(std::string const& message,
-                                         anon_logger_ptr const& logger) noexcept
+auto GitRepo::CommitDirectory(std::filesystem::path const& dir,
+                              std::string const& message,
+                              anon_logger_ptr const& logger) noexcept
     -> std::optional<std::string> {
 #ifdef BOOTSTRAP_BUILD_TOOL
     return std::nullopt;
@@ -600,7 +601,7 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message,
     try {
         // only possible for real repository!
         if (IsRepoFake()) {
-            (*logger)("cannot stage and commit files using a fake repository!",
+            (*logger)("cannot commit directory using a fake repository!",
                       true /*fatal*/);
             return std::nullopt;
         }
@@ -610,11 +611,23 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message,
         // cannot perform this operation on a bare repository; this has to be
         // checked because git_index_add_bypath will not do it for us!
         if (not FileSystemManager::Exists(GetGitPath() / ".git")) {
-            (*logger)("cannot stage and commit files in a bare repository!",
+            (*logger)("cannot commit directory in a bare repository!",
                       true /*fatal*/);
             return std::nullopt;
         }
 
+        // the index approach to adding entries requires them to be reachable
+        // from root path
+        auto rel_path = dir.lexically_relative(GetGitPath());
+        if (rel_path.empty() or rel_path.string().starts_with("..")) {
+            (*logger)(
+                fmt::format("unsupported directory {}: not a subpath of {}",
+                            dir.string(),
+                            GetGitPath().string()),
+                true /*fatal*/);
+            return std::nullopt;
+        }
+
         // add all files to be staged
         git_index* index_ptr{nullptr};
         git_repository_index(&index_ptr, repo_->Ptr());
@@ -624,19 +637,23 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message,
         // due to mismanagement of .gitignore rules by libgit2 when doing a
         // forced add all, we resort to using git_index_add_bypath manually for
         // all entries, instead of git_index_add_all with GIT_INDEX_ADD_FORCE.
-        auto use_entry = [&index](std::filesystem::path const& name,
-                                  bool is_tree) {
+        auto use_entry = [&index, rel_path](std::filesystem::path const& name,
+                                            bool is_tree) {
             return is_tree or
-                   git_index_add_bypath(index.get(), name.c_str()) == 0;
+                   git_index_add_bypath(
+                       index.get(),
+                       (rel_path / name)
+                           .lexically_relative(".")  // remove "." prefix
+                           .c_str()) == 0;
         };
         if (not FileSystemManager::ReadDirectoryEntriesRecursive(
-                GetGitPath(),
+                dir,
                 use_entry,
                 /*ignored_subdirs=*/{".git"})) {
-            (*logger)(fmt::format(
-                          "staging files in git repository {} failed with:\n{}",
-                          GetGitPath().string(),
-                          GitLastError()),
+            (*logger)(fmt::format("staging entries in git repository {} failed "
+                                  "with:\n{}",
+                                  GetGitPath().string(),
+                                  GitLastError()),
                       true /*fatal*/);
             return std::nullopt;
         }
@@ -711,9 +728,8 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message,
         git_buf_dispose(&buffer);
         return commit_hash;  // success!
     } catch (std::exception const& ex) {
-        Logger::Log(LogLevel::Error,
-                    "stage and commit all failed with:\n{}",
-                    ex.what());
+        Logger::Log(
+            LogLevel::Error, "commit subdir failed with:\n{}", ex.what());
         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 1e5ec839..31d9eb82 100644
--- a/src/buildtool/file_system/git_repo.hpp
+++ b/src/buildtool/file_system/git_repo.hpp
@@ -151,13 +151,16 @@ class GitRepo {
     using anon_logger_t = std::function<void(std::string const&, bool)>;
     using anon_logger_ptr = std::shared_ptr<anon_logger_t>;
 
-    /// \brief Stage all in current path and commit with given message.
+    /// \brief Create tree from entries at given directory and commit it with
+    /// given message. Currently, the caller must guarantee that given path is
+    /// a subdirectory of the repository root path.
     /// Only possible with real repository and thus non-thread-safe.
-    /// Returns the commit hash, or nullopt if failure.
-    /// It guarantees the logger is called exactly once with fatal if failure.
-    [[nodiscard]] auto StageAndCommitAllAnonymous(
-        std::string const& message,
-        anon_logger_ptr const& logger) noexcept -> std::optional<std::string>;
+    /// \returns The commit hash, or nullopt if failure. It guarantees the
+    /// logger is called exactly once with fatal if failure.
+    [[nodiscard]] auto CommitDirectory(std::filesystem::path const& dir,
+                                       std::string const& message,
+                                       anon_logger_ptr const& logger) noexcept
+        -> std::optional<std::string>;
 
     /// \brief Create annotated tag for given commit.
     /// Only possible with real repository and thus non-thread-safe.
diff --git a/src/buildtool/serve_api/serve_service/source_tree.cpp b/src/buildtool/serve_api/serve_service/source_tree.cpp
index 5de4c389..dd42ab83 100644
--- a/src/buildtool/serve_api/serve_service/source_tree.cpp
+++ b/src/buildtool/serve_api/serve_service/source_tree.cpp
@@ -470,7 +470,7 @@ auto SourceTreeService::CommonImportToGit(
         });
     // stage and commit all
     auto commit_hash =
-        git_repo->StageAndCommitAllAnonymous(commit_message, wrapped_logger);
+        git_repo->CommitDirectory(root_path, commit_message, wrapped_logger);
     if (not commit_hash) {
         return unexpected{err};
     }
diff --git a/src/other_tools/git_operations/git_operations.cpp b/src/other_tools/git_operations/git_operations.cpp
index 8f5b7ad7..bfd1d18f 100644
--- a/src/other_tools/git_operations/git_operations.cpp
+++ b/src/other_tools/git_operations/git_operations.cpp
@@ -42,8 +42,9 @@ auto CriticalGitOps::GitInitialCommit(GitOpParams const& crit_op_params,
                 fatal);
         });
     // Stage and commit all at the target location
-    auto commit_hash = git_repo->StageAndCommitAllAnonymous(
-        crit_op_params.message.value(), wrapped_logger);
+    auto commit_hash = git_repo->CommitDirectory(crit_op_params.target_path,
+                                                 crit_op_params.message.value(),
+                                                 wrapped_logger);
     if (commit_hash == std::nullopt) {
         return {.git_cas = nullptr, .result = std::nullopt};
     }
diff --git a/test/buildtool/file_system/git_repo.test.cpp b/test/buildtool/file_system/git_repo.test.cpp
index 969f2072..5a0831cc 100644
--- a/test/buildtool/file_system/git_repo.test.cpp
+++ b/test/buildtool/file_system/git_repo.test.cpp
@@ -182,7 +182,7 @@ TEST_CASE("Single-threaded real repository local operations", "[git_repo]") {
                         std::string(msg));
         });
 
-    SECTION("Stage and commit all") {
+    SECTION("Commit directory") {
         // make blank repo
         auto repo_commit_path = TestUtils::GetRepoPath();
         auto repo_commit =
@@ -196,9 +196,9 @@ TEST_CASE("Single-threaded real repository local operations", "[git_repo]") {
         REQUIRE(FileSystemManager::WriteFile(
             "test no 2", repo_commit_path / "test2.txt", true));
 
-        // stage and commit all
-        auto commit =
-            repo_commit->StageAndCommitAllAnonymous("test commit", logger);
+        // commit subdir
+        auto commit = repo_commit->CommitDirectory(
+            repo_commit_path, "test commit", logger);
         CHECK(commit);
     }