Merge pull request #19 from oreiche/stable-1.4

Release 1.4.2

CHANGELOG.md

@@ -1,3 +1,17 @@
+## Release `1.4.2` (2025-01-13)
+
+Bug fixes on top of `1.4.1`.
+
+### Fixes
+
+- A race condition in the use of `libgit2` was fixed that could
+  result in a segmentation fault.
+- The Git cache root repository on a `just serve` endpoint is now
+  ensured to always exist and be initialized before being operated on.
+- `just-mr` properly enforces that repository `subdir` entries are
+  non-upwards relative paths.
+- Various improvements in the documentation.
+
 ## Release `1.4.1` (2024-12-03)
 
 Bug fixes on top of `1.4.0`.

share/man/just-graph-file.5.md

@@ -3,7 +3,7 @@
 NAME
 ====
 
-just graph file - The format of the action graph used by **`just`**(1)
+just-graph-file - The format of the action graph used by **`just`**(1)
 
 DESCRIPTION
 ===========

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

@@ -3,7 +3,7 @@
 NAME
 ====
 
-just-mr repository config - The format of the repository config used by
+just-mr-repository-config - The format of the repository config used by
 **`just-mr`**(1)
 
 DESCRIPTION
@@ -184,7 +184,8 @@ part of a Git repository, its Git tree identifier is used; otherwise,
 the workspace root will be realized as a Git tree in the Git repository
 in **`just`**'s local build root.
 
-For all workspace roots except *`"distdir"`* the pragma key *`"special"`* is
+For all workspace roots except *`"distdir"`*, *`"computed"`*,
+and *`"tree structure"`*, the pragma key *`"special"`* is
 supported. If its value is *`"ignore"`* then it indicates that the workspace
 root should ignore all special (i.e., neither file, executable, nor tree)
 entries. For a *`"file"`* workspace root or for an *`"archive"`* workspace root
@@ -240,7 +241,9 @@ The repository configuration format is structured as a JSON object. The
 following fields are supported:
 
  - *`"main"`* contains a JSON string that determines which of the
-   provided repositories is considered the main repository.
+   provided repositories is considered the main repository. This entry
+   is optional, and if ommitted, it will be ommitted in the generated
+   **`just-repository-config`**.
 
  - *`"repositories"`* contains a JSON object, where each key is the
    global name of a repository and its corresponding value is the

share/man/just-mrrc.5.md

@@ -3,8 +3,7 @@
 NAME
 ====
 
-just-mr configuration - The format of the configuration used by
-**`just-mr`**(1)
+just-mrrc - The format of the configuration used by **`just-mr`**(1)
 
 DESCRIPTION
 ===========

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

@@ -3,7 +3,7 @@
 NAME
 ====
 
-just repository config - The format of the repository config used by
+just-repository-config - The format of the repository config used by
 **`just`**(1)
 
 DESCRIPTION
@@ -86,8 +86,8 @@ keys:
 
  - *`"main"`* contains a string, which defines the repository name to
    consider by default if not explicitly specified on the command line
-   (i.e., via **`--main`**). This entry is optional and if omitted the
-   empty string is used.
+   (i.e., via **`--main`**). This entry is optional and if omitted **`just`**
+   will use the lexicographically first repository, as usual.
 
  - *`"repositories"`* contains a JSON object that defines all
    repositories by mapping global repository names to *repository

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

@@ -3,8 +3,8 @@
 NAME
 ====
 
-**`just`** **`serve`** configuration - The format of the configuration used by
-the **`serve`** subcommand of **`just`**(1)
+just-serve-config - The format of the configuration used by the **`serve`**
+subcommand of **`just`**(1)
 
 DESCRIPTION
 ===========

share/man/just.1.md

@@ -487,7 +487,7 @@ element per failure, where the element is a pair (array of length
 2) consisting of the configured target (serialized, as usual, as a
 pair of qualified target name an configuration) and a string with
 the hex representation of the blob identifier of the log; the log
-itself is guaranteed to be available on the remote-execution side.
+itself is guaranteed to be available on the remote-execution side.  
 Supported by: analyse|build|install.
 
 **`-P`**, **`--print-to-stdout`** *`LOGICAL_PATH`*  

src/buildtool/execution_api/common/common_api.cpp

