refactor: git: replace Mutex with RwLock in Repository
This commit is contained in:
parent
52df2114e5
commit
73b416e3a0
5 changed files with 39 additions and 34 deletions
|
@ -1,5 +1,5 @@
|
||||||
//
|
//
|
||||||
use std::sync::{atomic::AtomicBool, Arc, Mutex};
|
use std::sync::{atomic::AtomicBool, Arc, RwLock};
|
||||||
|
|
||||||
use derive_more::Deref as _;
|
use derive_more::Deref as _;
|
||||||
|
|
||||||
|
@ -80,7 +80,7 @@ struct RealRepositoryFactory;
|
||||||
impl RepositoryFactory for RealRepositoryFactory {
|
impl RepositoryFactory for RealRepositoryFactory {
|
||||||
fn open(&self, gitdir: &GitDir) -> Result<Box<dyn OpenRepositoryLike>> {
|
fn open(&self, gitdir: &GitDir) -> Result<Box<dyn OpenRepositoryLike>> {
|
||||||
let gix_repo = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?.to_thread_local();
|
let gix_repo = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?.to_thread_local();
|
||||||
let repo = RealOpenRepository::new(Arc::new(Mutex::new(gix_repo)));
|
let repo = RealOpenRepository::new(Arc::new(RwLock::new(gix_repo.into())));
|
||||||
Ok(Box::new(repo))
|
Ok(Box::new(repo))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -93,7 +93,7 @@ impl RepositoryFactory for RealRepositoryFactory {
|
||||||
)?
|
)?
|
||||||
.fetch_only(gix::progress::Discard, &AtomicBool::new(false))?;
|
.fetch_only(gix::progress::Discard, &AtomicBool::new(false))?;
|
||||||
tracing::info!("created");
|
tracing::info!("created");
|
||||||
let repo = RealOpenRepository::new(Arc::new(Mutex::new(gix_repo)));
|
let repo = RealOpenRepository::new(Arc::new(RwLock::new(gix_repo.into())));
|
||||||
Ok(Box::new(repo))
|
Ok(Box::new(repo))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -10,7 +10,7 @@ pub mod ofake;
|
||||||
|
|
||||||
use std::{
|
use std::{
|
||||||
path::Path,
|
path::Path,
|
||||||
sync::{Arc, Mutex},
|
sync::{Arc, RwLock},
|
||||||
};
|
};
|
||||||
|
|
||||||
use crate as git;
|
use crate as git;
|
||||||
|
@ -39,8 +39,8 @@ pub enum OpenRepository {
|
||||||
|
|
||||||
#[cfg(not(tarpaulin_include))]
|
#[cfg(not(tarpaulin_include))]
|
||||||
pub fn real(gix_repo: gix::Repository) -> OpenRepository {
|
pub fn real(gix_repo: gix::Repository) -> OpenRepository {
|
||||||
OpenRepository::Real(oreal::RealOpenRepository::new(Arc::new(Mutex::new(
|
OpenRepository::Real(oreal::RealOpenRepository::new(Arc::new(RwLock::new(
|
||||||
gix_repo,
|
gix_repo.into(),
|
||||||
))))
|
))))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -4,14 +4,14 @@ use git_next_config as config;
|
||||||
|
|
||||||
use std::{
|
use std::{
|
||||||
path::Path,
|
path::Path,
|
||||||
sync::{Arc, Mutex},
|
sync::{Arc, RwLock},
|
||||||
};
|
};
|
||||||
|
|
||||||
#[derive(Clone, Debug, Default)]
|
#[derive(Clone, Debug, Default)]
|
||||||
pub struct FakeOpenRepository {
|
pub struct FakeOpenRepository {
|
||||||
default_push_remote: Arc<Mutex<Option<git::GitRemote>>>,
|
default_push_remote: Arc<RwLock<Option<git::GitRemote>>>,
|
||||||
default_fetch_remote: Arc<Mutex<Option<git::GitRemote>>>,
|
default_fetch_remote: Arc<RwLock<Option<git::GitRemote>>>,
|
||||||
operations: Arc<Mutex<Vec<String>>>,
|
operations: Arc<RwLock<Vec<String>>>,
|
||||||
}
|
}
|
||||||
impl FakeOpenRepository {
|
impl FakeOpenRepository {
|
||||||
pub fn new() -> Self {
|
pub fn new() -> Self {
|
||||||
|
@ -26,7 +26,7 @@ impl FakeOpenRepository {
|
||||||
match direction {
|
match direction {
|
||||||
git::repository::Direction::Push => self
|
git::repository::Direction::Push => self
|
||||||
.default_push_remote
|
.default_push_remote
|
||||||
.lock()
|
.write()
|
||||||
.map(|mut o| match remote {
|
.map(|mut o| match remote {
|
||||||
Some(gr) => o.replace(gr),
|
Some(gr) => o.replace(gr),
|
||||||
None => o.take(),
|
None => o.take(),
|
||||||
|
@ -34,7 +34,7 @@ impl FakeOpenRepository {
|
||||||
.unwrap(),
|
.unwrap(),
|
||||||
git::repository::Direction::Fetch => self
|
git::repository::Direction::Fetch => self
|
||||||
.default_fetch_remote
|
.default_fetch_remote
|
||||||
.lock()
|
.write()
|
||||||
.map(|mut o| match remote {
|
.map(|mut o| match remote {
|
||||||
Some(gr) => o.replace(gr),
|
Some(gr) => o.replace(gr),
|
||||||
None => o.take(),
|
None => o.take(),
|
||||||
|
@ -45,7 +45,7 @@ impl FakeOpenRepository {
|
||||||
|
|
||||||
pub fn operations(&self) -> Vec<String> {
|
pub fn operations(&self) -> Vec<String> {
|
||||||
self.operations
|
self.operations
|
||||||
.lock()
|
.read()
|
||||||
.map(|operations| operations.clone())
|
.map(|operations| operations.clone())
|
||||||
.unwrap_or_default()
|
.unwrap_or_default()
|
||||||
}
|
}
|
||||||
|
@ -60,12 +60,12 @@ impl git::repository::OpenRepositoryLike for FakeOpenRepository {
|
||||||
match direction {
|
match direction {
|
||||||
git::repository::Direction::Push => self
|
git::repository::Direction::Push => self
|
||||||
.default_push_remote
|
.default_push_remote
|
||||||
.lock()
|
.read()
|
||||||
.map(|r| r.clone())
|
.map(|r| r.clone())
|
||||||
.unwrap_or(None),
|
.unwrap_or(None),
|
||||||
git::repository::Direction::Fetch => self
|
git::repository::Direction::Fetch => self
|
||||||
.default_fetch_remote
|
.default_fetch_remote
|
||||||
.lock()
|
.read()
|
||||||
.map(|r| r.clone())
|
.map(|r| r.clone())
|
||||||
.unwrap_or(None),
|
.unwrap_or(None),
|
||||||
}
|
}
|
||||||
|
@ -73,7 +73,7 @@ impl git::repository::OpenRepositoryLike for FakeOpenRepository {
|
||||||
|
|
||||||
fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> {
|
fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> {
|
||||||
self.operations
|
self.operations
|
||||||
.lock()
|
.write()
|
||||||
.map_err(|_| crate::fetch::Error::Lock)
|
.map_err(|_| crate::fetch::Error::Lock)
|
||||||
.map(|mut operations| operations.push("fetch".to_string()))?;
|
.map(|mut operations| operations.push("fetch".to_string()))?;
|
||||||
Ok(())
|
Ok(())
|
||||||
|
@ -89,7 +89,7 @@ impl git::repository::OpenRepositoryLike for FakeOpenRepository {
|
||||||
let forge_alias = repo_details.forge.forge_alias();
|
let forge_alias = repo_details.forge.forge_alias();
|
||||||
let repo_alias = &repo_details.repo_alias;
|
let repo_alias = &repo_details.repo_alias;
|
||||||
self.operations
|
self.operations
|
||||||
.lock()
|
.write()
|
||||||
.map_err(|_| crate::fetch::Error::Lock)
|
.map_err(|_| crate::fetch::Error::Lock)
|
||||||
.map(|mut operations| {
|
.map(|mut operations| {
|
||||||
operations.push(format!(
|
operations.push(format!(
|
||||||
|
|
|
@ -7,20 +7,20 @@ use git_next_config as config;
|
||||||
use gix::bstr::BStr;
|
use gix::bstr::BStr;
|
||||||
use std::{
|
use std::{
|
||||||
path::Path,
|
path::Path,
|
||||||
sync::{Arc, Mutex},
|
sync::{Arc, RwLock},
|
||||||
};
|
};
|
||||||
use tracing::{info, warn};
|
use tracing::{info, warn};
|
||||||
|
|
||||||
#[derive(Clone, Debug, Constructor)]
|
#[derive(Clone, Debug, Constructor)]
|
||||||
pub struct RealOpenRepository(Arc<Mutex<gix::Repository>>);
|
pub struct RealOpenRepository(Arc<RwLock<gix::ThreadSafeRepository>>);
|
||||||
impl super::OpenRepositoryLike for RealOpenRepository {
|
impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
fn remote_branches(&self) -> git::push::Result<Vec<config::BranchName>> {
|
fn remote_branches(&self) -> git::push::Result<Vec<config::BranchName>> {
|
||||||
let refs = self
|
let refs = self
|
||||||
.0
|
.0
|
||||||
.lock()
|
.read()
|
||||||
.map_err(|_| git::push::Error::Lock)
|
.map_err(|_| git::push::Error::Lock)
|
||||||
.and_then(|repo| {
|
.and_then(|repo| {
|
||||||
Ok(repo.references()?).and_then(|refs| {
|
Ok(repo.to_thread_local().references()?).and_then(|refs| {
|
||||||
Ok(refs.remote_branches().map(|rb| {
|
Ok(refs.remote_branches().map(|rb| {
|
||||||
rb.filter_map(|rbi| rbi.ok())
|
rb.filter_map(|rbi| rbi.ok())
|
||||||
.map(|r| r.name().to_owned())
|
.map(|r| r.name().to_owned())
|
||||||
|
@ -37,11 +37,12 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
Ok(refs)
|
Ok(refs)
|
||||||
}
|
}
|
||||||
fn find_default_remote(&self, direction: git::repository::Direction) -> Option<git::GitRemote> {
|
fn find_default_remote(&self, direction: git::repository::Direction) -> Option<git::GitRemote> {
|
||||||
let Ok(repository) = self.0.lock() else {
|
let Ok(repository) = self.0.read() else {
|
||||||
#[cfg(not(tarpaulin_include))] // don't test mutex lock failure
|
#[cfg(not(tarpaulin_include))] // don't test mutex lock failure
|
||||||
return None;
|
return None;
|
||||||
};
|
};
|
||||||
let Some(Ok(remote)) = repository.find_default_remote(direction.into()) else {
|
let thread_local = repository.to_thread_local();
|
||||||
|
let Some(Ok(remote)) = thread_local.find_default_remote(direction.into()) else {
|
||||||
#[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes
|
#[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes
|
||||||
return None;
|
return None;
|
||||||
};
|
};
|
||||||
|
@ -51,12 +52,13 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
#[tracing::instrument(skip_all)]
|
#[tracing::instrument(skip_all)]
|
||||||
#[cfg(not(tarpaulin_include))] // would require writing to external service
|
#[cfg(not(tarpaulin_include))] // would require writing to external service
|
||||||
fn fetch(&self) -> Result<(), git::fetch::Error> {
|
fn fetch(&self) -> Result<(), git::fetch::Error> {
|
||||||
let Ok(repository) = self.0.lock() else {
|
let Ok(repository) = self.0.read() else {
|
||||||
#[cfg(not(tarpaulin_include))] // don't test mutex lock failure
|
#[cfg(not(tarpaulin_include))] // don't test mutex lock failure
|
||||||
return Err(git::fetch::Error::Lock);
|
return Err(git::fetch::Error::Lock);
|
||||||
};
|
};
|
||||||
|
let thread_local = repository.to_thread_local();
|
||||||
let Some(Ok(remote)) =
|
let Some(Ok(remote)) =
|
||||||
repository.find_default_remote(git::repository::Direction::Fetch.into())
|
thread_local.find_default_remote(git::repository::Direction::Fetch.into())
|
||||||
else {
|
else {
|
||||||
#[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes
|
#[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes
|
||||||
return Err(git::fetch::Error::NoFetchRemoteFound);
|
return Err(git::fetch::Error::NoFetchRemoteFound);
|
||||||
|
@ -98,7 +100,7 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
.into();
|
.into();
|
||||||
let git_dir = self
|
let git_dir = self
|
||||||
.0
|
.0
|
||||||
.lock()
|
.read()
|
||||||
.map_err(|_| git::push::Error::Lock)
|
.map_err(|_| git::push::Error::Lock)
|
||||||
.map(|r| r.git_dir().to_path_buf())?;
|
.map(|r| r.git_dir().to_path_buf())?;
|
||||||
let ctx = gix::diff::command::Context {
|
let ctx = gix::diff::command::Context {
|
||||||
|
@ -125,12 +127,13 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
false => 50,
|
false => 50,
|
||||||
};
|
};
|
||||||
self.0
|
self.0
|
||||||
.lock()
|
.read()
|
||||||
.map_err(|_| git::commit::log::Error::Lock)
|
.map_err(|_| git::commit::log::Error::Lock)
|
||||||
.map(|repo| {
|
.map(|repo| {
|
||||||
let branch = format!("remotes/origin/{branch_name}");
|
let branch = format!("remotes/origin/{branch_name}");
|
||||||
let branch = BStr::new(&branch);
|
let branch = BStr::new(&branch);
|
||||||
let branch_head = repo
|
let thread_local = repo.to_thread_local();
|
||||||
|
let branch_head = thread_local
|
||||||
.rev_parse_single(branch)
|
.rev_parse_single(branch)
|
||||||
.map_err(|e| e.to_string())
|
.map_err(|e| e.to_string())
|
||||||
.map_err(as_gix_error(branch_name.clone()))?;
|
.map_err(as_gix_error(branch_name.clone()))?;
|
||||||
|
@ -142,7 +145,7 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
.try_into_commit()
|
.try_into_commit()
|
||||||
.map_err(|e| e.to_string())
|
.map_err(|e| e.to_string())
|
||||||
.map_err(as_gix_error(branch_name.clone()))?;
|
.map_err(as_gix_error(branch_name.clone()))?;
|
||||||
let walk = repo
|
let walk = thread_local
|
||||||
.rev_walk([commit.id])
|
.rev_walk([commit.id])
|
||||||
.all()
|
.all()
|
||||||
.map_err(|e| e.to_string())
|
.map_err(|e| e.to_string())
|
||||||
|
@ -183,13 +186,15 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
file_name: &Path,
|
file_name: &Path,
|
||||||
) -> git::file::Result<String> {
|
) -> git::file::Result<String> {
|
||||||
self.0
|
self.0
|
||||||
.lock()
|
.read()
|
||||||
.map_err(|_| git::file::Error::Lock)
|
.map_err(|_| git::file::Error::Lock)
|
||||||
.and_then(|repo| {
|
.and_then(|repo| {
|
||||||
let fref = repo.find_reference(format!("origin/{}", branch_name).as_str())?;
|
let thread_local = repo.to_thread_local();
|
||||||
|
let fref =
|
||||||
|
thread_local.find_reference(format!("origin/{}", branch_name).as_str())?;
|
||||||
let id = fref.try_id().ok_or(git::file::Error::TryId)?;
|
let id = fref.try_id().ok_or(git::file::Error::TryId)?;
|
||||||
let oid = id.detach();
|
let oid = id.detach();
|
||||||
let obj = repo.find_object(oid)?;
|
let obj = thread_local.find_object(oid)?;
|
||||||
let commit = obj.into_commit();
|
let commit = obj.into_commit();
|
||||||
let tree = commit.tree()?;
|
let tree = commit.tree()?;
|
||||||
let ent = tree
|
let ent = tree
|
||||||
|
|
|
@ -5,7 +5,7 @@ use git_next_config as config;
|
||||||
|
|
||||||
use std::{
|
use std::{
|
||||||
path::Path,
|
path::Path,
|
||||||
sync::{Arc, Mutex, RwLock},
|
sync::{Arc, RwLock},
|
||||||
};
|
};
|
||||||
|
|
||||||
pub type OnFetchFn =
|
pub type OnFetchFn =
|
||||||
|
@ -154,7 +154,7 @@ impl TestOpenRepository {
|
||||||
fetch_counter: Arc::new(RwLock::new(0)),
|
fetch_counter: Arc::new(RwLock::new(0)),
|
||||||
on_push,
|
on_push,
|
||||||
push_counter: Arc::new(RwLock::new(0)),
|
push_counter: Arc::new(RwLock::new(0)),
|
||||||
real: git::repository::RealOpenRepository::new(Arc::new(Mutex::new(gix))),
|
real: git::repository::RealOpenRepository::new(Arc::new(RwLock::new(gix.into()))),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue