From 9fb9e732c165f77ac3f65a80b3165b0c896dbd2a Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 7 Aug 2024 18:50:13 +0900
Subject: [PATCH] git: resolve relative core.excludesFile path at workspace
 root

The "git" command appears to chdir() to the --work-tree directory first, then
read() the core.excludesFile file. There's no manual relative path resolution
in "git".

Fixes #4222
---
 cli/src/cli_util.rs          | 11 +++++++----
 cli/tests/test_gitignores.rs | 31 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index 067fe1c6e..4771ddc7b 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -800,17 +800,20 @@ impl WorkspaceCommandHelper {
 
     #[instrument(skip_all)]
     pub fn base_ignores(&self) -> Result<Arc<GitIgnoreFile>, GitIgnoreError> {
-        fn get_excludes_file_path(config: &gix::config::File) -> Option<PathBuf> {
+        let get_excludes_file_path = |config: &gix::config::File| -> Option<PathBuf> {
             // TODO: maybe use path() and interpolate(), which can process non-utf-8
             // path on Unix.
             if let Some(value) = config.string("core.excludesFile") {
-                str::from_utf8(&value)
+                let path = str::from_utf8(&value)
                     .ok()
-                    .map(crate::git_util::expand_git_path)
+                    .map(crate::git_util::expand_git_path)?;
+                // The configured path is usually absolute, but if it's relative,
+                // the "git" command would read the file at the work-tree directory.
+                Some(self.workspace_root().join(path))
             } else {
                 xdg_config_home().ok().map(|x| x.join("git").join("ignore"))
             }
-        }
+        };
 
         fn xdg_config_home() -> Result<PathBuf, VarError> {
             if let Ok(x) = std::env::var("XDG_CONFIG_HOME") {
diff --git a/cli/tests/test_gitignores.rs b/cli/tests/test_gitignores.rs
index 20be00561..3e5a5dd51 100644
--- a/cli/tests/test_gitignores.rs
+++ b/cli/tests/test_gitignores.rs
@@ -64,6 +64,37 @@ fn test_gitignores() {
     "###);
 }
 
+#[test]
+fn test_gitignores_relative_excludes_file_path() {
+    let test_env = TestEnvironment::default();
+    let workspace_root = test_env.env_root().join("repo");
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "repo"]);
+
+    let mut file = std::fs::OpenOptions::new()
+        .append(true)
+        .open(workspace_root.join(".git").join("config"))
+        .unwrap();
+    file.write_all(b"[core]\nexcludesFile=../my-ignores\n")
+        .unwrap();
+    drop(file);
+    std::fs::write(test_env.env_root().join("my-ignores"), "ignored\n").unwrap();
+
+    std::fs::write(workspace_root.join("ignored"), "").unwrap();
+    std::fs::write(workspace_root.join("not-ignored"), "").unwrap();
+
+    // core.excludesFile should be resolved relative to the workspace root, not
+    // to the cwd.
+    std::fs::create_dir(workspace_root.join("sub")).unwrap();
+    let stdout = test_env.jj_cmd_success(&workspace_root.join("sub"), &["diff", "-s"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    A ../not-ignored
+    "###);
+    let stdout = test_env.jj_cmd_success(test_env.env_root(), &["-Rrepo", "diff", "-s"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    A repo/not-ignored
+    "###);
+}
+
 #[test]
 fn test_gitignores_ignored_file_in_target_commit() {
     let test_env = TestEnvironment::default();