diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 229fb9c8..aee089b2 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -224,8 +224,8 @@ def open(cls, root_spec) -> "Project": if not present: raise Refusal( "bump-leg-root-not-a-project", - f"{path} does not exist, so this is not an assembly root of a " - "project that has elected this shape", + f"{shown_path(path)} does not exist, so this is not an " + "assembly root of a project that has elected this shape", "Remediation: --root takes the ASSEMBLY ROOT — the repository " "an engineer clones — not a leg and not a family holder. A " "family's member pins are moved with `scripts/family.py " @@ -238,7 +238,7 @@ def open(cls, root_spec) -> "Project": if data.get("kind") != "project-manifest": raise Refusal( "bump-leg-wrong-kind", - f"{path}: kind is {data.get('kind')!r}, expected " + f"{shown_path(path)}: kind is {data.get('kind')!r}, expected " "'project-manifest'", "Remediation: a family holder declares `kind: " "family-manifest` and its members move with `scripts/" diff --git a/scripts/family.py b/scripts/family.py index 7ade9e12..cd459b06 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -573,11 +573,12 @@ def open(cls, root_spec) -> "Family": exc.remediation = _unparsable_in_an_act(root) raise if not isinstance(data, dict): - raise Refusal("family-unreadable", f"{path}: not a mapping") + raise Refusal("family-unreadable", + f"{shown_path(path)}: not a mapping") if data.get("kind") != "family-manifest": raise Refusal( "family-wrong-kind", - f"{path}: kind is {data.get('kind')!r}, expected " + f"{shown_path(path)}: kind is {data.get('kind')!r}, expected " "'family-manifest'") _checked_members_readable(path, data.get("members")) return cls(root, data) diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index 1410d1e6..0cc7e489 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -1429,9 +1429,9 @@ def main(argv: list[str] | None = None) -> int: if not _is_file(root, manifest_path): raise Refusal( "manifest-missing", - f"{manifest_path} does not exist. A project that has elected " - "this schema declares it in `project.yaml`; a project that has " - "not, does not run this validator.", + f"{_shown(manifest_path)} does not exist. A project that has " + "elected this schema declares it in `project.yaml`; a project " + "that has not, does not run this validator.", ) manifest = _load(root, manifest_path) if not isinstance(manifest, dict): diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index 448efd21..c0c035a6 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -371,6 +371,15 @@ def _anything(mode: int) -> bool: return stat.S_IFMT(mode) != 0 +def _shown(path) -> str: + """A path as it is, or as Python's `repr` spells it where a character in + it is not printable: `repo_shape.shown_path`, which a holder's library + may predate (#610), as the assembly root's `validate-manifest.py` spells + it for itself (#1288).""" + text = str(path) + return text if text.isprintable() else repr(text) + + def unreadable(code: str, named: str, exc: OSError, what: str) -> Refusal: """`code`'s refusal of a path this command must ask about and the operating system will not let it: its `stat` refused (`_is_a`), as in a @@ -1977,14 +1986,14 @@ def members_of(root: Path) -> tuple[str, list[dict]]: if not present: raise Refusal( "family-manifest-missing", - f"{manifest_path} does not exist, so there is no declaration of " - "which members to bootstrap", + f"{_shown(manifest_path)} does not exist, so there is no " + "declaration of which members to bootstrap", "Remediation: this command bootstraps a FAMILY holder. A project " "runs its own `scripts/bootstrap.py` instead.") manifest = load_yaml(manifest_path) if not isinstance(manifest, dict): raise Refusal("family-manifest-unreadable", - f"{manifest_path}: not a mapping") + f"{_shown(manifest_path)}: not a mapping") name = str(manifest.get("name") or root.name) # A `members:` THAT IS NOT A LIST, OR HOLDS AN ENTRY THAT IS NOT A # MAPPING, IS REFUSED BY NAME (#864), in `validate-family.py`'s words: diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 12baa46b..23061e94 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -337,6 +337,17 @@ def _anything(mode: int) -> bool: return stat.S_IFMT(mode) != 0 +def _shown(path) -> str: + """A path as it is, or as Python's `repr` spells it where a character in + it is not printable: `repo_shape.shown_path`, which a holder's library + may predate (#610), as the assembly root's `validate-manifest.py` spells + it for itself (#1288). Written here, not imported from `bootstrap.py`: + a holder that kept its own `bootstrap.py` (`--accept-local`) would be + refused `update-accept-local-missing-name` for the name (#536).""" + text = str(path) + return text if text.isprintable() else repr(text) + + def member_unreadable(named: str, exc: OSError, what: str, then: str = "Then run the same command again.", code: str = "member-unreadable") -> Refusal: @@ -1744,7 +1755,7 @@ def _load_family(args: argparse.Namespace) -> tuple[Path, str, list[dict], manifest = load_yaml(root / MANIFEST) if not isinstance(manifest, dict): raise Refusal("family-manifest-unreadable", - f"{root / MANIFEST}: not a mapping") + f"{_shown(root / MANIFEST)}: not a mapping") return root, name, rows, manifest diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 0231d953..6b79b69b 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -601,6 +601,15 @@ def _refuse_an_unreadable_root(start: Path) -> None: return +def _shown(path) -> str: + """A path as it is, or as Python's `repr` spells it where a character in + it is not printable: `repo_shape.shown_path`, which a holder's library + may predate (#610), as the assembly root's `validate-manifest.py` spells + it for itself (#1288).""" + text = str(path) + return text if text.isprintable() else repr(text) + + def _asked(path: Path, kind: Callable[[int], bool], code: str, named: str) -> bool: """`_is_a(path, kind)`, or `code`'s refusal naming `path` as `named` @@ -1838,14 +1847,16 @@ def _load_shape_pin(root: Path) -> tuple[dict, str]: pin_path = root / "contracts" / "shape-pin.yaml" if not _asked(pin_path, stat.S_ISREG, "shape-pin-unreadable", "contracts/shape-pin.yaml"): - raise Refusal("shape-pin-missing", f"{pin_path} does not exist") + raise Refusal("shape-pin-missing", + f"{_shown(pin_path)} does not exist") pin = load_yaml(pin_path) # POSIX, on every platform: a finding names a path in THIS repository, # and `str()` of a relative Path on Windows spells it with backslashes — # a path the reader cannot paste back into git. rel = pin_path.relative_to(root).as_posix() if not isinstance(pin, dict): - raise Refusal("shape-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("shape-pin-unreadable", + f"{_shown(pin_path)}: not a mapping") return pin, rel @@ -4819,16 +4830,16 @@ def _run_checks(args: argparse.Namespace, MANIFEST): raise Refusal( "family-manifest-missing", - f"{manifest_path} does not exist. A family holder declares " - f"itself in `{MANIFEST}`; a repository without one is not a " - "family and does not run this validator.", + f"{_shown(manifest_path)} does not exist. A family holder " + f"declares itself in `{MANIFEST}`; a repository without one is " + "not a family and does not run this validator.", f"Remediation: `{PYTHON} scripts/family.py init --org " "--family ` creates one. An ordinary project runs " "`validate-manifest.py` instead.") manifest = load_yaml(manifest_path) if not isinstance(manifest, dict): raise Refusal("family-manifest-unreadable", - f"{manifest_path}: not a mapping") + f"{_shown(manifest_path)}: not a mapping") policy_path = args.policy or (root / "contracts" / "repository-naming.yaml") policy = NamingPolicy.load(policy_path) diff --git a/tests/test_pinned_copy_text.py b/tests/test_pinned_copy_text.py index dc27b0f9..481226ea 100644 --- a/tests/test_pinned_copy_text.py +++ b/tests/test_pinned_copy_text.py @@ -48,6 +48,7 @@ from test_link_source_chain_defaults import ( # noqa: F401 VALIDATOR, manifest, naming, pinned, scaffold, tree, validate_pins, ) +from test_pinned_copy_columns import copies # noqa: F401 sys.path.insert(0, str(REPO / "scripts")) from repo_shape import ( # noqa: E402 @@ -61,6 +62,8 @@ "validate-pins.py") TEMPLATE_FAMILY = (REPO / "templates" / "family-root" / "scripts" / "validate-family.py") +FAMILY_BOOTSTRAP = (REPO / "templates" / "family-root" / "scripts" / + "bootstrap.py") TEMPLATE_MANIFEST = (REPO / "templates" / "assembly-root" / "scripts" / "validate-manifest.py") POLICY = REPO / "contracts" / "repository-naming.yaml" @@ -772,17 +775,24 @@ def test_the_naming_gate_names_a_printable_link_source_as_typed(tmp_path): @pytest.mark.parametrize("path", [ "plain/dir", "x\x1b[31mRED", "a\nb", "a\rb", "caf\u00e9", "\udcff", - "tab\there", "", + "tab\there", "", "pl\u202ean", ]) -def test_the_pinned_scripts_spell_a_path_as_the_library_does(path): - """`validate-pins.py`, `validate-repository-naming.py` and, since #909, - `validate-manifest.py` each write their own `_shown`, because a root - whose `scripts/repo_shape.py` is older than they are has no - `shown_path` to import. Each must spell as `shown_path` spells.""" +def test_the_pinned_scripts_spell_a_path_as_the_library_does(copies, path): + """`validate-pins.py`, `validate-repository-naming.py`, since #909 + `validate-manifest.py`, and since #1288 a family holder's + `validate-family.py`, `bootstrap.py` and `siblings.py` each write their + own `_shown`, because a root or a holder whose `scripts/repo_shape.py` + is older than they are has no `shown_path` to import. Each must spell + as `shown_path` spells, a bidi control, U+202E, among the rest. + `siblings.py` is `test_pinned_copy_columns.py`'s copy, its `bootstrap` + import answered by the holder's `bootstrap.py` beside it.""" for script, name in ((TEMPLATE_PINS, "pins"), (VALIDATOR, "naming"), - (TEMPLATE_MANIFEST, "manifest")): + (TEMPLATE_MANIFEST, "manifest"), + (TEMPLATE_FAMILY, "family"), + (FAMILY_BOOTSTRAP, "family_bootstrap")): module = load_without_bytecode(script, f"pinned_copy_text_{name}") assert module._shown(path) == shown_path(path), script + assert copies["siblings"]._shown(path) == shown_path(path) #: Every pinned script that imports from `repo_shape`, each beside whatever @@ -823,3 +833,28 @@ def test_no_pinned_script_imports_what_an_older_library_lacks(name): assert imported, f"{script.name} imports nothing from repo_shape?" for newer in NEWER_THAN_A_KEPT_LIBRARY: assert newer not in imported, (script, newer) + + +def test_siblings_keeps_its_own_shown_and_takes_none_from_bootstrap(): + """THE #536 CHOICE, held by itself. `siblings.py` takes names from + `bootstrap.py` already, and a holder that kept its own `bootstrap.py` + (`update-shape.py apply --accept-local`) is refused + `update-accept-local-missing-name` for each name `siblings.py` takes that + its kept copy does not define. So `siblings.py` defines its own `_shown`, + as `validate-manifest.py` does, and takes none from `bootstrap.py`: not + by a `from bootstrap import` at module level or inside a function, and + not as `bootstrap._shown`.""" + tree = ast.parse(PINNED_IMPORTERS["siblings"].read_text(encoding="utf-8")) + defined = [node for node in tree.body + if isinstance(node, ast.FunctionDef) and node.name == "_shown"] + assert len(defined) == 1, "siblings.py defines no `_shown` of its own" + taken = [alias.name + for node in ast.walk(tree) + if isinstance(node, ast.ImportFrom) and node.module == "bootstrap" + for alias in node.names] + assert "_shown" not in taken, "siblings.py imports `_shown` from bootstrap" + reached = [node for node in ast.walk(tree) + if isinstance(node, ast.Attribute) and node.attr == "_shown" + and isinstance(node.value, ast.Name) + and node.value.id == "bootstrap"] + assert not reached, "siblings.py reads `bootstrap._shown`" diff --git a/tests/test_refusal_paths_shown_more.py b/tests/test_refusal_paths_shown_more.py new file mode 100644 index 00000000..5cedf2bf --- /dev/null +++ b/tests/test_refusal_paths_shown_more.py @@ -0,0 +1,368 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Fourteen missing, unreadable and wrong-kind refusals, and four more of +`update-shape.py`'s `check_pin`, name the path as `shown_path` names it +(#1288). + +#1279 spelled the line after `load_yaml` in seven readers. These still +named the path raw, each a refusal of a file that is not there, does not +read as a mapping, or is of another kind: a bidi override, U+202E, in a +folder's name reordered the line, an ESC reached the terminal as the control +it is, and a line feed began a line of the path's choosing. Each is spelled +now: + +- `bump-leg.py --root`: `bump-leg-root-not-a-project` and + `bump-leg-wrong-kind`; +- an assembly root's own `validate-manifest.py`: `manifest-missing`; +- `family.py`: `family-unreadable` and `family-wrong-kind`; +- a family holder's `bootstrap.py`: `family-manifest-missing` and + `family-manifest-unreadable`; +- its `siblings.py`: `family-manifest-unreadable`, reached only past a + `bootstrap.py` whose `members_of` reads no `family.yaml`. The holder's own + refuses a list first, in the same words, and a row runs that too; +- its `validate-family.py`: `shape-pin-missing`, `shape-pin-unreadable`, + `family-manifest-missing` and `family-manifest-unreadable`; +- `update-shape.py check --root`: `update-pin-missing`, and each refusal of + `check_pin` after #1279's `update-pin-unreadable`: `update-pin-commit`, + `update-pin-digest-definition`, `update-pin-row` (two ways) and + `update-pin-no-files`. `update-pin-row`'s remediation names the root in a + `git -C` line, which is spelled too where the root does not print. + +The tools a checkout of the standard runs import `shown_path`. A root's +`validate-manifest.py` and a holder's three scripts are copies, kept beside a +`scripts/repo_shape.py` that may be older than they are, so each spells with +a `_shown` of its own (`test_pinned_copy_text.py` holds each to +`shown_path`); the last rows run one refusal of each beside a library from +before #1168. A printable path is named byte for byte as it always was. + +NO NETWORK AND NO GITHUB: temporary files, local repositories, and the +session's `gh` guard first on PATH for every tool run. +""" + +from __future__ import annotations + +import ast +import shlex +import shutil +import sys +from pathlib import Path +from typing import Callable, NamedTuple + +import pytest + +from conftest import ( + PRINTABLE_NAME, REPO, assert_shown_safe, git, named_as, run_script, spelt, + unsafe_folder, +) +from test_load_yaml_refusals import ( + A_COMMIT, LIBRARY, NAMES, TEMPLATE_SCRIPTS, older_library, refused_lines, +) +from test_mapping_and_kind_refusals_shown import ( + A_LIST, ANOTHER_KIND, NOT_A_MAPPING, holding, +) + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import TREE_DIGEST_DEFINITION # noqa: E402 + +FAMILY_SCRIPTS = REPO / "templates" / "family-root" / "scripts" +BUMP_LEG = REPO / "scripts" / "bump-leg.py" +FAMILY = REPO / "scripts" / "family.py" +UPDATE_SHAPE = REPO / "update-shape.py" +SHAPE_PIN = "contracts/shape-pin.yaml" + + +def maybe(path: Path, text: str | None) -> Path: + """`path`, holding `text`, or left absent where `text` is None.""" + return path if text is None else holding(path, text) + + +def a_repository(folder: Path, name: str, scripts: tuple, + library: bytes) -> Path: + """A git repository at `/` carrying each of `scripts` in + its `scripts/`, beside `library` as `scripts/repo_shape.py`: a root or a + holder as it carries its copies.""" + root = folder / name + (root / "scripts").mkdir(parents=True) + git("init", "-q", str(root), cwd=folder) + for script in scripts: + shutil.copyfile(script, root / "scripts" / script.name) + (root / "scripts" / "repo_shape.py").write_bytes(library) + return root + + +# --- how one run reaches each refusal ---------------------------------------- +# +# Each builder takes the folder the name was made as and the library a copy +# is kept beside (ignored by a tool run from this checkout), and gives back +# `(the path the refusal names, the run's argv)`. + + +def bump_leg(text: str | None) -> Callable: + """`bump-leg.py --root --dry-run`, `project.yaml` holding + `text`.""" + def build(folder: Path, _library: bytes) -> tuple: + return maybe(folder / "project.yaml", text), ( + BUMP_LEG, "--root", str(folder), "--leg", "spec", "--to", + A_COMMIT, "--dry-run") + return build + + +def manifest_gate(folder: Path, library: bytes) -> tuple: + """A root's own `validate-manifest.py`, over a root with no + `project.yaml`.""" + root = a_repository(folder, "Atlas", + (TEMPLATE_SCRIPTS / "validate-manifest.py",), library) + return root / "project.yaml", (root / "scripts" / "validate-manifest.py", + "--root", str(root)) + + +def family_bump(text: str) -> Callable: + """`family.py bump --dry-run --family-root `, `family.yaml` + holding `text`.""" + def build(folder: Path, _library: bytes) -> tuple: + return holding(folder / "family.yaml", text), ( + FAMILY, "bump", "--dry-run", "--family-root", str(folder), + "--member", "Atlas", "--to", A_COMMIT) + return build + + +def holder_bootstrap(text: str | None) -> Callable: + """A holder's own `bootstrap.py --no-fetch`, `family.yaml` holding + `text`.""" + def build(folder: Path, library: bytes) -> tuple: + root = a_repository(folder, "Acme", (FAMILY_SCRIPTS / "bootstrap.py",), + library) + return maybe(root / "family.yaml", text), ( + root / "scripts" / "bootstrap.py", "--root", str(root), + "--no-fetch") + return build + + +def members_unread() -> str: + """A holder's `bootstrap.py` whose `members_of` reads no `family.yaml`, + with the holder's own beside it as `bootstrap_real.py` for every other + name `siblings.py` imports from `bootstrap`: past it, `siblings.py`'s own + read of `family.yaml` is the one that refuses. The names are read from + `siblings.py`'s own `from bootstrap import ...`, by `ast`, so a name a + later change adds is answered too and these rows keep testing what they + say.""" + tree = ast.parse((FAMILY_SCRIPTS / "siblings.py").read_text( + encoding="utf-8")) + names = [alias.name + for node in ast.walk(tree) + if isinstance(node, ast.ImportFrom) and node.module == "bootstrap" + for alias in node.names if alias.name != "members_of"] + assert names, "siblings.py imports nothing but members_of from bootstrap?" + return ("from bootstrap_real import ( # noqa: F401\n " + + ",\n ".join(names) + ",\n)\n\n\n" + "def members_of(root):\n" + " return 'Acme', []\n") + + +def holder_siblings(members_read: bool) -> Callable: + """A holder's own `siblings.py --dry-run`, `family.yaml` a list, beside + the holder's `bootstrap.py` (`members_read`), whose `members_of` refuses + it first, or beside `members_unread()`'s, which does not.""" + def build(folder: Path, library: bytes) -> tuple: + bootstrap = FAMILY_SCRIPTS / "bootstrap.py" + root = a_repository( + folder, "Acme", (FAMILY_SCRIPTS / "siblings.py", + *((bootstrap,) if members_read else ())), library) + if not members_read: + shutil.copyfile(bootstrap, root / "scripts" / "bootstrap_real.py") + holding(root / "scripts" / "bootstrap.py", members_unread()) + return holding(root / "family.yaml", A_LIST), ( + root / "scripts" / "siblings.py", "--root", str(root), + "--dry-run") + return build + + +def holder_validator(text: str | None) -> Callable: + """A holder's own `validate-family.py`, `family.yaml` holding `text`.""" + def build(folder: Path, library: bytes) -> tuple: + root = a_repository(folder, "Acme", + (FAMILY_SCRIPTS / "validate-family.py",), library) + return maybe(root / "family.yaml", text), ( + root / "scripts" / "validate-family.py", "--root", str(root)) + return build + + +#: A `family.yaml` that reads, so `validate-family.py` goes on to the pin. +A_FAMILY = "kind: family-manifest\nname: Acme\norg: acme\nmembers: []\n" + + +def holder_pin(text: str | None) -> Callable: + """A holder's own `validate-family.py`, `family.yaml` reading and + `contracts/shape-pin.yaml` holding `text`.""" + def build(folder: Path, library: bytes) -> tuple: + _manifest, argv = holder_validator(A_FAMILY)(folder, library) + root = Path(argv[2]) + policy = root / "contracts" / "repository-naming.yaml" + policy.parent.mkdir() + shutil.copyfile(REPO / "contracts" / "repository-naming.yaml", policy) + return maybe(root / SHAPE_PIN, text), argv + return build + + +def update_pin(text: str | None) -> Callable: + """`update-shape.py check --root `, a repository whose shape pin + holds `text`.""" + def build(folder: Path, _library: bytes) -> tuple: + git("init", "-q", str(folder), cwd=folder.parent) + return maybe(folder / SHAPE_PIN, text), ( + UPDATE_SHAPE, "check", "--root", str(folder)) + return build + + +def a_pin(*lines: str) -> str: + """A shape pin whose `commit:` and `digest_definition:` read, then + `lines`.""" + return "".join(f"{line}\n" for line in ( + f"commit: {A_COMMIT}", f"digest_definition: {TREE_DIGEST_DEFINITION}", + *lines)) + + +class Site(NamedTuple): + """A refusal: its code, how one run reaches it, what it says after the + path, and whether its remediation names the root in a `git -C` line.""" + code: str + build: Callable + after: str + remedy: bool = False + + +#: What each refusal says after the path, in its own words. +MAPPING = f": {NOT_A_MAPPING}" +NOT_A_ROOT = (" does not exist, so this is not an assembly root of a project " + "that has elected this shape") +NO_MANIFEST = (" does not exist. A project that has elected this schema " + "declares it in `project.yaml`; a project that has not, does " + "not run this validator.") +NO_MEMBERS = (" does not exist, so there is no declaration of which members " + "to bootstrap") +NO_FAMILY = (" does not exist. A family holder declares itself in " + "`family.yaml`; a repository without one is not a family and " + "does not run this validator.") +NO_PIN = (" does not exist, so there is no record of which files were copied " + "out of openRepoShape or which revision they came from. Nothing can " + "be re-synced against nothing.") +NOT_HEX = (": `commit:` is 'nothex', not 40 hex, so there is no revision to " + "compare the upstream against") +NOT_THIS_DIGEST = ( + f": digest_definition is 'other', and this tool recomputes " + f"{TREE_DIGEST_DEFINITION!r}. Rewriting the tree digest under a " + "definition the pin does not claim would record a number whose meaning " + "nobody stated.") +FILES_NOT_A_LIST = (": `files:` is 5, not a list of `path:`/`sha256:` rows, " + "so no copied file can be read from it") +ROW_NOT_A_ROW = ": a `files:` row is not `path:`/`sha256:`" +NO_ROWS = (": no `files:` rows, so the pin asserts nothing about any copied " + "file and there is nothing to re-sync") + +#: The issue's table and its four read ones: each refusal, by an id that +#: names the code and, where two tools share one, the tool. +SITES = { + "bump-leg-root-not-a-project": Site( + "bump-leg-root-not-a-project", bump_leg(None), NOT_A_ROOT), + "bump-leg-wrong-kind": Site( + "bump-leg-wrong-kind", bump_leg(ANOTHER_KIND), + ": kind is 'other', expected 'project-manifest'"), + "manifest-missing": Site("manifest-missing", manifest_gate, NO_MANIFEST), + "family-unreadable": Site( + "family-unreadable", family_bump(A_LIST), MAPPING), + "family-wrong-kind": Site( + "family-wrong-kind", family_bump(ANOTHER_KIND), + ": kind is 'other', expected 'family-manifest'"), + "family-manifest-missing-bootstrap": Site( + "family-manifest-missing", holder_bootstrap(None), NO_MEMBERS), + "family-manifest-unreadable-bootstrap": Site( + "family-manifest-unreadable", holder_bootstrap(A_LIST), MAPPING), + "family-manifest-unreadable-siblings": Site( + "family-manifest-unreadable", holder_siblings(False), MAPPING), + "family-manifest-unreadable-siblings-members-of": Site( + "family-manifest-unreadable", holder_siblings(True), MAPPING), + "shape-pin-missing": Site( + "shape-pin-missing", holder_pin(None), " does not exist"), + "shape-pin-unreadable": Site( + "shape-pin-unreadable", holder_pin(A_LIST), MAPPING), + "family-manifest-missing-validate-family": Site( + "family-manifest-missing", holder_validator(None), NO_FAMILY), + "family-manifest-unreadable-validate-family": Site( + "family-manifest-unreadable", holder_validator(A_LIST), MAPPING), + "update-pin-missing": Site( + "update-pin-missing", update_pin(None), NO_PIN), + "update-pin-commit": Site( + "update-pin-commit", update_pin("commit: nothex\n"), NOT_HEX), + "update-pin-digest-definition": Site( + "update-pin-digest-definition", + update_pin(f"commit: {A_COMMIT}\ndigest_definition: other\n"), + NOT_THIS_DIGEST), + "update-pin-row-not-a-list": Site( + "update-pin-row", update_pin(a_pin("files: 5")), FILES_NOT_A_LIST, + remedy=True), + "update-pin-row-entry": Site( + "update-pin-row", update_pin(a_pin("files:", " - 5")), ROW_NOT_A_ROW, + remedy=True), + "update-pin-no-files": Site( + "update-pin-no-files", update_pin(a_pin("files: []")), NO_ROWS), +} + + +def refused_at(tmp_path: Path, site: str, name: str, library: bytes) -> tuple: + """`(path, result)`: one run of `site`'s tool over a folder `name`, and + what it must print: exit 2, no traceback, one `REFUSED` line, exactly + `REFUSED : `, and + no U+202E, ESC or forged line anywhere in the output.""" + code, build, after, _remedy = SITES[site] + path, argv = build(unsafe_folder(tmp_path, name).resolve(), library) + result = run_script(*argv, cwd=tmp_path) + output = result.stdout + result.stderr + assert result.returncode == 2, ascii(output) + assert "Traceback" not in output, ascii(output) + assert refused_lines(result.stderr) == [ + f"REFUSED {code}: {named_as(path, name)}{after}"], ascii(output) + assert chr(0x202E) not in output, ascii(output) + assert_shown_safe(result.stdout, result.stderr) + return path, result + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("site", sorted(SITES)) +def test_each_refusal_names_the_path_as_shown_path_names_it( + tmp_path, site, name): + """THE ISSUE'S PROBE, one real run per refusal: the path as `repr` + spells it where its folder's name holds U+202E, an ESC or a line feed, + so nothing of that name reaches the terminal raw and the detail stays + one line. At #1284's head each named it raw. The printable row is the + control, named byte for byte as before, with no quotes. + `update-pin-row`'s remediation names the root in its `git -C` line as + `shown_path` does where the root does not print, and quoted for a POSIX + shell, as `_by_hand` quotes it, where it does.""" + path, result = refused_at(tmp_path, site, name, LIBRARY.read_bytes()) + if SITES[site].remedy: + root = path.parent.parent + line = (shlex.join(["git", "-C", str(root), "log", "--", SHAPE_PIN]) + if name == PRINTABLE_NAME + else f"git -C {spelt(root)} log -- {SHAPE_PIN}") + assert f"(`{line}` names the commit that changed it)" in ( + result.stderr), ascii(result.stderr) + + +#: One refusal of each copy a root or a holder keeps beside its library. +COPIES = ("family-manifest-unreadable-bootstrap", + "family-manifest-unreadable-siblings", + "family-manifest-unreadable-validate-family", "manifest-missing") + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("site", COPIES) +def test_each_copy_spells_the_path_beside_an_older_library( + tmp_path, site, name): + """EACH COPY REFUSES WITH THE PATH SPELLED beside a `repo_shape.py` from + before #1168, as a root or a holder has one that kept its own + (`update-shape.py apply --accept-local`): it runs, refuses by name, and + spells the path as it does beside this checkout's library. That library + (`51a9c5f10`) already defines `shown_path`, so these rows do not forbid + a copy from importing it. The row that does is + `test_pinned_copy_text.py`'s + `test_no_pinned_script_imports_what_an_older_library_lacks`.""" + refused_at(tmp_path, site, name, older_library()) diff --git a/update-shape.py b/update-shape.py index 8595d379..983940e5 100755 --- a/update-shape.py +++ b/update-shape.py @@ -870,8 +870,16 @@ def pin_rows_remediation(root: Path) -> str: history, or the wrong one (codex, PR #343). It is spelled `git -C ` by `_by_hand`, this module's one quoter for a line a person pastes, so a root holding a space is one argument there. + + A ROOT THAT DOES NOT PRINT IS SPELLED (#1288), as `shown_path` spells + it and as `validate-pins.py`'s `_quoted` spells a word of a command it + prints (#712): quoted, an ESC or a line break in it reached the + terminal as the control it is, under a `REFUSED` line that spelled it. + Spelled, the line cannot be pasted as the root it names, and no quoting + could make it so. A printable root is `_by_hand`'s line, as before. """ - log = _by_hand(root, "log", "--", SHAPE_PIN) + log = (_by_hand(root, "log", "--", SHAPE_PIN) if str(root).isprintable() + else f"git -C {shown_path(root)} log -- {SHAPE_PIN}") return ( "Remediation: `files:` is a list with one `path:`/`sha256:` row per " "copied file. The tool that cut this root wrote it and only " @@ -907,9 +915,9 @@ def read_pin(root: Path) -> tuple[dict, list[Row]]: if not _is_a(pin_path, stat.S_ISREG): raise Refusal( "update-pin-missing", - f"{pin_path} does not exist, so there is no record of which files " - "were copied out of openRepoShape or which revision they came " - "from. Nothing can be re-synced against nothing.", + f"{shown_path(pin_path)} does not exist, so there is no record of " + "which files were copied out of openRepoShape or which revision " + "they came from. Nothing can be re-synced against nothing.", "Remediation: this command updates a project that was scaffolded " "or adopted with this shape. A project without a shape pin is " "adopted with `adopt-project.py`, not updated.") @@ -948,12 +956,12 @@ def check_pin(pin, pin_path, root: Path) -> tuple[dict, list[Row]]: if not COMMIT_RE.match(commit): raise Refusal( "update-pin-commit", - f"{pin_path}: `commit:` is {pin.get('commit')!r}, not 40 hex, so " - "there is no revision to compare the upstream against") + f"{shown_path(pin_path)}: `commit:` is {pin.get('commit')!r}, not " + "40 hex, so there is no revision to compare the upstream against") if pin.get("digest_definition") != TREE_DIGEST_DEFINITION: raise Refusal( "update-pin-digest-definition", - f"{pin_path}: digest_definition is " + f"{shown_path(pin_path)}: digest_definition is " f"{pin.get('digest_definition')!r}, and this tool recomputes " f"{TREE_DIGEST_DEFINITION!r}. Rewriting the tree digest under a " "definition the pin does not claim would record a number whose " @@ -970,21 +978,22 @@ def check_pin(pin, pin_path, root: Path) -> tuple[dict, list[Row]]: if not isinstance(files, list): raise Refusal( "update-pin-row", - f"{pin_path}: `files:` is {files!r}, not a list of " + f"{shown_path(pin_path)}: `files:` is {files!r}, not a list of " "`path:`/`sha256:` rows, so no copied file can be read from it", pin_rows_remediation(root)) rows: list[Row] = [] for raw in files: if not isinstance(raw, dict) or not raw.get("path"): raise Refusal("update-pin-row", - f"{pin_path}: a `files:` row is not `path:`/`sha256:`", + f"{shown_path(pin_path)}: a `files:` row is not " + "`path:`/`sha256:`", pin_rows_remediation(root)) rows.append(Row(str(raw["path"]), str(raw.get("sha256") or ""))) if not rows: raise Refusal( "update-pin-no-files", - f"{pin_path}: no `files:` rows, so the pin asserts nothing about " - "any copied file and there is nothing to re-sync") + f"{shown_path(pin_path)}: no `files:` rows, so the pin asserts " + "nothing about any copied file and there is nothing to re-sync") return pin, rows