Repository navigation
adopt-project.py: the --plan path in check and execute, and the --local-remote-dir path in execute's bare, leg and split lines, are named with _shown, as #1154 names --out and --work-dir, so a bidi control, an escape, a line feed or a byte that is not UTF-8 in either no longer reaches the terminal as it was typed (#1152) - #1177
brettheap wants to merge 1 commit into
Conversation
… `--local-remote-dir` path in `execute`'s `bare`, leg and `split` lines, are named with `_shown`, as #1154 names `--out` and `--work-dir`, so a bidi control, an escape, a line feed or a byte that is not UTF-8 in either no longer reaches the terminal as it was typed (#1152) ## What #1154 (#1115) names `--out` and `--work-dir` in `plan`'s and `check`'s report with `_shown`. W1115 found two groups of paths it leaves raw (#1152): the `--plan` path, and the `--local-remote-dir` path. A bidi control (U+202E), an ESC or a line break in either reached the terminal as it was typed, where the other plan lines name a path with `_shown`. This change spells each of the 21 sites with `_shown(str(...))`, so a path with a character that is not printable is named as `_spelled` names it, between backticks, and every other path prints byte for byte as it did. The `--plan` path, 16 sites in `check` and `execute`: - the five `N finding(s) in <plan>` lines: `_refuse_a_mistyped_plan`, `_refuse_what_check_finds`, the `plan-split-branch-taken` refusal, and `cmd_check`'s two branches; - the `<plan>: ...` refusals: `plan-wrong-kind` (the code's name for it; the issue calls it `plan-bad-kind`), `plan-unsupported-mode`, `plan-unreadable`, `plan-no-source`, the three `plan-bad-id` ones, `plan-not-the-source`, `adopt-topic-too-long`, and the `legs.assembly` and `org:` ones. The `--local-remote-dir` path, 5 sites in `execute`: - the `<bare> already exists` refusal; - the ` bare <bare>` line; - the two leg lines, `_extract_leg` and `_seed_leg`, which end `-> <url>`; - the `split <commit> on <branch> -> <url>` line, which names the source's directory under `--local-remote-dir`. The `NEXT:` clone command is not touched: it already quotes the url with `shlex.quote`, and a row pins that it still does. A path that is not UTF-8 now prints as `\udcff` between backticks, where it printed raw before. No refusal code is new, and none changes its name or its exit code. The changed lines that ran past 79 columns are rewrapped; the message text is unchanged. `repo_shape.load_yaml`'s own refusals still print a path raw, as `yaml-unreadable`. They live in another file and are a separate follow-up (see #1168). ## Test `tests/test_adopt_plan_and_remote_paths_spelled.py` is new, ASCII only and POSIX only, with every non-ASCII character written as an escape. It has 148 rows: - 135 plan rows: 14 refusal families (27 family and command pairs) by 5 path forms (U+202E, ESC, a line feed, the byte `\xff`, and a plain control); - 5 `--local-remote-dir` rows for the `bare` and leg lines (the code leg is extracted, the spec leg seeded); - 3 `split` rows. `execute` refuses a source path that holds U+202E, ESC, a line feed or `\xff` as `adopt-unwritable-value` before any leg exists, so these rows use U+200B in the parent and a name that ends in a space, and a plain control; - 5 in-process rows for `{bare} already exists`, which call `_create_leg_remote`. On main's `adopt-project.py`, 118 of the 148 fail, each for the raw path, and the 30 that pass are exactly the plain controls. On this commit all 148 pass. 23 mutants (each new spelling put back to raw, one at a time, and `_shown` swapped for `_spelled` at two sites) are each killed by the rows that name them. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR hardens terminal-facing diagnostics in Sequence diagram for safely displaying plan and remote pathssequenceDiagram
participant User
participant AdoptProject as adopt-project.py
participant Shown as _shown
participant Terminal
User->>AdoptProject: check or execute with --plan / --local-remote-dir
AdoptProject->>Shown: _shown(str(path))
Shown-->>AdoptProject: escaped printable representation
AdoptProject->>Terminal: print refusal or progress message
Terminal-->>User: path shown safely between backticks
Flow diagram for path rendering in adopt-project.pyflowchart LR
PlanPath[--plan path] --> PlanMessages[check and execute diagnostics]
RemotePath[--local-remote-dir path] --> RemoteMessages[bare and split progress/refusals]
PlanMessages --> Shown["_shown(str(path))"]
RemoteMessages --> Shown
Shown --> Terminal[Terminal output]
Printable[Printable path] --> Same[Byte-for-byte output]
Unsafe[Bidi, ESC, line feed, or non-UTF-8 byte] --> Escaped[Escaped path between backticks]
Shown --> Printable
Shown --> Unsafe
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
|
RV1177: independent review of #1177 at Independent adversarial review for lane 4. No MUST and no SHOULD. There are five NITs, below; two of them (N3, N4) are follow-up issues outside #1152's words. Non-ASCII characters are written as escapes here. Merge with current main
Completeness (#1152's two groups)
Probes: real runs of main's tool and the merge's tool on the same paths
The
|
| Shard | Result |
|---|---|
| g0 | 564 passed |
g1 (test_non_utf8_name_callers.py among them) |
568 passed |
| g2 | 567 passed |
| g3 | 568 passed |
g4 (test_stdout_encoding.py) |
568 passed, 1 skipped (its Windows-default row) |
g5 (test_adopt_plan_escapes.py, test_repo_hygiene.py) |
567 passed |
g6 (the new file, 148; test_path_guard_hygiene.py, 49) |
567 passed |
g7 (test_scaffold_leg_path_collisions.py) |
562 passed |
That is 4531 passed and 1 skipped, every shard exit 0. #1111's rule (test_path_guard_hygiene.py) passes on the merge with current main.
test_shape_doctor.py: 448 ids in 8 shards, run from a bash script with mapfile, every list non-empty, the lists together holding each collected id once (25 ids hold spaces), and no timeout under 600 s.
| Shard | Result |
|---|---|
| d0 | 56 passed |
| d1 | 56 passed |
| d2 | 55 passed, 1 skipped |
| d3 | 55 passed, 1 skipped |
| d4 | 55 passed, 1 skipped |
| d5 | 56 passed |
| d6 | 56 passed |
| d7 | 56 passed |
That is 445 passed and 3 skipped (the Windows-only rows), every shard exit 0, the same as #1154's doctor count. d3 to d7 first errored at setup because the lane coordinator had removed the shards' basetemp parent folder to free disk; rerun with the folder restored, they passed as above.
Mutants (mine; each on adopt-project.py in its own worktree, restored with git checkout, sha256 checked after each)
W1152b's 23 are each a site put back raw or swapped to _spelled. These are new, on the changed lines and their call sites, run against the new file (148 rows):
| Mutant | Result |
|---|---|
K0: check's last finding(s) in line raw again (sanity) |
killed: 24 failed |
K1: that line spelled by isprintable() alone (no #900 rules) |
survives: 148 passed |
K2: that line names plan.path.absolute() |
survives: 148 passed |
K4: the PLACEMENT plan's plan-wrong-kind names the plan raw |
survives: 148 passed |
K5: adopt-org-too-long names the plan raw (adopt-bad-org kept) |
survives: 148 passed |
| K6: the extracted leg line spells every non-ASCII url | survives: 148 passed |
K7: split names source.path, not urls['assembly'] |
survives: 171 passed (with test_adopt_github_exits.py) |
K8: bare spelled by isprintable() alone |
survives: EQUIVALENT (see below) |
K9: split spelled by isprintable() alone |
killed: 1 failed (the trailing-space row) |
K10: already exists through _line_shown |
killed: 5 failed |
| K12: the seeded leg line spells only a line break | killed: 3 failed |
K13: the mistyped check line names plan.path.name |
killed: 5 failed |
- K8 is equivalent. The bare path is absolute and ends in
.git, so adopt-project.py: a kept or dropped name reaches the split commit and the pull request body unspelled, and_spelled,_shownandplan's report print text that reads as another name, or raw #900's trailing-space and backtick rules can never fire there. - Every other survivor leaves behaviour the PR gets right, as measured above: a trailing-space
--planis spelled (K1), a relative--planprints as typed (K2), the placement-plan andadopt-org-too-longrefusals spell U+202E and ESC (K4, K5), and acaf\u00e9remote prints as typed (K6). K7 is GitHub mode, whereurls['assembly']ishttps://github.com/<org>/<name>.git;test_adopt_github_exits.py:452asks only that asplitline exists.
Weighed: why N1 and N5 stay NITs.
- None of the six live survivors is wrong output; each was measured right on this head.
- K4 and K5 reach their refusal through a value (
detail,named) that rows already pin on its other branch. - K1, K2 and K6 are about printable paths, not a terminal control reaching the terminal.
- K7 predates adopt-project.py: the
--planpath incheckandexecuteand the--local-remote-dirpath inexecuteare printed raw, so a bidi control in either reaches the terminal, where #1115 spells--outand--work-dirwith_shown#1152. - RV1115 graded the same class NIT for adopt-project.py:
plan written toandcloning <repository> (read only) intoname the caller's--outand--work-dirwith_shown, as the other lines of the report name a path, so a bidi control, a line feed or a tab in either no longer reaches the terminal as it was typed (#1115) #1154, with the follow-up tests/test_adopt_plan_paths_spelled.py: follow-up of PR #1154 (#1115) from its review: seven mutants of theplan written toandcloning ... intolines that none of its 18 rows kills (R1-R4, R6, R7, R12) #1167.
Merge-trees
git merge-tree --write-tree against all 18 open PRs that touch adopt-project.py, run both from this head and from the scratch merge: exit 0 every time. They are #1180 70f324859, #1178 fa56168ab, #1171 159815d75, #1170 24ee133aa, #1169 b8e63bcf0, #1154 8a7828332, #1153 ff33ba12a, #1143 7af5fabc3, #1141 7a2711d03, #1125 9549006fe, #1123 5c3908f8d, #1122 c01c4d73d, #1121 1c5173758, #1120 c55e55e33, #1119 bba168bdd, #1118 2307036ab, #1106 504a1dd65 and #1105 b28f40de0.
- New and moved heads. adopt-project.py:
checkandexecuterun withGIT_NO_LAZY_FETCH=1and refuse a partial-clone source that lacks an object its branches, tags or HEAD reach assource-partial-clone, so neither fetches into the source any more (#1145) #1180 and adopt-project.py:plan's table pads each name by the columns a terminal gives it, not the code points it holds, so a wide or combining name'slegcolumn lines up underlegwith every other row's (#1034) #1178 are newer than the body's list, and adopt-project.py:checknames each case twin at the shape's copy, above it and at the manifest, before a link there or after it, and at a pin's file whose exact spelling holds nothing, as_KeptTreekeeps every row, not the first (#1138) #1170's head moved from4df784ac4. - No new raw prints. No added line in any of them prints
plan.path,{bare},urls[...],finding(s) inoralready exists, apart from adopt-project.py:execute's NEXT printspython3 scripts/bootstrap.pywhere GNU make at the root reads a keptGNUmakefileormakefile, and its NOTE names that file and what it takes formake bootstrapto run there (#1047) #1121's tuple(branch, urls["assembly"]), which goes to_next_steps. - adopt-project.py and repo_shape.py: a name that holds a default-ignorable character, which a terminal draws as nothing, is spelled by its code point in
plan's lines and in theunsafe-valuerefusal, so a branchma<U+034F>inno longer prints asmain(#1128) #1171 widens_shown(default-ignorables), and these 21 sites inherit that.
Findings
- N1 (NIT), the body's sentence and a row.
- What is wrong. "Every other path prints byte for byte as it did" is not exact. A
--planthat ends in a space, or a relative--planthat begins and ends with a backtick, is printable and is now spelled (`.../plan.yaml `,`\x60plan.yaml\x60`). That is adopt-project.py: a kept or dropped name reaches the split commit and the pull request body unspelled, and_spelled,_shownandplan's report print text that reads as another name, or raw #900's rule, and the right output, but no row holds it, so K1 survives. - Fix: reword the sentence. Add a row whose
--planends in a space (plan.yaml), which must print spelled.where_the_plan_isputs each character mid-name (pl<c>an.yaml), so this one needs its own path.
- What is wrong. "Every other path prints byte for byte as it did" is not exact. A
- N2 (NIT), the behaviour-change wording.
- What is wrong. All 16
--plansites print to stderr, and Python gives stderrbackslashreplacealways. So on main a 0xff in--planalready printed as the six characters\udcff, with no backticks, under bothutf-8:surrogateescapeandutf-8:strict(read withod -c). Main wrote the raw byte only on the stdout lines:bare, the two legs andsplit. The new file's docstring also says such a byte "raisedUnicodeEncodeErroron a strict stdout", which has not held since adopt-project.pyplanraisesUnicodeEncodeErrorin its terminal report, after the plan file is written, for a printable name stdout cannot encode (日本.mdunder cp1252, as on a redirected Windows stdout), soplanexits 1 beforeplan written to#214'sescape_what_stdout_cannot_encode. - Fix: say that the
--planlines gain backticks around an escape they already wrote, and that the--local-remote-dirlines no longer write the raw byte; drop theUnicodeEncodeErrorclause.
- What is wrong. All 16
- N3 (NIT, follow-up issue), a refused command still names
--local-remote-dirraw.- What is wrong. A real
executewith--local-remote-dir <D>/re\u202emoteand a globalpre-receivehook that refuses the leg push spells thebarelines and the notice. But its REFUSED block prints the raw U+202E three times:CommandFailed.loudly'sgit push -q <D>/re\u202emote/Thing-spec.git HEAD:refs/heads/main, and git's ownTo <D>/re\u202emote/...anderror: failed to push some refs to '...'. - Why it is a follow-up.
CommandFailedisscripts/shape_materialize.py's, shared by every tool, and adopt-project.py: the--planpath incheckandexecuteand the--local-remote-dirpath inexecuteare printed raw, so a bidi control in either reaches the terminal, where #1115 spells--outand--work-dirwith_shown#1152 names only the lines this PR spells. - Fix: a new issue to spell
loudly's argv, its(in <cwd>)and its output as_first_saidspells gh's.
- What is wrong. A real
- N4 (NIT, follow-up),
NEXT:prints Cf characters raw.- What is wrong.
shlex.quoteneutralises no character. A source underso\u200burceorso\u200furcepassesadopt-unwritable-value; thesplitline spells it, and theNEXT:clone command one line below prints it raw inside single quotes. U+200F is RIGHT-TO-LEFT MARK, a bidi mark outside the nineBIDI_CONTROLS. The new file's split row pins the raw U+200B inNEXT:on purpose. - Why it is a follow-up. adopt-project.py: the
--planpath incheckandexecuteand the--local-remote-dirpath inexecuteare printed raw, so a bidi control in either reaches the terminal, where #1115 spells--outand--work-dirwith_shown#1152 itself callsNEXT:safe, and the line is a command to paste (_branch_deletion's rule). - Fix: a follow-up decision, perhaps beside adopt-project.py: follow-ups from RV1120's review of #1120 (#1035):
_shownlets 267 invisible characters through, and two cases the tests do not pin #1128 or validate-repository-naming.py, validate-manifest.py, validate-pins.py, repo_shape.py, shape_materialize.py and adopt-project.py: the helpers that spell a name for a line or for the plan teststr.isprintable()or userepr, so a combining mark, a variation selector or a Hangul filler in a name prints raw, andmaU+034Finreads asmain#1165: refuse such a source as the README's clone line does, or note underNEXT:that the path holds characters a terminal does not show.
- What is wrong.
- N5 (NIT), the sites no row holds (K2, K4, K5, K6, K7).
- Fix, in a follow-up or tests/test_adopt_plan_paths_spelled.py: follow-up of PR #1154 (#1115) from its review: seven mutants of the
plan written toandcloning ... intolines that none of its 18 rows kills (R1-R4, R6, R7, R12) #1167's: a relative--plan, which must print as typed; a PLACEMENT plan'splan-wrong-kind, and anorg:of 40 characters (adopt-org-too-long), each with U+202E in--plan; acaf\u00e9--local-remote-dir, which must print unchanged; and a GitHub-modesplitline naminghttps://github.com/<org>/<name>.git.
- Fix, in a follow-up or tests/test_adopt_plan_paths_spelled.py: follow-up of PR #1154 (#1115) from its review: seven mutants of the
Lane: openreposhape-4 (openRepoShape-4)
…` before `git filter-repo`, so under a Latin-1 locale a source under a directory named with the byte 0xE9 is extracted, where filter-repo died printing that url as U+FFFD, and every branch of the source is still rewritten as before (#1247) (#1251) **Head `abeac66d` is a fix round for RV1251's review of `825923c7`** (#1251 (comment)): one SHOULD and two NITs, set out under "Review round 1" below. The sections after that describe this head. `_extract_leg` (`adopt-project.py`) now unsets the leg clone's `remote.origin.url`, in the clone's own config file (`--file <work>/.git/config`), right after its `checkout -B` and before `git filter-repo` runs. filter-repo then prints no url, so a Latin-1 stdout has no U+FFFD to choke on, and it still turns every remote-tracking branch into a branch first, so the legs are the ones main makes. ## The defect, reproduced on main - **Where:** main `b1f45fa7`, git 2.43.0, Python 3.12.3, git-filter-repo 2.47.0 (`--version` prints `a40bce548d2c`). - **Launcher:** pip's entry point, `#!.../frvenv/bin/python3`, WITHOUT `-E`. So any `PYTHON*` variable would reach filter-repo, and the reproduction sets none: no `PYTHONIOENCODING`, no `PYTHONUTF8`. - **Locale:** Latin-1, built as #1214 and #1106 build it (`localedef -c`, `LOCPATH`, `LC_ALL=en_US.ISO-8859-1`). - **Source:** `make_source_repo` under a directory `caf` plus U+00E9, made as the byte 0xE9. The probe runs `plan`, `check` and `execute --local-remote-dir` as the existing row does, but without its `PYTHONIOENCODING=utf-8:strict`. - **Result:** `plan` exit 0, `check` exit 0 (`plan ok`), and `execute` exit 2: ```text creating the leg repositories bare .../remotes/Northwind-spec.git bare .../remotes/Northwind-code.git REFUSED extracting the spec leg. THE EXACT COMMAND WAS: git filter-repo --paths-from-file .../work/spec-paths.txt --force (in .../work/Northwind-spec) exit 1 --- output --- Traceback (most recent call last): File ".../frvenv/bin/git-filter-repo", line 8, in <module> sys.exit(main()) File ".../git_filter_repo.py", line 4973, in main filter.run() File ".../git_filter_repo.py", line 4878, in run self._migrate_origin_to_heads() File ".../git_filter_repo.py", line 4462, in _migrate_origin_to_heads print(m) UnicodeEncodeError: 'latin-1' codec can't encode character '\ufffd' in position 237: ordinal not in range(256) --- end output --- ``` Both bare leg repositories were left behind. So the defect does not need a `-E` launcher: a person in a Latin-1 locale sets no `PYTHONIOENCODING`, and pip's launcher fails as well. **The existing row, `test_adopt_template_values.py::test_a_utf8_source_directory_still_adopts_with_local_remotes`, under Latin-1 on main:** - With pip's launcher it PASSES (1 passed). Its `PYTHONIOENCODING=utf-8:strict` reaches filter-repo through `execute`'s environment. - With a copy of that launcher whose only change is `-E` on the shebang (a model of the pipx launcher #1247 quotes), it FAILS with the traceback above. That matches W1196's log. - A check that `-E` drops the variable: with `PYTHONIOENCODING=bogus-codec`, pip's launcher dies at start-up, while the `-E` copy prints its version. - On this head the row passes under Latin-1 with both launchers. ## The fix, and the two candidates measured first **Candidate 1 in #1247, `git remote remove origin` before filter-repo, changes what the legs hold.** It is rejected. - `git remote remove` deletes `refs/remotes/origin/*` with the remote. filter-repo then no longer makes the source's other branches into branches, and never rewrites them. - **Measured** with `execute` on main and on candidate 1, over two ASCII sources with fixed dates. For each leg's bare repository the probe compared: every ref, every commit with its tree, parents and full message, and every reachable object. - **The standard source** (`make_source_repo`, one branch): identical. - **A source with a `release` branch** whose commit R changes `specs/001-feature/tasks.md`, no tag reaching it, and on `main` a `git cherry-pick -x R`: the spec leg's tip differs, `0316169e` on main against `82176493` on candidate 1, with the same tree `9ef5b676`. On main its message reads `(cherry picked from commit 407d03a0...)`, R as rewritten. Under candidate 1 it reads `(cherry picked from commit 30e9dd30...)`, R's id in the source. - **It also changes behaviour that `check` describes.** As a mutant, it fails the existing `test_adopt_split_branch.py::test_the_extraction_stops_at_a_source_branch_in_its_way` (both rows, `DID NOT RAISE CommandFailed`). With it, a source branch in git's way no longer stops the extraction. **Candidate 2, `LC_ALL=C.UTF-8` for the child, depends on the host.** - **Measured here:** under `python3 -E`, a pipe's encoding is `utf-8` under `LC_ALL=C.UTF-8`. It is also `utf-8` under `C`, `POSIX` and a locale name that does not exist, because CPython falls back to C and turns on its UTF-8 mode. - **But** it hands the child a locale by name and leans on that interpreter behaviour. It changes the locale of every git command filter-repo starts. And on Windows, `LC_ALL` does not set a pipe's encoding (read, not run). - No `PYTHON*` variable is a cure, because a `-E` launcher drops it. **What this PR does: unset the url only (`git config --file <work>/.git/config --unset remote.origin.url`).** It depends on no locale. It names the clone's own config file, so `GIT_CONFIG` in the environment cannot redirect it to a file of the person's (RV1251's SHOULD-1, below). It goes on from a clone that has no url to unset, and from no other failure (NIT-1, below). - filter-repo's `_migrate_origin_to_heads` (2.47.0) turns `refs/remotes/origin/*` into `refs/heads/*` first. Then it returns when `remote.origin.url` is not in the config, before the NOTICE that printed the url and before its own `git remote rm origin`. - **The legs are byte-identical to main's** in both probes above, the cherry-pick source included (`diff` of the dumps: empty). - **What stays in each work clone** (measured on this head, a plain `execute`): the url-less `[remote "origin"]` with its fetch refspec `+refs/heads/*:refs/remotes/origin/*`, and `branch.main.remote = origin` and `branch.main.merge = refs/heads/main`. Nothing reads any of it. `_push_the_leg` pushes to the leg's url, and `_mount_the_legs` mounts the clone by its path. RV1251 measured that the mounted submodules hash the same as main's. - The change is one `run` in `_extract_leg` (now with a `try`), with a comment. No pinned or template copy of `adopt-project.py` exists, and no document describes this step. ## Review round 1 (RV1251 at `825923c7`) **SHOULD-1, fixed: the unset named no config file.** `GIT_CONFIG` in the environment (a legacy variable git still reads) names the file a bare `git config` writes. - **Reproduced** with `GIT_CONFIG` naming a file that holds `[remote "origin"] url = ...` and the standard source, in `execute`: - main: exit 0, `adoption verified`, the file unchanged. - `825923c7`: exit 2, `REFUSED extracting the spec leg`, both bare legs made and nothing pushed, and the file empty. The first leg's unset took the only key out of it, and filter-repo, reading that empty file as the config, died with a `ValueError` in `get_config_settings`. (RV1251's file ended in an unset exit 5 instead.) - this head: exit 0, `adoption verified`, the file byte for byte as it was. - **Fix:** `run(["git", GIT_CONFIG, "--file", str(work / ".git" / "config"), "--unset", "remote.origin.url"], cwd=work)`. `--local` is no way round it: with `GIT_CONFIG` set, git 2.43 exits 129 (only one config file at a time). - **`work / ".git"` is a directory wherever `execute` reaches the clone:** - the clone is made by one `git clone -q <source> <work>`, and `work` is `work_root / <name>`, where `work_root` is `os.path.realpath(--work-dir)` or a `tempfile.mkdtemp` folder (`_work_root`): an absolute path; - an exported `GIT_DIR` does not move a clone (measured: the clone has its own `.git`); - an exported `GIT_WORK_TREE` does make the clone another shape (the git dir is `work` itself), but `execute` stops on it first, on main and on this head alike, at the legs' `git init --bare` (`REFUSED a git or gh command failed`); - a path whose directory is not there gives exit 255 (`could not lock config file ... No such file or directory`), which is refused. - **Not changed:** filter-repo's own `git config --list` also reads the file `GIT_CONFIG` names (the `ValueError` above came from it), so under `GIT_CONFIG` it sees that file's settings, as on main. On this head the run ends `adoption verified` with the legs main makes. **NIT-1, handled: a clone with no `origin` url.** A global `clone.defaultRemoteName = upstream` gives the clone a remote that is not `origin`, so `git config --unset remote.origin.url` has no key and exits 5 (measured, git 2.43). It can happen. - **Before:** main fails at `mounting the legs` (`No such remote 'origin'`), after both legs are pushed. `825923c7` failed earlier, at the unset (exit 5, an empty output block), with nothing pushed. - **Now:** `_extract_leg` goes on from exit 5 and from no other exit (`GIT_CONFIG_KEY_ABSENT = 5`). The run fails where main fails, not earlier and not worse. Any other exit of the unset (255 for a file git cannot lock, 129 for a usage it refuses) is refused as any failed command is. - **Not mended here:** `clone.defaultRemoteName` itself. `execute` fails on it on main too. It is a follow-up for its own issue. - Exit 5 also covers a config with more than one `remote.origin.url` (measured); that too goes on, as main does. **NIT-2, corrected in this body:** the claim that the fix "depends on no locale and no variable" (it now names its config file, so `GIT_CONFIG` cannot redirect it); the work clone's leftovers, now listed under "What stays in each work clone" (the url-less `[remote "origin"]`, `branch.main.remote` and `branch.main.merge`); and the note that #1141 conflicts, dropped: it merges clean at `0b8fb320`. ## Rows `tests/test_adopt_filter_repo_origin_url.py`, new, five rows: - **`test_filter_repo_starts_with_no_origin_url_and_every_branch`** (any locale). `_extract_leg` itself runs over an ASCII source with a second branch, with `run` spied at the `git filter-repo` call. When filter-repo starts, the clone has no `remote.origin.url` and still has `refs/remotes/origin/main` and `refs/remotes/origin/beside`. filter-repo's output holds neither `Removing 'origin' remote` nor the source path. Afterwards the clone has `refs/heads/beside`, and the leg holds the clone's HEAD. - **`test_execute_adopts_a_source_named_in_latin1_under_latin1`** (Latin-1). It runs `plan`, `check` and `execute --local-remote-dir` for #1247's source (a directory `caf` plus the byte 0xE9) in a Latin-1 locale the test builds as #1106 builds one. No `PYTHON*` variable is set except `PYTHONUTF8=0`. It asserts no traceback, `check` exit 0, `execute` exit 0 and `adoption verified`. - It skips, by name and with `localedef`'s words, where no Latin-1 locale can be built, and on Windows. It also skips where a name that is not UTF-8 cannot be made (`skip_where_a_name_cannot_be_not_utf_8`). - It builds its locale whatever the gate's locale is, so it fails on main under a UTF-8 gate too, which is what CI runs. - **`test_execute_leaves_the_file_git_config_names_alone`** (new, SHOULD-1). `execute` with `GIT_CONFIG` naming a file that holds a `[remote "origin"]` url: `adoption verified`, and the file is byte for byte as it was. The child gets none of the gate's locale variables, so the row runs alike under a UTF-8 and a Latin-1 gate. - **`test_a_clone_with_no_origin_url_goes_on_to_filter_repo`** (new, NIT-1). `_extract_leg` itself under a global `clone.defaultRemoteName = upstream`: the unset exits 5 (recorded as it is raised), the clone's `remote.upstream.url` is the source, and the leg is pushed. - **`test_an_unset_that_fails_for_any_other_reason_stops_the_leg`** (new, NIT-1). A `config.lock` in the clone makes the unset exit 255: `CommandFailed`, before `git filter-repo` and with nothing pushed. It needs no git-filter-repo. - The earlier rows' setup is now two helpers, `extract_the_code_leg` and `adopt_each_step`. The file is ASCII with no backslash escape; each non-ASCII character is built with `chr` or `bytes`. **Counts** (5 rows; the same file in each tree): | code | UTF-8 gate, pip and `-E` | Latin-1 gate, pip and `-E` | |---|---|---| | main (`_extract_leg` as on `dffc4226`; unchanged in `5926ccca`) | 4 failed, 1 passed (the GIT_CONFIG row passes: main leaves the file alone) | 4 failed, 1 passed | | `825923c7` (the previous head) | 2 failed, 3 passed (the GIT_CONFIG row and the exit-5 row fail) | 2 failed, 3 passed | | this head | 5 passed | 5 passed | The two earlier rows fail on main and pass here. The GIT_CONFIG row fails on `825923c7` and passes here. The exit-5 row fails on `825923c7` and on main (no unset to record) and passes here. The lock row passes on `825923c7`, which refuses every exit; it guards the new `except`. ## Mutants Each mutant was applied in a worktree at this head, the five rows plus the two `test_the_extraction_stops_at_a_source_branch_in_its_way` rows were run under UTF-8 and under Latin-1, and `adopt-project.py` was restored with its sha256 checked after each one (`RESTORED sha256 ok`, tree as before). | mutant | UTF-8 | Latin-1 | killed by | |---|---|---|---| | M1: drop the unset | 4 failed, 3 passed | same | the two earlier rows, the exit-5 row and the lock row (no unset to see) | | M2: the unset moved after `git filter-repo` | 2 failed, 5 passed | same | the two earlier rows | | M3: `git remote remove origin` instead | 5 failed, 2 passed | same | the any-locale row, the exit-5 row, the lock row and both split-branch rows | | M4: drop `--file <path>` | 1 failed, 6 passed | same | the GIT_CONFIG row | | M5: `--local` instead of `--file <path>` (exit 129 under `GIT_CONFIG`) | 1 failed, 6 passed | same | the GIT_CONFIG row | | M6: accept every exit of the unset | 1 failed, 6 passed | same | the lock row | | M7: accept none (no `try`) | 1 failed, 6 passed | same | the exit-5 row | | M8: `--file <work>/config`, a path with no file (exit 5, carried on) | 3 failed, 4 passed | same | the any-locale row, the Latin-1 row and the lock row | ## Gates All runs used `/usr/bin/python3 -m pytest -p no:cacheprovider` with one reused basetemp per run kind and `-o tmp_path_retention_policy=failed`, at most 2 pytest runs at a time. The environment had a stub `gh` first on PATH, `GH_TOKEN=bogus`, an empty `GH_CONFIG_DIR`, `GIT_CONFIG_GLOBAL` an empty file and `GIT_CONFIG_NOSYSTEM=1`, and no `GIT_ALLOW_PROTOCOL`, `GIT_SSH_COMMAND`, `GIT_CONFIG`, `LANES_*` or `PYTHON*` variable. filter-repo was pip's launcher (`a40bce548d2c`) unless a row says otherwise. **The tree gated is a scratch merge** of this head and `origin/main` `5926ccca` (built with `merge-tree` and `commit-tree`; its diff against main is this PR's two files). One hygiene row caught a path in the first attempt (`/home/someone/...` in the new test, `test_no_committed_file_names_a_host_absolute_path`); the row now names `https://example.invalid/keep-me.git`, and everything below ran on the final head. **Gate A, UTF-8 (`LC_ALL=C.UTF-8`), one pytest, 23 files:** **1518 passed, 3 skipped** on the scratch merge. - the new file, `test_adopt_template_values.py`, `test_adopt_split_branch.py` and `test_adopt_e2e.py`; - the other four files that name `_extract_leg` or `_push_the_leg`: `test_adopt_filter_repo_failure.py`, `test_adopt_history_gitlinks.py`, `test_adopt_late_failure.py` and `test_adopt_plan_escapes.py`; - `test_adopt_check_throwaway_repos.py`, `test_adopt_partial_clone_source.py`, `test_adopt_pins.py`, `test_adopt_git_rm_bytes.py` and `test_adopt_plan_partial_clone.py` (new on main); - `test_repo_hygiene.py`, `test_path_guard_hygiene.py`, `test_cli_reference.py` and `test_scaffold_leg_path_collisions.py`; - the files that scan every test file: `test_no_auto_maintenance.py`, `test_non_utf8_git_failures.py`, `test_git_output_decoding.py`, `test_setup_project_repo_misdrawn.py`, `test_shape_advisory.py` and `test_openreposhape_package.py`. - **Main `5926ccca`, the same 22 files without the new one:** 1513 passed, 3 skipped. The head's extra 5 are the five rows. - **Skips (`-rs`):** the same 3 on both, none for lack of git-filter-repo: `test_adopt_check_throwaway_repos.py:482` (a disk that keeps case) and `test_no_auto_maintenance.py:222` x2 (`GIT_CONFIG_GLOBAL` set). **`tests/test_shape_doctor.py`, UTF-8, 4 shards from a bash script** (449 ids): **Head (scratch merge):** 118, 110, 118 and 100 passed, with 3 skipped in the last shard (the PowerShell line, Windows only): 446 passed, 3 skipped. RV1251 and W1247 measured 446 passed and 3 skipped on main at `11560eb9` and `b1f45fa7`; it was not re-run on `5926ccca`, because this PR touches nothing the doctor reads. **Latin-1** (the locale built as above, `PYTHONUTF8=0`), the new file and `test_adopt_template_values.py`: | run | main `5926ccca` | this head (scratch merge) | |---|---|---| | new file + `test_adopt_template_values.py`, pip launcher | 4 failed, 69 passed, 5 skipped (template values only) | 4 failed, 74 passed, 5 skipped | | the same, `-E` launcher | 5 failed, 68 passed, 5 skipped (template values only) | 4 failed, 74 passed, 5 skipped | - **The 4 failures are the same rows on main and on this head, under both launchers.** All four already fail on main and are #1214's to judge, not this PR's: `test_plan_refuses_a_flag_no_file_can_hold` at `--elected-by` and `--reference` with U+DCE9, `test_plan_and_check_say_the_same_sentence`, and `test_execute_refuses_a_local_source_directory_that_is_not_utf8` (under Latin-1 the byte 0xE9 is valid text). - The fifth failure on main under `-E` is `test_a_utf8_source_directory_still_adopts_with_local_remotes`, which this head fixes. ## Merge-trees `git merge-tree --write-tree` of this head against: - **main:** clean against `5926ccca`, and the merged `adopt-project.py` keeps the `--file` call. - **#1177 (`5208e874`) and #1225 (`73c9f57e`)**, the only open PRs that touch `_extract_leg`: both clean, each keeps the `--file` call. - **#1141 (`0b8fb320`):** clean. The earlier note that it conflicts is stale and is dropped. - RV1251 merge-tree'd the 27 other open PRs that touch `adopt-project.py` against the previous head: 26 clean, and #1237 conflicts in `tests/test_adopt_check_throwaway_repos.py` with main alone. This round re-ran main, #1177 and #1225 (the only PRs that touch `_extract_leg`) and #1141 (the one the earlier body got wrong). ## Not done, and limits - Windows, another one-byte locale (cp1252 included) and a real pipx install were not run. The `-E` launcher here is a one-line copy of pip's. - **Read from the code, not run:** filter-repo prints nothing else that holds the url. Its other prints of `origin` are under `--sensitive-data-removal`, which `execute` never passes. Closes #1247 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>



Closes #1152
This PR lands only after the v1.2.0 tag.
What
#1154 (#1115) names
--outand--work-dirinplan's andcheck's report with_shown. #1152 lists the two groups of paths it leaves raw: the--planpath incheckandexecute, and the--local-remote-dirpath inexecute. A bidi control (U+202E), an ESC or a line break in either reached the terminal as it was typed, where the other plan lines name a path with_shown.This change spells each of the 21 sites with
_shown(str(...)), inadopt-project.pyonly. A path with a character that is not printable is named as_spellednames it, between backticks. Every other path prints byte for byte as it did, and no refusal code is new.The
--planpath, 16 sites:N finding(s) in <plan>lines:_refuse_a_mistyped_plan,_refuse_what_check_finds, theplan-split-branch-takenrefusal, andcmd_check's two branches;<plan>: ...refusals:plan-wrong-kind,plan-unsupported-mode,plan-unreadable,plan-no-source, the threeplan-bad-idones,plan-not-the-source,adopt-topic-too-long, and thelegs.assemblyandorg:ones.The
--local-remote-dirpath, 5 sites:<bare> already existsrefusal;bare <bare>line;_extract_legand_seed_leg, which end-> <url>;split <commit> on <branch> -> <url>line. It names the source's directory under--local-remote-dir.The
NEXT:clone command is not touched. It already quotes the url withshlex.quote, and a row pins that it still does.Behaviour change
A path that is not UTF-8 now prints
\udcffbetween backticks, as #1154's paths do, where it printed raw before. A plain path prints as it did.Names
#1152 calls the wrong-kind refusal
plan-bad-kind. The code's name for it isplan-wrong-kind, and that is the code this change spells. No refusal was renamed.Not in this PR
repo_shape.load_yaml's own refusals,yaml-unreadable, still print a path raw. They live inscripts/repo_shape.py, and they are a separate follow-up: see #1168.Proof
tests/test_adopt_plan_and_remote_paths_spelled.pyis new, ASCII only and POSIX only, with every non-ASCII character written as an escape.Rows: 148.
\xffthroughsurrogateescape, and a plain control).--local-remote-dirrows for thebareand leg lines (the code leg is extracted, the spec leg seeded).splitrows.executerefuses a source path that holds U+202E, ESC, a line feed or\xffasadopt-unwritable-valuebefore any leg exists, so these rows use U+200B in the parent directory and a name that ends in a space, and a plain control.{bare} already exists, which call_create_leg_remote.On main's
adopt-project.py: 118 failed, 30 passed. Each failure is the raw path where the spelled one was expected. The 30 that pass are exactly the 30 plain controls.On this head: 148 passed.
Mutants: 23, all killed. 21 put one spelling back to raw, one at a time, and 2 swap
_shownfor_spelledat one site. Each was restored with its sha256 checked.Gate: a scratch merge of this head with main
961640c68(tree61063bb76, which is the tree of this PR merged into that main). Targeted files only: the new file,test_stdout_encoding.py,test_non_utf8_name_callers.py,test_adopt_plan_escapes.py,test_path_guard_hygiene.py,test_repo_hygiene.py,test_scaffold_leg_path_collisions.py, and every test file that greps forfinding(s) inoralready exists(47 files), in 6 shards, andtest_shape_doctor.py(448 ids) in 8 shards. Every shard exited 0, with 0 failed.That is 3671 passed and 1 skipped in the file shards, and 445 passed and 3 skipped in the doctor shards. The skips are Windows-only rows.
Merge-trees (
git merge-tree --write-tree, this head against each), all clean:origin/main961640c68;plan written toandcloning <repository> (read only) intoname the caller's--outand--work-dirwith_shown, as the other lines of the report name a path, so a bidi control, a line feed or a tab in either no longer reaches the terminal as it was typed (#1115) #1154's head8a782733: clean, so this PR is based on main and not stacked on it;adopt-project.py, at their heads: adopt-project.py and repo_shape.py: a name that holds a default-ignorable character, which a terminal draws as nothing, is spelled by its code point inplan's lines and in theunsafe-valuerefusal, so a branchma<U+034F>inno longer prints asmain(#1128) #1171159815d75, adopt-project.py:checknames each case twin at the shape's copy, above it and at the manifest, before a link there or after it, and at a pin's file whose exact spelling holds nothing, as_KeptTreekeeps every row, not the first (#1138) #11704df784ac4, adopt-project.py:check's rehearsal and_stored_as_writtentake a parent's-csettings,attr.treetoo, and the source's objects,GIT_ATTR_SOURCEis refused by name before any question, and each throwaway reads its environment first (#1136) #1169b8e63bcf0, adopt-project.py: under a one-byte locale such as Latin-1, a clonecheckmakes again for a plan's clone record that neither spelling names is compared with both, so it is kept and the next run reads it rather than cloning again (#1112) #1153ff33ba12a, adopt-project.py: a--work-dirlink into a loop that Python 3.12 reads past a link that does not loop names the loop, Windows' winerror 1921 counts as one, and a path read as the loop's link says so, with tests for r1065's mutants (#1074) #11437af5fabc3, adopt-project.py: a recorded source whose.gitthis user cannot read is refusedsource-unreadable, naming that.git, socheckandexecuteno longer call a cloneplanmade with its.gitat mode 000 no longer whole and clone it again (#1099) #11417a2711d03, adopt-project.py:checkandexecuteask a plan'sorg:andid:right after its three repository names, so a bad one beside an unanswered entry, a moved source, a refused name or another repository is that one refusal in both, and apins:entry that is no pin is said as_read_assays it (#1020) #11259549006fe, README.md, adopt-project.py and the handbook: a kept directory entrymakefile/is tested, README saysmakeat the root reads a keptGNUmakefileormakefileinstead, and a docstring says the shape's Makefile alone holdsCONTRACTS_DIR(#1061) #11235c3908f8d, adopt-project.py: anorg/repo--sourcetoo long for the disk is refused as a path is, naming no GitHub repository and offering no url, so--source <300 a's>/Thingis no longer offeredhttps://github.com/<300 a's>/Thing, which can never clone (#1003) #1122c01c4d73d, adopt-project.py:execute's NEXT printspython3 scripts/bootstrap.pywhere GNU make at the root reads a keptGNUmakefileormakefile, and its NOTE names that file and what it takes formake bootstrapto run there (#1047) #11211c5173758, adopt-project.py:plan'ssourceline names the source's branch as_shownnames a path, so a bidi control, a line boundary or a byte that is not UTF-8 in it is spelled and no longer reaches the terminal raw (#1035) #1120c55e55e33, adopt-project.py: check asks the split's own clone of the source in the work directory, so a clone this git will not make is adopt-git-unasked before any leg, and _resolved and _answers make their repositories as check's other throwaways do (#1083) #1119bba168bdd, adopt-project.py, shape_materialize.py and tests: a named bad link left out of the copy and pin findings takes no other path's finding with it, and the docstrings and the materializer's refusal name the #979 exception and what helps it (#1089) #11182307036ab, adopt-project.py: under a one-byte locale such as Latin-1,checkreads a plan written in UTF-8 whose source is there, asking the plan's UTF-8 bytes where the spelling as written names nothing, rather than refusingplan-source-gone(#1078) #1106504a1dd65, shape_materialize.py and the pinned AGENTS-shape.md: where the root keeps aGNUmakefileor amakefile, which GNU make reads first, the Makefile collision follow-up no longer says the shape's make targets arrive with the merge, and the pinned file saysmakereads the first of the three and the block reaches it only once that file holds it (#1048) #1105b28f40de0.The other open PRs do not touch
adopt-project.py. None of the 16 that do adds a line that namesplan.path,{bare},finding(s) inoralready exists, apart from a tuple in adopt-project.py:execute's NEXT printspython3 scripts/bootstrap.pywhere GNU make at the root reads a keptGNUmakefileormakefile, and its NOTE names that file and what it takes formake bootstrapto run there (#1047) #1121 that is not printed.🤖 Generated with Claude Code
Lane: openreposhape-4 (openRepoShape-4)
Summary by Sourcery
Prevent unsafe user-supplied plan and local remote paths from reaching terminal output unescaped.
Bug Fixes:
--planand--local-remote-dirpaths in checks, refusals, and execution output so terminal-control characters and non-UTF-8 bytes cannot be emitted raw.Tests: