Remove the Mutex wrapper on the Repository

Author: MitMaro

src/application.rs

@@ -23,7 +23,6 @@ pub(crate) struct Application<ModuleProvider>
 where ModuleProvider: module::ModuleProvider + Send + 'static
 {
 	_config: Config,
-	_repository: Repository,
 	process: Process<ModuleProvider>,
 	threads: Option<Vec<Box<dyn Threadable>>>,
 	thread_statuses: ThreadStatuses,
@@ -44,7 +43,7 @@ where ModuleProvider: module::ModuleProvider + Send + 'static
 
 		let module_handler = ModuleHandler::new(
 			EventHandler::new(KeyBindings::new(&config.key_bindings)),
-			ModuleProvider::new(&config, repository.clone(), &todo_file),
+			ModuleProvider::new(&config, repository, &todo_file),
 		);
 
 		let display = Display::new(tui, &config.theme);
@@ -90,7 +89,6 @@ where ModuleProvider: module::ModuleProvider + Send + 'static
 
 		Ok(Self {
 			_config: config,
-			_repository: repository,
 			process,
 			threads: Some(threads),
 			thread_statuses,

src/git/commit.rs

@@ -228,9 +228,7 @@ mod tests {
 	#[test]
 	fn try_from_success() {
 		with_temp_repository(|repository| {
-			let repo = repository.repository();
-			let repo_lock = repo.lock();
-			let reference = repo_lock.find_reference("refs/heads/main").unwrap();
+			let reference = repository.repository().find_reference("refs/heads/main").unwrap();
 			let commit = Commit::try_from(&reference).unwrap();
 
 			assert_eq!(commit.reference.unwrap().shortname(), "main");
@@ -241,11 +239,10 @@ mod tests {
 	fn try_from_error() {
 		with_temp_repository(|repository| {
 			let repo = repository.repository();
-			let repo_lock = repo.lock();
-			let blob = repo_lock.blob(b"foo").unwrap();
-			_ = repo_lock.reference("refs/blob", blob, false, "blob").unwrap();
+			let blob = repo.blob(b"foo").unwrap();
+			_ = repo.reference("refs/blob", blob, false, "blob").unwrap();
 
-			let reference = repo_lock.find_reference("refs/blob").unwrap();
+			let reference = repo.find_reference("refs/blob").unwrap();
 			assert_err_eq!(Commit::try_from(&reference), GitError::CommitLoad {
 				cause: git2::Error::new(
 					git2::ErrorCode::InvalidSpec,

src/git/commit_diff_loader.rs

@@ -1,10 +1,7 @@
-use std::{
-	path::PathBuf,
-	sync::{Arc, LazyLock},
-};
+use std::{path::PathBuf, sync::LazyLock};
 
 use git2::{DiffFindOptions, DiffOptions, Oid, Repository};
-use parking_lot::{Mutex, MutexGuard};
+use parking_lot::Mutex;
 
 use crate::git::{
 	Commit,
@@ -20,31 +17,29 @@ use crate::git::{
 
 static UNKNOWN_PATH: LazyLock<PathBuf> = LazyLock::new(|| PathBuf::from("unknown"));
 
-pub(crate) struct CommitDiffLoader<'options> {
+pub(crate) struct CommitDiffLoader<'options, 'repo> {
 	config: &'options CommitDiffLoaderOptions,
-	repo: Arc<Mutex<Repository>>,
+	repository: &'repo Repository,
 }
 
-impl<'options> CommitDiffLoader<'options> {
-	pub(crate) const fn new(repo: Arc<Mutex<Repository>>, config: &'options CommitDiffLoaderOptions) -> Self {
-		Self { config, repo }
+impl<'options, 'repo> CommitDiffLoader<'options, 'repo> {
+	pub(crate) const fn new(repository: &'repo Repository, config: &'options CommitDiffLoaderOptions) -> Self {
+		Self { config, repository }
 	}
 
 	pub(crate) fn load_from_hash(&self, oid: Oid) -> Result<CommitDiff, git2::Error> {
-		let repo = self.repo.lock();
-		let commit = repo.find_commit(oid)?;
+		let commit = self.repository.find_commit(oid)?;
 		// only the first parent matter for the diff, the second parent, if it exists, was only used
 		// for conflict resolution
 		let parent = commit.parents().next();
 
-		self.load_diff(&repo, parent, &commit)
+		self.load_diff(parent, &commit)
 	}
 
 	#[expect(clippy::as_conversions, reason = "Mostly safe difference between APIs.")]
 	#[expect(clippy::unwrap_in_result, reason = "Unwrap usage failure considered a bug.")]
 	fn load_diff(
 		&self,
-		repo: &MutexGuard<'_, Repository>,
 		parent: Option<git2::Commit<'_>>,
 		commit: &git2::Commit<'_>,
 	) -> Result<CommitDiff, git2::Error> {
@@ -75,10 +70,12 @@ impl<'options> CommitDiffLoader<'options> {
 			.copies_from_unmodified(self.config.copies);
 
 		let mut diff = if let Some(p) = parent {
-			repo.diff_tree_to_tree(Some(&p.tree()?), Some(&commit.tree()?), Some(&mut diff_options))?
+			self.repository
+				.diff_tree_to_tree(Some(&p.tree()?), Some(&commit.tree()?), Some(&mut diff_options))?
 		}
 		else {
-			repo.diff_tree_to_tree(None, Some(&commit.tree()?), Some(&mut diff_options))?
+			self.repository
+				.diff_tree_to_tree(None, Some(&commit.tree()?), Some(&mut diff_options))?
 		};
 
 		diff.find_similar(Some(&mut diff_find_options))?;

src/git/reference_kind.rs

@@ -42,13 +42,7 @@ mod tests {
 	fn from_git2_reference_branch() {
 		with_temp_repository(|repository| {
 			assert_eq!(
-				ReferenceKind::from(
-					&repository
-						.repository()
-						.lock()
-						.find_reference("refs/heads/main")
-						.unwrap()
-				),
+				ReferenceKind::from(&repository.repository().find_reference("refs/heads/main").unwrap()),
 				ReferenceKind::Branch
 			);
 		});
@@ -57,13 +51,12 @@ mod tests {
 	#[test]
 	fn from_git2_reference_note() {
 		with_temp_repository(|repository| {
-			let git2_repository = repository.repository();
-			let git2_lock = git2_repository.lock();
+			let repo = repository.repository();
 			let sig = git2::Signature::new("name", "[email protected]", &git2::Time::new(JAN_2021_EPOCH, 0)).unwrap();
-			let head_id = git2_lock.refname_to_id("HEAD").unwrap();
-			_ = git2_lock.note(&sig, &sig, None, head_id, "note", false).unwrap();
+			let head_id = repo.refname_to_id("HEAD").unwrap();
+			_ = repo.note(&sig, &sig, None, head_id, "note", false).unwrap();
 			assert_eq!(
-				ReferenceKind::from(&git2_lock.find_reference("refs/notes/commits").unwrap()),
+				ReferenceKind::from(&repo.find_reference("refs/notes/commits").unwrap()),
 				ReferenceKind::Note
 			);
 		});
@@ -72,12 +65,11 @@ mod tests {
 	#[test]
 	fn from_git2_reference_remote() {
 		with_temp_repository(|repository| {
-			let git2_repository = repository.repository();
-			let git2_lock = git2_repository.lock();
-			let mut remote = git2_lock.remote("origin", git2_lock.path().to_str().unwrap()).unwrap();
+			let repo = repository.repository();
+			let mut remote = repo.remote("origin", repo.path().to_str().unwrap()).unwrap();
 			remote.fetch(&["main"], None, None).unwrap();
 			assert_eq!(
-				ReferenceKind::from(&git2_lock.find_reference("refs/remotes/origin/main").unwrap()),
+				ReferenceKind::from(&repo.find_reference("refs/remotes/origin/main").unwrap()),
 				ReferenceKind::Remote
 			);
 		});
@@ -86,13 +78,12 @@ mod tests {
 	#[test]
 	fn from_git2_reference_tag() {
 		with_temp_repository(|repository| {
-			let git2_repository = repository.repository();
-			let git2_lock = git2_repository.lock();
+			let repo = repository.repository();
 			let sig = git2::Signature::new("name", "[email protected]", &git2::Time::new(JAN_2021_EPOCH, 0)).unwrap();
-			let head_id = git2_lock.revparse_single("HEAD").unwrap();
-			_ = git2_lock.tag("tag", &head_id, &sig, "note", false).unwrap();
+			let head_id = repo.revparse_single("HEAD").unwrap();
+			_ = repo.tag("tag", &head_id, &sig, "note", false).unwrap();
 			assert_eq!(
-				ReferenceKind::from(&git2_lock.find_reference("refs/tags/tag").unwrap()),
+				ReferenceKind::from(&repo.find_reference("refs/tags/tag").unwrap()),
 				ReferenceKind::Tag
 			);
 		});
@@ -101,12 +92,11 @@ mod tests {
 	#[test]
 	fn from_git2_reference_other() {
 		with_temp_repository(|repository| {
-			let git2_repository = repository.repository();
-			let git2_lock = git2_repository.lock();
-			let blob = git2_lock.blob(b"foo").unwrap();
-			_ = git2_lock.reference("refs/blob", blob, false, "blob").unwrap();
+			let repo = repository.repository();
+			let blob = repo.blob(b"foo").unwrap();
+			_ = repo.reference("refs/blob", blob, false, "blob").unwrap();
 			assert_eq!(
-				ReferenceKind::from(&git2_lock.find_reference("refs/blob").unwrap()),
+				ReferenceKind::from(&repo.find_reference("refs/blob").unwrap()),
 				ReferenceKind::Other
 			);
 		});

src/git/repository.rs

@@ -1,16 +1,10 @@
-use std::{
-	fmt::{Debug, Formatter},
-	sync::Arc,
-};
-
-use parking_lot::Mutex;
+use std::fmt::{Debug, Formatter};
 
 use crate::git::{CommitDiff, CommitDiffLoader, CommitDiffLoaderOptions, Config, GitError, RepositoryLoadKind};
 
 /// A light cloneable, simple wrapper around the `git2::Repository` struct
-#[derive(Clone)]
 pub(crate) struct Repository {
-	repository: Arc<Mutex<git2::Repository>>,
+	repository: git2::Repository,
 }
 
 impl Repository {
@@ -27,20 +21,15 @@ impl Repository {
 				cause: e,
 			}
 		})?;
-		Ok(Self {
-			repository: Arc::new(Mutex::new(repository)),
-		})
+		Ok(Self { repository })
 	}
 
 	/// Load the git configuration for the repository.
 	///
 	/// # Errors
 	/// Will result in an error if the configuration is invalid.
 	pub(crate) fn load_config(&self) -> Result<Config, GitError> {
-		self.repository
-			.lock()
-			.config()
-			.map_err(|e| GitError::ConfigLoad { cause: e })
+		self.repository.config().map_err(|e| GitError::ConfigLoad { cause: e })
 	}
 
 	/// Load a diff for a commit hash
@@ -54,12 +43,10 @@ impl Repository {
 	) -> Result<CommitDiff, GitError> {
 		let oid = self
 			.repository
-			.lock()
 			.revparse_single(hash)
 			.map_err(|e| GitError::CommitLoad { cause: e })?
 			.id();
-		let diff_loader_repository = Arc::clone(&self.repository);
-		let loader = CommitDiffLoader::new(diff_loader_repository, config);
+		let loader = CommitDiffLoader::new(&self.repository, config);
 
 		loader
 			.load_from_hash(oid)
@@ -69,29 +56,23 @@ impl Repository {
 
 impl From<git2::Repository> for Repository {
 	fn from(repository: git2::Repository) -> Self {
-		Self {
-			repository: Arc::new(Mutex::new(repository)),
-		}
+		Self { repository }
 	}
 }
 
 impl Debug for Repository {
 	fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
 		f.debug_struct("Repository")
-			.field("[path]", &self.repository.lock().path())
+			.field("[path]", &self.repository.path())
 			.finish()
 	}
 }
 
 #[cfg(test)]
 mod tests {
-	use std::{
-		path::{Path, PathBuf},
-		sync::Arc,
-	};
+	use std::path::{Path, PathBuf};
 
 	use git2::{Oid, Signature};
-	use parking_lot::Mutex;
 
 	use crate::git::{Commit, GitError, Reference, Repository, RepositoryLoadKind};
 
@@ -107,18 +88,16 @@ mod tests {
 					cause: e,
 				}
 			})?;
-			Ok(Self {
-				repository: Arc::new(Mutex::new(repository)),
-			})
+			Ok(Self { repository })
 		}
 
 		/// Find a reference by the reference name.
 		///
 		/// # Errors
 		/// Will result in an error if the reference cannot be found.
 		pub(crate) fn find_reference(&self, reference: &str) -> Result<Reference, GitError> {
-			let repo = self.repository.lock();
-			let git2_reference = repo
+			let git2_reference = self
+				.repository
 				.find_reference(reference)
 				.map_err(|e| GitError::ReferenceNotFound { cause: e })?;
 			Ok(Reference::from(&git2_reference))
@@ -129,39 +108,35 @@ mod tests {
 		/// # Errors
 		/// Will result in an error if the reference cannot be found or is not a commit.
 		pub(crate) fn find_commit(&self, reference: &str) -> Result<Commit, GitError> {
-			let repo = self.repository.lock();
-			let git2_reference = repo
+			let reference = self
+				.repository
 				.find_reference(reference)
 				.map_err(|e| GitError::ReferenceNotFound { cause: e })?;
-			Commit::try_from(&git2_reference)
+			Commit::try_from(&reference)
 		}
 
 		pub(crate) fn repo_path(&self) -> PathBuf {
-			self.repository.lock().path().to_path_buf()
+			self.repository.path().to_path_buf()
 		}
 
 		pub(crate) fn head_id(&self, head_name: &str) -> Result<Oid, git2::Error> {
-			let repo = self.repository.lock();
 			let ref_name = format!("refs/heads/{head_name}");
-			let revision = repo.revparse_single(ref_name.as_str())?;
+			let revision = self.repository.revparse_single(ref_name.as_str())?;
 			Ok(revision.id())
 		}
 
 		pub(crate) fn commit_id_from_ref(&self, reference: &str) -> Result<Oid, git2::Error> {
-			let repo = self.repository.lock();
-			let commit = repo.find_reference(reference)?.peel_to_commit()?;
+			let commit = self.repository.find_reference(reference)?.peel_to_commit()?;
 			Ok(commit.id())
 		}
 
 		pub(crate) fn add_path_to_index(&self, path: &Path) -> Result<(), git2::Error> {
-			let repo = self.repository.lock();
-			let mut index = repo.index()?;
+			let mut index = self.repository.index()?;
 			index.add_path(path)
 		}
 
 		pub(crate) fn remove_path_from_index(&self, path: &Path) -> Result<(), git2::Error> {
-			let repo = self.repository.lock();
-			let mut index = repo.index()?;
+			let mut index = self.repository.index()?;
 			index.remove_path(path)
 		}
 
@@ -172,15 +147,16 @@ mod tests {
 			committer: &Signature<'_>,
 			message: &str,
 		) -> Result<(), git2::Error> {
-			let repo = self.repository.lock();
-			let tree = repo.find_tree(repo.index()?.write_tree()?)?;
-			let head = repo.find_reference(reference)?.peel_to_commit()?;
-			_ = repo.commit(Some("HEAD"), author, committer, message, &tree, &[&head])?;
+			let tree = self.repository.find_tree(self.repository.index()?.write_tree()?)?;
+			let head = self.repository.find_reference(reference)?.peel_to_commit()?;
+			_ = self
+				.repository
+				.commit(Some("HEAD"), author, committer, message, &tree, &[&head])?;
 			Ok(())
 		}
 
-		pub(crate) fn repository(&self) -> Arc<Mutex<git2::Repository>> {
-			Arc::clone(&self.repository)
+		pub(crate) fn repository(&self) -> &git2::Repository {
+			&self.repository
 		}
 	}
 }
@@ -267,10 +243,11 @@ mod unix_tests {
 	fn load_commit_diff_with_non_commit() {
 		with_temp_repository(|repository| {
 			let blob_ref = {
-				let git2_repository = repository.repository();
-				let git2_lock = git2_repository.lock();
-				let blob = git2_lock.blob(b"foo").unwrap();
-				_ = git2_lock.reference("refs/blob", blob, false, "blob").unwrap();
+				let blob = repository.repository().blob(b"foo").unwrap();
+				_ = repository
+					.repository()
+					.reference("refs/blob", blob, false, "blob")
+					.unwrap();
 				blob.to_string()
 			};