diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index e32c504f..67b66a78 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -868,9 +868,18 @@ def load_yaml(path: Path) -> Any: # --------------------------------------------------------------------------- -def git_out(args: list[str], cwd: Path, binary: bool = False) -> Any: +def git_out(args: list[str], cwd: Path, binary: bool = False, + env: dict[str, str] | None = None) -> Any: + """`git ` in `cwd`, its stdout, or a `git-failed` refusal. + + `env` is the whole environment git runs in, None for this process's + own. A git that cannot be STARTED (none on PATH) raises `OSError` from + here, never a refusal: whether that is a question the caller cannot + answer is the caller's to say (#368). + """ proc = subprocess.run( - ["git", *args], cwd=str(cwd), capture_output=True, check=False + ["git", *args], cwd=str(cwd), capture_output=True, check=False, + env=env, ) if proc.returncode != 0: raise Refusal( @@ -1030,11 +1039,29 @@ def file_sha256(path: Path) -> str: #: ` SP SP TAB ` and `ls-tree` emits #: ` SP SP TAB ` — the same shape with a DIFFERENT #: third column, so the two cannot share one index. +#: +#: BOTH LISTINGS TAKE THE PATH LITERALLY (#367), with the global +#: `--literal-pathspecs`. A pathspec is a glob to `ls-files`, so a row whose +#: hand-edited `path:` was `members/*` read the first member's gitlink and +#: answered with it, a neighbour's commit, for every caller. Not +#: `:(literal)` magic on the path, which a caller's own exported +#: `GIT_LITERAL_PATHSPECS=1` switches off, and then it matches nothing. GITLINK_LISTINGS = ( - (["ls-files", "-s", "--"], 1, 2), - (["ls-tree", "HEAD", "--"], 2, None), + (["--literal-pathspecs", "ls-files", "-s", "--"], 1, 2), + (["--literal-pathspecs", "ls-tree", "HEAD", "--"], 2, None), ) +#: The two pathspec settings a caller's environment can carry that git +#: refuses BESIDE `--literal-pathspecs` ("global 'literal' pathspec setting +#: is incompatible with all other global pathspec settings", exit 128). Both +#: listings would then fail, and `recorded_gitlink` reads a failed listing as +#: "no gitlink here", so an operator who exports either would see every gate +#: refuse a gitlink that is there. They are left out of those two git calls +#: alone. `GIT_NOGLOB_PATHSPECS` stays: git takes it beside the literal +#: setting, and with it the path is literal anyway. +PATHSPEC_ENV_REFUSED_BESIDE_LITERAL = ("GIT_GLOB_PATHSPECS", + "GIT_ICASE_PATHSPECS") + def recorded_gitlink(repo: Path, path: str) -> str | None: """The commit the SUPERPROJECT records for the submodule at `path`. @@ -1049,10 +1076,17 @@ def recorded_gitlink(repo: Path, path: str) -> str | None: bump and passes a stale pin. HEAD remains the fallback, for a path the commit records and the index does not hold at stage 0 — one the index never had, and one a conflicted merge holds only at stages 1, 2 and 3. + + `path` IS TAKEN LITERALLY (#367): `members/*` is the entry of that name, + which no index holds, and never every gitlink a glob would match. A git + that cannot be started raises `OSError` through this, never "no + gitlink": only a listing git RAN and refused is read as nothing (#368). """ + env = {name: value for name, value in os.environ.items() + if name.upper() not in PATHSPEC_ENV_REFUSED_BESIDE_LITERAL} for args, oid_at, stage_at in GITLINK_LISTINGS: try: - out = git_out([*args, path], cwd=repo) + out = git_out([*args, path], cwd=repo, env=env) except Refusal: continue for line in out.splitlines(): diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 72f3a145..ceece161 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -38,13 +38,25 @@ 3. the mounted tree carries a `project.yaml` whose `id` is the `id` this row records — a repository at the right commit is not by itself the project the row claims. - and, across the rows, that no two record ONE `id`: that is one project + A row whose `path:` is not `members/` is the `member-path` + finding and none of the three is asked of it: nothing read through a + path the row may not have is a fact about the member (#367). + And, across the rows, that no two record ONE `id`: that is one project pinned twice, however each row spelled its repository (#206). This one reads `family.yaml` alone, so `--no-members` asks it too. - that no two sections of `.gitmodules` give ONE path (#300): a recursive clone fetches that path from only one of them, so the other is a url the next edit can make the one. It reads that file alone, so `--no-members` asks it too. + - the REVERSE of every member check (#361): that each gitlink this + repository records in `members/` (the index at stage 0, else HEAD), and + each path a `.gitmodules` section gives, is a path some row names. A + member mounted by hand, a row deleted by hand, a section `family.py + remove` left behind: every check above starts from a row, so none of + them ever asked. Neither needs a member checked out, so `--no-members` + asks them too. A path a row names in another case is still one of these, + and what it says to do depends on which of the two is the member's own: + it names no command that would un-register the member. - the COPIED shape files, each against its `sha256` row in `contracts/shape-pin.yaml`. Editing a copy in place is drift and the exit is to carry it upstream, never to update the digest. @@ -53,8 +65,8 @@ 0 the manifest is valid, every member is in lockstep, every copy matches 1 a FINDING: drift a reviewer can act on 2 a REFUSAL: the question could not be asked — no manifest, an - uninitialized member, an unreadable pin. An unanswerable question is - never an implicit pass. + uninitialized member, an unreadable pin, no git to run. An + unanswerable question is never an implicit pass. """ from __future__ import annotations @@ -64,7 +76,10 @@ import re import shlex import sys +from collections import Counter +from contextlib import contextmanager from pathlib import Path +from typing import Iterator, NamedTuple sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 @@ -78,6 +93,9 @@ MANIFEST = "family.yaml" GITMODULES = ".gitmodules" DEFAULT_MEMBERS_DIR = "members" +#: A submodule's entry in a tree or the index, as `ls-files -s` and +#: `ls-tree` spell its mode. +GITLINK_MODE = b"160000" LOCKSTEP = ( "THE LOCKSTEP RULE: a member's gitlink and its `members[].pin.commit` in " @@ -99,6 +117,34 @@ def note(self, text: str) -> None: self.notes.append(f" ok {text}") +@contextmanager +def _git_runs(unread: str) -> Iterator[None]: + """The `git-not-runnable` refusal, exit 2, for a check that reads this + holder through git and could not start it (#368). + + `git_out` raises `OSError` when there is no git to run, and nothing here + caught it: the run ended in a `FileNotFoundError` traceback, which exits + 1, a FINDING's code, though nothing was found. So every check that runs + git, in every mode, runs it inside this, and `unread` says which check + it was and what it could not read. ONE code for the whole validator, + whichever check met the missing git first: it is one fact about the + machine, and `gitmodules-unreadable` keeps meaning a file git ran and + could not read. + + Around the git calls ALONE, and never around a whole check: a + `project.yaml` nobody may read is an `OSError` too, and it is not this. + """ + try: + yield + except OSError as exc: + raise Refusal( + "git-not-runnable", + f"{unread}: git could not be run ({exc.strerror or exc})", + "Remediation: put git on PATH and run this again. This check " + "reads the holder the way git does, and nothing could clone it " + "or fetch its members without git either.") from exc + + def _check_schema_kind_and_id(manifest: dict, report: Report) -> None: """`schema_version`, `kind` and `id` — three fixed-shape fields that carry no lookup and no branch of their own, split from `_check_envelope` for @@ -244,12 +290,14 @@ def _member_project_name(row: dict, seen: dict, report: Report) -> str | None: def _check_member_repository_and_path(row: dict, project: str, - report: Report) -> str: + report: Report) -> str | None: """`repository:` and `path:` against what they must equal for `project`. Split from `_check_member` for #137. Returns `path`, defaulted the same way the row itself defaults it, since the gitlink and digest checks that - follow need the same value. + follow need the same value. Returns None where that is not the one path + the row may have, the `member-path` finding: nothing may be read through + it (#367). """ repository = row.get("repository") if not isinstance(repository, str) or not QUALIFIED_RE.match(repository): @@ -267,7 +315,11 @@ def _check_member_repository_and_path(row: dict, project: str, expected = f"{DEFAULT_MEMBERS_DIR}/{project}" if path != expected: report.finding("member-path", - f"{project}: path is {path!r}, expected {expected!r}") + f"{project}: path is {path!r}, expected {expected!r}, " + f"the one path a row for {project} may have. Its " + "gitlink, digest and project id are not checked " + "through any other.") + return None return path @@ -369,7 +421,21 @@ def _check_member_tree(member: Path, pin: dict, path: str, commit: str, def _check_member(root: Path, row: dict, seen: dict, report: Report) -> None: - """One member: the row itself, the gitlink, the digest, the identity.""" + """One member: the row itself, the gitlink, the digest, the identity. + + NOTHING IS READ THROUGH A PATH THE ROW MAY NOT HAVE (#367). A hand-edited + `path:` is `member-path`, and this member's checks stop once its pin is + read: the pin is a fact of the row, and everything after it is read at + the path. They used to go on. At `members/*` the gitlink listing took the + path for a glob and answered with a neighbour's commit, gathered as this + member's `member-gitlink-mismatch`, and the tree check then refused + `member-uninitialized`, whose remedy cannot help, and whose refusal + dropped both findings unprinted. + + The git each check runs is run inside `_git_runs`, which names the check + when there is no git to run (#368); the identity check reads a file and + runs none. + """ project = _member_project_name(row, seen, report) if project is None: return @@ -382,13 +448,18 @@ def _check_member(root: Path, row: dict, seen: dict, report: Report) -> None: "a mapping") return commit = _pin_commit(pin, project, report) + if path is None: + return # ---- the gitlink ------------------------------------------------------- - _check_member_gitlink(root, path, commit, project, report) + with _git_runs(f"the gitlink recorded at {path} could not be read"): + _check_member_gitlink(root, path, commit, project, report) # ---- the digest and the identity -------------------------------------- member = root / path - _check_member_tree(member, pin, path, commit, project, report) + with _git_runs(f"{project}'s pinned commit and its tree digest could not " + f"be read in {path}"): + _check_member_tree(member, pin, path, commit, project, report) _check_member_identity(member, row, project, report) @@ -656,56 +727,66 @@ def _check_member_ids_unique(root: Path, members: list, f"name {first} there instead to keep {project}.") -def _check_gitmodules_paths(root: Path, report: Report) -> None: - """No two sections of `.gitmodules` give ONE path (#300). - - `family.py add` committed a second section for a mount path that a - section under another name already gave — a half-removed member, a hand - edit, a merge that kept both sides — and this gate passed the result. A - recursive clone fetches the path from only one of them, the last in the - file, so the other is a url the next edit to the file can make the one - fetched. `add` refuses to write one now; this finds a holder that - already has one. +def _gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: + """Every `submodule` section of `.gitmodules`, `{name: {key: value}}`, + each key in the order git lists it: the file read ONCE for the two + questions asked of it, two sections for one path (#300) and a path no + row names (#361). READ AS GIT READS IT, with `git config -z --file .gitmodules --list` as `family.py` reads it, and no parser of this file's own: the working-tree file, which in CI is HEAD's. A holder with no members has no `.gitmodules`, and that is no finding; one git cannot read is a refusal, - because nothing could be cloned from it either. A section whose path no - row names is another question, and not asked here. + because nothing could be cloned from it either. EVERY KEY a section + holds is kept, not its path and url alone, because the remedy for a + section no row names takes each of them out (`family.py`'s + `_submodule_paths` says why: a key left behind is inherited). + + No git to run is `git-not-runnable` (#368). It used to be + `gitmodules-unreadable`, which now means only a file git ran and could + not read: the missing git is one fact about the machine, and every check + here that meets it says so under one code. """ if not os.path.lexists(root / GITMODULES): - return - try: - listed = git_out(["config", "-z", "--file", GITMODULES, "--list"], - cwd=root, binary=True) - except Refusal as exc: - raise Refusal( - "gitmodules-unreadable", - f"{GITMODULES} could not be read as git reads it: {exc.detail}", - "Remediation: git cannot read it either, so none of this " - "holder's submodules can be cloned from it as it stands. Mend " - f"the file by hand (`git config -f {GITMODULES} --list` says " - "where it breaks), then run this again.") from exc - except OSError as exc: - # NO GIT TO RUN is a question this run cannot answer, so a refusal - # (exit 2), never the traceback that exits 1, a finding's code: - # `--no-members` ran no git at all before this check. - raise Refusal( - "gitmodules-unreadable", - f"{GITMODULES} could not be read as git reads it: git could not " - f"be run ({exc.strerror or exc})", - "Remediation: put git on PATH and run this again; this check " - "reads the file the way git does, and nothing could fetch this " - "holder's submodules without git either.") from exc + return {} + with _git_runs(f"{GITMODULES} could not be read as git reads it"): + try: + listed = git_out(["config", "-z", "--file", GITMODULES, "--list"], + cwd=root, binary=True) + except Refusal as exc: + raise Refusal( + "gitmodules-unreadable", + f"{GITMODULES} could not be read as git reads it: " + f"{exc.detail}", + "Remediation: git cannot read it either, so none of this " + "holder's submodules can be cloned from it as it stands. Mend " + f"the file by hand (`git config -f {GITMODULES} --list` says " + "where it breaks), then run this again.") from exc sections: dict[str, dict[str, str]] = {} for record in listed.split(b"\0"): key, _, value = record.decode("utf-8", "surrogateescape").partition("\n") section, _, variable = key.rpartition(".") - if section.startswith("submodule.") and variable in ("path", "url"): + if section.startswith("submodule."): sections.setdefault(section[len("submodule."):], {})[variable] = value + return sections + + +def _check_gitmodules_paths(sections: dict[str, dict[str, str]], + report: Report) -> None: + """No two sections of `.gitmodules` give ONE path (#300). + + `family.py add` committed a second section for a mount path that a + section under another name already gave — a half-removed member, a hand + edit, a merge that kept both sides — and this gate passed the result. A + recursive clone fetches the path from only one of them, the last in the + file, so the other is a url the next edit to the file can make the one + fetched. `add` refuses to write one now; this finds a holder that + already has one. `sections` is `_gitmodules_sections`'s reading. A + section whose path no row names is `_check_gitmodules_named`'s question + (#361). + """ paths: dict[str, list[str]] = {} for name, keys in sections.items(): if "path" in keys: @@ -730,17 +811,348 @@ def _check_gitmodules_paths(root: Path, report: Report) -> None: f"inherits none of them, then commit {GITMODULES}.") +def _member_gitlinks(root: Path) -> dict[str, str]: + """`{path: commit}` for every gitlink this repository records in + `members/`, the directory spelt in any case: what `_check_gitlinks_named` + asks of each (#361). + + IN THE ORDER `recorded_gitlink` READS ONE: the index at stage 0 first, + HEAD second. A path the index holds at stage 0 as a gitlink is answered + from there, any other gitlink HEAD records from HEAD, so this lists + exactly the paths at which that function finds a gitlink, and each with + the commit it would answer. + + THE WHOLE INDEX AND THE WHOLE TREE, WITH NO PATHSPEC, as `family.py`'s + `_index_entries` lists the index (#302): a pathspec matches `members/` + only in the case it is spelt, and `Members/Zed` is in `members/` on + macOS and Windows. `-z`, so a name git would quote comes back as itself. + + A HEAD git cannot list is read as recording nothing, as + `recorded_gitlink` reads it: a holder before its first commit has none. + An index git cannot list is `gitlinks-unreadable`: this is the one check + of `members/` that `--no-members` runs, and a directory nobody could + list is never an implicit pass. + """ + with _git_runs(f"the gitlinks this repository records in " + f"{DEFAULT_MEMBERS_DIR}/ could not be listed"): + try: + head = git_out(["ls-tree", "-r", "-z", "HEAD"], cwd=root, + binary=True) + except Refusal: + head = b"" + try: + index = git_out(["ls-files", "-s", "-z"], cwd=root, binary=True) + except Refusal as exc: + raise Refusal( + "gitlinks-unreadable", + f"the index could not be listed, so the gitlinks it records " + f"in {DEFAULT_MEMBERS_DIR}/ could not be read: {exc.detail}", + "Remediation: git status in this repository says what is " + "wrong with its index. Mend that, then run this " + "again.") from exc + found: dict[str, str] = {} + for listing, oid_at, stage_at in ((head, 2, None), (index, 1, 2)): + for record in listing.split(b"\0"): + meta, _, name = record.partition(b"\t") + fields = meta.split() + if len(fields) != 3 or fields[0] != GITLINK_MODE: + continue + if stage_at is not None and fields[stage_at] != b"0": + continue + path = name.decode("utf-8", "surrogateescape") + if path.split("/", 1)[0].casefold() == DEFAULT_MEMBERS_DIR: + found[path] = fields[oid_at].decode("ascii").lower() + return found + + +class RowPath(NamedTuple): + """What a row names by the path it gives: `who` names the row in a + finding, and `canonical` is the one path its project may have, + `members/`, or None for a row with no `project:`.""" + + who: str + canonical: str | None + + +def _row_paths(members: list) -> dict[str, RowPath]: + """`{path: RowPath}` for every row that gives a path: its `path:`, or + `members/` where it has none, defaulted as + `_check_member_repository_and_path` defaults it. `who` names the row in + a finding: `the row for `, or, for a row with no `project:` + (`member-project`'s), `a row with no project:`. + + A row NAMES the path it gives, AS WRITTEN (#361), and nothing else: a + hand-edited `path:` names that string, so the member's own mount is then + a gitlink no row names, beside the row's `member-path` finding. The + row's canonical path rides along, because what to say about a path that + folds to the row's depends on which of the two is the canonical one. + """ + named: dict[str, RowPath] = {} + for row in members: + if not isinstance(row, dict): + continue + project = row.get("project") + if not isinstance(project, str) or not project: + project = None + canonical = (f"{DEFAULT_MEMBERS_DIR}/{project}" + if project is not None else None) + if row.get("path"): + path = str(row["path"]) + elif canonical is not None: + path = canonical + else: + continue + named.setdefault(path, RowPath( + f"the row for {project}" if project else "a row with no project:", + canonical)) + return named + + +def _case_variant(path: str, + named: dict[str, RowPath]) -> tuple[str, RowPath] | None: + """`(row path, row)` of a row naming `path` in another case, or None. + + `str.casefold()`, as `member-duplicate` folds (#331): on macOS and + Windows `Members/Atlas` IS `members/Atlas`, so a finding about the one + says which row the other belongs to. + """ + folded = path.casefold() + for row_path, row in named.items(): + if row_path.casefold() == folded: + return row_path, row + return None + + +#: The three things a path that folds to a row's path can be, and so what a +#: finding about it may tell an operator to run: `STRAY`, a second path to +#: the member's own directory, which a command can take out; `ROW_EDITED`, +#: the member's own path, whose row was hand-edited into a case variant of +#: it; and `WRONG_CASE`, the only record of the member there is, in the +#: wrong case. +STRAY, ROW_EDITED, WRONG_CASE = "stray", "row-edited", "wrong-case" + +#: What a gitlink or a section nobody can account for says to do when the +#: fault may be in the row, in `git log -p -- family.yaml`'s words. +RESTORE_ROW = ("If its row was deleted or its path: edited by hand, put the " + "row back as git last had it (git log -p -- family.yaml shows " + "the change).") + + +def _variant_case(path: str, row_path: str, row: RowPath, + holds: set[str]) -> str: + """Which of `STRAY`, `ROW_EDITED` and `WRONG_CASE` the path `path`, which + folds to the path `row_path` that `row` names, is (#361). + + `holds` is every exact path that holds what is being asked about: the + gitlinks, or the paths the sections give. A command that takes `path` out + leaves the member standing in ONE state only, `STRAY`: the row's path is + its canonical `members/` AND that exact path holds a gitlink (a + section) too, so `path` is a second one. In the other two it takes the + member's own record out, and every mode that does not check members would + then say `family ok`: + + - `ROW_EDITED`: `path` IS the row's canonical path and the row spells it + another way, a mount at `members/Nova` and a row written `Members/Nova`. + The row is the hand edit, and the cure is in `family.yaml`. + - `WRONG_CASE`, any other: the row's path is canonical with nothing at it, + so `path` is the only record there is, in the wrong case; or the row's + path is no canonical one either and `path` is a third spelling, which + this does not decide while the row is wrong. + """ + if row_path == row.canonical and row_path in holds: + return STRAY + if path == row.canonical: + return ROW_EDITED + return WRONG_CASE + + +def _variant_said(row_path: str, row: RowPath) -> str: + """The sentence that says a path is a case variant of a row's.""" + return (f" It is a case variant of {row_path}, which {row.who} names, and " + "macOS and Windows hold the two as one directory.") + + +def _variant_advice(kind: str, path: str, row_path: str, row: RowPath, + what: str) -> str: + """What a `ROW_EDITED` or `WRONG_CASE` path says, in place of a command. + + `what` is `gitlink` or `section`. No command is printed in either: the + `git rm --cached` or the `--unset-all` lines that suit a `STRAY` would + take the member's own record out, and a family whose members a runner + could not check out would then pass. A `ROW_EDITED` says the row is what + is wrong, and where to put its path back; a `WRONG_CASE` says the member + is recorded in the wrong case, and sends the operator to `git log -p -- + family.yaml`, as an unnamed path that no row folds to does (`RESTORE_ROW`). + """ + if kind == ROW_EDITED: + return (f" It is {path}, the one path {row.who} may have, and the row " + f"names it as {row_path} instead, which macOS and Windows " + "hold as one directory with it. The row's path: is the hand " + f"edit and this {what} is the member's own: put {path} back " + "in the row's path: (git log -p -- family.yaml shows the " + f"change). No command is printed, since this {what} is not " + "what is wrong.") + return (_variant_said(row_path, row) + + " The member is recorded in the wrong case, so no command is " + f"printed: taking this {what} out could un-register it. " + + RESTORE_ROW) + + +def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], + named: dict[str, RowPath], report: Report) -> None: + """Every gitlink in `members/` is at a path some row names EXACTLY — the + `member-gitlink-unnamed` finding (#361). + + Every member check starts from a row, so a gitlink no row names was + never asked about: a member mounted by hand with `git submodule add`, or + a row deleted by hand. A recursive clone of the holder fetches it all + the same, and so does `bootstrap.py`'s `git submodule update`, while + nothing pins, digests or identifies it. + + EXACTLY, as git compares paths. A gitlink a row names in another case is + still a finding, and says which of three it is (`_variant_case`). A + second path to the member's own directory is taken out of the index + alone with `git rm --cached`, which touches no file, and committed. The + member's own gitlink, which the ROW misspells, and the only record of a + member there is, in the wrong case, print no command: either could + un-register the member. Any other is taken out with a plain `git rm`, + which refuses while its checkout holds changes, then mounted with + `family.py add`: `add` cannot adopt an existing mount, and after `git rm + --cached` alone it refuses the checkout left in the way + (`member-path-taken`). `add` then names the object store `git rm` leaves + in `.git/modules`, if it is still there. `--member` is quoted as the + paths are, its project alone: `` is the operator's to fill in. + """ + holds = set(gitlinks) + for path, commit in sorted(gitlinks.items()): + if path in named: + continue + said = (f"{path} is a gitlink at {commit} that no row of members: " + "names, so a recursive clone of this holder fetches it and " + "nothing here pins, digests or identifies it.") + variant = _case_variant(path, named) + if variant is not None: + row_path, row = variant + kind = _variant_case(path, row_path, row, holds) + if kind != STRAY: + report.finding( + "member-gitlink-unnamed", + said + _variant_advice(kind, path, row_path, row, + "gitlink")) + continue + report.finding( + "member-gitlink-unnamed", + f"{said}{_variant_said(row_path, row)} Take it out of the " + "index alone, which touches no file, and commit " + f"that:\n git rm --cached -- {shlex.quote(path)}") + continue + parts = path.split("/") + member = (shlex.quote(parts[1]) if len(parts) == 2 and parts[1] + else "") + report.finding( + "member-gitlink-unnamed", + f"{said} {RESTORE_ROW} If it was mounted by hand, take it out " + "with the first command below, which refuses while its checkout " + f"holds changes and takes its {GITMODULES} section out too, then " + "mount it with the second, from a checkout of openRepoShape: " + "family.py add writes its row and its gitlink in one commit, and " + "names the object store git rm leaves under .git/modules if one " + "is still there.\n" + f" git rm -- {shlex.quote(path)}\n" + f" {PYTHON} scripts/family.py add --family-root " + f"{shlex.quote(str(root))} --member /{member}") + + +def _check_gitmodules_named(sections: dict[str, dict[str, str]], + gitlinks: dict[str, str], + named: dict[str, RowPath], report: Report) -> None: + """Every path a `.gitmodules` section gives is a path some row names + EXACTLY — the `gitmodules-path-unnamed` finding (#361). + + A section with no row is inert to a clone, which follows gitlinks, but + it is what the next `family.py add` of its name or its path meets: under + its own name git rewrites it in place and the member inherits every key + it held, and under another name `add` refuses `member-path-registered`. + It is what `family.py remove` leaves of two sections for one path + (#360), and a section for `Members/Atlas` is one `add` commits beside + the member's own, since git compares the two exactly: that one is still + a finding, and says which of three it is (`_variant_case`). Only the + section that is a second one beside the member's own is taken out. A + section with no path gives none and is not one of these. + + The remedy is #300's, by key, every key the section holds, one command + to a line: `--remove-section` cannot address a section whose name holds + `]`, and a key left behind is inherited. + + A section at the path of a gitlink whose finding says to `git rm` it + prints no command: that `git rm` takes the section out too, and a + `--unset-all` run after it exits 5, the key being gone. Where two sections give the + path (`gitmodules-path-twice`) it takes out only the one git reads for + it, the last in the file, so each of those keeps its own commands. + """ + holds = {keys["path"] for keys in sections.values() if "path" in keys} + given = Counter(keys["path"] for keys in sections.values() + if "path" in keys) + for name, keys in sections.items(): + path = keys.get("path") + if path is None or path in named: + continue + url = f"url {keys['url']}" if "url" in keys else "no url" + said = (f'{GITMODULES} gives the path {path} to the submodule named ' + f'"{name}" ({url}), and no row of members: names that path, ' + "so nothing pins what it registers, and the next family.py " + "add of its name or its path meets it.") + variant = _case_variant(path, named) + lead = "Take" + if variant is not None: + row_path, row = variant + kind = _variant_case(path, row_path, row, holds) + if kind != STRAY: + report.finding( + "gitmodules-path-unnamed", + said + _variant_advice(kind, path, row_path, row, + "section")) + continue + said += _variant_said(row_path, row) + else: + said += (" If its row was deleted or its path: edited by hand, " + "put the row back as git last had it.") + lead = "Otherwise take" + if path in gitlinks and given[path] == 1: + report.finding( + "gitmodules-path-unnamed", + f"{said} Otherwise the git rm printed above for the " + f"gitlink at {path} takes this section out too, so no " + "command is printed here.") + continue + removal = "".join( + f"\n git config -f {GITMODULES} --unset-all " + f"{shlex.quote(f'submodule.{name}.{key}')}" for key in keys) + report.finding( + "gitmodules-path-unnamed", + f"{said} {lead} the section out by hand, in a POSIX shell, every " + "key it holds, so a submodule later given its name inherits none " + f"of them, then commit {GITMODULES}:{removal}") + + def _check_members(root: Path, members: list, no_members: bool, report: Report) -> None: """Every row in `members:`, or the `--no-members` skip line. - Split from `main` for #137. The two checks that read a file alone — - `family.yaml`'s ids (#206) and `.gitmodules`'s paths (#300) — come - first, so `--no-members` asks them too. + Split from `main` for #137. Every check that needs no member checked + out comes first, so `--no-members` asks them too: the ones that read a + file alone, `family.yaml`'s names (#331) and ids (#206) and the paths + `.gitmodules` gives twice (#300), then the gitlinks and sections no row + names, which read the index and HEAD as well (#361). """ _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) - _check_gitmodules_paths(root, report) + sections = _gitmodules_sections(root) + _check_gitmodules_paths(sections, report) + named = _row_paths(members) + gitlinks = _member_gitlinks(root) + _check_gitlinks_named(root, gitlinks, named, report) + _check_gitmodules_named(sections, gitlinks, named, report) if no_members: print(f" -- {len(members)} member(s) SKIPPED (--no-members): " "their gitlinks, digests and project ids are not checked " diff --git a/tests/test_family_add_refusals.py b/tests/test_family_add_refusals.py index bec25186..9bba6078 100644 --- a/tests/test_family_add_refusals.py +++ b/tests/test_family_add_refusals.py @@ -1734,7 +1734,9 @@ def test_a_section_that_is_not_another_name_at_the_mount_path_is_added_past( member's own name for its own path — what `remove` leaves behind when a ghost came after the member's section (#300) — which git rewrites in place. Each commits ONE section for the mount path, with the member's - url, and the holder passes its own gate.""" + url. The holder passes its own gate, except where the other section's + path is one no row names, which is now a finding of its own (#361) and + the only one: the add did not make it, and did not make it worse.""" if other is not None: path, name = other gitmodules_with(holder, (f'[submodule "{name}"]\n\tpath = {path}\n' @@ -1751,7 +1753,16 @@ def test_a_section_that_is_not_another_name_at_the_mount_path_is_added_past( f"submodule.members/{SPARE}.url", cwd=holder).stdout.strip() == url assert git("status", "--porcelain", cwd=holder).stdout == "" - assert validate(holder).returncode == 0 + checked = validate(holder) + if other is None or other[0] == f"members/{SPARE}": + assert checked.returncode == 0, checked.stderr + checked.stdout + else: + assert checked.returncode == 1, checked.stderr + checked.stdout + assert [line.split(":", 1)[0] for line in checked.stderr.splitlines() + if line.startswith("FINDING")] == [ + "FINDING gitmodules-path-unnamed"], checked.stderr + assert (f".gitmodules gives the path {other[0]} to the submodule " + f'named "{other[1]}"') in checked.stderr offline.assert_left_nothing() @@ -1884,14 +1895,18 @@ def test_a_validator_with_no_git_to_run_refuses_instead_of_crashing( cannot answer — a refusal, exit 2 — never the traceback that exits 1, a finding's code. The holder's own validator is run IN THIS PROCESS with its one git call failing as a missing program does, so no PATH is - changed for anything else.""" + changed for anything else. The code is `git-not-runnable`, the one the + whole validator gives a missing git (#368, + `tests/test_validate_family_without_git.py`), and no longer + `gitmodules-unreadable`, which means a file git ran and could not + read.""" monkeypatch.setattr(sys, "path", list(sys.path)) spec = importlib.util.spec_from_file_location( "holder_validate_family", holder / "scripts" / "validate-family.py") module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) - def no_git(args, cwd, binary=False): + def no_git(args, cwd, binary=False, env=None): raise FileNotFoundError(2, "No such file or directory", "git") monkeypatch.setattr(module, "git_out", no_git) @@ -1899,11 +1914,11 @@ def no_git(args, cwd, binary=False): out, err = capsys.readouterr() assert code == 2, out + err assert err.splitlines() == [ - "REFUSED gitmodules-unreadable: .gitmodules could not be read as git " + "REFUSED git-not-runnable: .gitmodules could not be read as git " "reads it: git could not be run (No such file or directory)", - "Remediation: put git on PATH and run this again; this check reads " - "the file the way git does, and nothing could fetch this holder's " - "submodules without git either."] + "Remediation: put git on PATH and run this again. This check reads " + "the holder the way git does, and nothing could clone it or fetch " + "its members without git either."] def test_every_key_of_a_ghost_goes_so_a_later_member_of_its_name_inherits_none( diff --git a/tests/test_validate_family_duplicates.py b/tests/test_validate_family_duplicates.py index ac71aa92..f39d21ee 100644 --- a/tests/test_validate_family_duplicates.py +++ b/tests/test_validate_family_duplicates.py @@ -29,7 +29,9 @@ member, and a fixture with two directories whose names differ only in case cannot exist on macOS or Windows (`mount_twice` spells its second row `IRRS2` for that reason). That one test says so and skips there; every other row of -this module runs on all three platforms. +this module runs on all three platforms. Each member the holder mounts keeps +a row (`keeping_mounts`): a mount no row names is a finding of its own since +#361. """ from __future__ import annotations @@ -89,6 +91,17 @@ def rewrite(holder, rows: list) -> None: "Rewrite the members", cwd=holder) +def keeping_mounts(holder, rows: list) -> list: + """`rows`, then the holder's own row for each member it mounts that no + row in `rows` gives the path of. A mount no row names is a finding of + its own (#361, `member-gitlink-unnamed` and `gitmodules-path-unnamed`), + and every test here asks about the names `rows` spells, not about that, + so each member the holder mounts keeps a row.""" + given = {row.get("path") for row in rows} + return [*rows, *(row for row in manifest(holder)["members"] + if row["path"] not in given)] + + def findings(result) -> list[str]: """Every `FINDING` line, in the order they were printed.""" return [line for line in result.stderr.splitlines() @@ -156,8 +169,9 @@ def test_rows_that_differ_only_in_case_are_found_under_no_members( """Both rows are named as WRITTEN, the earlier one first, whichever of the two spellings that is, and the finding says why they are one.""" base = irrs_row(holder) - rewrite(holder, [spelled(base, first, "irrs-first"), - spelled(base, second, "irrs-second")]) + rewrite(holder, keeping_mounts(holder, [ + spelled(base, first, "irrs-first"), + spelled(base, second, "irrs-second")])) result = validate(holder, "--no-members") assert result.returncode == 1, result.stderr + result.stdout assert len(findings(result)) == 1, result.stderr @@ -173,7 +187,8 @@ def test_a_case_variant_is_not_the_exact_finding(holder): some disks. A reader told the wrong one goes looking for a row that is not there.""" base = irrs_row(holder) - rewrite(holder, [base, spelled(base, "irrs", "irrs-twin")]) + rewrite(holder, keeping_mounts( + holder, [base, spelled(base, "irrs", "irrs-twin")])) result = validate(holder, "--no-members") assert result.returncode == 1, result.stderr + result.stdout assert len(findings(result)) == 1, result.stderr @@ -192,8 +207,9 @@ def test_rows_that_a_disk_folds_but_lower_does_not_are_found_under_no_members( first, second = pair assert first.lower() != second.lower(), "the point is that lower() fails" base = irrs_row(holder) - rewrite(holder, [spelled(base, first, "irrs-first"), - spelled(base, second, "irrs-second")]) + rewrite(holder, keeping_mounts(holder, [ + spelled(base, first, "irrs-first"), + spelled(base, second, "irrs-second")])) result = run_script(holder / "scripts" / "validate-family.py", "--no-members", cwd=holder, env={"PYTHONUTF8": "1"}) @@ -210,7 +226,8 @@ def test_the_case_finding_does_not_assume_one_project(holder): because `family.py remove` cannot take out a row that was never mounted, which is the only kind a macOS or Windows holder can hold.""" base = irrs_row(holder) - rewrite(holder, [base, spelled(base, "irrs", "irrs-twin")]) + rewrite(holder, keeping_mounts( + holder, [base, spelled(base, "irrs", "irrs-twin")])) result = validate(holder, "--no-members") assert result.returncode == 1, result.stderr + result.stdout (line,) = findings(result) @@ -231,7 +248,8 @@ def test_each_later_row_is_one_finding_against_the_first_that_holds_its_name( base = irrs_row(holder) lower = spelled(base, "irrs", "irrs-two") title = spelled(base, "Irrs", "irrs-three") - rewrite(holder, [base, lower, title, dict(lower)]) + rewrite(holder, keeping_mounts(holder, + [base, lower, title, dict(lower)])) result = validate(holder, "--no-members") assert result.returncode == 1, result.stderr + result.stdout found = findings(result) diff --git a/tests/test_validate_family_member_path_glob.py b/tests/test_validate_family_member_path_glob.py new file mode 100644 index 00000000..681e1467 --- /dev/null +++ b/tests/test_validate_family_member_path_glob.py @@ -0,0 +1,138 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A hand-edited `path:` is `member-path`, and nothing is read through it +(#367); `recorded_gitlink` takes its path literally, for every caller. + +THREE THINGS COMBINED. `_check_member` found a row's `path:` wrong, recorded +`member-path`, and then went on to check the member's gitlink, tree and +identity AT that path. `recorded_gitlink` handed the path to `git ls-files` +as a pathspec, so `members/*` was a glob and answered with the first +member's gitlink, gathered as a false `member-gitlink-mismatch` for this +one. The tree check then refused `member-uninitialized`, whose remedy (`git +submodule update --init`, `make bootstrap`) cannot help, and a refusal +prints none of the findings gathered before it, so the one naming the hand +edit was never seen. + +Two of the issue's three decisions are taken. A row whose path is wrong +stops after its pin is read, and the finding names the one path it may +have. `recorded_gitlink` passes git's global `--literal-pathspecs` to both +of its listings, never `:(literal)` magic, which a caller's exported +`GIT_LITERAL_PATHSPECS=1` turns off; and the two pathspec settings git +refuses beside that option are kept out of those two calls, so a caller who +exports one does not read every gitlink as absent. The third decision, a +refusal that prints the findings gathered before it, is not taken. + +The row's own mount is then a gitlink and a section no row names (#361, +`tests/test_validate_family_unnamed_mounts.py`), reported beside +`member-path`: the row names the path it gives, as written. + +The same family and offline rules as `tests/test_family.py`. The hand edit +is to `family.yaml` alone, written as bytes; nothing named `*` is ever put +on disk, so every test here runs on Windows too. +""" + +from __future__ import annotations + +import pytest + +from conftest import run_script +from repo_shape import recorded_gitlink +from test_family import family, holder, manifest, validate # noqa: F401 + +#: What a hand edit can leave in IRSS's `path:`, from the issue: globs, +#: pathspec magic, a `..` and a trailing slash. Each is `member-path`. +SPELLINGS = ["members/*", "members/I*", "members/[I]*", ":(glob)members/IRSS", + "members/IRSS/..", "members/IRSS/"] + + +def hand_edit(holder, spelling: str) -> None: + """IRSS's row given `path: ""`, as BYTES, so Windows does not + rewrite the manifest with CRLF.""" + manifest_path = holder / "family.yaml" + text = manifest_path.read_bytes() + line = b" path: members/IRSS\n" + assert text.count(line) == 1, "fixture drift: IRSS's path line" + manifest_path.write_bytes(text.replace( + line, f' path: "{spelling}"\n'.encode("utf-8"))) + + +def codes(result) -> list[str]: + return [line.split(":", 1)[0][len("FINDING "):] + for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + + +def gitlinks(holder) -> dict[str, str]: + return {row["project"]: row["pin"]["commit"] + for row in manifest(holder)["members"]} + + +@pytest.mark.parametrize("extra", [(), ("--pins",)], + ids=["members-checked", "pins"]) +@pytest.mark.parametrize("spelling", SPELLINGS) +def test_a_hand_edited_path_is_member_path_and_nothing_read_through_it( + holder, spelling, extra): + """THE ISSUE'S REPRO, in both modes that check members: the run used to + end in `REFUSED member-uninitialized`, exit 2, with the `member-path` + finding and a false `member-gitlink-mismatch` gathered and dropped. Now + `member-path` is printed, naming the one path the row may have, nothing + is read at the path the row gives, and the run is a finding, exit 1. + IRSS's own mount is then a gitlink and a section no row names (#361), + and nothing else is said.""" + hand_edit(holder, spelling) + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + assert "REFUSED" not in result.stderr, result.stderr + assert "member-uninitialized" not in result.stderr + assert "member-gitlink-mismatch" not in result.stderr + assert codes(result) == ["member-gitlink-unnamed", + "gitmodules-path-unnamed", "member-path"] + assert (f"FINDING member-path: IRSS: path is {spelling!r}, expected " + "'members/IRSS', the one path a row for IRSS may have. Its " + "gitlink, digest and project id are not checked through any " + "other.") in result.stderr.splitlines() + + +def test_the_other_rows_are_still_checked(holder): + """Only the edited row stops: IRRS's gitlink, digest and id are read as + before, and say so.""" + hand_edit(holder, "members/*") + result = validate(holder) + assert " ok members/IRRS: gitlink == pin" in result.stdout + assert " ok members/IRRS: project.yaml id irrs" in result.stdout + assert "members/IRSS: gitlink" not in result.stdout + + +@pytest.mark.parametrize("spelling", ["members/*", "members/I*", + "members/[I]*", ":(glob)members/IRRS"]) +def test_recorded_gitlink_takes_its_path_literally(holder, spelling): + """In a holder with members, and no file named `*` on disk, a glob or a + magic pathspec names no entry, so there is no gitlink there; the + member's own path still answers with its pin. The standard's + `repo_shape.py` is the one under test: every holder and every project + receives a copy of it.""" + pins = gitlinks(holder) + assert not any("*" in entry.name + for entry in (holder / "members").iterdir()) + assert recorded_gitlink(holder, spelling) is None + assert recorded_gitlink(holder, "members/IRRS") == pins["IRRS"] + + +@pytest.mark.parametrize("variable", [ + "GIT_GLOB_PATHSPECS", "GIT_ICASE_PATHSPECS", "GIT_LITERAL_PATHSPECS", + "GIT_NOGLOB_PATHSPECS"]) +def test_a_callers_pathspec_setting_neither_widens_nor_breaks_it( + holder, monkeypatch, variable): + """git refuses `--literal-pathspecs` beside an exported + `GIT_GLOB_PATHSPECS` or `GIT_ICASE_PATHSPECS` (exit 128), and a listing + git refused reads as no gitlink, so those two are kept out of the + listings' environment: the gitlink is found, and a glob still finds + none. `GIT_LITERAL_PATHSPECS`, which turns `:(literal)` magic off, and + `GIT_NOGLOB_PATHSPECS` change nothing either. The holder's own gate, + run with the variable exported, passes.""" + monkeypatch.setenv(variable, "1") + pins = gitlinks(holder) + assert recorded_gitlink(holder, "members/IRRS") == pins["IRRS"] + assert recorded_gitlink(holder, "members/*") is None + result = run_script(holder / "scripts" / "validate-family.py", "--pins", + cwd=holder, env={variable: "1"}) + assert result.returncode == 0, result.stderr + result.stdout diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py new file mode 100644 index 00000000..5791a49a --- /dev/null +++ b/tests/test_validate_family_unnamed_mounts.py @@ -0,0 +1,715 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-family.py` finds a gitlink in `members/`, and a `.gitmodules` +section, whose path no `members:` row names, in every mode (#361). + +EVERY CHECK STARTED FROM A ROW. `_check_members` walked `members:` and asked +each row for its gitlink, digest and identity, and nothing walked back from +what the holder records to a row. So a member mounted by hand with `git +submodule add`, a row deleted by hand, the section `family.py remove` leaves +of two for one path (#360), and a section for `Members/IRRS`, which `add` +commits beside the member's own, all passed the holder's own gate, and the +doctor, which runs that gate, called the holder COMPLIANT. + +TWO FINDINGS, each with its own code, both made before the `--no-members` +return, as `member-duplicate` (#331) is, since a private family's CI falls +back to that mode: `member-gitlink-unnamed` for a gitlink, read from the +index at stage 0, else HEAD, the order `recorded_gitlink` reads one in, and +`gitmodules-path-unnamed` for a section, read as git reads the file, as +`gitmodules-path-twice` (#300) reads it. "Unnamed" is decided EXACTLY, as +git compares paths. A path a row names in another case is still a finding, +and it names that row: macOS and Windows hold the two as one directory. + +A PATH THAT FOLDS TO A ROW'S is one of three things, and what it says to run +depends on which (review of #406, SHOULD-1). A SECOND path to the member's own +directory is taken out with `git rm --cached`. A row HAND-EDITED into a case +variant of its own mount (`path: Members/IRSS` over a mount at +`members/IRSS`) is the row's fault, and a member whose ONLY record is at a +case variant (`members/irss`, the row saying `members/IRSS`) is recorded in +the wrong case: the same command, run in either, takes the member's own +record out of the index, after which `--no-members`, the run a private +family's CI falls back to, says `family ok` over a member that is no longer +there. Those two print no command. + +When a hand mount's gitlink says to `git rm` it, its section finding says that +the `git rm` takes the section out too and prints no `--unset-all` line: run +after it they exit 5, the keys being gone. + +The same family and the same offline rules as `tests/test_family.py`, whose +fixture is imported. EVERY STATE BUT ONE IS BUILT IN GIT'S INDEX AND +OBJECTS, never through a checkout, so each runs on all three platforms: a +gitlink is an index record written as bytes on stdin (`update-index +--index-info`), and a section is appended to `.gitmodules` by +`tests/test_family_add_refusals.py`'s `gitmodules_with`, which hashes the +file's bytes the same way. The one exception runs the printed remedy, and +mounts the spare member for real with `git submodule add`, as +`tests/test_validate_family_duplicates.py` does, because what that remedy +must clear is what a real mount leaves. + +THE CASE-VARIANT STATES HOLD ON macOS AND WINDOWS the same way: a disk that +folds case makes `members/IRSS` and `members/irss` one directory, so none of +them puts a second directory on disk. A gitlink is only ever an index record +(`stage_gitlink`, `git rm --cached`) and a commit of the index, which reads no +directory, so the checkout of `members/IRSS` stays the one directory it is and +the index says whichever spelling the test wants. A section is hashed into +the objects and the one `.gitmodules` file is written beside it, and the one +hand edit, `family.yaml`, is a single file written as bytes. +""" + +from __future__ import annotations + +import shlex +import subprocess + +import pytest + +from conftest import git, rmtree, run_script +from repo_shape import PYTHON +from test_family import ( # noqa: F401 + FAMILY, ORG, SPARE, family, family_module, holder, manifest, validate, +) +from test_family_add_refusals import git_section, gitmodules_with +from test_validate_family_member_path_glob import hand_edit + +MODES = pytest.mark.parametrize( + "extra", [(), ("--pins",), ("--no-members",)], + ids=["members-checked", "pins", "no-members"]) + +#: What every unnamed gitlink says after its path and commit, and what one +#: that is NOT a case variant of a row's path says to do about it. A +#: holder's CI matches on text, so it is pinned here and not rebuilt from the +#: validator's own strings. +GITLINK_SAID = ( + "FINDING member-gitlink-unnamed: {path} is a gitlink at {commit} that " + "no row of members: names, so a recursive clone of this holder fetches " + "it and nothing here pins, digests or identifies it.") +GITLINK_REMEDY = ( + " If its row was deleted or its path: edited by hand, put the row back " + "as git last had it (git log -p -- family.yaml shows the change). If it " + "was mounted by hand, take it out with the first command below, which " + "refuses while its checkout holds changes and takes its .gitmodules " + "section out too, then mount it with the second, from a checkout of " + "openRepoShape: family.py add writes its row and its gitlink in one " + "commit, and names the object store git rm leaves under .git/modules if " + "one is still there.") + +#: The same two halves for an unnamed `.gitmodules` section. +SECTION_SAID = ( + "FINDING gitmodules-path-unnamed: .gitmodules gives the path {path} to " + "the submodule named \"{name}\" ({url}), and no row of members: names " + "that path, so nothing pins what it registers, and the next family.py " + "add of its name or its path meets it.") +SECTION_REMEDY = ( + " If its row was deleted or its path: edited by hand, put the row back " + "as git last had it. Otherwise take the section out by hand, in a POSIX " + "shell, every key it holds, so a submodule later given its name inherits " + "none of them, then commit .gitmodules:") + +#: What a path a row names in another case adds, in place of the "deleted +#: or edited" sentence: the row it belongs to. +VARIANT = (" It is a case variant of {row_path}, which the row for " + "{project} names, and macOS and Windows hold the two as one " + "directory.") + +#: What a path that is the member's own, in a row hand-edited into a case +#: variant of it, says: the row is the hand edit, and no command. `{what}` is +#: `gitlink` or `section`. +ROW_EDITED = ( + " It is {path}, the one path the row for {project} may have, and the row " + "names it as {row_path} instead, which macOS and Windows hold as one " + "directory with it. The row's path: is the hand edit and this {what} is " + "the member's own: put {path} back in the row's path: (git log -p -- " + "family.yaml shows the change). No command is printed, since this {what} " + "is not what is wrong.") + +#: What the only record of a member, in the wrong case, says: no command, and +#: the restore sentence the unnamed gitlink always had. +WRONG_CASE = ( + VARIANT + " The member is recorded in the wrong case, so no command is " + "printed: taking this {what} out could un-register it. If its row was " + "deleted or its path: edited by hand, put the row back as git last had " + "it (git log -p -- family.yaml shows the change).") + +#: What a hand mount's section says once its gitlink's `git rm` takes it out. +SECTION_UNDER_GIT_RM = ( + " If its row was deleted or its path: edited by hand, put the row back as " + "git last had it. Otherwise the git rm printed above for the gitlink at " + "{path} takes this section out too, so no command is printed here.") + + +def findings(result) -> list[str]: + """Every finding WHOLE, in the order printed: its `FINDING` line and + each indented line under it, the commands a remedy prints.""" + found: list[list[str]] = [] + for line in result.stderr.splitlines(): + if line.startswith("FINDING "): + found.append([line]) + elif found and line.startswith(" "): + found[-1].append(line) + return ["\n".join(lines) for lines in found] + + +def codes(result) -> list[str]: + """The code of every finding, in the order printed.""" + return [finding.split(":", 1)[0][len("FINDING "):] + for finding in findings(result)] + + +def commands(finding: str) -> list[str]: + """The commands a finding prints, one to an indented line.""" + return [line.strip() for line in finding.splitlines()[1:]] + + +def stage_gitlink(root, path: str, commit: str) -> None: + """A gitlink at `path` in `root`'s index and nowhere else: an index + record written as BYTES on stdin, so no disk has to hold `path` and no + clone is made. `--index-info` replaces a record already there.""" + subprocess.run(["git", "update-index", "--add", "--index-info"], + cwd=str(root), capture_output=True, check=True, + input=f"160000 {commit}\t{path}\n".encode("utf-8")) + + +def stage_unmerged_gitlink(root, path: str, commit: str) -> None: + """A gitlink at `path` held at the conflict stages 1, 2 and 3 and NOT at + stage 0, as a conflicted merge leaves it. As BYTES on stdin, like + `stage_gitlink`: first the zero-mode record that drops any stage-0 entry + (`--index-info` will not place a higher stage over one), then the three + stages.""" + records = f"0 {'0' * 40}\t{path}\n" + "".join( + f"160000 {commit} {stage}\t{path}\n" for stage in (1, 2, 3)) + subprocess.run(["git", "update-index", "--index-info"], cwd=str(root), + capture_output=True, check=True, + input=records.encode("utf-8")) + + +def commit(root, message: str) -> None: + git("commit", "-qm", message, cwd=root) + + +def record_in_another_case(holder, project: str, spelling: str) -> str: + """`project`'s gitlink taken out of the index at its own path, recorded at + `spelling` instead, and committed: in the index and objects alone, so no + disk has to hold both names and the checkout does not move. Returns the + commit the gitlink names.""" + named = pinned(holder, project) + git("rm", "-q", "--cached", "--", f"members/{project}", cwd=holder) + stage_gitlink(holder, spelling, named) + commit(holder, f"Record {project} at {spelling}") + return named + + +def spare_commits(family) -> tuple[str, str]: + """The spare member's two commits, older first: real commits a gitlink + can name, though the holder never mounts them.""" + work = family["work"] / SPARE + older, newer = (git("rev-parse", rev, cwd=work).stdout.strip() + for rev in ("HEAD~1", "HEAD")) + return older, newer + + +def pinned(holder, project: str) -> str: + return next(row["pin"]["commit"] for row in manifest(holder)["members"] + if row["project"] == project) + + +def with_path_replaced(holder, old: bytes, new: bytes) -> None: + """HEAD's `.gitmodules` with `old` replaced by `new` (once), hashed into + git's objects and committed, and the same bytes written to disk: + `gitmodules_with`'s mechanics for an edit in place rather than an append, + as BYTES, so no platform rewrites a line ending.""" + data = subprocess.run(["git", "show", "HEAD:.gitmodules"], + cwd=str(holder), capture_output=True, + check=True).stdout + assert data.count(old) == 1, data + data = data.replace(old, new) + blob = subprocess.run(["git", "hash-object", "-w", "--stdin"], + cwd=str(holder), input=data, capture_output=True, + check=True).stdout.decode("ascii").strip() + git("update-index", "--cacheinfo", f"100644,{blob},.gitmodules", + cwd=holder) + commit(holder, "A section whose path is another case") + (holder / ".gitmodules").write_bytes(data) + + +def with_section(holder, tmp_path, name: str, path: str, url: str) -> None: + """`[submodule ""]` giving `path` and `url`, appended to HEAD's + `.gitmodules` in git's objects and committed, with the same bytes on + disk.""" + gitmodules_with(holder, git_section(tmp_path / "section", name, + path=path, url=url), "the work tree") + + +def removal(name: str, *keys: str) -> list[str]: + """The commands that take every key of section `name` out, as printed.""" + return [f"git config -f .gitmodules --unset-all " + f"{shlex.quote(f'submodule.{name}.{key}')}" for key in keys] + + +def take_out_as_printed(holder, finding: str) -> None: + """Run every command `finding` prints, split as a POSIX shell splits it + and run with no shell, then commit `.gitmodules`, as it says.""" + for command in commands(finding): + argv = shlex.split(command) + assert argv[0] == "git", command + git(*argv[1:], cwd=holder) + git("add", "--", ".gitmodules", cwd=holder) + commit(holder, "Take the section out as printed") + + +def assert_passes_in_every_mode(holder, members: int) -> None: + for extra in ((), ("--pins",), ("--no-members",)): + result = validate(holder, *extra) + assert result.returncode == 0, (extra, result.stderr + result.stdout) + assert (f"family ok: InkRouter (inkrouter), {members} member(s)" + in result.stdout), extra + + +# --- the issue's three rows -------------------------------------------------- + +@MODES +def test_a_member_mounted_by_hand_is_two_findings(family, holder, tmp_path, + extra): + """ROW (a): what `git submodule add` of a member leaves, with no row: a + gitlink in `members/` that every recursive clone fetches, and the + section that registers it. Neither is named, so each is its own + finding, in every mode: neither needs the member checked out. + + The gitlink says to `git rm` it, and that takes its section out too, so + the section's finding says that and prints no `--unset-all` line: run + after the `git rm` they exit 5, the keys being gone.""" + _, tip = spare_commits(family) + stage_gitlink(holder, f"members/{SPARE}", tip) + with_section(holder, tmp_path, f"members/{SPARE}", f"members/{SPARE}", + f"/nowhere/{SPARE}.git") + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + assert "REFUSED" not in result.stderr, result.stderr + gitlink, section = findings(result) + assert gitlink.splitlines()[0] == GITLINK_SAID.format( + path=f"members/{SPARE}", commit=tip) + GITLINK_REMEDY + assert commands(gitlink) == [ + f"git rm -- members/{SPARE}", + f"{PYTHON} scripts/family.py add --family-root " + f"{shlex.quote(str(holder.resolve()))} --member /{SPARE}"] + assert section == SECTION_SAID.format( + path=f"members/{SPARE}", name=f"members/{SPARE}", + url=f"url /nowhere/{SPARE}.git") + SECTION_UNDER_GIT_RM.format( + path=f"members/{SPARE}") + assert commands(section) == [] + assert "--unset-all" not in result.stderr + + +@MODES +def test_a_section_no_row_names_is_a_finding(holder, tmp_path, extra): + """ROW (b): a section with no row and no gitlink, the state `family.py + remove` leaves when the file held two sections for one path (#360). A + clone does not follow it, but the next `add` of its name or its path + meets it, so it is a finding, in every mode.""" + with_section(holder, tmp_path, "members/Old", "members/Old", + "/nowhere/old.git") + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + (section,) = findings(result) + assert section.splitlines()[0] == SECTION_SAID.format( + path="members/Old", name="members/Old", + url="url /nowhere/old.git") + SECTION_REMEDY + assert commands(section) == removal("members/Old", "path", "url") + + +@MODES +def test_a_section_for_a_case_variant_of_a_members_path_names_that_row( + holder, tmp_path, extra): + """ROW (c): a section under another name for `Members/IRRS`, beside + IRRS's own for `members/IRRS`. git compares the two paths exactly, so + `add` committed IRRS's section beside it and `gitmodules-path-twice` + does not pair them, but on macOS and Windows they are one directory. + So it is unnamed, decided exactly, and the finding names the row whose + path it is the variant of.""" + with_section(holder, tmp_path, "ghost", "Members/IRRS", + "/nowhere/ghost.git") + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + (section,) = findings(result) + assert section.splitlines()[0] == ( + SECTION_SAID.format(path="Members/IRRS", name="ghost", + url="url /nowhere/ghost.git") + + VARIANT.format(row_path="members/IRRS", project="IRRS") + + " Take the section out by hand, in a POSIX shell, every key it " + "holds, so a submodule later given its name inherits none of them, " + "then commit .gitmodules:") + assert commands(section) == removal("ghost", "path", "url") + + +# --- the two findings' edges ------------------------------------------------- + +@MODES +def test_a_gitlink_at_a_case_variant_of_a_members_path_names_that_row( + holder, extra): + """A gitlink at `Members/IRRS` beside IRRS's at `members/IRRS`, in the + index only, so the same on every disk. Its directory IS IRRS's where + case is folded, so its remedy is `git rm --cached`, which touches no + file, and never the plain `git rm` that would take IRRS's checkout with + it there.""" + commit_ = pinned(holder, "IRRS") + stage_gitlink(holder, "Members/IRRS", commit_) + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + (gitlink,) = findings(result) + assert gitlink.splitlines()[0] == ( + GITLINK_SAID.format(path="Members/IRRS", commit=commit_) + + VARIANT.format(row_path="members/IRRS", project="IRRS") + + " Take it out of the index alone, which touches no file, and " + "commit that:") + assert commands(gitlink) == ["git rm --cached -- Members/IRRS"] + + +def test_the_case_variant_remedy_run_as_printed_clears_it_and_no_file(holder): + """Run as printed, it takes the variant out of the index and leaves + IRRS's checkout and gitlink as they were, and the holder passes.""" + stage_gitlink(holder, "Members/IRRS", pinned(holder, "IRRS")) + (gitlink,) = findings(validate(holder, "--no-members")) + (command,) = commands(gitlink) + git(*shlex.split(command)[1:], cwd=holder) + assert (holder / "members" / "IRRS" / "project.yaml").is_file() + assert_passes_in_every_mode(holder, 2) + + +def test_a_gitlink_is_read_from_the_index_first_and_from_head_second( + family, holder): + """THE ORDER `recorded_gitlink` READS ONE. A gitlink HEAD records at one + commit and the index stages at another is reported at the staged one, + which the next commit will record; once the index no longer holds it, + HEAD's is the gitlink, and it is still a finding until a commit drops + it.""" + older, newer = spare_commits(family) + path = f"members/{SPARE}" + stage_gitlink(holder, path, older) + commit(holder, "A gitlink no row names") + stage_gitlink(holder, path, newer) + (staged,) = findings(validate(holder, "--no-members")) + assert staged.startswith(GITLINK_SAID.format(path=path, commit=newer)) + git("rm", "-q", "--cached", "--", path, cwd=holder) + (committed,) = findings(validate(holder, "--no-members")) + assert committed.startswith(GITLINK_SAID.format(path=path, commit=older)) + commit(holder, "Drop it") + assert_passes_in_every_mode(holder, 2) + + +def test_only_members_is_asked_about(family, holder): + """`members/` in any case, and nothing beside it: a gitlink elsewhere + in the holder is not a member's mount, and no row could name it.""" + _, tip = spare_commits(family) + stage_gitlink(holder, f"tools/{SPARE}", tip) + stage_gitlink(holder, f"membersX/{SPARE}", tip) + stage_gitlink(holder, f"MEMBERS/{SPARE}", tip) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + (gitlink,) = findings(result) + assert gitlink.startswith(GITLINK_SAID.format(path=f"MEMBERS/{SPARE}", + commit=tip)) + + +def test_a_removed_row_is_named_by_its_gitlink_and_its_section(holder): + """A row deleted by hand leaves the member's gitlink and its section, + and both are found; putting the row back, as both say, clears them.""" + text = (holder / "family.yaml").read_bytes() + rows = manifest(holder)["members"] + irss = next(row for row in rows if row["project"] == "IRSS") + family_module().Family.open(holder).write_members( + [row for row in rows if row["project"] != "IRSS"]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + assert codes(result) == ["member-gitlink-unnamed", + "gitmodules-path-unnamed"] + assert findings(result)[0].startswith(GITLINK_SAID.format( + path="members/IRSS", commit=irss["pin"]["commit"]) + GITLINK_REMEDY) + (holder / "family.yaml").write_bytes(text) + assert_passes_in_every_mode(holder, 2) + + +@pytest.mark.parametrize("row", ["b", "c"]) +def test_a_sections_removal_run_as_printed_clears_it(holder, tmp_path, row): + """The commands an unnamed section prints, typed exactly as printed, + take every key out, and the holder passes again.""" + name, path = (("members/Old", "members/Old") if row == "b" else + ("ghost", "Members/IRRS")) + with_section(holder, tmp_path, name, path, "/nowhere/x.git") + (section,) = findings(validate(holder, "--no-members")) + take_out_as_printed(holder, section) + assert name not in (holder / ".gitmodules").read_text(encoding="utf-8") + assert_passes_in_every_mode(holder, 2) + + +def test_an_index_git_cannot_list_is_a_refusal_not_a_pass(holder): + """The gitlink check is the one reader of `members/` that `--no-members` + runs, so an index git cannot list is a question it cannot answer: a + refusal, exit 2, never an implicit pass.""" + (holder / ".git" / "index").write_bytes(b"not an index\n") + result = validate(holder, "--no-members") + assert result.returncode == 2, result.stderr + result.stdout + assert result.stderr.startswith( + "REFUSED gitlinks-unreadable: the index could not be listed, so the " + "gitlinks it records in members/ could not be read: `git ls-files " + "-s -z` in "), result.stderr + assert result.stderr.splitlines()[-1] == ( + "Remediation: git status in this repository says what is wrong with " + "its index. Mend that, then run this again.") + + +# --- a path that folds to a row's: only a second one is taken out ------------ + +def url_of(holder, project: str) -> str: + """The url `family.py add` wrote in `project`'s section.""" + return git("config", "-f", ".gitmodules", "--get", + f"submodule.members/{project}.url", + cwd=holder).stdout.strip() + + +@MODES +def test_a_row_hand_edited_into_a_case_variant_of_its_mount_is_the_edit( + holder, extra): + """STATE (i), REVIEW SHOULD-1: IRSS's row says `path: Members/IRSS` and + the real mount is `members/IRSS`. The mount is the canonical path and the + ROW is the hand edit, so each finding says that and names no command: the + `git rm --cached` and the `--unset-all` lines these used to print took + the member's own gitlink and section out, after which `--no-members` said + `family ok` over a family with one member. Members checked, `member-path` + names the edit beside them; `--no-members` never runs it, which is why + the finding must say it.""" + original = (holder / "family.yaml").read_bytes() + hand_edit(holder, "Members/IRSS") + commit_ = pinned(holder, "IRSS") + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == ( + ["member-gitlink-unnamed", "gitmodules-path-unnamed"] + + ([] if extra == ("--no-members",) else ["member-path"])) + said = dict(path="members/IRSS", project="IRSS", row_path="Members/IRSS") + gitlink, section = findings(result)[:2] + assert gitlink == GITLINK_SAID.format( + path="members/IRSS", commit=commit_) + ROW_EDITED.format( + what="gitlink", **said) + assert section == SECTION_SAID.format( + path="members/IRSS", name="members/IRSS", + url=f"url {url_of(holder, 'IRSS')}") + ROW_EDITED.format( + what="section", **said) + assert "git rm" not in result.stderr, result.stderr + assert "--unset-all" not in result.stderr, result.stderr + # Said and done: the path put back in the row clears all three. + (holder / "family.yaml").write_bytes(original) + assert_passes_in_every_mode(holder, 2) + + +@MODES +def test_a_member_recorded_only_in_another_case_says_so_and_prints_no_command( + holder, extra): + """STATE (ii), REVIEW SHOULD-1: IRSS's row is canonical, and the only + gitlink the holder records for it is at `members/irss`. `git rm --cached` + of that took the member's only record out, and `--no-members` then said + `family ok` over a holder that records one gitlink fewer than it has rows. + It says the member is recorded in the wrong case instead, and sends the + operator to git's history, as every other unnamed gitlink does. + + Members checked, the row's own gitlink is read at its exact path, finds + none and is the refusal `member-gitlink-absent`, which prints no finding, + so only `--no-members` shows the finding; a git that matched the pathspec + in any case would find the variant and print it, so either is accepted + there, and what no run may do is print a command.""" + named = record_in_another_case(holder, "IRSS", "members/irss") + result = validate(holder, *extra) + assert "git rm" not in result.stderr + result.stdout + assert "--unset-all" not in result.stderr + result.stdout + if result.returncode == 2: + assert extra != ("--no-members",), result.stderr + assert result.stderr.startswith( + "REFUSED member-gitlink-absent: no gitlink (mode 160000) " + "recorded at 'members/IRSS'."), result.stderr + return + assert result.returncode == 1, result.stderr + result.stdout + (gitlink,) = findings(result) + assert gitlink == GITLINK_SAID.format( + path="members/irss", commit=named) + WRONG_CASE.format( + row_path="members/IRSS", project="IRSS", what="gitlink") + + +@MODES +def test_a_sections_only_path_in_another_case_says_so_and_prints_no_command( + holder, extra): + """The section's own state (ii): IRSS's row is canonical, its gitlink is + where it belongs, and the ONLY section that registers it gives + `members/irss`. `--unset-all` of that left the member with no section.""" + url = url_of(holder, "IRSS") + with_path_replaced(holder, b"\tpath = members/IRSS\n", + b"\tpath = members/irss\n") + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + (section,) = findings(result) + assert section == SECTION_SAID.format( + path="members/irss", name="members/IRSS", url=f"url {url}" + ) + WRONG_CASE.format(row_path="members/IRSS", project="IRSS", + what="section") + assert "--unset-all" not in result.stderr, result.stderr + + +def test_a_third_spelling_beside_an_edited_row_prints_no_command_either( + holder): + """The row is hand-edited AND a gitlink sits at a third spelling, + `MEMBERS/IRSS`: neither the row's path nor the canonical one. Nothing here + says it is a second path to a member that is standing, so it is not told + to `git rm --cached`; the row is wrong first.""" + hand_edit(holder, "Members/IRSS") + stage_gitlink(holder, "MEMBERS/IRSS", pinned(holder, "IRSS")) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + third = next(f for f in findings(result) + if f.startswith("FINDING member-gitlink-unnamed: MEMBERS/")) + assert commands(third) == [] + assert "The member is recorded in the wrong case, so no command" in third + assert "git rm" not in result.stderr, result.stderr + + +def test_two_sections_for_a_hand_mounts_path_each_keep_their_commands( + family, holder, tmp_path): + """`git rm` of a gitlink takes out ONE section for its path, the last the + file gives, so the `git rm` finding cannot say it takes out both: where a + path is given twice (`gitmodules-path-twice`), each section keeps its own + `--unset-all` lines. The last assertion is the premise: after the `git + rm` the finding prints, one of the two sections is still there.""" + _, tip = spare_commits(family) + path = f"members/{SPARE}" + stage_gitlink(holder, path, tip) + with_section(holder, tmp_path, path, path, f"/nowhere/{SPARE}.git") + with_section(holder, tmp_path, "second", path, "/nowhere/second.git") + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + assert codes(result) == ["gitmodules-path-twice", "member-gitlink-unnamed", + "gitmodules-path-unnamed", + "gitmodules-path-unnamed"] + first, second = findings(result)[2:] + assert commands(first) == removal(path, "path", "url") + assert commands(second) == removal("second", "path", "url") + assert first.splitlines()[0].endswith(SECTION_REMEDY) + assert "takes this section out too" not in result.stderr + git(*shlex.split(commands(findings(result)[1])[0])[1:], cwd=holder) + listed = git("config", "-f", ".gitmodules", "--list", cwd=holder).stdout + assert listed.count(f".path={path}") == 1, listed + + +# --- a conflicted index records no gitlink at stage 0 ------------------------ + +def test_a_gitlink_held_only_at_conflict_stages_is_not_recorded(family, + holder): + """`_member_gitlinks` reads the index at STAGE 0, as `recorded_gitlink` + does: a conflicted merge holds a path at stages 1, 2 and 3, and none of + them is a commit anybody is about to make. A path HEAD never recorded and + the index holds only so is no gitlink the holder records.""" + _, tip = spare_commits(family) + stage_unmerged_gitlink(holder, f"members/{SPARE}", tip) + listed = git("ls-files", "-s", "--", f"members/{SPARE}", + cwd=holder).stdout.split() + assert listed[2::4] == ["1", "2", "3"], listed + assert_passes_in_every_mode(holder, 2) + + +def test_a_gitlink_head_records_is_head_s_when_the_index_holds_it_unmerged( + family, holder): + """The fallback to HEAD, as `recorded_gitlink` falls back: HEAD's gitlink + is the one named, and no stage of the conflict is, whichever came last.""" + older, newer = spare_commits(family) + path = f"members/{SPARE}" + stage_gitlink(holder, path, older) + commit(holder, "A gitlink no row names") + stage_unmerged_gitlink(holder, path, newer) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + (gitlink,) = findings(result) + assert gitlink.startswith(GITLINK_SAID.format(path=path, commit=older)) + + +# --- the add remedy quotes the member it names ------------------------------- + +@pytest.mark.parametrize("project,quoted", [ + ("My Proj", "/'My Proj'"), + ("O'Brien", "/'O'\"'\"'Brien'")], ids=["space", "quote"]) +def test_the_add_remedy_quotes_the_member_it_names(family, holder, project, + quoted): + """`--member` is quoted as the paths beside it are, its project alone: + `` is the operator's to fill in, and quoted whole it would be a + literal org of that name. Split as a POSIX shell splits it, the value is + the org and the project as written.""" + _, tip = spare_commits(family) + stage_gitlink(holder, f"members/{project}", tip) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + (gitlink,) = findings(result) + rm, add = commands(gitlink) + assert add.endswith(f" --member {quoted}"), add + assert shlex.split(add)[-1] == f"/{project}" + assert shlex.split(rm)[-1] == f"members/{project}" + + +# --- the remedy, run as printed, from a real mount --------------------------- + +def test_a_hand_mount_is_taken_out_and_added_back_as_printed(family, holder): + """ROW (a) FOR REAL, and the gitlink's remedy run as printed. `git rm` + takes the checkout and its section out; `family.py add`, run as printed + with the org filled in, then refuses `member-git-dir-cached` for the + object store `git rm` leaves, as the remedy says it will name; with that + gone, the same `add` mounts and pins the member with its row, and the + holder passes in every mode. + + `git rm --cached`, which the issue proposed, is NOT the remedy: it + leaves the checkout at the mount path, and `add` refuses that as + `member-path-taken` before it gets as far as the store.""" + git("-c", "protocol.file.allow=always", "submodule", "add", "-q", "--", + str(family["remotes"] / f"{SPARE}.git"), f"members/{SPARE}", + cwd=holder) + commit(holder, "Mount IRQS by hand") + result = validate(holder) + assert result.returncode == 1, result.stderr + result.stdout + assert codes(result) == ["member-gitlink-unnamed", + "gitmodules-path-unnamed"] + rm, add = commands(findings(result)[0]) + assert commands(findings(result)[1]) == [] + git(*shlex.split(rm)[1:], cwd=holder) + assert not (holder / "members" / SPARE).exists() + assert f"members/{SPARE}" not in (holder / ".gitmodules").read_text( + encoding="utf-8") + argv = [arg.replace("", ORG) for arg in shlex.split(add)] + assert argv[:3] == [PYTHON, "scripts/family.py", "add"], add + + def add_as_printed(): + return run_script(FAMILY, *argv[2:], "--local-remote-dir", + str(family["remotes"])) + + refused = add_as_printed() + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith("REFUSED member-git-dir-cached: ") + rmtree(holder / ".git" / "modules" / "members" / SPARE) + added = add_as_printed() + assert added.returncode == 0, added.stdout + added.stderr + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert_passes_in_every_mode(holder, 3) + + +# --- a holder whose rows name everything ------------------------------------- + +@MODES +def test_a_holder_whose_rows_name_every_gitlink_and_section_passes(holder, + extra): + """THE CONTROL: the family `init` and `add` made, two members, every + gitlink and every section named by a row.""" + result = validate(holder, *extra) + assert result.returncode == 0, result.stderr + result.stdout + assert findings(result) == [] + assert "family ok: InkRouter (inkrouter), 2 member(s)" in result.stdout + + +def test_a_member_added_by_family_py_is_named(family, holder): + """And one more member, added the way a member is added: its gitlink and + its section are its row's, so nothing is unnamed.""" + added = run_script(FAMILY, "add", "--family-root", str(holder), + "--member", f"{ORG}/{SPARE}", "--local-remote-dir", + str(family["remotes"])) + assert added.returncode == 0, added.stdout + added.stderr + assert_passes_in_every_mode(holder, 3) diff --git a/tests/test_validate_family_without_git.py b/tests/test_validate_family_without_git.py new file mode 100644 index 00000000..1ca0e9ca --- /dev/null +++ b/tests/test_validate_family_without_git.py @@ -0,0 +1,183 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-family.py` with no git to run refuses `git-not-runnable`, exit +2, in every mode, naming the check that could not run it (#368). + +`git_out` raises `OSError` (`FileNotFoundError`) when there is no git to +start, and nothing in the validator caught it outside #317's one `.gitmodules` +reader: the default and `--pins` modes ended in a traceback, which exits 1, +a FINDING's code, though nothing was found. The docstring's own exit codes +make 1 "drift a reviewer can act on" and 2 "the question could not be +asked ... never an implicit pass". + +Caught IN THE VALIDATOR, around every check that runs git (#317's +`.gitmodules` reader, #361's gitlink listing, and each member's gitlink and +tree), never in `git_out`, and never around a whole check: a +`project.yaml` nobody may read is an `OSError` too. ONE code for the +validator, whichever check met the missing git first; `gitmodules-unreadable` +keeps meaning a file git ran and could not read. And `recorded_gitlink`'s +`except Refusal` does not turn the missing git into "no gitlink": a +`FileNotFoundError` is no refusal and passes straight through it. + +THE HOLDER'S OWN VALIDATOR, RUN IN THIS PROCESS, with git failing as a +missing program does, as `tests/test_family_add_refusals.py`'s no-git test +runs it: no PATH is changed for anything else, so each test means the same +on every OS. Two names reach git: the validator's own `git_out`, and the +one `recorded_gitlink` and `tree_digest` call inside `repo_shape`, so each +test says which of them fails, and where. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +from test_family import family, holder # noqa: F401 + +#: The remediation every `git-not-runnable` refusal ends with. +REMEDIATION = ( + "Remediation: put git on PATH and run this again. This check reads the " + "holder the way git does, and nothing could clone it or fetch its " + "members without git either.") + +MODES = pytest.mark.parametrize( + "extra", [(), ("--pins",), ("--no-members",)], + ids=["members-checked", "pins", "no-members"]) +MEMBER_MODES = pytest.mark.parametrize( + "extra", [(), ("--pins",)], ids=["members-checked", "pins"]) + + +def holder_validator(holder, monkeypatch): + """The holder's `scripts/validate-family.py`, loaded as a module. It + puts its own `scripts/` on `sys.path`, which is put back afterwards.""" + monkeypatch.setattr(sys, "path", list(sys.path)) + spec = importlib.util.spec_from_file_location( + "holder_validate_family_without_git", + holder / "scripts" / "validate-family.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def failing(error: OSError, real=None, *, outside: Path | None = None): + """A `git_out` that raises `error` as a program that will not start + does. With `outside`, only where git runs anywhere but there (in a + member, for instance), and `real` everywhere else.""" + def git_out(args, cwd, binary=False, env=None): + if outside is not None and Path(cwd).resolve() == outside.resolve(): + return real(args, cwd, binary=binary, env=env) + raise error + return git_out + + +def without_git(module, monkeypatch, *, validator: bool, helpers: bool, + error: OSError | None = None, + outside: Path | None = None) -> None: + """Make git fail for the validator's own calls (`validator`), for the + ones `recorded_gitlink` and `tree_digest` make in `repo_shape` + (`helpers`), or both.""" + error = error or FileNotFoundError(2, "No such file or directory", "git") + real = module.git_out + fake = failing(error, real, outside=outside) + if validator: + monkeypatch.setattr(module, "git_out", fake) + if helpers: + monkeypatch.setattr(sys.modules[module.recorded_gitlink.__module__], + "git_out", fake) + + +def run(module, holder, capsys, *extra) -> tuple[int, list[str]]: + code = module.main(["--root", str(holder), *extra]) + out, err = capsys.readouterr() + assert "Traceback" not in out + err + return code, err.splitlines() + + +@MODES +@pytest.mark.parametrize("error", [ + FileNotFoundError(2, "No such file or directory", "git"), + PermissionError(13, "Permission denied", "git")], + ids=["no-git", "git-not-executable"]) +def test_no_git_at_all_is_one_refusal_in_every_mode(holder, monkeypatch, + capsys, extra, error): + """THE HOLDER AS `add` LEFT IT: `.gitmodules` is read first, so that is + the check named. Exit 2 and the one code in every mode, where #317 said + `gitmodules-unreadable`, a code that now means a file git ran and could + not read.""" + module = holder_validator(holder, monkeypatch) + without_git(module, monkeypatch, validator=True, helpers=True, + error=error) + code, err = run(module, holder, capsys, *extra) + assert code == 2, err + assert err == [ + "REFUSED git-not-runnable: .gitmodules could not be read as git " + f"reads it: git could not be run ({error.strerror})", REMEDIATION] + + +@MODES +def test_with_no_gitmodules_the_gitlink_listing_is_named(holder, monkeypatch, + capsys, extra): + """THE ISSUE'S RESIDUE: a holder with member rows and its `.gitmodules` + gone from the working tree, where main still ended in the traceback in + the default and `--pins` modes. The first check to run git is now #361's + listing of `members/`, which every mode runs, `--no-members` included: + where it passed before, having run no git at all, it now refuses.""" + (holder / ".gitmodules").unlink() + module = holder_validator(holder, monkeypatch) + without_git(module, monkeypatch, validator=True, helpers=True) + code, err = run(module, holder, capsys, *extra) + assert code == 2, err + assert err == [ + "REFUSED git-not-runnable: the gitlinks this repository records in " + "members/ could not be listed: git could not be run (No such file " + "or directory)", REMEDIATION] + + +@MEMBER_MODES +def test_a_members_gitlink_that_git_cannot_read_is_named_not_absent( + holder, monkeypatch, capsys, extra): + """GIT FAILS ONLY IN `recorded_gitlink`, so every check before the + members' runs, and the first member's gitlink is the check named. Were + the missing git read as "no gitlink here", this would say + `member-gitlink-absent` and tell the operator to `add` a member that is + mounted.""" + module = holder_validator(holder, monkeypatch) + without_git(module, monkeypatch, validator=False, helpers=True) + code, err = run(module, holder, capsys, *extra) + assert code == 2, err + assert err == [ + "REFUSED git-not-runnable: the gitlink recorded at members/IRRS " + "could not be read: git could not be run (No such file or " + "directory)", REMEDIATION] + + +def test_no_members_runs_no_member_check_that_needs_git(holder, monkeypatch, + capsys): + """And `--no-members`, which reads no member, passes with the same git: + the refusal is the check's, not the mode's.""" + module = holder_validator(holder, monkeypatch) + without_git(module, monkeypatch, validator=False, helpers=True) + code = module.main(["--root", str(holder), "--no-members"]) + out, err = capsys.readouterr() + assert code == 0, out + err + assert "family ok: InkRouter (inkrouter), 2 member(s)" in out + + +@MEMBER_MODES +def test_a_members_tree_that_git_cannot_read_is_named(holder, monkeypatch, + capsys, extra): + """GIT RUNS IN THE HOLDER AND NOT IN A MEMBER, so the gitlink is read + and the pinned commit and its digest, read inside the member, are the + check named.""" + module = holder_validator(holder, monkeypatch) + without_git(module, monkeypatch, validator=True, helpers=True, + outside=holder) + code, err = run(module, holder, capsys, *extra) + assert code == 2, err + assert err == [ + "REFUSED git-not-runnable: IRRS's pinned commit and its tree digest " + "could not be read in members/IRRS: git could not be run (No such " + "file or directory)", REMEDIATION] +