git_backend: don't fully canonicalize .git symlink

Apparently, libgit2 doesn't deduce "core.bare" config from the directory name,
but gitoxide implements it correctly. So we shouldn't blindly canonicalize
the Git repository path. Fortunately, the saved git_target path isn't a fully-
canonicalized form (unless user explicitly sepcified "--git-repo ./.git"), so
we don't need a hack to remap git_target back to the symlink path.

is_colocated_git_workspace() is adjusted since the git_workdir is no longer
resolved from the fully-canonicalized repo path, at least in our code. Still we
have the ".git/.." fallback because test_init_git_colocated_symlink_gitlink()
would otherwise fail. I haven't figured out why, and the test might be actually
wrong compared to the git CLI behavior, but let's not change that for now.

Fixes #2668

cli/src/cli_util.rs

@@ -1671,19 +1671,18 @@ fn is_colocated_git_workspace(workspace: &Workspace, repo: &ReadonlyRepo) -> boo
     let Some(git_backend) = repo.store().backend_impl().downcast_ref::<GitBackend>() else {
         return false;
     };
-    let Some(git_workdir) = git_backend
-        .git_workdir()
-        .and_then(|path| path.canonicalize().ok())
-    else {
+    let Some(git_workdir) = git_backend.git_workdir() else {
         return false; // Bare repository
     };
-    // Colocated workspace should have ".git" directory, file, or symlink. Since the
-    // backend is loaded from the canonical path, its working directory should also
-    // be resolved from the canonical ".git" path.
+    if git_workdir == workspace.workspace_root() {
+        return true;
+    }
+    // Colocated workspace should have ".git" directory, file, or symlink. Compare
+    // its parent as the git_workdir might be resolved from the real ".git" path.
     let Ok(dot_git_path) = workspace.workspace_root().join(".git").canonicalize() else {
         return false;
     };
-    Some(git_workdir.as_ref()) == dot_git_path.parent()
+    git_workdir.canonicalize().ok().as_deref() == dot_git_path.parent()
 }
 
 pub fn start_repo_transaction(

cli/tests/test_init_command.rs

@@ -297,13 +297,10 @@ fn test_init_git_colocated_symlink_directory_without_bare_config() {
     git_repo.config().unwrap().remove("core.bare").unwrap();
     std::fs::rename(workspace_root.join(".git"), &git_repo_path).unwrap();
     std::os::unix::fs::symlink(&git_repo_path, workspace_root.join(".git")).unwrap();
-    // FIXME: Working copy shouldn't be updated
     let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["init", "--git-repo", "."]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
-    Working copy now at: sqpuoqvx f6950fc1 (empty) (no description set)
-    Parent commit      : mwrttmos 8d698d4a my-branch | My commit message
-    Added 1 files, modified 0 files, removed 0 files
+    Done importing changes from the underlying Git repo.
     Initialized repo in "."
     "###);
 
@@ -315,12 +312,12 @@ fn test_init_git_colocated_symlink_directory_without_bare_config() {
     ~
     "###);
 
-    // FIXME: Check that the Git repo's HEAD moves
+    // Check that the Git repo's HEAD moves
     test_env.jj_cmd_ok(&workspace_root, &["new"]);
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
     insta::assert_snapshot!(stdout, @r###"
-    ◉  sqpuoqvx test.user@example.com 2001-02-03 04:05:07.000 +07:00 f6950fc1
-    │  (empty) (no description set)
+    ◉  sqpuoqvx test.user@example.com 2001-02-03 04:05:07.000 +07:00 HEAD@git f61b77cd
+    │  (no description set)
     ~
     "###);
 }

lib/src/git_backend.rs

@@ -17,10 +17,10 @@
 use std::any::Any;
 use std::fmt::{Debug, Error, Formatter};
 use std::io::{Cursor, Read};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::ExitStatus;
 use std::sync::{Arc, Mutex, MutexGuard};
-use std::{fs, str};
+use std::{fs, io, str};
 
 use async_trait::async_trait;
 use gix::objs::{CommitRefIter, WriteTo};
@@ -185,7 +185,7 @@ impl GitBackend {
     ) -> Result<Self, Box<GitBackendInitError>> {
         let canonical_git_repo_path = {
             let path = store_path.join(git_repo_path);
-            path.canonicalize()
+            canonicalize_git_repo_path(&path)
                 .context(&path)
                 .map_err(GitBackendInitError::Path)?
         };
@@ -240,8 +240,7 @@ impl GitBackend {
                 .context(&target_path)
                 .map_err(GitBackendLoadError::Path)?;
             let git_repo_path = store_path.join(git_repo_path_str);
-            git_repo_path
-                .canonicalize()
+            canonicalize_git_repo_path(&git_repo_path)
                 .context(&git_repo_path)
                 .map_err(GitBackendLoadError::Path)?
         };
@@ -368,6 +367,21 @@ impl GitBackend {
     }
 }
 
+/// Canonicalizes the given `path` except for the last `".git"` component.
+///
+/// The last path component matters when opening a Git repo without `core.bare`
+/// 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> {
+    if path.ends_with(".git") {
+        let workdir = path.parent().unwrap();
+        workdir.canonicalize().map(|dir| dir.join(".git"))
+    } else {
+        path.canonicalize()
+    }
+}
+
 fn gix_open_opts_from_settings(settings: &UserSettings) -> gix::open::Options {
     let user_name = settings.user_name();
     let user_email = settings.user_email();