Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/prek/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub(crate) static GIT_ROOT: LazyLock<Result<PathBuf, Error>> = 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.
/// <https://stackoverflow.com/questions/65639403/git-pre-commit-hook-how-can-i-get-added-modified-files-when-commit-with-a-flag/65647202#65647202>
pub(crate) static GIT_ENV_TO_REMOVE: LazyLock<Vec<(String, String)>> = LazyLock::new(|| {
pub(crate) static GIT_ENVS_TO_REMOVE: LazyLock<Vec<(String, String)>> = LazyLock::new(|| {
let keep = &[
"GIT_EXEC_PATH",
"GIT_SSH",
Expand Down
2 changes: 1 addition & 1 deletion crates/prek/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
100 changes: 100 additions & 0 deletions crates/prek/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <hash>`.
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::<Vec<_>>();

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:
Expand Down
Loading