diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 8c5a470e..1c5052c8 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -149,6 +149,9 @@ # On its own line, so the list above, which other changes extend, is # not a line two of them both rewrite (#715). from shape_materialize import readable_branch # noqa: E402 +# U+FEFF by the reader's own name, which `rewrite_pin` takes off line 1 as +# `parse_yaml` does, and puts back (#933). On its own line, as the one above. +from repo_shape import BYTE_ORDER_MARK # noqa: E402 from shape_materialize import ( # noqa: E402 CommandFailed, captured_child_environment, check_program, commit_trailers, escape_what_stdout_cannot_encode, is_nested_key, pin_values_in, @@ -370,10 +373,22 @@ def rewrite_pin(text: str, commit: str, digest: str) -> str: line rewrite cannot carry is refused HERE as this tool's failure, before any byte is written. The digest is the one this command RECOMPUTED from the leg's own objects; the read-back only asks that it landed. + + A BYTE-ORDER MARK IN FRONT OF LINE 1 IS LOOKED PAST, AND KEPT (#933). + `parse_yaml` takes off every U+FEFF the text starts with (#855), so a + pin whose first line is `commit:` after one is read with that key by + the project's validators; the pin is read here as `utf-8`, and the + mark was the key's first character, so the pin was refused as having + 0 `commit` lines. The marks are taken off before the lines are looked + at and put back in front of them, so `write_lf` writes them back byte + for byte, as every other line's bytes are. A mark anywhere else is + text, to the reader and here. """ + unmarked = text.lstrip(BYTE_ORDER_MARK) + marks = text[:len(text) - len(unmarked)] out: list[str] = [] seen = {"commit": 0, "tree_sha256": 0} - for line in text.splitlines(): + for line in unmarked.splitlines(): if line.startswith("commit:"): seen["commit"] += 1 out.append(with_scalar(line, "commit", commit)) @@ -393,7 +408,7 @@ def rewrite_pin(text: str, commit: str, digest: str) -> str: "does not recognise. Fix it by hand this once — `commit:` at " "column 0 and `tree_sha256:` indented under `digests:` — or " "re-scaffold.") - rewritten = "\n".join(out) + "\n" + rewritten = marks + "\n".join(out) + "\n" why = rewrite_fault(text, rewritten, commit, digest) if why is not None: raise Refusal( diff --git a/tests/test_bom_rewrites.py b/tests/test_bom_rewrites.py new file mode 100644 index 00000000..8712d123 --- /dev/null +++ b/tests/test_bom_rewrites.py @@ -0,0 +1,165 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A pin or a manifest whose first line is its key after a byte-order mark +is rewritten, and the mark is kept (#933). + +THE DEFECT, measured on main dc7cd49b7 by calling the three line rewriters +in process. Since #855 `parse_yaml` takes off every U+FEFF the text starts +with, so `check`, `load_yaml` and both validators read such a file. The +rewriters found their key at column 0 of the raw text, which their tools +read as `utf-8`, so the mark was the key's first character and each said +the key was not there: + +- `update-shape.py`'s `rewrite_shape_pin`, on a shape pin whose first line + is `commit:`: `update-pin-unrecognised`, "0 `commit` line(s)"; +- its `rewrite_manifest`, on a `project.yaml` whose first line is + `shape:`, through `_shape_block_bounds` and `_is_block_header`: + `update-manifest-no-shape`, "has no `shape:` block"; +- `scripts/bump-leg.py`'s `rewrite_pin`, on a leg pin whose first line is + `commit:`: `bump-leg-pin-unrecognised`, "0 `commit` line(s)". + +THE RULE NOW. Each takes the marks off the start of the text, as the reader +does, finds its key, and puts the marks back in front of what it returns, +so they are written back byte for byte. A file with no mark is rewritten +as it always was. A mark anywhere else is text, to the reader and to the +finders, and keeps today's answer. + +Every mark is written by this file as bytes (`codecs.BOM_UTF8`), never +typed. Each file is the scaffold's rendering +(`tests/test_pin_rewrite_layout.py`) with the key moved to line 1, where +no template puts it. +""" + +from __future__ import annotations + +import codecs +from typing import NamedTuple + +import pytest + +from test_pin_rewrite_layout import MANIFEST as MANIFEST_TEXT +from test_pin_rewrite_layout import ( # noqa: F401 + NEW_COMMIT, NEW_ROWS, NEW_TREE, OLD_COMMIT, OLD_ROWS, OLD_TREE, make_pin, + refusal_code, rewrite, tools, +) + +from repo_shape import Refusal +from shape_materialize import pin_values_in + +MARK = codecs.BOM_UTF8 +#: The mark as the one character the reader reads it as, U+FEFF. +MARK_TEXT = MARK.decode("utf-8") +#: A line a person may put above the first key before saving the file. +COMMENT = "# saved by hand\n" +#: The leg's own `commit:` in the manifest, above its `shape:` block. +MEMBER = "e" * 40 + + +class Finder(NamedTuple): + #: The key its file's line 1 holds here. + key: str + #: The block the validator reads the pin under. + within: tuple + #: The scaffold's pin kind, or None for the manifest. + kind: str | None + #: What it says of a file it finds no key in. + refused: str + + +FINDERS = { + "shape-pin-commit": Finder("commit", (), "assembly shape pin", + "has 0 `commit` line(s)"), + "manifest-shape-header": Finder("shape", ("shape",), None, + "has no `shape:` block"), + "leg-pin-commit": Finder("commit", (), "spec pin", + "has 0 `commit` line(s)"), +} + + +def key_first(text: str, key: str) -> str: + """`text` with its column-0 `:` line, and the block under it, moved + to line 1: an order the reader ignores and no template writes.""" + lines = text.split("\n") + start = next(index for index, line in enumerate(lines) + if line.startswith(f"{key}:")) + end = next((index for index in range(start + 1, len(lines)) + if lines[index] and not lines[index].startswith(" ")), + len(lines)) + return "\n".join(lines[start:end] + lines[:start] + lines[end:]) + + +def the_file(finder: str, commit: str, tree: str, rows) -> str: + """The finder's file pinned at `commit` and `tree`, its key on line 1.""" + key, _within, kind, _refused = FINDERS[finder] + if kind is None: + text = MANIFEST_TEXT.format(member=MEMBER, commit=commit, tree=tree) + else: + text = make_pin(kind, commit, tree, rows) + return key_first(text, key) + + +def rewritten(loaded, finder: str, text: str) -> str: + """The finder's rewriter onto the target, called as its tool calls it; + `loaded` is the `tools` fixture.""" + kind = FINDERS[finder].kind + if kind is None: + return loaded.update_shape.rewrite_manifest( + text, NEW_COMMIT, NEW_TREE, loaded.update_shape.PROJECT_KIND) + return rewrite(loaded, kind, text, NEW_COMMIT, NEW_TREE) + + +def read(finder: str, text: str) -> tuple[str, str]: + """`commit` and `digests.tree_sha256`, where the validator reads them.""" + return pin_values_in(text, FINDERS[finder].within) + + +@pytest.mark.parametrize("marks", [1, 2], ids=["mark", "two-marks"]) +@pytest.mark.parametrize("finder", list(FINDERS)) +def test_a_first_key_after_the_mark_is_rewritten_and_the_mark_kept( + tools, finder, marks): # noqa: F811 + """THE ISSUE'S THREE LINES. The file every reader reads is rewritten: + its first bytes are the marks it had, every byte after them is the + rewrite of the same file without them, and the validator reads the + target. Two marks are taken off and put back as one is, as the reader + takes off every mark the text starts with.""" + before = MARK_TEXT * marks + the_file(finder, OLD_COMMIT, OLD_TREE, + OLD_ROWS) + assert read(finder, before) == (OLD_COMMIT, OLD_TREE), ( + "fixture: the reader reads the marked file") + + new = rewritten(tools, finder, before) + + assert new.encode("utf-8") == MARK * marks + the_file( + finder, NEW_COMMIT, NEW_TREE, NEW_ROWS).encode("utf-8") + assert read(finder, new) == (NEW_COMMIT, NEW_TREE) + + +@pytest.mark.parametrize("finder", list(FINDERS)) +def test_the_same_file_without_the_mark_is_rewritten_as_before( + tools, finder): # noqa: F811 + """THE CONTROL, which passes on main as well: with no mark, what is + returned is the file at the target, with not a byte in front of it.""" + before = the_file(finder, OLD_COMMIT, OLD_TREE, OLD_ROWS) + + new = rewritten(tools, finder, before) + + assert new == the_file(finder, NEW_COMMIT, NEW_TREE, NEW_ROWS) + assert read(finder, new) == (NEW_COMMIT, NEW_TREE) + + +@pytest.mark.parametrize("finder", list(FINDERS)) +def test_a_mark_after_line_1_keeps_todays_answer(tools, finder): # noqa: F811 + """LINE 1 ONLY. A mark on line 2, in front of the key, is the key's + first character to the reader, which reads no pin there; each finder + refuses the file as it did before, in its own words.""" + before = COMMENT + MARK_TEXT + the_file(finder, OLD_COMMIT, OLD_TREE, + OLD_ROWS) + with pytest.raises(ValueError): + read(finder, before) + + with pytest.raises(Refusal) as refused: + rewritten(tools, finder, before) + + kind = FINDERS[finder].kind + assert refused.value.code == ( + "update-manifest-no-shape" if kind is None else refusal_code(kind)) + assert FINDERS[finder].refused in refused.value.detail diff --git a/update-shape.py b/update-shape.py index b42c5d4f..a7ae7505 100755 --- a/update-shape.py +++ b/update-shape.py @@ -178,6 +178,10 @@ # is one to the reader (#437), and with the reader's rule for a key, so a # quoted key or a space before the colon is one here too (#674). from repo_shape import _split_key, _strip_comment # noqa: E402 +# U+FEFF by the reader's own name: the two line rewrites take it off line 1 +# 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 from shape_materialize import ( # noqa: E402 ATTRIBUTES, COPIED_FROM_SHAPE, COPIED_VERBATIM, FAMILY_COPIED_FROM_SHAPE, FAMILY_COPIED_VERBATIM, SHAPE_REPOSITORY, CommandFailed, check_program, @@ -1707,6 +1711,24 @@ def _header_as_it_was(line: str) -> str: return body + (tail if tail.strip() else "") +def _marks_off(text: str) -> tuple[str, str]: + """The byte-order marks `text` starts with, and the text after them. + + LINE 1 ONLY, AS THE READER TAKES THEM (#933). `parse_yaml` takes off + every U+FEFF the text starts with (#855), so a pin whose first line is + `commit:` after one, or a manifest whose first line is `shape:`, is + read with that key by `check` and both validators. `_repin` reads the + two files as `utf-8`, so the finders below met the mark as the first + character of the key and refused the file, in words that said the key + was not there. They look at the text after the marks now, and each + rewrite puts the marks back in front of what it returns, so they are + written back byte for byte, as every other line's bytes are. A mark + anywhere else is text, to the reader and here. + """ + after = text.lstrip(BYTE_ORDER_MARK) + return text[:len(text) - len(after)], after + + def rewrite_shape_pin(text: str, commit: str, tree: str, rows: list[Row]) -> str: """`commit:`, `digests.tree_sha256:` and the whole `files:` block. @@ -1743,8 +1765,13 @@ def rewrite_shape_pin(text: str, commit: str, tree: str, WHAT IS RETURNED HAS BEEN READ BACK, as `rewrite_manifest`'s is (`shape_materialize.rewrite_fault`): a layout this line rewrite cannot carry is refused HERE as this tool's failure, before any byte is written. + + A BYTE-ORDER MARK IN FRONT OF LINE 1 is taken off before the lines are + looked at and put back in front of them (`_marks_off`, #933), so a pin + whose first line is `commit:` after one is rewritten, mark and all. """ - lines = text.splitlines() + marks, unmarked = _marks_off(text) + lines = unmarked.splitlines() out: list[str] = [] seen = {"commit": 0, "tree": 0, "files": 0} for index, line in enumerate(lines): @@ -1772,7 +1799,7 @@ def rewrite_shape_pin(text: str, commit: str, tree: str, "exactly one was expected, so this tool will not rewrite it", "Remediation: the pin was hand-edited into a shape this tool " "does not recognise. Fix it by hand this once, or re-scaffold.") - rewritten = "\n".join(out) + "\n" + rewritten = marks + "\n".join(out) + "\n" why = rewrite_fault(text, rewritten, commit, tree) if why is not None: raise Refusal( @@ -1810,8 +1837,15 @@ def rewrite_manifest(text: str, commit: str, tree: str, kind: Kind) -> str: carry — a value on the line below its key — is refused HERE, as this tool's failure, rather than by a validator after the fact (`_refuse_unless_it_reads_back`). + + A BYTE-ORDER MARK IN FRONT OF LINE 1 is taken off before the block is + looked for and put back in front of the lines (`_marks_off`, #933), so + `_shape_block_bounds` finds a `shape:` header that is line 1 after one, + which it refused `update-manifest-no-shape` while the validators read + the block. """ - lines = text.splitlines() + marks, unmarked = _marks_off(text) + lines = unmarked.splitlines() start, end = _shape_block_bounds(lines, kind) seen = {"commit": 0, "tree": 0} for index in range(start + 1, end): @@ -1831,7 +1865,8 @@ def rewrite_manifest(text: str, commit: str, tree: str, kind: Kind) -> str: "where exactly one was expected", "Remediation: fix the block by hand this once; this tool will " "not guess at a manifest it does not recognise.") - rewritten = "\n".join(lines) + ("\n" if text.endswith("\n") else "") + rewritten = (marks + "\n".join(lines) + + ("\n" if text.endswith("\n") else "")) _refuse_unless_it_reads_back(text, rewritten, commit, tree, kind) return rewritten