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 <scratch> 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -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 <scratch>` 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 <scratch> 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`
|
||||
|
||||
@@ -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<PathBuf> {
|
||||
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)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user