diff --git a/crates/prek/src/git.rs b/crates/prek/src/git.rs index bde476bdc..063124b06 100644 --- a/crates/prek/src/git.rs +++ b/crates/prek/src/git.rs @@ -45,7 +45,7 @@ pub(crate) static GIT_ROOT: LazyLock> = LazyLock::new(|| /// and set `GIT_INDEX_FILE` to point to it. /// We need to keep the `GIT_INDEX_FILE` env var to make sure `git write-tree` works correctly. /// -pub(crate) static GIT_ENV_TO_REMOVE: LazyLock> = LazyLock::new(|| { +pub(crate) static GIT_ENVS_TO_REMOVE: LazyLock> = LazyLock::new(|| { let keep = &[ "GIT_EXEC_PATH", "GIT_SSH", diff --git a/crates/prek/src/process.rs b/crates/prek/src/process.rs index c428817ea..7d15fd379 100644 --- a/crates/prek/src/process.rs +++ b/crates/prek/src/process.rs @@ -408,7 +408,7 @@ impl Cmd { /// Remove some git-specific environment variables to make git commands isolated. pub fn remove_git_envs(&mut self) -> &mut Self { - for (key, _) in crate::git::GIT_ENV_TO_REMOVE.iter() { + for (key, _) in crate::git::GIT_ENVS_TO_REMOVE.iter() { self.inner.env_remove(key); } self diff --git a/crates/prek/tests/run.rs b/crates/prek/tests/run.rs index 5e5112b65..c02454528 100644 --- a/crates/prek/tests/run.rs +++ b/crates/prek/tests/run.rs @@ -2118,6 +2118,106 @@ fn git_commit_a() -> Result<()> { Ok(()) } +#[cfg(unix)] +#[test] +fn git_commit_a_currently_fails_when_hook_writes_to_temp_git_index() -> Result<()> { + let context = TestContext::new(); + context.init_project(); + + // Repro for #1786 documenting the current behavior. + // `git commit -a` exports `GIT_INDEX_FILE=.git/index.lock` to the pre-commit hook + // process. If the hook inherits that env var and then runs a git command that writes + // to an index in a different repository, Git will write those entries into the parent + // repo's temporary index instead. + // + // The important detail is that the temp repo stages `file.txt`, matching a tracked + // path in the parent repo. That makes prek's post-hook `git diff` read the corrupted + // parent index entry and fail with `fatal: unable to read `. + context + .work_dir() + .child("hook.sh") + .write_str(indoc::indoc! {r#" + set -eu + tmpdir="$(mktemp -d)" + trap 'rm -rf "$tmpdir"' EXIT + cd "$tmpdir" + git init >/dev/null 2>&1 + printf 'hook version\n' > file.txt + git add file.txt + "#})?; + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: local + hooks: + - id: write-temp-index + name: write-temp-index + language: system + entry: sh hook.sh + pass_filenames: false + always_run: true + verbose: true + "}); + + let cwd = context.work_dir(); + let file = cwd.child("file.txt"); + file.write_str("Hello, world!\n")?; + + cmd_snapshot!(context.filters(), context.install(), @r#" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "#); + + context.git_add("."); + context.git_commit("Initial commit"); + + // `git commit` does not set `GIT_INDEX_FILE`; `git commit -a` does. + // The repro only triggers on the `-a` path. + file.write_str("Hello again!\n")?; + + let mut commit = git_cmd(cwd); + commit + .arg("commit") + .arg("-a") + .arg("-m") + .arg("Update file") + .env(EnvVars::PREK_HOME, &**context.home_dir()); + + let filters = context + .filters() + .into_iter() + .chain([ + (r"\[master \w{7}\]", r"[master COMMIT]"), + ( + r"fatal: unable to read [0-9a-f]{40}", + "fatal: unable to read [HASH]", + ), + ]) + .collect::>(); + + cmd_snapshot!(filters, commit, @r" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + error: Command `git diff` exited with an error: + + [status] + exit status: 128 + + [stderr] + fatal: unable to read [HASH] + " + ); + + Ok(()) +} + fn write_pre_commit_config(path: &Path, hooks: &[(&str, &str)]) -> Result<()> { let mut yaml = String::from(indoc::indoc! {" repos: