From 43d32eb3929c56776553c3129b2cc7bd8e6aa4fe Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 22:56:53 +0000 Subject: [PATCH 1/5] siblings.py and family.py: `make siblings` asks about the family folder before it clones anything, clones each member on the tracking branch its pinned `project.yaml` declares, and `family.py siblings` runs the holder's own copy, so a misplaced holder no longer gets every member cloned twice, a member no longer lands on its remote's default branch, and the two entry points no longer run different code (#543) `templates/family-root/scripts/siblings.py` (the copy every holder pins): - The parent-folder check runs FIRST. A holder whose parent folder is not named after the family now stops before any credential is resolved or any member cloned: the WARNING says nothing was cloned and why, prints the same `mv` lines, and the run exits 1 ("siblings: stopped before cloning ... Nothing was cloned, fetched or moved."). `--dry-run` stops the same way. It used to clone every member into the wrong folder, then warn, so the re-run after the move cloned each member a second time. - A new sibling is cloned with `--branch=`, the `tracking_branch:` of the member's own `project.yaml` in the pinned copy under `members/`. A plain clone took the remote's default branch. With no pinned copy checked out, or no `tracking_branch:` there, the clone keeps the default branch as before and its line says why. `.gitmodules` carries no `branch=` and `family.yaml`'s `tracking_branch:` is the holder's own, so neither is read. A declared branch the remote lacks is git's refusal, a CLONE FAILED finding, exit 1. - The clone is `--no-local`: a member url that is a path on disk is read through git and not copied file by file, which a repack in the remote can break (#440; the half of #461's line #543 names). `scripts/family.py`: `siblings` runs `/scripts/siblings.py` with this interpreter, in its own process, from the holder, as `make siblings` does, and returns its exit code. Every word of that command is a constant (`scripts/siblings.py --root .`, run in the holder), so no value from the command line reaches its argv. It used to `runpy` the standard's template copy inside this process, where that file imported this checkout's `repo_shape`. A holder without the file is refused, `siblings-script-missing`, exit 2, in the words the holder's Makefile refuses with (#503). `runpy` is no longer imported. Docs: AGENTS.md (family item 5, same line count), the holder's README.md and AGENTS-shape.md say the run stops before cloning; the README names the tracking branch it clones on. Tests: tests/test_family_siblings_placement.py (new); three existing tests that held exit 0 for a misplaced holder now hold exit 1, one test docstring that named `runpy` is corrected, and tests/test_family_makefile_siblings.py's `cut()` puts its holder copies in a folder named after the family, where `make siblings` runs to the end. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 12 +- scripts/family.py | 83 ++++-- templates/family-root/AGENTS-shape.md | 7 +- templates/family-root/README.md | 15 +- templates/family-root/scripts/siblings.py | 153 ++++++++--- tests/test_family.py | 9 +- tests/test_family_makefile_siblings.py | 5 +- tests/test_family_printed_quoting.py | 4 +- tests/test_family_siblings_placement.py | 318 ++++++++++++++++++++++ 9 files changed, 517 insertions(+), 89 deletions(-) create mode 100644 tests/test_family_siblings_placement.py diff --git a/AGENTS.md b/AGENTS.md index e579245c..68d8ff9c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -405,12 +405,12 @@ python3 scripts/family.py siblings --family-root `members/` inside the holder is pinned and DETACHED, for `bootstrap` and `validate`; the sibling is where the human works. **What it never does, and neither do you:** it MOVES NOTHING — when the - parent folder is not named after the family it warns and prints an `mv`, - and running that `mv` yourself relocates a checkout out from under a - shell, an editor and every agent lane, and breaks any linked worktree - (its `.git` file carries an absolute path). An existing clone gets only - `git fetch` — no checkout, no reset, no pull — because somebody works in - there; one of a different repository is reported and skipped, and the + parent folder is not named after the family it clones nothing, prints an + `mv` and exits 1, and running that `mv` yourself relocates a checkout out + from under a shell, an editor and every agent lane, and breaks any linked + worktree (its `.git` file carries an absolute path). An existing clone gets + only `git fetch` — no checkout, no reset, no pull — because somebody works + in there; one of a different repository is reported and skipped, and the exit is the human's hand, not an overwrite. 6. `update-shape.py` updates a family root exactly as it updates a project, with the same four refusals, and mirrors into `family.yaml`. An existing diff --git a/scripts/family.py b/scripts/family.py index 3d05f10d..9769a45a 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -49,8 +49,8 @@ repository and keeps its own name, and `family.yaml` is what tells the two apart. `siblings` is the holder's own `make siblings`, run from a checkout of this standard so it works before the holder is anywhere else; there is ONE -implementation of it, `templates/family-root/scripts/siblings.py`, and this -subcommand runs that file rather than a second copy of its logic. +implementation of it, the holder's pinned `scripts/siblings.py`, and this +subcommand runs that file rather than a second copy of its logic (#543). EXIT CODES: 0 done, or a `--dry-run` printed what it would do · 1 nothing to do · 2 a refusal, and a refusal made before the first write is the same @@ -60,7 +60,7 @@ branch, or onto no branch at all, as `bump-leg.py` does for a leg; a dry run commits nothing, so it runs on any branch and says where the real run's commit would land, or that it would refuse there and the command that makes a branch. -`siblings` exits with the template script's own code, unchanged. A `bump` whose +`siblings` exits with the holder's script's own code, unchanged. A `bump` whose commit would record nothing — the member is already pinned at `--to`, in the gitlink and in `family.yaml` alike — is `nothing`: it says `nothing to do`, as `bump-leg.py` does for a leg, and writes no commit; its dry run says the same @@ -122,7 +122,6 @@ import json import os import re -import runpy import shlex import shutil import stat @@ -884,8 +883,10 @@ def _local_remote_is_empty(bare: Path) -> bool | None: # --------------------------------------------------------------------------- -SIBLINGS_SCRIPT = (SHAPE_ROOT / "templates" / "family-root" / "scripts" / - "siblings.py") +#: The holder's own copy of the workstation utility, relative to the holder: +#: the path its Makefile's `SIBLINGS_SCRIPT` names, and the file `siblings` +#: runs (#543). +SIBLINGS_SCRIPT = "scripts/siblings.py" def _family_script() -> str: @@ -925,10 +926,10 @@ def _standard_checkout(start: Path) -> Path | None: A checkout is a directory holding `scripts/family.py` beside `templates/family-root/`, which is what this file needs of its own - (`SIBLINGS_SCRIPT`, `FAMILY_TEMPLATE`), so it is found whatever the - directory is called and whichever copy of the standard is asked: this - one, another clone, an install. `os.path`'s tests, which answer False - for a directory this user cannot search rather than raising. + (`FAMILY_TEMPLATE`), so it is found whatever the directory is called and + whichever copy of the standard is asked: this one, another clone, an + install. `os.path`'s tests, which answer False for a directory this user + cannot search rather than raising. """ for directory in (start, *start.parents): if os.path.isfile(directory / "scripts" / "family.py") and \ @@ -4682,16 +4683,25 @@ def cmd_siblings(args) -> int: ONE IMPLEMENTATION, TWO ENTRY POINTS (Brett Heap, 2026-09-09, ruling 1b: *"One implementation, two entry points… No second code path."*). The - implementation is `templates/family-root/scripts/siblings.py`, the file a - holder CARRIES and its Makefile calls; this subcommand runs that same file + implementation is the HOLDER'S OWN `scripts/siblings.py`, the pinned copy + its Makefile's `siblings` target runs; this subcommand runs that same file against `--family-root`, exactly as the root `bootstrap` shim runs the - assembly root's `scripts/bootstrap.py`. That is what makes it work before - the holder is cloned anywhere else — the standard has the utility, the - holder is only where it usually lives. - - `runpy` rather than an import: the script is a `__main__` program that - ends in `sys.exit(main())`, so its exit code is the one this process - exits with, unchanged and unrewritten. + assembly root's `scripts/bootstrap.py`: from a checkout of the standard, + against a holder anywhere on disk. + + NOT THE STANDARD'S COPY (#543). It used to run this checkout's + `templates/family-root/scripts/siblings.py`, through `runpy`, so a holder + pinned at another shape ran other code under the two entry points, and + did so inside THIS process, whose `repo_shape` its file then imported in + place of the holder's own. So it runs as `make siblings` runs it: the + holder's file, by this interpreter, in a process of its own, standing in + the holder, with the holder's own `repo_shape.py` and `bootstrap.py` + beside it, and its exit code is the one this subcommand returns, + unchanged. + + A HOLDER WITHOUT ONE IS REFUSED BY NAME, as its Makefile refuses (#489, + #503): `siblings-script-missing`, exit 2, before anything is cloned, in + the Makefile's words, which name `update-shape.py` and its `--add`. """ root = Path(args.family_root).expanduser().resolve() if not (root / MANIFEST).is_file(): @@ -4701,18 +4711,33 @@ def cmd_siblings(args) -> int: "Remediation: `family.py init --org --family ` creates " "one; point --family-root at the HOLDER (the `/` " "clone), not at the family folder around it.") - if not SIBLINGS_SCRIPT.is_file(): + script = root / SIBLINGS_SCRIPT + if not script.is_file(): raise Refusal( "siblings-script-missing", - f"{SIBLINGS_SCRIPT} is missing from this checkout of the standard", - "Remediation: this subcommand runs the holder's own copy of that " - "file and has no second implementation to fall back on. Re-clone " - "openRepoShape, or run `make siblings` in the holder, which uses " - "its own copy.") - sys.argv = ["siblings", "--root", str(root), - *(["--dry-run"] if args.dry_run else [])] - runpy.run_path(str(SIBLINGS_SCRIPT), run_name="__main__") - return 0 # pragma: no cover - the script exits before returning here + f"family.py siblings needs {SIBLINGS_SCRIPT}, which this holder " + f"does not have: there is no {script}. It runs the holder's own " + "copy, as `make siblings` does, and has no second implementation " + "to fall back on, so nothing was cloned or fetched.", + "Remediation: update-shape.py copies a changed Makefile but adds " + "a new file only on the human's word, so a holder updated without " + f"--add {SIBLINGS_SCRIPT} lands here. From a checkout of " + "openRepoShape run `" + + shlex.join(["python3", "update-shape.py", "check", "--root", + str(root)]) + + f"`, which reports {SIBLINGS_SCRIPT} as upstream-added. When " + "the human says to take it, run the apply command it prints with " + f"--add {SIBLINGS_SCRIPT} added. If it reports copy-missing " + "instead, a pinned copy was deleted: restore it from this " + "holder's history rather than with --add.") + # STANDING IN THE HOLDER, AS MAKE STANDS THERE, so every word of the + # command is a constant and none is a value from this command line: the + # holder is the directory it runs in, and `--root .` names it. + sys.stdout.flush() + return subprocess.run( + [sys.executable or PYTHON, SIBLINGS_SCRIPT, "--root", ".", + *(["--dry-run"] if args.dry_run else [])], + cwd=str(root), check=False).returncode # --------------------------------------------------------------------------- diff --git a/templates/family-root/AGENTS-shape.md b/templates/family-root/AGENTS-shape.md index 12533af8..58cdaf98 100644 --- a/templates/family-root/AGENTS-shape.md +++ b/templates/family-root/AGENTS-shape.md @@ -94,9 +94,10 @@ holder (`../`). Two copies of every member is correct and intended — this family follows it. What it will NOT do, and you must not do either: * **It moves nothing.** When the parent folder is not named after the family - it WARNS and prints the exact `mv` for a human to run. Do not run that `mv` - on your own initiative: relocating a checkout breaks an open shell, an - editor and any LINKED WORKTREE, whose `.git` file carries an absolute path. + it WARNS, prints the exact `mv` for a human to run and stops, exit 1, + before it clones anything. Do not run that `mv` on your own initiative: + relocating a checkout breaks an open shell, an editor and any LINKED + WORKTREE, whose `.git` file carries an absolute path. * **It never touches an existing clone beyond `git fetch`.** No checkout, no reset, no pull, no stash, no branch switch — somebody is working in there. A directory that is a clone of a different repository is reported and diff --git a/templates/family-root/README.md b/templates/family-root/README.md index d7d246fd..04668b4c 100644 --- a/templates/family-root/README.md +++ b/templates/family-root/README.md @@ -43,8 +43,9 @@ clones beside it: ``` `make siblings` is what places it: for each row in `family.yaml` it clones the -member at `../` from the same url `make bootstrap` uses, then runs -that member's own `scripts/bootstrap.py` so its legs are on their tracking +member at `../` from the same url `make bootstrap` uses, on the +tracking branch the member's pinned `project.yaml` declares, then runs that +member's own `scripts/bootstrap.py` so its legs are on their tracking branches at their pins. A member that is already there is **fetched and nothing else** — no checkout, no reset, no pull — because somebody is working in it. A directory that is a clone of a DIFFERENT repository is reported and @@ -63,10 +64,12 @@ named after the family because that is what the folder holds. `family.yaml` is the only thing that tells the two apart, and it is inside the holder. **Nothing ever moves a checkout.** When this holder's parent folder is not -named after the family, `make siblings` WARNS and prints the exact `mv` for a -human to run — it moves nothing itself. Relocating a directory out from under -a shell, an editor or an agent lane breaks linked worktrees, whose `.git` -files carry absolute paths, and this standard does not act by surprise. +named after the family, `make siblings` WARNS, prints the exact `mv` for a +human to run and stops before it clones anything, so that the run after the +move clones each member once — it moves nothing itself. Relocating a +directory out from under a shell, an editor or an agent lane breaks linked +worktrees, whose `.git` files carry absolute paths, and this standard does +not act by surprise. From a checkout of `{{SHAPE_REPOSITORY}}` the same utility runs against a holder anywhere on disk, which is what to use before this repository is diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 7038a8ad..a835bfcd 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -23,14 +23,44 @@ it afterwards with `family.py bump`. Neither copy is the other's mirror, and this command never touches the pinned one. -WHAT IT DOES, per member row in `family.yaml`: +WHAT IT DOES: + + FIRST THE PARENT-FOLDER CHECK, BEFORE ANYTHING IS CLONED (#543). When + the holder's parent directory is not named after the family, this + WARNS, prints the exact `mv` and STOPS, exit 1: nothing is cloned, + fetched or moved. A member cloned first would land in the wrong + folder, and the re-run after the move would clone it a second time + beside the moved holder. It MOVES NOTHING itself. Relocating a + checkout out from under somebody's shell, editor and agent lanes + breaks linked worktrees, whose `.git` files carry absolute paths, + and is the one act this standard refuses everywhere else: done by + surprise, with no hand on it. The doctor pattern, not a mover. + + then, per member row in `family.yaml`: ABSENT `git clone --recurse-submodules` from the SAME url `make bootstrap` resolves — `.gitmodules` first, `family.yaml`'s `repository:` as - the fallback — with the same credential, resolved the same way, and - then the member's OWN `scripts/bootstrap.py`, which puts its legs - on their tracking branches AT their pins. The result is a working - clone on its branch, not a detached one. + the fallback — with the same credential, resolved the same way, ON + THE MEMBER'S TRACKING BRANCH, and then the member's OWN + `scripts/bootstrap.py`, which puts its legs on their tracking + branches AT their pins. The result is a working clone on its + branch, not a detached one. + + THE TRACKING BRANCH IS THE ONE THE MEMBER DECLARES (#543): the + `tracking_branch:` of its own `project.yaml`, read in the PINNED + copy under `members/`, which is this holder's record of the member. + A plain clone checks out the remote's DEFAULT branch, and the two + differ when a member tracks another branch. A member whose pinned + copy is not checked out (`make bootstrap` checks it out), or that + declares none, is cloned on its remote's default branch, as it + always was, and its line says so. Nothing else is read for it: + `.gitmodules` carries no `branch=`, and `family.yaml`'s + `tracking_branch:` is the HOLDER's branch, the one `family.py` + refuses to commit on. + + `--no-local` (#461): a url that is a path on this machine is read + through git, as a remote is, and not copied file by file, so a + repack running in it cannot fail the clone half way (#440). PRESENT `git fetch`, AND NOTHING ELSE. No checkout, no reset, no pull, no stash, no branch switch. Somebody is working in that clone — that @@ -40,14 +70,6 @@ row's repository and its own `project.yaml` id is the row's `id`); when it is not, that is reported and SKIPPED, never overwritten. - THEN THE PARENT-FOLDER CHECK. When the holder's parent directory is not - named after the family, this WARNS and prints the exact `mv` — and - MOVES NOTHING. Relocating a checkout out from under somebody's - shell, editor and agent lanes breaks linked worktrees, whose `.git` - files carry absolute paths, and is the one act this standard - refuses everywhere else: done by surprise, with no hand on it. The - doctor pattern, not a mover. - IDEMPOTENT. A second run is every member `present`, one `git fetch` each, and exit 0. That is what makes it safe to put in a handoff document. @@ -86,8 +108,11 @@ EXIT CODES 0 every member is beside the holder; nothing to report. With `--make`: every member RAN and exited 0 - 1 a FINDING a human can act on: a sibling that is a DIFFERENT repository, - a clone that failed, a member's own bootstrap that went red. With + 1 a FINDING a human can act on: a holder whose parent folder is not + named after the family (nothing is cloned until it is moved), a + sibling that is a DIFFERENT repository, a clone that failed (a + tracking branch its remote does not have included), a member's own + bootstrap that went red. With `--make`: any member that was skipped or whose verb exited non-zero — a `make resume` that skipped every member because nobody had run `make siblings` must never read as success @@ -273,6 +298,35 @@ def clone_url(root: Path, row: dict, urls: dict[str, str]) -> tuple[str, str]: return f"https://github.com/{repository}.git", f"{MANIFEST} repository:" +def tracking_branch(root: Path, row: dict) -> tuple[str | None, str]: + """`(the member's tracking branch, where it was read)`, or `(None, why + there is none)` — the branch a NEW working clone is checked out on. + + THE MEMBER'S OWN DECLARATION, as this holder pins it (#543): the + `tracking_branch:` of `project.yaml` in the pinned copy under `members/`, + the file the member's own bootstrap reads for the same answer. It is + read from disk, as `validate-family.py` reads that copy's `id:`, and + only `load_yaml` is used to read it, a name every `repo_shape.py` a + holder may have kept carries. None, with the reason, when the copy is + not checked out or declares no branch: the clone then takes the remote's + default branch, as it always did, and says so. + """ + label = (str(row.get("path") or f"members/{row.get('project')}") + + "/project.yaml") + manifest = member_path(root, row) / "project.yaml" + if not manifest.is_file(): + return None, (f"{label} is not checked out, so its tracking branch " + "is not known here (`make bootstrap` checks it out)") + try: + data = load_yaml(manifest) + except (Refusal, OSError, ValueError): + return None, f"{label} could not be read" + branch = data.get("tracking_branch") if isinstance(data, dict) else None + if not isinstance(branch, str) or not branch.strip(): + return None, f"{label} declares no tracking_branch" + return branch, label + + # --------------------------------------------------------------------------- # git, with the credential the family already resolves # --------------------------------------------------------------------------- @@ -445,14 +499,20 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], return sibling if not present: + branch, where = tracking_branch(root, row) + on = (f"on {branch}, its tracking branch ({where})" if branch else + f"on its remote's default branch: {where}") if dry_run: sibling.state = "would clone" - print(f" [{project}] would clone {url} ({source}) -> {target}") + print(f" [{project}] would clone {url} ({source}) -> {target}, " + f"{on}") return sibling - print(f" [{project}] cloning {url} ({source})") + print(f" [{project}] cloning {url} ({source}) {on}") sys.stdout.flush() proc = subprocess.run( - ["git", *prefix, "clone", "-q", "--recurse-submodules", "--", + ["git", *prefix, "clone", "-q", "--no-local", + "--recurse-submodules", + *([f"--branch={branch}"] if branch else []), "--", url, str(target)], capture_output=True, text=True, encoding="utf-8", errors="replace", check=False) @@ -742,6 +802,11 @@ def parent_folder_warning(root: Path, family: str) -> str | None: editor and every agent lane standing in it, and would break linked worktrees, whose `.git` files carry ABSOLUTE paths. The warning plus the command gives the same outcome with the human's hand on it. + + AND IT IS ASKED BEFORE ANYTHING IS CLONED (#543), which is why it says + nothing was. It used to come after the placement: every member was + cloned into the wrong folder first, and the re-run the warning asked for + cloned each one again beside the moved holder. """ folder = root.parent if folder.name == family: @@ -752,16 +817,20 @@ def parent_folder_warning(root: Path, family: str) -> str | None: f" {root} should sit in a plain folder named {family}, beside " "the members'", " working clones — that is the layout `make siblings` places. " - "The siblings", - f" go beside the holder either way, so they are in {folder}.", + "So", + " NOTHING WAS CLONED: a member cloned now would land in " + f"{folder}, and", + " the re-run after the move would clone it a second time, " + "beside the", + " moved holder.", "", - " NOTHING WAS MOVED. Relocating a checkout breaks an open " - "shell, an editor", - " and any LINKED WORKTREE (its `.git` file carries an absolute " - "path), so it", - " is yours to run, when nothing is standing in it, in a POSIX " - "shell (Git Bash", - " on Windows):", + " NOTHING WAS MOVED either. Relocating a checkout breaks an " + "open", + " shell, an editor and any LINKED WORKTREE (its `.git` file " + "carries", + " an absolute path), so it is yours to run, when nothing is " + "standing", + " in it, in a POSIX shell (Git Bash on Windows):", "", ] wanted = folder.parent / family @@ -786,7 +855,7 @@ def parent_folder_warning(root: Path, family: str) -> str | None: lines += ["", " Then re-run `make siblings` from the moved holder; the " "members are", - " cloned beside it, in the folder."] + " cloned beside it, in the folder, once."] return "\n".join(lines) @@ -905,12 +974,18 @@ def _report_placement(root: Path, siblings: list[Sibling], def _run_placement(root: Path, name: str, rows: list[dict], manifest: dict, dry_run: bool) -> int: - """THE PLACEMENT RUN: every member cloned or fetched beside the holder, - the layout table, the parent-folder warning, and the exit code. + """THE PLACEMENT RUN: the parent-folder check, then every member cloned + or fetched beside the holder, the layout table, and the exit code. Split from `main` for #137: once `_resolve_make_target` and `_load_family` are also lifted out of it, `main` is left with only the dispatch-or-placement choice. + + THE FOLDER IS ASKED FIRST (#543). A holder whose parent folder is not + named after the family stops here, exit 1, before any credential is + resolved or any member cloned, and `--dry-run` stops in the same words: + once the holder is moved, the run after it clones each member once, in + the right folder. """ family = family_folder_name(manifest, root) print(f"siblings: {name} ({root}) — {len(rows)} member(s)") @@ -921,6 +996,17 @@ def _run_placement(root: Path, name: str, rows: list[dict], manifest: dict, "validate, the") print(" sibling is where you work.") + warning = parent_folder_warning(root, family) + if warning: + print() + sys.stdout.flush() # so the warning lands where it belongs + print(warning, file=sys.stderr) + print(f"\nsiblings: stopped before cloning: the holder is not in a " + f"folder named {family}. Nothing was cloned, fetched or moved.", + file=sys.stderr) + sys.stderr.flush() + return 1 + prefix, source, token = git_prefix() urls = submodule_urls(root) print(f"\n(a) the members, beside the holder (credential source: " @@ -929,13 +1015,6 @@ def _run_placement(root: Path, name: str, rows: list[dict], manifest: dict, _print_layout_table(root, siblings, rows) - warning = parent_folder_warning(root, family) - if warning: - print() - sys.stdout.flush() # so the warning lands where it belongs - print(warning, file=sys.stderr) - sys.stderr.flush() - return _report_placement(root, siblings, dry_run) diff --git a/tests/test_family.py b/tests/test_family.py index 610df4c4..c1743536 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -1884,14 +1884,15 @@ def test_siblings_warns_about_the_parent_folder_and_moves_nothing(family, """THE DOCTOR PATTERN, NOT A MOVER (#76). The holder here sits under a folder that is not named after the family, which is what a hand-arranged workstation looks like — and the answer is a warning, the exact `mv`, and - nothing moved.""" + nothing moved. Asked before anything is cloned, so it stops there, exit 1 + (#543; `test_family_siblings_placement.py`).""" wrong = tmp_path / "somewhere-else" wrong.mkdir() holder = wrong / NAME shutil.copytree(family["root"], holder, symlinks=True) result = siblings(holder, "--dry-run") - assert result.returncode == 0, result.stderr + result.stdout + assert result.returncode == 1, result.stderr + result.stdout assert "WARNING the holder's parent folder is 'somewhere-else', not " \ "'InkRouter'." in result.stderr assert "NOTHING WAS MOVED" in result.stderr @@ -1923,8 +1924,8 @@ def test_siblings_dry_run_clones_nothing(estate): def test_family_py_siblings_runs_the_holders_own_file(family, tmp_path): """ONE IMPLEMENTATION, TWO ENTRY POINTS (ruling 1b, 2026-09-09). - `family.py siblings` runs `templates/family-root/scripts/siblings.py` - through `runpy`, the way the root `bootstrap` shim runs the assembly + `family.py siblings` runs the holder's own `scripts/siblings.py` (#543), + the way the root `bootstrap` shim runs the assembly root's bootstrap — so a person with a checkout of the standard and a holder on disk gets the same run, and its exit code, without a second code path to keep in step. The proof is the layout table: byte for byte diff --git a/tests/test_family_makefile_siblings.py b/tests/test_family_makefile_siblings.py index e2faacdd..1cda87ab 100644 --- a/tests/test_family_makefile_siblings.py +++ b/tests/test_family_makefile_siblings.py @@ -393,8 +393,9 @@ def standard(tmp_path_factory) -> dict: def cut(standard, tmp_path_factory, name: str) -> Path: - """A private copy of the holder as it was cut.""" - target = tmp_path_factory.mktemp(name) / FAMILY + """A private copy of the holder as it was cut, in a folder named after + the family: `make siblings` stops before cloning in any other (#543).""" + target = tmp_path_factory.mktemp(name) / FAMILY / FAMILY shutil.copytree(standard["holder"], target, symlinks=True) return target diff --git a/tests/test_family_printed_quoting.py b/tests/test_family_printed_quoting.py index c1af5b64..e240a457 100644 --- a/tests/test_family_printed_quoting.py +++ b/tests/test_family_printed_quoting.py @@ -373,7 +373,7 @@ def test_the_siblings_mv_lines_paste_whole_and_move_the_holder(family, holder = copied_holder(family, tmp_path / folder) result = run_script(FAMILY, "siblings", "--family-root", str(holder), "--dry-run") - assert result.returncode == 0, result.stdout + result.stderr + assert result.returncode == 1, result.stdout + result.stderr # #543 assert "NOTHING WAS MOVED" in result.stderr assert "in a POSIX shell (Git Bash" in result.stderr commands = [shlex.split(line) for line in result.stderr.splitlines() @@ -434,7 +434,7 @@ def test_siblings_runs_beside_a_repo_shape_without_the_quoter(family, assert helped.stdout.startswith("usage: siblings.py"), helped.stdout warned = run_script(siblings, "--dry-run", cwd=holder) - assert warned.returncode == 0, warned.stdout + warned.stderr + assert warned.returncode == 1, warned.stdout + warned.stderr # #543 assert "NOTHING WAS MOVED" in warned.stderr, warned.stderr commands = [shlex.split(line) for line in warned.stderr.splitlines() if line.startswith(" " * 12) and line.strip()[:3] in diff --git a/tests/test_family_siblings_placement.py b/tests/test_family_siblings_placement.py new file mode 100644 index 00000000..8f0e395f --- /dev/null +++ b/tests/test_family_siblings_placement.py @@ -0,0 +1,318 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`make siblings` and `family.py siblings`: where a member is cloned, on +which branch, and by which copy of the utility (#543). + +Three defects, one file, because `scripts/siblings.py` is a pinned copy in +every family holder and the three are fixed in one change and one re-pin: + +* A holder whose parent folder is not named after the family cloned every + member into that folder FIRST and then printed the `mv` that moves the + holder, so the re-run the warning asked for cloned each member a second + time beside the moved holder. The folder is now asked first: the run stops, + exit 1, nothing cloned, and the run after the move clones each member once. +* A new sibling was a plain `git clone`, on its remote's DEFAULT branch, not + on the member's tracking branch. It is now cloned on the `tracking_branch:` + the member's own `project.yaml` declares in the pinned copy under + `members/`; with no pinned copy checked out it keeps the default branch + and says so. +* `family.py siblings` ran the STANDARD's `templates/family-root/` copy, in + its own process, so a holder pinned at another shape ran other code than + `make siblings` does. It now runs the holder's own `scripts/siblings.py` as + the Makefile runs it, and refuses a holder without one in the Makefile's + words (#489, #503). + +And the half of #461 that is product code: the clone is `--no-local`, so a +member remote that is a path on disk is read through git, not copied file by +file under a repack (#440). + +THE SAME FAMILY AS `test_family.py` (its module fixture, this module's own +instance), with one change made once: IRSS's remote's default branch is moved +off `main`, the branch IRSS tracks, so a clone that took the default branch +is visibly on the wrong one. Each test works on its own copy of the holder. +""" + +from __future__ import annotations + +import os +import shlex +import shutil +from pathlib import Path + +import pytest + +from conftest import REPO, git, run_script +#: The family fixture, its names and the `make siblings` runner, imported +#: from the module that owns them. +from test_family import ( # noqa: F401 + ALLOW_FILE_PROTOCOL, FAMILY, MEMBERS, NAME, family, siblings, +) + +#: The branch IRSS's remote is pointed at instead of `main`. +DEFAULT = "elsewhere" +MAKEFILE = REPO / "templates" / "family-root" / "Makefile" + + +@pytest.fixture(scope="module") +def diverged(family) -> dict: + """The family, with IRSS's remote's DEFAULT branch made `elsewhere` + (`main~1`) while IRSS still declares `tracking_branch: "main"`: what a + member whose default branch is not its tracking branch looks like.""" + bare = family["remotes"] / "IRSS.git" + git("branch", "-f", DEFAULT, "main~1", cwd=bare) + git("symbolic-ref", "HEAD", f"refs/heads/{DEFAULT}", cwd=bare) + return family + + +def holder_in(family: dict, parent: Path) -> Path: + """A private copy of the holder at `/`.""" + parent.mkdir(parents=True, exist_ok=True) + holder = parent / NAME + shutil.copytree(family["root"], holder, symlinks=True) + return holder + + +def branch_of(path: Path) -> str: + return git("rev-parse", "--abbrev-ref", "HEAD", cwd=path).stdout.strip() + + +def clone_line(stdout: str, project: str) -> str: + """The line that says how `project` is cloned, or would be.""" + lines = [line for line in stdout.splitlines() + if line.startswith(f" [{project}] ") and "clon" in line] + assert len(lines) == 1, stdout + return lines[0] + + +def move_commands(stderr: str) -> list[list[str]]: + """The `mv` and `mkdir -p` lines the parent-folder warning prints, split + as a POSIX shell splits them.""" + return [shlex.split(line) for line in stderr.splitlines() + if line.startswith(" " * 12) and line.strip()[:3] in + ("mv ", "mkd")] + + +def flat(text: str) -> str: + return " ".join(text.split()) + + +# --- the tracking branch ----------------------------------------------------- + + +def test_the_fixture_member_tracks_main_and_its_remote_defaults_elsewhere( + diverged): + bare = diverged["remotes"] / "IRSS.git" + head = git("symbolic-ref", "--short", "HEAD", cwd=bare).stdout.strip() + assert head == DEFAULT + declared = (diverged["root"] / "members" / "IRSS" / + "project.yaml").read_text(encoding="utf-8") + assert 'tracking_branch: "main"' in declared + + +def test_each_member_is_cloned_on_the_tracking_branch_it_declares( + diverged, tmp_path): + """Through `make siblings`' own script: IRSS lands on `main`, the branch + its pinned `project.yaml` declares, and not on `elsewhere`, the branch a + plain clone of its remote checks out; and that branch tracks `origin`'s, + as a working clone's does.""" + holder = holder_in(diverged, tmp_path / NAME) + result = siblings(holder) + assert result.returncode == 0, result.stdout + result.stderr + for project in MEMBERS: + sibling = holder.parent / project + assert branch_of(sibling) == "main", project + upstream = git("rev-parse", "--abbrev-ref", "@{upstream}", + cwd=sibling).stdout.strip() + assert upstream == "origin/main", project + assert clone_line(result.stdout, "IRSS").endswith( + " on main, its tracking branch (members/IRSS/project.yaml)") + + +def test_a_member_whose_pinned_copy_is_not_checked_out_keeps_the_default( + diverged, tmp_path): + """No pinned copy, no record of the branch here: the clone takes the + remote's default branch, as it always did, and its line says why.""" + holder = holder_in(diverged, tmp_path / NAME) + git("submodule", "deinit", "-q", "-f", "--", "members/IRSS", cwd=holder) + assert not (holder / "members" / "IRSS" / "project.yaml").exists() + + result = siblings(holder) + assert result.returncode == 0, result.stdout + result.stderr + assert branch_of(holder.parent / "IRSS") == DEFAULT + assert branch_of(holder.parent / "IRRS") == "main" + said = clone_line(result.stdout, "IRSS") + assert " on its remote's default branch: members/IRSS/project.yaml is " \ + "not checked out" in said, said + + +def test_a_tracking_branch_its_remote_lacks_is_a_finding_and_no_clone( + diverged, tmp_path): + """The member declares a branch its remote does not have: git refuses + the clone, the run says so and exits 1, nothing is left at the path, and + the other member is still placed.""" + holder = holder_in(diverged, tmp_path / NAME) + manifest = holder / "members" / "IRSS" / "project.yaml" + before = manifest.read_bytes() + after = before.replace(b'tracking_branch: "main"', + b'tracking_branch: "no-such-branch"') + assert after != before + manifest.write_bytes(after) + + result = siblings(holder) + assert result.returncode == 1, result.stdout + result.stderr + assert "CLONE FAILED" in result.stdout + assert "FINDING IRSS: git clone exited" in result.stderr + assert "no-such-branch" in result.stderr + assert not (holder.parent / "IRSS").exists() + assert branch_of(holder.parent / "IRRS") == "main" + + +def test_the_dry_run_names_the_branch_it_would_clone_on(diverged, tmp_path): + holder = holder_in(diverged, tmp_path / NAME) + result = siblings(holder, "--dry-run") + assert result.returncode == 0, result.stdout + result.stderr + said = clone_line(result.stdout, "IRSS") + assert "would clone" in said + assert said.endswith( + ", on main, its tracking branch (members/IRSS/project.yaml)") + assert not (holder.parent / "IRSS").exists() + + +def test_a_sibling_is_read_through_git_and_shares_no_file_with_its_remote( + diverged, tmp_path): + """`--no-local` (#461's product half, #440): a clone of a path on disk + used to link or copy the remote's object files one by one, which a + repack running in the remote can break half way. Read through git, the + sibling's object store holds files of its own, none of them the + remote's (a local clone hard-links them where the disk allows).""" + holder = holder_in(diverged, tmp_path / NAME) + result = siblings(holder) + assert result.returncode == 0, result.stdout + result.stderr + + def inodes(objects: Path) -> set: + found = set() + for path in objects.rglob("*"): + if path.is_file(): + stat = path.stat() + found.add((stat.st_dev, stat.st_ino)) + return found + + remote = inodes(diverged["remotes"] / "IRRS.git" / "objects") + sibling = inodes(holder.parent / "IRRS" / ".git" / "objects") + assert remote + assert sibling + assert not remote & sibling + + +# --- the parent folder, asked before anything is cloned ---------------------- + + +def test_a_misplaced_holder_clones_nothing_and_the_moved_one_clones_once( + diverged, tmp_path): + """The issue's own flow: `make siblings` in a holder under a folder not + named after the family, the printed move, and `make siblings` again. + The first run clones nothing; the second clones each member once, + beside the moved holder; the old folder is left empty.""" + wrong = tmp_path / "somewhere-else" + holder = holder_in(diverged, wrong) + + first = siblings(holder) + assert first.returncode == 1, first.stdout + first.stderr + assert "cloning" not in first.stdout + assert "(a) the members" not in first.stdout + assert "NOTHING WAS CLONED" in first.stderr + assert "NOTHING WAS MOVED" in first.stderr + assert flat(first.stderr).endswith( + "siblings: stopped before cloning: the holder is not in a folder " + f"named {NAME}. Nothing was cloned, fetched or moved.") + assert sorted(os.listdir(wrong)) == [NAME] + + commands = move_commands(first.stderr) + staged = tmp_path / f"{NAME}.holder" + wanted = tmp_path / NAME + assert commands == [["mv", str(holder), str(staged)], + ["mkdir", "-p", str(wanted)], + ["mv", str(staged), str(wanted / NAME)]] + # The lines are a POSIX shell's (`test_family_printed_quoting.py` runs + # them in one); here they are done as written on every system. + for argv in commands: + if argv[0] == "mkdir": + Path(argv[2]).mkdir(parents=True, exist_ok=True) + else: + os.rename(argv[1], argv[2]) + + moved = wanted / NAME + second = siblings(moved) + assert second.returncode == 0, second.stdout + second.stderr + assert "WARNING" not in second.stderr + for project in MEMBERS: + assert (wanted / project / ".git").is_dir(), project + assert branch_of(wanted / project) == "main", project + assert sorted(os.listdir(wrong)) == [] + + +def test_a_misplaced_holders_dry_run_stops_in_the_same_words(diverged, + tmp_path): + """A dry run asks what the real run asks: it stops at the folder, exit + 1, and names no member it would clone.""" + wrong = tmp_path / "somewhere-else" + holder = holder_in(diverged, wrong) + result = siblings(holder, "--dry-run") + assert result.returncode == 1, result.stdout + result.stderr + assert "would clone" not in result.stdout + assert "NOTHING WAS CLONED" in result.stderr + assert sorted(os.listdir(wrong)) == [NAME] + + +# --- family.py siblings runs the holder's own copy --------------------------- + + +def test_family_py_siblings_runs_the_holders_own_copy_with_its_imports( + diverged, tmp_path): + """The holder's `scripts/siblings.py` is made to say that it ran and + which `repo_shape.py` it imported. `family.py siblings` runs THAT file, + as `make siblings` does, in a process of its own, so the `repo_shape` + it imports is the holder's copy beside it and not this checkout's.""" + holder = holder_in(diverged, tmp_path / NAME) + script = holder / "scripts" / "siblings.py" + text = script.read_bytes() + entry = b"def main(argv: list[str] | None = None) -> int:\n" + assert text.count(entry) == 1 + script.write_bytes(text.replace(entry, entry + ( + b" import repo_shape as _shape\n" + b" print('HOLDER COPY RAN, repo_shape from'," + b" Path(_shape.__file__).resolve())\n"))) + + result = run_script(FAMILY, "siblings", "--family-root", str(holder), + "--dry-run", env=ALLOW_FILE_PROTOCOL) + assert result.returncode == 0, result.stdout + result.stderr + library = (holder / "scripts" / "repo_shape.py").resolve() + assert f"HOLDER COPY RAN, repo_shape from {library}" in result.stdout + + +def test_family_py_siblings_refuses_a_holder_without_its_own_copy( + diverged, tmp_path): + """No `scripts/siblings.py` in the holder: refused by name, exit 2, + nothing cloned, in the words the holder's Makefile refuses with (#503), + the `--root` value quoted for a POSIX shell.""" + holder = holder_in(diverged, tmp_path / NAME) + (holder / "scripts" / "siblings.py").unlink() + + result = run_script(FAMILY, "siblings", "--family-root", str(holder), + env=ALLOW_FILE_PROTOCOL) + assert result.returncode == 2, result.stdout + result.stderr + said = flat(result.stderr) + assert said.startswith("REFUSED siblings-script-missing: family.py " + "siblings needs scripts/siblings.py, which this " + "holder does not have"), said + assert sorted(os.listdir(holder.parent)) == [NAME] + + makefile = MAKEFILE.read_text(encoding="utf-8") + start = makefile.index("SIBLINGS_MISSING = ") + message = flat(makefile[start:makefile.index("SIBLINGS_GUARD", start)] + .replace("\\\n", " ") + .replace("$(SIBLINGS_SCRIPT)", "scripts/siblings.py")) + head, tail = message.split(' --root "$(CURDIR)"`') + head = head.split("which this holder does not have. ", 1)[1] + assert head in said + assert f" --root {shlex.quote(str(holder))}`" in said + assert flat(tail) in said From 2156a012813f6de9596009bf68c53ee3d56f7779 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:44:45 +0000 Subject: [PATCH 2/5] siblings.py and family.py: the printed move runs as typed on a disk that ignores case and at a filesystem or drive root, `family.py siblings` keeps family.py's escaped stdout in the child it runs, `init --work-dir` no longer promises a `make siblings` that stops, and a `tracking_branch:` that is no branch name is shown escaped and not used, so a stopped holder is never left with a remedy that cannot run (#543) The review round of #543 (rv543: S1-S3, N1-N5, C2, C3). `templates/family-root/scripts/siblings.py` (the copy every holder pins): - S3. The parent-folder name test stays exact on every disk, so the printed move now runs as typed on every disk. When `/` is this folder under another case (a disk that ignores case, found with `os.path.samefile`), the FOLDER is renamed through `.folder`, two `mv` lines: the old `mkdir -p` + `mv` moved the holder onto itself and failed. When the parent is a filesystem or drive root, so that `/` is the holder itself, the holder moves through `.holder`, the staged move the common case already used; the old lines were `mv X X/X`. A case-sensitive disk is unchanged, and a link named `` onto the folder is never renamed through. The new `same_directory` asks equal paths first, so a drive root can be asked of a `PureWindowsPath`. The warning names a root as `'/'` rather than `''`. - N1. A `tracking_branch:` that is not a branch name is not used: a number or a list says so, and a string holding a character that does not print (an ESC sequence, a newline) is shown in its `repr`, never raw. The clone keeps the remote's default branch, as for no declaration. No import was added. - C3. The placement header says `parent folder` until the folder check passes, and `family folder` after. - N2. The `_run_placement` docstring now says the stop comes before any `git` command runs, which a test pins; "before any credential is resolved" was an environment read nothing could observe. `scripts/family.py`: - S1. `siblings` gives its child `inherited_child_environment()`, as `update-shape.py` does for the validators it runs (#254): this stdout's own encoding and a handler that cannot raise. The child process had its own strict stdout, so `PYTHONIOENCODING=ascii` (or a Windows code page without a character in a path) ended it with a UnicodeEncodeError on its first line, where the in-process run had escaped it (#214). The helper already guards a stdout with no encoding and a handler only this process registered. - S2. `init --work-dir ` makes no family folder, and `make siblings` stops before cloning in a holder whose parent is not named after the family. Where `` is not so named, the NEXT STEPS now say there is no family folder and that `make siblings` clones nothing until the holder sits in a folder named ``, instead of promising clones there. The `--into` output is byte for byte what it was. Docs: - N4. AGENTS.md family item 5 and the holder's AGENTS-shape.md said the stop "exits 1"; `make siblings` itself exits 2, make's own code. Both now say `exit 1 (`make`: 2)`, rewrapped within the same lines (AGENTS.md stays at 490). - C2. `templates/family-root/family.yaml`'s `tracking_branch:` comment said the family bootstrap places each member on that branch. It is the holder's own branch, which `family.py` refuses to commit on; the bootstrap leaves each member detached, and `make siblings` reads the member's own `project.yaml`. No test or pin compares those bytes. Tests, all in tests/test_family_siblings_placement.py (13 new): the ASCII stdout through `family.py siblings`; the `--work-dir` NEXT STEPS; the case-only rename (with `Path.exists` and `os.path.samefile` answering as a disk that ignores case, then the moves done as written); a case-sensitive disk with nothing, a folder or a link at ``; `/`, `C:\` and `C:\` in another case as pure paths; three `tracking_branch` values; `GIT_TRACE` showing no `git` ran before the stop; and the header in two existing tests. The template script is loaded in-process with `load_without_bytecode`, so nothing is written under `templates/`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 6 +- scripts/family.py | 34 +++- templates/family-root/AGENTS-shape.md | 8 +- templates/family-root/family.yaml | 10 +- templates/family-root/scripts/siblings.py | 76 ++++++- tests/test_family_siblings_placement.py | 235 +++++++++++++++++++++- 6 files changed, 338 insertions(+), 31 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 68d8ff9c..0f294fd5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -405,9 +405,9 @@ python3 scripts/family.py siblings --family-root `members/` inside the holder is pinned and DETACHED, for `bootstrap` and `validate`; the sibling is where the human works. **What it never does, and neither do you:** it MOVES NOTHING — when the - parent folder is not named after the family it clones nothing, prints an - `mv` and exits 1, and running that `mv` yourself relocates a checkout out - from under a shell, an editor and every agent lane, and breaks any linked + parent folder is not named after the family it clones nothing, prints an `mv` + and exits 1 (`make`: 2), and running that `mv` yourself relocates a checkout + out from under a shell, an editor and every agent lane, and breaks any linked worktree (its `.git` file carries an absolute path). An existing clone gets only `git fetch` — no checkout, no reset, no pull — because somebody works in there; one of a different repository is reported and skipped, and the diff --git a/scripts/family.py b/scripts/family.py index 9769a45a..e2e7f8c5 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -142,8 +142,8 @@ from shape_materialize import ( # noqa: E402 FAMILY_RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, check_program, commit_trailers, env_commit, escape_what_stdout_cannot_encode, - force_stage, materialize_family_root, refuse_bad_branch, run, - stage_written, trailer_line, write_lf, + force_stage, inherited_child_environment, materialize_family_root, + refuse_bad_branch, run, stage_written, trailer_line, write_lf, ) # What `init` asks of the holder's own reader and validator before it creates # anything (#307), imported as its own unit and not added to the lists above. @@ -1321,8 +1321,24 @@ def _print_next_steps(args, work: Path, folder: Path | None) -> None: ONE COMMAND A LINE, `cd` apart from `make siblings`: Windows PowerShell 5.1, the shell `setup-project.py` documents, rejects `&&` between two statements, and these lines are quoted for the shell of the host (#486). + + `--work-dir` makes no family folder, and `make siblings` stops before it + clones anything in a holder whose parent folder is not named after the + family (#543), so there the block says so instead of promising clones. """ switch = shell_line(["git", "-C", work, "switch", "-c"]) + beside = ("`make siblings` then puts each member's WORKING clone\n" + "beside the holder, in the family folder, on its tracking " + "branch.") + where = f"family folder {folder if folder is not None else work.parent}" + if folder is None and work.parent.name != work.name: + # --work-dir MAKES NO FAMILY FOLDER, and `make siblings` clones + # nothing until the holder sits in one (#543), so none is promised. + beside = ("--work-dir made no family folder, so `make siblings`\n" + "clones nothing: it prints the `mv` that puts the holder " + f"in a folder\nnamed {work.name}, and stops until it is " + "there.") + where = f"family folder none: {work.parent} is not named {work.name}" print(f""" NEXT STEPS @@ -1334,15 +1350,14 @@ def _print_next_steps(args, work: Path, folder: Path | None) -> None: `add` mounts the member at {MEMBERS_DIR}/, records its pin, and writes ONE commit on that branch: it refuses {args.tracking_branch}, the holder's tracking branch. Push the branch as its NEXT line says and land it -as a pull request. `make siblings` then puts each member's WORKING clone -beside the holder, in the family folder, on its tracking branch. +as a pull request. {beside} TWO COPIES OF EVERY MEMBER, on purpose: pinned and DETACHED inside the holder under {MEMBERS_DIR}/ (what `make bootstrap` and `make validate` read), and a working clone BESIDE the holder — that second one is where you work, and `family.py bump` is how the family follows it. -family folder {folder if folder is not None else work.parent} +{where} holder {work} """) @@ -4732,12 +4747,17 @@ def cmd_siblings(args) -> int: "holder's history rather than with --add.") # STANDING IN THE HOLDER, AS MAKE STANDS THERE, so every word of the # command is a constant and none is a value from this command line: the - # holder is the directory it runs in, and `--root .` names it. + # holder is the directory it runs in, and `--root .` names it. The child + # writes into this stdout but takes its encoding from its environment, + # not from the `reconfigure` `main()` did (#214): told this stdout's own + # encoding and a handler that cannot raise, it escapes what the stream + # cannot hold, as the in-process run did, instead of dying on an em dash. sys.stdout.flush() return subprocess.run( [sys.executable or PYTHON, SIBLINGS_SCRIPT, "--root", ".", *(["--dry-run"] if args.dry_run else [])], - cwd=str(root), check=False).returncode + cwd=str(root), check=False, + env=inherited_child_environment()).returncode # --------------------------------------------------------------------------- diff --git a/templates/family-root/AGENTS-shape.md b/templates/family-root/AGENTS-shape.md index 58cdaf98..0a056c6a 100644 --- a/templates/family-root/AGENTS-shape.md +++ b/templates/family-root/AGENTS-shape.md @@ -94,10 +94,10 @@ holder (`../`). Two copies of every member is correct and intended — this family follows it. What it will NOT do, and you must not do either: * **It moves nothing.** When the parent folder is not named after the family - it WARNS, prints the exact `mv` for a human to run and stops, exit 1, - before it clones anything. Do not run that `mv` on your own initiative: - relocating a checkout breaks an open shell, an editor and any LINKED - WORKTREE, whose `.git` file carries an absolute path. + it WARNS, prints the exact `mv` for a human to run and stops, exit 1 + (`make`: 2), before it clones anything. Do not run that `mv` on your own + initiative: relocating a checkout breaks an open shell, an editor and any + LINKED WORKTREE, whose `.git` file carries an absolute path. * **It never touches an existing clone beyond `git fetch`.** No checkout, no reset, no pull, no stash, no branch switch — somebody is working in there. A directory that is a clone of a different repository is reported and diff --git a/templates/family-root/family.yaml b/templates/family-root/family.yaml index 0aff4ac8..27d05c8b 100644 --- a/templates/family-root/family.yaml +++ b/templates/family-root/family.yaml @@ -45,9 +45,13 @@ created_on: "{{CREATED_ON}}" # is what lets a reader tell why the members need a credential in CI. visibility: "{{VISIBILITY}}" -# The branch `scripts/bootstrap.py` places each member on, AT its pinned -# commit. `.gitmodules` carries no `branch=`: there is exactly one -# authoritative answer to "what commit of this member is this family". +# The HOLDER's own branch: the one a new holder is created on, and the one +# `scripts/family.py` refuses to commit `add`, `bump` and `remove` on. It +# places no member on any branch: `scripts/bootstrap.py` leaves each member +# DETACHED at its pinned commit, and `make siblings` clones each working copy +# on the `tracking_branch:` of that member's own `project.yaml`. `.gitmodules` +# carries no `branch=`: there is exactly one authoritative answer to "what +# commit of this member is this family". tracking_branch: "{{TRACKING_BRANCH}}" # The directory every member is mounted under. One name, so a reader and a diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index a835bfcd..f8f132e2 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -52,7 +52,8 @@ A plain clone checks out the remote's DEFAULT branch, and the two differ when a member tracks another branch. A member whose pinned copy is not checked out (`make bootstrap` checks it out), or that - declares none, is cloned on its remote's default branch, as it + declares none, or a value that is no branch name (`5`, a control + character), is cloned on its remote's default branch, as it always was, and its line says so. Nothing else is read for it: `.gitmodules` carries no `branch=`, and `family.yaml`'s `tracking_branch:` is the HOLDER's branch, the one `family.py` @@ -310,6 +311,12 @@ def tracking_branch(root: Path, row: dict) -> tuple[str | None, str]: holder may have kept carries. None, with the reason, when the copy is not checked out or declares no branch: the clone then takes the remote's default branch, as it always did, and says so. + + A VALUE THAT IS NOT A BRANCH NAME IS NOT USED AS ONE, and is shown in its + `repr`: `5` or `[main]`, and a string holding a character that does not + print. An ESC or a newline would reach this command's output raw, where + the member's own bootstrap escapes the same field (#391); `repr` escapes + exactly what `isprintable` refuses, so the two agree with no import. """ label = (str(row.get("path") or f"members/{row.get('project')}") + "/project.yaml") @@ -322,8 +329,15 @@ def tracking_branch(root: Path, row: dict) -> tuple[str | None, str]: except (Refusal, OSError, ValueError): return None, f"{label} could not be read" branch = data.get("tracking_branch") if isinstance(data, dict) else None - if not isinstance(branch, str) or not branch.strip(): + if branch is None or branch == "": return None, f"{label} declares no tracking_branch" + if not isinstance(branch, str) or not branch.strip(): + return None, (f"{label} declares tracking_branch {branch!r}, which " + "is not a branch name") + if not branch.isprintable(): + return None, (f"{label} declares tracking_branch {branch!r}, which " + "holds a character that does not print, so it is not " + "used as a branch") return branch, label @@ -794,6 +808,23 @@ def family_folder_name(manifest: dict, root: Path) -> str: return str(manifest.get("name") or root.name) +def same_directory(one: Path, two: Path) -> bool: + """`one` and `two` are one directory: the same path, or on THIS disk. + + Equal paths first, which needs no disk, so a drive root can be asked + with a `PureWindowsPath` (whose equality already ignores case). Then the + disk's own answer, `os.path.samefile`, which is how `C:\\p\\InkRouter` + and `C:\\p\\inkrouter` are found to be one folder on a disk that ignores + case and two on one that does not. + """ + if one == two: + return True + try: + return one.exists() and os.path.samefile(one, two) + except OSError: + return False + + def parent_folder_warning(root: Path, family: str) -> str | None: """The WARNING and the exact `mv`, or None when the layout is right. @@ -807,13 +838,27 @@ def parent_folder_warning(root: Path, family: str) -> str | None: nothing was. It used to come after the placement: every member was cloned into the wrong folder first, and the re-run the warning asked for cloned each one again beside the moved holder. + + THE NAME TEST IS EXACT ON EVERY DISK, so the printed move has to run as + typed on every disk too (#543). Three shapes of it: + - `/` is the HOLDER ITSELF, because the parent is a + filesystem or drive root (`/`, `C:\\`): the holder is moved through a + staging name, as when `/` does not exist at all. + - `/` is THIS FOLDER under another case, on a disk that + ignores case (macOS, Windows): the FOLDER is renamed, through a + staging name, because a rename that changes only the case is a move + onto itself there. Linux, where the two are two folders, is unchanged. + - otherwise the holder moves into `/`, made if missing. + A LINK named `` onto this folder is never renamed through: the + holder's path is resolved before this is asked, so a family folder + reached through a link is a question #543 leaves as it was. """ folder = root.parent if folder.name == family: return None lines = [ - f"WARNING the holder's parent folder is {folder.name!r}, not " - f"{family!r}.", + 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'", " working clones — that is the layout `make siblings` places. " @@ -834,13 +879,21 @@ def parent_folder_warning(root: Path, family: str) -> str | None: "", ] wanted = folder.parent / family - if root.name == family and not wanted.exists(): + if same_directory(wanted, root) or (root.name == family + and not wanted.exists()): # The common case: the holder was cloned straight into ~/projects, so # `/` IS the holder itself and the move needs a - # staging name — `mv A A/A` cannot be spelled in one command. + # 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" steps = [["mv", root, staged], ["mkdir", "-p", wanted], ["mv", staged, wanted / family]] + elif (folder.name.casefold() == family.casefold() + and not wanted.is_symlink() and same_directory(wanted, folder)): + # THIS FOLDER under another case, on a disk that ignores case: the + # folder is renamed, in two steps, and the holder stays inside it. + staged = folder.parent / f"{family}.folder" + steps = [["mv", folder, staged], ["mv", staged, wanted]] else: steps = [["mkdir", "-p", wanted], ["mv", root, wanted / family]] # EVERY PATH QUOTED (#486): under `my work/` a bare `mv` moved nothing @@ -982,21 +1035,22 @@ def _run_placement(root: Path, name: str, rows: list[dict], manifest: dict, dispatch-or-placement choice. THE FOLDER IS ASKED FIRST (#543). A holder whose parent folder is not - named after the family stops here, exit 1, before any credential is - resolved or any member cloned, and `--dry-run` stops in the same words: + named after the family stops here, exit 1, before any `git` command + runs or any member is cloned, and `--dry-run` stops in the same words: once the holder is moved, the run after it clones each member once, in - the right folder. + the right folder. Until the check passes, the header calls that folder + the PARENT folder, not the family's. """ family = family_folder_name(manifest, root) + warning = parent_folder_warning(root, family) print(f"siblings: {name} ({root}) — {len(rows)} member(s)") - print(f" family folder {root.parent}") + print(f" {'parent' if warning else 'family'} folder {root.parent}") print(" each member is cloned BESIDE the holder and stays PINNED inside " "it under") print(" members/ — the pinned copy is detached for bootstrap and " "validate, the") print(" sibling is where you work.") - warning = parent_folder_warning(root, family) if warning: print() sys.stdout.flush() # so the warning lands where it belongs diff --git a/tests/test_family_siblings_placement.py b/tests/test_family_siblings_placement.py index 8f0e395f..3eb1197d 100644 --- a/tests/test_family_siblings_placement.py +++ b/tests/test_family_siblings_placement.py @@ -29,6 +29,13 @@ instance), with one change made once: IRSS's remote's default branch is moved off `main`, the branch IRSS tracks, so a clone that took the default branch is visibly on the wrong one. Each test works on its own copy of the holder. + +The review round (#543) adds: the child process keeps family.py's escaped +stdout (#214); `init --work-dir` no longer promises a `make siblings` that +stops; the printed move runs as typed on a disk that ignores case and at a +filesystem or drive root, each simulated in this process so all three +systems ask the same question; a `tracking_branch:` that is no branch name is +shown escaped and not used; and the stop comes before any `git` command. """ from __future__ import annotations @@ -36,11 +43,13 @@ import os import shlex import shutil -from pathlib import Path +import subprocess +import sys +from pathlib import Path, PurePosixPath, PureWindowsPath import pytest -from conftest import REPO, git, run_script +from conftest import REPO, git, load_without_bytecode, run_script #: The family fixture, its names and the `make siblings` runner, imported #: from the module that owns them. from test_family import ( # noqa: F401 @@ -95,6 +104,51 @@ def flat(text: str) -> str: return " ".join(text.split()) +@pytest.fixture +def placement(monkeypatch): + """The TEMPLATE's `siblings.py` in this process, for its move arithmetic. + + `load_without_bytecode`, so nothing is written under `templates/`; its + `bootstrap` import is answered by the template's own `bootstrap.py`, the + copy beside it in a holder. `sys.path`, which the file extends, and + `sys.modules` are put back when the test ends.""" + scripts = REPO / "templates" / "family-root" / "scripts" + monkeypatch.setattr(sys, "path", list(sys.path)) + monkeypatch.setitem(sys.modules, "bootstrap", load_without_bytecode( + scripts / "bootstrap.py", "placement_family_bootstrap")) + return load_without_bytecode(scripts / "siblings.py", + "placement_family_siblings") + + +def stored(path) -> Path | None: + """`path` as a disk that ignores case finds it, each part matched to a + name in its folder whatever its case; None when a part is missing.""" + path = Path(os.path.abspath(path)) + found = Path(path.anchor) + for part in path.parts[1:]: + try: + names = os.listdir(found) + except OSError: + return None + match = [name for name in names if name.casefold() == part.casefold()] + if not match: + return None + found = found / match[0] + return found + + +def ignoring_case(patched) -> None: + """`Path.exists` and `os.path.samefile` answering as macOS and Windows + disks do, here on any system: a spelling in another case exists, and is + the same file.""" + samefile = os.path.samefile + patched.setattr(Path, "exists", + lambda self, **_: stored(self) is not None) + patched.setattr(os.path, "samefile", lambda one, two: ( + None not in (stored(one), stored(two)) + and samefile(stored(one), stored(two)))) + + # --- the tracking branch ----------------------------------------------------- @@ -170,6 +224,7 @@ def test_the_dry_run_names_the_branch_it_would_clone_on(diverged, tmp_path): holder = holder_in(diverged, tmp_path / NAME) result = siblings(holder, "--dry-run") assert result.returncode == 0, result.stdout + result.stderr + assert f"\n family folder {holder.parent}\n" in result.stdout said = clone_line(result.stdout, "IRSS") assert "would clone" in said assert said.endswith( @@ -177,6 +232,34 @@ def test_the_dry_run_names_the_branch_it_would_clone_on(diverged, tmp_path): assert not (holder.parent / "IRSS").exists() +@pytest.mark.parametrize("declared, shown, why", [ + (b'"\\x1b[2Jcleared"', r"'\x1b[2Jcleared'", + "holds a character that does not print, so it is not used as a branch"), + (b"5", "5", "is not a branch name"), + (b"[main]", "['main']", "is not a branch name"), +], ids=["escape-sequence", "a-number", "a-list"]) +def test_a_tracking_branch_that_is_no_branch_name_is_shown_and_not_used( + diverged, tmp_path, declared, shown, why): + """A value no branch can have is not handed to git and not printed raw: + an ESC sequence would clear the screen of whoever reads the output, a + number or a list is not a name at all. Each is shown as `repr` shows it, + and the clone keeps the remote's default branch.""" + holder = holder_in(diverged, tmp_path / NAME) + manifest = holder / "members" / "IRSS" / "project.yaml" + before = manifest.read_bytes() + after = before.replace(b'tracking_branch: "main"', + b"tracking_branch: " + declared) + assert after != before + manifest.write_bytes(after) + + result = siblings(holder, "--dry-run") + assert result.returncode == 0, result.stdout + result.stderr + assert "\x1b" not in result.stdout + assert clone_line(result.stdout, "IRSS").endswith( + ", on its remote's default branch: members/IRSS/project.yaml " + f"declares tracking_branch {shown}, which {why}") + + def test_a_sibling_is_read_through_git_and_shares_no_file_with_its_remote( diverged, tmp_path): """`--no-local` (#461's product half, #440): a clone of a path on disk @@ -253,7 +336,8 @@ def test_a_misplaced_holder_clones_nothing_and_the_moved_one_clones_once( def test_a_misplaced_holders_dry_run_stops_in_the_same_words(diverged, tmp_path): """A dry run asks what the real run asks: it stops at the folder, exit - 1, and names no member it would clone.""" + 1, and names no member it would clone. Its header calls that folder the + PARENT folder: the warning below it says it is not the family's.""" wrong = tmp_path / "somewhere-else" holder = holder_in(diverged, wrong) result = siblings(holder, "--dry-run") @@ -261,6 +345,118 @@ def test_a_misplaced_holders_dry_run_stops_in_the_same_words(diverged, assert "would clone" not in result.stdout assert "NOTHING WAS CLONED" in result.stderr assert sorted(os.listdir(wrong)) == [NAME] + assert f"\n parent folder {wrong}\n" in result.stdout + assert "family folder" not in result.stdout + + +def test_a_misplaced_holder_stops_before_any_git_command_runs(diverged, + tmp_path): + """The stop asks nothing of git: no `.gitmodules` read, no credential + used, no member cloned. `GIT_TRACE` names a file every `git` the run + starts writes to, so the file is absent after the stop; a `git` run with + the same variable writes it, so the probe is shown to work here.""" + holder = holder_in(diverged, tmp_path / "somewhere-else") + trace = tmp_path / "git-trace.log" + result = siblings(holder, env={"GIT_TRACE": str(trace)}) + assert result.returncode == 1, result.stdout + result.stderr + assert "stopped before cloning" in result.stderr + assert not trace.exists(), trace.read_text(encoding="utf-8") + control = tmp_path / "git-trace-control.log" + subprocess.run(["git", "--version"], capture_output=True, check=True, + env={**os.environ, "GIT_TRACE": str(control)}) + assert control.is_file() + + +# --- the printed move runs as typed on every disk ---------------------------- + + +def test_a_parent_folder_in_another_case_is_renamed_on_a_disk_that_ignores_it( + placement, monkeypatch, tmp_path): + """macOS and Windows: the holder sits in `inkrouter/`, and `InkRouter` + "exists" because it IS that folder. The exact name test stops the run + on every disk, so the remedy renames the FOLDER through a staging name, + which a disk that ignores case can do and a one-step rename onto itself + cannot. The moves, done as written, leave the holder where the next run + finds no warning.""" + folder = tmp_path / NAME.lower() + root = folder / NAME + root.mkdir(parents=True) + with monkeypatch.context() as patched: + ignoring_case(patched) + warning = placement.parent_folder_warning(root, NAME) + staged = tmp_path / f"{NAME}.folder" + commands = move_commands(warning) + assert commands == [["mv", str(folder), str(staged)], + ["mv", str(staged), str(tmp_path / NAME)]] + + for _, source, target in commands: + os.rename(source, target) + assert sorted(os.listdir(tmp_path)) == [NAME] + assert placement.parent_folder_warning(tmp_path / NAME / NAME, + NAME) is None + + +@pytest.mark.parametrize("there", ["nothing", "a folder", "a link"]) +def test_a_case_sensitive_disk_keeps_its_move(placement, monkeypatch, + tmp_path, there): + """Linux, where `InkRouter` and `inkrouter` are two folders: with + nothing at `InkRouter`, the holder moves there through a staging name; + with a folder of its own there, the holder moves into it, as it always + did. A LINK named `InkRouter` onto this folder is never renamed through: + a family folder reached through a link is resolved first, a question + #543 leaves as it was.""" + folder = tmp_path / NAME.lower() + root = folder / NAME + root.mkdir(parents=True) + wanted = tmp_path / NAME + exists, samefile = Path.exists, os.path.samefile + + def same(one, two) -> bool: + pair = {Path(one), Path(two)} + if wanted in pair: + return there == "a link" and pair == {wanted, folder} + return samefile(one, two) + + with monkeypatch.context() as patched: + patched.setattr(Path, "exists", lambda self, **_: ( + there != "nothing" if self == wanted else exists(self))) + patched.setattr(Path, "is_symlink", lambda self: ( + self == wanted and there == "a link")) + patched.setattr(os.path, "samefile", same) + commands = move_commands(placement.parent_folder_warning(root, NAME)) + staged = tmp_path / f"{NAME}.holder" + expected = { + "nothing": [["mv", str(root), str(staged)], + ["mkdir", "-p", str(wanted)], + ["mv", str(staged), str(wanted / NAME)]], + "a folder": [["mkdir", "-p", str(wanted)], + ["mv", str(root), str(wanted / NAME)]], + } + if there in expected: + assert commands == expected[there] + else: + assert ["mv", str(folder), str(tmp_path / f"{NAME}.folder")] \ + not in commands + + +@pytest.mark.parametrize("root", [ + PurePosixPath(f"/{NAME}"), + PureWindowsPath(f"C:\\{NAME}"), + PureWindowsPath(f"C:\\{NAME.lower()}"), +], ids=["filesystem-root", "drive-root", "drive-root-other-case"]) +def test_a_holder_at_a_filesystem_or_drive_root_moves_through_a_staging_name( + placement, root): + """`/InkRouter` or `C:\\InkRouter`: the parent is the root, whose name is + empty, so `/InkRouter` is the holder itself and `mv X X/X` cannot + run. The holder moves through a staging name. Asked of the paths alone, + which `PurePosixPath` and `PureWindowsPath` spell as each system does.""" + warning = placement.parent_folder_warning(root, NAME) + assert f"parent folder is {str(root.parent)!r}, not {NAME!r}" in warning + staged = root.parent / f"{NAME}.holder" + wanted = root.parent / NAME + assert move_commands(warning) == [ + ["mv", str(root), str(staged)], ["mkdir", "-p", str(wanted)], + ["mv", str(staged), str(wanted / NAME)]] # --- family.py siblings runs the holder's own copy --------------------------- @@ -316,3 +512,36 @@ def test_family_py_siblings_refuses_a_holder_without_its_own_copy( assert head in said assert f" --root {shlex.quote(str(holder))}`" in said assert flat(tail) in said + + +def test_family_py_siblings_escapes_what_its_stdout_cannot_encode( + diverged, tmp_path): + """#214, kept across the child process: family.py escapes what its + stdout cannot hold, and the holder's copy it now runs in a process of its + own writes into that same stdout, so it is told the same encoding and a + handler that cannot raise. An ASCII stdout gets `\\u2014` for the em dash + of the first line, and the run goes on.""" + holder = holder_in(diverged, tmp_path / NAME) + result = run_script(FAMILY, "siblings", "--family-root", str(holder), + "--dry-run", env={**ALLOW_FILE_PROTOCOL, + "PYTHONIOENCODING": "ascii"}) + assert result.returncode == 0, result.stdout + result.stderr + assert "UnicodeEncodeError" not in result.stderr + assert f"({holder}) \\u2014 2 member(s)" in result.stdout + + +# --- init --work-dir makes no family folder ---------------------------------- + + +def test_init_with_a_work_dir_promises_no_family_folder(diverged): + """`--work-dir ` puts the holder at `/` and makes no + family folder; `` is not named after the family, so `make + siblings` there clones nothing. The NEXT STEPS say that, and no longer + that it puts each member's working clone in the family folder.""" + said = diverged["init"].stdout + work = (diverged["base"] / "fam").resolve() + assert f"\nfamily folder none: {work} is not named {NAME}\n" in said + assert ("--work-dir made no family folder, so `make siblings` clones " + "nothing: it prints the `mv` that puts the holder in a folder " + f"named {NAME}, and stops until it is there.") in flat(said) + assert "in the family folder, on its tracking branch" not in flat(said) From 5ce5b6831d7de5b6deae9a149c317065b28ed5ab Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 12:32:20 +0000 Subject: [PATCH 3/5] siblings.py: a link named after the family, beside the holder's parent folder and leading to it, passes the folder check, so a holder in a symlinked family folder is no longer stopped with a printed move that cannot run (#543) The review round's N3, which round 3 found worse than first read: with `/InkRouter` a link to `/real-x`, which holds the holder, both entry points resolve the holder to `/real-x/InkRouter` and stopped, and the printed `mkdir -p /InkRouter` then `mv /real-x/InkRouter /InkRouter/InkRouter` fails with `cannot move ... to a subdirectory of itself`. This PR made that warning a stop, so it made the dead end. `templates/family-root/scripts/siblings.py`: - New `family_link(root, family)`: with R the resolved holder, P its parent and G P's parent, `G/` when `os.path.islink` says it is a link and `same_directory` (`os.path.samefile`) says it is P. `parent_folder_warning` passes on it. The holder is then reachable as `G//`, and the members are cloned into P, the link's own target, so the layout holds through the link. `make siblings` (through `find_repo_root`) and `family.py siblings` (the child stands in the resolved holder) give the same answer. - A link named after the family that leads anywhere else is not accepted: the run stops and its printed move runs as typed, putting the holder into the link's target, where the next run passes. A link elsewhere in the tree (`~/InkRouter` to `/data/real-x`) stops too, and its staged move runs as typed. - The header says `family folder

(through the link G/)` when the check passes through a link, so it names the folder the clones go into. - The case-only rename of the review round is unchanged and stays strict. A comment says why the two rules never meet: on a disk that ignores case, a case-only alias is one directory entry, never a link. - The module docstring (FIRST, EXIT CODES) and `parent_folder_warning`'s say so; so do the holder's README.md and AGENTS-shape.md. AGENTS.md's family item 5 is unchanged: its eight-line paragraph cannot take the clause without a ninth line, and the file is at its 490-line cap. tests/test_family_siblings_placement.py: the link layout passes through both entry points and `make siblings`' script clones beside the holder, into the link's target; a link named after the family to another folder stops, and its move done as written leads to a passing run; the simulated `a link` case of the case-sensitive disk test now patches `os.path.islink` too and expects no warning. Links are made with `os.symlink`, and a test skips first where the system refuses one. No link is made under `templates/`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- templates/family-root/AGENTS-shape.md | 11 +-- templates/family-root/README.md | 7 +- templates/family-root/scripts/siblings.py | 52 +++++++++++-- tests/test_family_siblings_placement.py | 89 +++++++++++++++++++++-- 4 files changed, 137 insertions(+), 22 deletions(-) diff --git a/templates/family-root/AGENTS-shape.md b/templates/family-root/AGENTS-shape.md index 0a056c6a..d4343818 100644 --- a/templates/family-root/AGENTS-shape.md +++ b/templates/family-root/AGENTS-shape.md @@ -93,11 +93,12 @@ holder (`../`). Two copies of every member is correct and intended — `validate`; the sibling is where a human works, and `family.py bump` is how this family follows it. What it will NOT do, and you must not do either: -* **It moves nothing.** When the parent folder is not named after the family - it WARNS, prints the exact `mv` for a human to run and stops, exit 1 - (`make`: 2), before it clones anything. Do not run that `mv` on your own - initiative: relocating a checkout breaks an open shell, an editor and any - LINKED WORKTREE, whose `.git` file carries an absolute path. +* **It moves nothing.** When the parent folder is not named after the family, + and no link of that name beside it leads to it, it WARNS, prints the exact + `mv` for a human to run and stops, exit 1 (`make`: 2), before it clones + anything. Do not run that `mv` on your own initiative: relocating a checkout + breaks an open shell, an editor and any LINKED WORKTREE, whose `.git` file + carries an absolute path. * **It never touches an existing clone beyond `git fetch`.** No checkout, no reset, no pull, no stash, no branch switch — somebody is working in there. A directory that is a clone of a different repository is reported and diff --git a/templates/family-root/README.md b/templates/family-root/README.md index 04668b4c..08eda1b8 100644 --- a/templates/family-root/README.md +++ b/templates/family-root/README.md @@ -64,9 +64,10 @@ named after the family because that is what the folder holds. `family.yaml` is the only thing that tells the two apart, and it is inside the holder. **Nothing ever moves a checkout.** When this holder's parent folder is not -named after the family, `make siblings` WARNS, prints the exact `mv` for a -human to run and stops before it clones anything, so that the run after the -move clones each member once — it moves nothing itself. Relocating a +named after the family, and no link of that name beside it leads to it, +`make siblings` WARNS, prints the exact `mv` for a human to run and stops +before it clones anything, so that the run after the move clones each member +once — it moves nothing itself. Relocating a directory out from under a shell, an editor or an agent lane breaks linked worktrees, whose `.git` files carry absolute paths, and this standard does not act by surprise. diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index f8f132e2..244f6199 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -36,6 +36,11 @@ and is the one act this standard refuses everywhere else: done by surprise, with no hand on it. The doctor pattern, not a mover. + A LINK named after the family, beside the holder's (resolved) + parent directory and leading to it, passes the check: that is the + family folder reached through a link, and the members are cloned + into the directory it leads to. + then, per member row in `family.yaml`: ABSENT `git clone --recurse-submodules` from the SAME url `make bootstrap` @@ -110,7 +115,8 @@ 0 every member is beside the holder; nothing to report. With `--make`: every member RAN and exited 0 1 a FINDING a human can act on: a holder whose parent folder is not - named after the family (nothing is cloned until it is moved), a + named after the family, nor linked as it (nothing is cloned until + it is moved), a sibling that is a DIFFERENT repository, a clone that failed (a tracking branch its remote does not have included), a member's own bootstrap that went red. With @@ -825,6 +831,30 @@ def same_directory(one: Path, two: Path) -> bool: return False +def family_link(root: Path, family: str) -> Path | None: + """`/` when it is a LINK to the holder's parent + folder, or None: the family folder reached through a link (#543). + + The holder's path is RESOLVED before it is asked about (`find_repo_root` + for `make siblings`, and `family.py siblings` runs this file standing in + the resolved holder), so `~/p/InkRouter`, a link to `~/p/real-x`, reads + as `real-x`. The holder is then reachable as `~/p/InkRouter/InkRouter` + and its members are cloned into `real-x`, the link's own target, so the + family layout holds through the link and the run goes on. A link named + `` that leads anywhere else is no family folder of this holder. + + Only a LINK, asked with `os.path.islink`, and never a second spelling: + on a disk that ignores case, `InkRouter` beside `inkrouter` is the same + directory entry and no link, so it is the strict test's case-only + rename below and never this. + """ + folder = root.parent + link = folder.parent / family + if os.path.islink(link) and same_directory(link, folder): + return link + return None + + def parent_folder_warning(root: Path, family: str) -> str | None: """The WARNING and the exact `mv`, or None when the layout is right. @@ -849,12 +879,14 @@ def parent_folder_warning(root: Path, family: str) -> str | None: staging name, because a rename that changes only the case is a move onto itself there. Linux, where the two are two folders, is unchanged. - otherwise the holder moves into `/`, made if missing. - A LINK named `` onto this folder is never renamed through: the - holder's path is resolved before this is asked, so a family folder - reached through a link is a question #543 leaves as it was. + + A LINK named `` beside this folder that leads to it passes the + 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. """ folder = root.parent - if folder.name == family: + if folder.name == family or family_link(root, family) is not None: return None lines = [ f"WARNING the holder's parent folder is {folder.name or str(folder)!r}" @@ -892,6 +924,9 @@ def parent_folder_warning(root: Path, family: str) -> str | None: and not wanted.is_symlink() and same_directory(wanted, folder)): # THIS FOLDER under another case, on a disk that ignores case: the # folder is renamed, in two steps, and the holder stays inside it. + # Never a link: a link to this folder passed above (`family_link`), + # and a case-only alias is one directory entry, never a link, so + # the two rules never meet. staged = folder.parent / f"{family}.folder" steps = [["mv", folder, staged], ["mv", staged, wanted]] else: @@ -1039,12 +1074,15 @@ def _run_placement(root: Path, name: str, rows: list[dict], manifest: dict, runs or any member is cloned, and `--dry-run` stops in the same words: once the holder is moved, the run after it clones each member once, in the right folder. Until the check passes, the header calls that folder - the PARENT folder, not the family's. + the PARENT folder, not the family's. Passed through a link named after + the family, it names the folder the clones go into, and the link. """ family = family_folder_name(manifest, root) warning = parent_folder_warning(root, family) + link = None if warning else family_link(root, family) print(f"siblings: {name} ({root}) — {len(rows)} member(s)") - print(f" {'parent' if warning else 'family'} folder {root.parent}") + print(f" {'parent' if warning else 'family'} folder {root.parent}" + + (f" (through the link {link})" if link else "")) print(" each member is cloned BESIDE the holder and stays PINNED inside " "it under") print(" members/ — the pinned copy is detached for bootstrap and " diff --git a/tests/test_family_siblings_placement.py b/tests/test_family_siblings_placement.py index 3eb1197d..a1452d85 100644 --- a/tests/test_family_siblings_placement.py +++ b/tests/test_family_siblings_placement.py @@ -402,9 +402,10 @@ def test_a_case_sensitive_disk_keeps_its_move(placement, monkeypatch, """Linux, where `InkRouter` and `inkrouter` are two folders: with nothing at `InkRouter`, the holder moves there through a staging name; with a folder of its own there, the holder moves into it, as it always - did. A LINK named `InkRouter` onto this folder is never renamed through: - a family folder reached through a link is resolved first, a question - #543 leaves as it was.""" + did. A LINK named `InkRouter` onto this folder is the family folder + reached through a link, so nothing is printed at all: never a rename of + the folder through it. Simulated, so a system that refuses to make a + link asks it too.""" folder = tmp_path / NAME.lower() root = folder / NAME root.mkdir(parents=True) @@ -422,8 +423,10 @@ def same(one, two) -> bool: there != "nothing" if self == wanted else exists(self))) patched.setattr(Path, "is_symlink", lambda self: ( self == wanted and there == "a link")) + patched.setattr(os.path, "islink", lambda path: ( + Path(path) == wanted and there == "a link")) patched.setattr(os.path, "samefile", same) - commands = move_commands(placement.parent_folder_warning(root, NAME)) + warning = placement.parent_folder_warning(root, NAME) staged = tmp_path / f"{NAME}.holder" expected = { "nothing": [["mv", str(root), str(staged)], @@ -433,10 +436,9 @@ def same(one, two) -> bool: ["mv", str(root), str(wanted / NAME)]], } if there in expected: - assert commands == expected[there] + assert move_commands(warning) == expected[there] else: - assert ["mv", str(folder), str(tmp_path / f"{NAME}.folder")] \ - not in commands + assert warning is None @pytest.mark.parametrize("root", [ @@ -459,6 +461,79 @@ def test_a_holder_at_a_filesystem_or_drive_root_moves_through_a_staging_name( ["mv", str(staged), str(wanted / NAME)]] +# --- the family folder reached through a link -------------------------------- + + +def link_or_skip(link: Path, target: Path) -> None: + """`link`, a directory link to `target`, made before anything else the + test does, or a skip where this system refuses one (Windows without the + right to make links).""" + try: + os.symlink(target, link, target_is_directory=True) + except (OSError, NotImplementedError) as refused: + pytest.skip(f"this system makes no directory link here: {refused}") + + +def test_a_family_folder_reached_through_a_link_passes_and_clones_into_it( + diverged, tmp_path): + """`/InkRouter` is a link to `/real-x`, which holds the holder, so + the holder is reachable as `/InkRouter/InkRouter`: the layout holds + through the link. Both entry points ask about the resolved holder and + pass, and `make siblings`' own script clones each member into `real-x`, + the link's target, beside the holder. The header names that folder and + the link.""" + real = tmp_path / "real-x" + real.mkdir() + link = tmp_path / NAME + link_or_skip(link, real) + holder_in(diverged, real) + logical = link / NAME + + shim = run_script(FAMILY, "siblings", "--family-root", str(logical), + "--dry-run", env=ALLOW_FILE_PROTOCOL) + assert shim.returncode == 0, shim.stdout + shim.stderr + assert "WARNING" not in shim.stderr + + result = siblings(logical) + assert result.returncode == 0, result.stdout + result.stderr + assert "WARNING" not in result.stderr + assert (f"\n family folder {real} (through the link {link})\n" + in result.stdout) + for project in MEMBERS: + assert (real / project / ".git").is_dir(), project + assert branch_of(link / project) == "main", project + + +def test_a_link_named_after_the_family_to_another_folder_is_not_accepted( + diverged, tmp_path): + """`/InkRouter` leads to `/other`, not to `/real-x`, where the + holder is: no family folder of this holder, so the run stops. Its move, + done as written, puts the holder into `other` through the link, where + the next run passes.""" + real, other = tmp_path / "real-x", tmp_path / "other" + real.mkdir() + other.mkdir() + link = tmp_path / NAME + link_or_skip(link, other) + holder = holder_in(diverged, real) + + stopped = siblings(holder, "--dry-run") + assert stopped.returncode == 1, stopped.stdout + stopped.stderr + assert "NOTHING WAS CLONED" in stopped.stderr + assert f"\n parent folder {real}\n" in stopped.stdout + commands = move_commands(stopped.stderr) + assert commands == [["mkdir", "-p", str(link)], + ["mv", str(holder), str(link / NAME)]] + for argv in commands: + if argv[0] == "mkdir": + Path(argv[2]).mkdir(parents=True, exist_ok=True) + else: + os.rename(argv[1], argv[2]) + assert (other / NAME / "family.yaml").is_file() + moved = siblings(link / NAME, "--dry-run") + assert moved.returncode == 0, moved.stdout + moved.stderr + + # --- family.py siblings runs the holder's own copy --------------------------- From 0112dcb6339189e66a303421580c069cbaa0a545 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 12:35:22 +0000 Subject: [PATCH 4/5] tests/test_family_siblings_placement.py: the simulated case-sensitive disk with a separate `InkRouter` folder is skipped on Windows, where WindowsPath equality ignores case, so tests-windows no longer fails on a layout a Windows disk cannot hold (#543) Found by rv543b. `same_directory()` asks equal paths first, and a WindowsPath compares without case, so on Windows `

/InkRouter` and `

/inkrouter` are one path and the case-only branch prints the folder rename. That answer is right for Windows, whose disk cannot hold two folders that differ only in case, but it is not the case-sensitive disk this parameter simulates. The `[a folder]` parameter is now skipped there by a `skipif` mark, before any fixture or call; `[nothing]` and `[a link]` still run everywhere. Emulated on Linux with rv543b's mechanism (`pathlib._is_case_sensitive` returning False during each test's call): the file gives `1 failed, 24 passed` with the parameter collected, `os.name` being `posix` here, and `24 passed, 1 deselected` without it. Without the emulation: `25 passed`. A second commit rather than an amend: the round's rules forbid amending. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_family_siblings_placement.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_family_siblings_placement.py b/tests/test_family_siblings_placement.py index a1452d85..2cb17303 100644 --- a/tests/test_family_siblings_placement.py +++ b/tests/test_family_siblings_placement.py @@ -396,7 +396,14 @@ def test_a_parent_folder_in_another_case_is_renamed_on_a_disk_that_ignores_it( NAME) is None -@pytest.mark.parametrize("there", ["nothing", "a folder", "a link"]) +@pytest.mark.parametrize("there", [ + "nothing", + pytest.param("a folder", marks=pytest.mark.skipif( + os.name == "nt", reason="WindowsPath equality ignores case, so a " + "case-sensitive disk with a separate folder cannot be simulated " + "there; a Windows disk cannot hold the two folders anyway")), + "a link", +]) def test_a_case_sensitive_disk_keeps_its_move(placement, monkeypatch, tmp_path, there): """Linux, where `InkRouter` and `inkrouter` are two folders: with From 8d3d1cceb30dfc93c7ceec130131038dc4f89e40 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 12:58:21 +0000 Subject: [PATCH 5/5] siblings.py: a Windows directory junction named after the family, beside the holder's parent folder and leading to it, passes the folder check as a link does, so a family folder reached through a junction no longer stops with a printed move that cannot run (#543) Round 4, from rv543b's review of 0112dcb6. - S-1. `family_link` asked only `os.path.islink`, which is False for an NTFS directory junction, the link Windows makes without privilege. `Path.resolve()` follows a junction, so that layout stopped and its printed `mv` failed, where bfed143 warned and cloned. It now accepts `os.path.isjunction` too (Python 3.12 and later, False off Windows), read with `getattr` so an older Python still runs the file. Not an "any alias" rule: that would make a link spelled in another case a dead end on macOS. - F2. AGENTS.md family item 5 says "(nor linked as it)", as the holder's README.md and AGENTS-shape.md already do, rewrapped within the same eight lines by dropping one "yourself". AGENTS.md stays at 490. - F3. The comment beside the case-only rename no longer says the two rules never meet: a link spelled in another case stops on Linux, with a staged move that runs, and passes on a disk that ignores case. The Windows skip's reason now says the simulation assumes Windows' default path equality, since an NTFS folder made case-sensitive can hold the two folders. - F4. A test for each condition no test guarded: `init --work-dir` into a folder named after the family keeps the promise; a blank `tracking_branch` is not a branch name; a second path to the folder under another name, neither link nor junction, does not get the case-only folder rename. Tests: tests/test_family_siblings_placement.py, 29 tests, among them a simulated junction that every system runs. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 14 ++-- templates/family-root/scripts/siblings.py | 20 ++++-- tests/test_family_siblings_placement.py | 80 +++++++++++++++++++++-- 3 files changed, 96 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0f294fd5..ff4d9ea2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -405,13 +405,13 @@ python3 scripts/family.py siblings --family-root `members/` inside the holder is pinned and DETACHED, for `bootstrap` and `validate`; the sibling is where the human works. **What it never does, and neither do you:** it MOVES NOTHING — when the - parent folder is not named after the family it clones nothing, prints an `mv` - and exits 1 (`make`: 2), and running that `mv` yourself relocates a checkout - out from under a shell, an editor and every agent lane, and breaks any linked - worktree (its `.git` file carries an absolute path). An existing clone gets - only `git fetch` — no checkout, no reset, no pull — because somebody works - in there; one of a different repository is reported and skipped, and the - exit is the human's hand, not an overwrite. + parent folder is not named after the family (nor linked as it) it clones + nothing, prints an `mv` and exits 1 (`make`: 2), and running that `mv` + relocates a checkout out from under a shell, an editor and every agent lane, + and breaks any linked worktree (its `.git` file carries an absolute path). An + existing clone gets only `git fetch` — no checkout, no reset, no pull — + because somebody works in there; one of a different repository is reported + and skipped, and the exit is the human's hand, not an overwrite. 6. `update-shape.py` updates a family root exactly as it updates a project, with the same four refusals, and mirrors into `family.yaml`. An existing holder receives `scripts/siblings.py` that way: `check` reports it as diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 244f6199..d7496b5a 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -843,14 +843,18 @@ def family_link(root: Path, family: str) -> Path | None: family layout holds through the link and the run goes on. A link named `` that leads anywhere else is no family folder of this holder. - Only a LINK, asked with `os.path.islink`, and never a second spelling: - on a disk that ignores case, `InkRouter` beside `inkrouter` is the same - directory entry and no link, so it is the strict test's case-only - rename below and never this. + Only a LINK: a symbolic link (`os.path.islink`), or a Windows directory + junction (`os.path.isjunction`, Python 3.12 and later), the link Windows + makes without privilege and that `Path.resolve()` follows the same way. + Never a second spelling: on a disk that ignores case, `InkRouter` beside + `inkrouter` is one directory entry and no link, so it is the strict + test's case-only rename below. """ folder = root.parent link = folder.parent / family - if os.path.islink(link) and same_directory(link, folder): + junction = getattr(os.path, "isjunction", lambda _: False) + if ((os.path.islink(link) or junction(link)) + and same_directory(link, folder)): return link return None @@ -925,8 +929,10 @@ def parent_folder_warning(root: Path, family: str) -> str | None: # THIS FOLDER under another case, on a disk that ignores case: the # folder is renamed, in two steps, and the holder stays inside it. # Never a link: a link to this folder passed above (`family_link`), - # and a case-only alias is one directory entry, never a link, so - # the two rules never meet. + # and a case-only alias is one directory entry, never a link. They + # 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" steps = [["mv", folder, staged], ["mv", staged, wanted]] else: diff --git a/tests/test_family_siblings_placement.py b/tests/test_family_siblings_placement.py index 2cb17303..c5d6e449 100644 --- a/tests/test_family_siblings_placement.py +++ b/tests/test_family_siblings_placement.py @@ -137,6 +137,21 @@ def stored(path) -> Path | None: return found +def another_path_to(patched, alias: Path, folder: Path) -> None: + """`alias` exists and is the same directory as `folder`, as a disk with + a second path to that folder answers; every other path as it is.""" + exists, samefile = Path.exists, os.path.samefile + + def same(one, two) -> bool: + pair = {Path(one), Path(two)} + return pair == {alias, folder} if alias in pair else samefile(one, + two) + + patched.setattr(Path, "exists", + lambda self, **_: self == alias or exists(self)) + patched.setattr(os.path, "samefile", same) + + def ignoring_case(patched) -> None: """`Path.exists` and `os.path.samefile` answering as macOS and Windows disks do, here on any system: a spelling in another case exists, and is @@ -237,7 +252,8 @@ def test_the_dry_run_names_the_branch_it_would_clone_on(diverged, tmp_path): "holds a character that does not print, so it is not used as a branch"), (b"5", "5", "is not a branch name"), (b"[main]", "['main']", "is not a branch name"), -], ids=["escape-sequence", "a-number", "a-list"]) + (b'" "', "' '", "is not a branch name"), +], ids=["escape-sequence", "a-number", "a-list", "blank"]) def test_a_tracking_branch_that_is_no_branch_name_is_shown_and_not_used( diverged, tmp_path, declared, shown, why): """A value no branch can have is not handed to git and not printed raw: @@ -399,9 +415,10 @@ def test_a_parent_folder_in_another_case_is_renamed_on_a_disk_that_ignores_it( @pytest.mark.parametrize("there", [ "nothing", pytest.param("a folder", marks=pytest.mark.skipif( - os.name == "nt", reason="WindowsPath equality ignores case, so a " - "case-sensitive disk with a separate folder cannot be simulated " - "there; a Windows disk cannot hold the two folders anyway")), + os.name == "nt", reason="the simulation assumes Windows' default " + "path equality, which ignores case, so a separate folder whose name " + "differs only in case cannot be told apart there (an NTFS folder " + "made case-sensitive can hold one)")), "a link", ]) def test_a_case_sensitive_disk_keeps_its_move(placement, monkeypatch, @@ -541,6 +558,43 @@ def test_a_link_named_after_the_family_to_another_folder_is_not_accepted( assert moved.returncode == 0, moved.stdout + moved.stderr +def test_a_family_folder_reached_through_a_windows_junction_passes( + placement, monkeypatch, tmp_path): + """A directory junction, the link Windows makes without privilege: + `os.path.islink` says no and `os.path.isjunction` (3.12) says yes, and + `Path.resolve()` follows it as it follows a link. It is the family + folder reached through a link. Simulated, so every system asks it.""" + folder = tmp_path / "real-x" + root = folder / NAME + root.mkdir(parents=True) + junction = tmp_path / NAME + with monkeypatch.context() as patched: + another_path_to(patched, junction, folder) + patched.setattr(os.path, "islink", lambda path: False) + patched.setattr(os.path, "isjunction", + lambda path: Path(path) == junction, raising=False) + found = placement.family_link(root, NAME) + warning = placement.parent_folder_warning(root, NAME) + assert found == junction + assert warning is None + + +def test_a_folder_is_renamed_only_for_a_difference_of_case( + placement, monkeypatch, tmp_path): + """A second path to this folder under another NAME that is neither a + link nor a junction (a bind mount, say) is not answered by renaming the + folder: that rename is for a name that differs only in case.""" + folder = tmp_path / "real-x" + root = folder / NAME + root.mkdir(parents=True) + alias = tmp_path / NAME + with monkeypatch.context() as patched: + another_path_to(patched, alias, folder) + warning = placement.parent_folder_warning(root, NAME) + assert ["mv", str(folder), str(tmp_path / f"{NAME}.folder")] \ + not in move_commands(warning) + + # --- family.py siblings runs the holder's own copy --------------------------- @@ -627,3 +681,21 @@ def test_init_with_a_work_dir_promises_no_family_folder(diverged): "nothing: it prints the `mv` that puts the holder in a folder " f"named {NAME}, and stops until it is there.") in flat(said) assert "in the family folder, on its tracking branch" not in flat(said) + + +def test_init_with_a_work_dir_named_after_the_family_keeps_the_promise( + tmp_path): + """`--work-dir /InkRouter` lands the holder at + `/InkRouter/InkRouter`: a folder named after the family, where `make + siblings` runs to the end, so the NEXT STEPS promise the clones there.""" + work = tmp_path / NAME + result = run_script(FAMILY, "init", "--org", NAME, "--family", NAME, + "--created-by", "Test Human", + "--created-on", "2026-10-15", + "--local-remote-dir", str(tmp_path / "remotes"), + "--work-dir", str(work), cwd=tmp_path) + assert result.returncode == 0, result.stdout + result.stderr + assert f"\nfamily folder {work.resolve()}\n" in result.stdout + assert ("`make siblings` then puts each member's WORKING clone beside " + "the holder, in the family folder, on its tracking branch." + ) in flat(result.stdout)