diff --git a/scripts/family.py b/scripts/family.py index 4e870ff4..03c41335 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -67,6 +67,7 @@ import os import re import runpy +import shlex import shutil import stat import subprocess @@ -1133,11 +1134,22 @@ def _index_entries(root: Path, path: str) -> list[tuple[str, str]]: return entries -def _submodule_paths(root: Path) -> dict[str, str]: - """`{name: path}` of every submodule the holder's `.gitmodules` names, - read from where `git submodule add` reads it: the file in the working - tree, or the index's copy where there is none, or HEAD's where the index - has none either.""" +def _submodule_paths( + root: Path) -> dict[str, tuple[str, str | None, tuple[str, ...]]]: + """`{name: (path, url, keys)}` of every submodule the holder's + `.gitmodules` names, read from where `git submodule add` reads it: the + file in the working tree, or the index's copy where there is none, or + HEAD's where the index has none either. `url` is None for a section that + gives no url; a section that gives no path is no submodule and is not + listed. `keys` is every key the section holds, path and url included, + once each, in the order git lists them. + + THE ONE READER OF THE FILE, for both questions `_checked_mount_path` + asks of it: the path a NAME is given (`member-name-taken`, #242), and + the names a PATH is given, each with its url and its keys + (`member-path-registered`, #300). A second reader could read a second + copy of the file. + """ if os.path.lexists(root / GITMODULES): source = ["--file", GITMODULES] else: @@ -1152,14 +1164,15 @@ def _submodule_paths(root: Path) -> dict[str, str]: return {} listed = git_out(["config", "-z", *source, "--list"], cwd=root, binary=True) - found = {} + found: dict[str, dict[str, str]] = {} for record in listed.split(b"\0"): key, _, value = record.decode("utf-8", "surrogateescape").partition("\n") section, _, variable = key.rpartition(".") - if section.startswith("submodule.") and variable == "path": - found[section[len("submodule."):]] = value - return found + if section.startswith("submodule."): + found.setdefault(section[len("submodule."):], {})[variable] = value + return {name: (keys["path"], keys.get("url"), tuple(keys)) + for name, keys in found.items() if "path" in keys} def _checked_mount_path(root: Path, project: str, path: str) -> None: @@ -1190,6 +1203,18 @@ def _checked_mount_path(root: Path, project: str, path: str) -> None: the member under a new one (`members/IRRS1`) instead. Git before 2.51 rewrites the other submodule's section to point at the member. Both are refused here, on every git. + - THE PATH UNDER ANOTHER NAME (#300), the mirror of the name: a section + that already gives the mount path to a submodule named something + else — a half-removed member, a hand edit, a merge that kept both + sides. This one git does not make at all, with `--force` or without + it: it writes the member's own section beside that one, so `add` + committed two sections for one path, `validate-family.py` passed + them, and a recursive clone followed the LATER of the two, the other + one's url as soon as an edit put it last. It is refused, naming the + section and its url, and left exactly as it is: it is somebody's + registration, and dropping or rewriting it as part of the add would + decide whose without asking. The path is compared exactly; git reads + `.gitmodules` that way too. """ if os.path.lexists(root / path): raise Refusal( @@ -1251,7 +1276,8 @@ def _checked_mount_path(root: Path, project: str, path: str) -> None: "names a submodule, take that out (`git rm`); anything else it " "names, a held `index.lock` say, is fixed where git says. Then " "re-run.") from exc - other = _submodule_paths(root).get(path) + sections = _submodule_paths(root) + other = sections.get(path, (None, None))[0] if other is not None and other != path: raise Refusal( "member-name-taken", @@ -1263,6 +1289,40 @@ def _checked_mount_path(root: Path, project: str, path: str) -> None: f"its section to point at {project} (git before 2.51), or mount " f"{project} under another name this tool does not follow (git " "2.51 and later, with the `--force` this tool mounts with).") + ghosts = [(name, url, keys) for name, (at, url, keys) in sections.items() + if at == path and name != path] + if ghosts: + named = " and ".join( + f"`{name}` ({'no url' if url is None else 'url ' + url})" + for name, url, _ in ghosts) + # BY KEY, NOT BY SECTION: `--remove-section` cannot address a + # section whose name holds `]`, nor one whose header a hand spelled + # `[Submodule "..."]` (git 2.43: `no such section`), while a key + # reaches every section `git config --list` names. EVERY KEY it + # holds, not its path and url alone: a `branch`, `shallow` or + # `update` left behind is inherited by the submodule a later `add` + # gives that name, which git writes into the same section. git + # drops the header with the last key. + removal = " and ".join( + f"`git config -f {GITMODULES} --unset-all " + f"{shlex.quote('submodule.' + name + '.' + key)}`" + for name, _, keys in ghosts for key in keys) + raise Refusal( + "member-path-registered", + f"{GITMODULES} already gives the path {path} to the submodule" + + ("s" if len(ghosts) > 1 else "") + f" named {named}, and that " + f"is the path {project} is mounted at", + "Remediation: git would write the member's own section beside " + "it, and a recursive clone of the holder follows whichever " + "section for that path comes last in the file. A section " + "somebody wrote is not this tool's to drop or rewrite, so take it " + f"out by hand, in a POSIX shell — {removal}, then `git add " + f"{GITMODULES}` — or, if it was a member, with `family.py " + "remove`; then re-run." + + ("" if os.path.lexists(root / GITMODULES) else + f" With no {GITMODULES} in the working tree, put back the " + f"copy git read first: `git checkout -- {GITMODULES}` (the " + f"index's), or `git checkout HEAD -- {GITMODULES}`.")) def _checked_add_target(family: Family, repository: str, project: str, diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 0991ba1b..3046054a 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -37,6 +37,10 @@ and, across the rows, that no two record ONE `id`: that is one project pinned twice, however each row spelled its repository (#206). This one reads `family.yaml` alone, so `--no-members` asks it too. + - 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 COPIED shape files, each against its `sha256` row in `contracts/shape-pin.yaml`. Editing a copy in place is drift and the exit is to carry it upstream, never to update the digest. @@ -52,6 +56,7 @@ from __future__ import annotations import argparse +import os import re import shlex import sys @@ -67,6 +72,7 @@ DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") QUALIFIED_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*$") MANIFEST = "family.yaml" +GITMODULES = ".gitmodules" DEFAULT_MEMBERS_DIR = "members" LOCKSTEP = ( @@ -570,13 +576,90 @@ def _check_member_ids_unique(root: Path, members: list, f"name {first} there instead to keep {project}.") +def _check_gitmodules_paths(root: Path, report: Report) -> None: + """No two sections of `.gitmodules` give ONE path (#300). + + `family.py add` committed a second section for a mount path that a + section under another name already gave — a half-removed member, a hand + edit, a merge that kept both sides — and this gate passed the result. A + 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. + + READ AS GIT READS IT, with `git config -z --file .gitmodules --list` as + `family.py` reads it, and no parser of this file's own: the working-tree + file, which in CI is HEAD's. A holder with no members has no + `.gitmodules`, and that is no finding; one git cannot read is a refusal, + because nothing could be cloned from it either. A section whose path no + row names is another question, and not asked here. + """ + if not os.path.lexists(root / GITMODULES): + return + try: + listed = git_out(["config", "-z", "--file", GITMODULES, "--list"], + cwd=root, binary=True) + except Refusal as exc: + raise Refusal( + "gitmodules-unreadable", + f"{GITMODULES} could not be read as git reads it: {exc.detail}", + "Remediation: git cannot read it either, so none of this " + "holder's submodules can be cloned from it as it stands. Mend " + f"the file by hand (`git config -f {GITMODULES} --list` says " + "where it breaks), then run this again.") from exc + except OSError as exc: + # NO GIT TO RUN is a question this run cannot answer, so a refusal + # (exit 2), never the traceback that exits 1, a finding's code: + # `--no-members` ran no git at all before this check. + raise Refusal( + "gitmodules-unreadable", + f"{GITMODULES} could not be read as git reads it: git could not " + f"be run ({exc.strerror or exc})", + "Remediation: put git on PATH and run this again; this check " + "reads the file the way git does, and nothing could fetch this " + "holder's submodules without git either.") from exc + sections: dict[str, dict[str, str]] = {} + for record in listed.split(b"\0"): + key, _, value = record.decode("utf-8", + "surrogateescape").partition("\n") + section, _, variable = key.rpartition(".") + if section.startswith("submodule.") and variable in ("path", "url"): + sections.setdefault(section[len("submodule."):], + {})[variable] = value + paths: dict[str, list[str]] = {} + for name, keys in sections.items(): + if "path" in keys: + paths.setdefault(keys["path"], []).append(name) + for path, names in paths.items(): + if len(names) < 2: + continue + listing = ", ".join( + f"`{name}` (" + (f"url {sections[name]['url']}" + if "url" in sections[name] else "no url") + ")" + for name in names) + report.finding( + "gitmodules-path-twice", + f"{GITMODULES} gives the path {path} to {len(names)} " + f"submodules, {listing}. A recursive clone fetches that path " + "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"-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}.") + + def _check_members(root: Path, members: list, no_members: bool, report: Report) -> None: """Every row in `members:`, or the `--no-members` skip line. - Split from `main` for #137. + Split from `main` for #137. The two checks that read a file alone — + `family.yaml`'s ids (#206) and `.gitmodules`'s paths (#300) — come + first, so `--no-members` asks them too. """ _check_member_ids_unique(root, members, report) + _check_gitmodules_paths(root, 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 c038f605..3f744dc1 100644 --- a/tests/test_family_add_refusals.py +++ b/tests/test_family_add_refusals.py @@ -33,6 +33,7 @@ import importlib.util import os import re +import shlex import shutil import stat import subprocess @@ -1545,3 +1546,409 @@ def test_a_conflicted_entry_at_the_mount_path_is_counted_once_per_stage( "Remediation: this is a half-finished add or a hand edit. `git rm -r " f"--cached -- {SPARE_MOUNT}` takes it out of the index and touches no " "file; then re-run.") + + +# --- a section under ANOTHER name at the mount path (#300) ------------------ +# +# The mirror of `member-name-taken` (#242), which refuses a section whose NAME +# is the mount path for another path: here a section under another name +# already gives the mount PATH. git writes the member's own section beside it +# without a word, so `add` exited 0 and committed two sections for one path, +# `validate-family.py` said `family ok`, and a recursive clone followed the +# one LAST in the file — the other's url, as soon as an edit put it there. +# Each holder below is built in git's index and objects, never through a +# checkout of the file. + +#: A section under another name whose path is the spare's mount path, with a +#: url nothing can fetch: what a half-removed member, a hand edit or a merge +#: that kept both sides leaves behind. +GHOST = (f'[submodule "ghost"]\n\tpath = members/{SPARE}\n' + "\turl = /nowhere/ghost.git\n").encode("utf-8") + +GHOST_REFUSED = ( + "REFUSED member-path-registered: .gitmodules already gives the path " + f"members/{SPARE} to the submodule named `ghost` (url " + f"/nowhere/ghost.git), and that is the path {SPARE} is mounted at") + +#: How the refusal spells taking GHOST out: by key, which reaches every +#: section, one command for each key it holds, in the order git lists +#: them. +GHOST_REMOVAL = ( + "`git config -f .gitmodules --unset-all submodule.ghost.path` and " + "`git config -f .gitmodules --unset-all submodule.ghost.url`") + + +def ghost_remediation(removal: str, off_disk: bool = False) -> str: + """What `member-path-registered` tells the human to do, around the + `git config` command(s), `removal`, that take each section out — and, + only where the working tree has no `.gitmodules` (`off_disk`), how to + put back the copy git read.""" + return ( + "Remediation: git would write the member's own section beside it, " + "and a recursive clone of the holder follows whichever section for " + "that path comes last in the file. A section somebody wrote is not " + "this tool's to drop or rewrite, so take it out by hand, in a POSIX " + f"shell — {removal}, then `git add .gitmodules` — or, if it was a " + "member, with `family.py remove`; then re-run." + + (" With no .gitmodules in the working tree, put back the copy git " + "read first: `git checkout -- .gitmodules` (the index's), or `git " + "checkout HEAD -- .gitmodules`." if off_disk else "")) + + +def gitmodules_with(root: Path, extra: bytes, kept_in: str) -> bytes: + """HEAD's `.gitmodules` with `extra` appended, kept ONLY where `kept_in` + says, and built in git's index and objects rather than through a + checkout: "the work tree only" writes it to disk and neither stages nor + commits it — a hand edit nobody has staged, the commonest ghost, which + only a reader that asks the working tree FIRST sees; "the work tree" + commits it and writes the same bytes to disk; "the index" stages it over + HEAD's and leaves no file on disk; "HEAD" commits it, then takes it off + the disk AND out of the index, where `git submodule add` still reads it. + Returns the bytes.""" + data = subprocess.run(["git", "show", "HEAD:.gitmodules"], + cwd=str(root), capture_output=True, + check=True).stdout + extra + if kept_in == "the work tree only": + (root / ".gitmodules").write_bytes(data) + return data + blob = subprocess.run(["git", "hash-object", "-w", "--stdin"], + cwd=str(root), input=data, capture_output=True, + check=True).stdout.decode("ascii").strip() + git("update-index", "--cacheinfo", f"100644,{blob},.gitmodules", + cwd=root) + if kept_in != "the index": + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A section of somebody else's", cwd=root) + if kept_in == "the work tree": + (root / ".gitmodules").write_bytes(data) + else: + (root / ".gitmodules").unlink() + if kept_in == "HEAD": + git("rm", "-q", "--cached", "--", ".gitmodules", cwd=root) + return data + + +def git_section(scratch: Path, name: str, **values: str) -> bytes: + """`[submodule ""]` holding `values`, in the bytes GIT writes for + them, for `gitmodules_with` to append. A value read from git must be + escaped as git escapes it: a Windows path's backslashes written raw + (`D:\\a\\...`) are escapes git does not know, and the file is then one it + cannot read at all (`bad config line`). `scratch` is a file of the + test's own, written afresh.""" + if scratch.exists(): + scratch.unlink() + for key, value in values.items(): + git("config", "-f", str(scratch), f"submodule.{name}.{key}", value, + cwd=scratch.parent) + return scratch.read_bytes() + + +def gitmodules_paths(root: Path) -> list[str]: + """Every `path` HEAD's `.gitmodules` gives, in file order, as git lists + them.""" + listed = git("config", "--blob", "HEAD:.gitmodules", "--list", + cwd=root).stdout.splitlines() + return [line.partition("=")[2] for line in listed + if line.partition("=")[0].rpartition(".")[2] == "path"] + + +def take_out_as_printed(root: Path, remediation: str) -> list[str]: + """Run each `git config` command `remediation` prints, exactly as + printed (split as a POSIX shell splits it, run with no shell), then + stage `.gitmodules`, as the remediation says. Returns the commands.""" + commands = re.findall(r"`(git config [^`]*)`", remediation) + for command in commands: + git(*shlex.split(command)[1:], cwd=root) + git("add", "--", ".gitmodules", cwd=root) + return commands + + +def refused_unchanged(result: subprocess.CompletedProcess, root: Path, + before: dict) -> tuple[str, str]: + """Refused before the member was read — nothing on stdout, nothing + cloned or mounted — with the holder byte for byte as `before` recorded + it. Returns `(the REFUSED line, the remediation)`.""" + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "" + assert snapshot(root) == before + assert not (root / "members" / SPARE).exists() + assert not (root / ".git" / "modules" / "members" / SPARE).exists() + return refusal(result.stderr) + + +@pytest.mark.parametrize("kept_in", ["the work tree only", "the work tree", + "the index", "HEAD"]) +def test_a_section_under_another_name_at_the_mount_path_is_refused( + family, holder, offline, kept_in): + """THE ISSUE'S PROBE, from each copy of the file `git submodule add` + reads: the working tree's — an unstaged hand edit as well as a committed + ghost — else the index's, else HEAD's. A section named `ghost` gives + `members/IRQS` to a url nothing can fetch, and `add --member + InkRouter/IRQS` is refused before the member is read — not one line on + stdout, nothing cloned, nothing mounted — naming the section and its + url, and the holder is byte for byte as it was, the ghost included: it + is somebody's registration, not this tool's to drop. Only where the + working tree has no `.gitmodules` does the remediation say how to put + one back.""" + gitmodules_with(holder, GHOST, kept_in) + before = snapshot(holder) + result = offline.add(holder, SPARE, remotes=family["remotes"]) + assert refused_unchanged(result, holder, before) == ( + GHOST_REFUSED, ghost_remediation( + GHOST_REMOVAL, off_disk=kept_in in ("the index", "HEAD"))) + offline.assert_left_nothing() + + +def test_a_section_named_the_mount_path_in_another_case_is_another_name( + family, holder, offline): + """git's submodule names are case-sensitive, so `Members/IRQS` is + ANOTHER name than the `members/IRQS` git gives the mount, and its + section for the mount path is a ghost like any other: refused, named in + its own spelling.""" + gitmodules_with(holder, (f'[submodule "Members/{SPARE}"]\n' + f"\tpath = members/{SPARE}\n" + "\turl = /nowhere/ghost.git\n").encode(), + "the work tree") + before = snapshot(holder) + result = offline.add(holder, SPARE, remotes=family["remotes"]) + said, _ = refused_unchanged(result, holder, before) + assert said == ( + "REFUSED member-path-registered: .gitmodules already gives the path " + f"members/{SPARE} to the submodule named `Members/{SPARE}` (url " + f"/nowhere/ghost.git), and that is the path {SPARE} is mounted at") + offline.assert_left_nothing() + + +@pytest.mark.parametrize("other", [ + None, (f"members/{SPARE}X", "ghost"), (f"members/{SPARE}/", "ghost"), + (f"members/{SPARE}", f"members/{SPARE}")], + ids=["no other section", "another path", "a trailing slash", + "the member's own name"]) +def test_a_section_that_is_not_another_name_at_the_mount_path_is_added_past( + family, holder, offline, other): + """THE CONTROL. The plain add still exits 0, and so does one past a + section for ANOTHER path: one that only begins like the mount path, or + one that ends in a slash — git compares a section's path exactly, so a + recursive clone never fetches the mount path from it, and neither does + this refusal. So does one past a section that already carries the + member's own name for its own path — what `remove` leaves behind when a + ghost came after the member's section (#300) — which git rewrites in + place. Each commits ONE section for the mount path, with the member's + url, and the holder passes its own gate.""" + if other is not None: + path, name = other + gitmodules_with(holder, (f'[submodule "{name}"]\n\tpath = {path}\n' + "\turl = /nowhere/ghost.git\n").encode(), + "the work tree") + # The member's url, spelled the way `add` spelled IRRS's beside it. + url = git("config", "-f", ".gitmodules", "submodule.members/IRRS.url", + cwd=holder).stdout.strip()[:-len("IRRS.git")] + f"{SPARE}.git" + result = offline.add(holder, SPARE, remotes=family["remotes"]) + assert result.returncode == 0, result.stdout + result.stderr + assert result.stderr == "" + assert gitmodules_paths(holder).count(f"members/{SPARE}") == 1 + assert git("config", "--blob", "HEAD:.gitmodules", + f"submodule.members/{SPARE}.url", + cwd=holder).stdout.strip() == url + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 + offline.assert_left_nothing() + + +def test_every_section_at_the_mount_path_is_named_and_taken_out_as_printed( + family, holder, offline): + """TWO SECTIONS under other names at one mount path, one named with a + space and one with no url: the refusal names both, each with its url or + the want of one, and its `git config` commands, typed exactly as + printed, take both out — after which the same `add` runs, and the holder + gives the mount path to one section, the member's.""" + gitmodules_with(holder, GHOST + (f'[submodule "old {SPARE}"]\n' + f"\tpath = members/{SPARE}\n").encode(), + "the work tree") + before = snapshot(holder) + refused = offline.add(holder, SPARE, remotes=family["remotes"]) + said, remediation = refused_unchanged(refused, holder, before) + assert said == ( + "REFUSED member-path-registered: .gitmodules already gives the path " + f"members/{SPARE} to the submodules named `ghost` (url " + f"/nowhere/ghost.git) and `old {SPARE}` (no url), and that is the " + f"path {SPARE} is mounted at") + assert remediation == ghost_remediation( + f"{GHOST_REMOVAL} and `git config -f .gitmodules --unset-all " + f"'submodule.old {SPARE}.path'`") + assert len(take_out_as_printed(holder, remediation)) == 3, remediation + added = offline.add(holder, SPARE, remotes=family["remotes"]) + assert added.returncode == 0, added.stdout + added.stderr + assert gitmodules_paths(holder) == [ + "members/IRRS", "members/IRSS", f"members/{SPARE}"] + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert validate(holder).returncode == 0 + offline.assert_left_nothing() + + +@pytest.mark.parametrize("header, name", [ + (f'[submodule "old]{SPARE}"]', f"old]{SPARE}"), + ('[Submodule "ghost"]', "ghost")], + ids=["a name with ]", "a header a hand capitalised"]) +def test_a_section_git_cannot_name_by_section_is_taken_out_as_printed( + family, holder, offline, header, name): + """THE PRINTED COMMANDS WORK FOR EVERY SECTION. `git config + --remove-section` cannot address a section whose name holds `]`, nor one + whose header a hand spelled `[Submodule "..."]` (git 2.43 says `no such + section` for both), so the refusal takes a section out by its keys, + which reach every section `git config --list` names. Run exactly as + printed, they clear the way for the same `add`.""" + gitmodules_with(holder, (f"{header}\n\tpath = members/{SPARE}\n" + "\turl = /nowhere/ghost.git\n").encode(), + "the work tree only") + before = snapshot(holder) + refused = offline.add(holder, SPARE, remotes=family["remotes"]) + said, remediation = refused_unchanged(refused, holder, before) + assert said == ( + "REFUSED member-path-registered: .gitmodules already gives the path " + f"members/{SPARE} to the submodule named `{name}` (url " + f"/nowhere/ghost.git), and that is the path {SPARE} is mounted at") + quoted = [shlex.quote(f"submodule.{name}.{key}") + for key in ("path", "url")] + assert remediation == ghost_remediation( + f"`git config -f .gitmodules --unset-all {quoted[0]}` and " + f"`git config -f .gitmodules --unset-all {quoted[1]}`") + assert len(take_out_as_printed(holder, remediation)) == 2, remediation + added = offline.add(holder, SPARE, remotes=family["remotes"]) + assert added.returncode == 0, added.stdout + added.stderr + assert gitmodules_paths(holder) == [ + "members/IRRS", "members/IRSS", f"members/{SPARE}"] + assert validate(holder).returncode == 0 + offline.assert_left_nothing() + + +@pytest.mark.parametrize("same_url", [False, True], + ids=["another url", "the same url"]) +def test_the_validator_finds_two_sections_for_one_path(family, holder, + tmp_path, same_url): + """`gitmodules-path-twice`, so a holder that already has a ghost fails + its own gate instead of passing it. One section per path is clean; one + more for `members/IRRS`, under another name and LAST in the file — the + one a recursive clone follows — is the only finding, under `--pins` and + `--no-members` as well, because it reads `.gitmodules` alone. Two + sections for one path are a finding whatever their urls: the same url + twice is still one path given twice, and the next edit can change + either.""" + assert validate(holder).returncode == 0 + url = git("config", "-f", ".gitmodules", "submodule.members/IRRS.url", + cwd=holder).stdout.strip() + other = url if same_url else "/nowhere/ghost.git" + # Written by git, so the member's url — a path with backslashes on + # Windows — is escaped as git escapes it. + gitmodules_with(holder, git_section(tmp_path / "ghost-section", "ghost", + path="members/IRRS", url=other), + "the work tree") + for extra in ((), ("--pins",), ("--no-members",)): + result = validate(holder, *extra) + assert result.returncode == 1, result.stderr + result.stdout + assert [line for line in result.stderr.splitlines() + if line.startswith("FINDING")] == [ + "FINDING gitmodules-path-twice: .gitmodules gives the path " + f"members/IRRS to 2 submodules, `members/IRRS` (url {url}), " + f"`ghost` (url {other}). A recursive clone fetches that path " + "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 " + "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 + + +def test_a_gitmodules_git_cannot_read_is_a_refusal_not_a_pass(holder): + """The validator reads `.gitmodules` as git reads it, so a file git + cannot read is a question it cannot answer: a refusal, exit 2, never an + implicit pass.""" + (holder / ".gitmodules").write_bytes(b'[submodule "broken"\n') + result = validate(holder) + assert result.returncode == 2, result.stderr + result.stdout + assert result.stderr.startswith( + "REFUSED gitmodules-unreadable: .gitmodules could not be read as git " + "reads it: `git config -z --file .gitmodules --list` in ") + assert result.stderr.splitlines()[-1] == ( + "Remediation: git cannot read it either, so none of this holder's " + "submodules can be cloned from it as it stands. Mend the file by " + "hand (`git config -f .gitmodules --list` says where it breaks), " + "then run this again.") + + +def test_a_validator_with_no_git_to_run_refuses_instead_of_crashing( + holder, monkeypatch, capsys): + """`--no-members` ran no git before `.gitmodules` was read, so a + machine without git now meets one here. That is a question this run + cannot answer — a refusal, exit 2 — never the traceback that exits 1, a + finding's code. The holder's own validator is run IN THIS PROCESS with + its one git call failing as a missing program does, so no PATH is + changed for anything else.""" + monkeypatch.setattr(sys, "path", list(sys.path)) + spec = importlib.util.spec_from_file_location( + "holder_validate_family", holder / "scripts" / "validate-family.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + def no_git(args, cwd, binary=False): + raise FileNotFoundError(2, "No such file or directory", "git") + + monkeypatch.setattr(module, "git_out", no_git) + code = module.main(["--root", str(holder), "--no-members"]) + out, err = capsys.readouterr() + assert code == 2, out + err + assert err.splitlines() == [ + "REFUSED gitmodules-unreadable: .gitmodules could not be read as git " + "reads it: git could not be run (No such file or directory)", + "Remediation: put git on PATH and run this again; this check reads " + "the file the way git does, and nothing could fetch this holder's " + "submodules without git either."] + + +def test_every_key_of_a_ghost_goes_so_a_later_member_of_its_name_inherits_none( + family, holder, offline, tmp_path): + """EVERY KEY, NOT ITS PATH AND URL ALONE. A ghost named `members/IRQZ` + for the spare's mount path also carries `branch`, `shallow` and + `update`. The refusal prints one `git config --unset-all` per key it + holds, and run as printed they take the whole section out, header and + all. Had any key stayed, the add of a member that IS called IRQZ would + have had git write its path and url into that same section and inherit + the rest — a `branch = dev` that `git submodule update --remote` then + fails on, under a gate that says ok. Here IRQZ's section holds its path + and its url and nothing else.""" + remotes = tmp_path / "remotes" + remotes.mkdir() + for name in (SPARE, "IRQZ"): + shutil.copytree(family["remotes"] / f"{SPARE}.git", + remotes / f"{name}.git", symlinks=True) + gitmodules_with(holder, ( + '[submodule "members/IRQZ"]\n' + f"\tpath = members/{SPARE}\n\turl = /nowhere/ghost.git\n" + "\tbranch = dev\n\tshallow = true\n\tupdate = none\n").encode(), + "the work tree") + before = snapshot(holder) + refused = offline.add(holder, SPARE, remotes=remotes) + said, remediation = refused_unchanged(refused, holder, before) + assert said == ( + "REFUSED member-path-registered: .gitmodules already gives the path " + f"members/{SPARE} to the submodule named `members/IRQZ` (url " + f"/nowhere/ghost.git), and that is the path {SPARE} is mounted at") + assert remediation == ghost_remediation(" and ".join( + "`git config -f .gitmodules --unset-all " + f"submodule.members/IRQZ.{key}`" + for key in ("path", "url", "branch", "shallow", "update"))) + assert len(take_out_as_printed(holder, remediation)) == 5, remediation + assert git("config", "-f", ".gitmodules", "--get-regexp", + r"^submodule\.members/IRQZ\.", cwd=holder, + check=False).stdout == "" + assert "members/IRQZ" not in (holder / ".gitmodules").read_text( + encoding="utf-8") + added = offline.add(holder, "IRQZ", remotes=remotes) + assert added.returncode == 0, added.stdout + added.stderr + listed = git("config", "--blob", "HEAD:.gitmodules", "--get-regexp", + r"^submodule\.members/IRQZ\.", cwd=holder).stdout + assert [line.split(" ", 1)[0] for line in listed.splitlines()] == [ + "submodule.members/IRQZ.path", "submodule.members/IRQZ.url"] + assert git("status", "--porcelain", cwd=holder).stdout == "" + offline.assert_left_nothing()