workspace: canonicalize workspace and repo paths internally

We depend on comparing the workspace root with the Git repo's path to
know if we're sharing the working copy with it. For that to work
reliably, we need the paths to be canonicalized, so that's what this
patch tries to do.

lib/src/repo.rs

@@ -127,6 +127,7 @@ impl Debug for ReadonlyRepo {
 
 impl ReadonlyRepo {
     pub fn init_local(settings: &UserSettings, repo_path: PathBuf) -> Arc<ReadonlyRepo> {
+        let repo_path = repo_path.canonicalize().unwrap();
         ReadonlyRepo::init_repo_dir(&repo_path);
         let store = Store::init_local(repo_path.join("store"));
         ReadonlyRepo::init(settings, repo_path, store)
@@ -134,6 +135,7 @@ impl ReadonlyRepo {
 
     /// Initializes a repo with a new Git backend in .jj/git/ (bare Git repo)
     pub fn init_internal_git(settings: &UserSettings, repo_path: PathBuf) -> Arc<ReadonlyRepo> {
+        let repo_path = repo_path.canonicalize().unwrap();
         ReadonlyRepo::init_repo_dir(&repo_path);
         let store = Store::init_internal_git(repo_path.join("store"));
         ReadonlyRepo::init(settings, repo_path, store)
@@ -145,6 +147,7 @@ impl ReadonlyRepo {
         repo_path: PathBuf,
         git_repo_path: PathBuf,
     ) -> Arc<ReadonlyRepo> {
+        let repo_path = repo_path.canonicalize().unwrap();
         ReadonlyRepo::init_repo_dir(&repo_path);
         let store = Store::init_external_git(repo_path.join("store"), git_repo_path);
         ReadonlyRepo::init(settings, repo_path, store)

lib/src/testutils.rs

@@ -15,7 +15,7 @@
 use std::fs;
 use std::fs::OpenOptions;
 use std::io::{Read, Write};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use itertools::Itertools;
@@ -53,7 +53,7 @@ pub fn user_settings() -> UserSettings {
 }
 
 pub struct TestRepo {
-    pub temp_dir: TempDir,
+    _temp_dir: TempDir,
     pub repo: Arc<ReadonlyRepo>,
 }
 
@@ -71,11 +71,14 @@ pub fn init_repo(settings: &UserSettings, use_git: bool) -> TestRepo {
         ReadonlyRepo::init_local(settings, repo_dir)
     };
 
-    TestRepo { temp_dir, repo }
+    TestRepo {
+        _temp_dir: temp_dir,
+        repo,
+    }
 }
 
 pub struct TestWorkspace {
-    pub temp_dir: TempDir,
+    temp_dir: TempDir,
     pub workspace: Workspace,
     pub repo: Arc<ReadonlyRepo>,
 }
@@ -101,6 +104,12 @@ pub fn init_workspace(settings: &UserSettings, use_git: bool) -> TestWorkspace {
     }
 }
 
+impl TestWorkspace {
+    pub fn root_dir(&self) -> PathBuf {
+        self.temp_dir.path().join("repo").join("..")
+    }
+}
+
 pub fn read_file(store: &Store, path: &RepoPath, id: &FileId) -> Vec<u8> {
     let mut reader = store.read_file(path, id).unwrap();
     let mut content = vec![];

lib/src/workspace.rs

@@ -87,6 +87,19 @@ fn init_working_copy(
 }
 
 impl Workspace {
+    fn new(
+        workspace_root: PathBuf,
+        working_copy: WorkingCopy,
+        repo_loader: RepoLoader,
+    ) -> Workspace {
+        let workspace_root = workspace_root.canonicalize().unwrap();
+        Workspace {
+            workspace_root,
+            repo_loader,
+            working_copy,
+        }
+    }
+
     pub fn init_local(
         user_settings: &UserSettings,
         workspace_root: PathBuf,
@@ -103,11 +116,7 @@ impl Workspace {
             WorkspaceId::default(),
         );
         let repo_loader = repo.loader();
-        let workspace = Workspace {
-            workspace_root,
-            repo_loader,
-            working_copy,
-        };
+        let workspace = Self::new(workspace_root, working_copy, repo_loader);
         Ok((workspace, repo))
     }
 
@@ -127,11 +136,7 @@ impl Workspace {
             WorkspaceId::default(),
         );
         let repo_loader = repo.loader();
-        let workspace = Workspace {
-            workspace_root,
-            repo_loader,
-            working_copy,
-        };
+        let workspace = Workspace::new(workspace_root, working_copy, repo_loader);
         Ok((workspace, repo))
     }
 
@@ -152,11 +157,7 @@ impl Workspace {
             WorkspaceId::default(),
         );
         let repo_loader = repo.loader();
-        let workspace = Workspace {
-            workspace_root,
-            repo_loader,
-            working_copy,
-        };
+        let workspace = Workspace::new(workspace_root, working_copy, repo_loader);
         Ok((workspace, repo))
     }
 
@@ -177,11 +178,7 @@ impl Workspace {
         let repo_loader = RepoLoader::init(user_settings, repo_dir);
         let (working_copy, repo) =
             init_working_copy(user_settings, repo, &workspace_root, &jj_dir, workspace_id);
-        let workspace = Workspace {
-            workspace_root,
-            repo_loader,
-            working_copy,
-        };
+        let workspace = Workspace::new(workspace_root, working_copy, repo_loader);
         Ok((workspace, repo))
     }
 
@@ -220,11 +217,7 @@ impl Workspace {
             workspace_root.clone(),
             working_copy_state_path,
         );
-        Ok(Self {
-            workspace_root,
-            repo_loader,
-            working_copy,
-        })
+        Ok(Workspace::new(workspace_root, working_copy, repo_loader))
     }
 
     pub fn workspace_root(&self) -> &PathBuf {

lib/tests/test_init.rs

@@ -12,21 +12,29 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::path::{Path, PathBuf};
+
 use jujutsu_lib::op_store::WorkspaceId;
 use jujutsu_lib::settings::UserSettings;
 use jujutsu_lib::testutils;
 use jujutsu_lib::workspace::Workspace;
 use test_case::test_case;
 
+fn canonicalize(input: &Path) -> (PathBuf, PathBuf) {
+    let uncanonical = input.join("..").join(input.file_name().unwrap());
+    let canonical = uncanonical.canonicalize().unwrap();
+    (canonical, uncanonical)
+}
+
 #[test]
 fn test_init_local() {
     let settings = testutils::user_settings();
     let temp_dir = tempfile::tempdir().unwrap();
-    let wc_path = temp_dir.path().to_owned();
-    let (workspace, repo) = Workspace::init_local(&settings, wc_path.clone()).unwrap();
+    let (canonical, uncanonical) = canonicalize(temp_dir.path());
+    let (workspace, repo) = Workspace::init_local(&settings, uncanonical).unwrap();
     assert!(repo.store().git_repo().is_none());
-    assert_eq!(repo.repo_path(), &wc_path.join(".jj").join("repo"));
-    assert_eq!(workspace.workspace_root(), &wc_path);
+    assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
+    assert_eq!(workspace.workspace_root(), &canonical);
 
     // Just test that we can write a commit to the store
     let mut tx = repo.start_transaction("test");
@@ -37,11 +45,11 @@ fn test_init_local() {
 fn test_init_internal_git() {
     let settings = testutils::user_settings();
     let temp_dir = tempfile::tempdir().unwrap();
-    let wc_path = temp_dir.path().to_owned();
-    let (workspace, repo) = Workspace::init_internal_git(&settings, wc_path.clone()).unwrap();
+    let (canonical, uncanonical) = canonicalize(temp_dir.path());
+    let (workspace, repo) = Workspace::init_internal_git(&settings, uncanonical).unwrap();
     assert!(repo.store().git_repo().is_some());
-    assert_eq!(repo.repo_path(), &wc_path.join(".jj").join("repo"));
-    assert_eq!(workspace.workspace_root(), &wc_path);
+    assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
+    assert_eq!(workspace.workspace_root(), &canonical);
 
     // Just test that we ca write a commit to the store
     let mut tx = repo.start_transaction("test");
@@ -52,15 +60,18 @@ fn test_init_internal_git() {
 fn test_init_external_git() {
     let settings = testutils::user_settings();
     let temp_dir = tempfile::tempdir().unwrap();
-    let git_repo_path = temp_dir.path().join("git");
+    let (canonical, uncanonical) = canonicalize(temp_dir.path());
+    let git_repo_path = uncanonical.join("git");
     git2::Repository::init(&git_repo_path).unwrap();
-    let wc_path = temp_dir.path().join("jj");
-    std::fs::create_dir(&wc_path).unwrap();
+    std::fs::create_dir(&uncanonical.join("jj")).unwrap();
     let (workspace, repo) =
-        Workspace::init_external_git(&settings, wc_path.clone(), git_repo_path).unwrap();
+        Workspace::init_external_git(&settings, uncanonical.join("jj"), git_repo_path).unwrap();
     assert!(repo.store().git_repo().is_some());
-    assert_eq!(repo.repo_path(), &wc_path.join(".jj").join("repo"));
-    assert_eq!(workspace.workspace_root(), &wc_path);
+    assert_eq!(
+        repo.repo_path(),
+        &canonical.join("jj").join(".jj").join("repo")
+    );
+    assert_eq!(workspace.workspace_root(), &canonical.join("jj"));
 
     // Just test that we can write a commit to the store
     let mut tx = repo.start_transaction("test");

lib/tests/test_workspace.rs

@@ -54,7 +54,7 @@ fn test_init_additional_workspace(use_git: bool) {
     let workspace = &test_workspace.workspace;
 
     let ws2_id = WorkspaceId::new("ws2".to_string());
-    let ws2_root = test_workspace.temp_dir.path().join("ws2_root");
+    let ws2_root = test_workspace.root_dir().join("ws2_root");
     std::fs::create_dir(&ws2_root).unwrap();
     let (ws2, repo) = Workspace::init_workspace_with_existing_repo(
         &settings,
@@ -76,7 +76,7 @@ fn test_init_additional_workspace(use_git: bool) {
         *ws2.repo_path(),
         workspace.repo_path().canonicalize().unwrap()
     );
-    assert_eq!(*ws2.workspace_root(), ws2_root);
+    assert_eq!(*ws2.workspace_root(), ws2_root.canonicalize().unwrap());
     let same_workspace = Workspace::load(&settings, ws2_root);
     assert!(same_workspace.is_ok());
     let same_workspace = same_workspace.unwrap();