From 8ec615f44da16cf5ac79c6f33eb6570b71fce981 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 11:05:25 +0000 Subject: [PATCH 1/3] shape-doctor.py: a gitlink beside a `.gitmodules` the leg's index tracks and this disk lacks is named from the index's copy, which git reads, with its own way out that restores the file first (`git restore .gitmodules`), as #1080 names one beside other sections (#1093) #1092 read the `.gitmodules` on this disk, so where the leg's index tracks one and the disk lacks it (deleted, not staged) the doctor named nothing, though git reads the index's copy and `git submodule status` exits 128 beside one with no section for the gitlink. - `unregistered_gitlinks` reads the index's copy there (`registered_in_the_index`: `:.gitmodules`, up to its first bad line, as `gitmodules_entry` reads it) and names each gitlink it does not register, whether it holds no section or sections for other paths, under its own `unregistered_beside: index_copy`. - `index_copy_clause` says it, a note as #914's and #1080's are: restore the file first, then a section in it, or the staged `git rm --cached`. Each way out typed as printed exits 0, and the index's copy keeps every other section. - An unmerged `.gitmodules` still names nothing (git reads no copy then); the limits sentence now says when. - HEAD's and the index's copy are read by one helper, `registered_in_blob`. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 164 ++++++++++++++++++++++------ tests/test_doctor_plan_overrides.py | 100 +++++++++++++++-- 2 files changed, 221 insertions(+), 43 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index e8954a84..4afa586d 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -255,19 +255,22 @@ no section registers that submodule: the leg's entry names each under `unregistered_submodules`, and the row says how to register it or remove it. So it does for a submodule beside a `.gitmodules` whose sections register -other paths, and for one in a leg whose index tracks no `.gitmodules` -(#1080), where the leg's entry says which under `unregistered_beside`. There -git reads the one on this disk, untracked, or else HEAD's copy, and so does -the doctor: a submodule that copy registers is not named. A recursive clone -of the leg leaves each empty. The doctor names nothing, though `git -submodule status` counts the submodule unregistered (exit 128), in these -states: beside a `.gitmodules` the index tracks and this disk lacks, where -git reads the index's copy, `git add .gitmodules` would stage the file's -deletion, and no section can be written in it until it is restored (#1093); -for a submodule whose path is not UTF-8 or holds a control character -(`UNWRITABLE_IN_A_PLAN`); beside a `.gitmodules` that is unmerged; and -beside a `.gitmodules` git refuses, or one on this disk that is not a -regular file: a link, which git reads through, or a directory. +other paths, for one in a leg whose index tracks no `.gitmodules` (#1080), +and for one beside a `.gitmodules` the index tracks and this disk lacks +(#1093), where the leg's entry says which under `unregistered_beside`. In a +leg whose index tracks none, git reads the one on this disk, untracked, or +else HEAD's copy; beside one this disk lacks, the index's copy; and so does +the doctor: a submodule that copy registers is not named. Beside one this +disk lacks, the way out restores it first (`git restore .gitmodules`), +since `git add .gitmodules` would stage its deletion. A recursive clone of +the leg leaves each empty. The doctor names nothing, though `git submodule +status` counts the submodule unregistered (exit 128), in these states: for +a submodule whose path is not UTF-8 or holds a control character +(`UNWRITABLE_IN_A_PLAN`); beside a `.gitmodules` that is unmerged, while +its copy on this disk is the conflict, registers the submodule or is not +there, since git reads no copy then; and beside a `.gitmodules` git +refuses, or one on this disk that is not a regular file: a link, which git +reads through, or a directory. A SUBMODULE MOVED OR REMOVED SINCE THE ADOPTION (#525). Its row is a person's to change, by hand, in the pull request that bumps the pins; no @@ -659,12 +662,14 @@ def not_in_the_standard(detail: str) -> int: #: The key of a leg's entry naming each submodule it holds beside a #: `.gitmodules` that holds no section, so that nothing registers it (#914), #: or beside one whose sections register other paths, or in a leg whose -#: index tracks none (#1080); present only on a leg that has one. +#: index tracks none (#1080), or beside one its index tracks and this disk +#: lacks (#1093); present only on a leg that has one. UNREGISTERED_SUBMODULES = "unregistered_submodules" #: The key of a leg's entry saying what its `unregistered_submodules` sit -#: beside where that is not a `.gitmodules` with no section (#1080): one -#: of the two values below. Absent beside one with no section, so that the -#: entry #914 gives stays as it was; present only on a leg that has one. +#: beside where that is not a `.gitmodules` on this disk with no section +#: (#1080): one of the three values below. Absent beside one with no +#: section, so that the entry #914 gives stays as it was; present only on a +#: leg that has one. UNREGISTERED_BESIDE = "unregistered_beside" #: A `.gitmodules` with sections, none of which registers the submodule. BESIDE_OTHER_SECTIONS = "other_sections" @@ -672,6 +677,10 @@ def not_in_the_standard(detail: str) -> int: #: untracked on this disk or else HEAD's, does not register the submodule, #: or there is none. BESIDE_NO_GITMODULES = "no_gitmodules" +#: The leg's index tracks a `.gitmodules` this disk lacks, and the index's +#: copy, which git reads then, does not register the submodule, whether it +#: holds no section or sections for other paths (#1093). +BESIDE_INDEX_COPY = "index_copy" #: The key of a leg's entry naming each `answered_paths:` row for that leg #: whose path is a directory of it written without its `/` (#655); present #: only on a leg that has one. @@ -8134,6 +8143,7 @@ def held_clauses(detail: dict) -> str: unregistered_clause(legs), other_sections_clause(legs), no_gitmodules_clause(legs), + index_copy_clause(legs), answers_clause(detail.get(ANSWERED_PATHS) or []), unslashed_clause(legs), slashed_clause(legs), @@ -8212,9 +8222,10 @@ def unregistered_clause(legs: list) -> str: the leg does not know. ONLY THE LEGS WHOSE ENTRY GIVES NO `unregistered_beside` (#1080): a - submodule beside other sections, or in a leg whose index tracks no - `.gitmodules`, has its own clause (`other_sections_clause`, - `no_gitmodules_clause`). + submodule beside other sections, in a leg whose index tracks no + `.gitmodules`, or beside one its index tracks and this disk lacks + (#1093), has its own clause (`other_sections_clause`, + `no_gitmodules_clause`, `index_copy_clause`). """ held = unregistered_beside(legs, None) if not held: @@ -8291,6 +8302,47 @@ def no_gitmodules_clause(legs: list) -> str: "` in its leg") +def index_copy_clause(legs: list) -> str: + """The clause naming each submodule a leg holds beside a `.gitmodules` + its index tracks and this disk lacks, where the index's copy does not + register it (`unregistered_gitlinks`), or "" where there is none + (#1093). + + SAID, as `unregistered_clause` is: git reads the index's copy there, + so `git submodule status` exits 128, and a recursive clone of a commit + of that index leaves the submodule empty (measured, git 2.43). + + THE FILE GOES BACK FIRST. `git add .gitmodules`, typed where the file + is gone, stages its deletion and takes every other submodule's section + out of the index with it, and no section can be written in a file + that is not there. `git restore .gitmodules` puts the index's copy + back on this disk, and then each way out is typed as printed: a + section written in it gives `git submodule status` 0; `git add + .gitmodules`, which stages nothing once the file is restored, then + `git rm --cached` each exit 0, and the gitlink leaves the index while + the index's copy keeps every section it held (measured, git 2.43). + + ONE STATE TAKES MORE. Where the index's copy holds a line git refuses, + git reads it only up to that line, and once restored it is a file on + this disk that git refuses whole: `git submodule status` exits 128 + with `bad config line` (measured), and the doctor names nothing, as + beside any file git refuses. The removal still holds as printed; a + section holds once that line is mended too. + """ + held = unregistered_beside(legs, BESIDE_INDEX_COPY) + if not held: + return "" + return (f"{len(held)} submodule(s) sit beside a `.gitmodules` their " + "leg's index tracks and this disk lacks, whose copy in the " + "index, which git reads, has no section for them, so nothing " + "registers them and a recursive clone of the leg leaves each " + f"empty (#1093): {first_three(held)}; first restore that " + "`.gitmodules` in its leg (`git restore .gitmodules`), then give " + "each a section with its `path` and `url` in it, or, once it is " + "staged (`git add .gitmodules`), remove its gitlink with `git rm " + "--cached ` in its leg") + + def slashed_clause(legs: list) -> str: """The clause naming each `answered_paths:` row that names a file of its leg with the `/` a directory's row ends with, or "" where none does @@ -8771,9 +8823,10 @@ def unregistered_gitlinks(adopt, mount: Path, rel: str, tracked: list, """Each submodule `held` that no section of the leg's `.gitmodules` registers, as its path in the row, and what it sits beside, or `([], None)` (#1080): a `.gitmodules` whose sections register other paths - (`BESIDE_OTHER_SECTIONS`), or no `.gitmodules` in the leg's index - (`BESIDE_NO_GITMODULES`). `tracked` is every path the leg's index - lists, `paths` its files. + (`BESIDE_OTHER_SECTIONS`), no `.gitmodules` in the leg's index + (`BESIDE_NO_GITMODULES`), or one the leg's index tracks and this disk + lacks (`BESIDE_INDEX_COPY`, #1093). `tracked` is every path the leg's + index lists, `paths` its files. WHERE THE INDEX TRACKS NO `.gitmodules`, the copy git reads instead is read (`registered_unindexed`): the untracked file on this disk, or else @@ -8781,11 +8834,14 @@ def unregistered_gitlinks(adopt, mount: Path, rel: str, tracked: list, status` finds its mapping there and exits 0; one it does not, or every one where there is no copy at all, is named beside none. - NOT WHERE THE INDEX TRACKS A `.gitmodules` THIS DISK LACKS (#1093). git - reads the index's copy there, and `git submodule status` exits 128 - beside one with no section for the submodule, but this names nothing: - `git add .gitmodules` would stage the file's deletion, and no section - can be written in a file that is not there until it is restored. + WHERE THE INDEX TRACKS A `.gitmodules` THIS DISK LACKS (#1093), git + reads the index's copy, and so does this (`registered_in_the_index`): + a submodule it registers is not named, and one it does not is named + beside it, whether it holds no section or sections for other paths. + Its clause (`index_copy_clause`) has the file restored first: `git add + .gitmodules` would stage the file's deletion, and no section can be + written in a file that is not there. Not while it is unmerged: git + reads no copy then, and this names nothing. ASKED ONLY WHERE `empty_gitmodules` NAMED NONE: beside a `.gitmodules` that holds no section, #914's clause names each, and the entry #914 @@ -8796,14 +8852,19 @@ def unregistered_gitlinks(adopt, mount: Path, rel: str, tracked: list, with `no submodule mapping found in .gitmodules` (measured, git 2.43). A `.gitmodules` the doctor cannot read as git does (`registered_paths`) names nothing, as `holds_no_section` calls no such file empty: one that - is unmerged, that git refuses, or that is not a regular file here (a - link, which git reads through, or a directory). + is unmerged, while its copy on this disk is the conflict, registers the + submodule or is not there; one that git refuses; or one that is not a + regular file here (a link, which git reads through, or a directory). """ if not held: return [], None if adopt.GITMODULES not in tracked: registered = registered_unindexed(adopt, mount) beside = BESIDE_NO_GITMODULES + elif (adopt.GITMODULES in paths + and not os.path.lexists(mount / adopt.GITMODULES)): + registered = registered_in_the_index(adopt, mount) + beside = BESIDE_INDEX_COPY else: registered = (registered_paths(mount / adopt.GITMODULES) if adopt.GITMODULES in paths else None) @@ -8839,7 +8900,43 @@ def registered_unindexed(adopt, mount: Path) -> set | None: head = f"HEAD:{adopt.GITMODULES}" if git_answer(["rev-parse", "-q", "--verify", head], mount) is None: return set() - listing = gitmodules_listing(mount, ["--blob", head], partial=True) + return registered_in_blob(mount, head) + + +def registered_in_the_index(adopt, mount: Path) -> set | None: + """The paths registered by the index's copy of `.gitmodules`, in a leg + whose index tracks one and whose disk lacks it, or None where git reads + no copy, or refuses that one whole (#1093). + + WHERE GIT READS IT, in the order `gitmodules_entry` reads in (#668): + with no file on this disk, git's `config_from_gitmodules` reads the + index's copy, `:.gitmodules`, before HEAD's, and reads it up to its + first bad line (`registered_in_blob`). Measured, git 2.43: `git + submodule status` exits 0 beside an index copy with a section for the + submodule, whatever HEAD's holds, and 128 beside one that holds no + section, or only sections for other paths, though HEAD's register it. + + NONE WHILE THE FILE IS UNMERGED: the index holds it only at stages 1, + 2 and 3, so `rev-parse --verify` finds no `:.gitmodules` and git reads + no copy at all (`is_gitmodules_unmerged`). `git submodule status` + exits 128 there, but this names nothing, as for an unmerged file whose + copy on this disk holds the conflict: reading no copy as one with no + section would name a submodule whose section the merge's result may + hold. + """ + index = f":{adopt.GITMODULES}" + if git_answer(["rev-parse", "-q", "--verify", index], mount) is None: + return None + return registered_in_blob(mount, index) + + +def registered_in_blob(mount: Path, blob: str) -> set | None: + """The paths registered by the copy of `.gitmodules` at `blob`, HEAD's + or the index's, or None where git refuses it whole (#1080, #1093). + Read as `gitmodules_entry` reads a copy that is not the working tree's: + up to its first bad line, keeping every key before it (`submodule_keys`, + #733).""" + listing = gitmodules_listing(mount, ["--blob", blob], partial=True) keys = (None if listing is None else submodule_keys(listing, working_tree=False)) return None if keys is None else registered_by(keys) @@ -8979,8 +9076,9 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, registers its submodules, could never read `ok`. One that holds no section, in a leg that holds no submodule, asks nothing at all (`empty_gitmodules`, #655, #914). A submodule no section registers is - named, beside sections, or in a leg whose index tracks no `.gitmodules`, - too (`unregistered_gitlinks`, #1080). + named, beside sections, in a leg whose index tracks no `.gitmodules`, or + beside one its index tracks and this disk lacks, too + (`unregistered_gitlinks`, #1080, #1093). AND `answered_paths:` IS READ FOR THE QUESTIONS AND FOR THE PATHS IN THE WRONG LEG ALIKE (#525, #655): `answered_questions` takes from each those diff --git a/tests/test_doctor_plan_overrides.py b/tests/test_doctor_plan_overrides.py index 04343da3..afb37668 100644 --- a/tests/test_doctor_plan_overrides.py +++ b/tests/test_doctor_plan_overrides.py @@ -47,7 +47,8 @@ ) from test_shape_doctor import ( # noqa: F401 adoption_module, needs_filter_repo, project, record_gitlink, - record_submodules, scaffolded_here, stage, stage_gitlink, standard, + record_submodules, scaffolded_here, stage, stage_gitlink, stage_unmerged, + standard, ) sys.path.insert(0, str(REPO / "scripts")) @@ -453,10 +454,22 @@ def test_a_gitmodules_that_registers_its_submodule_names_none(standard, "empty (#1080): code/vendor/lib; give each a section with its `path` " "and `url` in a `.gitmodules` added to its leg, or remove its gitlink " "with `git rm --cached ` in its leg") +#: What the row says of it beside a `.gitmodules` the leg's index tracks +#: and this disk lacks, whose copy in the index git reads (#1093). +INDEX_COPY_CLAUSE = ( + "1 submodule(s) sit beside a `.gitmodules` their leg's index tracks and " + "this disk lacks, whose copy in the index, which git reads, has no " + "section for them, so nothing registers them and a recursive clone of " + "the leg leaves each empty (#1093): code/vendor/lib; first restore that " + "`.gitmodules` in its leg (`git restore .gitmodules`), then give each a " + "section with its `path` and `url` in it, or, once it is staged (`git " + "add .gitmodules`), remove its gitlink with `git rm --cached ` in its leg") #: Each state's clause, by what the leg's entry says the submodule sits #: beside (`unregistered_beside`). BESIDE = {"other_sections": OTHER_SECTIONS_CLAUSE, - "no_gitmodules": NO_GITMODULES_CLAUSE} + "no_gitmodules": NO_GITMODULES_CLAUSE, + "index_copy": INDEX_COPY_CLAUSE} def submodule_status(leg: Path): @@ -472,6 +485,11 @@ def submodule_status(leg: Path): #: does, names nothing, as #914's reading calls no such file empty. A limit #: the module docstring of `shape-doctor.py` names. REFUSED = b'[submodule "lib"]\n\tpath\n' +#: A `.gitmodules` git refuses whole whichever copy it reads, a value git +#: dies on (`git_dies_on`): `git submodule status` exits 128 beside it, and +#: the doctor names nothing, as beside `REFUSED` on this disk (#1093). A +#: bare `path` is not that in the index's copy, which git reads up to it. +DIES = SECTIONED + b"\tupdate = bogus\n" def pinned_at_its_head(root: Path, role: str) -> None: @@ -504,7 +522,9 @@ def gitmodules_beside_gitlink(root: Path, body, kept: str) -> Path: - `head`: committed with the gitlink, then removed with `git rm`, so that the index and this disk hold none and git reads HEAD's copy. The leg's new HEAD is made its pin (`pinned_at_its_head`), so that - the verdict is still the placement row's. + the verdict is still the placement row's; + - `deleted`: in the index, then deleted from this disk with the + deletion not staged, so that git reads the index's copy (#1093). """ leg = root / "code" if kept == "untracked": @@ -519,6 +539,8 @@ def gitmodules_beside_gitlink(root: Path, body, kept: str) -> Path: git("commit", "-q", "-m", "a .gitmodules in HEAD", cwd=leg) pinned_at_its_head(root, "code") git("rm", "-q", "--", GITMODULES, cwd=leg) + if kept == "deleted": + (leg / GITMODULES).unlink() return leg @@ -542,13 +564,24 @@ def gitmodules_beside_gitlink(root: Path, body, kept: str) -> Path: (SECTIONED, False, "head", None), (OTHER_SECTION, False, "head", "no_gitmodules"), (OTHER_SECTION + b"!!!\n" + SECTIONED, False, "head", "no_gitmodules"), + (SECTIONED, False, "deleted", None), + (b"", False, "deleted", "index_copy"), + (OTHER_SECTION, False, "deleted", "index_copy"), + (OTHER_SECTION, True, "deleted", "index_copy"), + (OTHER_SECTION + b"!!!\n" + SECTIONED, False, "deleted", "index_copy"), + (REFUSED, False, "deleted", "index_copy"), + (DIES, False, "deleted", None), ], ids=["other-sections", "a-section-with-no-key", "its-path-given-over", "a-name-git-ignores", "its-path-with-a-slash", "other-sections-and-an-unstaged-edit", "no-gitmodules", "no-gitmodules-and-recorded", "its-path-given-last", "untracked-and-registering", "untracked-with-other-sections", "untracked-and-refused", "in-head-and-registering", - "in-head-with-other-sections", "in-head-read-to-its-first-bad-line"]) + "in-head-with-other-sections", "in-head-read-to-its-first-bad-line", + "deleted-and-registering", "deleted-with-no-section", + "deleted-with-other-sections", "deleted-and-recorded", + "deleted-read-to-its-first-bad-line", "deleted-read-to-a-bare-path", + "deleted-and-refused-whole"]) def test_a_gitlink_no_section_registers_is_said(standard, project, body, recorded, kept, beside): """#1080: #914 named a submodule beside a `.gitmodules` that holds no @@ -572,23 +605,34 @@ def test_a_gitlink_no_section_registers_is_said(standard, project, body, section, a bad line, and then the submodule's, git exits 128 and the submodule is named. + AND FROM THE INDEX'S COPY where the leg's index tracks a `.gitmodules` + this disk lacks (#1093), which git reads there: where it registers the + submodule, nothing is named; where it holds no section, or sections + for other paths, the submodule is named beside it, under its own + `unregistered_beside`. It is read up to its first bad line, as HEAD's + is, so a bare `path` in it ends the reading rather than refusing it, + and a copy git refuses whole (`DIES`) names nothing. + THE REMOVAL TYPED AS PRINTED, in each state: beside sections it stages the edited `.gitmodules` first, as #914's does, and `git rm --cached` alone dies there (rc 128); where the index tracks no `.gitmodules` there is nothing to stage, beside an untracked one or HEAD's too, and - `git rm --cached` alone exits 0.""" + `git rm --cached` alone exits 0. Beside the index's copy it restores + the file first, then stages it, and the index's copy is the one it + was: every other section it held is kept.""" leg = gitmodules_beside_gitlink(project, body, kept) if recorded: record_submodules(project, (GITLINK_PATH, "code")) status = submodule_status(leg) assert status.returncode == ( - 0 if beside is None and body != REFUSED else 128), status.stderr + 0 if beside is None and body not in (REFUSED, DIES) else 128), ( + status.stderr) result, verdict, row = placement(standard, project) assert result.returncode == 0, result.stdout + result.stderr assert verdict == COMPLIANT code = leg_entry(row, "code") questions = [item["path"] for item in row["detail"]["review_required"]] - indexed = body is not None and kept in ("index", "unstaged") + indexed = body is not None and kept in ("index", "unstaged", "deleted") assert questions == (["code/.gitmodules"] if indexed and not recorded else []), row assert row["status"] == ("ok" if recorded else "note"), row @@ -605,16 +649,22 @@ def test_a_gitlink_no_section_registers_is_said(standard, project, body, check=False) assert (alone.returncode, "stage your changes to .gitmodules" in ( alone.stderr)) == (128, True), alone.stderr + copy = git("ls-files", "--stage", "--", GITMODULES, cwd=leg).stdout typed_in(leg, BESIDE[beside]) assert git("ls-files", "--stage", "--", GITLINK_PATH, cwd=leg).stdout == "" + if kept == "deleted": + assert copy, copy + assert (leg / GITMODULES).read_bytes() == body + assert git("ls-files", "--stage", "--", GITMODULES, + cwd=leg).stdout == copy @pytest.mark.parametrize("body, kept", [ (OTHER_SECTION, "index"), (None, "index"), (OTHER_SECTION, "untracked"), - (OTHER_SECTION, "head"), + (OTHER_SECTION, "head"), (OTHER_SECTION, "deleted"), ], ids=["other-sections", "no-gitmodules", "untracked-with-other-sections", - "in-head-with-other-sections"]) + "in-head-with-other-sections", "deleted-with-other-sections"]) def test_a_section_written_for_the_gitlink_registers_it(standard, project, body, kept): """#1080's other way out, typed: a section with the submodule's `path` @@ -622,9 +672,17 @@ def test_a_section_written_for_the_gitlink_registers_it(standard, project, then reads a mapping for it (`git submodule status` exits 0), and the row names nothing. Where the leg's index tracks no `.gitmodules`, the section is written in the untracked one on this disk, or in a new one - beside HEAD's, and the file added (`git add .gitmodules`).""" + beside HEAD's, and the file added (`git add .gitmodules`). Where this + disk lacks the one the index tracks (#1093), the clause's first + command puts the index's copy back, and the section is written in it. + """ leg = gitmodules_beside_gitlink(project, body, kept) assert submodule_status(leg).returncode == 128 + if kept == "deleted": + restore = printed_git(INDEX_COPY_CLAUSE)[0] + assert restore == ["restore", GITMODULES], restore + git(*restore, cwd=leg) + assert (leg / GITMODULES).read_bytes() == body stage_bytes(leg, GITMODULES, (body or b"") + SECTIONED) status = submodule_status(leg) assert status.returncode == 0, status.stderr @@ -635,6 +693,28 @@ def test_a_section_written_for_the_gitlink_registers_it(standard, project, assert "nothing registers them" not in row["reason"], row["reason"] +def test_an_unmerged_gitmodules_this_disk_lacks_names_nothing(standard, + project): + """#1093's limit, kept: while the leg's `.gitmodules` is unmerged, git + reads no copy of it (`git submodule status` exits 128, and `git restore + .gitmodules` refuses the path), and with none on this disk the index + holds no `:.gitmodules` either. The doctor names nothing there, as + beside an unmerged file whose copy on this disk holds the conflict, + rather than read no copy as one with no section.""" + leg = project / "code" + stage_gitlink(leg, GITLINK_PATH) + stage_unmerged(leg, GITMODULES, {1: "file", 2: "file", 3: "file"}) + assert not (leg / GITMODULES).exists() + assert submodule_status(leg).returncode == 128 + restore = git("restore", GITMODULES, cwd=leg, check=False) + assert "is unmerged" in restore.stderr, restore.stderr + row = placement(standard, project)[2] + code = leg_entry(row, "code") + assert "unregistered_submodules" not in code, code + assert "unregistered_beside" not in code, code + assert "nothing registers them" not in row["reason"], row["reason"] + + @pytest.fixture def rules_removed(against_clone) -> Path: """The code leg's only submodule removed as `git rm` does it: its From ab14050045eb8f0a01a2181ad8260c3a49094551 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 14:03:32 +0000 Subject: [PATCH 2/3] shape-doctor.py and its tests: two rows over a HEAD whose `.gitmodules` gives the other answer hold that the index's copy is read first, and the docstrings name the skip-worktree limit and why the staging step stays (#1093) Review round 1 (RV1109: READY, no MUST). - SHOULD-1: `gitmodules_beside_gitlink` takes `deleted-over-head`, which commits the opposite copy with the gitlink, makes it the pin, then stages the case's copy and deletes it from disk. The rows `deleted-over-head-registering` and `deleted-over-head-with-other-sections` kill RV1109's M3b (HEAD's copy read where HEAD has one). - SHOULD-3: `.gitmodules` marked skip-worktree is named, but `git restore .gitmodules` exits 1 there; `index_copy_clause` and the module docstring name it as a limit (family.py's `gitmodules-sparse`). - NIT-2: a recursive clone exits 128 with `bad config line` where the copy holds a line git refuses. - NIT-5: `git add .gitmodules` stays, since `git rm --cached` exits 128 once a section was written for another submodule. Docstrings and tests only: no non-docstring string of shape-doctor.py changed (ast). Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 37 +++++++++++++++++++--------- tests/test_doctor_plan_overrides.py | 38 +++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 4afa586d..01e7f4c0 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -262,15 +262,17 @@ else HEAD's copy; beside one this disk lacks, the index's copy; and so does the doctor: a submodule that copy registers is not named. Beside one this disk lacks, the way out restores it first (`git restore .gitmodules`), -since `git add .gitmodules` would stage its deletion. A recursive clone of -the leg leaves each empty. The doctor names nothing, though `git submodule -status` counts the submodule unregistered (exit 128), in these states: for -a submodule whose path is not UTF-8 or holds a control character -(`UNWRITABLE_IN_A_PLAN`); beside a `.gitmodules` that is unmerged, while -its copy on this disk is the conflict, registers the submodule or is not -there, since git reads no copy then; and beside a `.gitmodules` git -refuses, or one on this disk that is not a regular file: a link, which git -reads through, or a directory. +since `git add .gitmodules` would stage its deletion; where the index marks +that file skip-worktree, as a sparse checkout does (family.py's +`gitmodules-sparse`), the submodule is named but that first step exits 1, a +limit. A recursive clone of the leg leaves each empty. The doctor names +nothing, though `git submodule status` counts the submodule unregistered +(exit 128), in these states: for a submodule whose path is not UTF-8 or +holds a control character (`UNWRITABLE_IN_A_PLAN`); beside a `.gitmodules` +that is unmerged, while its copy on this disk is the conflict, registers +the submodule or is not there, since git reads no copy then; and beside a +`.gitmodules` git refuses, or one on this disk that is not a regular file: +a link, which git reads through, or a directory. A SUBMODULE MOVED OR REMOVED SINCE THE ADOPTION (#525). Its row is a person's to change, by hand, in the pull request that bumps the pins; no @@ -8321,6 +8323,10 @@ def index_copy_clause(legs: list) -> str: .gitmodules`, which stages nothing once the file is restored, then `git rm --cached` each exit 0, and the gitlink leaves the index while the index's copy keeps every section it held (measured, git 2.43). + The `git add .gitmodules` step stays because a person may do both: + once a section is written for one submodule, `git rm --cached` of + another exits 128 with `please stage your changes to .gitmodules` + until the file is staged (measured). ONE STATE TAKES MORE. Where the index's copy holds a line git refuses, git reads it only up to that line, and once restored it is a file on @@ -8328,6 +8334,14 @@ def index_copy_clause(legs: list) -> str: with `bad config line` (measured), and the doctor names nothing, as beside any file git refuses. The removal still holds as printed; a section holds once that line is mended too. + + AND ONE IS A LIMIT: a `.gitmodules` the index marks skip-worktree, as a + sparse checkout leaves it (`S` in `git ls-files -t`; family.py's + `gitmodules-sparse`). git reads the index's copy there too, and the + submodule is named, but `git restore .gitmodules` exits 1 (`pathspec + '.gitmodules' did not match any file(s) known to git`), as `git + checkout -- .gitmodules` does (measured, git 2.43): the clause's first + step is not true until the mark is cleared. """ held = unregistered_beside(legs, BESIDE_INDEX_COPY) if not held: @@ -8849,8 +8863,9 @@ def unregistered_gitlinks(adopt, mount: Path, rel: str, tracked: list, IN EACH STATE A RECURSIVE CLONE OF THE LEG exits 0 and leaves the submodule's directory empty, and `git submodule status` there exits 128 - with `no submodule mapping found in .gitmodules` (measured, git 2.43). - A `.gitmodules` the doctor cannot read as git does (`registered_paths`) + with `no submodule mapping found in .gitmodules` (measured, git 2.43), + or, where the copy holds a line git refuses, both exit 128 with `bad + config line`. A `.gitmodules` the doctor cannot read as git does (`registered_paths`) names nothing, as `holds_no_section` calls no such file empty: one that is unmerged, while its copy on this disk is the conflict, registers the submodule or is not there; one that git refuses; or one that is not a diff --git a/tests/test_doctor_plan_overrides.py b/tests/test_doctor_plan_overrides.py index afb37668..5bdd450a 100644 --- a/tests/test_doctor_plan_overrides.py +++ b/tests/test_doctor_plan_overrides.py @@ -524,9 +524,22 @@ def gitmodules_beside_gitlink(root: Path, body, kept: str) -> Path: The leg's new HEAD is made its pin (`pinned_at_its_head`), so that the verdict is still the placement row's; - `deleted`: in the index, then deleted from this disk with the - deletion not staged, so that git reads the index's copy (#1093). + deletion not staged, so that git reads the index's copy (#1093); + - `deleted-over-head`: as `deleted`, over a HEAD whose copy gives the + other answer (`SECTIONED` or `OTHER_SECTION`), committed with the + gitlink and made its pin, so that only the index's copy, read + before HEAD's as git reads it, gives the answer the case expects. """ leg = root / "code" + if kept == "deleted-over-head": + stage_bytes(leg, GITMODULES, + OTHER_SECTION if body == SECTIONED else SECTIONED) + stage_gitlink(leg, GITLINK_PATH) + git("commit", "-q", "-m", "a .gitmodules in HEAD", cwd=leg) + pinned_at_its_head(root, "code") + stage_bytes(leg, GITMODULES, body) + (leg / GITMODULES).unlink() + return leg if kept == "untracked": (leg / GITMODULES).write_bytes(body) elif body is not None: @@ -571,6 +584,8 @@ def gitmodules_beside_gitlink(root: Path, body, kept: str) -> Path: (OTHER_SECTION + b"!!!\n" + SECTIONED, False, "deleted", "index_copy"), (REFUSED, False, "deleted", "index_copy"), (DIES, False, "deleted", None), + (SECTIONED, False, "deleted-over-head", None), + (OTHER_SECTION, False, "deleted-over-head", "index_copy"), ], ids=["other-sections", "a-section-with-no-key", "its-path-given-over", "a-name-git-ignores", "its-path-with-a-slash", "other-sections-and-an-unstaged-edit", "no-gitmodules", @@ -581,7 +596,8 @@ def gitmodules_beside_gitlink(root: Path, body, kept: str) -> Path: "deleted-and-registering", "deleted-with-no-section", "deleted-with-other-sections", "deleted-and-recorded", "deleted-read-to-its-first-bad-line", "deleted-read-to-a-bare-path", - "deleted-and-refused-whole"]) + "deleted-and-refused-whole", "deleted-over-head-registering", + "deleted-over-head-with-other-sections"]) def test_a_gitlink_no_section_registers_is_said(standard, project, body, recorded, kept, beside): """#1080: #914 named a submodule beside a `.gitmodules` that holds no @@ -606,12 +622,13 @@ def test_a_gitlink_no_section_registers_is_said(standard, project, body, submodule is named. AND FROM THE INDEX'S COPY where the leg's index tracks a `.gitmodules` - this disk lacks (#1093), which git reads there: where it registers the - submodule, nothing is named; where it holds no section, or sections - for other paths, the submodule is named beside it, under its own - `unregistered_beside`. It is read up to its first bad line, as HEAD's - is, so a bare `path` in it ends the reading rather than refusing it, - and a copy git refuses whole (`DIES`) names nothing. + this disk lacks (#1093), which git reads there, before HEAD's (the + `deleted-over-head` rows, where HEAD's gives the other answer): where + it registers the submodule, nothing is named; where it holds no + section, or sections for other paths, the submodule is named beside + it, under its own `unregistered_beside`. It is read up to its first + bad line, as HEAD's is, so a bare `path` in it ends the reading rather + than refusing it, and a copy git refuses whole (`DIES`) names nothing. THE REMOVAL TYPED AS PRINTED, in each state: beside sections it stages the edited `.gitmodules` first, as #914's does, and `git rm --cached` @@ -632,7 +649,8 @@ def test_a_gitlink_no_section_registers_is_said(standard, project, body, assert verdict == COMPLIANT code = leg_entry(row, "code") questions = [item["path"] for item in row["detail"]["review_required"]] - indexed = body is not None and kept in ("index", "unstaged", "deleted") + indexed = body is not None and kept in ("index", "unstaged", "deleted", + "deleted-over-head") assert questions == (["code/.gitmodules"] if indexed and not recorded else []), row assert row["status"] == ("ok" if recorded else "note"), row @@ -653,7 +671,7 @@ def test_a_gitlink_no_section_registers_is_said(standard, project, body, typed_in(leg, BESIDE[beside]) assert git("ls-files", "--stage", "--", GITLINK_PATH, cwd=leg).stdout == "" - if kept == "deleted": + if kept.startswith("deleted"): assert copy, copy assert (leg / GITMODULES).read_bytes() == body assert git("ls-files", "--stage", "--", GITMODULES, From e446ff58bdfc7c407ca658990a8f089c7c340f83 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 21:17:05 +0000 Subject: [PATCH 3/3] tests/test_path_guard_hygiene.py and shape-doctor.py: the doctor's new `os.path.lexists(mount / adopt.GITMODULES)` in `unregistered_gitlinks` is on `BENIGN` as asked after git listed the leg's index, so the path-guard test passes with #1111 merged, and the docstring paragraph on a recursive clone is wrapped to 79 (#1093) Review round 2 (RV1109: SHOULD-2 and NIT-6), on top of the merge of origin/main at 038dafba9 (#1111). - SHOULD-2: with #1111 merged, `test_no_bare_guard_outside_a_stat_helper_or_the_allowlist[shape-doctor.py]` failed on the bare guard at `unregistered_gitlinks`. The entry sits in `BENIGN` with the table's alphabetical order, tagged ASKED: the guard is asked only after `.gitmodules` is among the paths git listed in the leg's index (`leg_tracked_files`), by `lstat`, in a mount `mount_read` read. - NIT-6: the `unregistered_gitlinks` docstring paragraph that began at 88 characters is rewrapped to 75. With docstrings stripped, `ast` shows shape-doctor.py identical to the merge. Co-Authored-By: Claude Sonnet 5.5 --- shape-doctor.py | 11 ++++++----- tests/test_path_guard_hygiene.py | 3 +++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 27527ae9..14cb17af 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -8884,11 +8884,12 @@ def unregistered_gitlinks(adopt, mount: Path, rel: str, tracked: list, submodule's directory empty, and `git submodule status` there exits 128 with `no submodule mapping found in .gitmodules` (measured, git 2.43), or, where the copy holds a line git refuses, both exit 128 with `bad - config line`. A `.gitmodules` the doctor cannot read as git does (`registered_paths`) - names nothing, as `holds_no_section` calls no such file empty: one that - is unmerged, while its copy on this disk is the conflict, registers the - submodule or is not there; one that git refuses; or one that is not a - regular file here (a link, which git reads through, or a directory). + config line`. A `.gitmodules` the doctor cannot read as git does + (`registered_paths`) names nothing, as `holds_no_section` calls no such + file empty: one that is unmerged, while its copy on this disk is the + conflict, registers the submodule or is not there; one that git + refuses; or one that is not a regular file here (a link, which git + reads through, or a directory). """ if not held: return [], None diff --git a/tests/test_path_guard_hygiene.py b/tests/test_path_guard_hygiene.py index 070f5597..ea4e90ea 100644 --- a/tests/test_path_guard_hygiene.py +++ b/tests/test_path_guard_hygiene.py @@ -490,6 +490,9 @@ def _entries(table: dict) -> dict: (SD, "unclonable", "sibling.is_symlink()"): ("DELIBERATE", "the same"), (SD, "unloaded_manifest_row", "used.is_file()"): ( "ASKED", "`validator_for` asked by `is_a` (slice C)"), + (SD, "unregistered_gitlinks", + "os.path.lexists(mount / adopt.GITMODULES)"): ( + "ASKED", "a leg git listed the index of first, by `lstat` (#1093)"), (SD, "validator_row", "used.is_file()"): ( "ASKED", "`validator_for` asked by `is_a` (slice C)"), # --- src/openreposhape/ -------------------------------------------------