mirror of
https://github.com/openai/codex.git
synced 2026-05-16 01:02:48 +00:00
Ignore configured hooks in git helpers (#22843)
## What - Internal Git helper commands now ignore configured hook directories during repository bookkeeping. ## Why - These helper flows should stay consistent even when a repository has hook-directory configuration of its own. ## How - Pass a command-local `core.hooksPath` override in the shared helper path and the Git-info helper path. - Add regressions for the baseline index rewrite flow and the metadata status flow. ## Validation - `cargo fmt --manifest-path /Users/bookholt/code/codex/codex-rs/Cargo.toml --all --check` - `cargo test --manifest-path /Users/bookholt/code/codex/codex-rs/Cargo.toml -p codex-git-utils` - `cargo test --manifest-path /Users/bookholt/code/codex/codex-rs/Cargo.toml -p codex-core test_get_has_changes_`
This commit is contained in:
@@ -379,6 +379,50 @@ async fn test_get_has_changes_ignores_repo_fsmonitor_config() {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn test_get_has_changes_ignores_configured_hooks_path() {
|
||||
let temp_dir = TempDir::new().expect("Failed to create temp dir");
|
||||
let repo_path = create_test_git_repo(&temp_dir).await;
|
||||
let hooks_dir = repo_path.join(".git/hooks-path-test");
|
||||
let hook_path = hooks_dir.join("post-index-change");
|
||||
let marker_path = repo_path.join("hook-ran");
|
||||
|
||||
fs::create_dir_all(&hooks_dir).expect("create hook dir");
|
||||
fs::write(
|
||||
&hook_path,
|
||||
format!(
|
||||
"#!/bin/sh\nprintf ran > \"{}\"\n",
|
||||
marker_path.to_string_lossy()
|
||||
),
|
||||
)
|
||||
.expect("write post-index-change hook");
|
||||
let mut permissions = fs::metadata(&hook_path)
|
||||
.expect("read hook metadata")
|
||||
.permissions();
|
||||
permissions.set_mode(0o755);
|
||||
fs::set_permissions(&hook_path, permissions).expect("mark hook executable");
|
||||
|
||||
Command::new("git")
|
||||
.args([
|
||||
"config",
|
||||
"core.hooksPath",
|
||||
hooks_dir.to_string_lossy().as_ref(),
|
||||
])
|
||||
.current_dir(&repo_path)
|
||||
.output()
|
||||
.await
|
||||
.expect("configure hooks path");
|
||||
|
||||
fs::write(repo_path.join("test.txt"), "test content").expect("refresh tracked file");
|
||||
|
||||
assert_eq!(get_has_changes(&repo_path).await, Some(false));
|
||||
assert!(
|
||||
!marker_path.exists(),
|
||||
"metadata collection should not invoke configured hook directories"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_get_git_working_tree_state_clean_repo() {
|
||||
let temp_dir = TempDir::new().expect("Failed to create temp dir");
|
||||
|
||||
@@ -584,6 +584,51 @@ mod tests {
|
||||
assert_eq!(git_stdout(&root, &["ls-files"]), "MEMORY.md\n");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn write_index_ignores_configured_hooks_path() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let home = TempDir::new().expect("tempdir");
|
||||
let root = home.path().join("repo");
|
||||
let hooks_dir = root.join(".git/hooks-path-test");
|
||||
let marker_path = root.join("hook-ran");
|
||||
let hook_path = hooks_dir.join("post-index-change");
|
||||
|
||||
fs::create_dir_all(&root).expect("create root");
|
||||
fs::write(root.join("MEMORY.md"), "baseline").expect("write memory");
|
||||
reset_git_repository(&root).await.expect("reset repo");
|
||||
fs::create_dir_all(&hooks_dir).expect("create hook dir");
|
||||
fs::write(
|
||||
&hook_path,
|
||||
format!(
|
||||
"#!/bin/sh\nprintf ran > \"{}\"\n",
|
||||
marker_path.to_string_lossy()
|
||||
),
|
||||
)
|
||||
.expect("write post-index-change hook");
|
||||
let mut permissions = fs::metadata(&hook_path)
|
||||
.expect("read hook metadata")
|
||||
.permissions();
|
||||
permissions.set_mode(0o755);
|
||||
fs::set_permissions(&hook_path, permissions).expect("mark hook executable");
|
||||
git_stdout(
|
||||
&root,
|
||||
&[
|
||||
"config",
|
||||
"core.hooksPath",
|
||||
hooks_dir.to_string_lossy().as_ref(),
|
||||
],
|
||||
);
|
||||
|
||||
write_index_from_head(&root).expect("rewrite baseline index");
|
||||
|
||||
assert!(
|
||||
!marker_path.exists(),
|
||||
"baseline index writes should not invoke configured hook directories"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn diff_reports_added_modified_and_deleted_files() {
|
||||
let home = TempDir::new().expect("tempdir");
|
||||
|
||||
@@ -40,6 +40,7 @@ pub fn get_git_repo_root(base_dir: &Path) -> Option<PathBuf> {
|
||||
|
||||
/// Timeout for git commands to prevent freezing on large repositories
|
||||
const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5);
|
||||
const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)]
|
||||
pub struct GitInfo {
|
||||
@@ -375,6 +376,8 @@ async fn run_git_command_with_timeout(args: &[&str], cwd: &Path) -> Option<std::
|
||||
let mut command = Command::new("git");
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
// Keep internal Git helper commands independent of configured hook directories.
|
||||
.args(["-c", &format!("core.hooksPath={DISABLED_HOOKS_PATH}")])
|
||||
.args(["-c", "core.fsmonitor=false"])
|
||||
.args(args)
|
||||
.current_dir(cwd)
|
||||
|
||||
@@ -6,6 +6,8 @@ use std::process::Command;
|
||||
|
||||
use crate::GitToolingError;
|
||||
|
||||
const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
|
||||
|
||||
pub(crate) fn ensure_git_repository(path: &Path) -> Result<(), GitToolingError> {
|
||||
match run_git_for_stdout(
|
||||
path,
|
||||
@@ -98,7 +100,12 @@ where
|
||||
{
|
||||
let iterator = args.into_iter();
|
||||
let (lower, upper) = iterator.size_hint();
|
||||
let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower));
|
||||
let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower) + 2);
|
||||
// Keep internal Git helper commands independent of configured hook directories.
|
||||
args_vec.push(OsString::from("-c"));
|
||||
args_vec.push(OsString::from(format!(
|
||||
"core.hooksPath={DISABLED_HOOKS_PATH}"
|
||||
)));
|
||||
for arg in iterator {
|
||||
args_vec.push(OsString::from(arg.as_ref()));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user