From f3247e540330e794163f217f6969625adf08c8a0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 21:56:29 +0000 Subject: [PATCH 1/2] shape-doctor.py: the `leg shape files` note for a leg file this user cannot read gives the gate's advice as its WHY, and each branch #975 added for a mount this user cannot reach is built by a test (#1175) `shape_file_state` keeps the `OSError` of a refused `stat` or read beside its words, `leg_shape_comparison` hands each one back, and `check_leg_shape_files` gives the note `cannot_read_why` of them: the permission advice or the other one, ending "Then run this doctor again". Beside a missing file's line the advice follows that line's why. The NEXT stays None and the row a note, so the verdict does not change. A whole mount gets no why here: the `legs` row names it and gives its advice. tests/test_doctor_unreadable_paths.py builds the five branches rv975's mutants MI, MJ, MK1, MK2 and ML found unbuilt: a leg with no gitlink over a mount it cannot list, and a holder with two members, one unreachable beside one a checkout cures, by the holder's own bootstrap and by the git line, in process and with a real mode 000. ML's term is not dead: the `rerun_sentence` it adds is not the advice's own "run this doctor again", and the cases tell the two apart. The note's why is asserted word for word, for a permission and for an I/O error, for a refused `stat` and a refused read, beside a missing file, and under a real mode 000. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 70 ++++++--- tests/test_doctor_unreadable_paths.py | 214 +++++++++++++++++++++++++- 2 files changed, 256 insertions(+), 28 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 118558ed..41c43191 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -7694,8 +7694,9 @@ def copy_command(source, target, platform: str | None = None) -> str: LABEL_LEG_SHAPE_FILES = "leg shape files" -def shape_file_state(here: Path, there: Path, name: str) -> str: - """What ONE leg shape file is, against the template's copy of it. +def shape_file_state(here: Path, there: Path, name: str) -> tuple: + """`(what ONE leg shape file is, against the template's copy of it, the + `OSError` of a read the operating system refused or None)`. The five answers this returns are the whole vocabulary of the row above, and they are five because a byte comparison is only meaningful for two of @@ -7714,31 +7715,36 @@ def shape_file_state(here: Path, there: Path, name: str) -> str: `stat` or read the operating system refuses (#430). It was `absent` on Python 3.14, with a `cp` of the template over a file that is there, and up to 3.13 the row `could not be run`. `there` is the standard's own copy. + + ITS ERROR IS KEPT BESIDE IT (#1175): the words carry only the reason, and + the row's `why` is the gate's advice for the cause (`cannot_read_why`), + which a permission and an I/O error are told apart by. """ try: if not is_a(here, stat.S_ISREG): - return "absent" + return "absent", None except OSError as exc: - return f"cannot be read ({os_reason(exc)})" + return f"cannot be read ({os_reason(exc)})", exc if name in LEG_RENDERED: - return "rendered" + return "rendered", None if not there.is_file(): - return "present (the template has none)" + return "present (the template has none)", None try: mine = here.read_bytes() except OSError as exc: - return f"cannot be read ({os_reason(exc)})" + return f"cannot be read ({os_reason(exc)})", exc if mine == there.read_bytes(): - return "identical" - return "differs" + return "identical", None + return "differs", None def leg_shape_comparison(ctx: Context, role: str, rel: str) -> tuple: """ONE leg's shape files, and the ones that are not there at all. - Returns the `--json` entry for the leg and the `(role, leg, file)` triples - the row names as missing, so that the caller adds them up rather than - being handed a half-built dictionary to finish. + Returns the `--json` entry for the leg, the `(role, leg, file)` triples + the row names as missing and the `OSError` of each file this user cannot + read (#1175), so that the caller adds them up rather than being handed a + half-built dictionary to finish. EVERY EARLY RETURN IS A LEG THERE IS NOTHING TO COMPARE, and neither of them is a fault of the repository's: a leg nobody has bootstrapped has no @@ -7754,20 +7760,25 @@ def leg_shape_comparison(ctx: Context, role: str, rel: str) -> tuple: template = ctx.shape / "templates" / f"{role}-root" populated, _own_git, unread = mount_read(ctx.root, rel) if unread is not None: - return {"role": role, "state": unread[0], MOUNT_UNREAD: unread[0]}, [] + return ({"role": role, "state": unread[0], MOUNT_UNREAD: unread[0]}, + [], []) if not populated: - return {"role": role, "state": "not populated"}, [] + return {"role": role, "state": "not populated"}, [], [] if not template.is_dir(): return {"role": role, - "state": f"no templates/{role}-root/ to compare against"}, [] + "state": f"no templates/{role}-root/ to compare against"}, [], [] files: dict[str, str] = {} absent: list[tuple[str, str, str]] = [] + refused: list[OSError] = [] for name in LEG_SHAPE_FILES: - state = shape_file_state(mount / name, template / name, name) + state, exc = shape_file_state(mount / name, template / name, name) files[name] = state if state == "absent": absent.append((role, rel, name)) - return {"role": role, "state": "compared", "files": files}, absent + if exc is not None: + refused.append(exc) + return ({"role": role, "state": "compared", "files": files}, absent, + refused) def leg_shape_summary(rel: str, entry: dict) -> str: @@ -7848,6 +7859,15 @@ def check_leg_shape_files(ctx: Context) -> Row: is a `note`, because what is missing is a read, the `legs` row asserts the mount, and "compared" is not a claim to make about a file nobody read. It offers no copy: the file is there. + + A FILE THIS USER CANNOT READ IS TOLD THE CURE (#1175): the note's `why` + is the gate's advice for the cause (`cannot_read_why`), as every other + path this doctor cannot read is told it, and beside a missing file's + copy it follows what that line's `why` says. A mount is not: the `legs` + row names it and gives its advice. The NEXT stays None and the row stays + a note, so the verdict is what it was: no leg file gates anything, so a + file nobody could read cannot change it, and the advice ends by saying + to run this doctor again. """ needs = "there are no legs to compare" waiting = (waiting_row(ctx, "leg-shape-files", LABEL_LEG_SHAPE_FILES, @@ -7862,15 +7882,19 @@ def check_leg_shape_files(ctx: Context) -> Row: "no legs to compare", None, {"legs": []}) per_leg: dict[str, dict] = {} missing: list[tuple[str, str, str]] = [] + #: The `OSError` of each leg file this user cannot read (#1175). + refused: list[OSError] = [] for leg in legs: role = str(leg.get("role") or "?") rel = str(leg.get("path") or role) - entry, absent = leg_shape_comparison(ctx, role, rel) + entry, absent, unread_files = leg_shape_comparison(ctx, role, rel) per_leg[rel] = entry missing.extend(absent) + refused.extend(unread_files) detail = {"legs": per_leg, "compared": list(LEG_SHAPE_FILES)} summary = "; ".join(leg_shape_summary(rel, entry) for rel, entry in per_leg.items()) + advice = cannot_read_why(refused) if refused else None if missing: named = ", ".join(f"{leg}/{file}" for _, leg, file in missing) # `note`, NOT `FINDING`. This is the row that printed `INVALID` @@ -7882,14 +7906,14 @@ def check_leg_shape_files(ctx: Context) -> Row: # rule this standard never made. The difference is worth a reader's # eye and is not a verdict. fix, why = missing_shape_file_fix(ctx, missing) + # A file this user cannot read beside it: its advice follows (#1175). + why = ". ".join(said for said in (why, advice) if said) or None return Row("leg-shape-files", LABEL_LEG_SHAPE_FILES, NOTE, f"{summary} (missing: {named})", fix, detail, why=why) - unread = any(MOUNT_UNREAD in entry - or any(state.startswith("cannot be read") - for state in (entry.get("files") or {}).values()) - for entry in per_leg.values()) + unread = bool(refused) or any(MOUNT_UNREAD in entry + for entry in per_leg.values()) return Row("leg-shape-files", LABEL_LEG_SHAPE_FILES, - NOTE if unread else OK, summary, None, detail) + NOTE if unread else OK, summary, None, detail, why=advice) # --------------------------------------------------------------------------- diff --git a/tests/test_doctor_unreadable_paths.py b/tests/test_doctor_unreadable_paths.py index 9685c49f..b3b4ab38 100644 --- a/tests/test_doctor_unreadable_paths.py +++ b/tests/test_doctor_unreadable_paths.py @@ -32,6 +32,12 @@ verdict is INVALID naming the red rows and never DRIFTED; and a root this user cannot search is refused by name, exit 3, before any row. +#1175 BUILDS THE BRANCHES NO CASE REACHED: a leg with no gitlink over a mount +this user cannot reach, and a member it cannot reach beside one a checkout +cures, by the holder's own bootstrap and by the git line. And the `leg shape +files` note for a leg file it cannot read now carries the gate's advice as +its WHY, as every other path here does. + NOT HERE: the `shape currency` row's `copy-missing` under 3.14 for a `scripts/` or a `.github/workflows/` this user cannot search comes from `update-shape.py`'s comparison, which #430's slice B fixes. No test here @@ -62,9 +68,12 @@ #: #827's stand-in for a `stat` the operating system refuses, from the file #: that owns it (#383). from test_bump_leg_unopenable import refused_stat -#: A private copy of the session's holder, from the file that owns the -#: `members` row's tests. -from test_doctor_legs_members_next import holder_copy +#: A private copy of the session's holder, and the ways a leg or a member can +#: be wrong, from the file that owns the `legs` and `members` rows' tests. +from test_doctor_legs_members_next import ( + add_member_row, holder_copy, holder_without, make_no_gitlink, + member_unbootstrapped, +) #: This doctor's own line for a root, as `unreadable_next` spells it. from test_doctor_recorded_answers import doctor_again #: #847's helpers and the gate's advice, word for word, from the file that @@ -75,7 +84,8 @@ ) #: The doctor's fixtures, from the module that owns them. from test_shape_doctor import ( # noqa: F401 - FAMILY_MEMBER, doctor, holder, project, scaffolded_here, standard, + FAMILY_MEMBER, doctor, holder, project, record_gitlink, 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 @@ -89,6 +99,8 @@ MANIFEST_TEMPLATE = "templates/assembly-root/scripts/validate-manifest.py" PINS_GATE = "scripts/validate-pins.py" MEMBER = f"members/{FAMILY_MEMBER}" +#: A second member, which the cases beside a checkout make unreachable. +GHOST = "members/Ghost" #: What every row said of a path it could not reach, and what none may say. COULD_NOT_RUN = "could not be run" #: The `members` row's words for an empty mount, which a member this user @@ -119,6 +131,42 @@ def assert_nothing_could_not_run(rows: dict) -> None: assert COULD_NOT_RUN not in row["reason"], row +def with_a_member_beside_one_a_checkout_cures(root: Path) -> None: + """A holder with TWO members (#1175): `Ghost`, at the first one's + commit, with a `.gitmodules` section that gives its path and the first + one's url and an empty mount that each case makes unreachable; and the + first member emptied, so a checkout cures it and the row names a line. + The session's holder has one member, so no other case needs one.""" + add_member_row(root, "Ghost") + record_gitlink(root, GHOST, git("rev-parse", f"HEAD:{MEMBER}", + cwd=root).stdout.strip()) + url = git("config", "-f", ".gitmodules", f"submodule.{MEMBER}.url", + cwd=root).stdout.strip() + for key, value in (("path", GHOST), ("url", url)): + git("config", "-f", ".gitmodules", f"submodule.{GHOST}.{key}", value, + cwd=root) + (root / GHOST).mkdir() + member_unbootstrapped(root) + + +def beside_the_checkout(module, why: str) -> bool: + """Whether a `members` row's why ends as it must beside a member this + user cannot reach: the sentence that the next run names what the line + leaves (`rerun_sentence`, which only the refusal asks for here), then + the gate's advice. The advice's own "Then run this doctor again" is not + that sentence, so the two are told apart.""" + return why.endswith(module.rerun_sentence("member") + ". " + + PERMISSION_WHY) + + +def bootstrap_line(module, root: Path) -> str: + """The holder's own bootstrap, as the `members` row names it.""" + resolved = root.resolve() + return (f"{module.PYTHON} " + f"{module.quote_arg(resolved / 'scripts' / 'bootstrap.py')} " + f"--root {module.quote_arg(resolved)}") + + # --- the mode cases: a real mode, the doctor run as a person runs it --------- @CANNOT_RUN_AS_ROOT @@ -205,6 +253,48 @@ def test_a_member_this_user_cannot_reach_is_named_and_no_drift( assert_nothing_could_not_run(rows) +@CANNOT_RUN_AS_ROOT +def test_a_member_at_mode_000_beside_one_a_checkout_cures(standard, holder, + tmp_path): + """A member's mount at 000 beside a member the holder's own bootstrap + cures (#1175): the row names both, the bootstrap is the NEXT, and its why + ends on the advice for the mount. Up to Python 3.13 the `.gitmodules` + asked in each member to say what the bootstrap carries raised the + refused `stat` out of the row, so that member is not asked.""" + root = holder_copy(holder, tmp_path) + with_a_member_beside_one_a_checkout_cures(root) + module = in_process(standard, root)[0] + with mode_of(root, GHOST, 0): + rows, _verdict = doctored(standard, root) + members = rows["members"] + assert members["status"] == "FINDING", members + assert members["reason"].split("; ")[:2] == [ + f"{MEMBER}: {UNBOOTSTRAPPED} submodule", + unread_mount(listed(GHOST, os.strerror(errno.EACCES)))] + assert members["next"] == bootstrap_line(module, root) + assert beside_the_checkout(module, members["why"]), members["why"] + assert_nothing_could_not_run(rows) + + +@CANNOT_RUN_AS_ROOT +def test_a_leg_file_at_mode_000_is_told_the_cure_and_gates_nothing( + standard, project): + """`spec/CLAUDE.md` at 000, as rv975 measured it (#1175): the `leg shape + files` note names the file and has no NEXT, as before, and its why is + now the gate's advice. The verdict stays COMPLIANT: no leg file gates + anything, so one nobody could read changes nothing.""" + denied = os.strerror(errno.EACCES) + with mode_of(project, "spec/CLAUDE.md", 0): + rows, verdict = doctored(standard, project) + row = rows["leg-shape-files"] + assert row["status"] == "note", row + assert f"spec: AGENTS.md rendered, CLAUDE.md cannot be read ({denied})" \ + in row["reason"], row + assert row["next"] is None, row + assert row["why"] == PERMISSION_WHY + assert verdict == "COMPLIANT" + + def root_refusal(root: Path, reason: str) -> str: """The first line of the doctor's refusal of a root it cannot search.""" return (f"REFUSED shape-doctor-root-unreadable: --root {root}: cannot be " @@ -384,6 +474,27 @@ def test_beside_a_checkout_line_the_unread_mount_gets_the_advice( assert legs.why.endswith(". " + PERMISSION_WHY), legs.why +def test_a_leg_with_no_gitlink_names_the_mount_it_cannot_reach_too( + standard, project, monkeypatch): + """A leg the root records no gitlink for, over a mount this user cannot + list (#1175): the lockstep fact is said first, the mount's line follows + it, the gate's line is the NEXT with the advice in its why, and the leg + is no drift.""" + make_no_gitlink(project) + module, ctx = in_process(standard, project) + refusing(monkeypatch, "iterdir", "spec", + PermissionError(errno.EACCES, DENIED)) + legs = module.check_legs(ctx) + assert legs.reason == ("spec: no gitlink recorded, so this leg is not " + "mounted as a submodule at all, and " + + listed("spec")) + assert legs.detail["legs"][0]["mount_cannot_be_read"] == listed("spec") + assert legs.next_command == module.lockstep_next(ctx, module.PINS_GATE, + "leg")[0] + assert legs.why.endswith(". " + PERMISSION_WHY), legs.why + assert module.drifted_reasons({"legs": legs}, {}) == [] + + def test_a_member_mount_refused_is_named_on_every_python( standard, holder, tmp_path, monkeypatch): """The `members` row for a member whose mount's `stat` is refused (a @@ -402,10 +513,58 @@ def test_a_member_mount_refused_is_named_on_every_python( assert module.drifted_reasons({"members": members}, {}) == [] +def gitmodules_refused_in(monkeypatch, folder: Path) -> None: + """`Path.is_file` raising a refused `stat` for a name in `folder`, as + Python 3.9 to 3.13 raise it in a folder this user cannot search. 3.14 + answers False, so this stand-in reaches it on every Python.""" + real = Path.is_file + + def is_file(self, *args, **kwargs): + if self.parent == folder: + raise PermissionError(errno.EACCES, DENIED, str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "is_file", is_file) + + +@pytest.mark.parametrize("own_bootstrap", [ + pytest.param(True, id="the-holders-bootstrap"), + pytest.param(False, id="the-git-line"), +]) +def test_beside_a_member_checkout_the_unreached_member_gets_the_advice( + standard, holder, tmp_path, monkeypatch, own_bootstrap): + """One member's mount cannot be listed, and the other's is empty, which + a checkout cures (#1175): the holder's own bootstrap where it can run, + else the git line. Either way the row names both, its why says that the + next run names what the line leaves and ends on the advice for the + mount, and the unreached member's `.gitmodules`, whose `stat` would be + refused, is never asked.""" + root = holder_copy(holder, tmp_path) + with_a_member_beside_one_a_checkout_cures(root) + if not own_bootstrap: + holder_without(root, "bootstrap.py") + module, ctx = in_process(standard, root) + refusing(monkeypatch, "iterdir", "Ghost", + PermissionError(errno.EACCES, DENIED)) + gitmodules_refused_in(monkeypatch, ctx.root / GHOST) + members = module.check_members(ctx) + assert members.reason.split("; ")[:2] == [ + f"{MEMBER}: {UNBOOTSTRAPPED} submodule", unread_mount(listed(GHOST))] + assert members.detail["members"][1]["mount_cannot_be_read"] == \ + listed(GHOST) + if own_bootstrap: + assert members.next_command == bootstrap_line(module, root) + else: + assert members.next_command.endswith(f" -- {MEMBER}"), \ + members.next_command + assert beside_the_checkout(module, members.why), members.why + + def test_a_leg_file_this_user_cannot_read_is_said_not_called_absent( standard, project, monkeypatch): """A leg's shape file whose `stat`, or whose read, is refused is `cannot - be read`, never `absent` with a copy over it: the row is a `note`.""" + be read`, never `absent` with a copy over it: the row is a `note` with + no NEXT, and its why is the gate's advice (#1175).""" module, ctx = in_process(standard, project) refused_stat(monkeypatch, ".gitignore") refusing(monkeypatch, "read_bytes", "CLAUDE.md", @@ -417,6 +576,51 @@ def test_a_leg_file_this_user_cannot_read_is_said_not_called_absent( assert row.status == module.NOTE assert row.next_command is None assert "absent" not in row.reason, row.reason + assert row.why == PERMISSION_WHY + + +@pytest.mark.parametrize("method, name", [ + pytest.param("stat", ".gitignore", id="stat"), + pytest.param("read_bytes", "CLAUDE.md", id="read"), +]) +def test_a_leg_file_refused_for_another_reason_gets_the_other_advice( + standard, project, monkeypatch, method, name): + """A leg file whose `stat`, or whose read, alone fails for a reason that + is no permission (an I/O error): the system's reason, a `note`, and the + gate's other advice, which is not told to `chmod` (#1175).""" + module, ctx = in_process(standard, project) + refusing(monkeypatch, method, name, OSError(errno.EIO, IO_ERROR)) + row = module.check_leg_shape_files(ctx) + assert row.detail["legs"]["spec"]["files"][name] == \ + f"cannot be read ({IO_ERROR})" + assert row.status == module.NOTE + assert row.next_command is None + assert row.why == OTHER_WHY + + +@pytest.mark.parametrize("name", [".gitignore", "AGENTS.md"]) +def test_beside_a_missing_leg_file_the_advice_follows_its_line( + standard, project, monkeypatch, name): + """A leg file missing beside one this user cannot read (#1175): the + NEXT is the missing file's line, as before, and the advice is the why + where that line's says nothing (a copy), or follows it (a file the + scaffold renders, which no copy makes).""" + module, ctx = in_process(standard, project) + (project / "code" / name).unlink() + refusing(monkeypatch, "read_bytes", "CLAUDE.md", + PermissionError(errno.EACCES, DENIED)) + row = module.check_leg_shape_files(ctx) + line, said = module.missing_shape_file_fix(ctx, [("code", "code", name)]) + assert row.status == module.NOTE + assert row.reason.endswith(f"(missing: code/{name})"), row.reason + assert row.next_command == line + if name == ".gitignore": + assert said is None + assert row.why == PERMISSION_WHY + else: + assert said.startswith("the missing file(s) are RENDERED"), said + # `Row` flattens the typography of what it is handed (`ascii_text`). + assert row.why == module.ascii_text(f"{said}. {PERMISSION_WHY}") def test_a_bootstrap_this_user_cannot_reach_cannot_run_on_every_python( From 5fb38294a4d18a949a2267a76b79c8175bca0626 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 08:05:03 +0000 Subject: [PATCH 2/2] shape-doctor.py: a leg or member whose git directory this user cannot read is named in the gate's words, `spec: .git/modules/spec: cannot be read ()`, so the `legs` and `members` rows never count it as drift, `placement` never calls it "not a git repository", and no HEAD is read for it (#1174) `mount_read`, which the `legs`, `leg shape files`, `placement` and `members` rows share, now asks the mount's git directory as `validate-pins.py` asks it (`_leg_git_dir` and `_refuse_an_unreadable_git_dir`, #968): the `.git` itself where it is no regular file, else the directory its gitfile's `gitdir: ` line names (`git_dir_of`), and then the `stat` of that directory's `HEAD`. A refused read of the gitfile, or a refused `stat` of the `HEAD`, is the mount's `mount_cannot_be_read` line, named as the gate names it (`git_dir_named`): `spec/.git`, or, where the directory is not under the mount, `spec: .git/modules/spec`, so every row's line starts with the mount. The `legs`, `placement` and `members` rows then say what they say of a mount this user cannot reach: this doctor again as the NEXT, the gate's advice as the WHY, no drift, and the verdict is the `pins` (or `family`) row's INVALID. The `leg shape files` row runs no git, so it asks with `git_dir=False` and still compares the files of such a leg, as it did before. The `members` row's note on what the holder's bootstrap checks out recursively still skips a member whose mount line is set, now one whose git directory cannot be read too: git can move nothing in it. Before, a leg at its pin was `populated, but git cannot read a HEAD in it`, sent to the lockstep gate and counted as drift; a member was `checked out at ?`; `placement` said "it is not a git repository"; and a `.git` directory this user cannot search was passed over by git, which read the ROOT's HEAD as the leg's and named a checkout over it. Each verdict was DRIFTED. A gitfile that names no directory, or a git directory that is not there, is left to git as before. Also pins that the `leg shape files` note for a mount this user cannot reach has no WHY (RV1193's NIT 1 on #1193). Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 85 ++++++- tests/test_doctor_unreadable_paths.py | 320 +++++++++++++++++++++++++- 2 files changed, 395 insertions(+), 10 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 41c43191..cd2096c8 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3450,17 +3450,54 @@ def cannot_be_listed(named: str, exc: OSError) -> str: #: The key of a leg's or a member's `--json` entry that holds the gate's line -#: for a mount this user cannot reach (#430), beside a leg's `cannot_be_read`, -#: which is its PIN's (#847). +#: for a mount this user cannot reach (#430), or whose git directory it +#: cannot read (#1174), beside a leg's `cannot_be_read`, which is its PIN's +#: (#847). MOUNT_UNREAD = "mount_cannot_be_read" -def mount_read(root: Path, rel: str) -> tuple: +def git_dir_of(dotgit: Path) -> Path | None: + """The git directory of a mount whose `.git` is `dotgit`, found as + `validate-pins.py`'s `_leg_git_dir` finds it (#968, #1174): `dotgit` + itself where it is no regular file (a checkout's own directory); the + directory a gitfile's `gitdir: ` line names, joined to the gitfile's own + directory as git joins it, its `..` left for the system to follow; and + None where the gitfile names none, which is left to git, as the gate + leaves it. Raises the `OSError` of a `stat` or a read the operating + system refused.""" + if not is_a(dotgit, stat.S_ISREG): + return dotgit + data = dotgit.read_bytes() + if not data.startswith(b"gitdir: "): + return None + return dotgit.parent / os.fsdecode(data[len(b"gitdir: "):].rstrip(b"\r\n")) + + +def git_dir_named(root: Path, rel: str, gitdir: Path) -> str: + """`gitdir` as the gate names it (`_named`, #968): normalized, from the + root and POSIX (`.git/modules/spec`), or whole where it is not under the + root. Where it is not under the mount at `rel`, the mount's path goes + first (`spec: .git/modules/spec`), so the line of every row that reads + the mount starts with that mount, as `leg_shape_summary`, + `leg_audit_summary` and `member_problem` print it (#1174).""" + path = Path(os.path.normpath(gitdir)) + try: + named = path.relative_to(root).as_posix() + except ValueError: + named = str(path) + mount = root / rel + if path == mount or mount in path.parents: + return named + return f"{rel}: {named}" + + +def mount_read(root: Path, rel: str, git_dir: bool = True) -> tuple: """`(populated, own_git, refused)` for the leg or member mounted at `rel`: whether a directory with anything in it is there, whether it holds a `.git` of its own, and None, or `(the gate's line, the OSError, whether it was the listing)` where the operating system refused the mount's `stat`, - its listing or its `.git`'s `stat` (#430). + its listing or its `.git`'s `stat` (#430), or a read of its git + directory (#1174). ONE QUESTION FOR THE FOUR ROWS THAT READ A MOUNT -- `legs`, `leg shape files`, `placement` and `members` -- each of which asked `mount.is_dir() @@ -3481,6 +3518,29 @@ def mount_read(root: Path, rel: str) -> tuple: is asked only of a mount with something in it, which is the answer `(mount / ".git").exists()` gave an empty, absent or non-folder mount. A listing that fails because nothing is there any more is nothing there. + + ITS GIT DIRECTORY IS ASKED AS THE GATE ASKS IT (#1174, the gate's + `_refuse_an_unreadable_git_dir`, #968): the `.git` itself where it is no + regular file, else the directory its gitfile's `gitdir: ` line names + (`git_dir_of`), and then the `stat` of that directory's `HEAD`. Git run + in a mount whose git directory this user cannot search failed, so the + `legs` and `members` rows read no HEAD and counted a leg or a member at + its pin as one off it, and `placement` said "it is not a git + repository"; and a `.git` DIRECTORY this user cannot search was passed + over by git, which read the ROOT's HEAD as the leg's and named a + checkout over it. A refused read of the gitfile is `/.git: cannot + be read ()`, and a refused `stat` of the `HEAD` names the git + directory as the gate names it (`git_dir_named`): `/.git`, or + `: .git/modules/` for one outside the mount. A gitfile that + names no directory, or a git directory with no `HEAD`, is not a read + this user was refused: it is left to git, which answers for it as it + always did. + + `git_dir=False` ASKS NO GIT DIRECTORY, for the `leg shape files` row, + which compares a leg's files on disk and runs no git: a git directory + this user cannot search keeps no file in the mount from being read, so + that row compares them as it always did, and the `legs` row names the + directory and gives its advice (#1174). """ mount = root / rel try: @@ -3496,11 +3556,18 @@ def mount_read(root: Path, rel: str) -> tuple: return False, False, (cannot_be_listed(rel, exc), exc, True) if not populated: return False, False, None + dotgit = mount / ".git" + named = (Path(rel) / ".git").as_posix() try: - return True, is_a(mount / ".git", lambda _mode: True), None + if not is_a(dotgit, lambda _mode: True): + return True, False, None + gitdir = git_dir_of(dotgit) if git_dir else None + if gitdir is not None: + named = git_dir_named(root, rel, gitdir) + is_a(gitdir / "HEAD", lambda _mode: True) except OSError as exc: - return True, False, (cannot_be_read((Path(rel) / ".git").as_posix(), - exc), exc, False) + return True, False, (cannot_be_read(named, exc), exc, False) + return True, True, None def mount_unread_said(line: str) -> str: @@ -7753,12 +7820,14 @@ def leg_shape_comparison(ctx: Context, role: str, rel: str) -> tuple: which they are, in the `state` the row prints. A leg this user cannot reach is a third, and its `state` is the gate's line for it, kept under `mount_cannot_be_read` as the `legs` row keeps it (`mount_read`, #430). + A leg whose git directory this user cannot read is not: no file here is + read through git, so its files are compared (#1174). Split out of `check_leg_shape_files` for #134. """ mount = ctx.root / rel template = ctx.shape / "templates" / f"{role}-root" - populated, _own_git, unread = mount_read(ctx.root, rel) + populated, _own_git, unread = mount_read(ctx.root, rel, git_dir=False) if unread is not None: return ({"role": role, "state": unread[0], MOUNT_UNREAD: unread[0]}, [], []) diff --git a/tests/test_doctor_unreadable_paths.py b/tests/test_doctor_unreadable_paths.py index b3b4ab38..0b59b489 100644 --- a/tests/test_doctor_unreadable_paths.py +++ b/tests/test_doctor_unreadable_paths.py @@ -38,6 +38,17 @@ files` note for a leg file it cannot read now carries the gate's advice as its WHY, as every other path here does. +#1174 ASKS A MOUNT'S GIT DIRECTORY TOO, as `validate-pins.py` asks it (#968). +A leg's or a member's git directory this user cannot search -- the one its +gitfile names under `.git/modules/`, or a `.git` that is a directory of its +own -- or a gitfile it cannot open made git fail in the mount, so the `legs` +or `members` row read no HEAD and the verdict was DRIFTED for a mount at its +pin, and `placement` called the leg not a git repository; for a `.git` +directory git read the ROOT's HEAD as the leg's, and a checkout was named +over it. Each of those rows now names the directory as the gate does, and +the verdict is the gate's INVALID. The `leg shape files` row runs no git, so +it still compares the files of such a leg. + NOT HERE: the `shape currency` row's `copy-missing` under 3.14 for a `scripts/` or a `.github/workflows/` this user cannot search comes from `update-shape.py`'s comparison, which #430's slice B fixes. No test here @@ -80,13 +91,16 @@ #: owns them. from test_doctor_unreadable_pin import ( DENIED, GATE, IO_ERROR, OTHER_FOLDER_WHY, OTHER_WHY, PERMISSION_WHY, - cannot, doctored, in_process, refusing, + cannot, doctored, in_process, refusing, sources, ) #: The doctor's fixtures, from the module that owns them. from test_shape_doctor import ( # noqa: F401 FAMILY_MEMBER, doctor, holder, project, record_gitlink, scaffolded_here, standard, ) +#: A leg whose `.git` is a git directory of its own, from the file that owns +#: the gate's cases for one (#968). +from test_validate_pins_spelled_and_named import leg_with_its_own_git_directory #: 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. @@ -223,6 +237,100 @@ def test_a_leg_mount_this_user_cannot_reach_is_named_and_no_drift( assert_nothing_could_not_run(rows) +#: `(whether the leg's .git is a directory of its own, the path set to the +#: mode, the mode, the line the leg is named by)` for each git directory of a +#: leg this user cannot search, and the gitfile it cannot open: the gate's +#: cells (#968), each the path the gate names (#1174). +GIT_DIR_CELLS = [ + pytest.param(False, ".git/modules/spec", 0, "spec: .git/modules/spec", + id="modules-000"), + pytest.param(False, ".git/modules/spec", 0o444, + "spec: .git/modules/spec", id="modules-r--"), + pytest.param(False, "spec/.git", 0, "spec/.git", id="gitfile-000"), + pytest.param(True, "spec/.git", 0, "spec/.git", id="git-directory-000"), + pytest.param(True, "spec/.git", 0o444, "spec/.git", + id="git-directory-r--"), +] + + +def assert_compared(row: dict) -> None: + """The `leg shape files` row, as `--json` prints it, over legs whose git + directory this user cannot read (#1174): it compares files on disk and + runs no git, so each leg's files are compared, as before, the row is + `ok`, and it has no WHY. The `legs` row names the git directory.""" + assert row["status"] == "ok", row + assert [leg["state"] for leg in row["detail"]["legs"].values()] == [ + "compared", "compared"], row + assert row["why"] is None, row + + +@CANNOT_RUN_AS_ROOT +@pytest.mark.parametrize("own_directory, relative, bits, named", + GIT_DIR_CELLS) +def test_a_leg_whose_git_directory_this_user_cannot_search_is_no_drift( + standard, project, own_directory, relative, bits, named): + """rv975's cell, `.git/modules/spec` at 000, and the gate's others + (#968). Each was DRIFTED (#1174): `spec: populated, but git cannot read + a HEAD in it` with the lockstep gate as its NEXT, or, for a `.git` that + is a directory, the ROOT's HEAD quoted as the leg's with a checkout over + it; and `placement` said the leg was no repository. The `legs` and + `placement` rows now name the path the `pins` row names, after the leg + where it is outside the mount, no HEAD is read, the NEXT is this doctor + again, and the verdict is the gate's INVALID. The `leg shape files` row + reads no git, so it compares the leg's files as it always did.""" + if own_directory: + leg_with_its_own_git_directory(project) + denied = os.strerror(errno.EACCES) + line = cannot(named, denied) + with mode_of(project, relative, bits): + rows, verdict = doctored(standard, project) + assert cannot(relative, denied) in rows["pins"]["reason"], rows["pins"] + legs = rows["legs"] + assert legs["status"] == "FINDING", legs + assert legs["reason"] == unread_mount(line) + spec = legs["detail"]["legs"][0] + assert spec["mount_cannot_be_read"] == line + assert spec["head"] is None, spec + assert legs["next"] == doctor_again(standard, project) + assert legs["why"] == PERMISSION_WHY + assert_compared(rows["leg-shape-files"]) + placement = rows["placement"]["reason"] + assert f": {line}; code: " in placement, placement + assert "repository" not in placement, placement + assert verdict == "INVALID (pins)" + assert_nothing_could_not_run(rows) + + +#: The git directory of the session holder's member, where its gitfile +#: says it is. +MEMBER_GIT_DIR = f".git/modules/{MEMBER}" + + +@CANNOT_RUN_AS_ROOT +@pytest.mark.parametrize("bits", [pytest.param(0, id="000"), + pytest.param(0o444, id="r--")]) +def test_a_member_whose_git_directory_this_user_cannot_search_is_no_drift( + standard, holder, tmp_path, bits): + """A member's git directory this user cannot search: the `members` row + said `checked out at ?`, and the verdict was DRIFTED (#1174). It names + the directory after the member, no HEAD is read, and the verdict is the + `family` row's INVALID.""" + root = holder_copy(holder, tmp_path) + line = cannot(f"{MEMBER}: {MEMBER_GIT_DIR}", os.strerror(errno.EACCES)) + with mode_of(root, MEMBER_GIT_DIR, bits): + rows, verdict = doctored(standard, root) + members = rows["members"] + assert members["status"] == "FINDING", members + assert members["reason"] == unread_mount(line) + entry = members["detail"]["members"][0] + assert entry["mount_cannot_be_read"] == line + assert entry["head"] is None, entry + assert members["next"] == doctor_again(standard, root) + assert members["why"] == PERMISSION_WHY + assert verdict == "INVALID (family)" + assert_nothing_could_not_run(rows) + + @CANNOT_RUN_AS_ROOT @pytest.mark.parametrize("relative, bits, line", [ pytest.param("members", 0, cannot(MEMBER, os.strerror(errno.EACCES)), @@ -459,11 +567,190 @@ def test_a_mount_whose_git_cannot_be_reached_is_not_one_without_a_git( assert placement.reason.endswith(f"{spec}; {code}"), placement.reason +def git_dir_line(leg: str, reason: str = DENIED) -> str: + """The line for a leg whose gitfile names `.git/modules/`, outside + the mount, where the `stat` of that directory's `HEAD` is refused: the + gate's line after the leg's path (#1174).""" + return f"{leg}: {cannot(f'.git/modules/{leg}', reason)}" + + +def refused_head(error: OSError): + """A stand-in for a git directory this user cannot search: the `stat` + of the `HEAD` in each one is refused with `error`, except the root's + own `.git/HEAD`, so the root's repository stays one this user can read + and each row reaches the mounts (#1174).""" + def refuse(monkeypatch, root: Path) -> None: + real = Path.stat + own = root / ".git" / "HEAD" + + def refused(self, *args, **kwargs): + if self.name == "HEAD" and self != own: + raise type(error)(error.errno, error.strerror, str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", refused) + return refuse + + +def refused_gitfile(monkeypatch, _root: Path) -> None: + """The read of each gitfile `.git` refused (#1174).""" + refusing(monkeypatch, "read_bytes", ".git", + PermissionError(errno.EACCES, DENIED)) + + +@pytest.mark.parametrize("refuse, spec, code, why", [ + pytest.param(refused_head(PermissionError(errno.EACCES, DENIED)), + git_dir_line("spec"), git_dir_line("code"), PERMISSION_WHY, + id="head-stat"), + pytest.param(refused_head(OSError(errno.EIO, IO_ERROR)), + git_dir_line("spec", IO_ERROR), + git_dir_line("code", IO_ERROR), OTHER_WHY, + id="head-io-error"), + pytest.param(refused_gitfile, cannot("spec/.git"), cannot("code/.git"), + PERMISSION_WHY, id="gitfile-read"), +]) +def test_a_leg_git_directory_refused_is_named_on_every_python( + standard, project, monkeypatch, refuse, spec, code, why): + """The `legs` and `placement` rows for legs whose gitfile cannot be + read, or whose git directory's `HEAD` cannot be `stat`ed (#1174), in + process: each row says the gate's line, no HEAD is read, the NEXT is + this doctor again with the advice for the cause, and no leg is drift. + The `leg shape files` row, which reads no git, compares the files.""" + module, ctx = in_process(standard, project) + refuse(monkeypatch, ctx.root) + legs = module.check_legs(ctx) + assert legs.reason == f"{unread_mount(spec)}; {unread_mount(code)}" + assert [leg["head"] for leg in legs.detail["legs"]] == [None, None] + assert legs.next_command == doctor_again(standard, project) + assert legs.why == why + assert module.drifted_reasons({"legs": legs}, {}) == [] + shape_files = module.check_leg_shape_files(ctx) + assert_compared({"status": shape_files.status, + "detail": shape_files.detail, "why": shape_files.why}) + placement = module.check_placement(ctx) + assert placement.reason.endswith(f"{spec}; {code}"), placement.reason + + +def test_a_git_directory_of_its_own_refused_is_not_the_roots( + standard, project, monkeypatch): + """A leg whose `.git` is a directory of its own, whose `HEAD` cannot be + `stat`ed: git, run in the leg, passed over it to the root's repository, + and the row quoted the ROOT's HEAD as the leg's with a checkout over it + (#1174). It names the leg's `.git`, as the gate does.""" + leg_with_its_own_git_directory(project) + module, ctx = in_process(standard, project) + refused_head(PermissionError(errno.EACCES, DENIED))(monkeypatch, ctx.root) + legs = module.check_legs(ctx) + assert legs.reason == (f"{unread_mount(cannot('spec/.git'))}; " + f"{unread_mount(git_dir_line('code'))}") + assert [leg["head"] for leg in legs.detail["legs"]] == [None, None] + assert legs.next_command == doctor_again(standard, project) + assert module.drifted_reasons({"legs": legs}, {}) == [] + + +def test_a_member_git_directory_refused_is_named_on_every_python( + standard, holder, tmp_path, monkeypatch): + """The `members` row for a member whose git directory's `HEAD` cannot + be `stat`ed (#1174): named after the member, no HEAD read, the advice, + and no drift.""" + root = holder_copy(holder, tmp_path) + module, ctx = in_process(standard, root) + refused_head(PermissionError(errno.EACCES, DENIED))(monkeypatch, ctx.root) + members = module.check_members(ctx) + line = cannot(f"{MEMBER}: {MEMBER_GIT_DIR}") + assert members.reason == unread_mount(line) + entry = members.detail["members"][0] + assert entry["mount_cannot_be_read"] == line + assert entry["head"] is None, entry + assert members.next_command == doctor_again(standard, root) + assert members.why == PERMISSION_WHY + assert module.drifted_reasons({"members": members}, {}) == [] + + +def gitfile_naming_nothing(project: Path, _aside: Path) -> None: + """`spec/.git` a file with no `gitdir: ` line.""" + (project / "spec" / ".git").write_bytes(b"not a gitfile\n") + + +def git_directory_moved_away(project: Path, aside: Path) -> None: + """The git directory `spec`'s gitfile names moved out of the root.""" + (project / ".git" / "modules" / "spec").rename(aside) + + +@pytest.mark.parametrize("unmake", [ + pytest.param(gitfile_naming_nothing, id="gitfile-names-none"), + pytest.param(git_directory_moved_away, id="git-directory-gone"), +]) +def test_a_git_directory_that_is_not_there_is_left_to_git( + standard, project, tmp_path, unmake): + """The controls: a gitfile that names no directory, and a git directory + that is not there, are no read this user was refused. They are left to + git, as the gate leaves them, and the row says what it said.""" + unmake(project, tmp_path / "spec-git-directory-aside") + module, ctx = in_process(standard, project) + legs = module.check_legs(ctx) + assert legs.reason == "spec: populated, but git cannot read a HEAD in it" + assert "mount_cannot_be_read" not in legs.detail["legs"][0] + + +#: What a gitfile may hold, as the gate's own cases write it (#968). +GITFILES = [ + pytest.param(b"gitdir: ../.git/modules/spec\n", id="relative"), + pytest.param(b"gitdir: ../m\r\n", id="cr-lf"), + pytest.param(b"gitdir: ../m", id="no-line-end"), + pytest.param(b"gitdir: ../m\n\n", id="blank-line-after"), + pytest.param(b"not a gitfile\n", id="names-none"), + pytest.param(None, id="a-directory"), +] + + +@pytest.mark.parametrize("written", GITFILES) +def test_the_doctor_finds_a_git_directory_as_the_gate_finds_it( + sources, tmp_path, written): + """`git_dir_of` is the gate's `_leg_git_dir` (#968): the same directory, + or None, for each gitfile and for a `.git` that is a directory, so the + doctor asks the `HEAD` the gate asks (#1174).""" + dotgit = tmp_path / "leg" / ".git" + if written is None: + dotgit.mkdir(parents=True) + else: + dotgit.parent.mkdir() + dotgit.write_bytes(written) + assert sources.doctor.git_dir_of(dotgit) == \ + sources.gate._leg_git_dir(dotgit) + + +@pytest.mark.parametrize("rel, gitdir, named", [ + pytest.param("spec", ("spec", ".git"), "spec/.git", id="its-own"), + pytest.param("spec", ("spec", "..", ".git", "modules", "spec"), + "spec: .git/modules/spec", id="under-the-root"), + pytest.param(MEMBER, (MEMBER, "..", "..", ".git", "modules", MEMBER), + f"{MEMBER}: {MEMBER_GIT_DIR}", id="a-members"), + pytest.param("spec", ("..", "elsewhere", "spec"), None, + id="outside-the-root"), +]) +def test_a_git_directory_is_named_as_the_gate_names_it_after_its_mount( + sources, tmp_path, rel, gitdir, named): + """`git_dir_named` is the gate's `_named` of the normalized directory, + from the root, or whole where it is outside the root, and the mount's + path goes first where the directory is not under the mount (#1174).""" + root = tmp_path / "root" + path = root.joinpath(*gitdir) + gate = sources.gate._named(root, Path(os.path.normpath(path))) + if named is None: + named = f"spec: {tmp_path / 'elsewhere' / 'spec'}" + assert sources.doctor.git_dir_named(root, rel, path) == named + assert named.endswith(gate), (named, gate) + + def test_beside_a_checkout_line_the_unread_mount_gets_the_advice( standard, project, monkeypatch): """One leg's mount cannot be listed, and the other leg has a commit past its pin while its gitlink is at the pin: the checkout line is the NEXT - for that leg, and its why ends on the advice for the mount.""" + for that leg, and its why ends on the advice for the mount. The `leg + shape files` note for that mount has no why: the `legs` row gives the + mount's advice, and only a leg FILE this user cannot read is told it + there (#1175, RV1193's NIT 1).""" module, ctx = in_process(standard, project) git("commit", "-q", "--allow-empty", "-m", "past its pin", cwd=project / "code") @@ -472,6 +759,7 @@ def test_beside_a_checkout_line_the_unread_mount_gets_the_advice( legs = module.check_legs(ctx) assert legs.next_command.endswith(" -- code"), legs.next_command assert legs.why.endswith(". " + PERMISSION_WHY), legs.why + assert module.check_leg_shape_files(ctx).why is None def test_a_leg_with_no_gitlink_names_the_mount_it_cannot_reach_too( @@ -560,6 +848,34 @@ def test_beside_a_member_checkout_the_unreached_member_gets_the_advice( assert beside_the_checkout(module, members.why), members.why +def test_a_member_whose_git_directory_is_refused_is_not_asked_what_it_carries( + standard, holder, tmp_path, monkeypatch): + """A member whose git directory this user cannot read, beside one the + holder's own bootstrap cures (#1174): its mount can be read, and so can + the `.gitmodules` in it, but the row does not ask that file whether the + bootstrap's recursive update checks out legs the member carries. Git can + move nothing in a member whose repository it cannot read, so the why + says no keep line for one: it is the same with that file and without + it, and it ends on the advice for the git directory.""" + root = holder_copy(holder, tmp_path) + with_a_member_beside_one_a_checkout_cures(root) + ghost = root / GHOST + (ghost / ".git").write_bytes(b"gitdir: ../../.git/modules/members/Ghost\n") + gitmodules = ghost / ".gitmodules" + gitmodules.write_bytes(b'[submodule "spec"]\n\tpath = spec\n') + module, ctx = in_process(standard, root) + refused_head(PermissionError(errno.EACCES, DENIED))(monkeypatch, ctx.root) + members = module.check_members(ctx) + assert members.reason.split("; ")[:2] == [ + f"{MEMBER}: {UNBOOTSTRAPPED} submodule", + unread_mount(cannot(f"{GHOST}: .git/modules/{GHOST}"))] + assert members.next_command == bootstrap_line(module, root) + assert beside_the_checkout(module, members.why), members.why + assert "git -C LEG branch NAME" not in members.why, members.why + gitmodules.rename(tmp_path / "ghost-gitmodules-aside") + assert module.check_members(ctx).why == members.why + + def test_a_leg_file_this_user_cannot_read_is_said_not_called_absent( standard, project, monkeypatch): """A leg's shape file whose `stat`, or whose read, is refused is `cannot