From 19dcd5ce730ca960f7156b081a05024ebaba9538 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Tue, 23 Dec 2025 10:48:59 +0800 Subject: [PATCH] Respect `GIT_DIR` set by git (#1258) * Respect `GIT_DIR` set by git * test * Set GIT_WORK_TREE * Add test --- crates/prek-consts/src/env_vars.rs | 2 + crates/prek/src/cli/install.rs | 8 +-- crates/prek/src/main.rs | 28 ++++++-- crates/prek/tests/hook_impl.rs | 67 +++++++++++++++-- crates/prek/tests/install.rs | 112 ++++------------------------- crates/prek/tests/run.rs | 1 + 6 files changed, 102 insertions(+), 116 deletions(-) diff --git a/crates/prek-consts/src/env_vars.rs b/crates/prek-consts/src/env_vars.rs index 04c39267..27de3367 100644 --- a/crates/prek-consts/src/env_vars.rs +++ b/crates/prek-consts/src/env_vars.rs @@ -10,6 +10,8 @@ impl EnvVars { pub const CI: &'static str = "CI"; // Git related + pub const GIT_DIR: &'static str = "GIT_DIR"; + pub const GIT_WORK_TREE: &'static str = "GIT_WORK_TREE"; pub const GIT_TERMINAL_PROMPT: &'static str = "GIT_TERMINAL_PROMPT"; pub const SKIP: &'static str = "SKIP"; diff --git a/crates/prek/src/cli/install.rs b/crates/prek/src/cli/install.rs index 6047c4c0..9582ea34 100644 --- a/crates/prek/src/cli/install.rs +++ b/crates/prek/src/cli/install.rs @@ -286,18 +286,12 @@ fn install_hook_script( /// The version of the hook script. Increment this when the script changes in a way that /// requires re-installation. -pub(crate) static CUR_SCRIPT_VERSION: usize = 3; +pub(crate) static CUR_SCRIPT_VERSION: usize = 4; static HOOK_TMPL: &str = r#"[SHEBANG] # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 -# Unset GIT_DIR to avoid git taking current directory as the git dir. -# See: -# https://github.com/pre-commit/pre-commit/issues/2295 -# https://www.spinics.net/lists/git/msg374197.html -unset GIT_DIR - ARGS=([PREK_ARGS]) HERE="$(cd "$(dirname "$0")" && pwd)" diff --git a/crates/prek/src/main.rs b/crates/prek/src/main.rs index b38129f0..3e57f7df 100644 --- a/crates/prek/src/main.rs +++ b/crates/prek/src/main.rs @@ -9,6 +9,7 @@ use anyhow::{Context, Result}; use clap::{CommandFactory, Parser}; use clap_complete::CompleteEnv; use owo_colors::OwoColorize; +use prek_consts::env_vars::EnvVars; use tracing::debug; use tracing::level_filters::LevelFilter; use tracing_subscriber::filter::Directive; @@ -137,7 +138,7 @@ fn setup_logging(level: Level, log_file: LogFile, store: &Store) -> Result<()> { Ok(()) } -async fn run(mut cli: Cli) -> Result { +async fn run(cli: Cli) -> Result { // Enabled ANSI colors on Windows. let _ = anstyle_query::windows::enable_ansi_colors(); @@ -175,12 +176,24 @@ async fn run(mut cli: Cli) -> Result { warnings::enable(); } - if cli.command.is_none() { - cli.command = Some(Command::Run(Box::new(cli.run_args.clone()))); - } - debug!("prek: {}", version::version()); + // If `GIT_DIR` is set, prek may be running from a git hook. + // Git exports `GIT_DIR` but *not* `GIT_WORK_TREE`. Without `GIT_WORK_TREE`, git + // treats the current working directory as the working tree. If prek changes the current + // working directory (with `--cd`), git commands run by prek may behave unexpectedly. + // + // To make git behavior stable, we set `GIT_WORK_TREE` ourselves to where prek is run from. + // If `GIT_WORK_TREE` is already set, we leave it alone. + // If `GIT_DIR` is not set, we let git discover `.git` after an optional `cd`. + // See: https://www.spinics.net/lists/git/msg374197.html + // https://github.com/pre-commit/pre-commit/issues/2295 + if EnvVars::is_set(EnvVars::GIT_DIR) && !EnvVars::is_set(EnvVars::GIT_WORK_TREE) { + let cwd = std::env::current_dir().context("Failed to get current directory")?; + debug!("Setting {} to `{}`", EnvVars::GIT_WORK_TREE, cwd.display()); + unsafe { std::env::set_var(EnvVars::GIT_WORK_TREE, cwd) } + } + if let Some(dir) = cli.globals.cd.as_ref() { debug!("Changing current directory to: `{}`", dir.display()); std::env::set_current_dir(dir)?; @@ -203,7 +216,10 @@ async fn run(mut cli: Cli) -> Result { } show_settings!(cli.globals, false); - match cli.command.unwrap() { + let command = cli + .command + .unwrap_or_else(|| Command::Run(Box::new(cli.run_args))); + match command { Command::Install(args) => { show_settings!(args); diff --git a/crates/prek/tests/hook_impl.rs b/crates/prek/tests/hook_impl.rs index c500d785..7d74decb 100644 --- a/crates/prek/tests/hook_impl.rs +++ b/crates/prek/tests/hook_impl.rs @@ -1,9 +1,10 @@ +use std::process::Command; + use assert_cmd::assert::OutputAssertExt; use assert_fs::fixture::{FileWriteStr, PathChild, PathCreateDir}; use indoc::indoc; use prek_consts::CONFIG_FILE; use prek_consts::env_vars::EnvVars; -use std::process::Command; use crate::common::TestContext; use crate::common::cmd_snapshot; @@ -187,6 +188,8 @@ fn hook_impl_pre_push() -> anyhow::Result<()> { fn run_worktree() -> anyhow::Result<()> { let context = TestContext::new(); context.init_project(); + context.configure_git_author(); + context.disable_auto_crlf(); context.write_pre_commit_config(indoc! { r" repos: - repo: local @@ -197,8 +200,6 @@ fn run_worktree() -> anyhow::Result<()> { entry: always fail always_run: true "}); - context.configure_git_author(); - context.disable_auto_crlf(); context.git_add("."); context.git_commit("Initial commit"); @@ -249,6 +250,61 @@ fn run_worktree() -> anyhow::Result<()> { Ok(()) } +/// Test prek hooks runs with `GIT_DIR` respected. +#[test] +fn git_dir_respected() { + let context = TestContext::new(); + context.init_project(); + context.configure_git_author(); + context.disable_auto_crlf(); + context.write_pre_commit_config(indoc! { r#" + repos: + - repo: local + hooks: + - id: print-git-dir + name: Print Git Dir + language: python + entry: python -c 'import os, sys; print("GIT_DIR:", os.environ.get("GIT_DIR")); print("GIT_WORK_TREE:", os.environ.get("GIT_WORK_TREE")); sys.exit(1)' + pass_filenames: false + "#}); + context.git_add("."); + let cwd = context.work_dir(); + + cmd_snapshot!(context.filters(), context.install(), @r#" + success: true + exit_code: 0 + ----- stdout ----- + prek installed at `.git/hooks/pre-commit` + + ----- stderr ----- + "#); + + let mut commit = Command::new("git"); + commit + .arg("--git-dir") + .arg(cwd.join(".git")) + .arg("--work-tree") + .arg(&**cwd) + .current_dir(context.home_dir()) + .arg("commit") + .arg("-m") + .arg("Test commit with GIT_DIR set"); + + cmd_snapshot!(context.filters(), commit, @r" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + Print Git Dir............................................................Failed + - hook id: print-git-dir + - exit code: 1 + + GIT_DIR: [TEMP_DIR]/.git + GIT_WORK_TREE: . + "); +} + #[test] fn workspace_hook_impl_root() -> anyhow::Result<()> { let context = TestContext::new(); @@ -455,6 +511,7 @@ fn workspace_hook_impl_worktree_subdirectory() -> anyhow::Result<()> { let mut commit = Command::new("git"); commit .current_dir(cwd.child("worktree")) + .env(EnvVars::PREK_HOME, &**context.home_dir()) .arg("commit") .arg("-m") .arg("Test commit from subdirectory") @@ -494,14 +551,14 @@ fn workspace_hook_impl_no_project_found() -> anyhow::Result<()> { context.git_add("."); // Install hook that allows missing config - cmd_snapshot!(context.filters(), context.install(), @r#" + cmd_snapshot!(context.filters(), context.install(), @r" success: true exit_code: 0 ----- stdout ----- prek installed at `.git/hooks/pre-commit` ----- stderr ----- - "#); + "); // Try to run hook-impl from directory without config let mut commit = Command::new("git"); diff --git a/crates/prek/tests/install.rs b/crates/prek/tests/install.rs index d94a1871..afb4fbac 100644 --- a/crates/prek/tests/install.rs +++ b/crates/prek/tests/install.rs @@ -38,13 +38,7 @@ fn install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -84,13 +78,7 @@ fn install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -119,13 +107,7 @@ fn install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=post-commit --script-version=3) + ARGS=(hook-impl --hook-type=post-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -162,13 +144,7 @@ fn install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -191,13 +167,7 @@ fn install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=post-commit --script-version=3) + ARGS=(hook-impl --hook-type=post-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -269,13 +239,7 @@ fn install_with_hooks() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -450,13 +414,7 @@ fn init_template_dir() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --skip-on-missing-config --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --skip-on-missing-config --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -493,13 +451,7 @@ fn init_template_dir() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --skip-on-missing-config --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --skip-on-missing-config --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -533,13 +485,7 @@ fn init_template_dir() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --config="non-exist-config" --skip-on-missing-config --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --config="non-exist-config" --skip-on-missing-config --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -639,13 +585,7 @@ fn workspace_install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -682,13 +622,7 @@ fn workspace_install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --cd="project3" --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --cd="project3" --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -722,13 +656,7 @@ fn workspace_install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl project3/ --skip=project2/ --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl project3/ --skip=project2/ --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -774,13 +702,7 @@ fn workspace_install() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl project3/ --hook-type=pre-commit --script-version=3) + ARGS=(hook-impl project3/ --hook-type=pre-commit --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") @@ -1005,13 +927,7 @@ fn workspace_init_template_dir() -> anyhow::Result<()> { # File generated by prek: https://github.com/j178/prek # ID: 182c10f181da4464a3eec51b83331688 - # Unset GIT_DIR to avoid git taking current directory as the git dir. - # See: - # https://github.com/pre-commit/pre-commit/issues/2295 - # https://www.spinics.net/lists/git/msg374197.html - unset GIT_DIR - - ARGS=(hook-impl --hook-type=pre-commit --skip-on-missing-config --script-version=3) + ARGS=(hook-impl --hook-type=pre-commit --skip-on-missing-config --script-version=4) HERE="$(cd "$(dirname "$0")" && pwd)" ARGS+=(--hook-dir "$HERE" -- "$@") diff --git a/crates/prek/tests/run.rs b/crates/prek/tests/run.rs index e23811f8..9b54e7b6 100644 --- a/crates/prek/tests/run.rs +++ b/crates/prek/tests/run.rs @@ -2040,6 +2040,7 @@ fn git_commit_a() -> Result<()> { .arg("-a") .arg("-m") .arg("Update file") + .env(EnvVars::PREK_HOME, &**context.home_dir()) .current_dir(cwd); let filters = context