From b9e80245f3a7f772356cd193a9e5815ea2f7efe8 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 12:37:16 +0000 Subject: [PATCH 1/2] shape_materialize.py and the pinned AGENTS-shape.md: where the root keeps a `GNUmakefile` or a `makefile`, which GNU make reads first, the Makefile collision follow-up no longer says the shape's make targets arrive with the merge, and the pinned file says `make` reads the first of the three and the block reaches it only once that file holds it (#1048) r1046 measured it in a clone that keeps all three names: after `cat shape/Makefile >> Makefile`, `make -n bootstrap` and `make contracts-dir` at the root still exit 2, "No rule to make target", for GNU make reads `GNUmakefile`, then `makefile`, then `Makefile`, and only that one. - `collision_follow_up` takes `reads_first`, the file GNU make reads before the shape's `Makefile`, and `_predict_collisions` reads it from the plan's root paths as `_makefile_follow_up` does. Where it is set, the follow-up names the file and says the targets reach `make` only once it holds them or includes a Makefile that does. Where the root keeps only the source's `Makefile`, the sentence is what it was, byte for byte. - `templates/assembly-root/AGENTS-shape.md` says `make contracts-dir` prints the block wherever `make` reads the shape's Makefile, names the three files in the order the code reads them, and says a kept `GNUmakefile` or `makefile`, beside a `Makefile` or instead of one, is the one `make` reads. The pin of every copy is rendered from these bytes at adopt and scaffold time, so no digest is stored in this repository; each project re-pins through `update-shape.py` as for any template change. - `tests/test_pinned_copy_text.py` holds the new text, and `tests/test_adopt_collision_make_reads.py` holds both sentences to `make -n` in a real adoption's clone, for each kept name. The `_next_steps` NOTE in adopt-project.py, the third box of #1048, is left to #1047. Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 12 +- scripts/shape_materialize.py | 72 ++++-- templates/assembly-root/AGENTS-shape.md | 15 +- tests/test_adopt_collision_make_reads.py | 301 +++++++++++++++++++++++ tests/test_pinned_copy_text.py | 16 +- 5 files changed, 390 insertions(+), 26 deletions(-) create mode 100644 tests/test_adopt_collision_make_reads.py diff --git a/adopt-project.py b/adopt-project.py index 169ad39f..45280dc3 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -6339,7 +6339,11 @@ def _predict_collisions(entries: list[Entry], *, Each follow-up is told the WHOLE list: the Makefile's names what runs until it is merged, and that depends on whether `scripts/bootstrap.py` - went aside too (#493's review, F2). + went aside too (#493's review, F2). It is told which file GNU make reads + before the shape's `Makefile` as well, where the plan keeps a + `GNUmakefile` or a `makefile`, read as `_makefile_follow_up` reads it + (`GNU_MAKE_READS`, `_root_keeps`): the merge brings the shape's make + targets only where `make` reads the merged file (#1048). `attributes_link` says the source's root `.gitattributes` is a symlink, read from the source tree's mode as `execute` reads it from the index: @@ -6349,8 +6353,12 @@ def _predict_collisions(entries: list[Entry], *, beside = {path: f"{COLLISION_DIR}/{path}" for path in sorted(set(assembly_root_paths())) if _root_keeps(surviving, path)} + reads_first = next((name for name in GNU_MAKE_READS + if name != SHAPE_MAKEFILE + and _root_keeps(surviving, name)), None) return [collision_follow_up(path, actual, beside, - attributes_link=attributes_link) + attributes_link=attributes_link, + reads_first=reads_first) for path, actual in beside.items()] diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 451a3d72..4056a6b8 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -736,9 +736,19 @@ def readme_trailing_shape_line(text: str) -> "re.Match | None": ASSISTANT_INSTRUCTIONS = ("AGENTS.md", "CLAUDE.md") #: The make targets of the shape's own `Makefile` that a person runs. A root #: that kept the source's `Makefile` has none of them until the shape's copy -#: is merged into it (#480), and the collision follow-up says so. +#: is merged into it (#480), and, where the root also keeps a `GNUmakefile` or +#: a `makefile`, which GNU make reads first, not even then (#1048). The +#: collision follow-up says both. #: `tests/test_adopt_e2e.py` holds this to the template's own targets. SHAPE_MAKE_TARGETS = ("bootstrap", "validate", "pins", "park", "resume") +#: The names GNU make looks for when no `-f` is given, in its order: it reads +#: the first it finds, and only that one. `collision_follow_up` names them +#: (#1048), and so does the pinned `AGENTS-shape.md`, in this order. +#: `adopt-project.py` keeps the same tuple under the same name for +#: `_makefile_follow_up` (#977) and `_predict_collisions`, which picks the +#: file `reads_first` names; +#: `tests/test_adopt_collision_make_reads.py` holds the two tuples equal. +GNU_MAKE_READS = ("GNUmakefile", "makefile", "Makefile") # --------------------------------------------------------------------------- @@ -958,7 +968,8 @@ def _tracked_at(target: Path, rel: str) -> str | None: def collision_follow_up(intended: str, actual: str, beside: dict | None = None, *, - attributes_link: bool = False) -> str: + attributes_link: bool = False, + reads_first: str | None = None) -> str: """The follow-up for ONE collision, in one place. Both callers say it: `adopt-project.py plan` PREDICTS the collisions from @@ -973,15 +984,30 @@ def collision_follow_up(intended: str, actual: str, its place needs (#471's review, SHOULD-2). `plan` knows it from the source tree's mode. - THE MAKEFILE'S MERGE IS WHAT BRINGS THE SHAPE'S MAKE TARGETS (#480). Until - it is made, `make bootstrap` in the root exits 2 with `No rule to make - target 'bootstrap'`, so the follow-up says that, and what runs meanwhile: - `python3 scripts/bootstrap.py`, unless that script went aside as well. - `beside` maps each shape file written beside one the source kept to where - it went, the whole list, and only the Makefile's follow-up reads it. - Where the root's `scripts/bootstrap.py` is the source's own, that command - runs the source's script and exits 0, so the follow-up names no command, - and says both merges come first (#493's review, F2). + THE MAKEFILE'S MERGE IS WHAT BRINGS THE SHAPE'S MAKE TARGETS (#480), WHERE + `make` READS THE MERGED FILE. Until it is made, `make bootstrap` in the + root exits 2 with `No rule to make target 'bootstrap'`, so the follow-up + says that, and what runs meanwhile: `python3 scripts/bootstrap.py`, unless + that script went aside as well. `beside` maps each shape file written + beside one the source kept to where it went, the whole list, and only the + Makefile's follow-up reads it. Where the root's `scripts/bootstrap.py` is + the source's own, that command runs the source's script and exits 0, so + the follow-up names no command, and says both merges come first (#493's + review, F2). + + BUT GNU MAKE READS THE FIRST OF `GNUmakefile`, `makefile` AND `Makefile` + IT FINDS, AND ONLY THAT ONE (#1048, measured on #1046's review). Where + the root keeps a `GNUmakefile` or a `makefile` beside the source's own + `Makefile`, the merge makes no target arrive: after `cat shape/Makefile + >> Makefile`, `make -n bootstrap` at the root still exits 2 with the same + `No rule`, for `make` reads the other file. `reads_first` names that + file, the one `adopt-project.py`'s `_makefile_follow_up` names (#977), as + its caller reads it from the plan's root paths; the follow-up then says + `make` reads it, that the targets reach `make` there only once it holds + them or includes a Makefile that does, and, as before, what runs + meanwhile. With none, the Makefile's follow-up is what it was, byte for + byte: a root whose only Makefile is the source's is the one where the + merge is all it takes. It is read by no other collision. AND A PINNED COPY'S FOLLOW-UP SAYS WHAT HAPPENS TO ITS ROW (#502). The shape pin has a row for a copied file at the path it landed on, so once @@ -1033,12 +1059,26 @@ def collision_follow_up(intended: str, actual: str, return merge targets = ", ".join(f"`{target}`" for target in SHAPE_MAKE_TARGETS) script = (beside or {}).get(BOOTSTRAP) + if reads_first is None: + arrive = (f"The shape's make targets ({targets}) arrive only with " + "that merge") + joint = ", and" + meanwhile = ("`python3 scripts/bootstrap.py` is what `make " + "bootstrap` runs") + else: + listed = _listed([f"`{name}`" for name in GNU_MAKE_READS]) + arrive = ( + f"GNU make at the root reads the source's `{reads_first}`, the " + f"first of {listed} it finds, so the shape's make targets " + f"({targets}) reach `make` there only once `{reads_first}` holds " + "them, or includes a Makefile that does, and not with that merge " + "alone") + joint = "; and" + meanwhile = ("`python3 scripts/bootstrap.py` is what the shape's " + "`bootstrap` target runs") if script is None: - return (f"{merge}. The shape's make targets ({targets}) arrive only " - "with that merge; until then `python3 scripts/bootstrap.py` " - "is what `make bootstrap` runs") - return (f"{merge}. The shape's make targets ({targets}) arrive only with " - f"that merge, and the root's {BOOTSTRAP} is the source's " + return f"{merge}. {arrive}; until then {meanwhile}" + return (f"{merge}. {arrive}{joint} the root's {BOOTSTRAP} is the source's " f"own until {script} is merged into it as well, so neither `make " f"bootstrap` nor `python3 {BOOTSTRAP}` runs the shape's " "bootstrap before both merges are made") diff --git a/templates/assembly-root/AGENTS-shape.md b/templates/assembly-root/AGENTS-shape.md index f410515d..941835a3 100644 --- a/templates/assembly-root/AGENTS-shape.md +++ b/templates/assembly-root/AGENTS-shape.md @@ -79,11 +79,16 @@ ruled and measured on the MedxEHR conversion: `contracts/` moved to the spec leg, and the code leg's tooling now takes `CONTRACTS_DIR` from the environment rather than each script guessing at `../`. Where this root was ADOPTED, the shape's Makefile exports `CONTRACTS_DIR` once, as the `contracts/` of the spec -leg at the path `project.yaml` mounts it, and `make contracts-dir` prints it. -Where the source kept a Makefile of its own, that block waits in -`shape/Makefile` until the two are merged. A SCAFFOLDED root's Makefile exports -none: its legs start from the scaffold's seed, with no `contracts/`, so -nothing reads across them yet. +leg at the path `project.yaml` mounts it, and `make contracts-dir` prints it +wherever `make` reads that Makefile. GNU make reads the first of `GNUmakefile`, +`makefile` and `Makefile` it finds, and only that one. Where the source kept a +Makefile of its own, that block waits in `shape/Makefile` until the two are +merged. Where the source kept a `GNUmakefile` or a `makefile`, beside a +`Makefile` or instead of one, `make` at the root reads the first of them and +never the shape's Makefile, so `make contracts-dir` has no rule there until +that file holds the block, or includes a Makefile that does. A SCAFFOLDED +root's Makefile exports none: its legs start from the scaffold's seed, with no +`contracts/`, so nothing reads across them yet. ## The five commands diff --git a/tests/test_adopt_collision_make_reads.py b/tests/test_adopt_collision_make_reads.py new file mode 100644 index 00000000..e9f35f70 --- /dev/null +++ b/tests/test_adopt_collision_make_reads.py @@ -0,0 +1,301 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The Makefile's collision follow-up and the pinned `AGENTS-shape.md` name +the file GNU make reads first (#1048). + +#977 and #1046 corrected the `CONTRACTS_DIR` line and README: GNU make reads +the first of `GNUmakefile`, `makefile` and `Makefile` it finds, and only that +one, so a source that keeps `makefile` or `GNUmakefile` has a root where +`make` never reads the shape's `Makefile`. Two sentences still made the claim +#977 corrects (r1046's NIT-B): + + * the collision follow-up for a kept `Makefile` said the shape's make + targets "arrive only with that merge". Where the root also keeps a + `GNUmakefile` or a `makefile`, `cat shape/Makefile >> Makefile` leaves + `make -n bootstrap` at the root exiting 2, "No rule to make target + 'bootstrap'", for `make` reads the other file; + * the pinned `AGENTS-shape.md` said, of a kept `Makefile`, that the block + "waits in `shape/Makefile` until the two are merged", and of a kept + `makefile` or `GNUmakefile` nothing. + +Both name the file `make` reads first now. The collision follow-up is told it +by `_predict_collisions` (`reads_first`), from the plan's root paths as the +`CONTRACTS_DIR` line reads them; the pinned file is one text for every +project, so it names the three names, in the order the code reads them. Where +the root keeps only the source's `Makefile` the collision follow-up is what it +was, byte for byte (`tests/test_adopt_e2e.py` pins it). + +The tests that run `execute` need `git filter-repo` and skip without it, and +the ones that run `make` skip without it. 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 shutil +import sys +from pathlib import Path + +import pytest + +from conftest import ADOPT, REPO, git, load_without_bytecode +from test_adopt_contracts_dir import (SPEC_PATH, SPLIT_BRANCH, + adopt_with_legs_mounted_away, + needs_filter_repo) +from test_adopt_make_reads_first import (disk_keeps_case, make, needs_make, + tree_keeping) +from test_adopt_printed_sentences import NAMES, entries_of, makefile_lines +from test_pinned_copy_text import ADOPTED_SENTENCES, TEMPLATE_AGENTS, flat + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from shape_materialize import GNU_MAKE_READS, SHAPE_MAKE_TARGETS # noqa: E402 + + +@pytest.fixture(scope="module") +def adopter(): + """`adopt-project.py` loaded as a module, for its helpers.""" + return load_without_bytecode(ADOPT, "adopt_collision_make_reads") + + +#: The shape's make targets, as the collision follow-up names them. +TARGETS = ", ".join(f"`{name}`" for name in SHAPE_MAKE_TARGETS) +#: How the collision follow-up for the Makefile begins. +MERGE_MAKEFILE = "merge shape/Makefile into Makefile: " +LISTED = (", ".join(f"`{name}`" for name in GNU_MAKE_READS[:-1]) + + f" and `{GNU_MAKE_READS[-1]}`") +#: The last sentence of that follow-up where the root keeps no file GNU make +#: reads before `Makefile`: the merge is all it takes (#480). +MERGE_IS_ENOUGH = ( + f"The shape's make targets ({TARGETS}) arrive only with that merge; " + "until then `python3 scripts/bootstrap.py` is what `make bootstrap` runs") +MEANWHILE = ("; until then `python3 scripts/bootstrap.py` is what the " + "shape's `bootstrap` target runs") +SCRIPT_ASIDE = ( + "; and the root's scripts/bootstrap.py is the source's own until " + "shape/scripts/bootstrap.py is merged into it as well, so neither `make " + "bootstrap` nor `python3 scripts/bootstrap.py` runs the shape's " + "bootstrap before both merges are made") + + +def reads_instead(first: str, then: str) -> str: + """The last sentence of that follow-up where the root keeps `first`, + which GNU make reads before `Makefile`; `then` is how it ends.""" + return (f"GNU make at the root reads the source's `{first}`, the first of " + f"{LISTED} it finds, so the shape's make targets ({TARGETS}) " + f"reach `make` there only once `{first}` holds them, or includes " + f"a Makefile that does, and not with that merge alone{then}") + + +#: `(the root paths the plan keeps, the Makefile collision follow-up's last +#: sentence, the file GNU make reads first: None where it is `Makefile`)`. +COLLISIONS = [ + pytest.param(("Makefile",), MERGE_IS_ENOUGH, None, id="Makefile"), + pytest.param(("Makefile", "makefile"), + reads_instead("makefile", MEANWHILE), "makefile", + id="Makefile+makefile"), + pytest.param(("Makefile", "GNUmakefile"), + reads_instead("GNUmakefile", MEANWHILE), "GNUmakefile", + id="Makefile+GNUmakefile"), + pytest.param(("Makefile", "makefile", "GNUmakefile"), + reads_instead("GNUmakefile", MEANWHILE), "GNUmakefile", + id="all-three"), + # `scripts/` kept too: the source's `scripts/bootstrap.py` is the root's, + # and neither command runs the shape's bootstrap before both merges. + pytest.param( + ("Makefile", "scripts/"), + f"The shape's make targets ({TARGETS}) arrive only with that merge" + + SCRIPT_ASIDE.replace("; and", ", and", 1), None, + id="Makefile+scripts"), + pytest.param(("Makefile", "GNUmakefile", "scripts/"), + reads_instead("GNUmakefile", SCRIPT_ASIDE), "GNUmakefile", + id="Makefile+GNUmakefile+scripts"), +] + + +def collisions_of(adopter, *kept: str) -> list: + """`_predict_collisions` over root entries for each path in `kept`.""" + return adopter._predict_collisions( + entries_of(*[(path, "root") for path in kept])) + + +@pytest.mark.parametrize("kept, tail, first", COLLISIONS) +def test_the_makefile_collision_names_the_file_gnu_make_reads_first( + adopter, kept, tail, first): + """THE MEASURED CLAIM (#1048, r1046's NIT-B). "The shape's make targets + arrive only with that merge" holds where the merged `Makefile` is the + file `make` reads. Where the root keeps `GNUmakefile` or `makefile` + beside it, `make` reads that one, so the follow-up names it, and does not + say the merge brings the targets. Where the root keeps only the source's + `Makefile`, the sentence is what it was, byte for byte.""" + (line,) = [item for item in collisions_of(adopter, *kept) + if item.startswith(MERGE_MAKEFILE)] + assert line.endswith(". " + tail), line + assert ("arrive only with that merge" in line) is (first is None) + assert (f"reads the source's `{first}`, the first of {LISTED} it finds" + in line) is (first is not None) + + +@pytest.mark.parametrize("kept, tail, first", COLLISIONS) +def test_the_two_makefile_lines_of_one_plan_name_the_same_file( + adopter, monkeypatch, kept, tail, first): + """The follow-up that says which Makefile wires `CONTRACTS_DIR` (#977) + and the collision follow-up beside it, in the same plan, name the same + file `make` reads, or one says `make` reads `GNUmakefile` and the next + that the merge brings it the targets.""" + monkeypatch.setattr(adopter, "cross_leg_references", + lambda source, entries: []) + entries = entries_of(("contracts/", "spec"), ("src/", "code"), + *[(path, "root") for path in kept]) + ups = adopter.follow_ups_for(None, entries, NAMES, + adopter._predict_collisions(entries)) + (wires,) = makefile_lines(ups) + (collision,) = [item for item in ups if item.startswith(MERGE_MAKEFILE)] + clause = f"GNU make at the root reads the source's `{first}`, the first of" + assert (clause in wires) is (first is not None) + assert (clause in collision) is (first is not None) + + +@pytest.mark.parametrize("kept", [ + pytest.param(("makefile",), id="makefile"), + pytest.param(("GNUmakefile",), id="GNUmakefile"), + pytest.param(("makefile", "GNUmakefile"), id="makefile+GNUmakefile"), + pytest.param((), id="none"), + pytest.param(("MAKEFILE",), id="MAKEFILE"), +]) +def test_no_makefile_collision_where_the_source_keeps_no_makefile( + adopter, kept): + """These names do not collide with the shape's `Makefile` where case is + kept: it is written as the root's, and the follow-up that says `make` + reads another file is the `CONTRACTS_DIR` line's (#977).""" + assert [item for item in collisions_of(adopter, *kept) + if item.startswith(MERGE_MAKEFILE)] == [] + + +def test_a_name_in_a_leg_is_not_the_file_make_reads_at_the_root(adopter): + """The names come from the plan's ROOT paths: a `GNUmakefile` the plan + sends to a leg is in no root, so the merge is all it takes.""" + entries = entries_of(("Makefile", "root"), ("GNUmakefile", "spec"), + ("makefile", "code")) + (line,) = [item for item in adopter._predict_collisions(entries) + if item.startswith(MERGE_MAKEFILE)] + assert line.endswith(". " + MERGE_IS_ENOUGH) + + +def test_the_two_lists_of_the_names_gnu_make_reads_are_one(adopter): + """The materializer's `collision_follow_up` and `adopt-project.py`'s + lookup each hold the tuple, and it is GNU make's own order: two lists + that differ is how one sentence names `makefile` first and the next + `GNUmakefile`.""" + assert adopter.GNU_MAKE_READS == shape_materialize.GNU_MAKE_READS + assert GNU_MAKE_READS == ("GNUmakefile", "makefile", "Makefile") + + +def test_the_pinned_agent_file_names_every_file_the_code_reads_first(): + """#1048's second sentence. "Where the source kept a Makefile of its own, + that block waits in `shape/Makefile` until the two are merged" said a + kept `Makefile` is what holds the block back, and left out the + `GNUmakefile` and the `makefile` GNU make reads first. The pinned file + names the three, in the order the code reads them, and every one the code + checks before the shape's `Makefile`: a name added to the tuple fails + here until the file names it. It stays a verbatim copy.""" + said = flat(TEMPLATE_AGENTS) + assert ADOPTED_SENTENCES in said + assert (f"GNU make reads the first of {LISTED} it finds, and only that " + "one.") in said + before = [name for name in GNU_MAKE_READS if name != "Makefile"] + kept = " or ".join(f"a `{name}`" for name in before) + assert f"Where the source kept {kept}, " in said + assert "$(CURDIR)" not in said and "{{" not in said + + +# --- the sentences, held to what `make` does in a real adoption's clone ------ + +#: `(the files the source keeps, the file GNU make reads first at the root of +#: a clone on a disk that keeps case: None where it is `Makefile`)`. +CLONES = [ + pytest.param(("Makefile",), None, id="Makefile"), + pytest.param(("makefile",), "makefile", id="makefile"), + pytest.param(("GNUmakefile",), "GNUmakefile", id="GNUmakefile"), + pytest.param(("makefile", "GNUmakefile"), "GNUmakefile", + id="makefile+GNUmakefile"), + pytest.param(("Makefile", "makefile"), "makefile", + id="Makefile+makefile"), + pytest.param(("Makefile", "GNUmakefile"), "GNUmakefile", + id="Makefile+GNUmakefile"), + pytest.param(("Makefile", "makefile", "GNUmakefile"), "GNUmakefile", + id="all-three"), +] + + +def appended(path: Path, text: str) -> None: + """`text` at the end of `path`, as `cat >>` leaves it.""" + with open(path, "a", encoding="utf-8", newline="\n") as handle: + handle.write(text) + + +def copied(clone: Path, name: str) -> Path: + """A copy of the whole clone, so each experiment starts from it.""" + shutil.copytree(clone, clone.parent / name, symlinks=True) + return clone.parent / name + + +@needs_filter_repo +@needs_make +@pytest.mark.parametrize("kept, first", CLONES) +def test_the_sentences_are_true_of_what_make_does_in_the_clone( + tmp_path, kept, first): + """r1046's measurement, taken for each kept name (#1048). After a real + `execute`: the split message says the shape's make targets arrive with + the merge exactly where `make -n bootstrap` exits 0 after `cat + shape/Makefile >> Makefile`, and names the file `make` reads where it + does not. The pinned `AGENTS-shape.md` is in the clone, and what it says + of a `GNUmakefile` or a `makefile` is what `make` does: `make + contracts-dir` has no rule at the root until that file holds the block, + or includes a Makefile that does, and then it prints the spec leg's + `contracts/`.""" + if not disk_keeps_case(tmp_path): + pytest.skip("this disk ignores case: it cannot hold the rows apart") + adopted = adopt_with_legs_mounted_away(tmp_path, by_hand=False, + tree=tree_keeping(kept)) + clone = adopted["clone"] + message = " ".join(git("log", "-1", "--format=%B", SPLIT_BRANCH, + cwd=adopted["source"]).stdout.split()) + assert ADOPTED_SENTENCES in flat(clone / "AGENTS-shape.md") + shapes = "shape/Makefile" if "Makefile" in kept else "Makefile" + # Nothing at the root has the targets before an edit. + assert make(clone, "-n", "bootstrap").returncode != 0 + assert make(clone, "-n", "contracts-dir").returncode != 0 + if "Makefile" in kept: + # THE MERGE THE FOLLOW-UP ASKS FOR. + appended(clone / "Makefile", (clone / shapes).read_text( + encoding="utf-8")) + brought = make(clone, "-n", "bootstrap") + assert (brought.returncode == 0) is (first is None), ( + brought.stdout + brought.stderr) + arrive = (f"The shape's make targets ({TARGETS}) arrive only with " + "that merge") + assert (arrive in message) is (first is None) + # Named once by the `CONTRACTS_DIR` line and once by the collision's. + assert message.count( + f"GNU make at the root reads the source's `{first}`, the " + "first of") == (0 if first is None else 2) + # `Makefile` now has the targets: `make -f` reads it. + named = make(clone, "-f", "Makefile", "-n", "bootstrap") + assert named.returncode == 0, named.stdout + named.stderr + else: + assert "make targets" not in message + if first is not None: + # THE FILE `make` READS: it holds the block, or includes a Makefile + # that does. + merged = copied(clone, "holds") + appended(merged / first, (merged / shapes).read_text( + encoding="utf-8")) + including = copied(clone, "includes") + appended(including / first, "\ninclude Makefile\n") + for fixed in (merged, including): + assert make(fixed, "-n", "bootstrap").returncode == 0 + printed = make(fixed, "contracts-dir") + assert printed.returncode == 0, printed.stderr + assert Path(printed.stdout.strip()) == ( + fixed.resolve() / SPEC_PATH / "contracts") diff --git a/tests/test_pinned_copy_text.py b/tests/test_pinned_copy_text.py index 0c88ff7a..e38cde47 100644 --- a/tests/test_pinned_copy_text.py +++ b/tests/test_pinned_copy_text.py @@ -70,12 +70,22 @@ #: unless the source kept one of its own (rv610's F1 on #634: then the block #: waits in `shape/Makefile`, and `make contracts-dir` has no rule at the #: root), and of a SCAFFOLDED one, whose legs are the scaffold's seed (F5). +#: And of the file `make` reads: GNU make reads the first of `GNUmakefile`, +#: `makefile` and `Makefile` it finds, so where the source kept either of the +#: first two the shape's Makefile is never the one read, and merging into +#: `Makefile` brings nothing (#1048, measured on #1046's review). ADOPTED_SENTENCES = ( "Where this root was ADOPTED, the shape's Makefile exports " "`CONTRACTS_DIR` once, as the `contracts/` of the spec leg at the path " - "`project.yaml` mounts it, and `make contracts-dir` prints it. Where the " - "source kept a Makefile of its own, that block waits in `shape/Makefile` " - "until the two are merged.") + "`project.yaml` mounts it, and `make contracts-dir` prints it wherever " + "`make` reads that Makefile. GNU make reads the first of `GNUmakefile`, " + "`makefile` and `Makefile` it finds, and only that one. Where the source " + "kept a Makefile of its own, that block waits in `shape/Makefile` until " + "the two are merged. Where the source kept a `GNUmakefile` or a " + "`makefile`, beside a `Makefile` or instead of one, `make` at the root " + "reads the first of them and never the shape's Makefile, so `make " + "contracts-dir` has no rule there until that file holds the block, or " + "includes a Makefile that does.") SCAFFOLDED_SENTENCE = ( "A SCAFFOLDED root's Makefile exports none: its legs start from the " "scaffold's seed, with no `contracts/`, so nothing reads across them " From b28f40de09cfd5bc285b09592d735460003dce8d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 16:17:11 +0000 Subject: [PATCH 2/2] shape_materialize.py and the pinned AGENTS-shape.md: where the file GNU make reads first ends in `include Makefile`, the Makefile collision follow-up no longer says the merge alone does not bring the shape's make targets, its script-aside tail names only what the script's merge lets run, and the pinned file says `make` reads the shape's Makefile there only where that file includes it (#1048) #1105's review (RV1105) measured it in a real adoption: where the kept `GNUmakefile` or `makefile` ends in `include Makefile`, `make -n bootstrap` at the root exits 2 before the merge and 0 after `cat shape/Makefile >> Makefile` alone, so "and not with that merge alone" was untrue there. - `collision_follow_up`, where `reads_first` is set: says the targets reach `make` only once that file "has them, or includes a Makefile that does", and no more (SHOULD-1). It uses the words of `execute`'s NEXT (#1047): "has them", and "what the shape's `make bootstrap` runs" (NIT-1). Where `scripts/bootstrap.py` went aside too, it says neither command runs the shape's bootstrap until the script is merged, and then `python3 scripts/bootstrap.py` runs it, and no longer reads as if `make bootstrap` runs it after both merges (NIT-2). Where the root keeps only the source's `Makefile`, the sentence is what it was, byte for byte. - Its docstring, the `SHAPE_MAKE_TARGETS` comment and the `ADOPTED_SENTENCES` comment say the merge alone brings the targets only where that file includes `Makefile`. The docstring's opening says what is so: `plan` predicts the follow-ups, the split commit carries the plan's, `execute` names its collisions by path only, and `Materialized.follow_ups()` has no caller (NIT-4). - `templates/assembly-root/AGENTS-shape.md`: `make` at the root reads the first of the kept files, "and not the shape's Makefile unless that file includes it" (NIT-3). `ADOPTED_SENTENCES` holds the new text. No digest of the file is stored in this repository. - `tests/test_adopt_collision_make_reads.py`: the rows hold the new text, two rows hold the script-aside tail where `make` reads another file, and two rows in a real adoption's clone, for a `GNUmakefile` and a `makefile` that end in `include Makefile`, hold that the merge alone brings the shape's recipe and that the split message says nothing untrue of it. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/shape_materialize.py | 55 ++++++----- templates/assembly-root/AGENTS-shape.md | 7 +- tests/test_adopt_collision_make_reads.py | 115 +++++++++++++++++++++-- tests/test_pinned_copy_text.py | 11 ++- 4 files changed, 149 insertions(+), 39 deletions(-) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 8e93aabe..72252023 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -768,9 +768,10 @@ def readme_trailing_shape_line(text: str) -> "re.Match | None": ASSISTANT_INSTRUCTIONS = ("AGENTS.md", "CLAUDE.md") #: The make targets of the shape's own `Makefile` that a person runs. A root #: that kept the source's `Makefile` has none of them until the shape's copy -#: is merged into it (#480), and, where the root also keeps a `GNUmakefile` or -#: a `makefile`, which GNU make reads first, not even then (#1048). The -#: collision follow-up says both. +#: is merged into it (#480); where the root also keeps a `GNUmakefile` or a +#: `makefile`, which GNU make reads first, the merge alone brings them only +#: where that file includes `Makefile` (#1048, #1105's review). The collision +#: follow-up says what each root takes. #: `tests/test_adopt_e2e.py` holds this to the template's own targets. SHAPE_MAKE_TARGETS = ("bootstrap", "validate", "pins", "park", "resume") #: The names GNU make looks for when no `-f` is given, in its order: it reads @@ -1004,10 +1005,14 @@ def collision_follow_up(intended: str, actual: str, reads_first: str | None = None) -> str: """The follow-up for ONE collision, in one place. - Both callers say it: `adopt-project.py plan` PREDICTS the collisions from - the plan's surviving root paths, and `execute` reports the ones that - actually happened. Two spellings of the same instruction is how the plan a - human approves stops matching the commit message they read afterwards. + `adopt-project.py plan` says it: `_predict_collisions` PREDICTS the + collisions from the plan's surviving root paths, the plan stores each + follow-up in `follow_ups:`, and `execute` carries those items into the + split commit's message, so the plan a human approves and the commit + message they read afterwards hold one spelling of each. `execute` names + the collisions that actually happened by path only: its `beside` lines, + and the split message's "WRITTEN BESIDE" list. `Materialized.follow_ups()` + would say this of them, and nothing calls it (#1105's review, NIT-4). A `.gitattributes` THAT IS A SYMLINK gets no block (`_append_eol_block`), so `attributes_link` says which of two things happened, and the follow-up @@ -1030,16 +1035,23 @@ def collision_follow_up(intended: str, actual: str, BUT GNU MAKE READS THE FIRST OF `GNUmakefile`, `makefile` AND `Makefile` IT FINDS, AND ONLY THAT ONE (#1048, measured on #1046's review). Where the root keeps a `GNUmakefile` or a `makefile` beside the source's own - `Makefile`, the merge makes no target arrive: after `cat shape/Makefile - >> Makefile`, `make -n bootstrap` at the root still exits 2 with the same - `No rule`, for `make` reads the other file. `reads_first` names that - file, the one `adopt-project.py`'s `_makefile_follow_up` names (#977), as - its caller reads it from the plan's root paths; the follow-up then says - `make` reads it, that the targets reach `make` there only once it holds + `Makefile`, `make` reads that file, so the merge alone brings the targets + only where that file includes `Makefile`: after `cat shape/Makefile >> + Makefile`, `make -n bootstrap` at the root still exits 2 with the same + `No rule` where it does not, and exits 0 where it ends in `include + Makefile` (#1105's review, SHOULD-1). `reads_first` names that file, the + one `adopt-project.py`'s `_makefile_follow_up` names (#977), as its + caller reads it from the plan's root paths; the follow-up then says + `make` reads it, that the targets reach `make` there only once it has them or includes a Makefile that does, and, as before, what runs - meanwhile. With none, the Makefile's follow-up is what it was, byte for - byte: a root whose only Makefile is the source's is the one where the - merge is all it takes. It is read by no other collision. + meanwhile, in the words `execute`'s NEXT uses (#1047). It says nothing of + the merge alone, which turns on that file's own lines. Where + `scripts/bootstrap.py` went aside as well, it says `python3 + scripts/bootstrap.py` runs the shape's bootstrap once that script is + merged, and not that `make bootstrap` does. With none, the Makefile's + follow-up is what it was, byte for byte: a root whose only Makefile is + the source's is one where `make` reads the merged file. It is read by no + other collision. AND A PINNED COPY'S FOLLOW-UP SAYS WHAT HAPPENS TO ITS ROW (#502). The shape pin has a row for a copied file at the path it landed on, so once @@ -1097,23 +1109,24 @@ def collision_follow_up(intended: str, actual: str, joint = ", and" meanwhile = ("`python3 scripts/bootstrap.py` is what `make " "bootstrap` runs") + until = "before both merges are made" else: listed = _listed([f"`{name}`" for name in GNU_MAKE_READS]) arrive = ( f"GNU make at the root reads the source's `{reads_first}`, the " f"first of {listed} it finds, so the shape's make targets " - f"({targets}) reach `make` there only once `{reads_first}` holds " - "them, or includes a Makefile that does, and not with that merge " - "alone") + f"({targets}) reach `make` there only once `{reads_first}` has " + "them, or includes a Makefile that does") joint = "; and" meanwhile = ("`python3 scripts/bootstrap.py` is what the shape's " - "`bootstrap` target runs") + "`make bootstrap` runs") + until = f"until then, and then `python3 {BOOTSTRAP}` runs it" if script is None: return f"{merge}. {arrive}; until then {meanwhile}" return (f"{merge}. {arrive}{joint} the root's {BOOTSTRAP} is the source's " f"own until {script} is merged into it as well, so neither `make " f"bootstrap` nor `python3 {BOOTSTRAP}` runs the shape's " - "bootstrap before both merges are made") + f"bootstrap {until}") def _attributes_link_follow_up(intended: str, actual: str, diff --git a/templates/assembly-root/AGENTS-shape.md b/templates/assembly-root/AGENTS-shape.md index 941835a3..6f926e77 100644 --- a/templates/assembly-root/AGENTS-shape.md +++ b/templates/assembly-root/AGENTS-shape.md @@ -84,9 +84,10 @@ wherever `make` reads that Makefile. GNU make reads the first of `GNUmakefile`, `makefile` and `Makefile` it finds, and only that one. Where the source kept a Makefile of its own, that block waits in `shape/Makefile` until the two are merged. Where the source kept a `GNUmakefile` or a `makefile`, beside a -`Makefile` or instead of one, `make` at the root reads the first of them and -never the shape's Makefile, so `make contracts-dir` has no rule there until -that file holds the block, or includes a Makefile that does. A SCAFFOLDED +`Makefile` or instead of one, `make` at the root reads the first of them, and +not the shape's Makefile unless that file includes it, so `make contracts-dir` +has no rule there until that file holds the block, or includes a Makefile that +does. A SCAFFOLDED root's Makefile exports none: its legs start from the scaffold's seed, with no `contracts/`, so nothing reads across them yet. diff --git a/tests/test_adopt_collision_make_reads.py b/tests/test_adopt_collision_make_reads.py index e9f35f70..cc4f13f1 100644 --- a/tests/test_adopt_collision_make_reads.py +++ b/tests/test_adopt_collision_make_reads.py @@ -24,6 +24,17 @@ the root keeps only the source's `Makefile` the collision follow-up is what it was, byte for byte (`tests/test_adopt_e2e.py` pins it). +And neither says more than `make` does (#1105's review). Where the file +`make` reads ends in `include Makefile`, a common idiom, the merge alone +brings the targets, so the follow-up says they reach `make` once that file has +them or includes a Makefile that does, and not that the merge alone fails; +where `scripts/bootstrap.py` went aside too, it says what the script's merge +lets `python3 scripts/bootstrap.py` run, and not that `make bootstrap` runs it +after both merges (NIT-2); and the pinned file says `make` reads the shape's +Makefile there only where that file includes it (NIT-3). The words are those +of `execute`'s NEXT (#1047): "has them", and "what the shape's `make +bootstrap` runs". + The tests that run `execute` need `git filter-repo` and skip without it, and the ones that run `make` skip without it. No network and no GitHub: the legs are bare repositories on disk, and `tests/conftest.py` keeps a `gh` guard @@ -70,12 +81,24 @@ def adopter(): f"The shape's make targets ({TARGETS}) arrive only with that merge; " "until then `python3 scripts/bootstrap.py` is what `make bootstrap` runs") MEANWHILE = ("; until then `python3 scripts/bootstrap.py` is what the " - "shape's `bootstrap` target runs") + "shape's `make bootstrap` runs") +#: How the follow-up goes on where `scripts/bootstrap.py` went aside too. SCRIPT_ASIDE = ( - "; and the root's scripts/bootstrap.py is the source's own until " + " the root's scripts/bootstrap.py is the source's own until " "shape/scripts/bootstrap.py is merged into it as well, so neither `make " "bootstrap` nor `python3 scripts/bootstrap.py` runs the shape's " - "bootstrap before both merges are made") + "bootstrap ") +#: ... where `make` reads the merged `Makefile`: `make bootstrap` waits for +#: the two merges and nothing else (#493's review, F2). +BOTH_MERGES = ", and" + SCRIPT_ASIDE + "before both merges are made" +#: ... where `make` reads another file: the script's merge is what lets +#: `python3 scripts/bootstrap.py` run the shape's bootstrap, and `make +#: bootstrap` runs it only once that file has the targets or includes a +#: Makefile that does, with or without the `Makefile` merge (#1105's review, +#: NIT-2), which the sentence before says. +SCRIPT_MERGED = ("; and" + SCRIPT_ASIDE + + "until then, and then `python3 scripts/bootstrap.py` runs " + "it") def reads_instead(first: str, then: str) -> str: @@ -83,8 +106,13 @@ def reads_instead(first: str, then: str) -> str: which GNU make reads before `Makefile`; `then` is how it ends.""" return (f"GNU make at the root reads the source's `{first}`, the first of " f"{LISTED} it finds, so the shape's make targets ({TARGETS}) " - f"reach `make` there only once `{first}` holds them, or includes " - f"a Makefile that does, and not with that merge alone{then}") + f"reach `make` there only once `{first}` has them, or includes " + f"a Makefile that does{then}") + + +#: The claim #1105's review measured untrue (SHOULD-1): where the file `make` +#: reads ends in `include Makefile`, the merge alone brings the targets. +MERGE_ALONE_FAILS = "not with that merge alone" #: `(the root paths the plan keeps, the Makefile collision follow-up's last @@ -105,10 +133,10 @@ def reads_instead(first: str, then: str) -> str: pytest.param( ("Makefile", "scripts/"), f"The shape's make targets ({TARGETS}) arrive only with that merge" - + SCRIPT_ASIDE.replace("; and", ", and", 1), None, + + BOTH_MERGES, None, id="Makefile+scripts"), pytest.param(("Makefile", "GNUmakefile", "scripts/"), - reads_instead("GNUmakefile", SCRIPT_ASIDE), "GNUmakefile", + reads_instead("GNUmakefile", SCRIPT_MERGED), "GNUmakefile", id="Makefile+GNUmakefile+scripts"), ] @@ -119,17 +147,40 @@ def collisions_of(adopter, *kept: str) -> list: entries_of(*[(path, "root") for path in kept])) +@pytest.mark.parametrize("first", ["GNUmakefile", "makefile"]) +def test_where_make_reads_another_file_the_script_tail_says_no_make_bootstrap( + adopter, first): + """#1105's review, NIT-2. Where `scripts/bootstrap.py` went aside too and + `make` reads a kept `GNUmakefile` or `makefile`, "neither `make + bootstrap` nor `python3 scripts/bootstrap.py` runs the shape's bootstrap + before both merges are made" read as if `make bootstrap` ran it after + them, which it does not where that file neither has the targets nor + includes a Makefile that does; and where that file has them, `make + bootstrap` runs it once the script is merged, with no `Makefile` merge at + all. The tail says what the + script's merge does, and names `python3 scripts/bootstrap.py` as what runs + then, as `execute`'s NEXT does (#1047).""" + (line,) = [item for item in collisions_of(adopter, "Makefile", first, + "scripts/") + if item.startswith(MERGE_MAKEFILE)] + assert "before both merges are made" not in line, line + assert line.endswith(SCRIPT_MERGED), line + + @pytest.mark.parametrize("kept, tail, first", COLLISIONS) def test_the_makefile_collision_names_the_file_gnu_make_reads_first( adopter, kept, tail, first): """THE MEASURED CLAIM (#1048, r1046's NIT-B). "The shape's make targets arrive only with that merge" holds where the merged `Makefile` is the file `make` reads. Where the root keeps `GNUmakefile` or `makefile` - beside it, `make` reads that one, so the follow-up names it, and does not - say the merge brings the targets. Where the root keeps only the source's - `Makefile`, the sentence is what it was, byte for byte.""" + beside it, `make` reads that one, so the follow-up names it, and says + neither that the merge brings the targets nor that it does not: that + turns on whether the file includes `Makefile` (#1105's review, + SHOULD-1). Where the root keeps only the source's `Makefile`, the + sentence is what it was, byte for byte.""" (line,) = [item for item in collisions_of(adopter, *kept) if item.startswith(MERGE_MAKEFILE)] + assert MERGE_ALONE_FAILS not in line, line assert line.endswith(". " + tail), line assert ("arrive only with that merge" in line) is (first is None) assert (f"reads the source's `{first}`, the first of {LISTED} it finds" @@ -299,3 +350,47 @@ def test_the_sentences_are_true_of_what_make_does_in_the_clone( assert printed.returncode == 0, printed.stderr assert Path(printed.stdout.strip()) == ( fixed.resolve() / SPEC_PATH / "contracts") + + +#: A kept file GNU make reads before `Makefile` that adds a rule of its own +#: and then reads `Makefile` too: a common idiom (#1105's review, SHOULD-1). +INCLUDING = "{name}-only:\n\t@echo {name} only\n\ninclude Makefile\n" + + +@needs_filter_repo +@needs_make +@pytest.mark.parametrize("first", ["GNUmakefile", "makefile"]) +def test_where_the_file_make_reads_includes_makefile_the_merge_alone_is_enough( + tmp_path, first): + """#1105's review, SHOULD-1, measured in a real adoption's clone. The + source keeps `Makefile` and a `GNUmakefile` or a `makefile` that ends in + `include Makefile`. Before the merge the follow-up asks for, `make -n + bootstrap` at the root has no rule; after `cat shape/Makefile >> + Makefile` alone it runs the shape's recipe, and `make contracts-dir` + prints the spec leg's `contracts/`. The split message's follow-up says + `make` reads that file and that the targets reach it once it includes a + Makefile that has them, which the merge makes so, and does not say they + do not arrive with that merge alone.""" + if not disk_keeps_case(tmp_path): + pytest.skip("this disk ignores case: it cannot hold the rows apart") + tree = tree_keeping(("Makefile", first)) + tree[first] = INCLUDING.format(name=first) + adopted = adopt_with_legs_mounted_away(tmp_path, by_hand=False, tree=tree) + clone = adopted["clone"] + message = " ".join(git("log", "-1", "--format=%B", SPLIT_BRANCH, + cwd=adopted["source"]).stdout.split()) + before = make(clone, "-n", "bootstrap") + assert before.returncode != 0, before.stdout + before.stderr + # THE MERGE THE FOLLOW-UP ASKS FOR, and nothing else. + appended(clone / "Makefile", (clone / "shape" / "Makefile").read_text( + encoding="utf-8")) + after = make(clone, "-n", "bootstrap") + assert after.returncode == 0, after.stdout + after.stderr + assert "python3 scripts/bootstrap.py" in after.stdout, after.stdout + printed = make(clone, "contracts-dir") + assert printed.returncode == 0, printed.stderr + assert Path(printed.stdout.strip()) == ( + clone.resolve() / SPEC_PATH / "contracts") + # What the follow-up says of it. + assert MERGE_ALONE_FAILS not in message, message + assert reads_instead(first, MEANWHILE) in message, message diff --git a/tests/test_pinned_copy_text.py b/tests/test_pinned_copy_text.py index e38cde47..dc27b0f9 100644 --- a/tests/test_pinned_copy_text.py +++ b/tests/test_pinned_copy_text.py @@ -72,8 +72,9 @@ #: root), and of a SCAFFOLDED one, whose legs are the scaffold's seed (F5). #: And of the file `make` reads: GNU make reads the first of `GNUmakefile`, #: `makefile` and `Makefile` it finds, so where the source kept either of the -#: first two the shape's Makefile is never the one read, and merging into -#: `Makefile` brings nothing (#1048, measured on #1046's review). +#: first two `make` reads that file, and the shape's Makefile, or a `Makefile` +#: it is merged into, only where that file includes it (#1048, measured on +#: #1046's review; the include on #1105's, NIT-3 and SHOULD-1). ADOPTED_SENTENCES = ( "Where this root was ADOPTED, the shape's Makefile exports " "`CONTRACTS_DIR` once, as the `contracts/` of the spec leg at the path " @@ -83,9 +84,9 @@ "kept a Makefile of its own, that block waits in `shape/Makefile` until " "the two are merged. Where the source kept a `GNUmakefile` or a " "`makefile`, beside a `Makefile` or instead of one, `make` at the root " - "reads the first of them and never the shape's Makefile, so `make " - "contracts-dir` has no rule there until that file holds the block, or " - "includes a Makefile that does.") + "reads the first of them, and not the shape's Makefile unless that file " + "includes it, so `make contracts-dir` has no rule there until that file " + "holds the block, or includes a Makefile that does.") SCAFFOLDED_SENTENCE = ( "A SCAFFOLDED root's Makefile exports none: its legs start from the " "scaffold's seed, with no `contracts/`, so nothing reads across them "