git: return a new repo instance from the store instead of the store's instance

Returning the store's internal `git2::Repository` instance wrapped in
a `Mutex` makes it easy to run into deadlocks. Let's return a freshly
loaded repo instance instead.

lib/src/git.rs

@@ -30,10 +30,9 @@ pub fn push_commit(
     remote_branch: &str,
 ) -> Result<(), GitPushError> {
     let git_repo = commit.store().git_repo().ok_or(GitPushError::NotAGitRepo)?;
-    let locked_git_repo = git_repo.lock().unwrap();
     // Create a temporary ref to work around https://github.com/libgit2/libgit2/issues/3178
     let temp_ref_name = format!("refs/jj/git-push/{}", commit.id().hex());
-    let mut temp_ref = locked_git_repo
+    let mut temp_ref = git_repo
         .reference(
             &temp_ref_name,
             git2::Oid::from_bytes(&commit.id().0).unwrap(),
@@ -46,13 +45,16 @@ pub fn push_commit(
                 err
             ))
         })?;
-    let mut remote = locked_git_repo.find_remote(remote_name).map_err(|err| {
-        match (err.class(), err.code()) {
-            (git2::ErrorClass::Config, git2::ErrorCode::NotFound) => GitPushError::NoSuchRemote,
-            (git2::ErrorClass::Config, git2::ErrorCode::InvalidSpec) => GitPushError::NoSuchRemote,
-            _ => panic!("unhandled git error: {:?}", err),
-        }
-    })?;
+    let mut remote =
+        git_repo
+            .find_remote(remote_name)
+            .map_err(|err| match (err.class(), err.code()) {
+                (git2::ErrorClass::Config, git2::ErrorCode::NotFound) => GitPushError::NoSuchRemote,
+                (git2::ErrorClass::Config, git2::ErrorCode::InvalidSpec) => {
+                    GitPushError::NoSuchRemote
+                }
+                _ => panic!("unhandled git error: {:?}", err),
+            })?;
     // Need to add "refs/heads/" prefix due to https://github.com/libgit2/libgit2/issues/1125
     let refspec = format!("{}:refs/heads/{}", temp_ref_name, remote_branch);
     remote

lib/src/git_store.rs

@@ -155,8 +155,9 @@ impl Store for GitStore {
         20
     }
 
-    fn git_repo(&self) -> Option<&Mutex<git2::Repository>> {
-        Some(&self.repo)
+    fn git_repo(&self) -> Option<git2::Repository> {
+        let path = self.repo.lock().unwrap().path().to_owned();
+        Some(git2::Repository::open(&path).unwrap())
     }
 
     fn read_file(&self, _path: &FileRepoPath, id: &FileId) -> StoreResult<Box<dyn Read>> {

lib/src/local_store.rs

@@ -28,7 +28,6 @@ use crate::store::{
     ChangeId, Commit, CommitId, Conflict, ConflictId, ConflictPart, FileId, MillisSinceEpoch,
     Signature, Store, StoreError, StoreResult, SymlinkId, Timestamp, Tree, TreeId, TreeValue,
 };
-use std::sync::Mutex;
 
 impl From<std::io::Error> for StoreError {
     fn from(err: std::io::Error) -> Self {
@@ -111,7 +110,7 @@ impl Store for LocalStore {
         64
     }
 
-    fn git_repo(&self) -> Option<&Mutex<git2::Repository>> {
+    fn git_repo(&self) -> Option<git2::Repository> {
         None
     }
 

lib/src/store.rs

@@ -21,7 +21,6 @@ use std::vec::Vec;
 use crate::repo_path::DirRepoPath;
 use crate::repo_path::FileRepoPath;
 use std::borrow::Borrow;
-use std::sync::Mutex;
 use thiserror::Error;
 
 #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)]
@@ -333,7 +332,7 @@ impl Tree {
 pub trait Store: Send + Sync + Debug {
     fn hash_length(&self) -> usize;
 
-    fn git_repo(&self) -> Option<&Mutex<git2::Repository>>;
+    fn git_repo(&self) -> Option<git2::Repository>;
 
     fn read_file(&self, path: &FileRepoPath, id: &FileId) -> StoreResult<Box<dyn Read>>;
 

lib/src/store_wrapper.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::collections::HashMap;
-use std::sync::{Arc, Mutex, RwLock, Weak};
+use std::sync::{Arc, RwLock, Weak};
 
 use crate::commit::Commit;
 use crate::repo_path::{DirRepoPath, FileRepoPath};
@@ -24,7 +24,6 @@ use crate::store::{
 };
 use crate::tree::Tree;
 use crate::tree_builder::TreeBuilder;
-use git2::Repository;
 use std::io::Read;
 
 /// Wraps the low-level store and makes it return more convenient types. Also
@@ -60,7 +59,7 @@ impl StoreWrapper {
         self.store.hash_length()
     }
 
-    pub fn git_repo(&self) -> Option<&Mutex<Repository>> {
+    pub fn git_repo(&self) -> Option<git2::Repository> {
         self.store.git_repo()
     }