From 10f25e4b4de7bce817a77cdb4365dd4a30f9372e Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 16:09:20 +0000 Subject: [PATCH 1/2] WIP (#906): adopt-project.py's five printed sentences say what the run does (the kept root Makefile's block waits in `shape/Makefile`, the spec leg's own path is printed beside the mounts, `execute` is said to stop where it does, a link target names an attribute only with a space or tab, no visibility under `--local-remote-dir`); not finished: the fail-without-fix run, the remaining test files, README line 810 (#906) Lane 3's triage of #457 at 50178305 found five lines that said what the run does not do. All five are changed and measured; the verification is not finished (see the end). L2-72. `follow_ups_for` said "the root Makefile wires `CONTRACTS_DIR`" where the source keeps its own root Makefile. Then the shape's, with the block, is written to `shape/Makefile`, and `make contracts-dir` at the root answers `No rule to make target 'contracts-dir'` (measured on an `execute --local-remote-dir` of a source with a Makefile; the block is only in `shape/Makefile`). Where the root keeps the Makefile it now says "the shape's Makefile wires ...; but the source keeps a Makefile of its own at the root, so that block waits in `shape/Makefile` until the two are merged, and the run adds none of it to the root's Makefile before then", as #634 words it for the pinned AGENTS-shape.md. Otherwise the line is byte for byte what it was. The test is `_root_keeps`, the reading `_predict_collisions` already used to move the shape's copy, now one function for both. L2-73. `contracts_from_code_leg` gave a ready path only where the spec leg holds `contracts/`, so a code file that names `specs/` got the key-form line and no path. v1.1.0 printed none either: the `../spec/specs/` was v1.0.1's, before #600 took the path out of the follow-up. It now takes the plan's follow-ups too, and where one says a file must `read across the assembly root` (`holds_a_read_across`, found in both lines `plan` writes for it) the two sentences that print the mounts end ", so from the code leg the spec leg is `../spec/`", and, where the spec leg holds `contracts/`, " and the spec leg's `contracts/` is `../spec/contracts/`". With no read-across line the sentence is what it was. Measured on a source naming `specs/` only, for `spec`/`code`, `legs/spec`/`legs/code` and `a/b/spec`/`a/code`, and by following the path from the code leg of a recursive clone to the spec file. L1-91. `_refused_finding` said `execute` "would stop there, after the leg repositories exist", `there` being the step git refuses. Measured with `_refuse_what_check_finds` patched out, over a `--local-remote-dir`: a refused `git add` (`core.safecrlf=true` over a kept CRLF file) stops `execute` at `REFUSED committing the split`; `* working-tree-encoding=UTF-16LE` with `.gitattributes` exempted stops it at `REFUSED mounting the legs`, `failed to encode '.gitmodules'`; a smudge filter that fails stops it at `REFUSED extracting the spec leg`, neither leg pushed. The finding says "no later than committing the split, and sooner where a step before it meets the same rule: extracting a leg, or mounting the legs, where the rule reaches the `.gitmodules` it writes" for a refused store, and "at its first clone of the source" for a refused checkout. L1-93. The `_kept_nested_attributes` docstring said "a target that is a path names no attribute". Measured with git 2.43: a kept `.github/.gitattributes` linking to `config/attributes` (and to `docs/read me.md`) left the workflow copy LF; linking to `*.yml eol=crlf`, or `*.yml` TAB `eol=crlf`, wrote it CRLF. It now says it of a target with no space or tab only. The printed sentence in scripts/shape_materialize.py (`a path names no attribute`) is not changed here. L1-75. Under `--local-remote-dir`, WHAT WILL HAPPEN printed `create -spec and -code (private)`. It prints no visibility there (`execute` makes both with `git init --bare`); against GitHub the plan's visibility is printed as before. Tests. tests/test_adopt_printed_sentences.py is new (46 tests, all pass). tests/test_adopt_leg_keys.py, tests/test_adopt_contracts_dir.py (which also holds OWN_MAKEFILE_FOLLOW_UP beside MAKEFILE_FOLLOW_UP) and tests/test_adopt_reencoded_files.py move with the sentences they assert. Run and green with the stubbed-gh environment: test_adopt_leg_keys, test_adopt_contracts_dir, test_adopt_reencoded_files, test_adopt_leg_paths, test_adopt_literal_names, test_adopt_plan, test_adopt_entry_paths, test_adopt_leg_path_collisions, test_adopt_execute_refusals, test_adopt_c_locale_bytes, test_adopt_plan_escapes, test_stdout_encoding and test_shape_advisory together: 1088 passed, 2 skipped. Not finished: the run of the new tests against the base adopt-project.py (fail-without-fix); test_adopt_nested_attributes, test_adopt_attributes_eol_block, test_adopt_eol_block_through_links, test_adopt_e2e, test_adopt_plan_yaml_quoting, test_adopt_submodules, test_shape_doctor, test_scaffold_leg_path_collisions, test_cli_reference and test_repo_hygiene; and README.md line 810, which says the root Makefile exports `CONTRACTS_DIR` without the kept-Makefile case. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 178 ++++++++-- tests/test_adopt_contracts_dir.py | 19 +- tests/test_adopt_leg_keys.py | 19 +- tests/test_adopt_printed_sentences.py | 483 ++++++++++++++++++++++++++ tests/test_adopt_reencoded_files.py | 9 +- 5 files changed, 664 insertions(+), 44 deletions(-) create mode 100644 tests/test_adopt_printed_sentences.py diff --git a/adopt-project.py b/adopt-project.py index b3d7554c..efcc51e0 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -3149,9 +3149,10 @@ def cross_leg_references(source: "Source", does: both are paths in the SOURCE, which no edit of the plan moves. The Makefile follow-up (#522's review, F1) and the tracking branch's (#493's review, F3) are written the same way. The path from the code leg to the - spec leg's `contracts/`, which a person has then to work out from the two - mounts, is printed next to the CURRENT mounts by the two sentences that - name them (`contracts_from_code_leg`, #600's review, F1). + spec leg, and to its `contracts/` where it holds one, which a person has + then to work out from the two mounts, is printed next to the CURRENT + mounts by the two sentences that name them (`contracts_from_code_leg`, + #600's review, F1; #906). EVERY NAME IS A NAME (#216). Both inputs are source paths, and git reads each as something else unless told not to. A spec directory passed bare @@ -3255,11 +3256,29 @@ def spec_leg_from_code_leg(spec_path: str, code_path: str) -> str: return "/".join([".."] * (len(code) - shared) + spec[shared:]) -def contracts_from_code_leg(spec_path, code_path, spec_leg_paths) -> str: - """What the two sentences that print the plan's CURRENT mounts end with - when the spec leg holds `contracts/`: ", so from the code leg the spec - leg's `contracts/` is `../spec/contracts/`", or "" when there is nothing - true to say (#600's review, F1). +#: What each read-across follow-up says of what the file must do: both the +#: line `cross_leg_references` writes for a file that names a spec directory, +#: and `_search_by_hand`'s for a name git could not search for (#906). +READ_ACROSS = "read across the assembly root" + + +def holds_a_read_across(follow_ups) -> bool: + """Whether `follow_ups` holds a read-across follow-up: a line that says + a file must `READ_ACROSS`. Asked of the plan's own list, as the plan + stands, and not of a `git grep`, which `check` and the split message + never run (#906).""" + return any(isinstance(item, str) and READ_ACROSS in item + for item in follow_ups) + + +def contracts_from_code_leg(spec_path, code_path, spec_leg_paths, + follow_ups=()) -> str: + """What the two sentences that print the plan's CURRENT mounts end with: + ", so from the code leg the spec leg is `../spec/`" where the plan holds a + read-across follow-up, and ", so from the code leg the spec leg's + `contracts/` is `../spec/contracts/`" where the spec leg holds + `contracts/`, both joined by " and " where both are true; or "" when + there is nothing true to say (#600's review, F1; #906). THE READ-ACROSS FOLLOW-UP NAMES `legs.code_path` AND `legs.spec_path` AND NO PATH, so no hand edit of either can contradict it, and it leaves the @@ -3272,23 +3291,42 @@ def contracts_from_code_leg(spec_path, code_path, spec_leg_paths) -> str: `execute` made. It is never stored in `follow_ups:`, so it cannot go stale between a hand edit and the next `check`, and it needs no `git grep`. - NOTHING, in three cases. No path of the spec leg is `contracts/` or lies - under it, so there is no `contracts/` to lead to (the Makefile follow-up's - own condition). A mount is not text, or the two are not mount paths - `execute` accepts, or they overlap or differ only in case: `check` says - so as a finding, and a path computed from two strings that are not two - mounts leads nowhere. `spec_leg_paths` are the paths the plan assigns to - the spec leg, as `_leg_paths` reads them. + THE SPEC LEG'S OWN PATH, NOT ONLY ITS `contracts/` (#906). The follow-up + is written for a code file that names ANY directory the plan sends to + the spec leg, `specs/` as much as `contracts/`, and a person finishing + it needs the path to that directory. It gave one only where the spec leg + held `contracts/`, so a file naming `specs/` got the key-form line and no + ready path, where v1.0.1 printed `../spec/specs/`. The spec leg's own + path is the one every such directory lies below: from it the person + reaches `specs/` or any other. It is printed wherever `follow_ups` holds + a read-across line (`holds_a_read_across`), `contracts/` or not, and the + `contracts/` clause stays for the spec leg that holds one. + + NOTHING, in three cases. The plan holds no read-across follow-up and no + path of the spec leg is `contracts/` or lies under it (the Makefile + follow-up's own condition), so there is nothing to lead to. A mount is + not text, or the two are not mount paths `execute` accepts, or they + overlap or differ only in case: `check` says so as a finding, and a path + computed from two strings that are not two mounts leads nowhere. + `spec_leg_paths` are the paths the plan assigns to the spec leg, as + `_leg_paths` reads them, and `follow_ups` the plan's, as + `_plan_follow_ups` reads them. """ - if not any(path == "contracts/" or path.startswith("contracts/") - for path in spec_leg_paths): + holds_contracts = any(path == "contracts/" or path.startswith("contracts/") + for path in spec_leg_paths) + reads_across = holds_a_read_across(follow_ups) + if not (holds_contracts or reads_across): return "" if leg_path_problems(("legs.spec_path", spec_path), ("legs.code_path", code_path)): return "" across = spec_leg_from_code_leg(spec_path, code_path) - return (", so from the code leg the spec leg's `contracts/` is " - f"`{across}/contracts/`") + said = [] + if reads_across: + said.append(f"the spec leg is `{across}/`") + if holds_contracts: + said.append(f"the spec leg's `contracts/` is `{across}/contracts/`") + return ", so from the code leg " + " and ".join(said) def _search_by_hand(named: str, why: str) -> str: @@ -3300,6 +3338,20 @@ def _search_by_hand(named: str, why: str) -> str: "spec leg") +def _root_keeps(surviving: set[str], path: str) -> bool: + """Whether the root keeps `path`: as its own entry, or inside a + directory it keeps there. `surviving` is each path the plan's entries + send to the root. + + ONE DEFINITION, for `_predict_collisions`, which moves the shape's copy + of a path the root keeps under `shape/`, and for `follow_ups_for`, whose + Makefile line says where the block waits when that copy is the one that + moved (#906). Two readings of "the source keeps its own `Makefile`" is + how a follow-up says the root wires what `shape/Makefile` holds. + """ + return path in surviving or f"{path.split('/')[0]}/" in surviving + + def follow_ups_for(source: "Source", entries: list["Entry"], names: dict, collisions: list[str]) -> list[str]: """The code changes an in-place split MAKES NECESSARY, named in the plan. @@ -3336,19 +3388,36 @@ def follow_ups_for(source: "Source", entries: list["Entry"], names: dict, one (#522's review, F1). So does the read-across line that `cross_leg_references` writes, for both mounts, and this function takes no mount path: no value of either key can reach a line it writes (#600). - The path from the code leg to the spec leg's `contracts/` is printed - beside the CURRENT mounts instead, where `check` and the split message - name them (`contracts_from_code_leg`). + The path from the code leg to the spec leg, and to its `contracts/`, is + printed beside the CURRENT mounts instead, where `check` and the split + message name them (`contracts_from_code_leg`). + + WHERE THE ROOT KEEPS A `Makefile` OF THE SOURCE'S OWN, the shape's goes + beside it as `shape/Makefile` with the block in it, and the root's is + the source's, untouched: `make contracts-dir` there answers `No rule to + make target` until the two are merged. The line then says the block + waits in `shape/Makefile`, as #634 words it for the pinned + `AGENTS-shape.md`, of the plan's entries as `_predict_collisions` reads + them (`_root_keeps`), the reading that moves the copy (#906). """ out = cross_leg_references(source, entries) if any(e.leg == "spec" and (e.path == "contracts/" or e.path.startswith("contracts/")) for e in entries): + own = _root_keeps({e.path for e in entries if e.leg == "root"}, + SHAPE_MAKEFILE) + whose = "the shape's" if own else "the root" + waits = ( + "; but the source keeps a Makefile of its own at the root, so " + f"that block waits in `{COLLISION_DIR}/{SHAPE_MAKEFILE}` until " + "the two are merged, and the run adds none of it to the root's " + "Makefile before then") out.append( - "the root Makefile wires `CONTRACTS_DIR` to the `contracts/` of " + f"{whose} Makefile wires `CONTRACTS_DIR` to the `contracts/` of " "the spec leg, where the plan's `legs.spec_path` mounts it, and " "exports it, so the code leg's tooling takes the path from the " - "environment instead of assuming it sits beside it") + "environment instead of assuming it sits beside it" + + (waits if own else "")) if any(e.path in ("CODEOWNERS", ".github/CODEOWNERS") for e in entries): out.append( "CODEOWNERS in the assembly root now names paths that live in the " @@ -5533,7 +5602,7 @@ def _predict_collisions(entries: list[Entry], *, surviving = {e.path for e in entries if e.leg == "root"} beside = {path: f"{COLLISION_DIR}/{path}" for path in sorted(set(assembly_root_paths())) - if path in surviving or f"{path.split('/')[0]}/" in surviving} + if _root_keeps(surviving, path)} return [collision_follow_up(path, actual, beside, attributes_link=attributes_link) for path, actual in beside.items()] @@ -8576,8 +8645,12 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, # Every value below that is the plan's own is `_shown`, as a name is: # since #247 the reader gives back a lone surrogate, which raised on a # strict stdout before the FINDING about it was printed (#255). - print(f" create {names['spec']} and {names['code']} " - f"({_shown(str(plan.get('visibility', 'private')))})") + # A bare repository on disk has no visibility, so under + # `--local-remote-dir` none is printed: `execute` creates the legs with + # `git init --bare`, and never calls `gh repo create` (#906). + visibility = ("" if local else + f" ({_shown(str(plan.get('visibility', 'private')))})") + print(f" create {names['spec']} and {names['code']}{visibility}") print(f" extract {len(moved)} path(s) with `git filter-repo`, history " "preserved") for role in seeded: @@ -8591,7 +8664,8 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, # Beside the mounts, the path the read-across follow-up leaves to them, # from the mounts as the plan stands (#600's review, F1). across = contracts_from_code_leg(mounts[0], mounts[1], - _leg_paths(plan)["spec"]) + _leg_paths(plan)["spec"], + _plan_follow_ups(plan)) print(f" ONE split commit on branch " f"{_shown(str(plan.get('adopt_branch', ADOPT_BRANCH)))} " f"of {names['assembly']}: `git rm -r` those paths, mount the " @@ -9672,7 +9746,12 @@ def _kept_nested_attributes(plan: Plan, tree: list) -> list[tuple]: blob is its target, which git then reads as the file's rules: measured with git 2.43, a kept `.github/.gitattributes` linking to `*.yml eol=crlf` checked `.github/workflows/validate.yml` out with CRLF. A - target that is a path names no attribute, and changes nothing.""" + target with no space or tab, a path, is a pattern and names no + attribute, and changes nothing; one with a space or a tab is a pattern + and the attributes after it, as any line of the file is read, so it is + asked like a file (#906: measured with `config/attributes`, which left + the copy LF, and `*.yml eol=crlf` and `*.yml` TAB `eol=crlf`, which + wrote it CRLF).""" blobs = {path: oid for path, _, oid, _ in tree} return [(path, mode, blobs[path], entry) for path, mode, entry in _kept_in_root(plan.entries, tree) @@ -10128,17 +10207,36 @@ def _reencoded_attributes_findings(plan: Plan, source: Source, "large one, those checked out in the second git wrote its " "index)") -#: What git refused, and the way out, for each step a refusal can stop. +#: Where `execute` stops for each of git's refusals, measured with the +#: refusal's own fixtures and the check that refuses them patched out (#906). +#: A clone of the source meets the rule that refuses a checkout the first +#: time `execute` makes one: extracting a leg (`REFUSED extracting the spec +#: leg`, neither leg yet pushed). A `git add` is the split's last step, and +#: stops it there (`REFUSED committing the split`), unless a step before it +#: meets the same rule: the extraction, on a filter that fails, or the +#: mount, where a rule over every file reaches the `.gitmodules` it writes +#: (`REFUSED mounting the legs`, `failed to encode '.gitmodules'`). +STOPS_AT_A_CLONE = "at its first clone of the source" +STOPS_AT_THE_SPLIT = ( + "no later than committing the split, and sooner where a step before it " + "meets the same rule: extracting a leg, or mounting the legs, where the " + "rule reaches the `.gitmodules` it writes") + +#: What git refused, where `execute` would stop, and the way out, for each +#: step a refusal can stop. CHECKOUT_REFUSED = ( "to check the source's commit out as the split's clone does", + STOPS_AT_A_CLONE, "Change the rule or the setting git names, and re-run `plan`.") STORE_REFUSED = ( "to store again what the plan keeps in the assembly root, as the split's " "`git add -A` does", + STOPS_AT_THE_SPLIT, "Commit the file git names in the source as git stores it, or change the " "rule or the setting that refuses it, and re-run `plan`.") COPY_REFUSED = ( "to store the pinned shape copies the split writes", + STOPS_AT_THE_SPLIT, "Keep the rule off the copy git names with a line after it in the " "source's root `.gitattributes`, or narrow the rule, commit, and re-run " "`plan`.") @@ -10441,11 +10539,17 @@ def _copies_finding(copies: list[str], at_the_root: dict) -> str: "the rules, commit, and re-run `plan`.") -def _refused_finding(step: tuple[str, str], said: str) -> str: - what, way_out = step +def _refused_finding(step: tuple[str, str, str], said: str) -> str: + """`check`'s line for a step of the split that git refuses: what it + refuses, in git's words, and where `execute` stops over it (`step`'s + `STOPS_AT_*`), which is never past the step git refuses and is not always + that step (#906: it said "there", and stopped on the mount's + `.gitmodules`, before the split's `git add`, for `* working-tree-encoding` + with `.gitattributes` exempted).""" + what, stops, way_out = step return (f"FINDING {REENCODED_FILES}: this machine's git refuses {what} " - f"(git said {_spelled(said)}), so `execute` would stop there, " - f"after the leg repositories exist. {way_out}") + f"(git said {_spelled(said)}), so `execute` would stop after the " + f"leg repositories exist, {stops}. {way_out}") def _reencoded_files_findings(plan: Plan, source: Source, tree: list, @@ -13481,6 +13585,10 @@ def none_line(role: str) -> list[str]: f"was assigned to it. It is mounted and pinned like the other " f"leg and carries no history from here, there being none.)"] + # Beside the mounts `execute` made, the path the read-across follow-up + # leaves to them (#600's review, F1; #906). + across = contracts_from_code_leg(spec_path, code_path, paths_for["spec"], + follow_ups) lines = [ f"Adopt the three-repository shape: {names['spec']} and " f"{names['code']}", @@ -13491,7 +13599,7 @@ def none_line(role: str) -> list[str]: "moved file keeps the commits that made it. Nothing was deleted: what " "leaves this tree arrives in a leg at the same path, mounted back " f"here at {spec_path}/ and {code_path}/" - f"{contracts_from_code_leg(spec_path, code_path, paths_for['spec'])}.", + f"{across}.", "", f"MOVED TO THE SPEC LEG ({names['spec']} @ {leg_commits['spec'][:12]}):", ] diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index 34fe8298..37d9fd05 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -86,6 +86,20 @@ "code leg's tooling takes the path from the environment instead of " "assuming it sits beside it") +#: The same line for a source that keeps a root `Makefile` of its own (#906): +#: the shape's, with the block, is written beside it as `shape/Makefile`, so +#: the block waits there until the two are merged, as #634 words it for the +#: pinned `AGENTS-shape.md`, and the root's Makefile is the source's, which +#: the run adds nothing to. +OWN_MAKEFILE_FOLLOW_UP = ( + "the shape's Makefile wires `CONTRACTS_DIR` to the `contracts/` of the " + "spec leg, where the plan's `legs.spec_path` mounts it, and exports it, " + "so the code leg's tooling takes the path from the environment instead " + "of assuming it sits beside it; but the source keeps a Makefile of its " + "own at the root, so that block waits in `shape/Makefile` until the two " + "are merged, and the run adds none of it to the root's Makefile before " + "then") + #: A source with nothing for the plan to ask about, no Makefile of its own #: (so the shape's, with the block, is the root's), and one code file that #: names `contracts/`, which the rules send to the spec leg. @@ -436,8 +450,11 @@ def test_the_path_the_split_message_prints_leads_to_the_contracts( marker = "the spec leg's `contracts/` is `" printed = message.split(marker, 1)[1].split("`", 1)[0] assert printed == "../../legs/spec/contracts/" + # The plan holds the read-across line, so the spec leg's own path comes + # first (#906), and then the path to its `contracts/`. assert (f"mounted back here at {SPEC_PATH}/ and {CODE_PATH}/, so from " - f"the code leg {marker}{printed}`.") in message + f"the code leg the spec leg is `../../legs/spec/` and " + f"{marker}{printed}`.") in message code_leg = adopted_by_hand["clone"].joinpath(*CODE_PATH.split("/")) target = code_leg.joinpath(*printed.rstrip("/").split("/"), POLICY) assert target.resolve() == adopted_by_hand["clone"].joinpath( diff --git a/tests/test_adopt_leg_keys.py b/tests/test_adopt_leg_keys.py index 6d95a339..597bae05 100644 --- a/tests/test_adopt_leg_keys.py +++ b/tests/test_adopt_leg_keys.py @@ -263,10 +263,18 @@ def test_a_hand_edited_mount_leaves_no_plan_time_path_in_a_follow_up( EDITED_ROWS = [pytest.param(*row.values[1:], id=row.id) for row in ACROSS_ROWS] -def clause(across: str) -> str: - """What the two sentences that print the mounts end with.""" - return (", so from the code leg the spec leg's `contracts/` is " - f"`{across}`") +def clause(across: str, reads_across: bool = True) -> str: + """What the two sentences that print the mounts end with. Where the plan + holds the read-across follow-up, as every plan here does, it is the spec + leg's own path first and then the path to its `contracts/` (#906); a + split message rendered with no follow-ups has the `contracts/` path + alone.""" + contracts = f"the spec leg's `contracts/` is `{across}`" + if not reads_across: + return f", so from the code leg {contracts}" + spec_leg = across.removesuffix("contracts/") + return (f", so from the code leg the spec leg is `{spec_leg}` and " + f"{contracts}") def split_line(output: str) -> str: @@ -359,7 +367,8 @@ def test_the_split_message_prints_the_path_to_the_contracts_beside_the_mounts( which are the ones `execute` made (the pull request's body is this text).""" message = split_message_at(adopter, *edited) assert (f"mounted back here at {edited[0]}/ and {edited[1]}/" - f"{clause(across)}.") in message.splitlines()[2] + f"{clause(across, reads_across=False)}.") \ + in message.splitlines()[2] assert posixpath.normpath(posixpath.join(edited[1], across)) == \ f"{edited[0]}/contracts" diff --git a/tests/test_adopt_printed_sentences.py b/tests/test_adopt_printed_sentences.py new file mode 100644 index 00000000..e44c4d9f --- /dev/null +++ b/tests/test_adopt_printed_sentences.py @@ -0,0 +1,483 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Five sentences `adopt-project.py` prints say what the run does (#906). + +Lane 3's triage of #457 at the v1.1.1 release commit found five lines that +said what the run does not do: + +* L2-72. The Makefile follow-up said "the root Makefile wires `CONTRACTS_DIR`" + where the source keeps a root `Makefile` of its own. Then the shape's, with + the block, is written beside it as `shape/Makefile`, and `make + contracts-dir` at the root has no rule until the two are merged. It says + the block waits in `shape/Makefile` there, as #634 words it for the pinned + `AGENTS-shape.md`. +* L2-73. `check`'s `mount the legs at X/ and Y/` and the split message's + `mounted back here at X/ and Y/` ended with a ready path only where the + spec leg holds `contracts/`, so a code file that names `specs/` got the + key-form line and no path, where v1.0.1 printed `../spec/specs/` (v1.1.0 + already printed none: #600 took the path out of the follow-up). The spec + leg's own path from the code leg is printed wherever the plan holds a + read-across follow-up. +* L1-91. `plan-file-reencoded` said `execute` "would stop there, after the + leg repositories exist", where `there` was the step git refuses. For `* + working-tree-encoding=UTF-16LE` with `.gitattributes` exempted `execute` + stops earlier, mounting the legs, on the `.gitmodules` it writes. The + finding says where `execute` stops: measured below with the check that + refuses it patched out. +* L1-93. A docstring said "a target that is a path names no attribute", which + holds only for a link target with no space or tab. +* L1-75. Under `--local-remote-dir`, WHAT WILL HAPPEN printed `create + -spec and -code (private)`, though a bare repository has + no visibility. + +The tests that run `execute` need `git filter-repo` and 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 posixpath +import shutil +import subprocess +import sys +from argparse import Namespace +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from conftest import (ADOPT, REPO, clone_just_pushed, findings, git, + load_without_bytecode, make_source_repo, planned, + run_script, write_plan) +from test_adopt_contracts_dir import (EDITS, MAKEFILE_FOLLOW_UP, MOUNTS, + OWN_MAKEFILE, OWN_MAKEFILE_FOLLOW_UP, + PROJECT, READER, SPLIT_BRANCH, TREE, + adopt_with_legs_mounted_away, + needs_filter_repo) +from test_adopt_literal_names import follow_up +from test_adopt_nested_attributes import (GITHUB_ATTRIBUTES, KEEP_GITHUB, + NESTED, ROOT, ROOT_ATTRIBUTES, + SOURCE, index_link) +from test_adopt_reencoded_files import (CRLF, NOTES, REQUIRED, REQUIRED_ONLY, + SAFECRLF, TEXT_EOL_LF, configure, + raw_plan) + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import load_yaml # noqa: E402 + + +@pytest.fixture(scope="module") +def adopter(): + """`adopt-project.py` loaded as a module, for its helpers.""" + return load_without_bytecode(ADOPT, "adopt_printed_sentences") + + +def split_line(output: str) -> str: + """`check`'s one `ONE split commit ... mount the legs at X/ and Y/` + line.""" + (line,) = [line for line in output.splitlines() + if line.startswith(" ONE split commit")] + return line + + +def entries_of(*rows: tuple) -> list: + """Plan entries as `follow_ups_for` and `_predict_collisions` read them: + each `(path, leg)` is a row with those two attributes.""" + return [SimpleNamespace(path=path, leg=leg) for path, leg in rows] + + +# --- L2-72: the Makefile follow-up beside a root Makefile -------------------- + +NAMES = {"assembly": PROJECT, "spec": f"{PROJECT}-spec", + "code": f"{PROJECT}-code"} + + +def makefile_lines(follow_ups: list) -> list: + return [item for item in follow_ups if "Makefile wires `CONTRACTS_DIR`" + in item] + + +@pytest.mark.parametrize("makefile_leg, said", [ + pytest.param("root", OWN_MAKEFILE_FOLLOW_UP, id="the-root-keeps-it"), + pytest.param("code", MAKEFILE_FOLLOW_UP, id="it-goes-to-the-code-leg"), + pytest.param(None, MAKEFILE_FOLLOW_UP, id="the-source-has-none"), +]) +def test_the_makefile_line_follows_the_entry_that_moves_the_shapes_copy( + adopter, monkeypatch, makefile_leg, said): + """The line reads the plan's entries as `_predict_collisions` does + (`_root_keeps`): where the root keeps the source's `Makefile`, the + shape's goes beside it under `shape/` and the block waits there; where + it goes to a leg, or the source has none, the shape's is the root's.""" + # The read-across lines are the `git grep`'s, which this asks nothing of. + monkeypatch.setattr(adopter, "cross_leg_references", + lambda source, entries: []) + rows = [("contracts/", "spec"), ("src/", "code")] + if makefile_leg is not None: + rows.append(("Makefile", makefile_leg)) + entries = entries_of(*rows) + collisions = adopter._predict_collisions(entries) + ups = adopter.follow_ups_for(None, entries, NAMES, collisions) + assert makefile_lines(ups) == [said] + # One reading, two lines: the follow-up says the block waits in + # `shape/Makefile` exactly where the collision follow-up moves it there. + moved = [item for item in collisions + if item.startswith("merge shape/Makefile into Makefile")] + assert bool(moved) is (said == OWN_MAKEFILE_FOLLOW_UP) + + +def test_plan_says_where_a_kept_makefile_leaves_the_block(tmp_path): + """On `50178305`, a source that keeps its own root `Makefile` got `the + root Makefile wires `CONTRACTS_DIR` ...`, and its root wired nothing: the + block was in `shape/Makefile`.""" + source = make_source_repo(tmp_path / "Thing", + {**TREE, "Makefile": OWN_MAKEFILE}, edits=EDITS) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + follow_ups = [str(item) for item in load_yaml(plan)["follow_ups"]] + assert makefile_lines(follow_ups) == [OWN_MAKEFILE_FOLLOW_UP] + assert MAKEFILE_FOLLOW_UP not in follow_ups + assert any(item.startswith("merge shape/Makefile into Makefile") + for item in follow_ups) + checked = run_script(ADOPT, "check", "--plan", str(plan)) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert f" follow-up: {OWN_MAKEFILE_FOLLOW_UP}" in checked.stdout + + +@needs_filter_repo +@pytest.mark.skipif(shutil.which("make") is None or sys.platform == "win32", + reason="needs make, and a POSIX `$(CURDIR)` spelling") +def test_a_kept_makefile_wires_nothing_where_the_split_message_says_so( + tmp_path): + """The measurement the line rests on. After a real `execute` the split + commit's message, which is the pull request's body, holds the line that + says the block waits in `shape/Makefile`, the root's own Makefile is the + source's bytes, and `make contracts-dir` there has no rule while the + same target in `shape/Makefile` prints the spec leg's `contracts/`.""" + adopted = adopt_with_legs_mounted_away( + tmp_path, by_hand=False, tree={**TREE, "Makefile": OWN_MAKEFILE}) + message = " ".join(git("log", "-1", "--format=%B", SPLIT_BRANCH, + cwd=adopted["source"]).stdout.split()) + assert OWN_MAKEFILE_FOLLOW_UP in message + assert MAKEFILE_FOLLOW_UP not in message + assert adopted["makefile"].decode("utf-8").splitlines() == \ + OWN_MAKEFILE.splitlines() + clone = adopted["clone"] + at_the_root = subprocess.run( + ["make", "--no-print-directory", "-s", "contracts-dir"], + cwd=str(clone), capture_output=True, text=True, encoding="utf-8", + errors="replace", check=False) + assert at_the_root.returncode != 0, at_the_root.stdout + assert "No rule to make target 'contracts-dir'" in at_the_root.stderr + in_the_shapes = subprocess.run( + ["make", "--no-print-directory", "-s", "-f", "shape/Makefile", + "contracts-dir"], + cwd=str(clone), capture_output=True, text=True, encoding="utf-8", + errors="replace", check=False) + assert in_the_shapes.returncode == 0, in_the_shapes.stderr + assert Path(in_the_shapes.stdout.strip()).is_dir() + + +# --- L2-73: the spec leg's own path, beside the mounts ----------------------- + +#: A source whose one code file names `specs/`, which the rules send to the +#: spec leg: no `contracts/` anywhere, so the clause for it has nothing to +#: lead to, and the read-across line is the plan's only follow-up about paths. +SPECS_ONLY = { + "README.md": "# Thing\n", + "specs/001-feature/spec.md": "# The feature\n", + READER: "SPEC = 'specs/001-feature/spec.md'\n", + "tests/test_main.py": "def test_ok():\n assert True\n", +} +READ_ACROSS_SPECS = follow_up(READER, "specs/") +READ_ACROSS_CONTRACTS = follow_up(READER, "contracts/") +#: Plan lines that say nothing is to be read across. +OTHER_FOLLOW_UPS = [ + "merge shape/README.md into README.md: the source repository already " + "has that name, so the shape's copy was written beside it and NOTHING " + "was overwritten", + "CODEOWNERS in the assembly root now names paths that live in the legs; " + "re-scope it here and add one to each leg", + MAKEFILE_FOLLOW_UP, +] + + +@pytest.mark.parametrize("spec_path, code_path, across", MOUNTS) +def test_the_spec_legs_own_path_is_printed_where_the_plan_reads_across( + adopter, spec_path, code_path, across): + """A read-across follow-up, with no `contracts/` in the spec leg: the + sentence that prints the mounts ends with the spec leg's own path from + the code leg, which leads to the spec leg's mount for every layout.""" + said = adopter.contracts_from_code_leg( + spec_path, code_path, ["specs/", "docs/"], [READ_ACROSS_SPECS]) + assert said == f", so from the code leg the spec leg is `{across}/`" + assert posixpath.normpath(posixpath.join(code_path, across)) == spec_path + + +def test_both_clauses_are_said_where_the_spec_leg_holds_contracts_too( + adopter): + said = adopter.contracts_from_code_leg( + "legs/spec", "impl/code", ["contracts/", "specs/"], + [READ_ACROSS_CONTRACTS, READ_ACROSS_SPECS]) + assert said == (", so from the code leg the spec leg is " + "`../../legs/spec/` and the spec leg's `contracts/` is " + "`../../legs/spec/contracts/`") + + +@pytest.mark.parametrize("spec_leg_paths, follow_ups, said", [ + pytest.param(["specs/"], [], "", id="no-follow-up-no-contracts"), + pytest.param(["specs/"], OTHER_FOLLOW_UPS, "", + id="follow-ups-that-read-nothing-across"), + pytest.param(["contracts/"], OTHER_FOLLOW_UPS, + ", so from the code leg the spec leg's `contracts/` is " + "`../spec/contracts/`", id="contracts-alone-is-as-before"), + pytest.param(["contracts/"], [READ_ACROSS_SPECS], + ", so from the code leg the spec leg is `../spec/` and the " + "spec leg's `contracts/` is `../spec/contracts/`", + id="both"), +]) +def test_the_clauses_follow_the_plan_and_the_spec_leg(adopter, spec_leg_paths, + follow_ups, said): + assert adopter.contracts_from_code_leg( + "spec", "code", spec_leg_paths, follow_ups) == said + + +@pytest.mark.parametrize("spec_path, code_path", [ + pytest.param("spec", "spec", id="one-path-for-both"), + pytest.param("spec", "SPEC", id="two-paths-that-differ-in-case"), + pytest.param("../spec", "code", id="parent-segment"), + pytest.param("spec", None, id="a-mount-that-is-not-text"), +]) +def test_no_path_is_computed_from_values_that_are_not_two_mounts( + adopter, spec_path, code_path): + assert adopter.contracts_from_code_leg( + spec_path, code_path, ["specs/"], [READ_ACROSS_SPECS]) == "" + + +def test_every_read_across_line_the_plan_writes_is_found(adopter, tmp_path): + """`holds_a_read_across` reads the plan's own lines, so it has to find + the two lines `plan` writes for a file that names a spec directory: the + one for a file git found, in a real plan, and the one for a name git + could not search for, which tells the person to look by hand.""" + source = make_source_repo(tmp_path / "Thing", SPECS_ONLY, edits=()) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + follow_ups = [str(item) for item in load_yaml(plan)["follow_ups"]] + assert [item for item in follow_ups if item == READ_ACROSS_SPECS] + assert adopter.holds_a_read_across(follow_ups) + by_hand = adopter._search_by_hand("specs/", "git said no") + assert adopter.holds_a_read_across([by_hand]) + assert not adopter.holds_a_read_across(OTHER_FOLLOW_UPS) + + +@pytest.mark.parametrize("spec_path, code_path, across", [ + pytest.param(*row.values, id=row.id) + for row in MOUNTS if row.id in ("defaults", "siblings-in-legs", + "code-nested", "one-segment-shared")]) +def test_check_prints_the_spec_legs_path_beside_the_mounts( + tmp_path, spec_path, code_path, across): + """On `50178305`, a code file that names `specs/` got the read-across + line and a `mount the legs at X/ and Y/` that ended there. It ends with + the spec leg's path from the code leg now, for every layout.""" + source = make_source_repo(tmp_path / "Thing", SPECS_ONLY, edits=()) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT, + extra=("--spec-path", spec_path, + "--code-path", code_path)) + assert written.returncode == 0, written.stderr + written.stdout + checked = run_script(ADOPT, "check", "--plan", str(plan)) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert split_line(checked.stdout).endswith( + f"mount the legs at {spec_path}/ and {code_path}/, so from the code " + f"leg the spec leg is `{across}/`") + assert f" follow-up: {READ_ACROSS_SPECS}" in checked.stdout.splitlines() + + +def split_message_of(adopter, follow_ups: list) -> str: + return adopter._split_message( + NAMES, {"spec": ["specs/"], "code": ["src/"], "drop": [], + "root": ["README.md"]}, + {"spec": "a" * 40, "code": "b" * 40}, "spec", "code", follow_ups, []) + + +def test_the_split_message_prints_it_where_the_plan_reads_across(adopter): + said = split_message_of(adopter, [READ_ACROSS_SPECS]).splitlines()[2] + assert ("mounted back here at spec/ and code/, so from the code leg the " + "spec leg is `../spec/`.") in said + # With no read-across line, the sentence is the mounts alone, as before. + bare = split_message_of(adopter, OTHER_FOLLOW_UPS).splitlines()[2] + assert "mounted back here at spec/ and code/." in bare + assert "`../spec/`" not in bare + + +@needs_filter_repo +def test_the_path_the_split_message_prints_leads_to_the_spec_leg(tmp_path): + """End to end. The split commit's message, which is also the pull + request's body, names the spec leg's path from the code leg, and + followed from the code leg of a recursive clone it reaches the file the + code names.""" + source = make_source_repo(tmp_path / "Thing", SPECS_ONLY, edits=()) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(tmp_path / "remotes"), + "--work-dir", str(tmp_path / "work")) + assert result.returncode == 0, result.stderr + result.stdout + assert "adoption verified" in result.stdout, result.stdout + message = git("log", "-1", "--format=%B", SPLIT_BRANCH, cwd=source).stdout + assert ("mounted back here at spec/ and code/, so from the code leg the " + "spec leg is `../spec/`.") in message + clone = tmp_path / PROJECT + proc = clone_just_pushed(source, clone, branch=SPLIT_BRANCH) + assert proc.returncode == 0, proc.stderr + reached = (clone / "code").joinpath("..", "spec", "specs", "001-feature", + "spec.md") + assert reached.resolve().is_file() + + +# --- L1-91: where `execute` stops -------------------------------------------- + +#: What `check` says of where `execute` stops for a `git add` that is +#: refused: the last step, or one before it that meets the same rule first. +STOPS_AT_THE_SPLIT = ( + "no later than committing the split, and sooner where a step before it " + "meets the same rule: extracting a leg, or mounting the legs, where the " + "rule reaches the `.gitmodules` it writes") +STOPS_AT_A_CLONE = "at its first clone of the source" +#: `* working-tree-encoding=UTF-16LE`, with the one file that holds the rules +#: exempted: the case #906 names, which stops mounting the legs. +ENCODED_EXCEPT_RULES = (b"* working-tree-encoding=UTF-16LE\n" + b".gitattributes -working-tree-encoding\n") + +#: `(the source's files, plan answers, git configuration, the stop `check` +#: names, the step `execute` REFUSES at when the check is patched out)`. +STOPS = [ + pytest.param({ROOT_ATTRIBUTES: TEXT_EOL_LF, NOTES: CRLF}, {NOTES: ROOT}, + SAFECRLF, STOPS_AT_THE_SPLIT, + "REFUSED committing the split.", id="the-common-case"), + pytest.param({ROOT_ATTRIBUTES: ENCODED_EXCEPT_RULES}, {}, (), + STOPS_AT_THE_SPLIT, "REFUSED mounting the legs.", + id="encoding-over-every-file-but-the-rules"), + pytest.param({ROOT_ATTRIBUTES: f"*.txt filter={REQUIRED}\n".encode(), + NOTES: b"abc\n"}, {NOTES: ROOT}, + (*REQUIRED_ONLY, (f"filter.{REQUIRED}.smudge", "false")), + STOPS_AT_A_CLONE, "REFUSED extracting the spec leg.", + id="a-checkout-git-refuses"), +] + + +@pytest.mark.parametrize(("files", "answers", "config", "stops", "step"), + STOPS) +def test_the_finding_says_where_execute_would_stop(monkeypatch, tmp_path, + files, answers, config, + stops, step): + plan = raw_plan(tmp_path, files, answers) + configure(monkeypatch, config) + ((code, detail),) = findings(plan) + assert code == "plan-file-reencoded" + assert ("so `execute` would stop after the leg repositories exist, " + f"{stops}. ") in detail + assert "would stop there" not in detail + + +@needs_filter_repo +@pytest.mark.parametrize(("files", "answers", "config", "stops", "step"), + STOPS) +def test_execute_stops_where_the_finding_says(adopter, monkeypatch, capsys, + tmp_path, files, answers, + config, stops, step): + """The measurement. With `check`'s refusal patched out, `execute` runs + on and stops at the step the finding names, after both leg repositories + exist: committing the split for a `git add` that is refused, mounting + the legs on the `.gitmodules` it writes where a rule reaches every file + but the rules', and the first clone of the source, extracting the spec + leg, for a checkout git refuses. The finding named it as `there` before, + and `there` was the split's `git add` in all three.""" + plan = raw_plan(tmp_path, files, answers) + configure(monkeypatch, config) + monkeypatch.setattr(adopter, "_refuse_what_check_finds", + lambda *args, **kwargs: None) + args = Namespace(plan=str(plan), source=None, + local_remote_dir=tmp_path / "remotes", + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work") + assert adopter.cmd_execute(args) == 2 + said = capsys.readouterr().err + assert step in said, said + if "mounting the legs" in step: + assert "failed to encode '.gitmodules'" in said, said + assert (tmp_path / "remotes" / "Northwind-spec.git").is_dir() + assert (tmp_path / "remotes" / "Northwind-code.git").is_dir() + # What the finding names covers where it stopped. + named = {"committing the split": "committing the split", + "mounting the legs": "mounting the legs", + "extracting the spec leg": "first clone of the source"} + (where,) = [word for word in named if word in step] + assert named[where] in stops + + +# --- L1-93: what a link target names ----------------------------------------- + +@pytest.mark.parametrize(("target", "found"), [ + pytest.param("config/attributes", False, id="a-path-with-no-space"), + pytest.param("docs/read me.md", False, + id="a-space-and-an-attribute-no-line-ending-reads"), + pytest.param("*.yml eol=crlf", True, id="a-pattern-and-an-attribute"), + pytest.param("*.yml\teol=crlf", True, id="a-tab-between-them"), +]) +def test_a_link_target_names_an_attribute_only_with_a_space_or_a_tab( + tmp_path, target, found): + """Measured with git 2.43: a kept `.github/.gitattributes` linking to + `config/attributes` left `.github/workflows/validate.yml` LF, and one + linking to `*.yml eol=crlf`, or to `*.yml` and `eol=crlf` with a tab + between them, wrote it CRLF: git reads the target as one line of rules. + A target with a space that names no line-ending attribute changes + nothing either.""" + source = make_source_repo(tmp_path / "Thing", tree=dict(SOURCE), edits=()) + index_link(source, GITHUB_ATTRIBUTES, target) + got = findings(planned(source, tmp_path, KEEP_GITHUB)) + assert [code for code, _ in got] == ([NESTED] if found else []), got + + +def test_the_docstring_says_it_of_a_target_with_no_space_only(adopter): + said = " ".join(adopter._kept_nested_attributes.__doc__.split()) + assert "A target that is a path names no attribute" not in said + assert ("A target with no space or tab, a path, is a pattern and names " + "no attribute, and changes nothing; one with a space or a tab " + "is a pattern and the attributes after it") in said + + +# --- L1-75: no visibility for a bare repository ------------------------------ + +def what_will_happen_creates(output: str) -> list[str]: + return [line for line in output.splitlines() + if line.startswith(" create ")] + + +@pytest.mark.parametrize("visibility", ["private", "public", "internal"]) +def test_a_local_remote_dir_prints_no_visibility_for_a_bare_repository( + tmp_path, visibility): + """On `50178305`, `check --local-remote-dir` printed `create + Northwind-spec and Northwind-code (private)`: `execute` makes both with + `git init --bare`, which has no visibility. Against GitHub the plan's + visibility is still the one the legs are created at.""" + source = make_source_repo(tmp_path / "Thing", TREE, edits=EDITS) + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + text = plan.read_text(encoding="utf-8") + assert "visibility: private\n" in text, text + with open(plan, "w", encoding="utf-8", newline="\n") as handle: + handle.write(text.replace("visibility: private\n", + f"visibility: {visibility}\n")) + local = run_script(ADOPT, "check", "--plan", str(plan), + "--local-remote-dir", str(tmp_path / "remotes")) + assert local.returncode == 0, local.stderr + local.stdout + assert what_will_happen_creates(local.stdout) == [ + f" create {PROJECT}-spec and {PROJECT}-code"] + assert f"({visibility})" not in local.stdout + github = run_script(ADOPT, "check", "--plan", str(plan)) + assert github.returncode == 0, github.stderr + github.stdout + assert what_will_happen_creates(github.stdout) == [ + f" create {PROJECT}-spec and {PROJECT}-code ({visibility})"] diff --git a/tests/test_adopt_reencoded_files.py b/tests/test_adopt_reencoded_files.py index 98985e4e..302d8c68 100644 --- a/tests/test_adopt_reencoded_files.py +++ b/tests/test_adopt_reencoded_files.py @@ -339,9 +339,12 @@ def test_a_refusal_says_where_execute_stops_and_the_way_out(monkeypatch, "this machine's git refuses to store again what the plan keeps in " "the assembly root, as the split's `git add -A` does (git said " "`fatal: CRLF would be replaced by LF in notes.txt`), so `execute` " - "would stop there, after the leg repositories exist. Commit the file " - "git names in the source as git stores it, or change the rule or the " - "setting that refuses it, and re-run `plan`.")] + "would stop after the leg repositories exist, no later than " + "committing the split, and sooner where a step before it meets the " + "same rule: extracting a leg, or mounting the legs, where the rule " + "reaches the `.gitmodules` it writes. Commit the file git names in " + "the source as git stores it, or change the rule or the setting that " + "refuses it, and re-run `plan`.")] def test_the_finding_names_ten_paths_and_counts_the_rest(tmp_path): From 369f27818e02cade12bc60b1f31eb9ed9f9c2b4c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 18:49:54 +0000 Subject: [PATCH 2/2] adopt-project.py, shape_materialize.py: `execute` is said to stop at seeding a leg too, a link target names an attribute with a carriage return as with a space or tab, and a root `.gitattributes` link's follow-up says no rule reaches the pinned copies only for a link to a path (#906) r906's review of #964 found three of the sentences the first commit wrote or left false in cases it measured. Each is fixed here, with the measurement under a test. - `STOPS_AT_THE_SPLIT` names seeding: "extracting or seeding a leg, or mounting the legs". With the code leg seeded (`src/` and `.github/` answered root) and `core.autocrlf=true` with `core.safecrlf=true`, `execute` stops at `REFUSED seeding the code leg`: `_seed_leg` runs its own `git add -A`, which meets the rule before the split does. The STOPS row `a-seeded-leg-meets-the-same-config` holds it, and its finding test fails against the first commit's `adopt-project.py`. - `_kept_nested_attributes`'s docstring says a link target with no space, tab or carriage return names no attribute, and one with any of them is read as rules, since git separates a line's fields on all three. Measured with git 2.43: `*.yml` CR `eol=crlf` wrote `validate.yml` CRLF, as a space or a tab does. `test_a_link_target_names_an_attribute_only_with_a_separator` has the row `a-carriage-return-between-them`, and the docstring test follows the words. - `shape_materialize.py`'s `_attributes_link_follow_up` printed "a path names no attribute", which holds only for a link to a path, and so did `_append_eol_block`'s docstring and the comment in `adopt-project.py` above the nested-file finding. Measured in clones with git 2.43: a root `.gitattributes` link to `config/attributes` or `docs/read me.md` left a committed-LF `notes.txt` LF, and one to `* eol=crlf`, with a space, a tab or a carriage return between the fields, wrote it CRLF (`test_a_root_link_is_read_as_rules_only_with_a_separator`). The follow-up now says the link's text is read as one line of rules, which names an attribute only where it holds a space, a tab or a carriage return, so that, for a link to a path, no line-ending rule reaches the pinned copies; `tests/test_adopt_attributes_eol_block.py` asserts it. - The Makefile follow-up no longer ends "before then", which suggested the run acts when the two Makefiles are merged. - `test_a_kept_makefile_wires_nothing_where_the_split_message_says_so` asserts that `make -f shape/Makefile contracts-dir` prints the clone's own `legs/spec/contracts`, not any directory. - The `STOPS_AT_*` comment no longer says "neither leg yet pushed", which r906 found false where the spec leg is seeded, or that `git add` is the split's last step: the push of the split branch follows. Not changed: a kept lower-case `makefile` or `GNUmakefile`, and `README.md:810`, whose section is bound to the handbook. Both are follow-ups. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 40 +++++----- scripts/shape_materialize.py | 21 +++--- tests/test_adopt_attributes_eol_block.py | 8 +- tests/test_adopt_contracts_dir.py | 3 +- tests/test_adopt_printed_sentences.py | 94 +++++++++++++++++------- tests/test_adopt_reencoded_files.py | 8 +- 6 files changed, 111 insertions(+), 63 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index efcc51e0..09658be5 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -3411,7 +3411,7 @@ def follow_ups_for(source: "Source", entries: list["Entry"], names: dict, "; but the source keeps a Makefile of its own at the root, so " f"that block waits in `{COLLISION_DIR}/{SHAPE_MAKEFILE}` until " "the two are merged, and the run adds none of it to the root's " - "Makefile before then") + "Makefile") out.append( f"{whose} Makefile wires `CONTRACTS_DIR` to the `contracts/` of " "the spec leg, where the plan's `legs.spec_path` mounts it, and " @@ -9567,11 +9567,13 @@ def _refuse_symlinks_checked_out_as_files(plan: Plan, source: Source, # once those lines are in is named again by the next `check`, under the # file that then decides it. A deeper kept file that restores LF leaves # nothing to find, and one that alone keeps the copy LF is not named. A root -# `.gitattributes` that is a symlink, which gives no rule (git follows no such -# link, and a checkout reads only the link's own text, a path, which names -# no attribute), is #471's follow-up, not this finding: its copies are asked -# against the block that follow-up asks a regular file to hold, so a kept -# nested file is still found, and the finding names that block's rule. +# `.gitattributes` that is a symlink to a path, which gives no rule (git +# follows no such link, and a checkout reads only the link's own text, the +# path it leads to, which names no attribute unless it holds a space, a tab +# or a carriage return, #906), is #471's follow-up, not this finding: its +# copies are asked against the block that follow-up asks a regular file to +# hold, so a kept nested file is still found, and the finding names that +# block's rule. # Asked as a disk that keeps case does and again as one that ignores it, as # #262's findings are: there `core.ignorecase` folds git's patterns, and a # kept `.GitHub/.gitattributes` is read for `.github/`. A finding only the @@ -9746,12 +9748,12 @@ def _kept_nested_attributes(plan: Plan, tree: list) -> list[tuple]: blob is its target, which git then reads as the file's rules: measured with git 2.43, a kept `.github/.gitattributes` linking to `*.yml eol=crlf` checked `.github/workflows/validate.yml` out with CRLF. A - target with no space or tab, a path, is a pattern and names no - attribute, and changes nothing; one with a space or a tab is a pattern - and the attributes after it, as any line of the file is read, so it is - asked like a file (#906: measured with `config/attributes`, which left - the copy LF, and `*.yml eol=crlf` and `*.yml` TAB `eol=crlf`, which - wrote it CRLF).""" + target with no space, tab or carriage return, a path, is a pattern and + names no attribute, and changes nothing; one with any of them is a + pattern and the attributes after it, as any line of the file is read, so + every link is asked like a file (#906: measured with `config/attributes`, + which left the copy LF, and `*.yml eol=crlf`, `*.yml` TAB `eol=crlf` and + `*.yml` CR `eol=crlf`, which wrote it CRLF).""" blobs = {path: oid for path, _, oid, _ in tree} return [(path, mode, blobs[path], entry) for path, mode, entry in _kept_in_root(plan.entries, tree) @@ -10211,16 +10213,18 @@ def _reencoded_attributes_findings(plan: Plan, source: Source, #: refusal's own fixtures and the check that refuses them patched out (#906). #: A clone of the source meets the rule that refuses a checkout the first #: time `execute` makes one: extracting a leg (`REFUSED extracting the spec -#: leg`, neither leg yet pushed). A `git add` is the split's last step, and -#: stops it there (`REFUSED committing the split`), unless a step before it -#: meets the same rule: the extraction, on a filter that fails, or the -#: mount, where a rule over every file reaches the `.gitmodules` it writes +#: leg`, where both legs are extracted, before either is pushed). The split's +#: `git add -A` stops it at `REFUSED committing the split`, a step the push +#: of the split branch follows, unless a step before it meets the same rule: +#: the extraction, on a filter that fails; the seeding of a leg, which runs +#: a `git add -A` of its own (`REFUSED seeding the code leg`); or the mount, +#: where a rule over every file reaches the `.gitmodules` it writes #: (`REFUSED mounting the legs`, `failed to encode '.gitmodules'`). STOPS_AT_A_CLONE = "at its first clone of the source" STOPS_AT_THE_SPLIT = ( "no later than committing the split, and sooner where a step before it " - "meets the same rule: extracting a leg, or mounting the legs, where the " - "rule reaches the `.gitmodules` it writes") + "meets the same rule: extracting or seeding a leg, or mounting the legs, " + "where the rule reaches the `.gitmodules` it writes") #: What git refused, where `execute` would stop, and the way out, for each #: step a refusal can stop. diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 922f791a..4b623731 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -867,10 +867,11 @@ def _append_eol_block(target: Path, result: "Materialized", A SYMLINK IS NOT WRITTEN THROUGH: what it leads to may be another file of the source's, or nothing in this repository, and git follows no `.gitattributes` that is a link anyway: a checkout reads only the link's - own text, the path it leads to, which names no attribute (#654). It is - left as it is, and the follow-up says so and lists the lines a regular - file would need. A link is one on the disk OR in git's index - (`_tracked_as_link`).""" + own text, the path it leads to, as one line of rules, which names an + attribute only where that text holds a space, a tab or a carriage + return (#654, #906). It is left as it is, and the follow-up says so and + lists the lines a regular file would need. A link is one on the disk OR + in git's index (`_tracked_as_link`).""" if (ATTRIBUTES, f"{collision_dir}/{ATTRIBUTES}") not in result.collisions: return path = target / ATTRIBUTES @@ -987,8 +988,9 @@ def collision_follow_up(intended: str, actual: str, def _attributes_link_follow_up(intended: str, actual: str, beside: dict | None) -> str: """The follow-up for a source `.gitattributes` that is a symlink: no - block was appended, nothing git reads gives the root's pinned copies a - line ending, and these are the lines a regular file needs. + block was appended, for a link to a path nothing git reads gives the + root's pinned copies a line ending, and these are the lines a regular + file needs. The copies are the pinned ones that kept their own names, in the pin's order: every one `beside` does not send under the collision directory, @@ -1001,9 +1003,10 @@ def _attributes_link_follow_up(intended: str, actual: str, f"never writes through a link) and wrote the shape's copy beside it " f"as {actual}, which governs only its own directory. Git follows no " f"{intended} that is a symlink: a checkout reads only the link's own " - "text, the path it leads to, and a path names no attribute, so no " - "line-ending rule reaches the pinned copies outside it: a Windows " - "clone writes them with CRLF and " + "text, the path it leads to, as one line of rules, which names an " + "attribute only where that text holds a space, a tab or a carriage " + "return, so, for a link to a path, no line-ending rule reaches the " + "pinned copies outside it: a Windows clone writes them with CRLF and " "reports shape-copy-drift on each one. To give every clone one, " f"replace the link with a regular {intended} holding the lines of " "the file it leads to that should still apply, then these, one per " diff --git a/tests/test_adopt_attributes_eol_block.py b/tests/test_adopt_attributes_eol_block.py index be7d9797..f6ac0b32 100644 --- a/tests/test_adopt_attributes_eol_block.py +++ b/tests/test_adopt_attributes_eol_block.py @@ -395,9 +395,11 @@ def test_the_follow_up_for_a_linked_attributes_file_lists_the_lines_it_needs( assert said.startswith(f"{ATTRIBUTES} is a symlink, so the split " "appended nothing to it"), said assert (f"Git follows no {ATTRIBUTES} that is a symlink: a checkout " - "reads only the link's own text, the path it leads to, and a " - "path names no attribute, so no line-ending rule reaches the " - "pinned copies outside it") in said + "reads only the link's own text, the path it leads to, as one " + "line of rules, which names an attribute only where that text " + "holds a space, a tab or a carriage return, so, for a link to a " + "path, no line-ending rule reaches the pinned copies outside " + "it") in said assert f"replace the link with a regular {ATTRIBUTES}" in said assert ", ".join(f"`{line}`" for line in needed) in said assert needed[0] == EOL_BLOCK_HEAD diff --git a/tests/test_adopt_contracts_dir.py b/tests/test_adopt_contracts_dir.py index 37d9fd05..a8fc64f3 100644 --- a/tests/test_adopt_contracts_dir.py +++ b/tests/test_adopt_contracts_dir.py @@ -97,8 +97,7 @@ "so the code leg's tooling takes the path from the environment instead " "of assuming it sits beside it; but the source keeps a Makefile of its " "own at the root, so that block waits in `shape/Makefile` until the two " - "are merged, and the run adds none of it to the root's Makefile before " - "then") + "are merged, and the run adds none of it to the root's Makefile") #: A source with nothing for the plan to ask about, no Makefile of its own #: (so the shape's, with the block, is the root's), and one code file that diff --git a/tests/test_adopt_printed_sentences.py b/tests/test_adopt_printed_sentences.py index e44c4d9f..6736bd75 100644 --- a/tests/test_adopt_printed_sentences.py +++ b/tests/test_adopt_printed_sentences.py @@ -20,11 +20,12 @@ * L1-91. `plan-file-reencoded` said `execute` "would stop there, after the leg repositories exist", where `there` was the step git refuses. For `* working-tree-encoding=UTF-16LE` with `.gitattributes` exempted `execute` - stops earlier, mounting the legs, on the `.gitmodules` it writes. The - finding says where `execute` stops: measured below with the check that - refuses it patched out. -* L1-93. A docstring said "a target that is a path names no attribute", which - holds only for a link target with no space or tab. + stops earlier, mounting the legs, on the `.gitmodules` it writes, and for a + seeded leg, seeding it. The finding says where `execute` stops: measured + below with the check that refuses it patched out. +* L1-93. A docstring, and `shape_materialize.py`'s printed follow-up, said + "a path names no attribute", which holds only for a link target with no + space, tab or carriage return: git reads the target as one line of rules. * L1-75. Under `--local-remote-dir`, WHAT WILL HAPPEN printed `create -spec and -code (private)`, though a bare repository has no visibility. @@ -46,9 +47,9 @@ import pytest -from conftest import (ADOPT, REPO, clone_just_pushed, findings, git, - load_without_bytecode, make_source_repo, planned, - run_script, write_plan) +from conftest import (ADOPT, REPO, clone_just_pushed, cloned_just_pushed, + findings, git, load_without_bytecode, make_source_repo, + planned, run_script, write_plan) from test_adopt_contracts_dir import (EDITS, MAKEFILE_FOLLOW_UP, MOUNTS, OWN_MAKEFILE, OWN_MAKEFILE_FOLLOW_UP, PROJECT, READER, SPLIT_BRANCH, TREE, @@ -175,7 +176,10 @@ def test_a_kept_makefile_wires_nothing_where_the_split_message_says_so( cwd=str(clone), capture_output=True, text=True, encoding="utf-8", errors="replace", check=False) assert in_the_shapes.returncode == 0, in_the_shapes.stderr - assert Path(in_the_shapes.stdout.strip()).is_dir() + contracts = Path(in_the_shapes.stdout.strip()) + assert contracts.is_dir() + assert contracts.resolve() == (clone / "legs" / "spec" + / "contracts").resolve() # --- L2-73: the spec leg's own path, beside the mounts ----------------------- @@ -340,11 +344,12 @@ def test_the_path_the_split_message_prints_leads_to_the_spec_leg(tmp_path): # --- L1-91: where `execute` stops -------------------------------------------- #: What `check` says of where `execute` stops for a `git add` that is -#: refused: the last step, or one before it that meets the same rule first. +#: refused: committing the split, which the push of its branch follows, or a +#: step before it that meets the same rule first. STOPS_AT_THE_SPLIT = ( "no later than committing the split, and sooner where a step before it " - "meets the same rule: extracting a leg, or mounting the legs, where the " - "rule reaches the `.gitmodules` it writes") + "meets the same rule: extracting or seeding a leg, or mounting the legs, " + "where the rule reaches the `.gitmodules` it writes") STOPS_AT_A_CLONE = "at its first clone of the source" #: `* working-tree-encoding=UTF-16LE`, with the one file that holds the rules #: exempted: the case #906 names, which stops mounting the legs. @@ -365,6 +370,10 @@ def test_the_path_the_split_message_prints_leads_to_the_spec_leg(tmp_path): (*REQUIRED_ONLY, (f"filter.{REQUIRED}.smudge", "false")), STOPS_AT_A_CLONE, "REFUSED extracting the spec leg.", id="a-checkout-git-refuses"), + pytest.param({}, {"src/": ROOT, ".github/": ROOT}, + (("core.autocrlf", "true"), ("core.safecrlf", "true")), + STOPS_AT_THE_SPLIT, "REFUSED seeding the code leg.", + id="a-seeded-leg-meets-the-same-config"), ] @@ -392,16 +401,18 @@ def test_execute_stops_where_the_finding_says(adopter, monkeypatch, capsys, on and stops at the step the finding names, after both leg repositories exist: committing the split for a `git add` that is refused, mounting the legs on the `.gitmodules` it writes where a rule reaches every file - but the rules', and the first clone of the source, extracting the spec - leg, for a checkout git refuses. The finding named it as `there` before, - and `there` was the split's `git add` in all three.""" + but the rules', seeding a leg, whose own `git add -A` meets the same + config before the split does, and the first clone of the source, + extracting the spec leg, for a checkout git refuses. The finding named it + as `there` before, and `there` was the split's `git add` in all four.""" plan = raw_plan(tmp_path, files, answers) configure(monkeypatch, config) monkeypatch.setattr(adopter, "_refuse_what_check_finds", lambda *args, **kwargs: None) args = Namespace(plan=str(plan), source=None, local_remote_dir=tmp_path / "remotes", - allow_empty_leg=[], yes=True, work_dir=tmp_path / "work") + allow_empty_leg=["code"] if "seeding" in step else [], + yes=True, work_dir=tmp_path / "work") assert adopter.cmd_execute(args) == 2 said = capsys.readouterr().err assert step in said, said @@ -412,7 +423,8 @@ def test_execute_stops_where_the_finding_says(adopter, monkeypatch, capsys, # What the finding names covers where it stopped. named = {"committing the split": "committing the split", "mounting the legs": "mounting the legs", - "extracting the spec leg": "first clone of the source"} + "extracting the spec leg": "first clone of the source", + "seeding the code leg": "seeding a leg"} (where,) = [word for word in named if word in step] assert named[where] in stops @@ -420,32 +432,60 @@ def test_execute_stops_where_the_finding_says(adopter, monkeypatch, capsys, # --- L1-93: what a link target names ----------------------------------------- @pytest.mark.parametrize(("target", "found"), [ - pytest.param("config/attributes", False, id="a-path-with-no-space"), + pytest.param("config/attributes", False, id="a-path-with-no-separator"), pytest.param("docs/read me.md", False, id="a-space-and-an-attribute-no-line-ending-reads"), pytest.param("*.yml eol=crlf", True, id="a-pattern-and-an-attribute"), pytest.param("*.yml\teol=crlf", True, id="a-tab-between-them"), + pytest.param("*.yml\reol=crlf", True, + id="a-carriage-return-between-them"), ]) -def test_a_link_target_names_an_attribute_only_with_a_space_or_a_tab( +def test_a_link_target_names_an_attribute_only_with_a_separator( tmp_path, target, found): """Measured with git 2.43: a kept `.github/.gitattributes` linking to `config/attributes` left `.github/workflows/validate.yml` LF, and one - linking to `*.yml eol=crlf`, or to `*.yml` and `eol=crlf` with a tab - between them, wrote it CRLF: git reads the target as one line of rules. - A target with a space that names no line-ending attribute changes - nothing either.""" + linking to `*.yml eol=crlf`, or to `*.yml` and `eol=crlf` with a tab or a + carriage return between them, wrote it CRLF: git reads the target as one + line of rules, and a space, a tab and a carriage return all separate its + fields. A target with a space that names no line-ending attribute + changes nothing either.""" source = make_source_repo(tmp_path / "Thing", tree=dict(SOURCE), edits=()) index_link(source, GITHUB_ATTRIBUTES, target) got = findings(planned(source, tmp_path, KEEP_GITHUB)) assert [code for code, _ in got] == ([NESTED] if found else []), got -def test_the_docstring_says_it_of_a_target_with_no_space_only(adopter): +@pytest.mark.parametrize(("target", "crlf"), [ + pytest.param("config/attributes", False, id="a-path-with-no-separator"), + pytest.param("docs/read me.md", False, + id="a-space-and-an-attribute-no-line-ending-reads"), + pytest.param("* eol=crlf", True, id="a-space-between-them"), + pytest.param("*\teol=crlf", True, id="a-tab-between-them"), + pytest.param("*\reol=crlf", True, id="a-carriage-return-between-them"), +]) +def test_a_root_link_is_read_as_rules_only_with_a_separator(tmp_path, target, + crlf): + """The measurement `_attributes_link_follow_up`'s sentence rests on. + Measured with git 2.43: a clone of a source whose root `.gitattributes` + links to `config/attributes` writes a text file it commits LF with LF, + and one whose link leads to `* eol=crlf`, or to `*` and `eol=crlf` with a + tab or a carriage return between them, writes it CRLF. The printed + follow-up says no line-ending rule reaches the pinned copies "for a link + to a path" for that reason, and not for every link.""" + source = make_source_repo(tmp_path / "Thing", tree={NOTES: "one\ntwo\n"}, + edits=()) + index_link(source, ROOT_ATTRIBUTES, target) + clone = cloned_just_pushed(source, tmp_path / "clone", recurse=False) + written = (clone / NOTES).read_bytes() + assert written == (b"one\r\ntwo\r\n" if crlf else b"one\ntwo\n") + + +def test_the_docstring_says_it_of_a_target_with_no_separator_only(adopter): said = " ".join(adopter._kept_nested_attributes.__doc__.split()) assert "A target that is a path names no attribute" not in said - assert ("A target with no space or tab, a path, is a pattern and names " - "no attribute, and changes nothing; one with a space or a tab " - "is a pattern and the attributes after it") in said + assert ("A target with no space, tab or carriage return, a path, is a " + "pattern and names no attribute, and changes nothing; one with " + "any of them is a pattern and the attributes after it") in said # --- L1-75: no visibility for a bare repository ------------------------------ diff --git a/tests/test_adopt_reencoded_files.py b/tests/test_adopt_reencoded_files.py index 302d8c68..899a9aeb 100644 --- a/tests/test_adopt_reencoded_files.py +++ b/tests/test_adopt_reencoded_files.py @@ -341,10 +341,10 @@ def test_a_refusal_says_where_execute_stops_and_the_way_out(monkeypatch, "`fatal: CRLF would be replaced by LF in notes.txt`), so `execute` " "would stop after the leg repositories exist, no later than " "committing the split, and sooner where a step before it meets the " - "same rule: extracting a leg, or mounting the legs, where the rule " - "reaches the `.gitmodules` it writes. Commit the file git names in " - "the source as git stores it, or change the rule or the setting that " - "refuses it, and re-run `plan`.")] + "same rule: extracting or seeding a leg, or mounting the legs, where " + "the rule reaches the `.gitmodules` it writes. Commit the file git " + "names in the source as git stores it, or change the rule or the " + "setting that refuses it, and re-run `plan`.")] def test_the_finding_names_ten_paths_and_counts_the_rest(tmp_path):