From c2f288483c5e6693fe20945842b32635fa9f4551 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 22:59:25 +0000 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 From d660ec564e652be9e2d44de93c53f22b962e757c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 12:12:05 +0000 Subject: [PATCH 04/10] bootstrap.py and validate-family.py: a member no credential would check out fails the family bootstrap, every row's gitlink is asked before any row's checkout, and `--no-members` prints the case-variant move, so a clone of a holder whose HEAD lost `.gitmodules` or of #363's state no longer ends `family bootstrap ok` or hides the variant behind `member-uninitialized` (#648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (a) `templates/family-root/scripts/bootstrap.py` said "every member and leg fetched" and ended `family bootstrap ok`, exit 0, over members it had not checked out: in a fresh recursive clone of a holder whose HEAD lost `.gitmodules` (git exits 0), and in a plain clone of that holder or of #363's state, where git dies `No url found for submodule path` (exit 128). After the fetch it now asks this holder's own records of each row whose member is not checked out: no gitlink at the row's path (`recorded_gitlink`, as `member-gitlink-absent` reads it), or no section of `.gitmodules` giving that path, or, where git exited 0, any such row. Each is named on one `NOT CHECKED OUT, and not for want of a credential` line with why, which names `validate-family.py`, and the run ends `family bootstrap FAILED:

(not checked out)`, exit 1. "every member and leg fetched" is said only where git exited 0 and every row is checked out. A row git could not fetch for want of a credential is still the skip it was (`test_bootstrap_degrades_when_a_member_cannot_be_fetched`), and a good holder's run prints what it printed. (b) `validate-family.py` asked each row whole, gitlink then checkout, before the next. In a plain clone of #363's state plus `git submodule update --init --recursive`, which dies at `Members/

` before it checks out any member, an earlier row was refused `member-uninitialized` (naming that same update) and the later row's variant refusal was never reached. `_check_member` now stops at the gitlink and returns the member (`Gitlinked`); `_check_member_checkout` asks the digest and identity of each once every row's gitlink has been asked. (c) Under `--no-members`, `member-gitlink-unnamed` for a member's only gitlink, in the wrong case, printed no command. Where the row's path is its canonical one and its pin a commit, it now prints the index move the row's members-mode refusal prints, word for word (`_wrong_case_move`, shared by both), to the pinned commit, every variant on one line. Docs: the validator's WHAT IS CHECKED docstring (the order; the move under --no-members) and bootstrap's (a) and (c). README.md's "A member it cannot fetch is reported and skipped, never a failure" is no longer true, so it says "One it lacks a credential for is reported and skipped, not a failure", in the same line count (README is at its 1449-line cap), and docs/handbook.html says the same, with its § Families marker set to the segment's new blob. Pinned copies changed: templates/family-root/scripts/bootstrap.py and templates/family-root/scripts/validate-family.py. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- docs/handbook.html | 4 +- templates/family-root/scripts/bootstrap.py | 135 ++++- .../family-root/scripts/validate-family.py | 177 +++++-- tests/test_family_bootstrap_gate_order.py | 468 ++++++++++++++++++ tests/test_validate_family_unnamed_mounts.py | 32 +- 6 files changed, 728 insertions(+), 90 deletions(-) create mode 100644 tests/test_family_bootstrap_gate_order.py diff --git a/README.md b/README.md index 2664a842..4ff2989f 100644 --- a/README.md +++ b/README.md @@ -942,7 +942,7 @@ at the right commit. **A family pins assembly roots, never legs**: the GitHub App or `SHAPE_LEGS_TOKEN` credential the workflow does, as a member is a private assembly root mounting two private legs — then runs each member's own `make bootstrap`; `make validate` runs the family validator, then each -member's. A member it cannot fetch is reported and skipped, never a failure. +member's. One it lacks a credential for is reported and skipped, not a failure. The holder carries the same COPY pin an assembly root does, so `update-shape.py` re-syncs it the same way; it mirrors into `family.yaml` diff --git a/docs/handbook.html b/docs/handbook.html index 801cc4ac..82ec73d9 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1268,7 +1268,7 @@

Adopting an existing repository

- +

Reference

Families: a holder for projects that ship separately

@@ -1361,7 +1361,7 @@

The InkRouter example

resolving the same GitHub App or SHAPE_LEGS_TOKEN credential the workflow does — and then runs each member’s own make bootstrap. make validate runs the family validator and then each member’s own - make validate. A member that could not be fetched is reported and skipped, never + make validate. A member it lacks a credential for is reported and skipped, not a failure. The holder carries the same COPY pin an assembly root does, so update-shape.py re-syncs it the same way; it mirrors into family.yaml instead of project.yaml and must leave diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index d9ddcc30..e3fb3b10 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -27,6 +27,18 @@ refused to do anything because one member is unreachable would be worse than one that says which. + "every member and leg fetched" is said only where git exited 0 AND + every row's member is checked out (#648). A member left NOT CHECKED + OUT where no credential would check it out is said here instead, with + why, and fails the run (c): this holder records no gitlink at the + row's path (a gitlink recorded only in another case, #363), no + section of `.gitmodules` gives that path (a holder whose HEAD lost the + file, #611), or git exited 0 and checked nothing out there. Git exits + 0 over the second where a recursive clone ran first, and 128 (`No url + found for submodule path`) in a plain clone, as it does for a missing + credential, so what the holder records is the test, never git's + words. The line names `validate-family.py`, which says what to commit. + (b) RUNS EACH MEMBER'S OWN `make bootstrap`. Each member is a whole project and knows how to bootstrap itself — its legs onto their tracking branches at their pinned commits, its own validators, its own @@ -35,9 +47,10 @@ `--members-make validate` runs `make validate` instead, which is what the family Makefile's `validate` target uses. - (c) EXITS 0 unless a member's own command failed. A member that is not - checked out is REPORTED and skipped, not a failure: it is the state a - missing credential leaves, and it is already reported in (a). + (c) EXITS 0 unless a member's own command failed, or (a) left a member not + checked out that no credential would check out. Any other member that + is not checked out is REPORTED and skipped, not a failure: it is the + state a missing credential leaves, and it is already reported in (a). THE FAMILY CONFERS NOTHING and neither does this command. It fetches, it delegates, and it says what it could not reach. @@ -54,10 +67,12 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 - Refusal, escape_unencodable_stdout, find_repo_root, load_yaml, spelled, + PYTHON, Refusal, escape_unencodable_stdout, find_repo_root, git_out, + load_yaml, recorded_gitlink, spelled, ) MANIFEST = "family.yaml" +GITMODULES = ".gitmodules" DEFAULT_MEMBERS_DIR = "members" #: The two environment variables a leg/member credential may arrive in, in the @@ -79,8 +94,13 @@ def credential() -> tuple[str, str]: return "none", "" -def fetch_members(root: Path) -> bool: - """(a) `git submodule update --init --recursive`, authenticated or not.""" +def fetch_members(root: Path, rows: list[dict]) -> list[str]: + """(a) `git submodule update --init --recursive`, authenticated or not. + + Returns a failure line for each of `rows` whose member it leaves not + checked out where no credential would check it out (`unfetchable`), for + the run to end FAILED on (#648); every member and leg is said to be + fetched only where there is none and git exited 0.""" source, token = credential() args = ["git"] if token: @@ -92,21 +112,83 @@ def fetch_members(root: Path) -> bool: sys.stdout.flush() proc = subprocess.run(args, cwd=str(root), capture_output=True, text=True, encoding="utf-8", errors="replace", check=False) - if proc.returncode == 0: + if proc.returncode != 0: + # The token can appear in git's own error text (it is in the URL it + # tried), so the output is scrubbed before it is printed. + message = (proc.stderr + proc.stdout).strip() + if token: + message = message.replace(token, "") + print(f" FETCH INCOMPLETE (git exit {proc.returncode}) using " + f"credential source '{source}':\n{message}", file=sys.stderr) + print(" If the members are PRIVATE, set SHAPE_LEGS_TOKEN (a " + "fine-grained PAT with contents:read on them) or export an " + "App-minted SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive " + "is bootstrapped below.", file=sys.stderr) + left = unfetchable(root, rows, proc.returncode == 0) + if left: + print(" NOT CHECKED OUT, and not for want of a credential: " + + ", ".join(f"{spelled(path)} ({why})" for _, path, why in left) + + f". `{PYTHON} scripts/validate-family.py` names the cause and " + "the commit that cures it.", file=sys.stderr) + elif proc.returncode == 0: print(" every member and leg fetched") - return True - # The token can appear in git's own error text (it is in the URL it tried), - # so the output is scrubbed before it is printed. - message = (proc.stderr + proc.stdout).strip() - if token: - message = message.replace(token, "") - print(f" FETCH INCOMPLETE (git exit {proc.returncode}) using credential " - f"source '{source}':\n{message}", file=sys.stderr) - print(" If the members are PRIVATE, set SHAPE_LEGS_TOKEN (a fine-grained " - "PAT with contents:read on them) or export an App-minted " - "SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive is bootstrapped " - "below.", file=sys.stderr) - return False + return [f"{spelled(row.get('project') or '?')} (not checked out)" + for row, _, _ in left] + + +def gitmodules_paths(root: Path) -> set[str]: + """Every path a section of `.gitmodules` gives, read as git reads it: + the work tree's file, the one `git submodule update` reads, which a + fresh clone takes from HEAD. None where there is no such file, or one + git cannot read: git has no url for any submodule then either.""" + if not os.path.lexists(root / GITMODULES): + return set() + try: + listed = git_out(["config", "-z", "--file", GITMODULES, "--list"], + cwd=root, binary=True) + except Refusal: + return set() + paths: set[str] = set() + for record in listed.split(b"\0"): + key, _, value = record.decode("utf-8", + "surrogateescape").partition("\n") + if key.startswith("submodule.") and key.endswith(".path"): + paths.add(value) + return paths + + +def unfetchable(root: Path, rows: list[dict], + exited_0: bool) -> list[tuple[dict, str, str]]: + """`(row, path, why)` for each row whose member (a) left NOT CHECKED OUT + where no credential would check it out (#648), in row order. + + Asked of this holder's own records, never of git's words: a row at + whose path the holder records no gitlink (`recorded_gitlink`, + as `validate-family.py`'s `member-gitlink-absent` reads one), or whose + gitlink no section of `.gitmodules` gives a path (as its + `member-gitmodules-absent` reads HEAD's file, which is the work tree's + in a clone); and, where git exited 0, any row at all, since git then + lacked nothing a credential gives. Any other row not checked out is the + missing credential's, and stays the skip it always was. A path two rows + give is said once.""" + left: dict[str, dict] = {} + for row in rows: + if not (member_path(root, row) / ".git").exists(): + left.setdefault(row_path(row), row) + if not left: + return [] + given = gitmodules_paths(root) + found: list[tuple[dict, str, str]] = [] + for path, row in left.items(): + if recorded_gitlink(root, path) is None: + found.append((row, path, "no gitlink is recorded at that path")) + elif path not in given: + found.append((row, path, f"no section of {GITMODULES} gives that " + "path")) + elif exited_0: + found.append((row, path, "git exited 0 and checked nothing out " + "there")) + return found def members_of(root: Path) -> tuple[str, list[dict]]: @@ -128,9 +210,14 @@ def members_of(root: Path) -> tuple[str, list[dict]]: return name, rows +def row_path(row: dict) -> str: + """The path a row mounts its member at, as the holder spells it.""" + return str(row.get("path") + or f"{DEFAULT_MEMBERS_DIR}/{row.get('project')}") + + def member_path(root: Path, row: dict) -> Path: - return root / str(row.get("path") - or f"{DEFAULT_MEMBERS_DIR}/{row.get('project')}") + return root / row_path(row) def run_member_target(root: Path, row: dict, target: str) -> str | None: @@ -187,13 +274,13 @@ def main(argv: list[str] | None = None) -> int: f"{len(rows)} member(s)") print("\n(a) members and their legs") + failed: list[str] = [] if args.no_fetch: print(" skipped (--no-fetch)") else: - fetch_members(root) + failed += fetch_members(root, rows) print(f"\n(b) each member's own `make {spelled(args.members_make)}`") - failed: list[str] = [] if args.skip_members: print(" skipped (--skip-members)") elif not rows: diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index d4283032..aca84f2b 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -43,7 +43,11 @@ 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, the index move that puts it back, and what to do - with a directory the variant leaves on disk (#611). + with a directory the variant leaves on disk (#611). The first is asked + of every row before the other two are asked of any (#648), so a later + row's gitlink refusal is never hidden behind an earlier row's + `member-uninitialized`, as a plain clone whose `git submodule update` + died at that gitlink left it. 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. @@ -59,7 +63,10 @@ them ever asked. Neither needs a member checked out, so `--no-members` asks them too. A path a row names in another case is still one of these, and what it says to do depends on which of the two is the member's own: - it names no command that would un-register the member. + it names no command that would un-register the member, and for a member + recorded only in the wrong case it names the index move that records it + at its row's path, as the row's own refusal does members checked + (#648). - that a section of HEAD's `.gitmodules` gives the path of every row's 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 @@ -456,11 +463,12 @@ def _gitlink_in_another_case(path: str, commit: str, project: str, 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. - `--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`). + prints the same move for the same gitlink under `--no-members` (#648), + and no command that takes it out ALONE, which 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 @@ -473,11 +481,6 @@ def _gitlink_in_another_case(path: str, commit: str, project: str, """ 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 " @@ -485,17 +488,31 @@ def _gitlink_in_another_case(path: str, commit: str, project: str, "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, 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" + "Remediation: the member is recorded in the wrong case. " + + _wrong_case_move(path, commit, variants)) + + +def _wrong_case_move(path: str, commit: str, variants: list[str]) -> str: + """The index move for a member recorded only at `variants`, case + variants of its row's `path`, to `path` at the pinned `commit`, with + what it leaves on disk: what `_gitlink_in_another_case` says, and + `member-gitlink-unnamed` under `--no-members` (#648), in one text.""" + 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 ( + "Move its gitlink to the row's path, at the commit the row pins, in " + "the index alone, which touches no file, with the two lines below, " + f"and commit 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 " + f"is, a directory left at {' or '.join(variants)} stays there, " + "untracked, and family.py add refuses it as " + "family-members-dir-variant: move each one that is there out of the " + f"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}')}") @@ -554,9 +571,22 @@ def _check_member_tree(member: Path, pin: dict, path: str, commit: str, report.note(f"{path}: tree digest recomputes ({recorded[:12]}…)") +class Gitlinked(NamedTuple): + """A member `_check_member` asked up to its gitlink, with what + `_check_member_checkout` reads its checkout by.""" + + row: dict + project: str + path: str + pin: dict + commit: str + + 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. + gitlinks: dict[str, str]) -> Gitlinked | None: + """One member up to its checkout: the row itself and the gitlink. The + member, for `_check_member_checkout` to ask its digest and identity of, + or None where nothing more is asked of it. NOTHING IS READ THROUGH A PATH THE ROW MAY NOT HAVE (#367). A hand-edited `path:` is `member-path`, and this member's checks stop once its pin is @@ -570,18 +600,10 @@ def _check_member(root: Path, row: dict, seen: dict, report: Report, 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: - return + return None path = _check_member_repository_and_path(row, project, report) @@ -589,23 +611,38 @@ def _check_member(root: Path, row: dict, seen: dict, report: Report, if not isinstance(pin, dict): report.finding("member-pin", f"{project}: `pin:` is {pin!r}, expected " "a mapping") - return + return None commit = _pin_commit(pin, project, report) if path is None: - return + return None # ---- the gitlink ------------------------------------------------------- with _git_runs(f"the gitlink recorded at {path} could not be read"): _check_member_gitlink(root, path, commit, project, gitlinks, report) + return Gitlinked(row, project, path, pin, commit) + - # ---- the digest and the identity -------------------------------------- - member = root / path - if path in unregistered and not (member / ".git").exists(): +def _check_member_checkout(root: Path, asked: Gitlinked, report: Report, + unregistered: set[str]) -> None: + """One member's digest and identity, read in its checkout, asked once + every row's gitlink has been (`_check_members`, #648). + + A MEMBER NO SECTION OF HEAD'S `.gitmodules` GIVES (`unregistered`, the + `member-gitmodules-absent` finding, #611) that is not checked out is + asked nothing here. 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. + """ + member = root / asked.path + if asked.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) - _check_member_identity(member, row, project, report) + with _git_runs(f"{asked.project}'s pinned commit and its tree digest " + f"could not be read in {asked.path}"): + _check_member_tree(member, asked.pin, asked.path, asked.commit, + asked.project, report) + _check_member_identity(member, asked.row, asked.project, report) def _check_member_identity(member: Path, row: dict, project: str, @@ -1074,11 +1111,14 @@ def _member_gitlinks(root: Path) -> dict[str, str]: class RowPath(NamedTuple): """What a row names by the path it gives: `who` names the row in a - finding, and `canonical` is the one path its project may have, - `members/`, or None for a row with no `project:`.""" + finding, `canonical` is the one path its project may have, + `members/`, or None for a row with no `project:`, and `pin` is + the commit its `pin:` names, lowered, or None where that is no 40-hex + commit (#648).""" who: str canonical: str | None + pin: str | None = None def _row_paths(members: list) -> dict[str, RowPath]: @@ -1109,9 +1149,11 @@ def _row_paths(members: list) -> dict[str, RowPath]: path = canonical else: continue + pin = row.get("pin") + commit = str(pin.get("commit") or "") if isinstance(pin, dict) else "" named.setdefault(path, RowPath( f"the row for {project}" if project else "a row with no project:", - canonical)) + canonical, commit.lower() if COMMIT_RE.match(commit) else None)) return named @@ -1190,6 +1232,10 @@ def _variant_advice(kind: str, path: str, row_path: str, row: RowPath, is wrong, and where to put its path back; a `WRONG_CASE` says the member is recorded in the wrong case, and sends the operator to `git log -p -- family.yaml`, as an unnamed path that no row folds to does (`RESTORE_ROW`). + A `WRONG_CASE` gitlink whose row's path is its canonical one and whose + pin is a commit never comes here: `_check_gitlinks_named` prints the + index move for it, which records the member as it takes the gitlink out + (#648). """ if kind == ROW_EDITED: return (f" It is {path}, the one path {row.who} may have, and the row " @@ -1220,9 +1266,15 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], still a finding, and says which of three it is (`_variant_case`). A second path to the member's own directory is taken out of the index alone with `git rm --cached`, which touches no file, and committed. The - member's own gitlink, which the ROW misspells, and the only record of a - member there is, in the wrong case, print no command: either could - un-register the member. Any other is taken out with a plain `git rm`, + member's own gitlink, which the ROW misspells, prints no command, since + taking it out could un-register the member. The only record of a member + there is, in the wrong case, prints the index move members mode's + `member-gitlink-absent` prints (`_wrong_case_move`, #648), which records + the member at its row's path as it takes the variants out, so + `--no-members`, the run CI falls back to when the members cannot be + checked out, names the cure too; where the row's path is not its + canonical one, or its pin no commit, it prints no command either. Any + other is taken out with a plain `git rm`, which refuses while its checkout holds changes, then mounted with `family.py add`: `add` cannot adopt an existing mount, and after `git rm --cached` alone it refuses the checkout left in the way @@ -1247,6 +1299,16 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], if variant is not None: row_path, row = variant kind = _variant_case(path, row_path, row, holds) + if kind == WRONG_CASE and row_path == row.canonical and row.pin: + variants = sorted(other for other in holds + if other != row_path + and other.casefold() == row_path.casefold()) + report.finding( + "member-gitlink-unnamed", + f"{said}{_variant_said(row_path, row)} The member is " + "recorded in the wrong case. " + + _wrong_case_move(row_path, row.pin, variants)) + continue if kind != STRAY: report.finding( "member-gitlink-unnamed", @@ -1436,6 +1498,16 @@ def _check_members(root: Path, members: list, no_members: bool, `.gitmodules` gives twice (#300), then the gitlinks and sections no row names, which read the index and HEAD as well (#361), and the gitlinks no section of HEAD's `.gitmodules` gives (#611). + + THEN EVERY ROW'S GITLINK, AND ONLY THEN ANY ROW'S CHECKOUT (#648). Each + row used to be asked whole before the next was asked at all, so in a + plain clone of a holder whose later row's gitlink is recorded only in + another case, where `git submodule update --init --recursive` dies at + that gitlink (`No url found for submodule path`, exit 128) before it + checks out any member, an earlier row was refused + `member-uninitialized`, naming that same update, and the later row's + `member-gitlink-absent`, which names the move that cures both, was + never reached. """ _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) @@ -1453,12 +1525,17 @@ def _check_members(root: Path, members: list, no_members: bool, "here") return seen: dict[str, bool] = {} + gitlinked: list[Gitlinked] = [] for row in members: if not isinstance(row, dict): report.finding("member-row", f"a member is not a mapping: {row!r}") continue - _check_member(root, row, seen, report, gitlinks, unregistered) + asked = _check_member(root, row, seen, report, gitlinks) + if asked is not None: + gitlinked.append(asked) + for asked in gitlinked: + _check_member_checkout(root, asked, report, unregistered) if not members: report.note("no members yet: a family with none is empty, not " "wrong") diff --git a/tests/test_family_bootstrap_gate_order.py b/tests/test_family_bootstrap_gate_order.py new file mode 100644 index 00000000..9962f731 --- /dev/null +++ b/tests/test_family_bootstrap_gate_order.py @@ -0,0 +1,468 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A holder's own bootstrap and gate after a clone, in every order and mode +(#648). + +THE ISSUE'S THREE LINES, each reproduced on #639's head before this change: + +- (a) `scripts/bootstrap.py` ended `family bootstrap ok`, exit 0, having + checked no member out. In a fresh recursive clone of a holder whose HEAD + lost `.gitmodules`, `git submodule update` exits 0, and it said "every + member and leg fetched", `NOT CHECKED OUT; skipped` for each member, then + ok. In a plain clone of that holder, or of #363's state (a member's + gitlink recorded only at `Members/

`), git dies `No url found for + submodule path` (exit 128), and it still ended ok. It fails now, saying + which members no credential would check out and why, and names + `validate-family.py`. A missing credential is still the skip it was. +- (b) In a plain clone of #363's state followed by `git submodule update + --init --recursive`, as CI and `make bootstrap` run it, the update dies at + `Members/

` before it checks out any member, so where that row is not + the first, the gate refused `member-uninitialized` for an earlier row, + naming that same update, and never reached the variant's refusal. Every + row's gitlink is asked before any row's checkout now. +- (c) Under `--no-members`, the mode CI falls back to there, + `member-gitlink-unnamed` for that gitlink printed no command. It prints + the index move members mode prints now. + +THE SAME FAMILY, OFFLINE RULES AND STATES as +`test_family_head_lost_gitmodules.py` (#611), whose helpers are imported. +Every remedy is typed as printed. The case-variant states are index records +and commits, never two directories whose names differ only in case, and +the holder's own are asked again with `core.ignorecase=true`. A clone's +checkout is git's: on a disk that folds case, `Members/

` IS the row's +directory, so the `mv` the text gives for a case-sensitive disk is typed +only where the disk holds the two apart, as the text says. +""" + +from __future__ import annotations + +import itertools +import os +import re +import shlex +import subprocess +from pathlib import Path + +import pytest + +from conftest import clone_just_pushed, git, run_script +from repo_shape import PYTHON +from test_family import NAME, family, validate # noqa: F401 +from test_family_add_refusals import holder, offline # noqa: F401 +from test_family_dry_run import isolated +from test_family_head_lost_gitmodules import ( + DISKS, assert_passes_in_every_mode, commit_all, head_lost, on_disk, + pinned, recorded_at, variant_refused, +) +from test_family_member_path import MEMBER, OTHER, raw +from test_validate_family_unnamed_mounts import ( + GITLINK_SAID, VARIANT, WRONG_CASE, commands, findings, stage_gitlink, +) + +#: The line `(a)` prints for the members it left NOT CHECKED OUT where no +#: credential would check them out, and the one the run then ends on. +NOT_FETCHED = (" NOT CHECKED OUT, and not for want of a credential: {left}. " + f"`{PYTHON} scripts/validate-family.py` names the cause and " + "the commit that cures it.") +NO_SECTION = "no section of .gitmodules gives that path" +NO_GITLINK = "no gitlink is recorded at that path" +EXITED_0 = "git exited 0 and checked nothing out there" + +#: Each test pushes a branch of its own into the module's family remote. Not +#: `tmp_path.name`: a passed test's directory is removed under +#: `tmp_path_retention_policy=failed`, and the next test is given its number. +BRANCHES = itertools.count() + +#: What members mode's variant refusal says before the move it prints. +WRONG_CASE_LEAD = "Remediation: the member is recorded in the wrong case. " + +#: `bootstrap.py --skip-members`'s stdout after its first line, in a clone +#: where every member was checked out, and where not every one was: the +#: second says nothing of (a)'s fetch on stdout, and ends on stderr. +FETCHED = ["", "(a) members and their legs", " credential source: none", + " every member and leg fetched", "", + "(b) each member's own `make bootstrap`", + " skipped (--skip-members)", "", "family bootstrap ok", ""] +NOT_ALL_FETCHED = ["", "(a) members and their legs", + " credential source: none", "", + "(b) each member's own `make bootstrap`", + " skipped (--skip-members)", "", ""] + + +def with_config(env: dict, key: str, value: str) -> dict: + """`env` with `key` set to `value`, one more `GIT_CONFIG_*` entry after + the ones it already holds, for every git the run starts.""" + index = int(env.get("GIT_CONFIG_COUNT", "0")) + return {**env, "GIT_CONFIG_COUNT": str(index + 1), + f"GIT_CONFIG_KEY_{index}": key, f"GIT_CONFIG_VALUE_{index}": value} + + +def fetching(env: dict) -> dict: + """`env` under which `git submodule update` may clone the fixture's + local members: `protocol.file.allow`, as `test_family.py` gives the + holder's own bootstrap.""" + return with_config(env, "protocol.file.allow", "always") + + +def variant(holder: Path, env: dict, project: str = OTHER) -> str: + """#363's state: `project`'s gitlink recorded only at `Members/

`, + committed, its row and section still `members/