@@ -58,8 +58,8 @@ auto CommonRetrieveToFds(
                     fd);
                 // locally we might be able to fallback to Git in native mode
                 try {
-                    if (fallback and not(*fallback)(info, fd)) {
-                        return false;
+                    if (fallback) {
+                        success = (*fallback)(info, fd);
                     }
                 } catch (std::exception const& ex) {
                     Logger::Log(LogLevel::Error,
@@ -71,6 +71,9 @@ auto CommonRetrieveToFds(
                     return false;
                 }
             }
+            if (not success) {
+                return false;
+            }
         }
         else {
             Logger::Log(

src/buildtool/file_system/TARGETS

@@ -68,7 +68,8 @@
   , "name": ["git_cas"]
   , "hdrs": ["git_cas.hpp"]
   , "srcs": ["git_cas.cpp"]
-  , "deps": ["git_context", "git_utils", ["@", "gsl", "", "gsl"]]
+  , "deps":
+    ["git_context", "git_utils", "object_type", ["@", "gsl", "", "gsl"]]
   , "stage": ["src", "buildtool", "file_system"]
   , "private-deps":
     [ ["", "libgit2"]
@@ -137,6 +138,7 @@
     , ["src/utils/cpp", "gsl"]
     , ["src/utils/cpp", "hex_string"]
     , ["src/utils/cpp", "path"]
+    , ["src/utils/cpp", "tmp_dir"]
     ]
   , "cflags": ["-pthread"]
   }

src/buildtool/file_system/git_cas.cpp

@@ -57,19 +57,83 @@ GitCAS::GitCAS() noexcept {
 
 auto GitCAS::Open(std::filesystem::path const& repo_path) noexcept
     -> GitCASPtr {
-#ifndef BOOTSTRAP_BUILD_TOOL
+#ifdef BOOTSTRAP_BUILD_TOOL
+    return nullptr;
+#else
+    auto result = std::make_shared<GitCAS>();
+
+    // lock as git_repository API has no thread-safety guarantees
+    static std::mutex repo_mutex{};
+    std::unique_lock lock{repo_mutex};
+    git_repository* repo_ptr = nullptr;
+    if (git_repository_open_ext(&repo_ptr,
+                                repo_path.c_str(),
+                                GIT_REPOSITORY_OPEN_NO_SEARCH,
+                                nullptr) != 0 or
+        repo_ptr == nullptr) {
+        Logger::Log(LogLevel::Error,
+                    "Opening git repository {} failed with:\n{}",
+                    repo_path.string(),
+                    GitLastError());
+        return nullptr;
+    }
+    result->repo_.reset(repo_ptr);
+
+    git_odb* odb_ptr = nullptr;
+    if (git_repository_odb(&odb_ptr, result->repo_.get()) != 0 or
+        odb_ptr == nullptr) {
+        Logger::Log(LogLevel::Error,
+                    "Obtaining git object database {} failed with:\n{}",
+                    repo_path.string(),
+                    GitLastError());
+        return nullptr;
+    }
+    result->odb_.reset(odb_ptr);
+
+    auto const git_path =
+        git_repository_is_bare(result->repo_.get()) != 0
+            ? ToNormalPath(git_repository_path(result->repo_.get()))
+            : ToNormalPath(git_repository_workdir(result->repo_.get()));
+
     try {
-        auto cas = std::make_shared<GitCAS>();
-        if (cas->OpenODB(repo_path)) {
-            return std::static_pointer_cast<GitCAS const>(cas);
-        }
-    } catch (std::exception const& ex) {
+        result->git_path_ = std::filesystem::absolute(git_path);
+    } catch (std::exception const& e) {
         Logger::Log(LogLevel::Error,
-                    "opening git object database failed with:\n{}",
-                    ex.what());
+                    "Failed to obtain absolute path for {}: {}",
+                    git_path.string(),
+                    e.what());
+        return nullptr;
     }
+    return std::const_pointer_cast<GitCAS const>(result);
 #endif
+}
+
+auto GitCAS::CreateEmpty() noexcept -> GitCASPtr {
+#ifdef BOOTSTRAP_BUILD_TOOL
     return nullptr;
+#else
+    auto result = std::make_shared<GitCAS>();
+
+    git_odb* odb_ptr{nullptr};
+    if (git_odb_new(&odb_ptr) != 0 or odb_ptr == nullptr) {
+        Logger::Log(LogLevel::Error,
+                    "creating an empty database failed with:\n{}",
+                    GitLastError());
+        return nullptr;
+    }
+    result->odb_.reset(odb_ptr);  // retain odb pointer
+
+    git_repository* repo_ptr = nullptr;
+    if (git_repository_wrap_odb(&repo_ptr, result->odb_.get()) != 0 or
+        repo_ptr == nullptr) {
+        Logger::Log(LogLevel::Error,
+                    "creating an empty repository failed with:\n{}",
+                    GitLastError());
+        return nullptr;
+    }
+    result->repo_.reset(repo_ptr);  // retain repo pointer
+    return std::const_pointer_cast<GitCAS const>(result);
+#endif
 }
 
 auto GitCAS::ReadObject(std::string const& id, bool is_hex_id) const noexcept
@@ -87,15 +151,12 @@ auto GitCAS::ReadObject(std::string const& id, bool is_hex_id) const noexcept
     }
 
     git_odb_object* obj = nullptr;
-    {
-        std::shared_lock lock{mutex_};
-        if (git_odb_read(&obj, odb_.get(), &oid.value()) != 0) {
-            Logger::Log(LogLevel::Error,
-                        "reading git object {} from database failed with:\n{}",
-                        is_hex_id ? id : ToHexString(id),
-                        GitLastError());
-            return std::nullopt;
-        }
+    if (git_odb_read(&obj, odb_.get(), &oid.value()) != 0) {
+        Logger::Log(LogLevel::Error,
+                    "reading git object {} from database failed with:\n{}",
+                    is_hex_id ? id : ToHexString(id),
+                    GitLastError());
+        return std::nullopt;
     }
 
     std::string data(static_cast<char const*>(git_odb_object_data(obj)),
@@ -120,16 +181,13 @@ auto GitCAS::ReadHeader(std::string const& id, bool is_hex_id) const noexcept
 
     std::size_t size{};
     git_object_t type{};
-    {
-        std::shared_lock lock{mutex_};
-        if (git_odb_read_header(&size, &type, odb_.get(), &oid.value()) != 0) {
-            Logger::Log(LogLevel::Error,
-                        "reading git object header {} from database failed "
-                        "with:\n{}",
-                        is_hex_id ? id : ToHexString(id),
-                        GitLastError());
-            return std::nullopt;
-        }
+    if (git_odb_read_header(&size, &type, odb_.get(), &oid.value()) != 0) {
+        Logger::Log(LogLevel::Error,
+                    "reading git object header {} from database failed "
+                    "with:\n{}",
+                    is_hex_id ? id : ToHexString(id),
+                    GitLastError());
+        return std::nullopt;
     }
 
     if (auto obj_type = GitTypeToObjectType(type)) {
@@ -138,55 +196,3 @@ auto GitCAS::ReadHeader(std::string const& id, bool is_hex_id) const noexcept
 #endif
     return std::nullopt;
 }
-
-auto GitCAS::OpenODB(std::filesystem::path const& repo_path) noexcept -> bool {
-    static std::mutex repo_mutex{};
-#ifdef BOOTSTRAP_BUILD_TOOL
-    return false;
-#else
-    {  // lock as git_repository API has no thread-safety guarantees
-        std::unique_lock lock{repo_mutex};
-        git_repository* repo = nullptr;
-        if (git_repository_open(&repo, repo_path.c_str()) != 0) {
-            Logger::Log(LogLevel::Debug,
-                        "opening git repository {} failed with:\n{}",
-                        repo_path.string(),
-                        GitLastError());
-            return false;
-        }
-        git_odb* odb_ptr{nullptr};
-        git_repository_odb(&odb_ptr, repo);
-        odb_.reset(odb_ptr);  // retain odb pointer
-        // set root
-        std::filesystem::path git_path{};
-        if (git_repository_is_bare(repo) != 0) {
-            git_path = ToNormalPath((git_repository_path(repo)));
-        }
-        else {
-            git_path = ToNormalPath(git_repository_workdir(repo));
-        }
-        if (not git_path.is_absolute()) {
-            try {
-                git_path = std::filesystem::absolute(git_path);
-            } catch (std::exception const& e) {
-                Logger::Log(LogLevel::Error,
-                            "Failed to obtain absolute path for {}: {}",
-                            git_path.string(),
-                            e.what());
-                return false;
-            }
-        }
-        git_path_ = git_path;
-        // release resources
-        git_repository_free(repo);
-    }
-    if (not odb_) {
-        Logger::Log(LogLevel::Error,
-                    "obtaining git object database {} failed with:\n{}",
-                    repo_path.string(),
-                    GitLastError());
-        return false;
-    }
-    return true;
-#endif
-}