From 93b68f626ec80bd55663565709d78e1817621257 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:19:57 +0000 Subject: [PATCH 1/3] validate-pins.py and validate-family.py: the count ends at THE LOCKSTEP RULE only where a leg or a member is out of step, a lone pin digest or workflow reference names the `bump-leg.py` line that cures it, and a holder remedy naming `family.py add` or `remove` makes the `git switch -c` branch first, so a lone `shape-copy-drift` is not told to move a gitlink and no remedy costs a refusal first (#539) Main at bfed143e89f97d3480cf5373051cb1ace82db312. The three lines of #539, from the v1.1.0 triage of the v1.0.0 known limits on #459 and #460, in one change to the two pinned validators, so every root and holder re-pins once. - validate-pins.py, the count. The report ended ` finding(s).` and THE LOCKSTEP RULE whatever was found, so a lone `shape-copy-drift` (an edited copy, put right by `update-shape.py` or a revert) was followed by a rule about moving a gitlink, a pin and the workflows together. The rule now closes the count only where `pin-gitlink-mismatch`, `pin-digest-mismatch` or `pin-workflow-ref-mismatch` was found (`LOCKSTEP_CODES`). - validate-pins.py, the two findings that named no command. A lone `pin-digest-mismatch` now ends on one `bump-leg.py --to ` line, the commit the gitlink and the pin already name, which recomputes the digest from the leg's own objects. A lone `pin-workflow-ref-mismatch` ends on two, as #478's gitlink does: `--to` the gitlink's commit puts the reference back, `--to` the reference's own moves the gitlink and the pin to it, and which stays is the person's call. Named only where a line can commit, by #478's rule: the root has this gate of its own and every finding is the leg's own. Beside a finding no bump clears they say to clear it first, in #478's held-back words; beside the leg's own `pin-gitlink-mismatch` whose two lines are named they say those clear this too; with another leg out of step they end at the rule as before, since no line commits and any first step named would contradict the other leg's. Each is run as printed in the new tests and the doctor then says COMPLIANT. The `pins` row is unchanged: it quotes a finding's first line, and its NEXT, the gate, now names the line. `test_every_other_pins_row_is_the_gates_own` held the old output in place; it now holds that the gate names a line for these two and for no other finding there. #478's own text is byte-identical (`_gitlink_repairs` and `_gitlink_repairs_held_back` compared against bfed143). - validate-family.py, the branch step. Since #487 a real `add`, `bump` or `remove` refuses the holder's tracking branch. `member-duplicate-id`'s `remove`, a hand mount's `member-gitlink-unnamed` `add` and `member-gitlink-absent`'s `add` now name `git -C switch -c family/-` first, the branch `family.py`'s `_suggested_branch` gives (a test holds the two to one rule) and the holder's docs show, POSIX-quoted as every other line this gate prints. `member-gitlink-absent`'s `add` names the holder by its path, where `--family-root .` named the checkout of openRepoShape the line runs from. The hand-mount remedy is run as printed from the tracking branch, and so is the `remove`. - validate-family.py, the count, by the same rule: THE LOCKSTEP RULE closes it only where `member-gitlink-mismatch` or `member-digest-mismatch` was found. Not on #539's lines, but under its title, and in the same file, so holders do not re-pin a second time for it. Both files are pinned copies: `update-shape.py check` reports each `upstream-changed` in every assembly root, or holder, until `apply` re-copies it. Nothing in this repository records their digests. Left out: the ride-along on #539 (the two `git-not-runnable` rules) is not covered by this change; `member-gitlink-absent`'s `add` is still refused `member-already-present`, because the row is kept, and `remove` there dies in `git submodule deinit` (measured; outside #539); the holder's LOCKSTEP sentence names `family.py bump` without `--family-root` or a branch; an untracked workflow naming the leg makes `bump-leg.py` refuse the reference line by name, as #478's lines are; the lines are POSIX-quoted (#532). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- .../assembly-root/scripts/validate-pins.py | 200 +++++++++-- .../family-root/scripts/validate-family.py | 89 ++++- tests/test_doctor_gitlink_repairs.py | 12 +- tests/test_validate_family_branch_step.py | 213 +++++++++++ tests/test_validate_family_unnamed_mounts.py | 48 ++- tests/test_validate_pins_lockstep_lines.py | 335 ++++++++++++++++++ 6 files changed, 842 insertions(+), 55 deletions(-) create mode 100644 tests/test_validate_family_branch_step.py create mode 100644 tests/test_validate_pins_lockstep_lines.py diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index d387fcf1..f5d4eb36 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -97,6 +97,26 @@ "or the next pull request is red on somebody else's change." ) +#: The findings THE LOCKSTEP RULE is about (#539): the three facts it names, +#: out of step, and the pin's digest of the commit they name. The rule ends +#: each of them, and the closing count only where one of them was found: a +#: lone `shape-copy-drift` is an edited copy, not a leg moved alone. +LOCKSTEP_CODES = frozenset({"pin-gitlink-mismatch", "pin-digest-mismatch", + "pin-workflow-ref-mismatch"}) + + +class Curable(NamedTuple): + """A `pin-digest-mismatch` or a `pin-workflow-ref-mismatch`, for + `_end_curable_findings` (#539): where it is in `findings`, the leg, the + commit its gitlink and its pin name, and a reference's own commit (None + for a digest).""" + + index: int + role: object + path: str + commit: str + reference: str | None + class Report: def __init__(self) -> None: @@ -108,6 +128,9 @@ def __init__(self) -> None: #: Each `pin-gitlink-mismatch`, as `(its index in findings, role, #: path, gitlink, pin commit)`, for `_end_gitlink_mismatches` (#478). self.mismatches: list[tuple[int, object, str, str, str]] = [] + #: Each `pin-digest-mismatch` and `pin-workflow-ref-mismatch`, for + #: `_end_curable_findings` (#539). + self.curable: list[Curable] = [] def finding(self, code: str, detail: str, leg=None) -> None: """`leg` is the role of the leg a `bump-leg.py` line moves when that @@ -363,24 +386,74 @@ def _gitlink_repairs(root: Path, role, gitlink: str, commit: str) -> str: where the reader's checkout is, so the script is relative to one and the root, which it does know, is absolute and quoted. """ - bump = (f"{PYTHON} scripts/bump-leg.py --root {shlex.quote(str(root))} " + return _two_repairs(root, role, "the first command keeps the gitlink's " + "commit and the second the pin's", gitlink, commit) + + +def _bump_command(root: Path, role) -> str: + """`bump-leg.py` for this root's leg `role`, up to its `--to` (#478).""" + return (f"{PYTHON} scripts/bump-leg.py --root {shlex.quote(str(root))} " f"--leg {shlex.quote(str(role))} --to") - return ("Which commit stays is the person's call. From a checkout of " - "openRepoShape, the first command keeps the gitlink's commit and " - "the second the pin's; run either with `--dry-run` first. Either " - "puts all three at it in one commit, on a branch other than the " - "tracking branch, once this validator and validate-manifest.py " - "pass on its rewrite (it rolls the bump back when either is red), " - "and refuses a commit no branch of the leg's remote contains, so " - f"push one made in the leg first:\n {bump} {gitlink}\n" - f" {bump} {commit}") -def _gitlink_repairs_held_back(paths: list[str], others: list[str]) -> str: +#: What a `bump-leg.py` line needs to commit, said once for every finding +#: that names one (#478, #539). +BUMP_COMMITS = ("on a branch other than the tracking branch, once this " + "validator and validate-manifest.py pass on its rewrite (it " + "rolls the bump back when either is red), and refuses a " + "commit no branch of the leg's remote contains") + + +def _two_repairs(root: Path, role, choice: str, first: str, + second: str) -> str: + """Two `bump-leg.py` lines for one leg, `--to` `first` and `--to` + `second`, and `choice`, saying what each keeps: which commit stays is + the person's call, so neither is chosen (#478, #539).""" + bump = _bump_command(root, role) + return ("Which commit stays is the person's call. From a checkout of " + f"openRepoShape, {choice}; run either with `--dry-run` first. " + f"Either puts all three at it in one commit, {BUMP_COMMITS}, so " + f"push one made in the leg first:\n {bump} {first}\n" + f" {bump} {second}") + + +def _digest_repair(root: Path, role, commit: str) -> str: + """The one `bump-leg.py` line a lone `pin-digest-mismatch` ends on + (#539): `--to` the commit the gitlink and the pin already name, which + recomputes the digest from the leg's own objects and writes it into the + pin, the one fact it moves. Never a hand edit of the digest, which is + the drift this check exists to find.""" + return ("From a checkout of openRepoShape, this recomputes the digest " + f"at {commit[:12]}, the commit the gitlink and the pin already " + "name, from the leg's own objects and writes it into the pin; " + "run it with `--dry-run` first. It commits " + f"{BUMP_COMMITS}, so push that commit from the leg first if no " + f"branch holds it:\n {_bump_command(root, role)} {commit}") + + +def _reference_repairs(root: Path, role, gitlink: str, + reference: str) -> str: + """The two `bump-leg.py` lines a lone `pin-workflow-ref-mismatch` ends + on (#539), as `pin-gitlink-mismatch` ends on two (#478): a reference + edited by hand wants the first, which puts it back at the commit the + gitlink and the pin name, unless the edit was meant, and then the + second moves the gitlink and the pin to the reference's commit.""" + return _two_repairs( + root, role, "the first command keeps the commit the gitlink and the " + "pin name and moves every reference to the leg's repository to it, " + "and the second moves the gitlink and the pin to this reference's", + gitlink, reference) + + +def _gitlink_repairs_held_back(paths: list[str], others: list[str], + lines: str = "the two bump-leg.py lines" + ) -> str: """Why a `pin-gitlink-mismatch` names no command, and what comes first (#478): `bump-leg.py` runs this validator before it commits and rolls the bump back when it is red, so no single line commits while another - leg is off its pin or another finding stands.""" + leg is off its pin or another finding stands. A `pin-digest-mismatch` + held back by another finding says the same, naming `lines`, the one + line it would end on (#539).""" what, first = [], [] if others: what.append("this validator also reports " + ", ".join(others)) @@ -397,13 +470,20 @@ def _gitlink_repairs_held_back(paths: list[str], others: list[str]) -> str: + " and ".join(what) + ": bump-leg.py moves one leg and runs " "this validator before it commits, rolling the bump back when it " "is red. First " + ", and ".join(first) + "; this validator then " - "names the two bump-leg.py lines for " + f"names {lines} for " + ("the leg that is left." if len(paths) > 1 else "that leg.")) -def _end_gitlink_mismatches(root: Path, report: Report) -> None: +def _has_own_gate(root: Path) -> bool: + """Whether `root` has a `scripts/validate-pins.py` of its own, the gate + `bump-leg.py` runs before it commits and skips when it is absent + (#478).""" + return os.path.isfile(root / "scripts" / "validate-pins.py") + + +def _end_gitlink_mismatches(root: Path, report: Report) -> object: """What each `pin-gitlink-mismatch` ends on, once every check has run - (#478). + (#478). Returns the role of the leg whose two lines it named, or None. `bump-leg.py` RUNS THIS WHOLE VALIDATOR after its rewrite and before it commits, and rolls the bump back when it is red. So its two lines are @@ -418,9 +498,8 @@ def _end_gitlink_mismatches(root: Path, report: Report) -> None: the finding ends at THE LOCKSTEP RULE, as it did before #478, and the root's own gate is the thing to put back first. """ - if not report.mismatches \ - or not os.path.isfile(root / "scripts" / "validate-pins.py"): - return + if not report.mismatches or not _has_own_gate(root): + return None roles = [role for _, role, _, _, _ in report.mismatches] others: list[str] = [] for code, leg in report.cleared_by: @@ -430,11 +509,78 @@ def _end_gitlink_mismatches(root: Path, report: Report) -> None: index, role, _, gitlink, commit = report.mismatches[0] report.findings[index] += "\n " + _gitlink_repairs(root, role, gitlink, commit) - return + return role why = _gitlink_repairs_held_back( [path for _, _, path, _, _ in report.mismatches], others) for index, *_ in report.mismatches: report.findings[index] += "\n " + why + return None + + +def _end_curable_findings(root: Path, report: Report, named) -> None: + """What each `pin-digest-mismatch` and `pin-workflow-ref-mismatch` ends + on, once every check has run (#539). `named` is the role of the leg + whose `pin-gitlink-mismatch` names its two lines, or None. + + THEY ENDED AT THE LOCKSTEP RULE AND NAMED NO COMMAND, though + `bump-leg.py --to` the commit the gitlink and the pin name cures both: + it recomputes the pin's digest from the leg's own objects and moves + every workflow reference to the leg's repository, in one commit. So a + digest names that line, and a reference names it and the line `--to` + its own commit, since an edit to the reference may have been meant: + which commit stays is the person's call, as it is for a gitlink (#478). + + ONLY WHERE A LINE CAN COMMIT, by `_end_gitlink_mismatches`' rule: the + root has this gate of its own, and every finding is the leg's own, + which a bump of it clears. Where another finding stands with them and + no other leg is out of step, the finding says to clear that first, in + the words a held-back `pin-gitlink-mismatch` uses. Where the leg's own + gitlink is off its pin, that finding's lines clear this one too, and + this says so where they are named. Where ANOTHER leg is out of step too, + no line can commit and the finding ends at THE LOCKSTEP RULE, as it did + before: no bump-leg.py line clears either leg's finding first, so there + is no first step this could name that the other's would not contradict. + """ + if not report.curable or not _has_own_gate(root): + return + legs, untagged = _out_of_step(report) + off = {role for _, role, _, _, _ in report.mismatches} + for one in report.curable: + ending = _curable_ending(root, one, legs, untagged, off, named) + if ending: + report.findings[one.index] += "\n " + ending + + +def _out_of_step(report: Report) -> tuple[list, list[str]]: + """`(legs, codes)`: each leg a finding names as the one whose bump + clears it, and the code of each finding no bump clears, each once and + in the order found (#539).""" + legs: list = [] + untagged: list[str] = [] + for code, leg in report.cleared_by: + if leg is None and code not in untagged: + untagged.append(code) + elif leg is not None and leg not in legs: + legs.append(leg) + return legs, untagged + + +def _curable_ending(root: Path, one: Curable, legs: list, + untagged: list[str], off: set, named) -> str: + """What `_end_curable_findings` adds to the finding `one`, or `""`.""" + if one.role in off: + return (f"Either bump-leg.py line under {one.path}'s " + "pin-gitlink-mismatch above clears this finding too." + if one.role == named else "") + if one.role is None or legs != [one.role]: + return "" + if untagged: + return _gitlink_repairs_held_back( + [one.path], untagged, "the bump-leg.py line" + if one.reference is None else "the two bump-leg.py lines") + if one.reference is None: + return _digest_repair(root, one.role, one.commit) + return _reference_repairs(root, one.role, one.commit, one.reference) def _check_gitlink_matches_pin(root: Path, role, path: str, commit: str, @@ -519,6 +665,8 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, f"are not the bytes the pin was written against.\n " + LOCKSTEP, leg=role, ) + report.curable.append(Curable(len(report.findings) - 1, role, + path, commit, None)) else: report.note(f"{path}: tree digest recomputes ({recorded[:12]}…)") @@ -542,6 +690,8 @@ def _check_workflow_refs_for_leg(refs, repository: str, path: str, f"{gitlink} recorded at {path!r}\n " + LOCKSTEP, leg=role, ) + report.curable.append(Curable(len(report.findings) - 1, role, + path, gitlink, wf_sha)) if seen: report.note(f"{repository}: {seen} workflow @ reference(s) " f"agree with the gitlink") @@ -1109,7 +1259,7 @@ def _run_all_checks(root: Path, report: Report, _check_leg(root, leg, report, refs) _check_shape_pin(root, manifest, report) _check_neutral_product_pins(root, manifest, report, pin_source_values) - _end_gitlink_mismatches(root, report) + _end_curable_findings(root, report, _end_gitlink_mismatches(root, report)) def _emit_report(report: Report, quiet: bool) -> int: @@ -1128,6 +1278,12 @@ def _emit_report(report: Report, quiet: bool) -> int: lives in `shape_materialize.py`, which no project carries, and this file imports nothing but `repo_shape`, so here such a byte prints as `\\udcXX`, as it already does on stderr. + + THE COUNT ENDS AT THE LOCKSTEP RULE ONLY WHERE IT APPLIES (#539): where + one of `LOCKSTEP_CODES` was found. Every other finding names its own + exit, and a lone `shape-copy-drift`, an edited copy that `update-shape.py` + or a revert puts right, was followed by a rule about moving a gitlink, a + pin and the workflows together. """ if getattr(sys.stdout, "errors", None) in ("strict", "surrogateescape"): try: @@ -1140,7 +1296,9 @@ def _emit_report(report: Report, quiet: bool) -> int: for finding in report.findings: print(finding, file=sys.stderr) if report.findings: - print(f"\n{len(report.findings)} finding(s). " + LOCKSTEP, file=sys.stderr) + rule = any(code in LOCKSTEP_CODES for code, _ in report.cleared_by) + print(f"\n{len(report.findings)} finding(s)." + + (" " + LOCKSTEP if rule else ""), file=sys.stderr) return 1 print("pins ok") return 0 diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 8eff3766..c7f440d8 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -104,14 +104,46 @@ "what this refuses." ) +#: The findings THE LOCKSTEP RULE is about (#539): a member's gitlink and its +#: pin out of step, and the pin's digest of the commit they name. The closing +#: count ends at the rule only where one of them was found. +LOCKSTEP_CODES = frozenset({"member-gitlink-mismatch", + "member-digest-mismatch"}) + +#: A branch-name word as `family.py` suggests one (#487, its +#: `BRANCH_WORD_RE`): dot-separated words of lowercase letters, digits, `_` +#: and `-`, and no trailing `.lock`. Anything else is said as `member`. +BRANCH_WORD_RE = re.compile(r"[a-z0-9][a-z0-9_-]*(?:\.[a-z0-9_-]+)*") + + +def _switch_line(root: Path, verb: str, name: str | None) -> str: + """`git -C switch -c family/-`: the branch + `family.py add`, `bump` and `remove` refuse to commit without, since a + holder's tracking branch is refused (`family-on-tracking-branch`, #487), + spelled as `family.py` suggests it and the holder's docs show it (#539). + `name` is the member's row `id:` for `remove` and its project for + `add`, lowercased; None is the docs' own ``. POSIX-quoted, as + every other line this file prints is.""" + if name is None: + return (f"git -C {shlex.quote(str(root))} switch -c " + f"family/{verb}-") + word = name.lower() + if not BRANCH_WORD_RE.fullmatch(word) or word.endswith(".lock"): + word = "member" + return shlex.join(["git", "-C", str(root), "switch", "-c", + f"family/{verb}-{word}"]) + class Report: def __init__(self) -> None: self.findings: list[str] = [] self.notes: list[str] = [] + #: Each finding's code, in step with `findings` (#539). + self.codes: list[str] = [] def finding(self, code: str, detail: str) -> None: self.findings.append(f"FINDING {code}: {detail}") + self.codes.append(code) def note(self, text: str) -> None: self.notes.append(f" ok {text}") @@ -347,7 +379,11 @@ def _check_member_gitlink(root: Path, path: str, commit: str, project: str, """The submodule gitlink this repository records at `path`, against the pin — THE LOCKSTEP RULE. Split from `_check_member` for #137. Raises `Refusal` when there is no gitlink at all: a row naming a member this - repository does not record as a submodule.""" + repository does not record as a submodule. Its remedy is two lines, the + `git switch -c` that `add` needs first (#539) and the `add`, from a + checkout of openRepoShape and naming this holder by its path, as the + holder's other remedies do: `--family-root .` named the checkout the + line runs from.""" gitlink = recorded_gitlink(root, path) if gitlink is None: raise Refusal( @@ -355,9 +391,14 @@ def _check_member_gitlink(root: Path, path: str, commit: str, project: str, f"no gitlink (mode 160000) recorded at {path!r}. {project} is " f"named in {MANIFEST} but this repository does not record it as a " "submodule.", - f"Remediation: `{PYTHON} scripts/family.py add --family-root . " - f"--member /{project}` mounts and pins it in one commit; a " - "row with no submodule is a claim about a tree that is not here.") + "Remediation: from a checkout of openRepoShape, the second " + "command below mounts and pins it in one commit, on a branch " + "other than the holder's tracking branch, which it refuses, so " + "the first makes one; a row with no submodule is a claim about a " + "tree that is not here.\n" + f" {_switch_line(root, 'add', project)}\n" + f" {PYTHON} scripts/family.py add --family-root " + f"{shlex.quote(str(root))} --member /{shlex.quote(project)}") if gitlink != commit: report.finding( "member-gitlink-mismatch", @@ -707,7 +748,9 @@ def _check_member_ids_unique(root: Path, members: list, run a private family's CI falls back to when its members cannot be checked out. The remediation is runnable as printed: openRepoShape's `scripts/family.py` from a checkout of the standard, this holder's own - path, and the later row's own spelling, which `remove` matches exactly. + path, and the later row's own spelling, which `remove` matches exactly; + and before it the `git switch -c` that `remove` needs on a holder that + stands on its tracking branch, which it refuses (#487, #539). """ ids: dict[str, str] = {} projects: set[str] = set() @@ -729,7 +772,10 @@ def _check_member_ids_unique(root: Path, members: list, "member-duplicate-id", f"{project} and {first} both record id {member_id!r}: one " "project is pinned twice. A family pins each project once. From " - f"a checkout of openRepoShape, `{PYTHON} scripts/family.py remove " + "a checkout of openRepoShape, and on a branch other than the " + "holder's tracking branch, which `remove` refuses " + f"(`{_switch_line(root, 'remove', member_id)}` makes one), " + f"`{PYTHON} scripts/family.py remove " f"--family-root {shlex.quote(str(root))} --member " f"{shlex.quote(project)}` takes the later row, {project}, out; " f"name {first} there instead to keep {project}.") @@ -1030,6 +1076,9 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], (`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. + A BRANCH FIRST (#539): `add` refuses the holder's tracking branch + (#487), so the first of the three lines is the `git switch -c` that + makes the branch `add` suggests. """ holds = set(gitlinks) for path, commit in sorted(gitlinks.items()): @@ -1055,17 +1104,19 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], 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 "") + project = parts[1] if len(parts) == 2 and parts[1] else None + member = "" if project is None else shlex.quote(project) 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"{said} {RESTORE_ROW} If it was mounted by hand, make a branch " + "with the first command below, since family.py add refuses the " + "holder's tracking branch, take it out with the second, which " + f"refuses while its checkout holds changes and takes its " + f"{GITMODULES} section out too, then mount it with the third, " + "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" {_switch_line(root, 'add', project)}\n" f" git rm -- {shlex.quote(path)}\n" f" {PYTHON} scripts/family.py add --family-root " f"{shlex.quote(str(root))} --member /{member}") @@ -1260,8 +1311,12 @@ def main(argv: list[str] | None = None) -> int: for finding in report.findings: print(finding, file=sys.stderr) if report.findings: - print(f"\n{len(report.findings)} finding(s) in {MANIFEST}. {LOCKSTEP}", - file=sys.stderr) + # THE LOCKSTEP RULE ONLY WHERE IT APPLIES (#539): a lone + # `shape-copy-drift` or `member-duplicate-id` is no gitlink moved + # alone. + rule = any(code in LOCKSTEP_CODES for code in report.codes) + print(f"\n{len(report.findings)} finding(s) in {MANIFEST}." + + (f" {LOCKSTEP}" if rule else ""), file=sys.stderr) return 1 print(f"family ok: {manifest.get('name')} ({manifest.get('id')}), " f"{len(members)} member(s)") diff --git a/tests/test_doctor_gitlink_repairs.py b/tests/test_doctor_gitlink_repairs.py index 59c95c02..ddb56bbc 100644 --- a/tests/test_doctor_gitlink_repairs.py +++ b/tests/test_doctor_gitlink_repairs.py @@ -403,12 +403,19 @@ def refused_with_the_gitlink_moved(root: Path) -> None: "pin-submodule-uninitialized": (refused_with_the_gitlink_moved, 2), } +#: The two of those whose finding, alone, names its own `bump-leg.py` line +#: in the gate's report (#539): the gate's, below its finding's first line, +#: which is all the row quotes, so the row is still the gate's own. +GATE_NAMES_THE_LINE = {"pin-digest-mismatch", "pin-workflow-ref-mismatch"} + @pytest.mark.parametrize("code", sorted(OTHER_FINDINGS)) def test_every_other_pins_row_is_the_gates_own(standard, project, code): """Byte for byte the row the gate alone makes, with no repair named: four findings a gitlink at its pin can have, and a refusal over a - gitlink that is not.""" + gitlink that is not. The gate names a `bump-leg.py` line for a lone + digest and a lone workflow reference, and for nothing else here + (#539, `tests/test_validate_pins_lockstep_lines.py`).""" root = project.resolve() make, exit_code = OTHER_FINDINGS[code] make(root) @@ -419,7 +426,8 @@ def test_every_other_pins_row_is_the_gates_own(standard, project, code): assert "gitlink_repairs" not in row["detail"], row["detail"] assert "bump-leg" not in row["reason"], row["reason"] gate = run_script(root / "scripts" / "validate-pins.py", cwd=root) - assert "bump-leg.py" not in gate.stdout + gate.stderr + named = "scripts/bump-leg.py" in gate.stdout + gate.stderr + assert named == (code in GATE_NAMES_THE_LINE), gate.stderr # --- named only where a line can commit ------------------------------------- diff --git a/tests/test_validate_family_branch_step.py b/tests/test_validate_family_branch_step.py new file mode 100644 index 00000000..d55915fb --- /dev/null +++ b/tests/test_validate_family_branch_step.py @@ -0,0 +1,213 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-family.py`: a remedy that names `family.py add` or `remove` +makes the branch first, and THE LOCKSTEP RULE only where it applies (#539). + +THE BRANCH STEP. Since #487 a real `add`, `bump` or `remove` refuses the +holder's tracking branch (`family-on-tracking-branch`) with the `git switch +-c` that makes a branch, and `init` leaves a holder on that branch. Three of +the holder gate's remedies named `add` or `remove` and no branch, so each +cost one refusal first: `member-duplicate-id`'s `remove`, a hand mount's +`member-gitlink-unnamed` `add` (run as printed in +`tests/test_validate_family_unnamed_mounts.py`, now from the tracking +branch), and `member-gitlink-absent`'s `add`. Each now names `git -C + switch -c family/-` first, the branch `family.py` +suggests and the holder's docs show, POSIX-quoted as every line this gate +prints is, and the `add` of `member-gitlink-absent` names the holder by its +path, where `--family-root .` named the checkout the line runs from. + +THE COUNT. The report ended ` finding(s) in family.yaml.` and THE +LOCKSTEP RULE whatever was found; it ends at the rule only where a member's +gitlink and its pin, or the pin's digest, were found out of step. + +THE SAME FAMILY AS `tests/test_family.py`, this module's own instance, and +the holder copy `tests/test_family_tracking_branch.py` puts back on its +tracking branch, where a person stands after `init`. +""" + +from __future__ import annotations + +import importlib.util +import re +import shlex +import sys + +import pytest + +from conftest import REPO, git, run_script +from repo_shape import PYTHON +from test_family import ( # noqa: F401 + family, family_module, manifest, mount_twice, validate, +) +from test_family_tracking_branch import on_main # noqa: F401 + +#: The template every holder's `scripts/validate-family.py` is a copy of. +TEMPLATE = (REPO / "templates" / "family-root" / "scripts" / + "validate-family.py") + +#: How the holder's THE LOCKSTEP RULE starts. +RULE = "THE LOCKSTEP RULE: a member's gitlink and its `members[].pin.commit`" + + +def gate_module(monkeypatch): + """The template `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( + "validate_family_branch_step", TEMPLATE) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def count_line(result) -> str: + """The line the gate's report ends on.""" + return result.stderr.rstrip("\n").splitlines()[-1] + + +def switch_argv(holder, verb: str, name: str) -> list[str]: + """The `git switch -c` a remedy names, as words: `family.py`'s own + suggestion for `verb` and `name`, in this holder.""" + branch = family_module()._suggested_branch(verb, name) + return ["git", "-C", str(holder), "switch", "-c", branch] + + +# --- the branch `family.py` would suggest, spelled the same way -------------- + +@pytest.mark.parametrize("verb,name", [ + ("add", "IRQS"), ("remove", "irrs"), ("add", "Atlas.Docs"), + ("add", "My Proj"), ("add", "O'Brien"), ("remove", "x.lock"), + ("add", "a..b"), ("remove", "-x"), +], ids=["project", "id", "dotted", "space", "quote", "lock", "dots", + "dash"]) +def test_the_switch_names_the_branch_family_py_suggests(monkeypatch, tmp_path, + verb, name): + """The branch word is `family.py`'s `_suggested_branch`'s, whatever the + member is called: a name no branch word spells is said as `member`. + The holder is quoted for a POSIX shell, as the gate's other lines are.""" + holder = tmp_path / "it's a holder" + line = gate_module(monkeypatch)._switch_line(holder, verb, name) + assert shlex.split(line) == switch_argv(holder, verb, name), line + + +# --- member-duplicate-id: switch, then remove, from the tracking branch ----- + +def test_the_duplicate_id_remedy_makes_the_branch_then_removes(family, + on_main): + """From the tracking branch, the remedy's two commands, run as printed + from a checkout of openRepoShape: the `switch` makes `family.py`'s own + branch for the `remove`, and the `remove` then takes the later row out + on it, where without the branch it is refused.""" + holder = on_main + mount_twice(holder, family["remotes"]) + found = validate(holder) + assert found.returncode == 1, found.stderr + found.stdout + commands = re.findall(r"`([^`]*)`", found.stderr.split( + "FINDING member-duplicate-id: ", 1)[1].splitlines()[0]) + switch, remove = [command for command in commands + if command.startswith(("git -C", PYTHON))] + assert shlex.split(switch) == switch_argv(holder, "remove", "irrs") + assert found.stderr.index(switch) < found.stderr.index(remove) + refused = run_script(REPO / "scripts" / "family.py", + *shlex.split(remove)[2:], cwd=REPO) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert "REFUSED family-on-tracking-branch: " in refused.stderr + git(*shlex.split(switch)[1:], cwd=holder.parent) + argv = shlex.split(remove) + assert argv[:3] == [PYTHON, "scripts/family.py", "remove"], remove + removed = run_script(REPO / argv[1], *argv[2:], cwd=REPO) + assert removed.returncode == 0, removed.stderr + removed.stdout + assert git("symbolic-ref", "--short", "HEAD", + cwd=holder).stdout.strip() == "family/remove-irrs" + assert "IRRS2" not in [row["project"] + for row in manifest(holder)["members"]] + assert validate(holder).returncode == 0 + + +# --- member-gitlink-absent: the branch, then the add, naming the holder ----- + +def test_a_missing_gitlink_remedy_makes_the_branch_and_names_the_holder( + on_main): + """The refusal's two lines: the `switch` that makes `family.py`'s + branch for the `add`, and the `add` from a checkout of openRepoShape, + `--family-root` this holder by its path rather than `.`, which names + the checkout the line runs from. The `switch`, run as printed, puts + the holder on that branch.""" + holder = on_main + git("rm", "-r", "-q", "-f", "--", "members/IRRS", cwd=holder) + git("commit", "-qm", "Drop the gitlink but keep the row", cwd=holder) + result = validate(holder) + assert result.returncode == 2, result.stdout + result.stderr + lines = result.stderr.splitlines() + assert lines[0].startswith("REFUSED member-gitlink-absent: "), lines + assert lines[1] == ( + "Remediation: from a checkout of openRepoShape, the second command " + "below mounts and pins it in one commit, on a branch other than the " + "holder's tracking branch, which it refuses, so the first makes one; " + "a row with no submodule is a claim about a tree that is not here.") + switch, add = (line.strip() for line in lines[2:]) + assert lines[2:] == [" " + switch, " " + add], lines + assert shlex.split(switch) == switch_argv(holder, "add", "IRRS") + assert shlex.split(add) == [PYTHON, "scripts/family.py", "add", + "--family-root", str(holder), "--member", + "/IRRS"], add + git(*shlex.split(switch)[1:], cwd=holder.parent) + assert git("symbolic-ref", "--short", "HEAD", + cwd=holder).stdout.strip() == "family/add-irrs" + + +# --- the count ends at the rule only where it applies ----------------------- + +def copy_edited(holder) -> None: + """A pinned copy edited in place: a lone `shape-copy-drift`.""" + target = holder / "scripts" / "bootstrap.py" + target.write_bytes(target.read_bytes() + b"\n# edit\n") + + +def gitlink_moved(holder) -> None: + """IRRS's gitlink moved alone, staged at an EMPTY commit past its pin, + so the pin's digest still recomputes: a lone + `member-gitlink-mismatch`.""" + git("commit", "-q", "--allow-empty", "-m", "The member moved alone", + cwd=holder / "members" / "IRRS") + git("add", "--", "members/IRRS", cwd=holder) + + +def digest_written_wrong(holder) -> None: + """IRSS's digest written wrong: a lone `member-digest-mismatch`.""" + row = {r["project"]: r for r in manifest(holder)["members"]}["IRSS"] + text = (holder / "family.yaml").read_bytes() + (holder / "family.yaml").write_bytes(text.replace( + row["pin"]["tree_sha256"].encode("ascii"), b"b" * 64, 1)) + + +#: Each lone finding, how it is made, and whether the count ends at the rule. +LONE = { + "shape-copy-drift": (copy_edited, False), + "member-duplicate-id": (lambda holder: None, False), + "member-gitlink-mismatch": (gitlink_moved, True), + "member-digest-mismatch": (digest_written_wrong, True), +} + + +@pytest.mark.parametrize("code", sorted(LONE)) +def test_the_count_ends_at_the_rule_only_where_it_applies(family, on_main, + code): + """A lone `shape-copy-drift`, an edited copy, and a lone + `member-duplicate-id`, one project pinned twice, end on the count; a + gitlink and a pin out of step, or a digest, end on the count and THE + LOCKSTEP RULE.""" + holder = on_main + make, rule = LONE[code] + if code == "member-duplicate-id": + mount_twice(holder, family["remotes"]) + make(holder) + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.count("FINDING ") == 1, result.stderr + assert f"FINDING {code}: " in result.stderr + if rule: + assert count_line(result).startswith( + "1 finding(s) in family.yaml. " + RULE), result.stderr + else: + assert count_line(result) == "1 finding(s) in family.yaml." + assert "LOCKSTEP RULE" not in result.stdout + result.stderr diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py index 5791a49a..09552f2e 100644 --- a/tests/test_validate_family_unnamed_mounts.py +++ b/tests/test_validate_family_unnamed_mounts.py @@ -85,12 +85,20 @@ 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.") + "was mounted by hand, make a branch with the first command below, since " + "family.py add refuses the holder's tracking branch, take it out with " + "the second, which refuses while its checkout holds changes and takes " + "its .gitmodules section out too, then mount it with the third, 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.") + + +def switch_for(holder, member: str) -> str: + """The first command a hand mount's gitlink prints (#539): the branch + `family.py add` suggests for `member`, in this holder.""" + return (f"git -C {shlex.quote(str(holder.resolve()))} switch -c " + f"family/add-{member.lower()}") #: The same two halves for an unnamed `.gitmodules` section. SECTION_SAID = ( @@ -287,6 +295,7 @@ def test_a_member_mounted_by_hand_is_two_findings(family, holder, tmp_path, assert gitlink.splitlines()[0] == GITLINK_SAID.format( path=f"members/{SPARE}", commit=tip) + GITLINK_REMEDY assert commands(gitlink) == [ + switch_for(holder, SPARE), f"git rm -- members/{SPARE}", f"{PYTHON} scripts/family.py add --family-root " f"{shlex.quote(str(holder.resolve()))} --member /{SPARE}"] @@ -590,7 +599,7 @@ def test_two_sections_for_a_hand_mounts_path_each_keep_their_commands( 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) + git(*shlex.split(commands(findings(result)[1])[1])[1:], cwd=holder) listed = git("config", "-f", ".gitmodules", "--list", cwd=holder).stdout assert listed.count(f".path={path}") == 1, listed @@ -642,25 +651,31 @@ def test_the_add_remedy_quotes_the_member_it_names(family, holder, project, result = validate(holder, "--no-members") assert result.returncode == 1, result.stderr + result.stdout (gitlink,) = findings(result) - rm, add = commands(gitlink) + switch, 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}" + # No branch word spells the project, so the branch says `member`, as + # `family.py add` would suggest it (#539). + assert shlex.split(switch)[-1] == "family/add-member", switch # --- 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. + """ROW (a) FOR REAL, and the gitlink's remedy run as printed, from the + holder's TRACKING BRANCH, where `init` leaves it and `add` refuses to + commit (#487): the first line makes the branch `add` suggests (#539). + `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 + on that branch, 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("switch", "-q", "-C", "main", cwd=holder) git("-c", "protocol.file.allow=always", "submodule", "add", "-q", "--", str(family["remotes"] / f"{SPARE}.git"), f"members/{SPARE}", cwd=holder) @@ -669,8 +684,9 @@ def test_a_hand_mount_is_taken_out_and_added_back_as_printed(family, holder): assert result.returncode == 1, result.stderr + result.stdout assert codes(result) == ["member-gitlink-unnamed", "gitmodules-path-unnamed"] - rm, add = commands(findings(result)[0]) + switch, rm, add = commands(findings(result)[0]) assert commands(findings(result)[1]) == [] + git(*shlex.split(switch)[1:], cwd=holder.parent) git(*shlex.split(rm)[1:], cwd=holder) assert not (holder / "members" / SPARE).exists() assert f"members/{SPARE}" not in (holder / ".gitmodules").read_text( @@ -689,6 +705,8 @@ def add_as_printed(): added = add_as_printed() assert added.returncode == 0, added.stdout + added.stderr assert git("status", "--porcelain", cwd=holder).stdout == "" + assert git("symbolic-ref", "--short", "HEAD", + cwd=holder).stdout.strip() == f"family/add-{SPARE.lower()}" assert_passes_in_every_mode(holder, 3) diff --git a/tests/test_validate_pins_lockstep_lines.py b/tests/test_validate_pins_lockstep_lines.py new file mode 100644 index 00000000..83830136 --- /dev/null +++ b/tests/test_validate_pins_lockstep_lines.py @@ -0,0 +1,335 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-pins.py`: THE LOCKSTEP RULE only where it applies, and the +`bump-leg.py` lines a pin's digest and a workflow reference end on (#539). + +THE COUNT. The report ended ` finding(s).` and THE LOCKSTEP RULE whatever +was found, so a lone `shape-copy-drift`, an edited copy that `update-shape.py` +or a revert puts right, was followed by a rule about moving a gitlink, a pin +and the workflows together. The count ends at the rule only where a finding +the rule is about was found: a gitlink off its pin, a pin's digest, or a +workflow reference. + +TWO FINDINGS NAMED NO COMMAND. `pin-digest-mismatch` and +`pin-workflow-ref-mismatch` ended at THE LOCKSTEP RULE, though `bump-leg.py +--to` the commit the gitlink and the pin name cures both: it recomputes the +digest from the leg's own objects and moves every reference to the leg's +repository, in one commit. Each now names its lines where one can commit, +by #478's rule for a gitlink: the root has this gate of its own, and every +finding is the leg's own. A digest names one line. A reference names two, +`--to` the gitlink's commit and `--to` its own, since an edit to it may have +been meant, and which commit stays is the person's call. Held back by +another finding, they say what comes first in #478's words. + +EVERY LINE HERE IS RUN AS PRINTED, from the standard's checkout, split as a +POSIX shell splits it (the gate's lines are POSIX-quoted, #532), over a root +under a directory whose name holds a blank and an apostrophe; the doctor +then says COMPLIANT. + +The fixtures are `tests/test_shape_doctor.py`'s and +`tests/test_doctor_gitlink_repairs.py`'s, imported, so this module has its +own scaffold and its own remotes, and the push `advanced` makes moves no +other module's. +""" + +from __future__ import annotations + +import os +import shlex +import subprocess +from pathlib import Path + +import pytest + +from conftest import git, run_script +from repo_shape import PYTHON +from test_doctor_gitlink_repairs import ( # noqa: F401 + OTHER_SHA, WORKING, advanced, assert_compliant, copy_drifted, + digest_moved, moved_by_hand, path_moved, pin_of, under_an_awkward_name, + workflow_at, +) +from test_shape_doctor import ( # noqa: F401 + GIT_IDENTITY, PROJECT, project, scaffolded_here, standard, +) + +#: How THE LOCKSTEP RULE starts, wherever the gate prints it. +RULE = "THE LOCKSTEP RULE: a leg's gitlink, its `contracts/-pin.yaml` " + +#: What a `bump-leg.py` line needs to commit, as every gate line says it. +COMMITS = ("on a branch other than the tracking branch, once this validator " + "and validate-manifest.py pass on its rewrite (it rolls the bump " + "back when either is red), and refuses a commit no branch of the " + "leg's remote contains") + + +def gate(root: Path) -> subprocess.CompletedProcess: + """The root's own `validate-pins.py`, run in the root.""" + return run_script(root / "scripts" / "validate-pins.py", cwd=root) + + +def block(stderr: str, code: str) -> list[str]: + """The one finding `code` in the gate's report: its `FINDING` line and + every indented line under it.""" + lines = stderr.splitlines() + starts = [at for at, line in enumerate(lines) + if line.startswith(f"FINDING {code}: ")] + assert len(starts) == 1, stderr + found = [lines[starts[0]]] + for line in lines[starts[0] + 1:]: + if not line.startswith(" "): + break + found.append(line) + return found + + +def count_line(stderr: str) -> str: + """The line the gate's report ends on.""" + return stderr.rstrip("\n").splitlines()[-1] + + +def bump_of(root: Path, role: str = "spec") -> str: + """A gate line for `role`, up to its `--to`.""" + return (f"{PYTHON} scripts/bump-leg.py --root {shlex.quote(str(root))} " + f"--leg {role} --to") + + +def on_a_branch(project: Path, tmp_path: Path, change) -> Path: + """The root under an awkward name, on a branch (`bump-leg.py` refuses + the tracking branch), with `change(root)` committed there: a clean root, + which is what `bump-leg.py` asks for.""" + root = under_an_awkward_name(project, tmp_path) + git("checkout", "-q", "-b", WORKING, cwd=root) + change(root) + git("add", "-A", "--", ".", cwd=root) + git("commit", "-qm", "the change under test", cwd=root) + return root + + +def run_from_the_standard(standard: Path, line: str) -> None: + """Run a gate line exactly as printed, from the standard's checkout, + as a POSIX shell splits it; and require exit 0.""" + argv = shlex.split(line) + assert argv[:2] == [PYTHON, "scripts/bump-leg.py"], line + ran = subprocess.run(argv, cwd=str(standard), capture_output=True, + text=True, encoding="utf-8", errors="replace", + check=False, stdin=subprocess.DEVNULL, + env={**os.environ, **GIT_IDENTITY, + "PYTHONDONTWRITEBYTECODE": "1"}) + assert ran.returncode == 0, line + "\n" + ran.stdout + ran.stderr + + +def gitlink_moved(root: Path) -> None: + """The spec leg one empty commit past its pin, and that commit staged + as its gitlink: `pin-gitlink-mismatch`, with no refusal beside it.""" + leg = root / "spec" + git("commit", "-q", "--allow-empty", "-m", "past the pin", cwd=leg) + moved = git("rev-parse", "HEAD", cwd=leg).stdout.strip() + git("update-index", "--cacheinfo", f"160000,{moved},spec", cwd=root) + + +# --- the count ends at the rule only where it applies ------------------------ + +#: A finding the rule is not about, alone, and how to make it. +NOT_THE_RULES = { + "shape-copy-drift": copy_drifted, + "pin-path-mismatch": path_moved, + "shape-copy-missing": lambda root: (root / "Makefile").unlink(), +} + + +@pytest.mark.parametrize("code", sorted(NOT_THE_RULES)) +def test_a_finding_the_rule_is_not_about_ends_on_the_count_alone(project, + code): + """The issue's case, a lone `shape-copy-drift`, and two more findings + no leg moved alone: the count, and no word of THE LOCKSTEP RULE.""" + root = project.resolve() + NOT_THE_RULES[code](root) + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.count("FINDING ") == 1, result.stderr + assert f"FINDING {code}: " in result.stderr + assert "LOCKSTEP RULE" not in result.stdout + result.stderr + assert count_line(result.stderr) == "1 finding(s)." + + +#: A finding the rule IS about, and how to make it. +THE_RULES = { + "pin-gitlink-mismatch": gitlink_moved, + "pin-digest-mismatch": digest_moved, + "pin-workflow-ref-mismatch": workflow_at("spec", OTHER_SHA), +} + + +@pytest.mark.parametrize("code", sorted(THE_RULES)) +def test_the_count_ends_at_the_rule_where_the_rule_applies(project, code): + """Each of the three, beside a `shape-copy-drift`: the count still ends + at the rule, because one of its findings is there.""" + root = project.resolve() + THE_RULES[code](root) + copy_drifted(root) + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + assert f"FINDING {code}: " in result.stderr + assert count_line(result.stderr).startswith("2 finding(s). " + RULE) + + +# --- a lone digest names one line, and the line cures it --------------------- + +def test_a_lone_digest_names_its_line_and_the_line_cures_it(standard, project, + tmp_path): + """`--to` the commit the gitlink and the pin already name: run as + printed, it writes ONE commit, the recomputed digest, and the doctor + says COMPLIANT.""" + root = on_a_branch(project, tmp_path, digest_moved) + pinned = pin_of(root) + base = git("rev-parse", "HEAD", cwd=root).stdout.strip() + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + lines = block(result.stderr, "pin-digest-mismatch") + assert lines[3].startswith(" " + RULE), lines + assert lines[4:] == [ + " From a checkout of openRepoShape, this recomputes the digest at " + f"{pinned[:12]}, the commit the gitlink and the pin already name, " + "from the leg's own objects and writes it into the pin; run it with " + f"`--dry-run` first. It commits {COMMITS}, so push that commit from " + "the leg first if no branch holds it:", + f" {bump_of(root)} {pinned}"], lines + assert count_line(result.stderr).startswith("1 finding(s). " + RULE) + run_from_the_standard(standard, lines[5]) + assert git("rev-list", "--count", f"{base}..HEAD", + cwd=root).stdout.strip() == "1" + assert pin_of(root) == pinned + assert gate(root).returncode == 0 + assert_compliant(standard, root) + + +# --- a lone reference names two lines, and either cures it ------------------- + +@pytest.mark.parametrize("which", ["keep", "take"]) +def test_a_lone_reference_names_two_lines_and_either_cures_it( + standard, project, advanced, tmp_path, which): + """A workflow naming the spec leg at a commit its remote has, while the + gitlink and the pin agree on another: `keep` puts the reference back at + theirs, `take` moves the gitlink and the pin to the reference's. Either, + run as printed, writes ONE commit putting all three at its commit.""" + root = on_a_branch(project, tmp_path, workflow_at("spec", advanced)) + pinned = pin_of(root) + base = git("rev-parse", "HEAD", cwd=root).stdout.strip() + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + lines = block(result.stderr, "pin-workflow-ref-mismatch") + assert lines[0] == ( + f"FINDING pin-workflow-ref-mismatch: legs.yml: `testorg/{PROJECT}-spec" + f"@{advanced}` != the gitlink {pinned} recorded at 'spec'"), lines + assert lines[1].startswith(" " + RULE), lines + assert lines[2:] == [ + " Which commit stays is the person's call. From a checkout of " + "openRepoShape, the first command keeps the commit the gitlink and " + "the pin name and moves every reference to the leg's repository to " + "it, and the second moves the gitlink and the pin to this " + "reference's; run either with `--dry-run` first. Either puts all " + f"three at it in one commit, {COMMITS}, so push one made in the leg " + "first:", + f" {bump_of(root)} {pinned}", + f" {bump_of(root)} {advanced}"], lines + want = pinned if which == "keep" else advanced + run_from_the_standard(standard, lines[3 if which == "keep" else 4]) + assert git("rev-list", "--count", f"{base}..HEAD", + cwd=root).stdout.strip() == "1" + assert git("rev-parse", "HEAD:spec", cwd=root).stdout.strip() == want + assert pin_of(root) == want + workflow = (root / ".github" / "workflows" / "legs.yml").read_text( + encoding="utf-8") + assert f"@{want}\n" in workflow, workflow + assert_compliant(standard, root) + + +# --- named only where a line can commit -------------------------------------- + +#: A lone finding a bump of its leg clears, how to make it, and how the +#: held-back sentence names what the gate names once it is alone. +HELD = { + "pin-digest-mismatch": (digest_moved, "the bump-leg.py line"), + "pin-workflow-ref-mismatch": (workflow_at("spec", OTHER_SHA), + "the two bump-leg.py lines"), +} + + +@pytest.mark.parametrize("code", sorted(HELD)) +def test_another_finding_holds_the_lines_back_and_says_what_comes_first( + project, code): + """`bump-leg.py` runs this whole gate before it commits and rolls the + bump back when it is red, so beside a finding no bump clears the gate + names no line, says to clear that first, as a held-back gitlink says it + (#478), and once that is done names the lines.""" + root = project.resolve() + make, named = HELD[code] + make(root) + copy_drifted(root) + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + assert block(result.stderr, code)[-1] == ( + " No single bump-leg.py line can commit while this validator also " + "reports shape-copy-drift: bump-leg.py moves one leg and runs this " + "validator before it commits, rolling the bump back when it is red. " + f"First clear that finding; this validator then names {named} for " + "that leg.") + assert "scripts/bump-leg.py" not in result.stderr, result.stderr + git("checkout", "--", "scripts/bootstrap.py", cwd=root) + cleared = gate(root) + assert cleared.returncode == 1, cleared.stdout + cleared.stderr + assert block(cleared.stderr, code)[-1] == f" {bump_of(root)} " + ( + pin_of(root) if code == "pin-digest-mismatch" else OTHER_SHA) + + +def test_another_leg_out_of_step_names_no_line(project): + """The spec leg's digest and a workflow reference to the code leg: a + bump of either leg leaves the other's finding red, so no line commits, + and neither finding names one or a first step the other's would + contradict. Each ends at THE LOCKSTEP RULE, as before.""" + root = project.resolve() + digest_moved(root) + workflow_at("code", OTHER_SHA)(root) + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + for code in ("pin-digest-mismatch", "pin-workflow-ref-mismatch"): + assert block(result.stderr, code)[-1].startswith(" " + RULE) + assert "bump-leg.py" not in result.stderr, result.stderr + + +@pytest.mark.parametrize("code", ["pin-digest-mismatch", + "pin-workflow-ref-mismatch"]) +def test_beside_its_legs_gitlink_it_points_at_the_gitlinks_lines( + project, advanced, tmp_path, code): + """The spec gitlink committed by hand, and the spec pin's digest written + wrong or a reference to the spec leg left at the pin: the gitlink's two + lines clear this finding too (#478 runs them), so it says so and names + no line of its own.""" + pinned = pin_of(project) + also = (digest_moved if code == "pin-digest-mismatch" + else workflow_at("spec", pinned)) + root = moved_by_hand(project, tmp_path, {"spec": advanced}, also)["root"] + result = gate(root) + assert result.returncode == 1, result.stdout + result.stderr + assert block(result.stderr, code)[-1] == ( + " Either bump-leg.py line under spec's pin-gitlink-mismatch above " + "clears this finding too.") + lines = result.stderr.splitlines() + assert len([line for line in lines if "scripts/bump-leg.py" in line]) == 2 + + +def test_a_root_without_its_own_gate_is_told_no_line(standard, project): + """`bump-leg.py` runs the root's own `validate-pins.py` and skips an + absent one, so the standard's template copy, run over a root that has + none, cannot say what the bump's run will: the digest ends at THE + LOCKSTEP RULE, as a gitlink does there (#478).""" + root = project.resolve() + digest_moved(root) + (root / "scripts" / "validate-pins.py").unlink() + template = run_script( + standard / "templates" / "assembly-root" / "scripts" / + "validate-pins.py", "--root", str(root), cwd=root, + env={"PYTHONPATH": str(standard / "scripts")}) + assert template.returncode == 1, template.stdout + template.stderr + assert block(template.stderr, "pin-digest-mismatch")[-1].startswith( + " " + RULE) + assert "bump-leg.py" not in template.stderr, template.stderr From aee6a3ff80bda91bcf25a148b04b0b0ebd45bbda Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:49:40 +0000 Subject: [PATCH 2/3] validate-family.py: a gitlink whose path names no project says `` in both the branch and `--member` of its remedy, so the two lines no longer spell the one value two ways (#539) `member-gitlink-unnamed` for a gitlink under `members/` that is not at `members/

` could read no project from its path. Its remedy then printed `git -C switch -c family/add-` beside `family.py add ... --member /`: the branch word and the member are the one value, said as `` and as ``. `PROJECT_PLACEHOLDER` is now the one spelling, used by `_switch_line` and by the `--member` line, so the two cannot drift apart again. The test stages a gitlink at `members/deep/er` and holds both lines to ``; it fails on the previous template with `family/add-`. A pinned template, so it moves as the PR's first commit moved the two validators: the template changes here, this repository records no digest of it, and each holder takes it with `update-shape.py check` and `apply`. The PR has not landed, so a holder still re-pins once. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- .../family-root/scripts/validate-family.py | 15 +++++++++++---- tests/test_validate_family_unnamed_mounts.py | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index c7f440d8..450bac3d 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -115,6 +115,11 @@ #: and `-`, and no trailing `.lock`. Anything else is said as `member`. BRANCH_WORD_RE = re.compile(r"[a-z0-9][a-z0-9_-]*(?:\.[a-z0-9_-]+)*") +#: What a remedy says where it cannot name the member's project, a gitlink +#: whose path gives none: ONE placeholder for the one value, in the branch +#: and in `--member` alike (#539). +PROJECT_PLACEHOLDER = "" + def _switch_line(root: Path, verb: str, name: str | None) -> str: """`git -C switch -c family/-`: the branch @@ -122,11 +127,12 @@ def _switch_line(root: Path, verb: str, name: str | None) -> str: holder's tracking branch is refused (`family-on-tracking-branch`, #487), spelled as `family.py` suggests it and the holder's docs show it (#539). `name` is the member's row `id:` for `remove` and its project for - `add`, lowercased; None is the docs' own ``. POSIX-quoted, as - every other line this file prints is.""" + `add`, lowercased; None says `PROJECT_PLACEHOLDER`, the one the + `--member` beside it says. POSIX-quoted, as every other line this file + prints is.""" if name is None: return (f"git -C {shlex.quote(str(root))} switch -c " - f"family/{verb}-") + f"family/{verb}-{PROJECT_PLACEHOLDER}") word = name.lower() if not BRANCH_WORD_RE.fullmatch(word) or word.endswith(".lock"): word = "member" @@ -1105,7 +1111,8 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], continue parts = path.split("/") project = parts[1] if len(parts) == 2 and parts[1] else None - member = "" if project is None else shlex.quote(project) + member = (PROJECT_PLACEHOLDER if project is None + else shlex.quote(project)) report.finding( "member-gitlink-unnamed", f"{said} {RESTORE_ROW} If it was mounted by hand, make a branch " diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py index 09552f2e..bcce579c 100644 --- a/tests/test_validate_family_unnamed_mounts.py +++ b/tests/test_validate_family_unnamed_mounts.py @@ -660,6 +660,25 @@ def test_the_add_remedy_quotes_the_member_it_names(family, holder, project, assert shlex.split(switch)[-1] == "family/add-member", switch +def test_a_gitlink_that_names_no_project_says_one_placeholder_for_it( + family, holder): + """`members/deep/er` is two levels down, so no project is read from its + path, and the remedy cannot name one. The branch and `--member` beside + it stand for the one project, so they say it the one way, `` + (#539): `family/add-` over `--member /` was two + spellings for one value.""" + _, tip = spare_commits(family) + stage_gitlink(holder, "members/deep/er", tip) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stderr + result.stdout + (gitlink,) = findings(result) + switch, rm, add = commands(gitlink) + assert shlex.split(switch)[-1] == "family/add-", switch + assert shlex.split(add)[-1] == "/", add + assert shlex.split(rm)[-1] == "members/deep/er", rm + assert "" not in gitlink, gitlink + + # --- the remedy, run as printed, from a real mount --------------------------- def test_a_hand_mount_is_taken_out_and_added_back_as_printed(family, holder): From 0680799c7b211688ee3cf644d53916788fa5d980 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:49:45 +0000 Subject: [PATCH 3/3] tests/test_doctor_gitlink_repairs.py: the gate's output for `pin-path-mismatch`, `shape-copy-drift` and `pin-submodule-uninitialized` is held to no `bump-leg.py` in any spelling, so the old strength is kept beside the two codes that name a line (#539) The PR narrowed the old `"bump-leg.py" not in` check to a `"scripts/bump-leg.py"` comparison for every code in `OTHER_FINDINGS`. For the three codes whose findings must name no `bump-leg.py` at all, that let a mention without the `scripts/` prefix pass. The two codes that name a line (`pin-digest-mismatch`, `pin-workflow-ref-mismatch`) are still held to `scripts/bump-leg.py`; the other three are held to `bump-leg.py` not appearing, as before the PR. A mutant that adds `(bump-leg.py does not do this)` to the `shape-copy-drift` remedy, or a second line naming it to `pin-path-mismatch`, passes the narrowed check and fails this one. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_doctor_gitlink_repairs.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_doctor_gitlink_repairs.py b/tests/test_doctor_gitlink_repairs.py index 24f40260..21d91e61 100644 --- a/tests/test_doctor_gitlink_repairs.py +++ b/tests/test_doctor_gitlink_repairs.py @@ -424,8 +424,12 @@ def test_every_other_pins_row_is_the_gates_own(standard, project, code): assert "gitlink_repairs" not in row["detail"], row["detail"] assert "bump-leg" not in row["reason"], row["reason"] gate = run_script(root / "scripts" / "validate-pins.py", cwd=root) - named = "scripts/bump-leg.py" in gate.stdout + gate.stderr - assert named == (code in GATE_NAMES_THE_LINE), gate.stderr + said = gate.stdout + gate.stderr + if code in GATE_NAMES_THE_LINE: + assert "scripts/bump-leg.py" in said, gate.stderr + else: + # The old strength: not even the file's name, in any spelling. + assert "bump-leg.py" not in said, gate.stderr # --- named only where a line can commit -------------------------------------