diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 5e86be7c..229fb9c8 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -234,7 +234,7 @@ def open(cls, root_spec) -> "Project": data = load_yaml(path) if not isinstance(data, dict): raise Refusal("bump-leg-manifest-unreadable", - f"{path}: not a mapping") + f"{shown_path(path)}: not a mapping") if data.get("kind") != "project-manifest": raise Refusal( "bump-leg-wrong-kind", diff --git a/scripts/path_classify.py b/scripts/path_classify.py index a61d5753..da32ebba 100644 --- a/scripts/path_classify.py +++ b/scripts/path_classify.py @@ -42,7 +42,7 @@ from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parent)) -from repo_shape import Refusal, load_yaml # noqa: E402 +from repo_shape import Refusal, load_yaml, shown_path # noqa: E402 LEGS = ("spec", "code", "root", "ambiguous") #: The answers a rule that asks may name as its `default:` (#528). `drop` @@ -182,11 +182,12 @@ def __init__(self, data: dict): def load(cls, path: Path) -> "PathPolicy": data = load_yaml(path) if not isinstance(data, dict): - raise Refusal("path-policy-unreadable", f"{path}: not a mapping") + raise Refusal("path-policy-unreadable", + f"{shown_path(path)}: not a mapping") if data.get("kind") != "path-classification-policy": raise Refusal( "path-policy-wrong-kind", - f"{path}: kind is {data.get('kind')!r}, expected " + f"{shown_path(path)}: kind is {data.get('kind')!r}, expected " "'path-classification-policy'") return cls(data) diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 3219399f..55c4192a 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -2252,7 +2252,7 @@ def load(cls, path: Path) -> "NamingPolicy": if data.get("kind") != "repository-naming-policy": raise Refusal( "policy-wrong-kind", - f"{path}: kind is {data.get('kind')!r}, expected " + f"{shown_path(path)}: kind is {data.get('kind')!r}, expected " "'repository-naming-policy'", ) return cls(data) diff --git a/scripts/validate-repository-naming.py b/scripts/validate-repository-naming.py index c568e33b..f86ce949 100755 --- a/scripts/validate-repository-naming.py +++ b/scripts/validate-repository-naming.py @@ -1229,7 +1229,7 @@ def _project_manifest(path: Path) -> dict: """The `project.yaml` mapping `--project` names, or a `Refusal`.""" data = load_yaml(path) if not isinstance(data, dict): - raise Refusal("manifest-unreadable", f"{path}: not a mapping") + raise Refusal("manifest-unreadable", f"{_shown(path)}: not a mapping") return data diff --git a/shape-doctor.py b/shape-doctor.py index 373f094b..c08b1c8e 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2761,10 +2761,13 @@ def naming_script_refusal(shape: Path, whole: str) -> Refusal | None: `project.yaml` the script was asked to read -- each is about what the script was asked, and is not this. - IN EITHER SPELLING (#1168): `load_yaml` and `policy-unreadable` name the - file as `shown_path` does, `repr`'d where this checkout's path holds a - character that is not printable, and `policy-wrong-kind` names it raw. - For a printable path the two are one. + IN EITHER SPELLING (#1168): `load_yaml`, `policy-unreadable` and, since + #1279, `policy-wrong-kind` name the file as `shown_path` does, `repr`'d + where this checkout's path holds a character that is not printable. It + is named raw only where this checkout's own `repo_shape.py` is older + than its doctor: the script that prints the line is this checkout's, and + imports this checkout's library, so a root's library never reaches this + function. For a printable path the two are one. """ policy = f"{shape / NAMING_COPY}: " spelt = f"{shown_path(shape / NAMING_COPY)}: " diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index 1f9df591..1410d1e6 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -1435,7 +1435,8 @@ def main(argv: list[str] | None = None) -> int: ) manifest = _load(root, manifest_path) if not isinstance(manifest, dict): - raise Refusal("manifest-unreadable", f"{manifest_path}: not a mapping") + raise Refusal("manifest-unreadable", + f"{_shown(manifest_path)}: not a mapping") policy_path = args.policy or (root / "contracts" / "repository-naming.yaml") policy = _load(root, policy_path, NamingPolicy.load) # Inside the `try`: a declared referent's pin file this user cannot diff --git a/tests/test_load_yaml_refusals.py b/tests/test_load_yaml_refusals.py index df92d5b5..da9734cb 100644 --- a/tests/test_load_yaml_refusals.py +++ b/tests/test_load_yaml_refusals.py @@ -528,13 +528,17 @@ def policy_refusal(shape: Path, text: str) -> Refusal: @pytest.mark.parametrize("text, code", [ pytest.param(UNPARSABLE, "yaml-unparsable", id="load-yaml"), pytest.param("- a list\n", "policy-unreadable", id="not-a-mapping"), + pytest.param("kind: something-else\n", "policy-wrong-kind", + id="wrong-kind"), ]) @pytest.mark.parametrize("name", NAMES) def test_the_doctor_finds_its_own_policy_in_the_naming_scripts_refusal( tmp_path, name, text, code): """`naming_script_refusal` knows the checkout's own policy by the path - at the start of the detail. `load_yaml` and `policy-unreadable` spell - it now, and the doctor asks that spelling too.""" + at the start of the detail. `load_yaml`, `policy-unreadable` and, since + #1279, `policy-wrong-kind` spell it now, and the doctor asks that + spelling too. Named raw, a line feed split the wrong kind's line, and + the doctor did not find its own policy in it.""" shape = unsafe_folder(tmp_path, name) refusal = policy_refusal(shape, text) found = doctor_module().naming_script_refusal(shape, f"{refusal}\n") @@ -547,12 +551,20 @@ def test_the_doctor_finds_its_own_policy_in_the_naming_scripts_refusal( pytest.param(PRINTABLE_NAME, id="printable"), ]) def test_the_doctor_still_finds_its_own_policy_named_raw(tmp_path, name): - """`policy-wrong-kind` names the policy raw, as it always did, and the - doctor still finds it by that prefix. A line feed splits that line, so - it is not among the names here, as on main.""" + """A `repo_shape.py` from before #1279 names the policy raw in + `policy-wrong-kind`, and the doctor still finds it by that prefix. Only + this checkout's own library can print that line: the naming script the + doctor reads is this checkout's and imports this checkout's library, so + the library is one older than its doctor, never a root's. The line is + built as that library prints it, since this checkout's spells it. A + line feed splits that line, so it is not among the names here.""" shape = unsafe_folder(tmp_path, name) - refusal = policy_refusal(shape, "kind: something-else\n") - assert refusal.code == "policy-wrong-kind" + policy = policy_refusal(shape, "kind: something-else\n") + assert policy.code == "policy-wrong-kind" + refusal = Refusal(policy.code, + f"{shape / 'contracts' / 'repository-naming.yaml'}: " + "kind is 'something-else', expected " + "'repository-naming-policy'") found = doctor_module().naming_script_refusal(shape, f"{refusal}\n") assert found is not None, ascii(str(refusal)) assert found.detail == refusal.detail diff --git a/tests/test_mapping_and_kind_refusals_shown.py b/tests/test_mapping_and_kind_refusals_shown.py new file mode 100644 index 00000000..6172403c --- /dev/null +++ b/tests/test_mapping_and_kind_refusals_shown.py @@ -0,0 +1,193 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A file that reads as YAML but is not a mapping, or is a policy of another +kind, is refused naming the file as `shown_path` names it (#1279). + +#1168 gave that spelling to `load_yaml`'s three refusals and to +`policy-unreadable`. The refusals on the next line of each reader, where the +YAML parsed but is a list, or declares another `kind:`, still named the path +raw: 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: + +- `policy-wrong-kind`, in `repo_shape.NamingPolicy.load`; +- `manifest-unreadable`, the naming gate's `--project`; +- `bump-leg-manifest-unreadable`, `bump-leg.py --root`, where `project.yaml` + is not a mapping (a root it cannot search was spelled already, and the + last row below pins that spelling); +- `path-policy-unreadable` and `path-policy-wrong-kind`, in + `path_classify.PathPolicy.load`, which `adopt-project.py plan + --path-policy` reads; +- `update-pin-unreadable`, `update-shape.py check --root`; +- the root's `manifest-unreadable`, its own `validate-manifest.py`. + +Each run is one of `test_load_yaml_refusals.py`'s, with the file it builds +holding YAML that parses. The naming gate and the root's +`validate-manifest.py` are copied into every project, beside a +`scripts/repo_shape.py` that may be older than they are, so each spells the +path with the `_shown` it already carries: the rows that run them beside a +library from before #1168 hold that for any name that library lacks. It has +`shown_path` already, so a gate that imported that name would pass those rows; +the import is held out by `test_pinned_copy_text.py`'s +`test_no_pinned_script_imports_what_an_older_library_lacks`. +A printable path is named byte for byte as it always was. +`adopt-project.py`'s `plan-unreadable` is #1177's. + +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 shutil +from pathlib import Path + +import pytest + +from conftest import ( + REPO, assert_shown_safe, named_as, run_script, unsafe_folder, +) +from test_bump_leg_unopenable import needs_a_mode +from test_bump_leg_unsearchable import unsearchable_root +from test_load_yaml_refusals import ( + NAMES, TEMPLATE_SCRIPTS, TOOLS, a_root, older_library, refused_lines, +) + +#: YAML that parses, to a list, and to a mapping of another kind. +A_LIST = "- a\n" +ANOTHER_KIND = "kind: other\n" + +#: What each refusal says after the path. +NOT_A_MAPPING = "not a mapping" +NOT_A_NAMING_POLICY = "kind is 'other', expected 'repository-naming-policy'" +NOT_A_PATH_POLICY = "kind is 'other', expected 'path-classification-policy'" + +#: The issue's table, less #1177's `plan-unreadable`: each refusal, as +#: `(code, the run in test_load_yaml_refusals.TOOLS that reaches it, what +#: its file holds, what the refusal says after the path)`. +SITES = { + "policy-wrong-kind": ( + "policy-wrong-kind", "naming--policy", ANOTHER_KIND, + NOT_A_NAMING_POLICY), + "manifest-unreadable-naming--project": ( + "manifest-unreadable", "naming--project", A_LIST, NOT_A_MAPPING), + "bump-leg-manifest-unreadable": ( + "bump-leg-manifest-unreadable", "bump-leg--root", A_LIST, + NOT_A_MAPPING), + "path-policy-unreadable": ( + "path-policy-unreadable", "adopt-plan--path-policy", A_LIST, + NOT_A_MAPPING), + "path-policy-wrong-kind": ( + "path-policy-wrong-kind", "adopt-plan--path-policy", ANOTHER_KIND, + NOT_A_PATH_POLICY), + "update-pin-unreadable": ( + "update-pin-unreadable", "update-shape-check--root", A_LIST, + NOT_A_MAPPING), + "manifest-unreadable-validate-manifest": ( + "manifest-unreadable", "validate-manifest--root", A_LIST, + NOT_A_MAPPING), +} + + +def holding(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def assert_refused_once(result, code: str, named: str, said: str) -> None: + """Exit 2, one `REFUSED` line, exactly `REFUSED : : + `, and no U+202E, ESC or forged line anywhere in the output.""" + output = result.stdout + result.stderr + assert result.returncode == 2, ascii(output) + assert refused_lines(result.stderr) == [ + f"REFUSED {code}: {named}: {said}"], ascii(output) + assert chr(0x202E) not in output, ascii(output) + assert_shown_safe(result.stdout, result.stderr) + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("site", sorted(SITES)) +def test_each_refusal_names_the_file_as_shown_path_names_it( + tmp_path, site, name): + """THE ISSUE'S PROBE, one real run per refusal: the file named 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 #1274's head each named it raw. The printable row is the + control, named byte for byte as before, with no quotes.""" + code, tool, text, said = SITES[site] + path, argv = TOOLS[tool](unsafe_folder(tmp_path, name), tmp_path) + holding(path, text) + result = run_script(*argv, cwd=tmp_path) + assert_refused_once(result, code, named_as(path, name), said) + + +#: The naming gate, which every project carries as it is here. +NAMING = REPO / "scripts" / "validate-repository-naming.py" + +#: The root's own manifest gate. +MANIFEST_GATE = "validate-manifest.py" + + +def naming_gate(folder: Path, library: bytes) -> tuple: + """The naming gate as a root carries it, beside `library` and this + checkout's naming policy, asked of a `project.yaml` that is a list.""" + root = a_root(folder, NAMING.read_bytes(), library, NAMING.name) + policy = root / "contracts" / "repository-naming.yaml" + policy.parent.mkdir() + shutil.copyfile(REPO / "contracts" / "repository-naming.yaml", policy) + manifest = holding(folder / "n" / "project.yaml", A_LIST) + return manifest, (root / "scripts" / NAMING.name, "--project", + str(manifest)) + + +def manifest_gate(folder: Path, library: bytes) -> tuple: + """The root's `validate-manifest.py` beside `library`, over a root whose + `project.yaml` is a list.""" + root = a_root(folder, (TEMPLATE_SCRIPTS / MANIFEST_GATE).read_bytes(), + library, MANIFEST_GATE) + return holding(root / "project.yaml", A_LIST), ( + root / "scripts" / MANIFEST_GATE, "--root", str(root)) + + +#: The two copied gates whose own refusal changed. +COPIED_GATES = { + "naming-gate--project": naming_gate, + "validate-manifest--root": manifest_gate, +} + + +@pytest.mark.parametrize("name", NAMES) +@pytest.mark.parametrize("gate", sorted(COPIED_GATES)) +def test_a_copied_gate_spells_the_path_beside_an_older_library( + tmp_path, gate, name): + """THE SKEW RULE: each copied gate beside a root's `repo_shape.py` from + before #1168, which has none of #1279's lines. The gate spells the path + with its own `_shown`, so it runs, refuses `manifest-unreadable` by + name, and spells the path as it does beside this checkout's library.""" + path, argv = COPIED_GATES[gate](unsafe_folder(tmp_path, name), + older_library()) + result = run_script(*argv, cwd=tmp_path) + assert "Traceback" not in result.stderr, ascii(result.stderr) + assert_refused_once(result, "manifest-unreadable", named_as(path, name), + NOT_A_MAPPING) + + +@needs_a_mode +@pytest.mark.parametrize("name", NAMES) +def test_a_root_that_cannot_be_searched_is_named_as_shown_path_names_it( + tmp_path, name): + """THE SPELLING THIS CHANGE LEFT ALONE: a `--root` + this user cannot search is `bump-leg-manifest-unreadable` from + `_manifest_unopened_refusal`, which names `project.yaml` as `shown_path` + does and did before #1279. No other row ran it under a path holding + U+202E, an ESC or a line feed (`test_bump_leg_unsearchable.py` uses a + printable one), so it was one `{path}` from raw with every row green. + This row passes on #1274's head too: it pins a spelling, and fails + nothing first. The root's mode is put back in a `finally`.""" + path, argv = TOOLS["bump-leg--root"](unsafe_folder(tmp_path, name), + tmp_path) + with unsearchable_root(path.parent, 0): + result = run_script(*argv, cwd=tmp_path) + assert_refused_once(result, "bump-leg-manifest-unreadable", + named_as(path, name), "cannot be read: Permission " + "denied") diff --git a/update-shape.py b/update-shape.py index 35f27aac..8595d379 100755 --- a/update-shape.py +++ b/update-shape.py @@ -185,6 +185,10 @@ # as `parse_yaml` does, and put it back (#933). On a line of its own, for the # reason the branch rule's import below gives. from repo_shape import BYTE_ORDER_MARK # noqa: E402 +# How a refusal names a path it was handed (#1279): as it is, or as `repr` +# spells it where a character in it is not printable, as `load_yaml` names +# the same pin since #1168. On a line of its own, as the two above are. +from repo_shape import shown_path # noqa: E402 from shape_materialize import ( # noqa: E402 ATTRIBUTES, COPIED_FROM_SHAPE, COPIED_VERBATIM, FAMILY_COPIED_FROM_SHAPE, FAMILY_COPIED_VERBATIM, SHAPE_REPOSITORY, CommandFailed, check_program, @@ -938,7 +942,8 @@ def check_pin(pin, pin_path, root: Path) -> tuple[dict, list[Row]]: whose history a refusal's `git log` reads (#290). """ if not isinstance(pin, dict): - raise Refusal("update-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("update-pin-unreadable", + f"{shown_path(pin_path)}: not a mapping") commit = str(pin.get("commit") or "").lower() if not COMMIT_RE.match(commit): raise Refusal(