`. OTHER is the + second row, so a plain clone's update dies before the first row's + checkout. Returns the commit.""" + return recorded_at(holder, env, project, f"Members/{project}") + + +def cloned(holder: Path, env: dict, family, tmp_path: Path, + *, recurse: bool) -> Path: + """HEAD pushed to a branch of the family's own remote that no other + test pushes (`BRANCHES`), then a fresh clone of it, recursive or plain, + which must exit 0.""" + remote = family["remotes"] / f"{NAME}.git" + branch = f"w648/{next(BRANCHES)}" + raw(holder, env, "push", "-q", str(remote), f"HEAD:refs/heads/{branch}") + clone = tmp_path / "clone" + made = clone_just_pushed(remote, clone, branch, recurse=recurse) + assert made.returncode == 0, made.stdout + made.stderr + return clone + + +def bootstrap(root: Path, env: dict, *extra: str): + return run_script(root / "scripts" / "bootstrap.py", *extra, cwd=root, + env=env) + + +def not_fetched(*left: tuple[str, str]) -> str: + """`NOT_FETCHED` naming each `(path, why)`.""" + return NOT_FETCHED.format( + left=", ".join(f"{path} ({why})" for path, why in left)) + + +def failed_on(*projects: str) -> str: + return "family bootstrap FAILED: " + ", ".join( + f"{project} (not checked out)" for project in projects) + + +def after_first_line(result) -> list[str]: + """Bootstrap's stdout after its first line, which names the holder and + its two members.""" + lines = result.stdout.split("\n") + assert lines[0].startswith(f"bootstrap: {NAME} ("), result.stdout + assert lines[0].endswith(" 2 member(s)"), result.stdout + return lines[1:] + + +def other_older(family) -> str: + """A commit of OTHER that is not the one the holder pins.""" + return git("rev-parse", "main~1", + cwd=family["remotes"] / f"{OTHER}.git").stdout.strip() + + +def type_line(root: Path, env: dict, line: str) -> None: + """One printed `git` line, typed as printed: split as a POSIX shell + splits it, run with no shell.""" + argv = shlex.split(line) + assert argv[0] == "git", line + raw(root, env, *argv[1:]) + + +def moved_out_where_apart(root: Path, remediation: str) -> None: + """Each backticked `mv` the text gives for a case-sensitive disk, typed + as printed where the disk holds its source apart from the row's own + directory, as the text says; on a disk that folds case it is the row's + directory, and the text says to leave it.""" + for move in re.findall(r"`(mv -- [^`]*)`", remediation): + argv = shlex.split(move) + source = root / argv[2] + own = root / "members" / argv[2].split("/", 1)[1] + if not source.exists() or (own.exists() + and os.path.samefile(source, own)): + continue + subprocess.run(argv, cwd=str(root), check=True) + assert not source.exists() + + +# --- (a) bootstrap ----------------------------------------------------------- + +#: `(set-up, flow, the members (a) names with why, the rows it fails on)`. +UNFETCHED = { + "head-lost-recursive": ( + head_lost, True, + ((f"members/{MEMBER}", NO_SECTION), (f"members/{OTHER}", NO_SECTION)), + (MEMBER, OTHER)), + "head-lost-plain": ( + head_lost, False, + ((f"members/{MEMBER}", NO_SECTION), (f"members/{OTHER}", NO_SECTION)), + (MEMBER, OTHER)), + "variant-recursive": ( + variant, True, ((f"members/{OTHER}", NO_GITLINK),), (OTHER,)), + "variant-plain": ( + variant, False, ((f"members/{OTHER}", NO_GITLINK),), (OTHER,)), +} + + +@pytest.mark.parametrize("set_up, recurse, left, projects", + UNFETCHED.values(), ids=UNFETCHED) +def test_bootstrap_fails_over_members_no_credential_would_check_out( + family, holder, offline, tmp_path, set_up, recurse, left, projects): + """THE ISSUE'S (a), both of rv611b's states, after a recursive clone and + after a plain one. Git exits 0 over a holder whose HEAD lost + `.gitmodules` where the clone was recursive, and 128 (`No url found for + submodule path`) where it was plain, and 128 over #363's state in a + plain clone: the run said ok each time. It names the members no + credential would check out, says why, names `validate-family.py`, and + fails; it never says every member was fetched.""" + env = isolated(offline, tmp_path) + set_up(holder, env) + clone = cloned(holder, env, family, tmp_path, + recurse=recurse) + + result = bootstrap(clone, fetching(env), "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert after_first_line(result) == NOT_ALL_FETCHED + errors = result.stderr.splitlines() + assert errors[-2:] == [not_fetched(*left), failed_on(*projects)], ( + result.stderr) + if not recurse: + assert errors[0].startswith(" FETCH INCOMPLETE (git exit 128) "), ( + result.stderr) + assert "No url found for submodule path" in result.stderr + + +def test_a_good_holder_bootstraps_as_it_did(family, holder, offline, + tmp_path): + """A GOOD HOLDER'S RUN IS KEPT: a fresh recursive clone checks every + member out, (a) says every member and leg was fetched, and the run ends + ok with nothing on stderr.""" + env = isolated(offline, tmp_path) + clone = cloned(holder, env, family, tmp_path, recurse=True) + + result = bootstrap(clone, fetching(env), "--skip-members") + + assert result.returncode == 0, result.stdout + result.stderr + assert after_first_line(result) == FETCHED + assert result.stderr == "" + + +def test_a_missing_credential_is_still_the_skip_it_was(family, holder, + offline, tmp_path): + """No misfire: a plain clone whose update git refuses for want of a + credential (no `protocol.file.allow` for the local members, as a + private member with no token is refused) records every gitlink at its + row's path, and `.gitmodules` gives each. A credential is the cure, so + the members are skipped, the run ends ok, and no line says otherwise.""" + env = isolated(offline, tmp_path) + clone = cloned(holder, env, family, tmp_path, + recurse=False) + + result = bootstrap(clone, env) + + assert result.returncode == 0, result.stdout + result.stderr + assert "FETCH INCOMPLETE" in result.stderr + assert "not for want of a credential" not in result.stderr + for project in (MEMBER, OTHER): + assert (f" [{project}] {project}: NOT CHECKED OUT; skipped. See the " + "fetch above.") in result.stdout.splitlines() + assert result.stdout.endswith("\nfamily bootstrap ok\n"), result.stdout + + +def test_an_update_that_exits_0_and_skips_a_member_fails(family, holder, + offline, tmp_path): + """Git exited 0, so it lacked nothing a credential gives, though the + holder's records give OTHER: here `submodule..update=none` set + for the run skips it. (a) does not say every member was fetched, and + the run fails on OTHER alone.""" + env = isolated(offline, tmp_path) + clone = cloned(holder, env, family, tmp_path, + recurse=False) + skip = with_config(fetching(env), f"submodule.members/{OTHER}.update", + "none") + + result = bootstrap(clone, skip, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert (clone / "members" / MEMBER / "project.yaml").is_file() + assert "every member and leg fetched" not in result.stdout + assert result.stderr.splitlines() == [ + not_fetched((f"members/{OTHER}", EXITED_0)), failed_on(OTHER)] + + +def test_a_row_written_twice_is_named_once(family, holder, offline, + tmp_path): + """OTHER's row written twice in a holder whose HEAD lost `.gitmodules`: + one path, so one name in (a) and one failure.""" + env = isolated(offline, tmp_path) + head_lost(holder, env) + rows = (holder / "family.yaml").read_bytes() + start = rows.index(f" - project: {OTHER}\n".encode()) + (holder / "family.yaml").write_bytes(rows + rows[start:]) + raw(holder, env, "add", "--", "family.yaml") + commit_all(holder, env, "OTHER's row written twice") + clone = cloned(holder, env, family, tmp_path, recurse=True) + + result = bootstrap(clone, fetching(env), "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.splitlines() == [ + not_fetched((f"members/{MEMBER}", NO_SECTION), + (f"members/{OTHER}", NO_SECTION)), + failed_on(MEMBER, OTHER)] + + +# --- (a) and (b) together: the plain clone of #363's state, typed through --- + +def test_a_plain_clone_of_a_case_variant_is_named_and_mended_as_printed( + family, holder, offline, tmp_path): + """The round trip a person makes. In a plain clone of #363's state, with + the variant at the SECOND row, `make bootstrap`'s script fails naming + OTHER and the validator; the validator, members checked, names the + variant and the move, where it refused `member-uninitialized` for the + first row before. Typed as printed in the clone, with the commit and + the `mv` where the disk holds the two apart, the same script checks + both members out and runs each one's own bootstrap, and the gate passes + in every mode.""" + env = fetching(isolated(offline, tmp_path)) + commit = variant(holder, env) + clone = cloned(holder, env, family, tmp_path, recurse=False) + + booted = bootstrap(clone, env) + + assert booted.returncode == 1, booted.stdout + booted.stderr + assert booted.stderr.splitlines()[-2:] == [ + not_fetched((f"members/{OTHER}", NO_GITLINK)), failed_on(OTHER)] + gate = validate(clone) + assert gate.returncode == 2, gate.stdout + gate.stderr + lines = gate.stderr.splitlines() + assert lines == variant_refused(f"members/{OTHER}", commit, + [f"Members/{OTHER}"], + {f"Members/{OTHER}": commit}) + for line in lines[2:]: + type_line(clone, env, line) + commit_all(clone, env, "Move the gitlink as printed") + moved_out_where_apart(clone, lines[1]) + + again = bootstrap(clone, env) + + assert again.returncode == 0, again.stdout + again.stderr + assert " every member and leg fetched" in again.stdout.splitlines() + for project in (MEMBER, OTHER): + assert f" --- {project}: make bootstrap ---" in again.stdout + assert again.stdout.endswith("\nfamily bootstrap ok\n"), again.stdout + assert_passes_in_every_mode(clone, 2) + + +# --- (b) the gate asks every row's gitlink before any row's checkout -------- + +@pytest.mark.parametrize("project", [MEMBER, OTHER], ids=["first", "second"]) +def test_a_plain_clone_after_the_update_names_the_variant_in_any_row( + family, holder, offline, tmp_path, project): + """THE ISSUE'S (b), as CI and `make bootstrap` run it: a plain clone, + then `git submodule update --init --recursive`, which dies at + `Members/

` (exit 128) before it checks out either member. Members + checked and `--pins` alike, the gate refuses the variant with the move, + whichever row it is; for the second it refused `member-uninitialized` + for the first row, naming that same update. Under `--no-members`, CI's + fallback, the finding prints the same two lines (c); typed as printed in + the clone and committed, the update checks both out and the gate + passes.""" + env = fetching(isolated(offline, tmp_path)) + commit = variant(holder, env, project) + clone = cloned(holder, env, family, tmp_path, recurse=False) + update = raw(clone, env, "submodule", "update", "--init", "--recursive", + check=False) + assert update.returncode == 128, update.stderr + assert (f"No url found for submodule path 'Members/{project}'".encode() + in update.stderr) + expected = variant_refused(f"members/{project}", commit, + [f"Members/{project}"], + {f"Members/{project}": commit}) + + for extra in ((), ("--pins",)): + gate = validate(clone, *extra) + assert gate.returncode == 2, (extra, gate.stdout + gate.stderr) + assert gate.stderr.splitlines() == expected, extra + fallback = validate(clone, "--no-members") + assert fallback.returncode == 1, fallback.stdout + fallback.stderr + (finding,) = findings(fallback) + assert commands(finding) == [line.strip() for line in expected[2:]] + + for line in commands(finding): + type_line(clone, env, line) + commit_all(clone, env, "Move the gitlink as CI's gate printed") + moved_out_where_apart(clone, finding) + raw(clone, env, "submodule", "update", "--init", "--recursive") + assert_passes_in_every_mode(clone, 2) + + +# --- (c) `--no-members` prints the move members mode prints ----------------- + +def unnamed_moved(variant_path: str, at: str, pin: str, + variants: list[str]) -> str: + """`member-gitlink-unnamed` WHOLE for the gitlink at `variant_path` (at + `at`), OTHER recorded only at `variants`: what every unnamed gitlink + says, the row it is a variant of, and members mode's move to the pin, + word for word.""" + path = f"members/{OTHER}" + refused = variant_refused(path, pin, variants, + dict.fromkeys(variants, at)) + assert refused[1].startswith(WRONG_CASE_LEAD + "Move its gitlink "), ( + "premise: members mode's remedy text") + return "\n".join([ + GITLINK_SAID.format(path=variant_path, commit=at) + + VARIANT.format(row_path=path, project=OTHER) + + " The member is recorded in the wrong case. " + + refused[1][len(WRONG_CASE_LEAD):], *refused[2:]]) + + +@DISKS +@pytest.mark.parametrize("spellings", [ + ("Members/{p}",), ("MEMBERS/{p}", "Members/{p}")], ids=["one", "two"]) +def test_no_members_prints_the_move_members_mode_prints( + family, holder, offline, tmp_path, ignorecase, spellings): + """THE ISSUE'S (c). OTHER's only gitlink at a case variant (two, in the + second case), at a commit that is not its pin. Under `--no-members` each + variant's `member-gitlink-unnamed` printed no command; it prints the + move members mode prints now, to the commit the row PINS, every variant + taken out on one line. Typed as printed and committed, on either disk, + the holder passes in every mode.""" + env = on_disk(isolated(offline, tmp_path), ignorecase) + variants = sorted(spelling.format(p=OTHER) for spelling in spellings) + at = other_older(family) + pin = pinned(holder, OTHER) + assert pin != at, "premise: the variant is not at the pin" + raw(holder, env, "rm", "-q", "--cached", "--", f"members/{OTHER}") + for spelling in variants: + stage_gitlink(holder, spelling, at) + commit_all(holder, env, f"{OTHER} recorded in another case") + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result) == [unnamed_moved(path, at, pin, variants) + for path in variants] + for line in commands(findings(result)[0]): + type_line(holder, env, line) + commit_all(holder, env, "Move the gitlink as printed") + assert_passes_in_every_mode(holder, 2) + + +def test_a_row_whose_pin_is_no_commit_still_prints_no_command( + holder, offline, tmp_path): + """The move records the gitlink at the commit the row pins, so a row + whose `pin.commit` is no 40-hex commit (members checked, the refusal + `member-pin-commit`) gets the text it had, and no command.""" + env = isolated(offline, tmp_path) + commit = variant(holder, env) + rows = (holder / "family.yaml").read_bytes() + line = f' commit: "{commit}"\n'.encode() + assert rows.count(line) == 1, "fixture drift: OTHER's pin line" + (holder / "family.yaml").write_bytes( + rows.replace(line, f' commit: "{commit[:12]}"\n'.encode())) + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stdout + result.stderr + (finding,) = findings(result) + assert finding == GITLINK_SAID.format( + path=f"Members/{OTHER}", commit=commit) + WRONG_CASE.format( + row_path=f"members/{OTHER}", project=OTHER, what="gitlink") + assert "git rm" not in result.stderr, result.stderr diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py index 3144f710..a9ab815c 100644 --- a/tests/test_validate_family_unnamed_mounts.py +++ b/tests/test_validate_family_unnamed_mounts.py @@ -28,7 +28,9 @@ the wrong case: the same command, run in either, takes the member's own record out of the index, after which `--no-members`, the run a private family's CI falls back to, says `family ok` over a member that is no longer -there. Those two print no command. +there. Those two print no such command: the first none, and the second, +since #648, the index move members mode prints, which records the member +at its row's path as it takes the variant out. When a hand mount's gitlink says to `git rm` it, its section finding says that the `git rm` takes the section out too and prints no `--unset-all` line: run @@ -512,14 +514,13 @@ def test_a_row_hand_edited_into_a_case_variant_of_its_mount_is_the_edit( @MODES -def test_a_member_recorded_only_in_another_case_says_so_and_prints_no_command( +def test_a_member_recorded_only_in_another_case_says_so_and_prints_the_move( holder, extra): """STATE (ii), REVIEW SHOULD-1: IRSS's row is canonical, and the only gitlink the holder records for it is at `members/irss`. `git rm --cached` of that took the member's only record out, and `--no-members` then said `family ok` over a holder that records one gitlink fewer than it has rows. - It says the member is recorded in the wrong case instead, and sends the - operator to git's history, as every other unnamed gitlink does. + It says the member is recorded in the wrong case instead. Members checked, the row's own gitlink is read at its exact path, finds none and is the refusal `member-gitlink-absent`, which prints no finding, @@ -528,26 +529,31 @@ def test_a_member_recorded_only_in_another_case_says_so_and_prints_no_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).""" + (`tests/test_family_head_lost_gitmodules.py` types it), and since #648 + the finding prints the same two lines + (`tests/test_family_bootstrap_gate_order.py` types them).""" named = record_in_another_case(holder, "IRSS", "members/irss") result = validate(holder, *extra) assert "--unset-all" not in result.stderr + result.stdout + move = [ + " git rm -r --cached --ignore-unmatch -- members/irss", + f" git update-index --add --cacheinfo 160000,{named},members/IRSS"] 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 --ignore-unmatch -- members/irss", - " git update-index --add --cacheinfo " - f"160000,{named},members/IRSS"], result.stderr + assert result.stderr.splitlines()[-2:] == move, 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( - path="members/irss", commit=named) + WRONG_CASE.format( - row_path="members/IRSS", project="IRSS", what="gitlink") + said = GITLINK_SAID.format(path="members/irss", commit=named) + ( + VARIANT.format(row_path="members/IRSS", project="IRSS")) + assert gitlink.startswith( + said + " 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, "), gitlink + assert gitlink.splitlines()[1:] == move, gitlink @MODES From 4e2b211bd2442be2c671153216e5f6734a61ebf0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:26:26 +0000 Subject: [PATCH 05/10] bootstrap.py and AGENTS-shape.md: a leg with an operation in progress or an ignored file in the way is refused, P9 names the branch it left, a HEAD outside `refs/heads/` is left on every path, and no git is `git-not-runnable`, so `make bootstrap` no longer ends a merge, overwrites a person's file or ends in a traceback (#704) (#694) #704, measured on lane 5's #682 head 8b39cd19 with git 2.43.0: - Line 1. The four moves that change files (P9's checkout of the branch at the pin from another branch, T7's checkout after `branch -f`, the on-branch `merge --ff-only` and the missing branch's `checkout -b`) overwrote an IGNORED file at a path the pin tracks, silently. Each now carries `--no-overwrite-ignore`, so git refuses it as it refuses an untracked file, in its own words. The missing-branch path now prints git's lines too, through `_say_refused`, and says where the leg stays (`leaving it on `), where it said `leaving detached` of a leg on a branch. - Line 2 (Brett Heap's R1). A checkout ENDED a merge, a cherry-pick or a revert in progress (head file gone, staged result carried onto the branch) from another branch, from HEAD detached at the pin and from HEAD already on the branch at the pin, and put a stopped rebase or `am` on the branch. `checkout_tracking_branch` now reads the leg's git dir (`rev-parse --git-dir`) for `rebase-apply/applying`, `rebase-apply`, `rebase-merge`, `MERGE_HEAD`, `CHERRY_PICK_HEAD` and `REVERT_HEAD`, in that order, and refuses before any path is chosen: it names the operation, says nothing moved, and prints `git -C --continue` and `--abort`. - Line 3. `place_at_pin` calls `_say_left` when HEAD was on another branch, as `fast_forward_to_pin` does. - Line 4 (R2). `templates/assembly-root/AGENTS-shape.md` now says bootstrap places a leg at the pin ONLY where that loses no commit, and otherwise leaves its checkout where it is, saying why. The code is unchanged. - Line 5 (the issue's recommended answer). A HEAD that is a symbolic ref outside `refs/heads/` is left by `place_at_pin` (`which is neither`, as T7 already said) and by the missing-branch path (`which is not a branch`), since a commit made there lands on that ref. - Line 7. The family root's docstring paragraph (b) says the member puts its legs on their tracking branches at their pins where that loses no commit, and says why where it does not. The missing-branch path moves into its own function, `create_at_pin`, so `checkout_tracking_branch` stays within Sonar's cognitive-complexity limit. #694: with no git on PATH both roots ended in a `FileNotFoundError` traceback, exit 1. The assembly root's step (a) (per leg) and its offline upstream notice now run git inside `_git_runs`, and the family root's step (a) inside a guard around `git submodule update`; each refuses `git-not-runnable`, the validators' code with their first remedy sentence, and exits 2. The run stops there: every later step runs git too. Which `OSError` is git's is read by NAME, as `validate-pins.py` reads it. Tests: tests/test_bootstrap_leg_moves.py (lines 1, 2, 5, every path the issue names), tests/test_bootstrap_without_git.py (#694, both roots), and tests/test_bootstrap_after_bump.py's P9 tests now read the line naming the branch left. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- templates/assembly-root/AGENTS-shape.md | 4 +- templates/assembly-root/scripts/bootstrap.py | 317 +++++++++++--- templates/family-root/scripts/bootstrap.py | 53 ++- tests/test_bootstrap_after_bump.py | 42 +- tests/test_bootstrap_leg_moves.py | 435 +++++++++++++++++++ tests/test_bootstrap_without_git.py | 174 ++++++++ 6 files changed, 924 insertions(+), 101 deletions(-) create mode 100644 tests/test_bootstrap_leg_moves.py create mode 100644 tests/test_bootstrap_without_git.py diff --git a/templates/assembly-root/AGENTS-shape.md b/templates/assembly-root/AGENTS-shape.md index 4d8ce5e9..ddf4ba88 100644 --- a/templates/assembly-root/AGENTS-shape.md +++ b/templates/assembly-root/AGENTS-shape.md @@ -92,8 +92,8 @@ make park commit, push and RECORD every open feature, so another make resume recreate the parked features here, from that record ``` -`make bootstrap` places each leg on its tracking branch AT the pin where that -loses no commit, and leaves one detached at any other commit, saying why. The +`make bootstrap` places a leg on its tracking branch AT the pin only where that +loses no commit, and otherwise leaves its checkout where it is, saying why. The line `authority is not wallet-carried in this org` is a report, not a fault. **`make park` BEFORE the human leaves a workstation; on the next one, clone, diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index c70284c6..45ac5386 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -28,7 +28,15 @@ branch is at the pin, behind it, ahead of it or not there at all, and whatever the branch or `origin` holds: a commit made there is on no branch, and a checkout would leave it only in the reflog, so bootstrap - says where the leg is instead. A branch no command can be given (a NUL, + says where the leg is instead, and so is one whose HEAD is on a ref + outside `refs/heads/`, where a commit lands on that ref (#704). A leg + with a merge, a cherry-pick, a revert, a rebase or an `am` in progress + is not moved at all: a checkout ends the first three, carrying what + they staged onto the branch, and leaves the last two to go on over + it, so the line names the operation and the commands that finish or + abort it (#704). A move git would make over an IGNORED file at a path + the pin tracks is refused, in git's words, as one over an untracked + file is. A branch no command can be given (a NUL, or a lone surrogate this system cannot encode) is never tried: the leg is left as it is and the line says why (#455); step (b)'s manifest validator names the key when the manifest holds it. Advancing a pin @@ -51,7 +59,8 @@ wallet-carried authority is not misconfigured, and a bootstrap that failed there would have made the layout load-bearing again. - (d) EXITS 0 unless a validator failed. + (d) EXITS 0 unless a validator failed, and 2 where git cannot be run, + refused `git-not-runnable` as the validators refuse it (#694). SCHEMA-NEUTRAL BY CONTRACT. A one-repository project runs the same command: step (a) is a no-op where there are no submodules, and steps (c) and (d) are @@ -68,6 +77,8 @@ import shlex import subprocess import sys +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parent)) @@ -96,6 +107,23 @@ NOTHING_ELSE = (" nothing else is tried; the recorded pin is " "authoritative. Deal with what git said and run bootstrap " "again.") +#: What a leg's git dir holds while git has an operation in progress there, +#: each with what step (a) calls it and the command that ends it, asked in +#: this order (#704): the two rebases first, as git's own `status` asks +#: them, since a pick a rebase stops on is the rebase's to finish or abort. +IN_PROGRESS = ( + ("rebase-apply/applying", "an `am`", "am"), + ("rebase-apply", "a rebase", "rebase"), + ("rebase-merge", "a rebase", "rebase"), + ("MERGE_HEAD", "a merge", "merge"), + ("CHERRY_PICK_HEAD", "a cherry-pick", "cherry-pick"), + ("REVERT_HEAD", "a revert", "revert"), +) +#: The remedy of the `git-not-runnable` refusal (#694). +GIT_NOT_RUNNABLE = ( + "Remediation: put git on PATH and run this again. Step (a) reads each " + "leg's pin and moves the leg through git, and the validators read the " + "pins through git, so without it nothing here can be placed or checked.") REGISTER_RELPATH = Path("governance") / "review-authority" / "register.yaml" VALIDATORS = ( ("naming", ["scripts/validate-repository-naming.py", "--project", @@ -122,6 +150,32 @@ def _try_git(args: list[str], cwd: Path) -> str | None: return None +@contextmanager +def _git_runs(unread: str) -> Iterator[None]: + """`git-not-runnable`, exit 2, where git could not be STARTED in the + block (#694), with `unread` saying what was being done. + + `git_out` and `_git_says` raise `OSError` then, and nothing here caught + it: `make bootstrap` with no git on PATH ended in a `FileNotFoundError` + traceback, exit 1, which is `bootstrap FAILED`'s code. The code and the + words are the validators'. Which `OSError` is git's is read as + `validate-pins.py` reads it, by NAME: one whose `filename` is `git`, or + a nameless `FileNotFoundError` or `PermissionError`, which is what + Windows raises for a program it cannot start. An error that names a + directory, which `subprocess` raises for a `cwd` it cannot enter, is + not git's, and goes on as it did. + """ + try: + yield + except OSError as exc: + if exc.filename != "git" and (exc.filename is not None or not ( + isinstance(exc, (FileNotFoundError, PermissionError)))): + raise + raise Refusal("git-not-runnable", + f"{unread}: git could not be run " + f"({exc.strerror or exc})", GIT_NOT_RUNNABLE) from exc + + def _argument_fault(branch: str) -> str | None: """What stops `branch` being an argument to a command, or `None` (#455). @@ -263,8 +317,12 @@ def _check_out(sub: Path, branch: str, post-checkout hook's exit status is the checkout's own, so a hook that fails after HEAD has moved makes git exit non-zero over a checkout that happened, which is not a refusal. + + `--no-overwrite-ignore` (#704): git's default overwrites an IGNORED + file at a path the branch tracks, silently, where it refuses an + untracked one; with it, git refuses both, in the same words. """ - why = _git_says(["checkout", "-q", branch], sub) + why = _git_says(["checkout", "-q", "--no-overwrite-ignore", branch], sub) if _head_of(sub) == (pin, f"refs/heads/{branch}"): return True, why return False, why or [ @@ -279,12 +337,10 @@ def _move_and_check_out(sub: Path, branch: str, From the pin the branch is by then the commit already checked out, so no file in the leg changes, a local edit included. From another branch the checkout changes files, as `place_at_pin`'s does, and git refuses - it where a local change to a file that differs, or an untracked file, is - in the way: the branch has moved by then, and HEAD has not. It does not - refuse an IGNORED file at a path the pin tracks: git overwrites that, - as it does for `place_at_pin` and the on-branch `merge --ff-only`, and - it ends a merge in progress on the branch HEAD leaves (#458's to rule - on, #603). + it where a local change to a file that differs, or an untracked or + IGNORED file (#704), is in the way: the branch has moved by then, and + HEAD has not. A leg with an operation in progress is never brought + here: `checkout_tracking_branch` refuses it first (#704). Returns whether the branch moved, whether HEAD is on it at the pin, and git's lines (`None` when both steps exited 0). The checkout runs only @@ -345,14 +401,16 @@ def _place_of(current: str | None) -> str: def _say_left(head: str | None, current: str, named: str) -> None: - """The line under `fast-forwarded` when the leg started on ANOTHER - branch, `current`, and is on the tracking branch `named` now (#603). + """The line under `fast-forwarded` (#603), or under `on ` + (#704), when the leg started on ANOTHER branch, `current`, and is on + the tracking branch `named` now. The person is told, since their next commit lands on `named`. P9 - (`place_at_pin`) says nothing of the branch it leaves; that is #458's. - `head` is `None` for a branch with no commit yet, which has no commit to - keep, so it is said so. Its own function, so that `fast_forward_to_pin` - stays within Sonar's cognitive-complexity limit (S3776). + (`place_at_pin`) used to say nothing of the branch it left, where T7 + (`fast_forward_to_pin`) said it for the same move (#704). `head` is + `None` for a branch with no commit yet, which has no commit to keep, so + it is said so. Its own function, so that `fast_forward_to_pin` stays + within Sonar's cognitive-complexity limit (S3776). """ kept = ("a branch with no commit yet" if head is None else "which keeps its commits") @@ -373,14 +431,66 @@ def _say_neither(label: str, state: str, head: str | None, """The checkout is not one a move starts from: `state` says what the branch is to the pin, and the line names where the leg stands instead. Nothing moves. A commit made on a leg detached off the pin is on no - branch, so checking the branch out would leave it only in the reflog. - A leg on another branch is not said here (#603): that branch holds its - commits, so the leg is moved off it.""" + branch, so checking the branch out would leave it only in the reflog, + and one made on a HEAD on a ref outside `refs/heads/` lands on that ref + (#704). A leg on another branch is not said here (#603): that branch + holds its commits, so the leg is moved off it.""" print(f" {label}: {state}, and the checkout is at {_short(head)} " f"({_place_of(current)}), which is neither") _say_keep() +def _on_a_branch(current: str | None) -> bool: + """Whether HEAD is on a branch, `refs/heads/`, one with no commit + yet included: the one checkout besides the pin a move starts from.""" + return current is not None and current.startswith("refs/heads/") + + +def _in_progress(sub: Path) -> tuple[str, str, str] | None: + """The `IN_PROGRESS` row of the operation git has in progress in the + leg, or `None` (#704). + + Asked of the leg's own git dir, which `git rev-parse --git-dir` names + (a submodule's is under the root's `.git/modules/`, a linked + worktree's under the leg's `worktrees/`, and each operation's state is + that worktree's own). Read as bytes and decoded as the system decodes a + path, so no name of a directory can fail the read. A leg git names no + git dir for has nothing to read here, and the move after this says what + git says of it. + """ + try: + said = git_out(["rev-parse", "--git-dir"], cwd=sub, binary=True) + except Refusal: + return None + gitdir = sub / os.fsdecode(said.rstrip(b"\r\n")) + for row in IN_PROGRESS: + if os.path.lexists(gitdir / row[0]): + return row + return None + + +def _say_in_progress(label: str, path: str, + row: tuple[str, str, str]) -> None: + """A leg with an operation in progress (#704): what is in progress, + that nothing moved, and the two commands that end it, the leg's path + shell-quoted, then spelled, as `move_back_to_pin` prints its command. + + No move is made from here. A checkout of the tracking branch, even from + HEAD on it already or detached at the pin, ends a merge, a cherry-pick + or a revert, taking away the head file `--continue` reads and carrying + the staged result onto the branch, and it puts a rebase or an `am` on + the branch, which goes on from there; `merge --ff-only` refuses some of + them and not others. It is the operation's own to finish or abort, so + every such leg is refused alike, before any path of step (a) is + chosen.""" + marker, doing, verb = row + command = f"git -C {spelled(shlex.quote(path))} {verb}" + print(f" {label}: {doing} is in progress in the leg ({marker} is in its " + "git dir), so nothing moved") + print(f" finish it (`{command} --continue`) or abort it " + f"(`{command} --abort`), then run bootstrap again.") + + def place_at_pin(sub: Path, label: str, branch: str, pin: str) -> None: """Step (a) for a leg whose tracking branch IS the pin: check it out, and say `on ` only when HEAD is on it afterwards. @@ -397,11 +507,16 @@ def place_at_pin(sub: Path, label: str, branch: str, pin: str) -> None: bootstrap, checked the branch out and printed `on at ` over a commit that was then on no branch, held by HEAD's reflog alone. Detached AT the pin, or already on the branch, the checkout is as it was. - On ANOTHER branch it is as it was too: that branch holds its commits. + On ANOTHER branch it is as it was too: that branch holds its commits, + and the line under `on ` now says the leg left it, as T7's does + (#704). A HEAD on a ref outside `refs/heads/` (`git symbolic-ref HEAD + refs/remotes/origin/main`) is left where it is, as T7 leaves it: a + commit made there lands on that ref, and this placed the leg while T7 + said `which is neither` of the same HEAD (#704). """ named = spelled(branch) head, current = _head_of(sub) - if current is None and head != pin: + if not (_on_a_branch(current) or (current is None and head == pin)): _say_neither(label, f"branch {named} is at the pin {_short(pin)}", head, current) return @@ -412,6 +527,8 @@ def place_at_pin(sub: Path, label: str, branch: str, pin: str) -> None: f"{_short(pin)}, and git refused to check it out:", why) return print(f" {label}: on {named} at {_short(pin)} (branch tip == pin)") + if current not in (None, f"refs/heads/{branch}"): + _say_left(head, current, named) _say_after(why, moved=not already) @@ -428,7 +545,8 @@ def fast_forward_to_pin(sub: Path, label: str, branch: str, local: str, - HEAD on the branch itself: `merge --ff-only`, which moves the working tree too, and moves nothing when a local change to a file the move - changes, or an untracked file, is in the way. `merge.autostash` is + changes, or an untracked file, is in the way, or an ignored one + (`--no-overwrite-ignore`, #704). `merge.autostash` is off for it: with it on, git stashes the change, moves the branch, applies the stash back into conflicts and exits 0, and the leg was left unmerged under a line saying it had been fast-forwarded. `-c`, @@ -443,33 +561,36 @@ def fast_forward_to_pin(sub: Path, label: str, branch: str, local: str, this checkout when it is AT the pin; leaving the leg for a branch BEHIND the pin was a difference nobody could explain (#458). The checkout changes files from here, so git can refuse it over a local - edit or an untracked file, and `_move_and_check_out` says what is - left then. A HEAD on a branch with no commit yet takes this path too, - as it does in `place_at_pin`. When the checkout is made, one line - under the `fast-forwarded` one says the leg left that branch, and - that it keeps its commits, since the next commit made in the leg is - on the tracking branch now. NO COMMIT IS LOST, BUT TWO THINGS ARE NOT - REFUSED, as in `place_at_pin`, in the missing-branch path and, for - the first, in the on-branch `merge --ff-only`: git overwrites an - IGNORED file at a path the pin tracks, and a merge in progress on the - other branch is ended, its `MERGE_HEAD` removed and its staged result - carried onto the tracking branch. Both are #458's to rule on. + edit or an untracked or ignored file, and `_move_and_check_out` says + what is left then. A HEAD on a branch with no commit yet takes this + path too, as it does in `place_at_pin`. When the checkout is made, + one line under the `fast-forwarded` one says the leg left that + branch, and that it keeps its commits, since the next commit made in + the leg is on the tracking branch now. NO COMMIT IS LOST, AND NOTHING + ELSE A PERSON HAS (#704): git used to overwrite an IGNORED file at a + path the pin tracks, here, in `place_at_pin`, in the missing-branch + path and in the on-branch `merge --ff-only`, and a checkout ended a + merge in progress on the other branch, its `MERGE_HEAD` removed and + its staged result carried onto the tracking branch. Every move now + refuses the first, and no leg with an operation in progress reaches + here (`_say_in_progress`). A LEG DETACHED OFF THE PIN IS LEFT WHERE IT IS: a commit made on the detached leg after the bump is on no branch, and checking the branch out would leave it only in the reflog. That is said, with what to do, - and nothing moves. + and nothing moves. So is a HEAD on a ref outside `refs/heads/`, which + `place_at_pin` now leaves too (#704). `label` is the leg as `checkout_tracking_branch` prints it, already spelled; the branch names and git's own lines are spelled here (#391). """ named = spelled(branch) head, current = _head_of(sub) - other_branch = (current is not None and current.startswith("refs/heads/") + other_branch = (_on_a_branch(current) and current != f"refs/heads/{branch}") if current == f"refs/heads/{branch}": why = _git_says(["-c", "merge.autostash=false", "merge", "--ff-only", - "-q", pin], sub) + "--no-overwrite-ignore", "-q", pin], sub) moved = placed = why is None elif other_branch or (current is None and head == pin): moved, placed, why = _move_and_check_out(sub, branch, pin) @@ -564,8 +685,65 @@ def _stays(sub: Path, branch: str, pin: str) -> str: return f"at {_short(head)} ({_place_of(current)})" +def create_at_pin(sub: Path, label: str, branch: str, pin: str, + remote: str | None) -> None: + """Step (a) for a leg whose tracking branch IS NOT THERE: `checkout -b + `, from HEAD detached at the pin or on a branch, a branch + with no commit yet included. + + A checkout git refuses is said in git's own words under the line that + says the branch could not be made (#704), with `--no-overwrite-ignore`, + so an IGNORED file at a path the pin tracks is refused as an untracked + one is, where git overwrote it silently. The line used to say `leaving + detached` of a leg on another branch, too; it says where the leg stays. + A HEAD on a ref outside `refs/heads/` is left where it is, as the + branch-at-the-pin and the behind-branch paths leave it (#704). + """ + named = spelled(branch) + head, current = _head_of(sub) + if head is not None and current is None and head != pin: + # `checkout -b ` from a leg detached anywhere else + # leaves the commit made there on no branch (#533), as placing a + # branch that exists would. The branch is not there to check + # out, so the way out names the pin alone. A HEAD git cannot + # read (the leg's gitdir is gone) is no commit to keep: the + # checkout below fails, and says it could not. + print(f" {label}: branch {named} does not exist, and the " + f"checkout is at {_short(head)} (detached), which is not " + f"the pin {_short(pin)}") + _say_keep("the pin") + return + if current is not None and not _on_a_branch(current): + print(f" {label}: branch {named} does not exist, and the checkout " + f"is at {_short(head)} ({_place_of(current)}), which is not a " + "branch") + _say_keep("the pin") + return + why = _git_says(["checkout", "-q", "--no-overwrite-ignore", "-b", branch, + pin], sub) + if why is not None: + stays = "detached" if current is None else f"it {_place_of(current)}" + _say_refused(f" {label}: could not create branch {branch!r} at " + f"the pin {_short(pin)}; leaving {stays}", why) + return + if remote is not None: + _try_git(["branch", "-q", f"--set-upstream-to=origin/{branch}", + branch], sub) + state = "at the pin" + if remote is not None and remote != pin: + state = (f"at the pin; origin/{named} is {_short(remote)}, which " + f"the pin deliberately does NOT follow") + print(f" {label}: branch {named} created at " + f"{_short(pin)} — {state}") + + def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None: - """Step (a) for one leg. Prints; never raises; never moves the pin.""" + """Step (a) for one leg. Prints; never moves the pin; and never raises, + but an `OSError` where git cannot be started (#694), which + `_checkout_all_tracking_branches` turns into `git-not-runnable`. + + A leg with an operation in progress is refused before any path is + chosen (`_say_in_progress`, #704), so no move below meets one.""" path = str(leg.get("path") or "") sub = root / path # What is PRINTED of the leg is spelled (#391): `role`, `path` and `branch` @@ -586,37 +764,16 @@ def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None: f"refs/heads/{branch}"], sub) remote = _try_git(["rev-parse", "--verify", "--quiet", f"refs/remotes/origin/{branch}"], sub) + doing = _in_progress(sub) + if doing is not None: + _say_in_progress(where, path, doing) + return if local is None: if pin is None: print(f" {where}: no gitlink recorded; leaving as is") - return - head, current = _head_of(sub) - if head is not None and current is None and head != pin: - # `checkout -b ` from a leg detached anywhere else - # leaves the commit made there on no branch (#533), as placing a - # branch that exists would. The branch is not there to check - # out, so the way out names the pin alone. A HEAD git cannot - # read (the leg's gitdir is gone) is no commit to keep: the - # checkout below fails, and says it could not. - print(f" {where}: branch {named} does not exist, and the " - f"checkout is at {_short(head)} (detached), which is not " - f"the pin {_short(pin)}") - _say_keep("the pin") - return - if _try_git(["checkout", "-q", "-b", branch, pin], sub) is None: - print(f" {where}: could not create branch {branch!r} at " - f"the pin {_short(pin)}; leaving detached") - return - if remote is not None: - _try_git(["branch", "-q", f"--set-upstream-to=origin/{branch}", - branch], sub) - state = "at the pin" - if remote is not None and remote != pin: - state = (f"at the pin; origin/{named} is {_short(remote)}, which " - f"the pin deliberately does NOT follow") - print(f" {where}: branch {named} created at " - f"{_short(pin)} — {state}") + else: + create_at_pin(sub, where, branch, pin, remote) return if local == pin: @@ -647,7 +804,9 @@ def shape_upstream_notice(root: Path) -> None: be on this machine — a clone at `SHAPE_UPSTREAM_PATH`, or a tip somebody cached at `.shape-upstream-tip` — saying so costs nothing and closes the gap that had two projects updated by hand. Where it is not, this is silent: - an absent answer is not a finding, and it never fails the bootstrap. + an absent answer is not a finding, and it never fails the bootstrap. A + clone named there that git cannot be STARTED to read is no absent + answer but a machine with no git, refused `git-not-runnable` (#694). """ pin_path = root / "contracts" / "shape-pin.yaml" if not pin_path.is_file(): @@ -662,7 +821,9 @@ def shape_upstream_notice(root: Path) -> None: tip = None upstream = os.environ.get(SHAPE_UPSTREAM_ENV) if upstream and Path(upstream).is_dir(): - tip = _try_git(["rev-parse", "HEAD"], Path(upstream)) + with _git_runs(f"the clone {SHAPE_UPSTREAM_ENV} names, " + f"{spelled(upstream)}, could not be read"): + tip = _try_git(["rev-parse", "HEAD"], Path(upstream)) if tip is None: cache = root / SHAPE_UPSTREAM_CACHE if cache.is_file(): @@ -831,7 +992,9 @@ def _checkout_all_tracking_branches(root: Path, sub_legs: list[dict], branch_arg: str | None) -> None: """Step (a): place every submodule leg on its tracking branch. - Split from `main` for #132. + Split from `main` for #132. Git that cannot be started is + `git-not-runnable`, naming the leg it met it at (#694): one fact about + the machine, so no leg after it is tried. """ if not sub_legs: print(" no submodule legs declared. A one-repository project runs " @@ -839,7 +1002,11 @@ def _checkout_all_tracking_branches(root: Path, sub_legs: list[dict], return branch = branch_arg or (manifest or {}).get("tracking_branch") or "main" for leg in sub_legs: - checkout_tracking_branch(root, leg, str(leg.get("branch") or branch)) + with _git_runs(f"the {spelled(leg.get('role'))} leg at " + f"{spelled(leg.get('path') or '')} could not be " + "placed on its tracking branch"): + checkout_tracking_branch(root, leg, + str(leg.get("branch") or branch)) def _run_neutral_validators(root: Path, manifest: dict | None, @@ -893,11 +1060,19 @@ def main(argv: list[str] | None = None) -> int: name = (manifest or {}).get("name") or root.name print(f"bootstrap: {spelled(name)} ({spelled(root)})") - print("\n(a) legs on tracking branches, pins untouched") - _checkout_all_tracking_branches(root, sub_legs, manifest, args.branch) + try: + print("\n(a) legs on tracking branches, pins untouched") + _checkout_all_tracking_branches(root, sub_legs, manifest, args.branch) - print("\n(b) neutral validators") - failed = _run_neutral_validators(root, manifest, args.skip_validators) + print("\n(b) neutral validators") + failed = _run_neutral_validators(root, manifest, args.skip_validators) + except Refusal as exc: + # Only `git-not-runnable` reaches here (#694): nothing else in the + # two steps raises one. Stdout first, so a log of both streams + # reads in the order the run went. + sys.stdout.flush() + print(str(exc), file=sys.stderr) + return 2 print("\n(c) review authority") read_authority(root, legs) diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index e3fb3b10..061702d5 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -40,8 +40,9 @@ words. The line names `validate-family.py`, which says what to commit. (b) RUNS EACH MEMBER'S OWN `make bootstrap`. Each member is a whole project - and knows how to bootstrap itself — its legs onto their tracking - branches at their pinned commits, its own validators, its own + and knows how to bootstrap itself — it puts its legs on their + tracking branches at their pins where that loses no commit, and says + why where it does not; its own validators; its own review-authority readout. This command does not reimplement any of that; it walks the members `family.yaml` names and calls theirs. `--members-make validate` runs `make validate` instead, which is what @@ -51,6 +52,9 @@ checked out that no credential would check out. Any other member that is not checked out is REPORTED and skipped, not a failure: it is the state a missing credential leaves, and it is already reported in (a). + Where (a) cannot start git, the run STOPS there, exit 2, refused + `git-not-runnable` as `validate-family.py` refuses it (#694): each + member's own bootstrap runs git too, so (b) would only say it again. THE FAMILY CONFERS NOTHING and neither does this command. It fetches, it delegates, and it says what it could not reach. @@ -100,7 +104,8 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: Returns a failure line for each of `rows` whose member it leaves not checked out where no credential would check it out (`unfetchable`), for the run to end FAILED on (#648); every member and leg is said to be - fetched only where there is none and git exited 0.""" + fetched only where there is none and git exited 0. Raises the + `git-not-runnable` refusal where git cannot be started (#694).""" source, token = credential() args = ["git"] if token: @@ -110,8 +115,15 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: args += ["submodule", "update", "--init", "--recursive"] print(f" credential source: {source}") sys.stdout.flush() - proc = subprocess.run(args, cwd=str(root), capture_output=True, text=True, - encoding="utf-8", errors="replace", check=False) + try: + proc = subprocess.run(args, cwd=str(root), capture_output=True, + text=True, encoding="utf-8", errors="replace", + check=False) + except OSError as exc: + refusal = git_not_started(exc) + if refusal is None: + raise + raise refusal from exc if proc.returncode != 0: # The token can appear in git's own error text (it is in the URL it # tried), so the output is scrubbed before it is printed. @@ -136,6 +148,31 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: for row, _, _ in left] +def git_not_started(exc: OSError) -> Refusal | None: + """The `git-not-runnable` refusal for an `OSError` that says git could + not be STARTED (#694), else `None`, and the error goes on as it did. + + Nothing caught one: `make bootstrap` with no git on PATH ended (a) in a + `FileNotFoundError` traceback, exit 1, the code of `family bootstrap + FAILED`, though nothing had failed. Git's is told by its NAME, as + `validate-pins.py` tells it: a `filename` of `git`, or none at all on a + `FileNotFoundError` or a `PermissionError`, as Windows raises them; one + naming a directory is not git's. The code, and the remedy's first + sentence, are `validate-family.py`'s.""" + nameless = exc.filename is None and isinstance( + exc, (FileNotFoundError, PermissionError)) + if exc.filename != "git" and not nameless: + return None + return Refusal( + "git-not-runnable", + f"the members could not be fetched: git could not be run " + f"({exc.strerror or exc})", + "Remediation: put git on PATH and run this again. Step (a) fetches " + "every member through git, and each member's own bootstrap places " + "its legs through git too, so without it nothing here can be " + "fetched or bootstrapped.") + + def gitmodules_paths(root: Path) -> set[str]: """Every path a section of `.gitmodules` gives, read as git reads it: the work tree's file, the one `git submodule update` reads, which a @@ -278,7 +315,11 @@ def main(argv: list[str] | None = None) -> int: if args.no_fetch: print(" skipped (--no-fetch)") else: - failed += fetch_members(root, rows) + try: + failed += fetch_members(root, rows) + except Refusal as exc: + print(str(exc), file=sys.stderr) + return 2 print(f"\n(b) each member's own `make {spelled(args.members_make)}`") if args.skip_members: diff --git a/tests/test_bootstrap_after_bump.py b/tests/test_bootstrap_after_bump.py index 0acbdb59..8c368a89 100644 --- a/tests/test_bootstrap_after_bump.py +++ b/tests/test_bootstrap_after_bump.py @@ -57,16 +57,15 @@ the tracking branch to the pin and the checkout puts the leg on it, as from the pin; the other branch is not touched. One line under the `fast-forwarded` one says the leg left that branch and that it keeps its commits (or has none yet), -since the next commit made in the leg is on the tracking branch now; no other -start prints it, and nor does a refusal. From another branch the checkout -changes files, so git can refuse it over a local edit or an untracked file, -which a checkout from the pin never meets: the branch is then left at the pin -and HEAD where it was, and the next run meets the branch-at-the-pin state. Two -things are NOT refused, as in the branch-at-the-pin and missing-branch cases -(#458's to rule on): an IGNORED file at a path the pin tracks is overwritten, -and a merge in progress on the other branch is ended, its `MERGE_HEAD` removed -and its staged result carried onto `main`. A leg detached at a commit that is -not the pin is still left, whatever it holds. +since the next commit made in the leg is on the tracking branch now; the +branch-at-the-pin path prints it under `on main` too (#704), and no other start +prints it, nor does a refusal. From another branch the checkout changes files, +so git can refuse it over a local edit or an untracked file, which a checkout +from the pin never meets: the branch is then left at the pin and HEAD where it +was, and the next run meets the branch-at-the-pin state. An IGNORED file in the +way and an operation in progress are refused too, on every path (#704): +`tests/test_bootstrap_leg_moves.py`. A leg detached at a commit that is not the +pin is still left, whatever it holds. EACH TEST GOES THE WAY A PERSON DOES: a scaffold into bare repositories on disk, cloned recursively and bootstrapped, so each leg is on `main` at its @@ -882,7 +881,9 @@ def test_a_leg_on_another_branch_with_its_own_branch_at_the_pin_is_as_it_was( is made. `fast_forward_to_pin` did not do the same for a branch BEHIND the pin until #603 (D1, option A of its checklist triage) made the two one rule: the T7 tests at the end of this module are this test's twins - for a branch behind the pin.""" + for a branch behind the pin. The line under `on main` says the leg left + `elsewhere`, as T7's does under `fast-forwarded`; it said nothing of the + branch, and the next commit was on `main` unannounced (#704).""" leg, new = branch_at_the_new_pin(root, bootstrapped["remotes"], tmp_path, "advance-other-branch") git("checkout", "-q", "-b", "elsewhere", cwd=leg) @@ -890,8 +891,8 @@ def test_a_leg_on_another_branch_with_its_own_branch_at_the_pin_is_as_it_was( result = bootstrap(root) assert result.returncode == 0, result.stderr + result.stdout - assert f" [spec] spec: on main at {new[:12]} (branch tip == pin)\n" in ( - result.stdout), result.stdout + assert (f" [spec] spec: on main at {new[:12]} (branch tip == pin)\n" + + left_branch()) in result.stdout, result.stdout assert head_ref(leg) == MAIN and rev(leg) == new assert rev(leg, "refs/heads/elsewhere") != new, "its commit is on it" @@ -1291,9 +1292,8 @@ def test_a_checkout_off_another_branch_git_refuses_leaves_main_at_the_pin( git("checkout", "-q", "--", EDITED, cwd=leg) again = skip_validators(root) - assert f" [spec] spec: on main at {new[:12]} (branch tip == pin)\n" in ( - again.stdout), again.stdout - assert LEFT not in again.stdout, "P9 says nothing of the branch it left" + assert (f" [spec] spec: on main at {new[:12]} (branch tip == pin)\n" + + left_branch()) in again.stdout, "P9 says the branch it left" assert head_ref(leg) == MAIN and rev(leg) == new @@ -1334,8 +1334,8 @@ def test_a_leg_on_a_branch_with_no_commit_yet_is_placed_as_on_any_branch( """HEAD on a branch with NO COMMIT YET (`git switch --orphan`): there is no commit to keep, and `place_at_pin` checks `main` out from it, so a `main` behind the pin does the same (#603). The four lines side by side: - free, `main` goes to the pin and the leg onto it, T7 says the leg left a - branch with no commit yet and P9 says nothing of it; with an untracked + free, `main` goes to the pin and the leg onto it, and T7 and P9 both say + the leg left a branch with no commit yet (P9 since #704); with an untracked file in the way, git refuses the checkout, HEAD stays on the unborn branch, `main` is at the pin and nothing is said of the leg leaving.""" old = rev(root / "spec") @@ -1368,11 +1368,9 @@ def test_a_leg_on_a_branch_with_no_commit_yet_is_placed_as_on_any_branch( assert (leg / ADDED).read_bytes() == b"mine\n" assert LEFT not in result.stdout else: - assert ((fast_forwarded(old, new) + left_branch("orphan", True) - if where == "behind" else + assert ((fast_forwarded(old, new) if where == "behind" else f" [spec] spec: on main at {new[:12]} (branch tip == pin)\n") - in result.stdout), result.stdout - assert (LEFT in result.stdout) == (where == "behind") + + left_branch("orphan", True) in result.stdout), result.stdout assert head_ref(leg) == MAIN and rev(leg) == new assert git("status", "--porcelain", cwd=leg).stdout == "" diff --git a/tests/test_bootstrap_leg_moves.py b/tests/test_bootstrap_leg_moves.py new file mode 100644 index 00000000..16985ec5 --- /dev/null +++ b/tests/test_bootstrap_leg_moves.py @@ -0,0 +1,435 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`make bootstrap` moves a leg only where nothing the person has is lost, and +reads every HEAD one way (#704). + +THE ISSUE'S LINES, each measured on lane 5's #682 head (8b39cd19, git 2.43.0) +before this change, in a scaffold's recursive clone bootstrapped once: + +- LINE 1. Every move that changes files overwrote an IGNORED file at a path + the pin tracks, silently: the checkout of the branch AT the pin from another + branch (P9, `place_at_pin`), the checkout of the branch moved from BEHIND the + pin (T7, `fast_forward_to_pin`), the `merge --ff-only` of a branch behind the + pin with HEAD on it, and the `checkout -b` of a branch that is not there. A + `README.md` the leg's `info/exclude` lists, holding the person's notes, was + the pin's `README.md` after each, under `on main at `. Each now carries + `--no-overwrite-ignore`, so git refuses it as it refuses an untracked file, + in its own words, and the leg stays where it is. +- LINE 2. A merge, a cherry-pick or a revert in progress on the branch HEAD + left was ENDED by each of those moves: its head file (`MERGE_HEAD`, + `CHERRY_PICK_HEAD`, `REVERT_HEAD`) removed and what it had staged carried + onto `main`. A merge was ended from HEAD detached at the pin, from HEAD + already on `main` at the pin (a checkout that moves nothing) and by the move + back of a branch past the pin, too; a rebase or an `am` stopped there was + put on `main`, to go on over it. Brett Heap's R1 (2026-10-16T13:07:34Z): a + leg in the way of a move REFUSES. A leg whose git dir holds any of the + five markers is refused before any path is chosen, naming the operation, + saying nothing moved, and giving the commands that finish or abort it. +- LINE 5. A HEAD that is a symbolic ref outside `refs/heads/` (`git + symbolic-ref HEAD refs/remotes/origin/main`) was placed by P9 and the + missing-branch path and left by T7 (`which is neither`). The issue's + recommended answer: every path leaves it, since a commit made there lands + on that ref. + +Lines 3 and 4 are elsewhere: P9's line naming the branch it left is in +`tests/test_bootstrap_after_bump.py` (its other-branch tests, now reading +the line), and line 4 is the narrowed sentence in `AGENTS-shape.md` (R2). + +Every state is made the way a person makes it, with git's own commands in the +leg, and every way out is run as printed, from the root. +""" + +from __future__ import annotations + +import itertools +import shlex +from pathlib import Path + +import pytest + +from conftest import git +from test_bootstrap_after_bump import ( # noqa: F401 + ADDED, EDITED, MAIN, NOTHING_ELSE, bootstrapped, bumped, head_ref, + moved_and_refused, rev, root, skip_validators, +) + +#: The pin's tracked file the branch `other` deletes, so that checking the +#: tracking branch out from `other` writes it. +IN_THE_WAY = "README.md" +#: What the person keeps in the ignored file. +MINE = b"MY LOCAL NOTES\n" +#: The three paths of the issue, by the state of `main` beside the pin. +PATHS = pytest.mark.parametrize("path", ["at", "behind", "missing"], + ids=["P9 at the pin", "T7 behind the pin", + "missing branch"]) +#: A branch name per `bumped` call: each pushes to the module's spec remote. +#: Not `tmp_path.name`, which a passed test's removal hands to the next one. +ADVANCES = itertools.count() + + +def keep(options: str) -> str: + """The line under a checkout step (a) will not move off.""" + return (" not moving the checkout off it. Keep any commit made " + f"there on a branch, check out {options} in the leg, and run " + "bootstrap again.\n") + + +def tracking(root: Path, remotes: Path, tmp_path: Path, path: str, + touch: str | None = None) -> tuple[str, str]: + """`main` as `path` names it, with the leg left DETACHED at the pin: + AT the pin (P9: the bootstrapped clone's own state), BEHIND it (T7: a + bump, `bumped`, with `touch` as it takes it) or NOT THERE (the missing- + branch path). Returns the pin and the commit a branch made beside + `main` starts from: `main`'s own, or the pin where there is none.""" + leg = root / "spec" + if path == "behind": + start = rev(leg, MAIN) + pin = bumped(root, remotes, tmp_path, f"moves-{next(ADVANCES)}", + touch) + return pin, start + pin = rev(leg) + git("checkout", "-q", "--detach", pin, cwd=leg) + if path == "missing": + git("branch", "-q", "-D", "main", cwd=leg) + return pin, pin + + +def main_of(leg: Path) -> str | None: + """`main`'s commit, or None where the leg has no `main`.""" + return git("rev-parse", "--verify", "--quiet", MAIN, cwd=leg, + check=False).stdout.strip() or None + + +def ignore(leg: Path, name: str) -> None: + """`name` listed in the leg's own `info/exclude`, a person's ignore + that no commit carries, and the person's bytes written there.""" + exclude = leg / git("rev-parse", "--git-path", "info/exclude", + cwd=leg).stdout.strip() + listed = exclude.read_bytes() if exclude.is_file() else b"" + exclude.parent.mkdir(exist_ok=True) + exclude.write_bytes(listed + name.encode("utf-8") + b"\n") + (leg / name).write_bytes(MINE) + + +def snapshot(leg: Path) -> tuple: + """Where the leg is: HEAD's ref and commit, `main`, and the status.""" + return (head_ref(leg), rev(leg), main_of(leg), + git("status", "--porcelain", cwd=leg).stdout) + + +def placed_once_cleared(root: Path, pin: str) -> None: + """The way out works: a run after it puts the leg on `main` AT the + pin.""" + again = skip_validators(root) + assert again.returncode == 0, again.stderr + again.stdout + assert head_ref(root / "spec") == MAIN, again.stdout + assert rev(root / "spec") == pin, again.stdout + + +# --- line 1: an ignored file in the way is refused on every path ------------ + +def refused_first_line(path: str, pin: str, start: str) -> str: + """What step (a) prints first over a move git refused, by path.""" + if path == "at": + return (f" [spec] spec: branch main is at the pin {pin[:12]}, and " + "git refused to check it out:\n") + if path == "behind": + return moved_and_refused(start, pin) + return (f" [spec] spec: could not create branch 'main' at the pin " + f"{pin[:12]}; leaving it on other\n") + + +@PATHS +def test_an_ignored_file_a_move_off_another_branch_writes_is_refused( + root, bootstrapped, tmp_path, path): + """HEAD on `other`, a branch that deleted `README.md`, which the pin + tracks, and the person's own `README.md` ignored there. The checkout + of `main` wrote the pin's over it (P9, the issue's own measure; T7; the + missing branch's `checkout -b`). Git now refuses it in its own words, + the file is the person's still, and HEAD stays on `other`. T7's + `branch -f` has moved `main` to the pin by then, as over an untracked + file, and says so; the next run meets P9's state.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, path) + leg = root / "spec" + git("checkout", "-q", "-b", "other", start, cwd=leg) + git("rm", "-q", "--", IN_THE_WAY, cwd=leg) + git("commit", "-q", "-m", "other keeps no README", cwd=leg) + ignore(leg, IN_THE_WAY) + + result = skip_validators(root) + assert result.returncode == 0, result.stderr + result.stdout + assert refused_first_line(path, pin, start) in result.stdout, ( + result.stdout) + assert f" {IN_THE_WAY}\n" in result.stdout, "git names the file" + assert NOTHING_ELSE in result.stdout + assert (leg / IN_THE_WAY).read_bytes() == MINE, "the file is the person's" + assert head_ref(leg) == "refs/heads/other", "HEAD stays on `other`" + assert main_of(leg) == {"at": pin, "behind": pin, "missing": None}[path] + + (leg / IN_THE_WAY).unlink() + placed_once_cleared(root, pin) + + +def test_an_ignored_file_the_fast_forward_writes_is_refused( + root, bootstrapped, tmp_path): + """T7 with HEAD ON `main` behind the pin: `merge --ff-only` wrote the + pin's `NEW-CONTRACT.md` over the person's ignored one and said + `fast-forwarded`. It now refuses, and `main` stays where it was.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, "behind") + leg = root / "spec" + git("checkout", "-q", "main", cwd=leg) + ignore(leg, ADDED) + + result = skip_validators(root) + assert result.returncode == 0, result.stderr + result.stdout + assert (f" [spec] spec: branch main tip {start[:12]} is behind the PIN " + f"{pin[:12]}, and git refused to move it:\n") in result.stdout, ( + result.stdout) + assert f" {ADDED}\n" in result.stdout, "git names the file" + assert NOTHING_ELSE in result.stdout + assert (leg / ADDED).read_bytes() == MINE + assert head_ref(leg) == MAIN + assert rev(leg) == start, "`main` is not moved" + + (leg / ADDED).unlink() + placed_once_cleared(root, pin) + + +# --- line 2: an operation in progress is refused before any move ------------ + +def theirs(leg: Path, start: str) -> None: + """A branch `theirs` from `start`, one commit adding `b.txt`; HEAD is + left on it.""" + git("checkout", "-q", "-b", "theirs", start, cwd=leg) + (leg / "b.txt").write_bytes(b"b\n") + git("add", "--", "b.txt", cwd=leg) + git("commit", "-q", "-m", "theirs", cwd=leg) + + +def merge_theirs(leg: Path) -> None: + """`theirs` merged where HEAD is, `--no-commit --no-ff`: `MERGE_HEAD`, + and `b.txt` staged.""" + git("merge", "-q", "--no-commit", "--no-ff", "theirs", cwd=leg) + + +def merging(leg: Path, start: str) -> None: + """`other` from `start`, merging `theirs`.""" + theirs(leg, start) + git("checkout", "-q", "-b", "other", start, cwd=leg) + merge_theirs(leg) + + +def cherry_picking(leg: Path, start: str) -> None: + """`other` from `start`, picking `theirs`'s second commit, which changes + the README its first one wrote: a conflict, resolved and staged, so + `CHERRY_PICK_HEAD` is there and nothing is unmerged.""" + git("checkout", "-q", "-b", "theirs", start, cwd=leg) + for text in (b"first\n", b"second\n"): + (leg / EDITED).write_bytes(text) + git("commit", "-q", "-am", text.decode().strip(), cwd=leg) + pick = rev(leg) + git("checkout", "-q", "-b", "other", start, cwd=leg) + assert git("cherry-pick", pick, cwd=leg, check=False).returncode != 0 + (leg / EDITED).write_bytes(b"resolved\n") + git("add", "--", EDITED, cwd=leg) + + +def reverting(leg: Path, start: str) -> None: + """`other` from `start` with a commit adding `y.txt`, reverted with + `--no-commit`: `REVERT_HEAD`, and the deletion staged.""" + git("checkout", "-q", "-b", "other", start, cwd=leg) + (leg / "y.txt").write_bytes(b"y\n") + git("add", "--", "y.txt", cwd=leg) + git("commit", "-q", "-m", "y", cwd=leg) + git("revert", "--no-commit", "HEAD", cwd=leg) + + +#: Each operation: what makes it, the marker, what the line calls it, and +#: the git command that ends it. +OPERATIONS = { + "merge": (merging, "MERGE_HEAD", "a merge", "merge"), + "cherry-pick": (cherry_picking, "CHERRY_PICK_HEAD", "a cherry-pick", + "cherry-pick"), + "revert": (reverting, "REVERT_HEAD", "a revert", "revert"), + "rebase": (None, "rebase-merge", "a rebase", "rebase"), + "rebase --apply": (None, "rebase-apply", "a rebase", "rebase"), + "am": (None, "rebase-apply/applying", "an `am`", "am"), +} + + +def in_progress(operation: str) -> str: + """The two lines step (a) prints for a leg with `operation` in progress + (#704), byte for byte.""" + _, marker, doing, verb = OPERATIONS[operation] + return (f" [spec] spec: {doing} is in progress in the leg ({marker} is " + "in its git dir), so nothing moved\n" + f" finish it (`git -C spec {verb} --continue`) or abort " + f"it (`git -C spec {verb} --abort`), then run bootstrap again.\n") + + +def refused_until_aborted(root: Path, operation: str, pin: str) -> None: + """Bootstrap refuses the leg as `in_progress` says and nothing moves: + HEAD, `main`, the index and the operation's marker are as they were. + Then the `--abort` the line names, run as printed from the root, and a + run after it places the leg at the pin.""" + leg = root / "spec" + marker = OPERATIONS[operation][1] + gitdir = leg / git("rev-parse", "--git-dir", cwd=leg).stdout.strip() + before = snapshot(leg) + assert (gitdir / marker).exists(), f"{operation} is in progress" + + result = skip_validators(root) + assert result.returncode == 0, result.stderr + result.stdout + assert in_progress(operation) in result.stdout, result.stdout + assert snapshot(leg) == before, "nothing moved" + assert (gitdir / marker).exists(), f"the {operation} goes on" + + # The `--abort` of the second line, which is in the output as it is in + # `in_progress`: the fourth span of backticks there. + abort = in_progress(operation).splitlines()[1].split("`")[3] + assert abort.endswith(" --abort"), abort + git(*shlex.split(abort)[1:], cwd=root) + placed_once_cleared(root, pin) + + +@PATHS +@pytest.mark.parametrize("operation", ["merge", "cherry-pick", "revert"]) +def test_an_operation_in_progress_on_the_branch_a_move_leaves_is_refused( + root, bootstrapped, tmp_path, path, operation): + """HEAD on `other` with a merge, a cherry-pick or a revert in progress + there, on each of the three paths that check `main` out from another + branch. Each move ENDED it: the head file gone, the staged result on + `main`, and `on main` or `fast-forwarded` or `created` said over it.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, path) + OPERATIONS[operation][0](root / "spec", start) + refused_until_aborted(root, operation, pin) + + +def detached_at_the_pin(root: Path, remotes: Path, tmp_path: Path) -> str: + """HEAD detached AT the pin, `main` there too (P9 from the pin).""" + return tracking(root, remotes, tmp_path, "at")[0] + + +def on_main_at_the_pin(root: Path, remotes: Path, tmp_path: Path) -> str: + """HEAD ON `main` AT the pin: the leg already placed, where a checkout + of `main` moves nothing and still ends the merge.""" + pin = rev(root / "spec") + assert head_ref(root / "spec") == MAIN + return pin + + +def past_the_pin(root: Path, remotes: Path, tmp_path: Path) -> str: + """`main` at `origin/main` one commit PAST the pin, HEAD detached at the + pin: `move_back_to_pin`'s state, a fresh clone of a leg that moved on.""" + leg = root / "spec" + pin = rev(leg) + ahead = git("commit-tree", rev(leg, f"{pin}^{{tree}}"), "-p", pin, "-m", + "the remote moved on", cwd=leg).stdout.strip() + git("update-ref", "refs/remotes/origin/main", ahead, cwd=leg) + git("checkout", "-q", "--detach", pin, cwd=leg) + git("branch", "-q", "-f", "main", ahead, cwd=leg) + return pin + + +@pytest.mark.parametrize("state", [detached_at_the_pin, on_main_at_the_pin, + past_the_pin], + ids=["detached at the pin", "on main at the pin", + "main past the pin"]) +def test_a_merge_in_progress_where_no_branch_is_left_is_not_ended_either( + root, bootstrapped, tmp_path, state): + """A merge started where HEAD already is: detached at the pin, on `main` + at the pin, and detached at the pin under a `main` the move back takes + to the pin. The checkout ended it from all three, though none leaves a + branch: git's checkout drops `MERGE_HEAD` whenever it runs.""" + pin = state(root, bootstrapped["remotes"], tmp_path) + leg = root / "spec" + where = head_ref(leg) + theirs(leg, pin) + git("checkout", "-q", *(["main"] if where == MAIN else ["--detach", pin]), + cwd=leg) + merge_theirs(leg) + refused_until_aborted(root, "merge", pin) + + +def rebased(leg: Path, pin: str, start: str, *backend: str) -> None: + """`mine` from `start`, changing the README the pin changes too, rebased + onto the pin: it stops on the conflict with HEAD detached AT the pin, + resolved and staged.""" + git("checkout", "-q", "-b", "mine", start, cwd=leg) + (leg / EDITED).write_bytes(b"mine\n") + git("commit", "-q", "-am", "mine", cwd=leg) + assert git("rebase", "-q", *backend, pin, cwd=leg, + check=False).returncode != 0 + assert head_ref(leg) is None and rev(leg) == pin + (leg / EDITED).write_bytes(b"resolved\n") + git("add", "--", EDITED, cwd=leg) + + +def applied(leg: Path, pin: str, start: str, tmp_path: Path) -> None: + """`other` at the pin, applying with `am -3` a patch made beside it that + changes the README the pin changes: it stops on the conflict, resolved + and staged, with HEAD on `other`.""" + git("checkout", "-q", "-b", "theirs", start, cwd=leg) + (leg / EDITED).write_bytes(b"theirs\n") + git("commit", "-q", "-am", "theirs", cwd=leg) + patch = tmp_path / "theirs.patch" + patch.write_bytes(git("format-patch", "-1", "--stdout", "HEAD", + cwd=leg).stdout.encode("utf-8")) + git("checkout", "-q", "-b", "other", pin, cwd=leg) + assert git("am", "-q", "-3", str(patch), cwd=leg, + check=False).returncode != 0 + (leg / EDITED).write_bytes(b"resolved\n") + git("add", "--", EDITED, cwd=leg) + + +@pytest.mark.parametrize("operation", ["rebase", "rebase --apply", "am"]) +def test_a_rebase_or_an_am_stopped_in_the_leg_is_not_moved_onto_main( + root, bootstrapped, tmp_path, operation): + """A rebase stopped with HEAD detached AT the pin (either backend), and + an `am` stopped on another branch, each over a conflict resolved and + staged. The checkout of `main` was made and the operation was left in + progress over it, its `--continue` then committing onto `main`. The pin + changes the README (`touch`), and `main` is moved to it, as a pull in + the leg would: the branch-at-the-pin state.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, "behind", + touch=EDITED) + leg = root / "spec" + git("branch", "-q", "-f", "main", pin, cwd=leg) + if operation == "am": + applied(leg, pin, start, tmp_path) + else: + rebased(leg, pin, start, *(["--apply"] if "apply" in operation + else [])) + refused_until_aborted(root, operation, pin) + + +# --- line 5: a HEAD on a ref outside refs/heads/ is left by every path ------ + +@PATHS +def test_a_head_on_a_remote_tracking_ref_is_left_where_it_is( + root, bootstrapped, tmp_path, path): + """`git symbolic-ref HEAD refs/remotes/origin/main`, at `main`'s commit + or the pin's where there is no `main`. P9 placed the leg (`on main`) and + the missing-branch path made `main` there, while T7 said `which is + neither` of the same HEAD. Every path leaves it now, as T7 did.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, path) + leg = root / "spec" + git("update-ref", "refs/remotes/origin/main", start, cwd=leg) + git("symbolic-ref", "HEAD", "refs/remotes/origin/main", cwd=leg) + before = snapshot(leg) + place = f"at {start[:12]} (on refs/remotes/origin/main)" + expected = { + "at": (f" [spec] spec: branch main is at the pin {pin[:12]}, and the " + f"checkout is {place}, which is neither\n" + + keep("the pin or the branch")), + "behind": (f" [spec] spec: branch main tip {start[:12]} is behind " + f"the PIN {pin[:12]}, and the checkout is {place}, which " + "is neither\n" + keep("the pin or the branch")), + "missing": (f" [spec] spec: branch main does not exist, and the " + f"checkout is {place}, which is not a branch\n" + + keep("the pin")), + }[path] + + result = skip_validators(root) + assert result.returncode == 0, result.stderr + result.stdout + assert expected in result.stdout, result.stdout + assert snapshot(leg) == before, "nothing moved" diff --git a/tests/test_bootstrap_without_git.py b/tests/test_bootstrap_without_git.py new file mode 100644 index 00000000..4f2985dd --- /dev/null +++ b/tests/test_bootstrap_without_git.py @@ -0,0 +1,174 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Both roots' `bootstrap.py` with no git to run refuse `git-not-runnable`, +exit 2, as the validators beside them do (#694). + +THE DEFECT, measured on lane 5's #682 head (8b39cd19) with `PATH` set to an +empty directory and the interpreter named by its path: the family root's +`make bootstrap` printed `(a) members and their legs`, `credential source: +none`, then a `FileNotFoundError: [Errno 2] No such file or directory: 'git'` +traceback, exit 1, which is the code of `family bootstrap FAILED`, though +nothing had failed. The assembly root's ended the same way in step (a), at +the first leg's `recorded_gitlink`, with `--skip-validators` too; and in a +clone whose legs are not initialized, where step (a) runs no git, its +offline upstream notice did, after `validate-pins.py` had refused +`git-not-runnable` itself. + +THE RULE NOW. Each refuses `git-not-runnable`, the validators' code, with +their first remedy sentence (`put git on PATH and run this again`), exit 2, +and no traceback. The run STOPS there: every later step runs git too, so +going on would only say it again. Which `OSError` is git's is read by NAME, +as `validate-pins.py` reads it: `filename` `git`, or none on a +`FileNotFoundError` or a `PermissionError`, which is how Windows raises +them; an error naming a directory goes on as it did. + +The end-to-end runs are a person's: a scaffold's recursive clone, a plain +clone, and a family holder, each run with no git on `PATH`. The rule's other +errors are asked of each root's own function, in this process. +""" + +from __future__ import annotations + +import errno +import os +from pathlib import Path + +import pytest + +from conftest import (PROJECT, REPO, clone_just_pushed, load_without_bytecode, + run_script) +from test_family import family, holder # noqa: F401 + +#: What `subprocess` says when there is no git to start, here. +NO_GIT = ("The system cannot find the file specified" if os.name == "nt" + else os.strerror(errno.ENOENT)) +#: The assembly root's remedy, after the validators' first sentence. +ASSEMBLY_REMEDY = ( + "Remediation: put git on PATH and run this again. Step (a) reads each " + "leg's pin and moves the leg through git, and the validators read the " + "pins through git, so without it nothing here can be placed or checked.") +#: The family root's. +FAMILY_REMEDY = ( + "Remediation: put git on PATH and run this again. Step (a) fetches every " + "member through git, and each member's own bootstrap places its legs " + "through git too, so without it nothing here can be fetched or " + "bootstrapped.") + + +def without_git(root: Path, tmp_path: Path, *extra: str, + env: dict | None = None): + """`scripts/bootstrap.py` in `root` with `PATH` an empty directory: no + git, and no `gh` either. The interpreter is `sys.executable`, named by + its path.""" + empty = tmp_path / "no-git" + empty.mkdir(exist_ok=True) + return run_script(root / "scripts" / "bootstrap.py", *extra, cwd=root, + env={**(env or {}), "PATH": str(empty)}) + + +def refused(result, *lines: str) -> None: + """Exit 2, no traceback, and stderr ending on `lines`.""" + assert result.returncode == 2, result.stderr + result.stdout + assert "Traceback" not in result.stderr, result.stderr + said = result.stderr.splitlines() + assert said[-len(lines):] == list(lines), result.stderr + + +@pytest.mark.parametrize("extra", [(), ("--skip-validators",)], + ids=["bootstrap", "skip-validators"]) +def test_an_assembly_root_with_no_git_refuses_in_step_a(project, tmp_path, + extra): + """THE ISSUE'S ASSEMBLY-ROOT CASE, run: a recursive clone. The first leg + is where git is first needed, and the refusal names it; no leg after it + is tried and no step after (a) runs.""" + result = without_git(project, tmp_path, *extra) + refused(result, + "REFUSED git-not-runnable: the spec leg at spec could not be " + f"placed on its tracking branch: git could not be run ({NO_GIT})", + ASSEMBLY_REMEDY) + assert result.stdout.endswith( + "\n(a) legs on tracking branches, pins untouched\n"), result.stdout + + +def test_the_offline_upstream_notice_with_no_git_refuses_too(scaffolded, + tmp_path): + """A PLAIN clone: both legs NOT INITIALIZED, so step (a) runs no git, and + `SHAPE_UPSTREAM_PATH` names a directory, so the notice after the + validators runs one. `validate-pins.py` refuses `git-not-runnable` + itself; the notice then ended the run in a traceback. It refuses, and + the review-authority step is not reached.""" + clone = tmp_path / PROJECT + made = clone_just_pushed(scaffolded["remotes"] / f"{PROJECT}.git", clone, + recurse=False) + assert made.returncode == 0, made.stderr + upstream = tmp_path / "upstream" + upstream.mkdir() + + result = without_git(clone, tmp_path, + env={"SHAPE_UPSTREAM_PATH": str(upstream)}) + refused(result, + "REFUSED git-not-runnable: the clone SHAPE_UPSTREAM_PATH names, " + f"{upstream}, could not be read: git could not be run ({NO_GIT})", + ASSEMBLY_REMEDY) + assert " [spec] spec: NOT INITIALIZED." in result.stdout, result.stdout + assert "(c) review authority" not in result.stdout + + +@pytest.mark.parametrize("extra", [(), ("--skip-members",)], + ids=["bootstrap", "skip-members"]) +def test_a_family_holder_with_no_git_refuses_at_its_fetch(holder, tmp_path, + extra): + """THE ISSUE'S FAMILY-ROOT CASE, run. Step (a)'s `git submodule update` + is where git is first needed; the run stops there, since each member's + own bootstrap would only refuse the same way (#694's question for + #648).""" + result = without_git(holder, tmp_path, *extra) + refused(result, + "REFUSED git-not-runnable: the members could not be fetched: git " + f"could not be run ({NO_GIT})", FAMILY_REMEDY) + assert result.stdout.endswith( + "\n(a) members and their legs\n credential source: none\n"), ( + result.stdout) + + +# --- which `OSError` is git's: each root's own function --------------------- + +@pytest.fixture(scope="module", params=["assembly-root", "family-root"]) +def root_module(request): + """Each root's `bootstrap.py`, loaded by path under a name of its own.""" + return load_without_bytecode( + REPO / "templates" / request.param / "scripts" / "bootstrap.py", + f"bootstrap_without_git_{request.param.replace('-', '_')}") + + +def made_of(module, error: OSError): + """What `module` makes of `error` raised where it starts git: the + `Refusal` it raises instead, or the error itself, raised on.""" + if hasattr(module, "_git_runs"): + try: + with module._git_runs("unread"): + raise error + except Exception as raised: # noqa: BLE001 - what is asked + return raised + return module.git_not_started(error) or error + + +@pytest.mark.parametrize("error", [ + FileNotFoundError(errno.ENOENT, "No such file or directory", "git"), + PermissionError(errno.EACCES, "Permission denied", "git"), + OSError(errno.ENOEXEC, "Exec format error", "git"), + FileNotFoundError(2, "The system cannot find the file specified"), +], ids=["no git", "git not executable", "git no program", "windows"]) +def test_each_way_git_cannot_start_is_git_not_runnable(root_module, error): + made = made_of(root_module, error) + assert made.code == "git-not-runnable", made + assert made.detail.endswith( + f": git could not be run ({error.strerror})"), made.detail + + +@pytest.mark.parametrize("error", [ + FileNotFoundError(errno.ENOENT, "No such file or directory", "/nowhere"), + PermissionError(errno.EACCES, "Permission denied", "/a/leg"), + OSError(errno.EIO, "Input/output error"), +], ids=["a directory not there", "a directory not entered", "nameless"]) +def test_an_error_that_is_not_gits_goes_on_as_it_did(root_module, error): + assert made_of(root_module, error) is error From 1a69505d8814ae0cc37e0c4281353c5caaedefaf Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 18:57:59 +0000 Subject: [PATCH 06/10] bootstrap.py: a plain clone's update that dies at a gitlink no section gives a url fails the run naming every row, and a member git exited 0 over is given the update that checks it out, so a stray case variant, a hand mount or a url-less section no longer ends `family bootstrap ok` (#648, rv648) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix round for rv648's review of 8b39cd19 (1 MUST, 2 SHOULD, 5 NIT). M1. In a plain clone, `git submodule update --init` dies in its init step at the first gitlink, in index order, that no section of `.gitmodules` gives a url, and checks out no member. `unfetchable` asked only each row's own gitlink and section, so a stray `Members/

` beside `members/

` (#361's STRAY), a member mounted by hand with no section, or a row's section that lost its url (rv648's S1, H1 and U1) passed every row as the credential skip, and the run ended ok. `init_stop` now asks the holder's records which gitlink git's init stops at: the index's gitlinks in order, each against the work tree's sections (`gitmodules_sections`, which replaces `gitmodules_paths`) and the git config. Measured before it was taken, rv648's prototype failed three states that are a missing credential: a recursive clone of the stray or of the hand mount with no credential, and a url-less section an earlier `git submodule init` registered. So two of git's rules are kept: where the config sets `submodule.active` (a recursive clone sets `.`), init asks only the gitlinks a section gives, and a section with no url does not stop it where the config holds a url under its name. Where git exited non-zero and init stops at a gitlink, every row not checked out is named `git stopped at , a gitlink no section of .gitmodules gives a url`, and the run fails. What cures it: `validate-family.py --no-members` names a gitlink no row names and the commit that cures it (members checked, it refuses `member-uninitialized` for the first row in these states, a checklist candidate). Where a row names the gitlink, nothing in the validator does, so the line says to write that section back as `family.py add` writes one. Each why now carries its own sentence. S1. For "git exited 0 and checked nothing out there" (a subset clone, an `update = none`), the line printed the validator sentence, whose `member-uninitialized` remedy exits 0 and checks nothing out. It prints `git submodule update --init --checkout --recursive -- ` instead; typed, it checks the member out in both states and the run ends ok. S2. `test_a_missing_credential_is_still_the_skip_it_was` asserts that " every member and leg fetched" is not printed (rv648's mutant R2). NITs. README and the handbook say `make bootstrap` skips a member it lacks a credential for, and the § Families marker is the new segment's blob, b3d19092; README stays at 1459 lines, its cap. The docstrings say "an empty mapping" and "a missing credential exits non-zero too", and `validate-family.py`'s WHAT IS CHECKED says the move under `--no-members` is printed only for a canonical path and a 40-hex pin. Tests: the variant-plain case and the round trip name MEMBER too; new tests for S1 and H1 (typed through `--no-members` and committed, the run checks both out), U1 (the section written back), the two recursive clones with no credential and the registered url (still the skip), and the update that exits 0, by `update = none` and by a subset clone, with its printed line typed. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- docs/handbook.html | 6 +- templates/family-root/scripts/bootstrap.py | 181 ++++++++-- .../family-root/scripts/validate-family.py | 7 +- tests/test_family_bootstrap_gate_order.py | 310 +++++++++++++++--- 5 files changed, 424 insertions(+), 82 deletions(-) diff --git a/README.md b/README.md index 0f17b2b8..4a4e78fa 100644 --- a/README.md +++ b/README.md @@ -952,7 +952,7 @@ at the right commit. **A family pins assembly roots, never legs**: the GitHub App or `SHAPE_LEGS_TOKEN` credential the workflow does, as a member is a private assembly root mounting two private legs — then runs each member's own `make bootstrap`; `make validate` runs the family validator, then each -member's. One it lacks a credential for is reported and skipped, not a failure. +member's. `make bootstrap` reports and skips one it lacks a credential for. The holder carries the same COPY pin an assembly root does, so `update-shape.py` re-syncs it the same way; it mirrors into `family.yaml` diff --git a/docs/handbook.html b/docs/handbook.html index f2e382e2..0c63875c 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1282,7 +1282,7 @@

Adopting an existing repository

- +

Reference

Families: a holder for projects that ship separately

@@ -1375,8 +1375,8 @@

The InkRouter example

resolving the same GitHub App or SHAPE_LEGS_TOKEN credential the workflow does — and then runs each member’s own make bootstrap. make validate runs the family validator and then each member’s own - make validate. A member it lacks a credential for is reported and skipped, not - a failure. The holder carries the same COPY pin an assembly root does, so + make validate. A member make bootstrap lacks a credential for is + reported and skipped, not a failure. The holder carries the same COPY pin an assembly root does, so update-shape.py re-syncs it the same way; it mirrors into family.yaml instead of project.yaml and must leave validate-family.py green.

diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index e3fb3b10..4e089c99 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -31,13 +31,22 @@ every row's member is checked out (#648). A member left NOT CHECKED OUT where no credential would check it out is said here instead, with why, and fails the run (c): this holder records no gitlink at the - row's path (a gitlink recorded only in another case, #363), no + row's path (a gitlink recorded only in another case, #363); no section of `.gitmodules` gives that path (a holder whose HEAD lost the - file, #611), or git exited 0 and checked nothing out there. Git exits - 0 over the second where a recursive clone ran first, and 128 (`No url - found for submodule path`) in a plain clone, as it does for a missing - credential, so what the holder records is the test, never git's - words. The line names `validate-family.py`, which says what to commit. + file, #611); git exited 0 and checked nothing out there; or git + stopped at a gitlink no section gives a url, the row's own or another + (a stray case variant, a member mounted by hand, a section that lost + its url), since git's init dies at the first such gitlink it asks + about, before it checks out any member. Git exits 0 over the second + where a recursive clone ran first, and 128 (`No url found for + submodule path`) in a plain clone; a missing credential exits + non-zero too, so what the holder records is the test, never git's + words. The line says what cures each: `validate-family.py` names the + commit for the first two; for the third it prints the `git submodule + update --init --checkout` that checks the member out whatever the + clone chose; and for the last, `validate-family.py --no-members` + names a gitlink no row names, and where the gitlink is a row's own + the line says to write its section back. (b) RUNS EACH MEMBER'S OWN `make bootstrap`. Each member is a whole project and knows how to bootstrap itself — its legs onto their tracking @@ -60,6 +69,7 @@ import argparse import os +import shlex import shutil import subprocess import sys @@ -74,6 +84,17 @@ MANIFEST = "family.yaml" GITMODULES = ".gitmodules" DEFAULT_MEMBERS_DIR = "members" +#: A submodule's entry in the index, as `ls-files -s` spells its mode. +GITLINK_MODE = b"160000" + +#: Why (a) left a row's member NOT CHECKED OUT where no credential would +#: check it out (#648), in the order `unfetchable` asks. `STOPPED` is +#: filled with the gitlink git's init died at (`init_stop`). +NO_GITLINK = "no gitlink is recorded at that path" +NO_SECTION = f"no section of {GITMODULES} gives that path" +EXITED_0 = "git exited 0 and checked nothing out there" +STOPPED = ("git stopped at {}, a gitlink no section of " + GITMODULES + + " gives a url") #: The two environment variables a leg/member credential may arrive in, in the #: order the `validate` workflow resolves them. An App-minted installation @@ -124,71 +145,159 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: "fine-grained PAT with contents:read on them) or export an " "App-minted SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive " "is bootstrapped below.", file=sys.stderr) - left = unfetchable(root, rows, proc.returncode == 0) + left, stopper = unfetchable(root, rows, proc.returncode == 0) if left: print(" NOT CHECKED OUT, and not for want of a credential: " - + ", ".join(f"{spelled(path)} ({why})" for _, path, why in left) - + f". `{PYTHON} scripts/validate-family.py` names the cause and " - "the commit that cures it.", file=sys.stderr) + + ", ".join(f"{spelled(path)} (" + + (STOPPED.format(spelled(stopper)) + if why == STOPPED else why) + ")" + for _, path, why in left) + + ". " + cures(left, stopper, rows), file=sys.stderr) elif proc.returncode == 0: print(" every member and leg fetched") return [f"{spelled(row.get('project') or '?')} (not checked out)" for row, _, _ in left] -def gitmodules_paths(root: Path) -> set[str]: - """Every path a section of `.gitmodules` gives, read as git reads it: - the work tree's file, the one `git submodule update` reads, which a - fresh clone takes from HEAD. None where there is no such file, or one - git cannot read: git has no url for any submodule then either.""" +def cures(left: list[tuple[dict, str, str]], stopper: str | None, + rows: list[dict]) -> str: + """What (a)'s line says cures the rows `left` names, one sentence for + each why among them (#648). `validate-family.py`, which names the + commit, for a gitlink or a section the holder lacks at the row's path. + For the gitlink git stopped at, `validate-family.py --no-members`, which + names a gitlink no row names and the commit that cures it (members + checked, it refuses `member-uninitialized` for the first row there + instead), or, where a row names it, its section written back. For a row + git exited 0 over, the update that checks it out whatever the clone + chose: `--init` with the path registers a member a subset clone left + out, and `--checkout` overrides an `update = none`.""" + whys = {why for _, _, why in left} + said = [] + if whys & {NO_GITLINK, NO_SECTION}: + said.append(f"`{PYTHON} scripts/validate-family.py` names the cause " + "and the commit that cures it.") + if STOPPED in whys and stopper in {row_path(row) for row in rows}: + said.append(f"Write the section of {GITMODULES} for " + f"{spelled(stopper)} back as `family.py add` writes one, " + f"with its path and url, and commit {GITMODULES}.") + elif STOPPED in whys: + said.append(f"`{PYTHON} scripts/validate-family.py --no-members` " + f"names each gitlink in {DEFAULT_MEMBERS_DIR}/ that no " + "row names, and the commit that cures it.") + exited = [path for _, path, why in left if why == EXITED_0] + if exited: + update = shlex.join(["git", "submodule", "update", "--init", + "--checkout", "--recursive", "--", *exited]) + said.append(f"`{spelled(update)}` checks " + f"{'it' if len(exited) == 1 else 'them'} out.") + return " ".join(said) + + +def gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: + """`{name: {key: value}}` of every section of `.gitmodules`, read as git + reads it: the work tree's file, the one `git submodule update` reads, + which a fresh clone takes from HEAD. An empty mapping where there is no + such file, or one git cannot read: git has no url for any submodule + then either.""" if not os.path.lexists(root / GITMODULES): - return set() + return {} try: listed = git_out(["config", "-z", "--file", GITMODULES, "--list"], cwd=root, binary=True) except Refusal: - return set() - paths: set[str] = set() + return {} + sections: dict[str, dict[str, str]] = {} for record in listed.split(b"\0"): key, _, value = record.decode("utf-8", "surrogateescape").partition("\n") - if key.startswith("submodule.") and key.endswith(".path"): - paths.add(value) - return paths + section, _, variable = key.rpartition(".") + if section.startswith("submodule."): + sections.setdefault(section[len("submodule."):], + {})[variable] = value + return sections + + +def init_stop(root: Path, sections: dict[str, dict[str, str]]) -> str | None: + """The gitlink `git submodule update --init` dies at, or None: the first + in the index, in index order, that its init step asks about and finds + no url for (`No url found for submodule path`, exit 128), before it + checks out any member (#648). + + ASKED AS GIT 2.43 ASKS IT, of the index, `sections` and this + repository's git config. Every gitlink the index records is asked, but + where the config sets `submodule.active`, as a recursive clone sets it + (`.`), only those a section gives a path: git counts no other active. + One no section gives stops it; so does one whose section gives no url, + unless the config has one under the section's name, as an earlier init + writes it. So a stray case variant or a member mounted by hand stops a + plain clone's update, and leaves a recursive clone's to the credential. + Read nothing, it answers None.""" + try: + index = git_out(["ls-files", "-s", "-z"], cwd=root, binary=True) + except Refusal: + return None + try: + config = git_out(["config", "-z", "--get-regexp", r"^submodule\."], + cwd=root, binary=True) + except Refusal: + config = b"" + keys = {record.decode("utf-8", "surrogateescape").partition("\n")[0] + for record in config.split(b"\0")} + names: dict[str, list[str]] = {} + for name, held in sections.items(): + if "path" in held: + names.setdefault(held["path"], []).append(name) + for record in index.split(b"\0"): + meta, _, entry = record.partition(b"\t") + if meta.split()[:1] != [GITLINK_MODE]: + continue + path = entry.decode("utf-8", "surrogateescape") + given = names.get(path, []) + if not given and "submodule.active" in keys: + continue + if not any("url" in sections[name] or f"submodule.{name}.url" in keys + for name in given): + return path + return None -def unfetchable(root: Path, rows: list[dict], - exited_0: bool) -> list[tuple[dict, str, str]]: +def unfetchable(root: Path, rows: list[dict], exited_0: bool + ) -> tuple[list[tuple[dict, str, str]], str | None]: """`(row, path, why)` for each row whose member (a) left NOT CHECKED OUT - where no credential would check it out (#648), in row order. + where no credential would check it out (#648), in row order, and the + gitlink git's init died at (`init_stop`), or None. Asked of this holder's own records, never of git's words: a row at whose path the holder records no gitlink (`recorded_gitlink`, as `validate-family.py`'s `member-gitlink-absent` reads one), or whose gitlink no section of `.gitmodules` gives a path (as its `member-gitmodules-absent` reads HEAD's file, which is the work tree's - in a clone); and, where git exited 0, any row at all, since git then - lacked nothing a credential gives. Any other row not checked out is the - missing credential's, and stays the skip it always was. A path two rows - give is said once.""" + in a clone); where git exited 0, any row at all, since git then lacked + nothing a credential gives; and where git's init died at a gitlink no + section gives a url, any row at all, since git then checked out no + member and asked for no credential. Any other row not checked out is + the missing credential's, and stays the skip it always was. A path two + rows give is said once.""" left: dict[str, dict] = {} for row in rows: if not (member_path(root, row) / ".git").exists(): left.setdefault(row_path(row), row) if not left: - return [] - given = gitmodules_paths(root) + return [], None + sections = gitmodules_sections(root) + given = {held["path"] for held in sections.values() if "path" in held} + stopper = None if exited_0 else init_stop(root, sections) found: list[tuple[dict, str, str]] = [] for path, row in left.items(): if recorded_gitlink(root, path) is None: - found.append((row, path, "no gitlink is recorded at that path")) + found.append((row, path, NO_GITLINK)) elif path not in given: - found.append((row, path, f"no section of {GITMODULES} gives that " - "path")) + found.append((row, path, NO_SECTION)) elif exited_0: - found.append((row, path, "git exited 0 and checked nothing out " - "there")) - return found + found.append((row, path, EXITED_0)) + elif stopper is not None: + found.append((row, path, STOPPED)) + return found, stopper def members_of(root: Path) -> tuple[str, list[dict]]: diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 1e593d20..cee36ce5 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -64,9 +64,10 @@ 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, and for a member - recorded only in the wrong case it names the index move that records it - at its row's path, as the row's own refusal does members checked - (#648). + recorded only in the wrong case, where its row's path is its canonical + `members/` and its pin a 40-hex commit, it names the index move + that records it at that path, as the row's own refusal does members + checked (#648). - that a section of HEAD's `.gitmodules` gives the path of every row's 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 diff --git a/tests/test_family_bootstrap_gate_order.py b/tests/test_family_bootstrap_gate_order.py index 9962f731..bbe04750 100644 --- a/tests/test_family_bootstrap_gate_order.py +++ b/tests/test_family_bootstrap_gate_order.py @@ -11,8 +11,13 @@ ok. In a plain clone of that holder, or of #363's state (a member's gitlink recorded only at `Members/

`), git dies `No url found for submodule path` (exit 128), and it still ended ok. It fails now, saying - which members no credential would check out and why, and names - `validate-family.py`. A missing credential is still the skip it was. + which members no credential would check out and why, and what cures + them. So it does where git's init dies at a gitlink no section gives a + url, a stray case variant, a member mounted by hand or a section that + lost its url (rv648's S1, H1 and U1), and where git exits 0 over a member + a subset clone or `update = none` leaves out, whose line prints the + update that checks it out. A missing credential is still the skip it + was, after a recursive clone too. - (b) In a plain clone of #363's state followed by `git submodule update --init --recursive`, as CI and `make bootstrap` run it, the update dies at `Members/

` before it checks out any member, so where that row is not @@ -26,11 +31,13 @@ THE SAME FAMILY, OFFLINE RULES AND STATES as `test_family_head_lost_gitmodules.py` (#611), whose helpers are imported. Every remedy is typed as printed. The case-variant states are index records -and commits, never two directories whose names differ only in case, and -the holder's own are asked again with `core.ignorecase=true`. A clone's -checkout is git's: on a disk that folds case, `Members/

` IS the row's -directory, so the `mv` the text gives for a case-sensitive disk is typed -only where the disk holds the two apart, as the text says. +and commits, and no test makes two directories whose names differ only in +case; the holder's own are asked again with `core.ignorecase=true`. A +clone's checkout is git's: on a disk that folds case, `Members/

` IS the +row's directory, so the `mv` the text gives for a case-sensitive disk is +typed only where the disk holds the two apart, as the text says, and a +clone of a stray (`Members/

` beside `members/

`, rv648's S1) gets its +two empty mounts there as one directory, which no assertion reads. """ from __future__ import annotations @@ -46,26 +53,45 @@ from conftest import clone_just_pushed, git, run_script from repo_shape import PYTHON -from test_family import NAME, family, validate # noqa: F401 +from test_family import NAME, SPARE, family, validate # noqa: F401 from test_family_add_refusals import holder, offline # noqa: F401 from test_family_dry_run import isolated from test_family_head_lost_gitmodules import ( - DISKS, assert_passes_in_every_mode, commit_all, head_lost, on_disk, - pinned, recorded_at, variant_refused, + DISKS, GITMODULES, assert_passes_in_every_mode, commit_all, + committed_gitmodules, head_lost, on_disk, pinned, recorded_at, + variant_refused, ) from test_family_member_path import MEMBER, OTHER, raw from test_validate_family_unnamed_mounts import ( - GITLINK_SAID, VARIANT, WRONG_CASE, commands, findings, stage_gitlink, + GITLINK_SAID, VARIANT, WRONG_CASE, commands, findings, spare_commits, + stage_gitlink, ) #: The line `(a)` prints for the members it left NOT CHECKED OUT where no -#: credential would check them out, and the one the run then ends on. +#: credential would check them out, each with why, then what cures them; +#: the run then ends on `failed_on`. NOT_FETCHED = (" NOT CHECKED OUT, and not for want of a credential: {left}. " - f"`{PYTHON} scripts/validate-family.py` names the cause and " - "the commit that cures it.") + "{cure}") NO_SECTION = "no section of .gitmodules gives that path" NO_GITLINK = "no gitlink is recorded at that path" EXITED_0 = "git exited 0 and checked nothing out there" +STOPPED = ("git stopped at {path}, a gitlink no section of .gitmodules " + "gives a url") + +#: What the line says cures each why: the validator for the first two; for +#: the gitlink git stopped at, the validator's `--no-members`, or, where a +#: row names that gitlink, its section written back; and for a member git +#: exited 0 over, the update that checks it out. +VALIDATOR_CURE = (f"`{PYTHON} scripts/validate-family.py` names the cause " + "and the commit that cures it.") +UNNAMED_CURE = (f"`{PYTHON} scripts/validate-family.py --no-members` names " + "each gitlink in members/ that no row names, and the commit " + "that cures it.") +SECTION_CURE = ("Write the section of .gitmodules for {path} back as " + "`family.py add` writes one, with its path and url, and " + "commit .gitmodules.") +CHECKOUT_CURE = ("`git submodule update --init --checkout --recursive -- " + "{path}` checks it out.") #: Each test pushes a branch of its own into the module's family remote. Not #: `tmp_path.name`: a passed test's directory is removed under @@ -130,10 +156,43 @@ def bootstrap(root: Path, env: dict, *extra: str): env=env) -def not_fetched(*left: tuple[str, str]) -> str: - """`NOT_FETCHED` naming each `(path, why)`.""" +def not_fetched(*left: tuple[str, str], cure: str = VALIDATOR_CURE) -> str: + """`NOT_FETCHED` naming each `(path, why)`, then `cure`.""" return NOT_FETCHED.format( - left=", ".join(f"{path} ({why})" for path, why in left)) + left=", ".join(f"{path} ({why})" for path, why in left), cure=cure) + + +def stray(holder: Path, env: dict, family) -> str: + """#361's STRAY: OTHER's gitlink recorded at `Members/

` too, at its + pin, beside its own, and committed; no section gives the second. Index + order puts it before every member's. Returns its path.""" + stage_gitlink(holder, f"Members/{OTHER}", pinned(holder, OTHER)) + commit_all(holder, env, "A stray case variant of OTHER's gitlink") + return f"Members/{OTHER}" + + +def hand_mounted(holder: Path, env: dict, family) -> str: + """SPARE's gitlink recorded at `members/` by hand, with no row + and no section, and committed. Returns its path.""" + stage_gitlink(holder, f"members/{SPARE}", spare_commits(family)[1]) + commit_all(holder, env, "A member mounted by hand") + return f"members/{SPARE}" + + +#: The two gitlinks no row names at which git's init dies in a plain clone +#: (rv648's S1 and H1). +UNNAMED_STOPS = {"stray": stray, "hand-mount": hand_mounted} + + +def without_url(data: bytes, project: str) -> bytes: + """`.gitmodules` bytes with the `url` line of `project`'s section taken + out, the rest as `family.py add` wrote it (rv648's U1). A checkout's + copy may end its lines CRLF.""" + section = re.compile(rb'(\[submodule "members/' + project.encode() + + rb'"\]\r?\n(?:\t[^\n]*\n)*?)\turl = [^\n]*\n') + edited, count = section.subn(rb"\1", data) + assert count == 1, data + return edited def failed_on(*projects: str) -> str: @@ -182,34 +241,42 @@ def moved_out_where_apart(root: Path, remediation: str) -> None: # --- (a) bootstrap ----------------------------------------------------------- -#: `(set-up, flow, the members (a) names with why, the rows it fails on)`. +#: `(set-up, flow, the members (a) names with why, the rows it fails on, +#: what it says cures them)`. In #363's plain clone git dies at the variant, +#: `Members/

`, before it checks out MEMBER, so MEMBER is named too, and +#: the variant is a gitlink no row names (rv648's M1). UNFETCHED = { "head-lost-recursive": ( head_lost, True, ((f"members/{MEMBER}", NO_SECTION), (f"members/{OTHER}", NO_SECTION)), - (MEMBER, OTHER)), + (MEMBER, OTHER), VALIDATOR_CURE), "head-lost-plain": ( head_lost, False, ((f"members/{MEMBER}", NO_SECTION), (f"members/{OTHER}", NO_SECTION)), - (MEMBER, OTHER)), + (MEMBER, OTHER), VALIDATOR_CURE), "variant-recursive": ( - variant, True, ((f"members/{OTHER}", NO_GITLINK),), (OTHER,)), + variant, True, ((f"members/{OTHER}", NO_GITLINK),), (OTHER,), + VALIDATOR_CURE), "variant-plain": ( - variant, False, ((f"members/{OTHER}", NO_GITLINK),), (OTHER,)), + variant, False, + ((f"members/{MEMBER}", STOPPED.format(path=f"Members/{OTHER}")), + (f"members/{OTHER}", NO_GITLINK)), + (MEMBER, OTHER), f"{VALIDATOR_CURE} {UNNAMED_CURE}"), } -@pytest.mark.parametrize("set_up, recurse, left, projects", +@pytest.mark.parametrize("set_up, recurse, left, projects, cure", UNFETCHED.values(), ids=UNFETCHED) def test_bootstrap_fails_over_members_no_credential_would_check_out( - family, holder, offline, tmp_path, set_up, recurse, left, projects): + family, holder, offline, tmp_path, set_up, recurse, left, projects, + cure): """THE ISSUE'S (a), both of rv611b's states, after a recursive clone and after a plain one. Git exits 0 over a holder whose HEAD lost `.gitmodules` where the clone was recursive, and 128 (`No url found for submodule path`) where it was plain, and 128 over #363's state in a plain clone: the run said ok each time. It names the members no - credential would check out, says why, names `validate-family.py`, and - fails; it never says every member was fetched.""" + credential would check out, says why and what cures them, and fails; + it never says every member was fetched.""" env = isolated(offline, tmp_path) set_up(holder, env) clone = cloned(holder, env, family, tmp_path, @@ -220,8 +287,8 @@ def test_bootstrap_fails_over_members_no_credential_would_check_out( assert result.returncode == 1, result.stdout + result.stderr assert after_first_line(result) == NOT_ALL_FETCHED errors = result.stderr.splitlines() - assert errors[-2:] == [not_fetched(*left), failed_on(*projects)], ( - result.stderr) + assert errors[-2:] == [not_fetched(*left, cure=cure), + failed_on(*projects)], result.stderr if not recurse: assert errors[0].startswith(" FETCH INCOMPLETE (git exit 128) "), ( result.stderr) @@ -259,31 +326,191 @@ def test_a_missing_credential_is_still_the_skip_it_was(family, holder, assert result.returncode == 0, result.stdout + result.stderr assert "FETCH INCOMPLETE" in result.stderr assert "not for want of a credential" not in result.stderr + assert (" every member and leg fetched" + not in result.stdout.splitlines()), result.stdout for project in (MEMBER, OTHER): assert (f" [{project}] {project}: NOT CHECKED OUT; skipped. See the " "fetch above.") in result.stdout.splitlines() assert result.stdout.endswith("\nfamily bootstrap ok\n"), result.stdout +def skip_by_update_none(clone: Path, env: dict) -> dict: + """`submodule..update=none` for OTHER, set for every git the run + starts, as a `.gitmodules` committed with `update = none` sets it.""" + return with_config(env, f"submodule.members/{OTHER}.update", "none") + + +def skip_by_subset(clone: Path, env: dict) -> dict: + """`submodule.active` in the clone's own git config naming MEMBER alone, + as `git clone --recurse-submodules=members/

` stores it.""" + raw(clone, env, "config", "submodule.active", f"members/{MEMBER}") + return env + + +@pytest.mark.parametrize("skip", [skip_by_update_none, skip_by_subset], + ids=["update-none", "subset"]) def test_an_update_that_exits_0_and_skips_a_member_fails(family, holder, - offline, tmp_path): + offline, tmp_path, + skip): """Git exited 0, so it lacked nothing a credential gives, though the - holder's records give OTHER: here `submodule..update=none` set - for the run skips it. (a) does not say every member was fetched, and - the run fails on OTHER alone.""" - env = isolated(offline, tmp_path) + holder's records give OTHER: an `update = none`, or a clone that chose + a subset of members, skips it (rv648's N1 and P1). (a) does not say + every member was fetched, and the run fails on OTHER alone, printing + the update that checks it out: `--init` with its path registers it, and + `--checkout` overrides the `none`. `git submodule update --init + --recursive`, `member-uninitialized`'s remedy, exits 0 there and checks + nothing out. Typed as printed, the line lets the same run end ok.""" + env = fetching(isolated(offline, tmp_path)) clone = cloned(holder, env, family, tmp_path, recurse=False) - skip = with_config(fetching(env), f"submodule.members/{OTHER}.update", - "none") + env = skip(clone, env) - result = bootstrap(clone, skip, "--skip-members") + result = bootstrap(clone, env, "--skip-members") assert result.returncode == 1, result.stdout + result.stderr assert (clone / "members" / MEMBER / "project.yaml").is_file() assert "every member and leg fetched" not in result.stdout assert result.stderr.splitlines() == [ - not_fetched((f"members/{OTHER}", EXITED_0)), failed_on(OTHER)] + not_fetched((f"members/{OTHER}", EXITED_0), + cure=CHECKOUT_CURE.format(path=f"members/{OTHER}")), + failed_on(OTHER)] + raw(clone, env, "submodule", "update", "--init", "--recursive") + assert not (clone / "members" / OTHER / ".git").exists(), ( + "premise: the validator's remedy checks nothing out here") + (line,) = re.findall(r"`(git submodule update [^`]*)`", result.stderr) + type_line(clone, env, line) + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + assert again.stderr == "" + + +@pytest.mark.parametrize("set_up", UNNAMED_STOPS.values(), ids=UNNAMED_STOPS) +def test_a_gitlink_no_row_names_that_stops_a_plain_clones_update_fails( + family, holder, offline, tmp_path, set_up): + """rv648's S1 and H1. In a plain clone of a holder that records a stray + case variant of a member's gitlink, or a member mounted by hand, git's + init dies at that gitlink (`No url found for submodule path`, exit 128) + before it checks out any member, though every row's gitlink and section + are there: the run said ok. It names both rows, says where git stopped, + and names `--no-members`, which names that gitlink and the commit that + cures it. Typed as printed (all but the hand mount's `family.py add`, + which mounts it again) and committed, the same run checks both members + out.""" + env = fetching(isolated(offline, tmp_path)) + stopper = set_up(holder, env, family) + commit = raw(holder, env, "rev-parse", + f"HEAD:{stopper}").stdout.decode("ascii").strip() + clone = cloned(holder, env, family, tmp_path, recurse=False) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + why = STOPPED.format(path=stopper) + assert result.stderr.splitlines()[-2:] == [ + not_fetched((f"members/{MEMBER}", why), (f"members/{OTHER}", why), + cure=UNNAMED_CURE), + failed_on(MEMBER, OTHER)], result.stderr + assert f"No url found for submodule path '{stopper}'" in result.stderr + fallback = validate(clone, "--no-members") + assert fallback.returncode == 1, fallback.stdout + fallback.stderr + (finding,) = findings(fallback) + assert finding.startswith(GITLINK_SAID.format(path=stopper, + commit=commit)), finding + for line in commands(finding): + if not line.startswith(f"{PYTHON} "): + type_line(clone, env, line) + commit_all(clone, env, "Take the gitlink out as printed") + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + + +def test_a_section_with_no_url_that_stops_the_update_says_to_write_it_back( + family, holder, offline, tmp_path): + """rv648's U1. OTHER's section lost its url, committed: in a plain + clone git's init dies at OTHER's own gitlink, after MEMBER's and before + it checks out either. The run said ok, and no mode of the validator + names it. It names both rows and says to write that section back; + written back as `family.py add` wrote it and committed, the same run + checks both members out.""" + env = fetching(isolated(offline, tmp_path)) + data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout + committed_gitmodules(holder, env, without_url(data, OTHER)) + clone = cloned(holder, env, family, tmp_path, recurse=False) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + why = STOPPED.format(path=f"members/{OTHER}") + assert result.stderr.splitlines()[-2:] == [ + not_fetched((f"members/{MEMBER}", why), (f"members/{OTHER}", why), + cure=SECTION_CURE.format(path=f"members/{OTHER}")), + failed_on(MEMBER, OTHER)], result.stderr + assert (f"No url found for submodule path 'members/{OTHER}'" + in result.stderr) + (clone / GITMODULES).write_bytes(data) + raw(clone, env, "add", "--", GITMODULES) + commit_all(clone, env, "OTHER's section written back") + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + + +@pytest.mark.parametrize("set_up", UNNAMED_STOPS.values(), ids=UNNAMED_STOPS) +def test_a_recursive_clone_with_no_credential_is_still_the_skip( + family, holder, offline, tmp_path, set_up): + """No misfire. A recursive clone sets `submodule.active` (`.`), and + git's init then asks only the gitlinks a section gives a path, so the + stray or the hand mount stops nothing there, and git fails on the + members' own fetch, as a private member with no token fails. The + members are the credential's: skipped, and the run ends ok.""" + env = isolated(offline, tmp_path) + set_up(holder, env, family) + remote = family["remotes"] / f"{NAME}.git" + branch = f"w648/{next(BRANCHES)}" + raw(holder, env, "push", "-q", str(remote), f"HEAD:refs/heads/{branch}") + clone = tmp_path / "clone" + # The holder is fetched, as a person with access to it fetches it, and + # its members are not: `user` refuses the file transport to the clones + # git makes for submodules. + made = clone_just_pushed(remote, clone, branch, + config=["-c", "protocol.file.allow=user"]) + assert made.returncode != 0, "premise: the members were not fetched" + assert "transport 'file' not allowed" in made.stderr, made.stderr + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 0, result.stdout + result.stderr + assert "FETCH INCOMPLETE" in result.stderr + assert "not for want of a credential" not in result.stderr + + +def test_a_url_an_earlier_init_registered_is_still_the_skip(family, holder, + offline, + tmp_path): + """No misfire. `git submodule init` wrote each member's url into the + clone's git config, then the work tree's section for OTHER lost its + url: git's init reads the url it wrote and does not stop there, and git + fails on the fetch for want of a credential. The members are skipped and + the run ends ok.""" + env = isolated(offline, tmp_path) + clone = cloned(holder, env, family, tmp_path, recurse=False) + raw(clone, env, "submodule", "init") + data = (clone / GITMODULES).read_bytes() + (clone / GITMODULES).write_bytes(without_url(data, OTHER)) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 0, result.stdout + result.stderr + assert "FETCH INCOMPLETE" in result.stderr + assert "not for want of a credential" not in result.stderr def test_a_row_written_twice_is_named_once(family, holder, offline, @@ -314,7 +541,8 @@ def test_a_plain_clone_of_a_case_variant_is_named_and_mended_as_printed( family, holder, offline, tmp_path): """The round trip a person makes. In a plain clone of #363's state, with the variant at the SECOND row, `make bootstrap`'s script fails naming - OTHER and the validator; the validator, members checked, names the + both rows, since git stopped at the variant before it checked out + MEMBER, and the validator; the validator, members checked, names the variant and the move, where it refused `member-uninitialized` for the first row before. Typed as printed in the clone, with the commit and the `mv` where the disk holds the two apart, the same script checks @@ -328,7 +556,11 @@ def test_a_plain_clone_of_a_case_variant_is_named_and_mended_as_printed( assert booted.returncode == 1, booted.stdout + booted.stderr assert booted.stderr.splitlines()[-2:] == [ - not_fetched((f"members/{OTHER}", NO_GITLINK)), failed_on(OTHER)] + not_fetched((f"members/{MEMBER}", + STOPPED.format(path=f"Members/{OTHER}")), + (f"members/{OTHER}", NO_GITLINK), + cure=f"{VALIDATOR_CURE} {UNNAMED_CURE}"), + failed_on(MEMBER, OTHER)] gate = validate(clone) assert gate.returncode == 2, gate.stdout + gate.stderr lines = gate.stderr.splitlines() From a1f2433f94c7efb5742e97c0bfa07974754cfbe9 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 19:07:13 +0000 Subject: [PATCH 07/10] bootstrap.py: a cherry-pick or revert sequence whose stop was committed by hand is refused as in progress, the missing-branch path names the branch it left and judges `checkout -b` by where HEAD is, and HEAD's ref is read as bytes, so rv704's S1-S3 no longer happen (#704) The fix round for rv704's review of #745 at 4e2b211b. Every change is in templates/assembly-root/scripts/bootstrap.py and tests/test_bootstrap_leg_moves.py. - S1. When `git cherry-pick A B` or `git revert A B` stops and the person commits the stop by hand, the head file is gone and only `sequencer/todo` is left. Bootstrap moved the leg onto `main`, and the `--continue` that `git status` names then committed B there. `_in_progress` now asks `sequencer/todo` after the `REVERT_HEAD` row. The todo's first word picks the label: `revert` is a revert, anything else is a cherry-pick. Docstring (a) and `_say_in_progress` say so. - S2. `create_at_pin` prints `_say_left` under `created` when HEAD was on another branch, as P9 and T7 do. - S3. `create_at_pin` judges `checkout -b` by where HEAD is afterwards, as `_check_out` does (#533). Under a post-checkout hook that fails, it now says `created`, with git's words after it. It used to print `could not create ...` and `nothing else is tried` over a leg that was on `main` at the pin. - The S3 check reads HEAD back right after `checkout -b`. `_head_of` read `symbolic-ref` through `git_out`, which decodes strict UTF-8, so a branch whose name is not UTF-8 (`--branch` a name holding U+DCFF) would end the first run in a `UnicodeDecodeError`. On base the second run already ended that way, in `place_at_pin`. `_head_of` now reads the ref as bytes and decodes it with `os.fsdecode`. - N1. Docstring (d) now names where the exit is 2: step (a) and the upstream notice. - N2. The composite assert in the rebase helper is now two asserts. New tests: a sequence of each kind whose stop was committed by hand (the refusal, an unchanged snapshot, then the typed `--continue` finishing it on `other`), the failing hook from detached and from `other`, and a non-UTF-8 branch run twice. The ignored-file test now asserts the line saying the leg left `other`, on every path. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- templates/assembly-root/scripts/bootstrap.py | 87 +++++++-- tests/test_bootstrap_leg_moves.py | 190 +++++++++++++++++-- 2 files changed, 250 insertions(+), 27 deletions(-) diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index 45ac5386..f92f8047 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -33,8 +33,10 @@ with a merge, a cherry-pick, a revert, a rebase or an `am` in progress is not moved at all: a checkout ends the first three, carrying what they staged onto the branch, and leaves the last two to go on over - it, so the line names the operation and the commands that finish or - abort it (#704). A move git would make over an IGNORED file at a path + it, as it does a cherry-pick or a revert of several commits whose + stop was committed by hand (its `sequencer/todo` is still there), so + the line names the operation and the commands that finish or abort + it (#704). A move git would make over an IGNORED file at a path the pin tracks is refused, in git's words, as one over an untracked file is. A branch no command can be given (a NUL, or a lone surrogate this system cannot encode) is never tried: the leg @@ -59,8 +61,9 @@ wallet-carried authority is not misconfigured, and a bootstrap that failed there would have made the layout load-bearing again. - (d) EXITS 0 unless a validator failed, and 2 where git cannot be run, - refused `git-not-runnable` as the validators refuse it (#694). + (d) EXITS 0 unless a validator failed, and 2 where step (a) or the + upstream notice cannot start git, refused `git-not-runnable` as the + validators refuse it (#694). SCHEMA-NEUTRAL BY CONTRACT. A one-repository project runs the same command: step (a) is a no-op where there are no submodules, and steps (c) and (d) are @@ -119,6 +122,10 @@ ("CHERRY_PICK_HEAD", "a cherry-pick", "cherry-pick"), ("REVERT_HEAD", "a revert", "revert"), ) +#: What a cherry-pick or a revert of SEVERAL commits leaves while it is not +#: done, asked after `IN_PROGRESS`: its stop committed by hand takes the +#: head file away and leaves this, whose first word says which it is. +SEQUENCER_TODO = "sequencer/todo" #: The remedy of the `git-not-runnable` refusal (#694). GIT_NOT_RUNNABLE = ( "Remediation: put git on PATH and run this again. Step (a) reads each " @@ -260,9 +267,21 @@ def _git_says(args: list[str], cwd: Path) -> list[str] | None: def _head_of(sub: Path) -> tuple[str | None, str | None]: """The commit the leg has checked out, and the ref HEAD is on (`None` - when it is detached).""" + when it is detached). + + The ref is read as BYTES and decoded as this system decodes a name it + is given (`os.fsdecode`), so a branch whose name is not UTF-8 reads + back as the `refs/heads/` built from the same name. `git_out`'s + strict decoding raised `UnicodeDecodeError` over it: a second run with + `--branch` such a name ended in a traceback in `place_at_pin`, and the + check of where `checkout -b` left HEAD would end the first (#704).""" + try: + ref = git_out(["symbolic-ref", "--quiet", "HEAD"], cwd=sub, + binary=True) + except Refusal: + ref = None return (_try_git(["rev-parse", "--verify", "--quiet", "HEAD"], sub), - _try_git(["symbolic-ref", "--quiet", "HEAD"], sub)) + None if ref is None else os.fsdecode(ref.rstrip(b"\r\n"))) def _is_ancestor(sub: Path, commit: str | None, of: str | None) -> bool: @@ -401,9 +420,9 @@ def _place_of(current: str | None) -> str: def _say_left(head: str | None, current: str, named: str) -> None: - """The line under `fast-forwarded` (#603), or under `on ` - (#704), when the leg started on ANOTHER branch, `current`, and is on - the tracking branch `named` now. + """The line under `fast-forwarded` (#603), or under `on ` or + `created` (#704), when the leg started on ANOTHER branch, `current`, + and is on the tracking branch `named` now. The person is told, since their next commit lands on `named`. P9 (`place_at_pin`) used to say nothing of the branch it left, where T7 @@ -466,7 +485,30 @@ def _in_progress(sub: Path) -> tuple[str, str, str] | None: for row in IN_PROGRESS: if os.path.lexists(gitdir / row[0]): return row - return None + return _sequence_in_progress(gitdir) + + +def _sequence_in_progress(gitdir: Path) -> tuple[str, str, str] | None: + """A cherry-pick or a revert of several commits that is not done, or + `None` (rv704's S1 on #704). + + `git cherry-pick A B` stops on a conflict in A, the person resolves it + and commits by hand: `CHERRY_PICK_HEAD` is gone, and `git status` still + says `Cherry-pick currently in progress.` and to run `git cherry-pick + --continue`, which picks B onto wherever HEAD is by then. A move onto + the tracking branch put B there. `sequencer/todo` is what is left, and + its first word is `revert` for a revert and `pick` for a cherry-pick; + a file that cannot be read is taken as the cherry-pick, the commoner.""" + todo = gitdir / SEQUENCER_TODO + if not os.path.lexists(todo): + return None + try: + first = todo.read_bytes().split(None, 1)[:1] + except OSError: + first = [] + if first == [b"revert"]: + return SEQUENCER_TODO, "a revert", "revert" + return SEQUENCER_TODO, "a cherry-pick", "cherry-pick" def _say_in_progress(label: str, path: str, @@ -479,10 +521,11 @@ def _say_in_progress(label: str, path: str, HEAD on it already or detached at the pin, ends a merge, a cherry-pick or a revert, taking away the head file `--continue` reads and carrying the staged result onto the branch, and it puts a rebase or an `am` on - the branch, which goes on from there; `merge --ff-only` refuses some of - them and not others. It is the operation's own to finish or abort, so - every such leg is refused alike, before any path of step (a) is - chosen.""" + the branch, which goes on from there, as a cherry-pick or a revert of + several commits does once its stop is committed by hand (its + `sequencer/todo`); `merge --ff-only` refuses some of them and not + others. It is the operation's own to finish or abort, so every such leg + is refused alike, before any path of step (a) is chosen.""" marker, doing, verb = row command = f"git -C {spelled(shlex.quote(path))} {verb}" print(f" {label}: {doing} is in progress in the leg ({marker} is in its " @@ -698,6 +741,13 @@ def create_at_pin(sub: Path, label: str, branch: str, pin: str, detached` of a leg on another branch, too; it says where the leg stays. A HEAD on a ref outside `refs/heads/` is left where it is, as the branch-at-the-pin and the behind-branch paths leave it (#704). + + The checkout is judged by WHERE HEAD IS AFTERWARDS, as `_check_out` + judges one (#533; rv704's S3): a post-checkout hook that fails after + the branch is made makes git exit non-zero, and the line said `could + not create` over a leg on the branch at the pin. It says `created`, + and git's words follow it (`_say_after`). From another branch, the line + under `created` says the leg left it, as P9's and T7's do (rv704's S2). """ named = spelled(branch) head, current = _head_of(sub) @@ -721,10 +771,12 @@ def create_at_pin(sub: Path, label: str, branch: str, pin: str, return why = _git_says(["checkout", "-q", "--no-overwrite-ignore", "-b", branch, pin], sub) - if why is not None: + if _head_of(sub) != (pin, f"refs/heads/{branch}"): stays = "detached" if current is None else f"it {_place_of(current)}" _say_refused(f" {label}: could not create branch {branch!r} at " - f"the pin {_short(pin)}; leaving {stays}", why) + f"the pin {_short(pin)}; leaving {stays}", why or [ + "`git checkout -b` exited 0, and HEAD is not on the " + "branch at the pin"]) return if remote is not None: _try_git(["branch", "-q", f"--set-upstream-to=origin/{branch}", @@ -735,6 +787,9 @@ def create_at_pin(sub: Path, label: str, branch: str, pin: str, f"the pin deliberately does NOT follow") print(f" {label}: branch {named} created at " f"{_short(pin)} — {state}") + if current not in (None, f"refs/heads/{branch}"): + _say_left(head, current, named) + _say_after(why) def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None: diff --git a/tests/test_bootstrap_leg_moves.py b/tests/test_bootstrap_leg_moves.py index 16985ec5..b0a950a9 100644 --- a/tests/test_bootstrap_leg_moves.py +++ b/tests/test_bootstrap_leg_moves.py @@ -29,6 +29,13 @@ missing-branch path and left by T7 (`which is neither`). The issue's recommended answer: every path leaves it, since a commit made there lands on that ref. +- THE REVIEW'S THREE (rv704 on #745, at 4e2b211b). S1: a cherry-pick or a + revert of several commits whose stop was committed by hand leaves only + `sequencer/todo`, and was moved onto `main`, where its `--continue` then + committed; it is refused as in progress. S2: the missing-branch path + says the branch it left, as P9 and T7 do. S3: it judges its `checkout + -b` by where HEAD is, so a failing post-checkout hook is not called a + branch that could not be made. Lines 3 and 4 are elsewhere: P9's line naming the branch it left is in `tests/test_bootstrap_after_bump.py` (its other-branch tests, now reading @@ -41,15 +48,18 @@ from __future__ import annotations import itertools +import os import shlex +import subprocess from pathlib import Path import pytest -from conftest import git +from conftest import IDENTITY_CONFIG, git, run_script from test_bootstrap_after_bump import ( # noqa: F401 - ADDED, EDITED, MAIN, NOTHING_ELSE, bootstrapped, bumped, head_ref, - moved_and_refused, rev, root, skip_validators, + ADDED, EDITED, MAIN, NOTHING_ELSE, bootstrapped, bumped, + failing_post_checkout, head_ref, left_branch, moved_and_refused, rev, + root, skip_validators, ) #: The pin's tracked file the branch `other` deletes, so that checking the @@ -116,13 +126,27 @@ def snapshot(leg: Path) -> tuple: git("status", "--porcelain", cwd=leg).stdout) -def placed_once_cleared(root: Path, pin: str) -> None: +def placed_once_cleared(root: Path, pin: str) -> str: """The way out works: a run after it puts the leg on `main` AT the - pin.""" + pin. Returns what that run printed.""" again = skip_validators(root) assert again.returncode == 0, again.stderr + again.stdout assert head_ref(root / "spec") == MAIN, again.stdout assert rev(root / "spec") == pin, again.stdout + return again.stdout + + +def typed(root: Path, command: str) -> None: + """`command`, a `git -C ...` step (a) printed, run from the root as a + person types it, under the suite's identity, with an editor that keeps + the message git offers.""" + argv = shlex.split(command) + assert argv[0] == "git", argv + proc = subprocess.run(["git", *IDENTITY_CONFIG, *argv[1:]], + cwd=str(root), capture_output=True, text=True, + env={**os.environ, "GIT_EDITOR": "true"}, + check=False) + assert proc.returncode == 0, proc.stderr + proc.stdout # --- line 1: an ignored file in the way is refused on every path ------------ @@ -166,7 +190,8 @@ def test_an_ignored_file_a_move_off_another_branch_writes_is_refused( assert main_of(leg) == {"at": pin, "behind": pin, "missing": None}[path] (leg / IN_THE_WAY).unlink() - placed_once_cleared(root, pin) + again = placed_once_cleared(root, pin) + assert left_branch("other") in again, "it says the leg left `other`" def test_an_ignored_file_the_fast_forward_writes_is_refused( @@ -253,6 +278,9 @@ def reverting(leg: Path, start: str) -> None: "rebase": (None, "rebase-merge", "a rebase", "rebase"), "rebase --apply": (None, "rebase-apply", "a rebase", "rebase"), "am": (None, "rebase-apply/applying", "an `am`", "am"), + "cherry-pick sequence": (None, "sequencer/todo", "a cherry-pick", + "cherry-pick"), + "revert sequence": (None, "sequencer/todo", "a revert", "revert"), } @@ -266,11 +294,9 @@ def in_progress(operation: str) -> str: f"it (`git -C spec {verb} --abort`), then run bootstrap again.\n") -def refused_until_aborted(root: Path, operation: str, pin: str) -> None: +def refuses(root: Path, operation: str) -> None: """Bootstrap refuses the leg as `in_progress` says and nothing moves: - HEAD, `main`, the index and the operation's marker are as they were. - Then the `--abort` the line names, run as printed from the root, and a - run after it places the leg at the pin.""" + HEAD, `main`, the index and the operation's marker are as they were.""" leg = root / "spec" marker = OPERATIONS[operation][1] gitdir = leg / git("rev-parse", "--git-dir", cwd=leg).stdout.strip() @@ -283,6 +309,11 @@ def refused_until_aborted(root: Path, operation: str, pin: str) -> None: assert snapshot(leg) == before, "nothing moved" assert (gitdir / marker).exists(), f"the {operation} goes on" + +def refused_until_aborted(root: Path, operation: str, pin: str) -> None: + """`refuses`, then the `--abort` the line names, run as printed from the + root, and a run after it places the leg at the pin.""" + refuses(root, operation) # The `--abort` of the second line, which is in the output as it is in # `in_progress`: the fourth span of backticks there. abort = in_progress(operation).splitlines()[1].split("`")[3] @@ -359,7 +390,8 @@ def rebased(leg: Path, pin: str, start: str, *backend: str) -> None: git("commit", "-q", "-am", "mine", cwd=leg) assert git("rebase", "-q", *backend, pin, cwd=leg, check=False).returncode != 0 - assert head_ref(leg) is None and rev(leg) == pin + assert head_ref(leg) is None, "the rebase detached HEAD" + assert rev(leg) == pin, "at the pin, onto which it rebases" (leg / EDITED).write_bytes(b"resolved\n") git("add", "--", EDITED, cwd=leg) @@ -402,6 +434,142 @@ def test_a_rebase_or_an_am_stopped_in_the_leg_is_not_moved_onto_main( refused_until_aborted(root, operation, pin) +def picking_two(leg: Path, start: str) -> None: + """`other` from `start` with its own README, then `git cherry-pick + theirs~1 theirs`: the first pick, which changes the README too, stops + on the conflict; the second adds `z.txt`.""" + git("checkout", "-q", "-b", "theirs", start, cwd=leg) + (leg / EDITED).write_bytes(b"theirs\n") + git("commit", "-q", "-am", "theirs edits README", cwd=leg) + (leg / "z.txt").write_bytes(b"z\n") + git("add", "--", "z.txt", cwd=leg) + git("commit", "-q", "-m", "theirs adds z", cwd=leg) + git("checkout", "-q", "-b", "other", start, cwd=leg) + (leg / EDITED).write_bytes(b"other\n") + git("commit", "-q", "-am", "other edits README", cwd=leg) + assert git("cherry-pick", "theirs~1", "theirs", cwd=leg, + check=False).returncode != 0 + + +def reverting_two(leg: Path, start: str) -> None: + """`other` from `start` with three commits (the README to `a`, `z.txt` + added, the README to `c`), then `git revert `: + the first revert stops on the README conflict; the second takes + `z.txt` out.""" + git("checkout", "-q", "-b", "other", start, cwd=leg) + (leg / EDITED).write_bytes(b"a\n") + git("commit", "-q", "-am", "README a", cwd=leg) + (leg / "z.txt").write_bytes(b"z\n") + git("add", "--", "z.txt", cwd=leg) + git("commit", "-q", "-m", "adds z", cwd=leg) + (leg / EDITED).write_bytes(b"c\n") + git("commit", "-q", "-am", "README c", cwd=leg) + assert git("revert", "HEAD~2", "HEAD~1", cwd=leg, + check=False).returncode != 0 + + +#: Each sequence: what starts it, and the subject of `other`'s last commit +#: once its `--continue` is done, with whether that leaves `z.txt` there. +SEQUENCES = { + "cherry-pick sequence": (picking_two, "theirs adds z", True), + "revert sequence": (reverting_two, 'Revert "adds z"', False), +} + + +@pytest.mark.parametrize("operation", sorted(SEQUENCES)) +def test_a_sequence_whose_stop_was_committed_by_hand_is_refused( + root, bootstrapped, tmp_path, operation): + """rv704's S1: a cherry-pick or a revert of TWO commits stops on the + first, which the person resolves and commits by hand. Its head file is + gone, and `git status` still says it is in progress, with its + `--continue`. Bootstrap moved the leg onto `main`, and that + `--continue` then put the second commit there. It is refused by its + `sequencer/todo` now, and the `--continue` it prints, typed from the + root, finishes the sequence on `other`.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, "at") + leg = root / "spec" + make, subject, has_z = SEQUENCES[operation] + make(leg, start) + (leg / EDITED).write_bytes(b"resolved\n") + git("add", "--", EDITED, cwd=leg) + git("commit", "-q", "--no-edit", cwd=leg) + gitdir = leg / git("rev-parse", "--git-dir", cwd=leg).stdout.strip() + assert not (gitdir / "CHERRY_PICK_HEAD").exists(), "the stop is committed" + assert not (gitdir / "REVERT_HEAD").exists(), "the stop is committed" + + refuses(root, operation) + + typed(root, in_progress(operation).splitlines()[1].split("`")[1]) + assert head_ref(leg) == "refs/heads/other", "the sequence ends on `other`" + assert git("log", "-1", "--format=%s", cwd=leg).stdout.strip() == subject + assert (leg / "z.txt").exists() is has_z + assert main_of(leg) == pin, "`main` is not moved" + placed_once_cleared(root, pin) + + +@pytest.mark.parametrize("start_on", ["detached", "other"]) +def test_a_branch_made_under_a_failing_hook_is_said_to_be_made( + root, bootstrapped, tmp_path, start_on): + """rv704's S3: with no `main`, a post-checkout hook that fails after + `checkout -b main ` made the branch and put HEAD on it makes git + exit non-zero. The line said `could not create branch 'main' ...; + leaving ...` and `nothing else is tried` over a leg on `main` at the + pin. It is judged by where HEAD is, as a checkout of a branch that + exists is (#533): `created`, the line saying the leg left `other` + where it was there (S2), and the hook's words after them.""" + pin, start = tracking(root, bootstrapped["remotes"], tmp_path, "missing") + leg = root / "spec" + left = "" + if start_on == "other": + git("checkout", "-q", "-b", "other", start, cwd=leg) + left = left_branch("other") + + result = run_script(root / "scripts" / "bootstrap.py", "--skip-validators", + cwd=root, env=failing_post_checkout(tmp_path)) + assert result.returncode == 0, result.stderr + result.stdout + assert (f" [spec] spec: branch main created at {pin[:12]} — at the " + "pin\n" + left + " checked out, and then git said:\n" + " post-checkout hook failed\n") in result.stdout, ( + result.stdout) + assert "could not create" not in result.stdout + assert NOTHING_ELSE not in result.stdout + assert head_ref(leg) == MAIN + assert rev(leg) == pin + + +@pytest.mark.skipif(os.name == "nt", reason="U+DCFF is three bytes there, " + "and step (a) does not try such a branch at all") +def test_a_branch_whose_name_is_not_utf8_is_read_back_on_every_run( + root, tmp_path): + """`--branch` a name holding U+DCFF, the byte 0xff POSIX spells a name + that is not UTF-8 with, run twice. The first run makes the branch, and + the check of where `checkout -b` left HEAD (S3) reads HEAD back; the + second places the leg on it. Each read `symbolic-ref` decoded as UTF-8, + and on base the second run ended in a `UnicodeDecodeError` traceback. + Made on a disk that takes the byte, skipped on one that does not.""" + try: + (tmp_path / "probe-\udcff").write_bytes(b"") + except (OSError, ValueError): + pytest.skip("this disk takes no file name with the byte 0xff") + leg = root / "spec" + pin = rev(leg) + git("checkout", "-q", "--detach", pin, cwd=leg) + for said in (rf" [spec] spec: branch a\udcffb created at {pin[:12]} " + "— at the pin\n", + rf" [spec] spec: on a\udcffb at {pin[:12]} (branch tip == " + "pin)\n"): + result = run_script(root / "scripts" / "bootstrap.py", + "--skip-validators", "--branch", "a\udcffb", + cwd=root) + assert result.returncode == 0, result.stderr + result.stdout + assert "Traceback" not in result.stderr, result.stderr + assert said in result.stdout, result.stdout + head = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], + cwd=str(leg), capture_output=True, + check=True).stdout + assert head == b"refs/heads/a\xffb\n", head + + # --- line 5: a HEAD on a ref outside refs/heads/ is left by every path ------ @PATHS From 45c512eb797b6e6a8d614e34bb5750f5ccba5221 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 19:43:52 +0000 Subject: [PATCH 08/10] tests/test_doctor_legs_members_next.py: the bootstrap that cures nothing in #414's url-less member state now fails, as 1a69505d makes it, so the test pins exit 1 and `git stopped at members/Ghost` where it pinned the old `ok` (#648, rv648's U1) `test_414_3_there_the_bootstrap_cures_nothing_and_the_git_line_cures_one` builds rv648's U1 state: a member, Ghost, whose `.gitmodules` section gives its path and no url. Git's init stops the holder's update there, before it checks out the other member. The test asserted bootstrap's exit 0, the false `ok` rv648's M1 asked to end. It asserts exit 1 now, and the line naming where git stopped. Its point is unchanged: the bootstrap cures nothing, and the doctor's git line cures the other member. Found by the targeted run of every test file that drives the family bootstrap (1 failed, 3797 passed, 32 skipped); the file now passes alone, 124 passed. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_doctor_legs_members_next.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 8c908562..b6429df0 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -1584,7 +1584,10 @@ def test_414_3_there_the_bootstrap_cures_nothing_and_the_git_line_cures_one( f"{module.quote_arg(tried / 'scripts' / 'bootstrap.py')} " f"--root {module.quote_arg(tried)}", tmp_path / "boot") output = bootstrap.stdout + bootstrap.stderr - assert bootstrap.returncode == 0, output + # It cures nothing, and since #648 it says so and fails: git stopped + # at Ghost, whose section gives no url, before it checked out either. + assert bootstrap.returncode == 1, output + assert "git stopped at members/Ghost" in output, output assert "No url found" in output, output assert not any((tried / "members" / FAMILY_MEMBER).iterdir()), output result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") From 23e11056b320e874a9ecd88fad2ad98fa1ded74f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 00:03:06 +0000 Subject: [PATCH 09/10] bootstrap.py: `.gitmodules` is read as git's own parser reads it, a member git has no url for is its own reason, and a gitlink outside members/ gets a cure true for it, so a later duplicate section, a section under a name git ignores or a url git drops no longer ends `family bootstrap ok`, and a subset clone's printed update no longer dies `No url found` (#648, rv648's verify) rv648's verify of 45c512eb (1 MUST, 1 SHOULD, 3 NIT). M1. `init_stop` read `.gitmodules` with `git config --file --list`, which keeps entries git's submodule parser drops or overrides. In three plain clones git's init died (`No url found`, exit 128) and the run still said ok: a second section for a member's path, with no url, after its own (D1); the member's section under the name `../Nova` (Q1); and its url `-nowhere` (O1). `gitmodules_sections` now returns a `Gitmodules` with git 2.43's own reading beside the one the validator's findings use: - a section whose name is empty or has a `..` component, split on `/` and on a backslash, is ignored whole (`check_submodule_name`); - a `path` or `url` that begins with `-` is dropped, and the value before it stands (`looks_like_command_line_option`); - where two sections give one path, the later decides, and a section that gives a path again lets go of the one it gave, even where a later section gave that one too (`cache_put_path`, `cache_remove_path`). `has_url` asks the section git reads for a path, then the config under that section's name. `init_stop` and the reasons ask it. Where more than one section gives the path git has no url for, the line names `validate-family.py --no-members`, whose `gitmodules-path-twice` says to take the others out: a section written back would be one more (D1). S1. In a subset clone of a member whose section has no url (A1), git exits 0 and the line printed the exit-0 skip's update, which dies `No url found` when typed. "git has no url for that path" is now its own reason, asked before "git exited 0", with the write-it-back cure. The stop's sentence says "a gitlink it has no url for", which is true in D1 too, where a section does give the path a url. NIT 1. A gitlink outside members/ that git stopped at (H2) was sent to `--no-members`, which reads only members/ and says `family ok` there. It now gets `git rm --cached -- ` and a commit: measured, that ends ok and the gate passes in every mode, while a section written for it leaves `gitmodules-path-unnamed` in every mode. NIT 2. The `cures` docstring says what members mode refuses for #363's variant (`member-gitlink-absent`, with the move) as well as for a stray or a hand mount (`member-uninitialized`). Tests in tests/test_family_bootstrap_gate_order.py: U1, Q1 (three names) and O1 in one parametrized test, each written back; D1, typed through `gitmodules-path-twice`; D2, a name with dots and a url `-x` after a url, each still the credential's skip and then a true ok; A1, typed through both rounds; and H2, typed and the gate passing in every mode. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- templates/family-root/scripts/bootstrap.py | 270 +++++++++++++------- tests/test_family_bootstrap_gate_order.py | 280 ++++++++++++++++++--- 2 files changed, 436 insertions(+), 114 deletions(-) diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index 4e089c99..25893717 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -33,19 +33,25 @@ why, and fails the run (c): this holder records no gitlink at the row's path (a gitlink recorded only in another case, #363); no section of `.gitmodules` gives that path (a holder whose HEAD lost the - file, #611); git exited 0 and checked nothing out there; or git - stopped at a gitlink no section gives a url, the row's own or another - (a stray case variant, a member mounted by hand, a section that lost - its url), since git's init dies at the first such gitlink it asks - about, before it checks out any member. Git exits 0 over the second - where a recursive clone ran first, and 128 (`No url found for - submodule path`) in a plain clone; a missing credential exits - non-zero too, so what the holder records is the test, never git's - words. The line says what cures each: `validate-family.py` names the - commit for the first two; for the third it prints the `git submodule - update --init --checkout` that checks the member out whatever the - clone chose; and for the last, `validate-family.py --no-members` - names a gitlink no row names, and where the gitlink is a row's own + file, #611); git has no url for that path, in the section it reads + for it or in its config (a section that lost its url, or a second + one for the path after the member's); git exited 0 and checked + nothing out there; or git stopped at another gitlink it has no url + for (a stray case variant, a member mounted by hand), since git's + init dies at the first such gitlink it asks about, before it checks + out any member. Which section and which url git reads is its own + parser's answer, not `git config`'s (`gitmodules_sections`). Git + exits 0 over the second where a recursive clone ran first, and 128 + (`No url found for submodule path`) in a plain clone; a missing + credential exits non-zero too, so what the holder records is the + test, never git's words. The line says what cures each (`cures`): + `validate-family.py` names the commit for the first two; for the + fourth it prints the `git submodule update --init --checkout` that + checks the member out whatever the clone chose. For the third and + the last, `validate-family.py --no-members` names a path two + sections give, or a gitlink in members/ that no row names; `git rm + --cached` takes out one outside members/, which the validator does + not read; and where the gitlink is a row's own, with one section, the line says to write its section back. (b) RUNS EACH MEMBER'S OWN `make bootstrap`. Each member is a whole project @@ -73,7 +79,9 @@ import shutil import subprocess import sys +from collections import Counter from pathlib import Path +from typing import NamedTuple sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 @@ -92,9 +100,9 @@ #: filled with the gitlink git's init died at (`init_stop`). NO_GITLINK = "no gitlink is recorded at that path" NO_SECTION = f"no section of {GITMODULES} gives that path" +NO_URL = "git has no url for that path" EXITED_0 = "git exited 0 and checked nothing out there" -STOPPED = ("git stopped at {}, a gitlink no section of " + GITMODULES - + " gives a url") +STOPPED = "git stopped at {}, a gitlink it has no url for" #: The two environment variables a leg/member credential may arrive in, in the #: order the `validate` workflow resolves them. An App-minted installation @@ -145,14 +153,14 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: "fine-grained PAT with contents:read on them) or export an " "App-minted SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive " "is bootstrapped below.", file=sys.stderr) - left, stopper = unfetchable(root, rows, proc.returncode == 0) + left, stopper, modules = unfetchable(root, rows, proc.returncode == 0) if left: print(" NOT CHECKED OUT, and not for want of a credential: " + ", ".join(f"{spelled(path)} (" + (STOPPED.format(spelled(stopper)) if why == STOPPED else why) + ")" for _, path, why in left) - + ". " + cures(left, stopper, rows), file=sys.stderr) + + ". " + cures(left, stopper, rows, modules), file=sys.stderr) elif proc.returncode == 0: print(" every member and leg fetched") return [f"{spelled(row.get('project') or '?')} (not checked out)" @@ -160,30 +168,63 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: def cures(left: list[tuple[dict, str, str]], stopper: str | None, - rows: list[dict]) -> str: + rows: list[dict], modules: Gitmodules) -> str: """What (a)'s line says cures the rows `left` names, one sentence for - each why among them (#648). `validate-family.py`, which names the - commit, for a gitlink or a section the holder lacks at the row's path. - For the gitlink git stopped at, `validate-family.py --no-members`, which - names a gitlink no row names and the commit that cures it (members - checked, it refuses `member-uninitialized` for the first row there - instead), or, where a row names it, its section written back. For a row - git exited 0 over, the update that checks it out whatever the clone - chose: `--init` with the path registers a member a subset clone left - out, and `--checkout` overrides an `update = none`.""" + each kind of cause among them (#648): + + - A gitlink or a section the holder lacks at the row's path: + `validate-family.py`, which names it and the commit that cures it, + `member-gitlink-absent` (with the move, for a gitlink recorded only + in another case, #363) or `member-gitmodules-absent`. + - A path git has no url for (`has_url`), a row's own or the gitlink git + stopped at. Where more than one section gives it, as a merge that + kept both sides leaves them, `validate-family.py --no-members`, whose + `gitmodules-path-twice` says which to keep and to take the others + out: a section written back would be one more. Otherwise a row's own + is its section written back. A gitlink in `members/` no row names is + `--no-members` too, whose `member-gitlink-unnamed` names it and the + commit that cures it: members checked, the validator refuses + `member-uninitialized` for the first row where every row's gitlink + is there, as with a stray or a hand mount, and `member-gitlink-absent` + where a row's is only in another case, which the first sentence + names. A gitlink outside `members/`, which no mode of the validator + asks about, is taken out of the index: no row names it, and with no + url no clone can check it out. + - A row git exited 0 over: the update that checks it out whatever the + clone chose. `--init` with the path registers a member a subset + clone left out, and `--checkout` overrides an `update = none`.""" whys = {why for _, _, why in left} said = [] if whys & {NO_GITLINK, NO_SECTION}: said.append(f"`{PYTHON} scripts/validate-family.py` names the cause " "and the commit that cures it.") - if STOPPED in whys and stopper in {row_path(row) for row in rows}: - said.append(f"Write the section of {GITMODULES} for " - f"{spelled(stopper)} back as `family.py add` writes one, " - f"with its path and url, and commit {GITMODULES}.") - elif STOPPED in whys: + unread = [path for _, path, why in left if why == NO_URL] + if STOPPED in whys and stopper not in {path for _, path, _ in left}: + unread.append(stopper) + named = {row_path(row) for row in rows} + twice = [path for path in unread if modules.given[path] > 1] + own = [path for path in unread if path in named and path not in twice] + if twice: said.append(f"`{PYTHON} scripts/validate-family.py --no-members` " - f"names each gitlink in {DEFAULT_MEMBERS_DIR}/ that no " - "row names, and the commit that cures it.") + f"names each path more than one section of {GITMODULES} " + "gives, and how to take the others out.") + if own: + said.append(f"Write {'the' if len(own) == 1 else 'each'} section of " + f"{GITMODULES} for {', '.join(map(spelled, own))} back " + "as `family.py add` writes one, with its path and url, " + f"and commit {GITMODULES}.") + for path in unread: + if path in named or path in twice: + continue + if path.split("/", 1)[0].casefold() == DEFAULT_MEMBERS_DIR: + said.append(f"`{PYTHON} scripts/validate-family.py --no-members` " + f"names each gitlink in {DEFAULT_MEMBERS_DIR}/ that " + "no row names, and the commit that cures it.") + else: + removal = shlex.join(["git", "rm", "--cached", "--", path]) + said.append(f"No row names {spelled(path)} and no clone can check " + f"it out: `{spelled(removal)}` takes its gitlink " + "out, then commit that.") exited = [path for _, path, why in left if why == EXITED_0] if exited: update = shlex.join(["git", "submodule", "update", "--init", @@ -193,111 +234,170 @@ def cures(left: list[tuple[dict, str, str]], stopper: str | None, return " ".join(said) -def gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: - """`{name: {key: value}}` of every section of `.gitmodules`, read as git - reads it: the work tree's file, the one `git submodule update` reads, - which a fresh clone takes from HEAD. An empty mapping where there is no +class Gitmodules(NamedTuple): + """`.gitmodules` as (a) asks it (#648), read by `gitmodules_sections`. + + `given` counts the sections that give each path, every section kept, as + `validate-family.py` reads them for the findings (a)'s line names: + `member-gitmodules-absent` and `gitmodules-path-twice`. `read` is + `{path: name}`, the section git reads for each path, and `urls` is + `{name: url}`, the url git takes from each, as git's own parser has + them.""" + + given: Counter + read: dict[str, str] + urls: dict[str, str] + + +def gitmodules_sections(root: Path) -> Gitmodules: + """`.gitmodules` as `git submodule update` reads it: the work tree's + file, which a fresh clone takes from HEAD. Nothing where there is no such file, or one git cannot read: git has no url for any submodule - then either.""" + then either. + + `read` and `urls` keep git 2.43's own rules (submodule-config.c), which + drop what `git config --list` keeps. Entry by entry, in file order: + + - a section whose name is empty or has a `..` component, split on `/` + and on a backslash, is ignored whole (git warns `ignoring suspicious + submodule name`); + - a `path` or `url` that begins with `-` is dropped, and the value + before it stands (`may be interpreted as a command-line option`); + - where two sections give one path, the later in the file decides, as + a later value in one section replaces an earlier. A section that + gives a path again lets go of the one it gave, even where a later + section gave that one too: git then reads no section there.""" + nothing = Gitmodules(Counter(), {}, {}) if not os.path.lexists(root / GITMODULES): - return {} + return nothing try: listed = git_out(["config", "-z", "--file", GITMODULES, "--list"], cwd=root, binary=True) except Refusal: - return {} + return nothing sections: dict[str, dict[str, str]] = {} + read: dict[str, str] = {} + held: dict[str, str] = {} + urls: dict[str, str] = {} for record in listed.split(b"\0"): key, _, value = record.decode("utf-8", "surrogateescape").partition("\n") section, _, variable = key.rpartition(".") - if section.startswith("submodule."): - sections.setdefault(section[len("submodule."):], - {})[variable] = value - return sections + if not section.startswith("submodule."): + continue + name = section[len("submodule."):] + sections.setdefault(name, {})[variable] = value + if (not name or ".." in name.replace("\\", "/").split("/") + or value.startswith("-")): + continue + if variable == "path": + if name in held: + read.pop(held[name], None) + held[name] = value + read[value] = name + elif variable == "url": + urls[name] = value + return Gitmodules(Counter(keys["path"] for keys in sections.values() + if "path" in keys), read, urls) + + +def submodule_config(root: Path) -> set[str]: + """Every `submodule.` key this repository's git config sets, in every + scope git reads, as git's init asks them: `submodule.active`, and + `submodule..url` as an earlier init writes it. None where git + lists none.""" + try: + config = git_out(["config", "-z", "--get-regexp", r"^submodule\."], + cwd=root, binary=True) + except Refusal: + return set() + return {record.decode("utf-8", "surrogateescape").partition("\n")[0] + for record in config.split(b"\0")} + +def has_url(modules: Gitmodules, config: set[str], path: str) -> bool: + """Whether git's init finds a url for the gitlink at `path`: in the + section git reads for that path (`Gitmodules.read`), or in the git + config under that section's name, as an earlier init writes it. Where + git reads no section there it has none, whatever the config holds.""" + name = modules.read.get(path) + return name is not None and (name in modules.urls + or f"submodule.{name}.url" in config) -def init_stop(root: Path, sections: dict[str, dict[str, str]]) -> str | None: + +def init_stop(root: Path, modules: Gitmodules, config: set[str] + ) -> str | None: """The gitlink `git submodule update --init` dies at, or None: the first - in the index, in index order, that its init step asks about and finds - no url for (`No url found for submodule path`, exit 128), before it + in the index, in index order, that its init step asks about and has no + url for (`No url found for submodule path`, exit 128), before it checks out any member (#648). - ASKED AS GIT 2.43 ASKS IT, of the index, `sections` and this + ASKED AS GIT 2.43 ASKS IT, of the index, `modules` and `config`, this repository's git config. Every gitlink the index records is asked, but where the config sets `submodule.active`, as a recursive clone sets it - (`.`), only those a section gives a path: git counts no other active. - One no section gives stops it; so does one whose section gives no url, - unless the config has one under the section's name, as an earlier init - writes it. So a stray case variant or a member mounted by hand stops a - plain clone's update, and leaves a recursive clone's to the credential. - Read nothing, it answers None.""" + (`.`), only those a section git reads gives a path: git counts no other + active. A gitlink git has no url for (`has_url`) stops it. So a stray + case variant or a member mounted by hand stops a plain clone's update, + and leaves a recursive clone's to the credential. Read nothing, it + answers None.""" try: index = git_out(["ls-files", "-s", "-z"], cwd=root, binary=True) except Refusal: return None - try: - config = git_out(["config", "-z", "--get-regexp", r"^submodule\."], - cwd=root, binary=True) - except Refusal: - config = b"" - keys = {record.decode("utf-8", "surrogateescape").partition("\n")[0] - for record in config.split(b"\0")} - names: dict[str, list[str]] = {} - for name, held in sections.items(): - if "path" in held: - names.setdefault(held["path"], []).append(name) for record in index.split(b"\0"): meta, _, entry = record.partition(b"\t") if meta.split()[:1] != [GITLINK_MODE]: continue path = entry.decode("utf-8", "surrogateescape") - given = names.get(path, []) - if not given and "submodule.active" in keys: + if path not in modules.read and "submodule.active" in config: continue - if not any("url" in sections[name] or f"submodule.{name}.url" in keys - for name in given): + if not has_url(modules, config, path): return path return None def unfetchable(root: Path, rows: list[dict], exited_0: bool - ) -> tuple[list[tuple[dict, str, str]], str | None]: + ) -> tuple[list[tuple[dict, str, str]], str | None, + Gitmodules]: """`(row, path, why)` for each row whose member (a) left NOT CHECKED OUT - where no credential would check it out (#648), in row order, and the - gitlink git's init died at (`init_stop`), or None. + where no credential would check it out (#648), in row order; the + gitlink git's init died at (`init_stop`), or None; and `.gitmodules` + as it was read, for `cures`. Asked of this holder's own records, never of git's words: a row at whose path the holder records no gitlink (`recorded_gitlink`, as `validate-family.py`'s `member-gitlink-absent` reads one), or whose gitlink no section of `.gitmodules` gives a path (as its `member-gitmodules-absent` reads HEAD's file, which is the work tree's - in a clone); where git exited 0, any row at all, since git then lacked - nothing a credential gives; and where git's init died at a gitlink no - section gives a url, any row at all, since git then checked out no - member and asked for no credential. Any other row not checked out is - the missing credential's, and stays the skip it always was. A path two - rows give is said once.""" + in a clone); one git has no url for (`has_url`), where no credential + helps either, whatever git exited; where git exited 0, any row at all, + since git then lacked nothing a credential gives; and where git's init + died at a gitlink it has no url for, any row at all, since git then + checked out no member and asked for no credential. Any other row not + checked out is the missing credential's, and stays the skip it always + was. A path two rows give is said once.""" left: dict[str, dict] = {} for row in rows: if not (member_path(root, row) / ".git").exists(): left.setdefault(row_path(row), row) if not left: - return [], None - sections = gitmodules_sections(root) - given = {held["path"] for held in sections.values() if "path" in held} - stopper = None if exited_0 else init_stop(root, sections) + return [], None, Gitmodules(Counter(), {}, {}) + modules = gitmodules_sections(root) + config = submodule_config(root) + stopper = None if exited_0 else init_stop(root, modules, config) found: list[tuple[dict, str, str]] = [] for path, row in left.items(): if recorded_gitlink(root, path) is None: found.append((row, path, NO_GITLINK)) - elif path not in given: + elif not modules.given[path]: found.append((row, path, NO_SECTION)) + elif not has_url(modules, config, path): + found.append((row, path, NO_URL)) elif exited_0: found.append((row, path, EXITED_0)) elif stopper is not None: found.append((row, path, STOPPED)) - return found, stopper + return found, stopper, modules def members_of(root: Path) -> tuple[str, list[dict]]: diff --git a/tests/test_family_bootstrap_gate_order.py b/tests/test_family_bootstrap_gate_order.py index bbe04750..06598e5a 100644 --- a/tests/test_family_bootstrap_gate_order.py +++ b/tests/test_family_bootstrap_gate_order.py @@ -12,12 +12,15 @@ gitlink recorded only at `Members/

`), git dies `No url found for submodule path` (exit 128), and it still ended ok. It fails now, saying which members no credential would check out and why, and what cures - them. So it does where git's init dies at a gitlink no section gives a - url, a stray case variant, a member mounted by hand or a section that - lost its url (rv648's S1, H1 and U1), and where git exits 0 over a member - a subset clone or `update = none` leaves out, whose line prints the - update that checks it out. A missing credential is still the skip it - was, after a recursive clone too. + them. So it does where git's init dies at a gitlink it has no url for: + a stray case variant, a member mounted by hand, in `members/` or outside + it, a section that lost its url (rv648's S1, H1, H2 and U1), and, as + git's own parser reads `.gitmodules`, a second section for a member's + path after its own, a section under a name git ignores and a url git + drops (D1, Q1 and O1). So it does where git exits 0 over a member a + subset clone or `update = none` leaves out, whose line prints the update + that checks it out, unless git has no url for it (A1). A missing + credential is still the skip it was, after a recursive clone too. - (b) In a plain clone of #363's state followed by `git submodule update --init --recursive`, as CI and `make bootstrap` run it, the update dies at `Members/

` before it checks out any member, so where that row is not @@ -74,13 +77,15 @@ "{cure}") NO_SECTION = "no section of .gitmodules gives that path" NO_GITLINK = "no gitlink is recorded at that path" +NO_URL = "git has no url for that path" EXITED_0 = "git exited 0 and checked nothing out there" -STOPPED = ("git stopped at {path}, a gitlink no section of .gitmodules " - "gives a url") +STOPPED = "git stopped at {path}, a gitlink it has no url for" #: What the line says cures each why: the validator for the first two; for -#: the gitlink git stopped at, the validator's `--no-members`, or, where a -#: row names that gitlink, its section written back; and for a member git +#: a path git has no url for, a row's own or the gitlink git stopped at, +#: the validator's `--no-members` where two sections give it, a row's own +#: section written back, `--no-members` for a gitlink in members/ no row +#: names and `git rm --cached` for one outside it; and for a member git #: exited 0 over, the update that checks it out. VALIDATOR_CURE = (f"`{PYTHON} scripts/validate-family.py` names the cause " "and the commit that cures it.") @@ -90,6 +95,11 @@ SECTION_CURE = ("Write the section of .gitmodules for {path} back as " "`family.py add` writes one, with its path and url, and " "commit .gitmodules.") +TWICE_CURE = (f"`{PYTHON} scripts/validate-family.py --no-members` names " + "each path more than one section of .gitmodules gives, and how " + "to take the others out.") +OUTSIDE_CURE = ("No row names {path} and no clone can check it out: `git rm " + "--cached -- {path}` takes its gitlink out, then commit that.") CHECKOUT_CURE = ("`git submodule update --init --checkout --recursive -- " "{path}` checks it out.") @@ -184,17 +194,49 @@ def hand_mounted(holder: Path, env: dict, family) -> str: UNNAMED_STOPS = {"stray": stray, "hand-mount": hand_mounted} -def without_url(data: bytes, project: str) -> bytes: - """`.gitmodules` bytes with the `url` line of `project`'s section taken - out, the rest as `family.py add` wrote it (rv648's U1). A checkout's - copy may end its lines CRLF.""" +def url_edited(data: bytes, project: str, edit) -> bytes: + """`.gitmodules` bytes with the `url` line of `project`'s section put + through `edit`, the rest as `family.py add` wrote it. A checkout's copy + may end its lines CRLF.""" section = re.compile(rb'(\[submodule "members/' + project.encode() - + rb'"\]\r?\n(?:\t[^\n]*\n)*?)\turl = [^\n]*\n') - edited, count = section.subn(rb"\1", data) + + rb'"\]\r?\n(?:\t[^\n]*\n)*?)(\turl = [^\n]*\n)') + edited, count = section.subn( + lambda found: found.group(1) + edit(found.group(2)), data) assert count == 1, data return edited +def without_url(data: bytes, project: str) -> bytes: + """`.gitmodules` bytes with the `url` line of `project`'s section taken + out (rv648's U1).""" + return url_edited(data, project, lambda line: b"") + + +def renamed(data: bytes, name: str) -> bytes: + """`.gitmodules` bytes with OTHER's section under the name `name`, its + keys as `family.py add` wrote them, escaped as git config escapes a + subsection name.""" + own = f'[submodule "members/{OTHER}"]'.encode() + assert data.count(own) == 1, data + escaped = name.replace("\\", "\\\\").replace('"', '\\"') + return data.replace(own, f'[submodule "{escaped}"]'.encode("utf-8")) + + +#: A second section, `dup`, that gives OTHER's path and no url, as a merge +#: that kept both sides of `.gitmodules` can leave one (rv648's D1 and D2). +DUPLICATE = f'[submodule "dup"]\n\tpath = members/{OTHER}\n'.encode() + + +def edited_clone(holder: Path, env: dict, family, tmp_path: Path, + edit) -> tuple[Path, bytes]: + """HEAD's `.gitmodules` put through `edit` by hand and committed, then a + plain clone of it (`cloned`). Returns the clone and the bytes `family.py + add` wrote, which writing the file back restores.""" + data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout + committed_gitmodules(holder, env, edit(data)) + return cloned(holder, env, family, tmp_path, recurse=False), data + + def failed_on(*projects: str) -> str: return "family bootstrap FAILED: " + ", ".join( f"{project} (not checked out)" for project in projects) @@ -430,37 +472,217 @@ def test_a_gitlink_no_row_names_that_stops_a_plain_clones_update_fails( assert after_first_line(again) == FETCHED -def test_a_section_with_no_url_that_stops_the_update_says_to_write_it_back( - family, holder, offline, tmp_path): - """rv648's U1. OTHER's section lost its url, committed: in a plain - clone git's init dies at OTHER's own gitlink, after MEMBER's and before - it checks out either. The run said ok, and no mode of the validator - names it. It names both rows and says to write that section back; - written back as `family.py add` wrote it and committed, the same run - checks both members out.""" +#: `(edit, what git warns)`: OTHER's section with no url (rv648's U1), or +#: one git's own parser drops, though `git config --list` keeps it: a name +#: that is empty or has a `..` component, split on `/` or a backslash +#: (Q1), and a url that begins with `-` (O1). +NO_URL_FOR_OTHER = { + "no-url": (lambda data: without_url(data, OTHER), ""), + "dotdot-name": (lambda data: renamed(data, f"../{OTHER}"), + f"ignoring suspicious submodule name: ../{OTHER}\n"), + "empty-name": (lambda data: renamed(data, ""), + "ignoring suspicious submodule name: \n"), + "backslash-name": (lambda data: renamed(data, f"{OTHER}\\..\\x"), + "ignoring suspicious submodule name: " + f"{OTHER}\\..\\x\n"), + "option-url": (lambda data: url_edited(data, OTHER, + lambda line: b"\turl = -nowhere\n"), + "which may be interpreted as a command-line option: " + "-nowhere\n"), +} + + +@pytest.mark.parametrize("edit, warned", NO_URL_FOR_OTHER.values(), + ids=NO_URL_FOR_OTHER) +def test_a_member_git_has_no_url_for_stops_the_update_and_is_written_back( + family, holder, offline, tmp_path, edit, warned): + """rv648's U1, Q1 and O1. OTHER's section lost its url, or is one git + reads no url from, committed: in a plain clone git's init dies at + OTHER's own gitlink, after MEMBER's and before it checks out either. + The run said ok in Q1 and O1, reading the section with `git config`, + which keeps what git drops, and no mode of the validator names any of + the three. It names both rows, OTHER for want of a url and MEMBER + stopped there, and says to write OTHER's section back; written back as + `family.py add` wrote it and committed, the same run checks both + members out.""" env = fetching(isolated(offline, tmp_path)) - data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout - committed_gitmodules(holder, env, without_url(data, OTHER)) - clone = cloned(holder, env, family, tmp_path, recurse=False) + clone, data = edited_clone(holder, env, family, tmp_path, edit) result = bootstrap(clone, env, "--skip-members") assert result.returncode == 1, result.stdout + result.stderr - why = STOPPED.format(path=f"members/{OTHER}") assert result.stderr.splitlines()[-2:] == [ - not_fetched((f"members/{MEMBER}", why), (f"members/{OTHER}", why), + not_fetched((f"members/{MEMBER}", + STOPPED.format(path=f"members/{OTHER}")), + (f"members/{OTHER}", NO_URL), cure=SECTION_CURE.format(path=f"members/{OTHER}")), failed_on(MEMBER, OTHER)], result.stderr assert (f"No url found for submodule path 'members/{OTHER}'" in result.stderr) + assert warned in result.stderr + (clone / GITMODULES).write_bytes(data) + raw(clone, env, "add", "--", GITMODULES) + commit_all(clone, env, "OTHER's section written back") + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + + +def test_a_second_section_git_reads_for_a_row_s_path_names_the_two( + family, holder, offline, tmp_path): + """rv648's D1. A second section, `dup`, gives OTHER's path and no url, + after OTHER's own: git reads the later of two sections for one path, so + in a plain clone its init dies at OTHER's gitlink, after MEMBER's and + before it checks out either. The run said ok, since one section gave + the path a url. It names both rows and `--no-members`, whose + `gitmodules-path-twice` names the two sections and says to take the + other out: a section written back would be a third. Taken out as that + says and committed, the same run checks both members out.""" + env = fetching(isolated(offline, tmp_path)) + clone, _ = edited_clone(holder, env, family, tmp_path, + lambda data: data + DUPLICATE) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.splitlines()[-2:] == [ + not_fetched((f"members/{MEMBER}", + STOPPED.format(path=f"members/{OTHER}")), + (f"members/{OTHER}", NO_URL), cure=TWICE_CURE), + failed_on(MEMBER, OTHER)], result.stderr + assert (f"No url found for submodule path 'members/{OTHER}'" + in result.stderr) + fallback = validate(clone, "--no-members") + assert fallback.returncode == 1, fallback.stdout + fallback.stderr + (finding,) = [found for found in findings(fallback) + if found.startswith("FINDING gitmodules-path-twice: ")] + assert finding.startswith( + f"FINDING gitmodules-path-twice: .gitmodules gives the path " + f"members/{OTHER} to 2 submodules, `members/{OTHER}` (url "), finding + assert "`dup` (no url)" in finding, finding + assert "--unset-all submodule..` for every key" in finding + raw(clone, env, "config", "-f", GITMODULES, "--unset-all", + "submodule.dup.path") + raw(clone, env, "add", "--", GITMODULES) + commit_all(clone, env, "The second section taken out as printed") + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + + +#: `.gitmodules` edits git's own parser reads past: rv648's D2, the second +#: section placed BEFORE OTHER's own, which git then reads; a name whose +#: dots are no `..` component; and a url that begins with `-` after OTHER's +#: own, which git drops, keeping the one before it. +READ_PAST = { + "duplicate-first": lambda data: DUPLICATE + data, + "dots-in-a-name": lambda data: renamed(data, f"..{OTHER}.."), + "option-url-after-a-url": lambda data: url_edited( + data, OTHER, lambda line: line + b"\turl = -x\n"), +} + + +@pytest.mark.parametrize("edit", READ_PAST.values(), ids=READ_PAST) +def test_what_git_reads_past_is_still_the_skip_and_a_true_ok( + family, holder, offline, tmp_path, edit): + """No misfire. Where git's parser still reads a url for OTHER, a plain + clone's update with no credential for the members fails on their fetch, + not in its init, so they are the credential's: skipped, and the run + ends ok. With the credential, the same update checks both members out, + exit 0, and the run is a good holder's.""" + env = isolated(offline, tmp_path) + clone, _ = edited_clone(holder, env, family, tmp_path, edit) + + skipped = bootstrap(clone, env, "--skip-members") + + assert skipped.returncode == 0, skipped.stdout + skipped.stderr + assert "FETCH INCOMPLETE" in skipped.stderr + assert "not for want of a credential" not in skipped.stderr + fetched = bootstrap(clone, fetching(env), "--skip-members") + assert fetched.returncode == 0, fetched.stdout + fetched.stderr + assert after_first_line(fetched) == FETCHED + assert fetched.stderr == "" + + +def test_a_subset_clone_of_a_member_git_has_no_url_for_is_written_back( + family, holder, offline, tmp_path): + """rv648's A1. OTHER's section lost its url, committed, and the clone + chose MEMBER alone: git exits 0, and the line printed the exit-0 skip's + update, which dies `No url found` when typed (exit 128) and checks + nothing out. Git has no url for OTHER, in its section or its config, + so the line says to write the section back. Written back and + committed, the run is the exit-0 skip, and the update it prints, typed + as printed, lets the same run end ok.""" + env = fetching(isolated(offline, tmp_path)) + clone, data = edited_clone(holder, env, family, tmp_path, + lambda data: without_url(data, OTHER)) + env = skip_by_subset(clone, env) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.splitlines() == [ + not_fetched((f"members/{OTHER}", NO_URL), + cure=SECTION_CURE.format(path=f"members/{OTHER}")), + failed_on(OTHER)] (clone / GITMODULES).write_bytes(data) raw(clone, env, "add", "--", GITMODULES) commit_all(clone, env, "OTHER's section written back") + skipped = bootstrap(clone, env, "--skip-members") + assert skipped.returncode == 1, skipped.stdout + skipped.stderr + assert skipped.stderr.splitlines() == [ + not_fetched((f"members/{OTHER}", EXITED_0), + cure=CHECKOUT_CURE.format(path=f"members/{OTHER}")), + failed_on(OTHER)] + (line,) = re.findall(r"`(git submodule update [^`]*)`", skipped.stderr) + type_line(clone, env, line) + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + assert again.stderr == "" + + +def test_a_hand_mount_outside_members_is_taken_out_of_the_index( + family, holder, offline, tmp_path): + """rv648's H2. SPARE's gitlink recorded by hand at `vendor/`, + outside members/, with no row and no section, and committed: in a plain + clone git's init dies there before it checks out any member. No mode + of the validator reads a gitlink outside members/, and `--no-members` + says `family ok`, so the line names the `git rm --cached` that takes + the gitlink out. Typed as printed and committed, the same run checks + both members out, and the gate passes in every mode.""" + env = fetching(isolated(offline, tmp_path)) + stopper = f"vendor/{SPARE}" + stage_gitlink(holder, stopper, spare_commits(family)[1]) + commit_all(holder, env, "A gitlink mounted by hand outside members/") + clone = cloned(holder, env, family, tmp_path, recurse=False) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + why = STOPPED.format(path=stopper) + assert result.stderr.splitlines()[-2:] == [ + not_fetched((f"members/{MEMBER}", why), (f"members/{OTHER}", why), + cure=OUTSIDE_CURE.format(path=stopper)), + failed_on(MEMBER, OTHER)], result.stderr + assert f"No url found for submodule path '{stopper}'" in result.stderr + fallback = validate(clone, "--no-members") + assert fallback.returncode == 0, "premise: it reads only members/" + (line,) = re.findall(r"`(git rm [^`]*)`", result.stderr) + type_line(clone, env, line) + commit_all(clone, env, "The gitlink taken out as printed") again = bootstrap(clone, env, "--skip-members") assert again.returncode == 0, again.stdout + again.stderr assert after_first_line(again) == FETCHED + assert_passes_in_every_mode(clone, 2) @pytest.mark.parametrize("set_up", UNNAMED_STOPS.values(), ids=UNNAMED_STOPS) From f74f640ca69de5dcda0bd84dbbb441f601f26cde Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 00:29:07 +0000 Subject: [PATCH 10/10] bootstrap.py: a `.gitmodules` a merge left unmerged in the index is its own reason, since git reads no section from it, so a plain clone mid-merge whose update dies `No url found` no longer ends `family bootstrap ok` (#648, rv648's MM1) rv648's verify of 23e11056 (1 MUST). In a plain clone, a merge that conflicts in `.gitmodules`, resolved in the work tree and not added, leaves the index holding the file only at its three conflict stages. Git then reads no `.gitmodules` at all (`repo_read_gitmodules` asks `is_gitmodules_unmerged` first), so its init dies at the first gitlink, exit 128. `gitmodules_sections` read the work tree's file, found every section with its url, and the run ended ok. `gitmodules_sections` now asks `git ls-files -u -z -- .gitmodules` first. Where the index holds the file at a conflict stage, it returns nothing, with `Gitmodules.unmerged` set. `unfetchable` asks that first: every row not checked out gets "git reads no .gitmodules while a merge leaves it unmerged", and the line says to resolve the merge in .gitmodules and `git add .gitmodules`, then run this again. Added, the same run checks both members out. Test: rv648's MM1 in tests/test_family_bootstrap_gate_order.py, the two sides of the merge each giving OTHER's section another `branch`; the cure typed, and the run ends ok. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- templates/family-root/scripts/bootstrap.py | 44 +++++++++++---- tests/test_family_bootstrap_gate_order.py | 62 +++++++++++++++++++++- 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index 25893717..6dce353f 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -40,11 +40,14 @@ for (a stray case variant, a member mounted by hand), since git's init dies at the first such gitlink it asks about, before it checks out any member. Which section and which url git reads is its own - parser's answer, not `git config`'s (`gitmodules_sections`). Git - exits 0 over the second where a recursive clone ran first, and 128 - (`No url found for submodule path`) in a plain clone; a missing - credential exits non-zero too, so what the holder records is the - test, never git's words. The line says what cures each (`cures`): + parser's answer, not `git config`'s (`gitmodules_sections`), and + while a merge leaves `.gitmodules` unmerged in the index git reads + none at all: every such member is said to be that, and the cure is + the file resolved and added. Git exits 0 over the second where a + recursive clone ran first, and 128 (`No url found for submodule + path`) in a plain clone; a missing credential exits non-zero too, + so what the holder records is the test, never git's words. The line + says what cures each (`cures`): `validate-family.py` names the commit for the first two; for the fourth it prints the `git submodule update --init --checkout` that checks the member out whatever the clone chose. For the third and @@ -98,6 +101,7 @@ #: Why (a) left a row's member NOT CHECKED OUT where no credential would #: check it out (#648), in the order `unfetchable` asks. `STOPPED` is #: filled with the gitlink git's init died at (`init_stop`). +UNMERGED = f"git reads no {GITMODULES} while a merge leaves it unmerged" NO_GITLINK = "no gitlink is recorded at that path" NO_SECTION = f"no section of {GITMODULES} gives that path" NO_URL = "git has no url for that path" @@ -172,6 +176,8 @@ def cures(left: list[tuple[dict, str, str]], stopper: str | None, """What (a)'s line says cures the rows `left` names, one sentence for each kind of cause among them (#648): + - A `.gitmodules` a merge left unmerged in the index, which git reads + nothing from (`Gitmodules.unmerged`): the file resolved and added. - A gitlink or a section the holder lacks at the row's path: `validate-family.py`, which names it and the commit that cures it, `member-gitlink-absent` (with the move, for a gitlink recorded only @@ -195,6 +201,9 @@ def cures(left: list[tuple[dict, str, str]], stopper: str | None, clone left out, and `--checkout` overrides an `update = none`.""" whys = {why for _, _, why in left} said = [] + if UNMERGED in whys: + said.append(f"Resolve the merge in {GITMODULES} and `git add " + f"{GITMODULES}`, then run this again.") if whys & {NO_GITLINK, NO_SECTION}: said.append(f"`{PYTHON} scripts/validate-family.py` names the cause " "and the commit that cures it.") @@ -242,18 +251,23 @@ class Gitmodules(NamedTuple): `member-gitmodules-absent` and `gitmodules-path-twice`. `read` is `{path: name}`, the section git reads for each path, and `urls` is `{name: url}`, the url git takes from each, as git's own parser has - them.""" + them. `unmerged` says a merge left the file unmerged in the index, and + then git reads nothing from it, and the three are empty.""" given: Counter read: dict[str, str] urls: dict[str, str] + unmerged: bool = False def gitmodules_sections(root: Path) -> Gitmodules: """`.gitmodules` as `git submodule update` reads it: the work tree's file, which a fresh clone takes from HEAD. Nothing where there is no such file, or one git cannot read: git has no url for any submodule - then either. + then either. Nothing, and `unmerged`, where the index holds the file + only at a merge's conflict stages (`git ls-files -u`): git reads no + `.gitmodules` at all then, whatever the work tree holds + (`repo_read_gitmodules`), and its init dies at the first gitlink. `read` and `urls` keep git 2.43's own rules (submodule-config.c), which drop what `git config --list` keeps. Entry by entry, in file order: @@ -268,6 +282,14 @@ def gitmodules_sections(root: Path) -> Gitmodules: gives a path again lets go of the one it gave, even where a later section gave that one too: git then reads no section there.""" nothing = Gitmodules(Counter(), {}, {}) + try: + stages = git_out(["ls-files", "-u", "-z", "--", GITMODULES], + cwd=root, binary=True) + except Refusal: + stages = b"" + if any(record.partition(b"\t")[2] == GITMODULES.encode() + for record in stages.split(b"\0")): + return nothing._replace(unmerged=True) if not os.path.lexists(root / GITMODULES): return nothing try: @@ -364,7 +386,9 @@ def unfetchable(root: Path, rows: list[dict], exited_0: bool gitlink git's init died at (`init_stop`), or None; and `.gitmodules` as it was read, for `cures`. - Asked of this holder's own records, never of git's words: a row at + Asked of this holder's own records, never of git's words: any row at + all while a merge leaves `.gitmodules` unmerged in the index, since git + then reads no section and asks for no credential; a row at whose path the holder records no gitlink (`recorded_gitlink`, as `validate-family.py`'s `member-gitlink-absent` reads one), or whose gitlink no section of `.gitmodules` gives a path (as its @@ -387,7 +411,9 @@ def unfetchable(root: Path, rows: list[dict], exited_0: bool stopper = None if exited_0 else init_stop(root, modules, config) found: list[tuple[dict, str, str]] = [] for path, row in left.items(): - if recorded_gitlink(root, path) is None: + if modules.unmerged: + found.append((row, path, UNMERGED)) + elif recorded_gitlink(root, path) is None: found.append((row, path, NO_GITLINK)) elif not modules.given[path]: found.append((row, path, NO_SECTION)) diff --git a/tests/test_family_bootstrap_gate_order.py b/tests/test_family_bootstrap_gate_order.py index 06598e5a..3bd49d54 100644 --- a/tests/test_family_bootstrap_gate_order.py +++ b/tests/test_family_bootstrap_gate_order.py @@ -19,8 +19,10 @@ path after its own, a section under a name git ignores and a url git drops (D1, Q1 and O1). So it does where git exits 0 over a member a subset clone or `update = none` leaves out, whose line prints the update - that checks it out, unless git has no url for it (A1). A missing - credential is still the skip it was, after a recursive clone too. + that checks it out, unless git has no url for it (A1), and where a + merge leaves `.gitmodules` unmerged, which git then reads nothing from + (MM1). A missing credential is still the skip it was, after a recursive + clone too. - (b) In a plain clone of #363's state followed by `git submodule update --init --recursive`, as CI and `make bootstrap` run it, the update dies at `Members/

` before it checks out any member, so where that row is not @@ -78,6 +80,7 @@ NO_SECTION = "no section of .gitmodules gives that path" NO_GITLINK = "no gitlink is recorded at that path" NO_URL = "git has no url for that path" +UNMERGED = "git reads no .gitmodules while a merge leaves it unmerged" EXITED_0 = "git exited 0 and checked nothing out there" STOPPED = "git stopped at {path}, a gitlink it has no url for" @@ -100,6 +103,8 @@ "to take the others out.") OUTSIDE_CURE = ("No row names {path} and no clone can check it out: `git rm " "--cached -- {path}` takes its gitlink out, then commit that.") +UNMERGED_CURE = ("Resolve the merge in .gitmodules and `git add .gitmodules`, " + "then run this again.") CHECKOUT_CURE = ("`git submodule update --init --checkout --recursive -- " "{path}` checks it out.") @@ -685,6 +690,59 @@ def test_a_hand_mount_outside_members_is_taken_out_of_the_index( assert_passes_in_every_mode(clone, 2) +def with_branch(value: str): + """An edit giving OTHER's section a `branch` key of `value`, after its + url.""" + return lambda data: url_edited( + data, OTHER, lambda line: line + f"\tbranch = {value}\n".encode()) + + +def test_a_gitmodules_a_merge_left_unmerged_says_to_add_it( + family, holder, offline, tmp_path): + """rv648's MM1. In a plain clone, a merge conflicts in `.gitmodules` + (each side gave OTHER's section another `branch`), and the file is + resolved in the work tree but not added: the index holds it only at + the conflict's three stages. Git reads no `.gitmodules` at all then, + so its init dies at MEMBER's gitlink, the first, and the run said ok, + reading the work tree's file. It names both rows for that and says to + add the file once it is resolved; added, the same run checks both + members out.""" + env = fetching(isolated(offline, tmp_path)) + remote = family["remotes"] / f"{NAME}.git" + side = f"w648/{next(BRANCHES)}" + data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout + committed_gitmodules(holder, env, with_branch("dev")(data)) + raw(holder, env, "push", "-q", str(remote), f"HEAD:refs/heads/{side}") + raw(holder, env, "update-index", "-q", "--refresh", check=False) + raw(holder, env, "reset", "-q", "--keep", "HEAD~1") + clone, _ = edited_clone(holder, env, family, tmp_path, + with_branch("main")) + raw(clone, env, "fetch", "-q", "origin", side) + merged = raw(clone, env, "merge", "-q", "--no-edit", "FETCH_HEAD", + check=False) + assert merged.returncode == 1, merged.stdout + merged.stderr + stages = raw(clone, env, "ls-files", "-u", "--", GITMODULES).stdout + assert len(stages.splitlines()) == 3, "premise: .gitmodules is unmerged" + (clone / GITMODULES).write_bytes( + raw(clone, env, "show", f":2:{GITMODULES}").stdout) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert result.stderr.splitlines()[-2:] == [ + not_fetched((f"members/{MEMBER}", UNMERGED), + (f"members/{OTHER}", UNMERGED), cure=UNMERGED_CURE), + failed_on(MEMBER, OTHER)], result.stderr + assert (f"No url found for submodule path 'members/{MEMBER}'" + in result.stderr) + raw(clone, env, "add", "--", GITMODULES) + + again = bootstrap(clone, env, "--skip-members") + + assert again.returncode == 0, again.stdout + again.stderr + assert after_first_line(again) == FETCHED + + @pytest.mark.parametrize("set_up", UNNAMED_STOPS.values(), ids=UNNAMED_STOPS) def test_a_recursive_clone_with_no_credential_is_still_the_skip( family, holder, offline, tmp_path, set_up):