fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge PR #289 (8f218644) into the 2026-09-24 integration wave

Browse Source
This commit is contained in:
Edward Langley
2026-09-24 02:44:56 -07:00
parent 90ea09360f 8f218644fe
commit 58b8e7902e
2 changed files with 314 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/test-evidence/tui.md
+205 -2
View File
@@ -490,7 +490,38 @@ 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
### a_hostile_git_environment_changes_nothing
improvise-eahe. Each section below it once had a test of its own, and each of those tests re-ran this binary in child processes, one child per setting (a0i3 alone ran ten). By #276 that was 18 children, and each one spawns several `git` processes, so this binary took 2.1–3.3s against the 2s-per-binary tripwire (`context/design-principles.md` §7).
**What changed.** The six tests are now one test, and its two children carry every hostile setting at once:
- **Children:** one re-runs `a_gzipped_document_is_walked` and one re-runs `every_shipped_document_loads`.
- **Settings:** each child gets the template dir, the global and system config, `XDG_CONFIG_HOME`, `GIT_DIR`/`GIT_INDEX_FILE`, the five pathspec and attribute settings, the six trace variables, and the config's trace2 targets.
- **Outer index:** the test then asserts the outer repository's index is still empty. That is the check that makes the inherited `GIT_DIR` visible, as in its section below.
**Why one child still separates the settings.** The sections below show that each setting breaks the walk on its own. So a rival that disables the defence against one setting leaves that setting live in the child, and the child fails. Every rival was re-run against the combined test. The ones that were red against the per-setting tests are still red:
- dropping `--template=`;
- the `--local-env-vars` loop;
- the `core.excludesFile` or `core.attributesFile` null;
- each of `GIT_LITERAL_PATHSPECS`, `GIT_GLOB_PATHSPECS`, `GIT_NOGLOB_PATHSPECS` and `GIT_ATTR_SOURCE`;
- the trace prefix: no strip, the exact name only, the exact three names, and the prefix minus `GIT_TRACE`, `_SETUP` or `_PERFORMANCE`;
- the trace2 `0` override.
**The ones green before are green now.**
- `GIT_ICASE_PATHSPECS` is green, as its section explains.
- The prefix minus `GIT_TRACE2_EVENT` is green, because the `0` override covers it. It was red against the per-setting test at ddcdf93a, before #276 added the override.
- **Dropping `GIT_CONFIG_GLOBAL=/dev/null` or `GIT_CONFIG_NOSYSTEM=1` was green, both at #276's head and in eahe's first version.** Their witness was an excludes line, and vwjj's `-c core.excludesFile=/dev/null` outranks every config file, so the witness could not see them. Consolidation did not cause this. improvise-3j70 has since given them a witness that no `-c` reaches (see “The user's global and system excludes” below), and both rivals are red.
**Measured** on an Apple M4 Max (16 cores), three `cargo test --workspace` runs each, before and after. The median counts only binaries that ran a test:
| | this binary | load average | run median | ratio | next largest |
|---|---|---|---|---|---|
| before (#276, 18 children) | 2.10, 3.04, 3.31s | 37–39 (1 min) | 0.00–0.01s | ≈300× | 0.90s, one 2.66s spike |
| after (2 children) | 0.57, 0.59, 0.91s | 30–33 (1 min) | 0.00–0.02s | ≈30–90× | 1.02s, one 2.49s spike |
The median is at the 0.01s resolution cargo prints, so the ratio is coarse. Before, this binary was the outlier by more than two orders of magnitude and over the tripwire in every run. After, it is below the next-largest binary in every run and well under 2s. The occasional 2.5s spike in "next largest" is some other binary, in both runs; that is load, not this file.
### An inherited `GIT_DIR` (improvise-53zz)
improvise-53zz. The scratch repositories inherited the caller's git
environment, and two parts of it change what `git -C <scratch>` does.
@@ -526,7 +557,7 @@ Rivals, each run against the fixed file:
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
### The user's global and system excludes (improvise-53zz)
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
@@ -545,6 +576,178 @@ Rivals:
- 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.
**Both rivals later went green, and nothing noticed (improvise-3j70).**
improvise-vwjj added `-c core.excludesFile=/dev/null` to `run_git`, and a `-c`
argument outranks every config file. So a global or system `excludesFile` could
no longer reach `git add` whether or not the config was nulled, and the two
rivals above passed from vwjj onwards. The config lines were still worth having,
since they null every *other* key, but nothing pinned them.
The witness is now a second key in the same config, `core.sharedRepository =
nonsense`. Measured on git 2.55:
- `git init` and `git add` die on it (`fatal: bad boolean config value
'nonsense' for 'core.sharedrepository'`);
- `git ls-files` does not read it, so the walk itself, which does not null the
user's config, is unaffected;
- `run_git` passes no `-c` for it.
Two keys were tried first and rejected: `init.defaultObjectFormat` and
`init.defaultRefFormat` only **warn** on an unknown value, so `init` carries on.
The key sits in `a_hostile_git_environment_changes_nothing`'s config, which the
child reads as both `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`, so there is
still one child. Dropping either env line is red. Every other rival in the
combined test is unchanged.
### The user's default ignore and attributes files (improvise-vwjj)
improvise-vwjj, found reviewing PR #248 at `f5cfc7ed`. The test above sets
`core.excludesFile` *inside* a config file, so emptying the config files looks
like a complete fix. It is not. When `core.excludesFile` is unset, git falls
back to `$XDG_CONFIG_HOME/git/ignore`, or `~/.config/git/ignore` when
`XDG_CONFIG_HOME` is unset. No config file is involved, so
`GIT_CONFIG_GLOBAL=/dev/null` leaves the fallback in force. It is also the
commonest place to keep a global ignore, because it needs no config line.
`core.attributesFile` has the same kind of default, `git/attributes`.
The test points `XDG_CONFIG_HOME` at a temp directory and re-runs the gzip
shape test in a child twice:
- once with `git/ignore` holding `*.gz`;
- once with `git/attributes` holding `*.gz working-tree-encoding=UTF-16`.
The attributes line is chosen because it breaks `git add`: the one-byte file
cannot be decoded as UTF-16. An attribute that only changes line endings would
leave the add succeeding, and the case would witness nothing.
Before the fix (at `f5cfc7ed`) the ignore run failed at `git add`. With only
`-c core.excludesFile=/dev/null` added, the attributes run failed. So each half
of the fix is witnessed separately.
The fix is two `-c` arguments in `run_git`: `core.excludesFile=/dev/null` and
`core.attributesFile=/dev/null`. A `-c` argument outranks every config file and
every default path, so the `~/.config` fallback is closed by the same two lines.
This test sets only `XDG_CONFIG_HOME`, so the `~/.config` path is covered by
that argument, not exercised.
Rivals, each run against the fixed file:
- Drop `core.excludesFile=/dev/null`: red, `fails under a default git/ignore`.
- Drop `core.attributesFile=/dev/null`: red, `fails under a default
git/attributes`.
### The user's template directory (improvise-pr3i)
improvise-pr3i, found reviewing PR #255 at `56ac72a2`. This is a third path to the
53zz symptom. `git init` copies a template directory into the new `.git`, and
`GIT_TEMPLATE_DIR` names one. The variable is not in
`git rev-parse --local-env-vars`, so `git(root)` does not remove it. It also
bypasses config, so `GIT_CONFIG_GLOBAL=/dev/null` (which nulls
`init.templateDir`) does not reach it either. A template whose `info/exclude`
holds `*.gz` therefore became the scratch repository's own exclude file, and
`git add` refused the gzipped document.
The test points `GIT_TEMPLATE_DIR` at such a template and re-runs
`a_gzipped_document_is_walked` in a child. Red at `56ac72a2`, failing at `git add`
with `Use -f if you really want to add them`. That line appears in the failure
message only because this PR also makes `rerun_with_env`'s callers print the
child's **stderr** (`told`). The child's panic goes to stderr, and before this
change the message printed an empty stdout.
The fix is `git init -q --template=`. An empty template directory copies nothing,
whatever the environment, config or compiled-in default says. The obvious
alternative, `env_remove("GIT_TEMPLATE_DIR")`, passes this test equally well, so
the choice between them is made by argument, not by the test. Removing the
variable still lets git copy its compiled-in default template, which a
distribution may have edited. `--template=` closes that too, for the same reason
the `-c` arguments in `run_git` beat relying on the absence of a config file.
Rival: drop `--template=`. Red, as above.
### Pathspec and attribute settings (improvise-a0i3)
improvise-a0i3. `git(root)` strips what `git rev-parse --local-env-vars` names,
and that list holds only the variables that point at a repository (`GIT_DIR`,
`GIT_INDEX_FILE` and so on). Pathspec magic and the attribute source are
process-wide settings, so the list names none of them. They still change what
`git ls-files -- '*.improv' '*.improv.gz'` answers:
- `GIT_LITERAL_PATHSPECS=1` reads `*.improv` as a file literally named
`*.improv`, so the walk finds nothing.
- `GIT_GLOB_PATHSPECS=1` stops `*` at `/`, so only a document at the root
matches. `GIT_NOGLOB_PATHSPECS=1` reads `*` literally, like the literal
setting. Every document these tests walk sits below the root, so both
settings made both tests fail.
- `GIT_ATTR_SOURCE=HEAD` makes git read attributes from `HEAD`. The scratch
repository in the gzip shape test has no commit, so `git add` fails with
`bad --attr-source`.
The test reruns `a_gzipped_document_is_walked` and `every_shipped_document_loads`
in a child under each of five settings, which is ten runs, and collects every
failure before it asserts. Red at #260's head `322de370`: 7 of 10 runs failed.
`LITERAL`, `GLOB` and `NOGLOB` each broke both tests. `ATTR_SOURCE` broke only
the gzip shape test, because the real checkout has a `HEAD`. The ticket reported
`ATTR_SOURCE` breaking both tests. Here it broke one, so the difference is in
the checkout the ticket measured, not in the fix.
The fix is to remove the five variables in `git()`, next to the
`--local-env-vars` loop. A `--literal-pathspecs`-style command-line flag would
have to be chosen per variable, and `GIT_ATTR_SOURCE` has no negating flag.
Rivals, each dropping one name from `PATHSPEC_AND_ATTRIBUTE_SETTINGS`:
- Without `GIT_LITERAL_PATHSPECS`, `GIT_GLOB_PATHSPECS` or
`GIT_NOGLOB_PATHSPECS`: red, both tests under that setting.
- Without `GIT_ATTR_SOURCE`: red, the gzip shape test.
- Without `GIT_ICASE_PATHSPECS`: **green.** A case-insensitive match of
`*.improv` still finds every document, so this row of the test cannot fail.
The name stays in the list because it is the fourth of git's pathspec
variables, and a future spelling of the walk that depends on case would be
exposed to it. The test covers it, but that coverage is vacuous today.
### A git trace sent to stdout (improvise-2iws, improvise-vav0)
improvise-2iws, with improvise-vav0 folded in. The `GIT_TRACE*` family is
process-wide, like improvise-a0i3's settings, so `--local-env-vars` lists none
of them either. Pointed at `/dev/stdout`, git writes trace lines into the
`ls-files -z` output the walk parses, and they are read as paths.
`every_shipped_document_loads` panicked on "Cannot open <root>/17:39:40… git.c:502
trace: built-in: git ls-files …".
**The fix has two parts.**
- `untraced_git()` removes every variable whose name starts with `GIT_TRACE`. A
prefix match, because the family is open-ended. `git()` builds on it, and so
does the `rev-parse --local-env-vars` call, whose stdout is parsed as well.
- It then sets `GIT_TRACE2`, `GIT_TRACE2_EVENT` and `GIT_TRACE2_PERF` to `0`.
A user config's `trace2.normalTarget`, `eventTarget` or `perfTarget` is a
**config** setting, which no environment removal reaches, and the walk's git
does not null the user's config (only `run_git` does). An environment value of
`0` outranks it (improvise-vav0, verified by the #276 reviewer on git 2.55).
**The test** re-runs `a_gzipped_document_is_walked` and
`every_shipped_document_loads` in child processes. Each child gets all six
family variables that write to their target on git 2.55, set to `/dev/stdout`:
`GIT_TRACE`, `GIT_TRACE_SETUP`, `GIT_TRACE_PERFORMANCE`, `GIT_TRACE2`,
`GIT_TRACE2_EVENT` and `GIT_TRACE2_PERF`. It also gets a `GIT_CONFIG_GLOBAL`
whose `[trace2]` section sends all three targets to `/dev/stdout`. There is one
child per test, not one per setting, to keep this binary's child count down
(improvise-eahe). The test failed at #269's head 4cef40c9 for both tests.
The first version set only three variables, so an exact list of those three
names passed it: the review of #276 found that. With all six set, an exact
three-name list leaves `GIT_TRACE_SETUP` behind and fails.
Rivals, each injected into `untraced_git` and restored. Every one fails:
- **no strip**;
- **the exact names `GIT_TRACE`, `GIT_TRACE2_EVENT`, `GIT_TRACE_PERFORMANCE`**:
this is the rival the review found passing;
- **the prefix minus `GIT_TRACE`**, **minus `GIT_TRACE_SETUP`**, and **minus
`GIT_TRACE_PERFORMANCE`**: each of the three variables that the `0` override
does not cover breaks the walk on its own;
- **no `0` override** (removal only): the config's targets break the walk.
The `GIT_TRACE2*` variables have no removal rival of their own. The `0` override
covers them whether or not they are removed.
---
## `tests/web_demo_model.rs`
tests/shipped_documents_load.rs
+109 -39
View File
@@ -48,25 +48,58 @@ fn every_shipped_document_loads() {
}
}
/// Settings that change how git reads a pathspec or finds attributes.
const PATHSPEC_AND_ATTRIBUTE_SETTINGS: [&str; 5] = [
"GIT_LITERAL_PATHSPECS",
"GIT_GLOB_PATHSPECS",
"GIT_NOGLOB_PATHSPECS",
"GIT_ICASE_PATHSPECS",
"GIT_ATTR_SOURCE",
];
/// `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")
let names = untraced_git()
.args(["rev-parse", "--local-env-vars"])
.output()
.expect("git runs");
let mut git = std::process::Command::new("git");
let mut git = untraced_git();
for name in String::from_utf8(names.stdout).expect("UTF-8").lines() {
git.env_remove(name);
}
// Process-wide, so `--local-env-vars` lists none of them (improvise-a0i3).
for name in PATHSPEC_AND_ATTRIBUTE_SETTINGS {
git.env_remove(name);
}
git.arg("-C").arg(root);
git
}
/// `git` with every `GIT_TRACE*` variable removed: one pointed at stdout writes
/// into what we parse. A prefix, since the family is open-ended (improvise-2iws).
fn untraced_git() -> std::process::Command {
let mut git = std::process::Command::new("git");
for (name, _) in std::env::vars_os() {
if name.to_string_lossy().starts_with("GIT_TRACE") {
git.env_remove(name);
}
}
// A config `trace2.*Target` survives a removal; `0` here outranks it (vav0).
for name in TRACE2_TARGETS {
git.env(name, "0");
}
git
}
/// The trace2 targets a config file can also set, as `trace2.normalTarget`,
/// `trace2.eventTarget` and `trace2.perfTarget`.
const TRACE2_TARGETS: [&str; 3] = ["GIT_TRACE2", "GIT_TRACE2_EVENT", "GIT_TRACE2_PERF"];
/// 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");
run_git(dir.path(), &["init", "-q"]);
run_git(dir.path(), &["init", "-q", "--template="]);
dir
}
@@ -74,6 +107,8 @@ fn run_git(root: &Path, args: &[&str]) {
let status = git(root)
.env("GIT_CONFIG_GLOBAL", "/dev/null")
.env("GIT_CONFIG_NOSYSTEM", "1")
.args(["-c", "core.excludesFile=/dev/null"])
.args(["-c", "core.attributesFile=/dev/null"])
.args(args)
.status()
.expect("git runs");
@@ -163,29 +198,84 @@ fn rerun_with_env(test: &str, env: &[(&str, &Path)]) -> std::process::Output {
let stdout = String::from_utf8_lossy(&out.stdout);
assert!(
stdout.contains("running 1 test"),
"{test} did not run: {stdout}"
"{test} did not run: {}",
told(&out)
);
out
}
/// improvise-53zz: a hook in a linked worktree exports `GIT_DIR`, which beats
/// `-C`. Evidence: `context/test-evidence/tui.md`.
/// Everything a child test said, both streams: its panic lands on stderr.
fn told(out: &std::process::Output) -> String {
let stdout = String::from_utf8_lossy(&out.stdout);
let stderr = String::from_utf8_lossy(&out.stderr);
format!("\n--- stdout\n{stdout}\n--- stderr\n{stderr}")
}
/// Every hostile git setting these tests have met, at once, in one child per
/// test (improvise-eahe). Each setting's own witness is a rival: `context/test-evidence/tui.md`.
#[test]
fn a_scratch_repo_ignores_an_inherited_git_dir() {
fn a_hostile_git_environment_changes_nothing() {
// improvise-pr3i: a template whose `info/exclude` refuses `*.gz`.
let templates = tempfile::tempdir().expect("a temp dir");
write(templates.path(), "info/exclude", "*.gz\n");
// improvise-53zz: a global and a system config whose excludes refuse `*.gz`.
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");
// vav0: trace2 targets go to stdout. 3j70: a bad sharedRepository kills `init`
// and `add` but not `ls-files`, and no `-c` in run_git overrides it.
let text = format!(
"[core]\n\texcludesFile = {}\n[trace2]\n\tnormalTarget = /dev/stdout\n\teventTarget = /dev/stdout\n\tperfTarget = /dev/stdout\n[core]\n\tsharedRepository = nonsense\n",
excludes.display()
);
std::fs::write(&config, text).expect("writes");
// improvise-vwjj: git's default ignore and attributes files, no config line.
let xdg = tempfile::tempdir().expect("a temp dir");
write(xdg.path(), "git/ignore", "*.gz\n");
write(
xdg.path(),
"git/attributes",
"*.gz working-tree-encoding=UTF-16\n",
);
// improvise-53zz: an inherited `GIT_DIR`, which beats `-C`.
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 index = dot_git.join("index");
let stdout = Path::new("/dev/stdout");
let one = Path::new("1");
let hostile: Vec<(&str, &Path)> = vec![
("GIT_TEMPLATE_DIR", templates.path()),
("GIT_CONFIG_GLOBAL", &config),
("GIT_CONFIG_SYSTEM", &config),
("XDG_CONFIG_HOME", xdg.path()),
("GIT_DIR", &dot_git),
("GIT_INDEX_FILE", &index),
// improvise-a0i3: process-wide pathspec and attribute settings.
("GIT_LITERAL_PATHSPECS", one),
("GIT_GLOB_PATHSPECS", one),
("GIT_NOGLOB_PATHSPECS", one),
("GIT_ICASE_PATHSPECS", one),
("GIT_ATTR_SOURCE", Path::new("HEAD")),
// improvise-2iws: every trace that writes to its target on git 2.55.
("GIT_TRACE", stdout),
("GIT_TRACE_SETUP", stdout),
("GIT_TRACE_PERFORMANCE", stdout),
("GIT_TRACE2", stdout),
("GIT_TRACE2_EVENT", stdout),
("GIT_TRACE2_PERF", stdout),
];
for test in [
"a_gzipped_document_is_walked",
"every_shipped_document_loads",
] {
let out = rerun_with_env(test, &hostile);
assert!(
out.status.success(),
"{test} fails in a hostile git environment: {}",
told(&out)
);
}
let staged = git(outer.path())
.args(["ls-files"])
.output()
@@ -196,23 +286,3 @@ fn a_scratch_repo_ignores_an_inherited_git_dir() {
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)
);
}
}