From f5cfc7edd32424fb95137c011add3dcf84252fdf Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 11:10:11 -0700 Subject: [PATCH] Isolate shipped_documents_load's scratch repos from the caller's git env improvise-53zz. The scratch-repo shape tests inherited the caller's git environment. A hook in a linked worktree exports GIT_DIR (and GIT_INDEX_FILE), which override -C, so `git -C add` staged the scratch files into the outer checkout's index. The user's global excludesFile applied too, so a `*.gz` line there broke the gzip test. One helper, git(root), now removes every name that `git rev-parse --local-env-vars` prints. run_git also sets GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM=1. Two new tests re-run a shape test in a child process of this binary with the variables set. Both were red before the fix. Five rivals are recorded in context/test-evidence/tui.md. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/test-evidence/tui.md | 55 ++++++++++++++++++++ tests/shipped_documents_load.rs | 91 ++++++++++++++++++++++++++++++--- 2 files changed, 140 insertions(+), 6 deletions(-) diff --git a/context/test-evidence/tui.md b/context/test-evidence/tui.md index e19a8690..0ad20193 100644 --- a/context/test-evidence/tui.md +++ b/context/test-evidence/tui.md @@ -490,6 +490,61 @@ in a scratch repository and asserts the walk lists it. All four were red against the `.gitignore` walk. The account, including the rivals, is under `every_shipped_document_loads` above. +### a_scratch_repo_ignores_an_inherited_git_dir + +improvise-53zz. The scratch repositories inherited the caller's git +environment, and two parts of it change what `git -C ` does. + +**The repo-local variables beat `-C`.** A hook running in a linked worktree +exports `GIT_DIR` (and, during a commit, `GIT_INDEX_FILE`). Under those, +`git -C init` re-initialised the *outer* repository and `git add` +staged the scratch files into the *outer* index, so the test suite wrote into +the checkout that ran it. Three of the four shape tests still passed that way, +because `ls-files` read the same wrong index that `add` had just written. So a +green run was no evidence the walk had looked at the scratch repository. + +The fix is one helper, `git(root)`, which removes every name that +`git rev-parse --local-env-vars` prints before adding `-C`. It asks git rather +than hard-coding a list, so the list follows the installed git. Both +`shipped_documents` and the scratch helpers go through it, so the production +walk ignores an inherited `GIT_DIR` too. + +The test must set the variable, and a test thread cannot set it without +setting it for every other thread. So the test re-runs this same binary as a +child, restricted to `a_single_file_un_ignore_is_walked`, with `GIT_DIR` and +`GIT_INDEX_FILE` naming a throwaway outer repository. It asserts two things: +the child passed, and the outer index is still empty. Before the fix the second +assertion failed with `the outer index gained: .gitignore`. + +Rivals, each run against the fixed file: + +- Remove only `GIT_DIR`: red, because `GIT_INDEX_FILE` still sends the add to + the outer index. +- Remove only `GIT_INDEX_FILE`: red, because `GIT_DIR` does. +- Rename the child test so the child runs nothing: red on + `… did not run`. `rerun_with_env` asserts `running 1 test`, because + libtest exits 0 when a filter matches nothing, and both of these tests would + then pass without running anything. + +### a_scratch_repo_ignores_the_users_global_and_system_excludes + +improvise-53zz. The scratch repositories also read the user's own git config. A +`*.gz` line in a global `core.excludesFile` made `git add` refuse +`docs/model.improv.gz`, so `a_gzipped_document_is_walked` failed on that +machine and passed on others. `run_git` now sets `GIT_CONFIG_GLOBAL=/dev/null` +and `GIT_CONFIG_NOSYSTEM=1`. `shipped_documents` does not need them: `ls-files` +reads the index, and excludes never apply to a tracked file. + +The test writes a config whose `excludesFile` holds `*.gz`, and re-runs the +gzip shape test in a child once under `GIT_CONFIG_GLOBAL` and once under +`GIT_CONFIG_SYSTEM`. Before the fix the global run failed at `git add`. + +Rivals: + +- Drop `GIT_CONFIG_GLOBAL=/dev/null`: red, in the global run. +- Drop `GIT_CONFIG_NOSYSTEM=1`: red, in the system run. The system scope is + there so that this half of the fix has a witness at all. + --- ## `tests/web_demo_model.rs` diff --git a/tests/shipped_documents_load.rs b/tests/shipped_documents_load.rs index 777b1b1b..c2c28add 100644 --- a/tests/shipped_documents_load.rs +++ b/tests/shipped_documents_load.rs @@ -6,9 +6,7 @@ use std::path::{Path, PathBuf}; /// Every tracked `.improv` and `.improv.gz` under `root`. Outside a checkout this /// panics rather than answering empty (improvise-c0aj). fn shipped_documents(root: &Path) -> Vec { - let out = std::process::Command::new("git") - .arg("-C") - .arg(root) + let out = git(root) .args(["ls-files", "-z", "--", "*.improv", "*.improv.gz"]) .output() .expect("git runs: this test reads the index, so it needs a checkout"); @@ -50,6 +48,21 @@ fn every_shipped_document_loads() { } } +/// `git -C root`, with every repo-local variable a caller exported removed, +/// since `GIT_DIR` and friends override `-C` (improvise-53zz). +fn git(root: &Path) -> std::process::Command { + let names = std::process::Command::new("git") + .args(["rev-parse", "--local-env-vars"]) + .output() + .expect("git runs"); + let mut git = std::process::Command::new("git"); + for name in String::from_utf8(names.stdout).expect("UTF-8").lines() { + git.env_remove(name); + } + git.arg("-C").arg(root); + git +} + /// A scratch repository, so each way of shipping a document is set up for real. fn scratch_repo() -> tempfile::TempDir { let dir = tempfile::tempdir().expect("a temp dir"); @@ -58,9 +71,9 @@ fn scratch_repo() -> tempfile::TempDir { } fn run_git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .arg("-C") - .arg(root) + let status = git(root) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") .args(args) .status() .expect("git runs"); @@ -137,3 +150,69 @@ fn a_force_added_document_is_walked() { run_git(root, &["add", "-f", "secret/forced.improv"]); assert!(walked(root, "secret/forced.improv")); } + +/// Runs one of this binary's own tests in a child process with `env` added, +/// since the environment is what is under test and threads share ours. +fn rerun_with_env(test: &str, env: &[(&str, &Path)]) -> std::process::Output { + let mut child = std::process::Command::new(std::env::current_exe().expect("test binary")); + child.args([test, "--exact", "--nocapture"]); + for (name, value) in env { + child.env(name, value); + } + let out = child.output().expect("the test binary runs"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("running 1 test"), + "{test} did not run: {stdout}" + ); + out +} + +/// improvise-53zz: a hook in a linked worktree exports `GIT_DIR`, which beats +/// `-C`. Evidence: `context/test-evidence/tui.md`. +#[test] +fn a_scratch_repo_ignores_an_inherited_git_dir() { + let outer = scratch_repo(); + let dot_git = outer.path().join(".git"); + let out = rerun_with_env( + "a_single_file_un_ignore_is_walked", + &[ + ("GIT_DIR", &dot_git), + ("GIT_INDEX_FILE", &dot_git.join("index")), + ], + ); + assert!( + out.status.success(), + "the shape test fails under an inherited GIT_DIR: {}", + String::from_utf8_lossy(&out.stdout) + ); + let staged = git(outer.path()) + .args(["ls-files"]) + .output() + .expect("git runs"); + assert!( + staged.stdout.is_empty(), + "the outer index gained: {}", + String::from_utf8_lossy(&staged.stdout) + ); +} + +/// improvise-53zz: a user's `*.gz` exclude refused the gzip shape test's `add`. +/// Evidence: `context/test-evidence/tui.md`. +#[test] +fn a_scratch_repo_ignores_the_users_global_and_system_excludes() { + let home = tempfile::tempdir().expect("a temp dir"); + let excludes = home.path().join("ignore"); + std::fs::write(&excludes, "*.gz\n").expect("writes"); + let config = home.path().join("gitconfig"); + let text = format!("[core]\n\texcludesFile = {}\n", excludes.display()); + std::fs::write(&config, text).expect("writes"); + for scope in ["GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM"] { + let out = rerun_with_env("a_gzipped_document_is_walked", &[(scope, &config)]); + assert!( + out.status.success(), + "the gzip shape test fails under a `*.gz` exclude in {scope}: {}", + String::from_utf8_lossy(&out.stdout) + ); + } +}