diff --git a/adopt-project.py b/adopt-project.py index 922460ee..85a0c292 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -24,7 +24,8 @@ `leg: null`, `review_required: true` and the QUESTION to ask. A leg path over a path it keeps in the root itself is a NOTE, not a refusal: the answer is the person's to change. - check validates a plan against the source: every path covered exactly + check validates a plan against the source: each entry path bound for + a leg one `execute` will extract, every path covered exactly once, no unresolved legs, leg names conforming to the naming policy, each leg's mount path one `execute` can mount (`spec`, `legs/spec`; never `spec/` or `./spec`, a name Git or @@ -2252,11 +2253,237 @@ def _submodule_findings(plan: Plan, source: Source, return found, [SUBMODULE_REMEDIATION] if found else [] +#: The answers that never extract an entry into a leg. A path answered +#: `root` stays in the assembly root and reaches no `git` command but as a +#: line of the split commit's message; one answered `drop` reaches only +#: `git rm`, as a literal path after `--`, and that message. Neither is ever +#: a line of the path list `git filter-repo` reads, so `entry_path_problem` +#: does not judge them (#196). +NOT_EXTRACTED = ("root", "drop") + +#: Two characters Python's `str.splitlines` ends a line at that are not +#: control characters. `entry_path_problem` refuses them with the control +#: characters, as policy. +LINE_SEPARATORS = "\u2028\u2029" + +#: The surrogates `surrogateescape` reads a byte that is not UTF-8 as, and +#: writes back as that byte (`Source.tree`, `write_paths_listing`): a path's +#: byte 0x80 to 0xFF is one of these. `entry_path_problem` refuses them as +#: policy, and every other surrogate as a misreading. +NOT_UTF8_BYTES = frozenset(map(chr, range(0xDC80, 0xDD00))) + +#: What every policy refusal says of git and `git filter-repo`, because it is +#: true of each of them, measured: a policy refusal is not a misreading. +TAKEN_LITERALLY = "though git and `git filter-repo` would take it literally" + + +def entry_path_problem(path: str) -> str | None: + """Why `execute` will not extract the entry path `path` into a leg, or + None. + + ONE TEST, asked by `check`, which reports what it finds as the finding + `plan-unsafe-path`, and by `execute`, which refuses that finding in + `check`'s words through `_refuse_what_check_finds` before any leg exists + (#196). On main at e4decfb `check` asked nothing of an entry's path and + `execute` asked `checked_value`, so a source holding `My Notes.md` + checked `plan ok` and `execute` refused it as `unsafe-value`. + + AN ENTRY'S PATH IS A FACT OF THE SOURCE, NOT AN OPTION. `plan` writes it + from the tree, and a tree legitimately holds a space, a letter outside + ASCII, `#`, `$`, `(`, `'`, `"`, `;`, `&`, `[`, `*`: none of them is the + threat, as `repo_shape.SAFE_PATH_RE` says of a path the operator names. + Every command is argv with `shell=False`, and in `execute` an entry's + path reaches `git` in two places, each spelled so git reads it + LITERALLY: + + * as one `literal:` line of the path list `git filter-repo + --paths-from-file` reads (`_extract_leg`). Without the prefix a line + starting with `#` is a comment and one starting with `regex:`, + `glob:` or `literal:` is that directive, so `#notes.md` was left out + of its leg -- removed from the root by the split, then found nowhere; + * as a pathspec after `--` to `git --literal-pathspecs rm` + (`_mount_the_legs`). After `--` a path is no option, but it is still + a pathspec: `*` and `?` are wildcards and a leading `:` is magic, and + `git rm -- ':!keep.md'` removes every other path in the tree. + + Two kinds of path are refused, and each finding says which kind, and + why. First, `_misread`: a path one of those two places, or this tool's + own write of the list, would NOT carry as it stands. Then + `_refused_by_policy`: a path both would carry, measured, that this tool + still will not move into a leg. A path with a reason of each kind is + told the misreading. + + THE VALUES THAT ARE OPTIONS ARE ANOTHER MATTER: a leg's mount path, a + branch, a repository name, the organisation. They stay under + `checked_value`'s narrow alphabet and are refused as `unsafe-value` + (#168, #169). + """ + return _misread(path) or _refused_by_policy(path) + + +def _misread(path: str) -> str | None: + """Why `path`, as a `literal:` line or a literal pathspec, would not reach + git or `git filter-repo` as the path it is. Measured with git 2.43 and + git-filter-repo 2.47.0, whose `get_paths_from_file` reads its list a line + at a time, strips `\\r` and `\\n` from each line's end, skips a blank + line, and looks for `==>` before anything else: + + * an empty path: a `literal:` line with nothing after it matches every + path, and the leg took the whole tree (and `git rm` refuses an empty + pathspec); + * a line feed, anywhere: it ends the path's line, and the rest is read + as a line of its own -- `docs\\nregex:.*` kept every path; + * a carriage return at the END: stripped, so `Icon\\r` -- the macOS + folder-icon file -- was read as `Icon`, and the leg took that file + instead. A name of carriage returns alone leaves `literal:` with + nothing after it, which, as for the empty path, kept every path. A + carriage return elsewhere is carried, and is policy's; + * a NUL, which no command line can carry (`subprocess` raises), and + which git records in no path; + * a lone surrogate (category Cs) outside U+DC80 to U+DCFF, such as a + hand-edited `\\ud800`. `Source.tree` reads a byte that is not UTF-8 + as one of U+DC80 to U+DCFF (`surrogateescape`), so no path of the + source holds any other, and `_extract_leg`'s `write_paths_listing` + cannot write one: it raised `UnicodeEncodeError`, measured. (Such an + entry also covers nothing, so `check` reports `plan-empty-entry` + beside it.) A byte that is not UTF-8 is policy's; + * `==>`, which `git filter-repo` reads as a RENAME wherever it is on a + line, before it looks for `literal:`, and which nothing escapes: + `literal:a==>b.md` beside `literal:keep.md` extracted only + `keep.md`. + """ + if not path: + return ("it is empty: a `literal:` line with nothing after it " + "matches every path, so `git filter-repo` would put the " + "whole tree in the leg") + if "\n" in path: + return ("it holds a line feed (`\\n`), which ends its line of the " + "path list `git filter-repo` reads, so the rest of the name " + "would be read as a line of its own") + if path.endswith("\r"): + rest = path.rstrip("\r") + named = (f"would name {_spelled(rest)}, another path" if rest + else "would be `literal:` alone, which matches every path, " + "so `git filter-repo` would put the whole tree in the " + "leg") + return ("it ends in a carriage return (`\\r`), which `git " + "filter-repo` strips from the end of each line of the path " + f"list it reads, so its line {named}") + if "\x00" in path: + return ("it holds a NUL (`\\x00`), which no command line can carry " + "and git records in no path") + for char in path: + if "\ud800" <= char <= "\udfff" and char not in NOT_UTF8_BYTES: + return (f"it holds `{_spelled_character(char)}`, a surrogate no " + "byte of a path is read as, so it names no path of the " + "source, and the path list `git filter-repo` reads cannot " + "be written with it") + if "==>" in path: + return ("it holds `==>`, which `git filter-repo --paths-from-file` " + "reads as a rename wherever it is on the line, and nothing " + "on the line escapes it") + return None + + +def _refused_by_policy(path: str) -> str | None: + """Why this tool will not move `path` into a leg although git and `git + filter-repo` would take it literally. MEASURED, each as one `literal:` + line and as `git --literal-pathspecs rm -r -q -- `: a tab, `\\x01`, + ESC, DEL, `\\x85`, `\\x0b`, `\\x0c`, `\\x1c`, U+2028, a carriage return + that does not end the name, `-n.md`, ` ` and three spaces were each + extracted, and removed, as exactly that path and nothing else. So each + finding below says it is policy, and never that a line would break: + + * a name that is nothing but whitespace, which shows as blank wherever + a name is printed. (`git commit` even strips a line of spaces from + the split commit's message, which lists the moved paths.) + * a control character (C0, DEL, C1) or a line separator: a name that + holds one is almost never meant, and printed as it stands -- as + `git log` prints a commit message, ESC and `\\x01` raw, measured -- + it cannot be seen for what it is; + * a byte that is not UTF-8, which `Source.tree` keeps as one of + U+DC80 to U+DCFF (`surrogateescape`). Since #199 (PR #247) + `write_paths_listing` writes it as that byte, and with this refusal + disabled a source holding `caf\\xe9.md` was extracted and verified, + the byte in the leg, measured. It is held until adopting such a name + end to end is measured on macOS and Windows: a name that holds one + prints as an escape, never as itself, and the split commit's message + lists it with a `?` for the byte (`env_commit`); + * a leading `-`, the threat model's first rule + (`repo_shape.checked_value`), held although neither place in + `entry_path_problem` takes it as an option: a line of the filter's + path list is none, and `git rm` is given it after `--`. + """ + if not path.strip(): + return ("it is nothing but whitespace, which `execute` refuses as " + f"policy, {TAKEN_LITERALLY}: such a name shows as blank " + "wherever a name is printed") + for char in path: + if ord(char) < 0x20 or 0x7F <= ord(char) <= 0x9F \ + or char in LINE_SEPARATORS: + kind = ("line separator" if char in LINE_SEPARATORS + else "control character") + return (f"it holds the {kind} `{_spelled_character(char)}`, " + f"which `execute` refuses as policy, {TAKEN_LITERALLY}: " + "a name that holds one is almost never meant, and " + "printed as it stands, as `git log` prints a commit " + "message, it cannot be seen for what it is") + for char in path: + if char in NOT_UTF8_BYTES: + return (f"it holds `{_spelled_character(char)}`, which is not " + "UTF-8 and which `execute` refuses as policy, " + f"{TAKEN_LITERALLY}: a name that holds one prints as an " + "escape and never as itself, the split commit's message " + "would list it with a `?` for that byte, and adopting " + "one is not yet measured on macOS or Windows") + if path.startswith("-"): + return ("it begins with `-`, which `execute` refuses as policy, " + f"{TAKEN_LITERALLY} (a line of the filter's path list is no " + "option, and `git rm` is given it after `--`): no value this " + "tool hands git may begin with `-`, the threat model's first " + "rule") + return None + + +#: What a plan whose entry `entry_path_problem` refuses is told to do. Said by +#: `check`, once after its findings, and by `execute`'s refusal. +UNSAFE_PATH_REMEDIATION = ( + "Remediation: an entry's `path:` names a path in the source tree, so the " + "plan cannot respell it: the respelled entry would cover nothing and the " + "path itself would be uncovered. Rename the path in the source, commit, " + "and re-run `plan`. Or, if it belongs in no leg, answer the entry `root` " + "or `drop`: a path that stays in the assembly root reaches no `git` " + "command, and a dropped one reaches only `git rm`, as a literal path " + "after `--`. Only a path the plan names as an entry is judged: a name " + "below a directory entry is extracted with that directory and never " + "written on its own.") + + +def _unsafe_path_findings(plan: Plan) -> tuple[list[str], list[str]]: + """`entry_path_problem` for each entry not answered `root` or `drop`, as + `check` prints it, one FINDING each; and what to print ONCE after every + finding, as `_submodule_findings` does. An unanswered entry is judged + too, so the person answering its question knows what `spec` or `code` + would meet.""" + found = [] + for entry in plan.entries: + if str(entry.get("leg")) in NOT_EXTRACTED: + continue + path = str(entry.get("path")) + problem = entry_path_problem(path) + if problem: + found.append(f"FINDING plan-unsafe-path: {_spelled(path)} has " + f"leg: {y(entry.get('leg'))}, and `execute` will " + f"not extract it into a leg: {problem}") + return found, [UNSAFE_PATH_REMEDIATION] if found else [] + + def _entry_findings(plan: Plan, source: Source ) -> tuple[list[str], list[str]]: """Every FINDING `check` prints about the plan's ENTRIES, in its order, - and what it prints once after them: coverage, then each entry's `leg:` - and the two mount paths, then the source's own submodules. + and what it prints once after them: each entry's path, then coverage, + then each entry's `leg:` and the two mount paths, then the source's own + submodules. ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` @@ -2270,13 +2497,20 @@ def _entry_findings(plan: Plan, source: Source the root are read out of it (#172, #191): without it a mount path at, or beyond, a kept source path checks ok and dies at `git submodule add` after both legs are pushed. + + THE PATHS COME FIRST (#196), so that `_refuse_what_check_finds` wears + `plan-unsafe-path` for a plan that has one: a path that is a `git` option + is refused as that, and never as the uncovered file it also leaves. """ tree = source.tree() - findings = (_coverage_findings([str(e.get("path")) for e in plan.entries], - [path for path, _, _, _ in tree]) + unsafe, unsafe_remediation = _unsafe_path_findings(plan) + findings = (unsafe + + _coverage_findings([str(e.get("path")) + for e in plan.entries], + [path for path, _, _, _ in tree]) + _leg_findings(plan, tree)) submodule_findings, remediation = _submodule_findings(plan, source, tree) - return findings + submodule_findings, remediation + return findings + submodule_findings, unsafe_remediation + remediation def _topics_line(topic: str, local: bool) -> str: @@ -2504,9 +2738,14 @@ def _extract_leg(role: str, source: Source, work: Path, paths: list[str], # path contains - so on Windows every pattern would match nothing, the # filter would succeed, and the leg would be pushed EMPTY. A silent # extraction of nothing is the worst shape this failure could take. + # `literal:` on every line (#196): filter-repo reads a line starting with + # `#` as a comment and one starting with `regex:`, `glob:` or `literal:` + # as that directive, so a source file named `#notes.md` was left out of + # its leg after both legs existed. The prefix takes the rest of the line + # as it stands; what no line can carry, `entry_path_problem` refuses. # And byte-exact: a path that is not UTF-8 is written as the bytes # git records, where `write_lf` raised after both legs existed. - write_paths_listing(listing, paths) + write_paths_listing(listing, [f"literal:{path}" for path in paths]) run(["git", "filter-repo", "--paths-from-file", str(listing), "--force"], cwd=work) head = git_out(["rev-parse", "HEAD"], cwd=work).lower() @@ -2723,8 +2962,12 @@ def _mount_the_legs(assembly: Path, work_root: Path, names: dict, urls: dict, + paths_for["code"] + paths_for["drop"])), key=lambda p: (p != GITMODULES, p)) + # `--literal-pathspecs` (#196): after `--` a path is no option, but it is + # still a pathspec, where `*` and `?` are wildcards and a leading `:` is + # magic -- `git rm -- ':!keep.md'` removes every other path in the tree. for path in removed: - run(["git", "rm", "-r", "-q", "--", path], cwd=assembly) + run(["git", "--literal-pathspecs", "rm", "-r", "-q", "--", path], + cwd=assembly) # The plan moved (or dropped) the source's own .gitmodules, so its deletion # is staged, and `git submodule add` refuses to write into a file the index # says is going away. Give the assembly a fresh, EMPTY, staged one for its @@ -2817,10 +3060,16 @@ def _repository_urls(args, plan: Plan, names: dict, def _leg_paths(plan: Plan) -> dict: """Every plan entry's path, by the leg it was assigned to. - Each one is checked because it reaches `git filter-repo` as an argument. - Split out of `cmd_execute` for #138. + COLLECTED, NOT JUDGED (#196). An entry's path is a fact of the source, + not an option, and what `execute` cannot hand to `git` is + `entry_path_problem`'s to say: `check`'s finding `plan-unsafe-path`, + which `_refuse_what_check_finds` refuses on the next line of + `cmd_execute`, before any of these paths is used and before any leg + exists. Judging them here as well would be a second refusal, composed + apart from `check`'s list and said before it. Split out of `cmd_execute` + for #138. """ - return {leg: [checked_value("a plan path", e.get("path")) + return {leg: [str(e.get("path")) for e in plan.entries if str(e.get("leg")) == leg] for leg in LEG_VALUES} @@ -2872,12 +3121,16 @@ def _refuse_what_check_finds(plan: Plan, source: Source) -> None: `_leg_findings` also reports `plan-unresolved`, which that function has already refused in its own words, and a plan written against a tree that has since moved is `plan-stale`, not whatever its stale coverage happens - to lack. And it runs AFTER `_leg_paths` has checked, as a safe `git` - argument, the path of every entry whose `leg:` is one of the four words, - because a path that is an option is `unsafe-value` and must stay that, not - become an uncovered file. An entry with any other `leg:` is not checked - there and reaches no `git` command; it is refused here, under whichever - finding `check` lists first. It runs BEFORE `_refuse_unconsented_seeding`, + to lack. A VALUE THAT IS AN OPTION AND A PATH THAT IS A FACT ARE TWO + TESTS (#196). A leg's mount path, a branch, a repository name and the + organisation are options: `checked_value` refuses them as `unsafe-value` + in `_checked_plan_values` and `_repository_urls`, before this runs. An + entry's path is a fact of the source, judged by `entry_path_problem` as + `check`'s own finding `plan-unsafe-path`, which `_entry_findings` lists + FIRST: a path that is a `git` option is refused as that, and never as the + uncovered file it also leaves. `_leg_paths` collects the entries' paths + without judging them, and nothing uses one before this has run. It runs + BEFORE `_refuse_unconsented_seeding`, `_confirm` and `_create_leg_remotes`, so no leg repository is created and nothing is pushed until every refusal has passed. (`_work_root` has already made the work directory and an `org/repo` source has already been @@ -2890,7 +3143,7 @@ def _refuse_what_check_finds(plan: Plan, source: Source) -> None: nothing else, and `_refuse_an_unrunnable_plan` is back to its two states: a plan `check` rejects as uncovered and split is refused for both at once, and a submodule problem is refused here like any other, - after `_leg_paths` has checked every path as a `git` argument. + after the entries' paths, which are the first of these findings. """ findings, remediation = _entry_findings(plan, source) if not findings: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index f6505665..315e9d91 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -654,6 +654,27 @@ def env_commit(work: Path, message: str) -> None: A scaffold that fails on a machine with no `user.email` configured fails for a reason that has nothing to do with the project being scaffolded, so a fallback identity is supplied rather than assumed. + + THE MESSAGE GOES TO GIT AS UTF-8, whatever the locale (#196). Text-mode + stdin encodes with the locale's codec unless Python runs in UTF-8 mode + -- cp1252 on a Windows without `PYTHONUTF8=1`, ASCII under a POSIX `C` + locale with that mode off -- and git keeps the bytes it is given. An + adoption's split message already carried an em dash, which reached git + there as a cp1252 byte, and it names each entry path as the source spells + it: `Übersicht.md` would land as another byte that is not UTF-8, and + `文書.md` would raise `UnicodeEncodeError` after both legs are pushed. + Every caller's message goes the same way: `scaffold-project.py`'s + assembly commit, `scripts/family.py`'s holder commit, and each first + commit `git_init_commit` makes (a scaffolded leg, a seeded one). + + `errors="replace"` works BOTH WAYS. Decoding, git's own output cannot + raise. Encoding, a character UTF-8 cannot carry is written as `?`: the + one such character a message can hold is a lone surrogate, the byte of a + name that is not UTF-8 as `Source.tree` keeps it. `adopt-project.py` + refuses one in a path it extracts (`entry_path_problem`), but a `root` + or `drop` entry holding one is listed in the split message, so there the + message names it with a `?` in place of that byte, a path that does not + exist, rather than raising after both legs are pushed. """ env = dict(os.environ) for key, fallback in (("GIT_AUTHOR_NAME", "openRepoShape scaffold"), @@ -665,7 +686,8 @@ def env_commit(work: Path, message: str) -> None: env[key] = fallback check_program(COMMIT_COMMAND) proc = subprocess.run(COMMIT_COMMAND, cwd=str(work), input=message, - capture_output=True, text=True, check=False, env=env) + capture_output=True, text=True, encoding="utf-8", + errors="replace", check=False, env=env) if proc.returncode != 0: raise CommandFailed(COMMIT_COMMAND, work, proc.returncode, proc.stderr + proc.stdout) diff --git a/tests/test_adopt_e2e.py b/tests/test_adopt_e2e.py index 3644106a..b75efafe 100644 --- a/tests/test_adopt_e2e.py +++ b/tests/test_adopt_e2e.py @@ -317,7 +317,9 @@ def test_the_tool_never_touches_the_source_working_tree(adopted): def test_a_plan_path_that_is_a_git_option_is_refused(source_repo, tmp_path): """THE PLAN IS UNTRUSTED INPUT: it is edited between `plan` and `execute`, - by a human or by an AI, and every path in it becomes a git argument.""" + by a human or by an AI, and every path in it reaches git. An entry's path + that begins with `-` is `check`'s finding `plan-unsafe-path` (#196), and + `execute` refuses it before any leg exists.""" plan = tmp_path / "plan.yaml" assert write_plan(source_repo, plan, project=PROJECT).returncode == 0 for path, leg in ANSWERS: @@ -328,5 +330,5 @@ def test_a_plan_path_that_is_a_git_option_is_refused(source_repo, tmp_path): "--local-remote-dir", str(tmp_path / "remotes"), "--work-dir", str(tmp_path / "work")) assert result.returncode == 2 - assert "unsafe-value" in result.stderr + assert "REFUSED plan-unsafe-path" in result.stderr assert not (tmp_path / "remotes").exists() diff --git a/tests/test_adopt_entry_paths.py b/tests/test_adopt_entry_paths.py new file mode 100644 index 00000000..1a2e1d23 --- /dev/null +++ b/tests/test_adopt_entry_paths.py @@ -0,0 +1,591 @@ +# SPDX-License-Identifier: Apache-2.0 +"""An entry's path is a fact of the source, judged by ONE test (#196). + +On main at e4decfb `check` asked nothing of a plan entry's path and `execute` +asked `checked_value`, whose alphabet (`SAFE_ARG_RE`) is meant for the values +that are options -- a branch, a repository name, a leg's mount path. A source +holding `My Notes.md` therefore checked `plan ok`, and `execute` refused the +same plan as `unsafe-value`. Now both ask `entry_path_problem`: `check` +reports `plan-unsafe-path` and `execute` refuses it in `check`'s words before +any leg exists, and what a source tree legitimately holds -- a space, a letter +outside ASCII, `#`, `?`, `[`, `;` -- is extracted, because every place such a +path reaches `git` reads it literally. + +NO NETWORK AND NO GITHUB. `git filter-repo` is needed only by the end-to-end +test at the bottom, which skips without it as `test_adopt_e2e.py` does; the +Windows job installs none, so there it is the tests above it that run. They +load `adopt-project.py` through importlib and stub its probe for the tool, as +`test_adopt_execute_refusals.py` does: every refusal under test is raised +before the tool would be used. + +PLAN PATHS STAY POSIX, and a name that only a POSIX disk can hold (a tab, a +`?`) is created only where `os.name` is not `nt`. A name outside ASCII is +created only where the file system's encoding can spell it, which on every +CI runner it can (Windows is UTF-8 since PEP 529). +""" + +from __future__ import annotations + +import importlib.util +import os +import shutil +import subprocess +import sys +from argparse import Namespace +from pathlib import Path + +import pytest + +from conftest import ADOPT, REPO, git, make_source_repo, run_script, write_plan + +PROJECT = "Northwind" +UTF8 = "utf-8" +UNSAFE = "plan-unsafe-path" +REMOTES, WORK = "remotes", "work" +POSIX_ONLY = pytest.mark.skipif( + os.name == "nt", reason="NTFS cannot hold this character in a file name") +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +#: A name outside ASCII, in the Latin-1 range, so that a console still on a +#: Windows code page can print `plan`'s table of it too. +UMLAUT = "\u00dcbersicht.md" + + +def spellable(name: str) -> bool: + """Whether this file system's encoding can spell `name` at all.""" + try: + name.encode(sys.getfilesystemencoding()) + except UnicodeEncodeError: + return False + return True + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_entry_paths", ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture(autouse=True) +def no_filter_repo_probe(adopter, monkeypatch): + """No test here that runs `cmd_execute` in process gets as far as using + `git filter-repo`; the refusal before it is what is under test.""" + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + + +def execute_args(tmp_path: Path, plan: Path) -> Namespace: + """What `main` hands `cmd_execute`: `--yes`, a local remote directory that + does not exist yet, and a work directory.""" + return Namespace(plan=str(plan), source=None, + local_remote_dir=tmp_path / REMOTES, allow_empty_leg=[], + yes=True, work_dir=tmp_path / WORK) + + +def no_leg_was_created(tmp_path: Path) -> None: + assert not (tmp_path / REMOTES).exists() + for role in ("spec", "code"): + assert not (tmp_path / WORK / f"{PROJECT}-{role}").exists() + + +def run_check(plan: Path): + return run_script(ADOPT, "check", "--plan", str(plan)) + + +def answer(plan: Path, spelled: str, old: str, new: str) -> None: + """Re-answer the entry whose `path:` is written `spelled`, from `old` to + `new`, LF as `plan` wrote it (see `conftest.resolve`).""" + text = plan.read_text(encoding=UTF8) + needle = f" - path: {spelled}\n leg: {old}\n" + assert needle in text, f"{spelled} is not an entry with leg: {old}" + with plan.open("w", encoding=UTF8, newline="\n") as handle: + handle.write(text.replace( + needle, f" - path: {spelled}\n leg: {new}\n" + " resolution: \"answered by the test\"\n", 1)) + + +def planned(tmp_path: Path, tree: dict, + edits: tuple = ()) -> tuple[Path, Path]: + """`(source, plan)` for a source holding `tree`, planned as a person + would, with nothing left to answer.""" + 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 + assert "leg: null" not in plan.read_text(encoding=UTF8) + return source, plan + + +# --- the one test ---------------------------------------------------------- + +ADMITTED = [ + "My Notes.md", "docs/User Guide.md", "docs/", UMLAUT, "\u6587\u66f8.md", + "#notes.md", "q?.md", "*", "[id].tsx", ":!keep.md", "a-b.md", + "semi;colon$(x)'\"&.md", "a+b~c@d.md", " lead.md", "trail.md ", + "back\\slash.md", "regex:x.md", "glob:y.md", "literal:z.md", "a=>b.md", + "x==y.md", +] + +#: What git or `git filter-repo` would NOT take as the path it is, with what +#: the finding says: the misreadings `entry_path_problem` refuses. +MISREAD = [ + ("", "it is empty: a `literal:` line with nothing after it matches"), + ("docs\nregex:.*", "line feed (`\\n`), which ends its line"), + ("Icon\r", "ends in a carriage return (`\\r`), which `git filter-repo` " + "strips from the end of each line of the path list it reads, " + "so its line would name `Icon`, another path"), + # Nothing is left once the carriage returns are stripped: the line is + # `literal:` alone, which matches every path, as the empty path's does. + ("\r", "so its line would be `literal:` alone, which matches every path, " + "so `git filter-repo` would put the whole tree in the leg"), + ("\r\r", "so its line would be `literal:` alone, which matches every"), + ("nul\x00", "NUL (`\\x00`), which no command line can carry"), + # A surrogate no byte of a path is read as: `write_paths_listing` cannot + # write it. U+DC80 to U+DCFF, the bytes that are not UTF-8, are POLICY's. + ("\ud800.md", "`\\ud800`, a surrogate no byte of a path is read as"), + ("x\udc7f.md", "`\\udc7f`, a surrogate no byte of a path is read as"), + ("y\udd00.md", "`\\udd00`, a surrogate no byte of a path is read as"), + ("a==>b.md", "`==>`"), + ("docs/a==>b/", "`==>`"), + # A misreading is told before a policy reason the same name also has. + ("tab\t==>b.md", "`==>`"), +] + +#: What git and `git filter-repo` take literally -- measured, each as one +#: `literal:` line and as `git --literal-pathspecs rm` -- and this tool still +#: refuses to move into a leg, as policy, with what the finding says. +POLICY = [ + (" ", "nothing but whitespace"), + (" ", "nothing but whitespace"), + ("-dash.md", "begins with `-`"), + ("--output=never-written", "begins with `-`"), + ("tab\there.md", "control character `\\t`"), + ("a\rb.md", "control character `\\r`"), + ("esc\x1b[31m.md", "control character `\\x1b`"), + ("del\x7f.md", "control character `\\x7f`"), + ("nel\x85.md", "control character `\\x85`"), + ("vt\x0b.md", "control character `\\x0b`"), + ("line\u2028sep.md", "line separator `\\u2028`"), + # A byte that is not UTF-8, as `Source.tree` keeps it: since #199 the + # path list carries it byte-exact. + ("caf\udce9.md", "`\\xe9`, which is not UTF-8"), + ("docs/\udcff/", "`\\xff`, which is not UTF-8"), + ("\udc80.md", "`\\x80`, which is not UTF-8"), +] + + +@pytest.mark.parametrize("path", ADMITTED) +def test_what_a_source_tree_holds_is_admitted(adopter, path): + """A space, a letter outside ASCII, a `#`, a wildcard, a leading `:` and + a filter-repo directive's spelling: each reaches `git` literally, as a + `literal:` line of the filter's path list and a pathspec after `--` to + `git --literal-pathspecs rm`.""" + assert adopter.entry_path_problem(path) is None + + +@pytest.mark.parametrize("path,said", MISREAD) +def test_what_git_or_filter_repo_would_misread_is_refused(adopter, path, + said): + """Each finding says what would be misread, and never calls it policy.""" + problem = adopter.entry_path_problem(path) + assert problem is not None and said in problem + assert "as policy" not in problem + + +@pytest.mark.parametrize("path,said", POLICY) +def test_what_policy_refuses_says_it_is_policy_and_not_a_misreading( + adopter, path, said): + """A name git and `git filter-repo` take literally is refused as policy, + and the finding says so: it must not give a misreading's reason, as it + did when a tab was told that a line break would end its line.""" + problem = adopter.entry_path_problem(path) + assert problem is not None and said in problem + assert ("which `execute` refuses as policy, though git and `git " + "filter-repo` would take it literally") in problem + for misreading in ("line of its own", "ends its line", "strips", + "matches every path", "rename"): + assert misreading not in problem + + +# --- `check` and `execute` agree ------------------------------------------- + +def test_a_source_with_spaces_and_letters_beyond_ascii_checks_ok_and_runs( + adopter, tmp_path): + """THE ISSUE'S OWN CASE, where every runner can make it: `check` prints + `plan ok`, and `execute` raises no refusal over the entries' paths and + hands each one on as the source spells it.""" + tree = {"My Notes.md": "notes\n", "docs/User Guide.md": "guide\n", + "docs/architecture.md": "arch\n", "run tests.py": "print(1)\n", + "src/app.py": "VALUE = 1\n", "README.md": "# Thing\n"} + if spellable(UMLAUT): + tree[UMLAUT] = "ueber\n" + source, plan = planned(tmp_path, tree) + checked = run_check(plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + assert "plan ok" in checked.stdout + loaded = adopter.Plan.load(plan) + assert adopter._refuse_what_check_finds( + loaded, loaded.open_source(str(source), tmp_path / "read")) is None + paths_for = adopter._leg_paths(loaded) + assert "My Notes.md" in paths_for["spec"] + assert "docs/" in paths_for["spec"] + assert "run tests.py" in paths_for["code"] + if spellable(UMLAUT): + assert UMLAUT in paths_for["spec"] + + +#: Hand edits that put a path `execute` cannot extract where `specs/` was, +#: each as the plan file spells it and as the finding must name it: one for +#: each class `entry_path_problem` refuses. An empty path is the one whose +#: `literal:` line would be worst: `git filter-repo` matches an empty path +#: against every path, so the leg would take the whole tree. +HAND_EDITS = { + "empty": ('""', "``", "it is empty"), + "whitespace-only": ('" "', "` `", "nothing but whitespace"), + "leading-dash": ("--output=never-written", "`--output=never-written`", + "begins with `-`"), + "control-character": ('"spe\\tcs/"', "`spe\\tcs/`", + "control character `\\t`"), + "rename-arrow": ('"specs==>elsewhere/"', "`specs==>elsewhere/`", "`==>`"), +} +ONE_PER_EDIT = pytest.mark.parametrize("edit", sorted(HAND_EDITS)) + + +def hand_edited(tmp_path: Path, edit: str) -> tuple[Path, Path]: + """`(source, plan)`: the synthetic source, its questions answered, and + the `specs/` entry's path replaced as `HAND_EDITS[edit]` says.""" + source = make_source_repo(tmp_path / "Thing") + plan = tmp_path / "adoption-plan.yaml" + assert write_plan(source, plan, project=PROJECT).returncode == 0 + text = plan.read_text(encoding=UTF8) + for path, leg in (("examples/", "spec"), (".claude/", "root"), + ("release.yaml", "root")): + text = text.replace(f" - path: {path}\n leg: null\n", + f" - path: {path}\n leg: {leg}\n", 1) + assert "leg: null" not in text + spelled = HAND_EDITS[edit][0] + text = text.replace(" - path: specs/\n", f" - path: {spelled}\n", 1) + with plan.open("w", encoding=UTF8, newline="\n") as handle: + handle.write(text) + return source, plan + + +@ONE_PER_EDIT +def test_check_reports_the_path_first_and_says_what_to_do(tmp_path, edit): + """The finding names the path as `_spelled` spells it, comes before the + uncovered files the same edit leaves, and the remediation says the plan + cannot respell a path of the source.""" + _, plan = hand_edited(tmp_path, edit) + _, named, said = HAND_EDITS[edit] + result = run_check(plan) + assert result.returncode == 1, result.stderr + result.stdout + findings = [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + assert findings[0].startswith(f"FINDING {UNSAFE}: {named} has leg: spec") + assert said in findings[0] + assert any(line.startswith("FINDING plan-uncovered: `specs/") + for line in findings) + assert "Rename the path in the source, commit, and re-run `plan`" \ + in result.stderr + assert "plan ok" not in result.stdout + + +@ONE_PER_EDIT +def test_execute_refuses_it_in_checks_words_before_any_leg(adopter, tmp_path, + edit): + _, plan = hand_edited(tmp_path, edit) + printed = [line for line in run_check(plan).stderr.splitlines() + if line.startswith("FINDING ")] + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(execute_args(tmp_path, plan)) + assert caught.value.code == UNSAFE + assert f" {printed[0]}" in caught.value.detail.splitlines() + assert "Rename the path in the source" in caught.value.remediation + no_leg_was_created(tmp_path) + + +@pytest.mark.parametrize("answered", ["root", "drop"]) +def test_an_entry_answered_root_or_drop_is_not_judged(adopter, tmp_path, + answered): + """A path that is never extracted reaches no line of the filter's list: + `root` keeps it where it is and `drop` hands it to `git rm` after `--`. + So the remediation's `drop` is true; answered `spec` it is refused.""" + source, plan = planned(tmp_path, {"-notes.md": "notes\n", + "src/app.py": "VALUE = 1\n", + "docs/spec.md": "# spec\n"}) + findings = [line for line in run_check(plan).stderr.splitlines() + if line.startswith(f"FINDING {UNSAFE}:")] + assert findings == [f"FINDING {UNSAFE}: `-notes.md` has leg: spec, and " + "`execute` will not extract it into a leg: " + f"{adopter.entry_path_problem('-notes.md')}"] + assert "refuses as policy" in findings[0] + answer(plan, '"-notes.md"', "spec", answered) + checked = run_check(plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + loaded = adopter.Plan.load(plan) + assert adopter._refuse_what_check_finds( + loaded, loaded.open_source(str(source), tmp_path / "read")) is None + + +@POSIX_ONLY +def test_a_control_character_in_the_source_is_found_and_refused(adopter, + tmp_path): + """Not a hand edit: the SOURCE holds `notes\\t.md`, `plan` writes it into + the plan as it stands, and the plan parses. `check` names it and + `execute` refuses it before any leg exists.""" + source, plan = planned(tmp_path, {"notes\t.md": "notes\n", + "src/app.py": "VALUE = 1\n"}) + result = run_check(plan) + assert result.returncode == 1, result.stderr + result.stdout + assert f"FINDING {UNSAFE}: `notes\\t.md` has leg: spec" in result.stderr + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(execute_args(tmp_path, plan)) + assert caught.value.code == UNSAFE + no_leg_was_created(tmp_path) + assert git("branch", "--format=%(refname:short)", + cwd=source).stdout.split() == ["main"] + + +# --- a byte that is not UTF-8 --------------------------------------------- + +#: A name holding the byte 0xE9 -- Latin-1's e-acute, and not UTF-8 -- below +#: `docs/`, so that `plan` writes the entry `docs/` and never the name itself. +NOT_UTF8 = b"docs/caf\xe9.md" + + +def with_a_name_that_is_not_utf8(adopter, tmp_path: Path): + """`(source, plan)`: a source whose `docs/` holds `NOT_UTF8`, and its + plan, LOADED, with `docs/` split into an entry for each file in it, as a + person splitting the directory would write it. + + The name is put straight into git's index, as `git add` records it, so + that no file system has to hold it (APFS refuses one). Before #199 no + plan FILE could carry it -- `plan` raised writing a surrogate, and the + reader had no escape that gives one back -- and #199's writer spells it + `"docs/caf\\udce9.md"`, which its reader returns as the lone surrogate: + what the loaded entry holds here, with #199 or without it. Skipped + where git will not record the name or keep it in the tree, which only + git can say.""" + source = make_source_repo(tmp_path / "Thing", + {"docs/architecture.md": "arch\n", + "src/app.py": "VALUE = 1\n", + "README.md": "# Thing\n"}, edits=()) + blob = subprocess.run(["git", "hash-object", "-w", "--stdin"], + cwd=str(source), input=b"latin1\n", + capture_output=True, check=True).stdout.strip() + added = subprocess.run( + ["git", "update-index", "--add", "-z", "--index-info"], + cwd=str(source), input=b"100644 " + blob + b"\t" + NOT_UTF8 + b"\0", + capture_output=True, check=False) + if added.returncode: + pytest.skip(f"git update-index refuses {NOT_UTF8!r} here: " + f"{added.stderr!r}") + git("-c", "user.name=Source Human", + "-c", "user.email=human@source.invalid", + "commit", "-q", "-m", "Add a name that is not UTF-8", cwd=source) + held = subprocess.run(["git", "ls-tree", "-r", "-z", "--name-only", + "HEAD"], cwd=str(source), capture_output=True, + check=True).stdout.split(b"\0") + if NOT_UTF8 not in held: + pytest.skip(f"git does not keep {NOT_UTF8!r} in a tree here") + plan = tmp_path / "adoption-plan.yaml" + written = write_plan(source, plan, project=PROJECT) + assert written.returncode == 0, written.stderr + written.stdout + loaded = adopter.Plan.load(plan) + docs = next(e for e in loaded.entries if e.get("path") == "docs/") + at = loaded.entries.index(docs) + loaded.entries[at:at + 1] = [ + {**docs, "path": "docs/architecture.md"}, + {**docs, "path": NOT_UTF8.decode(UTF8, "surrogateescape")}] + return source, loaded + + +#: The finding `check` must print first for `NOT_UTF8`, spelled as +#: `_spelled` spells a byte that is not UTF-8. +NOT_UTF8_FINDING = ( + f"FINDING {UNSAFE}: `docs/caf\\xe9.md` has leg: spec, and `execute` " + "will not extract it into a leg: it holds `\\xe9`, which is not UTF-8 " + "and which `execute` refuses as policy, though git and `git filter-repo` " + "would take it literally: a name that holds one prints as an escape and " + "never as itself, the split commit's message would list it with a `?` " + "for that byte, and adopting one is not yet measured on macOS or Windows") + + +def test_a_byte_that_is_not_utf8_is_found_and_refused_before_any_leg( + adopter, tmp_path, monkeypatch, capsys): + """CATEGORY Cs. `Source.tree` keeps a byte that is not UTF-8 as a lone + surrogate. Before #199's `write_paths_listing`, on this branch merged + with #199's first head, `check` said `plan ok` and `execute` raised + `UnicodeEncodeError` writing the path list, after both leg repositories + existed. With it the name is extracted byte for byte, and it is still + refused, as policy, until adopting it end to end is measured on macOS + and Windows: `check` names it first, and `execute` refuses it in + `check`'s words before any leg exists.""" + source, loaded = with_a_name_that_is_not_utf8(adopter, tmp_path) + monkeypatch.setattr(adopter.Plan, "load", staticmethod(lambda _: loaded)) + assert adopter.cmd_check(Namespace(plan=str(loaded.path), source=None, + work_dir=tmp_path / "read")) == 1 + findings = [line for line in capsys.readouterr().err.splitlines() + if line.startswith("FINDING ")] + assert findings == [NOT_UTF8_FINDING] + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(execute_args(tmp_path, loaded.path)) + assert caught.value.code == UNSAFE + assert f" {NOT_UTF8_FINDING}" in caught.value.detail.splitlines() + no_leg_was_created(tmp_path) + assert git("branch", "--format=%(refname:short)", + cwd=source).stdout.split() == ["main"] + + +# --- the split commit's message -------------------------------------------- + +#: Run in a child whose locale is NOT UTF-8 and whose Python is not in UTF-8 +#: mode: cp1252 on Windows, ASCII under a POSIX `C` locale. `env_commit` must +#: hand git the message as UTF-8 there too. +#: The message is a Python literal in the code, spelled in ASCII: argv would +#: be decoded in that same foreign locale before `env_commit` saw it. +COMMIT_IN_A_FOREIGN_LOCALE = """ +import sys +from pathlib import Path +sys.path.insert(0, sys.argv[1]) +from shape_materialize import env_commit +env_commit(Path(sys.argv[2]), {message}) +""" + + +def test_the_split_message_reaches_git_as_utf8_whatever_the_locale(tmp_path): + """The split commit's message names every entry path it moved. On main at + e4decfb `env_commit` wrote it to `git commit -F -` in the locale's codec, + so this child raised `UnicodeEncodeError` -- after both legs had been + pushed, in a real run.""" + repo = tmp_path / "repo" + repo.mkdir() + git("init", "-q", "-b", "main", ".", cwd=repo) + (repo / "f.txt").write_text("x\n", encoding=UTF8) + git("add", "f.txt", cwd=repo) + message = f"Split\n\n {UMLAUT}\n \u6587\u66f8.md\n My Notes.md\n" + env = {**os.environ, "PYTHONUTF8": "0"} + if os.name != "nt": + env.update({"LC_ALL": "C", "LANG": "C", "PYTHONCOERCECLOCALE": "0"}) + child = subprocess.run( + [sys.executable, "-c", + COMMIT_IN_A_FOREIGN_LOCALE.format(message=ascii(message)), + str(REPO / "scripts"), str(repo)], + capture_output=True, check=False, env=env) + assert child.returncode == 0, child.stderr.decode(UTF8, "replace") + raw = subprocess.run(["git", "cat-file", "commit", "HEAD"], cwd=str(repo), + capture_output=True, check=True).stdout + body = raw.split(b"\n\n", 1)[1] + assert body.decode(UTF8) == message + + +# --- end to end ------------------------------------------------------------ + +#: Names only a POSIX disk holds, each with the leg the test gives it. `?`, +#: `*`, `"`, `:`, `>` and `\\` are not file-name characters on NTFS, and +#: Windows drops a trailing space. Beside each wildcard and `:!` name is a +#: file it would match as a pattern, which the plan keeps in the root, and +#: which SORTS BEFORE it: git stops reading a pathspec as a pattern once it +#: has matched a path exactly, and `git rm` walks the index in order, so a +#: decoy sorting after the name (`star-kept.md`) was never at risk. +POSIX_NAMES = { + "q?.md": "spec", "q1.md": "root", + "star*.md": "spec", "star(1).md": "root", + ":!keep.md": "spec", "keep.md": "root", + 'say "hi".md': "spec", " lead.md": "spec", "trail.md ": "spec", + "back\\slash.md": "spec", "glob:y.md": "spec", "regex:x.md": "spec", + "literal:z.md": "spec", "a=>b.md": "spec", +} +#: How `plan` answers the names above where the test answers otherwise: +#: `(as the plan file spells it, plan's answer, the test's answer)`. +POSIX_ANSWERS = (("q1.md", "spec", "root"), ('"star(1).md"', "spec", "root"), + ("keep.md", "spec", "root"), ('"trail.md "', "null", "spec")) + + +@needs_filter_repo +def test_a_source_with_such_names_is_adopted_and_verified(tmp_path): + """THE ACCEPTANCE: plan, check and execute a source whose entries hold + every kind of name `entry_path_problem` admits -- a space, a letter + outside ASCII, a leading `#`, `;`, `$`, `(`, `&`, a quote, a wildcard, a + leading `:`, a leading or trailing space, a backslash, and the spelling + of each `git filter-repo` directive -- to `adoption verified`, each file + in its leg and its history followable there. + + `#notes.md` is what the filter's `literal:` lines are for: as a bare line + it is a comment, the spec leg came out without it, and the split had + removed it from the root. So are `glob:y.md` and `literal:z.md`, which a + bare line reads as the pattern `y.md` and the path `z.md`. `[ID].tsx` + beside `D.tsx` is what `--literal-pathspecs` is for: as a pathspec + `[ID]` is a character class, so `git rm -- '[ID].tsx'` removed `D.tsx` + too, though the plan keeps it in the root; and so, where the disk + allows, did `q?.md` and `star*.md` the files beside them, while + `git rm -r -- ':!keep.md'` removed every path but `keep.md` and the run + died at the next `git rm`, after both legs were pushed. `-notes.md` is + dropped, which reaches only `git rm`, after `--`.""" + tree = {"My Notes.md": "notes\n", "docs/User Guide.md": "guide\n", + "docs/architecture.md": "arch\n", "#notes.md": "hash\n", + "-notes.md": "dash\n", "run tests.py": "print(1)\n", + "src/app.py": "VALUE = 1\n", "README.md": "# Thing\n", + "semi;colon$(x)'&.md": "shell\n", "[ID].tsx": "page\n", + "D.tsx": "kept in the root\n"} + spec_files = {"My Notes.md", "docs/User Guide.md", "docs/architecture.md", + "#notes.md", "semi;colon$(x)'&.md"} + root_files = {"README.md", "D.tsx"} + if spellable(UMLAUT): + tree[UMLAUT] = "ueber\n" + spec_files.add(UMLAUT) + if os.name != "nt": + tree.update({name: f"{name}\n" for name in POSIX_NAMES}) + spec_files.update(name for name, leg in POSIX_NAMES.items() + if leg == "spec") + root_files.update(name for name, leg in POSIX_NAMES.items() + if leg == "root") + edits = (("My Notes.md", "notes\n\nA second paragraph.\n", + "Extend My Notes"), + ("docs/User Guide.md", "guide\n\nA second section.\n", + "Extend the User Guide")) + 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 + answer(plan, '"-notes.md"', "spec", "drop") + answer(plan, "D.tsx", "code", "root") + if os.name != "nt": + for spelled, old, new in POSIX_ANSWERS: + answer(plan, spelled, old, new) + assert "leg: null" not in plan.read_text(encoding=UTF8) + checked = run_check(plan) + assert checked.returncode == 0, checked.stderr + checked.stdout + + remotes = tmp_path / REMOTES + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / WORK)) + assert result.returncode == 0, result.stderr + result.stdout + assert "adoption verified" in result.stdout + assert "drop 1 of" in result.stdout + + def files(repo: Path, rev: str) -> set[str]: + raw = subprocess.run(["git", "ls-tree", "-r", "-z", "--name-only", + rev], cwd=str(repo), capture_output=True, + check=True).stdout + return {name.decode(UTF8) for name in raw.split(b"\0") if name} + + spec_leg = remotes / f"{PROJECT}-spec.git" + assert files(spec_leg, "main") == spec_files + assert files(remotes / f"{PROJECT}-code.git", "main") == { + "run tests.py", "src/app.py", "[ID].tsx"} + root = files(source, "adopt/three-repo-shape") + assert root_files <= root and "-notes.md" not in root + assert not (spec_files | {"[ID].tsx"}) & root + + for path, edited in (("My Notes.md", "Extend My Notes"), + ("docs/User Guide.md", "Extend the User Guide")): + log = subprocess.run(["git", "log", "--follow", "--format=%s", "main", + "--", path], cwd=str(spec_leg), + capture_output=True, check=True + ).stdout.decode(UTF8).splitlines() + assert log == [edited, "Initial import"], f"{path}: {log}" diff --git a/tests/test_adopt_execute_refusals.py b/tests/test_adopt_execute_refusals.py index bc267f48..4c4e5b79 100644 --- a/tests/test_adopt_execute_refusals.py +++ b/tests/test_adopt_execute_refusals.py @@ -414,16 +414,21 @@ def test_a_stale_plan_is_still_reported_as_stale(adopter, world): def test_a_path_that_is_a_git_option_is_still_unsafe_not_uncovered(adopter, world): - """THE PLAN IS UNTRUSTED INPUT and its paths are checked as `git` - arguments first. This plan is also uncovered -- `specs/` has no entry -- - and would be refused as that, losing the reason it is dangerous, if the - coverage refusal ran before them. (`test_adopt_e2e.py` has the same case - behind `git filter-repo`; this one runs everywhere.)""" + """THE PLAN IS UNTRUSTED INPUT and its paths are judged first. This plan + is also uncovered -- `specs/` has no entry -- and would be refused as + that, losing the reason it is dangerous, if the coverage finding came + before them. Since #196 an entry's path is `check`'s own finding, + `plan-unsafe-path`, listed first, where it used to be `_leg_paths`' + `unsafe-value`; the uncovered files are named beside it. + (`test_adopt_e2e.py` has the same case behind `git filter-repo`; this one + runs everywhere.)""" world.plan.write_text(world.answered.replace( " - path: specs/\n", " - path: --output=never-written\n", 1), encoding=UTF8) refusal = refused_by(adopter, execute_args(world.tmp, world.plan)) - assert refusal.code == "unsafe-value" + assert refusal.code == "plan-unsafe-path" + assert "`--output=never-written` has leg: spec" in refusal.detail + assert "FINDING plan-uncovered: `specs/" in refusal.detail no_leg_was_created(world.tmp) diff --git a/tests/test_adopt_plan_escapes.py b/tests/test_adopt_plan_escapes.py index ba698100..e5a6d97f 100644 --- a/tests/test_adopt_plan_escapes.py +++ b/tests/test_adopt_plan_escapes.py @@ -451,9 +451,9 @@ def test_the_paths_listing_is_written_byte_exact(tmp_path): reason="git filter-repo is not installed: `pip install git-filter-repo`") def test_a_leg_extracts_a_path_that_is_not_utf8(adopter, tmp_path): """`_extract_leg` itself, given an entry path holding the byte 0xE9: - the leg holds the name byte for byte. On this branch `execute` refuses - such an entry before any leg exists (`SAFE_ARG_RE`); a `check` that - lets it through (#196) reaches this, so the extraction is tested here, + the leg holds the name byte for byte. `execute` refuses such an entry + before any leg exists (as policy, `plan-unsafe-path`, since #196); + lifting that refusal reaches this, so the extraction is tested here, directly. Skipped where no file can be named with that byte (Windows, macOS's APFS): filter-repo works in a clone, which checks the name out.""" try: diff --git a/tests/test_adopt_submodules.py b/tests/test_adopt_submodules.py index 695524cc..d97d01cc 100644 --- a/tests/test_adopt_submodules.py +++ b/tests/test_adopt_submodules.py @@ -898,7 +898,14 @@ def test_a_name_no_plan_line_can_hold_is_spelled_in_the_question( def test_a_newline_spelled_in_the_question_is_the_entry_to_write(tmp_path): """The spelling is the entry: `src/a\\nb.py` between double quotes is a YAML scalar holding the newline, so the split the question names covers - the file, where the name read back as `src/a b.py` covered nothing.""" + the file, where the name read back as `src/a b.py` covered nothing. + + COVERING IT IS NOT EXTRACTING IT (#196). A line break would end the + path's line in the list `git filter-repo` reads, so answered `code` the + entry is `plan-unsafe-path` and nothing else -- no file uncovered, no + entry covering nothing -- where on main at e4decfb `check` said `plan ok` + and `execute` refused the same plan as `unsafe-value`. Answered `drop`, + as the remediation offers, it checks ok.""" source = raw_source(tmp_path, b"src/vendored", b"src/a\nb.py") plan = planned(source, tmp_path)[0] named = REPLACE_WITH_RE.search(plan_rows(plan)["src/"]["question"]) @@ -907,7 +914,37 @@ def test_a_newline_spelled_in_the_question_is_the_entry_to_write(tmp_path): split = answered(plan, tmp_path, {GITMODULES: "code"}) edited(split, "src/", {f'"{path}"': "code" for path in own}) result = check_plan(split) + assert findings(result.stderr) == [( + "plan-unsafe-path", "`src/a\\nb.py` has leg: code, and `execute` " + "will not extract it into a leg: it holds a line feed (`\\n`), " + "which ends its line of the path list `git filter-repo` reads, so " + "the rest of the name would be read as a line of its own")], \ + result.stderr + newline_entry = f'"{own[0]}"' + edited(split, newline_entry, {newline_entry: "drop"}) + result = check_plan(split) + assert result.returncode == 0, result.stderr + + +def test_a_space_in_a_name_the_question_names_is_extracted(adopter, tmp_path): + """#196's own case of #166's split: `docs/User Guide.md` beside a + submodule at `docs/theme`. The question names it as an entry to write; + written so, the plan checks ok and `execute` refuses nothing over the + entries' paths, where on main at e4decfb it refused `docs/User Guide.md` + as `unsafe-value` after `check` had said `plan ok`.""" + source = raw_source(tmp_path, b"docs/theme", b"docs/User Guide.md") + plan = planned(source, tmp_path)[0] + named = REPLACE_WITH_RE.search(plan_rows(plan)["docs/"]["question"]) + own = re.findall(r"`([^`]+)`", named[1]) + assert own == ["docs/User Guide.md", "docs/theme"] + split = answered(plan, tmp_path, {GITMODULES: "spec"}) + edited(split, "docs/", {f'"{path}"': "spec" for path in own}) + result = check_plan(split) assert result.returncode == 0, result.stderr + loaded = adopter.Plan.load(split) + assert adopter._refuse_what_check_finds( + loaded, loaded.open_source(None, tmp_path / "read")) is None + assert adopter._leg_paths(loaded)["spec"][:2] == own @pytest.mark.parametrize("gitlink, beside", [ @@ -1081,10 +1118,21 @@ def test_an_unregistered_submodule_is_told_why_git_ignores_its_entry( adopter, tmp_path, source, answers, submodule, why): """#166's N2. An entry of `.gitmodules` that names the submodule's path and that git ignores is named, with the reason, rather than "no entry" - beside an entry the person can see.""" + beside an entry the person can see. + + A submodule at `-dep` is also an entry path that begins with `-`, which + no leg takes (#196): `plan-unsafe-path` comes first, as it does for every + plan, and the reason git ignores the entry is still said after it.""" plan = answered(planned(registered_source(tmp_path, **source), tmp_path)[0], tmp_path, answers) - assert findings(check_plan(plan).stderr) == [( + # `submodule` is spelled as a finding names it (#212); the entry's own + # path is the case's `orphan`, where it has one. + path = source.get("orphan", "") + unsafe = [("plan-unsafe-path", + f"{submodule} has leg: code, and `execute` will not extract " + f"it into a leg: {adopter.entry_path_problem(path)}") + ] if path.startswith("-") else [] + assert findings(check_plan(plan).stderr) == unsafe + [( UNREGISTERED, f"the submodule {submodule} has leg: code but {why}, " + adopter.NO_URL_IN_THE_LEG)]