Merge pull request #18 from oreiche/stable-1.4

Release 1.4.1

CHANGELOG.md

@@ -1,3 +1,16 @@
+## Release `1.4.1` (2024-12-03)
+
+Bug fixes on top of `1.4.0`.
+
+### Fixes
+
+- Fixes ensuring proper pointer life time and access check.
+- Git operations are now properly locked against each other, also
+  between processes where necessary.
+- `just traverse` now exits unconditionally after traversal, also
+  in case of failure.
+- Missing entries in the documentation have been added.
+
 ## Release `1.4.0` (2024-11-04)
 
 A feature release on top of `1.3.0`, backwards compatible with

share/man/just-repository-config.5.md

@@ -26,14 +26,16 @@ which determines the type and semantic of the subsequent elements:
    path to the file root.
 
  - *`"git tree"`* refers to a file root that is available as part of a
-   Git repository. The list has to be of length 3 with the remaining two
+   Git repository. The list has to be of length 2 or 3 with the remaining two
    elements being:
 
    1. The *`git tree hash`*, which is sufficient to describe the content
       of an entire tree including its sub-trees and blobs. The tree hash
       has to be specified in hex encoding.
    2. The path to a Git repository on the file system with the promise
-      that it contains the aforementioned *`git tree hash`*.
+      that it contains the aforementioned *`git tree hash`*; if this
+      entry is missing, the root is considered absent and any target
+      requiring this root has to come from a specified serve end point.
 
 Repository description
 ----------------------

src/buildtool/build_engine/target_map/target_map.cpp

@@ -1944,9 +1944,9 @@ auto CreateTargetMap(
             );
         }
 #ifndef BOOTSTRAP_BUILD_TOOL
-        else if (context->repo_config
-                     ->TargetRoot(key.target.ToModule().repository)
-                     ->IsAbsent()) {
+        else if (auto const* const file_root = context->repo_config->TargetRoot(
+                     key.target.ToModule().repository);
+                 file_root != nullptr and file_root->IsAbsent()) {
             if (context->serve == nullptr) {
                 (*logger)(
                     fmt::format("Root for target {} is absent, but no serve "

src/buildtool/file_system/TARGETS

@@ -133,6 +133,7 @@
     , ["src/buildtool/file_system", "file_system_manager"]
     , ["src/buildtool/logging", "log_level"]
     , ["src/buildtool/logging", "logging"]
+    , ["src/utils/cpp", "file_locking"]
     , ["src/utils/cpp", "gsl"]
     , ["src/utils/cpp", "hex_string"]
     , ["src/utils/cpp", "path"]

src/buildtool/file_system/git_repo.cpp

@@ -27,6 +27,7 @@
 #include "src/buildtool/file_system/file_system_manager.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
+#include "src/utils/cpp/file_locking.hpp"
 #include "src/utils/cpp/gsl.hpp"
 #include "src/utils/cpp/hex_string.hpp"
 #include "src/utils/cpp/path.hpp"
@@ -379,8 +380,8 @@ auto GitRepo::GuardedRepo::PtrRef() -> git_repository** {
 
 GitRepo::GuardedRepo::~GuardedRepo() noexcept {
 #ifndef BOOTSTRAP_BUILD_TOOL
+    std::unique_lock lock{*mutex_};
     if (repo_ != nullptr) {
-        std::unique_lock lock{*mutex_};
         git_repository_free(repo_);
     }
 #endif
@@ -518,40 +519,45 @@ auto GitRepo::InitAndOpen(std::filesystem::path const& repo_path,
 
         GitContext::Create();  // initialize libgit2
 
-        // check if init is actually needed
-        if (git_repository_open_ext(nullptr,
-                                    repo_path.c_str(),
-                                    GIT_REPOSITORY_OPEN_NO_SEARCH,
-                                    nullptr) == 0) {
-            return GitRepo(repo_path);  // success
-        }
-
         git_repository* tmp_repo{nullptr};
-        std::size_t max_attempts = kGitLockNumTries;  // number of tries
         int err = 0;
         std::string err_mess{};
-        while (max_attempts > 0) {
-            --max_attempts;
-            err = git_repository_init(&tmp_repo,
-                                      repo_path.c_str(),
-                                      static_cast<std::size_t>(is_bare));
+        for (auto attempt = 0U; attempt < kGitLockNumTries; ++attempt) {
+            // check if init is needed or has already happened in another
+            // process
+            if (git_repository_open_ext(nullptr,
+                                        repo_path.c_str(),
+                                        GIT_REPOSITORY_OPEN_NO_SEARCH,
+                                        nullptr) == 0) {
+                return GitRepo(repo_path);  // success
+            }
+
+            // Initialization must be guarded across processes trying to
+            // initialize the same repo.
+            if (auto const lock_file = LockFile::Acquire(
+                    repo_path.parent_path() / "init_open.lock",
+                    /*is_shared=*/false)) {
+                err = git_repository_init(&tmp_repo,
+                                          repo_path.c_str(),
+                                          static_cast<std::size_t>(is_bare));
+            }
+            else {
+                Logger::Log(LogLevel::Error,
+                            "initializing git repository {} failed to "
+                            "acquire lock.");
+                return std::nullopt;
+            }
+
             if (err == 0) {
                 git_repository_free(tmp_repo);
                 return GitRepo(repo_path);  // success
             }
-            err_mess = GitLastError();  // store last error message
+            err_mess = GitLastError();      // store last error message
+            git_repository_free(tmp_repo);  // cleanup before next attempt
             // only retry if failure is due to locking
             if (err != GIT_ELOCKED) {
                 break;
             }
-            git_repository_free(tmp_repo);  // cleanup before next attempt
-            // check if init hasn't already happened in another process
-            if (git_repository_open_ext(nullptr,
-                                        repo_path.c_str(),
-                                        GIT_REPOSITORY_OPEN_NO_SEARCH,
-                                        nullptr) == 0) {
-                return GitRepo(repo_path);  // success
-            }
             // repo still not created, so sleep and try again
             std::this_thread::sleep_for(
                 std::chrono::milliseconds(kGitLockWaitTime));
@@ -2097,6 +2103,21 @@ auto GitRepo::CreateTreeFromDirectory(std::filesystem::path const& dir,
 #endif  // BOOTSTRAP_BUILD_TOOL
 }
 
+void GitRepo::GitStrArray::AddEntry(std::string entry) {
+    std::size_t const prev_capacity = entries_.capacity();
+    entries_.emplace_back(std::move(entry));
+    if (prev_capacity == entries_.capacity()) {
+        entry_pointers_.push_back(entries_.back().data());
+    }
+    else {
+        // update pointers if reallocation happened
+        entry_pointers_.resize(entries_.size());
+        for (std::size_t i = 0; i < entries_.size(); ++i) {
+            entry_pointers_[i] = entries_[i].data();
+        }
+    }
+}
+
 auto GitRepo::GitStrArray::Get() & noexcept -> git_strarray {
     return git_strarray{.strings = entry_pointers_.data(),
                         .count = entry_pointers_.size()};

src/buildtool/file_system/git_repo.hpp

@@ -380,11 +380,7 @@ class GitRepo {
 
     class GitStrArray final {
       public:
-        void AddEntry(std::string entry) {
-            char* const entry_ptr =
-                entries_.emplace_back(std::move(entry)).data();
-            entry_pointers_.push_back(entry_ptr);
-        }
+        void AddEntry(std::string entry);
         [[nodiscard]] auto Get() & noexcept -> git_strarray;
 
       private: