diff --git a/adopt-project.py b/adopt-project.py index 3ed2498e..f7fb4de6 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1069,6 +1069,22 @@ def cross_leg_references(source: "Source", entries: list["Entry"], the spec leg, and the code that reads them reads ACROSS the assembly root mount. The read is one relative path from the code leg — but only once somebody edits it, so the plan says which file and the human decides when. + + EVERY NAME IS A NAME (#216). Both inputs are source paths, and git reads + each as something else unless told not to. A spec directory passed bare + sits where git reads OPTIONS: `-notes/` was a usage error (exit 129, no + follow-up), and `-O/` was `--open-files-in-pager`, which git runs + through a shell -- argv and `shell=False` stop Python running a shell, + not git -- so `-e` makes it the pattern. The code entries are pathspecs: + `:cache` read as the pathspec `cache` and `*` matched files in the spec + leg, so `--literal-pathspecs`. `-l` C-quoted a name outside printable + ASCII (`"src/\\303\\234bersicht.py"`), so `-z`, read as `Source.tree` + reads names, and each hit is spelled as a question spells a path, with + no backticks: one line of UTF-8 whatever its bytes; so is the spec + directory's own name. `--no-color`, because `color.grep=always` wraps + each name in escapes. A grep that FAILS is said, never read as "no file + names it", and so is one that cannot be asked: `-F` splits a pattern at + a line feed. """ code_paths = [e.path.rstrip("/") for e in entries if e.leg == "code"] spec_dirs = [e.path for e in entries @@ -1077,29 +1093,55 @@ def cross_leg_references(source: "Source", entries: list["Entry"], return [] out: list[str] = [] for spec_dir in spec_dirs: + named = "".join(map(_spelled_character, spec_dir)) + if "\n" in spec_dir: + # `-F` reads a pattern holding a line feed as one pattern per + # line: `ab/` was searched as `a` OR `b/` and named files + # that never mention it, the way `*` did. + out.append(_search_by_hand( + named, "`git grep` reads each line of a pattern as a pattern " + "of its own, so it cannot search for this name")) + continue proc = subprocess.run( - ["git", "grep", "-l", "-I", "-F", spec_dir, source.commit, "--", - *code_paths], - cwd=str(source.path), capture_output=True, text=True, check=False) - hits = [line.split(":", 1)[1] for line in proc.stdout.splitlines() - if ":" in line] + ["git", "--literal-pathspecs", "grep", "--no-color", "-z", "-l", + "-I", "-F", "-e", spec_dir, source.commit, "--", *code_paths], + cwd=str(source.path), capture_output=True, check=False) + if proc.returncode not in (0, 1): + why = _one_line(proc.stderr.decode("utf-8", "replace"))[:300] + out.append(_search_by_hand( + named, f"`git grep` exited {proc.returncode}: {why}")) + continue + # `:` per record; the commit is hex, so the first `:` + # ends it and the rest is the path, raw. + hits = [_git_path(record.partition(b":")[2]) + for record in proc.stdout.split(b"\x00") if b":" in record] # Fixtures last. A test fixture that mentions `contracts/` is usually # quoting a path rather than reading one, and burying the harness that # DOES read it under twenty of them is how a follow-up list stops # being read at all. hits.sort(key=lambda h: ("fixture" in h or h.startswith("tests/"), h)) for hit in hits[:8]: + shown = "".join(map(_spelled_character, hit)) out.append( - f"{code_path}/{hit} names `{spec_dir}`, which moves to the " + f"{code_path}/{shown} names `{named}`, which moves to the " f"spec leg: it must read across the assembly root — " - f"`../{spec_path}/{spec_dir}` relative to `{code_path}/` — or " + f"`../{spec_path}/{named}` relative to `{code_path}/` — or " "take the path from the environment") if len(hits) > 8: out.append(f"… and {len(hits) - 8} more file(s) under the code leg " - f"naming `{spec_dir}`") + f"naming `{named}`") return out +def _search_by_hand(named: str, why: str) -> str: + """The follow-up for a spec directory git did not search for: the + reason, and what to look for instead.""" + return (f"git could not search the files moving to the code leg for " + f"`{named}` ({why}); find the ones that name it by hand: each " + f"must read across the assembly root once `{named}` moves to the " + "spec leg") + + def follow_ups_for(source: "Source", entries: list["Entry"], names: dict, spec_path: str, code_path: str, collisions: list[str]) -> list[str]: @@ -3556,12 +3598,22 @@ def _stage_the_split(assembly: Path, materialized: Materialized) -> None: is staged as `_where_it_landed` spells it, and one whose link leads OUT of the assembly is not staged at all: it is not in this repository, and the verification reports the pinned file missing. + + AND EACH ONE IS A PATH, NOT A PATHSPEC (#216). A path that landed + through a link is the link's target, a name the SOURCE chose, and after + `--` git still reads a name as a pathspec: `:ci/workflows/validate.yml` + is the magic `:` and the path `ci/...`, which matches nothing and fails + the add (`fatal: pathspec ... did not match any files`) after both legs + are pushed; `:!ci/...` is an EXCLUDE, which forces nothing by that name, + so a file the source's `.gitignore` hides is left out of the split; and + `ga[0]/...` is a glob, which forces an ignored `ga0/...` too. So + `--literal-pathspecs`: every name is the path it spells. """ run(["git", "add", "-A", "--", "."], cwd=assembly) landed = (_where_it_landed(assembly, written) for written in materialized.written) - run(["git", "add", "-f", "--", *(path for path in landed if path)], - cwd=assembly) + run(["git", "--literal-pathspecs", "add", "-f", "--", + *(path for path in landed if path)], cwd=assembly) def _where_it_landed(assembly: Path, written: str) -> str | None: diff --git a/tests/test_adopt_ignored_shape_files.py b/tests/test_adopt_ignored_shape_files.py index 5a13e521..602ad61b 100644 --- a/tests/test_adopt_ignored_shape_files.py +++ b/tests/test_adopt_ignored_shape_files.py @@ -210,8 +210,8 @@ def test_the_forced_paths_are_the_written_ones_in_posix_spelling(adopter, assert calls == [ ["git", "add", "-A", "--", "."], - ["git", "add", "-f", "--", ".github/workflows/validate.yml", - "shape/.gitattributes"]] + ["git", "--literal-pathspecs", "add", "-f", "--", + ".github/workflows/validate.yml", "shape/.gitattributes"]] assert not any("\\" in part for call in calls for part in call) diff --git a/tests/test_adopt_literal_names.py b/tests/test_adopt_literal_names.py new file mode 100644 index 00000000..763bd0b4 --- /dev/null +++ b/tests/test_adopt_literal_names.py @@ -0,0 +1,364 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A source's name is a NAME wherever adopt hands it to git (#216). + +`plan`'s follow-ups come from `git grep` over the source commit: one search +per directory the plan sends to the spec leg, for that directory's name, +restricted to the entries it sends to the code leg. Both inputs are names the +source chose, and git read each as something else. The spec directory sat +where git reads OPTIONS: `-notes/` was a usage error that left no follow-up, +and `-O/` was `--open-files-in-pager`, a command git runs through a +shell. The code entries were PATHSPECS: `:cache` read as the magic `:` and +`*` matched files in the spec leg. `-l` C-quoted a name outside printable +ASCII, `color.ui=always` wrapped each name in escapes, a grep that failed +was read as "no file names it", and `-F` split a spec directory named across +a line feed into two patterns. Each of those is one test here, through `plan` +itself except the failing grep, which `plan` cannot provoke, and the line +feed, whose plan is #199's to write; each measured failing on main at +a5b09c3, and `notes/` and `s?c/` are the controls that already passed there. + +The split's forced `git add -f` asks the same question of a path the shape +wrote THROUGH a symlink the plan kept in the root: where it landed is the +link's target, a source name, and git read `:ci/...` as magic, `:!ci/...` as +an exclude and `ga[0]/...` as a glob. + +Local repositories only, no network, no `git filter-repo`. The tests whose +names an NTFS disk cannot hold (`:`, `*`, `?`, a line break) or that make a +symlink on the disk skip on Windows; the rest run there. +""" + +from __future__ import annotations + +import importlib.util +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import ADOPT, REPO, git, make_source_repo, write_plan + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import load_yaml # noqa: E402 +from shape_materialize import CommandFailed, Materialized # noqa: E402 + +#: Names Windows cannot put on its disk -- `:`, `*`, `?` and a line break are +#: illegal in an NTFS name, and git for Windows refuses them in the index +#: under its default `core.protectNTFS` -- and symlinks it checks out as +#: plain files. The defects these guard are POSIX layouts, and the POSIX legs +#: of CI run them. +posix_only = pytest.mark.skipif( + sys.platform == "win32", + reason="NTFS cannot hold this name, or this symlink, on the disk") + +#: A code file that names a spec directory, the way a harness names +#: `contracts/`. +READER = "SPEC = '{}'\n" +SPEC_PATH = "spec" +CODE_PATH = "code" + + +def follow_up(hit: str, spec_dir: str) -> str: + """The line `plan` writes for a code file that names `spec_dir`: the + wording main has always written, so an ordinary name is unchanged.""" + return (f"{CODE_PATH}/{hit} names `{spec_dir}`, which moves to the spec " + f"leg: it must read across the assembly root — " + f"`../{SPEC_PATH}/{spec_dir}` relative to `{CODE_PATH}/` — or " + "take the path from the environment") + + +def planned(source: Path, base: Path) -> dict: + out = base / "adoption-plan.yaml" + result = write_plan(source, out) + assert result.returncode == 0, result.stderr + result.stdout + return load_yaml(out) + + +def named(plan: dict) -> list[str]: + """Every follow-up that says a code file names a spec directory.""" + return [str(item) for item in plan["follow_ups"] if " names `" in str(item)] + + +def legs(plan: dict) -> dict: + return {str(entry["path"]): entry["leg"] for entry in plan["paths"]} + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_literal_names", ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +# --------------------------------------------------------------------------- +# `plan`'s follow-ups: the spec directory is a pattern, each code entry a path +# --------------------------------------------------------------------------- + +NOTES = {"notes/one.md": "# notes\n", "notes/two.md": "# more\n"} + +#: `(tree, the spec directory, the code entries, the files that name it)`. +#: The spec directories hold `.md` files and the code entries `.py` files, so +#: extension-majority puts each in the leg the case needs. +NAME_CLASSES = [ + pytest.param( + {**NOTES, "src/reader.py": READER.format("notes/"), + "src/other.py": "VALUE = 1\n"}, + "notes/", ["src/"], ["src/reader.py"], id="control-notes"), + pytest.param( + {"-notes/one.md": "# notes\n", "-notes/two.md": "# more\n", + "src/reader.py": READER.format("-notes/"), + "src/other.py": "VALUE = 1\n"}, + "-notes/", ["src/"], ["src/reader.py"], + id="a-spec-directory-starting-with-a-dash"), + pytest.param( + {**NOTES, ":cache/a.py": READER.format("notes/"), + ":cache/b.py": "VALUE = 2\n", "src/other.py": "VALUE = 1\n"}, + "notes/", [":cache/", "src/"], [":cache/a.py"], + id="a-code-entry-starting-with-a-colon", marks=posix_only), + pytest.param( + {"notes/one.md": "# notes\n", "notes/two.md": "see notes/ above\n", + "*/a.py": "VALUE = 1\n", "*/b.py": "VALUE = 2\n", + "src/reader.py": READER.format("notes/")}, + "notes/", ["*/", "src/"], ["src/reader.py"], + id="a-code-entry-named-star", marks=posix_only), + pytest.param( + {**NOTES, "sec/plan.md": "see notes/ for the plan\n", + "sec/other.md": "# other\n", "s?c/a.py": "VALUE = 1\n", + "s?c/b.py": "VALUE = 2\n", "src/reader.py": READER.format("notes/")}, + "notes/", ["s?c/", "src/"], ["src/reader.py"], + id="control-a-code-entry-with-a-question-mark", marks=posix_only), + pytest.param( + {**NOTES, "src/Übersicht.py": READER.format("notes/"), + "src/other.py": "VALUE = 1\n"}, + "notes/", ["src/"], ["src/Übersicht.py"], + id="a-code-file-named-outside-ascii"), +] + + +@pytest.mark.parametrize(("tree", "spec_dir", "code", "hits"), NAME_CLASSES) +def test_each_file_that_names_a_spec_directory_is_a_follow_up( + tmp_path, tree, spec_dir, code, hits): + """Exactly the code files that name the spec directory, each spelled as + the tree spells it. On main: `-notes/` was git's usage error (exit 129) + and no follow-up; `:cache` was searched as the pathspec `cache` and + found nothing; `*` also searched the spec leg and named + `code/notes/two.md`; and `Übersicht.py` was written + `code/"src/\\303\\234bersicht.py"`. `?` never widened, and does not.""" + source = make_source_repo(tmp_path / "Thing", tree=tree, edits=()) + plan = planned(source, tmp_path) + rows = legs(plan) + assert rows[spec_dir] == "spec", rows + assert all(rows[entry] == "code" for entry in code), rows + assert named(plan) == [follow_up(hit, spec_dir) for hit in hits] + + +def test_a_spec_directory_named_like_the_pager_option_runs_nothing(tmp_path): + """`-O` is `git grep --open-files-in-pager`, which git runs through + a shell. Read as that option, the directory's name made the planned + commit's hash the PATTERN, the WORKTREE was searched for it, and a + tracked file holding it was handed to ``: on main this created + `pager-ran` in the source and wrote no follow-up. Now the name is the + pattern, the commit is the commit, and nothing runs.""" + spec_dir = "-Otouch pager-ran;/" + source = make_source_repo( + tmp_path / "Thing", edits=(), + tree={f"{spec_dir}one.md": "# notes\n", f"{spec_dir}two.md": "# more\n", + "src/reader.py": READER.format(spec_dir), + "src/other.py": "VALUE = 1\n"}) + commit = git("rev-parse", "HEAD", cwd=source).stdout.strip() + # The premise the issue measured: a file under a code entry, in the index + # and holding the planned commit's hash. A clean checkout of that commit + # cannot hold its own hash, and git skips an untracked file. + (source / "src" / "scratch.txt").write_text(f"checked against {commit}\n", + encoding="utf-8") + git("add", "--", "src/scratch.txt", cwd=source) + before = sorted(os.listdir(source)) + + plan = planned(source, tmp_path) + + assert sorted(os.listdir(source)) == before, "git ran the pager" + assert legs(plan)[spec_dir] == "spec" + assert named(plan) == [follow_up("src/reader.py", spec_dir)] + + +def test_a_colour_setting_in_the_source_is_not_in_a_follow_up(tmp_path): + """`color.ui=always` in the source's own config made git colour each + name it listed even into a pipe: on main the follow-up read + `code/src/reader.py\\x1b[m names ...`, an escape byte in the plan.""" + source = make_source_repo( + tmp_path / "Thing", edits=(), + tree={**NOTES, "src/reader.py": READER.format("notes/"), + "src/other.py": "VALUE = 1\n"}) + git("config", "color.ui", "always", cwd=source) + plan = planned(source, tmp_path) + assert named(plan) == [follow_up("src/reader.py", "notes/")] + assert not any("\x1b" in str(item) for item in plan["follow_ups"]) + + +@posix_only +def test_a_name_with_a_line_break_or_a_byte_outside_utf8_is_one_spelled_line( + tmp_path): + """git's `-z` gives a name as its bytes, and the follow-up spells it as a + question spells a path: `\\n` for the line break, `\\xe9` for the byte, + on one line of UTF-8. On main each came back in git's C-quoted + spelling, quotes and octal included. The two files are put in the TREE + (`update-index -z --index-info`) and never on the disk, which macOS + refuses for a name that is not UTF-8.""" + source = make_source_repo( + tmp_path / "Thing", edits=(), + tree={**NOTES, "src/other.py": "VALUE = 1\n"}) + blob = subprocess.run( + ["git", "hash-object", "-w", "--stdin"], cwd=str(source), + input=READER.format("notes/").encode(), capture_output=True, + check=True).stdout.decode().strip() + records = b"".join(b"100644 " + blob.encode() + b"\t" + name + b"\x00" + for name in (b"src/line\nbreak.py", b"src/caf\xe9.py")) + subprocess.run(["git", "update-index", "-z", "--index-info"], + cwd=str(source), input=records, check=True) + git("-c", "user.name=Source Human", "-c", "user.email=human@source.invalid", + "commit", "-qm", "Two awkward names", cwd=source) + + plan = planned(source, tmp_path) + + assert named(plan) == [follow_up("src/caf\\xe9.py", "notes/"), + follow_up("src/line\\nbreak.py", "notes/")] + + +def test_a_grep_that_fails_is_said_and_never_read_as_no_file(adopter, + tmp_path): + """A `git grep` that exits past 1 has answered nothing. On main its + stdout was read anyway -- empty -- and the plan said nothing about the + spec directory; now the follow-up says git could not search, what git + said, and what to look for by hand. The commit here is one the source + does not have, which git refuses with exit 128.""" + path = make_source_repo( + tmp_path / "Thing", edits=(), + tree={**NOTES, "src/reader.py": READER.format("notes/"), + "src/other.py": "VALUE = 1\n"}) + source = adopter.Source.open(str(path), tmp_path / "work") + entries = adopter.walk(adopter.PathPolicy.load(adopter.PATH_POLICY), + [(name, size) for name, _, _, size in source.tree()]) + assert adopter.cross_leg_references(source, entries, SPEC_PATH, + CODE_PATH) \ + == [follow_up("src/reader.py", "notes/")] + + missing = adopter.Source(source.path, None, source.branch, "f" * 40) + said = adopter.cross_leg_references(missing, entries, SPEC_PATH, CODE_PATH) + + # git's own words differ between versions; its exit and its `fatal:` do + # not, and the sentence around them is this tool's. + assert len(said) == 1, said + assert said[0].startswith( + "git could not search the files moving to the code leg for `notes/` " + "(`git grep` exited 128: fatal: "), said[0] + assert said[0].endswith( + "); find the ones that name it by hand: each must read across the " + "assembly root once `notes/` moves to the spec leg"), said[0] + assert "\n" not in said[0] + + +@posix_only +def test_a_spec_directory_with_a_line_feed_is_left_to_a_human(adopter, + tmp_path): + """`git grep -F` reads a pattern holding a line feed as one pattern per + line, so on main `ab/` was searched as `a` OR `b/` and the follow-up + named `src/other.py`, whose only text is `a = 1`: a file that never + mentions the directory, the way `*` named one. Now no grep is asked for + that name, the follow-up says so with the name spelled, and `notes/` + beside it is searched as before. Asked of `cross_leg_references`, since + a plan holding that entry is #199's to write; the directory's files go + into the TREE and never on the disk.""" + path = make_source_repo( + tmp_path / "Thing", edits=(), + tree={**NOTES, "src/reader.py": READER.format("notes/"), + "src/other.py": "a = 1\n"}) + blob = subprocess.run( + ["git", "hash-object", "-w", "--stdin"], cwd=str(path), + input=b"# notes\n", capture_output=True, + check=True).stdout.decode().strip() + records = b"".join(b"100644 " + blob.encode() + b"\t" + name + b"\x00" + for name in (b"a\nb/one.md", b"a\nb/two.md")) + subprocess.run(["git", "update-index", "-z", "--index-info"], + cwd=str(path), input=records, check=True) + git("-c", "user.name=Source Human", "-c", "user.email=human@source.invalid", + "commit", "-qm", "A directory named across a line", cwd=path) + source = adopter.Source.open(str(path), tmp_path / "work") + entries = adopter.walk(adopter.PathPolicy.load(adopter.PATH_POLICY), + [(name, size) for name, _, _, size in source.tree()]) + assert {e.path: e.leg for e in entries}["a\nb/"] == "spec" + + said = adopter.cross_leg_references(source, entries, SPEC_PATH, CODE_PATH) + + assert said == [ + "git could not search the files moving to the code leg for `a\\nb/` " + "(`git grep` reads each line of a pattern as a pattern of its own, so " + "it cannot search for this name); find the ones that name it by " + "hand: each must read across the assembly root once `a\\nb/` moves " + "to the spec leg", + follow_up("src/reader.py", "notes/")] + + +# --------------------------------------------------------------------------- +# The split's forced add: a path that landed through a link is a path +# --------------------------------------------------------------------------- + +WORKFLOW = ".github/workflows/validate.yml" +PIN = "contracts/shape-pin.yaml" +#: The source hides every `validate.yml`, so the shape's workflow is staged +#: by the forced add alone, wherever the link put it: the case that add is +#: for (#167). `ga0/` and `*.env` hide what must never be forced. +IGNORE = "validate.yml\nga0/\n*.env\n" +#: Hidden by IGNORE, and so never in the clone `execute` splits until +#: something writes them there. The test does, so that a forced add reading a +#: name as more than its path has something to sweep in. +STRAY = ("ga0/workflows/validate.yml", "secret.env") + + +@posix_only +@pytest.mark.parametrize("target", [":ci", ":!ci", "ga[0]"], + ids=["magic-colon", "magic-exclude", "glob"]) +def test_a_shape_file_written_through_a_link_is_staged_by_its_own_name( + adopter, tmp_path, target): + """The plan kept `.github -> ` and `/` in the root, so + the shape's workflow landed at `/workflows/validate.yml`, which + the source's `.gitignore` hides: only the forced add stages it, by that + name. On main git read the name as a pathspec. `:ci/...` was the magic + `:` and the path `ci/...`, which matched nothing, and the add died + (`fatal: pathspec ... did not match any files`); `:!ci/...` was an + exclude, which forced nothing by that name, so the split lacked the + workflow; and `ga[0]/...` was a glob that also forced the ignored + `ga0/workflows/validate.yml`. Now each name is the path it spells: that + file and the pin are staged, and nothing else.""" + source = make_source_repo( + tmp_path / "source", edits=(), + tree={"README.md": "# Thing\n", ".gitignore": IGNORE, + f"{target}/workflows/ci.yml": "on: [push]\n"}) + os.symlink(target, source / ".github") + git("add", "-A", "--", ".", cwd=source) + git("-c", "user.name=Source Human", "-c", "user.email=human@source.invalid", + "commit", "-qm", "Link .github", cwd=source) + assembly = tmp_path / "assembly" + git("clone", "-q", str(source), str(assembly), cwd=tmp_path) + for name in (WORKFLOW, PIN, *STRAY): + (assembly / name).parent.mkdir(parents=True, exist_ok=True) + (assembly / name).write_text(f"{name}\n", encoding="utf-8") + landed = f"{target}/workflows/validate.yml" + assert (assembly / landed).is_file() + # The premise: each is ignored, so no `git add -A` stages it. Asked of + # the whole tree, because `check-ignore` takes each name as a pathspec. + ignored = git("status", "--porcelain", "-z", "--ignored=traditional", + "--untracked-files=all", cwd=assembly).stdout.split("\0") + assert {f"!! {name}" for name in (landed, *STRAY)} <= set(ignored), ignored + + materialized = Materialized() + materialized.written = [WORKFLOW, PIN] + try: + adopter._stage_the_split(assembly, materialized) + except CommandFailed as exc: + pytest.fail(f"the forced add read a name as a pathspec: {exc}") + + staged = git("diff", "--cached", "--name-status", "-z", + cwd=assembly).stdout.split("\0") + pairs = sorted(zip(staged[0:-1:2], staged[1::2])) + assert pairs == sorted([("A", landed), ("A", PIN)])