git-utils: make config-source race test deterministic

This commit is contained in:
Chris Bookholt
2026-07-02 05:23:30 -07:00
parent c363658387
commit 35847f6c3d
2 changed files with 92 additions and 21 deletions
+4 -21
View File
@@ -6,7 +6,6 @@ use std::fs::FileTimes;
use std::path::Path;
use std::thread;
use std::time::Duration;
use std::time::Instant;
use std::time::UNIX_EPOCH;
const PATCH: &str =
@@ -204,21 +203,6 @@ fn run_isolated_test(test_name: &str, env: &[(&str, &OsStr)]) {
);
}
fn wait_for_config_source_probe(trace: &Path) -> bool {
let deadline = Instant::now() + Duration::from_secs(10);
while Instant::now() < deadline {
let contents = std::fs::read_to_string(trace).unwrap_or_default();
if contents
.find("^include")
.is_some_and(|offset| contents[offset..].contains("\"event\":\"exit\""))
{
return true;
}
thread::yield_now();
}
false
}
#[test]
fn reverse_staging_uses_command_scoped_filter_override() {
if std::env::var_os("CODEX_GIT_UTILS_APPLY_FILTER_ENV_CHILD").is_none() {
@@ -342,23 +326,22 @@ fn apply_rejects_unseen_driver_added_through_worktree_include_after_source_probe
let trace = PathBuf::from(std::env::var_os("GIT_TRACE2_EVENT").expect("trace path"));
std::fs::write(&trace, "").expect("clear fixture trace");
let watcher_trace = trace.clone();
let phase = crate::git_config_sources::install_blocking_no_includes_query_phase();
let watcher = thread::spawn(move || {
let observed = wait_for_config_source_probe(&watcher_trace);
if observed {
phase.run(|| {
std::fs::write(
included,
format!("[filter \"fresh\"]\n\tclean = {FILTER_COMMAND}\n\trequired = true\n"),
)
.expect("add previously unseen filter driver");
}
observed
})
});
let error = apply_git_patch(&request(
root, /*revert*/ false, /*preflight*/ false,
))
.expect_err("reject worktree config source before final apply");
crate::git_config_sources::clear_blocking_no_includes_query_phase();
assert!(watcher.join().expect("config watcher"));
assert_eq!(error.kind(), io::ErrorKind::PermissionDenied);
@@ -3,6 +3,12 @@ use std::io;
use std::path::Path;
#[cfg(test)]
use std::path::PathBuf;
#[cfg(test)]
use std::sync::mpsc::Receiver;
#[cfg(test)]
use std::sync::mpsc::SyncSender;
#[cfg(test)]
use std::sync::mpsc::sync_channel;
use crate::git_command::GitRunner;
use crate::git_config::read_config_entries_without_includes;
@@ -30,6 +36,84 @@ const INCLUDE_CONFIG_PATTERN: &str = r"^include(\.path|if\..*\.path)$";
const MAX_CONFIG_INCLUDE_DEPTH: usize = 10;
const MAX_CONFIG_INCLUDE_FILES: usize = 1024;
#[cfg(test)]
struct BlockingNoIncludesQuerySignal {
reached: SyncSender<()>,
resume: Receiver<()>,
}
#[cfg(test)]
thread_local! {
static BLOCKING_NO_INCLUDES_QUERY_SIGNAL: std::cell::RefCell<
Option<BlockingNoIncludesQuerySignal>,
> = const { std::cell::RefCell::new(None) };
}
/// Test-only rendezvous at the blocking config-source policy boundary.
///
/// The production build does not compile this hook. Unit tests can use it to
/// mutate a config source after the no-includes query has completed and before
/// the returned include entries are validated, without relying on trace-file
/// polling or scheduler timing.
#[cfg(test)]
pub(crate) struct BlockingNoIncludesQueryPhase {
reached: Receiver<()>,
resume: SyncSender<()>,
}
#[cfg(test)]
impl BlockingNoIncludesQueryPhase {
pub(crate) fn run(self, action: impl FnOnce()) -> bool {
if self.reached.recv().is_err() {
return false;
}
action();
self.resume.send(()).is_ok()
}
}
#[cfg(test)]
pub(crate) fn install_blocking_no_includes_query_phase() -> BlockingNoIncludesQueryPhase {
let (reached, reached_receiver) = sync_channel(/*bound*/ 0);
let (resume, resume_receiver) = sync_channel(/*bound*/ 0);
BLOCKING_NO_INCLUDES_QUERY_SIGNAL.with(|slot| {
assert!(
slot.borrow().is_none(),
"blocking no-includes query phase already installed on this thread"
);
*slot.borrow_mut() = Some(BlockingNoIncludesQuerySignal {
reached,
resume: resume_receiver,
});
});
BlockingNoIncludesQueryPhase {
reached: reached_receiver,
resume,
}
}
#[cfg(test)]
pub(crate) fn clear_blocking_no_includes_query_phase() {
BLOCKING_NO_INCLUDES_QUERY_SIGNAL.with(|slot| {
slot.borrow_mut().take();
});
}
#[cfg(test)]
fn signal_blocking_no_includes_query_phase() {
let signal = BLOCKING_NO_INCLUDES_QUERY_SIGNAL.with(|slot| slot.borrow_mut().take());
if let Some(signal) = signal {
signal
.reached
.send(())
.expect("blocking no-includes query phase receiver");
signal
.resume
.recv()
.expect("blocking no-includes query phase release");
}
}
/// Reject configuration that an untrusted worktree writer can change between
/// a policy probe and the Git command it guards.
pub(crate) fn ensure_no_worktree_config_sources(
@@ -172,6 +256,10 @@ async fn ensure_no_worktree_config_sources_impl(
let entries = execution
.read_include_entries(&git_root, git_config_args, /*config_file*/ None)
.await?;
#[cfg(test)]
if matches!(execution, SourceExecution::Blocking(_)) {
signal_blocking_no_includes_query_phase();
}
let mut pending = Vec::new();
let mut include_budget = IncludeGraphBudget::default();
execution