diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index 418a898c..96645003 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -87,7 +87,11 @@ 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, + as far as make's lines tell it (`MakeErrors` says where they do not); + 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 @@ -998,18 +1002,89 @@ 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); `*** 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; and `[Makefile:44: bootstrap] Error 2 (ignored)`, with no +#: `***`, where a `-` before a command lets it fail with a make's code +#: (#884). +MAKE_FAILED_RE = re.compile( + rb"(?:\[(\d+)\])?: (?:\*\*\* (?:\[[^\n]*\] Error (\d+)\s*$)?" + rb"|\[[^\n]*\] Error 2 \((ignored)\)\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`, its last match on a + line, since a member's own text can come before make's on one), 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` at a shallower depth than the last + `***` line before it, whatever lines come between, is taken for a + make's parent saying it failed, since make exits 2 for any failure, + and 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. Where a + `-` let that make fail (`Error 2 (ignored)`), the depth is the + parent's again, so an `Error 2` of the parent's own after it is named. + + WHERE THE CODE IS NOT THE INNERMOST COMMAND'S, since make's lines do + not tell it: + - a recipe that runs a make, lets its failure pass in the shell (`|| + true`, or `;`), or with a `-` in a make run with `-s`, which then + prints no `(ignored)` line, and exits 2 itself: its `Error 2` is + read as that make's report, and the inner command's code is named; + - an inner make run with `env -u MAKELEVEL`, which prints its parent's + depth: the parent's `Error 2` is named; + - under `-j`, a job of the parent's that fails while the inner make is + still running: its line comes between, and the parent's `Error 2` is + named; + - a member's own line shaped like a deeper make's (`x[3]: *** y`) + before it exits 2 itself: its `Error 2` is passed over.""" + + 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 its last `MAKE_FAILED_RE`.""" + found = MAKE_FAILED_RE.findall(line) + if not found: + return + depth_said, code, ignored = found[-1] + depth = int(depth_said or 0) + if ignored: + self.depth = min(self.depth, depth) + return + reports_a_make = code == b"2" and depth < self.depth + self.depth = depth + if code 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 @@ -1017,25 +1092,58 @@ 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: a command make runs with no shell, + on a line with nothing a shell must read; the shell a `kill -KILL $$` + kills; or a command bash runs in its own place, a line's last, a line + of one command included. Make then prints the signal's name, `Killed` + or `Terminated`, and no code. Where the shell outlives the command, as + dash, `/bin/sh` on Debian and Ubuntu, does for every command, and bash + for one that is not a line's last, the shell prints the signal's name + itself and exits 128 and the signal's number, 137 for SIGKILL and 143 + for SIGTERM, which make names as the member's command's code; where + the line goes on after it, the shell exits as the rest of the line + does (#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..e388cc42 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,276 @@ 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, with 3 or with 1. Lines +#: between the two, and a `***` line with no code after an `Error` line, +#: change neither; a make a `-` let fail leaves its parent's own `Error 2` +#: named, and one that ignores a failure of its own (without `-s`, which +#: keeps make from saying so) reports nothing; and +#: make's `Error` line after a member's own `: *** ` on one line is read +#: (rv884's X11, X7 and X12, and NIT 1 and NIT 2, on #886). +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), + "InnerThenFinding": (b"bootstrap:\n\t@$(MAKE) -s nothere || exit 1\n", + 1), + "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), + "KeepsGoing": (b"bootstrap:\n\t@$(MAKE) -s -k inner\ninner: one two\n" + b"one:\n\t@echo 'Find: a finding' >&2; exit 1\n" + b"two:\n\t@echo 'two: done' >&2\n", 1), + "ThenKilled": (b"bootstrap: one two\n" + b"one:\n\t@echo 'Find: a finding' >&2; exit 1\n" + b"two:\n\t@kill -KILL $$$$\n", 1), + "LetFail": (b"bootstrap:\n\t-@$(MAKE) -s inner\n" + b"\t@echo 'Refuse: refused' >&2; exit 2\n" + b"inner:\n\t@echo 'Find: a finding' >&2; exit 1\n", 2), + "InnerLets": (b"bootstrap:\n\t@$(MAKE) inner; exit 2\n" + b"inner:\n\t-@exit 2\n\t@echo 'inner: done'\n", 2), + "OwnStars": (b"bootstrap:\n\t@printf 'x: *** y' >&2; exit 1\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]:`, and + so are the cases `INNERMOST` names beside them. 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/