From 9ddb7efe6e6ede70f8cef9cfdf804016ef93ad76 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 22:41:22 +0000 Subject: [PATCH 1/8] adopt-project.py: the Makefile block `execute` appends and the cross-leg follow-up name each leg where the plan mounts it, so `--spec-path legs/spec` exports a `CONTRACTS_DIR` that exists and a nested `--code-path` is told the `../` path that reaches the spec leg (#522) Both rendered a leg's mount path as if it were the default (#522, from the v1.1.0 triage of #457's known limits): - `ADOPT_MAKEFILE_BLOCK` was one constant exporting `CONTRACTS_DIR ?= $(CURDIR)/spec/contracts`, so an adoption with `--spec-path legs/spec` pointed the code leg's tooling at a directory that does not exist, while `plan`'s own follow-up said the Makefile wires `$(CURDIR)/legs/spec/contracts`. The block is now `shape_materialize.adopt_makefile_block(spec_path, code_path)`, rendered with `execute`'s checked mount paths. It stays the adoption's own append, digested with the copy by `_materialize`, so no pinned copy is edited by hand; a default adoption gets byte-identical bytes, and `ADOPT_MAKEFILE_BLOCK` remains as that default rendering. - auA-13: the follow-up for a code file naming a spec directory built `..//` "relative to `/`", one `..` at any depth; `--spec-path legs/spec --code-path legs/code` printed `../legs/spec/contracts/`. `spec_leg_from_code_leg` climbs once per segment of the code leg's path below what the two paths share. tests/test_adopt_contracts_dir.py checks each path by where it leads: the exported directory and the follow-up's read-across hold the source's contract in a recursive clone of a real split, and `make contracts-dir` prints that directory. Main: bfed143e89f97d3480cf5373051cb1ace82db312. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 39 +++- scripts/shape_materialize.py | 36 +++- tests/test_adopt_contracts_dir.py | 287 ++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+), 11 deletions(-) create mode 100644 tests/test_adopt_contracts_dir.py diff --git a/adopt-project.py b/adopt-project.py index e4fa0451..5ce82d5e 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -171,7 +171,7 @@ tree_digest, ) from shape_materialize import ( # noqa: E402 - ADOPT_MAKEFILE_BLOCK, PLACEHOLDER_RE, SHAPE_REPOSITORY, SYMLINK_HOPS, + PLACEHOLDER_RE, SHAPE_REPOSITORY, SYMLINK_HOPS, adopt_makefile_block, TEMPLATED, TOO_MANY_SYMLINKS, CommandFailed, Materialized, assembly_root_paths, collision_follow_up, copy_tree, default_reference, election_date, env_commit, escape_what_stdout_cannot_encode, @@ -1679,12 +1679,13 @@ def cross_leg_references(source: "Source", entries: list["Entry"], # DOES read it under twenty of them is how a follow-up list stops # being read at all. hits.sort(key=lambda h: ("fixture" in h or h.startswith("tests/"), h)) + across = spec_leg_from_code_leg(spec_path, code_path) for hit in hits[:8]: shown = "".join(map(_spelled_character, hit)) out.append( f"{code_path}/{shown} names `{named}`, which moves to the " f"spec leg: it must read across the assembly root — " - f"`../{spec_path}/{named}` relative to `{code_path}/` — or " + f"`{across}/{named}` relative to `{code_path}/` — or " "take the path from the environment") if len(hits) > 8: out.append(f"… and {len(hits) - 8} more file(s) under the code leg " @@ -1692,6 +1693,34 @@ def cross_leg_references(source: "Source", entries: list["Entry"], return out +def spec_leg_from_code_leg(spec_path: str, code_path: str) -> str: + """The spec leg's mount, `spec_path`, as a path relative to the code + leg's, `code_path`: `../spec` for the defaults, `../spec` for + `legs/spec` beside `legs/code`, and `../../spec` for `spec` beside + `impl/code`. + + ONE `..` PER SEGMENT OF THE CODE LEG'S PATH BELOW WHAT THE TWO SHARE + (#522, auA-13). The follow-up wrote `../`, which is right only + while the code leg sits in the root: with `--spec-path legs/spec + --code-path legs/code` it said `../legs/spec/contracts/` "relative to + `legs/code/`", a path that leads to `legs/legs/spec`. + + Both are canonical mount paths, asked by `plan` before it writes a + follow-up and by `execute` before it mounts a leg: `/` between + segments, and no segment empty, `.` or `..`. So the segments are + compared as text, and nothing is resolved against a disk. Two segments + that differ only in case are two segments here, which climbs one level + more than a disk that folds case needs and still leads to the same + directory. + """ + spec, code = spec_path.split("/"), code_path.split("/") + shared = 0 + while (shared < min(len(spec), len(code)) + and spec[shared] == code[shared]): + shared += 1 + return "/".join([".."] * (len(code) - shared) + spec[shared:]) + + def _search_by_hand(named: str, why: str) -> str: """The follow-up for a spec directory git did not search for: the reason, and what to look for instead.""" @@ -8132,8 +8161,10 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, step = "committing the split" # The plan's submodule answers outlive the plan only here (#189). # An empty record appends nothing, so a source with no submodule - # gets the manifest it always got. - append = {"Makefile": ADOPT_MAKEFILE_BLOCK} + # gets the manifest it always got. The Makefile's block names the + # spec leg where this plan mounts it (#522), and a default plan gets + # the block it always got. + append = {"Makefile": adopt_makefile_block(spec_path, code_path)} record = submodule_record(plan.entries, source.tree(), source.commit) if record: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index fe2c40e6..fd61b38e 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -975,17 +975,34 @@ def copy_in_the_way(target: Path, actual: str) -> str | None: return None -#: Appended to the assembly root's Makefile by `adopt-project.py` ONLY. A -#: scaffolded project's legs are empty, so nothing reads across them yet; an -#: ADOPTED project's code leg holds tooling that used to read `contracts/` -#: from beside it and now reads it from the other leg through the root. -ADOPT_MAKEFILE_BLOCK = """ +def adopt_makefile_block(spec_path: str, code_path: str) -> str: + """What `adopt-project.py` appends to the assembly root's Makefile, and + nothing else does. A scaffolded project's legs are empty, so nothing reads + across them yet; an ADOPTED project's code leg holds tooling that used to + read `contracts/` from beside it and now reads it from the other leg + through the root. + + RENDERED WITH THE PLAN'S MOUNT PATHS (#522). It was one constant naming + `spec/contracts`, so a project adopted with `--spec-path legs/spec` + exported a `CONTRACTS_DIR` that does not exist, while `plan`'s own + follow-up said the Makefile wires `$(CURDIR)/legs/spec/contracts`. The + block is the adoption's own append, made once by `execute` and digested + with the copy it is appended to (`_materialize` pins the bytes as + written), so the per-project text is written here and nowhere by hand. + A default adoption, `spec` and `code`, gets the bytes it always got + (`ADOPT_MAKEFILE_BLOCK`). + + Both paths are `execute`'s checked mount paths: canonical, and in + `checked_value`'s alphabet, which holds no character make reads as + anything but itself (no `$`, `#`, space or line break). + """ + return f""" # --- adopted project: reading ACROSS the legs ------------------------------- # The spec leg owns `contracts/`; the code leg holds the tooling that reads -# them. Mounted here, `spec/contracts` is one relative path from `code/`, so +# them. Mounted here, `{spec_path}/contracts` is one relative path from `{code_path}/`, so # the root exports it once and the code leg takes it from the environment # rather than each script guessing at `../`. -CONTRACTS_DIR ?= $(CURDIR)/spec/contracts +CONTRACTS_DIR ?= $(CURDIR)/{spec_path}/contracts export CONTRACTS_DIR .PHONY: contracts-dir @@ -994,6 +1011,11 @@ def copy_in_the_way(target: Path, actual: str) -> str | None: """ +#: The block a DEFAULT adoption appends, the legs at `spec` and `code`: +#: byte for byte the block every adoption appended before #522. +ADOPT_MAKEFILE_BLOCK = adopt_makefile_block("spec", "code") + + class CommandFailed(Exception): def __init__(self, args, cwd, code, output): super().__init__(" ".join(args)) diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py new file mode 100644 index 00000000..10ddb1a7 --- /dev/null +++ b/tests/test_adopt_contracts_dir.py @@ -0,0 +1,287 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A leg mounted away from its default path is read across WHERE IT IS (#522). + +Two things `adopt-project.py` writes named a leg's mount path as if it were +the default: + + * the block `execute` appends to the assembly root's Makefile exported + `CONTRACTS_DIR ?= $(CURDIR)/spec/contracts` whatever `--spec-path` said, + so a project adopted with `--spec-path legs/spec` pointed its code leg's + tooling at a directory that does not exist, while `plan`'s own follow-up + said the Makefile wires `$(CURDIR)/legs/spec/contracts`; + * the follow-up for a code file that names a spec directory said + `..//` "relative to `/`", one `..` whatever + the code leg's depth: `--spec-path legs/spec --code-path legs/code` + printed `../legs/spec/contracts/`, where `../spec/contracts/` is right + (auA-13 on #457). + +The block is now rendered with the plan's mount paths, and a DEFAULT +adoption gets the bytes it always got; the follow-up climbs once per segment +of the code leg's path below what the two paths share. Each relative path is +checked by where it LEADS, not only by how it is spelled. + +`plan` needs no `git filter-repo`; the end-to-end tests at the bottom do, and +skip without it, as `tests/test_adopt_e2e.py` does. No network and no +GitHub: the legs are bare repositories on disk, and `tests/conftest.py` +keeps a `gh` guard first on PATH. +""" + +from __future__ import annotations + +import hashlib +import importlib.util +import posixpath +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import (ADOPT, FILE_PROTOCOL, REPO, git, make_source_repo, + run_script, write_plan) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from repo_shape import load_yaml # noqa: E402 + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +PROJECT = "Northwind" +SPLIT_BRANCH = "adopt/three-repo-shape" + +#: The block every adoption appended before #522, byte for byte. A default +#: adoption (`spec`, `code`) must keep appending exactly this: its bytes are +#: what the pin of every project adopted so far digests. +DEFAULT_BLOCK = """ +# --- adopted project: reading ACROSS the legs ------------------------------- +# The spec leg owns `contracts/`; the code leg holds the tooling that reads +# them. Mounted here, `spec/contracts` is one relative path from `code/`, so +# the root exports it once and the code leg takes it from the environment +# rather than each script guessing at `../`. +CONTRACTS_DIR ?= $(CURDIR)/spec/contracts +export CONTRACTS_DIR + +.PHONY: contracts-dir +contracts-dir: +\t@echo $(CONTRACTS_DIR) +""" + +#: A source with nothing for the plan to ask about, no Makefile of its own +#: (so the shape's, with the block, is the root's), and one code file that +#: names `contracts/`, which the rules send to the spec leg. +READER = "src/app/main.py" +POLICY = "policy.yaml" +TREE = { + "README.md": "# Thing\n", + "specs/001-feature/spec.md": "# The feature\n", + f"contracts/{POLICY}": "schema_version: 1\nkind: policy\n", + READER: f"CONTRACTS = 'contracts/{POLICY}'\n", + "tests/test_main.py": "def test_ok():\n assert True\n", +} +EDITS = (("specs/001-feature/spec.md", + "# The feature\n\nA second paragraph.\n", + "Extend the specification"),) + +#: `(spec path, code path, the spec leg's mount relative to the code leg's)`. +MOUNTS = [ + pytest.param("spec", "code", "../spec", id="defaults"), + pytest.param("legs/spec", "legs/code", "../spec", id="siblings-in-legs"), + pytest.param("spec", "impl/code", "../../spec", id="code-nested"), + pytest.param("legs/spec", "code", "../legs/spec", id="spec-nested"), + pytest.param("legs/spec", "impl/code", "../../legs/spec", + id="both-nested-apart"), + pytest.param("a/b/spec", "a/code", "../b/spec", id="one-segment-shared"), + pytest.param("spec", "a/b/c/code", "../../../../spec", id="code-deep"), + pytest.param("Legs/spec", "legs/code", "../../Legs/spec", + id="case-differs"), +] + + +@pytest.fixture(scope="module") +def adopter(): + """`adopt-project.py` loaded as a module, for its helpers.""" + module_spec = importlib.util.spec_from_file_location( + "adopt_contracts_dir", ADOPT) + module = importlib.util.module_from_spec(module_spec) + module_spec.loader.exec_module(module) + return module + + +def contracts_dir_line(block: str) -> str: + (line,) = [line for line in block.splitlines() + if line.startswith("CONTRACTS_DIR ")] + return line + + +# --- the Makefile block ----------------------------------------------------- + +def test_a_default_adoption_appends_the_block_it_always_did(): + """`spec` and `code` render the pre-#522 block byte for byte, and the + constant that names it is that rendering.""" + render = shape_materialize.adopt_makefile_block + assert render("spec", "code") == DEFAULT_BLOCK + assert shape_materialize.ADOPT_MAKEFILE_BLOCK == DEFAULT_BLOCK + + +@pytest.mark.parametrize("spec_path, code_path", [ + ("legs/spec", "legs/code"), ("spec-v2", "code"), + ("legs/spec", "impl/code"), ("a..b", "x/code")]) +def test_the_block_exports_the_spec_leg_where_the_plan_mounts_it( + spec_path, code_path): + block = shape_materialize.adopt_makefile_block(spec_path, code_path) + assert contracts_dir_line(block) == ( + f"CONTRACTS_DIR ?= $(CURDIR)/{spec_path}/contracts") + assert "$(CURDIR)/spec/contracts" not in block + # The comment that says why names the same two paths. + assert (f"Mounted here, `{spec_path}/contracts` is one relative path " + f"from `{code_path}/`") in block + # Everything but those two lines is the default block's. + def rest(text: str) -> list[str]: + return [line for line in text.splitlines() + if not line.startswith(("CONTRACTS_DIR ", "# them. Mounted"))] + assert rest(block) == rest(DEFAULT_BLOCK) + + +# --- the cross-leg follow-up ------------------------------------------------ + +@pytest.mark.parametrize("spec_path, code_path, expected", MOUNTS) +def test_the_spec_leg_is_named_relative_to_where_the_code_leg_is( + adopter, spec_path, code_path, expected): + across = adopter.spec_leg_from_code_leg(spec_path, code_path) + assert across == expected + # And it LEADS there: from the code leg, the path is the spec leg. + assert posixpath.normpath(posixpath.join(code_path, across)) == spec_path + + +@pytest.fixture(scope="module") +def source(tmp_path_factory) -> Path: + """One source for every `plan` below; `plan` only reads it.""" + return make_source_repo(tmp_path_factory.mktemp("contracts-dir") / "Thing", + TREE, edits=EDITS) + + +def planned_follow_ups(source: Path, out: Path, spec_path: str, + code_path: str) -> list[str]: + written = write_plan(source, out, project=PROJECT, + extra=("--spec-path", spec_path, + "--code-path", code_path)) + assert written.returncode == 0, written.stderr + written.stdout + return [str(item) for item in load_yaml(out)["follow_ups"]] + + +@pytest.mark.parametrize("spec_path, code_path, expected", MOUNTS) +def test_plan_names_the_read_across_from_where_the_code_leg_is( + source, tmp_path, spec_path, code_path, expected): + """The follow-up for the code file that names `contracts/` gives the + path from the code leg's mount, and that path leads to the spec leg's + `contracts/`. The defaults print what they always printed.""" + follow_ups = planned_follow_ups(source, tmp_path / "plan.yaml", + spec_path, code_path) + (named,) = [item for item in follow_ups + if item.startswith(f"{code_path}/{READER} names ")] + assert named == ( + f"{code_path}/{READER} names `contracts/`, which moves to the spec " + f"leg: it must read across the assembly root — `{expected}/" + f"contracts/` relative to `{code_path}/` — or take the path from " + "the environment") + assert posixpath.normpath(posixpath.join( + code_path, f"{expected}/contracts/")) == f"{spec_path}/contracts" + # The Makefile follow-up and the block `execute` appends agree. + assert any(f"`CONTRACTS_DIR ?= $(CURDIR)/{spec_path}/contracts`" in item + for item in follow_ups), follow_ups + + +# --- `execute`, end to end -------------------------------------------------- + +SPEC_PATH, CODE_PATH, ACROSS = "legs/spec", "impl/code", "../../legs/spec" + + +@pytest.fixture(scope="module") +def adopted(tmp_path_factory) -> dict: + """One real adoption with both legs mounted away from their defaults, + and a recursive clone of its split branch. Its own source: `execute` + pushes the split branch into the repository it adopts.""" + base = tmp_path_factory.mktemp("contracts-dir-adopted") + source = make_source_repo(base / "Thing", TREE, edits=EDITS) + plan = base / "adoption-plan.yaml" + follow_ups = planned_follow_ups(source, plan, SPEC_PATH, CODE_PATH) + checked = run_script(ADOPT, "check", "--plan", str(plan)) + assert checked.returncode == 0, checked.stderr + checked.stdout + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work")) + assert result.returncode == 0, result.stderr + result.stdout + assert "adoption verified" in result.stdout, result.stdout + clone = base / PROJECT + proc = subprocess.run( + ["git", *FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", + "-b", SPLIT_BRANCH, str(source), str(clone)], + capture_output=True, text=True, check=False) + assert proc.returncode == 0, proc.stderr + makefile = subprocess.run( + ["git", "show", f"{SPLIT_BRANCH}:Makefile"], cwd=str(source), + capture_output=True, check=True).stdout + return {"source": source, "clone": clone, "follow_ups": follow_ups, + "makefile": makefile} + + +@needs_filter_repo +def test_the_adopted_makefile_exports_the_spec_legs_contracts(adopted): + """The split's Makefile (the shape's, at the root: the source kept none) + exports `$(CURDIR)/legs/spec/contracts`, and in a recursive clone that + is the directory holding the source's contracts.""" + text = adopted["makefile"].decode("utf-8") + line = contracts_dir_line(text) + exported = adopted["clone"].joinpath( + *line.split("$(CURDIR)/", 1)[1].split("/")) + assert (exported / POLICY).is_file(), line + assert line == f"CONTRACTS_DIR ?= $(CURDIR)/{SPEC_PATH}/contracts" + assert text.endswith( + shape_materialize.adopt_makefile_block(SPEC_PATH, CODE_PATH)) + + +@needs_filter_repo +@pytest.mark.skipif(shutil.which("make") is None or sys.platform == "win32", + reason="needs make, and a POSIX `$(CURDIR)` spelling") +def test_make_contracts_dir_prints_a_directory_that_exists(adopted): + made = subprocess.run( + ["make", "--no-print-directory", "-s", "contracts-dir"], + cwd=str(adopted["clone"]), capture_output=True, text=True, + encoding="utf-8", errors="replace", check=False) + assert made.returncode == 0, made.stdout + made.stderr + printed = Path(made.stdout.strip()) + assert printed == adopted["clone"].resolve() / SPEC_PATH / "contracts" + assert (printed / POLICY).is_file() + + +@needs_filter_repo +def test_the_pin_digests_the_rendered_makefile(adopted): + """The block is the adoption's own append, so the shape pin records the + Makefile as written, block included, and the project's own pin + validator passes over it.""" + rows = {row["path"]: row["sha256"] for row in load_yaml( + adopted["clone"] / "contracts" / "shape-pin.yaml")["files"]} + assert rows["Makefile"] == hashlib.sha256(adopted["makefile"]).hexdigest() + validated = subprocess.run( + [sys.executable, "scripts/validate-pins.py"], cwd=str(adopted["clone"]), + capture_output=True, text=True, encoding="utf-8", errors="replace", + check=False) + assert validated.returncode == 0, validated.stdout + validated.stderr + + +@needs_filter_repo +def test_the_follow_ups_read_across_leads_to_the_contracts(adopted): + """What the plan told the person to write in the code leg, followed from + the code leg of a recursive clone, reaches the spec leg's file.""" + (named,) = [item for item in adopted["follow_ups"] + if item.startswith(f"{CODE_PATH}/{READER} names ")] + assert f"`{ACROSS}/contracts/` relative to `{CODE_PATH}/`" in named + code_leg = adopted["clone"].joinpath(*CODE_PATH.split("/")) + assert (code_leg / READER).is_file() + target = code_leg.joinpath(*ACROSS.split("/"), "contracts", POLICY) + assert target.resolve().is_file() + assert target.resolve() == (adopted["clone"].joinpath( + *SPEC_PATH.split("/"), "contracts", POLICY)).resolve() From 6e4d2b844f60392c8b1d5e3815ad37f58d14bf36 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:03:25 +0000 Subject: [PATCH 2/8] adopt-project.py: the Makefile follow-up names the `legs.spec_path` key and no path, so a hand edit of the spec leg's mount no longer leaves the split commit's message saying `$(CURDIR)/spec/contracts` over a Makefile that says `$(CURDIR)/legs/spec/contracts` (#522) `plan` writes the follow-ups once. `execute` renders the Makefile's `CONTRACTS_DIR` block from the plan's CURRENT `legs.spec_path`, and the tool's own remediations ask for a hand edit of it (`MOUNT_REMEDIATION`, `plan-leg-path-collides`). The follow-up named the plan-time path, so after that edit the split commit's message, which is the pull request's body, and the Makefile in the same commit disagreed. It now says the Makefile wires `CONTRACTS_DIR` to the `contracts/` of the spec leg, where the plan's `legs.spec_path` mounts it, the way #493-F3 names `tracking_branch`. tests/test_adopt_contracts_dir.py holds the plan test to that one line for every mount and adds an end-to-end test: a default plan, edited to `legs/spec` and `impl/code`, then `check` and `execute`; the commit message carries no `$(CURDIR)` path and the Makefile names the edited one. The end-to-end fixture is now one helper for the flag and the hand-edit adoption. Review of PR #572 at 9ddb7efe, F1. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 15 ++++-- tests/test_adopt_contracts_dir.py | 84 +++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 13 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 5ce82d5e..7c2ff13c 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1747,15 +1747,24 @@ def follow_ups_for(source: "Source", entries: list["Entry"], names: dict, of `tracking_branch:`, which `plan-tracking-branch-missing`'s remediation asks for, in `check`'s output and in the pull request's own body (#493's review, F3). + + The Makefile follow-up does the same for the spec leg's mount: it names + `legs.spec_path` and no path. `execute` renders the Makefile's block + from the plan's CURRENT `legs.spec_path`, and `MOUNT_REMEDIATION` and + `plan-leg-path-collides` both ask for a hand edit of it, so a path named + here went on naming the old mount in the split commit's message, the + pull request's own body, while the Makefile in that commit named the new + one (#522's review, F1). """ out = cross_leg_references(source, entries, spec_path, code_path) if any(e.leg == "spec" and (e.path == "contracts/" or e.path.startswith("contracts/")) for e in entries): out.append( - f"the root Makefile wires `CONTRACTS_DIR ?= $(CURDIR)/{spec_path}/" - "contracts` and exports it, so the code leg's tooling takes the " - "path from the environment instead of assuming it sits beside it") + "the root Makefile wires `CONTRACTS_DIR` to the `contracts/` of " + "the spec leg, where the plan's `legs.spec_path` mounts it, and " + "exports it, so the code leg's tooling takes the path from the " + "environment instead of assuming it sits beside it") if any(e.path in ("CODEOWNERS", ".github/CODEOWNERS") for e in entries): out.append( "CODEOWNERS in the assembly root now names paths that live in the " diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index 10ddb1a7..f9c004b3 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -69,6 +69,16 @@ \t@echo $(CONTRACTS_DIR) """ +#: The follow-up that says what the root Makefile does. It names the KEY the +#: block is rendered from and no path: `execute` renders the block from the +#: plan's CURRENT `legs.spec_path`, and the tool's own remediations ask for a +#: hand edit of it after `plan` wrote this line (#522's review, F1). +MAKEFILE_FOLLOW_UP = ( + "the root Makefile wires `CONTRACTS_DIR` to the `contracts/` of the spec " + "leg, where the plan's `legs.spec_path` mounts it, and exports it, so the " + "code leg's tooling takes the path from the environment instead of " + "assuming it sits beside it") + #: A source with nothing for the plan to ask about, no Makefile of its own #: (so the shape's, with the block, is the root's), and one code file that #: names `contracts/`, which the rules send to the spec leg. @@ -189,9 +199,10 @@ def test_plan_names_the_read_across_from_where_the_code_leg_is( "the environment") assert posixpath.normpath(posixpath.join( code_path, f"{expected}/contracts/")) == f"{spec_path}/contracts" - # The Makefile follow-up and the block `execute` appends agree. - assert any(f"`CONTRACTS_DIR ?= $(CURDIR)/{spec_path}/contracts`" in item - for item in follow_ups), follow_ups + # The Makefile follow-up names the key, so it is the same line for every + # mount, and it names no path for a hand edit of the key to contradict. + assert follow_ups.count(MAKEFILE_FOLLOW_UP) == 1, follow_ups + assert not [item for item in follow_ups if "$(CURDIR)" in item] # --- `execute`, end to end -------------------------------------------------- @@ -199,15 +210,30 @@ def test_plan_names_the_read_across_from_where_the_code_leg_is( SPEC_PATH, CODE_PATH, ACROSS = "legs/spec", "impl/code", "../../legs/spec" -@pytest.fixture(scope="module") -def adopted(tmp_path_factory) -> dict: - """One real adoption with both legs mounted away from their defaults, - and a recursive clone of its split branch. Its own source: `execute` - pushes the split branch into the repository it adopts.""" - base = tmp_path_factory.mktemp("contracts-dir-adopted") +def adopt_with_legs_mounted_away(base: Path, *, by_hand: bool) -> dict: + """One real adoption with both legs mounted away from their defaults, at + SPEC_PATH and CODE_PATH, and a recursive clone of its split branch. Its + own source: `execute` pushes the split branch into the repository it + adopts. + + `by_hand` plans with the DEFAULT paths and then edits `legs.spec_path` and + `legs.code_path` in the plan, as `MOUNT_REMEDIATION` and + `plan-leg-path-collides` ask a person to. Otherwise `plan` is given them. + """ source = make_source_repo(base / "Thing", TREE, edits=EDITS) plan = base / "adoption-plan.yaml" - follow_ups = planned_follow_ups(source, plan, SPEC_PATH, CODE_PATH) + follow_ups = planned_follow_ups( + source, plan, *(("spec", "code") if by_hand else (SPEC_PATH, CODE_PATH))) + if by_hand: + text = plan.read_text(encoding="utf-8") + edited = (text.replace(" spec_path: spec\n", + f" spec_path: {SPEC_PATH}\n") + .replace(" code_path: code\n", + f" code_path: {CODE_PATH}\n")) + assert (f" spec_path: {SPEC_PATH}\n" in edited + and f" code_path: {CODE_PATH}\n" in edited), text + with open(plan, "w", encoding="utf-8", newline="\n") as handle: + handle.write(edited) checked = run_script(ADOPT, "check", "--plan", str(plan)) assert checked.returncode == 0, checked.stderr + checked.stdout result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", @@ -228,6 +254,20 @@ def adopted(tmp_path_factory) -> dict: "makefile": makefile} +@pytest.fixture(scope="module") +def adopted(tmp_path_factory) -> dict: + """The adoption whose `plan` was given both mount paths.""" + return adopt_with_legs_mounted_away( + tmp_path_factory.mktemp("contracts-dir-adopted"), by_hand=False) + + +@pytest.fixture(scope="module") +def adopted_by_hand(tmp_path_factory) -> dict: + """The adoption whose plan's mount paths were edited after `plan`.""" + return adopt_with_legs_mounted_away( + tmp_path_factory.mktemp("contracts-dir-by-hand"), by_hand=True) + + @needs_filter_repo def test_the_adopted_makefile_exports_the_spec_legs_contracts(adopted): """The split's Makefile (the shape's, at the root: the source kept none) @@ -285,3 +325,27 @@ def test_the_follow_ups_read_across_leads_to_the_contracts(adopted): assert target.resolve().is_file() assert target.resolve() == (adopted["clone"].joinpath( *SPEC_PATH.split("/"), "contracts", POLICY)).resolve() + + +@needs_filter_repo +def test_a_hand_edited_mount_leaves_the_follow_up_true_of_the_makefile( + adopted_by_hand): + """#522's review, F1. `plan` wrote the Makefile follow-up once, and the + split commit's message, which is also the pull request's body, holds it. + `execute` renders the Makefile in that same commit from the plan's mount + paths as they are NOW. After the hand edit the tool asks for, the message + still said `$(CURDIR)/spec/contracts` while the Makefile said + `$(CURDIR)/legs/spec/contracts`: the follow-up names the key, and so + cannot disagree with the Makefile about its value.""" + message = " ".join(git( + "log", "-1", "--format=%B", SPLIT_BRANCH, + cwd=adopted_by_hand["source"]).stdout.split()) + assert MAKEFILE_FOLLOW_UP in message + assert "$(CURDIR)" not in message + assert contracts_dir_line(adopted_by_hand["makefile"].decode("utf-8")) == ( + f"CONTRACTS_DIR ?= $(CURDIR)/{SPEC_PATH}/contracts") + # The edited paths are where `execute` mounted the legs. + mounts = git("show", f"{SPLIT_BRANCH}:.gitmodules", + cwd=adopted_by_hand["source"]).stdout + assert f"path = {SPEC_PATH}" in mounts + assert f"path = {CODE_PATH}" in mounts From 1543fde1287f6f071d39b434431eb81b77d38e06 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:06:11 +0000 Subject: [PATCH 3/8] adopt-project.py: `spec_leg_from_code_leg`'s docstring says how many levels a case-only difference climbs, so it no longer says one more where `A/B/spec` beside `a/b/code` climbs two (#522) The docstring said two segments that differ only in case climb "one level more". The comparison stops at the first such segment, so the path climbs one more level than a disk that folds case needs for every segment from there to the end of what that disk would call shared: `A/B/spec` beside `a/b/code` climbs two more, and so does `A/b/spec`. The printed path still leads to the spec leg. Review of PR #572 at 9ddb7efe, N4. Wording only. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 7c2ff13c..5ee8eb40 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1709,9 +1709,11 @@ def spec_leg_from_code_leg(spec_path: str, code_path: str) -> str: follow-up and by `execute` before it mounts a leg: `/` between segments, and no segment empty, `.` or `..`. So the segments are compared as text, and nothing is resolved against a disk. Two segments - that differ only in case are two segments here, which climbs one level - more than a disk that folds case needs and still leads to the same - directory. + that differ only in case are two segments here, and the comparison stops + at the first of them: the path climbs one level more than a disk that + folds case needs for every segment from there to the end of what such a + disk would call shared (`A/B/spec` beside `a/b/code` climbs two more, + and so does `A/b/spec`), and still leads to the same directory. """ spec, code = spec_path.split("/"), code_path.split("/") shared = 0 From 44b6efb99a9a3bce163d9558c0efe2d532e39e1b Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:06:12 +0000 Subject: [PATCH 4/8] adopt-project.py: `adopt_makefile_block` sits with the other lower-case names of the `shape_materialize` import, so the list reads in the order it did before #522 (#522) The import kept `ADOPT_MAKEFILE_BLOCK` first among the upper-case constants; the new function took its place there, ahead of `TEMPLATED`. It belongs with the lower-case names, in order. Review of PR #572 at 9ddb7efe, N5. No behaviour change. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 5ee8eb40..940ac37c 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -171,9 +171,10 @@ tree_digest, ) from shape_materialize import ( # noqa: E402 - PLACEHOLDER_RE, SHAPE_REPOSITORY, SYMLINK_HOPS, adopt_makefile_block, + PLACEHOLDER_RE, SHAPE_REPOSITORY, SYMLINK_HOPS, TEMPLATED, TOO_MANY_SYMLINKS, CommandFailed, Materialized, - assembly_root_paths, collision_follow_up, copy_tree, default_reference, + adopt_makefile_block, assembly_root_paths, collision_follow_up, copy_tree, + default_reference, election_date, env_commit, escape_what_stdout_cannot_encode, git_init_commit, materialize_assembly_root, naming_block, render, run, template_files, write_lf, From 68cbca3d5d1efa2d1651c4ce090e563b16f2c441 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:06:25 +0000 Subject: [PATCH 5/8] tests/test_adopt_contracts_dir.py: `MOUNTS` holds a two-segment shared prefix and a prefix of the text that is not a prefix of a segment, so the follow-up's `..` count is checked where `legs` sits beside `legsX` too (#522) `a/b/spec` beside `a/b/code` shares two segments and climbs once; `legs/spec` beside `legsX/code` shares none, though one path's text begins with the other's first segment, and climbs twice. Each runs the spec-leg helper test and the `plan` test, which checks that the printed path leads to the spec leg. Both pass on the head as it was. Review of PR #572 at 9ddb7efe, N6. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_contracts_dir.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index f9c004b3..5f73717d 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -107,6 +107,9 @@ pytest.param("spec", "a/b/c/code", "../../../../spec", id="code-deep"), pytest.param("Legs/spec", "legs/code", "../../Legs/spec", id="case-differs"), + pytest.param("a/b/spec", "a/b/code", "../spec", id="two-segments-shared"), + pytest.param("legs/spec", "legsX/code", "../../legs/spec", + id="prefix-of-the-text-not-of-a-segment"), ] From 0d3d9aec2a36acff508132e774f8779bcdca08b0 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:06:46 +0000 Subject: [PATCH 6/8] tests/test_adopt_contracts_dir.py: the end-to-end fixture clones the split branch with `clone_just_pushed`, so the macOS `failed to copy file` race of #440 cannot reach it (#522) `execute` pushes the split branch into the source, and the fixture then cloned the source by its plain path: git's local clone walks `objects/` while the maintenance the push left running can repack it. #554 moved every such clone onto `conftest.clone_just_pushed` and no guard enforces it for a test added after, so this PR's own fixture is moved too. `FILE_PROTOCOL`, which only that clone used, leaves the import. Review of PR #572, N7. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_contracts_dir.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index 5f73717d..05dc2e26 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -38,7 +38,7 @@ import pytest -from conftest import (ADOPT, FILE_PROTOCOL, REPO, git, make_source_repo, +from conftest import (ADOPT, REPO, clone_just_pushed, git, make_source_repo, run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) @@ -244,11 +244,10 @@ def adopt_with_legs_mounted_away(base: Path, *, by_hand: bool) -> dict: "--work-dir", str(base / "work")) assert result.returncode == 0, result.stderr + result.stdout assert "adoption verified" in result.stdout, result.stdout + # `execute` has just pushed into `source`: read it through git, not + # through its files (#440, #554). clone = base / PROJECT - proc = subprocess.run( - ["git", *FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", - "-b", SPLIT_BRANCH, str(source), str(clone)], - capture_output=True, text=True, check=False) + proc = clone_just_pushed(source, clone, branch=SPLIT_BRANCH) assert proc.returncode == 0, proc.stderr makefile = subprocess.run( ["git", "show", f"{SPLIT_BRANCH}:Makefile"], cwd=str(source), From c1a88d618d1bbc9a057eef4a494f35c03fbb12b5 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:07:28 +0000 Subject: [PATCH 7/8] tests/test_adopt_contracts_dir.py: the hand-edit fixture asserts each edited mount path on a line of its own, so Sonar's S9073 composite-assert rule has nothing to fire on (#522) The fixture that edits `legs.spec_path` and `legs.code_path` checked both landed with one `and`. Two asserts say which of them did not, and the repository's gate has refused a composite assert before (#565, #570). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_contracts_dir.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index 05dc2e26..95f814f1 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -233,8 +233,8 @@ def adopt_with_legs_mounted_away(base: Path, *, by_hand: bool) -> dict: f" spec_path: {SPEC_PATH}\n") .replace(" code_path: code\n", f" code_path: {CODE_PATH}\n")) - assert (f" spec_path: {SPEC_PATH}\n" in edited - and f" code_path: {CODE_PATH}\n" in edited), text + assert f" spec_path: {SPEC_PATH}\n" in edited, text + assert f" code_path: {CODE_PATH}\n" in edited, text with open(plan, "w", encoding="utf-8", newline="\n") as handle: handle.write(edited) checked = run_script(ADOPT, "check", "--plan", str(plan)) From 22ce69b77bc92b4e838b4751bf87e898cb6c2a36 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 11:42:46 +0000 Subject: [PATCH 8/8] tests/test_adopt_contracts_dir.py: `MOUNTS` also holds a spec segment that extends the code leg's (`legsX/spec` beside `legs/code`), so the follow-up's segment compare is held from both sides (#522) A mutant that compared segments by `code[i].startswith(spec[i])` was killed by `legs/spec` beside `legsX/code`; the same compare the other way round, `spec[i].startswith(code[i])`, survived, because no row had the spec leg's segment extend the code leg's. `legsX/spec` beside `legs/code` shares no segment and climbs twice: `../../legsX/spec`. Review of PR #572 at 9ddb7efe, N6 (beyond the two rows it named). Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_contracts_dir.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index 95f814f1..7a363b1a 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -110,6 +110,8 @@ pytest.param("a/b/spec", "a/b/code", "../spec", id="two-segments-shared"), pytest.param("legs/spec", "legsX/code", "../../legs/spec", id="prefix-of-the-text-not-of-a-segment"), + pytest.param("legsX/spec", "legs/code", "../../legsX/spec", + id="a-segment-that-extends-the-other"), ]