From 38a40abfb2c703600cba876e835859f93d5ccbb4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 13:55:43 +0000 Subject: [PATCH 1/4] WIP (account-swap checkpoint; untested on these bytes): siblings.py: the printed move runs as typed past a taken staging name or dangling link, a NUL or lone surrogate in `project:` or url is a finding, `.gitmodules` is read as git reads it, the warning's prose is spelled, and a tag-only branch is named (#690) Six lines of `templates/family-root/scripts/siblings.py` (`make siblings`, a pinned copy in every family holder, which takes this as one `upstream-changed` row), each re-measured on main 4ce90457 with a holder from `family.py init --local-remote-dir`: - `parent_folder_warning` printed its staging name whatever held it. With `/InkRouter.holder` a folder already, the three printed lines all exited 0 and left the holder at `/InkRouter/InkRouter/InkRouter`, one level too deep; `.folder` alike on a disk that ignores case. `staging_path` now gives the first of `.holder`, `.holder.1`, `.holder.2` ... at which `os.path.lexists` finds nothing, a link to nothing included. - A link to nothing named `` passed `not wanted.exists()`, so the staged move was printed, its first `mv` ran, and `mkdir -p` failed `File exists` with the holder half moved. The warning now says, before the moves, `First move aside: it is a link to nothing, named after the family, and in the way of the move. Then:`, and the moves under it are the ones that run once it is gone. - `place` handed a `project:` holding a lone surrogate or a NUL to `git clone` as the clone's path: `UnicodeEncodeError` or `ValueError: embedded null byte`, exit 1, and the members after it never placed. The url `clone_url` builds from a `repository:` holding a NUL ended the same way. `argument_fault`, #455's rule as #644 wrote it for `make bootstrap` (a NUL, or a surrogate `os.fsencode` cannot write as one byte), is now asked of both before anything: the member is `NOT AN ARGUMENT`, a finding (`: its `project:` in family.yaml holds U+D83D, a lone surrogate, which no command can be given as an argument, so nothing was cloned or fetched for this member: git was never run with it.`), the dry run says the same, and the other members are placed. - The warning's prose printed the holder, the parent folder and the family raw on stderr (an ESC in a folder name reached the terminal). Each is now `spelled`; the first line keeps its `repr`, and the `mv` lines stay as typed (#486's question). - `submodule_urls` read `git config --list` as text, split by `str.splitlines` and at the first `=`: a url holding NEL, U+2028, U+2029, VT, FF or U+001C to U+001E was cut there, and `[submodule "members/a=b"]` gave no url, so its member was cloned from the `repository:` guess. It now reads `git config -z --list`, records cut at NUL and each key from its value at the first newline, a section's name being all of its key between `submodule.` and the last dot: the reading `validate-family.py` makes and lane 3's #668 gives the doctor (#146). - A `tracking_branch:` naming only a tag of the member's remote was cloned DETACHED by `git clone --branch`, under a line saying "on , its tracking branch", exit 0. A line after the clone now says ` is not a branch of but a tag, which git clone checks out DETACHED: this clone is on no branch`. The exit is unchanged, as the issue asks. `tests/test_siblings_inputs.py` covers each line with the issue's inputs, runs the printed moves as typed in a POSIX shell, and holds the parity of the doctor's and `make siblings`' readings over one corpus (#668's names and breaks, and its Latin-1 name), skipped by name until #668's `gitmodules_listing` is in the doctor; it passes against PR #700's head. No doc sentence is made false, and no `--help` changes. The holder's header spelling U664 pins is untouched. Closes #690 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- templates/family-root/scripts/siblings.py | 132 +++++- tests/test_siblings_inputs.py | 531 ++++++++++++++++++++++ 2 files changed, 649 insertions(+), 14 deletions(-) create mode 100644 tests/test_siblings_inputs.py diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index c41e41f6..2ff3b93d 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -233,23 +233,37 @@ def submodule_urls(root: Path) -> dict[str, str]: from the manifest instead would clone a different remote from the one this holder is pinned against, silently, in exactly the estates that do not use the default spelling. + + AND READ AS GIT READS IT (#690): `git config -z --list`, the reading + `validate-family.py`'s `_gitmodules_sections` makes and #668 gives the + doctor's `members` row, so the two tools name one url for one member + (#146). Each record ends in a NUL and is the key, a newline and the + value; a key holds no newline and a value may, so a record is split at + its FIRST one. The plain listing this read was split by `str.splitlines` + and at the first `=`: a url holding NEL, U+2028, U+2029, VT, FF or + U+001C to U+001E, which git keeps whole, was cut there (`/caf` + of `cafe/IRRS.git`), and a section named `members/a=b` gave no url, + so its member was cloned from the `repository:` guess. A section's NAME + is all of its key between `submodule.` and the last dot, so a name + holding a space, a dot, a quote or an `=` is read whole; only its `path` + and `url` count, and the later value wins, as git reads the file. A + byte that is not UTF-8 is still replaced (#432). """ path = root / ".gitmodules" if not path.is_file(): return {} - proc = subprocess.run(["git", "config", "-f", str(path), "--list"], - capture_output=True, text=True, - encoding="utf-8", errors="replace", check=False) + proc = subprocess.run(["git", "config", "-z", "-f", str(path), "--list"], + capture_output=True, check=False) if proc.returncode != 0: return {} paths: dict[str, str] = {} urls: dict[str, str] = {} - for line in proc.stdout.splitlines(): - key, _, value = line.partition("=") - parts = key.split(".") - if len(parts) < 3 or parts[0] != "submodule": + for record in proc.stdout.decode("utf-8", "replace").split("\0"): + key, _, value = record.partition("\n") + section, _, field = key.rpartition(".") + if not section.startswith("submodule."): continue - name, field = ".".join(parts[1:-1]), parts[-1] + name = section[len("submodule."):] if field == "path": paths[name] = value elif field == "url": @@ -526,9 +540,47 @@ def run_member_bootstrap(path: Path, sibling: Sibling) -> None: "legs may not be placed") +def argument_fault(value: str) -> str | None: + """The character that stops `value` being an argument to a command, or + None (#690): #455's rule, as `make bootstrap` asks it of a branch. + + `place` hands a member's `project:` (as the clone's path) and its url + to `git clone` as argv, and `subprocess` cannot give a command a NUL + (`ValueError: embedded null byte`) or a lone surrogate the system cannot + encode (`UnicodeEncodeError`): `project: "IR\\ud83dRS"` ended `make + siblings` in a traceback at the clone, and the members after it were + never placed. A surrogate that is ONE BYTE of a name (POSIX spells a + name that is not UTF-8 with U+DC80 to U+DCFF) is handed on as that + byte, as before; whether one is, is `os.fsencode`'s own answer. + """ + for char in value: + if char == "\x00": + return "U+0000, a NUL" + if "\ud800" <= char <= "\udfff" and _bytes_for(char) != 1: + return f"U+{ord(char):04X}, a lone surrogate" + return None + + +def _bytes_for(char: str) -> int: + """How many bytes `os.fsencode` writes for `char`; 0 when it cannot.""" + try: + return len(os.fsencode(char)) + except UnicodeEncodeError: + return 0 + + def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], token: str, dry_run: bool) -> Sibling: - """Clone or fetch ONE member beside the holder. Never moves a HEAD.""" + """Clone or fetch ONE member beside the holder. Never moves a HEAD. + + A ROW NO COMMAND CAN BE GIVEN IS A FINDING, BEFORE ANYTHING (#690): a + `project:` or url holding a NUL or a lone surrogate (`argument_fault`) + is never made a path or handed to git, the dry run says so alike, and + the members after it are still placed. And a `tracking_branch:` that + names only a TAG of the member's remote is cloned DETACHED at it, which + `git clone --branch` does with a tag, so the line after the clone says + the name is no branch of that remote. + """ project = str(row.get("project") or "?") repository = str(row.get("repository") or "") target = root.parent / project @@ -539,6 +591,17 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], # `project`, `target` and `url` stay what the paths and git are given. who, at, src = spelled(project), spelled(target), spelled(url) + for named, value in ((f"its `project:` in {MANIFEST}", project), + (f"its url {src} ({source})", url)): + fault = argument_fault(value) + if fault is not None: + sibling.state = "NOT AN ARGUMENT" + sibling.finding = ( + f"{who}: {named} holds {fault}, which no command can be " + "given as an argument, so nothing was cloned or fetched for " + "this member: git was never run with it.") + return sibling + if target == root: # A MEMBER NAMED AFTER ITS OWN FAMILY. `/` is the # holder, so this member's sibling would BE the holder — and this @@ -588,6 +651,10 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], + scrub((proc.stderr + proc.stdout).strip(), token)) return sibling sibling.state = "cloned" + if branch and branch_of(target) == "detached": + print(f" [{who}] {spelled(branch)} is not a branch of {src} but " + "a tag, which git clone checks out DETACHED: this clone is " + "on no branch") run_member_bootstrap(target, sibling) sibling.branch = branch_of(target) return sibling @@ -935,6 +1002,23 @@ def family_link(root: Path, family: str) -> Path | None: return None +def staging_path(parent: Path, name: str) -> Path: + """`/`, or the first `/.` (n = 1, 2, ...) + at which nothing is: a name one step of the printed move passes through + (#690). + + `os.path.lexists`, which follows no link: a link to nothing is + something there, and so is the name in another case on a disk that + ignores case. `mv` onto what is there does not make the name: it moves + the holder INTO a folder, or fails on a link or a file. + """ + staged, number = parent / name, 0 + while os.path.lexists(staged): + number += 1 + staged = parent / f"{name}.{number}" + return staged + + def parent_folder_warning(root: Path, family: str) -> str | None: """The WARNING and the exact `mv`, or None when the layout is right. @@ -964,6 +1048,18 @@ def parent_folder_warning(root: Path, family: str) -> str | None: check (`family_link`): it is the family folder, reached through a link. One that leads elsewhere does not, and its move runs as typed: the holder moves into the link's target, where the link then passes. + + AND NOTHING ALREADY THERE SENDS IT ASTRAY (#690). A staging name was + printed whether or not something held it: with `.holder` a + directory already, `mv` put the holder INSIDE it, and the three lines + all exited 0 and left it at `//`, one level too + deep. Each staging name is now the first of `.holder`, + `.holder.1`, `.holder.2` ... (`.folder` alike) at which + nothing is (`staging_path`). A LINK TO NOTHING named `` is not + a folder to move into: `mkdir -p` fails on it, after the holder's first + move has run. It is named, to be moved aside first, and the moves under + it are the ones that run once it is gone. The prose names the holder, + the folders and the family spelled (#454); the commands stay as typed. """ folder = root.parent if folder.name == family or family_link(root, family) is not None: @@ -971,12 +1067,12 @@ def parent_folder_warning(root: Path, family: str) -> str | None: lines = [ f"WARNING the holder's parent folder is {folder.name or str(folder)!r}" f", not {family!r}.", - f" {root} should sit in a plain folder named {family}, beside " - "the members'", + f" {spelled(root)} should sit in a plain folder named " + f"{spelled(family)}, beside the members'", " working clones — that is the layout `make siblings` places. " "So", " NOTHING WAS CLONED: a member cloned now would land in " - f"{folder}, and", + f"{spelled(folder)}, and", " the re-run after the move would clone it a second time, " "beside the", " moved holder.", @@ -997,7 +1093,9 @@ def parent_folder_warning(root: Path, family: str) -> str | None: # `/` IS the holder itself and the move needs a # staging name — `mv A A/A` cannot be spelled in one command. At a # filesystem or drive root it is the holder even though it exists. - staged = folder.parent / f"{family}.holder" + # A LINK TO NOTHING named is here too: `exists` follows it, + # and these are the moves once it is moved aside (said below). + staged = staging_path(folder.parent, f"{family}.holder") steps = [["mv", root, staged], ["mkdir", "-p", wanted], ["mv", staged, wanted / family]] elif (folder.name.casefold() == family.casefold() @@ -1009,10 +1107,16 @@ def parent_folder_warning(root: Path, family: str) -> str | None: # differ by disk at one edge: a LINK spelled in another case # (`inkrouter` to `real-x`) stops on Linux, with a staged move that # runs, and passes as the family's link on a disk that ignores case. - staged = folder.parent / f"{family}.folder" + staged = staging_path(folder.parent, f"{family}.folder") steps = [["mv", folder, staged], ["mv", staged, wanted]] else: steps = [["mkdir", "-p", wanted], ["mv", root, wanted / family]] + if os.path.lexists(wanted) and not os.path.exists(wanted): + lines += [f" First move {spelled(wanted)} aside: it is a link " + "to nothing,", + " named after the family, and in the way of the " + "move. Then:", + ""] # EVERY PATH QUOTED (#486): under `my work/` a bare `mv` moved nothing # and pasted as four arguments. Quoted for the POSIX shell `mv` and # `mkdir -p` are commands of, as the sentence above says, by the standard diff --git a/tests/test_siblings_inputs.py b/tests/test_siblings_inputs.py new file mode 100644 index 00000000..26ebc6b8 --- /dev/null +++ b/tests/test_siblings_inputs.py @@ -0,0 +1,531 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`make siblings` either clones each member where it says, or stops before +cloning with a move that runs as typed, and no manifest or disk text ends it +in a traceback or a forged line (#690). + +Six lines of `templates/family-root/scripts/siblings.py`, a pinned copy in +every family holder, fixed in one change and one re-pin: + +* AN EXISTING STAGING NAME (#578's line). The parent-folder warning printed + its `mv` through `.holder` or `.folder` whatever was there, + and `mv` onto a folder moves INTO it: with `/InkRouter.holder` a folder + already, the three printed lines all exited 0 and left the holder at + `/InkRouter/InkRouter/InkRouter`, one level too deep. The staging name + is now the first of `.holder`, `.holder.1`, ... at which nothing is. +* A DANGLING LINK NAMED AFTER THE FAMILY (#578's line). `exists` follows the + link, so the staged move was printed, its first `mv` ran, and `mkdir -p` + failed `File exists` with the holder half moved. The link is now named, to + be moved aside first. +* A `project:` OR URL HOLDING A NUL OR A LONE SURROGATE (R454 O1). `git + clone` was handed it: `UnicodeEncodeError` or `ValueError: embedded null + byte`, a traceback, and the members after it never placed. It is now a + finding, the dry run's too, and the other members go on. +* THE WARNING'S PROSE (R454 O2) printed the holder, the parent folder and the + family raw on stderr: an ESC in a folder name reached the terminal. They + are spelled; the `mv` lines stay as typed (#486's question). +* `.gitmodules` READ AS GIT READS IT (R432 O2, and lane 3's line on #473): + `str.splitlines` cut a url at NEL or U+2028, and the split at the first + `=` read no url for a section named `members/a=b`. It is now `git config + -z --list`, as `validate-family.py` reads the file and #668 makes the + doctor read it. +* A `tracking_branch:` THAT IS ONLY A TAG (#578's line): `git clone --branch` + takes a tag and leaves the clone detached, under a line that said "on + , its tracking branch". A line after the clone now says so. + +THE SAME FAMILY AS `test_family.py` (its module fixture, this module's own +instance), and the move asked in this process as +`test_family_siblings_placement.py` asks it, with that module's helpers. The +printed moves are RUN AS TYPED in a POSIX shell: `os.rename` onto an empty +folder replaces it where `mv` moves into it, so a rename would not show the +defect. Output is split at `\\n` alone, never `str.splitlines`, which breaks +a command line at the U+2028 one of these tests puts in it. Files are edited +as bytes. NO NETWORK: local bare remotes, and a dry run wherever a run of the +old code would have asked GitHub. +""" + +from __future__ import annotations + +import os +import re +import shlex +import shutil +import subprocess +from pathlib import Path + +import pytest + +from conftest import REPO, clone_just_pushed, git, load_without_bytecode +from test_bootstrap_spelling import NEVER_RAW +from test_family import NAME, family, siblings # noqa: F401 +from test_family_siblings_placement import ( # noqa: F401 + branch_of, flat, holder_in, ignoring_case, link_or_skip, placement, + stored, +) + +#: The member whose row, mount or url a test bends; IRSS is left as it is, +#: so a run that goes on past IRRS is seen to. +BENT = "IRRS" +KEPT = "IRSS" +#: The row `family.py add` wrote for BENT. +ROW = (" - project: IRRS\n id: irrs\n repository: InkRouter/IRRS\n" + " path: members/IRRS\n") +ESC = "\x1b" + + +def printed_moves(text: str) -> list[str]: + """The `mv` and `mkdir -p` lines of the warning, as printed.""" + return [line.strip() for line in text.split("\n") + if line.startswith(" " * 12) and line.strip()[:3] in + ("mv ", "mkd")] + + +def moves(text: str) -> list[list[str]]: + """The same lines, split as a POSIX shell splits them.""" + return [shlex.split(line) for line in printed_moves(text)] + + +def run_as_typed(lines: list[str]) -> None: + """Each printed line run in a POSIX shell, as a person pastes it.""" + shell = shutil.which("sh") + if shell is None: + pytest.skip("no POSIX shell here to paste the printed move into") + proc = subprocess.run([shell, "-e", "-c", "\n".join(lines)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + + +def edit(path: Path, old: str, new: str) -> None: + """`old` replaced by `new` in `path`, which holds it exactly once; as + bytes, so no newline is translated.""" + data = path.read_bytes() + assert data.count(old.encode("utf-8")) == 1, data + path.write_bytes(data.replace(old.encode("utf-8"), new.encode("utf-8"))) + + +def gitmodules_url(holder: Path, project: str) -> str: + """The url `.gitmodules` gives `members/`, as git reads it.""" + return git("config", "-f", ".gitmodules", "--get", + f"submodule.members/{project}.url", + cwd=holder).stdout.rstrip("\n") + + +def table_row(stdout: str, project: str) -> list[str]: + """The layout table's `sibling` row for `project`, split.""" + rows = [line.split() for line in stdout.split("\n") + if line.split()[:2] == ["sibling", project]] + assert len(rows) == 1, stdout + return rows[0] + + +# --- an existing staging name ------------------------------------------------ + + +@pytest.mark.parametrize("taken, staged", [ + pytest.param(["a folder"], ".holder.1", id="a-folder"), + pytest.param(["a link to nothing"], ".holder.1", id="a-link-to-nothing"), + pytest.param(["a folder", "a folder"], ".holder.2", id="two-names"), +]) +def test_a_staging_name_something_holds_is_passed_over(family, tmp_path, + taken, staged): + """The issue's probe: the holder at `/t/InkRouter` and + `/InkRouter.holder` already a folder. The printed move passes through + the first staging name nothing holds, runs as typed, and leaves the + holder at `/InkRouter/InkRouter`, where the next run passes; what + held the names is untouched. A link to nothing holds a name too, and so + does a second name taken after the first.""" + holder = holder_in(family, tmp_path / "t") + held = [tmp_path / (f"{NAME}.holder" + (f".{n}" if n else "")) + for n in range(len(taken))] + for what, path in zip(taken, held): + if what == "a folder": + path.mkdir() + (path / "keep.txt").write_bytes(b"mine\n") + else: + link_or_skip(path, tmp_path / "nowhere") + + result = siblings(holder) + assert result.returncode == 1, result.stdout + result.stderr + wanted = tmp_path / NAME + passing = tmp_path / f"{NAME}{staged}" + assert moves(result.stderr) == [ + ["mv", str(holder), str(passing)], ["mkdir", "-p", str(wanted)], + ["mv", str(passing), str(wanted / NAME)]] + assert holder.is_dir(), "the run moves nothing" + + run_as_typed(printed_moves(result.stderr)) + assert (wanted / NAME / "family.yaml").is_file() + for what, path in zip(taken, held): + if what == "a folder": + assert sorted(os.listdir(path)) == ["keep.txt"], path + else: + assert os.path.islink(path), path + again = siblings(wanted / NAME, "--dry-run") + assert again.returncode == 0, again.stdout + again.stderr + + +def ignoring_case_through_os_path(patched) -> None: + """`ignoring_case`, with `os.path.lexists` and `os.path.exists` answering + as it does: the staging name and the dangling link are asked of + `os.path`, which a disk that ignores case answers alike.""" + ignoring_case(patched) + patched.setattr(os.path, "lexists", lambda path: stored(path) is not None) + patched.setattr(os.path, "exists", lambda path: stored(path) is not None) + + +@pytest.mark.parametrize("spelling", [ + pytest.param(f"{NAME}.folder", id="same-case"), + pytest.param(f"{NAME.lower()}.folder", id="another-case"), +]) +def test_a_folder_staging_name_something_holds_is_passed_over( + placement, monkeypatch, tmp_path, spelling): + """macOS and Windows, simulated: the holder in `inkrouter/`, renamed + through `InkRouter.folder`, which R578's writer found already there: the + move left `InkRouter/inkrouter/InkRouter`. Held under either case, the + name is passed over on a disk that ignores case, and the two lines run + as typed.""" + folder = tmp_path / NAME.lower() + root = folder / NAME + root.mkdir(parents=True) + there = tmp_path / spelling + there.mkdir() + (there / "keep.txt").write_bytes(b"mine\n") + with monkeypatch.context() as patched: + ignoring_case_through_os_path(patched) + warning = placement.parent_folder_warning(root, NAME) + passing = tmp_path / f"{NAME}.folder.1" + assert moves(warning) == [["mv", str(folder), str(passing)], + ["mv", str(passing), str(tmp_path / NAME)]] + assert "link to nothing" not in warning + + run_as_typed(printed_moves(warning)) + assert (tmp_path / NAME / NAME).is_dir() + assert sorted(os.listdir(there)) == ["keep.txt"] + + +# --- a dangling link named after the family ---------------------------------- + + +def test_a_link_to_nothing_named_after_the_family_is_named_first(family, + tmp_path): + """The issue's probe: `/InkRouter` a link to nothing, the holder at + `/t/InkRouter`. The run, and its dry run alike, stop with the link + named before the moves and nothing moved; moved aside, the printed + lines run as typed and the next run passes.""" + holder = holder_in(family, tmp_path / "t") + link = tmp_path / NAME + link_or_skip(link, tmp_path / "nowhere") + staged = tmp_path / f"{NAME}.holder" + for args in ((), ("--dry-run",)): + result = siblings(holder, *args) + assert result.returncode == 1, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert (f"First move {link} aside: it is a link to nothing, named " + "after the family, and in the way of the move. Then: mv " + in flat(result.stderr)), result.stderr + assert moves(result.stderr) == [ + ["mv", str(holder), str(staged)], ["mkdir", "-p", str(link)], + ["mv", str(staged), str(link / NAME)]] + assert os.path.islink(link) and holder.is_dir(), "nothing moved" + + os.rename(link, tmp_path / "moved-aside") + run_as_typed(printed_moves(result.stderr)) + assert (link / NAME / "family.yaml").is_file() + again = siblings(link / NAME, "--dry-run") + assert again.returncode == 0, again.stdout + again.stderr + + +# --- a row no command can be given ------------------------------------------- + +#: `(the row written for BENT, the name its lines print, what the finding +#: says holds the character)`: R454's two `project:` values, and a url built +#: from a `repository:` holding a NUL, for a row whose path `.gitmodules` +#: does not give (so `git clone` was handed the GitHub url it builds). +UNARGUABLE = [ + pytest.param(ROW.replace("project: IRRS", r'project: "IR\ud83dRS"'), + r"IR\ud83dRS", "its `project:` in family.yaml holds U+D83D, " + "a lone surrogate", id="project-lone-surrogate"), + pytest.param(ROW.replace("project: IRRS", r'project: "IR\x00RS"'), + r"IR\x00RS", "its `project:` in family.yaml holds U+0000, a " + "NUL", id="project-nul"), + pytest.param(ROW.replace("repository: InkRouter/IRRS", + r'repository: "InkRouter/IR\x00RS"') + .replace("path: members/IRRS", "path: members/Elsewhere"), + BENT, r"its url https://github.com/InkRouter/IR\x00RS.git " + "(family.yaml repository:) holds U+0000, a NUL", + id="url-nul"), +] + + +@pytest.mark.parametrize("row, shown, holds", UNARGUABLE) +def test_a_row_no_command_can_be_given_is_a_finding_and_the_rest_go_on( + family, tmp_path, row, shown, holds): + """`make siblings` over such a row exited 1 in a traceback at `git + clone`, and IRSS, after it, was never cloned; the dry run said `would + clone`. Both now name the row in one finding and exit 1, nothing is + made for it, and the real run still clones IRSS.""" + holder = holder_in(family, tmp_path / NAME) + edit(holder / "family.yaml", ROW, row) + finding = (f"FINDING {shown}: {holds}, which no command can be given as " + "an argument, so nothing was cloned or fetched for this " + "member: git was never run with it.") + for args, made in ((("--dry-run",), [NAME]), ((), sorted([NAME, KEPT]))): + result = siblings(holder, *args) + assert result.returncode == 1, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert finding in result.stderr.split("\n"), result.stderr + assert table_row(result.stdout, shown) == [ + "sibling", shown, "NOT", "AN", "ARGUMENT", "-"] + assert sorted(os.listdir(holder.parent)) == made + assert branch_of(holder.parent / KEPT) == "main" + + +@pytest.mark.parametrize("value, fault", [ + pytest.param("IRRS", None, id="plain"), + pytest.param("IR\x00RS", "U+0000, a NUL", id="nul"), + pytest.param("IR\ud83dRS", "U+D83D, a lone surrogate", + id="lone-surrogate"), + pytest.param("IR\udcffRS", None, marks=pytest.mark.skipif( + os.name == "nt", reason="Windows' os.fsencode writes three bytes for " + "every surrogate, so none is one byte of a name there"), + id="one-byte-of-a-posix-name"), +]) +def test_the_argument_rule_is_bootstraps(placement, value, fault): + """#455's rule, as `make bootstrap` asks it of a branch: a NUL, and a + surrogate `os.fsencode` cannot write as one byte. A name that is not + UTF-8 on POSIX (U+DC80 to U+DCFF) is still handed on, as before.""" + assert placement.argument_fault(value) == fault + + +# --- the warning's prose is spelled ------------------------------------------ + + +def test_the_warning_spells_the_holder_and_the_folder_it_names( + family, tmp_path): + """R454's probe: a parent folder named `x[31my`. Lines 2 and 4 of + the warning carried the raw ESC; they now spell it, and the `mv` line, + which a person pastes, is the one line that still holds it, as typed.""" + if os.name == "nt": + pytest.skip("a Windows disk holds no ESC in a folder name") + holder = holder_in(family, tmp_path / f"x{ESC}[31my") + escaped = "\\x1b" + shown_root = str(holder).replace(ESC, escaped) + shown_folder = str(holder.parent).replace(ESC, escaped) + result = siblings(holder) + assert result.returncode == 1, result.stdout + result.stderr + lines = result.stderr.split("\n") + assert lines[1] == (f" {shown_root} should sit in a plain folder " + f"named {NAME}, beside the members'"), lines + assert lines[3] == (" NOTHING WAS CLONED: a member cloned now " + f"would land in {shown_folder}, and"), lines + raw = [line.strip() for line in lines if ESC in line] + assert raw == printed_moves(result.stderr)[:1], raw + assert moves(result.stderr)[0] == [ + "mv", str(holder), str(tmp_path / f"{NAME}.holder")] + assert not NEVER_RAW & set(result.stdout), result.stdout + + +def test_the_warning_spells_the_family_it_names(family, tmp_path): + """And the family, the basename of `repository:`, which the first line + shows in its `repr` and the summary under it spelled: line 2 printed it + raw. A U+2028 in it now prints as its escape on every line but the + commands, which keep it as typed.""" + holder = holder_in(family, tmp_path / NAME) + edit(holder / "family.yaml", f"\nrepository: {NAME}/{NAME}\n", + f'\nrepository: "{NAME}/Ink\\u2028Router"\n') + result = siblings(holder, "--dry-run") + assert result.returncode == 1, result.stdout + result.stderr + commands = printed_moves(result.stderr) + prose = [line for line in result.stderr.split("\n") + if line.strip() not in commands] + assert prose[0] == (f"WARNING the holder's parent folder is '{NAME}', " + r"not 'Ink\u2028Router'."), prose + assert prose[1].endswith(r" should sit in a plain folder named " + r"Ink\u2028Router, beside the members'"), prose + assert not NEVER_RAW & set("\n".join(prose)), prose + family_folder = tmp_path / "Ink\u2028Router" + assert moves(result.stderr) == [ + ["mkdir", "-p", str(family_folder)], + ["mv", str(holder), str(family_folder / "Ink\u2028Router")]] + + +# --- .gitmodules read as git reads it ---------------------------------------- + +#: Section NAMES, as the bytes between the header's quotes, each holding a +#: character a reading of git's listing has cut at: a space, a dot, a quote +#: (escaped in the header) and an `=` (lane 3's `members/a=b`). The same four +#: as #668's corpus for the doctor. +NAMES = [ + pytest.param(b"members/My IRRS", id="space"), + pytest.param(b"members/IRRS.v2", id="dot"), + pytest.param(b'members/\\"IRRS\\"', id="quote"), + pytest.param(b"members/a=b", id="equals"), +] +#: A url through a directory holding each line break `str.splitlines` knows +#: besides `\n` and `\r`, all of which git keeps in a value: VT, FF, FS, GS, +#: RS, NEL, U+2028 and U+2029, in UTF-8 (R432 O2's list). +BREAKS = [pytest.param(f"caf{char}e".encode("utf-8"), + id=f"U+{ord(char):04X}") + for char in "\x0b\x0c\x1c\x1d\x1e\x85\u2028\u2029"] + + +def write_section(folder: Path, name: bytes, directory: bytes) -> bytes: + """A `.gitmodules` in `folder` whose one section, NAMED `name`, mounts + `members/IRRS` from `/r//IRRS.git`; that url, as written.""" + url = b"/r/" + directory + b"/IRRS.git" + (folder / ".gitmodules").write_bytes( + b'[submodule "' + name + b'"]\n\tpath = members/IRRS\n\turl = ' + + url + b"\n") + return url + + +def one_section(tmp_path: Path, name: bytes, directory: bytes) -> str: + """`write_section`, and its url as git reads it by its exact key; + skipped where git does not keep it whole.""" + url = write_section(tmp_path, name, directory) + key = "submodule." + name.replace(b'\\"', b'"').decode("ascii") + ".url" + read = subprocess.run(["git", "config", "-f", ".gitmodules", "--get", + key], cwd=str(tmp_path), capture_output=True, + check=False) + if read.stdout.rstrip(b"\n") != url: + pytest.skip(f"git here reads {key} as {read.stdout!r}, not {url!r}") + return url.decode("utf-8") + + +@pytest.mark.parametrize("name", NAMES) +def test_a_section_name_git_reads_whole_gives_its_url(placement, tmp_path, + name): + """`members/a=b` gave `{}`: the split at the first `=` cut the key. Each + name now gives the mount its url, as git reads it.""" + url = one_section(tmp_path, name, b"IRRS") + assert placement.submodule_urls(tmp_path) == {"members/IRRS": url} + + +@pytest.mark.parametrize("directory", BREAKS) +def test_a_url_holding_a_line_break_git_keeps_is_read_whole(placement, + tmp_path, + directory): + """R432 O2: the url was cut at the break, `/r/caf`.""" + url = one_section(tmp_path, b"members/IRRS", directory) + assert placement.submodule_urls(tmp_path) == {"members/IRRS": url} + + +def test_make_siblings_clones_from_a_url_holding_nel(family, tmp_path): + """A real run, the member's remote at `cafe/IRRS.git`: cut at the + NEL, the clone of `/caf` failed (`CLONE FAILED`). It now clones from + the whole url, which is the clone's `origin`.""" + holder = holder_in(family, tmp_path / NAME) + remote = tmp_path / "caf\u0085e" / "IRRS.git" + try: + remote.parent.mkdir() + except OSError as refused: + pytest.skip(f"this disk holds no NEL in a folder name: {refused}") + made = clone_just_pushed(family["remotes"] / "IRRS.git", remote, + recurse=False, bare=True) + assert made.returncode == 0, made.stderr + config = holder / ".gitmodules" + rewritten, count = re.subn( + rb"(?m)^(\turl = ).*/IRRS\.git$", + lambda found: found.group(1) + remote.as_posix().encode("utf-8"), + config.read_bytes()) + assert count == 1 + config.write_bytes(rewritten) + + result = siblings(holder) + assert result.returncode == 0, result.stdout + result.stderr + clone = holder.parent / BENT + assert git("remote", "get-url", "origin", + cwd=clone).stdout.rstrip("\n") == remote.as_posix() + assert table_row(result.stdout, BENT)[2] == "cloned" + assert not NEVER_RAW & set(result.stdout), result.stdout + + +def test_a_section_named_with_an_equals_sign_mounts_from_gitmodules( + family, tmp_path): + """Lane 3's `[submodule "members/a=b"]`: no url was read for it, so the + dry run named `https://github.com/InkRouter/IRRS.git (family.yaml + repository:)`, the remote this holder is not pinned against. It now + names the url `.gitmodules` gives. A dry run, so the old reading never + asks GitHub.""" + holder = holder_in(family, tmp_path / NAME) + url = gitmodules_url(holder, BENT) + edit(holder / ".gitmodules", f'[submodule "members/{BENT}"]', + '[submodule "members/a=b"]') + result = siblings(holder, "--dry-run") + assert result.returncode == 0, result.stdout + result.stderr + assert (f" [{BENT}] would clone {url} (.gitmodules) -> " + f"{holder.parent / BENT}, on main, its tracking branch " + f"(members/{BENT}/project.yaml)") in result.stdout.split("\n") + + +# --- one reading, the doctor's and make siblings' (#146, #668) --------------- + + +@pytest.fixture(scope="module") +def doctor(): + """`shape-doctor.py`, as a module, for its `submodule_config`: the + doctor's half of the one reading. Lane 3's #668 (PR #700) makes it `git + config -z --list`, cut as this file's half cuts it; whichever of the two + lands second adds this parity (#473, issuecomment-6216310747). Until + #668's `gitmodules_listing` is in the doctor here, the parity is + skipped, by name: the doctor's old reading cuts the same corpus.""" + module = load_without_bytecode(REPO / "shape-doctor.py", + "siblings_inputs_doctor") + if not hasattr(module, "gitmodules_listing"): + pytest.skip("shape-doctor.py here reads `.gitmodules` with " + "--get-regexp; the parity runs once #668 (PR #700) is in") + return module + + +#: One corpus for both readers: the names and the breaks above, and #668's +#: section NAMED in Latin-1, which a UTF-8 locale kept the doctor's +#: `--get-regexp` from matching. +CORPUS = ([pytest.param(name.values[0], b"IRRS", id=f"name-{name.id}") + for name in NAMES] + + [pytest.param(b"members/IRRS", directory.values[0], + id=f"url-{directory.id}") for directory in BREAKS] + + [pytest.param(b"members/caf\xe9", b"IRRS", id="name-latin-1")]) + + +@pytest.mark.parametrize("name, directory", CORPUS) +def test_the_doctor_and_make_siblings_read_one_url_for_one_mount( + placement, doctor, tmp_path, monkeypatch, name, directory): + """The doctor's `members` row and `make siblings` must name one url for + one member, or they disagree about one directory (#146): read through + both, every case of the corpus gives the same `{path: url}`, under a + UTF-8 locale.""" + monkeypatch.setenv("LC_ALL", "C.UTF-8") + write_section(tmp_path, name, directory) + config = doctor.submodule_config(tmp_path) + read = {fields["path"]: fields["url"] for fields in config.values()} + assert read == placement.submodule_urls(tmp_path), config + assert list(read) == ["members/IRRS"], config + + +# --- a tracking branch that is only a tag ------------------------------------ + +TAG = "v-track" + + +def test_a_tracking_branch_that_is_only_a_tag_says_the_clone_is_detached( + family, tmp_path): + """IRSS declares `tracking_branch: "v-track"`, a tag of its remote and no + branch: `git clone --branch` took the tag and left the clone detached, + under a line saying "on v-track, its tracking branch", exit 0. The run + still places it, and a line after the clone says the name is no branch + of that remote; IRRS, on a real branch, gets no such line.""" + git("tag", "-f", TAG, "main", cwd=family["remotes"] / f"{KEPT}.git") + holder = holder_in(family, tmp_path / NAME) + edit(holder / "members" / KEPT / "project.yaml", + 'tracking_branch: "main"', f'tracking_branch: "{TAG}"') + url = gitmodules_url(holder, KEPT) + + result = siblings(holder) + assert result.returncode == 0, result.stdout + result.stderr + said = [line for line in result.stdout.split("\n") + if "is not a branch of" in line] + assert said == [f" [{KEPT}] {TAG} is not a branch of {url} but a tag, " + "which git clone checks out DETACHED: this clone is on " + "no branch"], result.stdout + assert table_row(result.stdout, KEPT)[2:] == ["cloned", "detached"] + assert branch_of(holder.parent / KEPT) == "HEAD" + assert branch_of(holder.parent / BENT) == "main" From 25c7b14c23e2f640be69b8d8ee2c96416530ece9 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 10:34:39 +0000 Subject: [PATCH 2/4] tests/test_siblings_inputs.py: `[submodule "a=b"]` mounts and a CR in a section name cuts no key, the two cases lane 3 asked #690's rows for (#690) Lane 3's two comments on #690 (issuecomment-6232404184 and -6232689453) name two cases the `git config -z --list` read of `submodule_urls` already fixes on main's reading, and ask for a row each: - `[submodule "a=b"]`, the name as lane 3 wrote it. Main's read split each `--list` line at the first `=`, so the key read as `submodule.a`, failed the three-part test and was dropped; git, the doctor and CI mount it. `a=b` joins NAMES (the url read, and the doctor's parity over the same corpus) and the dry-run row that already held `members/a=b`. - A CR inside a section name, `[submodule "q\rx"]`, with `update = bogus`: `str.splitlines` cut that key in two, so the section was read as no section. `q\rx` joins NAMES, and a row reads the file with the CR section and with the same section named `qx`, each body holding `update = bogus`, and asserts git has each key whole by its exact name and that the two files read alike. What a reading does with a value git refuses (`git status` dies `invalid value for 'submodule.qx.update'`, with or without the CR) is lane 3's #733, so the row compares the two readings and pins no url. Run against the old reading (M1), the new rows fail: `equals-bare`, `cr`, the CR row, the `a=b` dry run, and the doctor parity for both names. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_siblings_inputs.py | 54 +++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/tests/test_siblings_inputs.py b/tests/test_siblings_inputs.py index 26ebc6b8..5575946c 100644 --- a/tests/test_siblings_inputs.py +++ b/tests/test_siblings_inputs.py @@ -352,13 +352,17 @@ def test_the_warning_spells_the_family_it_names(family, tmp_path): #: Section NAMES, as the bytes between the header's quotes, each holding a #: character a reading of git's listing has cut at: a space, a dot, a quote -#: (escaped in the header) and an `=` (lane 3's `members/a=b`). The same four -#: as #668's corpus for the doctor. +#: (escaped in the header) and an `=` (lane 3's `members/a=b`), the same four +#: as #668's corpus for the doctor; then lane 3's two more on #690 +#: (issuecomment-6232404184 and -6232689453): the name `a=b` as it wrote it, +#: whole, and a CR, which `str.splitlines` cut a key at where git keeps it. NAMES = [ pytest.param(b"members/My IRRS", id="space"), pytest.param(b"members/IRRS.v2", id="dot"), pytest.param(b'members/\\"IRRS\\"', id="quote"), pytest.param(b"members/a=b", id="equals"), + pytest.param(b"a=b", id="equals-bare"), + pytest.param(b"members/q\rx", id="cr"), ] #: A url through a directory holding each line break `str.splitlines` knows #: besides `\n` and `\r`, all of which git keeps in a value: VT, FF, FS, GS, @@ -409,6 +413,36 @@ def test_a_url_holding_a_line_break_git_keeps_is_read_whole(placement, assert placement.submodule_urls(tmp_path) == {"members/IRRS": url} +def test_a_cr_in_a_section_name_cuts_no_key_of_its_section(placement, + tmp_path): + """Lane 3's second case on #690 (issuecomment-6232689453): `[submodule + "q\\rx"]` holding `update = bogus`. `str.splitlines` cut the listing at + the CR, `submodule.q` from `x.path`, so the section was read as no + section at all and the `update` git refuses never reached a reader that + looks for it (lane 3's #733). Git keeps the key whole, and so does the + `-z` reading: the section with the CR is read exactly as the same + section without it. What the reading then does with a value git refuses + is #733's rule, not this one, so the two files are compared, not the + urls pinned.""" + irrs = (b'[submodule "members/IRRS"]\n\tpath = members/IRRS\n' + b"\turl = /r/IRRS.git\n") + body = b"\tpath = members/q\n\turl = /r/q.git\n\tupdate = bogus\n" + read = {} + for label, name in (("cr", b"q\rx"), ("plain", b"qx")): + folder = tmp_path / label + folder.mkdir() + (folder / ".gitmodules").write_bytes( + irrs + b'[submodule "' + name + b'"]\n' + body) + # Git has the section whole: each key by its exact name, CR and all. + for field, expect in (("url", "/r/q.git"), ("update", "bogus")): + got = git("config", "-f", ".gitmodules", "--get", + f"submodule.{name.decode('ascii')}.{field}", + cwd=folder) + assert got.stdout.rstrip("\n") == expect, (label, field) + read[label] = placement.submodule_urls(folder) + assert read["cr"] == read["plain"], read + + def test_make_siblings_clones_from_a_url_holding_nel(family, tmp_path): """A real run, the member's remote at `cafe/IRRS.git`: cut at the NEL, the clone of `/caf` failed (`CLONE FAILED`). It now clones from @@ -439,17 +473,19 @@ def test_make_siblings_clones_from_a_url_holding_nel(family, tmp_path): assert not NEVER_RAW & set(result.stdout), result.stdout +@pytest.mark.parametrize("name", ["members/a=b", "a=b"]) def test_a_section_named_with_an_equals_sign_mounts_from_gitmodules( - family, tmp_path): - """Lane 3's `[submodule "members/a=b"]`: no url was read for it, so the - dry run named `https://github.com/InkRouter/IRRS.git (family.yaml - repository:)`, the remote this holder is not pinned against. It now - names the url `.gitmodules` gives. A dry run, so the old reading never - asks GitHub.""" + family, tmp_path, name): + """Lane 3's `[submodule "members/a=b"]`, and `[submodule "a=b"]` as it + asked for the row on #690 (issuecomment-6232404184): no url was read for + it, so the dry run named `https://github.com/InkRouter/IRRS.git + (family.yaml repository:)`, the remote this holder is not pinned + against. It now names the url `.gitmodules` gives, as git, the doctor and + CI mount it. A dry run, so the old reading never asks GitHub.""" holder = holder_in(family, tmp_path / NAME) url = gitmodules_url(holder, BENT) edit(holder / ".gitmodules", f'[submodule "members/{BENT}"]', - '[submodule "members/a=b"]') + f'[submodule "{name}"]') result = siblings(holder, "--dry-run") assert result.returncode == 0, result.stdout + result.stderr assert (f" [{BENT}] would clone {url} (.gitmodules) -> " From 5ba1f8b37437892ba6fb2dbc89ca24cdd0fb9edd Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:21:24 +0000 Subject: [PATCH 3/4] siblings.py: a url `.gitmodules` gives whole, line feed and all, is written spelled in the clone and fetch failures that relay git's text, so it forges no FINDING line (#690) Lane 1's review of #805 at 25c7b14c found a regression the whole-url `-z` read made reachable. git's `die` and `error` quote a url RAW: a line feed stays and every other control becomes a `?`. A `.gitmodules` url holding a line feed was cut at it by main's reading, so the clone of `/r/nowhere` failed in one line; read whole, the clone of `/r/nowhereFINDING IRSS: forged` fails in two, and the second stood at column 0 of IRRS's CLONE FAILED finding as a FINDING line of its own. `said_of_url` writes the url git quotes as `spelled` writes it, the form every other line of `place` already prints. It looks for the url as given and as git quotes it (controls as `?`), because a url holding an ESC beside the line feed is quoted in the second form and the reviewer's raw `replace` alone leaves that forged line standing (measured: `line-feed-and-esc` fails with the raw form only). It is a helper, so `place` gains no branch. The other relay of git's text in `place` is the failed fetch's, which quotes the clone's `origin`: a clone made from such a url (a folder named `xFINDING IRSS: ...` holds one only since the whole-url read) has it for its origin, and its fetch error forged the same line (measured before the change). It goes through the same helper with `git remote get-url origin`. `verify_sibling`, the `cloning` and `would clone` lines and the findings already spell what they name; `run_member_bootstrap` and `--make` inherit the member's stdout and relay nothing of git's. `bootstrap.py`'s FETCH INCOMPLETE relays `git submodule update`'s text raw too, on main as here; it is not in this PR's file and stays for #459. tests/test_siblings_inputs.py: `test_a_url_holding_a_line_feed_forges_no_finding_line` (the reviewer's row, with an ESC variant) passes on main, fails on 25c7b14c, and passes with the change; `test_the_clone_failure_names_a_line_feed_url_spelled` pins the spelled url in the same text; `test_a_failed_fetch_forges_no_finding_line` runs a real fetch from a folder whose name holds the line feed. The composite assert at :228 that SonarCloud reports as S9073 is two asserts. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- templates/family-root/scripts/siblings.py | 30 +++++++- tests/test_siblings_inputs.py | 90 ++++++++++++++++++++++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 2ff3b93d..be945e12 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -421,6 +421,29 @@ def scrub(text: str, token: str) -> str: return text.replace(token, "") if token else text +def said_of_url(text: str, url: str | None) -> str: + """git's own text, with the url it quotes written as `spelled` writes it + (#690). + + A `.gitmodules` url is read whole now, line feed and all, as git reads + it, and git's `die` and `error` quote a url RAW: a line feed stays, so + the url's second line stood at column 0 of the finding that relays the + text, and `FINDING IRSS: forged` in a url was a FINDING line of its own. + Every other control is written as a `?` (a tab stays), so a url holding + an ESC as well is quoted in that form; both forms are replaced, and + nothing is, where the url prints as it is. `place` relays git's text + twice, and both go through here: the clone's, with the member's url, + and a failed fetch's, with the origin of a clone made from such a url. + """ + if not url or spelled(url) == url: + return text + quoted = "".join("?" if (char < " " or char == "\x7f") + and char not in "\t\n" else char for char in url) + for form in dict.fromkeys((url, quoted)): + text = text.replace(form, spelled(url)) + return text + + def git_text(args: list[str], cwd: Path) -> str | None: proc = subprocess.run(["git", *args], cwd=str(cwd), capture_output=True, text=True, encoding="utf-8", errors="replace", @@ -648,7 +671,8 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], sibling.state = "CLONE FAILED" sibling.finding = ( f"{who}: git clone exited {proc.returncode}:\n" - + scrub((proc.stderr + proc.stdout).strip(), token)) + + said_of_url(scrub((proc.stderr + proc.stdout).strip(), + token), url)) return sibling sibling.state = "cloned" if branch and branch_of(target) == "detached": @@ -678,7 +702,9 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], if fetched.returncode != 0: print(f" [{who}] fetch exited {fetched.returncode}; the clone is " "untouched:\n" - + scrub((fetched.stderr + fetched.stdout).strip(), token), + + said_of_url(scrub((fetched.stderr + fetched.stdout).strip(), + token), + git_text(["remote", "get-url", "origin"], target)), file=sys.stderr) sibling.branch = branch_of(target) return sibling diff --git a/tests/test_siblings_inputs.py b/tests/test_siblings_inputs.py index 5575946c..d0fc3367 100644 --- a/tests/test_siblings_inputs.py +++ b/tests/test_siblings_inputs.py @@ -225,7 +225,8 @@ def test_a_link_to_nothing_named_after_the_family_is_named_first(family, assert moves(result.stderr) == [ ["mv", str(holder), str(staged)], ["mkdir", "-p", str(link)], ["mv", str(staged), str(link / NAME)]] - assert os.path.islink(link) and holder.is_dir(), "nothing moved" + assert os.path.islink(link), "nothing moved" + assert holder.is_dir(), "nothing moved" os.rename(link, tmp_path / "moved-aside") run_as_typed(printed_moves(result.stderr)) @@ -473,6 +474,93 @@ def test_make_siblings_clones_from_a_url_holding_nel(family, tmp_path): assert not NEVER_RAW & set(result.stdout), result.stdout +#: IRRS's `.gitmodules` url as written in the file, and as the member's own +#: lines spell it: a line feed (`\n` in a quoted value), and one beside an +#: ESC, which git quotes as a `?`. Each holds a line a forger could write. +FORGED_URLS = { + "line-feed": (b'"/r/nowhere\\nFINDING ' + KEPT.encode() + b': forged"', + f"/r/nowhere\\nFINDING {KEPT}: forged"), + "line-feed-and-esc": ( + b'"/r/no\x1b[31mwhere\\nFINDING ' + KEPT.encode() + b': forged"', + f"/r/no\\x1b[31mwhere\\nFINDING {KEPT}: forged"), +} + + +def siblings_over_url(family: dict, parent: Path, url: bytes): + """`make siblings` over a holder whose IRRS url is `url`, as written.""" + holder = holder_in(family, parent / NAME) + config = holder / ".gitmodules" + rewritten, count = re.subn( + rb"(?m)^(\turl = ).*/IRRS\.git$", + lambda found: found.group(1) + url, config.read_bytes()) + assert count == 1 + config.write_bytes(rewritten) + return siblings(holder) + + +@pytest.mark.parametrize("url", [url for url, _ in FORGED_URLS.values()], + ids=list(FORGED_URLS)) +def test_a_url_holding_a_line_feed_forges_no_finding_line(family, tmp_path, + url): + """Lane 1's review of #805: git's clone error quotes the url raw, line + feed and all. Read whole since #690, a `.gitmodules` url holding a line + feed and `FINDING IRSS: ...` put that line at column 0 of IRRS's + finding, where main, which cut the url at the line feed, never had it. + The only FINDING line is IRRS's own, and no raw ESC reaches stderr.""" + result = siblings_over_url(family, tmp_path, url) + assert result.returncode == 1, result.stdout + result.stderr + findings = [line for line in result.stderr.split("\n") + if line.startswith("FINDING ")] + assert findings == [f"FINDING {BENT}: git clone exited 128:"], \ + result.stderr + assert not NEVER_RAW & set(result.stderr), result.stderr + + +@pytest.mark.parametrize("url, shown", FORGED_URLS.values(), + ids=list(FORGED_URLS)) +def test_the_clone_failure_names_a_line_feed_url_spelled(family, tmp_path, + url, shown): + """The same relay, read the other way: git's text still names the url + it could not clone, written as the lines above it write it. git quotes + an ESC as a `?`, so the url is found in that form too.""" + result = siblings_over_url(family, tmp_path, url) + assert f"'{shown}'" in result.stderr, result.stderr + + +def test_a_failed_fetch_forges_no_finding_line(family, tmp_path): + """The other relay of git's text (#805's review): a clone made from a + url holding a line feed (a folder named `xFINDING IRSS: ...`, which + only the whole-url read can reach) has it for its `origin`, and when + its fetch fails git quotes the origin raw. The re-run's stderr holds no + line of the folder's name at column 0.""" + holder = holder_in(family, tmp_path / NAME) + remote = tmp_path / f"x\nFINDING {KEPT}: forged" / "IRRS.git" + try: + remote.parent.mkdir() + except OSError as refused: + pytest.skip(f"this disk holds no line feed in a name: {refused}") + made = clone_just_pushed(family["remotes"] / "IRRS.git", remote, + recurse=False, bare=True) + assert made.returncode == 0, made.stderr + config = holder / ".gitmodules" + written = b'"' + remote.as_posix().replace("\n", "\\n").encode() + b'"' + rewritten, count = re.subn( + rb"(?m)^(\turl = ).*/IRRS\.git$", + lambda found: found.group(1) + written, config.read_bytes()) + assert count == 1 + config.write_bytes(rewritten) + first = siblings(holder) + assert first.returncode == 0, first.stdout + first.stderr + + os.rename(remote.parent, tmp_path / "gone") + result = siblings(holder) + assert f"[{BENT}] fetch exited 128" in result.stderr, result.stderr + assert f"x\\nFINDING {KEPT}: forged/IRRS.git'" in result.stderr, \ + result.stderr + assert not [line for line in result.stderr.split("\n") + if line.startswith("FINDING ")], result.stderr + + @pytest.mark.parametrize("name", ["members/a=b", "a=b"]) def test_a_section_named_with_an_equals_sign_mounts_from_gitmodules( family, tmp_path, name): From dc1c8e4574fa7cf0b3229b9e1b3d16836129dc51 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:48:50 +0000 Subject: [PATCH 4/4] siblings.py: a url longer than git's message buffer, quoted cut short in neither form, has the text that relays it spelled line by line, so it forges no FINDING line (#690) Lane 1's re-verification of #805 at 5ba1f8b3: git formats a `die` message into 4096 bytes, so a url longer than that is quoted cut short, and the text holds neither the url as given nor its `?` form, the two `said_of_url` looks for. Nothing was replaced, and a `.gitmodules` url `"/r/nowhereFINDING IRSS: forged/" + 30 parts of 200 characters` (6061 characters) still put the forged line at column 0 of IRRS's CLONE FAILED finding, where main, which cut the url at the line feed, had none. Where the text holds neither form, `said_of_url` now spells each of its lines and joins them with a newline and four spaces, so no line after the first can begin a FINDING line. The reviewer's five lines, as given. It takes effect only for a url that `spelled` changes and that git's text does not quote in full, so an ordinary failure's text is not changed. tests/test_siblings_inputs.py: `test_a_url_longer_than_gits_message_forges_no_finding_line`, the reviewer's row, next to the `FORGED_URLS` rows. It fails at 5ba1f8b3 and passes with the fallback; with the fallback removed (M9) it is the one row that fails. Left for #459: `git clone --recurse-submodules` relays git's text for a member's own leg urls raw, on main as here, and so does `make bootstrap`'s FETCH INCOMPLETE. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- templates/family-root/scripts/siblings.py | 10 ++++++++++ tests/test_siblings_inputs.py | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index be945e12..224ef102 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -434,11 +434,21 @@ def said_of_url(text: str, url: str | None) -> str: nothing is, where the url prints as it is. `place` relays git's text twice, and both go through here: the clone's, with the member's url, and a failed fetch's, with the origin of a clone made from such a url. + + A URL LONGER THAN GIT'S MESSAGE is quoted cut short, in neither form (a + message is formatted into 4096 bytes), so the forged line survived the + replacement: where the text holds neither, each of its lines is spelled + and every line after the first is indented, which no FINDING line is. """ if not url or spelled(url) == url: return text quoted = "".join("?" if (char < " " or char == "\x7f") and char not in "\t\n" else char for char in url) + if url not in text and quoted not in text: + # git cuts a message at 4096 bytes, so a long url is quoted in + # neither form: each line is spelled, and none after the first + # starts a line of this output. + return "\n ".join(spelled(line) for line in text.split("\n")) for form in dict.fromkeys((url, quoted)): text = text.replace(form, spelled(url)) return text diff --git a/tests/test_siblings_inputs.py b/tests/test_siblings_inputs.py index d0fc3367..6b7bec7e 100644 --- a/tests/test_siblings_inputs.py +++ b/tests/test_siblings_inputs.py @@ -516,6 +516,20 @@ def test_a_url_holding_a_line_feed_forges_no_finding_line(family, tmp_path, assert not NEVER_RAW & set(result.stderr), result.stderr +def test_a_url_longer_than_gits_message_forges_no_finding_line(family, + tmp_path): + """git writes a `die` message into 4096 bytes, so a url longer than that + is quoted cut short, in neither form `said_of_url` looks for.""" + url = (b'"/r/nowhere\\nFINDING ' + KEPT.encode() + b': forged/' + + b"/".join([b"a" * 200] * 30) + b'"') + result = siblings_over_url(family, tmp_path, url) + assert result.returncode == 1, result.stdout + result.stderr + findings = [line for line in result.stderr.split("\n") + if line.startswith("FINDING ")] + assert findings == [f"FINDING {BENT}: git clone exited 128:"], \ + result.stderr + + @pytest.mark.parametrize("url, shown", FORGED_URLS.values(), ids=list(FORGED_URLS)) def test_the_clone_failure_names_a_line_feed_url_spelled(family, tmp_path,