Merge pull request #244 from fiddlerwoaroof/tooling/lqs0-audit-exits
tooling: bd-audit-ids tells grep's no-match from a read error (lqs0)
This commit is contained in:
Ed Langley
GitHub
@@ -904,3 +904,75 @@ So the pass needs both the exclusions and git's file list.
|
||||
|
||||
After each rival the original was restored (`cmp` against a saved copy) and the
|
||||
suite re-run at 8 passed.
|
||||
|
||||
### The audit tells grep's no-match from its read error — `scripts/bd-audit-ids-test.sh` (improvise-lqs0)
|
||||
|
||||
**What broke.** `bd-audit-ids` treated xargs exit 123 as "a grep batch matched
|
||||
nothing". xargs also exits 123 when grep exits 2, so a cited file the scan could
|
||||
not read passed the audit, and any dead id inside it was missed. The usage text
|
||||
promised exit 4 for that. Two smaller mismatches with the stated contract:
|
||||
- run outside a git repository, the audit exited 1, which is the "dead id" code;
|
||||
- given a bd list whose objects carry no `id`, it reported every cited id dead
|
||||
(exit 1) where "bd did not answer" is 3.
|
||||
|
||||
**Reproduced before the fix, with six new cases:** 10 passed, 4 failed.
|
||||
- a dangling symlink among the cited files: exit 1, the read error ignored;
|
||||
- an unreadable (`chmod 000`) file holding the only dead id: **exit 0, "all
|
||||
resolve"**, which is the false pass;
|
||||
- outside a repository: exit 1;
|
||||
- id-less JSON: exit 1.
|
||||
|
||||
The control (both files readable, dead id found: exit 1) and the deleted-file
|
||||
case passed before and after.
|
||||
|
||||
**The fix:**
|
||||
- Each grep batch runs through `bash -c` and maps exit 1 to success and exit 2
|
||||
to 255. xargs stops on 255 with 124, so a read error is exit 4.
|
||||
- A tracked file deleted from the working tree is filtered out **on purpose**.
|
||||
Its ids are leaving the tree, which is the case the ticket asked to decide
|
||||
rather than leave to accident. A dangling symlink is still an entry, so it
|
||||
still counts as a read error.
|
||||
- Outside a repository is exit 4.
|
||||
- bd's output must be a JSON array whose every element has a string `id`, or
|
||||
the audit is exit 3.
|
||||
|
||||
**Rivals, each run against the suite (the script restored and checked with
|
||||
`cmp` after each):**
|
||||
- **read error mapped to success** (`-le 2`): fails both read-error cases, 12
|
||||
passed and 2 failed;
|
||||
- **the deleted-file filter removed**: fails "a tracked file deleted from the
|
||||
working tree is not scanned" with exit 4. So the decision is pinned, not
|
||||
incidental;
|
||||
- **the old `cd "$(git rev-parse --show-toplevel)"`**: fails "run outside a git
|
||||
repository is exit 4" with exit 1. A first attempt at this rival still exited
|
||||
4 and proved nothing; it was replaced;
|
||||
- **the id check reduced to `type == "array"`**: fails the id-less case with
|
||||
exit 1.
|
||||
|
||||
The `chmod 000` case is skipped when the suite runs as root, where the file
|
||||
stays readable. The dangling-symlink case covers the same path for every user.
|
||||
|
||||
### Absent is not deleted — `scripts/bd-audit-ids-test.sh` (improvise-tdfd)
|
||||
|
||||
**What broke.** The file filter `[ -e "$f" ] || [ -L "$f" ]` dropped every
|
||||
tracked path missing from the tree, and took each one to be a deletion. A path
|
||||
marked `--skip-worktree` (or left out by a sparse checkout) is absent too, but
|
||||
it is not deleted. Its ids were never read, and the audit passed. ChatGPT
|
||||
Codex found this on PR #244. The adversarial review reproduced it with
|
||||
`context/adr/001-concurrent-edits.md`, the only citation of one id: exit 1 → 0.
|
||||
|
||||
**Fix, Codex's suggestion.** An absent path is skipped only when
|
||||
`git ls-files -d` lists it. Any other absent tracked path is exit 4, and the
|
||||
path is named. The xargs comment no longer claims 124. That is GNU; BSD exits
|
||||
1. The script tests nonzero, so both work.
|
||||
|
||||
**Red first.** "a skip-worktree file missing from the tree is exit 4, and
|
||||
named" exited 0 against the pre-fix script, reporting "all 1 cited ids
|
||||
resolve".
|
||||
|
||||
**Rival:** every absent path reported as unread (the `ls-files -d` test
|
||||
removed). It fails "a tracked file deleted from the working tree is not
|
||||
scanned" alone, so the deleted-file case is what stops the fix from
|
||||
over-refusing. The rival ran from an ignored copy inside the worktree. A copy
|
||||
under `/tmp` fails "the real tree does not pass against a silent bd" for a
|
||||
reason unrelated to the rival: there is no repository there.
|
||||
|
||||
+38
-8
@@ -12,7 +12,9 @@ Run from anywhere inside the repository. Takes no arguments.
|
||||
exit 0 every cited id is a ticket bd lists
|
||||
exit 1 at least one cited id is not; each is printed
|
||||
exit 3 bd could not answer: missing, failing, or listing nothing
|
||||
exit 4 usage error, or the file scan itself failed
|
||||
exit 4 usage error, not inside a git repository, or a cited file
|
||||
could not be read or is absent (only a file `git ls-files -d`
|
||||
reports as deleted is skipped)
|
||||
|
||||
The comparison is POSITIVE: a cited id passes only by appearing in what
|
||||
`bd list --all` returns. The recipe this replaced looked for bd's "not found"
|
||||
@@ -36,15 +38,41 @@ EXCLUDE='^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|
|
||||
work=$(mktemp -d)
|
||||
trap 'rm -rf "$work"' EXIT
|
||||
|
||||
cd "$(git rev-parse --show-toplevel)"
|
||||
git ls-files -z --cached --others --exclude-standard -- "${ROOTS[@]}" > "$work/files"
|
||||
top=$(git rev-parse --show-toplevel 2>/dev/null) || {
|
||||
echo "bd-audit-ids: not inside a git repository, so nothing was checked." >&2
|
||||
exit 4
|
||||
}
|
||||
cd "$top"
|
||||
# A deleted tracked file is skipped ON PURPOSE: its ids are leaving the tree.
|
||||
# Only `ls-files -d` says deleted; any other absent path is exit 4 (lqs0, tdfd).
|
||||
: > "$work/absent"
|
||||
git ls-files -z --cached --others --exclude-standard -- "${ROOTS[@]}" |
|
||||
while IFS= read -r -d '' f; do
|
||||
if [ -e "$f" ] || [ -L "$f" ]; then printf '%s\0' "$f"
|
||||
elif [ -z "$(git --literal-pathspecs ls-files -d -- "$f")" ]; then
|
||||
printf '%s\n' "$f" >> "$work/absent"
|
||||
fi
|
||||
done > "$work/files"
|
||||
if [ -s "$work/absent" ]; then
|
||||
echo "bd-audit-ids: tracked but absent, and not deleted, so never read:" >&2
|
||||
sed 's/^/ /' "$work/absent" >&2
|
||||
exit 4
|
||||
fi
|
||||
|
||||
# xargs exits 123 when one grep batch matched nothing, which is not an error.
|
||||
# xargs reports grep's no-match (1) and its read error (2) alike, as 123, so
|
||||
# each batch maps 1 to success and 2 to 255: xargs stops, nonzero (GNU 124, BSD 1).
|
||||
: > "$work/raw"
|
||||
set +e
|
||||
xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' < "$work/files" > "$work/raw"
|
||||
if [ -s "$work/files" ]; then
|
||||
xargs -0 bash -c 'grep -d skip -ohE "$0" "$@"; [ $? -le 1 ] || exit 255' \
|
||||
'improvise-[a-z0-9]{3,8}\b' < "$work/files" > "$work/raw"
|
||||
fi
|
||||
rc=$?
|
||||
set -e
|
||||
[ "$rc" -eq 0 ] || [ "$rc" -eq 123 ] || { echo "bd-audit-ids: grep failed ($rc)" >&2; exit 4; }
|
||||
[ "$rc" -eq 0 ] || {
|
||||
echo "bd-audit-ids: a cited file could not be read (xargs $rc), so the audit is incomplete." >&2
|
||||
exit 4
|
||||
}
|
||||
sort -u "$work/raw" | { grep -vE "$EXCLUDE" || true; } > "$work/cited"
|
||||
|
||||
if ! command -v "$BD" >/dev/null 2>&1; then
|
||||
@@ -56,10 +84,12 @@ if ! "$BD" list --all --limit 0 --json > "$work/known.json" 2> "$work/bd.err"; t
|
||||
cat "$work/bd.err" >&2
|
||||
exit 3
|
||||
fi
|
||||
if ! jq -r '.[].id' "$work/known.json" 2>/dev/null | sort -u > "$work/known"; then
|
||||
echo "bd-audit-ids: '$BD list' did not print a JSON list of issues." >&2
|
||||
if ! jq -e 'type == "array" and all(.[]; (.id | type) == "string")' \
|
||||
"$work/known.json" > /dev/null 2>&1; then
|
||||
echo "bd-audit-ids: '$BD list' did not print a JSON list of issues with ids." >&2
|
||||
exit 3
|
||||
fi
|
||||
jq -r '.[].id' "$work/known.json" | sort -u > "$work/known"
|
||||
if [ ! -s "$work/known" ]; then
|
||||
echo "bd-audit-ids: '$BD list' returned no issues; refusing to call that a pass." >&2
|
||||
exit 3
|
||||
|
||||
@@ -77,6 +77,72 @@ got=$?
|
||||
[ "$got" = 3 ] && ok "the real tree does not pass against a silent bd" \
|
||||
|| bad "the real tree does not pass against a silent bd" "exit $got"
|
||||
|
||||
# improvise-lqs0: a file the scan cannot read, and the two exit-code mismatches.
|
||||
# A second repo, so the file list is exactly these cases' files.
|
||||
r2="$work/repo2"
|
||||
mkdir -p "$r2/context"
|
||||
git -C "$r2" init -q
|
||||
printf 'improvise-ab12\n' > "$r2/context/a.md"
|
||||
printf 'improvise-dead9\n' > "$r2/context/b.md"
|
||||
git -C "$r2" add -A
|
||||
git -C "$r2" -c user.name=t -c user.email=t@t commit -qm fixture
|
||||
export FAKE_BD_OUT="$work/known.json" FAKE_BD_RC=0
|
||||
|
||||
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 1 ] && grep -q 'improvise-dead9' "$work/out" \
|
||||
&& ok "control: both files readable, the dead id is found" \
|
||||
|| bad "control: both files readable, the dead id is found" "exit $got"
|
||||
|
||||
# A symlink whose target is gone reads as an error for any user, root included.
|
||||
ln -s missing-target "$r2/context/c.md"
|
||||
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 4 ] && ok "a cited file grep cannot read is exit 4, not a pass" \
|
||||
|| bad "a cited file grep cannot read is exit 4, not a pass" "exit $got: $(head -c 200 "$work/out")"
|
||||
rm -f "$r2/context/c.md"
|
||||
|
||||
if [ "$(id -u)" != 0 ]; then
|
||||
chmod 000 "$r2/context/b.md"
|
||||
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
chmod 644 "$r2/context/b.md"
|
||||
[ "$got" = 4 ] && ok "an unreadable cited file is exit 4, not a pass" \
|
||||
|| bad "an unreadable cited file is exit 4, not a pass" "exit $got: $(head -c 200 "$work/out")"
|
||||
fi
|
||||
|
||||
# Decided, not accidental: a tracked file deleted from the working tree is not
|
||||
# scanned. Its ids are leaving the tree, so its dead id is not reported.
|
||||
mv "$r2/context/b.md" "$work/b.md.away"
|
||||
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
mv "$work/b.md.away" "$r2/context/b.md"
|
||||
[ "$got" = 0 ] && ok "a tracked file deleted from the working tree is not scanned" \
|
||||
|| bad "a tracked file deleted from the working tree is not scanned" "exit $got: $(head -c 200 "$work/out")"
|
||||
|
||||
# Absent is not deleted: a skip-worktree path is missing too, but ls-files -d
|
||||
# does not list it, so its ids were never read (improvise-tdfd).
|
||||
git -C "$r2" update-index --skip-worktree context/b.md
|
||||
mv "$r2/context/b.md" "$work/b.md.away"
|
||||
(cd "$r2" && "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
mv "$work/b.md.away" "$r2/context/b.md"
|
||||
git -C "$r2" update-index --no-skip-worktree context/b.md
|
||||
[ "$got" = 4 ] && grep -q 'context/b.md' "$work/out" \
|
||||
&& ok "a skip-worktree file missing from the tree is exit 4, and named" \
|
||||
|| bad "a skip-worktree file missing from the tree is exit 4, and named" "exit $got: $(head -c 200 "$work/out")"
|
||||
|
||||
mkdir -p "$work/not-a-repo"
|
||||
(cd "$work/not-a-repo" && GIT_CEILING_DIRECTORIES="$work" "$AUDIT") > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 4 ] && ok "run outside a git repository is exit 4" \
|
||||
|| bad "run outside a git repository is exit 4" "exit $got"
|
||||
|
||||
printf '[{"title":"no id here"}]\n' > "$work/noid.json"
|
||||
FAKE_BD_OUT="$work/noid.json" \
|
||||
run_case "a bd listing issues with no id is exit 3, not every id dead" 3 'did not print'
|
||||
unset FAKE_BD_OUT FAKE_BD_RC
|
||||
|
||||
"$AUDIT" --help > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 4 ] && grep -q 'POSITIVE' "$work/out" \
|
||||
|
||||
Reference in New Issue
Block a user