From c2f288483c5e6693fe20945842b32635fa9f4551 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 22:59:25 +0000 Subject: [PATCH 1/3] validate-family.py and family.py: a row's gitlink that no section of HEAD's `.gitmodules` gives is `member-gitmodules-absent`, so a holder whose HEAD lost the file no longer says `family ok`, `remove` refuses it before `deinit` instead of leaving the mount empty, and the gitlink remedies name a case variant and the commit `add` needs (#611) 571(b). In a holder whose HEAD lost `.gitmodules` (the commit #407 describes), `family.py remove --dry-run` exited 0, and the real run did `git submodule deinit` and `git rm`, then died at its commit's `git --literal-pathspecs add -f -- .gitmodules family.yaml` (`pathspec '.gitmodules' did not match any files`); the rollback left `members/

` empty. `_checked_gitmodules_held` now refuses that holder, before any git command that writes and under `--dry-run` alike: no `.gitmodules` in the work tree, the index or HEAD while the index records the member's gitlink. Refused, not committed without the file, as #407 refuses a `.gitmodules` the work tree lacks. The remedy writes back, with `git config -f .gitmodules`, the section `family.py add` writes for each member whose gitlink the index records, then `git add -f .gitmodules`; `remove` then runs as it always has. 571(c), #407's decision 3. `validate-family.py` reads HEAD's `.gitmodules` (`_head_gitmodules`, HEAD's root tree listed with no pathspec) and makes `member-gitmodules-absent` for each row at `members/` whose gitlink no section gives, in every mode, `--no-members` included. Its two lines write the section back as `family.py add` writes one, keeping the url of a section of that name that has one. A member it names that is not checked out is not refused `member-uninitialized`: that refusal named a `git submodule update` that exits 0 there and dropped the finding, which is what a fresh clone and CI met. It reads HEAD, not the work tree, so a deletion nobody committed, an unstaged edit and a sparse checkout that leaves the file out are no finding, and a holder with no members is asked nothing. 574/2 (#363's item 3). A row whose gitlink HEAD records only at a case variant of its path (`Members/

`) is refused `member-gitlink-absent` naming each variant, and the move that puts it back in the index alone: `git rm -r --cached` of the variants and `git update-index --add --cacheinfo 160000,,members/

`, then a commit. `family.py add`, the plain refusal's remedy, cannot help there: the row exists. 574/3. A hand mount's `member-gitlink-unnamed` remedy says to commit the `git rm` before the `add`: since #574, `add` refuses a path under another spelling of `members/` that HEAD holds and the index no longer does. `templates/family-root/scripts/validate-family.py` is a pinned copy: each holder sees it as `upstream-changed` on its next `update-shape.py check`. Tests: tests/test_family_head_lost_gitmodules.py (new, 24); the pinned remedy text and the case-variant states in tests/test_validate_family_unnamed_mounts.py; a docstring in tests/test_family_remove_gitmodules.py. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 73 ++- .../family-root/scripts/validate-family.py | 214 ++++++- tests/test_family_head_lost_gitmodules.py | 596 ++++++++++++++++++ tests/test_family_remove_gitmodules.py | 12 +- tests/test_validate_family_unnamed_mounts.py | 34 +- 5 files changed, 902 insertions(+), 27 deletions(-) create mode 100644 tests/test_family_head_lost_gitmodules.py diff --git a/scripts/family.py b/scripts/family.py index a3eb0e08..85b2f87a 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -5209,7 +5209,8 @@ def _checked_gitmodules_as_staged(root: Path, verb: str) -> None: its line endings is refused here as `git rm` refuses it, and the refusal says so where `git add` would stage the index's own blob from it. With neither the index, HEAD nor the work tree holding a `.gitmodules`, - nothing is asked. + nothing is asked here; `remove` then asks `_checked_gitmodules_held` + (#611). `add` LEAVES A MISSING FILE TO GIT. `git submodule add` refuses to write a `.gitmodules` the work tree lacks while the index or HEAD holds one, @@ -5403,6 +5404,73 @@ def _checked_one_section(root: Path, project: str, path: str) -> None: f"{GITMODULES}` — and run the same command again.") +def _checked_gitmodules_held(family: Family, path: str) -> None: + """`member-gitmodules-absent`: `remove` refuses, before any git command + that writes and under `--dry-run` alike, a holder that holds no + `.gitmodules` at all, in the work tree, the index or HEAD, while its + index records the member's gitlink (#611). + + A HOLDER WHOSE HEAD LOST `.gitmodules`, as #407's commit left one. `git + rm` edits only a `.gitmodules` the work tree has, and `remove` commits + the file as `git rm` leaves it, so with none anywhere the commit's `git + add` died (`pathspec '.gitmodules' did not match any files`) after + `deinit` and `git rm` had emptied the member, and the rollback left its + directory empty; the dry run exited 0. `_checked_gitmodules_as_staged` + asks nothing here, since no copy exists to compare. + + REFUSED, NOT COMMITTED WITHOUT IT, as #407 refuses a `.gitmodules` the + work tree lacks: the holder is left byte for byte as it was. A clone + checks out none of its members, and the holder's own + `validate-family.py` finds `member-gitmodules-absent` for each, so the + remedy writes back, as `family.py add` writes one, the section of every + member whose gitlink the index records, this one included, which + `remove` then takes out as it always has. Staged, the file goes into + `remove`'s commit, as #407's untracked remedy has it, unless the person + commits it on its own first. + """ + root = family.root + if os.path.lexists(root / GITMODULES): + return + records = _index_records(root) + if GITMODULES.encode() in records or _head_oid(root, GITMODULES): + return + gitlinks = {name.decode("utf-8", "surrogateescape") + for name, held in records.items() + if any(record.startswith(b"160000 ") + and record.partition(b"\t")[0].endswith(b" 0") + for record in held)} + if path not in gitlinks: + return + sections = [] + for row in family.members: + project = str(row.get("project") or "") + member = f"{MEMBERS_DIR}/{project}" + if not project or member not in gitlinks: + continue + url = _member_url(str(row.get("repository") or ""), None) + sections += [f"`git config -f {GITMODULES} " + f"{shlex.quote(f'submodule.{member}.{key}')} " + f"{shlex.quote(value)}`" + for key, value in (("path", member), ("url", url))] + raise Refusal( + "member-gitmodules-absent", + f"the holder has no {GITMODULES}: neither the work tree, the index " + f"nor HEAD holds one, though the index records a gitlink at {path}. " + f"`git rm` edits only a {GITMODULES} the work tree has, and `remove` " + f"commits {GITMODULES} as `git rm` leaves it, so with none to commit " + f"it would fail after `git submodule deinit` and `git rm` had emptied " + f"{path}. A recursive clone of the holder checks out none of its " + "members, and its validate-family.py finds member-gitmodules-absent " + "for each. No git command that writes was run and nothing was " + "written.", + "Remediation: write back the section `family.py add` writes for " + "each member the index records, this one included, in a POSIX shell " + f"— {' and '.join(sections)} — then stage it, `git add -f " + f"{GITMODULES}` (`-f`, since an operator's excludes may match it), " + "and commit it on its own first if the command's commit should " + "carry only its own change, and run the same command again.") + + def _checked_index_unlocked(root: Path, path: str) -> None: """`family-index-locked`: the real `remove` refuses a held `index.lock` BEFORE it unmounts anything (#365). @@ -5591,6 +5659,9 @@ def cmd_remove(args) -> int: # carry a change nobody staged. Where the index holds a copy, every # reading of the work tree's file after this one reads that copy. _checked_gitmodules_as_staged(family.root, "remove") + # A holder that holds no `.gitmodules` anywhere, as one whose HEAD lost + # it does, is refused next (#611): the commit could not stage one. + _checked_gitmodules_held(family, path) # A `.gitmodules` that gives the path to a second submodule is refused # here, before any git command that writes (#360), so the dry run makes # the same refusal, as it makes every one above. diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 450bac3d..e4c2200c 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -40,7 +40,9 @@ project the row claims. A row whose `path:` is not `members/` is the `member-path` finding and none of the three is asked of it: nothing read through a - path the row may not have is a fact about the member (#367). + path the row may not have is a fact about the member (#367). A row + whose gitlink is recorded only at a case variant of its path is refused + naming that variant, and the index move that puts it back (#611). And, across the rows, that no two record ONE `id`: that is one project pinned twice, however each row spelled its repository (#206). This one reads `family.yaml` alone, so `--no-members` asks it too. @@ -57,6 +59,13 @@ asks them too. A path a row names in another case is still one of these, and what it says to do depends on which of the two is the member's own: it names no command that would un-register the member. + - that a section of HEAD's `.gitmodules` gives the path of every row's + gitlink (#611, #407's decision 3). A recursive clone reads HEAD's file, + so a gitlink no section of it gives is left empty there, by CI and by + `make bootstrap`, though the holder it was committed in keeps its + checkout. It needs no member checked out, so `--no-members` asks it + too, and a member it names that is not checked out is not then refused + as uninitialized. - the COPIED shape files, each against its `sha256` row in `contracts/shape-pin.yaml`. Editing a copy in place is drift and the exit is to carry it upstream, never to update the digest. @@ -381,7 +390,7 @@ def _pin_commit(pin: dict, project: str, report: Report) -> str: def _check_member_gitlink(root: Path, path: str, commit: str, project: str, - report: Report) -> None: + gitlinks: dict[str, str], report: Report) -> None: """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 @@ -389,9 +398,15 @@ def _check_member_gitlink(root: Path, path: str, commit: str, project: str, `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.""" + line runs from. Where `gitlinks` (`_member_gitlinks`) holds one at a + case variant of `path`, the refusal names that instead (#611).""" gitlink = recorded_gitlink(root, path) if gitlink is None: + variants = sorted(other for other in gitlinks if other != path + and other.casefold() == path.casefold()) + if variants: + raise _gitlink_in_another_case(path, commit, project, variants, + gitlinks) raise Refusal( "member-gitlink-absent", f"no gitlink (mode 160000) recorded at {path!r}. {project} is " @@ -414,6 +429,45 @@ def _check_member_gitlink(root: Path, path: str, commit: str, project: str, report.note(f"{path}: gitlink == pin {commit[:12]}") +def _gitlink_in_another_case(path: str, commit: str, project: str, + variants: list[str], + gitlinks: dict[str, str]) -> Refusal: + """`member-gitlink-absent` for a row whose gitlink this repository + records only at a case variant of its path (#611, #363's item 3), as a + holder committed in #363's state does: `Members/

`, under a row and a + `.gitmodules` section that say `members/

`. A fresh clone checks out + nothing at the row's path, and the plain refusal's `family.py add` + cannot help: the row is there already. + + EACH VARIANT IS NAMED, AND THE GITLINK IS MOVED IN THE INDEX ALONE, as + #574's remedies act on an index entry under another spelling of + `members/` (Brett Heap, 2026-10-14, "Refuse on every disk"): `git rm -r + --cached` takes each variant out and touches no file, and `git + update-index --cacheinfo` records the gitlink at the row's path, at the + commit the row pins, reading no disk either. So the two lines work on + every disk, wherever the checkout stands, and the commit after them + records the member once, at its own path. `member-gitlink-unnamed` + prints no command for the same gitlink, since taking it out ALONE would + un-register the member; these take it out and record it together. + """ + found = ", ".join(f"{variant} (at {gitlinks[variant]})" + for variant in variants) + taken_out = " ".join(shlex.quote(variant) for variant in variants) + return Refusal( + "member-gitlink-absent", + f"no gitlink (mode 160000) recorded at {path!r}. {project} is named " + f"in {MANIFEST}, and this repository records its gitlink only at a " + "case variant of that path, which macOS and Windows hold as one " + f"directory with it: {found}. A recursive clone checks out nothing " + f"at {path}.", + "Remediation: the member is recorded in the wrong case. Move its " + "gitlink to the row's path, at the commit the row pins, in the index " + "alone, which touches no file, and commit that:\n" + f" git rm -r --cached -- {taken_out}\n" + " git update-index --add --cacheinfo " + f"{shlex.quote(f'160000,{commit},{path}')}") + + def _check_member_tree(member: Path, pin: dict, path: str, commit: str, project: str, report: Report) -> None: """Is `member`'s own object store readable at `commit`, and does its @@ -467,7 +521,8 @@ def _check_member_tree(member: Path, pin: dict, path: str, commit: str, report.note(f"{path}: tree digest recomputes ({recorded[:12]}…)") -def _check_member(root: Path, row: dict, seen: dict, report: Report) -> None: +def _check_member(root: Path, row: dict, seen: dict, report: Report, + gitlinks: dict[str, str], unregistered: set[str]) -> None: """One member: the row itself, the gitlink, the digest, the identity. NOTHING IS READ THROUGH A PATH THE ROW MAY NOT HAVE (#367). A hand-edited @@ -482,6 +537,14 @@ def _check_member(root: Path, row: dict, seen: dict, report: Report) -> None: The git each check runs is run inside `_git_runs`, which names the check when there is no git to run (#368); the identity check reads a file and runs none. + + A MEMBER NO SECTION OF HEAD'S `.gitmodules` GIVES (`unregistered`, the + `member-gitmodules-absent` finding, #611) that is not checked out stops + after its gitlink. Refused `member-uninitialized`, it named `git + submodule update --init`, which exits 0 there and checks nothing out, + and the refusal dropped the finding that says why: the state a fresh + clone of a holder whose HEAD lost `.gitmodules` is in, and CI's. The + finding is made all the same, so the run still fails. """ project = _member_project_name(row, seen, report) if project is None: @@ -500,10 +563,12 @@ def _check_member(root: Path, row: dict, seen: dict, report: Report) -> None: # ---- the gitlink ------------------------------------------------------- with _git_runs(f"the gitlink recorded at {path} could not be read"): - _check_member_gitlink(root, path, commit, project, report) + _check_member_gitlink(root, path, commit, project, gitlinks, report) # ---- the digest and the identity -------------------------------------- member = root / path + if path in unregistered and not (member / ".git").exists(): + return with _git_runs(f"{project}'s pinned commit and its tree digest could not " f"be read in {path}"): _check_member_tree(member, pin, path, commit, project, report) @@ -822,6 +887,13 @@ def _gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: "holder's submodules can be cloned from it as it stands. Mend " f"the file by hand (`git config -f {GITMODULES} --list` says " "where it breaks), then run this again.") from exc + return _sections_listed(listed) + + +def _sections_listed(listed: bytes) -> dict[str, dict[str, str]]: + """`{name: {key: value}}` of every `submodule` section in what `git + config -z --list` printed, each key in the order git lists it: one + reading for the work tree's `.gitmodules` and HEAD's (#611).""" sections: dict[str, dict[str, str]] = {} for record in listed.split(b"\0"): key, _, value = record.decode("utf-8", @@ -833,6 +905,48 @@ def _gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: return sections +def _head_gitmodules(root: Path + ) -> tuple[bool, dict[str, dict[str, str]]] | None: + """`(whether HEAD holds a .gitmodules, its sections)`, read as + `_gitmodules_sections` reads the work tree's, or None for a HEAD git + cannot list, a holder before its first commit, which a clone cannot + take either (#611). + + HEAD'S, NOT THE WORK TREE'S: a recursive clone of this holder, and CI's + checkout, read the file HEAD holds. A work tree's copy nobody has + committed is no fact about what a clone gets, and neither is one a + sparse checkout leaves out of the work tree or a deletion nobody has + committed. HEAD's ROOT TREE is listed and the entry picked by name, with + no pathspec, as `_member_gitlinks` lists HEAD. One git cannot read is + `gitmodules-unreadable`, as the work tree's is. + """ + with _git_runs(f"HEAD's {GITMODULES} could not be read as git reads it"): + try: + tree = git_out(["ls-tree", "-z", "HEAD"], cwd=root, binary=True) + except Refusal: + return None + blob = next((meta.split()[2].decode("ascii") + for meta, _, name in (record.partition(b"\t") + for record in tree.split(b"\0")) + if name == GITMODULES.encode() + and meta.split()[1:2] == [b"blob"]), None) + if blob is None: + return False, {} + try: + listed = git_out(["config", "-z", "--blob", blob, "--list"], + cwd=root, binary=True) + except Refusal as exc: + raise Refusal( + "gitmodules-unreadable", + f"HEAD's {GITMODULES} could not be read as git reads it: " + f"{exc.detail}", + "Remediation: git cannot read it either, so none of this " + "holder's submodules can be cloned from it. Mend the file by " + f"hand (`git config -f {GITMODULES} --list` says where it " + "breaks), commit it, then run this again.") from exc + return True, _sections_listed(listed) + + def _check_gitmodules_paths(sections: dict[str, dict[str, str]], report: Report) -> None: """No two sections of `.gitmodules` give ONE path (#300). @@ -1079,7 +1193,10 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], which refuses while its checkout holds changes, then mounted with `family.py add`: `add` cannot adopt an existing mount, and after `git rm --cached` alone it refuses the checkout left in the way - (`member-path-taken`). `add` then names the object store `git rm` leaves + (`member-path-taken`). The removal is committed between the two (#611): + `add` refuses a path under another spelling of `members/` that HEAD + holds and the index no longer does (#574), which an uncommitted `git rm + -- Members/

` leaves. `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 @@ -1119,10 +1236,13 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], "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"{GITMODULES} section out too, and commit that removal, since " + "family.py add refuses a path under another spelling of members/ " + "that HEAD holds and the index no longer does, 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 " @@ -1201,6 +1321,73 @@ def _check_gitmodules_named(sections: dict[str, dict[str, str]], f"of them, then commit {GITMODULES}:{removal}") +def _check_gitlinks_registered(root: Path, members: list, + gitlinks: dict[str, str], + report: Report) -> set[str]: + """Every row's gitlink is at a path a section of HEAD's `.gitmodules` + gives — the `member-gitmodules-absent` finding (#611, #407's decision + 3). Returns the paths it was made for, of which `_check_member` asks no + checkout that is not there. + + A recursive clone checks a gitlink out only where a section gives its + path, so a gitlink with none is left empty in every clone, by CI and by + `make bootstrap` there, whose `git submodule update` exits 0 and checks + nothing out. The holder the commit was made in keeps its checkout, and + its git config the registration, so every other check here passed it: + `family.py remove` committed a `.gitmodules` the work tree had lost + (#407) and the gate said `family ok`. + + HEAD's file (`_head_gitmodules`), and the rows that `_check_member` + reads at their path: one whose `path:` is not `members/` is + `member-path`'s, and one with no gitlink there is + `member-gitlink-absent`'s (`gitlinks`, read as `recorded_gitlink` reads + one). A holder with no members holds no gitlink and is asked nothing. + + The remedy writes the section as `family.py add` writes it, named after + its path, with the url it gives a member, `https://github.com/` and the + row's `repository:`, or `` where that is not `/`. A + section of that name that has a url keeps it: only its path is written. + """ + head = _head_gitmodules(root) + if head is None: + return set() + held, sections = head + given = {keys["path"] for keys in sections.values() if "path" in keys} + unregistered: set[str] = set() + for row in members: + project = row.get("project") if isinstance(row, dict) else None + if not isinstance(project, str) or not project: + continue + path = f"{DEFAULT_MEMBERS_DIR}/{project}" + if (str(row.get("path") or path) != path or path in unregistered + or path not in gitlinks or path in given): + continue + unregistered.add(path) + repository = row.get("repository") + keys = {"path": shlex.quote(path)} + if "url" not in sections.get(path, {}): + keys["url"] = ( + shlex.quote(f"https://github.com/{repository}.git") + if isinstance(repository, str) + and QUALIFIED_RE.match(repository) else "") + lines = "".join( + f"\n git config -f {GITMODULES} " + f"{shlex.quote(f'submodule.{path}.{key}')} {value}" + for key, value in keys.items()) + report.finding( + "member-gitmodules-absent", + f"{path} is a gitlink at {gitlinks[path]} that a row of members: " + "names, and " + + (f"no section of HEAD's {GITMODULES} gives that path" if held + else f"HEAD holds no {GITMODULES}") + + ", so a recursive clone of this holder checks out nothing " + "there, and neither does make bootstrap in that clone, whatever " + "checkout this holder has of it. Write its section back by hand, " + "in a POSIX shell, as family.py add writes one, then stage and " + f"commit {GITMODULES}:{lines}") + return unregistered + + def _check_members(root: Path, members: list, no_members: bool, report: Report) -> None: """Every row in `members:`, or the `--no-members` skip line. @@ -1209,7 +1396,8 @@ def _check_members(root: Path, members: list, no_members: bool, out comes first, so `--no-members` asks them too: the ones that read a file alone, `family.yaml`'s names (#331) and ids (#206) and the paths `.gitmodules` gives twice (#300), then the gitlinks and sections no row - names, which read the index and HEAD as well (#361). + names, which read the index and HEAD as well (#361), and the gitlinks + no section of HEAD's `.gitmodules` gives (#611). """ _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) @@ -1219,6 +1407,8 @@ def _check_members(root: Path, members: list, no_members: bool, gitlinks = _member_gitlinks(root) _check_gitlinks_named(root, gitlinks, named, report) _check_gitmodules_named(sections, gitlinks, named, report) + unregistered = _check_gitlinks_registered(root, members, gitlinks, + report) if no_members: print(f" -- {len(members)} member(s) SKIPPED (--no-members): " "their gitlinks, digests and project ids are not checked " @@ -1230,7 +1420,7 @@ def _check_members(root: Path, members: list, no_members: bool, report.finding("member-row", f"a member is not a mapping: {row!r}") continue - _check_member(root, row, seen, report) + _check_member(root, row, seen, report, gitlinks, unregistered) if not members: report.note("no members yet: a family with none is empty, not " "wrong") diff --git a/tests/test_family_head_lost_gitmodules.py b/tests/test_family_head_lost_gitmodules.py new file mode 100644 index 00000000..159082b9 --- /dev/null +++ b/tests/test_family_head_lost_gitmodules.py @@ -0,0 +1,596 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A holder whose HEAD lost `.gitmodules`, and the gate's gitlink remedies +(#611). + +THE ISSUE'S FOUR LINES, each reproduced on main before this change: + +- 571(b). In a holder whose HEAD already lost `.gitmodules` (the commit #407 + describes), `family.py remove --dry-run` exited 0, and the real run did + `git submodule deinit` and `git rm`, then died at its commit's `git + --literal-pathspecs add -f -- .gitmodules family.yaml` (`pathspec + '.gitmodules' did not match any files`). Its rollback put HEAD and the + index back and left the member's directory EMPTY. `remove` now refuses + that holder before any git command that writes, `member-gitmodules-absent`, + under `--dry-run` alike, and its remedy writes back each member's section. +- 571(c). `validate-family.py` passed a `members:` row whose gitlink no + section of HEAD's `.gitmodules` gives (#407's decision 3): `family ok` in + the holder, `--no-members` too, while a fresh recursive clone left every + such member empty and its gate refused `member-uninitialized`, naming a `git + submodule update` that exits 0 and checks nothing out. It is the finding + `member-gitmodules-absent` now, in every mode, read from HEAD, and a member + it names that is not checked out is not refused as uninitialized. +- 574/2. `member-gitlink-absent` said nothing of a gitlink HEAD holds at a + case variant of the row's path (#363's item 3), and named `family.py add`, + which cannot help: the row is there. It names the variant now, and the + index move that records the member at its own path. +- 574/3. A hand mount's `member-gitlink-unnamed` remedy said `git rm`, then + `family.py add`. `add` refuses a path under another spelling of `members/` + that HEAD holds and the index no longer does (#574), so it says to commit + the removal between them now. + +THE SAME FAMILY, OFFLINE RULES AND READINGS as +`test_family_remove_gitmodules.py` (#407), whose helpers are imported. Every +remedy is typed as printed: split as a POSIX shell splits it, run with no +shell. The url a written-back section gives is the one `family.py add` +writes, `https://github.com//.git`, and a clone reaches the +fixture's local remotes through it by a `url..insteadOf` for the one +command, as `clone_just_pushed` reaches the legs. The case-variant states are +index records and commits (`stage_gitlink`), never two directories whose +names differ only in case, and each is asked again with +`core.ignorecase=true`, how macOS and Windows clone, so every one runs on all +three systems. +""" + +from __future__ import annotations + +import re +import shlex +from pathlib import Path + +import pytest + +from conftest import clone_just_pushed, run_script +from repo_shape import PYTHON +#: The family fixture, its names, the offline environment and the readings +#: of a holder, imported from the modules that own them rather than rebuilt. +from test_family import ( # noqa: F401 + FAMILY, NAME, ORG, SPARE, family, validate, +) +from test_family_add_excludes import empty_holder # noqa: F401 +from test_family_add_refusals import holder, offline # noqa: F401 +from test_family_dry_run import isolated +from test_family_member_path import MEMBER, OTHER, raw +from test_family_remove_gitmodules import ( + deleted, edited, head_paths, left_out, ran, refused_alike, + typed_as_printed, verb_args, +) +from test_scaffold_ignored_shape_files import excluding +from test_validate_family_unnamed_mounts import ( + commands, findings, spare_commits, stage_gitlink, +) + +GITMODULES = ".gitmodules" +MODES = pytest.mark.parametrize( + "extra", [(), ("--pins",), ("--no-members",)], + ids=["members-checked", "pins", "no-members"]) + +#: The two disks: git's own reading of a case-sensitive one, and how macOS +#: and Windows clone (`core.ignorecase=true`), set for every git command the +#: test types. +DISKS = pytest.mark.parametrize("ignorecase", ["false", "true"], + ids=["linux", "folding"]) + + +def on_disk(env: dict, ignorecase: str) -> dict: + """`env` with `core.ignorecase` set to `ignorecase`, one more + `GIT_CONFIG_*` entry after the ones it already holds.""" + index = int(env.get("GIT_CONFIG_COUNT", "0")) + return {**env, "GIT_CONFIG_COUNT": str(index + 1), + f"GIT_CONFIG_KEY_{index}": "core.ignorecase", + f"GIT_CONFIG_VALUE_{index}": ignorecase} + + +def url(project: str) -> str: + """The url `family.py add` writes for a member of the fixture's org.""" + return f"https://github.com/{ORG}/{project}.git" + + +#: What `remove` says in a holder that holds no `.gitmodules` anywhere. +REMOVE_REFUSED = ( + "REFUSED member-gitmodules-absent: the holder has no .gitmodules: neither " + "the work tree, the index nor HEAD holds one, though the index records a " + "gitlink at members/{member}. `git rm` edits only a .gitmodules the work " + "tree has, and `remove` commits .gitmodules as `git rm` leaves it, so " + "with none to commit it would fail after `git submodule deinit` and `git " + "rm` had emptied members/{member}. A recursive clone of the holder checks " + "out none of its members, and its validate-family.py finds " + "member-gitmodules-absent for each. No git command that writes was run " + "and nothing was written.") + + +def remove_remedy(*projects: str) -> str: + """`remove`'s remediation, writing back the section of each of + `projects`, in that order.""" + written = " and ".join( + f"`git config -f .gitmodules submodule.members/{project}.{key} " + f"{value}`" for project in projects + for key, value in (("path", f"members/{project}"), + ("url", url(project)))) + return ("Remediation: write back the section `family.py add` writes for " + "each member the index records, this one included, in a POSIX " + f"shell — {written} — then stage it, `git add -f .gitmodules` " + "(`-f`, since an operator's excludes may match it), and commit it " + "on its own first if the command's commit should carry only its " + "own change, and run the same command again.") + + +def absent(project: str, commit: str, *, held: bool, + with_url: bool = True) -> str: + """The `member-gitmodules-absent` finding for `project`'s gitlink at + `commit`, WHOLE: its line and the commands under it. `held` is whether + HEAD holds a `.gitmodules` at all; `with_url` whether the section to + write has no url of its own.""" + path = f"members/{project}" + lines = f"\n git config -f .gitmodules submodule.{path}.path {path}" + if with_url: + lines += (f"\n git config -f .gitmodules submodule.{path}.url " + f"{url(project)}") + return (f"FINDING member-gitmodules-absent: {path} is a gitlink at " + f"{commit} that a row of members: names, and " + + ("no section of HEAD's .gitmodules gives that path" if held + else "HEAD holds no .gitmodules") + + ", so a recursive clone of this holder checks out nothing " + "there, and neither does make bootstrap in that clone, whatever " + "checkout this holder has of it. Write its section back by hand, " + "in a POSIX shell, as family.py add writes one, then stage and " + f"commit .gitmodules:{lines}") + + +# --- the states -------------------------------------------------------------- + +def commit_all(root: Path, env: dict, message: str) -> None: + raw(root, env, "commit", "-q", "-m", message) + + +def head_lost(root: Path, env: dict) -> None: + """THE ISSUE'S STATE: `.gitmodules` deleted and the deletion committed, + as the commit #407 describes left it. The registrations stay in the + holder's git config, and its checkouts on disk.""" + raw(root, env, "rm", "-q", "--", GITMODULES) + commit_all(root, env, "The commit #407 describes") + + +def committed_gitmodules(root: Path, env: dict, data: bytes) -> None: + """`data` committed as `.gitmodules`, hashed into git's objects as + BYTES, and the same bytes on disk, so no platform rewrites a line + ending.""" + blob = raw(root, env, "hash-object", "-w", "--stdin", + stdin=data).stdout.decode("ascii").strip() + raw(root, env, "update-index", "--cacheinfo", + f"100644,{blob},{GITMODULES}") + commit_all(root, env, "A .gitmodules edited by hand") + (root / GITMODULES).write_bytes(data) + + +def without_section(root: Path, env: dict, project: str) -> None: + """HEAD's `.gitmodules` with `project`'s section taken out by hand and + committed, the other member's left as `family.py add` wrote it.""" + data = raw(root, env, "show", f"HEAD:{GITMODULES}").stdout + section = re.compile(rb'\[submodule "members/' + project.encode() + + rb'"\]\n(?:\t[^\n]*\n)*') + assert len(section.findall(data)) == 1, data + committed_gitmodules(root, env, section.sub(b"", data)) + + +def pinned(root: Path, project: str) -> str: + """The commit `family.yaml` pins `project` at, as git records its + gitlink after `family.py add`.""" + return raw(root, {}, "rev-parse", f"HEAD:members/{project}").stdout.decode( + "ascii").strip() + + +def typed(root: Path, env: dict, finding: str) -> None: + """Every command `finding` prints, typed as printed.""" + for command in commands(finding): + argv = shlex.split(command) + assert argv[0] == "git", command + raw(root, env, *argv[1:]) + + +def staged_and_committed(root: Path, env: dict, message: str) -> None: + """What a finding's "then stage and commit .gitmodules" says.""" + raw(root, env, "add", "--", GITMODULES) + commit_all(root, env, message) + + +def assert_passes_in_every_mode(root: Path, members: int) -> None: + for extra in ((), ("--pins",), ("--no-members",)): + result = validate(root, *extra) + assert result.returncode == 0, (extra, result.stderr + result.stdout) + assert (f"family ok: {NAME} (inkrouter), {members} member(s)" + in result.stdout), extra + + +def github_to(remotes: Path) -> list[str]: + """The `-c` that makes a clone read `https://github.com//` from the + fixture's local remotes, for the one command.""" + uri = remotes.resolve().as_uri().rstrip("/") + "/" + return ["-c", f"url.{uri}.insteadOf=https://github.com/{ORG}/"] + + +def pushed_and_cloned(root: Path, env: dict, family, tmp_path: Path, + branch: str) -> Path: + """HEAD pushed to `branch` of the family's own remote, then a fresh + `clone --recurse-submodules` of it, which must exit 0.""" + remote = family["remotes"] / f"{NAME}.git" + raw(root, env, "push", "-q", str(remote), f"HEAD:refs/heads/{branch}") + clone = tmp_path / f"clone-{branch.replace('/', '-')}" + cloned = clone_just_pushed(remote, clone, branch, + config=github_to(family["remotes"])) + assert cloned.returncode == 0, cloned.stdout + cloned.stderr + return clone + + +# --- 571(b): remove in a holder whose HEAD lost .gitmodules ------------------ + +@pytest.mark.parametrize("rules", [b"", b"*\n"], ids=["no-excludes", "star"]) +def test_remove_refuses_a_holder_whose_head_lost_gitmodules( + family, holder, offline, tmp_path, rules): + """`member-gitmodules-absent`, by `remove` and its dry run alike, exit 2, + with the holder byte for byte as it was, MEMBER's checkout included: + the real run used to empty it and fail at its commit. Typed as printed, + under an operator whose excludes hold `*` too (hence the `-f`), the + remedy writes both members' sections back and stages them; `remove` + then commits MEMBER's removal with OTHER's section back in HEAD, and + the gate passes.""" + env = {**isolated(offline, tmp_path), **excluding(tmp_path / "op", rules)} + head_lost(holder, env) + args = verb_args("remove", holder, family) + + said, remediation = refused_alike(holder, env, args) + + assert said == REMOVE_REFUSED.format(member=MEMBER) + assert remediation == remove_remedy(MEMBER, OTHER) + assert (holder / "members" / MEMBER / "project.yaml").is_file() + assert typed_as_printed(holder, env, remediation)[-1] == ( + "git add -f .gitmodules") + ran(holder, env, args) + assert head_paths(holder, env) == [f"members/{OTHER}"] + assert_passes_in_every_mode(holder, 1) + + +def test_the_holder_remove_leaves_clones_with_the_other_member( + family, holder, offline, tmp_path): + """THE ISSUE'S CLONE. After the remedy and the `remove`, a push and a + fresh `clone --recurse-submodules` check OTHER out, from the url the + written-back section gives, and the clone's own gate passes, where + before the change a clone of this holder checked out no member at all.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + args = verb_args("remove", holder, family) + typed_as_printed(holder, env, refused_alike(holder, env, args)[1]) + ran(holder, env, args) + + clone = pushed_and_cloned(holder, env, family, tmp_path, "w611/removed") + + assert (clone / "members" / OTHER / "project.yaml").is_file() + assert not (clone / "members" / MEMBER).exists() + gate = validate(clone) + assert gate.returncode == 0, gate.stdout + gate.stderr + + +def test_remove_of_a_member_whose_section_alone_is_gone_runs_as_before( + family, holder, offline, tmp_path): + """The refusal is for a holder with NO `.gitmodules`, where the commit + cannot stage one. Where HEAD's file lacks only the member's own section, + `git rm` finds none to take out and `remove` commits the gitlink and the + row, as it did before: the holder it leaves passes the gate.""" + env = isolated(offline, tmp_path) + without_section(holder, env, MEMBER) + + removed = run_script(FAMILY, *verb_args("remove", holder, family), + env=env) + + assert removed.returncode == 0, removed.stdout + removed.stderr + assert head_paths(holder, env) == [f"members/{OTHER}"] + assert_passes_in_every_mode(holder, 1) + + +# --- 571(c): a gitlink no section of HEAD's .gitmodules gives --------------- + +@MODES +def test_a_head_that_lost_gitmodules_is_a_finding_per_member(holder, offline, + tmp_path, extra): + """THE ISSUE'S GATE: `family ok` before the change, in every mode. One + finding per member, each naming its gitlink and the two lines that write + its section back, in every mode, since none needs a member checked out. + Typed as printed, staged and committed, the holder passes again.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + + result = validate(holder, *extra) + + assert result.returncode == 1, result.stderr + result.stdout + assert "REFUSED" not in result.stderr, result.stderr + assert findings(result) == [ + absent(project, pinned(holder, project), held=False) + for project in (MEMBER, OTHER)] + for finding in findings(result): + typed(holder, env, finding) + staged_and_committed(holder, env, "Write the sections back as printed") + assert_passes_in_every_mode(holder, 2) + + +@MODES +def test_a_section_head_lost_alone_is_one_finding(holder, offline, tmp_path, + extra): + """One member's section taken out of HEAD's `.gitmodules` by hand: a + recursive clone checks that member out nowhere, though the file is + there. One finding, for it alone, and the other member is asked + nothing.""" + env = isolated(offline, tmp_path) + without_section(holder, env, OTHER) + + result = validate(holder, *extra) + + assert result.returncode == 1, result.stderr + result.stdout + (finding,) = findings(result) + assert finding == absent(OTHER, pinned(holder, OTHER), held=True) + typed(holder, env, finding) + staged_and_committed(holder, env, "Write the section back as printed") + assert_passes_in_every_mode(holder, 2) + + +def test_a_section_of_the_name_add_gives_keeps_its_url(holder, offline, + tmp_path): + """A section named `members/

`, as `family.py add` names one, whose + path is another case (`members/irss`): no section gives OTHER's path, + so a clone checks it out nowhere. The section's url is kept, and only + its path is written, which clears this finding and the section's own + `gitmodules-path-unnamed` beside it.""" + env = isolated(offline, tmp_path) + data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout + old = f"\tpath = members/{OTHER}\n".encode() + assert data.count(old) == 1, data + committed_gitmodules(holder, env, data.replace( + old, f"\tpath = members/{OTHER.lower()}\n".encode())) + + result = validate(holder) + + assert result.returncode == 1, result.stderr + result.stdout + section, finding = findings(result) + assert section.startswith("FINDING gitmodules-path-unnamed: "), section + assert finding == absent(OTHER, pinned(holder, OTHER), held=True, + with_url=False) + typed(holder, env, finding) + staged_and_committed(holder, env, "Write the path back as printed") + assert_passes_in_every_mode(holder, 2) + + +def test_a_fresh_clone_of_it_says_the_finding_and_not_uninitialized( + family, holder, offline, tmp_path): + """BOOTSTRAP, VALIDATE AND A FRESH RECURSIVE CLONE AGREE. Pushed, the + holder whose HEAD lost `.gitmodules` clones with exit 0 and both + members empty; `make bootstrap`'s script there checks neither out, and + says so for each; and the clone's gate names both, members checked or + not, where before the change it refused `member-uninitialized`, whose + `git submodule update` exits 0 there. With the sections written back + as printed and pushed, a fresh clone checks both out and its gate + passes.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + clone = pushed_and_cloned(holder, env, family, tmp_path, "w611/lost") + expected = [absent(project, pinned(holder, project), held=False) + for project in (MEMBER, OTHER)] + + assert [path.name for path in (clone / "members").iterdir() + if any(path.iterdir())] == [] + booted = run_script(clone / "scripts" / "bootstrap.py", cwd=clone) + for project in (MEMBER, OTHER): + assert f" [{project}] {project}: NOT CHECKED OUT; skipped." in ( + booted.stdout), booted.stdout + for extra in ((), ("--no-members",)): + gate = validate(clone, *extra) + assert gate.returncode == 1, (extra, gate.stderr + gate.stdout) + assert "member-uninitialized" not in gate.stderr, gate.stderr + assert findings(gate) == expected, extra + + for finding in expected: + typed(holder, env, finding) + staged_and_committed(holder, env, "Write the sections back as printed") + mended = pushed_and_cloned(holder, env, family, tmp_path, "w611/mended") + for project in (MEMBER, OTHER): + assert (mended / "members" / project / "project.yaml").is_file() + assert_passes_in_every_mode(mended, 2) + + +#: `(set-up, what it is)`: the work tree's `.gitmodules` is not HEAD's, and +#: HEAD's gives every member's path, so a clone checks each out. +NOT_HEADS = { + "deleted, not committed": deleted, + "edited, not staged": edited, + "left out by a sparse checkout": left_out, +} + + +@pytest.mark.parametrize("set_up", NOT_HEADS.values(), ids=NOT_HEADS) +def test_a_work_tree_copy_that_is_not_heads_is_no_finding(holder, offline, + tmp_path, set_up): + """No misfire: the finding reads HEAD's file, which a clone reads, and + never the work tree's. A deletion nobody committed, an edit nobody + staged and a sparse checkout that leaves the file out are each no fact + about what a clone gets.""" + env = isolated(offline, tmp_path) + set_up(holder, env) + + for extra in ((), ("--no-members",)): + result = validate(holder, *extra) + assert result.returncode == 0, (extra, result.stderr + result.stdout) + + +def test_a_holder_with_no_members_is_asked_nothing(empty_holder): + """No misfire: a holder `init` made holds no `.gitmodules` and no + gitlink, and passes in every mode.""" + for extra in ((), ("--pins",), ("--no-members",)): + result = validate(empty_holder, *extra) + assert result.returncode == 0, (extra, result.stderr + result.stdout) + + +def test_an_untracked_copy_put_back_is_still_a_finding_until_committed( + holder, offline, tmp_path): + """A `.gitmodules` put back by hand, untracked: the holder's own + checkouts still work, but HEAD, which a clone reads, holds none, so the + finding stands until the file is committed, and the holder then + passes.""" + env = isolated(offline, tmp_path) + kept = (holder / GITMODULES).read_bytes() + head_lost(holder, env) + (holder / GITMODULES).write_bytes(kept) + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stderr + result.stdout + assert [finding.split(":", 1)[0] for finding in findings(result)] == [ + "FINDING member-gitmodules-absent"] * 2 + staged_and_committed(holder, env, "Put .gitmodules back") + assert_passes_in_every_mode(holder, 2) + + +# --- 574/2: a gitlink recorded only at a case variant of the row's path ----- + +def variant_refused(path: str, commit: str, variants: list[str], + gitlinks: dict[str, str]) -> list[str]: + """`member-gitlink-absent`'s lines for a row whose gitlink is recorded + only at `variants`.""" + project = path.split("/", 1)[1] + found = ", ".join(f"{variant} (at {gitlinks[variant]})" + for variant in variants) + return [ + f"REFUSED member-gitlink-absent: no gitlink (mode 160000) recorded at " + f"'{path}'. {project} is named in family.yaml, and this repository " + "records its gitlink only at a case variant of that path, which " + f"macOS and Windows hold as one directory with it: {found}. A " + f"recursive clone checks out nothing at {path}.", + "Remediation: the member is recorded in the wrong case. Move its " + "gitlink to the row's path, at the commit the row pins, in the index " + "alone, which touches no file, and commit that:", + " git rm -r --cached -- " + " ".join(variants), + f" git update-index --add --cacheinfo 160000,{commit},{path}"] + + +def recorded_at(root: Path, env: dict, project: str, + *spellings: str) -> str: + """`project`'s gitlink taken out of the index at its own path and + recorded at each of `spellings`, then committed: index records alone, + so no disk holds two names and the checkout does not move. Returns the + commit it names.""" + commit = pinned(root, project) + raw(root, env, "rm", "-q", "--cached", "--", f"members/{project}") + for spelling in spellings: + stage_gitlink(root, spelling, commit) + commit_all(root, env, f"{project} recorded at {', '.join(spellings)}") + return commit + + +@DISKS +@pytest.mark.parametrize("spellings", [ + ("Members/{p}",), ("MEMBERS/{p}", "Members/{p}")], ids=["one", "two"]) +def test_a_gitlink_only_at_a_case_variant_is_named_and_moved_as_printed( + holder, offline, tmp_path, ignorecase, spellings): + """#363's item 3: HEAD records MEMBER's gitlink at `Members/IRRS` (and, + in the second case, at `MEMBERS/IRRS` too) while its row and section say + `members/IRRS`. The refusal names each variant with its commit and, + since `family.py add` cannot help, prints the move: the variants out of + the index, the gitlink in at the row's path at the pinned commit. Typed + as printed and committed, on either disk, HEAD records the member once, + at its own path, and the holder passes in every mode.""" + env = on_disk(isolated(offline, tmp_path), ignorecase) + variants = sorted(spelling.format(p=MEMBER) for spelling in spellings) + commit = recorded_at(holder, env, MEMBER, *variants) + + result = validate(holder) + + assert result.returncode == 2, result.stderr + result.stdout + lines = result.stderr.splitlines() + path = f"members/{MEMBER}" + assert lines == variant_refused(path, commit, variants, + dict.fromkeys(variants, commit)) + for line in lines[2:]: + raw(holder, env, *shlex.split(line)[1:]) + commit_all(holder, env, "Move the gitlink as printed") + listed = raw(holder, env, "ls-tree", "-r", "--name-only", "HEAD").stdout + assert f"{path}\n".encode() in listed + for variant in variants: + assert f"{variant}\n".encode() not in listed + assert_passes_in_every_mode(holder, 2) + + +def test_a_fresh_clone_of_a_case_variant_says_it_too(family, holder, offline, + tmp_path): + """The same state pushed: a fresh clone names the variant, where the + refusal named `family.py add` and nothing of `Members/`. MEMBER is the + first row, so its gitlink is asked before any member's checkout.""" + env = isolated(offline, tmp_path) + commit = recorded_at(holder, env, MEMBER, f"Members/{MEMBER}") + remote = family["remotes"] / f"{NAME}.git" + raw(holder, env, "push", "-q", str(remote), "HEAD:refs/heads/w611/case") + clone = tmp_path / "clone" + cloned = clone_just_pushed(remote, clone, "w611/case", recurse=False) + assert cloned.returncode == 0, cloned.stdout + cloned.stderr + + result = validate(clone) + + assert result.returncode == 2, result.stderr + result.stdout + variant = f"Members/{MEMBER}" + assert result.stderr.splitlines() == variant_refused( + f"members/{MEMBER}", commit, [variant], {variant: commit}) + + +# --- 574/3: a hand mount's remedy commits the removal before the add --------- + +@DISKS +def test_a_hand_mount_under_members_spelt_another_way_is_added_as_printed( + family, holder, offline, tmp_path, ignorecase): + """A gitlink at `Members/IRQS` that no row names, with its section, as a + hand mount leaves one (index records, so no disk holds `Members/`). + Typed as printed, the branch, the `git rm`, the commit the text says + the removal needs, and the `add` with the org filled in mount IRQS at + `members/IRQS` with its row, and the holder passes in every mode. With + the removal left uncommitted, as the text said before, `add` refuses: + HEAD still holds `Members/IRQS`.""" + env = on_disk(isolated(offline, tmp_path), ignorecase) + raw(holder, env, "switch", "-q", "-C", "main") + _, tip = spare_commits(family) + hand = f"Members/{SPARE}" + stage_gitlink(holder, hand, tip) + data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout + ( + f'[submodule "{hand}"]\n\tpath = {hand}\n\turl = {url(SPARE)}\n' + .encode()) + committed_gitmodules(holder, env, data) + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stderr + result.stdout + gitlink = findings(result)[0] + assert gitlink.startswith(f"FINDING member-gitlink-unnamed: {hand} ") + assert (", and commit that removal, since family.py add refuses a path " + "under another spelling of members/ that HEAD holds and the " + "index no longer does, then mount it with the third, ") in gitlink + switch, rm, add = commands(gitlink) + raw(holder.parent, env, *shlex.split(switch)[1:]) + raw(holder, env, *shlex.split(rm)[1:]) + argv = [arg.replace("", ORG) for arg in shlex.split(add)] + assert argv[:3] == [PYTHON, "scripts/family.py", "add"], add + + def add_as_printed(): + return run_script(FAMILY, *argv[2:], "--local-remote-dir", + str(family["remotes"]), env=env) + + uncommitted = add_as_printed() + assert uncommitted.returncode == 2, uncommitted.stdout + uncommitted.stderr + assert "in HEAD and no longer in the index" in uncommitted.stderr + commit_all(holder, env, "Take the hand mount out, as printed") + added = add_as_printed() + assert added.returncode == 0, added.stdout + added.stderr + assert (holder / "members" / SPARE / "project.yaml").is_file() + assert_passes_in_every_mode(holder, 3) diff --git a/tests/test_family_remove_gitmodules.py b/tests/test_family_remove_gitmodules.py index cc7594f8..282a6b82 100644 --- a/tests/test_family_remove_gitmodules.py +++ b/tests/test_family_remove_gitmodules.py @@ -651,12 +651,12 @@ def test_a_holder_whose_head_has_no_gitmodules_is_not_asked( """Where none of the index, HEAD and the work tree holds a `.gitmodules` there is no copy to compare, and no `gitmodules-` refusal is made. - THE REAL RUN STILL FAILS HERE, a known case from before #407 that goes - with the gate's to #459: in a holder whose HEAD already lost the file, - `remove` runs `deinit` and `git rm`, then dies at its commit's `git add - -f -- .gitmodules family.yaml` and puts back only part of the unmount, - while its dry run exits 0. So this pins only that #407's check asks - nothing, and not the dry run's answer (the review's N3).""" + A holder whose HEAD already lost the file is #611's: `remove` and its + dry run refuse it `member-gitmodules-absent`, before any git command + that writes (`tests/test_family_head_lost_gitmodules.py`), where the + real run used to die at its commit's `git add -f -- .gitmodules + family.yaml` with the member emptied. So this pins only that #407's + check asks nothing (the review's N3).""" env = isolated(offline, tmp_path) deletion_staged(holder, env) raw(holder, env, "commit", "-q", "-m", "The commit #407 describes") diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py index bcce579c..7c4babf5 100644 --- a/tests/test_validate_family_unnamed_mounts.py +++ b/tests/test_validate_family_unnamed_mounts.py @@ -88,9 +88,11 @@ "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 " + "its .gitmodules section out too, and commit that removal, since " + "family.py add refuses a path under another spelling of members/ that " + "HEAD holds and the index no longer does, 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.") @@ -523,17 +525,24 @@ def test_a_member_recorded_only_in_another_case_says_so_and_prints_no_command( none and is the refusal `member-gitlink-absent`, which prints no finding, so only `--no-members` shows the finding; a git that matched the pathspec in any case would find the variant and print it, so either is accepted - there, and what no run may do is print a command.""" + there. What no run may do is print a command that takes the gitlink out + ALONE: since #611 the refusal names the variant and prints the move that + takes it out of the index and records it at the row's path together + (`tests/test_family_head_lost_gitmodules.py` types it).""" named = record_in_another_case(holder, "IRSS", "members/irss") result = validate(holder, *extra) - assert "git rm" not in result.stderr + result.stdout assert "--unset-all" not in result.stderr + result.stdout if result.returncode == 2: assert extra != ("--no-members",), result.stderr assert result.stderr.startswith( "REFUSED member-gitlink-absent: no gitlink (mode 160000) " "recorded at 'members/IRSS'."), result.stderr + assert result.stderr.splitlines()[-2:] == [ + " git rm -r --cached -- members/irss", + " git update-index --add --cacheinfo " + f"160000,{named},members/IRSS"], result.stderr return + assert "git rm" not in result.stderr + result.stdout assert result.returncode == 1, result.stderr + result.stdout (gitlink,) = findings(result) assert gitlink == GITLINK_SAID.format( @@ -546,13 +555,20 @@ def test_a_sections_only_path_in_another_case_says_so_and_prints_no_command( holder, extra): """The section's own state (ii): IRSS's row is canonical, its gitlink is where it belongs, and the ONLY section that registers it gives - `members/irss`. `--unset-all` of that left the member with no section.""" + `members/irss`. `--unset-all` of that left the member with no section. + Since #611 no section of HEAD's file giving `members/IRSS` is + `member-gitmodules-absent` beside it, whose one line writes the path + back into that section and keeps its url.""" url = url_of(holder, "IRSS") with_path_replaced(holder, b"\tpath = members/IRSS\n", b"\tpath = members/irss\n") result = validate(holder, *extra) assert result.returncode == 1, result.stderr + result.stdout - (section,) = findings(result) + section, unregistered = findings(result) + assert unregistered.startswith( + "FINDING member-gitmodules-absent: members/IRSS is a gitlink at ") + assert commands(unregistered) == [ + "git config -f .gitmodules submodule.members/IRSS.path members/IRSS"] assert section == SECTION_SAID.format( path="members/irss", name="members/IRSS", url=f"url {url}" ) + WRONG_CASE.format(row_path="members/IRSS", project="IRSS", @@ -685,7 +701,8 @@ 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, 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 + `git rm` takes the checkout and its section out, and the removal is + committed, as the text says since #611; `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 @@ -707,6 +724,7 @@ def test_a_hand_mount_is_taken_out_and_added_back_as_printed(family, holder): assert commands(findings(result)[1]) == [] git(*shlex.split(switch)[1:], cwd=holder.parent) git(*shlex.split(rm)[1:], cwd=holder) + commit(holder, "Take the hand mount out, as printed") assert not (holder / "members" / SPARE).exists() assert f"members/{SPARE}" not in (holder / ".gitmodules").read_text( encoding="utf-8") From 6f938ed1a385890cdb4c001e85b304c2618f638d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 23:11:21 +0000 Subject: [PATCH 2/3] tests/test_family_head_lost_gitmodules.py: the rows `member-gitmodules-absent` reads and the members `remove`'s refusal is for are pinned, so a finding for a row with no gitlink or a hand-edited path, or the refusal over a member with no gitlink, no longer passes (#611) Two tests. In a holder whose HEAD lost `.gitmodules`: a row with no gitlink (`member-gitlink-absent`'s) and a row whose `path:` was edited by hand (`member-path`'s, its mount `member-gitlink-unnamed`'s) get no `member-gitmodules-absent`; and `remove` of a member with no gitlink is not that refusal, but the dry run's `did not match any file(s) known to git`, as before. Each holds a condition of the first commit that its other tests did not: both pass on BASE 396c0ff4 and on that commit. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_family_head_lost_gitmodules.py | 43 +++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/test_family_head_lost_gitmodules.py b/tests/test_family_head_lost_gitmodules.py index 159082b9..62337ef1 100644 --- a/tests/test_family_head_lost_gitmodules.py +++ b/tests/test_family_head_lost_gitmodules.py @@ -279,6 +279,25 @@ def test_the_holder_remove_leaves_clones_with_the_other_member( assert gate.returncode == 0, gate.stdout + gate.stderr +def test_remove_of_a_member_with_no_gitlink_is_not_this_refusal( + family, holder, offline, tmp_path): + """The refusal is for a member whose gitlink the index records: that is + the unmount the commit's failure emptied. With none, the run fails at + its first git call, which writes nothing, and the dry run says so by + git's own words, as before; the remedy of this refusal could not help.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + raw(holder, env, "rm", "-q", "--cached", "--", f"members/{MEMBER}") + commit_all(holder, env, "MEMBER's gitlink taken out") + + dry = run_script(FAMILY, *verb_args("remove", holder, family), + "--dry-run", env=env) + + assert dry.returncode == 2, dry.stdout + dry.stderr + assert "member-gitmodules-absent" not in dry.stderr, dry.stderr + assert "did not match any file(s) known to git" in dry.stderr + + def test_remove_of_a_member_whose_section_alone_is_gone_runs_as_before( family, holder, offline, tmp_path): """The refusal is for a holder with NO `.gitmodules`, where the commit @@ -436,6 +455,30 @@ def test_a_holder_with_no_members_is_asked_nothing(empty_holder): assert result.returncode == 0, (extra, result.stderr + result.stdout) +def test_a_row_the_gitlink_checks_do_not_read_is_asked_nothing( + holder, offline, tmp_path): + """The rows `_check_member` reads at their path, and no other, in a + holder whose HEAD lost `.gitmodules`. OTHER's gitlink taken out too: a + row with no gitlink is `member-gitlink-absent`'s, members checked. + MEMBER's row given another `path:` by hand: that row is `member-path`'s, + and its mount `member-gitlink-unnamed`'s.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + raw(holder, env, "rm", "-q", "--cached", "--", f"members/{OTHER}") + commit_all(holder, env, "OTHER's gitlink taken out") + rows = (holder / "family.yaml").read_bytes() + line = f" path: members/{MEMBER}\n".encode() + assert rows.count(line) == 1, "fixture drift: MEMBER's path line" + (holder / "family.yaml").write_bytes(rows.replace( + line, f" path: members/{MEMBER}-moved\n".encode())) + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stderr + result.stdout + assert "member-gitlink-unnamed" in result.stderr, result.stderr + assert "member-gitmodules-absent" not in result.stderr, result.stderr + + def test_an_untracked_copy_put_back_is_still_a_finding_until_committed( holder, offline, tmp_path): """A `.gitmodules` put back by hand, untracked: the holder's own From 6aace429305eb4f0c9e1e3f7cfa05091b8fbb67f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 01:35:46 +0000 Subject: [PATCH 3/3] family.py and validate-family.py: `remove` refuses a holder with no `.gitmodules` over an unmerged gitlink too, and the case-variant move prints `--ignore-unmatch` and the `mv` a case-sensitive disk needs, so the mount is no longer emptied there and the printed lines work half typed or with both directories on disk (#611) Fix round 1, from the review of 6f938ed1. `_checked_gitmodules_held` (scripts/family.py) took the member's gitlink only at stage 0. `git submodule deinit` and `git rm` run on an unmerged gitlink as on a merged one, so with `members/

` held only at stages 1, 2 and 3 in a holder whose HEAD lost `.gitmodules`, the dry run exited 0 and the real run emptied the mount and died at its commit. It now takes a gitlink at any stage, and both runs refuse `member-gitmodules-absent` before `deinit`, with the holder byte for byte as it was. The docstring says so. The case-variant refusal of `member-gitlink-absent` (templates/family-root/scripts/validate-family.py, pinned) now prints `git rm -r --cached --ignore-unmatch -- `: a variant whose removal is staged and not committed is still named from HEAD, and the plain line failed there (`pathspec ... did not match`). Its remediation also says what is left on disk: where nothing is checked out at the row's path, as in a fresh clone, `git submodule update --init --recursive` then checks the member out; on a case-sensitive disk a directory left at a variant stays, untracked, and `family.py add` refuses it as `family-members-dir-variant`, so it gives an `mv` of each out of the holder, to `../`, and says to leave it on macOS and Windows, where it is the row's own directory. The validator's module docstring names `member-gitmodules-absent`, says what a staged, uncommitted `.gitmodules` does (HEAD's file is what counts, as #407's decision 3 says: an uncommitted `add` is the finding for its member, a staged section removal is none, a staged restore is still the finding), and gives EXIT CODES' one exception, an unregistered member that is not checked out (exit 1, not 2). Tests (tests/test_family_head_lost_gitmodules.py, 12 more, 38 in all): the unmerged gitlink refused alike and repaired as printed; one test per condition no test held (a checked-out member still gets its tree check beside the finding; HEAD's unreadable file refused in every mode; the `` placeholder; `remove`'s remedy names only members with a gitlink in the index; one finding per path for a duplicate row); the variant move half typed (both disks); and, on a case-sensitive disk, every printed line typed with both directories present, after which the gate passes and another member is added. tests/test_validate_family_unnamed_mounts.py pins the `--ignore-unmatch` line. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 12 +- .../family-root/scripts/validate-family.py | 60 +++- tests/test_family_head_lost_gitmodules.py | 292 ++++++++++++++++-- tests/test_validate_family_unnamed_mounts.py | 2 +- 4 files changed, 322 insertions(+), 44 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 85b2f87a..32cc1e34 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -5408,7 +5408,7 @@ def _checked_gitmodules_held(family: Family, path: str) -> None: """`member-gitmodules-absent`: `remove` refuses, before any git command that writes and under `--dry-run` alike, a holder that holds no `.gitmodules` at all, in the work tree, the index or HEAD, while its - index records the member's gitlink (#611). + index records the member's gitlink at any stage (#611). A HOLDER WHOSE HEAD LOST `.gitmodules`, as #407's commit left one. `git rm` edits only a `.gitmodules` the work tree has, and `remove` commits @@ -5416,7 +5416,11 @@ def _checked_gitmodules_held(family: Family, path: str) -> None: add` died (`pathspec '.gitmodules' did not match any files`) after `deinit` and `git rm` had emptied the member, and the rollback left its directory empty; the dry run exited 0. `_checked_gitmodules_as_staged` - asks nothing here, since no copy exists to compare. + asks nothing here, since no copy exists to compare. It asks only where + none of the work tree, the index and HEAD holds a `.gitmodules`, and the + index records the member's gitlink at any stage: `deinit` and `git rm` + run on a merged or an unmerged gitlink alike, and with no `.gitmodules` + anywhere the commit cannot stage one. REFUSED, NOT COMMITTED WITHOUT IT, as #407 refuses a `.gitmodules` the work tree lacks: the holder is left byte for byte as it was. A clone @@ -5436,9 +5440,7 @@ def _checked_gitmodules_held(family: Family, path: str) -> None: return gitlinks = {name.decode("utf-8", "surrogateescape") for name, held in records.items() - if any(record.startswith(b"160000 ") - and record.partition(b"\t")[0].endswith(b" 0") - for record in held)} + if any(record.startswith(b"160000 ") for record in held)} if path not in gitlinks: return sections = [] diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index e4c2200c..d4283032 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -42,7 +42,8 @@ finding and none of the three is asked of it: nothing read through a path the row may not have is a fact about the member (#367). A row whose gitlink is recorded only at a case variant of its path is refused - naming that variant, and the index move that puts it back (#611). + naming that variant, the index move that puts it back, and what to do + with a directory the variant leaves on disk (#611). And, across the rows, that no two record ONE `id`: that is one project pinned twice, however each row spelled its repository (#206). This one reads `family.yaml` alone, so `--no-members` asks it too. @@ -60,12 +61,18 @@ and what it says to do depends on which of the two is the member's own: it names no command that would un-register the member. - that a section of HEAD's `.gitmodules` gives the path of every row's - gitlink (#611, #407's decision 3). A recursive clone reads HEAD's file, - so a gitlink no section of it gives is left empty there, by CI and by - `make bootstrap`, though the holder it was committed in keeps its - checkout. It needs no member checked out, so `--no-members` asks it - too, and a member it names that is not checked out is not then refused - as uninitialized. + gitlink, the `member-gitmodules-absent` finding (#611, #407's decision + 3). A recursive clone reads HEAD's file, so a gitlink no section of it + gives is left empty there, by CI and by `make bootstrap`, though the + holder it was committed in keeps its checkout. It needs no member + checked out, so `--no-members` asks it too, and a member it names that + is not checked out is not then refused as uninitialized. A + `.gitmodules` staged and not committed counts for nothing until it is + committed, since HEAD's file is the one a clone reads: a `family.py + add` staged and not committed is this finding for its own member; a + section taken out and staged, not committed, is no finding; and a file + put back and staged, not committed, in a holder whose HEAD lost it is + still this finding for every member. - the COPIED shape files, each against its `sha256` row in `contracts/shape-pin.yaml`. Editing a copy in place is drift and the exit is to carry it upstream, never to update the digest. @@ -74,8 +81,10 @@ 0 the manifest is valid, every member is in lockstep, every copy matches 1 a FINDING: drift a reviewer can act on 2 a REFUSAL: the question could not be asked — no manifest, an - uninitialized member, an unreadable pin, no git to run. An - unanswerable question is never an implicit pass. + uninitialized member (but for one no section of HEAD's `.gitmodules` + gives: that is `member-gitmodules-absent`, exit 1), an unreadable + pin, no git to run. An unanswerable question is never an implicit + pass. """ from __future__ import annotations @@ -449,10 +458,26 @@ def _gitlink_in_another_case(path: str, commit: str, project: str, records the member once, at its own path. `member-gitlink-unnamed` prints no command for the same gitlink, since taking it out ALONE would un-register the member; these take it out and record it together. + `--ignore-unmatch`, since a variant whose removal is staged and not yet + committed is still named here, from HEAD, and the plain `git rm` of it + failed (`pathspec ... did not match`). + + WHAT IS LEFT ON DISK. Where nothing is checked out at the row's path, + as in a fresh clone, `git submodule update --init --recursive` checks + the member out there once the move is committed. On a case-sensitive + disk a directory at a variant (the empty one a fresh clone leaves, or a + checkout) stays behind, untracked, and `family.py add` refuses it as + `family-members-dir-variant` (#574), so the text says to move it out of + the holder, by an `mv` with nothing to fill in. On macOS and Windows + that directory is the row's own, and the text says to leave it there. """ found = ", ".join(f"{variant} (at {gitlinks[variant]})" for variant in variants) taken_out = " ".join(shlex.quote(variant) for variant in variants) + moved = " and ".join( + f"`mv -- {shlex.quote(variant)} " + f"{shlex.quote('../' + variant.replace('/', '-'))}`" + for variant in variants) return Refusal( "member-gitlink-absent", f"no gitlink (mode 160000) recorded at {path!r}. {project} is named " @@ -462,8 +487,16 @@ def _gitlink_in_another_case(path: str, commit: str, project: str, f"at {path}.", "Remediation: the member is recorded in the wrong case. Move its " "gitlink to the row's path, at the commit the row pins, in the index " - "alone, which touches no file, and commit that:\n" - f" git rm -r --cached -- {taken_out}\n" + "alone, which touches no file, with the two lines below, and commit " + f"that. Where nothing is checked out at {path}, as in a fresh clone, " + "`git submodule update --init --recursive` then checks the member " + "out there. On a case-sensitive disk, as Linux's usually is, a " + f"directory left at {' or '.join(variants)} stays there, untracked, " + "and family.py add refuses it as family-members-dir-variant: move " + f"each one that is there out of the holder, {moved}. On macOS and " + "Windows it is the row's own directory, so leave it. The two " + "lines:\n" + f" git rm -r --cached --ignore-unmatch -- {taken_out}\n" " git update-index --add --cacheinfo " f"{shlex.quote(f'160000,{commit},{path}')}") @@ -1342,6 +1375,11 @@ def _check_gitlinks_registered(root: Path, members: list, `member-path`'s, and one with no gitlink there is `member-gitlink-absent`'s (`gitlinks`, read as `recorded_gitlink` reads one). A holder with no members holds no gitlink and is asked nothing. + THREE SOURCES: the gitlinks from the index first, the rows from the + work tree's `family.yaml`, and the sections from HEAD alone, as #407's + decision 3 has it, so a `.gitmodules` staged and not committed counts + for nothing until it is committed (the module docstring gives the three + cases). The remedy writes the section as `family.py add` writes it, named after its path, with the url it gives a member, `https://github.com/` and the diff --git a/tests/test_family_head_lost_gitmodules.py b/tests/test_family_head_lost_gitmodules.py index 62337ef1..85f35308 100644 --- a/tests/test_family_head_lost_gitmodules.py +++ b/tests/test_family_head_lost_gitmodules.py @@ -28,6 +28,12 @@ that HEAD holds and the index no longer does (#574), so it says to commit the removal between them now. +AND THE STATES BESIDE THEM that the first round's review found: a member's +gitlink held only at the conflict stages, which `remove` emptied as it +emptied a merged one (its S1); a condition of each new check that no test +held (its S2 and N5); and the variant move typed half way, or leaving a +directory on a case-sensitive disk (its N1 and N2). + THE SAME FAMILY, OFFLINE RULES AND READINGS as `test_family_remove_gitmodules.py` (#407), whose helpers are imported. Every remedy is typed as printed: split as a POSIX shell splits it, run with no @@ -45,6 +51,8 @@ import re import shlex +import shutil +import subprocess from pathlib import Path import pytest @@ -54,19 +62,21 @@ #: The family fixture, its names, the offline environment and the readings #: of a holder, imported from the modules that own them rather than rebuilt. from test_family import ( # noqa: F401 - FAMILY, NAME, ORG, SPARE, family, validate, + FAMILY, NAME, ORG, SPARE, family, manifest, validate, ) from test_family_add_excludes import empty_holder # noqa: F401 from test_family_add_refusals import holder, offline # noqa: F401 -from test_family_dry_run import isolated +from test_family_dry_run import hand_edit_digest, isolated from test_family_member_path import MEMBER, OTHER, raw from test_family_remove_gitmodules import ( deleted, edited, head_paths, left_out, ran, refused_alike, typed_as_printed, verb_args, ) from test_scaffold_ignored_shape_files import excluding +from test_validate_family_duplicates import EXACT, irrs_row, rewrite from test_validate_family_unnamed_mounts import ( - commands, findings, spare_commits, stage_gitlink, + codes, commands, findings, spare_commits, stage_gitlink, + stage_unmerged_gitlink, ) GITMODULES = ".gitmodules" @@ -125,16 +135,17 @@ def remove_remedy(*projects: str) -> str: def absent(project: str, commit: str, *, held: bool, - with_url: bool = True) -> str: + with_url: bool = True, printed_url: str | None = None) -> str: """The `member-gitmodules-absent` finding for `project`'s gitlink at `commit`, WHOLE: its line and the commands under it. `held` is whether HEAD holds a `.gitmodules` at all; `with_url` whether the section to - write has no url of its own.""" + write has no url of its own; `printed_url` the url line's value where + it is not `url(project)`.""" path = f"members/{project}" lines = f"\n git config -f .gitmodules submodule.{path}.path {path}" if with_url: lines += (f"\n git config -f .gitmodules submodule.{path}.url " - f"{url(project)}") + f"{printed_url or url(project)}") return (f"FINDING member-gitmodules-absent: {path} is a gitlink at " f"{commit} that a row of members: names, and " + ("no section of HEAD's .gitmodules gives that path" if held @@ -315,6 +326,54 @@ def test_remove_of_a_member_whose_section_alone_is_gone_runs_as_before( assert_passes_in_every_mode(holder, 1) +def test_remove_refuses_it_where_the_members_gitlink_is_unmerged( + family, holder, offline, tmp_path): + """THE REVIEW'S S1: HEAD lost `.gitmodules`, and MEMBER's gitlink is held + only at the conflict stages 1, 2 and 3 (`stage_unmerged_gitlink`). + `deinit` and `git rm` run on an unmerged gitlink as on a merged one, so + while the refusal asked only for one at stage 0, the dry run exited 0 + and the real run emptied the mount and died at its commit. Both refuse + now, with the holder byte for byte as it was, the mount's files and the + three stages included. Typed as printed, the remedy writes both + sections back, and `remove` then commits.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + path = f"members/{MEMBER}" + stage_unmerged_gitlink(holder, path, pinned(holder, MEMBER)) + listed = raw(holder, env, "ls-files", "-s", "--", path).stdout + assert [line.split()[2] for line in listed.splitlines()] == [ + b"1", b"2", b"3"], "premise: unmerged, no stage 0" + args = verb_args("remove", holder, family) + + said, remediation = refused_alike(holder, env, args) + + assert said == REMOVE_REFUSED.format(member=MEMBER) + assert remediation == remove_remedy(MEMBER, OTHER) + assert (holder / path / "project.yaml").is_file() + typed_as_printed(holder, env, remediation) + ran(holder, env, args) + assert head_paths(holder, env) == [f"members/{OTHER}"] + assert_passes_in_every_mode(holder, 1) + + +def test_removes_remedy_names_only_members_whose_gitlink_is_indexed( + family, holder, offline, tmp_path): + """The remedy writes back the section of each member whose gitlink the + index records, and of no other. OTHER's gitlink taken out and its row + left: a section for it would register a path the holder records + nothing at, so the remedy writes MEMBER's section alone.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + raw(holder, env, "rm", "-q", "--cached", "--", f"members/{OTHER}") + commit_all(holder, env, "OTHER's gitlink taken out") + + said, remediation = refused_alike(holder, env, + verb_args("remove", holder, family)) + + assert said == REMOVE_REFUSED.format(member=MEMBER) + assert remediation == remove_remedy(MEMBER) + + # --- 571(c): a gitlink no section of HEAD's .gitmodules gives --------------- @MODES @@ -499,6 +558,104 @@ def test_an_untracked_copy_put_back_is_still_a_finding_until_committed( assert_passes_in_every_mode(holder, 2) +@pytest.mark.parametrize("extra", [(), ("--pins",)], + ids=["members-checked", "pins"]) +def test_a_checked_out_member_it_names_still_has_its_tree_checked( + holder, offline, tmp_path, extra): + """Only a member it names that is NOT checked out stops after its + gitlink. One that is checked out is asked its tree as before: MEMBER's + digest edited by hand, in a holder whose HEAD lost `.gitmodules`, is + `member-digest-mismatch` beside the two `member-gitmodules-absent`.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + hand_edit_digest(holder, env) + + result = validate(holder, *extra) + + assert result.returncode == 1, result.stderr + result.stdout + assert codes(result) == ["member-gitmodules-absent"] * 2 + [ + "member-digest-mismatch"] + assert findings(result)[2].startswith( + f"FINDING member-digest-mismatch: {MEMBER}: pin.tree_sha256 " + f"{'b' * 64}\n"), result.stderr + + +@MODES +def test_a_head_gitmodules_git_cannot_read_is_refused(holder, offline, + tmp_path, extra): + """HEAD's `.gitmodules` committed broken, the work tree's mended and not + committed. A clone reads HEAD's, which git cannot read either, so the + gate refuses `gitmodules-unreadable` in every mode, saying to commit the + mended file, and never reads the broken file as one that gives no + section. Committed, the holder passes.""" + env = isolated(offline, tmp_path) + mended = (holder / GITMODULES).read_bytes() + committed_gitmodules(holder, env, + f'[submodule "members/{MEMBER}"\n\tpath = x\n' + .encode("utf-8")) + (holder / GITMODULES).write_bytes(mended) + + result = validate(holder, *extra) + + assert result.returncode == 2, result.stderr + result.stdout + lines = result.stderr.splitlines() + assert lines[0].startswith( + "REFUSED gitmodules-unreadable: HEAD's .gitmodules could not be read " + "as git reads it: `git config -z --blob "), result.stderr + assert lines[-1] == ( + "Remediation: git cannot read it either, so none of this holder's " + "submodules can be cloned from it. Mend the file by hand (`git config " + "-f .gitmodules --list` says where it breaks), commit it, then run " + "this again.") + staged_and_committed(holder, env, "Commit the mended .gitmodules") + assert_passes_in_every_mode(holder, 2) + + +def test_a_repository_not_org_and_name_prints_the_url_placeholder( + holder, offline, tmp_path): + """The url line gives `https://github.com/.git`, the url + `family.py add` writes, only where the row's `repository:` is + `/`. From any other value that would be a url nobody chose, + so the line prints `` for the person to fill in.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + rows = (holder / "family.yaml").read_bytes() + line = f" repository: {ORG}/{MEMBER}\n".encode("utf-8") + assert rows.count(line) == 1, "fixture drift: MEMBER's repository line" + (holder / "family.yaml").write_bytes(rows.replace( + line, f" repository: {MEMBER}\n".encode("utf-8"))) + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stderr + result.stdout + assert findings(result) == [ + absent(MEMBER, pinned(holder, MEMBER), held=False, + printed_url=""), + absent(OTHER, pinned(holder, OTHER), held=False)] + + +def test_a_row_written_twice_is_one_finding_per_path(holder, offline, + tmp_path): + """MEMBER's row written twice in a holder whose HEAD lost `.gitmodules`: + `member-duplicate` once, and `member-gitmodules-absent` once per path, + not once per row.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + row = irrs_row(holder) + assert row["project"] == MEMBER, "fixture drift: MEMBER is IRRS" + other = next(r for r in manifest(holder)["members"] + if r["project"] == OTHER) + rewrite(holder, [row, dict(row), other]) + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stderr + result.stdout + assert findings(result) == [ + EXACT, + absent(MEMBER, pinned(holder, MEMBER), held=False), + absent(OTHER, pinned(holder, OTHER), held=False)] + + # --- 574/2: a gitlink recorded only at a case variant of the row's path ----- def variant_refused(path: str, commit: str, variants: list[str], @@ -508,6 +665,8 @@ def variant_refused(path: str, commit: str, variants: list[str], project = path.split("/", 1)[1] found = ", ".join(f"{variant} (at {gitlinks[variant]})" for variant in variants) + moved = " and ".join(f"`mv -- {variant} ../{variant.replace('/', '-')}`" + for variant in variants) return [ f"REFUSED member-gitlink-absent: no gitlink (mode 160000) recorded at " f"'{path}'. {project} is named in family.yaml, and this repository " @@ -516,11 +675,44 @@ def variant_refused(path: str, commit: str, variants: list[str], f"recursive clone checks out nothing at {path}.", "Remediation: the member is recorded in the wrong case. Move its " "gitlink to the row's path, at the commit the row pins, in the index " - "alone, which touches no file, and commit that:", - " git rm -r --cached -- " + " ".join(variants), + "alone, which touches no file, with the two lines below, and commit " + f"that. Where nothing is checked out at {path}, as in a fresh clone, " + "`git submodule update --init --recursive` then checks the member " + "out there. On a case-sensitive disk, as Linux's usually is, a " + f"directory left at {' or '.join(variants)} stays there, untracked, " + "and family.py add refuses it as family-members-dir-variant: move " + f"each one that is there out of the holder, {moved}. On macOS and " + "Windows it is the row's own directory, so leave it. The two lines:", + " git rm -r --cached --ignore-unmatch -- " + " ".join(variants), f" git update-index --add --cacheinfo 160000,{commit},{path}"] +def refused_at(root: Path, commit: str, variants: list[str]) -> list[str]: + """The members-checked gate in `root`, where MEMBER's gitlink is recorded + only at `variants`, at `commit`: exit 2, in `variant_refused`'s lines, + which it returns.""" + result = validate(root) + assert result.returncode == 2, result.stderr + result.stdout + lines = result.stderr.splitlines() + assert lines == variant_refused(f"members/{MEMBER}", commit, variants, + dict.fromkeys(variants, commit)) + return lines + + +def moved_as_printed(root: Path, env: dict, lines: list[str], + variants: list[str]) -> None: + """The refusal's two command lines typed as printed, and the commit it + says to make: HEAD then records MEMBER once, at its own path, and at + none of `variants`.""" + for line in lines[2:]: + raw(root, env, *shlex.split(line)[1:]) + commit_all(root, env, "Move the gitlink as printed") + listed = raw(root, env, "ls-tree", "-r", "--name-only", "HEAD").stdout + assert f"members/{MEMBER}\n".encode() in listed + for variant in variants: + assert f"{variant}\n".encode() not in listed + + def recorded_at(root: Path, env: dict, project: str, *spellings: str) -> str: """`project`'s gitlink taken out of the index at its own path and @@ -551,23 +743,74 @@ def test_a_gitlink_only_at_a_case_variant_is_named_and_moved_as_printed( variants = sorted(spelling.format(p=MEMBER) for spelling in spellings) commit = recorded_at(holder, env, MEMBER, *variants) - result = validate(holder) + lines = refused_at(holder, commit, variants) - assert result.returncode == 2, result.stderr + result.stdout - lines = result.stderr.splitlines() - path = f"members/{MEMBER}" - assert lines == variant_refused(path, commit, variants, - dict.fromkeys(variants, commit)) - for line in lines[2:]: - raw(holder, env, *shlex.split(line)[1:]) - commit_all(holder, env, "Move the gitlink as printed") - listed = raw(holder, env, "ls-tree", "-r", "--name-only", "HEAD").stdout - assert f"{path}\n".encode() in listed - for variant in variants: - assert f"{variant}\n".encode() not in listed + moved_as_printed(holder, env, lines, variants) assert_passes_in_every_mode(holder, 2) +@DISKS +def test_a_variant_whose_removal_is_staged_is_moved_as_printed( + holder, offline, tmp_path, ignorecase): + """HALF REPAIRED (the review's N1): the variant's removal staged and not + committed. HEAD still records the gitlink there, so the refusal still + names it, and the plain `git rm -r --cached` of it failed, `pathspec + ... did not match`. With `--ignore-unmatch` the lines typed as printed + and committed move the gitlink, and the holder passes.""" + env = on_disk(isolated(offline, tmp_path), ignorecase) + variant = f"Members/{MEMBER}" + commit = recorded_at(holder, env, MEMBER, variant) + raw(holder, env, "rm", "-q", "--cached", "--", variant) + + lines = refused_at(holder, commit, [variant]) + + moved_as_printed(holder, env, lines, [variant]) + assert_passes_in_every_mode(holder, 2) + + +@pytest.fixture +def case_sensitive_disk(tmp_path): + """Skips where `Members/` and `members/` would be one directory, as + macOS and Windows hold them by default (nothing is left behind there to + move), or where there is no `mv` to type.""" + (tmp_path / "CaseProbe").mkdir() + if (tmp_path / "caseprobe").exists(): + pytest.skip("this disk is case-insensitive: Members/IRRS is " + "members/IRRS here, so no second directory can exist") + if shutil.which("mv") is None: + pytest.skip("no mv on PATH to type the printed line with") + + +def test_a_variant_checkout_left_on_disk_is_moved_out_as_printed( + family, holder, offline, tmp_path, case_sensitive_disk): + """THE REVIEW'S N2, in its C3 layout: MEMBER's gitlink recorded only at + `Members/IRRS`, its checkout still at `members/IRRS`, and a second + checkout at `Members/IRRS`, as a case-sensitive disk can hold. The + index move leaves that directory on disk, untracked, where `family.py + add` refused any other member past it (`family-members-dir-variant`). + Every line the refusal prints, typed as printed: the two lines, their + commit, and the `mv` it gives for a case-sensitive disk. The gate then + passes in every mode, and SPARE is added.""" + env = isolated(offline, tmp_path) + variant = f"Members/{MEMBER}" + commit = recorded_at(holder, env, MEMBER, variant) + raw(holder, env, "clone", "-q", "--no-checkout", + str(family["remotes"] / f"{MEMBER}.git"), variant) + raw(holder / variant, env, "checkout", "-q", "--detach", commit) + + lines = refused_at(holder, commit, [variant]) + + moved_as_printed(holder, env, lines, [variant]) + (move,) = re.findall(r"`(mv [^`]*)`", lines[1]) + subprocess.run(shlex.split(move), cwd=str(holder), check=True) + assert not (holder / variant).exists() + assert (tmp_path / f"Members-{MEMBER}" / "project.yaml").is_file() + assert_passes_in_every_mode(holder, 2) + added = run_script(FAMILY, *verb_args("add", holder, family), env=env) + assert added.returncode == 0, added.stdout + added.stderr + assert_passes_in_every_mode(holder, 3) + + def test_a_fresh_clone_of_a_case_variant_says_it_too(family, holder, offline, tmp_path): """The same state pushed: a fresh clone names the variant, where the @@ -581,12 +824,7 @@ def test_a_fresh_clone_of_a_case_variant_says_it_too(family, holder, offline, cloned = clone_just_pushed(remote, clone, "w611/case", recurse=False) assert cloned.returncode == 0, cloned.stdout + cloned.stderr - result = validate(clone) - - assert result.returncode == 2, result.stderr + result.stdout - variant = f"Members/{MEMBER}" - assert result.stderr.splitlines() == variant_refused( - f"members/{MEMBER}", commit, [variant], {variant: commit}) + refused_at(clone, commit, [f"Members/{MEMBER}"]) # --- 574/3: a hand mount's remedy commits the removal before the add --------- diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py index 7c4babf5..3144f710 100644 --- a/tests/test_validate_family_unnamed_mounts.py +++ b/tests/test_validate_family_unnamed_mounts.py @@ -538,7 +538,7 @@ def test_a_member_recorded_only_in_another_case_says_so_and_prints_no_command( "REFUSED member-gitlink-absent: no gitlink (mode 160000) " "recorded at 'members/IRSS'."), result.stderr assert result.stderr.splitlines()[-2:] == [ - " git rm -r --cached -- members/irss", + " git rm -r --cached --ignore-unmatch -- members/irss", " git update-index --add --cacheinfo " f"160000,{named},members/IRSS"], result.stderr return