diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/buildtool/file_system/git_cas.cpp | 32 | ||||
| -rw-r--r-- | src/buildtool/file_system/git_cas.hpp | 6 | ||||
| -rw-r--r-- | src/buildtool/file_system/git_repo.cpp | 149 | ||||
| -rw-r--r-- | src/buildtool/file_system/git_repo.hpp | 35 | ||||
| -rw-r--r-- | src/buildtool/file_system/git_utils.cpp | 6 | ||||
| -rw-r--r-- | src/buildtool/file_system/git_utils.hpp | 3 | ||||
| -rw-r--r-- | src/other_tools/git_operations/git_repo_remote.cpp | 8 |
7 files changed, 165 insertions, 74 deletions
diff --git a/src/buildtool/file_system/git_cas.cpp b/src/buildtool/file_system/git_cas.cpp index 2a4a7142..1f8525fe 100644 --- a/src/buildtool/file_system/git_cas.cpp +++ b/src/buildtool/file_system/git_cas.cpp @@ -81,12 +81,15 @@ auto GitCAS::ReadObject(std::string const& id, bool is_hex_id) const noexcept } git_odb_object* obj = nullptr; - 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::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; + } } std::string data(static_cast<char const*>(git_odb_object_data(obj)), @@ -111,13 +114,16 @@ auto GitCAS::ReadHeader(std::string const& id, bool is_hex_id) const noexcept std::size_t size{}; git_object_t type{}; - 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; + { + 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 (auto obj_type = GitTypeToObjectType(type)) { diff --git a/src/buildtool/file_system/git_cas.hpp b/src/buildtool/file_system/git_cas.hpp index 44b61cb4..f95cb403 100644 --- a/src/buildtool/file_system/git_cas.hpp +++ b/src/buildtool/file_system/git_cas.hpp @@ -18,6 +18,7 @@ #include <filesystem> #include <memory> #include <optional> +#include <shared_mutex> #include <unordered_map> #include <vector> @@ -67,6 +68,11 @@ class GitCAS { // git folder path of repo std::filesystem::path git_path_{}; + // mutex to guard odb while setting up a "fake" repository; it needs to be + // uniquely owned while wrapping the odb, but then git operations are free + // to share it. + mutable std::shared_mutex mutex_{}; + [[nodiscard]] auto OpenODB(std::filesystem::path const& repo_path) noexcept -> bool; diff --git a/src/buildtool/file_system/git_repo.cpp b/src/buildtool/file_system/git_repo.cpp index 31bfed38..2e09f84d 100644 --- a/src/buildtool/file_system/git_repo.cpp +++ b/src/buildtool/file_system/git_repo.cpp @@ -243,6 +243,26 @@ auto const kInMemoryODBParent = CreateInMemoryODBParent(); } // namespace #endif // BOOTSTRAP_BUILD_TOOL +GitRepo::GuardedRepo::GuardedRepo(std::shared_mutex* mutex) noexcept + : mutex_{mutex} {} + +auto GitRepo::GuardedRepo::Ptr() -> git_repository* { + return repo_; +} + +auto GitRepo::GuardedRepo::PtrRef() -> git_repository** { + return &repo_; +} + +GitRepo::GuardedRepo::~GuardedRepo() noexcept { +#ifndef BOOTSTRAP_BUILD_TOOL + if (repo_ != nullptr) { + std::unique_lock lock{*mutex_}; + git_repository_free(repo_); + } +#endif +} + auto GitRepo::Open(GitCASPtr git_cas) noexcept -> std::optional<GitRepo> { #ifdef BOOTSTRAP_BUILD_TOOL return std::nullopt; @@ -270,21 +290,31 @@ auto GitRepo::Open(std::filesystem::path const& repo_path) noexcept GitRepo::GitRepo(GitCASPtr git_cas) noexcept { #ifndef BOOTSTRAP_BUILD_TOOL - if (git_cas != nullptr) { - git_repository* repo_ptr{nullptr}; - if (git_repository_wrap_odb(&repo_ptr, git_cas->odb_.get()) != 0) { + try { + if (git_cas != nullptr) { + auto repo_ptr = std::make_shared<GuardedRepo>(&git_cas->mutex_); + { + // acquire the odb lock exclusively + std::unique_lock lock{git_cas->mutex_}; + if (git_repository_wrap_odb(repo_ptr->PtrRef(), + git_cas->odb_.get()) != 0) { + Logger::Log(LogLevel::Error, + "could not create wrapper for git repository"); + return; + } + } + repo_ = std::move(repo_ptr); + is_repo_fake_ = true; + git_cas_ = std::move(git_cas); + } + else { Logger::Log(LogLevel::Error, - "could not create wrapper for git repository"); - git_repository_free(repo_ptr); - return; + "git repository creation attempted with null odb!"); } - repo_.reset(repo_ptr); // retain repo - is_repo_fake_ = true; - git_cas_ = std::move(git_cas); - } - else { + } catch (std::exception const& ex) { Logger::Log(LogLevel::Error, - "git repository creation attempted with null odb!"); + "opening git object database failed with:\n{}", + ex.what()); } #endif // BOOTSTRAP_BUILD_TOOL } @@ -296,8 +326,8 @@ GitRepo::GitRepo(std::filesystem::path const& repo_path) noexcept { std::unique_lock lock{repo_mutex}; auto cas = std::make_shared<GitCAS>(); // open repo, but retain it - git_repository* repo_ptr{nullptr}; - if (git_repository_open_ext(&repo_ptr, + auto repo_ptr = std::make_shared<GuardedRepo>(&cas->mutex_); + if (git_repository_open_ext(repo_ptr->PtrRef(), repo_path.c_str(), GIT_REPOSITORY_OPEN_NO_SEARCH, nullptr) != 0) { @@ -305,14 +335,12 @@ GitRepo::GitRepo(std::filesystem::path const& repo_path) noexcept { "opening git repository {} failed with:\n{}", repo_path.string(), GitLastError()); - git_repository_free(repo_ptr); - repo_ = nullptr; return; } - repo_.reset(repo_ptr); // retain repo pointer + repo_ = repo_ptr; // retain repo pointer // get odb git_odb* odb_ptr{nullptr}; - git_repository_odb(&odb_ptr, repo_.get()); + git_repository_odb(&odb_ptr, repo_->Ptr()); if (odb_ptr == nullptr) { Logger::Log(LogLevel::Error, "retrieving odb of git repository {} failed with:\n{}", @@ -326,7 +354,7 @@ GitRepo::GitRepo(std::filesystem::path const& repo_path) noexcept { is_repo_fake_ = false; // save root path cas->git_path_ = ToNormalPath(std::filesystem::absolute( - std::filesystem::path(git_repository_path(repo_.get())))); + std::filesystem::path(git_repository_path(repo_->Ptr())))); // retain the pointer git_cas_ = std::static_pointer_cast<GitCAS const>(cas); } catch (std::exception const& ex) { @@ -345,10 +373,13 @@ GitRepo::GitRepo(GitRepo&& other) noexcept } auto GitRepo::operator=(GitRepo&& other) noexcept -> GitRepo& { - git_cas_ = std::move(other.git_cas_); - repo_ = std::move(other.repo_); - is_repo_fake_ = other.is_repo_fake_; - other.git_cas_ = nullptr; + try { + git_cas_ = std::move(other.git_cas_); + repo_ = std::move(other.repo_); + is_repo_fake_ = other.is_repo_fake_; + other.git_cas_ = nullptr; + } catch (...) { + } return *this; } @@ -416,8 +447,7 @@ auto GitRepo::GetGitCAS() const noexcept -> GitCASPtr { return git_cas_; } -auto GitRepo::GetRepoRef() const noexcept - -> std::unique_ptr<git_repository, decltype(&repo_closer)> const& { +auto GitRepo::GetRepoRef() const noexcept -> GuardedRepoPtr { return repo_; } @@ -443,9 +473,12 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message, true /*fatal*/); return std::nullopt; } + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + // add all files to be staged git_index* index_ptr{nullptr}; - git_repository_index(&index_ptr, repo_.get()); + git_repository_index(&index_ptr, repo_->Ptr()); auto index = std::unique_ptr<git_index, decltype(&index_closer)>( index_ptr, index_closer); @@ -496,7 +529,7 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message, // get tree object git_tree* tree_ptr = nullptr; - if (git_tree_lookup(&tree_ptr, repo_.get(), &tree_oid) != 0) { + if (git_tree_lookup(&tree_ptr, repo_->Ptr(), &tree_oid) != 0) { (*logger)( fmt::format("tree lookup in git repository {} failed with:\n{}", GetGitCAS()->git_path_.string(), @@ -516,7 +549,7 @@ auto GitRepo::StageAndCommitAllAnonymous(std::string const& message, git_oid commit_oid; // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg,hicpp-vararg) if (git_commit_create_v(&commit_oid, - repo_.get(), + repo_->Ptr(), "HEAD", signature.get(), signature.get(), @@ -557,9 +590,12 @@ auto GitRepo::KeepTag(std::string const& commit, true /*fatal*/); return false; } + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + // get commit spec git_object* target_ptr{nullptr}; - if (git_revparse_single(&target_ptr, repo_.get(), commit.c_str()) != + if (git_revparse_single(&target_ptr, repo_->Ptr(), commit.c_str()) != 0) { (*logger)(fmt::format("rev-parse commit {} in repository {} failed " "with:\n{}", @@ -597,7 +633,7 @@ auto GitRepo::KeepTag(std::string const& commit, git_strarray tag_names{}; // check if tag hasn't already been added by another process - if (git_tag_list_match(&tag_names, name.c_str(), repo_.get()) == 0 and + if (git_tag_list_match(&tag_names, name.c_str(), repo_->Ptr()) == 0 and tag_names.count > 0) { git_strarray_dispose(&tag_names); return true; // success! @@ -610,7 +646,7 @@ auto GitRepo::KeepTag(std::string const& commit, while (max_attempts > 0) { --max_attempts; err = git_tag_create(&oid, - repo_.get(), + repo_->Ptr(), name.c_str(), target.get(), tagger.get(), @@ -625,7 +661,7 @@ auto GitRepo::KeepTag(std::string const& commit, break; } // check if tag hasn't already been added by another process - if (git_tag_list_match(&tag_names, name.c_str(), repo_.get()) == + if (git_tag_list_match(&tag_names, name.c_str(), repo_->Ptr()) == 0 and tag_names.count > 0) { git_strarray_dispose(&tag_names); @@ -662,9 +698,12 @@ auto GitRepo::GetHeadCommit(anon_logger_ptr const& logger) noexcept true /*fatal*/); return std::nullopt; } + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + // get root commit id git_oid head_oid; - if (git_reference_name_to_id(&head_oid, repo_.get(), "HEAD") != 0) { + if (git_reference_name_to_id(&head_oid, repo_->Ptr(), "HEAD") != 0) { (*logger)(fmt::format("retrieving head commit in git repository {} " "failed with:\n{}", GetGitCAS()->git_path_.string(), @@ -695,6 +734,9 @@ auto GitRepo::GetSubtreeFromCommit(std::string const& commit, LogLevel::Debug, "Subtree id retrieval from commit called on a real repository"); } + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + // get commit object git_oid commit_oid; if (git_oid_fromstr(&commit_oid, commit.c_str()) != 0) { @@ -708,7 +750,7 @@ auto GitRepo::GetSubtreeFromCommit(std::string const& commit, } git_commit* commit_ptr{nullptr}; - if (git_commit_lookup(&commit_ptr, repo_.get(), &commit_oid) != 0) { + if (git_commit_lookup(&commit_ptr, repo_->Ptr(), &commit_oid) != 0) { (*logger)(fmt::format("retrieving commit {} in git repository {} " "failed with:\n{}", commit, @@ -791,6 +833,9 @@ auto GitRepo::GetSubtreeFromTree(std::string const& tree_id, "Subtree id retrieval from tree called on a real " "repository"); } + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + // get tree object from tree id git_oid tree_oid; if (git_oid_fromstr(&tree_oid, tree_id.c_str()) != 0) { @@ -804,7 +849,7 @@ auto GitRepo::GetSubtreeFromTree(std::string const& tree_id, } git_tree* tree_ptr{nullptr}; - if (git_tree_lookup(&tree_ptr, repo_.get(), &tree_oid) != 0) { + if (git_tree_lookup(&tree_ptr, repo_->Ptr(), &tree_oid) != 0) { (*logger)(fmt::format( "retrieving tree {} in git repository {} failed " "with:\n{}", @@ -926,8 +971,13 @@ auto GitRepo::CheckCommitExists(std::string const& commit, } git_commit* commit_obj = nullptr; - auto lookup_res = - git_commit_lookup(&commit_obj, repo_.get(), &commit_oid); + int lookup_res{}; + { + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + lookup_res = + git_commit_lookup(&commit_obj, repo_->Ptr(), &commit_oid); + } if (lookup_res != 0) { if (lookup_res == GIT_ENOTFOUND) { // cleanup resources @@ -1027,7 +1077,12 @@ auto GitRepo::CheckTreeExists(std::string const& tree_id, } // get tree object git_tree* tree_ptr = nullptr; - auto lookup_res = git_tree_lookup(&tree_ptr, repo_.get(), &tree_oid); + int lookup_res{}; + { + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + lookup_res = git_tree_lookup(&tree_ptr, repo_->Ptr(), &tree_oid); + } git_tree_free(tree_ptr); if (lookup_res != 0) { if (lookup_res == GIT_ENOTFOUND) { @@ -1067,11 +1122,15 @@ auto GitRepo::ReadTree(std::string const& id, bool is_hex_id) const noexcept // lookup tree git_tree* tree_ptr{nullptr}; - if (git_tree_lookup(&tree_ptr, repo_.get(), &(*oid)) != 0) { - Logger::Log(LogLevel::Debug, - "failed to lookup Git tree {}", - is_hex_id ? std::string{id} : ToHexString(id)); - return std::nullopt; + { + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; + if (git_tree_lookup(&tree_ptr, repo_->Ptr(), &(*oid)) != 0) { + Logger::Log(LogLevel::Debug, + "failed to lookup Git tree {}", + is_hex_id ? std::string{id} : ToHexString(id)); + return std::nullopt; + } } auto tree = std::unique_ptr<git_tree, decltype(&tree_closer)>{tree_ptr, tree_closer}; @@ -1103,9 +1162,11 @@ auto GitRepo::CreateTree(tree_entries_t const& entries) const noexcept #ifndef NDEBUG gsl_ExpectsAudit(ValidateEntries(entries)); #endif // NDEBUG + // share the odb lock + std::shared_lock lock{GetGitCAS()->mutex_}; git_treebuilder* builder_ptr{nullptr}; - if (git_treebuilder_new(&builder_ptr, repo_.get(), nullptr) != 0) { + if (git_treebuilder_new(&builder_ptr, repo_->Ptr(), nullptr) != 0) { Logger::Log(LogLevel::Debug, "failed to create Git tree builder"); return std::nullopt; } diff --git a/src/buildtool/file_system/git_repo.hpp b/src/buildtool/file_system/git_repo.hpp index eba826d9..c66a54db 100644 --- a/src/buildtool/file_system/git_repo.hpp +++ b/src/buildtool/file_system/git_repo.hpp @@ -19,6 +19,10 @@ #include "src/buildtool/file_system/git_cas.hpp" +extern "C" { +struct git_repository; +} + /// \brief Git repository logic. /// Models both a real repository, owning the underlying ODB /// (non-thread-safe), as well as a "fake" repository, which only wraps an @@ -196,11 +200,35 @@ class GitRepo { -> std::optional<bool>; private: + /// \brief Wrapped git_repository with guarded destructor. + /// Kept privately nested to avoid misuse of its raw pointer members. + class GuardedRepo { + public: + GuardedRepo() noexcept = delete; + explicit GuardedRepo(std::shared_mutex* mutex) noexcept; + ~GuardedRepo() noexcept; + + // prohibit moves and copies + GuardedRepo(GuardedRepo const&) = delete; + GuardedRepo(GuardedRepo&& other) = delete; + auto operator=(GuardedRepo const&) = delete; + auto operator=(GuardedRepo&& other) = delete; + + // get the bare pointer + [[nodiscard]] auto Ptr() -> git_repository*; + [[nodiscard]] auto PtrRef() -> git_repository**; + + private: + std::shared_mutex* mutex_; + git_repository* repo_{nullptr}; + }; + + using GuardedRepoPtr = std::shared_ptr<GuardedRepo>; + // IMPORTANT! The GitCAS object must be defined before the repo object to // keep the GitContext alive until cleanup ends. GitCASPtr git_cas_{nullptr}; - std::unique_ptr<git_repository, decltype(&repo_closer)> repo_{nullptr, - repo_closer}; + GuardedRepoPtr repo_{nullptr}; // default to real repo, as that is non-thread-safe bool is_repo_fake_{false}; @@ -210,8 +238,7 @@ class GitRepo { /// \brief Open real repository at given location. explicit GitRepo(std::filesystem::path const& repo_path) noexcept; - [[nodiscard]] auto GetRepoRef() const noexcept - -> std::unique_ptr<git_repository, decltype(&repo_closer)> const&; + [[nodiscard]] auto GetRepoRef() const noexcept -> GuardedRepoPtr; [[nodiscard]] auto GetGitPath() const noexcept -> std::filesystem::path const&; diff --git a/src/buildtool/file_system/git_utils.cpp b/src/buildtool/file_system/git_utils.cpp index a38fae39..a3b503ab 100644 --- a/src/buildtool/file_system/git_utils.cpp +++ b/src/buildtool/file_system/git_utils.cpp @@ -69,12 +69,6 @@ auto GitObjectID(std::string const& id, bool is_hex_id) noexcept #endif // BOOTSTRAP_BUILD_TOOL } -void repo_closer(gsl::owner<git_repository*> repo) { -#ifndef BOOTSTRAP_BUILD_TOOL - git_repository_free(repo); -#endif -} - void odb_closer(gsl::owner<git_odb*> odb) { #ifndef BOOTSTRAP_BUILD_TOOL git_odb_free(odb); diff --git a/src/buildtool/file_system/git_utils.hpp b/src/buildtool/file_system/git_utils.hpp index 5749c8b5..fc43e542 100644 --- a/src/buildtool/file_system/git_utils.hpp +++ b/src/buildtool/file_system/git_utils.hpp @@ -23,7 +23,6 @@ extern "C" { struct git_oid; struct git_odb; -struct git_repository; struct git_tree; struct git_treebuilder; struct git_index; @@ -48,8 +47,6 @@ constexpr std::size_t kGitLockNumTries{10}; /// \brief Retrieve error message of last libgit2 call. [[nodiscard]] auto GitLastError() noexcept -> std::string; -void repo_closer(gsl::owner<git_repository*> repo); - void odb_closer(gsl::owner<git_odb*> odb); void tree_closer(gsl::owner<git_tree*> tree); diff --git a/src/other_tools/git_operations/git_repo_remote.cpp b/src/other_tools/git_operations/git_repo_remote.cpp index aa277c3e..0c925cc4 100644 --- a/src/other_tools/git_operations/git_repo_remote.cpp +++ b/src/other_tools/git_operations/git_repo_remote.cpp @@ -148,7 +148,7 @@ auto GitRepoRemote::GetCommitFromRemote(std::shared_ptr<git_config> cfg, // create remote git_remote* remote_ptr{nullptr}; if (git_remote_create_anonymous( - &remote_ptr, GetRepoRef().get(), repo_url.c_str()) != 0) { + &remote_ptr, GetRepoRef()->Ptr(), repo_url.c_str()) != 0) { (*logger)( fmt::format("Creating anonymous remote for git repository {} " "failed with:\n{}", @@ -279,7 +279,7 @@ auto GitRepoRemote::FetchFromRemote(std::shared_ptr<git_config> cfg, // create remote from repo git_remote* remote_ptr{nullptr}; if (git_remote_create_anonymous( - &remote_ptr, GetRepoRef().get(), repo_url.c_str()) != 0) { + &remote_ptr, GetRepoRef()->Ptr(), repo_url.c_str()) != 0) { (*logger)(fmt::format("Creating remote {} for git repository {} " "failed with:\n{}", repo_url, @@ -300,7 +300,7 @@ auto GitRepoRemote::FetchFromRemote(std::shared_ptr<git_config> cfg, if (not cfg) { // get config snapshot of current repo git_config* cfg_ptr{nullptr}; - if (git_repository_config_snapshot(&cfg_ptr, GetRepoRef().get()) != + if (git_repository_config_snapshot(&cfg_ptr, GetRepoRef()->Ptr()) != 0) { (*logger)(fmt::format("Retrieving config object in fetch from " "remote failed with:\n{}", @@ -621,7 +621,7 @@ auto GitRepoRemote::FetchViaTmpRepo(std::filesystem::path const& tmp_dir, auto GitRepoRemote::GetConfigSnapshot() const -> std::shared_ptr<git_config> { git_config* cfg_ptr{nullptr}; - if (git_repository_config_snapshot(&cfg_ptr, GetRepoRef().get()) != 0) { + if (git_repository_config_snapshot(&cfg_ptr, GetRepoRef()->Ptr()) != 0) { return nullptr; } return std::shared_ptr<git_config>(cfg_ptr, config_closer); |