From 672845cc564d95134da4207152eeb206986733b3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 14:59:48 +0000 Subject: [PATCH 01/12] validate-family.py: `gitmodules-path-twice` and the check for values git dies on read the `.gitmodules` git reads, the index's where the work tree's is gone, and every section's lines end `git add -- .gitmodules`, so a plain `git commit` after them takes the section out of HEAD (#1077) RV1069's review of #1069 (#1036) found four follow-ups, each measured on main before this change. NIT 1. `gitmodules-path-twice` read the work tree's file alone. A second section for members/IRRS staged in the index's file, the work tree's deleted, was `family ok` in every mode, and a clean clone of the commit after it failed its update and its gate. It reads the sections as git reads them now (`_sections_as_git_reads`), as `gitmodules-path-unnamed` does since #1036, and where the work tree's file is gone it names the line that puts back the file git reads first, recorded so no later finding puts it back over the hand edit. NIT 2. The value check read the work tree's file and HEAD's, never the index's. An `update = !true` staged in the index's file alone, in a named member's section, was `family ok` while git's own `submodule status` exited 128. It reads the file git reads (`_listed_as_git_reads`) and names the index's where that is the one read. Its lines put that file back, not HEAD's, and end "then stage", or "then stage and commit" where HEAD's gives the value too. Where HEAD's alone gives it, the text says to commit the index's file, with no line, so a fix staged there is kept, and `member-gitmodules-absent` then puts the file back itself (`_check_gitlinks_registered` asks `Report.put_back` alone). Git is asked in the order it was asked before, the work tree's file, then HEAD's, then the one read in its place, so `git-not-runnable` names what it named. NIT 3. Where HEAD's file held the section, `gitmodules-path-unnamed`'s lines ended "then commit .gitmodules" and staged nothing, so a plain `git commit` after them left the section in HEAD and a clone's gate failed. Every branch, in members/, outside it and in another case, prints `git add -- .gitmodules` after the keys and says "then stage and commit" there; path-twice ends "then stage and commit" too. NIT 4. A row now covers the case-variant branch where HEAD's file holds no section of the name (RV1069's probe `test_r2_stray_not_in_head`). tests/test_validate_family_index_gitmodules.py holds the new rows: each types only the printed lines, and what the text says in words, then a plain `git commit`. The older rows that pinned the old text, HEAD's put-back and #1036's control are updated to the new behaviour. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Sonnet 5.5 --- .../family-root/scripts/validate-family.py | 283 ++++++++------ tests/test_family_add_refusals.py | 3 +- tests/test_family_gitmodules_values.py | 40 +- tests/test_family_hand_mount_git_rm_lines.py | 43 ++- tests/test_family_holder_bootstrap_texts.py | 15 +- tests/test_family_holder_remedies_compose.py | 18 +- tests/test_family_unnamed_gitlink.py | 23 +- .../test_validate_family_index_gitmodules.py | 360 ++++++++++++++++++ tests/test_validate_family_remedies_typed.py | 8 +- tests/test_validate_family_unnamed_mounts.py | 19 +- 10 files changed, 634 insertions(+), 178 deletions(-) create mode 100644 tests/test_validate_family_index_gitmodules.py diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 2c1bd838..a23c30e5 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -62,15 +62,18 @@ reads `family.yaml` alone, so `--no-members` asks it too. - that no two sections of `.gitmodules` give ONE path (#300): a recursive clone fetches that path from only one of them, so the other is a url - the next edit can make the one. It reads that file alone, so - `--no-members` asks it too. + the next edit can make the one. It reads that file alone, as git + reads it: the work tree's, or where that is gone from disk the index's, + then HEAD's (#1077), so `--no-members` asks it too. - that no section of `.gitmodules` gives a value git dies on as it reads the file, before `git submodule update` checks out any member, the `gitmodules-value-refused` finding (#768): a `path`, `url`, `update`, `ignore` or `branch` with no value, an `update` other than `none`, `checkout`, `rebase` or `merge`, a `shallow` that is no boolean, or a `fetchRecurseSubmodules` that is neither a boolean nor `on-demand`. It - reads the work tree's file and HEAD's, so `--no-members` asks it too, + reads the file git reads, the work tree's, or where that is gone from + disk the index's, then HEAD's (#1077), and HEAD's, which a clone reads, + so `--no-members` asks it too, and where it is found no member that is not checked out is refused as uninitialized: git checked none out. - the REVERSE of every member check (#361): that each gitlink this @@ -1971,12 +1974,20 @@ def _head_listed(root: Path) -> tuple[bool, bytes] | None: "breaks), commit it, then run this again.") from exc -#: Where `gitmodules-value-refused` says a value is, by whether the work -#: tree's file and HEAD's give it (#768). +#: Where `gitmodules-value-refused` says a value is, by whether the file +#: git reads here and HEAD's give it (#768); `{here}` names the file git +#: reads here (`READ_HERE`), the work tree's, or where that is gone from +#: disk the index's (#1077). GIVEN_WHERE = {(True, True): "here and in HEAD, which a clone reads", - (True, False): "in the work tree, not in HEAD", - (False, True): "in HEAD, which a clone reads, not in the " - "work tree"} + (True, False): "in {here}, not in HEAD", + (False, True): "in HEAD, which a clone reads, not in {here}"} + +#: How `GIVEN_WHERE` names the file git reads here, by git's name for the +#: one it reads in place of the work tree's (`_listed_as_git_reads`): none +#: where the work tree's is on disk (#1077). +READ_HERE = {"": "the work tree", + f":{GITMODULES}": "the index's file, which git reads here", + f"HEAD:{GITMODULES}": "HEAD's file, which git reads here"} #: What git takes for a `submodule..` whose value it dies on as #: it reads `.gitmodules` (#768), as `gitmodules-value-refused` says it. @@ -1995,18 +2006,24 @@ def _check_gitmodules_values(root: Path, members: list, report: Report) -> tuple[bool, bool]: """No section of `.gitmodules` gives a value git dies on as it reads the file, the `gitmodules-value-refused` finding (#768). Returns - whether the work tree's file gives one, and whether the work tree's - or HEAD's does. + whether the file git reads here gives one, and whether that file or + HEAD's does. Git dies reading such a file, before `git submodule update` checks out any member, so `make bootstrap` checks none out either. In a plain clone the members modes refused `member-uninitialized` for the first row, naming that same update, and `--no-members` said `family ok`. The rules are `parse_config`'s in git's submodule-config.c - (`_values_refused`). The work tree's file is read, which git reads - here, and HEAD's, which a clone reads, as `member-gitmodules-absent` - reads it: no member needs to be checked out, so every mode asks it. - One finding for each key, naming each value git dies on there. + (`_values_refused`). The file git reads here is read, and HEAD's, + which a clone reads, as `member-gitmodules-absent` reads it: no member + needs to be checked out, so every mode asks it. One finding for each + key, naming each value git dies on there. + + THE FILE GIT READS HERE (#1077, RV1069's NIT 2 on #1069): the work + tree's where it is on disk, else the index's, then HEAD's + (`_listed_as_git_reads`). Read from the work tree's alone, an `update + = !rm` staged in the index's file, the work tree's then deleted, was + no finding while git's own `submodule status` died on it (exit 128). The remedy is by key, one command to a line, on the work tree's file, as `gitmodules-path-unnamed` takes a key out: `--unset-all` takes out @@ -2015,49 +2032,58 @@ def _check_gitmodules_values(root: Path, members: list, own section gets its `path` or `url` written back, as it last gave one, or else as `family.py add` writes it, named after its path, with `https://github.com/` and the row's `repository:` (`_written_back`). - Where HEAD's file alone gives the value, the work tree's is what to - commit, and where the work tree has no `.gitmodules`, the first such - finding puts HEAD's back first, whose value its lines take out, and - not the index's, which git reads there: a fix staged in the index's - file is lost (#768, lane 1's NIT 1 on #889). Every one of these is - recorded as a write (`Report.wrote`): a staged removal's finding after - it then puts no file back over the fix (lane 1's SHOULD 1).""" + Where HEAD's file alone gives the value, the file git reads here is + what to commit: the work tree's, staged, or, where that is gone, the + index's as it stands, with no line, so a fix staged in the index's + file is kept (#768, lane 1's NIT 1 on #889). Where the work tree has + no `.gitmodules`, the first finding that takes a key out puts back + the file git reads, whose value its lines take out (`_file_put_back`, + recorded in `Report.put_back`, so one put-back serves every finding), + and ends staging it: the index's file is what the commit records. + Every one of these is recorded as a write (`Report.wrote`): a staged + removal's finding after it then puts no file back over the fix (lane + 1's SHOULD 1).""" present = os.path.lexists(root / GITMODULES) - here = _submodule_entries(_gitmodules_listed(root)) + # The work tree's file, then HEAD's, then the one git reads in place of + # the work tree's: with no git to run, `git-not-runnable` names the + # first of them it meets, as before #1077. + on_disk = _gitmodules_listed(root) head = _head_listed(root) + listed, spec = (on_disk, "") if present else _listed_as_git_reads(root) + here = _submodule_entries(listed) there = _submodule_entries(head[1]) if head else [] in_work = _values_refused(root, here) in_head = _values_refused(root, there) rows = {f"{DEFAULT_MEMBERS_DIR}/{row['project']}": row.get("repository") for row in members if isinstance(row, dict) and isinstance(row.get("project"), str) and row["project"]} - put_back = present for name, key in dict.fromkeys([*in_work, *in_head]): report.wrote = True - said = _value_said(name, key, in_work, in_head) - if present and (name, key) not in in_work: - report.finding("gitmodules-value-refused", - f"{said} Stage and commit the work tree's " - f"{GITMODULES}, which does not give it.") + said = _value_said(name, key, in_work, in_head, READ_HERE[spec]) + if (name, key) not in in_work: + report.finding("gitmodules-value-refused", f"{said} " + ( + f"Stage and commit the work tree's {GITMODULES}, which does " + "not give it." if present else + f"Commit the index's {GITMODULES}, which git reads here and " + "does not give it.")) continue - back = _written_back(name, key, here if present else there, rows) + back = _written_back(name, key, here, rows) report.finding("gitmodules-value-refused", _value_taken_out( - said, f"submodule.{name}.{key}", back, put_back=put_back, - committed=(name, key) in in_head)) - if not put_back: - report.put_back = f"HEAD:{GITMODULES}" - put_back = True + said, f"submodule.{name}.{key}", back, + _file_put_back(root, report), committed=(name, key) in in_head, + staged=not present)) return bool(in_work), bool(in_work or in_head) def _value_said(name: str, key: str, in_work: dict[tuple[str, str], list[str | None]], - in_head: dict[tuple[str, str], list[str | None]]) -> str: + in_head: dict[tuple[str, str], list[str | None]], + here: str = READ_HERE[""]) -> str: """`gitmodules-value-refused` up to its remedy, for the key `key` of - the section `name` (#768): each value the work tree's file and HEAD's - give it, where, and what git takes. Where HEAD's alone gives one, the - file git reads here may not, so it says git dies on it in a clone, - which reads HEAD's.""" + the section `name` (#768): each value the file git reads here (`here`, + `READ_HERE`, #1077) and HEAD's give it, where, and what git takes. + Where HEAD's alone gives one, the file git reads here may not, so it + says git dies on it in a clone, which reads HEAD's.""" full = f"submodule.{name}.{key}" values = dict.fromkeys([*in_work.get((name, key), []), *in_head.get((name, key), [])]) @@ -2068,33 +2094,39 @@ def _value_said(name: str, key: str, "neither that nor make bootstrap checks one out") return (f"{GITMODULES} gives `{spelled(full)}` " + " and ".join( "no value" if value is None else f"the value `{spelled(value)}`" - for value in values) + f" {GIVEN_WHERE[where]}, and git dies " + for value in values) + f" {GIVEN_WHERE[where].format(here=here)}, " + "and git dies " f"reading a file that does, {dies}. Git takes {GIT_TAKES[key]} for " "that key.") -def _value_taken_out(said: str, full: str, back: str | None, *, - put_back: bool, committed: bool) -> str: +def _value_taken_out(said: str, full: str, back: str | None, + put_back: tuple[str, str] = ("", ""), *, + committed: bool, staged: bool = False) -> str: """`gitmodules-value-refused` whole, `said` (`_value_said`) and its - commands, one to a line, on the work tree's file (#768): HEAD's put - back first where the work tree has none and no finding before put it - back (`put_back` False), the key `full` taken out, and `back` written + commands, one to a line, on the work tree's file (#768): the file git + reads put back first where the work tree has none and no finding + before put it back (`put_back`, the line and whose file it puts back, + `_file_put_back`, #1077), the key `full` taken out, and `back` written in, where it is given, then the commit where HEAD's gives the value - (`committed`).""" + (`committed`), or, where the work tree's was gone and the index's + alone gave it (`staged`), the file staged, which the commit records.""" key = full.rpartition(".")[2] - lines = [] if put_back else [f"git checkout HEAD -- {GITMODULES}"] + line, whose = put_back + lines = [line] if line else [] lines.append(f"git config -f {GITMODULES} --unset-all " f"{shlex.quote(full)}") if back is not None: lines.append(f"git config -f {GITMODULES} {shlex.quote(full)} " f"{back}") return (f"{said} " - + ("" if put_back else f"The work tree holds no {GITMODULES}, " - "so the first line below puts HEAD's back. ") + + (f"The work tree holds no {GITMODULES}, so the first line " + f"below puts back {whose}. " if line else "") + "Take the key out" + (f" and write its {key} back" if back is not None else "") + " by hand, in a POSIX shell" - + (f", then stage and commit {GITMODULES}" if committed else "") + + (f", then stage and commit {GITMODULES}" if committed else + f", then stage {GITMODULES}" if staged else "") + ":" + "".join(f"\n {line}" for line in lines)) @@ -2245,7 +2277,8 @@ def _file_put_back(root: Path, report: Report | None) -> tuple[str, str]: return line, whose -def _check_gitmodules_paths(sections: dict[str, dict[str, str]], +def _check_gitmodules_paths(root: Path, + sections: dict[str, dict[str, str]], report: Report) -> None: """No two sections of `.gitmodules` give ONE path (#300). @@ -2255,11 +2288,20 @@ def _check_gitmodules_paths(sections: dict[str, dict[str, str]], recursive clone fetches the path from only one of them, the last in the file, so the other is a url the next edit to the file can make the one fetched. `add` refuses to write one now; this finds a holder that - already has one. `sections` is `_gitmodules_sections`'s reading. A - section whose path no row names is `_check_gitmodules_named`'s question - (#361). Its by-hand edit is recorded as a write (`Report.wrote`), so a - staged removal's finding after it puts no file back over the edit - (lane 1's SHOULD 1 on #889). + already has one. `sections` is `_sections_as_git_reads`'s reading: the + work tree's file, or where that is gone from disk the index's, then + HEAD's, as git reads it (#1077, RV1069's NIT 1 on #1069). Read from + the work tree's alone, a second section staged in the index's file, + the work tree's then deleted, was no finding, and a clean clone of the + commit after it failed its update and this gate. A section whose path + no row names is `_check_gitmodules_named`'s question (#361). Its + by-hand edit is recorded as a write (`Report.wrote`), so a staged + removal's finding after it puts no file back over the edit (lane 1's + SHOULD 1 on #889). Where the work tree's file is gone, the text names + the line that puts back the file git reads first, which is recorded + (`_file_put_back`), so no finding after it puts the file back over the + hand edit; and it ends "stage and commit", since a plain `git commit` + records the index's file, not the work tree's edit (RV1069's NIT 3). """ paths: dict[str, list[str]] = {} for name, keys in sections.items(): @@ -2272,6 +2314,10 @@ def _check_gitmodules_paths(sections: dict[str, dict[str, str]], f"`{name}` (" + (f"url {sections[name]['url']}" if "url" in sections[name] else "no url") + ")" for name in names) + put_back, whose = _file_put_back(root, report) + first = (f", put {GITMODULES}, gone from the work tree, back from " + f"{whose} first, `{put_back}`, so that every other section " + "stays, and " if put_back else " and ") report.finding( "gitmodules-path-twice", f"{GITMODULES} gives the path {path} to {len(names)} " @@ -2279,10 +2325,10 @@ def _check_gitmodules_paths(sections: dict[str, dict[str, str]], "from only one of them, the last in the file, so the order of " "the sections decides which repository lands there. Keep the one " "whose url is the member's (`family.py add` names it after its " - f"path, `{path}`) and take each other out by hand, `git config " + f"path, `{path}`){first}take each other out by hand, `git config " f"-f {GITMODULES} --unset-all submodule..` for every " "key that section holds, so a submodule later given its name " - f"inherits none of them, then commit {GITMODULES}.") + f"inherits none of them, then stage and commit {GITMODULES}.") report.wrote = True @@ -3209,10 +3255,9 @@ def _check_gitmodules_named(root: Path, sections: dict[str, dict[str, str]], said + _variant_advice(kind, path, *variant, "section")) continue - then, staged = _section_then(root, name) said += _variant_said(*variant) + _section_out( - name, keys, "Take", then, back=_file_put_back(root, report), - staged=staged) + name, keys, "Take", _section_then(root, name), + back=_file_put_back(root, report)) elif (path in gitlinks and path not in removed and given[path] == 1 and _in_members(path) and _hand_cached(root, path, gitlinks[path]) is None): @@ -3248,7 +3293,7 @@ def _section_unnamed_said(name: str, keys: dict[str, str]) -> str: def _section_out(name: str, keys: dict[str, str], lead: str, then: str, line: str = "", back: tuple[str, str] = ("", ""), - staged: bool = False) -> str: + staged: bool = True) -> str: """The end of `gitmodules-path-unnamed`'s text, which takes the section `name` out by key (#300), every key it holds (`keys`), after `lead`, ending `then`, with `line`, a `git rm` printed first, before those @@ -3262,12 +3307,14 @@ def _section_out(name: str, keys: dict[str, str], lead: str, then: str, --cached` takes the gitlink out with no file on disk, so `line` stays first. - `staged`, where HEAD's `.gitmodules` holds no section of `name` - (`_section_then`), prints `git add -- .gitmodules` after the keys' - lines: the commit records the index's file, which still held the - section `git submodule add` staged, so a `git commit` after the lines - recorded it, and `git commit -- .gitmodules` had nothing to commit - (RV1069's SHOULD 1 on #1069).""" + `staged` prints `git add -- .gitmodules` after the keys' lines, in + every branch: the commit records the index's file, not the work + tree's edit. Where HEAD's `.gitmodules` held no section of `name`, a + `git commit` after the lines recorded the section `git submodule add` + staged, and `git commit -- .gitmodules` had nothing to commit (RV1069's + SHOULD 1 on #1069); where HEAD's held one, a plain `git commit` after + the lines left it in HEAD, and a clone's gate failed (RV1069's NIT 3 + on #1069, #1077).""" removal = "".join( f"\n git config -f {GITMODULES} --unset-all " f"{shlex.quote(f'submodule.{name}.{key}')}" for key in keys) @@ -3284,18 +3331,19 @@ def _section_out(name: str, keys: dict[str, str], lead: str, then: str, f"them, {then}:{line}{removal}") -def _section_then(root: Path, name: str) -> tuple[str, bool]: +def _section_then(root: Path, name: str) -> str: """How `gitmodules-path-unnamed`'s text for the section `name` ends, - and whether its lines end staging `.gitmodules` (`_section_out`): the - commit where HEAD's file holds a section of that name, else the file - staged, with nothing of it left to commit, as after a `git submodule - add` not committed (RV1069's SHOULD 1 on #1069). Every branch asks - it, in `members/`, outside it and in another case.""" + its lines ending `git add -- .gitmodules` in every branch + (`_section_out`): staged and committed where HEAD's file holds a + section of that name (RV1069's NIT 3 on #1069, #1077), else staged, + with nothing of it left to commit, as after a `git submodule add` not + committed (RV1069's SHOULD 1). Every branch asks it, in `members/`, + outside it and in another case.""" in_head = _head_gitmodules(root) if in_head is not None and name in in_head[1]: - return f"then commit {GITMODULES}", False + return f"then stage and commit {GITMODULES}" return (f"then stage {GITMODULES}: HEAD's holds no section of that name, " - "so nothing of it is left to commit"), True + "so nothing of it is left to commit") def _gitlink_out_first(root: Path, name: str, keys: dict[str, str], @@ -3343,12 +3391,11 @@ def _gitlink_out_first(root: Path, name: str, keys: dict[str, str], path = keys["path"] if (path not in gitlinks or path in removed or _in_members(path) or path in taken): - then, staged = _section_then(root, name) - return _section_out(name, keys, "Otherwise take", then, back=back, - staged=staged) + return _section_out(name, keys, "Otherwise take", + _section_then(root, name), back=back) taken.add(path) head = _forced(root, path, gitlinks[path]) - then, staged = _section_then(root, name) + then = _section_then(root, name) stage = _stage_first(root, edited) nth = "the second line" if stage else "the first line" return (" Otherwise take its gitlink out first, since a gitlink no " @@ -3360,8 +3407,7 @@ def _gitlink_out_first(root: Path, name: str, keys: dict[str, str], + _forced_said(head, nth, _held_if_forced(root, path, head)) + "." + _section_out(name, keys, "Then, with the lines after it, take", then, (f"\n {STAGE_LINE}" if stage else "") - + f"\n {_cached_removal(path, head)}", back, - staged)) + + f"\n {_cached_removal(path, head)}", back)) def _section_lines(path: str, repository, @@ -3388,8 +3434,7 @@ def _section_lines(path: str, repository, def _check_gitlinks_registered(root: Path, members: list, gitlinks: dict[str, str], - report: Report, - refused: bool = False) -> set[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 @@ -3426,9 +3471,10 @@ def _check_gitlinks_registered(root: Path, members: list, WHERE THE WORK TREE'S FILE IS GONE FROM DISK, the first of these findings puts it back first (`_gitmodules_put_back`), and no other - does, nor any where `.gitmodules` gives a value git dies on - (`refused`), whose `gitmodules-value-refused` puts HEAD's back first - (#768): `git config -f` makes a file of the lines it is given, so + does, nor any where a `gitmodules-value-refused` finding put the file + git reads back first (#768, recorded in `Report.put_back` since + #1077; one that only says to commit the index's file puts none back): + `git config -f` makes a file of the lines it is given, so `member-gitmodules-absent`'s lines, staged and committed, dropped every other section (rv820's review, comment 6237354577 on #839), and a second `member-gitmodules-url-absent`'s put the file back over the url @@ -3446,7 +3492,7 @@ def _check_gitlinks_registered(root: Path, members: list, held, sections = head given = Counter(keys["path"] for keys in sections.values() if "path" in keys) - spec, line, whose = (("", "", "") if refused or report.put_back + spec, line, whose = (("", "", "") if report.put_back else _gitmodules_put_back(root)) back = (line, whose) unregistered: set[str] = set() @@ -3804,13 +3850,33 @@ def _blob_sections(root: Path, spec: str) -> dict[str, dict[str, str]]: """Every `submodule` section of the `.gitmodules` git names `spec`, `{name: {key: value}}`, read as `_blob_paths` reads its paths, and none where git cannot read it (#879, #1036).""" + return _sections_listed(_blob_listed(root, spec)) + + +def _blob_listed(root: Path, spec: str) -> bytes: + """What `git config -z --blob --list` prints of the + `.gitmodules` git names `spec`, or nothing where git cannot read it + (`_blob_sections`, `_listed_as_git_reads`, #1077).""" with _git_runs(f"{spec} could not be read as git reads it"): try: - listed = git_out(["config", "-z", "--blob", spec, "--list"], - cwd=root, binary=True) + return git_out(["config", "-z", "--blob", spec, "--list"], + cwd=root, binary=True) except Refusal: - return {} - return _sections_listed(listed) + return b"" + + +def _listed_as_git_reads(root: Path) -> tuple[bytes, str]: + """`(what git config -z --list prints of .gitmodules as git reads it, + git's name for the file it reads in place of the work tree's)`: the + work tree's where it is on disk (`_gitmodules_listed`) and no name, + else the index's, then HEAD's (`_gitmodules_put_back`, + `_blob_listed`), and nothing where no file is there (#1077). What + `gitmodules-value-refused` reads here, as `_sections_as_git_reads` + reads the sections.""" + if os.path.lexists(root / GITMODULES): + return _gitmodules_listed(root), "" + spec = _gitmodules_put_back(root)[0] + return (_blob_listed(root, spec) if spec else b""), spec def _put_back_lines(root: Path, path: str, @@ -4115,14 +4181,15 @@ def _check_members(root: Path, members: list, no_members: bool, the update that dies, and `_init_stop_said`, which would name a stop git never reaches there, is not asked. - `gitmodules-path-unnamed` READS THE FILE GIT READS (#1036): where the - work tree's `.gitmodules` is gone from disk, the index's, then HEAD's - (`_sections_as_git_reads`), not nothing. Not where either file gives a - value git dies on: that finding's lines put HEAD's file back, take the - key out and stage the file, which drops every section the index's file - alone gave, so a section's `--unset-all` after them met a key the file - did not hold (exit 5), and it reads what it read before. Every other - reader of `sections` is as it was. + `gitmodules-path-twice` AND `gitmodules-path-unnamed` READ THE FILE + GIT READS (#1036, #1077): where the work tree's `.gitmodules` is gone + from disk, the index's, then HEAD's (`_sections_as_git_reads`), not + nothing, and so does the value check (`_listed_as_git_reads`). Where + that file gives a value git dies on, both read nothing, as git reads + nothing; where HEAD's alone gives one, the value's finding puts no + file back (it says to commit the index's), so a section's lines put + back the file git reads themselves. The staged removal reads the work + tree's file (`worktree`), as before. """ _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) @@ -4134,24 +4201,20 @@ def _check_members(root: Path, members: list, no_members: bool, if not isinstance(row, dict): report.finding("member-row", f"a member is not a mapping: {row!r}") - sections = _gitmodules_sections(root) - as_read = sections + worktree = _gitmodules_sections(root) refused_here, refused = _check_gitmodules_values(root, members, report) - if refused_here: - sections = {} - _check_gitmodules_paths(sections, report) + sections = {} if refused_here else _sections_as_git_reads(root, worktree) + _check_gitmodules_paths(root, sections, report) named = _row_paths(members) gitlinks = _member_gitlinks(root) removed = _staged_removals(root, gitlinks) _check_gitlinks_named(root, gitlinks, named, removed, report) _check_gitlinks_elsewhere(root, gitlinks, named, removed, report) - _check_gitmodules_named(root, sections if refused - else _sections_as_git_reads(root, sections), - gitlinks, named, removed, report) + _check_gitmodules_named(root, sections, gitlinks, named, removed, report) unregistered = _check_gitlinks_registered(root, members, gitlinks, - report, refused) + report) unregistered |= _check_removals_staged(root, members, gitlinks, removed, - as_read, report) + worktree, report) if no_members: print(f" -- {len(members)} member(s) SKIPPED (--no-members): " "their gitlinks, digests and project ids are not checked " diff --git a/tests/test_family_add_refusals.py b/tests/test_family_add_refusals.py index bfd94607..185b4f3f 100644 --- a/tests/test_family_add_refusals.py +++ b/tests/test_family_add_refusals.py @@ -1928,7 +1928,8 @@ def test_the_validator_finds_two_sections_for_one_path(family, holder, "path, `members/IRRS`) and take each other out by hand, `git " "config -f .gitmodules --unset-all submodule..` for " "every key that section holds, so a submodule later given its " - "name inherits none of them, then commit .gitmodules."], extra + "name inherits none of them, then stage and commit " + ".gitmodules."], extra def test_a_gitmodules_git_cannot_read_is_a_refusal_not_a_pass(holder): diff --git a/tests/test_family_gitmodules_values.py b/tests/test_family_gitmodules_values.py index c2d1777a..fc89631a 100644 --- a/tests/test_family_gitmodules_values.py +++ b/tests/test_family_gitmodules_values.py @@ -98,7 +98,7 @@ ) from test_family_member_path import MEMBER, OTHER, raw from test_family_tracking_branch import on_main # noqa: F401 -from test_validate_family_typed_states import ROW, removal_staged +from test_validate_family_typed_states import GONE, ROW, removal_staged from test_validate_family_unnamed_mounts import commands, findings #: The issue's table, `update = None` first, then `branch` with no value: @@ -185,17 +185,20 @@ def said(key: str, value: str | None, where: str) -> str: def taken_out(key: str, value: str | None, *, back: str | None = None, - where: str = HERE_AND_HEAD, put_back: bool = False) -> str: + where: str = HERE_AND_HEAD, put_back: str = "") -> str: """`gitmodules-value-refused` WHOLE, with the commands that take the key - out of the work tree's file, after the one that puts HEAD's back where - `put_back`, and write `back` in where it is given.""" - lines = (["git checkout HEAD -- .gitmodules"] if put_back else []) + [ + out of the work tree's file, after the one that puts back the file git + reads in its place where the work tree's is gone as `put_back` + (`GONE`) says, the index's, or HEAD's where the index holds none + (#1077), and write `back` in where it is given.""" + whose, line = GONE[put_back] if put_back else ("", "") + lines = ([line] if put_back else []) + [ f"git config -f .gitmodules --unset-all {full(key)}"] if back is not None: lines.append(f"git config -f .gitmodules {full(key)} {back}") return (said(key, value, where) + ("The work tree holds no .gitmodules, so the first line below " - "puts HEAD's back. " if put_back else "") + f"puts back {whose}. " if put_back else "") + "Take the key out" + (f" and write its {key} back" if back is not None else "") + " by hand, in a POSIX shell" @@ -1094,10 +1097,11 @@ def test_a_value_git_dies_on_in_the_index_s_file_is_named_and_mended( family, holder, offline, tmp_path): """`update = None` committed, and the work tree's file lost, not committed: git reads the index's and dies all the same, and bootstrap - names it. The gate reads HEAD's, and, the work tree holding none, its - first line puts HEAD's back before the key is taken out. Typed as - printed and committed, the same run checks both members out and the - gate passes in every mode.""" + names it. The gate reads the index's too, as git reads it, and HEAD's, + which both give it, and, the work tree holding none, its first line + puts the index's back before the key is taken out, not HEAD's (#1077). + 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)) clone, _ = edited_clone(holder, env, family, tmp_path, with_line("update = None")) @@ -1113,16 +1117,18 @@ def test_a_value_git_dies_on_in_the_index_s_file_is_named_and_mended( failed_on(MEMBER, OTHER)], result.stderr found_in_every_mode_and_typed( clone, env, - taken_out("update", "None", where=HEAD_ONLY, put_back=True)) + taken_out("update", "None", put_back="deleted")) assert_a_good_holder_s_run(clone, env) -def test_two_keys_with_no_gitmodules_in_the_work_tree_put_head_s_back_once( +def test_two_keys_with_no_gitmodules_in_the_work_tree_put_it_back_once( holder, offline, tmp_path): """Two values git dies on committed, and the work tree's file lost: one - finding for each key, and only the first puts HEAD's back, since a - second `git checkout` would put back the key the first took out. Typed - as printed and committed, the holder passes in every mode.""" + finding for each key, and only the first puts the index's back, the + file git reads, which gives both, as HEAD's does (#1077: it put HEAD's + back), since a second `git checkout` would put back the key the first + took out. Typed as printed and committed, the holder passes in every + mode.""" env = isolated(offline, tmp_path) data = raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout # Each line goes in right after the url, so `update` comes first. @@ -1134,8 +1140,8 @@ def test_two_keys_with_no_gitmodules_in_the_work_tree_put_head_s_back_once( assert result.returncode == 1, result.stdout + result.stderr assert findings(result) == [ - taken_out("update", "None", where=HEAD_ONLY, put_back=True), - taken_out("shallow", "maybe", where=HEAD_ONLY)] + taken_out("update", "None", put_back="deleted"), + taken_out("shallow", "maybe")] for finding in findings(result): for command in commands(finding): type_line(holder, env, command) diff --git a/tests/test_family_hand_mount_git_rm_lines.py b/tests/test_family_hand_mount_git_rm_lines.py index c1d90ac1..071eed32 100644 --- a/tests/test_family_hand_mount_git_rm_lines.py +++ b/tests/test_family_hand_mount_git_rm_lines.py @@ -96,8 +96,8 @@ ) from test_validate_family_typed_states import GONE, ROW, gone from test_validate_family_unnamed_mounts import ( - SECTION_REMEDY, SECTION_SAID, SECTION_UNDER_GIT_RM, VARIANT, commands, - removal, with_section, + SECTION_REMEDY, SECTION_SAID, SECTION_UNDER_GIT_RM, STAGE, VARIANT, + commands, removal, with_section, ) HAND = f"members/{SPARE}" @@ -582,7 +582,8 @@ def ghost_out(how: str, *, put_back: bool) -> str: `Members/IRRS`, a case variant of MEMBER's path, url `/nowhere/ghost.git`, with the work tree's `.gitmodules` gone as `how` (`GONE`) says: its keys' lines, after the line that puts the file back - where `put_back`.""" + where `put_back`, then the line that stages the file, since HEAD's + holds the section (#1077, RV1069's NIT 3 on #1069).""" line = GONE[how][1] return (SECTION_SAID.format(path="Members/IRRS", name="ghost", url="url /nowhere/ghost.git") @@ -590,9 +591,9 @@ def ghost_out(how: str, *, put_back: bool) -> str: + " Take the section out by hand, in a POSIX shell, every key it " "holds, so a submodule later given its name inherits none of " "them, " + (put_back_said(how) if put_back else "") - + "then commit .gitmodules:" + + "then stage and commit .gitmodules:" + lines_of(*([line] if put_back else []), - *removal("ghost", "path", "url"))) + *removal("ghost", "path", "url"), STAGE)) def test_1036_a_section_the_index_alone_gives_comes_out_of_the_file_put_back( @@ -631,10 +632,9 @@ def test_1036_a_section_the_index_alone_gives_comes_out_of_the_file_put_back( put_back = GONE["deleted"][1] assert found[1] == ( SECTION_SAID.format(path=path, name=path, url=f"url {url}") - + SECTION_REMEDY.replace("then commit .gitmodules:", + + SECTION_REMEDY.replace("then stage and commit .gitmodules:", put_back_said("deleted") + THEN_STAGE + ":") - + lines_of(put_back, *removal(path, "path", "url"), - "git add -- .gitmodules")) + + lines_of(put_back, *removal(path, "path", "url"), STAGE)) def test_1036_outside_members_the_gitlink_goes_first_then_the_put_back( @@ -719,7 +719,8 @@ def test_1036_one_put_back_serves_two_sections(family, holder, offline, ghost_out("deleted", put_back=True), SECTION_SAID.format(path="members/Old", name="members/Old", url="url /nowhere/old.git") - + SECTION_REMEDY + lines_of(*removal("members/Old", "path", "url"))] + + SECTION_REMEDY + lines_of(*removal("members/Old", "path", "url"), + STAGE)] typed_in_order(holder, env, validate(holder, "--no-members")) staged_and_committed(holder, env, "Typed in order") assert raw(holder, env, "status", "--porcelain").stdout == b"" @@ -765,13 +766,16 @@ def test_1036_control_a_value_git_dies_on_keeps_what_it_read( """#1036's control: `update = None` committed in IRSS's section, a value git dies on, then IRQS's gitlink staged over a directory holding a file, its section staged in the index's `.gitmodules` alone, and the work - tree's file deleted. The value's lines put HEAD's file back and stage - it, which drops that section, so `gitmodules-path-unnamed` reads nothing - from the file git reads there, as before: its `--unset-all` of a key the - file put back does not hold exited 5. Typed in order, what is on disk + tree's file deleted. Git reads the index's file, which gives the value, + so `gitmodules-path-unnamed` reads nothing from it, as git reads + nothing, and as with the file on disk. The value's lines put back the + index's file now, the one git reads, not HEAD's, which dropped IRQS's + section (#1077), and take the key out. Typed in order, what is on disk deleted as the text says, then staged and committed as the value's - finding says, nothing is left, the gate passes in every mode and a - plain clone checks both out.""" + finding says, HEAD holds IRQS's section, as with the file on disk, and + the next run names it in every mode. Its lines typed, then a plain `git + commit`, nothing is left, the gate passes in every mode and a plain + clone checks both out.""" env = cloning(offline, tmp_path, family) value_committed(holder, env, OTHER_ROW) path = staged_by_hand(holding_a_file)(holder, env, family) @@ -788,8 +792,17 @@ def test_1036_control_a_value_git_dies_on_keeps_what_it_read( assert codes_of(found) == ["gitmodules-value-refused", "member-gitlink-unnamed"] + assert commands(found[0])[0] == GONE["deleted"][1] typed_around(holder, env, found, lambda: deleted(holder / path)) staged_and_committed(holder, env, "Typed in order") + assert head_paths(holder, env) == {ROW, OTHER_ROW, path} + assert found_alike(holder, *MODES) == [ + SECTION_SAID.format( + path=path, name=path, + url=f"url https://github.com/InkRouter/{SPARE}.git") + + SECTION_REMEDY + lines_of(*removal(path, "path", "url"), STAGE)] + typed_in_order(holder, env, validate(holder, "--no-members")) + what_is_staged_committed(holder, env) assert raw(holder, env, "status", "--porcelain").stdout == b"" assert head_paths(holder, env) == {ROW, OTHER_ROW} ends_well(holder, env, family, tmp_path) diff --git a/tests/test_family_holder_bootstrap_texts.py b/tests/test_family_holder_bootstrap_texts.py index a0436d68..313c8aef 100644 --- a/tests/test_family_holder_bootstrap_texts.py +++ b/tests/test_family_holder_bootstrap_texts.py @@ -991,14 +991,16 @@ def test_two_findings_that_write_the_file_put_it_back_once( assert_passes_in_every_mode(holder, 2) -def test_a_value_finding_that_put_head_s_back_is_not_put_back_over( +def test_a_value_finding_that_put_the_file_back_is_not_put_back_over( holder, offline, tmp_path): """No second put back after `gitmodules-value-refused`'s: MEMBER's section given `update = None`, OTHER's taken out, committed, and the - work tree's file deleted. The value's finding puts HEAD's back first - (#768) and takes the key out; OTHER's `member-gitmodules-absent` then - writes into that file, and puts nothing back over the key it took - out. Typed as printed and committed, the gate passes in every mode.""" + work tree's file deleted. The value's finding puts the index's back + first, the file git reads, which gives the value as HEAD's does (#768; + HEAD's until #1077), and takes the key out; OTHER's + `member-gitmodules-absent` then writes into that file, and puts nothing + back over the key it took out. Typed as printed and committed, the gate + passes in every mode.""" env = isolated(offline, tmp_path) data = raw(holder, env, "show", "HEAD:.gitmodules").stdout line = f"\tpath = members/{MEMBER}\n".encode() @@ -1012,8 +1014,7 @@ def test_a_value_finding_that_put_head_s_back_is_not_put_back_over( assert result.returncode == 1, result.stdout + result.stderr assert codes(result) == ["gitmodules-value-refused", "member-gitmodules-absent"] - assert commands(findings(result)[0])[0] == ( - "git checkout HEAD -- .gitmodules") + assert commands(findings(result)[0])[0] == "git checkout -- .gitmodules" assert findings(result)[1] == absent(OTHER, pinned(holder, OTHER), held=True) typed_and_committed(holder, env, result) diff --git a/tests/test_family_holder_remedies_compose.py b/tests/test_family_holder_remedies_compose.py index ee052fa6..9ba8c49b 100644 --- a/tests/test_family_holder_remedies_compose.py +++ b/tests/test_family_holder_remedies_compose.py @@ -1334,13 +1334,15 @@ def test_round1_nit_6_the_value_s_put_back_serves_the_removal_after_it( family, holder, offline, tmp_path): """Lane 1's NIT 6 on #889 (its mutant MX1): `update = None` committed in IRSS's section, `git rm --cached members/IRRS`, and the work tree's - `.gitmodules` deleted. The value finding puts back HEAD's file and - records that, so the removal after it prints no line for the file, - which HEAD's gives MEMBER's section in. Put back again from the - index's, which holds the value, it undid the fix, and the commit died - (`invalid value for 'submodule.members/IRSS.update'`, exit 128). - Typed in order and committed, the gate passes in every mode and a - plain clone checks both out.""" + `.gitmodules` deleted. The value finding puts back the index's file, + the one git reads, which gives the value as HEAD's does (#1077: it put + back HEAD's), and records that, so the removal after it prints no line + for the file, which gives MEMBER's section. Put back again from the + index's after the value's lines, which hold the value, it undid the + fix, and the commit died (`invalid value for + 'submodule.members/IRSS.update'`, exit 128). Typed in order and + committed, the gate passes in every mode and a plain clone checks both + out.""" env = cloning(offline, tmp_path, family) value_committed(holder, env, OTHER_ROW) raw(holder, env, "rm", "-q", "--cached", "--", ROW) @@ -1350,7 +1352,7 @@ def test_round1_nit_6_the_value_s_put_back_serves_the_removal_after_it( assert codes_of(found) == ["gitmodules-value-refused", "member-gitlink-removal-staged"] - assert commands(found[0])[0] == "git checkout HEAD -- .gitmodules" + assert commands(found[0])[0] == "git checkout -- .gitmodules" assert commands(found[1]) == [f"git reset -q -- {ROW}"] typed_in_order(holder, env, validate(holder, "--no-members")) staged_and_committed(holder, env, "Typed in order") diff --git a/tests/test_family_unnamed_gitlink.py b/tests/test_family_unnamed_gitlink.py index 8dd6db02..5dea32e5 100644 --- a/tests/test_family_unnamed_gitlink.py +++ b/tests/test_family_unnamed_gitlink.py @@ -69,7 +69,7 @@ from test_family_member_path import MEMBER, OTHER, raw from test_validate_family_remedies_typed import STAGED from test_validate_family_unnamed_mounts import ( - SECTION_REMEDY, SECTION_SAID, codes, commands, findings, removal, + SECTION_REMEDY, SECTION_SAID, STAGE, codes, commands, findings, removal, spare_commits, stage_gitlink, with_section, ) @@ -133,9 +133,11 @@ "hand, in a POSIX shell, every key it holds, so a submodule " "later given its name inherits none of them, {then}:") -#: How the section's text ends: the commit, or, where HEAD's `.gitmodules` -#: holds no section of its name, the file staged and nothing to commit. -THEN_COMMIT = "then commit .gitmodules" +#: How the section's text ends: the file staged and committed, or, where +#: HEAD's `.gitmodules` holds no section of its name, staged and nothing to +#: commit. Its lines end `git add -- .gitmodules` either way (#1077, +#: RV1069's NIT 3 on #1069). +THEN_COMMIT = "then stage and commit .gitmodules" THEN_STAGE = ("then stage .gitmodules: HEAD's holds no section of that name, " "so nothing of it is left to commit") @@ -159,7 +161,8 @@ def section_first(path: str, name: str, url: str, said: str, then: str, """`gitmodules-path-unnamed` whole for the section `name`, giving `path` and `url`, at a gitlink outside `members/`: its `git rm --cached`, carrying `flag`, after `git add -- .gitmodules` where `stage`, of which - it says `said`, then its own lines, and `then`.""" + it says `said`, then its own lines, and `then`, the lines ending `git + add -- .gitmodules` (#1077).""" nth = "the second line" if stage else "the first line" return (SECTION_SAID.format(path=path, name=name, url=f"url {url}") + ROW_BACK + GITLINK_FIRST + (STAGE_SAID if stage else "") @@ -168,7 +171,7 @@ def section_first(path: str, name: str, url: str, said: str, then: str, + f"\n git rm --cached{flag} -- {path}" + "".join(f"\n {line}" for line in removal(name, "path", "url")) - + (STAGE_LINE if then.endswith(THEN_STAGE) else "")) + + STAGE_LINE) def found_in_every_mode(root: Path, code: str) -> str: @@ -501,8 +504,8 @@ def test_two_sections_for_one_gitlink_outside_members_print_one_git_rm( "gitmodules-path-unnamed"], result.stderr first, second = findings(result)[1:] assert commands(first) == [f"git rm --cached -- {TOOLS}", - *removal(TOOLS, "path", "url")] - assert commands(second) == removal("second", "path", "url") + *removal(TOOLS, "path", "url"), STAGE] + assert commands(second) == [*removal("second", "path", "url"), STAGE] env = isolated(offline, tmp_path) for finding in (first, second): typed(holder, env, finding) @@ -541,8 +544,8 @@ def test_a_section_with_no_gitlink_in_the_index_prints_its_lines_alone( assert finding == ( SECTION_SAID.format(path=TOOLS, name=TOOLS, url="url /nowhere/x.git") - + SECTION_REMEDY + "".join(f"\n {line}" for line in removal( - TOOLS, "path", "url"))) + + SECTION_REMEDY + "".join(f"\n {line}" for line in [*removal( + TOOLS, "path", "url"), STAGE])) typed(holder, env, finding) staged_and_committed(holder, env, "Typed as printed") assert_passes_in_every_mode(holder, 2) diff --git a/tests/test_validate_family_index_gitmodules.py b/tests/test_validate_family_index_gitmodules.py new file mode 100644 index 00000000..317d9405 --- /dev/null +++ b/tests/test_validate_family_index_gitmodules.py @@ -0,0 +1,360 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The family holder's pinned `scripts/validate-family.py`: its +`.gitmodules` checks read the file git reads, and a section's lines leave +nothing for a plain `git commit` to get wrong (#1077). + +Git's submodule commands read the work tree's `.gitmodules`, and, where +that is gone from disk, the index's, then HEAD's (git 2.43; #1036 measured +it, and #1077's body measured that the index's is read over HEAD's). +`gitmodules-path-unnamed` reads that file since #1036. RV1069's review of +#1069 found two checks that did not, and two more things, each reproduced +on main 2a119f28 before this change: + +- NIT 1. `gitmodules-path-twice` read the work tree's file alone: a second + section for `members/IRRS`, staged in the index's file alone and the + work tree's deleted, was no finding in any mode, and a clean clone of + the commit after it failed its update and its gate. +- NIT 2. The check for values git dies on read the work tree's file and + HEAD's: an `update = !true` staged in the index's file alone, in a named + member's section, the work tree's deleted, was no finding, while git's + own `submodule status` dies on it. Its finding puts back the file git + reads now, not HEAD's, and where HEAD's alone gives the value it says + to commit the index's, with no line, so a `member-gitmodules-absent` + after it puts the file back itself. +- NIT 3. Where HEAD's file holds the section, its lines ended "then commit + .gitmodules" and staged nothing, so a plain `git commit` after them + left the section in HEAD, and a clone's gate failed. They end with `git + add -- .gitmodules` now, saying "then stage and commit". +- NIT 4. No row covered the case-variant branch's stage line where HEAD's + holds no section of the name. RV1069's probe `test_r2_stray_not_in_head` + is that row here. + +Every row types only the printed `git` lines, in order, and does by hand +only what the text says in words, then commits what is staged with a +plain `git commit`, the way #1036's Verify commits: HEAD's `.gitmodules` +is then right, the gate passes in every mode, and a plain clone checks +both members out. +""" + +from __future__ import annotations + +import re +import shlex +from pathlib import Path + +import pytest + +from test_family import family, validate # noqa: F401 +from test_family_add_refusals import holder, offline # noqa: F401 +from test_family_bootstrap_gate_order import type_line +from test_family_head_lost_gitmodules import ( + absent, committed_gitmodules, pinned, +) +from test_family_holder_bootstrap_texts import ( + put_back_first, section_taken_out, +) +from test_family_holder_remedies_compose import ( + MODES, OTHER_ROW, cloning, codes_of, ends_well, found_alike, head_paths, + typed_in_order, +) +from test_family_member_path import OTHER, raw +from test_family_unnamed_gitlink import at_tools +from test_validate_family_typed_states import GONE, ROW, gone +from test_validate_family_unnamed_mounts import ( + SECTION_SAID, STAGE, VARIANT, commands, removal, with_section, +) + +GITMODULES = ".gitmodules" + + +def staged_only(root: Path, env: dict, data: bytes) -> None: + """`data` staged as `.gitmodules` in the index alone, hashed into + git's objects as BYTES, and the work tree's file deleted: git reads + the index's in its place.""" + blob = raw(root, env, "hash-object", "-w", "--stdin", + stdin=data).stdout.decode("ascii").strip() + raw(root, env, "update-index", "--cacheinfo", + f"100644,{blob},{GITMODULES}") + (root / GITMODULES).unlink(missing_ok=True) + + +def committed_as_staged(root: Path, env: dict) -> None: + """A plain `git commit` of what is staged, and nothing else: no `git + add` the text does not print or say. Where nothing is staged it exits + 1, `nothing to commit`, and HEAD is as it was.""" + raw(root, env, "commit", "-q", "-m", "What is staged", check=False) + + +def head_bytes(root: Path, env: dict) -> bytes: + """HEAD's `.gitmodules`.""" + return raw(root, env, "show", f"HEAD:{GITMODULES}").stdout + + +# --- NIT 1: gitmodules-path-twice -------------------------------------------- + +SECOND = (f'[submodule "second"]\n\tpath = {ROW}\n' + "\turl = https://github.com/InkRouter/Second.git\n").encode() + +#: What `gitmodules-path-twice` says where the work tree's file is gone, +#: before what it says to take out. +TWICE_PUT_BACK = (", put .gitmodules, gone from the work tree, back from the " + "index's first, `git checkout -- .gitmodules`, so that " + "every other section stays, and take each other out by " + "hand") + + +def test_1077_a_second_section_the_index_alone_gives_is_path_twice( + family, holder, offline, tmp_path): + """NIT 1: a second section for `members/IRRS`, `second`, staged in the + index's `.gitmodules` alone, the work tree's deleted. Git reads the + index's file, so a recursive clone of the commit after it fetches + `members/IRRS` from `second`'s url. Main read the work tree's file + alone and said `family ok`. It is `gitmodules-path-twice` now, in + every mode, naming the put-back first. Done as it says, the put-back + line typed, `second`'s keys taken out by hand, then staged and + committed: HEAD gives the two members alone, and the gate and a plain + clone pass.""" + env = cloning(offline, tmp_path, family) + staged_only(holder, env, head_bytes(holder, env) + SECOND) + + found = found_alike(holder, *MODES) + + assert codes_of(found) == ["gitmodules-path-twice"] + (twice,) = found + assert f"gives the path {ROW} to 2 submodules" in twice, twice + assert TWICE_PUT_BACK in twice, twice + assert twice.endswith("then stage and commit .gitmodules."), twice + type_line(holder, env, re.search(r"`(git checkout [^`]*)`", + twice).group(1)) + for key in ("path", "url"): + type_line(holder, env, shlex.join( + ["git", "config", "-f", GITMODULES, "--unset-all", + f"submodule.second.{key}"])) + raw(holder, env, "add", "--", GITMODULES) + committed_as_staged(holder, env) + assert b"second" not in head_bytes(holder, env) + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +# --- NIT 2: a value git dies on, in the index's file ------------------------- + +def valued(data: bytes, path: str, line: bytes) -> bytes: + """`.gitmodules` bytes with `line` after the section giving `path`'s + path line.""" + given = f"\tpath = {path}\n".encode() + assert data.count(given) == 1, data + return data.replace(given, given + line) + + +#: What `gitmodules-value-refused` says of `update = !true` in MEMBER's +#: section, by where it is given, up to its remedy. +VALUE_SAID = ( + "FINDING gitmodules-value-refused: .gitmodules gives " + f"`submodule.{ROW}.update` the value `!true` {{where}}, and git dies " + "reading a file that does, {dies}. Git takes one of `none`, " + "`checkout`, `rebase` or `merge` for that key. ") +DIES_HERE = ("before `git submodule update` checks out any member, so " + "neither that nor make bootstrap checks one out") +DIES_IN_A_CLONE = ("so in a clone neither `git submodule update` nor make " + "bootstrap checks out any member") + + +def test_1077_a_value_the_index_alone_gives_is_named_and_taken_out( + family, holder, offline, tmp_path): + """NIT 2: `update = !true` in MEMBER's own section, a named member's, + staged in the index's `.gitmodules` alone, the work tree's deleted. + Git reads the index's file and dies on it (`git submodule status`, + exit 128). Main read the work tree's file and HEAD's and said `family + ok`. It is `gitmodules-value-refused` now, in every mode, naming the + index's file, and its lines put that file back, not HEAD's, and take + the key out; the text says to stage the file. Typed, staged as it + says and committed: nothing is left, and the gate and a plain clone + pass.""" + env = cloning(offline, tmp_path, family) + staged_only(holder, env, valued(head_bytes(holder, env), ROW, + b"\tupdate = !true\n")) + status = raw(holder, env, "submodule", "status", check=False) + assert status.returncode == 128, "premise: git dies reading it" + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="in the index's file, which git reads here, " + "not in HEAD", dies=DIES_HERE) + + "The work tree holds no .gitmodules, so the first line below puts " + "back the index's. Take the key out by hand, in a POSIX shell, then " + "stage .gitmodules:\n git checkout -- .gitmodules\n git " + f"config -f .gitmodules --unset-all submodule.{ROW}.update"] + typed_in_order(holder, env, validate(holder, "--no-members")) + raw(holder, env, "add", "--", GITMODULES) + committed_as_staged(holder, env) + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +def test_1077_a_value_head_alone_gives_says_to_commit_the_index_s_file( + family, holder, offline, tmp_path): + """NIT 2's other half, lane 1's NIT 1 on #889: `update = !true` + committed in MEMBER's section, then the file mended and staged, and + the work tree's deleted. Git reads the index's, which does not give + it, and a clone reads HEAD's, which does. Main put HEAD's file back + over the staged fix and took the key out of it again. It says to + commit the index's file now, with no line. A plain `git commit` then + records the fix, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + staged_only(holder, env, valued(data, ROW, b"\tupdate = !true\n")) + raw(holder, env, "commit", "-q", "-m", "update = !true committed") + staged_only(holder, env, data) + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="in HEAD, which a clone reads, not in the " + "index's file, which git reads here", + dies=DIES_IN_A_CLONE) + + "Commit the index's .gitmodules, which git reads here and does " + "not give it."] + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + raw(holder, env, "checkout", "--", GITMODULES) + ends_well(holder, env, family, tmp_path) + + +def test_1077_where_head_alone_gives_a_value_a_section_puts_the_file_back( + family, holder, offline, tmp_path): + """NIT 2's other half beside `member-gitmodules-absent`: `update = + !true` committed in MEMBER's section, with OTHER's section taken out, + then the file mended and staged, OTHER's still out, and the work + tree's deleted. The value's finding says to commit the index's file + and puts nothing back, so OTHER's finding puts it back first itself: + with no file there, its `git config -f` lines made a file of OTHER's + section alone, and staged and committed, MEMBER's was lost. Typed as + printed, staged and committed as it says: HEAD gives both members and + no value, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + committed_gitmodules(holder, env, section_taken_out( + valued(data, ROW, b"\tupdate = !true\n"), OTHER)) + staged_only(holder, env, section_taken_out(data, OTHER)) + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="in HEAD, which a clone reads, not in the " + "index's file, which git reads here", + dies=DIES_IN_A_CLONE) + + "Commit the index's .gitmodules, which git reads here and does " + "not give it.", + put_back_first(absent(OTHER, pinned(holder, OTHER), held=True), + "deleted")] + typed_in_order(holder, env, validate(holder, "--no-members")) + raw(holder, env, "add", "--", GITMODULES) + committed_as_staged(holder, env) + assert b"!true" not in head_bytes(holder, env) + assert head_paths(holder, env) == {ROW, OTHER_ROW} + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +# --- NIT 3: where HEAD's file holds the section ------------------------------ + +def stale_section(root: Path, env: dict, family) -> str: + """A section for `members/Old`, which no row names and no gitlink is + at, committed. Returns its name.""" + with_section(root, root.parent, "members/Old", "members/Old", + "/nowhere/old.git") + return "members/Old" + + +def committed_tools_x(root: Path, env: dict, family) -> str: + """IRSS mounted at tools/x with `git submodule add` and committed + (rv882's SHOULD 2 state). Returns its section's name.""" + return at_tools(root, env, family)[0] + + +#: `(how the section HEAD holds is made, what is left untracked)`. +HELD = {"a-stale-section": (stale_section, b""), + "tools-x-committed": (committed_tools_x, b"?? tools/\n")} + +#: How the work tree's `.gitmodules` is, or is not, there. +DISKS = ["on-disk", *GONE] + + +@pytest.mark.parametrize("disk", DISKS) +@pytest.mark.parametrize("make, left", HELD.values(), ids=HELD) +def test_1077_a_section_head_holds_comes_out_under_a_plain_commit( + family, holder, offline, tmp_path, make, left, disk): + """NIT 3: a section HEAD's `.gitmodules` holds and no row names, a + stale one or tools/x's with its gitlink, with the work tree's file on + disk, deleted, or its deletion staged. The lines ended "then commit + .gitmodules" and staged nothing: a plain `git commit` after them left + the section in HEAD (exit 1, `no changes added`, or, for tools/x, exit + 0 with the gitlink's removal alone), the local gate passed and a + clone's failed. They end with `git add -- .gitmodules` now, saying + "then stage and commit". Only the printed lines typed, then a plain + `git commit`: HEAD gives the two members alone, and the gate and a + plain clone pass.""" + env = cloning(offline, tmp_path, family) + name = make(holder, env, family) + if disk != "on-disk": + gone(holder, env, disk) + + found = found_alike(holder, *MODES) + + assert codes_of(found) == ["gitmodules-path-unnamed"] + assert commands(found[0])[-1] == STAGE, found[0] + assert "then stage and commit .gitmodules:" in found[0], found[0] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert f'[submodule "{name}"]'.encode() not in head_bytes(holder, env) + assert head_paths(holder, env) == {ROW, OTHER_ROW} + assert raw(holder, env, "status", "--porcelain").stdout == left + ends_well(holder, env, family, tmp_path) + + +# --- NIT 4: the case-variant branch, HEAD's holding no such section ---------- + +@pytest.mark.parametrize("disk", ["on-disk", "deleted"]) +def test_1077_a_stray_section_head_does_not_hold_is_staged_out( + family, holder, offline, tmp_path, disk): + """NIT 4, RV1069's probe `test_r2_stray_not_in_head`: a section named + `ghost` giving `Members/IRRS`, a case variant of MEMBER's path, staged + in the index's `.gitmodules` alone, the work tree's on disk or + deleted. HEAD's holds no section of that name, so the lines end with + `git add -- .gitmodules`; without it, a plain `git commit` after them + recorded `ghost`, and no row said so. Only the printed lines typed, + then a plain `git commit`: HEAD is as it was, nothing is left, and the + gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + ghost = (b'[submodule "ghost"]\n\tpath = Members/IRRS\n' + b"\turl = /nowhere/ghost.git\n") + if disk == "deleted": + staged_only(holder, env, data + ghost) + else: + (holder / GITMODULES).write_bytes(data + ghost) + raw(holder, env, "add", "--", GITMODULES) + + found = found_alike(holder, *MODES) + + put_back = [GONE["deleted"][1]] if disk == "deleted" else [] + assert found == [ + SECTION_SAID.format(path="Members/IRRS", name="ghost", + url="url /nowhere/ghost.git") + + VARIANT.format(row_path=ROW, project="IRRS") + + " Take the section out by hand, in a POSIX shell, every key it " + "holds, so a submodule later given its name inherits none of them, " + + ("after the line that puts .gitmodules, gone from the work tree, " + "back from the index's, so that every other section stays, " + if put_back else "") + + "then stage .gitmodules: HEAD's holds no section of that name, so " + "nothing of it is left to commit:" + + "".join(f"\n {line}" for line in + [*put_back, *removal("ghost", "path", "url"), STAGE])] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) diff --git a/tests/test_validate_family_remedies_typed.py b/tests/test_validate_family_remedies_typed.py index 559ea01b..b903ee15 100644 --- a/tests/test_validate_family_remedies_typed.py +++ b/tests/test_validate_family_remedies_typed.py @@ -63,8 +63,8 @@ from test_family_tracking_branch import on_main # noqa: F401 from test_validate_family_branch_step import BRANCH_LEFT from test_validate_family_unnamed_mounts import ( - GITLINK_REMEDY, GITLINK_SAID, GITLINK_SAID_FETCHED, VARIANT, codes, - commands, findings, removal, spare_commits, stage_gitlink, + GITLINK_REMEDY, GITLINK_SAID, GITLINK_SAID_FETCHED, STAGE, VARIANT, + codes, commands, findings, removal, spare_commits, stage_gitlink, stage_unmerged_gitlink, take_out_as_printed, with_section, ) @@ -152,7 +152,7 @@ def test_a_hand_mounts_staged_removal_is_committed_and_added_as_printed( commit_all(holder, env, "Take the hand mount out, as printed") else: (left,) = section - assert commands(left) == removal(HAND, "path", "url") + assert commands(left) == [*removal(HAND, "path", "url"), STAGE] take_out_as_printed(holder, left) assert (holder / HAND / ".git").is_file(), "premise: checkout left" taken = added_as_printed(family, holder, env, add) @@ -240,7 +240,7 @@ def test_a_section_git_rm_cached_leaves_keeps_its_own_commands( "gitmodules-path-unnamed"] gitlink, section = findings(result) assert "git rm" not in "".join(gitlink.splitlines()[1:]), gitlink - assert commands(section) == removal(HAND, "path", "url") + assert commands(section) == [*removal(HAND, "path", "url"), STAGE] assert "takes this section out too" not in result.stderr take_out_as_printed(holder, section) assert_passes_in_every_mode(holder, 2) diff --git a/tests/test_validate_family_unnamed_mounts.py b/tests/test_validate_family_unnamed_mounts.py index 23defc41..6c426f61 100644 --- a/tests/test_validate_family_unnamed_mounts.py +++ b/tests/test_validate_family_unnamed_mounts.py @@ -126,7 +126,13 @@ def switch_for(holder, member: str) -> str: " If its row was deleted or its path: edited by hand, put the row back " "as git last had it. Otherwise take the section out by hand, in a POSIX " "shell, every key it holds, so a submodule later given its name inherits " - "none of them, then commit .gitmodules:") + "none of them, then stage and commit .gitmodules:") + +#: The line every section's remedy prints after its keys' lines (#1077, +#: RV1069's NIT 3 on #1069): the commit records the index's `.gitmodules`, +#: so where HEAD's holds the section a plain `git commit` after the keys' +#: lines alone left it there. +STAGE = "git add -- .gitmodules" #: What a path a row names in another case adds, in place of the "deleted #: or edited" sentence: the row it belongs to. @@ -337,7 +343,8 @@ def test_a_section_no_row_names_is_a_finding(holder, tmp_path, extra): assert section.splitlines()[0] == SECTION_SAID.format( path="members/Old", name="members/Old", url="url /nowhere/old.git") + SECTION_REMEDY - assert commands(section) == removal("members/Old", "path", "url") + assert commands(section) == [*removal("members/Old", "path", "url"), + STAGE] @MODES @@ -360,8 +367,8 @@ def test_a_section_for_a_case_variant_of_a_members_path_names_that_row( + VARIANT.format(row_path="members/IRRS", project="IRRS") + " Take the section out by hand, in a POSIX shell, every key it " "holds, so a submodule later given its name inherits none of them, " - "then commit .gitmodules:") - assert commands(section) == removal("ghost", "path", "url") + "then stage and commit .gitmodules:") + assert commands(section) == [*removal("ghost", "path", "url"), STAGE] # --- the two findings' edges ------------------------------------------------- @@ -632,8 +639,8 @@ def test_two_sections_for_a_hand_mounts_path_each_keep_their_commands( "gitmodules-path-unnamed", "gitmodules-path-unnamed"] first, second = findings(result)[2:] - assert commands(first) == removal(path, "path", "url") - assert commands(second) == removal("second", "path", "url") + assert commands(first) == [*removal(path, "path", "url"), STAGE] + assert commands(second) == [*removal("second", "path", "url"), STAGE] assert first.splitlines()[0].endswith(SECTION_REMEDY) assert "takes this section out too" not in result.stderr git(*shlex.split(commands(findings(result)[1])[1])[1:], cwd=holder) From 8dfa32bd78027fb68646b4716318ac1407ce34b9 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 15:15:01 +0000 Subject: [PATCH 02/12] validate-family.py: `_listed_as_git_reads` asks no bare `lexists` of its own, so #1111's hygiene test passes, path-twice says "back first from" the file it puts back, and two rows pin the value's write-back and path-twice's recorded put-back (#1077) Fix round 1 on #1124, from RV1124's review at 672845cc. S1. `_listed_as_git_reads` opened with a bare `os.path.lexists(root / GITMODULES)`, which #1111's `test_path_guard_hygiene.py` fails by file and line. Its one caller, `_check_gitmodules_values`, asks it only where the work tree's file is gone, and `_gitmodules_put_back`, which the function calls next, asks the same `lexists` (it is on the test's allowlist), so the guard is dropped. N3. Two behaviours had no row; mutants A and B survived all 439 rows of the PR's nine files. A row for each, from RV1124's probes P10 and P7: - the value's write-back reads the file git reads, not HEAD's (A); - path-twice records its put-back, so `gitmodules-path-unnamed` after it prints none and a plain commit keeps neither `second` nor `members/Old` in HEAD (B), where the work tree's file is deleted or its deletion is staged. N4. Path-twice said "back from HEAD's, as the index holds none first": "first" sat on the wrong phrase. It says "back first from {whose}". N5. `_gitlink_out_first`'s docstring said its text ends "then commit .gitmodules"; every branch stages now, and it says which words each branch ends with. Co-Authored-By: Claude Sonnet 5.5 --- .../family-root/scripts/validate-family.py | 24 ++--- .../test_validate_family_index_gitmodules.py | 88 ++++++++++++++++++- 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index a23c30e5..dfb50769 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -2315,8 +2315,8 @@ def _check_gitmodules_paths(root: Path, if "url" in sections[name] else "no url") + ")" for name in names) put_back, whose = _file_put_back(root, report) - first = (f", put {GITMODULES}, gone from the work tree, back from " - f"{whose} first, `{put_back}`, so that every other section " + first = (f", put {GITMODULES}, gone from the work tree, back first " + f"from {whose}, `{put_back}`, so that every other section " "stays, and " if put_back else " and ") report.finding( "gitmodules-path-twice", @@ -3366,10 +3366,11 @@ def _gitlink_out_first(root: Path, name: str, keys: dict[str, str], submodule update --init` stops (`No url found for submodule path`, exit 128) before it checks out any member, and `git submodule status` exits 128 (`no submodule mapping found`). It ends as the section's - finding does, `then commit .gitmodules`, but where HEAD's `.gitmodules` - holds no section of its name, as after a `git submodule add` not - committed: staged, the file is HEAD's again, and nothing of it is - left to commit. + finding does (`_section_then`): `then stage and commit .gitmodules` + where HEAD's `.gitmodules` holds a section of its name, and `then stage + .gitmodules` where it holds none, as after a `git submodule add` not + committed: staged, the file is HEAD's again, and nothing of it is left + to commit (RV1124's N5). `git add -- .gitmodules` COMES FIRST where `edited`, where lines printed before it may leave the file with changes not staged (a @@ -3868,13 +3869,12 @@ def _blob_listed(root: Path, spec: str) -> bytes: def _listed_as_git_reads(root: Path) -> tuple[bytes, str]: """`(what git config -z --list prints of .gitmodules as git reads it, git's name for the file it reads in place of the work tree's)`: the - work tree's where it is on disk (`_gitmodules_listed`) and no name, - else the index's, then HEAD's (`_gitmodules_put_back`, - `_blob_listed`), and nothing where no file is there (#1077). What - `gitmodules-value-refused` reads here, as `_sections_as_git_reads` + index's, then HEAD's (`_gitmodules_put_back`, `_blob_listed`), and + nothing where no file is there (#1077). The caller asks it only where + the work tree's file is gone, which `_gitmodules_put_back` asks again, + so it asks no guard of its own (RV1124's S1: #1111's hygiene test). + What `gitmodules-value-refused` reads here, as `_sections_as_git_reads` reads the sections.""" - if os.path.lexists(root / GITMODULES): - return _gitmodules_listed(root), "" spec = _gitmodules_put_back(root)[0] return (_blob_listed(root, spec) if spec else b""), spec diff --git a/tests/test_validate_family_index_gitmodules.py b/tests/test_validate_family_index_gitmodules.py index 317d9405..f2b0fd3e 100644 --- a/tests/test_validate_family_index_gitmodules.py +++ b/tests/test_validate_family_index_gitmodules.py @@ -97,8 +97,8 @@ def head_bytes(root: Path, env: dict) -> bytes: #: What `gitmodules-path-twice` says where the work tree's file is gone, #: before what it says to take out. -TWICE_PUT_BACK = (", put .gitmodules, gone from the work tree, back from the " - "index's first, `git checkout -- .gitmodules`, so that " +TWICE_PUT_BACK = (", put .gitmodules, gone from the work tree, back first " + "from the index's, `git checkout -- .gitmodules`, so that " "every other section stays, and take each other out by " "hand") @@ -137,6 +137,54 @@ def test_1077_a_second_section_the_index_alone_gives_is_path_twice( ends_well(holder, env, family, tmp_path) +#: A section for a path no row names, in HEAD's file as the work tree's was +#: committed: `gitmodules-path-unnamed`'s, after path-twice's. +STALE = (b'[submodule "members/Old"]\n\tpath = members/Old\n' + b"\turl = /nowhere/old.git\n") + + +@pytest.mark.parametrize("how", GONE) +def test_1077_path_twice_records_its_put_back_for_the_next_finding( + family, holder, offline, tmp_path, how): + """RV1124's N3(b): HEAD's file holds `second`, a second section for + `members/IRRS`, and a stale `members/Old`, and `.gitmodules` is gone + from the work tree. Path-twice names the put-back first; it is + recorded (`_file_put_back`), so `gitmodules-path-unnamed` after it + puts nothing back. Unrecorded, that finding printed the put-back + again, and typed in order it put the file back over the hand edit: + the commit then kept `second` and `members/Old` in HEAD. Typed in + order as printed, then a plain `git commit`: HEAD loses both, and the + gate passes in every mode.""" + env = cloning(offline, tmp_path, family) + committed_gitmodules(holder, env, + head_bytes(holder, env) + SECOND + STALE) + gone(holder, env, how) + + found = found_alike(holder, *MODES) + + assert codes_of(found) == ["gitmodules-path-twice", + "gitmodules-path-unnamed"], found + twice, unnamed = found + put_back = GONE[how][1] + assert f"`{put_back}`" in twice, twice + assert put_back not in unnamed, unnamed + type_line(holder, env, put_back) + for key in ("path", "url"): + type_line(holder, env, shlex.join( + ["git", "config", "-f", GITMODULES, "--unset-all", + f"submodule.second.{key}"])) + raw(holder, env, "add", "--", GITMODULES) + for command in commands(unnamed): + if command.startswith("git "): + type_line(holder, env, command) + committed_as_staged(holder, env) + kept = head_bytes(holder, env) + assert b"second" not in kept and b"members/Old" not in kept + assert head_paths(holder, env) == {ROW, OTHER_ROW} + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + # --- NIT 2: a value git dies on, in the index's file ------------------------- def valued(data: bytes, path: str, line: bytes) -> bytes: @@ -258,6 +306,42 @@ def test_1077_where_head_alone_gives_a_value_a_section_puts_the_file_back( ends_well(holder, env, family, tmp_path) +def test_1077_a_url_git_dies_on_is_written_back_as_the_file_git_reads_gave_it( + family, holder, offline, tmp_path): + """RV1124's N3(a): the index's file, the work tree's deleted, gives + MEMBER's section a `url` that is not HEAD's, then a `url` with no + value, which git dies on. The line that writes the url back, as it + last gave one, takes it from the file git reads (the index's), not + HEAD's: from HEAD's it undid what was staged. Typed in order, then + staged and committed: HEAD gives the staged url, and the gate passes + in `--no-members`.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + url = "https://github.com/InkRouter/Staged.git" + edited, count = re.subn( + rb"(\tpath = " + re.escape(ROW.encode()) + rb"\n)\turl = [^\n]*\n", + rb"\1\turl = " + url.encode() + rb"\n\turl\n", data) + assert count == 1, data + assert url.encode() not in data + staged_only(holder, env, edited) + assert raw(holder, env, "submodule", "status", + check=False).returncode == 128, "premise: git dies reading it" + + found = found_alike(holder, *MODES) + + assert codes_of(found) == ["gitmodules-value-refused"], found + lines = commands(found[0]) + assert lines[0] == "git checkout -- .gitmodules", lines + assert lines[-1] == (f"git config -f .gitmodules submodule.{ROW}.url " + f"{url}"), lines + typed_in_order(holder, env, validate(holder, "--no-members")) + raw(holder, env, "add", "--", GITMODULES) + committed_as_staged(holder, env) + assert url.encode() in head_bytes(holder, env) + assert raw(holder, env, "status", "--porcelain").stdout == b"" + assert validate(holder, "--no-members").returncode == 0 + + # --- NIT 3: where HEAD's file holds the section ------------------------------ def stale_section(root: Path, env: dict, family) -> str: From 442615eb36916544182908cdbc72f0f0f13f7eba Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 22:24:05 +0000 Subject: [PATCH 03/12] validate-family.py: the check for values git dies on reads the index's `.gitmodules` beside the work tree's and its stage remedies end `git add -- .gitmodules`, an index file git cannot parse is refused, and an unmerged one is read as none (#1150) RV1124's review of #1124 (pull/1124#issuecomment-6321372469) found three things outside #1124's fix round, each older than #1124 and measured again on main 961640c6 and on #1124's head 8dfa32bd before this change: - N1: where the work tree's `.gitmodules` is on disk, the check never read the index's, which a plain `git commit` records. A value in both was taken out of the work tree's by a line that staged nothing, so the commit after it recorded the value in HEAD; a value in the index's alone was `family ok`. The check reads the index's too now, names the latter ("in the index's file alone"), and every remedy that says to stage the file ends with the line that does. - N2: an index's file with a line git cannot parse was read as nothing (`git config --blob` exits 128 there), so a value or a section before the line was never named. It is `gitmodules-unreadable` now, as HEAD's is. - N6: while a merge leaves `.gitmodules` unmerged in the index, git's submodule commands read no file, and the checks read the work tree's or HEAD's. They read none now; a value HEAD's gives says to resolve the merge without the key, then stage and commit. One finding changes: a value only the work tree's file gives, on disk, while unmerged, HEAD's clean, is no finding until the merge is resolved and staged, as git reads no file until then. Stacked on #1124 at 8dfa32bd: every item also holds on main, but the code each fix changes (`_blob_listed`, `READ_HERE`, `_listed_as_git_reads`, the stage line) is #1124's. Closes #1150 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- .../family-root/scripts/validate-family.py | 228 +++++++++-- tests/test_family_gitmodules_values.py | 41 +- .../test_validate_family_index_gitmodules.py | 23 +- ...st_validate_family_index_value_unmerged.py | 380 ++++++++++++++++++ 4 files changed, 613 insertions(+), 59 deletions(-) create mode 100644 tests/test_validate_family_index_value_unmerged.py diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index dfb50769..b81da47a 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -64,7 +64,8 @@ clone fetches that path from only one of them, so the other is a url the next edit can make the one. It reads that file alone, as git reads it: the work tree's, or where that is gone from disk the index's, - then HEAD's (#1077), so `--no-members` asks it too. + then HEAD's (#1077), and none while a merge leaves it unmerged in the + index (#1150), so `--no-members` asks it too. - that no section of `.gitmodules` gives a value git dies on as it reads the file, before `git submodule update` checks out any member, the `gitmodules-value-refused` finding (#768): a `path`, `url`, `update`, @@ -72,8 +73,12 @@ `checkout`, `rebase` or `merge`, a `shallow` that is no boolean, or a `fetchRecurseSubmodules` that is neither a boolean nor `on-demand`. It reads the file git reads, the work tree's, or where that is gone from - disk the index's, then HEAD's (#1077), and HEAD's, which a clone reads, - so `--no-members` asks it too, + disk the index's, then HEAD's (#1077), and none while a merge leaves + it unmerged in the index; HEAD's, which a clone reads; and, where the + work tree's is on disk, the index's, which a plain `git commit` + records, each line set that says to stage the file ending with `git + add -- .gitmodules` (#1150). An index's file git cannot parse is + `gitmodules-unreadable`, as HEAD's is. So `--no-members` asks it too, and where it is found no member that is not checked out is refused as uninitialized: git checked none out. - the REVERSE of every member check (#361): that each gitlink this @@ -1977,10 +1982,27 @@ def _head_listed(root: Path) -> tuple[bool, bytes] | None: #: Where `gitmodules-value-refused` says a value is, by whether the file #: git reads here and HEAD's give it (#768); `{here}` names the file git #: reads here (`READ_HERE`), the work tree's, or where that is gone from -#: disk the index's (#1077). +#: disk the index's (#1077). Neither does where the index's file alone +#: gives it, with the work tree's on disk (#1150, RV1124's N1 on #1124). GIVEN_WHERE = {(True, True): "here and in HEAD, which a clone reads", (True, False): "in {here}, not in HEAD", - (False, True): "in HEAD, which a clone reads, not in {here}"} + (False, True): "in HEAD, which a clone reads, not in {here}", + (False, False): "in the index's file alone, which a plain " + "`git commit` records, not in {here} or in " + "HEAD"} + +#: How git dies on a value, by where it is given (`GIVEN_WHERE`): here, +#: in a clone where HEAD's alone gives it, and in a clone of the commit +#: that records the index's where that file alone gives it (#1150). +VALUE_DIES = { + (True, True): "before `git submodule update` checks out any member, so " + "neither that nor make bootstrap checks one out", + (False, True): "so in a clone neither `git submodule update` nor make " + "bootstrap checks out any member", + (False, False): "so in a clone of the commit that records it neither " + "`git submodule update` nor make bootstrap checks out " + "any member"} +VALUE_DIES[(True, False)] = VALUE_DIES[(True, True)] #: How `GIVEN_WHERE` names the file git reads here, by git's name for the #: one it reads in place of the work tree's (`_listed_as_git_reads`): none @@ -1989,6 +2011,18 @@ def _head_listed(root: Path) -> tuple[bool, bytes] | None: f":{GITMODULES}": "the index's file, which git reads here", f"HEAD:{GITMODULES}": "HEAD's file, which git reads here"} +#: How `GIVEN_WHERE` names the file git reads here while a merge leaves +#: `.gitmodules` unmerged in the index (`_gitmodules_unmerged`): none +#: (#1150, RV1124's N6 on #1124). +UNMERGED_HERE = ("the file git reads here, which is none while a merge " + f"leaves {GITMODULES} unmerged") + +#: What the index's file adds to `gitmodules-value-refused` where the work +#: tree's, on disk, gives the value and HEAD's does not: a plain `git +#: commit` records the index's (#1150, RV1124's N1 on #1124). +INDEX_TOO = ("The index's file, which a plain `git commit` records, gives it " + "too.") + #: What git takes for a `submodule..` whose value it dies on as #: it reads `.gitmodules` (#768), as `gitmodules-value-refused` says it. #: This, `_values_refused` and `_git_takes_bool` say what bootstrap's @@ -2042,62 +2076,112 @@ def _check_gitmodules_values(root: Path, members: list, and ends staging it: the index's file is what the commit records. Every one of these is recorded as a write (`Report.wrote`): a staged removal's finding after it then puts no file back over the fix (lane - 1's SHOULD 1).""" + 1's SHOULD 1). + + THE INDEX'S FILE TOO, where the work tree's is on disk (#1150, + RV1124's N1 on #1124): git reads the work tree's here, but a plain + `git commit` records the index's. Read from the work tree's and HEAD's + alone, an `update = !true` given in the work tree's file and staged + was taken out of the work tree's file by a line that staged nothing, + so a plain `git commit` after it recorded the value in HEAD, and one + given in the index's file alone, the work tree's clean, was `family + ok`, exit 0. Every remedy that says to stage the file now ends with + the line that stages it, `git add -- .gitmodules` (`STAGE_LINE`). + + NO FILE WHILE A MERGE LEAVES `.gitmodules` UNMERGED (#1150, RV1124's + N6 on #1124): git's submodule commands read none then, the work + tree's, the index's or HEAD's (`_gitmodules_unmerged`), so none is + read here, and HEAD's, which a clone reads, is said to be resolved: + read as the work tree's, or HEAD's where that was gone, a value HEAD's + gave was "here", and its line put HEAD's back "as the index holds + none".""" present = os.path.lexists(root / GITMODULES) # The work tree's file, then HEAD's, then the one git reads in place of # the work tree's: with no git to run, `git-not-runnable` names the # first of them it meets, as before #1077. on_disk = _gitmodules_listed(root) head = _head_listed(root) - listed, spec = (on_disk, "") if present else _listed_as_git_reads(root) + unmerged = _gitmodules_unmerged(root) + listed, spec = ((b"", "") if unmerged else (on_disk, "") if present + else _listed_as_git_reads(root)) here = _submodule_entries(listed) there = _submodule_entries(head[1]) if head else [] in_work = _values_refused(root, here) in_head = _values_refused(root, there) + in_index = (_values_refused(root, _submodule_entries(_index_listed(root))) + if present else {}) rows = {f"{DEFAULT_MEMBERS_DIR}/{row['project']}": row.get("repository") for row in members if isinstance(row, dict) and isinstance(row.get("project"), str) and row["project"]} - for name, key in dict.fromkeys([*in_work, *in_head]): + for name, key in dict.fromkeys([*in_work, *in_head, *in_index]): report.wrote = True - said = _value_said(name, key, in_work, in_head, READ_HERE[spec]) + said = _value_said(name, key, (in_work, in_head, in_index), + UNMERGED_HERE if unmerged else READ_HERE[spec]) if (name, key) not in in_work: - report.finding("gitmodules-value-refused", f"{said} " + ( - f"Stage and commit the work tree's {GITMODULES}, which does " - "not give it." if present else - f"Commit the index's {GITMODULES}, which git reads here and " - "does not give it.")) + report.finding("gitmodules-value-refused", f"{said} " + + _value_not_here(present, unmerged, + (name, key) in in_head)) continue back = _written_back(name, key, here, rows) report.finding("gitmodules-value-refused", _value_taken_out( said, f"submodule.{name}.{key}", back, _file_put_back(root, report), committed=(name, key) in in_head, - staged=not present)) + staged=not present or (name, key) in in_index)) return bool(in_work), bool(in_work or in_head) +def _value_not_here(present: bool, unmerged: bool, committed: bool) -> str: + """`gitmodules-value-refused`'s remedy where the file git reads here + does not give the value, HEAD's or the index's does (#768, #1077, + #1150): the merge resolved without it where a merge leaves + `.gitmodules` unmerged (`unmerged`), or else the work tree's file, + which does not give it, staged, and committed where HEAD's gives it + (`committed`), or, where that file is gone from disk (`present` + False), the index's, which git reads here, committed as it stands, + with no line. Each that says to stage ends with the line that does + (`STAGE_LINE`).""" + if unmerged: + return (f"Resolve the merge in {GITMODULES} by hand, leaving that " + f"key out, then stage and commit {GITMODULES}:" + f"\n {STAGE_LINE}") + if not present: + return (f"Commit the index's {GITMODULES}, which git reads here and " + "does not give it.") + return (("Stage and commit" if committed else "Stage") + + f" the work tree's {GITMODULES}, which does not give it:" + f"\n {STAGE_LINE}") + + def _value_said(name: str, key: str, - in_work: dict[tuple[str, str], list[str | None]], - in_head: dict[tuple[str, str], list[str | None]], + given: tuple[dict[tuple[str, str], list[str | None]], + dict[tuple[str, str], list[str | None]], + dict[tuple[str, str], list[str | None]]], here: str = READ_HERE[""]) -> str: """`gitmodules-value-refused` up to its remedy, for the key `key` of the section `name` (#768): each value the file git reads here (`here`, `READ_HERE`, #1077) and HEAD's give it, where, and what git takes. - Where HEAD's alone gives one, the file git reads here may not, so it - says git dies on it in a clone, which reads HEAD's.""" + `given` holds what the file git reads here, HEAD's and the index's + give, in that order: where the work tree's is on disk, git reads that + one here, not the index's, but a plain `git commit` records the + index's (#1150). Where HEAD's alone gives one, the file git reads here + may not, so it says git dies on it in a clone, which reads HEAD's; + where the index's alone does, in a clone of the commit that records it + (`VALUE_DIES`); and where the work tree's gives it, not HEAD's, it says + that the index's gives it too where it does (`INDEX_TOO`), which is + why its lines stage it.""" + in_work, in_head, in_index = given full = f"submodule.{name}.{key}" - values = dict.fromkeys([*in_work.get((name, key), []), - *in_head.get((name, key), [])]) where = (name, key) in in_work, (name, key) in in_head - dies = ("so in a clone neither `git submodule update` nor make " - "bootstrap checks out any member" if where == (False, True) - else "before `git submodule update` checks out any member, so " - "neither that nor make bootstrap checks one out") + values = dict.fromkeys([*in_work.get((name, key), []), + *in_head.get((name, key), [])] + or in_index.get((name, key), [])) + staged_too = (f" {INDEX_TOO}" if where == (True, False) + and (name, key) in in_index else "") return (f"{GITMODULES} gives `{spelled(full)}` " + " and ".join( "no value" if value is None else f"the value `{spelled(value)}`" for value in values) + f" {GIVEN_WHERE[where].format(here=here)}, " - "and git dies " - f"reading a file that does, {dies}. Git takes {GIT_TAKES[key]} for " - "that key.") + f"and git dies reading a file that does, {VALUE_DIES[where]}. Git " + f"takes {GIT_TAKES[key]} for that key.{staged_too}") def _value_taken_out(said: str, full: str, back: str | None, @@ -2109,8 +2193,12 @@ def _value_taken_out(said: str, full: str, back: str | None, before put it back (`put_back`, the line and whose file it puts back, `_file_put_back`, #1077), the key `full` taken out, and `back` written in, where it is given, then the commit where HEAD's gives the value - (`committed`), or, where the work tree's was gone and the index's - alone gave it (`staged`), the file staged, which the commit records.""" + (`committed`), or, where the index's gives it, or the work tree's was + gone (`staged`), the file staged, which the commit records. EITHER + ENDS WITH THE LINE THAT STAGES IT, `git add -- .gitmodules` + (`STAGE_LINE`, #1150, RV1124's N1 on #1124): said in words alone, a + plain `git commit` after the lines typed recorded the index's file, + the value and all.""" key = full.rpartition(".")[2] line, whose = put_back lines = [line] if line else [] @@ -2119,6 +2207,8 @@ def _value_taken_out(said: str, full: str, back: str | None, if back is not None: lines.append(f"git config -f {GITMODULES} {shlex.quote(full)} " f"{back}") + if committed or staged: + lines.append(STAGE_LINE) return (f"{said} " + (f"The work tree holds no {GITMODULES}, so the first line " f"below puts back {whose}. " if line else "") @@ -2910,13 +3000,43 @@ def _sections_as_git_reads(root: Path, worktree: dict[str, dict[str, str]] staged in the index's file, whose work tree's copy was deleted, was never named, and the commit after the hand mount's lines recorded it. Git 2.43 reads the index's there: `git submodule status ` maps - the path, which HEAD's file does not give.""" + the path, which HEAD's file does not give. + + NONE WHILE A MERGE LEAVES `.gitmodules` UNMERGED in the index + (`_gitmodules_unmerged`, #1150, RV1124's N6 on #1124), on disk or not: + git reads no file then, and `git submodule status` dies at the first + gitlink (`no submodule mapping found`, exit 128). The work tree's, or + HEAD's where that was gone, was read in its place.""" + if _gitmodules_unmerged(root): + return {} if os.path.lexists(root / GITMODULES): return worktree spec = _gitmodules_put_back(root)[0] return _blob_sections(root, spec) if spec else {} +def _gitmodules_unmerged(root: Path) -> bool: + """Whether the index holds `.gitmodules` only at a merge's conflict + stages, 1 to 3, as `git ls-files -s` lists it, and not at stage 0 + (#1150, RV1124's N6 on #1124). Git's submodule commands then read no + `.gitmodules` at all, the work tree's, the index's or HEAD's + (`repo_read_gitmodules` asks `is_gitmodules_unmerged` first), and + `git submodule status` dies at the first gitlink (`no submodule + mapping found`, exit 128, git 2.43), as bootstrap's + `gitmodules_source` reads it. An index git cannot list holds none.""" + with _git_runs(f"whether a merge left {GITMODULES} unmerged could not " + "be read"): + try: + listed = git_out(["ls-files", "-s", "-z", "--", GITMODULES], + cwd=root, binary=True) + except Refusal: + return False + stages = {meta.split()[2] for meta, _, name in ( + record.partition(b"\t") for record in listed.split(b"\0")) + if name == GITMODULES.encode() and len(meta.split()) == 3} + return bool(stages) and b"0" not in stages + + #: Why a plain `git rm` of a hand mount's gitlink refuses it where the #: index stages it at a commit HEAD does not record there, with something #: on disk at it (`_staged_over`): git's two refusals, each exit 1, the @@ -3856,14 +3976,56 @@ def _blob_sections(root: Path, spec: str) -> dict[str, dict[str, str]]: def _blob_listed(root: Path, spec: str) -> bytes: """What `git config -z --blob --list` prints of the - `.gitmodules` git names `spec`, or nothing where git cannot read it - (`_blob_sections`, `_listed_as_git_reads`, #1077).""" + `.gitmodules` git names `spec`, one git resolves, or nothing for no + `spec` (`_blob_sections`, `_listed_as_git_reads`, #1077). + + ONE GIT CANNOT PARSE IS `gitmodules-unreadable` (#1150, RV1124's N2 on + #1124), as HEAD's is (`_head_listed`), which `_check_gitmodules_values` + asks before any blob is read here, so the blob refused here is the + index's. `git config --blob` prints the entries before a line it + cannot parse, then exits 128, and nothing was read: an `update = + !true` before the line was no finding, `family ok` in every mode, while + git's own `submodule status` read it and died on it (exit 128), and a + section before it was never named. Git's submodule commands read such + a blob up to the line where the work tree holds no `.gitmodules`, and + a clone of a commit that records it checks the file out, which git + dies reading.""" + if not spec: + return b"" with _git_runs(f"{spec} could not be read as git reads it"): try: return git_out(["config", "-z", "--blob", spec, "--list"], cwd=root, binary=True) + except Refusal as exc: + raise Refusal( + "gitmodules-unreadable", + f"the index's {GITMODULES} could not be read as git reads " + f"it: {exc.detail}", + "Remediation: git reads it only up to that line where the " + f"work tree holds no {GITMODULES}, and a clone of a commit " + "that records it dies reading it, before it checks out any " + f"member. Mend {GITMODULES} in the work tree by hand (`git " + f"config -f {GITMODULES} --list` says where it breaks; where " + f"the work tree holds none, `git checkout -- {GITMODULES}` " + f"puts the index's back first), stage it with `{STAGE_LINE}`, " + "then run this again.") from exc + + +def _index_listed(root: Path) -> bytes: + """What `git config -z --list` prints of the index's `.gitmodules`, + the one at stage 0, which a plain `git commit` records, read as + `_blob_listed` reads a blob, or nothing where the index holds none + there (#1150, RV1124's N1 on #1124): what `gitmodules-value-refused` + reads beside the work tree's file where that is on disk, and git reads + it here, not the index's.""" + with _git_runs(f"the index's {GITMODULES} could not be read as git " + "reads it"): + try: + git_out(["rev-parse", "--verify", "--quiet", f":{GITMODULES}"], + cwd=root) except Refusal: return b"" + return _blob_listed(root, f":{GITMODULES}") def _listed_as_git_reads(root: Path) -> tuple[bytes, str]: diff --git a/tests/test_family_gitmodules_values.py b/tests/test_family_gitmodules_values.py index fc89631a..e2c9ffef 100644 --- a/tests/test_family_gitmodules_values.py +++ b/tests/test_family_gitmodules_values.py @@ -99,7 +99,7 @@ from test_family_member_path import MEMBER, OTHER, raw from test_family_tracking_branch import on_main # noqa: F401 from test_validate_family_typed_states import GONE, ROW, removal_staged -from test_validate_family_unnamed_mounts import commands, findings +from test_validate_family_unnamed_mounts import STAGE, commands, findings #: The issue's table, `update = None` first, then `branch` with no value: #: each a line added to OTHER's section after its url, as rv648 (p11) and @@ -190,12 +190,17 @@ def taken_out(key: str, value: str | None, *, back: str | None = None, out of the work tree's file, after the one that puts back the file git reads in its place where the work tree's is gone as `put_back` (`GONE`) says, the index's, or HEAD's where the index holds none - (#1077), and write `back` in where it is given.""" + (#1077), and write `back` in where it is given, then, where HEAD's + gives the value, the line that stages the file, which the commit + records (`STAGE`, #1150): said in words alone, a plain `git commit` + after the lines recorded the value the index's file still gave.""" whose, line = GONE[put_back] if put_back else ("", "") lines = ([line] if put_back else []) + [ f"git config -f .gitmodules --unset-all {full(key)}"] if back is not None: lines.append(f"git config -f .gitmodules {full(key)} {back}") + if where != WORK_TREE_ONLY: + lines.append(STAGE) return (said(key, value, where) + ("The work tree holds no .gitmodules, so the first line below " f"puts back {whose}. " if put_back else "") @@ -794,6 +799,10 @@ def test_a_bare_key_in_a_blob_is_read_up_to_and_named_in_the_cure( #: A line of the person's own, staged with the line git cannot parse. OWN_LINE = b"# a line of the person's own, staged with it\n" +#: How the gate refuses an index's `.gitmodules` git cannot parse (#1150). +INDEX_UNREADABLE = ("REFUSED gitmodules-unreadable: the index's .gitmodules " + "could not be read as git reads it: ") + @pytest.mark.parametrize("place", ("inside-a-section", "before-the-first-section")) @@ -803,12 +812,13 @@ def test_the_index_s_file_cut_short_is_put_back_from_the_index( whole: the line inside OTHER's section, or first in the file (RV877's SHOULD-2), staged with a line of the person's own, not committed, and the work tree's file lost. Git reads the index's up to the line, the - first one reading nothing, and dies at OTHER or MEMBER. The gate - reads the work tree's file and HEAD's, not the index's, so it cannot - name the line, and HEAD's file put back would lose what was staged. - The cure puts the index's back, its line first included: typed as - printed, mended and committed, the person's own line is in HEAD, and - the run is the credential's skip.""" + first one reading nothing, and dies at OTHER or MEMBER. HEAD's file + put back would lose what was staged. The cure puts the index's back, + its line first included: typed as printed, mended and committed, the + person's own line is in HEAD, and the run is the credential's skip. + The gate read the index's file as nothing there, so it never named the + line; it refuses that file `gitmodules-unreadable` now, naming the + line, as it refuses HEAD's (#1150).""" env = isolated(offline, tmp_path) clone = cloned(holder, env, family, tmp_path, recurse=False) staged = raw(clone, env, "show", f"HEAD:{GITMODULES}").stdout + OWN_LINE @@ -817,8 +827,9 @@ def test_the_index_s_file_cut_short_is_put_back_from_the_index( lose_work_tree_file(clone, env) git_reads_up_to_the_line(clone, env, place) gate = validate(clone, "--no-members") - assert "bad config line" not in gate.stdout + gate.stderr, ( - "premise: the gate cannot name the line", gate.stdout + gate.stderr) + assert gate.returncode == 2, gate.stdout + gate.stderr + assert gate.stderr.startswith(INDEX_UNREADABLE), gate.stderr + assert "bad config line" in gate.stderr, gate.stderr result = bootstrap(clone, env, "--skip-members") @@ -1247,7 +1258,8 @@ def edited_not_committed(root, env, data: bytes) -> None: WHERE = { "head-only": (mended_not_committed, said("update", "None", HEAD_ONLY) + "Stage and commit the " - "work tree's .gitmodules, which does not give it."), + "work tree's .gitmodules, which does not give it:" + f"\n {STAGE}"), "work-tree-only": (edited_not_committed, taken_out("update", "None", where=WORK_TREE_ONLY)), } @@ -1258,9 +1270,10 @@ def test_the_gate_says_which_file_gives_it(holder, offline, tmp_path, set_up, expected): """In the holder, members checked out: HEAD's file alone gives the value, the work tree's mended and not committed, and the finding says - to commit that, printing no command; or the work tree's alone, which - git reads here and a clone does not, and the command takes it out with - nothing to commit. Done as it says, the holder passes in every mode.""" + to stage and commit that, its one line staging it (#1150); or the work + tree's alone, which git reads here and a clone does not, and the + command takes it out with nothing to commit. Done as it says, the + holder passes in every mode.""" env = isolated(offline, tmp_path) set_up(holder, env, raw(holder, env, "show", f"HEAD:{GITMODULES}").stdout) diff --git a/tests/test_validate_family_index_gitmodules.py b/tests/test_validate_family_index_gitmodules.py index f2b0fd3e..a3d474dc 100644 --- a/tests/test_validate_family_index_gitmodules.py +++ b/tests/test_validate_family_index_gitmodules.py @@ -215,10 +215,10 @@ def test_1077_a_value_the_index_alone_gives_is_named_and_taken_out( Git reads the index's file and dies on it (`git submodule status`, exit 128). Main read the work tree's file and HEAD's and said `family ok`. It is `gitmodules-value-refused` now, in every mode, naming the - index's file, and its lines put that file back, not HEAD's, and take - the key out; the text says to stage the file. Typed, staged as it - says and committed: nothing is left, and the gate and a plain clone - pass.""" + index's file, and its lines put that file back, not HEAD's, take the + key out, and stage the file, the last line as the text says (#1150). + Typed, then a plain `git commit`: nothing is left, and the gate and a + plain clone pass.""" env = cloning(offline, tmp_path, family) staged_only(holder, env, valued(head_bytes(holder, env), ROW, b"\tupdate = !true\n")) @@ -233,9 +233,9 @@ def test_1077_a_value_the_index_alone_gives_is_named_and_taken_out( + "The work tree holds no .gitmodules, so the first line below puts " "back the index's. Take the key out by hand, in a POSIX shell, then " "stage .gitmodules:\n git checkout -- .gitmodules\n git " - f"config -f .gitmodules --unset-all submodule.{ROW}.update"] + f"config -f .gitmodules --unset-all submodule.{ROW}.update\n " + f"{STAGE}"] typed_in_order(holder, env, validate(holder, "--no-members")) - raw(holder, env, "add", "--", GITMODULES) committed_as_staged(holder, env) assert raw(holder, env, "status", "--porcelain").stdout == b"" ends_well(holder, env, family, tmp_path) @@ -312,9 +312,9 @@ def test_1077_a_url_git_dies_on_is_written_back_as_the_file_git_reads_gave_it( MEMBER's section a `url` that is not HEAD's, then a `url` with no value, which git dies on. The line that writes the url back, as it last gave one, takes it from the file git reads (the index's), not - HEAD's: from HEAD's it undid what was staged. Typed in order, then - staged and committed: HEAD gives the staged url, and the gate passes - in `--no-members`.""" + HEAD's: from HEAD's it undid what was staged. Its last line stages + the file (#1150). Typed in order, then a plain `git commit`: HEAD + gives the staged url, and the gate passes in `--no-members`.""" env = cloning(offline, tmp_path, family) data = head_bytes(holder, env) url = "https://github.com/InkRouter/Staged.git" @@ -332,10 +332,9 @@ def test_1077_a_url_git_dies_on_is_written_back_as_the_file_git_reads_gave_it( assert codes_of(found) == ["gitmodules-value-refused"], found lines = commands(found[0]) assert lines[0] == "git checkout -- .gitmodules", lines - assert lines[-1] == (f"git config -f .gitmodules submodule.{ROW}.url " - f"{url}"), lines + assert lines[-2:] == [f"git config -f .gitmodules submodule.{ROW}.url " + f"{url}", STAGE], lines typed_in_order(holder, env, validate(holder, "--no-members")) - raw(holder, env, "add", "--", GITMODULES) committed_as_staged(holder, env) assert url.encode() in head_bytes(holder, env) assert raw(holder, env, "status", "--porcelain").stdout == b"" diff --git a/tests/test_validate_family_index_value_unmerged.py b/tests/test_validate_family_index_value_unmerged.py new file mode 100644 index 00000000..36fe928d --- /dev/null +++ b/tests/test_validate_family_index_value_unmerged.py @@ -0,0 +1,380 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The family holder's pinned `scripts/validate-family.py`: the check for +values git dies on reads the index's `.gitmodules` beside the work tree's, +an index's file git cannot parse is refused, and no file is read while a +merge leaves `.gitmodules` unmerged (#1150). + +RV1124's review of #1124 (pull/1124#issuecomment-6321372469) found three +things, each older than #1124, and each measured again on #1124's head +8dfa32bd and on main 961640c6 before this change: + +- N1. Where the work tree's file is on disk, the check never read the + index's, which a plain `git commit` records. An `update = !true` given + in the work tree's file and staged was taken out of the work tree's by + a line that staged nothing, so a plain `git commit` after it recorded + the value in HEAD; one given in the index's file alone, the work tree's + clean on disk, was `family ok`, exit 0, in every mode. +- N2. An index's file with a line git cannot parse was read as nothing, + since `git config --blob` exits 128 there: an `update = !true` before + the line was `family ok` while git's own `submodule status` died on it + (exit 128), and a section before the line was never named. +- N6. While a merge leaves `.gitmodules` unmerged in the index, git reads + no file at all, and the checks read the work tree's, or HEAD's where + that was gone: a value HEAD's gave was "here and in HEAD", its line put + HEAD's back "as the index holds none", and a section HEAD's gave was + named with that same line, which resolves the merge to HEAD's side. A + value the work tree's file alone gives, on disk, while the merge is + unresolved, is no finding now, as git reads no file; once the merge is + resolved and staged, it is named as N1's is. + +Every row types only the printed `git` lines, in order, and does by hand +only what the text says in words, then commits what is staged with a +plain `git commit`, as `tests/test_validate_family_index_gitmodules.py` +does: HEAD's `.gitmodules` is then right, the gate passes in every mode, +and a plain clone checks both members out. +""" + +from __future__ import annotations + +import re +import shutil +from pathlib import Path + +import pytest + +from test_family import NAME, family, validate # noqa: F401 +from test_family_add_refusals import holder, offline # noqa: F401 +from test_family_bootstrap_gate_order import cloned, type_line +from test_family_head_lost_gitmodules import committed_gitmodules +from test_family_holder_remedies_compose import ( + MODES, cloning, codes_of, ends_well, found_alike, typed_in_order, +) +from test_family_member_path import raw +from test_validate_family_index_gitmodules import ( + DIES_HERE, DIES_IN_A_CLONE, STALE, VALUE_SAID, committed_as_staged, + head_bytes, staged_only, valued, +) +from test_validate_family_typed_states import ROW +from test_validate_family_unnamed_mounts import STAGE, commands + +GITMODULES = ".gitmodules" +NOT_RUN = b"\tupdate = !true\n" + +#: What the value's finding adds where the work tree's file gives it, not +#: HEAD's, and the index's does too. +INDEX_TOO = ("The index's file, which a plain `git commit` records, gives " + "it too.") + +#: What the value's finding says where the work tree's file and the +#: index's give it, not HEAD's: the key taken out, then the file staged. +INDEX_TOO_TAKEN_OUT = ( + VALUE_SAID.format(where="in the work tree, not in HEAD", dies=DIES_HERE) + + INDEX_TOO + " Take the key out by hand, in a POSIX shell, then stage " + ".gitmodules:\n git config -f .gitmodules --unset-all " + f"submodule.{ROW}.update\n {STAGE}") + +#: How git dies on a value the index's file alone gives. +DIES_IN_THE_COMMIT = ("so in a clone of the commit that records it neither " + "`git submodule update` nor make bootstrap checks out " + "any member") + +#: A line git cannot parse, as `tests/test_family_gitmodules_values.py`'s +#: RV1124 probe P9 has it. +UNPARSED = b"this is no config line\n" + + +def a_plain_clone_of_what_is_staged(root: Path, env: dict, family, + tmp_path: Path): + """A copy of `root` whose staged `.gitmodules` a plain `git commit` + records, then a plain clone of that commit and its `git submodule + update --init --recursive`, whose result is returned: what the text + says a clone of the commit that records the file does.""" + copy = tmp_path / "staged-copy" / NAME + shutil.copytree(root, copy, symlinks=True) + raw(copy, env, "commit", "-q", "-m", "What is staged, committed") + where = tmp_path / "staged-clone" + where.mkdir() + clone = cloned(copy, env, family, where, recurse=False) + return raw(clone, env, "submodule", "update", "--init", "--recursive", + check=False) + + +# --- N1: the index's file beside the work tree's on disk --------------------- + +def test_a_value_the_work_tree_s_file_and_the_index_s_give_ends_staging_it( + family, holder, offline, tmp_path): + """RV1124's P1: `update = !true` in MEMBER's section, in the work + tree's file on disk and staged; HEAD's does not give it. The line + took the key out of the work tree's file and staged nothing, so a + plain `git commit` after it recorded the value in HEAD, and a plain + clone's update died on it. The text says the index's file gives it + too, and the lines end staging the file. Typed, then a plain `git + commit`: HEAD is as it was, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + (holder / GITMODULES).write_bytes(valued(data, ROW, NOT_RUN)) + raw(holder, env, "add", "--", GITMODULES) + + found = found_alike(holder, *MODES) + + assert found == [INDEX_TOO_TAKEN_OUT] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +def test_a_value_the_index_s_file_alone_gives_is_named_beside_a_clean_one( + family, holder, offline, tmp_path): + """RV1124's P1b: `update = !true` in MEMBER's section of the index's + file alone; the work tree's, on disk, and HEAD's do not give it. Git + reads the work tree's here, and `submodule status` exits 0, but a + plain `git commit` records the index's, and a plain clone of that + commit dies on it (exit 128). The gate said `family ok` in every + mode. It is `gitmodules-value-refused` now, naming the index's file + and the clone of the commit that records it, and its line stages the + work tree's file. Typed, then a plain `git commit`: HEAD is as it was, + and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + staged_only(holder, env, valued(data, ROW, NOT_RUN)) + (holder / GITMODULES).write_bytes(data) + raw(holder, env, "submodule", "status") + died = a_plain_clone_of_what_is_staged(holder, env, family, tmp_path) + assert died.returncode == 128, ("premise: a clone of the commit dies", + died.stderr) + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="in the index's file alone, which a plain " + "`git commit` records, not in the work tree or in " + "HEAD", dies=DIES_IN_THE_COMMIT) + + "Stage the work tree's .gitmodules, which does not give it:" + f"\n {STAGE}"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +# --- N2: an index's file git cannot parse ------------------------------------ + +#: What comes before the line git cannot parse in the index's file, the +#: work tree's deleted: `(the edit of HEAD's bytes, what git's own +#: `submodule status` exits, a word of what it says, the code the gate +#: finds once the line is mended)`. Before a value, git dies on the value +#: (RV1124's P9); after a section, it reads up to the line, says it, and +#: goes on. +BEFORE_THE_LINE = { + "a-value": (lambda data: valued(data, ROW, NOT_RUN), 128, + b"invalid value", "gitmodules-value-refused"), + "a-section": (lambda data: data + STALE, 0, b"bad config line", + "gitmodules-path-unnamed"), +} + +#: What the gate says of the index's file git cannot parse. +UNREADABLE_LEAD = ("REFUSED gitmodules-unreadable: the index's .gitmodules " + "could not be read as git reads it: `git config -z " + "--blob :.gitmodules --list` in ") +UNREADABLE_REMEDY = ( + "Remediation: git reads it only up to that line where the work tree " + "holds no .gitmodules, and a clone of a commit that records it dies " + "reading it, before it checks out any member. Mend .gitmodules in the " + "work tree by hand (`git config -f .gitmodules --list` says where it " + "breaks; where the work tree holds none, `git checkout -- .gitmodules` " + "puts the index's back first), stage it with `git add -- .gitmodules`, " + "then run this again.") + + +@pytest.mark.parametrize("edit, status, said, then", + BEFORE_THE_LINE.values(), ids=BEFORE_THE_LINE) +def test_an_index_s_file_git_cannot_parse_is_refused_unreadable( + family, holder, offline, tmp_path, edit, status, said, then): + """RV1124's P9: the index's file, the work tree's deleted, is HEAD's + with `update = !true` in MEMBER's section, or a stale section, and + then a line git cannot parse. `git config --blob` printed the entries + before the line and exited 128, and the gate read nothing: `family + ok` in every mode, while git's own `submodule status` died on the + value, or read the section. It is `gitmodules-unreadable` now, exit 2, + as HEAD's file is, and a clone of a commit that records the file dies + reading it, as the remediation says. Done as it says, the line taken + out by hand: the gate finds what the file gave before it, and that + finding's lines, typed, then a plain `git commit`, leave HEAD as it + was, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + staged_only(holder, env, edit(data) + UNPARSED) + git_said = raw(holder, env, "submodule", "status", check=False) + assert git_said.returncode == status, git_said.stderr + assert said in git_said.stderr, git_said.stderr + died = a_plain_clone_of_what_is_staged(holder, env, family, tmp_path) + assert died.returncode == 128, ("premise: a clone of the commit dies", + died.stderr) + + for extra in MODES: + gate = validate(holder, *extra) + assert gate.returncode == 2, (extra, gate.stdout + gate.stderr) + lead, remedy = gate.stderr.splitlines() + assert lead.startswith(UNREADABLE_LEAD), lead + assert "bad config line" in lead, lead + assert remedy == UNREADABLE_REMEDY, remedy + type_line(holder, env, re.search(r"`(git checkout [^`]*)`", + UNREADABLE_REMEDY).group(1)) + put_back = (holder / GITMODULES).read_bytes() + assert put_back.endswith(UNPARSED), put_back + (holder / GITMODULES).write_bytes(put_back[:-len(UNPARSED)]) + type_line(holder, env, re.search(r"`(git add [^`]*)`", + UNREADABLE_REMEDY).group(1)) + assert codes_of(found_alike(holder, *MODES)) == [then] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +# --- N6: an unmerged .gitmodules --------------------------------------------- + +def unmerged(root: Path, env: dict, base: bytes, ours: bytes, + theirs: bytes) -> None: + """`.gitmodules` held at a merge's conflict stages 1 to 3, `base`, + `ours` and `theirs`, and not at stage 0, as a conflicted merge leaves + it, by index records written as BYTES on stdin: first the zero-mode + record that drops the stage-0 entry, as + `tests/test_validate_family_unnamed_mounts.py`'s + `stage_unmerged_gitlink` does it.""" + def blob(data: bytes) -> str: + return raw(root, env, "hash-object", "-w", "--stdin", + stdin=data).stdout.decode("ascii").strip() + + records = (f"0 {'0' * 40}\t{GITMODULES}\n" + f"100644 {blob(base)} 1\t{GITMODULES}\n" + f"100644 {blob(ours)} 2\t{GITMODULES}\n" + f"100644 {blob(theirs)} 3\t{GITMODULES}\n") + raw(root, env, "update-index", "--index-info", stdin=records.encode()) + staged = raw(root, env, "ls-files", "-s", "--", GITMODULES).stdout + assert [line.split()[2] for line in staged.splitlines()] == [ + b"1", b"2", b"3"], staged + + +def git_reads_none(root: Path, env: dict) -> None: + """Git's `submodule status` in `root` reads no `.gitmodules` and dies + at the first gitlink (exit 128).""" + said = raw(root, env, "submodule", "status", check=False) + assert said.returncode == 128, said.stderr + assert b"no submodule mapping found" in said.stderr, said.stderr + + +#: How the work tree's file is left beside the unmerged index: deleted, or +#: on disk as the merge left it, HEAD's side, which gives the value. +LEFT = ["deleted", "on-disk"] + + +@pytest.mark.parametrize("left", LEFT) +def test_a_value_head_gives_while_a_merge_leaves_the_file_unmerged( + family, holder, offline, tmp_path, left): + """RV1124's P6: `update = !true` committed in MEMBER's section, then + `.gitmodules` at a merge's conflict stages, HEAD's side giving the + value and the other mending it, with the work tree's file deleted, or + on disk as HEAD's side. Git reads no file here, and its `submodule + status` dies at the first gitlink. The gate read HEAD's, or the work + tree's: "here and in HEAD", with a line that put HEAD's back "as the + index holds none", or "here" with no word of the merge. It names + HEAD's, which a clone reads, says git reads none here, and says to + resolve the merge without the key and stage it. Resolved by hand as + it says, the line typed, then a plain `git commit`: HEAD gives no + value, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + given = valued(data, ROW, NOT_RUN) + committed_gitmodules(holder, env, given) + unmerged(holder, env, data, given, data) + if left == "deleted": + (holder / GITMODULES).unlink() + git_reads_none(holder, env) + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="in HEAD, which a clone reads, not in the " + "file git reads here, which is none while a merge " + "leaves .gitmodules unmerged", dies=DIES_IN_A_CLONE) + + "Resolve the merge in .gitmodules by hand, leaving that key out, " + f"then stage and commit .gitmodules:\n {STAGE}"] + (holder / GITMODULES).write_bytes(data) + for line in commands(found[0]): + type_line(holder, env, line) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +def test_a_value_the_work_tree_alone_gives_while_unmerged_waits_for_git( + family, holder, offline, tmp_path): + """RV1124's P6b, the one change of finding this makes: `.gitmodules` + at a merge's conflict stages, the other side giving `update = !true` + in MEMBER's section, and the work tree's file on disk giving it, as a + merge resolved by hand to that side and not yet staged leaves it; + HEAD's does not give it. Git reads no file here, and its `submodule + status` dies at the first gitlink, not on the value. The gate read the + work tree's file and named the value "in the work tree, not in HEAD"; + it reads none while the merge is unresolved now, and every mode + passes, since no file git reads gives the value. Once the merge is + resolved with a plain `git add`, git reads the work tree's file, and + the gate names the value as N1's: its lines typed, then a plain `git + commit`, HEAD is as it was, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + given = valued(data, ROW, NOT_RUN) + unmerged(holder, env, data, data, given) + (holder / GITMODULES).write_bytes(given) + git_reads_none(holder, env) + + for extra in MODES: + gate = validate(holder, *extra) + assert gate.returncode == 0, (extra, gate.stdout + gate.stderr) + raw(holder, env, "add", "--", GITMODULES) + assert found_alike(holder, *MODES) == [INDEX_TOO_TAKEN_OUT] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +def test_no_section_is_read_while_a_merge_leaves_the_file_unmerged( + family, holder, offline, tmp_path): + """RV1124's N6, its sections: a stale section for `members/Old` + committed, then `.gitmodules` at a merge's conflict stages, HEAD's + side keeping it and the other taking it out, the work tree's deleted. + Git reads no file here. `gitmodules-path-unnamed` read HEAD's and + named the section with `git checkout HEAD -- .gitmodules` first, put + back "as the index holds none", which resolves the merge to HEAD's + side. No section is read here now, and no line resolves the merge. + Resolved by hand to HEAD's side and staged, git reads it, and the + section is named; its lines typed, then a plain `git commit`: HEAD + gives the two members alone, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + committed_gitmodules(holder, env, data + STALE) + unmerged(holder, env, data + STALE, data + STALE, data) + (holder / GITMODULES).unlink() + git_reads_none(holder, env) + + for extra in MODES: + gate = validate(holder, *extra) + assert gate.returncode == 0, (extra, gate.stdout + gate.stderr) + assert "git checkout HEAD" not in gate.stderr, gate.stderr + (holder / GITMODULES).write_bytes(data + STALE) + raw(holder, env, "add", "--", GITMODULES) + assert codes_of(found_alike(holder, *MODES)) == [ + "gitmodules-path-unnamed"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) From 4b70f2ec662045b17540a97008a6570863f4f1c8 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 12:10:22 +0000 Subject: [PATCH 04/12] test_validate_family_index_value_unmerged.py: a value HEAD's `.gitmodules` and the work tree's give, with no index's file, is pinned to end with the line that stages it, as HEAD's value, not the index's, is what adds that line (#1150) Mutant M3 of 442615eb (`if committed or staged:` cut to `if staged:` in `_value_taken_out`) survived all 170 rows of the three changed test files: in every row whose lines take out a value HEAD's file gives, the index's file gives it too, or the work tree holds no `.gitmodules`, so `staged` was true there anyway. The new row has the work tree's file and HEAD's give `update = !true`, and `git rm --cached` has taken `.gitmodules` out of the index. Under M3 the finding ends at the `--unset-all` line, and the plain `git commit` after it records `.gitmodules` gone from HEAD, and the gate then finds `member-gitmodules-absent`, exit 1. The row fails under M3, on #1124's head 8dfa32bd and on main 961640c6, and passes on 442615eb. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- ...st_validate_family_index_value_unmerged.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test_validate_family_index_value_unmerged.py b/tests/test_validate_family_index_value_unmerged.py index 36fe928d..f6ef8679 100644 --- a/tests/test_validate_family_index_value_unmerged.py +++ b/tests/test_validate_family_index_value_unmerged.py @@ -160,6 +160,36 @@ def test_a_value_the_index_s_file_alone_gives_is_named_beside_a_clean_one( ends_well(holder, env, family, tmp_path) +def test_a_value_here_and_in_head_ends_staging_it_with_no_index_s_file( + family, holder, offline, tmp_path): + """`update = !true` committed in MEMBER's section, the work tree's + file on disk giving it too, and `git rm --cached` has taken + `.gitmodules` out of the index: git reads the work tree's here, and + the index's, which a plain `git commit` records, gives nothing. HEAD's + giving the value is what makes the lines end staging the work tree's + file, not the index's: with no stage line, a plain `git commit` after + them records `.gitmodules` gone from HEAD. Typed, then a plain `git + commit`: HEAD gives no value, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + committed_gitmodules(holder, env, valued(data, ROW, NOT_RUN)) + raw(holder, env, "rm", "-q", "--cached", "--", GITMODULES) + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="here and in HEAD, which a clone reads", + dies=DIES_HERE) + + "Take the key out by hand, in a POSIX shell, then stage and " + "commit .gitmodules:\n git config -f .gitmodules --unset-all " + f"submodule.{ROW}.update\n {STAGE}"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + # --- N2: an index's file git cannot parse ------------------------------------ #: What comes before the line git cannot parse in the index's file, the From 8938d17df9b22f545a4851258057a11a9d2945d4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 21:41:02 +0000 Subject: [PATCH 05/12] tests/test_validate_family_index_gitmodules.py: #1208's row for a value HEAD gives where the deletion of `.gitmodules` is staged expects the stage line last and types no `git add` by hand, since this branch's lines end with it rv1227's M1 on #1227. Main's #1208 added the row while its finding ended at the `--unset-all` line; this branch's `_value_taken_out` ends every line set that says to stage the file with `git add -- .gitmodules`. Measured on the merge (git 2.43.0): the three printed lines typed in order, then a plain `git commit`, leave HEAD without the value, the index and the work tree equal to it and `git status` clean, and the gate passes; typed without the stage line, the commit exits 1 (`no changes added to commit`) and HEAD keeps the value. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_validate_family_index_gitmodules.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_validate_family_index_gitmodules.py b/tests/test_validate_family_index_gitmodules.py index 0c614f1d..7493c30d 100644 --- a/tests/test_validate_family_index_gitmodules.py +++ b/tests/test_validate_family_index_gitmodules.py @@ -284,8 +284,11 @@ def test_1077_a_value_head_gives_where_the_deletion_is_staged_puts_head_s_back( reads`, and its first line puts back HEAD's file, as the index holds none. A check that read the index's file alone, with no fallback to HEAD's, found nothing in it and said to commit the index's file, with - no line, over a deletion a plain `git commit` records. Typed in order, - staged and committed: HEAD's file holds no value, both members stay, + no line, over a deletion a plain `git commit` records. Its last line + stages the file, as the text says (#1150): typed without it, the + plain `git commit` after them exits 1 (`no changes added to commit`) + and HEAD keeps the value (rv1227's M1 on #1227). Typed in order, then + a plain `git commit`: HEAD's file holds no value, both members stay, nothing is left, and the gate and a plain clone pass.""" env = cloning(offline, tmp_path, family) committed_gitmodules(holder, env, valued( @@ -303,9 +306,8 @@ def test_1077_a_value_head_gives_where_the_deletion_is_staged_puts_head_s_back( "back HEAD's, as the index holds none. Take the key out by hand, in " "a POSIX shell, then stage and commit .gitmodules:\n git " "checkout HEAD -- .gitmodules\n git config -f .gitmodules " - f"--unset-all submodule.{ROW}.update"] + f"--unset-all submodule.{ROW}.update\n {STAGE}"] typed_in_order(holder, env, validate(holder, "--no-members")) - raw(holder, env, "add", "--", GITMODULES) committed_as_staged(holder, env) assert b"!true" not in head_bytes(holder, env) assert head_paths(holder, env) == {ROW, OTHER_ROW} From 0d65ea80c61b4f71f48393c3e2634581d7eb8782 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 22:03:19 +0000 Subject: [PATCH 06/12] validate-family.py: while `.gitmodules` is unmerged `gitlink-unnamed` and the init stop ask nothing, a value the work tree alone gives after a staging line is staged again, and another index value of the key is named, not "gives it too" rv1227's review of #1227 (issuecomment-6374575208), its M2, S1, N1, N2, N3, N4 and N5, each measured on git 2.43.0 on the branch merged with main e04826db4 (d95308e95) before this change: - M2. With `.gitmodules` at conflict stages, git reads no file: `git submodule status` and `update --init` exit 128 at every gitlink, and a plain clone of HEAD checks tools/x out. `_sections_as_git_reads`' none made `gitlink-unnamed` say no section gave tools/x and a plain clone stopped there, and `member-uninitialized` say to write back a section the file gave. `_check_gitlinks_elsewhere` and `_init_stop_said` ask nothing while unmerged now. The hand mount's reads keep reading none, as git's own `git rm` does there: its plain `git rm` of a clone dies (`could not lookup name for submodule`, exit 128), and `--cached`, the line it prints, exits 0. - S1. `_value_taken_out` takes `again`: after a finding whose lines end `git add -- .gitmodules`, a value the work tree's file alone gives ends with that line too, and says why (`STAGED_ABOVE`). - N4. Where the index's file gives the key another value, the finding names it (`INDEX_GIVES`, `_values_said`) in place of "gives it too". - N1, N5. `_blob_paths` and `_blob_sections` say they refuse an index's file git cannot parse; `GIVEN_WHERE`'s comment says `(False, False)`. tests/test_validate_family_index_value_unmerged.py: rows for M2 (e2d, two ids; e2e), S1 (e4o1, e4o2), N4 (e4c), N2 (e1b), N3 (a real modify/delete conflict, and the sections row's on-disk id). S2, the sparse checkout, is a Limit of #1227 and a follow-up issue. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- .../family-root/scripts/validate-family.py | 134 ++++++-- ...st_validate_family_index_value_unmerged.py | 306 +++++++++++++++++- 2 files changed, 404 insertions(+), 36 deletions(-) diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 71acafbd..18a0942d 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -109,7 +109,9 @@ of the index. One a section gives is that section's `gitmodules-path-unnamed`, which takes the gitlink out of the index first, then the section: taken out alone, the section left the - gitlink, and this gate passed it. Each `git rm --cached` comes after + gitlink, and this gate passed it. None is asked about while a merge + leaves `.gitmodules` unmerged in the index, as no section is: git + reads no file then (#1150). Each `git rm --cached` comes after `git add -- .gitmodules` where lines printed before it may leave that file with changes not staged, or it holds some, since git takes no gitlink out of the index then. Neither needs a member checked out, @@ -1982,8 +1984,9 @@ def _head_listed(root: Path) -> tuple[bool, bytes] | None: #: Where `gitmodules-value-refused` says a value is, by whether the file #: git reads here and HEAD's give it (#768); `{here}` names the file git #: reads here (`READ_HERE`), the work tree's, or where that is gone from -#: disk the index's (#1077). Neither does where the index's file alone -#: gives it, with the work tree's on disk (#1150, RV1124's N1 on #1124). +#: disk the index's (#1077). `(False, False)` is where neither gives it and +#: the index's file alone does, the work tree's being on disk (#1150, +#: RV1124's N1 on #1124). GIVEN_WHERE = {(True, True): "here and in HEAD, which a clone reads", (True, False): "in {here}, not in HEAD", (False, True): "in HEAD, which a clone reads, not in {here}", @@ -2023,6 +2026,18 @@ def _head_listed(root: Path) -> tuple[bool, bytes] | None: INDEX_TOO = ("The index's file, which a plain `git commit` records, gives it " "too.") +#: What it adds in `INDEX_TOO`'s place where the index's file gives that +#: key another value git dies on, `{values}` as `_values_said` says them +#: (rv1227's N4 on #1227). +INDEX_GIVES = ("The index's file, which a plain `git commit` records, gives " + "it {values}.") + +#: What it adds where the work tree's file alone gives the value and a line +#: an earlier finding of the check prints stages that file, the value with +#: it, so its own lines end staging the file again (rv1227's S1 on #1227). +STAGED_ABOVE = (f"A line above stages the work tree's {GITMODULES}, this " + "value with it, so the lines below end staging it again.") + #: What git takes for a `submodule..` whose value it dies on as #: it reads `.gitmodules` (#768), as `gitmodules-value-refused` says it. #: This, `_values_refused` and `_git_takes_bool` say what bootstrap's @@ -2088,6 +2103,13 @@ def _check_gitmodules_values(root: Path, members: list, ok`, exit 0. Every remedy that says to stage the file now ends with the line that stages it, `git add -- .gitmodules` (`STAGE_LINE`). + AFTER A LINE THAT STAGES THE FILE, every later finding that takes a + key out of it stages it again (`STAGED_ABOVE`, rv1227's S1 on #1227): + that `git add` stages the work tree's file as it stands, so an + `update = !true` staged for one section, then an `update = !false` + the work tree's file alone gave another, typed in order and committed + with a plain `git commit`, left `!false` in HEAD. + NO FILE WHILE A MERGE LEAVES `.gitmodules` UNMERGED (#1150, RV1124's N6 on #1124): git's submodule commands read none then, the work tree's, the index's or HEAD's (`_gitmodules_unmerged`), so none is @@ -2113,6 +2135,9 @@ def _check_gitmodules_values(root: Path, members: list, rows = {f"{DEFAULT_MEMBERS_DIR}/{row['project']}": row.get("repository") for row in members if isinstance(row, dict) and isinstance(row.get("project"), str) and row["project"]} + # Whether a finding printed above ends staging the work tree's file + # (rv1227's S1 on #1227). + staging = False for name, key in dict.fromkeys([*in_work, *in_head, *in_index]): report.wrote = True said = _value_said(name, key, (in_work, in_head, in_index), @@ -2123,10 +2148,13 @@ def _check_gitmodules_values(root: Path, members: list, (name, key) in in_head)) continue back = _written_back(name, key, here, rows) + committed = (name, key) in in_head + staged = not present or (name, key) in in_index report.finding("gitmodules-value-refused", _value_taken_out( said, f"submodule.{name}.{key}", back, - _file_put_back(root, report), committed=(name, key) in in_head, - staged=not present or (name, key) in in_index)) + _file_put_back(root, report), committed=committed, + staged=staged, again=staging)) + staging = staging or committed or staged return bool(in_work), bool(in_work or in_head) @@ -2168,25 +2196,40 @@ def _value_said(name: str, key: str, where the index's alone does, in a clone of the commit that records it (`VALUE_DIES`); and where the work tree's gives it, not HEAD's, it says that the index's gives it too where it does (`INDEX_TOO`), which is - why its lines stage it.""" + why its lines stage it, or, where the index's gives that key another + value git dies on, names that value (`INDEX_GIVES`, rv1227's N4 on + #1227): "gives it too" said of `!false` staged beside a work tree's + `!true`.""" in_work, in_head, in_index = given full = f"submodule.{name}.{key}" where = (name, key) in in_work, (name, key) in in_head values = dict.fromkeys([*in_work.get((name, key), []), *in_head.get((name, key), [])] or in_index.get((name, key), [])) - staged_too = (f" {INDEX_TOO}" if where == (True, False) - and (name, key) in in_index else "") - return (f"{GITMODULES} gives `{spelled(full)}` " + " and ".join( - "no value" if value is None else f"the value `{spelled(value)}`" - for value in values) + f" {GIVEN_WHERE[where].format(here=here)}, " - f"and git dies reading a file that does, {VALUE_DIES[where]}. Git " - f"takes {GIT_TAKES[key]} for that key.{staged_too}") + staged_too = "" + if where == (True, False) and (name, key) in in_index: + staged = dict.fromkeys(in_index[(name, key)]) + staged_too = " " + (INDEX_TOO if set(staged) == set(values) else + INDEX_GIVES.format(values=_values_said(staged))) + return (f"{GITMODULES} gives `{spelled(full)}` {_values_said(values)} " + f"{GIVEN_WHERE[where].format(here=here)}, and git dies reading " + f"a file that does, {VALUE_DIES[where]}. Git takes " + f"{GIT_TAKES[key]} for that key.{staged_too}") + + +def _values_said(values: dict[str | None, None]) -> str: + """Each of `values`, in order, as `gitmodules-value-refused` says the + values a file gives a key (`_value_said`): the value, backticked, or + `no value` for a key with none, joined by `and`.""" + return " and ".join("no value" if value is None + else f"the value `{spelled(value)}`" + for value in values) def _value_taken_out(said: str, full: str, back: str | None, put_back: tuple[str, str] = ("", ""), *, - committed: bool, staged: bool = False) -> str: + committed: bool, staged: bool = False, + again: bool = False) -> str: """`gitmodules-value-refused` whole, `said` (`_value_said`) and its commands, one to a line, on the work tree's file (#768): the file git reads put back first where the work tree has none and no finding @@ -2198,8 +2241,13 @@ def _value_taken_out(said: str, full: str, back: str | None, ENDS WITH THE LINE THAT STAGES IT, `git add -- .gitmodules` (`STAGE_LINE`, #1150, RV1124's N1 on #1124): said in words alone, a plain `git commit` after the lines typed recorded the index's file, - the value and all.""" + the value and all. So does one whose value the work tree's file alone + gives, where a finding printed above ends staging that file (`again`, + `STAGED_ABOVE`, rv1227's S1 on #1227): that line staged it as it + stood, this value with it, and a plain `git commit` after the lines + typed recorded the value in HEAD.""" key = full.rpartition(".")[2] + again = again and not (committed or staged) line, whose = put_back lines = [line] if line else [] lines.append(f"git config -f {GITMODULES} --unset-all " @@ -2207,16 +2255,16 @@ def _value_taken_out(said: str, full: str, back: str | None, if back is not None: lines.append(f"git config -f {GITMODULES} {shlex.quote(full)} " f"{back}") - if committed or staged: + if committed or staged or again: lines.append(STAGE_LINE) - return (f"{said} " + return (f"{said} " + (f"{STAGED_ABOVE} " if again else "") + (f"The work tree holds no {GITMODULES}, so the first line " f"below puts back {whose}. " if line else "") + "Take the key out" + (f" and write its {key} back" if back is not None else "") + " by hand, in a POSIX shell" + (f", then stage and commit {GITMODULES}" if committed else - f", then stage {GITMODULES}" if staged else "") + f", then stage {GITMODULES}" if staged or again else "") + ":" + "".join(f"\n {line}" for line in lines)) @@ -2954,10 +3002,21 @@ def _check_gitlinks_elsewhere(root: Path, gitlinks: dict[str, str], read from the file git reads in its place (`_given_as_git_reads`): read from the work tree, none was given, and this said "no section of .gitmodules gives" of a gitlink the index's and HEAD's gave, whose - plain clone checked it out (rv882's SHOULD 2 on #882).""" + plain clone checked it out (rv882's SHOULD 2 on #882). + + NOTHING IS ASKED WHILE A MERGE LEAVES `.gitmodules` UNMERGED in the + index (`_gitmodules_unmerged`, rv1227's M2 on #1227), as no section is + read then (`_sections_as_git_reads`): git reads no file, so its + `submodule status` and `update --init` exit 128 at every gitlink, and + which section the commit after the merge records is the merge's to + say. Read as giving no path, a gitlink HEAD's file gives, which a + plain clone of HEAD checks out (git 2.43.0, exit 0), was said to stop + a plain clone, and its `git rm --cached` then said to commit that, + which git refuses until the merge is resolved (exit 128). Resolved and + staged, the gitlink is asked about again.""" unnamed = [path for path in sorted(gitlinks) if path not in named and not _in_members(path)] - if not unnamed: + if not unnamed or _gitmodules_unmerged(root): return given = _given_as_git_reads(root) stage_first = _stage_first(root, @@ -2984,7 +3043,18 @@ def _given_as_git_reads(root: Path) -> set[str]: one git reads in its place (`_gitmodules_put_back`, the index's, then HEAD's), else none (rv882's SHOULD 2 on #882), read as `_sections_as_git_reads` reads it, so the hand mount's put-back and - `gitmodules-path-unnamed` agree on one file (#1036).""" + `gitmodules-path-unnamed` agree on one file (#1036). + + NONE WHILE A MERGE LEAVES THE FILE UNMERGED, which is what git's own + `git rm` reads then (rv1227's M2 on #1227, measured on git 2.43.0): a + plain `git rm` of a clone whose `.git` is a directory, at a path a + section gives, dies there (`could not lookup name for submodule`, exit + 128), and `git rm --cached` takes it, exit 0, so the hand mount's + `_hand_cached` says `--cached`; with the work tree's file gone, its + plain `git rm` takes the gitlink and no section, exit 0, so + `_plain_rm_said` puts nothing back, where HEAD's file put back would + resolve the merge to HEAD's side. `gitlink-unnamed`, which says what a + clone does, asks nothing then (`_check_gitlinks_elsewhere`).""" return {keys["path"] for keys in _sections_as_git_reads( root, _gitmodules_sections(root)).values() if "path" in keys} @@ -3961,16 +4031,18 @@ def _keys_back(path: str, sections: dict[str, dict[str, str]], def _blob_paths(root: Path, spec: str) -> set[str]: """Every path a section of the `.gitmodules` git names `spec` gives (`:.gitmodules`, the index's, or `HEAD:.gitmodules`), read as - `_gitmodules_sections` reads the work tree's; none where git cannot - read it (#879).""" + `_gitmodules_sections` reads the work tree's (#879): none for no + `spec`, and `gitmodules-unreadable` where git cannot parse it + (`_blob_listed`, #1150; rv1227's N1 on #1227).""" return {keys["path"] for keys in _blob_sections(root, spec).values() if "path" in keys} def _blob_sections(root: Path, spec: str) -> dict[str, dict[str, str]]: """Every `submodule` section of the `.gitmodules` git names `spec`, - `{name: {key: value}}`, read as `_blob_paths` reads its paths, and none - where git cannot read it (#879, #1036).""" + `{name: {key: value}}`, read as `_blob_paths` reads its paths (#879, + #1036): none for no `spec`, and `gitmodules-unreadable` where git + cannot parse it (`_blob_listed`, #1150; rv1227's N1 on #1227).""" return _sections_listed(_blob_listed(root, spec)) @@ -4213,6 +4285,16 @@ def _init_stop_said(root: Path, sections: dict[str, dict[str, str]], merge left unmerged) is not read here; where nothing is found the plain remedy stands. + NOTHING IS ASKED WHILE A MERGE LEAVES `.gitmodules` UNMERGED + (`_gitmodules_unmerged`, rv1227's M2 on #1227): git's init reads no + file then and dies at the first gitlink it asks about (measured, git + 2.43.0: `No url found for submodule path 'members/IRRS'`, exit 128), + whatever a section gives, and bootstrap names the cure for that, the + merge resolved and added. Asked of the sections read then, which are + none (`_sections_as_git_reads`), it named the first row's gitlink, and + its cure said to write back a section the work tree's file and HEAD's + both gave. + THE `.gitmodules` GIT READS is the work tree's (`sections`) where it is on disk, and where it is gone from disk the index's, else HEAD's (`_gitmodules_put_back`, rv820's SHOULD 2): read from the work tree @@ -4226,6 +4308,8 @@ def _init_stop_said(root: Path, sections: dict[str, dict[str, str]], THE CURE IS BOOTSTRAP'S FOR THE SAME GITLINK (`cures`), word for word but for the path, printed as this file prints one (`_init_stop_cure`). """ + if _gitmodules_unmerged(root): + return "" spec, put_back, whose = _gitmodules_put_back(root) with _git_runs("the gitlinks git submodule update asks could not be " "listed"): diff --git a/tests/test_validate_family_index_value_unmerged.py b/tests/test_validate_family_index_value_unmerged.py index f6ef8679..30d812aa 100644 --- a/tests/test_validate_family_index_value_unmerged.py +++ b/tests/test_validate_family_index_value_unmerged.py @@ -27,6 +27,24 @@ unresolved, is no finding now, as git reads no file; once the merge is resolved and staged, it is named as N1's is. +rv1227's review of this change (#1227, issuecomment-6374575208) found +more, each measured on its head b031f493 and on main e04826db before the +fix round: + +- M2. While a merge leaves `.gitmodules` unmerged, `gitlink-unnamed` read + no section, so a gitlink outside `members/` that HEAD's file gives, which + a plain clone of HEAD checks out, was said to stop a plain clone; and a + member not checked out was told to write back a section the file gave. + Neither asks anything while the merge is unresolved now. +- S1. After a finding whose lines stage the work tree's file, one whose + value the work tree's file alone gives ended at its `--unset-all`: the + `git add` above had staged that value, and a plain `git commit` after + the lines recorded it in HEAD. It ends staging the file again now. +- N2, N3, N4. An index's file git cannot parse beside a work tree's file + on disk, a real modify/delete conflict and the sections with the work + tree's file on disk while unmerged had no row; and where the index's + file gives the key another value, the finding said it "gives it too". + Every row types only the printed `git` lines, in order, and does by hand only what the text says in words, then commits what is staged with a plain `git commit`, as `tests/test_validate_family_index_gitmodules.py` @@ -44,17 +62,21 @@ from test_family import NAME, family, validate # noqa: F401 from test_family_add_refusals import holder, offline # noqa: F401 -from test_family_bootstrap_gate_order import cloned, type_line +from test_family_bootstrap_gate_order import ( + bootstrap, cloned, type_line, without_url, +) from test_family_head_lost_gitmodules import committed_gitmodules from test_family_holder_remedies_compose import ( - MODES, cloning, codes_of, ends_well, found_alike, typed_in_order, + MODES, OTHER_ROW, cloning, codes_of, ends_well, found_alike, head_paths, + typed_in_order, ) -from test_family_member_path import raw +from test_family_member_path import MEMBER, raw +from test_family_unnamed_gitlink import at_tools from test_validate_family_index_gitmodules import ( DIES_HERE, DIES_IN_A_CLONE, STALE, VALUE_SAID, committed_as_staged, head_bytes, staged_only, valued, ) -from test_validate_family_typed_states import ROW +from test_validate_family_typed_states import ROW, UNINITIALIZED, UPDATE from test_validate_family_unnamed_mounts import STAGE, commands GITMODULES = ".gitmodules" @@ -190,6 +212,99 @@ def test_a_value_here_and_in_head_ends_staging_it_with_no_index_s_file( ends_well(holder, env, family, tmp_path) +#: A second value git dies on, `update = !false`. +NOT_RUN_EITHER = b"\tupdate = !false\n" + +#: What the value's finding adds where a line printed above stages the work +#: tree's file, the value with it (rv1227's S1 on #1227). +STAGED_ABOVE = ("A line above stages the work tree's .gitmodules, this value " + "with it, so the lines below end staging it again.") + +#: What the first finding says, by how MEMBER's value was given: staged in +#: the index's file, or committed. +FIRST = { + "staged": INDEX_TOO_TAKEN_OUT, + "committed": ( + VALUE_SAID.format(where="here and in HEAD, which a clone reads", + dies=DIES_HERE) + + "Take the key out by hand, in a POSIX shell, then stage and " + "commit .gitmodules:\n git config -f .gitmodules --unset-all " + f"submodule.{ROW}.update\n {STAGE}"), +} + + +@pytest.mark.parametrize("first", FIRST) +def test_a_value_the_work_tree_alone_gives_after_a_staging_one_stages_again( + family, holder, offline, tmp_path, first): + """rv1227's S1 on #1227, its e4o1 and e4o2: `update = !true` in + MEMBER's section, staged in the index's file or committed, then + `update = !false` in OTHER's, given in the work tree's file alone. + The first finding's lines end `git add -- .gitmodules`, which stages + the work tree's file as it stands, OTHER's value with it, and the + second's ended at its `--unset-all`: typed in order, then a plain `git + commit`, HEAD gave OTHER `!false`, and the next run named it in HEAD. + The second ends staging the file again now, and says why. Typed, then + a plain `git commit`: HEAD gives neither value, and the gate and a + plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + given = valued(data, ROW, NOT_RUN) + if first == "committed": + committed_gitmodules(holder, env, given) + else: + (holder / GITMODULES).write_bytes(given) + raw(holder, env, "add", "--", GITMODULES) + (holder / GITMODULES).write_bytes(valued(given, OTHER_ROW, + NOT_RUN_EITHER)) + + found = found_alike(holder, *MODES) + + assert found == [ + FIRST[first], + "FINDING gitmodules-value-refused: .gitmodules gives " + f"`submodule.{OTHER_ROW}.update` the value `!false` in the work " + f"tree, not in HEAD, and git dies reading a file that does, " + f"{DIES_HERE}. Git takes one of `none`, `checkout`, `rebase` or " + f"`merge` for that key. {STAGED_ABOVE} Take the key out by hand, in " + "a POSIX shell, then stage .gitmodules:\n git config -f " + f".gitmodules --unset-all submodule.{OTHER_ROW}.update\n {STAGE}"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +def test_another_value_the_index_s_file_gives_the_key_is_named( + family, holder, offline, tmp_path): + """rv1227's N4 on #1227, its e4c: the index's file gives MEMBER's + section `update = !false`, the work tree's, on disk, `update = !true`, + and HEAD's neither. The finding said the index's file "gives it too", + of a value it does not give. It names the index's value now. Typed, + then a plain `git commit`: HEAD is as it was, and the gate and a plain + clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + (holder / GITMODULES).write_bytes(valued(data, ROW, NOT_RUN_EITHER)) + raw(holder, env, "add", "--", GITMODULES) + (holder / GITMODULES).write_bytes(valued(data, ROW, NOT_RUN)) + + found = found_alike(holder, *MODES) + + assert found == [ + VALUE_SAID.format(where="in the work tree, not in HEAD", + dies=DIES_HERE) + + "The index's file, which a plain `git commit` records, gives it " + "the value `!false`. Take the key out by hand, in a POSIX shell, " + "then stage .gitmodules:\n git config -f .gitmodules " + f"--unset-all submodule.{ROW}.update\n {STAGE}"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + # --- N2: an index's file git cannot parse ------------------------------------ #: What comes before the line git cannot parse in the index's file, the @@ -266,6 +381,41 @@ def test_an_index_s_file_git_cannot_parse_is_refused_unreadable( ends_well(holder, env, family, tmp_path) +def test_an_index_s_file_git_cannot_parse_beside_a_clean_one_is_refused( + family, holder, offline, tmp_path): + """rv1227's N2 on #1227, its e1b: the index's file is HEAD's and then + a line git cannot parse; the work tree's, on disk, is HEAD's. Git + reads the work tree's here, and its `submodule status` exits 0, but a + plain `git commit` records the index's, and a plain clone of that + commit dies reading it (exit 128). The gate said `family ok`. It is + `gitmodules-unreadable`, exit 2, in every mode. Done as it says, the + work tree's file, which git parses, staged with its `git add`: the + gate passes in every mode, and a plain `git commit` after it leaves + HEAD as it was.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + staged_only(holder, env, data + UNPARSED) + (holder / GITMODULES).write_bytes(data) + raw(holder, env, "submodule", "status") + died = a_plain_clone_of_what_is_staged(holder, env, family, tmp_path) + assert died.returncode == 128, ("premise: a clone of the commit dies", + died.stderr) + + for extra in MODES: + gate = validate(holder, *extra) + assert gate.returncode == 2, (extra, gate.stdout + gate.stderr) + lead, remedy = gate.stderr.splitlines() + assert lead.startswith(UNREADABLE_LEAD), lead + assert "bad config line" in lead, lead + assert remedy == UNREADABLE_REMEDY, remedy + type_line(holder, env, re.search(r"`(git add [^`]*)`", + UNREADABLE_REMEDY).group(1)) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + # --- N6: an unmerged .gitmodules --------------------------------------------- def unmerged(root: Path, env: dict, base: bytes, ours: bytes, @@ -376,15 +526,18 @@ def test_a_value_the_work_tree_alone_gives_while_unmerged_waits_for_git( ends_well(holder, env, family, tmp_path) +@pytest.mark.parametrize("left", LEFT) def test_no_section_is_read_while_a_merge_leaves_the_file_unmerged( - family, holder, offline, tmp_path): + family, holder, offline, tmp_path, left): """RV1124's N6, its sections: a stale section for `members/Old` committed, then `.gitmodules` at a merge's conflict stages, HEAD's - side keeping it and the other taking it out, the work tree's deleted. - Git reads no file here. `gitmodules-path-unnamed` read HEAD's and - named the section with `git checkout HEAD -- .gitmodules` first, put - back "as the index holds none", which resolves the merge to HEAD's - side. No section is read here now, and no line resolves the merge. + side keeping it and the other taking it out, the work tree's deleted, + or on disk as HEAD's side (rv1227's N3 on #1227, its e2f). Git reads + no file here. `gitmodules-path-unnamed` read HEAD's, or the work + tree's, and named the section, where the file was deleted with `git + checkout HEAD -- .gitmodules` first, put back "as the index holds + none", which resolves the merge to HEAD's side. No section is read + here now, and no line resolves the merge. Resolved by hand to HEAD's side and staged, git reads it, and the section is named; its lines typed, then a plain `git commit`: HEAD gives the two members alone, and the gate and a plain clone pass.""" @@ -392,7 +545,8 @@ def test_no_section_is_read_while_a_merge_leaves_the_file_unmerged( data = head_bytes(holder, env) committed_gitmodules(holder, env, data + STALE) unmerged(holder, env, data + STALE, data + STALE, data) - (holder / GITMODULES).unlink() + if left == "deleted": + (holder / GITMODULES).unlink() git_reads_none(holder, env) for extra in MODES: @@ -408,3 +562,133 @@ def test_no_section_is_read_while_a_merge_leaves_the_file_unmerged( assert head_bytes(holder, env) == data assert raw(holder, env, "status", "--porcelain").stdout == b"" ends_well(holder, env, family, tmp_path) + + +def test_no_section_is_read_in_a_modify_delete_conflict( + family, holder, offline, tmp_path): + """rv1227's N3 on #1227, its e2c, a real conflict: a stale section for + `members/Old` committed on this branch, and a branch that deletes + `.gitmodules` merged in. Git's modify/delete conflict holds the file + at stages 1 and 2 alone, not 3, and leaves the work tree's on disk as + this branch's; git reads no file here. Main read the work tree's and + named the section. No section is read here now, in every mode. + Resolved with a plain `git add`, which keeps this branch's side, the + section is named; its lines typed, then a plain `git commit`, which + records the merge: HEAD gives the two members alone, and the gate and + a plain clone pass. (A value git dies on cannot stand in for the + section here: with one in the work tree's file, `git merge` itself + dies, `invalid value`, exit 128.)""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + raw(holder, env, "switch", "-q", "-c", "deletes") + raw(holder, env, "rm", "-q", "--", GITMODULES) + raw(holder, env, "commit", "-q", "-m", ".gitmodules deleted") + raw(holder, env, "switch", "-q", "-") + committed_gitmodules(holder, env, data + STALE) + merged = raw(holder, env, "merge", "deletes", check=False) + assert merged.returncode == 1, merged.stdout + merged.stderr + staged = raw(holder, env, "ls-files", "-s", "--", GITMODULES).stdout + assert [line.split()[2] for line in staged.splitlines()] == [ + b"1", b"2"], staged + git_reads_none(holder, env) + + for extra in MODES: + gate = validate(holder, *extra) + assert gate.returncode == 0, (extra, gate.stdout + gate.stderr) + raw(holder, env, "add", "--", GITMODULES) + assert codes_of(found_alike(holder, *MODES)) == [ + "gitmodules-path-unnamed"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +# --- rv1227's M2: what a check that reads sections asks while unmerged ----- + +@pytest.mark.parametrize("left", LEFT) +def test_no_gitlink_outside_members_is_asked_about_while_unmerged( + family, holder, offline, tmp_path, left): + """rv1227's M2 on #1227, its e2d: IRSS mounted at tools/x with `git + submodule add`, its section and all, and committed; then `.gitmodules` + at a merge's conflict stages, every side HEAD's, with the work tree's + file deleted or on disk. Git reads no file here, so its `submodule + status` and `update --init` exit 128 at every gitlink, and a plain + clone of HEAD, which reads HEAD's file, checks tools/x out. + `gitlink-unnamed` read no section here, said none gave tools/x and a + plain clone stopped there, and said to commit its `git rm --cached`, + which git refuses while the merge is unresolved (exit 128). It asks + nothing while the merge is unresolved now, as no section is read: + every mode passes. Resolved with a plain `git add`, the section is + `gitmodules-path-unnamed`'s, as before: its lines typed, then a plain + `git commit`, HEAD gives the two members alone, and the gate and a + plain clone pass.""" + env = cloning(offline, tmp_path, family) + at_tools(holder, env, family) + data = head_bytes(holder, env) + where = tmp_path / "head-clone" + where.mkdir() + clone = cloned(holder, env, family, where, recurse=False) + raw(clone, env, "submodule", "update", "--init", "--recursive") + assert (clone / "tools" / "x" / ".git").exists(), "premise: checked out" + unmerged(holder, env, data, data, data) + if left == "deleted": + (holder / GITMODULES).unlink() + git_reads_none(holder, env) + + for extra in MODES: + gate = validate(holder, *extra) + assert gate.returncode == 0, (extra, gate.stdout + gate.stderr) + (holder / GITMODULES).write_bytes(data) + raw(holder, env, "add", "--", GITMODULES) + assert codes_of(found_alike(holder, *MODES)) == [ + "gitmodules-path-unnamed"] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_paths(holder, env) == {ROW, OTHER_ROW} + assert raw(holder, env, "status", "--porcelain").stdout == b"?? tools/\n" + ends_well(holder, env, family, tmp_path) + + +def test_a_member_not_checked_out_while_unmerged_is_told_the_plain_update( + family, holder, offline, tmp_path): + """rv1227's M2 on #1227, its e2e: MEMBER not checked out, and + `.gitmodules` at a merge's conflict stages, the work tree's file on + disk as a hand resolution left it, with MEMBER's url taken out. Git's + init reads no file here and dies at MEMBER's gitlink, the first, + whatever the file gives. `member-uninitialized` read the work tree's + file, or no section, and said to write MEMBER's section back and + commit it, which git refuses while the merge is unresolved. It asks + nothing while the merge is unresolved now, and says the plain update, + whose `make bootstrap` names the merge resolved and added as the + cure. Done as that says, then the update: the gate and a plain clone + pass.""" + env = cloning(offline, tmp_path, family) + raw(holder, env, "submodule", "deinit", "-f", "--", ROW) + data = head_bytes(holder, env) + theirs = without_url(data, MEMBER) + unmerged(holder, env, data, data, theirs) + (holder / GITMODULES).write_bytes(theirs) + stopped = raw(holder, env, "submodule", "update", "--init", "--recursive", + check=False) + assert stopped.returncode == 128, stopped.stderr + assert f"No url found for submodule path '{ROW}'".encode() in ( + stopped.stderr), stopped.stderr + + for extra in ((), ("--pins",)): + gate = validate(holder, *extra) + assert gate.returncode == 2, (extra, gate.stdout + gate.stderr) + assert gate.stderr.splitlines() == [ + UNINITIALIZED, f"Remediation: {UPDATE}"], gate.stderr + said = bootstrap(holder, env) + assert ("(git reads no .gitmodules while a merge leaves it unmerged). " + "Resolve the merge in .gitmodules and `git add .gitmodules`, then " + "run this again.") in said.stderr, said.stderr + (holder / GITMODULES).write_bytes(data) + raw(holder, env, "add", "--", GITMODULES) + raw(holder, env, "submodule", "update", "--init", "--recursive") + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) From 2d56c19062fca209d808f1721f0e6d9bdb521d48 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 14:35:55 +0000 Subject: [PATCH 07/12] tests/test_validate_family_index_value_unmerged.py: the stage line the S1 fix carries to every later finding is pinned for a third value and for a committed value the index lacks, and N4's row also for an index file giving the value and another rv1227c's independent review of #1227 (issuecomment-6722728204), its SHOULD S1, test-only. Two mutants of the S1 code survived all 181 rows of the PR's three files, and probes showed each puts a value git dies on into HEAD: K6 (`staging = committed or staged`, the stage line not carried past the next finding) and K8 (`staging = staging or staged`, a committed value's stage line not carried). The S1 row pinned only the finding right after a value that is both staged and committed. validate-family.py does not change. - test_every_later_value_the_work_tree_alone_gives_stages_again (new, rv1227c's p2a): `update = !true` staged for IRRS, then `update = !false` and `fetchRecurseSubmodules = sometimes` in IRSS's section, the work tree's file alone. The second and third findings each say `STAGED_ABOVE` and end with the stage line; typed in order, then a plain `git commit`, HEAD is as it was. K6 fails it. - test_a_committed_value_the_index_lacks_stages_the_later_ones_again (new, rv1227c's p8): IRRS `!true` committed, the index's file staged without it, the work tree's giving IRRS `!true` and IRSS `!false`. IRRS's finding is committed and not staged, and its `git add` stages IRSS's value all the same, so IRSS's ends staging again. K8 fails it. - test_another_value_the_index_s_file_gives_the_key_is_named gains an id (rv1227c's p3c, its K11): the index's file giving `!true` and `!false` beside the work tree's `!true` names both. The old id is `[another-value]` now, the new one `[this-value-and-another]`. - `staged_again` builds the later finding's text for the S1 row and the new rows, which the S1 row spelled out inline. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- ...st_validate_family_index_value_unmerged.py | 122 ++++++++++++++++-- 1 file changed, 109 insertions(+), 13 deletions(-) diff --git a/tests/test_validate_family_index_value_unmerged.py b/tests/test_validate_family_index_value_unmerged.py index 30d812aa..df795d2e 100644 --- a/tests/test_validate_family_index_value_unmerged.py +++ b/tests/test_validate_family_index_value_unmerged.py @@ -232,6 +232,28 @@ def test_a_value_here_and_in_head_ends_staging_it_with_no_index_s_file( f"submodule.{ROW}.update\n {STAGE}"), } +#: A third value git dies on, in another key of OTHER's section: a +#: `fetchRecurseSubmodules` that is neither a boolean nor `on-demand`. +NOT_A_BOOLEAN = b"\tfetchRecurseSubmodules = sometimes\n" + +#: What git takes for each key a value is given for, as the finding says it. +UPDATE_TAKES = "one of `none`, `checkout`, `rebase` or `merge`" +FETCH_TAKES = "a boolean or `on-demand`" + + +def staged_again(key: str, value: str, takes: str) -> str: + """What `gitmodules-value-refused` says of the value `value` that the + work tree's file alone gives OTHER's `key` (as git spells it) when a + line printed above stages the file: `STAGED_ABOVE`, and the stage line + last. `takes` is what git takes for that key.""" + full = f"submodule.{OTHER_ROW}.{key}" + return ("FINDING gitmodules-value-refused: .gitmodules gives " + f"`{full}` the value `{value}` in the work tree, not in HEAD, " + f"and git dies reading a file that does, {DIES_HERE}. Git " + f"takes {takes} for that key. {STAGED_ABOVE} Take the key out " + "by hand, in a POSIX shell, then stage .gitmodules:\n git " + f"config -f .gitmodules --unset-all {full}\n {STAGE}") + @pytest.mark.parametrize("first", FIRST) def test_a_value_the_work_tree_alone_gives_after_a_staging_one_stages_again( @@ -259,15 +281,42 @@ def test_a_value_the_work_tree_alone_gives_after_a_staging_one_stages_again( found = found_alike(holder, *MODES) + assert found == [FIRST[first], + staged_again("update", "!false", UPDATE_TAKES)] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +def test_every_later_value_the_work_tree_alone_gives_stages_again( + family, holder, offline, tmp_path): + """rv1227c's S1 on #1227, its p2a, with three values: `update = !true` + in MEMBER's section staged, then `update = !false` and + `fetchRecurseSubmodules = sometimes` in OTHER's, given in the work + tree's file alone. The first finding's `git add -- .gitmodules` stages + both of OTHER's values, so each later finding ends staging the file + again, not only the next one: a script that carried the stage line one + finding on (rv1227c's K6) left the third at its `--unset-all`, and + typed in order, then a plain `git commit`, HEAD gave OTHER + `fetchRecurseSubmodules = sometimes`, which the next run named in HEAD. + Typed, then a plain `git commit`: HEAD gives no value, and the gate and + a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + given = valued(data, ROW, NOT_RUN) + (holder / GITMODULES).write_bytes(given) + raw(holder, env, "add", "--", GITMODULES) + (holder / GITMODULES).write_bytes(valued( + given, OTHER_ROW, NOT_RUN_EITHER + NOT_A_BOOLEAN)) + + found = found_alike(holder, *MODES) + assert found == [ - FIRST[first], - "FINDING gitmodules-value-refused: .gitmodules gives " - f"`submodule.{OTHER_ROW}.update` the value `!false` in the work " - f"tree, not in HEAD, and git dies reading a file that does, " - f"{DIES_HERE}. Git takes one of `none`, `checkout`, `rebase` or " - f"`merge` for that key. {STAGED_ABOVE} Take the key out by hand, in " - "a POSIX shell, then stage .gitmodules:\n git config -f " - f".gitmodules --unset-all submodule.{OTHER_ROW}.update\n {STAGE}"] + FIRST["staged"], + staged_again("update", "!false", UPDATE_TAKES), + staged_again("fetchrecursesubmodules", "sometimes", FETCH_TAKES)] typed_in_order(holder, env, validate(holder, "--no-members")) committed_as_staged(holder, env) assert head_bytes(holder, env) == data @@ -275,17 +324,64 @@ def test_a_value_the_work_tree_alone_gives_after_a_staging_one_stages_again( ends_well(holder, env, family, tmp_path) -def test_another_value_the_index_s_file_gives_the_key_is_named( +def test_a_committed_value_the_index_lacks_stages_the_later_ones_again( family, holder, offline, tmp_path): + """rv1227c's S1 on #1227, its p8: `update = !true` in MEMBER's section + committed, the index's file then staged without it, and the work tree's + file giving MEMBER `!true` again and OTHER `update = !false`. MEMBER's + finding is committed and not staged, and its `git add -- .gitmodules` + stages OTHER's value all the same, so OTHER's ends staging the file + again: a script that carried the stage line only past a finding the + index's file gives (rv1227c's K8) left it at its `--unset-all`, and + typed in order, then a plain `git commit`, HEAD gave OTHER `!false`. + Typed, then a plain `git commit`: HEAD is as it was before the value + was committed, and the gate and a plain clone pass.""" + env = cloning(offline, tmp_path, family) + data = head_bytes(holder, env) + given = valued(data, ROW, NOT_RUN) + committed_gitmodules(holder, env, given) + (holder / GITMODULES).write_bytes(data) + raw(holder, env, "add", "--", GITMODULES) + (holder / GITMODULES).write_bytes(valued(given, OTHER_ROW, + NOT_RUN_EITHER)) + + found = found_alike(holder, *MODES) + + assert found == [FIRST["committed"], + staged_again("update", "!false", UPDATE_TAKES)] + typed_in_order(holder, env, validate(holder, "--no-members")) + committed_as_staged(holder, env) + assert head_bytes(holder, env) == data + assert raw(holder, env, "status", "--porcelain").stdout == b"" + ends_well(holder, env, family, tmp_path) + + +#: What the index's file gives MEMBER's `update` beside a work tree's file +#: that gives `!true`, and how the finding names it: another value, or this +#: one and another (rv1227c's p3c, its K11). +INDEX_GIVES_THE_KEY = { + "another-value": (NOT_RUN_EITHER, "the value `!false`"), + "this-value-and-another": ( + NOT_RUN + NOT_RUN_EITHER, "the value `!true` and the value `!false`"), +} + + +@pytest.mark.parametrize("in_index, said", INDEX_GIVES_THE_KEY.values(), + ids=INDEX_GIVES_THE_KEY) +def test_another_value_the_index_s_file_gives_the_key_is_named( + family, holder, offline, tmp_path, in_index, said): """rv1227's N4 on #1227, its e4c: the index's file gives MEMBER's section `update = !false`, the work tree's, on disk, `update = !true`, and HEAD's neither. The finding said the index's file "gives it too", of a value it does not give. It names the index's value now. Typed, then a plain `git commit`: HEAD is as it was, and the gate and a plain - clone pass.""" + clone pass. rv1227c's p3c, the same row with the index's file giving + `!true` and `!false`: a script that said "gives it too" wherever the + index's file gives the work tree's value and another (rv1227c's K11) + left the other value unnamed; it names both.""" env = cloning(offline, tmp_path, family) data = head_bytes(holder, env) - (holder / GITMODULES).write_bytes(valued(data, ROW, NOT_RUN_EITHER)) + (holder / GITMODULES).write_bytes(valued(data, ROW, in_index)) raw(holder, env, "add", "--", GITMODULES) (holder / GITMODULES).write_bytes(valued(data, ROW, NOT_RUN)) @@ -295,8 +391,8 @@ def test_another_value_the_index_s_file_gives_the_key_is_named( VALUE_SAID.format(where="in the work tree, not in HEAD", dies=DIES_HERE) + "The index's file, which a plain `git commit` records, gives it " - "the value `!false`. Take the key out by hand, in a POSIX shell, " - "then stage .gitmodules:\n git config -f .gitmodules " + f"{said}. Take the key out by hand, in a POSIX shell, then stage " + ".gitmodules:\n git config -f .gitmodules " f"--unset-all submodule.{ROW}.update\n {STAGE}"] typed_in_order(holder, env, validate(holder, "--no-members")) committed_as_staged(holder, env) From 0ab49ee200a5bb2067dcabef6144a90e7cb28269 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 19:23:00 +0000 Subject: [PATCH 08/12] validate-family.py: a member row's `id:` that is no project id is `member-id-malformed` in every mode, and so is a mounted member's own `project.yaml` `id:`, with the `bump` that writes the row from that file (#1204) `PROJECT_ID_RE` was asked of the family's own id alone. A row's `id:` met the mounted `project.yaml`, compared as text, and `_check_member_ids_unique`, and neither is a pattern: under `--no-members` a row id of "irrs\n", "irrs\r", "IRRS Capital" or "-x" was `family ok`, and the holder a pre-#1166 `add` wrote for a member whose own id read "irqs\n" was `family ok` in every mode. `_check_member_ids_well_formed` asks `_is_project_id(str(id))` of every row `_check_member_ids_unique` asks, from `_check_members` before the `--no-members` return. `_check_member_identity` asks the member's own id as `family.py add` asks it, `str(id or "")`, and says no `member-id-mismatch` where either id is malformed. Both remedies are a `bump`: to the commit the row pins, or to the commit that mends the member's own file. AGENTS.md names the code; `ID_READERS` counts the new reader. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- .../family-root/scripts/validate-family.py | 142 +++++- .../test_validate_family_member_id_pattern.py | 415 ++++++++++++++++++ tests/test_value_pattern_line_break.py | 4 +- 4 files changed, 557 insertions(+), 6 deletions(-) create mode 100644 tests/test_validate_family_member_id_pattern.py diff --git a/AGENTS.md b/AGENTS.md index d02c684d..1021e389 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -401,7 +401,7 @@ python3 scripts/family.py siblings --family-root `family.yaml`'s `members:` block, which the tool rewrites wholesale, but for a row repair `validate-family.py` names; the commit is then the person's. 4. **The member must be scaffolded or adopted FIRST.** A family cannot make a - project out of a repository; it can only pin one that already is. + project out of a repository; it can only pin one that already is. A row whose `id:` is no project id is `member-id-malformed`. 5. **`make siblings` in the holder — or `family.py siblings --family-root `, the same file — places the WORKSTATION layout**: each member cloned BESIDE the holder, in the family folder, on its tracking branch, diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 4683c3cf..c9a37c41 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -29,6 +29,12 @@ alone, so `--no-members` asks it too. - that every entry of `members:` is a mapping, the `member-row` finding (#864). It reads `family.yaml` alone, so `--no-members` asks it too. + - that every row's `id:` is a project id, `PROJECT_ID_RE` asked of the + whole of it, the `member-id-malformed` finding (#1204): an id that ends + in a line feed or holds a space names no project. It reads + `family.yaml` alone, so `--no-members` asks it too; with the members + checked, each member's own `project.yaml` `id:` is asked as `family.py + add` asks it. - EVERY MEMBER, three ways: 1. the GITLINK this repository records at `members/` equals `members[].pin.commit` — the same lockstep rule an assembly root @@ -1556,12 +1562,12 @@ def _check_member_checkout(root: Path, asked: Gitlinked, report: Report, return _check_member_tree(member, asked.pin, asked.path, asked.commit, asked.project, report, asked.rule, stop) - _check_member_identity(member, asked.row, asked.project, asked.path, + _check_member_identity(root, member, asked.row, asked.project, asked.path, report) -def _check_member_identity(member: Path, row: dict, project: str, path: str, - report: Report) -> None: +def _check_member_identity(root: Path, member: Path, row: dict, project: str, + path: str, report: Report) -> None: """The mounted tree is the PROJECT this row claims, not just a commit. A member is an assembly root, so it carries `project.yaml`, and that file @@ -1579,6 +1585,17 @@ def _check_member_identity(member: Path, row: dict, project: str, path: str, (`load_yaml`'s `yaml-unreadable` line), and gave no cure. `path` is the row's path as `_check_member_repository_and_path` resolved it, so a row that gives none is named at `members/

