diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 2c1bd838..dfb50769 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 first " + f"from {whose}, `{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], @@ -3318,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 @@ -3343,12 +3392,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 +3408,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 +3435,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 +3472,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 +3493,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 +3851,32 @@ 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 + 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.""" + 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..f2b0fd3e --- /dev/null +++ b/tests/test_validate_family_index_gitmodules.py @@ -0,0 +1,444 @@ +# 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 first " + "from the index's, `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) + + +#: 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: + """`.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) + + +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: + """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)