From 0d63223dad67cbafa37560617c741c68f13d5501 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Tue, 3 Oct 2023 18:05:43 +0900
Subject: [PATCH] git_backend: proxy git2::Repository methods to manage lock
 scope internally

Since git_repo() acquires Mutex, it's super easy to create a deadlock.
---
 cli/src/cli_util.rs    | 14 +++++++-------
 lib/src/git_backend.rs | 17 ++++++++++++++++-
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index 2ea37e953..212a04870 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -945,7 +945,7 @@ impl WorkspaceCommandHelper {
 
     pub fn git_config(&self) -> Result<git2::Config, git2::Error> {
         if let Some(git_backend) = self.git_backend() {
-            git_backend.git_repo().config()
+            git_backend.git_config()
         } else {
             git2::Config::open_default()
         }
@@ -975,10 +975,8 @@ impl WorkspaceCommandHelper {
             git_ignores = git_ignores.chain_with_file("", excludes_file_path);
         }
         if let Some(git_backend) = self.git_backend() {
-            git_ignores = git_ignores.chain_with_file(
-                "",
-                git_backend.git_repo().path().join("info").join("exclude"),
-            );
+            git_ignores = git_ignores
+                .chain_with_file("", git_backend.git_repo_path().join("info").join("exclude"));
         }
         git_ignores
     }
@@ -1676,8 +1674,10 @@ fn is_colocated_git_workspace(workspace: &Workspace, repo: &ReadonlyRepo) -> boo
     let Some(git_backend) = repo.store().backend_impl().downcast_ref::<GitBackend>() else {
         return false;
     };
-    let git_repo = git_backend.git_repo();
-    let Some(git_workdir) = git_repo.workdir().and_then(|path| path.canonicalize().ok()) else {
+    let Some(git_workdir) = git_backend
+        .git_workdir()
+        .and_then(|path| path.canonicalize().ok())
+    else {
         return false; // Bare repository
     };
     // Colocated workspace should have ".git" directory, file, or symlink. Since the
diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs
index e90b98141..8a4a771bd 100644
--- a/lib/src/git_backend.rs
+++ b/lib/src/git_backend.rs
@@ -19,7 +19,7 @@ use std::fmt::{Debug, Error, Formatter};
 use std::fs;
 use std::io::{Cursor, Read};
 use std::ops::Deref;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex, MutexGuard};
 
 use git2::Oid;
@@ -163,6 +163,21 @@ impl GitBackend {
         git2::Repository::open(path).unwrap()
     }
 
+    /// Git configuration for this repository.
+    pub fn git_config(&self) -> Result<git2::Config, git2::Error> {
+        self.git_repo().config()
+    }
+
+    /// Path to the `.git` directory or the repository itself if it's bare.
+    pub fn git_repo_path(&self) -> PathBuf {
+        self.git_repo().path().to_owned()
+    }
+
+    /// Path to the working directory if the repository isn't bare.
+    pub fn git_workdir(&self) -> Option<PathBuf> {
+        self.git_repo().workdir().map(|path| path.to_owned())
+    }
+
     fn cached_extra_metadata_table(&self) -> BackendResult<Arc<ReadonlyTable>> {
         let mut locked_head = self.cached_extra_metadata.lock().unwrap();
         match locked_head.as_ref() {