`, not `None` (r956's NIT 1). + + THE MEMBER'S OWN `id:` IS ASKED AS `family.py add` ASKS IT (#1204), + `PROJECT_ID_RE` of the whole of `str(id or "")` (`_read_member_project`'s + `member-bad-id`), so the gate refuses, members checked, every mounted + member `add` would refuse: `member-id-malformed`, naming the file, as + the source, and the `bump` that pins it once it is mended. A row that + spelled the same malformed text was `ok`, since the two compared equal. + Where either id is malformed, `member-id-mismatch` is not said as well: + the row's own finding, made in every mode, or this one, is the fault, + and a mismatch would send the person to copy a malformed id. `root` is + the holder each remedy names. """ manifest_path = member / "project.yaml" named = f"{path}/project.yaml" @@ -1610,11 +1627,32 @@ def _check_member_identity(member: Path, row: dict, project: str, path: str, return recorded_id = row.get("id") actual_id = manifest.get("id") + source_bad = not _is_project_id(str(actual_id or "")) + if source_bad: + word = str(recorded_id or project) + report.finding( + "member-id-malformed", + f"{named}: the member's own `id:` is {actual_id!r}, which does " + f"not match {PROJECT_ID_RE.pattern}, so the tree mounted at " + f"{path} is no project, and `family.py add` and `bump` refuse " + "it as `member-bad-id`. That file is the source every row pins, " + "so it is mended in the member's own repository, whose " + "validate-manifest.py refuses it as `manifest-id`; then, from a " + "checkout of openRepoShape and on a branch other than the " + "holder's tracking branch, which `bump` refuses " + f"(`{_switch_line(root, 'bump', word)}` makes one), " + f"`{_bump_to(root, project, None)}` pins the mended commit and " + f"writes the row's `id:` from it. {BRANCH_LEFT}") if recorded_id is None: report.finding( "member-id-absent", f"{project}: the row records no `id:`, so nothing checks that the " "tree mounted here is the project this row claims") + elif source_bad or not _is_project_id(str(recorded_id)): + # ONE FINDING FOR ONE FAULT (#1204): the member's own id was named + # just above, or the row's by `_check_member_ids_well_formed`, in + # every mode, with the `bump` that writes the row from the source. + return elif str(recorded_id) != str(actual_id): report.finding( "member-id-mismatch", @@ -1881,6 +1919,100 @@ def _check_member_ids_unique(root: Path, members: list, f"name {first} there instead to keep {project}. {BRANCH_LEFT}") +def _is_project_id(text: str) -> bool: + """Whether `text` is a project id: `PROJECT_ID_RE` asked of the WHOLE + text (#1204). + + `fullmatch`, NOT `match` (#1139): the pattern ends in `$`, which also + matches just before a trailing line feed, so `match` takes `irrs\\n`. + The pattern is the holder's own `repo_shape.py`'s, whose text each + finding prints, so it is asked whole, not respelled. ONE reader for + the two places a member's id is asked, its row's `id:` + (`_check_member_ids_well_formed`) and its mounted `project.yaml`'s + (`_check_member_identity`), so the two cannot drift into two + questions.""" + return PROJECT_ID_RE.fullmatch(text) is not None + + +def _bump_to(root: Path, project: str, commit: str | None) -> str: + """`family.py bump` of `project` in this holder, `--to commit`, or `--to + ` where no commit can be named, as THE LOCKSTEP RULE says it: + POSIX-quoted, as every line this file prints is (#1204).""" + to = "" if commit is None else shlex.quote(commit) + return (f"{PYTHON} scripts/family.py bump --family-root " + f"{shlex.quote(str(root))} --member {shlex.quote(project)} " + f"--to {to}") + + +def _check_member_ids_well_formed(root: Path, members: list, + report: Report) -> None: + """Every row's `id:` is a project id: the `member-id-malformed` finding + (#1204). + + Nothing asked it. A row's id met two checks, the mounted `project.yaml` + compared with it as text and `_check_member_ids_unique`, and neither is + a pattern, so under `--no-members` an id that ended in a line feed, held + a space or opened with `-` was `family ok`, and with the members checked + so was one the mounted `project.yaml` spelled the same: the row a + `family.py add` older than #1166 wrote for a member whose own `id:` read + `"irqs\\n"`. It reads `family.yaml` alone, so it runs under + `--no-members` too, as `--pins` and a full run do. + + THE ROWS IT ASKS ARE `_check_member_ids_unique`'s: a row that is no + mapping (`member-row`), has no `project:` (`member-project`) or spells + an earlier row's `project:` exactly (`member-duplicate`) is skipped, so + every row `_check_member_identity` reaches has been asked here. A row + with no `id:`, or `id: null`, is `member-id-absent`'s, as before. Any + other `id:` is asked as its text, `str()`, the text the identity check + compares: a number reads as its digits, so a row a holder written before + #259 keeps as `id: 2026` is an id, as `family.py`'s `_member_id` says + it passes; `true`, `1.5`, `-3`, a list and a mapping read as text no + id is, and are refused, each named as the reader read it. + + THE REMEDY IS `family.py bump` TO THE COMMIT THE ROW PINS, runnable as + printed: `bump` reads the member's own `project.yaml` at that commit, + asks its `id:` as `add` does (`member-bad-id`) and writes the row from + it, and the pin does not move. Not `remove` and then `add`, which the + issue suggested: in a holder with the member checked out, `remove` + leaves the member's object store under `.git/modules/` on purpose, and + `add` then refuses `member-git-dir-cached`, so the two as printed did + not run (measured for #1204); and `add` would clone the member again + and move the pin to its tip without an `--at`. `bump` needs the member + checked out, and its own `member-uninitialized` names the line that + checks it out. A pin that is no 40-hex commit is named `--to `, + with `member-pin-commit` beside it where members are checked. + """ + projects: set[str] = set() + for row in members: + if not isinstance(row, dict): + continue + project = row.get("project") + if not isinstance(project, str) or not project or project in projects: + continue + projects.add(project) + member_id = row.get("id") + if member_id is None or _is_project_id(str(member_id)): + continue + pin = row.get("pin") + commit = pin.get("commit") if isinstance(pin, dict) else None + if not isinstance(commit, str) or not COMMIT_RE.fullmatch(commit): + commit = None + report.finding( + "member-id-malformed", + f"{project}: the row records id {member_id!r}, which does not " + f"match {PROJECT_ID_RE.pattern}, so it names no project. The " + "member's own project.yaml `id:` is the source this row pins. " + "From a checkout of openRepoShape, with the member checked out, " + "and on a branch other than the holder's tracking branch, which " + "`bump` refuses " + f"(`{_switch_line(root, 'bump', str(member_id))}` makes one), " + f"`{_bump_to(root, project, commit)}` writes the row's `id:` " + "from that file at the commit the row pins, and the pin stays " + "where it is. Where that file's `id:` is no project id either, " + "`bump` refuses it as `member-bad-id`: mend it in the member's " + f"own repository first, and give `--to` that commit. {BRANCH_LEFT}") + + def _gitmodules_sections(root: Path) -> dict[str, dict[str, str]]: """Every `submodule` section of `.gitmodules`, `{name: {key: value}}`, each key in the order git lists it: the file read ONCE for the two @@ -4405,7 +4537,8 @@ def _check_members(root: Path, members: list, no_members: bool, Split from `main` for #137. Every check that needs no member checked 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 + file alone, `family.yaml`'s names (#331), ids (#206) and each id's + form, `member-id-malformed` (#1204), and the paths `.gitmodules` gives twice (#300), then the gitlinks and sections no row names, which read the index and HEAD as well (#361), the gitlinks no section of HEAD's `.gitmodules` gives (#611) or gives a url, and a @@ -4450,6 +4583,7 @@ def _check_members(root: Path, members: list, no_members: bool, """ _check_member_names_unique(members, report) _check_member_ids_unique(root, members, report) + _check_member_ids_well_formed(root, members, report) # A ROW THAT IS NOT A MAPPING reads `family.yaml` alone too, so it is # found here, in both modes (#864): found only in the loop below, past # the `--no-members` return, it was counted as a member there, and the diff --git a/tests/test_validate_family_member_id_pattern.py b/tests/test_validate_family_member_id_pattern.py new file mode 100644 index 00000000..07fc7f9b --- /dev/null +++ b/tests/test_validate_family_member_id_pattern.py @@ -0,0 +1,415 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-family.py` asks whether each member row's `id:` is a project id, +and with the members checked each member's own `project.yaml` `id:` too +(#1204, the follow-up of #1166's `member-bad-id`). + +THE DEFECT, measured on #1227's head merged onto main `81eaf757` +(`ccc2c2d2`, Python 3.12.3, git 2.43.0). `PROJECT_ID_RE` was asked of one +id only, the family's own. A row's `id:` met two checks and neither is a +pattern: the mounted `project.yaml` compared with it as text +(`member-id-mismatch`), and `_check_member_ids_unique`. So: + +- under `--no-members`, the run a private family's CI falls back to, a row + `id:` of `"irrs\\n"`, `"irrs\\r"`, `"IRRS Capital"` or `"-x"` was `family + ok`, exit 0; with the members checked each was `member-id-mismatch`, of a + row that only had to copy the member's id; +- the holder a `family.py add` older than #1166 wrote for a member whose own + `project.yaml` reads `id: "irqs\\n"`, its row recording the same, was + `family ok` in every mode: the two compared equal. + +THE RULE NOW. `member-id-malformed`, a finding of its own: of a row's `id:` +in every mode, since it reads `family.yaml` alone, and of the mounted +`project.yaml`'s `id:`, asked exactly as `family.py add` asks it, so the gate +refuses the members `add` refuses (`member-bad-id`) and none it would pin. +Where either id is malformed, `member-id-mismatch` is not said as well. Each +remedy is a `bump`, typed as printed below: to the commit the row pins, which +writes the row from the member's own `project.yaml` and leaves the pin, or to +the commit that mends that file. A number reads as its digits, as the +identity check always compared it, so a row a holder written before #259 +keeps as `id: 2026` passes; `null` and no `id:` are `member-id-absent`'s, as +before. + +The family is `tests/test_family.py`'s offline one: members scaffolded into +local bare repositories, no network and no GitHub, a `gh` stub on PATH. +""" + +from __future__ import annotations + +import os +import re +import shlex + +import pytest + +from conftest import REPO, git, run_script +from repo_shape import PYTHON, double_quoted, tree_digest +#: The family fixture, its names and the helpers that own a hand edit and a +#: member whose own `id:` is written by hand, imported from the modules that +#: own them rather than rebuilt. +from test_family import ( # noqa: F401 + ORG, SPARE, family, family_module, holder, manifest, validate, +) +from test_family_tracking_branch import on_main # noqa: F401 +from test_validate_family_branch_step import BRANCH_LEFT +from test_value_pattern_line_break import ( + ID_PATTERN, a_member_whose_id_is, findings, rewritten, +) + +LF = chr(10) +CR = chr(13) + +#: IRRS's row `id:` line, as the fixture's `add` wrote it. +IRRS_ID = " id: irrs\n" + +#: Table 3's four values that the pattern refuses, as the reader reads them. +REFUSED = [ + pytest.param("irrs" + LF, id="line-feed"), + pytest.param("irrs" + CR, id="carriage-return"), + pytest.param("IRRS Capital", id="space"), + pytest.param("-x", id="leading-dash"), +] + + +def with_row_id(holder, spelled: str) -> None: + """`family.yaml` with IRRS's `id:` written as `spelled`, as a hand edit + writes it.""" + rewritten(holder / "family.yaml", IRRS_ID, f" id: {spelled}\n") + + +def row_finding(project: str, value) -> str: + """How a row's finding starts, for `value` as the reader read it.""" + return (f"FINDING member-id-malformed: {project}: the row records id " + f"{value!r}, which does not match {ID_PATTERN}, so it names no " + "project.") + + +def source_finding(project: str, value) -> str: + """How a mounted member's own finding starts.""" + return (f"FINDING member-id-malformed: members/{project}/project.yaml: " + f"the member's own `id:` is {value!r}, which does not match " + f"{ID_PATTERN}, so the tree mounted at members/{project} is no " + "project, and `family.py add` and `bump` refuse it as " + "`member-bad-id`.") + + +def commands(line: str) -> list[str]: + """The `git -C` switch and the `family.py` line a finding names, in + order.""" + return [command for command in re.findall(r"`([^`]*)`", line) + if command.startswith(("git -C", PYTHON))] + + +def row_of(holder, project: str) -> dict: + return next(row for row in manifest(holder)["members"] + if row["project"] == project) + + +def mounted_as_add_wrote_it(family, holder, tmp_path, row_id: str) -> None: + """IRQS mounted from a copy of its remote whose `project.yaml` reads `id: + "irqs\\n"` (`a_member_whose_id_is`, which leaves its work clone at + `tmp_path / "edit"`), and its row written as a `family.py add` older + than #1166 wrote it, with its `id:` spelled `row_id` (#1166 refuses that + member now, so the row is written by hand). Committed, the pin and the + digest those of the mount: the id is the only thing wrong.""" + remotes = a_member_whose_id_is(family, tmp_path, 'id: "irqs\\n"\n') + git("-c", "protocol.file.allow=always", "submodule", "add", "-q", "--", + str(remotes / f"{SPARE}.git"), f"members/{SPARE}", cwd=holder) + mounted = holder / "members" / SPARE + commit = git("rev-parse", "HEAD", cwd=mounted).stdout.strip() + text = (holder / "family.yaml").read_text(encoding="utf-8") + (holder / "family.yaml").write_bytes((text.rstrip("\n") + "\n" + "\n".join([ + f" - project: {SPARE}", + f" id: {row_id}", + f" repository: {ORG}/{SPARE}", + f" path: members/{SPARE}", + " pin:", + " revision_kind: commit", + f' commit: "{commit}"', + " digest_algorithm: sha256", + " digest_definition: sorted-ls-tree-r-v1", + f' tree_sha256: "{tree_digest(mounted, commit)}"', + ]) + "\n").encode("utf-8")) + git("add", "--", ".gitmodules", f"members/{SPARE}", "family.yaml", + cwd=holder) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Mount IRQS as add mounted it before #1166", cwd=holder) + + +# --- a row's `id:`, under --no-members --------------------------------------- + +@pytest.mark.parametrize("value", REFUSED) +def test_a_row_id_the_pattern_refuses_is_found_under_no_members(holder, + value): + """THE ISSUE'S ROW, table 3: each was `family ok`, exit 0. It reads + `family.yaml` alone, so the run that skips the members finds it, and + it is the one finding.""" + with_row_id(holder, double_quoted(value)) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", value) + " "), found[0] + assert "member(s) SKIPPED (--no-members)" in result.stdout + + +@pytest.mark.parametrize("spelled", ['"0"', "irrs"], ids=["zero", "irrs"]) +def test_a_well_formed_row_id_is_family_ok_under_no_members(holder, spelled): + """Table 3's controls: `0` is an id the pattern takes, and `irrs` is the + fixture's own. Each is `family ok`, before and after.""" + with_row_id(holder, spelled) + result = validate(holder, "--no-members") + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.splitlines()[-1].startswith("family ok: "), \ + result.stdout + + +@pytest.mark.parametrize("extra", [(), ("--no-members",), ("--pins",)], + ids=["members", "no-members", "pins"]) +def test_the_fixture_is_family_ok_in_every_mode(holder, extra): + """The well-formed fixture, untouched: no new finding in any mode.""" + result = validate(holder, *extra) + assert result.returncode == 0, result.stdout + result.stderr + assert "member-id-malformed" not in result.stderr + + +# --- a row's `id:`, with the members checked -------------------------------- + +@pytest.mark.parametrize("value", REFUSED) +def test_with_members_checked_it_is_the_one_finding_not_a_mismatch(holder, + value): + """With the members checked each value was `member-id-mismatch`, which + tells a person to copy the member's id and names no command. It is the + row's own finding now, in `--pins` too, and only that: one finding for + one fault.""" + with_row_id(holder, double_quoted(value)) + for extra in ((), ("--pins",)): + result = validate(holder, *extra) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", value)), found[0] + + +def test_a_well_formed_row_id_that_is_not_the_members_is_still_a_mismatch( + holder): + """The control of the one above: `0` is an id, so `member-id-mismatch` + stays its finding, word for word.""" + with_row_id(holder, '"0"') + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-id-mismatch: members/IRRS: project.yaml declares id " + "'irrs' but family.yaml records '0' for IRRS. The project's own " + "manifest is the source; this row is a pin of it."], result.stderr + + +# --- a row `id:` that is not a string --------------------------------------- + +@pytest.mark.parametrize("spelled", ["true", "1.5", "-3", "[irrs]", + "{irrs: 1}"], + ids=["bool", "float", "negative", "list", "mapping"]) +def test_a_row_id_whose_text_is_no_id_is_found(holder, spelled): + """THE DECISION FOR A VALUE THAT IS NOT A STRING: it is asked as its + text, `str()`, the text the identity check compares, and none of these + reads as an id. Named as the reader read it.""" + with_row_id(holder, spelled) + value = row_of(holder, "IRRS")["id"] + assert not isinstance(value, str), value + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", value)), found[0] + + +def test_a_row_id_that_reads_as_a_number_is_its_digits(holder): + """A holder written before #259 keeps `id: 2026` for a member scaffolded + with `--id 2026`, which the reader reads as a number; `family.py`'s + `_member_id` says the validator passes it, and it still does.""" + with_row_id(holder, "2026") + assert row_of(holder, "IRRS")["id"] == 2026 + result = validate(holder, "--no-members") + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize("spelled", ["null", None], ids=["null", "no-id"]) +def test_a_row_with_no_id_is_still_member_id_absent_alone(holder, spelled): + """`id: null` reads as no `id:`, and a row with none is + `member-id-absent`'s where it was, members checked, and nothing under + `--no-members`, as before.""" + if spelled is None: + rewritten(holder / "family.yaml", IRRS_ID, "") + else: + with_row_id(holder, spelled) + assert validate(holder, "--no-members").returncode == 0 + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-id-absent: IRRS: the row records no `id:`, so nothing " + "checks that the tree mounted here is the project this row claims"], \ + result.stderr + + +# --- a mounted member's own `id:` -------------------------------------------- + +def test_the_holder_add_wrote_before_1166_is_refused_in_every_mode( + family, holder, tmp_path): + """THE ISSUE'S HOLDER. Its row and its member's own `project.yaml` both + read `id: "irqs\\n"`, so the two compared equal and every mode was + `family ok`, exit 0. The row is found in every mode; with the members + checked, the member's own file too, named as the source.""" + mounted_as_add_wrote_it(family, holder, tmp_path, + double_quoted("irqs" + LF)) + alone = validate(holder, "--no-members") + assert alone.returncode == 1, alone.stdout + alone.stderr + found = findings(alone.stderr) + assert len(found) == 1, alone.stderr + assert found[0].startswith(row_finding(SPARE, "irqs" + LF)), found[0] + for extra in ((), ("--pins",)): + result = validate(holder, *extra) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 2, result.stderr + assert found[0].startswith(row_finding(SPARE, "irqs" + LF)), found[0] + assert found[1].startswith(source_finding(SPARE, "irqs" + LF)), \ + found[1] + assert "member-id-mismatch" not in result.stderr + + +@pytest.mark.parametrize("row_id", [double_quoted("irqs" + LF), "irqs"], + ids=["equal-to-its-row", "row-well-formed"]) +def test_a_mounted_member_whose_own_id_is_malformed_is_found( + family, holder, tmp_path, row_id): + """A mounted member whose own `id:` is malformed is found whatever its + row says: equal to it (both refused, no mismatch), or a well-formed + `irqs`, which was `member-id-mismatch`, a finding that told the person + to copy the malformed id into the row. Its text names the file as the + source and the `bump` that pins it once mended.""" + mounted_as_add_wrote_it(family, holder, tmp_path, row_id) + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + own = [line for line in found if line.startswith( + "FINDING member-id-malformed: members/")] + assert len(own) == 1, result.stderr + assert own[0].startswith(source_finding(SPARE, "irqs" + LF) + " "), own[0] + assert "That file is the source every row pins" in own[0] + argv = shlex.split(commands(own[0])[1]) + assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], argv + assert os.path.samefile(argv[argv.index("--family-root") + 1], holder) + assert argv[argv.index("--member") + 1:] == [SPARE, "--to", ""] + assert own[0].endswith(". " + BRANCH_LEFT), own[0] + assert "member-id-mismatch" not in result.stderr + assert len(found) == (2 if row_id != "irqs" else 1), result.stderr + + +# --- the remedies, typed as printed ------------------------------------------ + +def test_the_rows_remedy_runs_as_printed_from_the_tracking_branch(on_main): + """A row `id:` written by hand and committed, on the tracking branch, + where a person stands after `init`. The finding's two commands, typed as + printed from a checkout of openRepoShape: the switch makes a branch, + and the `bump` to the commit the row pins writes the row's `id:` from + the member's own `project.yaml`, the pin where it was. The gate is + green after it, in every mode.""" + holder = on_main + with_row_id(holder, double_quoted("irrs" + LF)) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", + "A row id written by hand", cwd=holder) + pinned = row_of(holder, "IRRS")["pin"]["commit"] + found = validate(holder, "--no-members") + assert found.returncode == 1, found.stdout + found.stderr + line = found.stderr.split("FINDING member-id-malformed: ", + 1)[1].splitlines()[0] + assert line.endswith(". " + BRANCH_LEFT), line + switch, bump = commands(line) + argv = shlex.split(bump) + assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], bump + assert argv[argv.index("--member") + 1] == "IRRS", bump + assert argv[argv.index("--to") + 1] == pinned, bump + assert os.path.samefile(argv[argv.index("--family-root") + 1], holder) + git(*shlex.split(switch)[1:], cwd=holder.parent) + done = run_script(REPO / argv[1], *argv[2:], cwd=REPO) + assert done.returncode == 0, done.stdout + done.stderr + row = row_of(holder, "IRRS") + assert (row["id"], row["pin"]["commit"]) == ("irrs", pinned) + assert git("status", "--porcelain", cwd=holder).stdout == "" + for extra in ((), ("--no-members",)): + assert validate(holder, *extra).returncode == 0 + + +def test_the_members_remedy_pins_the_mended_commit_as_printed( + family, holder, tmp_path): + """The member's own `id:` mended in its own repository, then the + finding's two commands typed as printed, `` the mended one: the + `bump` pins it and writes the row's `id:` from it, and the gate is + green.""" + mounted_as_add_wrote_it(family, holder, tmp_path, + double_quoted("irqs" + LF)) + found = validate(holder) + line = found.stderr.split( + f"FINDING member-id-malformed: members/{SPARE}/project.yaml: ", + 1)[1].splitlines()[0] + switch, bump = commands(line) + work = tmp_path / "edit" + rewritten(work / "project.yaml", 'id: "irqs\\n"\n', 'id: "irqs"\n') + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", + "The id, mended", cwd=work) + git("push", "-q", "origin", "HEAD", cwd=work) + mended = git("rev-parse", "HEAD", cwd=work).stdout.strip() + git(*shlex.split(switch)[1:], cwd=holder.parent) + argv = shlex.split(bump.replace("", mended)) + done = run_script(REPO / argv[1], *argv[2:], cwd=REPO) + assert done.returncode == 0, done.stdout + done.stderr + row = row_of(holder, SPARE) + assert (row["id"], row["pin"]["commit"]) == ("irqs", mended) + assert validate(holder).returncode == 0 + + +# --- `add` and the gate ask one question ------------------------------------ + +#: `(the id, whether it is malformed)`: table 3's values and its controls, +#: and the edges of the pattern either side. +ONE_QUESTION = [ + pytest.param("irrs" + LF, True, id="line-feed"), + pytest.param("irrs" + CR, True, id="carriage-return"), + pytest.param("IRRS Capital", True, id="space"), + pytest.param("-x", True, id="leading-dash"), + pytest.param("", True, id="empty"), + pytest.param(" irrs", True, id="leading-space"), + pytest.param("IRRS", True, id="capitals"), + pytest.param("irrs.docs", True, id="dot"), + pytest.param("0", False, id="zero"), + pytest.param("irrs", False, id="irrs"), + pytest.param("irrs-2", False, id="hyphen"), + pytest.param("9-lives", False, id="digit-first"), +] + + +@pytest.mark.parametrize("value, malformed", ONE_QUESTION) +def test_add_refuses_exactly_the_ids_the_holders_own_validator_finds( + holder, tmp_path, value, malformed): + """THE GATE'S OWN ANSWER, in `test_family_add_member_id.py`'s way. For + each id, `add`'s question (`_read_member_project`, of a `project.yaml` + recording it, no clone) and the holder's `validate-family.py + --no-members` over IRRS's row recording it: `add` refuses + `member-bad-id` exactly when the gate finds `member-id-malformed`, and + both exactly when the id is malformed.""" + member = tmp_path / "member" + member.mkdir() + (member / "project.yaml").write_bytes( + ("schema_version: 1\nkind: project-manifest\n" + f"id: {double_quoted(value)}\nname: IRRS\n").encode("utf-8")) + module = family_module() + try: + module._read_member_project(member, "IRRS", f"{ORG}/IRRS") + except module.Refusal as exc: + assert exc.code == "member-bad-id", str(exc) + refused = True + else: + refused = False + with_row_id(holder, double_quoted(value)) + result = validate(holder, "--no-members") + found = "FINDING member-id-malformed: IRRS: " in result.stderr + assert (refused, found) == (malformed, malformed), result.stderr + assert result.returncode == (1 if malformed else 0), result.stderr diff --git a/tests/test_value_pattern_line_break.py b/tests/test_value_pattern_line_break.py index 2cded3c8..53c2484c 100644 --- a/tests/test_value_pattern_line_break.py +++ b/tests/test_value_pattern_line_break.py @@ -164,7 +164,9 @@ def test_every_spelling_of_a_value_pattern_is_in_the_table(): ("templates/assembly-root/scripts/validate-manifest.py", "fullmatch"): ( 1, "the manifest's `id:`"), ("templates/family-root/scripts/validate-family.py", "fullmatch"): ( - 1, "the family's `id:`"), + 2, "the family's `id:`; and `_is_project_id`, the one reader of a " + "member's row `id:` and of its mounted `project.yaml` `id:` " + "(#1204)"), ("scripts/family.py", "fullmatch"): ( 1, "a member's `project.yaml` `id:`, as `add` mounts it"), ("scripts/family.py", "match"): ( From f2dade54c2766705cc771356e51eb10668f55776 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 19:25:35 +0000 Subject: [PATCH 09/12] tests/test_validate_family_member_id_pattern.py: a mounted member whose own `id:` reads as the number 0 is refused, as `family.py add` refuses it, so the gate's `str(id or "")` reading of the member's own id is held (#1204) Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- .../test_validate_family_member_id_pattern.py | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tests/test_validate_family_member_id_pattern.py b/tests/test_validate_family_member_id_pattern.py index 07fc7f9b..44244606 100644 --- a/tests/test_validate_family_member_id_pattern.py +++ b/tests/test_validate_family_member_id_pattern.py @@ -104,14 +104,16 @@ def row_of(holder, project: str) -> dict: if row["project"] == project) -def mounted_as_add_wrote_it(family, holder, tmp_path, row_id: str) -> None: - """IRQS mounted from a copy of its remote whose `project.yaml` reads `id: - "irqs\\n"` (`a_member_whose_id_is`, which leaves its work clone at - `tmp_path / "edit"`), and its row written as a `family.py add` older - than #1166 wrote it, with its `id:` spelled `row_id` (#1166 refuses that - member now, so the row is written by hand). Committed, the pin and the - digest those of the mount: the id is the only thing wrong.""" - remotes = a_member_whose_id_is(family, tmp_path, 'id: "irqs\\n"\n') +def mounted_as_add_wrote_it(family, holder, tmp_path, row_id: str, + own: str = 'id: "irqs\\n"\n') -> None: + """IRQS mounted from a copy of its remote whose `project.yaml` writes + its `id:` as `own`, `id: "irqs\\n"` unless said (`a_member_whose_id_is`, + which leaves its work clone at `tmp_path / "edit"`), and its row written + as a `family.py add` older than #1166 wrote it, with its `id:` spelled + `row_id` (#1166 refuses that member now, so the row is written by + hand). Committed, the pin and the digest those of the mount: the id is + the only thing wrong.""" + remotes = a_member_whose_id_is(family, tmp_path, own) git("-c", "protocol.file.allow=always", "submodule", "add", "-q", "--", str(remotes / f"{SPARE}.git"), f"members/{SPARE}", cwd=holder) mounted = holder / "members" / SPARE @@ -303,6 +305,21 @@ def test_a_mounted_member_whose_own_id_is_malformed_is_found( assert len(found) == (2 if row_id != "irqs" else 1), result.stderr +def test_the_members_own_id_is_asked_as_add_asks_it(family, holder, tmp_path): + """`add` reads the member's own `id:` as `str(id or "")` + (`_read_member_project`), so a `project.yaml` whose `id:` reads as the + number 0 is `member-bad-id`, and the member's own `validate-manifest.py` + refuses it too, as no string. The gate asks it the same way: a row + spelling it `"0"`, which compared equal as text and was `family ok`, is + refused by the member's own finding, and only that.""" + mounted_as_add_wrote_it(family, holder, tmp_path, '"0"', own="id: 0\n") + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(source_finding(SPARE, 0) + " "), found[0] + + # --- the remedies, typed as printed ------------------------------------------ def test_the_rows_remedy_runs_as_printed_from_the_tracking_branch(on_main): From 9fc09305a073588935263013be46f4cbe4888da0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 19:27:17 +0000 Subject: [PATCH 10/12] tests/test_validate_family_member_id_pattern.py: each remedy's switch line makes the branch `family.py` suggests for `bump`, `family/bump-member` for an id no branch word spells (#1204) Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_validate_family_member_id_pattern.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_validate_family_member_id_pattern.py b/tests/test_validate_family_member_id_pattern.py index 44244606..9b13e4d5 100644 --- a/tests/test_validate_family_member_id_pattern.py +++ b/tests/test_validate_family_member_id_pattern.py @@ -50,7 +50,7 @@ ORG, SPARE, family, family_module, holder, manifest, validate, ) from test_family_tracking_branch import on_main # noqa: F401 -from test_validate_family_branch_step import BRANCH_LEFT +from test_validate_family_branch_step import BRANCH_LEFT, switch_argv from test_value_pattern_line_break import ( ID_PATTERN, a_member_whose_id_is, findings, rewritten, ) @@ -340,6 +340,7 @@ def test_the_rows_remedy_runs_as_printed_from_the_tracking_branch(on_main): 1)[1].splitlines()[0] assert line.endswith(". " + BRANCH_LEFT), line switch, bump = commands(line) + assert shlex.split(switch) == switch_argv(holder, "bump", "irrs" + LF) argv = shlex.split(bump) assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], bump assert argv[argv.index("--member") + 1] == "IRRS", bump @@ -368,6 +369,9 @@ def test_the_members_remedy_pins_the_mended_commit_as_printed( f"FINDING member-id-malformed: members/{SPARE}/project.yaml: ", 1)[1].splitlines()[0] switch, bump = commands(line) + words = shlex.split(switch) + assert words[-2:] == ["-c", "family/bump-member"], switch + assert os.path.samefile(words[2], holder), switch work = tmp_path / "edit" rewritten(work / "project.yaml", 'id: "irqs\\n"\n', 'id: "irqs"\n') git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", From d56f20840d22194fff581fbd8d8ba423871106f4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 19:36:20 +0000 Subject: [PATCH 11/12] tests/test_validate_family_member_id_pattern.py: a second row spelled as an earlier one, its `id:` malformed, is `member-duplicate`'s alone, as `_check_member_ids_unique` skips it (#1204) Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_validate_family_member_id_pattern.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_validate_family_member_id_pattern.py b/tests/test_validate_family_member_id_pattern.py index 9b13e4d5..18af8751 100644 --- a/tests/test_validate_family_member_id_pattern.py +++ b/tests/test_validate_family_member_id_pattern.py @@ -234,6 +234,22 @@ def test_a_row_id_that_reads_as_a_number_is_its_digits(holder): assert result.returncode == 0, result.stdout + result.stderr +def test_a_row_spelled_as_an_earlier_one_is_member_duplicates_alone(holder): + """THE ROWS ASKED ARE `_check_member_ids_unique`'s: a second row spelled + `IRRS`, its `id:` malformed, is `member-duplicate`'s, as the members + loop never reaches it either. One finding for one fault; whichever row + stays is asked on the next run.""" + family_root = family_module().Family.open(holder) + rows = family_root.members + irrs = next(row for row in rows if row["project"] == "IRRS") + family_root.write_members([*rows, {**irrs, "id": "irrs" + LF}]) + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-duplicate: IRRS appears more than once in " + "`members:`"], result.stderr + + @pytest.mark.parametrize("spelled", ["null", None], ids=["null", "no-id"]) def test_a_row_with_no_id_is_still_member_id_absent_alone(holder, spelled): """`id: null` reads as no `id:`, and a row with none is From 0657e39642e95d1bec9da2180871827e0c8019a0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 8 Nov 2026 20:47:13 +0000 Subject: [PATCH 12/12] validate-family.py and its member-id rows: the docstring of `_check_member_ids_well_formed` says `member-pin-commit` refuses the run first where members are checked, and rows hold a pin that is no commit as `--to `, the holder's path quoted in the printed `bump`, and a bare `id: 0` as its digits (#1204) Answers NITs 1-3 of review r1204 on #1273. No behavior of validate-family.py changes; only the docstring does. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- .../family-root/scripts/validate-family.py | 5 +- .../test_validate_family_member_id_pattern.py | 69 +++++++++++++++++-- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index c9a37c41..4e011252 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -1979,8 +1979,9 @@ def _check_member_ids_well_formed(root: Path, members: list, not run (measured for #1204); and `add` would clone the member again and move the pin to its tip without an `--at`. `bump` needs the member checked out, and its own `member-uninitialized` names the line that - checks it out. A pin that is no 40-hex commit is named `--to `, - with `member-pin-commit` beside it where members are checked. + checks it out. A pin that is no 40-hex commit is named `--to `; + where members are checked, `member-pin-commit` refuses the run first, and + this finding follows once the pin is mended. """ projects: set[str] = set() for row in members: diff --git a/tests/test_validate_family_member_id_pattern.py b/tests/test_validate_family_member_id_pattern.py index 18af8751..a704336b 100644 --- a/tests/test_validate_family_member_id_pattern.py +++ b/tests/test_validate_family_member_id_pattern.py @@ -38,6 +38,7 @@ import os import re import shlex +import shutil import pytest @@ -224,14 +225,31 @@ def test_a_row_id_whose_text_is_no_id_is_found(holder, spelled): assert found[0].startswith(row_finding("IRRS", value)), found[0] -def test_a_row_id_that_reads_as_a_number_is_its_digits(holder): +@pytest.mark.parametrize("number", [2026, 0], ids=["2026", "zero"]) +def test_a_row_id_that_reads_as_a_number_is_its_digits(holder, number): """A holder written before #259 keeps `id: 2026` for a member scaffolded with `--id 2026`, which the reader reads as a number; `family.py`'s - `_member_id` says the validator passes it, and it still does.""" - with_row_id(holder, "2026") - assert row_of(holder, "IRRS")["id"] == 2026 + `_member_id` says the validator passes it, and it still does. A bare + `id: 0` is the one number the two readings could part on: `add` asks the + member's own `id:` as `str(id or "")`, which makes `0` no id, and the row + is asked as `str()` alone, so `0` is the id `"0"`, an id the pattern + takes. Under `--no-members` it is `family ok`; with the members checked + it is the one `member-id-mismatch`, of a well-formed id that is not the + member's, with the number named as the reader read it.""" + with_row_id(holder, str(number)) + value = row_of(holder, "IRRS")["id"] + assert repr(value) == str(number), value result = validate(holder, "--no-members") assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.splitlines()[-1].startswith("family ok: "), \ + result.stdout + result = validate(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result.stderr) == [ + "FINDING member-id-mismatch: members/IRRS: project.yaml declares id " + f"'irrs' but family.yaml records {number} for IRRS. The project's " + "own manifest is the source; this row is a pin of it."], \ + result.stderr def test_a_row_spelled_as_an_earlier_one_is_member_duplicates_alone(holder): @@ -268,6 +286,35 @@ def test_a_row_with_no_id_is_still_member_id_absent_alone(holder, spelled): result.stderr +# --- the remedy for a pin that is no commit --------------------------------- + +def test_a_pin_that_is_no_commit_is_named_to_commit(holder): + """The remedy's `--to` is the commit the row pins, and where the row + pins none (`abc` here, not 40 hex) it is the placeholder ``, the + literal word: `bump` refuses `--to abc` as `member-to-not-a-commit`, so + printing the pin would print a line that cannot run. With the members + checked, `member-pin-commit` refuses the run first, so the row's finding + follows once the pin is mended.""" + pinned = row_of(holder, "IRRS")["pin"]["commit"] + with_row_id(holder, double_quoted("irrs" + LF)) + rewritten(holder / "family.yaml", f' commit: "{pinned}"\n', + ' commit: "abc"\n') + result = validate(holder, "--no-members") + assert result.returncode == 1, result.stdout + result.stderr + found = findings(result.stderr) + assert len(found) == 1, result.stderr + assert found[0].startswith(row_finding("IRRS", "irrs" + LF) + " "), \ + found[0] + argv = shlex.split(commands(found[0])[1]) + assert argv[:3] == [PYTHON, "scripts/family.py", "bump"], argv + assert argv[argv.index("--member") + 1:] == ["IRRS", "--to", ""] + refused = validate(holder) + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stderr.startswith("REFUSED member-pin-commit: IRRS: "), \ + refused.stderr + assert "member-id-malformed" not in refused.stderr + + # --- a mounted member's own `id:` -------------------------------------------- def test_the_holder_add_wrote_before_1166_is_refused_in_every_mode( @@ -338,14 +385,24 @@ def test_the_members_own_id_is_asked_as_add_asks_it(family, holder, tmp_path): # --- the remedies, typed as printed ------------------------------------------ -def test_the_rows_remedy_runs_as_printed_from_the_tracking_branch(on_main): +@pytest.mark.parametrize("where", [None, "it's a holder"], + ids=["plain", "space-and-apostrophe"]) +def test_the_rows_remedy_runs_as_printed_from_the_tracking_branch( + on_main, tmp_path, where): """A row `id:` written by hand and committed, on the tracking branch, where a person stands after `init`. The finding's two commands, typed as printed from a checkout of openRepoShape: the switch makes a branch, and the `bump` to the commit the row pins writes the row's `id:` from the member's own `project.yaml`, the pin where it was. The gate is - green after it, in every mode.""" + green after it, in every mode. The holder is also run from a path that + holds a space and an apostrophe, as `test_validate_family_branch_step.py` + does for `_switch_line`: the `bump` line quotes it for the reader's + shell, or what is typed is not what is printed.""" holder = on_main + if where is not None: + target = tmp_path / where + shutil.move(str(on_main), str(target)) + holder = target.resolve() with_row_id(holder, double_quoted("irrs" + LF)) git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qam", "A row id written by hand", cwd=holder)