Repository navigation
scripts/shape_materialize.py: adopt_makefile_block's docstring says the block lands where place writes the shape's Makefile, the root's or shape/Makefile where the source keeps one, not the root's alone (#1135) - #1160
Merged
Conversation
…ring says the block goes where place writes the shape's Makefile, the root's or shape/Makefile (#1135) The docstring said the block is appended to the assembly root's Makefile. execute passes append={"Makefile": ...} and _materialize adds it to the template's text before place writes it, so it lands at the root's Makefile, or at shape/Makefile where the source keeps a Makefile of its own (#906, #977, #1061). Docstring only: no code line changes. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideImplementationUpdate
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
(lane openRepoShape-1, 2026-10-22T14:29:09Z) Independent review r1160 of #1160 at Review of PR #1160 (head
|
| Case | Measured |
|---|---|
a. source keeps no Makefile |
The root's Makefile equals the template's text plus the block. shape/Makefile is absent. |
b. source keeps Makefile |
collisions == [('Makefile', 'shape/Makefile')]. The root's Makefile is byte-identical to the source's, with no block. shape/Makefile equals the template's text plus the block. |
c. source keeps makefile, case-sensitive disk |
No collision. The block lands at the root's Makefile, and makefile is untouched. |
| e. simulated case-insensitive disk | Path.exists is patched to fold case, which is the repo's own model in tests/test_adopt_root_shape_paths.py:488. The kept makefile takes the name, so the result is [('Makefile', 'shape/Makefile')] and the block is in shape/Makefile. I could not mount a real case-insensitive disk in scratch. |
f. case b without append |
The block is in neither file, so it appears only because append carries it. |
Each code claim matches the source:
_materialize: thefor rel in verbatim:loop at L4698-4701 doestext += (append or {}).get(rel, "")and then callsplace(rel, ...).Makefileis inCOPIED_VERBATIM(L449) and not inTEMPLATED.place: L4656-4691 checkspath.exists(), setsactual = f"{collision_dir}/{rel}", records the collision, and writes there.adopt-project.py:_commit_the_splitbuildsappend = {"Makefile": adopt_makefile_block(spec_path, code_path)}at L15284.- It passes
collision_dir=COLLISION_DIRat L15303. COLLISION_DIR = "shape"is at L1108.cmd_executecalls_commit_the_splitat L15433.
- "Waits until a person merges the two":
adopt-project.pyL3842-3845 says "that block waits inshape/Makefileuntil the two are merged, and the run adds none of it to the root's Makefile". Nothing in the code merges them. - Case-insensitive clause:
adopt-project.pyL3782-3787 says "place'sPath.exists()holds a keptmakefileto beMakefile(adopt-project.py: a kept lower-casemakefileorGNUmakefilestill gets "the root Makefile wiresCONTRACTS_DIR", and README.md'sCONTRACTS_DIRsentence has no kept-Makefile case (follow-ups to #906) #977)", which is the same claim. - Issue numbers: adopt-project.py: five printed sentences say what the run does not do: the Makefile follow-up beside a root Makefile, the
contracts/-only read-across path,execute would stop there,a path names no attribute, and WHAT WILL HAPPEN under--local-remote-dir#906 and adopt-project.py: a kept lower-casemakefileorGNUmakefilestill gets "the root Makefile wiresCONTRACTS_DIR", and README.md'sCONTRACTS_DIRsentence has no kept-Makefile case (follow-ups to #906) #977 are the closed issues on this exact rule. adopt-project.py, README.md and tests: three follow-ups of PR #1046 (#977) from its review: a keptmakefile/row no test asks (mutant R10), README's make sentence that leaves out a keptGNUmakefileormakefile, and an olderCONTRACTS_DIRdocstring #1061 is open and covers the follow-ups, and scripts/shape_materialize.py: adopt_makefile_block's docstring says the block goes to the assembly root's Makefile, but where the source keeps a Makefile it goes to shape/Makefile (follow-up of #1061) #1135's own text cites all three.
2. The diff is the docstring only
- One hunk, 13 additions and 2 deletions, in one file.
- I compared the AST of the base and head files with every docstring blanked. They are equal, 167 docstrings in each, and the only docstring that differs is
adopt_makefile_block's. git diff --checkis clean and the diff has no non-ASCII characters.- Added lines are 77 characters or fewer, inside the file's 79-column docstring width, and the all-caps lead and
(#NNN)history note match the file's style. - No pin or copy list holds this file's bytes.
git grep -e <base sha256>exits 1 (no match), and the file is in noCOPIED_*list.
3. The PR body
| Check | Result |
|---|---|
| Title length | 218 characters, within the 250 limit. |
| Closing keywords | Exactly one, the line Closes #1135. closingIssuesReferences is [#1135]. |
| HTML tags outside code spans | None. |
Operator address, @ signs, absolute paths |
None. |
Cited shape_materialize.py lines |
All correct: L4656-4691, L4659, L4667, L4684-4685, L4698, L4700 and L449. |
| "11 lower on base" | Correct: def place is at L4645 on base and L4656 on head, and the same holds for the other lines. |
Cited adopt-project.py lines |
L15284, L15303, L1108, L15337 and L15433 are all correct. |
| 84 files, 7599 tests | I collected the 84 files (85 grep hits including conftest.py) and got 7599. |
| Sums | The per-call counts add to 7562 passed and 37 skipped. |
| Base sentence | True: the core call gave 358 passed on base, the same as on head (see §4). |
Four NITs follow, two on the body and two on the docstring. None blocks.
NIT-1 (PR body, "Lines relied on"). It says L3821-3823 and L16058 "already say the block waits in shape/Makefile". L3821-3823 is the case-insensitive passage and says where the shape's Makefile is written, not that the block waits. The lines that say the block waits are:
- L3806-3808: "the shape's goes beside it as
shape/Makefilewith the block in it". - L3842-3845: "that block waits in {copy} until the two are merged".
L16058 only says the shape's Makefile sits at shape/Makefile until a person merges the two. The fix is to cite adopt-project.py L3806-3808 and L3842-3845 instead of L3821-3823.
NIT-2 (PR body, "Tests"). It says "in 13 calls", but the list under it has 17 results: 1 core, 4 doctor shards, 6 rest shards, 2 for rest2 and 4 for rest4. 13 was the planned count (1 + 4 + 8) before rest2 and rest4 were split. The fix is to say "17 calls (13 planned; rest2 and rest4 were split)". The "other twelve calls" sentence has the same planned-count basis.
NIT-3 (scripts/shape_materialize.py L1147-1148). The text reads "execute passes it as append={"Makefile": ...}". That is one entry of the dict. _commit_the_split (adopt-project.py L15286-15293) also adds append["project.yaml"] when a submodule record exists and append[SHAPE_MANIFEST] when the plan has answers. The issue text uses the same shorthand, so this is optional. The fix is to reflow L1147-1148 as:
THE ROOT'S (#906, #977, #1061). `execute` passes it as the `"Makefile"`
entry of `append`, and `_materialize` adds it to the template's text
(Only L1147-1148 change. L1149 onward stays as it is, so the reflow adds no lines.)
NIT-4 (scripts/shape_materialize.py L1150-1152). "place writes a copy to collision_dir when its name is taken" is incomplete in one case. Where shape/Makefile is also taken, place raises Refusal materialize-copy-blocked and writes nothing. My case d confirms it: "the shape's copy of Makefile cannot be written to shape/Makefile: shape/Makefile is already there, and nothing was written there". check reports this ahead of time as plan-root-blocks-shape-copy, and nothing lands in that case, so it is harmless. If the author wants it exact, change "writes a copy to collision_dir when its name is taken" to "writes a copy to collision_dir when its name is taken, and refuses when that is taken too".
Related wording, no change needed. "Where the source keeps a Makefile of its own" means the root keeps it. A Makefile that the plan sends to a leg is git rm'd from the clone by _mount_the_legs before place runs, and then the block lands at the root's Makefile. adopt-project.py L3806 uses "the root keeps a Makefile of the source's own". Say it that way only if NIT-3's reflow is done anyway.
Two things are out of scope and need no action here. adopt-project.py L3719 ("CONTRACTS_DIR is the root Makefile's alone") is #1061's item (c). tests/test_adopt_contracts_dir.py L7 ("appends to the assembly root's Makefile") describes #522's historical behaviour. The head commit's subject starts with "checkpoint:", so the squash needs the explicit message the landing procedure already calls for.
4. Tests
All runs used the stub gh first on PATH, GH_TOKEN=bogus-r1160, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null, GIT_SSH_COMMAND=false, EMAIL=r1160@openreposhape.invalid and env -u LANES_LANE. I also put a scratch symlink to git-filter-repo on PATH so the needs_filter_repo tests would not skip.
| Run | Result |
|---|---|
| Six files on head, one pytest call | 358 passed in 116.68s, 0 skipped. The per-file counts add to 358. |
Same six files on a scratch clone at 038dafba9 |
358 passed in 119.94s. |
Extra on head: test_adopt_make_reads_first.py and test_adopt_contracts_dir.py |
67 passed. |
| Collection of the 84 files, collect-only | 7599 tests. |
The six head files are test_update_shape_add.py (20), test_family_makefile_siblings.py (31), test_neutral_pin_entry_one_spelling.py (115), test_update_shape_unreadable.py (48), test_adopt_leg_path_collisions.py (95) and test_path_guard_hygiene.py (49).
I did not rerun the other 12 calls from the PR body. The AST-equality result covers them, because a docstring-only change cannot alter what they see. The head checkout is clean: git status --short is empty and HEAD is still 0068f5af8. I changed no repository file and posted nothing. The gh calls were read-only.
Scratch left in place: /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1160 (the probe, the base clone at base-tree, logs, bt-* basetemps, and the PR json and body copies).
Lane: openreposhape-1 (openRepoShape-1)
This was referenced Oct 22, 2026
brettheap
added a commit
that referenced
this pull request
Oct 23, 2026
… leaves out a pattern like `refs/heads*` that puts a branch under another name, its clauses move into `_left_out`, and the docstrings name the clashes left and the way back (#1100) (#1157) #1100 holds five follow-ups from RV1087's review of #1087 (issuecomment-6314349196). This PR does all five: 1. Take the clause-4 fix for a pattern such as `refs/heads*`, and name the clash that is left, `+refs/pull/7/head:refs/remotes/origin/main`, in the three places that list clashes. 2. Add the `WHICH_CLAUSE` row that kills R16. 3. In `bump-leg.py`, name the unforced exception and the way back, `git fetch --force origin`. 4. Write down the accepted cost where the cost is described. 5. Move `origin_fetch_refspecs`' four clauses into a helper, `_left_out`. Fix round 1 (`baa19972a`) answers the independent review rv1157/rv1157b (issuecomment-6347113460: NOT READY, 2 SHOULD, 4 NIT, no BLOCKER). It does all six. See "Fix round 1" below. The NIT round (`368b87e9b`) answers rc1157's re-check of fix round 1 (issuecomment-6352215076: READY, 3 NITs, all in this body). It adds one test row, which kills F2, the survivor of fix round 1, and corrects three statements in this body: the reason F2 was left, the longest line's length and the #1131 merge-tree line. The scripts are unchanged. See "NIT round, after the re-check of fix round 1" below. Measured by a probe on git 2.43 and git 2.47.1, with local bare remotes only. The two gits gave identical tables. For each case the probe ran: - the repository's own `git fetch origin`; - the tools' fetch, `git fetch -q --prune --no-prune-tags --refmap= <origin|path> <refspecs>`, with the refspecs from main's `origin_fetch_refspecs`, once through `origin` and once through a path; - the repository's own fetch again ("own-after"). Where own-after failed, it then ran `git fetch --force origin`, a plain fetch, and the tools' fetch followed by the repository's own fetch ("again"). Branches are `main`, `x` and `release/x` at three unrelated commits unless the case says otherwise. ``` case own main o/p own-after force, plain, again C25 +refs/heads*:refs/remotes/origin/h*, branch h/x 0 128/128 0/0 - C18b +refs/pull/7/head:refs/remotes/origin/main alone 0 128/128 0/0 - C02 release/* onto origin/*, unforced 0 0/0 1/1 0 (ref back), 0, 1 C02c same, x descends from release/x 0 0/0 1/1 0 (ref back), 0, 1 C05u release/x onto origin/x, unforced 0 0/0 1/1 0 (ref back), 0, 1 C20u tags onto origin/*, unforced, branch and tag x 0 0/0 1/1 0 (ref back), 0, 1 C32 heads/* onto origin/*-a, branches q and q-a 0 0/0 0/0 - C33 feat-* onto origin/feat-*-old, feat-1, feat-1-old 0 0/0 0/0 - ``` These match the issue's numbers. C32 and C33 exit 0 because #1087 leaves them out. The own-after failure is `! [rejected] release/x -> origin/x (non-fast-forward)`. C02b, where `release/x` descends from `x`, gives 0. So the condition is that the kept commit does not descend from the branch's, not that the two commits are unrelated. The old paragraph's words "not in line" were wrong for C02c, and the new text corrects them. Main has since moved to `961640c6` (#1155, which touched only `docs/release-notes/v1.2.0.md`). The four files this PR changes are byte-identical there, so nothing above changes. **`scripts/shape_materialize.py`** - **Clause 4** (`gives_a_branch_another_name`) now reads a source that is not under `refs/heads/` as naming branches when it is a pattern whose part before the `*` is a start of `refs/heads/`. This is how `can_name_a_tag` reads `refs/*` for tags: ``` fixed, star, _ = source.partition("*") if not source.startswith(HEADS_PREFIX): return bool(star) and HEADS_PREFIX.startswith(fixed) ``` Such a pattern's `*` also takes what the pattern leaves of `refs/heads/`. So no destination under `refs/remotes/origin/` gives a branch its own name, and every such refspec is left out. The only pattern that keeps the names, `refs/heads*:refs/remotes/origin*`, writes outside `origin/`, and clause 1 leaves it out first. The shorter starts (`refs/`, `*`) can name a tag, and clause 2 leaves them out first. - **The fix was taken only after measuring it.** A pure-function differential of main against the branch ran over 39,385 configs: 43 sources x 31 destinations, forced and unforced, negatives, pairs, doubles, and each one beside the first refspec. - The branch carries nothing that main does not. - It newly leaves out 207 distinct refspecs. Every one has a pattern source whose part before the `*` starts `refs/heads/` but neither `refs/heads/` nor `refs/tags/` (`refs/h*` to `refs/heads*`). - The helper alone (main plus `_left_out`) gives main's output on all 39,385 configs. - The fix alone and the fix plus the helper give the same output as each other on all 39,385. - **The probe of all 53 cases**, on both gits, changed no outcome outside that class: exits, own-after, force, plain and again are identical for the 46 cases outside the class, C03u, C17u and C32o included. The seven cases in the class changed: ``` case own main o/p branch o/p own-after (branch) C25 refs/heads* onto origin/h*, branch h/x 0 128/128 0/0 0/0 C25u same, unforced 0 128/128 0/0 1/1; --force 0, plain 0 C25n refs/heads* onto origin/h*, no clash 0 0/0 0/0 0/0 (pruned, written back) C25g default glob + refs/heads* onto origin/h* 0 0/0 0/0 0/0 (pruned, written back) C25b default glob + refs/he* onto origin/he*, heads/x 128 128/128 0/0 128/128 C25c refs/he* onto origin/he*, branch heads/x 0 128/128 0/0 0/0 C25h glob + refs/h* onto origin/hh/*, ref refs/hidden/1 0 0/0 0/0 0/0 (pruned, written back) ``` Wherever a tools' fetch exited 0, every remote branch's `refs/remotes/origin/<name>` held that branch's commit (all cases, both rules, both ways, both gits). A fetch that exited 128 changed no ref. - **`_left_out(source, destination)`** holds the four clauses, in their order and with their comments. `origin_fetch_refspecs` now asks it once. The SonarQube analyzer (`python:S3776`, `analyze_code_snippet`) raises nothing on an excerpt of the four functions as committed, with their docstrings dropped. With four more `if ...: continue` in the loop it reports "from 17 to the 15 allowed", so `origin_fetch_refspecs` measures 9 (it was 15). By hand, `_left_out` is 3 and `gives_a_branch_another_name` is 2. - **The docstrings:** - `gives_a_branch_another_name` no longer ends "A source not under `refs/heads/` names no branch". It names the pattern and points to WHAT CAN STILL CLASH. - The A BRANCH UNDER ANOTHER NAME paragraph names the pattern. - A new THE COST, ACCEPTED paragraph says that, after either tool runs, an unforced refspec's own fetch exits 1 on every run until `git fetch --force origin`. Where the refspec was carried, the tools' fetch exited 128 in those same states and left the ref alone. No commit is lost, and main already behaves this way for unforced tags onto the glob. - WHAT CAN STILL CLASH now names any carried refspec whose source names no branch, onto a name a branch has: the pull requests in `origin/pr/` and C18b's single name. - Fix round 1 changes three of these paragraphs and adds one (below). **`scripts/bump-leg.py`**, docstrings only. An AST comparison with the docstrings blanked shows the code is identical to main's, at both heads. - The #1066 paragraph of `_leg_fetch_refspecs` names the `refs/heads*` pattern. It adds NOT WHERE THE LEG'S REFSPEC WAS WRITTEN WITHOUT ITS `+`, with the way back, `git fetch --force origin`. - "Two ways remain" now names the second way as a source that names no branch, onto a name a branch has, in a folder or onto a single name (C18b). **`tests/test_family_bump_fetch_refspecs.py`** - New `WHICH_CLAUSE` rows (both tools' tests ask them): - `another-name-suffix`, `+refs/heads/*:refs/remotes/origin/*-old`, left out (item 2); - `another-name-heads-pattern`, `+refs/heads*:refs/remotes/origin/h*`, left out; - `another-name-heads-pattern-shorter`, `refs/he*:refs/remotes/origin/he*`, left out; - `carried-not-refs-heads`, `+refs/headsx/*:refs/remotes/origin/headsx/*`, carried; - fix round 1: `another-name-heads-pattern-refs-h` and `another-name-heads-pattern-suffix` (below). - New `BRANCH_UNDER_ANOTHER_NAME` row `heads-pattern-into-a-folder`, `+refs/heads*:refs/remotes/origin/h*`, with `x-w1066` and `h/x-w1066`. It runs end to end through `family.py bump` and through `bump-leg.py` via `origin` and via `--local-remote-dir`, with the clash premise first. - Fix round 1: `NO_BRANCH_A_PATTERN_TAKES` and its end-to-end rows (below). - NIT round: `carried-refs-h-not-a-pattern`, `+refs/h:refs/remotes/origin/hy`, carried (below). **`tests/test_bump_leg_fetch_refspecs.py`**: one docstring sentence, and in fix round 1 the leg's `NO_BRANCH_A_PATTERN_TAKES` rows. `scripts/family.py` is unchanged. Its `_origin_fetch_refspecs` paragraph "A mount whose refspec git cannot satisfy" gives examples and claims no complete list, and it stays true. One new commit, `baa19972a`, on `a35a97ea5`. No merge of main was needed: main's one new commit touches none of these files. The scripts' code is unchanged: `astsame.py` (AST with docstrings blanked) gives SAME CODE for `shape_materialize.py` against `a35a97ea5` and for `bump-leg.py` against main. **SHOULD 1 (finding 1): two mutants of the fix survived the rows.** Two `WHICH_CLAUSE` rows are added. Both tools' tests ask them (`test_family_bump_fetch_refspecs.py` holds the table, and `test_bump_leg_fetch_refspecs.py` imports it): - `another-name-heads-pattern-refs-h`, `+refs/h*:refs/remotes/origin/hh/*`, left out. This is the class's lower edge, the shortest start of `refs/heads/` that no earlier clause meets. - `another-name-heads-pattern-suffix`, `+refs/heads*x:refs/remotes/origin/h*x`, left out. Git accepts it and maps branch `x` onto `origin/h/x` and `ax` onto `origin/h/ax` (probe section B, both gits). On the pure function (`tools/rows.py`, `logs/rows.log`): ``` row main head B1 B9 F2 +refs/h*:refs/remotes/origin/hh/* carried left out carried left out left out +refs/heads*x:refs/remotes/origin/h*x carried left out left out carried left out ``` So each new row fails on main. B1 is killed by `-refs-h` and B9 by `-suffix`. F2 was the one survivor of fix round 1. The NIT round's row kills it (see "NIT round" and Mutants). **SHOULD 2 (finding 2): leaving the class out changes the answer from pin to refuse** for a commit that only a non-branch ref reaches, where the pattern takes that ref too. The three places that said nothing is lost now say so: - `shape_materialize.py`, `origin_fetch_refspecs`: LEFT OUT BY EITHER OF THE LAST TWO, NO BRANCH IS LOST TO THE ANSWER, followed by a new paragraph, BUT A COMMIT ONLY A REF THAT IS NO BRANCH REACHES IS REFUSED. It covers the pull requests onto the branches' names (left out since #946, refused on main too) and, since #1100, a ref of another namespace that a pattern from a start of `refs/heads/` takes. It says that every run of either tool refuses such a commit alike. It also says that a refspec whose source is that namespace itself is carried, and the commit is pinned through it. This is finding 2(b): one remote ref, two answers by refspec. - `shape_materialize.py`, `_left_out`'s comment: a ref that is no branch, which a pattern takes too, no longer answers for one, so a commit only it reaches is refused, as one only a tag reaches is. - `bump-leg.py`, the #1066 paragraph: "so no branch is lost to the answer", and a new paragraph, A COMMIT ONLY A REF OF ANOTHER NAMESPACE THE PATTERN TAKES REACHES IS REFUSED NOW (#1100), with the review's `heads-x-in` case and its `heads-x-out` control. New end-to-end rows pin the refusal, `NO_BRANCH_A_PATTERN_TAKES` (`+refs/heads*:refs/remotes/origin/h*` with `refs/heads-archive/old-w1100`, and `+refs/h*:refs/remotes/origin/hh/*` with `refs/hidden/old-w1100`): - `test_a_ref_a_heads_pattern_takes_answers_for_no_branch` runs `family.py bump --dry-run` and `bump`. Both refuse `member-to-local-only` in the plain words, with nothing in the holder moved and no ref of the mount moved. - `test_a_ref_a_heads_pattern_takes_answers_for_no_branch_of_the_leg` runs `bump-leg.py` via `origin` and via `--local-remote-dir`. Both runs refuse `bump-leg-commit-local-only` in the plain words, with nothing in the root moved and no ref of the leg moved. - Each first asserts its premises: the ref reaches the commit, no branch of the remote does, and no remote-tracking ref does. All 6 rows fail on main, where both runs pin (Rows failing on main). The docstrings' claims, measured by `tools/claims.py` (the tools' own code from an exported tree: `fetch_leg` end to end, then `family.py`'s reader and `_on_a_remote_branch`). It ran on main `961640c6` and on `baa19972a`, on git 2.43 and 2.47.1. The two gits gave identical output on each tree (`logs/claims-{main,head}-{243,247}.log`): ``` case refspec beside the glob ref holding C main head C25h-in +refs/h*:refs/remotes/origin/hh/* refs/hidden/1 PIN REFUSE C25h-unforced refs/h*:refs/remotes/origin/hh/* refs/hidden/1 PIN REFUSE heads-x-in +refs/heads*:refs/remotes/origin/h* refs/heads-archive/old PIN REFUSE heads-x-unforced refs/heads*:refs/remotes/origin/h* refs/heads-archive/old PIN REFUSE hidden-out +refs/hidden/*:refs/remotes/origin/hidden/* refs/hidden/1 PIN PIN heads-x-out +refs/heads-archive/*:refs/remotes/origin/h-archive/* refs/heads-archive/old PIN PIN pulls-out +refs/pull/*/head:refs/remotes/origin/pr/* refs/pull/7/head PIN PIN pulls-onto-glob +refs/pull/*/head:refs/remotes/origin/* refs/pull/7/head REFUSE REFUSE ``` - `family.py`'s answer equals `bump-leg.py`'s in every row, and so do the two readers' lists. - On the head, the tools' fetch exits 0 and prunes the class ref (`origin/hh/idden/1`, `origin/h-archive/old`). The repository's next own `git fetch origin` exits 0 and writes it back. **NIT 1 (finding 3), `bump-leg.py`'s tag paragraph.** It now says that such a ref is pruned or holds the branch of that name, and that the leg's own next fetch writes it back. It adds NOT WHERE THE LEG WROTE THAT REFSPEC WITHOUT ITS `+` and the tag's commit does not descend from the branch's: then the leg's own fetch exits 1 on that run and every one after it, until `git fetch --force origin`. Measured (probe section C, both gits, main and head alike): ``` case refspec own tool own-after --force (ref back) plain C20 +refs/tags/*:refs/remotes/origin/* 0 0 0,0 - - (origin/x back at the tag) C20u refs/tags/*:refs/remotes/origin/* 0 0 1,1 0 (back) 0 ! [rejected] x -> origin/x (non-fast-forward) C20u-desc same, the tag's commit a child of x 0 0 0,0 - - ``` **NIT 2 (finding 4).** The 109-character line is reflowed. The longest line this PR adds, over `961640c6..368b87e` in the four files, is 78 characters (`test_bump_leg_fetch_refspecs.py`'s `assert contains(bare_of(remotes), made, ref), f"premise: {ref} reaches it"`). In the scripts the longest added lines are 75 (`bump-leg.py`) and 76 (`shape_materialize.py`). `bump-leg.py`'s longest line, 81, is main's. This body said 79 until the re-check measured 78. **NIT 3 (finding 5), the overclaim in `gives_a_branch_another_name`.** It no longer says each such refspec puts a branch under another name. Where the part before the `*` is all of `refs/heads` and the destination's `*` comes right after a `/`, git refuses the name it gives each branch, and the fetch writes no branch. The docstring now says this, and says that the pattern still takes the refs of another namespace, pointing to the new paragraph. Probe section B, both gits: `+refs/heads*:refs/remotes/origin/h/*` with branches `main`, `x` and `ax` exits 0, says `error: * Ignoring funny ref 'refs/remotes/origin/h//x' locally` (and the same for `h//main` and `h//ax`), and writes only `origin/h/foo/x` for `refs/headsfoo/x`. **NIT 4 (finding 6), the stale merge-tree list.** Merge-trees were re-run at `baa19972a` against main and every open PR touching these four files (Merge-trees). One new commit, `368b87e9b`, on `baa19972a`, in `tests/test_family_bump_fetch_refspecs.py` only: one `WHICH_CLAUSE` row, a sentence in the table's comment and one in its test's docstring. The scripts are byte for byte `baa19972a`'s (`git diff --name-only baa1997 368b87e` names that one file). The row is `pytest.param("+refs/h:refs/remotes/origin/hy", True, id="carried-refs-h-not-a-pattern")`. `WHICH_CLAUSE` is one table, which `tests/test_bump_leg_fetch_refspecs.py` imports and asks of `_leg_fetch_refspecs`, so the one row is two test ids, one in each file, and both tools' tests ask it. **NIT 1 (finding 1): the reason given for leaving F2 was wrong, and the row it said could not be added can be.** The first body said F2 differs from this head only on a source that is no pattern and is a start of `refs/heads/` (`refs/`, `refs/h`, ... `refs/heads`), that no such full name can be a ref beside the remote's branches, and that a row for it would pin an arbitrary answer. Measured by `tools/claims.py` (the tools' own code from an exported tree: `fetch_leg` end to end, then `family.py`'s reader and `_on_a_remote_branch`), on main `961640c6`, on this head and on this head with F2 applied, on git 2.43 and 2.47.1 (`logs/claims-{main,head,f2}-{243,247}.log`; the two gits gave identical output on each tree): ``` case refspec beside the glob ref holding C main head F2 refs-h-no-pattern +refs/h:refs/remotes/origin/hy refs/h PIN PIN REFUSE refs-head-no-pattern +refs/head:refs/remotes/origin/hz refs/head PIN PIN REFUSE refs-heads-no-pattern +refs/heads:refs/remotes/origin/hx refs/heads the remote cannot hold refs/heads and refs/heads/main hidden-out +refs/hidden/*:refs/remotes/origin/hidden/* refs/hidden/1 PIN PIN PIN C25h-in +refs/h*:refs/remotes/origin/hh/* refs/hidden/1 PIN REFUSE REFUSE ``` - Only `refs/heads` itself cannot be such a ref: `git update-ref` refuses it with "cannot process 'refs/heads/main' and 'refs/heads' at the same time" (exit 128, no ref made). `refs/h` and `refs/head` are made beside `refs/heads/main` with exit 0. - With `+refs/h:refs/remotes/origin/hy` and a commit C that only `refs/h` reaches, main's code and this head's carry the refspec and PIN C. Under F2 both tools leave it out and REFUSE C (`bump-leg-commit-local-only`, `family_answer=False`): the tools' fetch prunes `origin/hy`, which the repository's own fetch had written, and the next own fetch writes it back. - So this head's answer is not arbitrary. It is the rule ANY OTHER SOURCE NAMES NO BRANCH, the same as `hidden-out`'s: a source that is no pattern takes the ref of its name. Where the remote has no ref of that name, git completes the name instead: with a branch named `refs/h` (the ref `refs/heads/refs/h`) beside `hy`, the tools' fetch line exits 128 with `Cannot fetch both refs/heads/hy and refs/heads/refs/h to refs/remotes/origin/hy`, on git 2.43.0 and 2.47.1, on main as here. That is the WHAT CAN STILL CLASH shape (lane 5's re-check, issuecomment-6371455922, NIT 1). - On the pure function (`tools/rows.py`, `logs/rows.log`, `origin_fetch_refspecs` alone): ``` row main head B1 B9 F2 +refs/h:refs/remotes/origin/hy carried carried carried carried left out +refs/head:refs/remotes/origin/hz carried carried carried carried left out +refs/h*:refs/remotes/origin/hh/* carried left out carried left out left out +refs/heads*x:refs/remotes/origin/h*x carried left out left out carried left out ``` - **The new row passes on main by design,** as `another-name-suffix` does, because main's clause 4 (`gives_a_branch_another_name`) answers `False` for every source not under `refs/heads/` (`961640c6`, `scripts/shape_materialize.py` line 1704), and `refs/h` meets no earlier clause. It passes on this head. It kills F2: with F2 applied to `scripts/shape_materialize.py`, `-k carried-refs-h-not-a-pattern` over both test files gives 2 failed (`test_which_clause_leaves_each_refspec_out[carried-refs-h-not-a-pattern]` and `test_which_clause_leaves_each_refspec_out_of_the_legs_fetch[carried-refs-h-not-a-pattern]`), and over the 90 rows of Mutants, 2 failed and 88 passed. The runs are in `logs/mut-summary.log`: the row alone, 2 passed on main's code (`logs/mut-ROW-main-code.log`), 2 passed on this head (`logs/mut-ROW-head.log`), 2 failed under F2 (`logs/mut-ROW-F2.log`). - The Mutants section below replaces the F2 sentence with this result. **NIT 2 (finding 2): the line length.** Fix round 1's NIT 2 sentence now says 78, and names the scripts' 75 and 76 (above). The new commit adds lines of at most 76 characters, all ASCII, and no assert that joins conditions with `and` or `or`. **NIT 3 (finding 3): the #1131 merge-tree line.** #1131's head is `55c30f20` now (its NIT round), not `a4beec9e`. `git merge-tree --write-tree 55c30f2 368b87e` exits 0 with tree `e036c02d6a12`; at `baa19972a` it gave `bd6ebc5cd3af`, as the re-check measured. See Merge-trees. Main's production code (`961640c6`: `scripts/shape_materialize.py` and `scripts/bump-leg.py`) was put in the worktree under this head's two changed test files. Then the committed files were put back, with sha256 and dirty=0 checked (`logs/mut-MAIN-code-head-tests.log`). The result is **17 failed, 106 passed** at `baa19972a`, and **17 failed, 108 passed** at `368b87e9b` (`logs/mut-MAIN-code-head-tests.log`, 125 rows): the NIT round's 2 rows pass on main, by design. The failures are exactly the rows the fix decides: - `WHICH_CLAUSE` `[another-name-heads-pattern]`, `[-shorter]`, `[-refs-h]` and `[-suffix]`, in both files: 8. Each is carried on main ("Left contains one more item"). - `test_a_branch_put_on_another_branchs_name_fails_no_fetch[heads-pattern-into-a-folder]` (member), and `..._of_the_leg[origin-heads-pattern-into-a-folder]` and `[local-remote-dir-heads-pattern-into-a-folder]` (leg): 3. The fetch exits 128 on main, so the runs give the failed-fetch note. - Fix round 1's `NO_BRANCH_A_PATTERN_TAKES` rows: `test_a_ref_a_heads_pattern_takes_answers_for_no_branch[heads-pattern]` and `[refs-h-pattern]` (member), and `..._of_the_leg[origin-heads-pattern]`, `[origin-refs-h-pattern]`, `[local-remote-dir-heads-pattern]` and `[local-remote-dir-refs-h-pattern]` (leg): 6. On main the pattern is carried, so the real run pins the commit instead of refusing it. The first round's 7 are among these. Fix round 1 adds 10: `-refs-h` and `-suffix` in both files, and the 6 end-to-end rows. `another-name-suffix` and `carried-not-refs-heads` pass on main by design. They kill mutants (below). NIT round, on `368b87e9b` with git 2.43, through `tools/mut.sh` (`logs/mut-summary.log`). The scripts are `baa19972a`'s, so these are fix round 1's mutants over the NIT round's tests. Each mutant was applied alone to the committed `scripts/shape_materialize.py`. The runs covered both changed test files with `-k "which_clause or another_branchs_name or heads_pattern_takes"` (90 rows, 35 deselected). After each run the committed files were put back, and their sha256 and dirty=0 were checked: 9 of 9 restores in the script matched (`shape_materialize.py` `16c6ec378c36...`, `bump-leg.py` `8f4af9c5e78c...`). ``` mutant failed result main's code (961640c) under these two files, all 125 rows 17 the rows above (108 passed) F2 the fix's `bool(star) and` dropped 2 killed (carried-refs-h-not-a-pattern x2) B1 the class drawn from `refs/he` up (`refs/h*` carried) 5 killed (-refs-h x2, refs-h-pattern x3) B9 the fix only where the `*` ends the source 2 killed (-suffix x2) B4 the fix undone (`return False` in its place) 17 killed (every row the fix decides) ``` F2 is killed now. At `baa19972a`, with 88 rows, it survived (88 passed); B1 (5 failed), B9 (2) and B4 (17) gave the same counts as here. The earlier sentence said F2 differs from this head only on a source that is no pattern and is a start of `refs/heads/`, and that no such full name can be a ref beside the branches. The first half holds: over the 39,385-config corpus F2 differs from this head only there (`refs/`, `refs/h`, ... `refs/heads`). The second half held for `refs/heads` alone. `refs/h` and `refs/head` are refs a remote can hold beside its branches, and the head carries `+refs/h:refs/remotes/origin/hy` and pins a commit only that ref reaches, where F2 refuses it (NIT round, NIT 1). The row `carried-refs-h-not-a-pattern` holds that answer. At `a35a97ea5`, whose code this head keeps, the first round measured these over `WHICH_CLAUSE` in both files (66 rows then): ``` mutant failed result R16 clause 4 compares only the part before the `*` 2 killed (another-name-suffix x2); on main's own rows it survives (102 passed) R17 clause 4 compares only the part after the `*` 8 killed (the four another-name rows x2) F3 only `refs/heads*` itself, not shorter starts 2 killed (another-name-heads-pattern-shorter x2) F4 any pattern starting `refs/h` (too broad) 2 killed (carried-not-refs-heads x2) F5 every pattern not under refs/heads/ left out 6 killed (carried pulls, not-refs-tags, not-refs-heads x2) H1 _left_out's OUTSIDE clause removed 2 killed H2 _left_out's tag clause removed 14 killed H3 _left_out's branches'-glob clause removed 2 killed (branches-glob-pull-requests x2) H4 _left_out's last clause removed 14 killed H5 `if _left_out(...)` negated 60 killed ``` Every run went through `run9.sh` on the commit `368b87e9b` (dirty=0 in each START line): gh stub first on PATH, a bogus token, `GIT_SSH_COMMAND=false`, `GIT_CONFIG_GLOBAL=/dev/null`, and `EMAIL` set. - git 2.43, the three files that name the functions (`test_family_bump_fetch_refspecs.py`, `test_bump_leg_fetch_refspecs.py`, `test_bump_leg_reachability_words.py`): **159 passed** (`logs/g243.log`; 157 at `baa19972a`, and the NIT round's 2 rows). - git 2.47.1 (a git 2.47.1 build first on PATH; `git --version` printed `git version 2.47.1` in the log): the same three files: **159 passed** (`logs/g247.log`). - git 2.43, `test_path_guard_hygiene.py`, `test_pinned_copy_text.py` and the trio (`test_repo_hygiene.py`, `test_agents_md_codes.py`, `test_cli_reference.py`): **306 passed** (`logs/hyg243.log`, the same count as at `baa19972a`). `git merge-tree --write-tree <target> 368b87e`, run at 2026-10-23T12:06:38Z against main and the head of every open PR that touches one of the four files (55 open PRs listed at 12:06:22Z; `logs/merge-trees.log`). Each exits 0 with no conflict: - `origin/main` `961640c6`: tree `19c8442d96b0`; - #1104 `de89aa81` (`shape_materialize.py`): tree `b76c36be588a`; - #1105 `b28f40de` (`shape_materialize.py`): tree `5f2a8b3706e4`; - #1118 `2307036a` (`shape_materialize.py`): tree `0c21c6b39607`; - #1131 `55c30f20` (`bump-leg.py`; its head moved from `a4beec9e` in its NIT round): tree `e036c02d6a12`; - #1160 `0068f5af` (`shape_materialize.py`): tree `bce90eb65c6c`. The head's own tree is `351a9b73cda8`. Main is still `961640c6`: its one commit past `038dafba`, #1155, touches only `docs/release-notes/v1.2.0.md`. - C18b, a source that names no branch onto one branch's name, still clashes: own 0, tools 128. Which names the remote's branches have is not known from the refspecs. Leaving out every such refspec would also leave out the pull requests, which #946 carries on purpose. The docstrings now say so. The coordinator adds it and C25 to #1027's list. - Leaving out the `refs/h*` class prunes what such a refspec kept until the repository's next own fetch, as for #1066's cases (C25n, C25g, C25h). An unforced one held under a branch's name costs the non-fast-forward of THE COST, ACCEPTED (C25u). - Leaving it out also changes the answer for a commit that only a non-branch ref the pattern takes reaches, from pin to refuse (fix round 1, SHOULD 2). This is the rule #613 and #1037 keep for a commit only a tag reaches: a bump pins only what a branch reaches. Every run gives the same answer, so a dry run and the real run after it never differ. Pinning such a commit instead would be a design question, not part of this fix. Review: lane 5's independent review (issuecomment-6347113460) and its re-check, READY at `baa19972` (issuecomment-6352215076); then rc1157b, lane 5's independent sonnet re-checker, which wrote none of #1157: READY at `368b87e9`, the head that lands (issuecomment-6371455922), with no MUST or SHOULD; its wording NIT is corrected in the body. CI (Brett Heap's ruling 27): H0 is the head 368b87e itself, which passed tests, tests-git-bash and SonarCloud; main has touched none of its files since its base 038dafb; and the local gate passed on its predicted squash tree 44b3b5c over main b055bb7: the bump_leg_fetch_refspecs, family_bump_fetch_refspecs, repo_hygiene, agents_md_codes, cli_reference sets, 296 passed. Landed on Brett Heap's word of 2026-10-23T13:10:26Z, relayed to every lane by lane 3 (#473 issuecomment-6365697123): "merge v1.2.1 PRs once reviewed and green". Closes #1100 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
brettheap
marked this pull request as ready for review
November 8, 2026 15:25
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 5 days and 23 hours by commenting |
brettheap
added a commit
that referenced
this pull request
Nov 8, 2026
…says a full name that is no pattern takes the one ref git completes it to, which may be a branch, where it said any other source names no branch (#1238) (#1244) #1238 is a wording NIT that the review of #1235 (#1230) found in the code: NIT 4 of rv1235's first review, and NIT 2 of its re-check. The docstring of `gives_a_branch_another_name` in `scripts/shape_materialize.py` ended "ANY OTHER SOURCE NAMES NO BRANCH", but git completes a source that is a full name and no pattern, and completes `refs/h` to the branch `refs/heads/refs/h` where the remote has one. The sentence now says what git does with such a source, as an open list ("such as"), and where the clash is. A reader of the docstring is no longer told that a source outside `refs/heads/` is never a branch. Wording only, in one docstring of one file: 9 lines added, 3 removed. No code, no row, no other file. The file's syntax tree with every docstring dropped is the same as main's. The sentence is at `scripts/shape_materialize.py:1720-1723`, the end of the docstring of `gives_a_branch_another_name` (line 1671): ``` 1720: BRANCH REACHES). A shorter start, such as `refs/*` or `*`, can name a 1721: tag, and that clause has left it out first. ANY OTHER SOURCE NAMES NO 1722: BRANCH, though a refspec from one can still put a ref on a branch's 1723: name: `origin_fetch_refspecs` says where (WHAT CAN STILL CLASH). ``` It is true of a source in another namespace such as `refs/pull/7/head`. It is not true of every other source. A source that is a full name and no pattern takes the one ref git completes the name to on the remote: for `refs/h`, `refs/h` itself, the branch `refs/heads/refs/h`, the tag `refs/tags/refs/h`, `refs/refs/h` or `refs/remotes/refs/h`, and where several exist git takes the exact ref over a tag over a branch. Where the remote has the branch, the source is a branch. The code is right: `gives_a_branch_another_name("refs/h", ...)` is False, because the source has no `*` and does not start with `refs/heads/`, so the refspec is carried. What fails afterwards is the clash the docstring's own next clause names. Only the words were untrue. Measured on this branch's base, with git 2.43.0 (`/usr/bin/git`) and git 2.47.1, which gave the same answers on every row. Local bare remotes, `GIT_CONFIG_GLOBAL=/dev/null`, `GIT_CONFIG_NOSYSTEM=1`, `GIT_SSH_COMMAND=false`. rv1235's probe scripts were run again here (copied unchanged, new output folders), and their logs are identical to rv1235's. The fetch was `+refs/heads/*:refs/remotes/origin/*` plus the refspec of the row, and the remote had `main` and `hy`: ``` the remote has refspec result branch refs/heads/refs/h +refs/h:refs/remotes/origin/hy 128 Cannot fetch both refs/heads/hy and refs/heads/refs/h to refs/remotes/origin/hy tag refs/tags/refs/h +refs/h:refs/remotes/origin/hy 128 Cannot fetch both refs/heads/hy and refs/tags/refs/h to refs/remotes/origin/hy exact ref refs/h (update-ref) +refs/h:refs/remotes/origin/hy 128 Cannot fetch both refs/heads/hy and refs/h to refs/remotes/origin/hy refs/refs/h or refs/remotes/refs/h +refs/h:refs/remotes/origin/hy 128, the same message naming that ref each of those five refs, alone onto origin/not-a-branch 0, holds that ref's commit branch refs/heads/refs/h onto origin/refs/h 0 (the branch's own name: no clash) refs/pull/7/head onto origin/main 128 Cannot fetch both refs/heads/main and refs/pull/7/head to refs/remotes/origin/main branch refs/heads/main onto origin/main 0 ``` So a name with no `*` can be a branch that its spelling does not show, and the clash is a ref that is not that branch put on a branch's name, which is what the second half of the sentence has to say. "ANY OTHER" was wrong in a second way: a carried refspec from a source that IS the branch does not clash onto that branch's own name. The docstring's last sentence, now lines 1721-1729: ``` tag, and that clause has left it out first. ANY OTHER SOURCE TAKES NO BRANCH BY A PATTERN, and a full name that is no pattern, such as `refs/h`, takes the one ref git completes it to on the remote: that ref itself, or one such as the branch `refs/heads/refs/h` or the tag `refs/tags/refs/h` (measured, git 2.43 and 2.47.1). Such a name can stand for a branch its spelling does not show, so it is not left out here. A refspec from any such source can still put a ref on a branch's name, if the ref is not that branch itself: `origin_fetch_refspecs` says where (WHAT CAN STILL CLASH). ``` - **"Takes no branch by a pattern"** holds for every source this sentence covers. Git matches a pattern against each ref's whole name, and a ref under `refs/heads/` starts with the part before the `*` only where that part is a start of `refs/heads/` or is under it, the two cases the function reads before this sentence. - **The list is open** ("one such as"): it names the exact ref, the branch and the tag, and the rows above show two more completions (`refs/refs/h`, `refs/remotes/refs/h`), so no reader takes the three as all. - **"So it is not left out here"** is what the code does: `gives_a_branch_another_name` is False for `refs/h` onto `origin/hy`, onto `origin/refs/h`, for `refs/pull/7/head` onto `origin/main`, and for the pattern `refs/pull/*/head`, and `origin_fetch_refspecs(["+refs/h:refs/remotes/origin/hy"])` returns it carried. It is True for `refs/heads/main` onto `origin/hy`, as before. - **"If the ref is not that branch itself"** is the condition the rows show: the clash is two different refs on one name, a branch's. The checks the issue's Verify lists: - **The diff is only that docstring.** The diff has one hunk, inside the docstring, which spans lines 1672-1724 on main and 1672-1730 here. The file's tree with every docstring dropped (module, class and function: 168 each side) is equal on main (`50869f384`) and at this head under `python3 -I`, `ast.dump`: 380120 characters of dump on both sides. A control that renames `gives_a_branch_another_name` on the head side gives 380121 and DIFFERENT, and the same trees with the docstrings kept differ (520956 against 521366), so the check can fail. Python 3.12.3. The check compares two dumps made by one interpreter and asserts no text that depends on the version. - **`git grep -n 'ANY OTHER SOURCE NAMES NO$' -- scripts/`** finds nothing at this head. On main it finds `shape_materialize.py:1721`. With any case and no line end, `git grep -n -i 'other source names no'` also finds nothing here, and nothing but that line on main. - **No added line is over 79 columns or holds a non-ASCII character.** 9 lines are added, the longest is 75 columns, and none has a tab or trailing space. The file's own width is unchanged: 20 lines over 79 columns (longest 118) and 90 lines with a non-ASCII character, as on main, none of them mine. - **No test, `README.md`, `docs/` page or `AGENTS.md` line pins the sentence.** The issue's grep holds at this head. The issue asks whether `shape_materialize.py:1874` (now 1880, in the docstring of `origin_fetch_refspecs`) and `bump-leg.py:1165` (in `_leg_fetch_refspecs`) want the same change. Both paragraphs were read, and both are left as they are: ``` shape_materialize.py:1880: WHAT CAN STILL CLASH is a refspec carried whose source names no branch, bump-leg.py:1165: whose source names no branch, onto a name a branch of the remote has ``` 1. **They define a shape by its examples and say nothing about what git completes a name to.** Each goes on at once to the pull requests (`origin/pr/` where a branch `pr/7` is, and `+refs/pull/7/head:refs/remotes/origin/main` beside a branch `main`), and neither names `refs/h`. They were not the sentence that told a reader a source is never a branch. 2. **"Names no branch" is spelling there, as it is in the sentence the function's docstring opens with.** "A SOURCE UNDER `refs/heads/` NAMES BRANCHES" is true by spelling too: `can_name_a_tag`'s docstring measures that `+refs/heads/zz` took the tag `refs/tags/refs/heads/zz` where the remote had no branch `zz`. The sentence changed here now says outright that the spelling of a name with no `*` does not show what it stands for, so the docstring that defines the class carries the caveat the two borrowers need. 3. **The mechanism they state is the one that fails `refs/h`:** "wherever a name it writes is a branch's". The `origin_fetch_refspecs` paragraph ends with why such a refspec is carried at all: "Which names the remote's branches have is not known from its refspecs, and left out, what such a refspec keeps would be pruned on every run". That holds for `refs/h` word for word. 4. **The cost of leaving them is one reading:** a reader who takes "names no branch" in git's sense meets the single case `refs/h` beside a remote branch `refs/heads/refs/h`. If the review wants a word there, it is a few words in each place, and no open PR touches either paragraph (merge-trees below), so it can be a fix round. None of either. No row is added or changed and no behaviour moved: the syntax tree outside the docstring is main's, so there is nothing for main to fail and nothing to mutate. `gives_a_branch_another_name` returns what it returned for every case in the table above (called in process at this head; the syntax tree outside the docstrings is main's). All through the lane's runner `run9.sh` (stub `gh` first on PATH, `GIT_SSH_COMMAND=false`, `GIT_CONFIG_GLOBAL=/dev/null`), on the committed head `36ec589a6` with the tree clean (`dirty=0`), as one run of seven files at a time: `tests/test_pinned_copy_text.py`, `tests/test_path_guard_hygiene.py`, `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_cli_reference.py` (the hygiene five), `tests/test_family_bump_fetch_refspecs.py` and `tests/test_bump_leg_fetch_refspecs.py`. - **The seven files, git 2.43.0** (`/usr/bin/git`): 504 passed in 469.44s (47 `test_agents_md_codes.py`, 68 `test_bump_leg_fetch_refspecs.py`, 23 `test_cli_reference.py`, 57 `test_family_bump_fetch_refspecs.py`, 121 `test_path_guard_hygiene.py`, 87 `test_pinned_copy_text.py`, 101 `test_repo_hygiene.py`; the counts are from `--collect-only -q` on the same head and add up to 504). Log `v131/n1238/gate-g243.log`. - **The seven files, git 2.47.1** (first on PATH, `git --version` printed into the log): 504 passed in 714.00s. Log `v131/n1238/gate-g247.log`. - **`-k carried-refs-h-not-a-pattern`** over `tests/test_family_bump_fetch_refspecs.py` and `tests/test_bump_leg_fetch_refspecs.py`: 2 passed, 123 deselected, on 2.43.0 and on 2.47.1 (`run-k-g243.log`, `run-k-g247.log`). - Both runs start with `dirty=0`. Python 3.12.3 (CI runs 3.14); nothing asserts text that depends on the version. The measured git behaviour above comes from the probe scripts, not the test files. `-k carried-refs-h-not-a-pattern` over the two fetch-refspec files is part of those runs. `git merge-tree --write-tree origin/main HEAD` exits 0, with tree `9d874bcf2caf`, equal to this head's tree, against main `50869f384` (the merge base). Against the head of each open PR that touches `scripts/shape_materialize.py` or `scripts/bump-leg.py` (read from `gh pr list --state open --limit 100 --json number,headRefName,headRefOid,files`, 52 open PRs at the last read, 7 of them touch one of the two files), `git merge-tree --write-tree HEAD <head>` exits 0 and the merged tree still holds the new sentence and not the old one: ``` #1223 f50b472 exit 0 #1207 6fa1edd exit 0 #1160 0068f5a exit 0 #1131 55c30f2 exit 0 #1118 2307036 exit 0 #1105 b28f40d exit 0 #1104 de89aa8 exit 0 ``` Main moved once while this was gated, to `4829418f0` (`adopt-project.py`, `docs/cli.md` and a new test file: none of the files this change or its runs touch), and `git merge-tree --write-tree origin/main HEAD` against it also exits 0. None of their hunks is within 100 lines of the sentence changed here. Review: rv1244 and rv1244b, lane 5's independent sonnet reviewers, which wrote none of #1244: READY at `36ec589a6`, H0 of this landing (issuecomment-6723280477), with no MUST or SHOULD; its four NITs are optional (two wordings, a body note and a follow-up). CI (Brett Heap's ruling 27): H0 is the head 473959f itself, which passed tests, tests-git-bash and SonarCloud; main has touched none of its files since its base 9729315; and the local gate passed on its predicted squash tree fb798f3 over main 9252ef2: the repo_hygiene, agents_md_codes, cli_reference sets, 171 passed. Landed on Brett Heap's word of 2026-10-23T13:10:26Z, relayed to every lane by lane 3 (#473 issuecomment-6365697123): "merge v1.2.1 PRs once reviewed and green". Closes #1238 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1135
Outside v1.2.0 by Brett Heap's ruling of 2026-10-21T10:32:04Z ("leave the follow-ups out of v1.2.0"): this lands after the tag, on a later word.
What
Docstring only:
scripts/shape_materialize.py+13/-2, no code line changes.adopt_makefile_blocksaid the block is appended to the assembly root's Makefile. It is appended to the template'sMakefiletext beforeplacewrites that text, so it lands in one of two places. Measured by a probe that callsmaterialize_assembly_rootinto a temp directory:Makefile: the root'sMakefileholds the block andshape/Makefileis absent;Makefile: the root'sMakefileis the source's, untouched and without the block,shape/Makefileholds the block, andcollisions == [('Makefile', 'shape/Makefile')].Old first sentence: "What
adopt-project.pyappends to the assembly root's Makefile, and nothing else does."New first sentence: "What
adopt-project.pyappends to the shape'sMakefile, and nothing else does."New paragraph, after the first one: it lands where
placewrites the shape'sMakefile, which is not always the root's (#906, #977, #1061).executepasses it asappend={"Makefile": ...}, and_materializeadds it to the template's text beforeplacewrites it: at the root'sMakefile, or, where the source keeps aMakefileof its own, atshape/Makefile(placewrites a copy tocollision_dirwhen its name is taken, and a disk that ignores case takes a keptmakefilefor it). The root's ownMakefileis then the source's, and the block waits inshape/Makefileuntil a person merges the two.Lines relied on (head,
scripts/shape_materialize.py; every line after the docstring is 11 lower on base 038dafb):place, nested in_materialize: L4656-4691; L4659path.exists(); L4667actual = f"{collision_dir}/{rel}"; L4684-4685 the collision is recorded andpath = target / actual;for rel in verbatim:L4698 adds(append or {}).get(rel, "")(L4700) to the text beforeplacewrites it;Makefileis inCOPIED_VERBATIM(L449);adopt-project.py, unchanged:_commit_the_splitL15284append = {"Makefile": adopt_makefile_block(spec_path, code_path)}, L15303collision_dir=COLLISION_DIR(COLLISION_DIR = "shape", L1108), called fromcmd_execute(L15337, call at L15433). Its own texts at L3806-3808 ("the shape's goes beside it asshape/Makefilewith the block in it") and L3842-3845 ("that block waits in {copy} until the two are merged") already say so.Why
Found by r1123, lane 1's independent review of #1123 (#1061). The docstring is the only place that names where the block goes, and it named one of two. No code changes, so no behaviour changes; no pin records this file's bytes (it is in no
COPIED_VERBATIMorCOPIED_FROM_SHAPElist, andgit grepof its base sha256 finds nothing), so nothing to re-pin.Tests
Mutants: none apply, the change is a docstring.
Head (0068f5a, docstring applied), probe environment,
env -u LANES_LANE, one--basetempper call, node ids from files. The 84 test files that mentionshape_materializecollect 7599 tests, in 17 calls (13 planned;rest2andrest4were split after they timed out under load):test_update_shape_add,test_family_makefile_siblings,test_neutral_pin_entry_one_spelling,test_update_shape_unreadable,test_adopt_leg_path_collisions,test_path_guard_hygiene): 358 passed;test_shape_doctor.pyin 4 shards: 111 passed 1 skipped; 112 passed; 111 passed 1 skipped; 111 passed 1 skipped;Base (the file at 038dafb, swapped into the worktree and restored after): the core call, which holds the five readers #1135 names and the hygiene scan, gave 358 passed, the same as head. The other sixteen calls were not run on base. A docstring changes no behaviour they can see, and all of them pass on head.
Not in this PR
scripts/shape_materialize.pyoradopt-project.py.adopt-project.pythat already say where the block lands.Lane: openreposhape-1 (openRepoShape-1)
🤖 Generated with Claude Code
Summary by Sourcery
Correct the documentation to describe the actual destination of the adopted Makefile block.
Enhancements:
Documentation:
adopt_makefile_blockdocstring to accurately describe where the Makefile block is placed and how an existing source Makefile affects the result.