move initialization of colocated git repo to lib

cli/src/commands/git.rs

@@ -519,8 +519,7 @@ fn do_git_clone(
     wc_path: &Path,
 ) -> Result<(WorkspaceCommandHelper, GitFetchStats), CommandError> {
     let (workspace, repo) = if colocate {
-        let git_repo = git2::Repository::init(wc_path)?;
-        Workspace::init_external_git(command.settings(), wc_path, git_repo.path())?
+        Workspace::init_colocated_git(command.settings(), wc_path)?
     } else {
         Workspace::init_internal_git(command.settings(), wc_path)?
     };

lib/src/git_backend.rs

@@ -126,27 +126,58 @@ impl GitBackend {
     }
 
     pub fn init_internal(store_path: &Path) -> Result<Self, Box<GitBackendInitError>> {
+        let git_repo_path = Path::new("git");
         let git_repo = gix::ThreadSafeRepository::init(
-            store_path.join("git"),
+            store_path.join(git_repo_path),
             gix::create::Kind::Bare,
             gix::create::Options::default(),
         )
         .map_err(GitBackendInitError::InitRepository)?;
-        let extra_path = store_path.join("extra");
-        fs::create_dir(&extra_path)
-            .context(&extra_path)
-            .map_err(GitBackendInitError::Path)?;
-        let target_path = store_path.join("git_target");
-        fs::write(&target_path, b"git")
-            .context(&target_path)
-            .map_err(GitBackendInitError::Path)?;
-        let extra_metadata_store = TableStore::init(extra_path, HASH_LENGTH);
-        Ok(GitBackend::new(git_repo, extra_metadata_store))
+        Self::init_with_repo(store_path, git_repo_path, git_repo)
     }
 
+    /// Initializes backend by creating a new Git repo at the specified
+    /// workspace path. The workspace directory must exist.
+    pub fn init_colocated(
+        store_path: &Path,
+        workspace_root: &Path,
+    ) -> Result<Self, Box<GitBackendInitError>> {
+        let canonical_workspace_root = {
+            let path = store_path.join(workspace_root);
+            path.canonicalize()
+                .context(&path)
+                .map_err(GitBackendInitError::Path)?
+        };
+        let git_repo = gix::ThreadSafeRepository::init(
+            canonical_workspace_root,
+            gix::create::Kind::WithWorktree,
+            gix::create::Options::default(),
+        )
+        .map_err(GitBackendInitError::InitRepository)?;
+        let git_repo_path = workspace_root.join(".git");
+        Self::init_with_repo(store_path, &git_repo_path, git_repo)
+    }
+
+    /// Initializes backend with an existing Git repo at the specified path.
     pub fn init_external(
         store_path: &Path,
         git_repo_path: &Path,
+    ) -> Result<Self, Box<GitBackendInitError>> {
+        let canonical_git_repo_path = {
+            let path = store_path.join(git_repo_path);
+            path.canonicalize()
+                .context(&path)
+                .map_err(GitBackendInitError::Path)?
+        };
+        let git_repo = gix::ThreadSafeRepository::open(canonical_git_repo_path)
+            .map_err(GitBackendInitError::OpenRepository)?;
+        Self::init_with_repo(store_path, git_repo_path, git_repo)
+    }
+
+    fn init_with_repo(
+        store_path: &Path,
+        git_repo_path: &Path,
+        git_repo: gix::ThreadSafeRepository,
     ) -> Result<Self, Box<GitBackendInitError>> {
         let extra_path = store_path.join("extra");
         fs::create_dir(&extra_path)
@@ -172,16 +203,8 @@ impl GitBackend {
                 .context(&target_path)
                 .map_err(GitBackendInitError::Path)?;
         };
-        let canonical_git_repo_path = {
-            let path = store_path.join(git_repo_path);
-            path.canonicalize()
-                .context(&path)
-                .map_err(GitBackendInitError::Path)?
-        };
-        let repo = gix::ThreadSafeRepository::open(canonical_git_repo_path)
-            .map_err(GitBackendInitError::OpenRepository)?;
         let extra_metadata_store = TableStore::init(extra_path, HASH_LENGTH);
-        Ok(GitBackend::new(repo, extra_metadata_store))
+        Ok(GitBackend::new(git_repo, extra_metadata_store))
     }
 
     pub fn load(store_path: &Path) -> Result<Self, Box<GitBackendLoadError>> {

lib/src/workspace.rs

@@ -149,8 +149,8 @@ impl Workspace {
         Self::init_with_backend(user_settings, workspace_root, backend_initializer)
     }
 
-    /// Initializes a workspace with a new Git backend in .jj/git/ (bare Git
-    /// repo)
+    /// Initializes a workspace with a new Git backend and bare Git repo in
+    /// `.jj/repo/store/git`.
     pub fn init_internal_git(
         user_settings: &UserSettings,
         workspace_root: &Path,
@@ -160,33 +160,61 @@ impl Workspace {
         Self::init_with_backend(user_settings, workspace_root, backend_initializer)
     }
 
-    /// Initializes a workspace with an existing Git backend at the specified
-    /// path
+    /// Initializes a workspace with a new Git backend and Git repo that shares
+    /// the same working copy.
+    pub fn init_colocated_git(
+        user_settings: &UserSettings,
+        workspace_root: &Path,
+    ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
+        let backend_initializer = {
+            let workspace_root = workspace_root.to_owned();
+            move |store_path: &Path| -> Result<Box<dyn Backend>, BackendInitError> {
+                // TODO: Clean up path normalization. store_path is canonicalized by
+                // ReadonlyRepo::init(). workspace_root will be canonicalized by
+                // Workspace::new(), but it's not yet here.
+                let store_relative_workspace_root =
+                    if let Ok(workspace_root) = workspace_root.canonicalize() {
+                        file_util::relative_path(store_path, &workspace_root)
+                    } else {
+                        workspace_root.to_owned()
+                    };
+                let backend =
+                    GitBackend::init_colocated(store_path, &store_relative_workspace_root)?;
+                Ok(Box::new(backend))
+            }
+        };
+        Self::init_with_backend(user_settings, workspace_root, &backend_initializer)
+    }
+
+    /// Initializes a workspace with an existing Git repo at the specified path.
     pub fn init_external_git(
         user_settings: &UserSettings,
         workspace_root: &Path,
         git_repo_path: &Path,
     ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
-        let workspace_root = workspace_root.to_owned();
-        let git_repo_path = git_repo_path.to_owned();
-        Self::init_with_backend(user_settings, &workspace_root.clone(), &move |store_path| {
-            // If the git repo is inside the workspace, use a relative path to it so the
-            // whole workspace can be moved without breaking.
-            // TODO: Clean up path normalization. store_path is canonicalized by
-            // ReadonlyRepo::init(). workspace_root will be canonicalized by
-            // Workspace::new(), but it's not yet here.
-            let store_relative_git_repo_path =
-                match (workspace_root.canonicalize(), git_repo_path.canonicalize()) {
-                    (Ok(workspace_root), Ok(git_repo_path))
-                        if git_repo_path.starts_with(&workspace_root) =>
-                    {
-                        file_util::relative_path(store_path, &git_repo_path)
-                    }
-                    _ => git_repo_path.to_owned(),
-                };
-            let backend = GitBackend::init_external(store_path, &store_relative_git_repo_path)?;
-            Ok(Box::new(backend) as Box<dyn Backend>)
-        })
+        let backend_initializer = {
+            let workspace_root = workspace_root.to_owned();
+            let git_repo_path = git_repo_path.to_owned();
+            move |store_path: &Path| -> Result<Box<dyn Backend>, BackendInitError> {
+                // If the git repo is inside the workspace, use a relative path to it so the
+                // whole workspace can be moved without breaking.
+                // TODO: Clean up path normalization. store_path is canonicalized by
+                // ReadonlyRepo::init(). workspace_root will be canonicalized by
+                // Workspace::new(), but it's not yet here.
+                let store_relative_git_repo_path =
+                    match (workspace_root.canonicalize(), git_repo_path.canonicalize()) {
+                        (Ok(workspace_root), Ok(git_repo_path))
+                            if git_repo_path.starts_with(&workspace_root) =>
+                        {
+                            file_util::relative_path(store_path, &git_repo_path)
+                        }
+                        _ => git_repo_path.to_owned(),
+                    };
+                let backend = GitBackend::init_external(store_path, &store_relative_git_repo_path)?;
+                Ok(Box::new(backend))
+            }
+        };
+        Self::init_with_backend(user_settings, workspace_root, &backend_initializer)
     }
 
     #[allow(clippy::too_many_arguments)]

lib/tests/test_init.rs

@@ -53,15 +53,49 @@ fn test_init_internal_git() {
     let temp_dir = testutils::new_temp_dir();
     let (canonical, uncanonical) = canonicalize(temp_dir.path());
     let (workspace, repo) = Workspace::init_internal_git(&settings, &uncanonical).unwrap();
-    assert!(repo
+    let git_backend = repo
+        .store()
+        .backend_impl()
+        .downcast_ref::<GitBackend>()
+        .unwrap();
+    assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
+    assert_eq!(workspace.workspace_root(), &canonical);
+    assert_eq!(
+        git_backend.git_repo_path(),
+        canonical.join(PathBuf::from_iter([".jj", "repo", "store", "git"])),
+    );
+    assert!(git_backend.git_workdir().is_none());
+    assert_eq!(
+        std::fs::read_to_string(repo.repo_path().join("store").join("git_target")).unwrap(),
+        "git"
+    );
+
+    // Just test that we can write a commit to the store
+    let mut tx = repo.start_transaction(&settings, "test");
+    write_random_commit(tx.mut_repo(), &settings);
+}
+
+#[test]
+fn test_init_colocated_git() {
+    let settings = testutils::user_settings();
+    let temp_dir = testutils::new_temp_dir();
+    let (canonical, uncanonical) = canonicalize(temp_dir.path());
+    let (workspace, repo) = Workspace::init_colocated_git(&settings, &uncanonical).unwrap();
+    let git_backend = repo
         .store()
         .backend_impl()
         .downcast_ref::<GitBackend>()
-        .is_some());
+        .unwrap();
     assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
     assert_eq!(workspace.workspace_root(), &canonical);
+    assert_eq!(git_backend.git_repo_path(), canonical.join(".git"));
+    assert_eq!(git_backend.git_workdir(), Some(canonical.as_ref()));
+    assert_eq!(
+        std::fs::read_to_string(repo.repo_path().join("store").join("git_target")).unwrap(),
+        "../../../.git"
+    );
 
-    // Just test that we ca write a commit to the store
+    // Just test that we can write a commit to the store
     let mut tx = repo.start_transaction(&settings, "test");
     write_random_commit(tx.mut_repo(), &settings);
 }
@@ -76,17 +110,24 @@ fn test_init_external_git() {
     std::fs::create_dir(uncanonical.join("jj")).unwrap();
     let (workspace, repo) =
         Workspace::init_external_git(&settings, &uncanonical.join("jj"), &git_repo_path).unwrap();
-
-    assert!(repo
+    let git_backend = repo
         .store()
         .backend_impl()
         .downcast_ref::<GitBackend>()
-        .is_some());
+        .unwrap();
     assert_eq!(
         repo.repo_path(),
         &canonical.join("jj").join(".jj").join("repo")
     );
     assert_eq!(workspace.workspace_root(), &canonical.join("jj"));
+    assert_eq!(
+        git_backend.git_repo_path(),
+        canonical.join("git").join(".git")
+    );
+    assert_eq!(
+        git_backend.git_workdir(),
+        Some(canonical.join("git").as_ref())
+    );
 
     // Just test that we can write a commit to the store
     let mut tx = repo.start_transaction(&settings, "test");