From 82a84f88b66fa4957bc380a96b81efb94b4358b1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 09:45:43 +0000 Subject: [PATCH 1/5] the family root's bootstrap.py: make's stderr is read a line at a time and the innermost command named, so a member whose make runs another no longer reads `exited 2` for a finding, and `make -k` keeps its `Error` line (#884) #884's three lines, from rv876's NIT 2, NIT 3 and NIT 4 on #876, in the family holder's pinned `templates/family-root/scripts/bootstrap.py` and `tests/test_family_holder_bootstrap_texts.py`. Line 1, tests only. A test now holds each of rv876's five surviving mutants: R10 and R11 (`credential_advice` ignoring `unreadable` or `refused`), in the two states where every member is checked out and git read nothing; R1 (the first `Error` line read) and R8 (the line read only as `make:`), under a real parent `make -k`; R7 (`passed_on`'s flush dropped), on a pseudo-terminal, where the member's question must show before the answer is typed. The `git mv` test asserts the store its docstring names. Line 2. `run_make` read the last `Error` line in the last 4096 bytes of make's stderr. `MakeErrors` now reads each line as it passes, so an `Error` line followed by more than 4096 bytes (`make -k`) is no longer said to be missing. An `Error 2` that follows straight on a deeper make's `***` line is that make's parent reporting it, and is passed over: a member whose make runs `$(MAKE) inner` is named by the inner command's code (1, a finding), not by make's 2, and an inner make's own failure names none. The docstring now says what a signal does: make prints its name where the process make started is killed, and names the shell's 128+n where the shell, as dash does, outlives the command. Line 3, docstring. Passing make's stderr on takes the terminal from the member's stderr, can reorder its stdout and stderr lines, lets a process left holding stderr hold the run, and passes the bytes on undecoded. `run_make`'s docstring says each. Reading only until make exits was measured: the run ends at once, but a process that writes to stderr afterwards dies of SIGPIPE, unsaid. Giving make the terminal back would lose the `Error` line, which nothing else make writes carries. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- templates/family-root/scripts/bootstrap.py | 125 +++++-- tests/test_family_holder_bootstrap_texts.py | 353 ++++++++++++++++++-- 2 files changed, 431 insertions(+), 47 deletions(-) diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index aec82b91..0f4db2e8 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -79,7 +79,10 @@ fails is named with that command's exit code, read from make's `Error ` line, since make exits 2 for any command that fails, so a member's finding (1) and its refusal (2) are told apart, and - from a failure of make's own, which prints no such line (#839). + from a failure of make's own, which prints no such line (#839). Where + the member's make runs another, the code is the innermost command's; + make's stderr reaches the terminal through this run, and `run_make` + says what that changes for a member (#884). (c) EXITS 0 unless a member's own command failed, or (a) left a member not checked out that no credential would check out, or found a line or a @@ -870,18 +873,64 @@ def run_member_target(root: Path, row: dict, target: str) -> str | None: "message above says what failed)") -#: The line GNU make prints where a command of the target exits non-zero, -#: `make: *** [Makefile:44: bootstrap] Error 1` (`make[1]:` in a make run by -#: make, `*** [bootstrap] Error 1` before make 4.0): the command's own exit -#: code, which make's does not carry, since make exits 2 for any command -#: that fails (#839). -MAKE_ERROR_RE = re.compile(rb"\*\*\* \[[^\n]*\] Error (\d+)\s*$") +#: What GNU make prints on stderr as a target fails, after its own name +#: and, in a make run by make, its depth (`make[1]:`): `*** [Makefile:44: +#: bootstrap] Error 1` where a command exits non-zero (`*** [bootstrap] +#: Error 1` before make 4.0), with the command's own exit code, which +#: make's does not carry, since make exits 2 for any command that fails +#: (#839); and `*** No rule to make target ...`, or `*** [Makefile:44: +#: bootstrap] Killed`, where make fails on its own or a signal ends what it +#: ran, with none (#884). +MAKE_FAILED_RE = re.compile( + rb"(?:\[(\d+)\])?: \*\*\* (?:\[[^\n]*\] Error (\d+)\s*$)?") +#: The bytes `MakeErrors` keeps of a line of make's stderr not yet ended: +#: an `Error ` line ends there, so the end of a line is enough. +LINE_KEPT = 4096 + + +class MakeErrors: + """What `run_make` reads of make's stderr, one line at a time as it + passes (#884): `code`, that of the last `Error ` line make printed + for a command of the member's (`MAKE_FAILED_RE`), or None. + + Read as it passes, an `Error` line is never lost to what follows it, + as one was to more than 4096 bytes after it (`make -k`) where only the + last 4096 were kept. An `Error 2` straight after a `***` line of a + deeper make is that make's parent saying it failed, since make exits 2 + for any failure, so it is passed over: where the member's make runs + another (`$(MAKE) inner`), the code is the innermost command's, and + where the inner make failed on its own there is none, as for make's + own failure.""" + + def __init__(self) -> None: + self.code: int | None = None + self.depth = -1 + self.unended = b"" + + def read(self, chunk: bytes) -> None: + """`chunk` of make's stderr: each line it ends, read + (`said`).""" + *ended, unended = (self.unended + chunk).split(b"\n") + for line in ended: + self.said(line) + self.unended = unended[-LINE_KEPT:] + + def said(self, line: bytes) -> None: + """One line of make's stderr, read for `MAKE_FAILED_RE`.""" + found = MAKE_FAILED_RE.search(line) + if found is None: + return + depth, code = int(found.group(1) or 0), found.group(2) + reports_a_make = code == b"2" and depth < self.depth + self.depth = depth + if code is not None and not reports_a_make: + self.code = int(code) def run_make(make: str, target: str, cwd: Path) -> tuple[int, int | None]: """(b) `make ` in a member, as `(make's exit code, the exit code - of the member's command it names)`, the second None where make printed - no `MAKE_ERROR_RE` line (#839). + of the member's command it names)`, the second None where make names + none (`MakeErrors`, #839, #884). MAKE'S OWN CODE TELLS NOTHING APART. GNU make exits 2 for any command that fails, measured (GNU Make 4.3) for a command exiting 1, 2, 3 or @@ -889,25 +938,55 @@ def run_make(make: str, target: str, cwd: Path) -> tuple[int, int | None]: line said `make bootstrap exited 2` for the member's own `bootstrap FAILED` (1, a finding), for a refusal (2) and for make's own failure alike (w764b, for lane 4). The command's code is the one make names - in its `Error ` line, the last on stderr, so make's stderr is - passed on to this run's own as it comes and that line read from the - end of it; a make that prints none, one of its own failures, a command - killed by a signal, or a make speaking another language, leaves make's - code alone. The member's command is never run around make: another - target than `bootstrap` (`--members-make validate`) has no script of - its own to call, as `make siblings` calls a member's - `scripts/bootstrap.py`.""" + in its `Error ` line, so make's stderr is passed on to this + run's own as it comes, and read a line at a time as it passes: the + last such line names it, or, where the member's make runs another, + the innermost command's line (`MakeErrors`). A make that prints none, + one of its own failures, or a make speaking another language, leaves + make's code alone. So does a command killed by a signal where what + make started is what was killed: the command itself, the shell a + `kill -KILL $$` kills, or bash, which runs a recipe line of one + command in its own place; make then prints `Killed` or `Terminated` + for it, and no code. Where the shell outlives the command, as dash, + `/bin/sh` on Debian and Ubuntu, does for that same line, the shell + prints `Killed` itself and exits 128 and the signal's number, 137 for + SIGKILL and 143 for SIGTERM, and make names that, as the member's + command's code (#884). The member's command is never run around make: + another target than `bootstrap` (`--members-make validate`) has no + script of its own to call, as `make siblings` calls a member's + `scripts/bootstrap.py`. + + WHAT PASSING STDERR ON CHANGES (#884), measured against a make that + writes to this run's stderr, a terminal, itself: + - The member's commands write stderr to a pipe, not a terminal, so a + test of `isatty(2)` reads false: git shows no progress meter, and a + command that colours or prompts by that test does neither. Stdin + and stdout stay this run's own. + - The member's stdout and stderr lines can come out of order: its + stderr passes through this run, a little after the stdout written + beside it. + - A process a command leaves running with stderr open holds the run + until it closes it or exits, since the pipe ends only then, and what + it writes until then is passed on. Reading only until make exits + would end the run at once, but that process's next write to stderr + would meet a closed pipe and end it (SIGPIPE), unsaid. A recipe that + starts one gives it a stderr of its own (`2>log`). + - The bytes pass on as written, never decoded. On Windows a member's + Python writes a pipe in the ANSI code page, and this run's stderr, a + console, reads them as UTF-8, so non-ASCII text comes out garbled + (read, not run). + Giving make the terminal back would lose its `Error ` line, + which nothing else make writes carries (`--debug=j` says only + "Reaping losing child"), and the line would say `exited 2` for every + failure again.""" proc = subprocess.Popen([make, target], cwd=str(cwd), stderr=subprocess.PIPE) - tail = b"" + errors = MakeErrors() for chunk in iter(lambda: proc.stderr.read1(65536), b""): passed_on(chunk) - tail = (tail + chunk)[-4096:] + errors.read(chunk) proc.stderr.close() - returncode = proc.wait() - found = [MAKE_ERROR_RE.search(line) for line in tail.splitlines()] - named = [int(match.group(1)) for match in found if match] - return returncode, (named[-1] if named else None) + return proc.wait(), errors.code def passed_on(chunk: bytes) -> None: diff --git a/tests/test_family_holder_bootstrap_texts.py b/tests/test_family_holder_bootstrap_texts.py index 2faa6988..d2329893 100644 --- a/tests/test_family_holder_bootstrap_texts.py +++ b/tests/test_family_holder_bootstrap_texts.py @@ -38,6 +38,25 @@ which a second `member-gitmodules-url-absent` did over the url the first had written. +#884, from rv876's review of #876, the change that made the lines above +true: + +- Five mutants of line 1's and line 2's code passed every test: the + advice where every member was checked out and git read nothing (R10, + R11), the first `Error` line read for the last (R1), the line read only + from a make run by no make (R8), and the prompt a member writes to + stderr held back until its answer was typed (R7). A test holds each + now, and line 3's test asserts the store its docstring names. +- Line 2's line misled in three cases: a member whose make runs another + read `exited 2` for its inner command's 1; under `make -k`, an `Error` + line followed by more than 4096 bytes was said not to be there; and a + command a signal killed under dash read `exited 137`, which + `run_make`'s docstring said could not happen. It names the innermost + command's code, reads each line as it passes, and its docstring says + what a signal does. +- Passing make's stderr on changes what a member's commands and a person + see: `run_make`'s docstring says what, and the tests hold what it says. + THE SAME FAMILY, OFFLINE RULES AND HELPERS as `tests/test_family_bootstrap_gate_order.py` (#648) and `tests/test_validate_family_remedies_typed.py` (#681). A missing credential @@ -50,9 +69,12 @@ import os import re +import select +import shlex import shutil import subprocess import sys +import time from pathlib import Path import pytest @@ -283,6 +305,41 @@ def test_beside_a_denial_the_advice_is_for_the_members_it_does_not_name( failed_on(OTHER)] +#: `(an edit to HEAD's .gitmodules, why git updated no member, what cures +#: it)`: a value git refuses, and a line it cannot parse (#768). +GIT_READ_NOTHING = { + "value-refused": (with_line("update = None"), + refused_why("update", "None"), value_cure("update")), + "unparsed": (lambda data: data + DAMAGED["conflict-marker"], UNPARSED, + MEND_CURE), +} + + +@pytest.mark.parametrize("edit, why, cure", GIT_READ_NOTHING.values(), + ids=GIT_READ_NOTHING) +def test_no_advice_beside_no_member_updated_with_every_member_checked_out( + holder, offline, tmp_path, edit, why, cure): + """LINE 1 with every member checked out, as in the holder itself, and + the edit committed: git dies reading `.gitmodules`, so it fetched + nothing, and the line says no member was updated, and not for want of + a credential. No row is left to deny, so nothing else keeps the advice + off the line before (rv876's R10 and R11, which dropped `unreadable` + or `refused` from `credential_advice` and printed it there, as + `tests/test_family_gitmodules_values.py`, asking only the last two + lines, let pass). Bootstrap's own lines are the fetch's, that one and + the failure, exactly.""" + env = isolated(offline, tmp_path) + data = raw(holder, env, "show", "HEAD:.gitmodules").stdout + committed_gitmodules(holder, env, edit(data)) + + result = bootstrap(holder, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert after_fetch_line(result) == [ + f" NO MEMBER UPDATED, and not for want of a credential: {why}. " + f"{cure}", "family bootstrap FAILED: .gitmodules (git cannot read it)"] + + # --- line 2: a member's finding, its refusal, and make's own failure -------- #: `{project: its Makefile}`, every recipe for both targets (#839's line 2). @@ -294,6 +351,19 @@ def test_beside_a_denial_the_advice_is_for_the_members_it_does_not_name( } +def with_members(holder: Path, makefiles: dict[str, bytes]) -> None: + """`holder`'s `family.yaml` naming one member for each of `makefiles`, + `{project: its Makefile}`, at `members/`, checked out there + as far as bootstrap asks: a `.git` and that Makefile.""" + (holder / "family.yaml").write_bytes( + b"name: InkRouter\nmembers:\n" + b"".join( + f" - project: {project}\n path: members/{project}\n" + .encode("ascii") for project in makefiles)) + for project, makefile in makefiles.items(): + (holder / "members" / project / ".git").mkdir(parents=True) + (holder / "members" / project / "Makefile").write_bytes(makefile) + + @pytest.mark.parametrize("target", ["bootstrap", "validate"]) def test_a_finding_a_refusal_and_make_s_own_failure_are_told_apart( holder, tmp_path, target): @@ -308,13 +378,7 @@ def test_a_finding_a_refusal_and_make_s_own_failure_are_told_apart( its own to call, the same.""" if shutil.which("make") is None: pytest.skip("no make on PATH") - (holder / "family.yaml").write_bytes( - b"name: InkRouter\nmembers:\n" + b"".join( - f" - project: {project}\n path: members/{project}\n" - .encode("ascii") for project in MAKEFILES)) - for project, makefile in MAKEFILES.items(): - (holder / "members" / project / ".git").mkdir(parents=True) - (holder / "members" / project / "Makefile").write_bytes(makefile) + with_members(holder, MAKEFILES) result = run_script(holder / "scripts" / "bootstrap.py", "--no-fetch", "--members-make", target, cwd=holder, @@ -322,18 +386,255 @@ def test_a_finding_a_refusal_and_make_s_own_failure_are_told_apart( assert result.returncode == 1, result.stdout + result.stderr errors = result.stderr.splitlines() - assert errors[-1] == ( - f"family bootstrap FAILED: Find (make {target}: the member's command " - f"exited 1), Refuse (make {target}: the member's command exited 2), " - f"NoRule (make {target} exited 2 and printed no `Error ` line " - "for a command of the member's: make's own message above says what " - "failed)"), result.stderr + assert errors[-1] == failed_in_make( + exited("Find", 1, target), exited("Refuse", 2, target), + named_none("NoRule", target)), result.stderr for said in ("Find: a finding", "Refuse: refused", f"No rule to make target '{target}'"): assert any(said in line for line in errors[:-1]), (said, errors) assert f" --- Fine: make {target} ---" in result.stdout.splitlines() +def exited(project: str, code: int, target: str = "bootstrap") -> str: + """What the line says of a member whose command make names.""" + return f"{project} (make {target}: the member's command exited {code})" + + +def named_none(project: str, target: str = "bootstrap") -> str: + """What the line says of a member whose make named no command's code.""" + return (f"{project} (make {target} exited 2 and printed no `Error " + "` line for a command of the member's: make's own message " + "above says what failed)") + + +def failed_in_make(*said: str) -> str: + """The run's last line, naming each member's failure as `said`.""" + return "family bootstrap FAILED: " + ", ".join(said) + + +def without_make() -> dict: + """This process's environment without the `MAKE`, `MAKEFLAGS`, + `MAKELEVEL` and `MFLAGS` a make running the tests would give it, so a + run starts as it does from a shell.""" + return {key: value for key, value in os.environ.items() + if not key.startswith(("MAKE", "MFLAGS"))} + + +def made(holder: Path, tmp_path: Path, *, + under_make: bool) -> subprocess.CompletedProcess: + """`bootstrap.py --no-fetch` in `holder`, every member's make with + `-k`, its output as bytes: run as it is, or run by a make of its own + (`under_make`), as the holder's own `make bootstrap` runs it, so that + the member's make is `make[1]` and prints that.""" + env = without_make() + argv = [sys.executable, "scripts/bootstrap.py", "--no-fetch"] + if under_make: + parent = tmp_path / "parent.mk" + parent.write_bytes(f"run:\n\t@{shlex.join(argv)}\n".encode()) + argv = ["make", "-k", "-f", str(parent)] + else: + env["MAKEFLAGS"] = "k" + return subprocess.run(argv, cwd=str(holder), env=env, + stdin=subprocess.DEVNULL, capture_output=True, + check=False) + + +#: Members whose make runs another make, or whose `Error` line is followed +#: by more than 4096 bytes on stderr, each with what the line says of it: +#: the innermost command's code, read from each line as it passes (#884's +#: line 2). An inner make that fails on its own names none, as make's own +#: failure does, but its parent's `Error 2` is passed over only: a command +#: that runs one and then fails itself is named. +INNERMOST = { + "Recurse": (b"bootstrap:\n\t@$(MAKE) -s inner\n" + b"inner:\n\t@echo 'Find: a finding' >&2; exit 1\n", 1), + "RecurseRefuse": (b"bootstrap:\n\t@$(MAKE) -s inner\n" + b"inner:\n\t@echo 'Refuse: refused' >&2; exit 2\n", 2), + "InnerOwn": (b"bootstrap:\n\t@$(MAKE) -s nothere\n", None), + "InnerThenOwn": (b"bootstrap:\n\t@$(MAKE) -s nothere; exit 3\n", 3), + "Long": (b"bootstrap: one two\n" + b"one:\n\t@echo 'Find: a finding' >&2; exit 1\n" + b"two:\n\t@printf '%05000d' 0 >&2; echo >&2\n", 1), +} +#: A member whose two commands fail, under `-k`: the last `Error` line +#: names the code (rv876's R1), and a member's finding, both under a +#: parent make, where make prints `make[1]:` (R8). +LAST = {"Find": (MAKEFILES["Find"], 1), + "Twice": (b"bootstrap: one two\n" + b"one:\n\t@echo 'one: a finding' >&2; exit 1\n" + b"two:\n\t@echo 'two: refused' >&2; exit 2\n", 2)} +#: A command a signal kills: where the shell outlives it, as dash does, +#: the shell exits 137, which make names; where it is the shell that is +#: killed, make prints `Killed` in place of a code (`run_make`'s +#: docstring). +SIGNALLED = {"Outlived": (b"bootstrap:\n\t@sh -c 'kill -KILL $$$$'; " + b"exit $$?\n", 137), + "Killed": (b"bootstrap:\n\t@kill -KILL $$$$\n", None)} +#: `(run by a make of its own?, {project: (its Makefile, the code the line +#: names, None for none)})`. +MAKE_CASES = {"last-under-make": (True, LAST), + "innermost-under-make": (True, INNERMOST), + "innermost": (False, INNERMOST), + "signal": (False, SIGNALLED)} + + +@pytest.mark.parametrize("under_make, members", MAKE_CASES.values(), + ids=MAKE_CASES) +def test_the_line_names_the_code_of_the_member_s_innermost_command( + holder, tmp_path, under_make, members): + """#884's LINE 2, and rv876's R1 and R8. At #876's head a member whose + make ran `$(MAKE) inner` read `exited 2` for the inner command's 1, a + refusal's code, since make's own `Error 2` for the inner make came + last; and under `make -k`, an `Error 1` followed by 5000 bytes read + "printed no `Error ` line", since only the last 4096 bytes were + read. Each is named now, run as it is and under a parent make, where + the member's make says `make[1]:` and the make it runs `make[2]:`. The + last `Error` line of a member's names its code, and a signal's code is + its shell's, where the shell outlives it, as `run_make` now says.""" + if shutil.which("make") is None: + pytest.skip("no make on PATH") + with_members(holder, {project: makefile + for project, (makefile, _) in members.items()}) + + result = made(holder, tmp_path, under_make=under_make) + + errors = result.stderr.decode("utf-8", "replace").splitlines() + assert result.returncode == (2 if under_make else 1), errors + assert [line for line in errors if line.startswith("family ")] == [ + failed_in_make(*(named_none(project) if code is None + else exited(project, code) + for project, (_, code) in members.items()))] + if under_make: + assert errors[-1].startswith("make: *** "), errors + assert not [line for line in errors[:-1] + if line.startswith("make: ")], errors + + +def loaded(script: str, name: str): + """The family holder's `scripts/