From 25fcc3e403ee11cd80c67ac255ae8d7dba7ad92d Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 30 Oct 2023 18:53:19 +0900
Subject: [PATCH] workspace: consider .git symlink when generating relative
 git_target path

Before, an absolute path would be saved in the git_target file if .git is a
symlink. That's not wrong, but seemed a bit weird. Let's consolidate the
behavior across .git file types.
---
 cli/tests/test_init_command.rs |  4 ++++
 lib/src/git_backend.rs         |  2 +-
 lib/src/workspace.rs           | 22 ++++++++++++----------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/cli/tests/test_init_command.rs b/cli/tests/test_init_command.rs
index 465b9edcd..79b8a9556 100644
--- a/cli/tests/test_init_command.rs
+++ b/cli/tests/test_init_command.rs
@@ -230,6 +230,7 @@ fn test_init_git_colocated_gitlink() {
     Done importing changes from the underlying Git repo.
     Initialized repo in "."
     "###);
+    insta::assert_snapshot!(read_git_target(&workspace_root), @"../../../.git");
 
     // Check that the Git repo's HEAD got checked out
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
@@ -265,6 +266,7 @@ fn test_init_git_colocated_symlink_directory() {
     Done importing changes from the underlying Git repo.
     Initialized repo in "."
     "###);
+    insta::assert_snapshot!(read_git_target(&workspace_root), @"../../../.git");
 
     // Check that the Git repo's HEAD got checked out
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
@@ -303,6 +305,7 @@ fn test_init_git_colocated_symlink_directory_without_bare_config() {
     Done importing changes from the underlying Git repo.
     Initialized repo in "."
     "###);
+    insta::assert_snapshot!(read_git_target(&workspace_root), @"../../../.git");
 
     // Check that the Git repo's HEAD got checked out
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
@@ -343,6 +346,7 @@ fn test_init_git_colocated_symlink_gitlink() {
     Done importing changes from the underlying Git repo.
     Initialized repo in "."
     "###);
+    insta::assert_snapshot!(read_git_target(&workspace_root), @"../../../.git");
 
     // Check that the Git repo's HEAD got checked out
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs
index c7f8dc06d..fa6b732ce 100644
--- a/lib/src/git_backend.rs
+++ b/lib/src/git_backend.rs
@@ -373,7 +373,7 @@ impl GitBackend {
 /// config. This config is usually set, but the "repo" tool will set up such
 /// repositories and symlinks. Opening such repo with fully-canonicalized path
 /// would turn a colocated Git repo into a bare repo.
-fn canonicalize_git_repo_path(path: &Path) -> io::Result<PathBuf> {
+pub fn canonicalize_git_repo_path(path: &Path) -> io::Result<PathBuf> {
     if path.ends_with(".git") {
         let workdir = path.parent().unwrap();
         workdir.canonicalize().map(|dir| dir.join(".git"))
diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs
index e06d72280..3138406b8 100644
--- a/lib/src/workspace.rs
+++ b/lib/src/workspace.rs
@@ -26,7 +26,7 @@ use thiserror::Error;
 use crate::backend::{Backend, BackendInitError, MergedTreeId};
 use crate::commit::Commit;
 use crate::file_util::{self, IoResultExt as _, PathError};
-use crate::git_backend::GitBackend;
+use crate::git_backend::{canonicalize_git_repo_path, GitBackend};
 use crate::local_backend::LocalBackend;
 use crate::local_working_copy::LocalWorkingCopy;
 use crate::op_store::{OperationId, WorkspaceId};
@@ -204,15 +204,17 @@ impl Workspace {
                 // 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 store_relative_git_repo_path = match (
+                    workspace_root.canonicalize(),
+                    canonicalize_git_repo_path(git_repo_path),
+                ) {
+                    (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(settings, store_path, &store_relative_git_repo_path)?;
                 Ok(Box::new(backend))