From a2b0e48c92c64f61ce16233f1dfc8cfd2577f563 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 20:51:44 +0000 Subject: [PATCH 1/3] shape-doctor.py: the `legs`, `manifest kinds` and `shape currency` rows name a pin or `contracts/` this user cannot read as validate-pins.py does, with its advice and never `git checkout HEAD --`, and the verdict is INVALID, not DRIFTED (#847) #712 (PR #827) makes validate-pins.py refuse a pin it cannot reach or open by name: `REFUSED yaml-unreadable: contracts/spec-pin.yaml: cannot be read (Permission denied)`, with a remedy for the cause. Three of the doctor's rows read the same files themselves and said something else. Measured on main fc867207, Python 3.12.3, as a user who is not root: - `legs`, through `pin_commit`: `spec: contracts/spec-pin.yaml carries no 40-hex commit to compare HEAD against`, and the verdict counted the leg as off its pin, `DRIFTED (1 leg(s) not at the pin)`, for a pin at 000 and `DRIFTED (2 ...)` for a `contracts/` at 000 or `r--`. - `manifest kinds`: for a pin at 000, `unreadable as YAML: ... ([Errno 13] Permission denied: '/...')` with NEXT `git -C checkout HEAD -- contracts/spec-pin.yaml`, which discards what the file holds that no commit does (`git status` calls a file it cannot open changed); for a `contracts/` at 000, `could not be run: [Errno 13] Permission denied: '/contracts'`. - `shape currency`: `could not be run: [Errno 13] ...` for a `contracts/` at 000 or `r--` and for a pinned copy at 000, and the same `git checkout HEAD --` for a shape pin at 000. Now each row asks as the gate asks (the `stat` first, then the read, the absent errnos read as the gate reads them) and says the gate's line: - `legs`: `spec: contracts/spec-pin.yaml: cannot be read (Permission denied), so there is no pinned commit to compare HEAD against`, kept in the leg's `cannot_be_read`; `drifted_reasons` does not count such a leg, so the verdict is the gate's `INVALID (pins, manifest-kinds)`. - `manifest kinds`: `contracts/spec-pin.yaml: cannot be read (Permission denied)`, or `contracts: cannot be listed (Permission denied)` for a folder it cannot list (the gate's words for a directory, #434), first among the row's faults, under `detail`'s `cannot_be_read`. - `shape currency`: `the copies cannot be compared: : cannot be read (Permission denied)`, for the shape pin (asked before `read_pin`, whose `is_file()` calls it missing on Python 3.14) and for a copy `prepare` cannot read inside the root. Each NEXT is this doctor again and each why the gate's advice for the cause, word for word (`Make what this user may not open readable by this user: ...`, or `Where the operating system gave another reason, ...`), then `Then run this doctor again`. A pin that is not there keeps main's words and is drift as before. The `pins` row already quotes the gate and is left alone. `Context.legs` and the readers of `legs:` are untouched. Tests: tests/test_doctor_unreadable_pin.py, 15 cases. Five run a real mode (a pin, `contracts/` at 000 and `r--`, the shape pin, a copy) and skip on Windows and as root; ten stand in for a refused `stat` (`refused_stat`, as Python 3.14 answers it), read or listing in process, on every platform and as root. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 345 ++++++++++++++++++++++-- tests/test_doctor_unreadable_pin.py | 405 ++++++++++++++++++++++++++++ 2 files changed, 729 insertions(+), 21 deletions(-) create mode 100644 tests/test_doctor_unreadable_pin.py diff --git a/shape-doctor.py b/shape-doctor.py index b7099555..33b303c4 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -342,6 +342,7 @@ import argparse import datetime as _dt +import errno import importlib.util import json import os @@ -2866,20 +2867,193 @@ def check_family(ctx: Context) -> Row: ["--root", str(ctx.root)]) -def manifest_candidates(root: Path) -> list[Path]: - """Every file in a root that might DECLARE a `kind:`. +# --------------------------------------------------------------------------- +# A file or folder this user cannot read (#847) +# --------------------------------------------------------------------------- +# +# `validate-pins.py` refuses a pin, the manifest or a pinned copy that the +# operating system will not let it reach or open BY NAME, since #712: +# `REFUSED yaml-unreadable: contracts/spec-pin.yaml: cannot be read +# (Permission denied)`, with a remedy for the cause. Three rows here read the +# same files themselves, and said something else: the `legs` row that the pin +# `carries no 40-hex commit`, which the verdict read as DRIFTED; `manifest +# kinds` and `shape currency` a bare `[Errno 13]`, or, for a pin at mode 000, +# a `git checkout HEAD --` that discards what the file holds. They now ask as +# the gate asks, and say what it says. + +#: The `OSError`s that mean NOTHING IS THERE to read, where any other is a +#: read the operating system refused: no such file, a part of the path that +#: is not a directory, a bad descriptor, a symlink loop, and Windows' "not +#: ready", "invalid name" and "cannot resolve". They are the ones +#: `Path.is_file()` answered False to on Python 3.9 to 3.13, and +#: `validate-pins.py` and `bump-leg.py` read them so (`_ABSENT_ERRNOS`, +#: #434, #712): read the same way here, a pin the gate calls missing is one +#: these rows call missing, and one it refuses by name they name. +ABSENT_ERRNOS = (errno.ENOENT, errno.ENOTDIR, errno.EBADF, errno.ELOOP) +ABSENT_WINERRORS = (21, 123, 1921) + + +def refused_read(exc: BaseException | None) -> OSError | None: + """`exc` where it is an `OSError` with which the operating system + REFUSED a `stat`, a read or a listing, else None: no `OSError` at all, or + one that says nothing is there (`ABSENT_ERRNOS`) (#847). + + Asked of a `Refusal`'s `__cause__` too: `load_yaml` raises + `yaml-unreadable` from the `OSError` of a file it could not open, which + is how `validate-pins.py` tells that file from one that does not parse + (`_load`, #712), whatever words the root's `repo_shape` gives it.""" + if not isinstance(exc, OSError): + return None + if (exc.errno in ABSENT_ERRNOS + or getattr(exc, "winerror", None) in ABSENT_WINERRORS): + return None + return exc + + +def is_a(path: Path, kind) -> bool: + """Whether `path` is there and of `kind` (`stat.S_ISREG`, `stat.S_ISDIR`), + raising the `OSError` of a `stat` the operating system refused: the + question `validate-pins.py`'s `_is_a` asks (#434, #847). + + NOT `Path.is_file()` OR `Path.is_dir()`, whose answer depends on the + Python. Up to 3.13 they raised a refused `stat`, which ended a row in + `could not be run: [Errno 13] ...`; from 3.14 they answer False to any + `OSError`, which would call a pin in a `contracts/` this user cannot + search a pin that is not there. A name the operating system cannot take + at all is nothing there, as `is_file()` answered it.""" + try: + mode = path.stat().st_mode + except OSError as exc: + if refused_read(exc) is None: + return False + raise + except ValueError: + return False + return kind(mode) + + +def os_reason(exc: OSError) -> str: + """The operating system's own reason for `exc` (`Permission denied`), or + the exception's name where it gives none, as `validate-pins.py` names + one (#712).""" + return exc.strerror or type(exc).__name__ + + +def cannot_be_read(named: str, exc: OSError) -> str: + """`: cannot be read ()`: the line + `validate-pins.py` refuses a file it cannot reach or open with + (`_cannot_read`, #712), so a row and the gate say one sentence (#847).""" + return f"{named}: cannot be read ({os_reason(exc)})" + + +def cannot_be_listed(named: str, exc: OSError) -> str: + """`: cannot be listed ()`: the + line `validate-pins.py` refuses a directory it cannot list with (#434), + for the `contracts/` the `manifest kinds` row lists (#847).""" + return f"{named}: cannot be listed ({os_reason(exc)})" + + +#: What a refused PERMISSION is told, in `validate-pins.py`'s words for the +#: same file (`_unreadable_file`, #712): the mode or the owner puts it right. +PERMISSION_ADVICE = ( + "Make what this user may not open readable by this user: `chmod u+r` on " + "a file, `chmod u+rx` on a directory (and on every directory above it " + "inside the root), or fix who owns it.") + +#: What any OTHER refusal is told, in the same gate's words (`OTHER_REASON`, +#: #712, after `bump-leg.py`, #383): an I/O error or a name the filesystem +#: will not take is not put right by `chmod`. +OTHER_REASON = ( + "Where the operating system gave another reason, put a regular file this " + "user can open back in its place, or take away one nobody wants, and make " + "sure nothing is changing the directory while this runs.") + + +def cannot_read_why(refused: list) -> str: + """The `why` of a row whose NEXT is this doctor again because a file or + folder it reads cannot be read, `refused` being each `OSError` (#847). + + THE GATE'S ADVICE, BY THE CAUSE, and never a `git checkout`: the file is + there, and HEAD's copy put in its place would discard whatever it holds + that no commit does, as `pin_restore`'s `uncommitted` line says of + itself. What a person does about a mode or an owner is theirs; then the + doctor reads the file again.""" + said = [] + if any(isinstance(exc, PermissionError) for exc in refused): + said.append(PERMISSION_ADVICE) + if not all(isinstance(exc, PermissionError) for exc in refused): + said.append(OTHER_REASON) + return " ".join(said) + " Then run this doctor again" + + +def doctor_line(ctx: Context) -> str: + """This doctor, run again over the root: the NEXT of a row that names a + file only a person can mend (`unreadable_next`, `cannot_read_why`).""" + return (f"{PYTHON} {quote_arg(ctx.shape / 'shape-doctor.py')} --root " + f"{quote_arg(ctx.root)}") + + +def pin_read(root: Path, relative: str) -> tuple: + """`(what the file at `relative` holds, None)`; `(None, the OSError)` + where the operating system refused its `stat` or its read; `(None, + None)` where no regular file is there, or it does not load for another + reason (#847). + + ASKED AS `validate-pins.py` ASKS IT (#712): the `stat` first, as its + `_is_file` does, so a pin in a `contracts/` this user cannot search is + named on every Python, and then the read, through `load_yaml`, whose + refusal is raised `from` the `OSError` of a file this user cannot open + (`_load`). Nothing there, or no regular file, is what the gate calls + missing, and is None here as it always was.""" + path = root / relative + try: + if not is_a(path, stat.S_ISREG): + return None, None + except OSError as exc: + return None, exc + try: + return load_yaml(path), None + except Refusal as exc: + return None, refused_read(exc.__cause__) + + +def manifest_candidates(root: Path) -> tuple[list[Path], list[tuple]]: + """Every file in a root that might DECLARE a `kind:`, and `(line, + OSError)` for each place the operating system would not let this user + look (#847). The two manifests at the top, and everything under `contracts/`. Not the whole tree: a leg is its own repository with its own contracts, and a holder's `members/` is a whole project -- walking into either would make this command report on repositories it was not pointed at. + + A `contracts/` THIS USER CANNOT LIST, OR A FILE IN IT WHOSE `stat` IS + REFUSED, IS NAMED, in `validate-pins.py`'s words (`cannot_be_listed`, + `cannot_be_read`). `iterdir()` and `is_file()` raised the refusal out of + the row, `could not be run: [Errno 13] Permission denied: '/ + contracts'`, and from Python 3.14 `is_file()` would answer False and the + row would count the manifests it never read as none (`is_a`). """ + refused: list[tuple] = [] + + def regular(path: Path) -> bool: + try: + return is_a(path, stat.S_ISREG) + except OSError as exc: + refused.append((cannot_be_read( + path.relative_to(root).as_posix(), exc), exc)) + return False + found = [root / PROJECT_MANIFEST, root / FAMILY_MANIFEST] contracts = root / "contracts" - if contracts.is_dir(): - found += sorted(p for p in contracts.iterdir() - if p.is_file() and p.suffix in (".yaml", ".yml")) - return [path for path in found if path.is_file()] + try: + listed = sorted(contracts.iterdir()) if is_a(contracts, + stat.S_ISDIR) else [] + except OSError as exc: + refused.append((cannot_be_listed("contracts", exc), exc)) + listed = [] + found += [path for path in listed if path.suffix in (".yaml", ".yml")] + return [path for path in found if regular(path)], refused #: Named once: the row this function returns and its own `CHECKS` entry below @@ -3442,6 +3616,21 @@ def check_manifest_kinds(ctx: Context) -> Row: reason still names every fault, and the next run names the rest. `restore` and `refusals` join `detail` only when a file does not load, so every other root reports the row it always did. + + A FILE OR A FOLDER THIS USER CANNOT READ IS NEITHER (#847). A pin at + mode 000 was `unreadable as YAML`, quoting `[Errno 13] Permission + denied: '/...'`, and `unreadable_next` read its git state: `git + status` calls a file it cannot open changed (` M`), so the NEXT was `git + checkout HEAD --` on it, which discards what it holds that no commit + does. A + `contracts/` this user cannot list or search ended the row in `could not + be run: [Errno 13]`. Each is named now as `validate-pins.py` names it + (`manifest_candidates`, `cannot_be_read`), and listed under `detail`'s + `cannot_be_read`, only when there is one. Its NEXT is this doctor again + and its `why` the gate's advice for the cause (`cannot_read_why`), and it + comes FIRST: the row cannot say what kind such a file declares, or + whether it loads, until it can be read, and a `contracts/` it cannot + list hides every manifest in it. No git state is asked of it. """ known: list[str] = [] #: `(path, kind)` for each manifest of a kind the registry does not know. @@ -3468,7 +3657,9 @@ def check_manifest_kinds(ctx: Context) -> Row: #: paths whose gate refuses another kind (review of PR #555). read_as = kinds_read_here(ctx) checked = kind_checked_by(ctx) - for path in manifest_candidates(ctx.root): + #: `(line, OSError)` for each file or folder this user cannot read (#847). + candidates, refused = manifest_candidates(ctx.root) + for path in candidates: if path == elsewhere: continue rel = path.relative_to(ctx.root).as_posix() @@ -3478,6 +3669,10 @@ def check_manifest_kinds(ctx: Context) -> Row: try: data = load_yaml(path) except Refusal as exc: + cause = refused_read(exc.__cause__) + if cause is not None: + refused.append((cannot_be_read(rel, cause), cause)) + continue unreadable.append(rel) refusals[rel] = {"code": exc.code, "detail": exc.detail.removeprefix(f"{path}: ")} @@ -3560,12 +3755,17 @@ def check_manifest_kinds(ctx: Context) -> Row: # ONLY WHEN THERE IS ONE, so every root without it reports the same # row, byte for byte, as it did before #211. detail["family_beside"] = beside[1] - if names or judged or unreadable: + if refused: + # Likewise (#847). + detail["cannot_be_read"] = [line for line, _exc in refused] + if names or judged or unreadable or refused: parts = [] #: The sentences of the `why` that name the other way out of each #: fault, in the row's order: never the NEXT's (#548), and never for #: a file a gate here reads (`taking_out`). ways = [] + if refused: + parts.append(", ".join(detail["cannot_be_read"])) if unknown: parts.append("no validator registered for kind " + ", ".join( detail["unregistered"])) @@ -3593,6 +3793,10 @@ def check_manifest_kinds(ctx: Context) -> Row: detail.update({"restore": restore, "refusals": refusals}) else: command, why = taking_out(ctx, names, states, judged, ways) + if refused: + # FIRST, and never a git command (#847): see the docstring. + command, why = doctor_line(ctx), cannot_read_why( + [exc for _line, exc in refused]) return Row("manifest-kinds", LABEL_MANIFEST_KINDS, FINDING, "; ".join(parts) + aside, command, detail, why=why) return Row("manifest-kinds", LABEL_MANIFEST_KINDS, OK, @@ -4538,8 +4742,7 @@ def unreadable_next(ctx: Context, unreadable: list, command, why = (commands.get(SHAPE_PIN) or next(iter(commands.values()), (None, None))) if command is None: - command = (f"{PYTHON} {quote_arg(ctx.shape / 'shape-doctor.py')} " - f"--root {quote_arg(ctx.root)}") + command = doctor_line(ctx) why = UNREADABLE_BY_HAND return restore, command, why @@ -4598,6 +4801,40 @@ def unreadable_pin_row(ctx: Context, exc: Refusal) -> Row: command, detail, why=why) +def unreadable_copy_row(ctx: Context, named: str, exc: OSError) -> Row: + """The `shape currency` row for a pin or a copy, `named` from the root, + that the operating system will not let this user read (#847). + + A STATEMENT ABOUT THEIR REPOSITORY, as `unreadable_pin_row`'s is: the + file is in the root, and its mode, its owner or its directory's is what + somebody did to THEIR tree, so no `environment`. NEVER `pin_restore`: + the file is there, `git status` calls a file it cannot open changed + (` M`), and the `git checkout HEAD --` it named would put HEAD's copy + over whatever the file holds that no commit does. The line is + `validate-pins.py`'s + (`cannot_be_read`), the NEXT this doctor again and the `why` the gate's + advice (`cannot_read_why`), as the `manifest kinds` row gives them for + the same file.""" + line = cannot_be_read(named, exc) + return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, + f"the copies cannot be compared: {line}", doctor_line(ctx), + {"path": named, "cannot_be_read": line}, + why=cannot_read_why([exc])) + + +def refused_in_root(root: Path, exc: OSError) -> str | None: + """Where `exc` is a read the operating system refused of a file inside + `root`, that file's path from the root, POSIX; else None (#847): an + `OSError` that says nothing is there, one that names no file, or one + about a file elsewhere, the checkout of the standard's own among them.""" + if refused_read(exc) is None or not exc.filename: + return None + try: + return Path(os.fsdecode(exc.filename)).relative_to(root).as_posix() + except ValueError: + return None + + #: A line `rev-list --missing=print` writes for an object the clone lacks: #: `?` and the object's id, SHA-1 or SHA-256. MISSING_OBJECT = re.compile(rb"\?(?:[0-9a-f]{40}|[0-9a-f]{64})") @@ -4732,6 +4969,21 @@ def check_shape_currency(ctx: Context) -> Row: a clone that lacks objects and has no promisor remote, a damaged one -- is `update-upstream-unreadable` from `prepare`, CANNOT ANSWER too, and never a copy called `both`. + + A PIN, OR A COPY, THIS USER CANNOT READ IS NAMED (#847), as + `validate-pins.py` names it: `the copies cannot be compared: + contracts/shape-pin.yaml: cannot be read (Permission denied)`, a + FINDING about the repository, with this doctor again as its NEXT and the + gate's advice for the cause as its `why` (`unreadable_copy_row`), the + NEXT and `why` the `manifest kinds` row gives the same file. A pin at + mode 000 was `unreadable_pin_row`'s, whose `pin_restore` read it as + changed and named `git checkout HEAD --` on it; a `contracts/` this user + cannot search, or a copy at mode 000, ended the row in `could not be + run: [Errno 13]`. The pin is asked by `pin_read`, before `read_pin`, + whose `is_file()` raised the refused `stat` up to Python 3.13 and, from + 3.14, calls the pin missing. A copy is asked by `prepare`, and only a + refused read of a file inside the root is the repository's: any other + `OSError` is raised as it was. """ us = ctx.update_shape() # A PIN OR MANIFEST KEPT AS A LINK IS ASKED ABOUT FIRST, before anything @@ -4740,6 +4992,9 @@ def check_shape_currency(ctx: Context) -> Row: linked = linked_record_row(ctx, us) if linked is not None: return linked + refused = pin_read(ctx.root, SHAPE_PIN)[1] + if refused is not None: + return unreadable_copy_row(ctx, SHAPE_PIN, refused) args = argparse.Namespace(root=str(ctx.root), upstream=str(ctx.shape), at=None) @@ -4778,6 +5033,11 @@ def check_shape_currency(ctx: Context) -> Row: why="this checkout of the standard does not carry the " "commit the pin names; fetch it, or run the doctor " "from a clone that has it") + except OSError as exc: + named = refused_in_root(ctx.root, exc) + if named is None: + raise + return unreadable_copy_row(ctx, named, exc) try: added = [add for add in additions if not add.present] reported = list(rows) + added @@ -4943,7 +5203,12 @@ def head_of(path: Path) -> str | None: def pin_commit(root: Path, relative: str) -> str | None: - pin = Context._read(root / relative) + """The 40-hex `commit:` of the pin at `relative`, lowercased, or None. + + Read by `pin_read`, as `validate-pins.py` reads a pin (#847): a pin whose + `stat` or read the operating system refused is None here as one that is + not there is, and `check_legs` asks `pin_read` why.""" + pin, _refused = pin_read(root, relative) if not isinstance(pin, dict): return None commit = str(pin.get("commit") or "").lower() @@ -5672,6 +5937,19 @@ def check_legs(ctx: Context) -> Row: WHERE THE GATE IS THE LINE, the legs no line cures are said beside it: an unborn HEAD or a leg with no url gets the same sentence it gets with no line at all, which ends by saying to run the doctor again. + + A PIN THIS USER CANNOT READ IS NAMED, AND IS NO DRIFT (#847). It read as + `carries no 40-hex commit`, a lockstep fact, and the verdict counted the + leg as off its pin: DRIFTED, for a leg nobody moved, where + `validate-pins.py` refuses the pin by name. `pin_read` asks why the pin + gave no commit, and a `stat` or a read the operating system refused is + `: contracts/-pin.yaml: cannot be read ()`, the + gate's line, kept in the leg's `cannot_be_read`, which `drifted_reasons` + reads. A person mends it, on a mode or an owner, so where no checkout + and no lockstep fact comes first the NEXT is this doctor again and the + `why` the gate's advice (`cannot_read_why`), as the `manifest kinds` and + `shape currency` rows give them; beside the gate's line, the advice is + said in its `why`. """ waiting = waiting_row(ctx, "legs", "legs", "the legs it declares are not known and none is " @@ -5693,6 +5971,8 @@ def check_legs(ctx: Context) -> Row: lockstep: list[str] = [] gone: list[str] = [] stuck: list[str] = [] + #: The `OSError` of each pin the operating system refused to read (#847). + refused: list[OSError] = [] rerun = False nested: set = set() for leg in legs: @@ -5732,9 +6012,17 @@ def check_legs(ctx: Context) -> Row: problem = f"{rel}: populated, but git cannot read a HEAD in it" lockstep.append(rel) elif pinned is None: - problem = (f"{rel}: contracts/{role}-pin.yaml carries no 40-hex " - "commit to compare HEAD against") - lockstep.append(rel) + pin = f"contracts/{role}-pin.yaml" + cause = pin_read(ctx.root, pin)[1] + if cause is None: + problem = (f"{rel}: {pin} carries no 40-hex commit to compare " + "HEAD against") + lockstep.append(rel) + else: + entry["cannot_be_read"] = cannot_be_read(pin, cause) + problem = (f"{rel}: {entry['cannot_be_read']}, so there is no " + "pinned commit to compare HEAD against") + refused.append(cause) elif head != pinned: problem = (f"{rel}: checked out at {head[:12]}, pinned at " f"{pinned[:12]}") @@ -5770,7 +6058,8 @@ def check_legs(ctx: Context) -> Row: problems.append(problem) detail = {"legs": rows} if problems: - rerun = rerun or bool(lockstep) or bool(stuck) + rerun = rerun or bool(lockstep) or bool(stuck) or bool(refused) + said = [cannot_read_why(refused)] if refused else [] if checkout: # `make bootstrap` IS NAMED ONLY WHERE IT CAN RUN, and said for # what it does: it puts a leg on its tracking branch, and @@ -5796,9 +6085,12 @@ def check_legs(ctx: Context) -> Row: next_command, why = lockstep_next(ctx, PINS_GATE, "leg") if why: why += "".join(gone_sentence(rel) for rel in gone) - why += "".join(f". {sentence}" for sentence in stuck) - elif stuck: - why = ". ".join(stuck) + why += "".join(f". {sentence}" for sentence in [*said, *stuck]) + elif said or stuck: + why = ". ".join([*said, *stuck]) + elif refused: + # A PERSON'S, on a mode or an owner, and never a git line (#847). + next_command, why = doctor_line(ctx), ". ".join([*said, *stuck]) else: # NO LINE CURES IT without choosing for the person (#414). next_command, why = None, ". ".join(stuck) @@ -9676,6 +9968,14 @@ def drifted_reasons(by_id: dict, detail: dict) -> list: Returns the empty list when none of them is, which is the caller's signal to go on down the ladder rather than a claim that anything is well. + A LEG WHOSE PIN CANNOT BE READ IS NOT OFF ITS PIN (#847): nothing says + where its pin is, and `validate-pins.py` refuses the pin by name, which + is the `pins` row and INVALID. Such a leg carries `cannot_be_read` in + the `legs` row, and is not counted. Every other leg whose HEAD is not + its pin is counted as before. Where there is none, the red row is drift + only where no leg carries `cannot_be_read`, as it always was: the gate + refuses that pin before it reports anything else, and INVALID names it. + Split out of `verdict_for` for #134. """ reasons: list[str] = [] @@ -9683,9 +9983,12 @@ def drifted_reasons(by_id: dict, detail: dict) -> list: reasons += drifted_file_reasons(detail.get("counts") or {}) legs = by_id.get("legs") if legs is not None and legs.status == FINDING: - off = [leg for leg in (legs.detail.get("legs") or []) - if leg.get("head") != leg.get("pin")] - reasons.append(f"{len(off) or 1} leg(s) not at the pin") + entries = legs.detail.get("legs") or [] + unread = [leg for leg in entries if "cannot_be_read" in leg] + off = [leg for leg in entries if "cannot_be_read" not in leg + and leg.get("head") != leg.get("pin")] + if off or not unread: + reasons.append(f"{len(off) or 1} leg(s) not at the pin") members = by_id.get("members") if members is not None and members.status == FINDING: reasons.append("a member is not at its pin") diff --git a/tests/test_doctor_unreadable_pin.py b/tests/test_doctor_unreadable_pin.py new file mode 100644 index 00000000..32fc616b --- /dev/null +++ b/tests/test_doctor_unreadable_pin.py @@ -0,0 +1,405 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A pin, or a `contracts/`, this user cannot read is named by the doctor's +rows as `validate-pins.py` names it (#847). + +THE DEFECT. #712 (PR #827) makes the gate refuse a pin the operating system +will not let it reach or open by name: `REFUSED yaml-unreadable: +contracts/spec-pin.yaml: cannot be read (Permission denied)`, with a remedy +for the permission. Three of the doctor's rows read the same files +themselves, and said something else: + + * `legs`, through `pin_commit`: `spec: contracts/spec-pin.yaml carries no + 40-hex commit to compare HEAD against`, which the verdict counted as a + leg off its pin, `DRIFTED (1 leg(s) not at the pin)`, for a pin at mode + 000 and for a `contracts/` at 000; + * `manifest kinds`: `unreadable as YAML: ... [Errno 13] Permission denied: + '/...'` for a pin at 000, with `git checkout HEAD -- ` as + its NEXT, which discards what the file holds that no commit does, and + `could not be run: [Errno 13] ...` for a `contracts/` at 000; + * `shape currency`: `could not be run: [Errno 13] ...` for a `contracts/` + at 000 or a pinned copy at 000, and for a shape pin at 000 the same + `git checkout HEAD --`. + +Each now says the gate's line, `: cannot be read ()` (a +`contracts/` it cannot list: `contracts: cannot be listed ()`, the +gate's words for a directory, #434), its NEXT is this doctor again and its +`why` the gate's advice for the cause, word for word, and the verdict is the +gate's INVALID, never DRIFTED. + +TWO KINDS OF CASE. The MODE cases change a real mode and run the doctor as +a person does, so they skip on Windows, whose modes refuse no read, and as +root, who reads a file at 000 (`CANNOT_RUN_AS_ROOT`). The STAND-IN cases run +the doctor's rows in process with the refusal put in by hand, as #827's +`refused_stat` rows do: a refused `stat` (`refused_stat`, which also answers +`is_file()` as Python 3.14 does, False to any `OSError`), a refused read or +a refused listing (`refusing`). They run on every platform and as root. + +NOTHING HERE REACHES A NETWORK: the doctor's own fixtures, a scaffold into +local bare remotes, and the session's `gh` guard for the `machine` row. +""" + +from __future__ import annotations + +import errno +import json +import os +import stat +from contextlib import contextmanager +from pathlib import Path + +import pytest + +#: #827's stand-in for a `stat` the operating system refuses, from the file +#: that owns it (#383), as `test_validate_pins_unreadable_workflows_dir.py` +#: takes it. +from test_bump_leg_unopenable import refused_stat +#: This doctor's own line for a root, as `unreadable_next` spells it. +from test_doctor_recorded_answers import doctor_again +#: The doctor's fixtures and helpers, from the module that owns them. +from test_shape_doctor import ( # noqa: F401 + SHAPE_PIN, doctor, doctor_module, project, rows_of, scaffolded_here, + standard, +) +#: The one skip for a mode that root reads through and Windows ignores. +from test_validate_pins_unreadable_workflow import CANNOT_RUN_AS_ROOT + +SPEC_PIN = "contracts/spec-pin.yaml" +CODE_PIN = "contracts/code-pin.yaml" +#: A pinned copy that lives under `contracts/`. +NAMING_COPY = "contracts/repository-naming.yaml" + +#: The reason every stand-in gives, as `refused_stat` raises it. +DENIED = "Permission denied" + +#: `validate-pins.py`'s advice for a refused permission, word for word +#: (`_unreadable_file`, #712), and the doctor's own last sentence. +PERMISSION_WHY = ( + "Make what this user may not open readable by this user: `chmod u+r` on " + "a file, `chmod u+rx` on a directory (and on every directory above it " + "inside the root), or fix who owns it. Then run this doctor again") +#: The same gate's advice for any other cause (`OTHER_REASON`, #712). +OTHER_WHY = ( + "Where the operating system gave another reason, put a regular file this " + "user can open back in its place, or take away one nobody wants, and make " + "sure nothing is changing the directory while this runs. Then run this " + "doctor again") + +#: What no row may name for a file this user cannot read (#847). +CHECKOUT = "checkout HEAD --" + + +def cannot(named: str, reason: str = DENIED) -> str: + """The gate's line for a file it cannot reach or open.""" + return f"{named}: cannot be read ({reason})" + + +def leg_line(leg: str, pin: str, reason: str = DENIED) -> str: + """The `legs` row's words for a leg whose pin cannot be read.""" + return (f"{leg}: {cannot(pin, reason)}, so there is no pinned commit to " + "compare HEAD against") + + +@contextmanager +def mode(path: Path, bits: int): + """`path` at `bits` for the block, and its own mode back after it, so + the basetemp can be removed.""" + held = stat.S_IMODE(path.stat().st_mode) + path.chmod(bits) + try: + yield + finally: + path.chmod(held) + + +def doctored(standard: Path, root: Path) -> tuple[dict, str]: + """`({id: row}, verdict)` from one `--json` run that printed no + traceback.""" + result = doctor(standard, root, "--json") + assert "Traceback" not in result.stderr, result.stderr + return rows_of(result), json.loads(result.stdout)["verdict"] + + +def assert_no_checkout(rows: dict) -> None: + """No row names a `git checkout HEAD --`, as its NEXT or in its why.""" + for row in rows.values(): + assert CHECKOUT not in (row["next"] or ""), row + assert CHECKOUT not in (row["why"] or ""), row + + +def refusing(monkeypatch, method: str, name: str, error: OSError) -> None: + """`Path.` raising `error`, with the path as its filename, for a + path called `name`, and the real method for every other: a read + (`read_text`, `read_bytes`) or a listing (`iterdir`) the operating + system refused, in process, on every platform and as root.""" + real = getattr(Path, method) + + def refused(self, *args, **kwargs): + if self.name == name: + raise type(error)(error.errno, error.strerror, str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, method, refused) + + +def in_process(standard: Path, project: Path): + """`(the doctor as a module, its Context over the root)`, the root + resolved as `main` resolves it.""" + module = doctor_module(standard) + return module, module.Context(project.resolve()) + + +# --- the mode cases: a real mode, the doctor run as a person runs it --------- + +@CANNOT_RUN_AS_ROOT +def test_a_pin_at_mode_000_is_named_as_the_gate_names_it(standard, project): + """The issue's first case. The `legs` row says the gate's line and is no + drift; `manifest kinds` says it too, and neither names a checkout.""" + again = doctor_again(standard, project) + denied = os.strerror(errno.EACCES) + with mode(project / SPEC_PIN, 0): + rows, verdict = doctored(standard, project) + legs, kinds = rows["legs"], rows["manifest-kinds"] + assert legs["status"] == "FINDING", legs + assert legs["reason"] == leg_line("spec", SPEC_PIN, denied) + assert legs["next"] == again + assert legs["why"] == PERMISSION_WHY + assert kinds["status"] == "FINDING", kinds + assert kinds["reason"] == cannot(SPEC_PIN, denied) + assert kinds["next"] == again + assert kinds["why"] == PERMISSION_WHY + assert verdict == "INVALID (pins, manifest-kinds)" + assert_no_checkout(rows) + + +@CANNOT_RUN_AS_ROOT +@pytest.mark.parametrize("bits", [ + pytest.param(0, id="contracts-000"), + pytest.param(0o444, id="contracts-r--"), +]) +def test_a_contracts_folder_this_user_cannot_search_is_named( + standard, project, bits): + """The issue's second case, and its listable cousin. At 000 the folder + cannot be listed; at `r--` it can, and no file in it can be reached. + Every row that reads it names what it cannot read, in the gate's words, + and the verdict is no drift.""" + again = doctor_again(standard, project) + denied = os.strerror(errno.EACCES) + contracts = project / "contracts" + listed = sorted(path.name for path in contracts.iterdir() + if path.suffix in (".yaml", ".yml")) + with mode(contracts, bits): + rows, verdict = doctored(standard, project) + expected = (f"contracts: cannot be listed ({denied})" if bits == 0 else + ", ".join(cannot(f"contracts/{name}", denied) + for name in listed)) + assert rows["manifest-kinds"]["reason"] == expected + assert rows["legs"]["reason"] == "; ".join(( + leg_line("spec", SPEC_PIN, denied), + leg_line("code", CODE_PIN, denied))) + assert rows["shape-currency"]["reason"] == ( + "the copies cannot be compared: " + cannot(SHAPE_PIN, denied)) + for row in ("manifest-kinds", "legs", "shape-currency"): + assert rows[row]["next"] == again, rows[row] + assert rows[row]["why"] == PERMISSION_WHY, rows[row] + assert verdict == "INVALID (manifest, pins, manifest-kinds)" + assert_no_checkout(rows) + + +@CANNOT_RUN_AS_ROOT +def test_a_shape_pin_at_mode_000_gets_one_answer_from_both_rows( + standard, project): + """The shape pin at 000: `shape currency` and `manifest kinds` print the + same NEXT and the same why for the same file, as they do for a pin that + does not load (#291), and neither is `git checkout HEAD --`.""" + denied = os.strerror(errno.EACCES) + with mode(project / SHAPE_PIN, 0): + rows, verdict = doctored(standard, project) + shape, kinds = rows["shape-currency"], rows["manifest-kinds"] + assert shape["reason"] == ("the copies cannot be compared: " + + cannot(SHAPE_PIN, denied)) + assert kinds["reason"] == cannot(SHAPE_PIN, denied) + assert shape["next"] == doctor_again(standard, project) + assert shape["next"] == kinds["next"] + assert shape["why"] == PERMISSION_WHY + assert shape["why"] == kinds["why"] + assert "environment" not in shape["detail"], shape + assert verdict == "INVALID (pins, manifest-kinds)" + assert_no_checkout(rows) + + +@CANNOT_RUN_AS_ROOT +def test_a_pinned_copy_at_mode_000_stops_the_comparison_by_name( + standard, project): + """A pinned copy that lives under `contracts/`, at 000: `shape currency` + was `could not be run: [Errno 13] ...`, and `manifest kinds` a `git + diff HEAD` whose why spoke of an uncommitted change. Both name it.""" + denied = os.strerror(errno.EACCES) + with mode(project / NAMING_COPY, 0): + rows, _verdict = doctored(standard, project) + assert rows["shape-currency"]["reason"] == ( + "the copies cannot be compared: " + cannot(NAMING_COPY, denied)) + assert rows["manifest-kinds"]["reason"] == cannot(NAMING_COPY, denied) + assert rows["manifest-kinds"]["why"] == PERMISSION_WHY + assert_no_checkout(rows) + + +# --- the stand-in cases: in process, on every platform and as root ---------- + +def refused_stat_of(monkeypatch, name: str) -> None: + """A `contracts/` this user cannot search, for the one file `name`.""" + refused_stat(monkeypatch, name) + + +def refused_read_of(monkeypatch, name: str) -> None: + """A file at mode 000: there, and its read refused.""" + refusing(monkeypatch, "read_text", name, + PermissionError(errno.EACCES, DENIED)) + + +@pytest.mark.parametrize("refuse", [ + pytest.param(refused_stat_of, id="stat-refused"), + pytest.param(refused_read_of, id="read-refused"), +]) +def test_a_refused_pin_is_named_on_every_python(standard, project, + monkeypatch, refuse): + """The `legs` and `manifest kinds` rows for a pin whose `stat` or read + is refused. A refused `stat` reaches `pin_commit` as Python 3.14's + `is_file()` would answer it, so the pin is asked as the gate asks it, + `stat` first; and a leg whose pin cannot be read is no drift.""" + module, ctx = in_process(standard, project) + again = doctor_again(standard, project) + refuse(monkeypatch, "spec-pin.yaml") + legs = module.check_legs(ctx) + kinds = module.check_manifest_kinds(ctx) + assert legs.reason == leg_line("spec", SPEC_PIN) + assert legs.next_command == again + assert legs.why == PERMISSION_WHY + assert legs.detail["legs"][0]["cannot_be_read"] == cannot(SPEC_PIN) + assert module.drifted_reasons({"legs": legs}, {}) == [] + assert kinds.reason == cannot(SPEC_PIN) + assert kinds.next_command == again + assert kinds.why == PERMISSION_WHY + assert kinds.detail["cannot_be_read"] == [cannot(SPEC_PIN)] + + +def test_another_reason_is_given_the_gates_other_advice( + standard, project, monkeypatch): + """A read refused for a reason that is no permission (an I/O error): the + system's reason, and the gate's other advice, which is not told to + `chmod`.""" + module, ctx = in_process(standard, project) + refusing(monkeypatch, "read_text", "spec-pin.yaml", + OSError(errno.EIO, "Input/output error")) + legs = module.check_legs(ctx) + kinds = module.check_manifest_kinds(ctx) + assert legs.reason == leg_line("spec", SPEC_PIN, "Input/output error") + assert legs.why == OTHER_WHY + assert kinds.reason == cannot(SPEC_PIN, "Input/output error") + assert kinds.why == OTHER_WHY + + +def test_a_shape_pin_whose_stat_is_refused_is_not_called_missing( + standard, project, monkeypatch): + """`read_pin` asks `is_file()`, which raised a refused `stat` up to + Python 3.13 and calls the pin missing from 3.14 (`update-pin-missing`). + The row asks first, and names the pin in the gate's words, with the + NEXT and why `manifest kinds` gives it.""" + module, ctx = in_process(standard, project) + refused_stat(monkeypatch, "shape-pin.yaml") + shape = module.check_shape_currency(ctx) + kinds = module.check_manifest_kinds(ctx) + assert shape.status == "FINDING" + assert shape.reason == ("the copies cannot be compared: " + + cannot(SHAPE_PIN)) + assert shape.next_command == doctor_again(standard, project) + assert shape.next_command == kinds.next_command + assert shape.why == PERMISSION_WHY + assert shape.why == kinds.why + assert shape.detail == {"path": SHAPE_PIN, + "cannot_be_read": cannot(SHAPE_PIN)} + + +def test_a_contracts_folder_that_cannot_be_listed_is_named( + standard, project, monkeypatch): + """`iterdir()` refused: the folder, in the gate's words for a directory + it cannot list, where the row ended in `could not be run`.""" + module, ctx = in_process(standard, project) + refusing(monkeypatch, "iterdir", "contracts", + PermissionError(errno.EACCES, DENIED)) + kinds = module.check_manifest_kinds(ctx) + assert kinds.reason == f"contracts: cannot be listed ({DENIED})" + assert kinds.next_command == doctor_again(standard, project) + assert kinds.why == PERMISSION_WHY + + +def test_a_copy_whose_read_is_refused_stops_the_comparison_by_name( + standard, project, monkeypatch): + """`update-shape.py`'s comparison reads each copy, and a refused read of + one in the root is the row's FINDING, named, where it ended the row in + `could not be run`.""" + module, ctx = in_process(standard, project) + refusing(monkeypatch, "read_bytes", "repository-naming.yaml", + PermissionError(errno.EACCES, DENIED)) + shape = module.check_shape_currency(ctx) + assert shape.reason == ("the copies cannot be compared: " + + cannot(NAMING_COPY)) + assert shape.next_command == doctor_again(standard, project) + assert shape.why == PERMISSION_WHY + + +def test_only_a_refused_read_inside_the_root_is_the_roots(standard, project): + """`refused_in_root` names a file inside the root whose read was + refused, and nothing else: a file that is not there, or one outside the + root, such as the checkout of the standard's own, stays an error.""" + module, ctx = in_process(standard, project) + inside = str(ctx.root / NAMING_COPY) + outside = str(standard.resolve() / NAMING_COPY) + assert module.refused_in_root(ctx.root, PermissionError( + errno.EACCES, DENIED, inside)) == NAMING_COPY + assert module.refused_in_root(ctx.root, FileNotFoundError( + errno.ENOENT, "No such file or directory", inside)) is None + assert module.refused_in_root(ctx.root, PermissionError( + errno.EACCES, DENIED, outside)) is None + + +def test_a_refused_stat_is_raised_on_every_python(standard, project, + monkeypatch): + """`is_a` raises a refused `stat` where `is_file()` on 3.14 answers + False, and answers False only where nothing is there.""" + module, _ctx = in_process(standard, project) + refused_stat(monkeypatch, "spec-pin.yaml") + with pytest.raises(PermissionError): + module.is_a(project / SPEC_PIN, stat.S_ISREG) + assert module.is_a(project / "contracts" / "absent.yaml", + stat.S_ISREG) is False + + +# --- controls: what must not change ------------------------------------------ + +def test_a_missing_pin_keeps_its_words_and_is_still_drift(standard, project): + """A pin that is not there is what the gate calls `pin-missing`, not a + file it cannot read: the row's words and the drift are main's.""" + module, ctx = in_process(standard, project) + (project / SPEC_PIN).unlink() + legs = module.check_legs(ctx) + assert legs.reason == (f"spec: {SPEC_PIN} carries no 40-hex commit to " + "compare HEAD against") + assert "cannot_be_read" not in legs.detail["legs"][0] + assert module.drifted_reasons({"legs": legs}, {}) == [ + "1 leg(s) not at the pin"] + + +def test_beside_a_lockstep_fact_the_gate_stays_the_next(standard, project, + monkeypatch): + """One leg's pin cannot be read and the other's is not there: the + missing pin is the gate's lockstep fact, so its line is the NEXT, the + gate's advice is said in the why, and the other leg is drift.""" + module, ctx = in_process(standard, project) + (project / CODE_PIN).unlink() + refused_read_of(monkeypatch, "spec-pin.yaml") + legs = module.check_legs(ctx) + gate, _said = module.lockstep_next(ctx, module.PINS_GATE, "leg") + assert legs.next_command == gate + assert legs.why.endswith(". " + PERMISSION_WHY), legs.why + assert module.drifted_reasons({"legs": legs}, {}) == [ + "1 leg(s) not at the pin"] From bcb8c58be4d37c01d7993df903c2f0427749d72f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 02:09:57 +0000 Subject: [PATCH 2/3] shape-doctor.py and its tests: rv860's round 1 on #860 (#847) SHOULD 1. tests/test_doctor_unreadable_pin.py loads shape-doctor.py, templates/assembly-root/scripts/validate-pins.py and scripts/bump-leg.py by path and holds the doctor's copies equal to theirs: `ABSENT_ERRNOS` and `ABSENT_WINERRORS` to the gate's and bump-leg's `_ABSENT_*`; the gate's `OTHER_REASON` to the doctor's and the one space the gate joins on; the remedy the gate's `_unreadable_file` and `_workflows_unlistable` build for each cause to end on the doctor's sentence and `RUN_AGAIN`; `cannot_read_why` to bump-leg's `_cannot_open_advice`; and `cannot_be_read` and `cannot_be_listed` to the gate's lines. NIT 3. A `contracts/` the `manifest kinds` row cannot list for another reason than permission is told the gate's sentence for a directory, `OTHER_REASON_FOLDER` ("put a directory this user can list back in its place"), not the file one: `manifest_candidates` marks a listing, and `cannot_read_why` takes the folders apart. NIT 5. Beside the `legs` row's checkout line, a pin this user cannot read has the gate's advice said in the WHY, after what the line does, as beside the gate's line. NIT 6. `check_shape_currency` reads the shape pin once where it reads: `pin_read` is asked only after `read_pin` raised (`pin_refusal_row`). NIT 7. Two docstring lines left over from a reflow, joined. NIT 8. The test file's own `mode()` is gone; it takes `mode_of` from tests/test_validate_pins_unreadable_workflows_dir.py, as tests/test_validate_pins_named_and_spelled.py does. NIT 2 and 4. New cases kill rv860's surviving mutants R1, R2, B1, B2, C1, E1, E2, G1, H2, I2, K1, L1, N1, N2, N4 and O1, among them a red `legs` row with no leg off its pin, which is drift by `len(off) or 1` alone and none beside a pin this user cannot read (documented as intended), and a pin that does not load for another reason, which keeps main's words. 44 cases in all, up from 15. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 129 ++++++---- tests/test_doctor_unreadable_pin.py | 358 +++++++++++++++++++++++++--- 2 files changed, 411 insertions(+), 76 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 46f4d961..e6312129 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2968,27 +2968,41 @@ def cannot_be_listed(named: str, exc: OSError) -> str: "user can open back in its place, or take away one nobody wants, and make " "sure nothing is changing the directory while this runs.") +#: What a FOLDER this user cannot list is told for any other reason, in the +#: gate's words for a directory it cannot list (`_workflows_unlistable`, +#: #434): it is a directory that goes back in its place, not a file. +OTHER_REASON_FOLDER = ( + "Where the operating system gave another reason, put a directory this " + "user can list back in its place, and make sure nothing is changing it " + "while this runs.") -def cannot_read_why(refused: list) -> str: + +def cannot_read_why(refused: list, unlisted: list = ()) -> str: """The `why` of a row whose NEXT is this doctor again because a file or - folder it reads cannot be read, `refused` being each `OSError` (#847). + folder it reads cannot be read: `refused` the `OSError` of each file, + `unlisted` that of each folder it cannot list (#847). THE GATE'S ADVICE, BY THE CAUSE, and never a `git checkout`: the file is there, and HEAD's copy put in its place would discard whatever it holds that no commit does, as `pin_restore`'s `uncommitted` line says of - itself. What a person does about a mode or an owner is theirs; then the - doctor reads the file again.""" + itself. A permission, of a file or a folder, is `PERMISSION_ADVICE`; any + other cause is `OTHER_REASON` for a file and `OTHER_REASON_FOLDER` for a + folder, as the gate tells the two apart. What a person does about a + mode or an owner is theirs; then the doctor reads the file again.""" said = [] - if any(isinstance(exc, PermissionError) for exc in refused): + if any(isinstance(exc, PermissionError) for exc in [*refused, *unlisted]): said.append(PERMISSION_ADVICE) - if not all(isinstance(exc, PermissionError) for exc in refused): + if any(not isinstance(exc, PermissionError) for exc in refused): said.append(OTHER_REASON) + if any(not isinstance(exc, PermissionError) for exc in unlisted): + said.append(OTHER_REASON_FOLDER) return " ".join(said) + " Then run this doctor again" def doctor_line(ctx: Context) -> str: """This doctor, run again over the root: the NEXT of a row that names a - file only a person can mend (`unreadable_next`, `cannot_read_why`).""" + file or folder only a person can mend (`cannot_read_why`), the same line + `unreadable_next` spells for a file that does not load.""" return (f"{PYTHON} {quote_arg(ctx.shape / 'shape-doctor.py')} --root " f"{quote_arg(ctx.root)}") @@ -3019,8 +3033,8 @@ def pin_read(root: Path, relative: str) -> tuple: def manifest_candidates(root: Path) -> tuple[list[Path], list[tuple]]: """Every file in a root that might DECLARE a `kind:`, and `(line, - OSError)` for each place the operating system would not let this user - look (#847). + OSError, whether it is a folder this user cannot list)` for each place + the operating system would not let this user look (#847). The two manifests at the top, and everything under `contracts/`. Not the whole tree: a leg is its own repository with its own contracts, and a @@ -3032,7 +3046,10 @@ def manifest_candidates(root: Path) -> tuple[list[Path], list[tuple]]: `cannot_be_read`). `iterdir()` and `is_file()` raised the refusal out of the row, `could not be run: [Errno 13] Permission denied: '/ contracts'`, and from Python 3.14 `is_file()` would answer False and the - row would count the manifests it never read as none (`is_a`). + row would count the manifests it never read as none (`is_a`). The third + value is what `cannot_read_why` tells a folder by: a folder this user + cannot list for another reason than permission is told to put a + directory back, as the gate tells one, not a file. """ refused: list[tuple] = [] @@ -3041,7 +3058,7 @@ def regular(path: Path) -> bool: return is_a(path, stat.S_ISREG) except OSError as exc: refused.append((cannot_be_read( - path.relative_to(root).as_posix(), exc), exc)) + path.relative_to(root).as_posix(), exc), exc, False)) return False found = [root / PROJECT_MANIFEST, root / FAMILY_MANIFEST] @@ -3050,7 +3067,7 @@ def regular(path: Path) -> bool: listed = sorted(contracts.iterdir()) if is_a(contracts, stat.S_ISDIR) else [] except OSError as exc: - refused.append((cannot_be_listed("contracts", exc), exc)) + refused.append((cannot_be_listed("contracts", exc), exc, True)) listed = [] found += [path for path in listed if path.suffix in (".yaml", ".yml")] return [path for path in found if regular(path)], refused @@ -3622,15 +3639,15 @@ def check_manifest_kinds(ctx: Context) -> Row: denied: '/...'`, and `unreadable_next` read its git state: `git status` calls a file it cannot open changed (` M`), so the NEXT was `git checkout HEAD --` on it, which discards what it holds that no commit - does. A - `contracts/` this user cannot list or search ended the row in `could not - be run: [Errno 13]`. Each is named now as `validate-pins.py` names it - (`manifest_candidates`, `cannot_be_read`), and listed under `detail`'s - `cannot_be_read`, only when there is one. Its NEXT is this doctor again - and its `why` the gate's advice for the cause (`cannot_read_why`), and it - comes FIRST: the row cannot say what kind such a file declares, or - whether it loads, until it can be read, and a `contracts/` it cannot - list hides every manifest in it. No git state is asked of it. + does. A `contracts/` this user cannot list or search ended the row in + `could not be run: [Errno 13]`. Each is named now as `validate-pins.py` + names it (`manifest_candidates`, `cannot_be_read`), and listed under + `detail`'s `cannot_be_read`, only when there is one. Its NEXT is this + doctor again and its `why` the gate's advice for the cause, a folder's + as the gate gives a folder (`cannot_read_why`), and it comes FIRST: the + row cannot say what kind such a file declares, or whether it loads, + until it can be read, and a `contracts/` it cannot list hides every + manifest in it. No git state is asked of it. """ known: list[str] = [] #: `(path, kind)` for each manifest of a kind the registry does not know. @@ -3657,7 +3674,8 @@ def check_manifest_kinds(ctx: Context) -> Row: #: paths whose gate refuses another kind (review of PR #555). read_as = kinds_read_here(ctx) checked = kind_checked_by(ctx) - #: `(line, OSError)` for each file or folder this user cannot read (#847). + #: `(line, OSError, folder)` for each file or folder this user cannot + #: read (#847). candidates, refused = manifest_candidates(ctx.root) for path in candidates: if path == elsewhere: @@ -3671,7 +3689,7 @@ def check_manifest_kinds(ctx: Context) -> Row: except Refusal as exc: cause = refused_read(exc.__cause__) if cause is not None: - refused.append((cannot_be_read(rel, cause), cause)) + refused.append((cannot_be_read(rel, cause), cause, False)) continue unreadable.append(rel) refusals[rel] = {"code": exc.code, @@ -3757,7 +3775,7 @@ def check_manifest_kinds(ctx: Context) -> Row: detail["family_beside"] = beside[1] if refused: # Likewise (#847). - detail["cannot_be_read"] = [line for line, _exc in refused] + detail["cannot_be_read"] = [entry[0] for entry in refused] if names or judged or unreadable or refused: parts = [] #: The sentences of the `why` that name the other way out of each @@ -3796,7 +3814,8 @@ def check_manifest_kinds(ctx: Context) -> Row: if refused: # FIRST, and never a git command (#847): see the docstring. command, why = doctor_line(ctx), cannot_read_why( - [exc for _line, exc in refused]) + [exc for _line, exc, folder in refused if not folder], + [exc for _line, exc, folder in refused if folder]) return Row("manifest-kinds", LABEL_MANIFEST_KINDS, FINDING, "; ".join(parts) + aside, command, detail, why=why) return Row("manifest-kinds", LABEL_MANIFEST_KINDS, OK, @@ -4869,10 +4888,9 @@ def unreadable_copy_row(ctx: Context, named: str, exc: OSError) -> Row: the file is there, `git status` calls a file it cannot open changed (` M`), and the `git checkout HEAD --` it named would put HEAD's copy over whatever the file holds that no commit does. The line is - `validate-pins.py`'s - (`cannot_be_read`), the NEXT this doctor again and the `why` the gate's - advice (`cannot_read_why`), as the `manifest kinds` row gives them for - the same file.""" + `validate-pins.py`'s (`cannot_be_read`), the NEXT this doctor again and + the `why` the gate's advice (`cannot_read_why`), as the `manifest kinds` + row gives them for the same file.""" line = cannot_be_read(named, exc) return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, f"the copies cannot be compared: {line}", doctor_line(ctx), @@ -4880,6 +4898,26 @@ def unreadable_copy_row(ctx: Context, named: str, exc: OSError) -> Row: why=cannot_read_why([exc])) +def pin_refusal_row(ctx: Context, exc: Exception) -> Row: + """The `shape currency` row for a shape pin `read_pin` would not read, + `exc` being what it raised: a `Refusal`, or the `OSError` of a refused + `stat` that `is_file()` raises up to Python 3.13 (#847). + + THE PIN IS ASKED AGAIN ONLY HERE, by `pin_read`, as `validate-pins.py` + asks it: a `stat` or a read the operating system refused is + `unreadable_copy_row`'s, the gate's line, whatever `read_pin` made of + it (`update-pin-missing` from 3.14, `yaml-unreadable` quoting `[Errno + 13]`). Any other refusal is `unreadable_pin_row`'s, as it always was, + and any other `OSError` is raised as it was. A pin that reads is read + once, by `read_pin` (review of PR #860).""" + refused = pin_read(ctx.root, SHAPE_PIN)[1] + if refused is not None: + return unreadable_copy_row(ctx, SHAPE_PIN, refused) + if isinstance(exc, Refusal): + return unreadable_pin_row(ctx, exc) + raise exc + + def refused_in_root(root: Path, exc: OSError) -> str | None: """Where `exc` is a read the operating system refused of a file inside `root`, that file's path from the root, POSIX; else None (#847): an @@ -5037,11 +5075,11 @@ def check_shape_currency(ctx: Context) -> Row: mode 000 was `unreadable_pin_row`'s, whose `pin_restore` read it as changed and named `git checkout HEAD --` on it; a `contracts/` this user cannot search, or a copy at mode 000, ended the row in `could not be - run: [Errno 13]`. The pin is asked by `pin_read`, before `read_pin`, - whose `is_file()` raised the refused `stat` up to Python 3.13 and, from - 3.14, calls the pin missing. A copy is asked by `prepare`, and only a - refused read of a file inside the root is the repository's: any other - `OSError` is raised as it was. + run: [Errno 13]`. Where `read_pin` will not read the pin, `pin_read` + asks why (`pin_refusal_row`): `read_pin`'s `is_file()` raised the + refused `stat` up to Python 3.13 and, from 3.14, calls the pin missing. + A copy is asked by `prepare`, and only a refused read of a file inside + the root is the repository's: any other `OSError` is raised as it was. """ us = ctx.update_shape() # A PIN OR MANIFEST KEPT AS A LINK IS ASKED ABOUT FIRST, before anything @@ -5050,16 +5088,13 @@ def check_shape_currency(ctx: Context) -> Row: linked = linked_record_row(ctx, us) if linked is not None: return linked - refused = pin_read(ctx.root, SHAPE_PIN)[1] - if refused is not None: - return unreadable_copy_row(ctx, SHAPE_PIN, refused) args = argparse.Namespace(root=str(ctx.root), upstream=str(ctx.shape), at=None) try: pin, _rows = us.read_pin(ctx.root) - except Refusal as exc: - return unreadable_pin_row(ctx, exc) + except (Refusal, OSError) as exc: + return pin_refusal_row(ctx, exc) partial = partial_clone_row(ctx, str(pin["commit"]).lower()) if partial is not None: return partial @@ -6056,8 +6091,8 @@ def check_legs(ctx: Context) -> Row: reads. A person mends it, on a mode or an owner, so where no checkout and no lockstep fact comes first the NEXT is this doctor again and the `why` the gate's advice (`cannot_read_why`), as the `manifest kinds` and - `shape currency` rows give them; beside the gate's line, the advice is - said in its `why`. + `shape currency` rows give them; beside the checkout line or the gate's + line, the advice is said in its `why`, after what the line does. """ waiting = waiting_row(ctx, "legs", "legs", "the legs it declares are not known and none is " @@ -6185,6 +6220,9 @@ def check_legs(ctx: Context) -> Row: next_command, why = submodule_update_next( ctx.root, checkout, off_pin, "leg", after, rerun, kept=kept, relinked=relinked, nested=nested) + # A pin this user cannot read is a person's to mend, so its + # advice is said beside the line as beside the gate's (#847). + why += "".join(f". {sentence}" for sentence in said) elif lockstep: # NO CHECKOUT CURES IT: the lockstep gate, one definition of it # for this row and the `pins` row (`lockstep_next`), which mount @@ -10083,9 +10121,12 @@ def drifted_reasons(by_id: dict, detail: dict) -> list: where its pin is, and `validate-pins.py` refuses the pin by name, which is the `pins` row and INVALID. Such a leg carries `cannot_be_read` in the `legs` row, and is not counted. Every other leg whose HEAD is not - its pin is counted as before. Where there is none, the red row is drift - only where no leg carries `cannot_be_read`, as it always was: the gate - refuses that pin before it reports anything else, and INVALID names it. + its pin is counted as before. Where none is, the red row is drift by + main's count of one (an empty mount whose pin is gone has no HEAD and no + pin to differ) only where no leg carries `cannot_be_read`, whichever leg + made the row red: the gate refuses that pin before it reports anything + else, and INVALID names it; the run after the pin is mended says the + drift. Split out of `verdict_for` for #134. """ diff --git a/tests/test_doctor_unreadable_pin.py b/tests/test_doctor_unreadable_pin.py index 32fc616b..16607837 100644 --- a/tests/test_doctor_unreadable_pin.py +++ b/tests/test_doctor_unreadable_pin.py @@ -26,13 +26,16 @@ `why` the gate's advice for the cause, word for word, and the verdict is the gate's INVALID, never DRIFTED. -TWO KINDS OF CASE. The MODE cases change a real mode and run the doctor as +THREE KINDS OF CASE. The MODE cases change a real mode and run the doctor as a person does, so they skip on Windows, whose modes refuse no read, and as root, who reads a file at 000 (`CANNOT_RUN_AS_ROOT`). The STAND-IN cases run the doctor's rows in process with the refusal put in by hand, as #827's `refused_stat` rows do: a refused `stat` (`refused_stat`, which also answers `is_file()` as Python 3.14 does, False to any `OSError`), a refused read or -a refused listing (`refusing`). They run on every platform and as root. +a refused listing (`refusing`). They run on every platform and as root. The +PARITY cases hold the doctor's copies of the gate's lists and words equal to +the gate's and to `bump-leg.py`'s, each file loaded by path (rv860's SHOULD +1): a copy that drifts is the disagreement #847 was about. NOTHING HERE REACHES A NETWORK: the doctor's own fixtures, a scaffold into local bare remotes, and the session's `gh` guard for the `machine` row. @@ -44,11 +47,13 @@ import json import os import stat -from contextlib import contextmanager +import sys from pathlib import Path +from types import SimpleNamespace import pytest +from conftest import REPO, git, load_without_bytecode, rmtree #: #827's stand-in for a `stat` the operating system refuses, from the file #: that owns it (#383), as `test_validate_pins_unreadable_workflows_dir.py` #: takes it. @@ -62,6 +67,9 @@ ) #: The one skip for a mode that root reads through and Windows ignores. from test_validate_pins_unreadable_workflow import CANNOT_RUN_AS_ROOT +#: A mode for the block and the old one back after it, from the file that +#: owns it, as `test_validate_pins_named_and_spelled.py` takes it. +from test_validate_pins_unreadable_workflows_dir import mode_of SPEC_PIN = "contracts/spec-pin.yaml" CODE_PIN = "contracts/code-pin.yaml" @@ -70,6 +78,8 @@ #: The reason every stand-in gives, as `refused_stat` raises it. DENIED = "Permission denied" +#: The reason the stand-ins for another cause give. +IO_ERROR = "Input/output error" #: `validate-pins.py`'s advice for a refused permission, word for word #: (`_unreadable_file`, #712), and the doctor's own last sentence. @@ -83,10 +93,23 @@ "user can open back in its place, or take away one nobody wants, and make " "sure nothing is changing the directory while this runs. Then run this " "doctor again") +#: The same gate's advice for a DIRECTORY it cannot list for any other +#: cause (`_workflows_unlistable`, #434). +OTHER_FOLDER_WHY = ( + "Where the operating system gave another reason, put a directory this " + "user can list back in its place, and make sure nothing is changing it " + "while this runs. Then run this doctor again") #: What no row may name for a file this user cannot read (#847). CHECKOUT = "checkout HEAD --" +#: The gate as the template ships it, and `bump-leg.py`: the two files whose +#: lists and words the doctor copies (rv860's SHOULD 1). +GATE = REPO / "templates" / "assembly-root" / "scripts" / "validate-pins.py" +BUMP = REPO / "scripts" / "bump-leg.py" +#: The directory the gate's own words for an unlistable one are about. +WORKFLOWS_DIR = ".github/workflows" + def cannot(named: str, reason: str = DENIED) -> str: """The gate's line for a file it cannot reach or open.""" @@ -99,18 +122,6 @@ def leg_line(leg: str, pin: str, reason: str = DENIED) -> str: "compare HEAD against") -@contextmanager -def mode(path: Path, bits: int): - """`path` at `bits` for the block, and its own mode back after it, so - the basetemp can be removed.""" - held = stat.S_IMODE(path.stat().st_mode) - path.chmod(bits) - try: - yield - finally: - path.chmod(held) - - def doctored(standard: Path, root: Path) -> tuple[dict, str]: """`({id: row}, verdict)` from one `--json` run that printed no traceback.""" @@ -148,6 +159,20 @@ def in_process(standard: Path, project: Path): return module, module.Context(project.resolve()) +@pytest.fixture(scope="module") +def sources(): + """The doctor, the gate and `bump-leg.py` of this working tree, each + loaded by path with no bytecode written beside it, and `sys.path` put + back after: each of the three puts its own `scripts/` on it.""" + with pytest.MonkeyPatch.context() as patch: + patch.setattr(sys, "path", list(sys.path)) + return SimpleNamespace( + doctor=load_without_bytecode(REPO / "shape-doctor.py", + "doctor_unreadable_pin_parity"), + gate=load_without_bytecode(GATE, "gate_unreadable_pin_parity"), + bump=load_without_bytecode(BUMP, "bump_leg_unreadable_pin_parity")) + + # --- the mode cases: a real mode, the doctor run as a person runs it --------- @CANNOT_RUN_AS_ROOT @@ -156,7 +181,7 @@ def test_a_pin_at_mode_000_is_named_as_the_gate_names_it(standard, project): drift; `manifest kinds` says it too, and neither names a checkout.""" again = doctor_again(standard, project) denied = os.strerror(errno.EACCES) - with mode(project / SPEC_PIN, 0): + with mode_of(project, SPEC_PIN, 0): rows, verdict = doctored(standard, project) legs, kinds = rows["legs"], rows["manifest-kinds"] assert legs["status"] == "FINDING", legs @@ -187,7 +212,7 @@ def test_a_contracts_folder_this_user_cannot_search_is_named( contracts = project / "contracts" listed = sorted(path.name for path in contracts.iterdir() if path.suffix in (".yaml", ".yml")) - with mode(contracts, bits): + with mode_of(project, "contracts", bits): rows, verdict = doctored(standard, project) expected = (f"contracts: cannot be listed ({denied})" if bits == 0 else ", ".join(cannot(f"contracts/{name}", denied) @@ -212,7 +237,7 @@ def test_a_shape_pin_at_mode_000_gets_one_answer_from_both_rows( same NEXT and the same why for the same file, as they do for a pin that does not load (#291), and neither is `git checkout HEAD --`.""" denied = os.strerror(errno.EACCES) - with mode(project / SHAPE_PIN, 0): + with mode_of(project, SHAPE_PIN, 0): rows, verdict = doctored(standard, project) shape, kinds = rows["shape-currency"], rows["manifest-kinds"] assert shape["reason"] == ("the copies cannot be compared: " @@ -234,7 +259,7 @@ def test_a_pinned_copy_at_mode_000_stops_the_comparison_by_name( was `could not be run: [Errno 13] ...`, and `manifest kinds` a `git diff HEAD` whose why spoke of an uncommitted change. Both name it.""" denied = os.strerror(errno.EACCES) - with mode(project / NAMING_COPY, 0): + with mode_of(project, NAMING_COPY, 0): rows, _verdict = doctored(standard, project) assert rows["shape-currency"]["reason"] == ( "the copies cannot be compared: " + cannot(NAMING_COPY, denied)) @@ -289,12 +314,12 @@ def test_another_reason_is_given_the_gates_other_advice( `chmod`.""" module, ctx = in_process(standard, project) refusing(monkeypatch, "read_text", "spec-pin.yaml", - OSError(errno.EIO, "Input/output error")) + OSError(errno.EIO, IO_ERROR)) legs = module.check_legs(ctx) kinds = module.check_manifest_kinds(ctx) - assert legs.reason == leg_line("spec", SPEC_PIN, "Input/output error") + assert legs.reason == leg_line("spec", SPEC_PIN, IO_ERROR) assert legs.why == OTHER_WHY - assert kinds.reason == cannot(SPEC_PIN, "Input/output error") + assert kinds.reason == cannot(SPEC_PIN, IO_ERROR) assert kinds.why == OTHER_WHY @@ -302,8 +327,8 @@ def test_a_shape_pin_whose_stat_is_refused_is_not_called_missing( standard, project, monkeypatch): """`read_pin` asks `is_file()`, which raised a refused `stat` up to Python 3.13 and calls the pin missing from 3.14 (`update-pin-missing`). - The row asks first, and names the pin in the gate's words, with the - NEXT and why `manifest kinds` gives it.""" + The row asks why, and names the pin in the gate's words, with the NEXT + and why `manifest kinds` gives it.""" module, ctx = in_process(standard, project) refused_stat(monkeypatch, "shape-pin.yaml") shape = module.check_shape_currency(ctx) @@ -319,17 +344,39 @@ def test_a_shape_pin_whose_stat_is_refused_is_not_called_missing( "cannot_be_read": cannot(SHAPE_PIN)} +@pytest.mark.parametrize("error, why", [ + pytest.param(PermissionError(errno.EACCES, DENIED), PERMISSION_WHY, + id="permission"), + pytest.param(OSError(errno.EIO, IO_ERROR), OTHER_FOLDER_WHY, + id="another-reason"), +]) def test_a_contracts_folder_that_cannot_be_listed_is_named( - standard, project, monkeypatch): + standard, project, monkeypatch, error, why): """`iterdir()` refused: the folder, in the gate's words for a directory - it cannot list, where the row ended in `could not be run`.""" + it cannot list, where the row ended in `could not be run`. For another + reason than permission (rv860's NIT 3) it is told what the gate tells a + directory, to put one back in its place, and not a regular file.""" module, ctx = in_process(standard, project) - refusing(monkeypatch, "iterdir", "contracts", - PermissionError(errno.EACCES, DENIED)) + refusing(monkeypatch, "iterdir", "contracts", error) kinds = module.check_manifest_kinds(ctx) - assert kinds.reason == f"contracts: cannot be listed ({DENIED})" + assert kinds.reason == f"contracts: cannot be listed ({error.strerror})" + assert kinds.next_command == doctor_again(standard, project) + assert kinds.why == why + + +def test_a_file_this_user_cannot_read_is_said_first(standard, project, + monkeypatch): + """Beside another fault, here a manifest of a kind nothing registers, a + file this user cannot read is said first, as its NEXT comes first: the + row cannot say what kind it declares until it can be read.""" + module, ctx = in_process(standard, project) + (project / "contracts" / "stray.yaml").write_text( + "kind: nothing-registers-this\n", encoding="utf-8") + refused_read_of(monkeypatch, "spec-pin.yaml") + kinds = module.check_manifest_kinds(ctx) + assert kinds.reason.split("; ")[0] == cannot(SPEC_PIN), kinds.reason + assert "no validator registered for kind" in kinds.reason, kinds.reason assert kinds.next_command == doctor_again(standard, project) - assert kinds.why == PERMISSION_WHY def test_a_copy_whose_read_is_refused_stops_the_comparison_by_name( @@ -349,8 +396,9 @@ def test_a_copy_whose_read_is_refused_stops_the_comparison_by_name( def test_only_a_refused_read_inside_the_root_is_the_roots(standard, project): """`refused_in_root` names a file inside the root whose read was - refused, and nothing else: a file that is not there, or one outside the - root, such as the checkout of the standard's own, stays an error.""" + refused, and nothing else: a file that is not there, one outside the + root, such as the checkout of the standard's own, or a refusal that + names no file at all stays an error.""" module, ctx = in_process(standard, project) inside = str(ctx.root / NAMING_COPY) outside = str(standard.resolve() / NAMING_COPY) @@ -360,6 +408,36 @@ def test_only_a_refused_read_inside_the_root_is_the_roots(standard, project): errno.ENOENT, "No such file or directory", inside)) is None assert module.refused_in_root(ctx.root, PermissionError( errno.EACCES, DENIED, outside)) is None + assert module.refused_in_root(ctx.root, PermissionError( + errno.EACCES, DENIED)) is None + + +@pytest.mark.parametrize("raised", [ + pytest.param(lambda standard, _root: PermissionError( + errno.EACCES, DENIED, str(standard.resolve() / "update-shape.py")), + id="outside-the-root"), + pytest.param(lambda _standard, _root: PermissionError( + errno.EACCES, DENIED), id="no-file-named"), + pytest.param(lambda _standard, root: FileNotFoundError( + errno.ENOENT, "No such file or directory", str(root / NAMING_COPY)), + id="nothing-there"), +]) +def test_an_oserror_the_root_did_not_refuse_is_raised_as_it_was( + standard, project, monkeypatch, raised): + """Only a refused read of a file inside the root is the `shape currency` + row's FINDING. An `OSError` out of the comparison about a file + elsewhere, one that names no file, or one that says nothing is there is + raised as it was, which `run_checks` reports as a check that could not + be run: it is not a statement about the repository.""" + module, ctx = in_process(standard, project) + error = raised(standard, ctx.root) + + def prepare(_args): + raise error + + monkeypatch.setattr(ctx.update_shape(), "prepare", prepare) + with pytest.raises(type(error)): + module.check_shape_currency(ctx) def test_a_refused_stat_is_raised_on_every_python(standard, project, @@ -374,6 +452,209 @@ def test_a_refused_stat_is_raised_on_every_python(standard, project, stat.S_ISREG) is False +def test_beside_the_checkout_line_the_why_gives_the_advice_too( + standard, project, monkeypatch): + """One leg's pin cannot be read, and the other leg has a commit past its + pin while its gitlink is at the pin, which the checkout line cures. The + line is the NEXT, and its why says what it does, that the next run names + what it leaves (the pin it cannot read), and then the gate's advice, as + beside the gate's line (rv860's NIT 5).""" + module, ctx = in_process(standard, project) + git("commit", "-q", "--allow-empty", "-m", "past its pin", + cwd=project / "code") + refused_read_of(monkeypatch, "spec-pin.yaml") + legs = module.check_legs(ctx) + assert legs.next_command.endswith(" -- code"), legs.next_command + assert legs.why.endswith( + module.rerun_sentence("leg") + ". " + PERMISSION_WHY), legs.why + + +def test_beside_an_unborn_head_the_doctor_again_still_names_it( + standard, project, monkeypatch): + """One leg's pin cannot be read, and the other leg's HEAD names a branch + with no commit, which no line cures: the NEXT is this doctor again, and + its why is the gate's advice and then what the unborn HEAD needs.""" + module, ctx = in_process(standard, project) + git("symbolic-ref", "HEAD", "refs/heads/never-made", + cwd=project / "code") + refused_read_of(monkeypatch, "spec-pin.yaml") + legs = module.check_legs(ctx) + assert legs.next_command == doctor_again(standard, project) + assert legs.why == (PERMISSION_WHY + ". " + + module.unborn_sentence("code", "leg")) + + +def test_where_the_gates_line_cannot_be_built_the_advice_is_still_said( + standard, project, monkeypatch): + """`lockstep_next` names no line where no `PYTHONPATH` can name the + template's `scripts/` (#278), a standard checked out under a directory + whose name holds `os.pathsep`: stood in for here, with the root's own + copy of the gate gone so the template's is the one asked. The missing + pin's lockstep fact then has no line, and the refused pin's advice is + still the row's why.""" + module, ctx = in_process(standard, project) + (project / CODE_PIN).unlink() + (project / "scripts" / "validate-pins.py").unlink() + monkeypatch.setattr(module, "pythonpath_unnamable", + lambda _shape, _own_copy: "no PYTHONPATH names it") + refused_read_of(monkeypatch, "spec-pin.yaml") + legs = module.check_legs(ctx) + assert legs.next_command is None + assert legs.why == PERMISSION_WHY + + +# --- the doctor's own helpers, asked directly ----------------------------- + +@pytest.mark.parametrize("contracts", [ + pytest.param(None, id="no-contracts"), + pytest.param(b"a file, not a folder\n", id="contracts-a-file"), +]) +def test_a_root_without_a_contracts_folder_refuses_nothing( + sources, tmp_path, contracts): + """A root with no `contracts/`, or with a `contracts` that is a file, has + nothing under it to list and nothing this user cannot read: `is_a` is + asked before `iterdir`, which raises `FileNotFoundError` or + `NotADirectoryError` there.""" + if contracts is not None: + (tmp_path / "contracts").write_bytes(contracts) + assert sources.doctor.manifest_candidates(tmp_path) == ([], []) + + +def test_is_a_answers_for_the_kind_and_for_a_name_no_system_takes( + sources, tmp_path): + """`is_a` answers whether the path is of `kind`, so a folder named like a + pin is no pin; and a name the operating system cannot take at all, a + NUL in it, is nothing there, as `is_file()` answered it, and not a + `ValueError` out of the row.""" + folder = tmp_path / "spec-pin.yaml" + folder.mkdir() + assert sources.doctor.is_a(folder, stat.S_ISREG) is False + assert sources.doctor.is_a(folder, stat.S_ISDIR) is True + assert sources.doctor.is_a(tmp_path / "a\0b.yaml", stat.S_ISREG) is False + + +@pytest.mark.parametrize("winerror", [21, 123, 1921]) +def test_a_windows_error_that_says_nothing_is_there_is_no_refusal( + sources, winerror): + """Windows' "not ready", "invalid name" and "cannot resolve" say nothing + is there, whatever errno they carry, as `is_file()` answered them; the + same errno with no such `winerror` is a refusal. Built by hand, since + only Windows raises one: an `OSError` with `winerror` set on it.""" + error = OSError(errno.EINVAL, "Invalid argument") + error.winerror = winerror + assert sources.doctor.refused_read(error) is None + assert sources.doctor.refused_read( + OSError(errno.EINVAL, "Invalid argument")) is not None + + +@pytest.mark.parametrize("beside, expected", [ + pytest.param([], ["1 leg(s) not at the pin"], id="alone"), + pytest.param(["spec-pin.yaml"], [], id="beside-a-pin-it-cannot-read"), +]) +def test_a_red_legs_row_with_no_leg_off_its_pin(standard, project, + monkeypatch, beside, + expected): + """An empty `code` mount whose pin is gone has no HEAD and no pin, so no + leg's HEAD is off its pin, and the red row is drift by the fallback + count, `len(off) or 1`, as on main. Beside a pin this user cannot read + the fallback is not said: the gate refuses that pin before anything + else, and its INVALID is the verdict (`drifted_reasons`, rv860's NIT 4, + documented as intended).""" + module, ctx = in_process(standard, project) + (project / CODE_PIN).unlink() + rmtree(project / "code") + (project / "code").mkdir() + for name in beside: + refused_read_of(monkeypatch, name) + legs = module.check_legs(ctx) + assert legs.status == "FINDING", legs + assert module.drifted_reasons({"legs": legs}, {}) == expected + + +# --- parity: the doctor's copies of the gate's lists and words (SHOULD 1) ---- + +@pytest.mark.parametrize("owner", ["gate", "bump"]) +def test_what_the_doctor_calls_nothing_there_is_what_the_gate_calls_so( + sources, owner): + """`ABSENT_ERRNOS` and `ABSENT_WINERRORS` copy `_ABSENT_ERRNOS` and + `_ABSENT_WINERRORS` from the gate and from `bump-leg.py`. A copy that + drifts makes the doctor call missing a pin the gate refuses by name, or + name one the gate calls missing.""" + other = getattr(sources, owner) + assert (sources.doctor.ABSENT_ERRNOS, sources.doctor.ABSENT_WINERRORS) \ + == (other._ABSENT_ERRNOS, other._ABSENT_WINERRORS) + + +def test_the_gates_other_reason_is_the_doctors_and_the_space_it_joins_on( + sources): + """The gate's `OTHER_REASON` is the doctor's and one space: the gate + joins its sentences with `+`, so each ends on the space before the next, + and the doctor joins with `" ".join`, so none does. Neither is changed + to match the other; the space is asked for exactly, not stripped.""" + assert sources.gate.OTHER_REASON == sources.doctor.OTHER_REASON + " " + + +@pytest.mark.parametrize("error, sentence", [ + pytest.param(PermissionError(errno.EACCES, DENIED), "PERMISSION_ADVICE", + id="permission"), + pytest.param(OSError(errno.EIO, IO_ERROR), "OTHER_REASON", + id="another-reason"), +]) +def test_the_doctor_gives_the_gates_advice_word_for_word(sources, error, + sentence): + """The gate's permission advice is written inline in `_unreadable_file`, + so it is read as a person reads it: the remedy the gate's own function + builds for the cause, which ends on the doctor's sentence for that + cause, one space and the gate's `RUN_AGAIN`.""" + remedy = sources.gate._unreadable_file("yaml-unreadable", SPEC_PIN, + error).remediation + said = getattr(sources.doctor, sentence) + assert remedy.endswith(f". {said} {sources.gate.RUN_AGAIN}"), remedy + + +def test_the_doctor_gives_the_gates_folder_advice_word_for_word(sources): + """The gate's sentence for a directory it cannot list for another reason + is inline in `_workflows_unlistable`, and read the same way.""" + root = Path("root") + remedy = sources.gate._workflows_unlistable( + root, root / WORKFLOWS_DIR, OSError(errno.EIO, IO_ERROR)).remediation + said = sources.doctor.OTHER_REASON_FOLDER + assert remedy.endswith(f". {said} {sources.gate.RUN_AGAIN}"), remedy + + +@pytest.mark.parametrize("errors", [ + pytest.param([PermissionError(errno.EACCES, DENIED)], id="permission"), + pytest.param([OSError(errno.EIO, IO_ERROR)], id="another-reason"), + pytest.param([PermissionError(errno.EACCES, DENIED), + OSError(errno.EIO, IO_ERROR)], id="both"), +]) +def test_the_doctor_gives_bump_legs_advice_for_each_cause(sources, errors): + """`bump-leg.py`'s `_cannot_open_advice` (#383), whose words the gate + took: each cause's sentence once, the permission's first, for files + whose causes are mixed as much as for one.""" + assert sources.doctor.cannot_read_why(errors) == ( + sources.bump._cannot_open_advice(errors) + + "Then run this doctor again") + + +@pytest.mark.parametrize("error", [ + pytest.param(PermissionError(errno.EACCES, DENIED), id="permission"), + pytest.param(OSError(errno.EIO, IO_ERROR), id="another-reason"), + pytest.param(OSError(), id="no-reason-given"), +]) +def test_the_doctor_names_a_file_and_a_folder_in_the_gates_words(sources, + error): + """`cannot_be_read` is the gate's `_cannot_read`, and `cannot_be_listed` + the line of its `_workflows_unlistable`, the error's name standing in + where the operating system gives no reason.""" + root = Path("root") + assert sources.doctor.cannot_be_read(SPEC_PIN, error) == \ + sources.gate._cannot_read(SPEC_PIN, error) + assert sources.doctor.cannot_be_listed(WORKFLOWS_DIR, error) == \ + sources.gate._workflows_unlistable(root, root / WORKFLOWS_DIR, + error).detail + + # --- controls: what must not change ------------------------------------------ def test_a_missing_pin_keeps_its_words_and_is_still_drift(standard, project): @@ -389,6 +670,19 @@ def test_a_missing_pin_keeps_its_words_and_is_still_drift(standard, project): "1 leg(s) not at the pin"] +def test_a_pin_that_does_not_load_for_another_reason_keeps_its_words( + standard, project): + """A pin this user can read that is not UTF-8 does not load, and is no + file this user cannot read: `pin_read` gives no cause for it, so the + row keeps main's words, as `validate-pins.py` keeps its own refusal.""" + module, ctx = in_process(standard, project) + (project / SPEC_PIN).write_bytes(b"commit: \xff\n") + legs = module.check_legs(ctx) + assert legs.reason == (f"spec: {SPEC_PIN} carries no 40-hex commit to " + "compare HEAD against") + assert "cannot_be_read" not in legs.detail["legs"][0] + + def test_beside_a_lockstep_fact_the_gate_stays_the_next(standard, project, monkeypatch): """One leg's pin cannot be read and the other's is not there: the From 1c4e3cdff954d7031b49784ee8cb905caa5050f7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 04:10:52 +0000 Subject: [PATCH 3/3] shape-doctor.py and its tests: RV860c's round 2 on #860 (#847) NIT A. `check_legs` asks `pin_read` why a pin gave no commit BEFORE the mount branches, not only where every one of them passed. A leg whose mount has no gitlink, is empty, has no `.git` of its own, or has a HEAD git cannot read or one on a branch with no commit, and whose pin this user cannot read, now says the mount's fact and then the gate's line, `: , and contracts/-pin.yaml: cannot be read ()`. The leg carries `cannot_be_read`, the pin's advice follows whatever line is the NEXT, and the verdict is the gate's INVALID, where an emptied `spec` beside its pin at 000 was `DRIFTED (1 leg(s) not at the pin)`. Six mode cases, one per mount branch, and one stand-in case. NIT B. `test_another_reason_is_given_the_gates_other_advice` refuses the pin's `stat` as well as its read, so a `stat` refused for another cause than permission is held to be named, not called missing (RV860c's mutant M5). 52 cases in all, up from 44. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 53 ++++++++++------ tests/test_doctor_unreadable_pin.py | 97 +++++++++++++++++++++++++++-- 2 files changed, 126 insertions(+), 24 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index e6312129..68015057 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -6086,13 +6086,18 @@ def check_legs(ctx: Context) -> Row: leg as off its pin: DRIFTED, for a leg nobody moved, where `validate-pins.py` refuses the pin by name. `pin_read` asks why the pin gave no commit, and a `stat` or a read the operating system refused is - `: contracts/-pin.yaml: cannot be read ()`, the - gate's line, kept in the leg's `cannot_be_read`, which `drifted_reasons` - reads. A person mends it, on a mode or an owner, so where no checkout - and no lockstep fact comes first the NEXT is this doctor again and the - `why` the gate's advice (`cannot_read_why`), as the `manifest kinds` and - `shape currency` rows give them; beside the checkout line or the gate's - line, the advice is said in its `why`, after what the line does. + `: contracts/-pin.yaml: cannot be read ()`, the gate's + line, kept in the leg's `cannot_be_read`, which `drifted_reasons` reads. It + is asked BEFORE the mount (review of PR #860): where the mount has no + gitlink, is empty, has no `.git` of its own, or has a HEAD git cannot read + or one on a branch with no commit, that fact is said first and the pin's + line follows it (`, and `), so the leg is never drift for + a pin the gate refuses by name. A person mends it, on a mode or an owner, + so where no checkout and no lockstep fact comes first the NEXT is this + doctor again and the `why` the gate's advice (`cannot_read_why`), as the + `manifest kinds` and `shape currency` rows give them; beside the checkout + line or the gate's line, the advice is said in its `why`, after what the + line does. """ waiting = waiting_row(ctx, "legs", "legs", "the legs it declares are not known and none is " @@ -6122,7 +6127,12 @@ def check_legs(ctx: Context) -> Row: role = str(leg.get("role") or "?") rel = str(leg.get("path") or role) mount = ctx.root / rel - pinned = pin_commit(ctx.root, f"contracts/{role}-pin.yaml") + pin = f"contracts/{role}-pin.yaml" + pinned = pin_commit(ctx.root, pin) + # ASKED BEFORE THE MOUNT (#847, review of PR #860): asked only where + # every mount branch passed, a pin this user cannot read went unnamed + # beside an empty mount, and the verdict read the leg as drift. + refusal = pin_read(ctx.root, pin)[1] if pinned is None else None gitlink = recorded_gitlink(ctx.root, rel) populated = mount.is_dir() and any(mount.iterdir()) own_git = (mount / ".git").exists() @@ -6133,8 +6143,14 @@ def check_legs(ctx: Context) -> Row: "gitlink": gitlink, "populated": populated, "head": head, "own_git": own_git, "unborn": unborn} rows.append(entry) + if refusal is not None: + entry["cannot_be_read"] = cannot_be_read(pin, refusal) + refused.append(refusal) problem = None cures = False + #: Whether the problem is the pin's line, which no mount fact came + #: before. + named = False if gitlink is None: problem = (f"{rel}: no gitlink recorded, so this leg is not " "mounted as a submodule at all") @@ -6154,18 +6170,14 @@ def check_legs(ctx: Context) -> Row: elif head is None: problem = f"{rel}: populated, but git cannot read a HEAD in it" lockstep.append(rel) + elif pinned is None and refusal is None: + problem = (f"{rel}: {pin} carries no 40-hex commit to compare " + "HEAD against") + lockstep.append(rel) elif pinned is None: - pin = f"contracts/{role}-pin.yaml" - cause = pin_read(ctx.root, pin)[1] - if cause is None: - problem = (f"{rel}: {pin} carries no 40-hex commit to compare " - "HEAD against") - lockstep.append(rel) - else: - entry["cannot_be_read"] = cannot_be_read(pin, cause) - problem = (f"{rel}: {entry['cannot_be_read']}, so there is no " - "pinned commit to compare HEAD against") - refused.append(cause) + problem = (f"{rel}: {entry['cannot_be_read']}, so there is no " + "pinned commit to compare HEAD against") + named = True elif head != pinned: problem = (f"{rel}: checked out at {head[:12]}, pinned at " f"{pinned[:12]}") @@ -6197,6 +6209,9 @@ def check_legs(ctx: Context) -> Row: # what the next run names. if pinned is None or gitlink != pinned: rerun = True + if refusal is not None and not named: + # A MOUNT FACT CAME FIRST: the pin is named beside it. + problem += f", and {entry['cannot_be_read']}" if problem: problems.append(problem) detail = {"legs": rows} diff --git a/tests/test_doctor_unreadable_pin.py b/tests/test_doctor_unreadable_pin.py index 16607837..6afe17a8 100644 --- a/tests/test_doctor_unreadable_pin.py +++ b/tests/test_doctor_unreadable_pin.py @@ -54,6 +54,11 @@ import pytest from conftest import REPO, git, load_without_bytecode, rmtree +#: The ways a leg's mount can be wrong, from the file that owns them. +from test_doctor_legs_members_next import ( + make_head_unreadable, make_mount_with_no_git_of_its_own, make_no_gitlink, + unbootstrap, +) #: #827's stand-in for a `stat` the operating system refuses, from the file #: that owns it (#383), as `test_validate_pins_unreadable_workflows_dir.py` #: takes it. @@ -268,6 +273,68 @@ def test_a_pinned_copy_at_mode_000_stops_the_comparison_by_name( assert_no_checkout(rows) +# --- a pin this user cannot read beside a mount fact (review of PR #860) ---- + +def spec_emptied(root: Path) -> None: + """`spec` as an unbootstrapped clone has it: empty, not registered, + and no gitdir kept.""" + unbootstrap(root, "spec") + + +def spec_unlinked(root: Path) -> None: + """`spec`'s files with no `.git` of their own, its gitdir kept, which + the checkout line links again.""" + (root / "spec" / ".git").unlink() + + +def spec_unborn(root: Path) -> None: + """`spec`'s HEAD on a branch with no commit.""" + git("symbolic-ref", "HEAD", "refs/heads/never-made", cwd=root / "spec") + + +#: `(how the mount is made wrong, the words the row says of it first)`, one +#: for each branch `check_legs` takes before it reaches the pin. +MOUNT_FACTS = [ + pytest.param(make_no_gitlink, "no gitlink recorded, so this leg is not " + "mounted as a submodule at all", id="no-gitlink"), + pytest.param(spec_emptied, "the gitlink is there and the directory is " + "empty -- an unbootstrapped clone", id="empty"), + pytest.param(spec_unlinked, "populated, but with no .git of its own, so " + "git cannot read a HEAD in it", id="no-git-of-its-own-kept"), + pytest.param(make_mount_with_no_git_of_its_own, "populated, but with no " + ".git of its own, so git cannot read a HEAD in it", + id="no-git-of-its-own-gone"), + pytest.param(spec_unborn, "HEAD names the branch never-made, which has " + "no commit", id="unborn-head"), + pytest.param(make_head_unreadable, "populated, but git cannot read a " + "HEAD in it", id="head-unreadable"), +] + + +@CANNOT_RUN_AS_ROOT +@pytest.mark.parametrize("unmount, fact", MOUNT_FACTS) +def test_a_pin_this_user_cannot_read_is_named_beside_the_mount( + standard, project, unmount, fact): + """A leg whose mount is wrong AND whose pin this user cannot read: the + mount's fact is said first, the gate's line for the pin follows it, the + leg carries `cannot_be_read`, the WHY gives the gate's advice beside + whatever line is the NEXT, and the verdict is the gate's INVALID, never + DRIFTED. The pin was asked only where every mount branch passed, so an + empty mount hid it and the verdict was `DRIFTED (1 leg(s) not at the + pin)` (review of PR #860).""" + denied = os.strerror(errno.EACCES) + unmount(project) + with mode_of(project, SPEC_PIN, 0): + rows, verdict = doctored(standard, project) + legs = rows["legs"] + spec = legs["detail"]["legs"][0] + assert spec["cannot_be_read"] == cannot(SPEC_PIN, denied) + assert legs["reason"] == f"spec: {fact}, and {cannot(SPEC_PIN, denied)}" + assert PERMISSION_WHY in legs["why"], legs["why"] + assert verdict == "INVALID (pins, manifest-kinds)" + assert_no_checkout(rows) + + # --- the stand-in cases: in process, on every platform and as root ---------- def refused_stat_of(monkeypatch, name: str) -> None: @@ -307,13 +374,15 @@ def test_a_refused_pin_is_named_on_every_python(standard, project, assert kinds.detail["cannot_be_read"] == [cannot(SPEC_PIN)] +@pytest.mark.parametrize("method", ["stat", "read_text"]) def test_another_reason_is_given_the_gates_other_advice( - standard, project, monkeypatch): - """A read refused for a reason that is no permission (an I/O error): the - system's reason, and the gate's other advice, which is not told to - `chmod`.""" + standard, project, monkeypatch, method): + """A `stat` or a read refused for a reason that is no permission (an + I/O error): the system's reason, and the gate's other advice, which is + not told to `chmod`. The `stat` is asked first, as the gate asks it, so + a pin whose `stat` fails for any reason is named, not called missing.""" module, ctx = in_process(standard, project) - refusing(monkeypatch, "read_text", "spec-pin.yaml", + refusing(monkeypatch, method, "spec-pin.yaml", OSError(errno.EIO, IO_ERROR)) legs = module.check_legs(ctx) kinds = module.check_manifest_kinds(ctx) @@ -484,6 +553,24 @@ def test_beside_an_unborn_head_the_doctor_again_still_names_it( + module.unborn_sentence("code", "leg")) +def test_beside_an_empty_mount_the_pin_is_still_named(standard, project, + monkeypatch): + """The empty mount of the case above, in process: the checkout line is + the NEXT, the pin is named beside the mount, its advice follows what the + line does, and the leg is no drift.""" + module, ctx = in_process(standard, project) + spec_emptied(project) + refused_read_of(monkeypatch, "spec-pin.yaml") + legs = module.check_legs(ctx) + assert legs.reason == ("spec: the gitlink is there and the directory is " + "empty -- an unbootstrapped clone, and " + f"{cannot(SPEC_PIN)}") + assert legs.detail["legs"][0]["cannot_be_read"] == cannot(SPEC_PIN) + assert legs.next_command.endswith(" -- spec"), legs.next_command + assert legs.why.endswith(". " + PERMISSION_WHY), legs.why + assert module.drifted_reasons({"legs": legs}, {}) == [] + + def test_where_the_gates_line_cannot_be_built_the_advice_is_still_said( standard, project, monkeypatch): """`lockstep_next` names no line where no `PYTHONPATH` can name the