just-mr: Fix shell out execution

... by avoiding reusing temp dirs for execute. While we are
at it, also refactor LocalFetchViaTmpRepo() to create its
own empty temp dirs, that cannot be reused by the caller.

3 files changed, 25 insertions, 19 deletions

diff --git a/src/other_tools/git_operations/TARGETS b/src/other_tools/git_operations/TARGETS
index dba6fc18..1cd49035 100644
--- a/src/other_tools/git_operations/TARGETS
+++ b/src/other_tools/git_operations/TARGETS
@@ -36,6 +36,7 @@
     , ["@", "json", "", "json"]
     , ["src/buildtool/system", "system_command"]
     , "git_config_settings"
+    , ["src/utils/cpp", "tmp_dir"]
     ]
   }
 , "git_config_settings":
diff --git a/src/other_tools/git_operations/git_repo_remote.cpp b/src/other_tools/git_operations/git_repo_remote.cpp
index 2a434928..f3964e74 100644
--- a/src/other_tools/git_operations/git_repo_remote.cpp
+++ b/src/other_tools/git_operations/git_repo_remote.cpp
@@ -20,6 +20,7 @@
 #include "src/buildtool/logging/logger.hpp"
 #include "src/buildtool/system/system_command.hpp"
 #include "src/other_tools/git_operations/git_config_settings.hpp"
+#include "src/utils/cpp/tmp_dir.hpp"
 
 extern "C" {
 #include <git2.h>
@@ -392,7 +393,6 @@ auto GitRepoRemote::FetchFromRemote(std::shared_ptr<git_config> cfg,
 }
 
 auto GitRepoRemote::UpdateCommitViaTmpRepo(
-    std::filesystem::path const& tmp_dir,
     std::string const& repo_url,
     std::string const& branch,
     std::vector<std::string> const& inherit_env,
@@ -401,6 +401,13 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo(
     anon_logger_ptr const& logger) const noexcept
     -> std::optional<std::string> {
     try {
+        auto tmp_dir = TmpDir::Create("update");
+        if (not tmp_dir) {
+            (*logger)("Failed to create temp dir for running 'git ls-remote'",
+                      /*fatal=*/true);
+            return std::nullopt;
+        }
+        auto const& tmp_path = tmp_dir->GetPath();
         // check for internally supported protocols
         if (IsSupported(repo_url)) {
             // preferably with a "fake" repository!
@@ -410,7 +417,7 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo(
             }
             // create the temporary real repository
             auto tmp_repo =
-                GitRepoRemote::InitAndOpen(tmp_dir, /*is_bare=*/true);
+                GitRepoRemote::InitAndOpen(tmp_path / "git", /*is_bare=*/true);
             if (tmp_repo == std::nullopt) {
                 return std::nullopt;
             }
@@ -456,7 +463,7 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo(
             system.Execute(cmdline,
                            env,
                            GetGitPath(),  // which path is not actually relevant
-                           tmp_dir);
+                           tmp_path);
 
         if (not command_output) {
             (*logger)(fmt::format("exec() on command failed."),
@@ -522,8 +529,7 @@ auto GitRepoRemote::UpdateCommitViaTmpRepo(
     }
 }
 
-auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir,
-                                    std::string const& repo_url,
+auto GitRepoRemote::FetchViaTmpRepo(std::string const& repo_url,
                                     std::optional<std::string> const& branch,
                                     std::vector<std::string> const& inherit_env,
                                     std::string const& git_bin,
@@ -531,6 +537,13 @@ auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir,
                                     anon_logger_ptr const& logger) noexcept
     -> bool {
     try {
+        auto tmp_dir = TmpDir::Create("fetch");
+        if (not tmp_dir) {
+            (*logger)("Failed to create temp dir for running 'git fetch'",
+                      /*fatal=*/true);
+            return false;
+        }
+        auto const& tmp_path = tmp_dir->GetPath();
         // check for internally supported protocols
         if (IsSupported(repo_url)) {
             // preferably with a "fake" repository!
@@ -542,7 +555,7 @@ auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir,
             // it can be bare, as the refspecs for this fetch will be given
             // explicitly.
             auto tmp_repo =
-                GitRepoRemote::InitAndOpen(tmp_dir, /*is_bare=*/true);
+                GitRepoRemote::InitAndOpen(tmp_path / "git", /*is_bare=*/true);
             if (tmp_repo == std::nullopt) {
                 return false;
             }
@@ -606,7 +619,7 @@ auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir,
         // run command
         SystemCommand system{repo_url};
         auto const command_output =
-            system.Execute(cmdline, env, GetGitPath(), tmp_dir);
+            system.Execute(cmdline, env, GetGitPath(), tmp_path);
 
         if (not command_output) {
             (*logger)(fmt::format("exec() on command failed."),
diff --git a/src/other_tools/git_operations/git_repo_remote.hpp b/src/other_tools/git_operations/git_repo_remote.hpp
index 614e7822..c59fb48e 100644
--- a/src/other_tools/git_operations/git_repo_remote.hpp
+++ b/src/other_tools/git_operations/git_repo_remote.hpp
@@ -82,15 +82,11 @@ class GitRepoRemote : public GitRepo {
     /// \brief Get commit from given branch on the remote. If URL is SSH, shells
     /// out to system git to perform an ls-remote call, ensuring correct
     /// handling of the remote connection settings (in particular proxy and
-    /// SSH). A temporary directory is needed to pipe the stdout and stderr to.
-    /// If URL is non-SSH, uses tmp dir to connect to remote and retrieve the
-    /// commit of a branch asynchronously using libgit2.
-    /// Caller needs to make sure the temporary directory exists and that the
-    /// given path is thread- and process-safe!
+    /// SSH). For non-SSH URLs, the branch commit is retrieved asynchronously
+    /// using libgit2.
     /// Returns the commit hash, as a string, or nullopt if failure.
     /// It guarantees the logger is called exactly once with fatal if failure.
     [[nodiscard]] auto UpdateCommitViaTmpRepo(
-        std::filesystem::path const& tmp_dir,
         std::string const& repo_url,
         std::string const& branch,
         std::vector<std::string> const& inherit_env,
@@ -101,16 +97,12 @@ class GitRepoRemote : public GitRepo {
 
     /// \brief Fetch from a remote. If URL is SSH, shells out to system git to
     /// retrieve packs in a safe manner, with the only side-effect being that
-    /// there can be some redundancy in the fetched packs. The tmp dir is used
-    /// to pipe the stdout and stderr to.
-    /// If URL is non-SSH, uses tmp dir to fetch asynchronously using libgit2.
-    /// Caller needs to make sure the temporary directory exists and that the
-    /// given path is thread- and process-safe!
+    /// there can be some redundancy in the fetched packs.
+    /// For non-SSH URLs an asynchronous fetch is performed using libgit2.
     /// Uses either a given branch, or fetches all (with base refspecs).
     /// Returns a success flag.
     /// It guarantees the logger is called exactly once with fatal if failure.
     [[nodiscard]] auto FetchViaTmpRepo(
-        std::filesystem::path const& tmp_dir,
         std::string const& repo_url,
         std::optional<std::string> const& branch,
         std::vector<std::string> const& inherit_env,