diff --git a/shape-doctor.py b/shape-doctor.py index d8fba751..cddcb398 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -47,7 +47,11 @@ it, and a `contracts/*.yaml` whose `kind` has no entry is a FINDING that says so by name. A manifest nobody validates is the failure this whole standard is against, and "we did not notice it was there" is how it happens; a future -inventory plugs in by adding one row to that table. +inventory plugs in by adding one row to that table. A kind whose check never +runs on THIS kind of root is that failure too, and the same FINDING (#327): +`validate-family.py` is registered for `family-manifest` and runs in a family +holder only, so a `contracts/family.yaml` in an assembly root -- or a +`contracts/project.yaml` in a holder -- is a manifest nothing here reads. THE ROWS, for an assembly root, in this order: @@ -65,7 +69,7 @@ pins the project's own `scripts/validate-pins.py` likewise: the lockstep invariant, and the shape-copy digests manifest kinds every `kind:`-bearing manifest in the root, against - MANIFEST_VALIDATORS + MANIFEST_VALIDATORS and the kind of root it is in shape currency `update-shape.py check`'s per-file verdicts, summarised legs each leg mounted, and its checked-out commit against the pin (no fetch) @@ -701,34 +705,73 @@ def __init__(self, check_id: str, label: str, applies_to: tuple, self.fix = fix +#: Each kind of root that carries manifests, as a sentence names it: the +#: `manifest kinds` row says where a validator runs in these words (#327). +ROOT_IN_WORDS = {PROJECT: "an assembly root", FAMILY: "a family holder"} + + class ManifestKind: """One `kind:` this standard knows a validator for. - `covered_by` is the id of the CHECK that validates it, so the row about - manifest kinds can say where a kind was answered rather than merely that - it was recognised. A kind with no entry here is a FINDING by name: a - manifest nobody validates is the gap this table exists to close. + `covered_by` is the ids of the CHECKS that validate it, one for each kind + of root it is validated in, so the row about manifest kinds can say where + a kind was answered rather than merely that it was recognised. A kind + with no entry here is a FINDING by name: a manifest nobody validates is + the gap this table exists to close. + + AND A KIND IS ANSWERED ONLY WHERE ONE OF THOSE CHECKS RUNS (#327). + `runs_in` reads each check's `applies_to` out of `CHECKS`, so this table + names checks and the registry alone says which root runs each of them -- + there is no second list of root kinds here to drift from it. The `family` + check applies to a holder and to nothing else, so `family-manifest` is + answered in a holder only, and a `contracts/family.yaml` in an assembly + root is a manifest no gate there reads; `manifest` likewise, so a + `contracts/project.yaml` in a holder is one too. The other kinds a holder + carries are answered there by `family`: its `validate-family.py` reads + `contracts/shape-pin.yaml` and loads the naming policy itself. + + `home` is the other exit for a manifest found in a root that never runs + its gate: a root of the kind that does, named by the commands that make + one. Only the two kinds a root of the other kind can hold need one. It + is one sentence on every platform, so it names `setup-project.py` beside + `setup.sh`, which is a bash script Windows cannot run -- the reason + `scaffold_command` spells that line per platform (Copilot, PR #102). """ - def __init__(self, kind: str, covered_by: str, how: str): + def __init__(self, kind: str, covered_by: tuple, how: str, + home: str | None = None): self.kind = kind self.covered_by = covered_by self.how = how + self.home = home + + def runs_in(self) -> tuple: + """The kinds of root a check that `covered_by` names runs on.""" + return tuple(root for root in ROOT_IN_WORDS + if any(check.id in self.covered_by + and root in check.applies_to + for check in CHECKS)) MANIFEST_VALIDATORS: dict[str, ManifestKind] = { "project-manifest": ManifestKind( - "project-manifest", "manifest", "scripts/validate-manifest.py"), + "project-manifest", ("manifest",), "scripts/validate-manifest.py", + "an assembly root of its own (`setup.sh`, or `setup-project.py` on " + "Windows, for a new one; `adopt-project.py plan` for a repository " + "that exists), which a holder pins with `family.py add`"), "family-manifest": ManifestKind( - "family-manifest", "family", "scripts/validate-family.py"), + "family-manifest", ("family",), "scripts/validate-family.py", + "a family holder of its own (`family.py init`, then `family.py add` " + "there)"), "pinned_contract_manifest": ManifestKind( - "pinned_contract_manifest", "pins", + "pinned_contract_manifest", ("pins", "family"), "scripts/validate-pins.py (or validate-family.py in a holder)"), "repository-naming-policy": ManifestKind( - "repository-naming-policy", "naming", - "scripts/validate-repository-naming.py, through NamingPolicy.load"), + "repository-naming-policy", ("naming", "family"), + "scripts/validate-repository-naming.py, through NamingPolicy.load " + "(validate-family.py, through the same, in a holder)"), "path-classification-policy": ManifestKind( - "path-classification-policy", "manifest-kinds", + "path-classification-policy", ("manifest-kinds",), "scripts/path_classify.py reads it; nothing validates it in a root"), } @@ -1465,6 +1508,169 @@ def family_beside_project(ctx: Context, data: dict) -> tuple[str, dict]: "add` here, which would mount the members inside a project") +def registered_elsewhere(ctx: Context, found: list) -> tuple[str, str, list]: + """Manifests whose kind is registered to a gate this root never runs: + the sentence, the next command, and the facts (#327). + + `found` is `(path, kind, entry)` for each, in the row's own order. The + sentence opens the way the unregistered kind's does -- "no validator + ... is registered for kind" -- because to this root it is the same + fact: the registry has a validator for the kind, and none that runs + here. Each file says which validator that is and where it does run. + + A FINDING, NOT A NOTE, for the reason an unregistered kind is one. A + `note` is a difference nothing asserts, and a file declaring `kind:` + asserts what it is; the registry exists so that no such file is + answered by nobody, and a kind the registry knows is no better answered + in a root whose gate never reads it. Nor could the same bytes be a + FINDING as `family.yaml` (#211) and a note as `contracts/family.yaml`. + + THE NEXT COMMAND IS ONE THAT EXISTS, FOR THE FILE AS GIT HAS IT. The + doctor reads the disk, so the file may be one nobody has committed yet, + and `git rm` takes out only a file whose every byte a commit holds: it + says "did not match any files" (exit 128) for one git does not track, + ignored or not, and refuses one with changes no commit holds. So + `git_states` asks first, and each file gets the exit that works: + committed `git rm`, quoted for the reader's shell like every path + this report prints, committed on a branch and landed as + a pull request, the way every change here lands; + untracked, deleting it, with nothing to commit; + ignored + changed committing or dropping those changes first, since `git + rm` will not discard them, and the next run names the + `git rm`. + Where git cannot say, the line is #211's, which holds in any state: + take it out of this root. Each file's state is `detail`'s `git`. + + The other exit is the kind's `home`, as the human decides. Renaming the + file to the root's own manifest name is ruled out in the line itself, + because that is the one move that looks like a fix: in a holder it + makes the root read as a project, and in an assembly root it is #211's + finding instead of this one. + """ + names = [rel for rel, _, _ in found] + one = len(names) == 1 + states = git_states(ctx.root, names) + clauses, detail = [], [] + for rel, kind, entry in found: + runs = entry.runs_in() + where = (f"{entry.how} runs only in " + + " or ".join(ROOT_IN_WORDS[root] for root in runs) + if runs else f"no check runs {entry.how}") + clauses.append(f"{rel} ({kind}: {where})") + detail.append({"manifest": rel, "kind": kind, + "validator": entry.how, "runs_in": list(runs), + "git": states.get(rel)}) + sentence = (f"no validator that runs in {ROOT_IN_WORDS[ctx.kind]} is " + "registered for kind " + ", ".join(clauses)) + homes = {rel: entry.home or "a root whose gate runs its validator" + for rel, _, entry in found} + if len(set(homes.values())) == 1: + other_exit = (f"move what {'it describes' if one else 'they describe'}" + f" to {homes[names[0]]}") + else: + other_exit = "move what each describes to " + "; ".join( + f"{rel}: {home}" for rel, home in homes.items()) + fix = (f"take {', '.join(names)} out of this root" + f"{removal_ways(ctx, names, states)}, or {other_exit}, as the " + f"human decides -- never rename {'it' if one else 'one'} to " + f"{PROJECT_MANIFEST} or {FAMILY_MANIFEST} in this root: a family " + "holder is not a project, and an assembly root is not a family " + "holder") + return sentence, fix, detail + + +def git_states(root: Path, names: list) -> dict: + """`{path: state}` for each of `names`, from ONE `git status`: + `committed`, `untracked`, `ignored` or `changed` -- or `{}` where git + cannot say (#327). + + A path `status` does not list is tracked, and HEAD, the index and the + work tree agree on it: `committed`, the one state `git rm` takes. `??` + alone is `untracked`, `!!` alone is `ignored` (`--ignored`, so a file + the root's excludes hide is still named), and anything else -- an edit, + a staged change, a file staged and never committed, a removal staged + while the file is still on disk (`D ` and `??` at once) -- is `changed`. + + READ-ONLY AND LITERAL. `--no-optional-locks`, so `status` refreshes the + index in memory and never writes it back: the doctor writes nothing into + the repository. `--literal-pathspecs`, so a name holding `[` or `*` is + that one file and not a pattern. `-z`, so a name comes back as itself, + and `--untracked-files=all`, so it is the file and not its directory. + + AND ONLY IN A REPOSITORY OF ITS OWN. `git -C` discovers: in a plain + directory inside another repository, `status` answers about THAT one, + with paths relative to its top, and a `git rm` there would change it. + `not_its_own_repository` declines to guess, as it does for a leg. + """ + if not_its_own_repository(root) is not None: + return {} + try: + raw = git_out(["--no-optional-locks", "--literal-pathspecs", "status", + "--porcelain=v1", "-z", "--untracked-files=all", + "--ignored", "--", *names], cwd=root, binary=True) + except (Refusal, OSError): + return {} + codes: dict[str, set] = {} + records = iter(raw.split(b"\0")) + for record in records: + if len(record) < 4: + continue + code = record[:2].decode("ascii", "replace") + codes.setdefault(record[3:].decode("utf-8", "surrogateescape"), + set()).add(code) + if code[0] in "RC": + next(records, None) # a rename's or copy's source path + states = {} + for name in names: + found = codes.get(name, set()) + states[name] = ("committed" if not found + else "untracked" if found == {"??"} + else "ignored" if found == {"!!"} + else "changed") + return states + + +def removal_ways(ctx: Context, names: list, states: dict) -> str: + """How each of `names` comes out of the root, as git has it: the + parenthesis the `manifest kinds` row's NEXT carries (#327), or nothing + where git could not say. A file is named again only where the files + do not all come out the same way.""" + root = quote_arg(ctx.root) + groups = {state: [name for name in names if states.get(name) in kinds] + for state, kinds in (("committed", ("committed",)), + ("loose", ("untracked", "ignored")), + ("changed", ("changed",)))} + groups = {state: group for state, group in groups.items() if group} + alone = len(groups) == 1 + + def subject(group: list, pronoun: str) -> str: + return pronoun if alone else ", ".join(group) + + ways = [] + if "committed" in groups: + ways.append(f"`git -C {root} rm -- " + + " ".join(quote_arg(n) for n in groups["committed"]) + + "`, committed on a branch and landed as a pull " + "request") + if "loose" in groups: + group = groups["loose"] + single = len(group) == 1 + ways.append(f"delete {subject(group, 'it' if single else 'them')}, " + "which git does not track, so there is nothing to " + "commit") + if "changed" in groups: + group = groups["changed"] + single = len(group) == 1 + ways.append( + f"{subject(group, 'it' if single else 'they')} " + f"{'has' if single else 'have'} changes no commit holds, which " + "`git rm` will not discard: commit or drop them (`git -C " + f"{root} status -- {' '.join(quote_arg(n) for n in group)}` " + "shows them), and the next run names the `git rm`") + return f" ({'; '.join(ways)})" if ways else "" + + def check_manifest_kinds(ctx: Context) -> Row: """Every `kind:`-bearing manifest in this root, against the registry. @@ -1475,10 +1681,14 @@ def check_manifest_kinds(ctx: Context) -> Row: a file that DOES declare a kind that nothing here validates, which is a manifest travelling in a shape root with no gate behind it. - A REGISTERED KIND WHOSE GATE THIS ROOT NEVER RUNS IS THAT CASE TOO - (#211): a `family.yaml` declaring `family-manifest` in an assembly root. - It is not counted `validated`; `family_beside_project` says why, and - checks the members it pins against the gitlinks. + A REGISTERED KIND WHOSE GATE THIS ROOT NEVER RUNS IS THAT CASE TOO, + wherever the file is (#211, #327). A kind counts `validated` only where a + check it is registered to RUNS -- `ManifestKind.runs_in` -- so a + `contracts/family.yaml` in an assembly root, or a `contracts/project.yaml` + in a holder, is listed under `registered_elsewhere` and is a FINDING + `registered_elsewhere()` explains. A `family.yaml` at the top of an + assembly root is the same fact with more to say: `family_beside_project` + also checks the members it pins against the gitlinks. THE ROOT'S OWN MANIFEST, WHEN IT DOES NOT LOAD, IS NOT THIS ROW'S (#205). The `manifest` (or `family`) row quotes its refusal and names its gate, @@ -1491,6 +1701,9 @@ def check_manifest_kinds(ctx: Context) -> Row: known: list[str] = [] unknown: list[str] = [] unreadable: list[str] = [] + #: `(path, kind, entry)` for each manifest whose registered gate never + #: runs on this kind of root (#327). + ungated: list[tuple] = [] elsewhere = (ctx.manifest_path if ctx.manifest_refusal is not None else None) beside: tuple[str, dict] | None = None @@ -1510,10 +1723,20 @@ def check_manifest_kinds(ctx: Context) -> Row: beside = family_beside_project(ctx, data) continue entry = MANIFEST_VALIDATORS.get(kind) - (known if entry else unknown).append(f"{rel} ({kind})") + if entry is None: + unknown.append(f"{rel} ({kind})") + elif ctx.kind not in entry.runs_in(): + ungated.append((rel, kind, entry)) + else: + known.append(f"{rel} ({kind})") detail = {"validated": known, "unregistered": unknown, "unreadable": unreadable, "registered_kinds": sorted(MANIFEST_VALIDATORS)} + ungated_said = registered_elsewhere(ctx, ungated) if ungated else None + if ungated_said is not None: + # ONLY WHEN THERE IS ONE, like `family_beside` below, so every root + # without such a file reports the row it reported before #327. + detail["registered_elsewhere"] = ungated_said[2] # Only when there is one, so every other root's row is the row it was. aside = "" if elsewhere is not None: @@ -1525,7 +1748,8 @@ def check_manifest_kinds(ctx: Context) -> Row: # ONLY WHEN THERE IS ONE, so every root without it reports the same # row, byte for byte, as it did before #211. detail["family_beside"] = beside[1] - if unknown or unreadable or beside is not None: + if (unknown or unreadable or ungated_said is not None + or beside is not None): parts = [] fixes = [] if unknown: @@ -1537,6 +1761,9 @@ def check_manifest_kinds(ctx: Context) -> Row: fixes.append("register the kind in shape-doctor.py's " "MANIFEST_VALIDATORS, beside the check that " "validates it, or remove the file from the root") + if ungated_said is not None: + parts.append(ungated_said[0]) + fixes.append(ungated_said[1]) if beside is not None: parts.append(beside[0]) fixes.append(FAMILY_BESIDE_FIX) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 86fb506b..77e6f34b 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -37,6 +37,7 @@ import sys from pathlib import Path, PurePosixPath, PureWindowsPath +from types import SimpleNamespace import pytest @@ -780,6 +781,408 @@ def test_a_root_with_no_family_yaml_beside_gets_the_row_it_always_got( assert "family.yaml (family-manifest)" in row["detail"]["validated"], row +# --- a registered kind in a root that never runs its gate (#327) ------------ +# +# #211 acted on ONE path: `family.yaml` at the top of an assembly root. Any +# other file declaring a registered kind was still counted `validated` +# wherever it sat, so a `contracts/family.yaml` in an assembly root and a +# `contracts/project.yaml` in a family holder were `ok` and COMPLIANT, exit +# 0, though the gate registered for each kind never runs on that kind of root. + +#: The words the row's next command closes with, spelt out here for the +#: reason `FAMILY_BESIDE_NEXT` is: a test that read them back from the doctor +#: would pass whatever it said. +NEVER_RENAME = ("never rename it to project.yaml or family.yaml in this " + "root: a family holder is not a project, and an assembly " + "root is not a family holder") + +#: Where each kind's manifest goes instead, as the next command offers it. +FAMILY_HOME = ("a family holder of its own (`family.py init`, then " + "`family.py add` there)") +#: `setup-project.py` beside `setup.sh`, on every OS: `setup.sh` is a bash +#: script, and a Windows reader has no bash to run it in. +PROJECT_HOME = ("an assembly root of its own (`setup.sh`, or " + "`setup-project.py` on Windows, for a new one; " + "`adopt-project.py plan` for a repository that exists), " + "which a holder pins with `family.py add`") + + +def removal_named(standard: Path, row: dict, payload: dict, + names: list) -> list: + """The `git rm` a registered-elsewhere row's NEXT names, as the argv it + is, after asserting the NEXT spells exactly that command. + + The spelling is held to the doctor's own `quote_arg` -- the quoter whose + two dialects the tests further down pin -- so the line is the one the + reader's shell hands back whole, on Windows as on POSIX. Where the + reader's shell IS POSIX `sh`, `shlex.split` reads the line back too. + """ + module = doctor_module(standard) + root = Path(payload["root"]) + spelled = (f"git -C {module.quote_arg(root)} rm -- " + + " ".join(module.quote_arg(name) for name in names)) + assert f"(`{spelled}`, committed on a branch and landed as a pull " \ + "request)" in row["next"], row["next"] + argv = ["git", "-C", str(root), "rm", "--", *names] + if os.name != "nt": + assert shlex.split(spelled) == argv, spelled + return argv + + +def run_the_removal(argv: list, root: Path) -> None: + """Run the NEXT's own `git rm`, and commit it as the line says.""" + proc = subprocess.run(argv, capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stdout + proc.stderr + commit_all(root, "Take the manifest no gate here reads out of the root") + + +def only_finding_is_manifest_kinds(result) -> dict: + rows = rows_of(result) + others = {name: row["status"] for name, row in rows.items() + if row["status"] == "FINDING" and name != "manifest-kinds"} + assert others == {}, others + return rows["manifest-kinds"] + + +def test_a_contracts_family_yaml_in_an_assembly_root_is_a_finding( + standard, project, holder): + """THE DEFECT #327 WAS FILED FOR, in an assembly root. + + The holder's own `family.yaml`, committed under the root's `contracts/`: + it declares `family-manifest`, whose registered gate `validate-family.py` + runs in a family holder and nowhere else. The doctor said `ok`, "every + declared kind has a registered validator", COMPLIANT, exit 0. It is the + FINDING the same bytes are at the top of the root (#211) now: not + `validated`, named with the validator and where it runs, and a next + command that exists -- run here, it puts the root back to COMPLIANT. + """ + shutil.copy2(holder["root"] / "family.yaml", + project / "contracts" / "family.yaml") + commit_all(project, "A family manifest under contracts/") + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert (payload["kind"], payload["verdict"]) == ( + "project", "INVALID (manifest-kinds)"), payload + row = only_finding_is_manifest_kinds(result) + assert row["status"] == "FINDING", row + assert row["reason"] == ( + "no validator that runs in an assembly root is registered for kind " + "contracts/family.yaml (family-manifest: scripts/validate-family.py " + "runs only in a family holder)"), row["reason"] + assert set(row["detail"]) == MANIFEST_KINDS_DETAIL_KEYS | { + "registered_elsewhere"}, row["detail"] + assert not any("contracts/family.yaml" in entry + for entry in row["detail"]["validated"]), row["detail"] + assert row["detail"]["registered_elsewhere"] == [ + {"manifest": "contracts/family.yaml", "kind": "family-manifest", + "validator": "scripts/validate-family.py", "runs_in": ["family"], + "git": "committed"}] + argv = removal_named(standard, row, payload, ["contracts/family.yaml"]) + assert row["next"].startswith( + "take contracts/family.yaml out of this root (`git -C "), row["next"] + assert row["next"].endswith( + f", or move what it describes to {FAMILY_HOME}, as the human " + f"decides -- {NEVER_RENAME}"), row["next"] + + text = doctor(standard, project) + assert text.returncode == 1, text.stdout + text.stderr + lines = text.stdout.splitlines() + [at] = [index for index, line in enumerate(lines) + if "manifest kinds" in line] + assert lines[at].split()[:3] == ["FINDING", "manifest", "kinds"], \ + lines[at] + assert lines[at].endswith(" " + row["reason"]), lines[at] + assert lines[at + 1].endswith(" " + row["next"]), lines[at + 1] + assert verdict_line(text) == "INVALID (manifest-kinds) (exit 1)", \ + text.stdout + + run_the_removal(argv, project) + after = doctor(standard, project, "--json") + assert after.returncode == 0, after.stdout + after.stderr + assert json.loads(after.stdout)["verdict"] == "COMPLIANT" + + +def test_a_contracts_project_yaml_in_a_family_holder_is_a_finding( + standard, scaffolded_here, holder, tmp_path): + """THE MIRROR CASE, in a family holder, which #220's review read from the + code and #327 ran: an assembly root's `project.yaml` committed under the + holder's `contracts/`. Its gate, `validate-manifest.py`, runs in an + assembly root and nowhere else. The holder's own two contracts are still + answered -- by `validate-family.py`, which reads the shape pin and the + naming policy itself -- so they stay `validated`, and only this file is + named.""" + root = tmp_path / FAMILY_NAME + shutil.copytree(holder["root"], root, symlinks=True) + shutil.copy2(scaffolded_here["clone"] / "project.yaml", + root / "contracts" / "project.yaml") + commit_all(root, "A project manifest under contracts/") + result = doctor(standard, root, "--json") + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert (payload["kind"], payload["verdict"]) == ( + "family", "INVALID (manifest-kinds)"), payload + row = only_finding_is_manifest_kinds(result) + assert row["status"] == "FINDING", row + assert row["reason"] == ( + "no validator that runs in a family holder is registered for kind " + "contracts/project.yaml (project-manifest: " + "scripts/validate-manifest.py runs only in an assembly root)"), \ + row["reason"] + assert row["detail"]["registered_elsewhere"] == [ + {"manifest": "contracts/project.yaml", "kind": "project-manifest", + "validator": "scripts/validate-manifest.py", "runs_in": ["project"], + "git": "committed"}] + assert sorted(row["detail"]["validated"]) == [ + "contracts/repository-naming.yaml (repository-naming-policy)", + "contracts/shape-pin.yaml (pinned_contract_manifest)", + "family.yaml (family-manifest)"], row["detail"] + argv = removal_named(standard, row, payload, ["contracts/project.yaml"]) + assert row["next"].endswith( + f", or move what it describes to {PROJECT_HOME}, as the human " + f"decides -- {NEVER_RENAME}"), row["next"] + + run_the_removal(argv, root) + after = doctor(standard, root, "--json") + assert after.returncode == 0, after.stdout + after.stderr + assert json.loads(after.stdout)["verdict"] == "COMPLIANT" + + +def test_a_contracts_family_yaml_and_one_beside_project_yaml_say_both( + standard, project, holder): + """Two family manifests in one assembly root, one under `contracts/` and + one at the top: two reasons in one row, each with its own exit, the + general one first and #211's -- which also checks the members -- after + it. Neither is counted `validated`.""" + shutil.copy2(holder["root"] / "family.yaml", + project / "contracts" / "family.yaml") + family_beside(project, holder) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + assert json.loads(result.stdout)["verdict"] == "INVALID (manifest-kinds)" + row = only_finding_is_manifest_kinds(result) + assert row["reason"] == ( + "no validator that runs in an assembly root is registered for kind " + "contracts/family.yaml (family-manifest: scripts/validate-family.py " + f"runs only in a family holder); {FAMILY_BESIDE_OPENING}: " + f"members/{FAMILY_MEMBER} has no gitlink (mode 160000)"), \ + row["reason"] + assert row["next"].endswith(f"{NEVER_RENAME}; {FAMILY_BESIDE_NEXT}"), \ + row["next"] + assert set(row["detail"]) == MANIFEST_KINDS_DETAIL_KEYS | { + "registered_elsewhere", "family_beside"}, row["detail"] + assert not any("family" in entry for entry in row["detail"]["validated"]) + + +# --- the NEXT, for the file as git has it (#327, review of PR #375) --------- +# +# The doctor reads the disk, and `git rm` takes out only a file a commit +# holds whole. The NEXT named it for every file, and it said "did not match +# any files" (exit 128) for one git does not track, ignored or not, and +# refused one with changes no commit holds. + +def plant_family_yaml(project: Path, holder: dict, + name: str = "family.yaml") -> Path: + """The holder's `family.yaml`, copied under the root's `contracts/` as + `name`, and nothing committed.""" + planted = project / "contracts" / name + shutil.copy2(holder["root"] / "family.yaml", planted) + return planted + + +def next_for_one(ways: str) -> str: + """The NEXT for one `contracts/family.yaml`, `ways` its parenthesis.""" + return (f"take contracts/family.yaml out of this root ({ways}), or move " + f"what it describes to {FAMILY_HOME}, as the human decides -- " + f"{NEVER_RENAME}") + + +def git_rm_refuses(project: Path, planted: Path) -> None: + """The `git rm` the NEXT used to name, run as it named it: it fails, and + the file is still there.""" + proc = subprocess.run( + ["git", "-C", str(project), "rm", "--", + planted.relative_to(project).as_posix()], + capture_output=True, text=True, check=False) + assert proc.returncode != 0, proc.stdout + proc.stderr + assert planted.is_file() + + +@pytest.mark.parametrize("state", ["untracked", "ignored"]) +def test_a_contracts_family_yaml_git_does_not_track_is_deleted( + standard, project, holder, state): + """A `contracts/family.yaml` nobody has committed -- copied in and not + added, or hidden by the root's own excludes -- is the same FINDING, and + its NEXT is to delete it, with nothing to commit. It names no `git rm`, + which fails on it; deleting it is COMPLIANT again.""" + planted = plant_family_yaml(project, holder) + if state == "ignored": + exclude = project / git("rev-parse", "--git-path", "info/exclude", + cwd=project).stdout.strip() + exclude.parent.mkdir(parents=True, exist_ok=True) + with open(exclude, "ab") as handle: + handle.write(b"\ncontracts/family.yaml\n") + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + assert json.loads(result.stdout)["verdict"] == "INVALID (manifest-kinds)" + row = only_finding_is_manifest_kinds(result) + assert row["next"] == next_for_one( + "delete it, which git does not track, so there is nothing to " + "commit"), row["next"] + assert row["detail"]["registered_elsewhere"] == [ + {"manifest": "contracts/family.yaml", "kind": "family-manifest", + "validator": "scripts/validate-family.py", "runs_in": ["family"], + "git": state}] + git_rm_refuses(project, planted) + + planted.unlink() + after = doctor(standard, project, "--json") + assert after.returncode == 0, after.stdout + after.stderr + assert json.loads(after.stdout)["verdict"] == "COMPLIANT" + + +@pytest.mark.parametrize("state", ["edited", "staged"]) +def test_a_contracts_family_yaml_with_changes_no_commit_holds( + standard, project, holder, state): + """Committed and then edited, or staged and never committed: `git rm` + refuses both, because it will not discard changes no commit holds. The + NEXT says to commit or drop them, names the `git status` that shows + them, and leaves the `git rm` to the next run -- which, once they are + committed, names it, and running that is COMPLIANT again.""" + planted = plant_family_yaml(project, holder) + if state == "edited": + commit_all(project, "A family manifest under contracts/") + with open(planted, "ab") as handle: + handle.write(b"# an edit no commit holds\n") + else: + git("add", "--", "contracts/family.yaml", cwd=project) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert payload["verdict"] == "INVALID (manifest-kinds)", payload + row = only_finding_is_manifest_kinds(result) + root = doctor_module(standard).quote_arg(Path(payload["root"])) + assert row["next"] == next_for_one( + "it has changes no commit holds, which `git rm` will not discard: " + f"commit or drop them (`git -C {root} status -- " + "contracts/family.yaml` shows them), and the next run names the " + "`git rm`"), row["next"] + assert [entry["git"] for entry in + row["detail"]["registered_elsewhere"]] == ["changed"], row + git_rm_refuses(project, planted) + + commit_all(project, "Commit what the row said to commit first") + again = doctor(standard, project, "--json") + row = only_finding_is_manifest_kinds(again) + argv = removal_named(standard, row, json.loads(again.stdout), + ["contracts/family.yaml"]) + run_the_removal(argv, project) + after = doctor(standard, project, "--json") + assert after.returncode == 0, after.stdout + after.stderr + assert json.loads(after.stdout)["verdict"] == "COMPLIANT" + + +def test_manifests_that_come_out_differently_are_each_named( + standard, project, holder): + """Three family manifests under `contracts/` -- one committed, one git + does not track, one edited since its commit -- get one NEXT with a way + for each, each naming its own file, in the row's order. + + AND THE DOCTOR ASKED GIT WITHOUT WRITING THE INDEX. The committed one is + rewritten with its own bytes and a newer mtime, so its index entry is + stat-dirty: a `git status` refreshes that entry and, without + `--no-optional-locks`, writes `.git/index` back. It must stay byte for + byte as it was, because the doctor writes nothing into the repository. + """ + committed = plant_family_yaml(project, holder) + edited = plant_family_yaml(project, holder, "x-family.yaml") + commit_all(project, "Two family manifests under contracts/") + with open(edited, "ab") as handle: + handle.write(b"# an edit no commit holds\n") + plant_family_yaml(project, holder, "old-family.yaml") + committed.write_bytes(committed.read_bytes()) + stat = committed.stat() + os.utime(committed, (stat.st_atime + 100, stat.st_mtime + 100)) + index = project / git("rev-parse", "--git-path", "index", + cwd=project).stdout.strip() + before = index.read_bytes() + + result = doctor(standard, project, "--json") + assert index.read_bytes() == before, "the doctor wrote the index" + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert payload["verdict"] == "INVALID (manifest-kinds)", payload + row = only_finding_is_manifest_kinds(result) + root = doctor_module(standard).quote_arg(Path(payload["root"])) + assert row["next"] == ( + "take contracts/family.yaml, contracts/old-family.yaml, " + "contracts/x-family.yaml out of this root " + f"(`git -C {root} rm -- contracts/family.yaml`, committed on a " + "branch and landed as a pull request; delete " + "contracts/old-family.yaml, which git does not track, so there is " + "nothing to commit; contracts/x-family.yaml has changes no commit " + "holds, which `git rm` will not discard: commit or drop them " + f"(`git -C {root} status -- contracts/x-family.yaml` shows them), " + "and the next run names the `git rm`), or move what they describe " + f"to {FAMILY_HOME}, as the human decides -- never rename one to " + "project.yaml or family.yaml in this root: a family holder is not a " + "project, and an assembly root is not a family holder"), row["next"] + assert [(entry["manifest"], entry["git"]) for entry in + row["detail"]["registered_elsewhere"]] == [ + ("contracts/family.yaml", "committed"), + ("contracts/old-family.yaml", "untracked"), + ("contracts/x-family.yaml", "changed")], row["detail"] + + +def test_git_states_answers_only_in_a_repository_of_its_own(tmp_path): + """`git -C` discovers. In a plain directory inside another repository, + `status` answers about THAT repository, with paths relative to its top, + so `contracts/family.yaml` was never listed, read as `committed`, and + the NEXT named a `git rm` aimed at the enclosing repository. Now git is + not asked there, and the NEXT carries no way at all: #211's "take it out + of this root", which holds in any state. Its own repository is asked.""" + module = doctor_module(REPO) + outer = tmp_path / "outer" + outer.mkdir() + git("init", "-q", "-b", "main", ".", cwd=outer) + root = outer / PROJECT + (root / "contracts").mkdir(parents=True) + (root / "contracts" / "family.yaml").write_bytes( + b"kind: family-manifest\n") + names = ["contracts/family.yaml"] + assert module.git_states(root, names) == {} + assert module.removal_ways(SimpleNamespace(root=root), names, {}) == "" + + git("init", "-q", "-b", "main", ".", cwd=root) + assert module.git_states(root, names) == { + "contracts/family.yaml": "untracked"} + + +def test_a_registered_kind_is_answered_only_where_its_check_runs(): + """THE RULE, read straight off the registry (#327). + + A kind is answered on a kind of root only where a check it is + `covered_by` APPLIES -- `CHECKS` says which, and nothing else does. So + the two root manifests are answered each in its own kind of root and + never in the other, and every kind a holder ships is answered there by + `family`, which is what keeps a fresh holder COMPLIANT. Every check a + kind names exists, or the kind would be answered nowhere at all. + """ + module = doctor_module(REPO) + runs = {kind: entry.runs_in() + for kind, entry in module.MANIFEST_VALIDATORS.items()} + assert runs == { + "project-manifest": ("project",), + "family-manifest": ("family",), + "pinned_contract_manifest": ("project", "family"), + "repository-naming-policy": ("project", "family"), + "path-classification-policy": ("project", "family"), + }, runs + checks = {check.id for check in module.CHECKS} + for kind, entry in module.MANIFEST_VALIDATORS.items(): + assert set(entry.covered_by) <= checks, (kind, entry.covered_by) + + def test_gitlinks_under_lists_one_directory_and_nothing_above_it(project): """`gitlinks_under` is asked for the members directory a hand-edited manifest names, so an empty one, an absolute one and one that climbs