workspace: convert external git repo path to store-relative by constructor

We could fix do_git_clone() instead, but it seemed a bit weird that the
git_repo_path is relative to the store path which is unknown to callers.

Fixes #2374

cli/src/commands/mod.rs

@@ -22,7 +22,7 @@ mod operation;
 use std::collections::{BTreeMap, HashSet};
 use std::fmt::Debug;
 use std::io::{BufRead, Read, Seek, SeekFrom, Write};
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::sync::Arc;
 use std::{fs, io};
 
@@ -1204,14 +1204,6 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
                 git_store_path.pop();
             }
         }
-        // If the git repo is inside the workspace, use a relative path to it so the
-        // whole workspace can be moved without breaking.
-        if let Ok(relative_path) = git_store_path.strip_prefix(&wc_path) {
-            git_store_path = PathBuf::from("..")
-                .join("..")
-                .join("..")
-                .join(relative_path);
-        }
         let (workspace, repo) =
             Workspace::init_external_git(command.settings(), &wc_path, &git_store_path)?;
         let git_repo = repo

cli/tests/test_git_clone.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::path::{self, PathBuf};
+
 use crate::common::TestEnvironment;
 
 pub mod common;
@@ -149,6 +151,15 @@ fn test_git_clone_colocate() {
     Nothing changed.
     "###);
 
+    // git_target path should be relative to the store
+    let store_path = test_env
+        .env_root()
+        .join(PathBuf::from_iter(["empty", ".jj", "repo", "store"]));
+    let git_target_file_contents = std::fs::read_to_string(store_path.join("git_target")).unwrap();
+    insta::assert_snapshot!(
+        git_target_file_contents.replace(path::MAIN_SEPARATOR, "/"),
+        @"../../../.git");
+
     // Set-up a non-empty repo
     let signature =
         git2::Signature::new("Some One", "some.one@example.com", &git2::Time::new(0, 0)).unwrap();

lib/src/workspace.rs

@@ -23,7 +23,7 @@ use std::sync::Arc;
 use thiserror::Error;
 
 use crate::backend::{Backend, BackendInitError};
-use crate::file_util::{IoResultExt as _, PathError};
+use crate::file_util::{self, IoResultExt as _, PathError};
 use crate::git_backend::GitBackend;
 use crate::index::IndexStore;
 use crate::local_backend::LocalBackend;
@@ -162,9 +162,23 @@ impl Workspace {
         git_repo_path: &Path,
     ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
         Self::init_with_backend(user_settings, workspace_root, |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(),
+                };
             Ok(Box::new(GitBackend::init_external(
                 store_path,
-                git_repo_path,
+                &store_relative_git_repo_path,
             )?))
         })
     }