From 153bab90088058933caca44b3a8e45e245a3b797 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 14:08:06 +0000 Subject: [PATCH 1/6] repo_shape.py, shape_materialize.py and scaffold-project.py: the YAML reader strips only YAML's space and tab, so a value a Unicode space opens reads as PyYAML reads it and no refusal quotes text the file does not hold, `- :` and `k: {?: 1}` are refused, `[a: b: c]` is refused as invalid YAML, and a leg-path line the reader refuses is a refusal of the value, not a traceback (#715) What #635 left in the reader, and three texts about it: lane 4's lines L4-148 to L4-155 on #459's checklist, from R392's review of #635. - L4-150. Every strip of a line in the reader (`_strip_comment`, `_split_key`, `_scalar`, `_flow`, `_split_flow`, `_significant_text` and `_block_scalar_body`) was `str.strip()` with no argument, and `_DASH_RE` used `\s`. Both also take U+001F and sixteen Unicode spaces (U+00A0, U+1680, U+2000 to U+200A, U+202F, U+205F and U+3000), which YAML keeps as text. So `k: \u3000]`, `k: \u2003@a` and `k: \xa0%a` were refused quoting `']'`, `'@a'` and `'%a'`, texts the file does not hold, and `k: [a,\u3000,b]` was refused as an empty entry; `k: \xa0` read as no value, `- \u3000` as `[None]` and `k: [a, \xa0]` as `['a']`. PyYAML reads each whole. One constant, `_YAML_WHITE = " \t"`, now names YAML's white space at every strip, and `_DASH_RE` is `^-([ \t]+|$)`. Measured with PyYAML 6.0.3 over 123 probe documents: 77 readings changed, 31 of them now as PyYAML reads them and 37 now refusals. The other 9 (7 inputs) now read a document PyYAML refuses, each in a class this reader already reads: U+001F as text, as a raw ESC is; a tab after a Unicode space inside a plain scalar, as in `k: a\tb`; and a `:` opening a value inside a block value that a Unicode space opens, as in `k: a: b` and `k: a:`. - L4-148. `- :` read `[{'': None}]` and `: a` read `{'': 'a'}`, where PyYAML refuses both. `_refuse_unsupported` now refuses a line or an item that opens with a `:` opening a value, beside the explicit and merge keys and in their words: `line N: empty keys are not supported`. A quoted `"": a` still reads `{'': 'a'}`. The fuzz in `tests/test_yaml_indicator_openers.py` no longer drops `- :` (`EMPTY_BLOCK_KEY` is gone), and seeds 1 and 3 draw it again. - L4-149. `k: {?: 1}` read the key `'?'`, where PyYAML reads `{None: 1}`. Of the issue's two options it is refused, by the one test `_refuse_entry_indicator` already makes of `[?]`, `{?}` and `{a: ?}`: a key that is a lone `?`, a `?` whose `:` opens a value, is `explicit keys are not supported: '?: 1'`. `[?: 1]` gives that refusal too, where it gave the single-pair one. `{?a: 1}` and `[?:a]` stay the text #252 keeps, and a block `?: 1` is the key `'?'` to PyYAML as well. - L4-155. `k: [a: b: c]` was refused with the single-pair words. A flow sequence item whose pair's value splits again at a `:` opening a value is now `a flow sequence entry with a second `:` opening a value is not valid YAML: 'a: b: c'`. `[a:b: c]` and `[a: "b: c"]` are real single pairs to PyYAML and keep the single-pair words. - L4-153. `_read_back` called `parse_yaml` unguarded. `checked_value` admits a leg path opening with `@`, which opens no plain scalar since #393, so a bare leg-path template line holding `--spec-path @x` raised `YamlError` out of `leg_path_problem`. The refusal is now the reading, and `_read_as` says it in `_reading_in_words`' words, "nothing at all, because it refuses the line (...)", so the scaffold refuses the value as `scaffold-bad-leg-path` with `LEG_PATH_REMEDIATION`. `_misread_reason`'s docstring now says the alphabet holds `@`. The four shipped leg-path lines are quoted (#259), so `--spec-path @x` still plans. - L4-154. `_neutral_pin_entry`'s docstring gave `id: -` as "the text", which the reader refuses since #393. Its example is now ` - - a`, a list of its own, where `id: - a` is the text. `scripts/repo_shape.py` is a pinned shape copy. No digest or blob of it is recorded in this repository (#635 changed it with its test alone, and #638 with two other scripts and a test), so nothing here is re-pinned; a root takes the new bytes at its next `update-shape.py` run. Tests: `tests/test_yaml_reader_spaces.py` (new) holds a row for each case, with each of U+3000, U+2003 and U+00A0, compares each with PyYAML where it is installed, finds the seventeen characters rather than listing them, and runs one project's own `validate-manifest.py` and one scaffold `--dry-run`. Eleven mutants, each reverting one part, are each killed. The targeted run at this head, 28 files (the new one, the reader's, the leg-path rule's, `_neutral_pin_entry`'s, `_strip_comment`'s callers', every file that gives a reader a Unicode space, the pinned copies', and `test_repo_hygiene`, `test_cli_reference` and `test_agents_md_codes`): 3386 passed, 8 skipped (`tests/test_leg_path_yaml_scalars.py`'s rows with nothing to refuse since #259). Closes #715 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scaffold-project.py | 9 +- scripts/repo_shape.py | 92 +++-- scripts/shape_materialize.py | 24 +- tests/test_yaml_indicator_openers.py | 27 +- tests/test_yaml_reader_spaces.py | 489 +++++++++++++++++++++++++++ 5 files changed, 595 insertions(+), 46 deletions(-) create mode 100644 tests/test_yaml_reader_spaces.py diff --git a/scaffold-project.py b/scaffold-project.py index 8f9c8f8e..0a728cc0 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -1715,10 +1715,11 @@ def _neutral_pin_entry(product: str) -> str: THE READER IS ASKED, as `family.py` asks it of a member's id (#259), rather than a list of the words it takes for something else, and it is - asked of the LIST ENTRY: ` - -` is a list of its own, where `id: -` is - the text. A name it reads back bare, `openGlass` and every other plain - name, is the line it always was; `007`, `2026`, `1.5`, `true` and `null` - are double-quoted, and a double-quoted name always reads back. + asked of the LIST ENTRY: ` - - a` is a list of its own, where `id: - a` + is the text (#715; `id: -` is refused since #393). A name it reads back + bare, `openGlass` and every other plain name, is the line it always was; + `007`, `2026`, `1.5`, `true` and `null` are double-quoted, and a + double-quoted name always reads back. """ try: if parse_yaml(f"neutral_product_pins:\n - {product}\n") \ diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 570b7b3a..1b0d912c 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -193,10 +193,11 @@ class YamlError(Exception): # with the `-` and `+` chomping indicators accepted and `-` honoured). # # REFUSED (raises YamlError): tabs used for indentation, anchors and aliases -# (`&`, `*`), tags (`!`), merge keys (`<<`), explicit keys (`? `), a -# single-pair mapping in a flow sequence (`[a: b]`, #392), and more than one -# document. Each of those has a meaning this reader would have to invent, and -# an invented meaning in a validator is a wrong answer with a confident tone. +# (`&`, `*`), tags (`!`), merge keys (`<<`), explicit keys (`? `), an empty +# block key (`: a`, #715), a single-pair mapping in a flow sequence (`[a: b]`, +# #392), and more than one document. Each of those has a meaning this reader +# would have to invent, and an invented meaning in a validator is a wrong +# answer with a confident tone. # # A QUOTE OPENS A QUOTED SCALAR ONLY WHERE A SCALAR BEGINS, as in YAML; in # the middle of a plain scalar (`it's`) it is a character of it (#249). A @@ -217,7 +218,22 @@ class YamlError(Exception): # `@a: 1`, and `|` or `>` opening a plain scalar. See `_scalar`, # `_refuse_entry_indicator` and `_split_flow`. -_DASH_RE = re.compile(r"^-(\s+|$)") +#: YAML'S WHITE SPACE IS THE SPACE AND THE TAB (#715), and every strip of a +#: line below names it. `str.strip()` with no argument, like the regex `\s`, +#: also takes U+001F and sixteen Unicode spaces (U+00A0 NO-BREAK SPACE, +#: U+1680, U+2000 to U+200A, U+202F, U+205F and U+3000 IDEOGRAPHIC SPACE), +#: which YAML keeps as text. So `k: \u3000]` was read as `k: ]` and refused +#: quoting `']'`, a text the file does not hold, `k: [a,\u3000,b]` was +#: refused as an empty entry, and `k: \xa0` read as no value, where PyYAML +#: reads `'\u3000]'`, `['a', '\u3000', 'b']` and `'\xa0'`. U+001F, a control +#: character, is now text, as a raw ESC or NUL already is. The other +#: characters `str.strip()` takes are line breaks to `str.splitlines`, which +#: ends a line at them, so none of them reaches a strip. +_YAML_WHITE = " \t" + +#: A sequence entry's `-`, then YAML's white space (`_YAML_WHITE`, #715) or +#: the end of the line: `-\u3000a` is a plain scalar, and no entry. +_DASH_RE = re.compile(r"^-([ \t]+|$)") class _Rec: @@ -315,8 +331,8 @@ def _strip_comment(text: str) -> str: (`_unquoted`), at the start of the text or after a space or tab.""" for i, ch in _unquoted(text): if ch == "#" and (i == 0 or text[i - 1] in " \t"): - return text[:i].rstrip() - return text.rstrip() + return text[:i].rstrip(_YAML_WHITE) + return text.rstrip(_YAML_WHITE) def _split_key(text: str) -> tuple[str, str] | None: @@ -333,14 +349,15 @@ def _split_key(text: str) -> tuple[str, str] | None: bracket = False for i, ch in _unquoted(text): if ch in "[{": - if not text[:i].strip(): + if not text[:i].strip(_YAML_WHITE): return None # a flow collection cannot be a key in this subset bracket = True elif ch == ":" and (i + 1 == len(text) or text[i + 1] in " \t"): + key = text[:i].strip(_YAML_WHITE) if bracket: raise YamlError("a key holding `[` or `{` is not supported: " - f"{text[:i].strip()!r}") - return _unquote_key(text[:i].strip()), text[i + 1:].strip() + f"{key!r}") + return _unquote_key(key), text[i + 1:].strip(_YAML_WHITE) return None @@ -526,7 +543,7 @@ def _scalar(raw: str) -> Any: a scalar (`k: a]`, `- see [1]`, `k: a, b`), and every quoted scalar (`k: "]"`), read as before. """ - text = raw.strip() + text = raw.strip(_YAML_WHITE) if text == "" or text in ("null", "~", "Null", "NULL"): return None if text[0] in "\"'": @@ -588,9 +605,19 @@ def _refuse_entry_indicator(text: str) -> None: indicator, as in YAML and #252: `[-]`, `[-a]`, `{-: 1}` and `{?a: b}` read as they did. A `]` or `}` is left to `_refuse_flow_indicator`, which names it in #301's words. + + A KEY THAT IS A LONE `?` IS REFUSED WITH THEM (#715): a `?` whose `:` + opens a value, with a space, a tab or nothing after it. `k: {?: 1}` read + the key `'?'`, where PyYAML reads an explicit key that has no key, + `{None: 1}`, while the lone `?` of `[?]`, `{?}` and `{a: ?}` was + refused as one; `[?: 1]` was refused as a single pair. Both are now + #252's refusal. `{?:a}` holds no `: `, and `[?:a]` is a `?` with text + after it, the text #252 keeps for `?a`; a block `?: 1` is the key `'?'` + to PyYAML too. """ first, after = text[:1], text[1:2] if (first in ":#@`%" or first in "-?" and after in (" ", "\t") + or text[:2] == "?:" and text[2:3] in ("", " ", "\t") or text == "?"): raise _indicator_refusal(text) @@ -620,26 +647,37 @@ def _flow_value(text: str, item: bool = False) -> Any: cannot spell. So `pins: [openGlass: x]` is a typo, refused as one. `{a: b}` in a sequence is a flow mapping and reads, and `[a:b]` is the text `a:b`, as before. + + AN ITEM WITH A SECOND `:` OPENING A VALUE IS NO PAIR AT ALL (#715). + YAML ends the pair's plain value at that `:`, and PyYAML refuses + `[a: b: c]` as invalid, so the single-pair words named a mapping the + file does not hold. It is refused as invalid YAML. `[a:b: c]`, whose key + is `a:b`, and `[a: "b: c"]`, whose value is quoted, are single pairs and + keep those words. """ if text.startswith(("[", "{")): return _flow(text) _refuse_flow_indicator(text) if item: _refuse_entry_indicator(text) - if _split_key(text) is not None: + pair = _split_key(text) + if pair is not None and _split_key(pair[1]) is not None: + raise YamlError("a flow sequence entry with a second `:` opening " + f"a value is not valid YAML: {text!r}") + if pair is not None: raise YamlError("a single-pair mapping in a flow sequence is not " f"supported: {text!r}") return _scalar(text) def _flow(text: str) -> Any: - text = text.strip() + text = text.strip(_YAML_WHITE) if text.startswith("[") and text.endswith("]"): - body = text[1:-1].strip() + body = text[1:-1].strip(_YAML_WHITE) return [] if not body else [_flow_value(p, item=True) for p in _split_flow(body)] if text.startswith("{") and text.endswith("}"): - body = text[1:-1].strip() + body = text[1:-1].strip(_YAML_WHITE) out: dict[str, Any] = {} if body: for part in _split_flow(body): @@ -710,9 +748,9 @@ def _split_flow(body: str) -> list[str]: elif ch in "]}": depth -= 1 elif ch == "," and depth == 0: - parts.append(body[start:i].strip()) + parts.append(body[start:i].strip(_YAML_WHITE)) start = i + 1 - parts.append(body[start:].strip()) + parts.append(body[start:].strip(_YAML_WHITE)) if len(parts) > 1 and parts[-1] == "": parts.pop() if "" in parts: @@ -736,9 +774,9 @@ def _significant_text(raw: str, line: int) -> str | None: refused outright, and a blank line or a whole-line comment carries no record at all. """ - if "\t" in raw[: len(raw) - len(raw.lstrip())]: + if "\t" in raw[: len(raw) - len(raw.lstrip(_YAML_WHITE))]: raise YamlError(f"line {line}: tab used for indentation") - stripped = raw.strip() + stripped = raw.strip(_YAML_WHITE) if not stripped or stripped.startswith("#"): return None return stripped @@ -777,16 +815,25 @@ def _peel_dashes(recs: list[_Rec], indent: int, content: str, def _refuse_unsupported(content: str, line: int) -> None: - """The two constructs this reader will not guess at, refused by name. + """The three constructs this reader will not guess at, refused by name. Split out of `_tokenise` for #135. An explicit key and a merge key each have a meaning the reader would have to invent, and an invented meaning in a validator is a wrong answer with a confident tone. + + SO DOES AN EMPTY KEY (#715): a line, or a sequence item, that opens with + a `:` opening a value, `: a` or `- :`. PyYAML refuses it, and this + reader read the key `''`, which no reader gives: `- :` was + `[{'': None}]`. Its flow twin, `{: a}`, is refused by + `_refuse_entry_indicator` (#392). A quoted empty key, `"": a`, is the + key `''` to PyYAML too, and still reads. """ if content.startswith("? "): raise YamlError(f"line {line}: explicit keys are not supported") if content.startswith("<<"): raise YamlError(f"line {line}: merge keys are not supported") + if content[:1] == ":" and content[1:2] in ("", " ", "\t"): + raise YamlError(f"line {line}: empty keys are not supported") def _opens_block_scalar(kv: tuple[str, str] | None) -> bool: @@ -808,13 +855,14 @@ def _block_scalar_body(raw_lines: list[str], i: int, Split out of `_tokenise` for #135. The body is every following line MORE INDENTED than the key, with the first such line's column taken as the block's own indentation and stripped from all of them. A blank line - belongs to the body whatever its column, because it has none. + belongs to the body whatever its column, because it has none. A line of + Unicode spaces is not blank (#715): it is text, as YAML reads it. """ body: list[str] = [] block_indent = None while i < len(raw_lines): nxt = raw_lines[i] - if nxt.strip() == "": + if nxt.strip(_YAML_WHITE) == "": body.append("") i += 1 continue diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 8e032b39..edbb8682 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -4172,8 +4172,18 @@ def _leg_path_lines(templates: Path) -> tuple[tuple[str, str, str], ...]: def _read_back(tree: str, file: str, rendered: str): """What the project's reader gives back from one rendered `key: value` line, read on its own: its scalar reads the same there as inside its - block.""" - read = parse_yaml(rendered + "\n") + block. + + A LINE THE READER REFUSES GIVES BACK THE REFUSAL, its `YamlError`, as + `_reading_of` gives it for a manifest's line (#715). Uncaught, it was a + traceback where the tool owes a refusal of the value: `checked_value` + lets a leg path open with `@`, which opens no plain scalar (#393), so a + bare leg-path line holding `--spec-path @x` is a line the project's + reader refuses. All four such lines are quoted today (#259).""" + try: + read = parse_yaml(rendered + "\n") + except YamlError as exc: + return exc if not (isinstance(read, dict) and len(read) == 1): raise Refusal("template-leg-path-line", f"templates/{tree}/{file}: `{rendered}` is not one " @@ -4193,7 +4203,10 @@ def _listed(names: list[str]) -> str: def _read_as(value) -> str: """What the YAML reader made of a value, in words: `the number 1.5`. None is `NOT_TEXT_KINDS`' "no value at all", as for a leg path read out - of a plan as no value (#181).""" + of a plan as no value (#181). A refusal (`_read_back`) is "nothing at + all", in `_reading_in_words`' words (#715).""" + if isinstance(value, YamlError): + return f"nothing at all, because it refuses the line ({value})" if isinstance(value, bool): return f"the boolean {str(value).lower()}" if isinstance(value, (int, float)): @@ -4213,7 +4226,10 @@ def _misread_reason(text: str) -> str | None: quoted one (#259) reads it back. The sentence quotes each line that misreads, as rendered. Asked after `checked_value`, whose alphabet holds no space, quote, backslash, bracket, brace, `#`, `:`, `&`, `*` or `!`, so - each line parses, bare or quoted, and a bare value is a plain scalar. + a quoted line always parses and a bare value is a plain scalar. The + alphabet holds `@`, though, which opens no plain scalar (#393): a bare + line of `@x` is refused, and the refusal is its reading (`_read_back`), + said as "nothing at all" (#715). """ values = dict.fromkeys(LEG_PATH_PLACEHOLDERS, text) misread: dict[tuple[str, str], list[str]] = {} diff --git a/tests/test_yaml_indicator_openers.py b/tests/test_yaml_indicator_openers.py index b2473bfc..7122fc47 100644 --- a/tests/test_yaml_indicator_openers.py +++ b/tests/test_yaml_indicator_openers.py @@ -33,8 +33,9 @@ space and more text as a VALUE (`k: - a`, `{x: ? a}`), which PyYAML refuses and `tests/test_yaml_flow_mappings.py` pins as each form's twin; a flow `?a`, which #252 keeps as text; `|` and `>`, block scalar headers; and a block KEY. -The empty key of `- :` is one: it reads `[{'': None}]`, where PyYAML refuses -it, and the fuzz leaves it out. +The empty key of `- :` was one, left out of the fuzz while it read +`[{'': None}]`; #715 refuses it, as PyYAML does, so the fuzz draws it again +(`tests/test_yaml_reader_spaces.py` holds its rows). NO NETWORK AND NO GITHUB: strings, a temporary file, one scaffolded project over local bare remotes, and an import. @@ -454,23 +455,19 @@ def test_the_projects_own_validator_refuses_it(project, old, new, words): #: The seeds, so a failure here is the same failure on every machine. The #: first is the writer's. Seeds 1 and 3 are the two on which the table below -#: drew `- :` (`EMPTY_BLOCK_KEY`) and the test went red before it was left out; -#: all three stay in, so that every draw of the table is held in place. +#: draws `- :`, the empty block key, which was left out of the fuzz until #715 +#: refused it; all three stay in, so that every draw of the table is held in +#: place. FUZZ_SEED = 392393 FUZZ_SEEDS = [FUZZ_SEED, 1, 3] -#: The empty block key, drawn by `:` as an item, and outside these issues: this -#: reader reads `- :` as `[{'': None}]` where PyYAML refuses it. -#: `fuzz_documents` leaves it out. -EMPTY_BLOCK_KEY = "- :" - #: Plain words that read as text in both readers: no number, bool or null. WORDS = ["a", "b", "ab", "x"] #: What a BLOCK value or item may open with, and the tails after it. A tail #: holds no space, ends in no `:`, and opens with no `&*!` (anchors and -#: tags, refused by design). One draw is outside these issues and is dropped -#: from the documents: `:` as an item, the empty block key (`EMPTY_BLOCK_KEY`). +#: tags, refused by design). `:` as an item is the empty block key, `- :`, +#: which #715 refuses. BLOCK_OPENERS = ["]", "}", ",", "@", "`", "%", "?", ":", "-", "a", "x"] BLOCK_TAILS = ["", "a", "ab", "]", "}", "[0]", ",b", "@", "%", "`", "?", "-", ":b", "#b", "]x", "a]"] @@ -489,9 +486,8 @@ def test_the_projects_own_validator_refuses_it(project, old, new, words): def fuzz_documents(seed: int) -> list[str]: """A few hundred small documents, drawn from the seed: block values and items, flow sequences and flow mappings, with every indicator these - issues name in every place it can open a scalar or an entry. The empty - block key `- :`, which the block table can draw and no issue names, is - dropped after the draw, so each seed's other documents are the same.""" + issues name in every place it can open a scalar or an entry, and the + empty block key `- :`, which #715 refuses.""" rng = random.Random(seed) docs: list[str] = [] for _ in range(120): @@ -506,7 +502,7 @@ def fuzz_documents(seed: int) -> list[str]: entries = [f"{rng.choice(FLOW_KEYS)}: {rng.choice(FLOW_VALUES)}" for _ in range(rng.randint(1, 2))] docs.append("k: {" + ", ".join(entries) + rng.choice(["", ","]) + "}") - return sorted(set(docs) - {EMPTY_BLOCK_KEY}) + return sorted(set(docs)) #: The two refusals this reader makes of a document PyYAML reads, by design: @@ -522,7 +518,6 @@ def test_the_fuzz_finds_no_document_read_where_pyyaml_refuses_or_misread(seed): the same; and where only PyYAML reads, the refusal is one this reader makes by design. Every disagreement is listed, not just the first.""" docs = fuzz_documents(seed) - assert EMPTY_BLOCK_KEY not in docs assert len(docs) >= 300, "the fuzz must stay a few hundred documents" read_not_refused, misread, undesigned = [], [], [] tally = {"both refuse": 0, "both read": 0, "by design": 0} diff --git a/tests/test_yaml_reader_spaces.py b/tests/test_yaml_reader_spaces.py new file mode 100644 index 00000000..19bc5176 --- /dev/null +++ b/tests/test_yaml_reader_spaces.py @@ -0,0 +1,489 @@ +# SPDX-License-Identifier: Apache-2.0 +"""What #635 left in the YAML reader, and three texts about it (#715). + +- L4-150, YAML'S WHITE SPACE IS THE SPACE AND THE TAB. The reader stripped + with `str.strip()`, which also takes U+001F and sixteen Unicode spaces that + YAML keeps as text. So `k: \\u3000]`, `k: \\u2003@a` and `k: \\xa0%a` were + refused quoting `']'`, `'@a'` and `'%a'`, texts the file does not hold, + and `k: [a,\\u3000,b]` was refused as an empty entry, where PyYAML reads + each whole; `k: \\xa0` read as no value, `- \\u3000` as `[None]` and + `k: [a, \\xa0]` as `['a']`. Every strip now names `_YAML_WHITE`. +- L4-148, THE EMPTY BLOCK KEY. `- :` read `[{'': None}]` and `: a` read + `{'': 'a'}`, where PyYAML refuses both. Each is refused by its line, and + the fuzz of `tests/test_yaml_indicator_openers.py` draws `- :` again. +- L4-149, A KEY THAT IS A LONE `?`. `k: {?: 1}` read the key `'?'`, where + PyYAML reads an explicit key with no key, while `[?]` and `{?}` were + refused as explicit keys. It is refused with them. +- L4-155, `k: [a: b: c]` was refused as a single pair, which it is not: + PyYAML refuses it as invalid YAML, and the words now say so. +- L4-153, `_read_back` raised the reader's `YamlError` for a template line + the reader refuses, a bare leg-path line holding `@x`: the refusal is now + the reading, and the value is refused in the leg-path rule's words. +- L4-154, `_neutral_pin_entry`'s docstring gave `id: -` as "the text", + which the reader refuses since #393; its example is now one it reads. + +PyYAML is the ORACLE ONLY, as in `tests/test_yaml_indicator_openers.py`, +whose helpers this file uses: where it is not installed, the comparisons +are skipped and this reader's answers are still asserted. + +NO NETWORK AND NO GITHUB: strings, a temporary file, one scaffolded project +over local bare remotes, one dry run, and imports. +""" + +from __future__ import annotations + +import re +import sys + +import pytest + +from conftest import REPO, run_script +from test_leg_path_yaml_scalars import ( # noqa: F401 - `bare_lines` + OFFER, SPEC, assert_nothing_was_created, bare_lines, scaffold, sentence) +from test_scaffold_pin_and_reuse import scaffolder # noqa: F401 - a fixture +from test_yaml_indicator_openers import (EXPLICIT_KEY, NEEDS_PYYAML, OPENS, + PAIR, fuzz_documents, ours, refused, + stock) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from repo_shape import (SAFE_ARG_RE, Refusal, YamlError, # noqa: E402 + checked_value, double_quoted, load_yaml, parse_yaml) + +#: The three spaces the issue names. Every row of the tables below is asked +#: with each of them. +ISSUE_SPACES = [pytest.param("\u3000", id="ideographic-space"), + pytest.param("\u2003", id="em-space"), + pytest.param("\xa0", id="no-break-space")] + +#: Every character `str.strip()` takes from inside a line that YAML keeps: +#: U+001F and the sixteen Unicode spaces. Found, not trusted, by the first +#: test below. +KEPT_BY_YAML = ("\x1f\xa0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006" + "\u2007\u2008\u2009\u200a\u202f\u205f\u3000") + + +def filled(value, space: str): + """`value` with every `{s}` in its text, its keys too, made `space`.""" + if isinstance(value, str): + return value.replace("{s}", space) + if isinstance(value, list): + return [filled(item, space) for item in value] + if isinstance(value, dict): + return {filled(key, space): filled(item, space) + for key, item in value.items()} + return value + + +def refusal_words(text: str) -> str: + """The words this reader refuses `text` with.""" + with pytest.raises(YamlError) as caught: + parse_yaml(text + "\n") + return str(caught.value) + + +# --- L4-150: a Unicode space is text ---------------------------------------- + +def test_the_table_is_every_character_strip_takes_and_yaml_keeps(): + found = "".join(char for char in map(chr, range(sys.maxunicode + 1)) + if char.isspace() and char not in " \t" + and len(f"a{char}b".splitlines()) == 1) + assert found == KEPT_BY_YAML + + +@pytest.mark.parametrize("char", KEPT_BY_YAML, + ids=lambda char: f"U+{ord(char):04X}") +def test_every_one_is_text_where_a_value_an_entry_or_a_key_holds_it(char): + assert parse_yaml(f"k: {char}]\n") == {"k": f"{char}]"} + assert parse_yaml(f"k: [a,{char},b]\n") == {"k": ["a", char, "b"]} + assert parse_yaml(f"{char}k: a{char}\n") == {f"{char}k": f"a{char}"} + assert parse_yaml(f"- {char}\n") == [char] + + +def test_a_unit_separator_is_text_as_an_escape_is(): + """U+001F is a control character, which YAML forbids raw and PyYAML + refuses. This reader takes a raw control as text, as `k: \\x1b]` shows, + and since #715 U+001F alike, which `str.strip()` used to take.""" + assert parse_yaml("k: \x1b]\n") == {"k": "\x1b]"} + assert parse_yaml("k: \x1f]\n") == {"k": "\x1f]"} + + +#: `(document, what it reads as)`, `{s}` standing for the space. The issue's +#: four first: on main each was refused, quoting the text left once the +#: space was stripped. Then the same strip wherever else it stood, which on +#: main read less than the file holds, or something else. +READ_WHOLE = [ + pytest.param("k: {s}]", {"k": "{s}]"}, id="closer"), + pytest.param("k: {s}@a", {"k": "{s}@a"}, id="at-sign"), + pytest.param("k: {s}%a", {"k": "{s}%a"}, id="percent"), + pytest.param("k: [a,{s},b]", {"k": ["a", "{s}", "b"]}, id="flow-entry"), + pytest.param("k: {s}", {"k": "{s}"}, id="alone-as-a-value"), + pytest.param("k: a{s}", {"k": "a{s}"}, id="after-a-value"), + pytest.param("k: {s}a", {"k": "{s}a"}, id="before-a-value"), + pytest.param("{s}k: a", {"{s}k": "a"}, id="before-a-key"), + pytest.param("k{s}: a", {"k{s}": "a"}, id="after-a-key"), + pytest.param("- {s}", ["{s}"], id="alone-as-an-item"), + pytest.param("k: [{s}]", {"k": ["{s}"]}, id="alone-as-a-flow-item"), + pytest.param("k: [a, {s}]", {"k": ["a", "{s}"]}, id="the-last-flow-item"), + pytest.param("k: {a: {s}b}", {"k": {"a": "{s}b"}}, id="flow-value"), + pytest.param("k: {{s}a: b}", {"k": {"{s}a": "b"}}, id="flow-key"), + pytest.param("k: {s}[a]", {"k": "{s}[a]"}, id="before-a-bracket"), + pytest.param('k: {s}"a"', {"k": '{s}"a"'}, id="before-a-quote"), + pytest.param('k: [{s}"a"]', {"k": ['{s}"a"']}, id="before-a-quote-item"), + pytest.param("k: {s}|", {"k": "{s}|"}, id="before-a-block-header"), + pytest.param("k: {s}-", {"k": "{s}-"}, id="before-a-lone-dash"), + pytest.param("k: {s}&a", {"k": "{s}&a"}, id="before-an-anchor"), + pytest.param("k: a{s} # c", {"k": "a{s}"}, id="before-a-comment"), + pytest.param("k: |\n a\n {s}\n b", {"k": "a\n{s}\nb\n"}, + id="a-literal-body-line"), + pytest.param("k: >\n a\n {s}\n b", {"k": "a {s} b\n"}, + id="a-folded-body-line"), +] + + +@pytest.mark.parametrize("space", ISSUE_SPACES) +@pytest.mark.parametrize("text, expected", READ_WHOLE) +def test_a_unicode_space_is_read_as_the_text_it_is(text, expected, space): + assert parse_yaml(filled(text, space) + "\n") == filled(expected, space) + + +@NEEDS_PYYAML +@pytest.mark.parametrize("space", ISSUE_SPACES) +@pytest.mark.parametrize("text, expected", READ_WHOLE) +def test_pyyaml_reads_each_the_same(text, expected, space): + assert stock(filled(text, space)) == filled(expected, space) + + +#: `(document, the words it is refused with)`: refusals that now quote the +#: text the file holds, where PyYAML refuses the document too. +QUOTED_WHOLE = [ + pytest.param("k: ]{s}", OPENS + " `]`, which closes a flow sequence in " + "YAML: {q}", "]{s}", id="after-a-closer"), + pytest.param('k: "a"{s}', "quoted scalar with text after its closing " + "quote: {q}", '"a"{s}', id="after-a-quoted-value"), + pytest.param('"k"{s}: a', "quoted scalar with text after its closing " + "quote: {q}", '"k"{s}', id="after-a-quoted-key"), + pytest.param('k: ["a"{s}]', "quoted scalar with text after its closing " + "quote: {q}", '"a"{s}', id="after-a-quoted-item"), + pytest.param("a: 1\n{s}\nb: 2", "line 2: expected `key: value`, got {q}", + "{s}", id="a-line-of-it"), + pytest.param("k: [a]{s}", "not a flow collection: {q}", "[a]{s}", + id="after-a-flow-sequence"), + pytest.param("- {a: b}{s}", "not a flow collection: {q}", "{a: b}{s}", + id="after-a-flow-mapping-item"), +] + + +@pytest.mark.parametrize("space", ISSUE_SPACES) +@pytest.mark.parametrize("text, words, quoted", QUOTED_WHOLE) +def test_a_refusal_quotes_the_text_the_file_holds(text, words, quoted, + space): + assert refusal_words(filled(text, space)) == words.replace( + "{q}", repr(filled(quoted, space))) + + +@NEEDS_PYYAML +@pytest.mark.parametrize("space", ISSUE_SPACES) +@pytest.mark.parametrize("text, words, quoted", QUOTED_WHOLE) +def test_pyyaml_refuses_each_quoted_row_too(text, words, quoted, space): + assert refused(stock(filled(text, space))) + + +#: What main READ, as less than the file holds, and is refused now, where +#: PyYAML reads it: a document that is a plain scalar (`\\u3000- a`, which +#: main read as the list `['a']`) and a key holding a bracket are outside +#: the subset (#249), and a flow mapping entry with no `: ` is refused as +#: `{a}` is. +NO_LONGER_MISREAD = [ + pytest.param("{s}- a", "line 1: expected `key: value`, got {q}", + "{s}- a", id="before-a-dash"), + pytest.param("-{s}a", "line 1: expected `key: value`, got {q}", "-{s}a", + id="after-a-dash"), + pytest.param("{s}a[b]: c", "a key holding `[` or `{` is not supported: " + "{q}", "{s}a[b]", id="a-key-holding-a-bracket"), + pytest.param("k: {{s}}", "flow mapping entry without a key: {q}", "{s}", + id="alone-in-a-flow-mapping"), +] + + +@pytest.mark.parametrize("space", ISSUE_SPACES) +@pytest.mark.parametrize("text, words, quoted", NO_LONGER_MISREAD) +def test_what_main_misread_is_refused_by_its_text(text, words, quoted, space): + assert refusal_words(filled(text, space)) == words.replace( + "{q}", repr(filled(quoted, space))) + + +@pytest.mark.parametrize("space", ISSUE_SPACES) +def test_a_tab_after_a_unicode_space_is_in_the_key_not_the_indentation( + space): + """The indentation is the spaces and tabs that open the line, so a tab + after a Unicode space is inside the plain scalar, as YAML reads it and + as this reader reads `k: a\\tb`; PyYAML refuses a tab there, in both. + A tab before any text is still refused as indentation.""" + assert parse_yaml(f"{space}\tk: a\n") == {f"{space}\tk": "a"} + assert parse_yaml("k: a\tb\n") == {"k": "a\tb"} + assert refusal_words(" \tk: a") == "line 1: tab used for indentation" + + +# --- L4-148: the empty block key -------------------------------------------- + +#: `(document, the line it is refused on)`. +EMPTY_BLOCK_KEYS = [ + pytest.param("- :", 1, id="the-fuzz-row"), + pytest.param(": a", 1, id="with-a-value"), + pytest.param("- : a", 1, id="an-item-with-a-value"), + pytest.param("a:\n - :", 2, id="a-nested-item"), + pytest.param(":", 1, id="alone"), + pytest.param(":\t a", 1, id="a-tab-after-the-colon"), + pytest.param("- - :", 1, id="an-item-of-an-item"), + pytest.param(": # c", 1, id="before-a-comment"), + pytest.param("a: 1\n: b", 2, id="after-a-key"), +] + + +@pytest.mark.parametrize("text, line", EMPTY_BLOCK_KEYS) +def test_an_empty_block_key_is_refused_by_its_line(text, line): + assert refusal_words(text) == f"line {line}: empty keys are not supported" + + +@NEEDS_PYYAML +@pytest.mark.parametrize("text, line", EMPTY_BLOCK_KEYS) +def test_pyyaml_refuses_each_empty_block_key_too(text, line): + assert refused(stock(text)), f"PyYAML reads {text!r} as {stock(text)!r}" + + +#: A quoted empty key is the key `''` to PyYAML too, and a `:` with a letter +#: after it opens no value. +EMPTY_KEY_STILL_READ = [ + pytest.param('"": a', {"": "a"}, id="double-quoted"), + pytest.param("'': a", {"": "a"}, id="single-quoted"), + pytest.param('- "": a', [{"": "a"}], id="double-quoted-in-an-item"), + pytest.param("- :a", [":a"], id="a-colon-and-a-letter"), +] + + +@pytest.mark.parametrize("text, expected", EMPTY_KEY_STILL_READ) +def test_a_quoted_empty_key_still_reads(text, expected): + assert parse_yaml(text + "\n") == expected + + +@NEEDS_PYYAML +@pytest.mark.parametrize("text, expected", EMPTY_KEY_STILL_READ) +def test_pyyaml_reads_each_quoted_empty_key_the_same(text, expected): + assert stock(text) == expected + + +@pytest.mark.parametrize("seed", [1, 3]) +def test_the_fuzz_draws_the_empty_block_key_again(seed): + """The two seeds on which the fuzz drew `- :` and left it out.""" + assert "- :" in fuzz_documents(seed) + + +def test_a_file_holding_one_is_yaml_unparsable_naming_the_file(tmp_path): + path = tmp_path / "project.yaml" + path.write_bytes(b"id: atlas\n: main\n") + with pytest.raises(Refusal) as caught: + load_yaml(path) + assert caught.value.code == "yaml-unparsable" + assert caught.value.detail == (f"{path}: line 2: empty keys are not " + "supported") + + +def test_the_projects_own_validator_refuses_it(project): + """The copy a project carries reads `project.yaml`: its own + `validate-manifest.py` exits 2, names the file and the line, and reads + nothing as data.""" + manifest = project / "project.yaml" + data = manifest.read_bytes() + old = b'tracking_branch: "main"' + assert old in data, f"fixture drift: {old!r} not in project.yaml" + edited = data.replace(old, b': "main"', 1) + manifest.write_bytes(edited) + line = edited[:edited.index(b': "main"')].count(b"\n") + 1 + result = run_script(project / "scripts" / "validate-manifest.py", + cwd=project) + assert result.returncode == 2, result.stderr + assert (f"REFUSED yaml-unparsable: {manifest}: line {line}: empty keys " + "are not supported\n") in result.stderr + assert "Traceback" not in result.stderr + assert "manifest ok" not in result.stdout + assert manifest.read_bytes() == edited + + +# --- L4-149: a key that is a lone `?` --------------------------------------- + +#: `(document, the entry refused, what PyYAML reads)`: an explicit key with +#: no key, `None`, where this reader read the key `'?'`, or refused the +#: sequence item as a single pair. +LONE_QUESTION_KEYS = [ + pytest.param("k: {?: 1}", "?: 1", {"k": {None: 1}}, id="the-issue-row"), + pytest.param("k: {?:}", "?:", {"k": {None: None}}, id="no-value"), + pytest.param("k: {?: }", "?:", {"k": {None: None}}, + id="a-space-and-no-value"), + pytest.param("k: {b: 2, ?: 1}", "?: 1", {"k": {"b": 2, None: 1}}, + id="after-another-entry"), + pytest.param("k: [?: 1]", "?: 1", {"k": [{None: 1}]}, + id="a-flow-sequence-item"), + pytest.param("k: [?:]", "?:", {"k": [{None: None}]}, + id="an-item-with-no-value"), +] + + +@pytest.mark.parametrize("text, entry, pyyaml_reads", LONE_QUESTION_KEYS) +def test_a_key_that_is_a_lone_question_mark_is_refused_as_an_explicit_key( + text, entry, pyyaml_reads): + assert refusal_words(text) == f"{EXPLICIT_KEY}: {entry!r}" + + +@NEEDS_PYYAML +@pytest.mark.parametrize("text, entry, pyyaml_reads", LONE_QUESTION_KEYS) +def test_pyyaml_reads_each_as_an_explicit_key_with_no_key(text, entry, + pyyaml_reads): + assert stock(text) == pyyaml_reads + + +def test_a_tab_after_the_colon_is_refused_too(): + """PyYAML refuses the tab there; this reader refuses the key first.""" + assert refusal_words("k: {?:\t1}") == f"{EXPLICIT_KEY}: '?:\\t1'" + + +#: What is no lone `?` key: a quoted one, a block key (the key `'?'` to +#: PyYAML as well), and a `?` with text after it, which #252 keeps. +QUESTION_MARK_STILL_READ = [ + pytest.param('k: {"?": 1}', {"k": {"?": 1}}, id="a-quoted-key"), + pytest.param("?: 1", {"?": 1}, id="a-block-key"), + pytest.param("k: {?a: 1}", {"k": {"?a": 1}}, id="a-letter-after-it"), + pytest.param("k: [?:a]", {"k": ["?:a"]}, id="a-colon-and-a-letter"), +] + + +@pytest.mark.parametrize("text, expected", QUESTION_MARK_STILL_READ) +def test_what_is_no_lone_question_mark_key_still_reads(text, expected): + assert parse_yaml(text + "\n") == expected + + +# --- L4-155: a flow sequence entry with a second `:` ------------------------ + +NOT_A_PAIR = [ + pytest.param("k: [a: b: c]", "a: b: c", id="the-issue-row"), + pytest.param("k: [a: b:]", "a: b:", id="a-second-colon-at-the-end"), + pytest.param("k: [a: b: c: d]", "a: b: c: d", id="three-colons"), + pytest.param("k: [a: b:\tc]", "a: b:\tc", id="a-tab-after-the-second"), + pytest.param('k: ["a": b: c]', '"a": b: c', id="a-quoted-key"), + pytest.param("k: [a: 'b': c]", "a: 'b': c", id="a-quoted-second-key"), + pytest.param("k: [x, a: b: c]", "a: b: c", id="after-another-item"), +] + + +@pytest.mark.parametrize("text, entry", NOT_A_PAIR) +def test_an_entry_with_a_second_colon_is_refused_as_invalid_yaml(text, + entry): + assert refusal_words(text) == ( + "a flow sequence entry with a second `:` opening a value is not " + f"valid YAML: {entry!r}") + + +@NEEDS_PYYAML +@pytest.mark.parametrize("text, entry", NOT_A_PAIR) +def test_pyyaml_refuses_each_as_invalid_too(text, entry): + assert refused(stock(text)), f"PyYAML reads {text!r} as {stock(text)!r}" + + +#: `(document, the entry, what PyYAML reads)`: single pairs, which keep the +#: single-pair words (`_flow_value` says why one is refused). +STILL_A_PAIR = [ + pytest.param("k: [a:b: c]", "a:b: c", {"k": [{"a:b": "c"}]}, + id="a-colon-inside-the-key"), + pytest.param('k: [a: "b: c"]', 'a: "b: c"', {"k": [{"a": "b: c"}]}, + id="a-quoted-value"), + pytest.param("k: [a: b:c]", "a: b:c", {"k": [{"a": "b:c"}]}, + id="a-colon-inside-the-value"), +] + + +@pytest.mark.parametrize("text, entry, pyyaml_reads", STILL_A_PAIR) +def test_a_single_pair_keeps_its_words(text, entry, pyyaml_reads): + assert refusal_words(text) == f"{PAIR}: {entry!r}" + + +@NEEDS_PYYAML +@pytest.mark.parametrize("text, entry, pyyaml_reads", STILL_A_PAIR) +def test_pyyaml_reads_each_as_a_single_pair(text, entry, pyyaml_reads): + assert stock(text) == pyyaml_reads + + +# --- L4-153: a template line the reader refuses ----------------------------- + +#: What the leg-path rule says the project's reader makes of a bare `@x`. +AT_X_READ = ("nothing at all, because it refuses the line (a plain scalar " + "cannot open with `@`, which is reserved in YAML: '@x')") + + +def test_of_the_alphabet_only_an_at_sign_opens_what_a_bare_line_refuses(): + """`_misread_reason`'s docstring, measured: `checked_value` passes a leg + path that opens with `@`, the one character of its alphabet that a bare + line cannot open with, and a quoted line reads it back.""" + openers = [char for char in map(chr, range(0x21, 0x7f)) + if SAFE_ARG_RE.fullmatch(f"{char}x")] + assert [char for char in openers + if refused(ours(f"path: {char}x"))] == ["@"] + assert checked_value(SPEC, "@x") == "@x" + assert parse_yaml('path: "@x"\n') == {"path": "@x"} + + +@pytest.mark.parametrize("what", [SPEC, "--code-path", "legs.spec_path"]) +def test_a_bare_line_the_reader_refuses_is_a_refusal_of_the_value( + bare_lines, what): # noqa: F811 + """Over templates whose leg-path lines are bare: the reader's refusal of + `path: @x` was a traceback out of `leg_path_problem`.""" + assert shape_materialize.leg_path_problem(what, "@x") == sentence( + what, "@x", AT_X_READ) + + +def test_the_scaffold_refuses_it_in_its_own_words(bare_lines): # noqa: F811 + with pytest.raises(Refusal) as caught: + shape_materialize.refuse_bad_leg_paths( + "scaffold-bad-leg-path", (SPEC, "@x"), ("--code-path", "code")) + assert caught.value.code == "scaffold-bad-leg-path" + assert caught.value.detail == sentence(SPEC, "@x", AT_X_READ) + assert caught.value.remediation == shape_materialize.LEG_PATH_REMEDIATION + + +def test_no_spelling_is_offered_that_a_bare_line_refuses( + bare_lines): # noqa: F811 + """`@x/` is not answered `Write '@x'.`: `_canonical_spelling` asks the + same reading, which was a traceback too.""" + problem = shape_materialize.leg_path_problem(SPEC, "@x/") + assert problem.startswith( + "--spec-path is '@x/', which is not a canonical leg path"), problem + assert OFFER not in problem + + +def test_the_real_templates_read_it_back_and_a_dry_run_plans_it(tmp_path): + """The four leg-path lines are quoted (#259), so `@x` is a path.""" + assert shape_materialize.leg_path_problem(SPEC, "@x") is None + result = scaffold(tmp_path, "--dry-run", SPEC, "@x") + assert result.returncode == 0, result.stderr + result.stdout + assert "REFUSED" not in result.stderr + assert_nothing_was_created(tmp_path) + + +# --- L4-154: `_neutral_pin_entry`'s example --------------------------------- + +def test_the_docstrings_example_is_one_the_reader_reads( + scaffolder): # noqa: F811 + """Its example, read out of the docstring and asked of the reader: the + list entry is a list of its own, the `id:` line is the text, so the + entry is asked and double-quoted. `id: -`, its old example, is refused + since #393.""" + found = re.search(r"` - (?P[^`]+)` is a list of its own, where " + r"`id: (?P[^`]+)`\s+is the text", + scaffolder._neutral_pin_entry.__doc__) + assert found, scaffolder._neutral_pin_entry.__doc__ + entry, value = found["entry"], found["value"] + assert entry == value + listed = parse_yaml(f"neutral_product_pins:\n - {entry}\n") + assert isinstance(listed["neutral_product_pins"][0], list), listed + assert parse_yaml(f"id: {value}\n") == {"id": value} + assert scaffolder._neutral_pin_entry(entry) == double_quoted(entry) + assert refused(ours("id: -")) From 53af978e5876cd64413d06bf631c8d06ea6fb7d1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 18:48:53 +0000 Subject: [PATCH 2/6] validate-manifest.py, bootstrap.py, bump-leg.py and update-shape.py: a branch holding a character that does not print or is white space is refused, so `tracking_branch: main` followed by U+00A0, which the reader now reads as YAML does, makes no second `main` and lets no commit onto `main` (#715) Fix round 1 for RV823's review of #823 (issuecomment-6236502554), on the coordinator's rulings. MUST-1, option (B). Since this PR the reader reads `tracking_branch: main` followed by U+00A0 as PyYAML reads it, `'main\xa0'`, where main read `'main'`. git takes that name, so on #823's first head bootstrap made a second `main` in every leg and checked it out, and bump-leg's guard of the tracking branch let a commit be made on `main`. The reader stays as it is, and every reader of a branch now refuses what `siblings.py` refuses, a character `str.isprintable()` rejects or `str.isspace()` matches: - `validate-manifest.py`'s `_branch_finding` names it as `manifest-tracking-branch` or `manifest-leg-branch`, the value spelled and the code point named. - `bootstrap.py`'s `_argument_fault` returns `UNSEEN`, so step (a) makes no ref and says why on the leg's line. A byte-carrying surrogate (U+DC80 to U+DCFF) is not asked, so a name that is not UTF-8 still reaches git. - `bump-leg.py`'s `Project.tracking` reads the value through `checked_value`, as `family.py` does, so every run is `unsafe-value`, exit 2, before anything is fetched or written. - `update-shape.py`'s `tracking_branch_of`, the same guard for `apply --branch`, reads it through `checked_value` too, and the ledger rolls the rewrite back. The rule now refuses three branches #455's tests passed to git: a zero-width joiner, a space and an ESC. Those rows move to the fault tables. #391's spelling tests took `tb` U+2028 `x` as their subject, a branch git accepts and a terminal must not be handed raw. Step (a) now refuses it, so they move onto `tb\udcffx`, the byte 0xff, the one name that does not print and still reaches git. They are POSIX-only and skipped where the disk refuses the byte, and one refusal row keeps U+2028. `test_the_branch_and_its_upstream_are_spelled_in_the_move_back` is skipped with its reason: reading an upstream named with that byte is #710's traceback, which #824 fixes, and whichever of #823 and #824 lands second drops the skip. NIT-1: a quoted flow sequence item's value is asked for a bracket before it is split again, so `k: ["a": b[: c]` names the `[` in #301's words. NIT-2: `startswith` at two tests. NIT-3: `_read_as` returns `_reading_in_words(value)`. SHOULD-2: `_block_scalar_body`'s docstring says a line of Unicode spaces is text only at the block's own indentation or deeper. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 9 +- scripts/repo_shape.py | 17 +- scripts/shape_materialize.py | 2 +- templates/assembly-root/scripts/bootstrap.py | 36 +++- .../scripts/validate-manifest.py | 30 ++- tests/test_bootstrap_after_bump.py | 33 ++- tests/test_bootstrap_branch_bytes.py | 35 +++- tests/test_bootstrap_spelling.py | 75 +++++-- tests/test_yaml_reader_spaces.py | 196 +++++++++++++++++- update-shape.py | 10 +- 10 files changed, 388 insertions(+), 55 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index b4e8e2b4..2e685f99 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -243,7 +243,14 @@ def display_name(self) -> str: @property def tracking(self) -> str: - return str(self.data.get("tracking_branch") or "main") + """The tracking branch, read through `checked_value`, as `family.py` + reads its own (#715). Compared raw, `tracking_branch: main` followed + by U+00A0, which the reader reads as YAML does since #715, was not + `main`, so `_branch_to_bump_from` let a commit be made on `main`. + Every run asks this before it fetches or writes, and a name + `checked_value` refuses is `unsafe-value`, exit 2.""" + return checked_value("tracking_branch", + self.data.get("tracking_branch") or "main") # --------------------------------------------------------------------------- diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 1b0d912c..e025e063 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -617,7 +617,7 @@ def _refuse_entry_indicator(text: str) -> None: """ first, after = text[:1], text[1:2] if (first in ":#@`%" or first in "-?" and after in (" ", "\t") - or text[:2] == "?:" and text[2:3] in ("", " ", "\t") + or text.startswith("?:") and text[2:3] in ("", " ", "\t") or text == "?"): raise _indicator_refusal(text) @@ -661,10 +661,14 @@ def _flow_value(text: str, item: bool = False) -> Any: if item: _refuse_entry_indicator(text) pair = _split_key(text) - if pair is not None and _split_key(pair[1]) is not None: - raise YamlError("a flow sequence entry with a second `:` opening " - f"a value is not valid YAML: {text!r}") if pair is not None: + # A QUOTED item skipped the test above, so its value is asked + # here, before it is split again (#715): `["a": b[: c]` names + # the `[` in #301's words, not a key holding it. + _refuse_flow_indicator(pair[1]) + if _split_key(pair[1]) is not None: + raise YamlError("a flow sequence entry with a second `:` " + f"opening a value is not valid YAML: {text!r}") raise YamlError("a single-pair mapping in a flow sequence is not " f"supported: {text!r}") return _scalar(text) @@ -832,7 +836,7 @@ def _refuse_unsupported(content: str, line: int) -> None: raise YamlError(f"line {line}: explicit keys are not supported") if content.startswith("<<"): raise YamlError(f"line {line}: merge keys are not supported") - if content[:1] == ":" and content[1:2] in ("", " ", "\t"): + if content.startswith(":") and content[1:2] in ("", " ", "\t"): raise YamlError(f"line {line}: empty keys are not supported") @@ -856,7 +860,8 @@ def _block_scalar_body(raw_lines: list[str], i: int, INDENTED than the key, with the first such line's column taken as the block's own indentation and stripped from all of them. A blank line belongs to the body whatever its column, because it has none. A line of - Unicode spaces is not blank (#715): it is text, as YAML reads it. + Unicode spaces at the block's own indentation or deeper is not blank + (#715): it is text, as YAML reads it. """ body: list[str] = [] block_indent = None diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index edbb8682..53db0388 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -4206,7 +4206,7 @@ def _read_as(value) -> str: of a plan as no value (#181). A refusal (`_read_back`) is "nothing at all", in `_reading_in_words`' words (#715).""" if isinstance(value, YamlError): - return f"nothing at all, because it refuses the line ({value})" + return _reading_in_words(value) if isinstance(value, bool): return f"the boolean {str(value).lower()}" if isinstance(value, (int, float)): diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index f92f8047..ce3903c5 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -183,6 +183,12 @@ def _git_runs(unread: str) -> Iterator[None]: f"({exc.strerror or exc})", GIT_NOT_RUNNABLE) from exc +#: What `_argument_fault` calls a character that does not print, or white +#: space (#715): git takes the name, and no person can tell it from the name +#: without it, so `_say_not_tried` gives it a line of its own. +UNSEEN = "a character that does not print or is white space" + + def _argument_fault(branch: str) -> str | None: """What stops `branch` being an argument to a command, or `None` (#455). @@ -199,8 +205,8 @@ def _argument_fault(branch: str) -> str | None: exception `_try_git` would then have to swallow: a `UnicodeDecodeError`, which `git_out` raises over output that is not UTF-8, is a `ValueError` too, and catching that there would read a leg whose HEAD is on such a - branch as detached. Only a NUL and a lone surrogate are looked for; a - character the system's encoding merely lacks is not this fix's. + branch as detached. A character the system's encoding merely lacks is + not this fix's. A surrogate that is ONE BYTE of a name is not refused. POSIX spells a name that is not UTF-8 with U+DC80 to U+DCFF and `os.fsencode` writes each back @@ -210,12 +216,24 @@ def _argument_fault(branch: str) -> str | None: is not UTF-8 for ANY of them, so a lone surrogate is never handed to git there either, where a `CreateProcess` would pass it on mangled. The answer is `os.fsencode`'s own, so it is the system's and not a list kept here. + + NOR IS A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715), `UNSEEN`, + which `validate-manifest.py` and `siblings.py` refuse in a branch too. + git takes `main` followed by U+00A0, and since #715 the reader reads that + no-break space as YAML does, where it used to strip it: so this step made + a second `main` beside the first in every leg and checked it out. Asked + once the name holds neither a NUL nor a lone surrogate, and never of a + surrogate, so the byte-carrying ones above still reach git. """ for char in branch: if char == "\x00": return "U+0000, a NUL" if "\ud800" <= char <= "\udfff" and _fsencoded_length(char) != 1: return f"U+{ord(char):04X}, a lone surrogate" + for char in branch: + if not "\ud800" <= char <= "\udfff" and ( + not char.isprintable() or char.isspace()): + return f"U+{ord(char):04X}, {UNSEEN}" return None @@ -230,7 +248,19 @@ def _fsencoded_length(char: str) -> int: def _say_not_tried(where: str, named: str, fault: str) -> None: """A branch `_argument_fault` found: no command was run with it. The leg stays where it is, and the line says which of the three places the - person's value can have come from holds it (#455).""" + person's value can have come from holds it (#455). + + `UNSEEN` (#715) has its own words: git would take the name, so "which no + command can be given" would be wrong, and the name to give is the one + without that character.""" + if fault.endswith(UNSEEN): + print(f" {where}: could not create branch {named}: it holds " + f"{fault}, which no person can tell from the name without it; " + "the leg is left as it is") + print(" the branch is the leg's `branch:`, else `--branch`, " + "else `tracking_branch`. Give that one a name with no such " + "character and run bootstrap again.") + return print(f" {where}: could not create branch {named}: it holds {fault}, " "which no command can be given as an argument; the leg is left as " "it is") diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index ab6344f2..7bb6f436 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -596,22 +596,32 @@ def _branch_finding(code: str, what: str, value) -> list[str]: `subprocess.py` and, with that fixed, would only say it could not place the leg, while this file said `manifest ok`. So no tool named the key. Any surrogate is found, as `name` is (#280): no UTF-8 file or console can - hold one, so none is a name anybody wrote. That is the whole of the - question. Whether the value is a name git will create (`a..b`, `x.lock`) - is `git check-ref-format`'s to answer, as the scaffold asks it for - `--tracking-branch` (#283); and a value that is not text is left alone, - as `bootstrap.py` turns it into text. + hold one, so none is a name anybody wrote. Whether the value is a name git + will create (`a..b`, `x.lock`) is `git check-ref-format`'s to answer, as + the scaffold asks it for `--tracking-branch` (#283); and a value that is + not text is left alone, as `bootstrap.py` turns it into text. + + NOR A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715), the branch + `siblings.py` refuses to use. git takes `main` followed by U+00A0, and + since #715 the reader reads that no-break space as YAML does, where it + used to strip it: so `bootstrap.py` made a second `main` beside the + first, and `bump-leg.py`'s guard of the tracking branch let a commit be + made on `main`. No person can tell such a name from the one without it. """ if not isinstance(value, str): return [] - char = next((c for c in value if c == "\x00" or "\ud800" <= c <= "\udfff"), + char = next((c for c in value if not c.isprintable() or c.isspace()), None) if char is None: return [] - kind = ("a NUL: no argument, file name or ref name can hold one" - if char == "\x00" else - "a lone surrogate: no UTF-8 file or console can hold it, so no " - "branch name is spelled with one") + if char == "\x00": + kind = "a NUL: no argument, file name or ref name can hold one" + elif "\ud800" <= char <= "\udfff": + kind = ("a lone surrogate: no UTF-8 file or console can hold it, so " + "no branch name is spelled with one") + else: + kind = ("a character that does not print or is white space: a person " + "reading the name cannot tell it from the name without it") return [_finding( code, f"{what} is {value!r}, which holds U+{ord(char):04X}, {kind}. Name a " diff --git a/tests/test_bootstrap_after_bump.py b/tests/test_bootstrap_after_bump.py index 8c368a89..1689d58c 100644 --- a/tests/test_bootstrap_after_bump.py +++ b/tests/test_bootstrap_after_bump.py @@ -90,6 +90,7 @@ from conftest import (ORG, PROJECT, REPO, SCAFFOLD, cloned_just_pushed, git, run_script) +from test_bootstrap_spelling import needs_a_byte_name BUMP = REPO / "scripts" / "bump-leg.py" MAIN = "refs/heads/main" @@ -493,15 +494,17 @@ def test_a_branch_another_worktree_has_checked_out_is_not_reset( def test_the_branch_is_spelled_in_the_lines_that_name_it(root): """A tracking branch git accepts and a terminal should not be handed - raw (U+2028), in the two lines that name it here: the checkout that is - neither, and the fast-forward. The validators are skipped, as in + raw (the byte 0xff, U+DCFF: since #715 step (a) refuses a U+2028), in + the two lines that name it here: the checkout that is neither, and + the fast-forward. The validators are skipped, as in `tests/test_bootstrap_spelling.py`: they are not what is asked.""" + needs_a_byte_name(root.parent) manifest = root / "project.yaml" text = manifest.read_bytes() assert text.count(b'tracking_branch: "main"\n') == 1, text manifest.write_bytes(text.replace(b'tracking_branch: "main"\n', - b'tracking_branch: "tb\\u2028x"\n')) - named = r"tb\u2028x" + b'tracking_branch: "tb\\udcffx"\n')) + named = r"tb\udcffx" leg = root / "spec" old = rev(leg) @@ -509,7 +512,7 @@ def printed() -> str: result = run_script(root / "scripts" / "bootstrap.py", "--skip-validators", cwd=root) assert result.returncode == 0, result.stderr + result.stdout - assert "\u2028" not in result.stdout, result.stdout + assert "\udcff" not in result.stdout, result.stdout return result.stdout assert f"branch {named} created at {old[:12]}" in printed() @@ -532,17 +535,25 @@ def printed() -> str: f"{new[:12]}, the pin") in printed() +@pytest.mark.skip(reason=( + "#710: step (a) reads a branch's upstream with a strict UTF-8 decode, " + "so an upstream named with the byte 0xff, the one name that does not " + "print and still reaches git since #715, ends it in a " + "UnicodeDecodeError. #824 fixes that; whichever of #823 and #824 " + "lands second drops this skip in its merge round.")) def test_the_branch_and_its_upstream_are_spelled_in_the_move_back(root): """The line that moves a branch back names the branch, its upstream and a command with both in it; each is spelled (#391). The state is built by hand: a commit past the pin, a remote-tracking ref at it, the branch - tracking that ref, and the leg detached at the pin.""" + tracking that ref, and the leg detached at the pin. Named with the byte + 0xff since #715, as the test above is.""" + needs_a_byte_name(root.parent) manifest = root / "project.yaml" text = manifest.read_bytes() assert text.count(b'tracking_branch: "main"\n') == 1, text manifest.write_bytes(text.replace(b'tracking_branch: "main"\n', - b'tracking_branch: "tb\\u2028x"\n')) - raw = "tb\u2028x" + b'tracking_branch: "tb\\udcffx"\n')) + raw = "tb\udcffx" leg = root / "spec" pin = rev(leg) tree = rev(leg, f"{pin}^{{tree}}") @@ -554,9 +565,9 @@ def test_the_branch_and_its_upstream_are_spelled_in_the_move_back(root): result = skip_validators(root) assert result.returncode == 0, result.stderr + result.stdout - assert "\u2028" not in result.stdout, result.stdout - command = r"git -C spec merge --ff-only 'origin/tb\u2028x'" - assert moved_back(ahead, pin, r"tb\u2028x", command=command) in ( + assert "\udcff" not in result.stdout, result.stdout + command = r"git -C spec merge --ff-only 'origin/tb\udcffx'" + assert moved_back(ahead, pin, r"tb\udcffx", command=command) in ( result.stdout), result.stdout assert rev(leg, f"refs/heads/{raw}") == pin and rev(leg) == pin assert head_ref(leg) is not None diff --git a/tests/test_bootstrap_branch_bytes.py b/tests/test_bootstrap_branch_bytes.py index bab82102..7c7ed8a3 100644 --- a/tests/test_bootstrap_branch_bytes.py +++ b/tests/test_bootstrap_branch_bytes.py @@ -180,7 +180,6 @@ def test_both_keys_are_named_when_both_hold_one(project, tmp_path): @pytest.mark.parametrize("scalar", [ '"release/1.2"', '"feature/日本"', - '"a\\u200db"', # a zero-width joiner is text, and is not asked about '"a\\xe9b"', # U+00E9 is a character, not a surrogate '"\\U0001F600"', # the one-escape spelling of an emoji #280 asks for "2026", # not text: `bootstrap.py` turns it into text @@ -188,8 +187,10 @@ def test_both_keys_are_named_when_both_hold_one(project, tmp_path): ]) def test_a_value_that_is_not_one_of_the_two_is_left_alone( project, tmp_path, scalar): - """The question is the two characters and no wider: neither key's other - values are the validator's to judge here.""" + """The question is the two characters, and since #715 a character that + does not print or is white space (`tests/test_yaml_reader_spaces.py` + holds those rows), and no wider: neither key's other values are the + validator's to judge here.""" tracking(project, scalar) leg_branch(project, "code", scalar) code, out, err = run(validator_of(project), encoding=ENCODING, @@ -377,11 +378,12 @@ def bootstrap_module(): @pytest.mark.parametrize("branch", [ - "main", "release/1.2", "feature/日本", "a‍b", "a\xe9b", "a b", - "a\x1bb", "a..b", "", "\udc80", "a\udcffb"]) + "main", "release/1.2", "feature/日本", "a\xe9b", "a..b", "", "\udc80", + "a\udcffb"]) def test_a_branch_the_system_carries_has_no_fault(bootstrap_module, branch): - """Nothing but a NUL and a lone surrogate with no byte is a fault: a - control, a space, `..`, a character the system's encoding may lack, and + """Nothing but a NUL, a lone surrogate with no byte and, since #715, a + character that does not print or is white space is a fault (the test + after next): `..`, a character the system's encoding may lack, and the byte-carrying surrogates U+DC80 to U+DCFF are the commands'. On a system whose `os.fsencode` does not carry U+DC80 and U+DCFF (Windows) the next test is the one that holds.""" @@ -408,6 +410,25 @@ def test_a_nul_and_a_surrogate_without_a_byte_are_the_fault( assert bootstrap_module._argument_fault(branch) == fault +@pytest.mark.parametrize("branch, point", [ + ("main\xa0", "U+00A0"), + ("a\N{ZERO WIDTH JOINER}b", "U+200D"), + ("a b", "U+0020"), + ("a\x1bb", "U+001B"), + # A byte-carrying surrogate is not asked, so the ESC after it is named. + ("a\udcff\x1b", "U+001B"), +]) +def test_a_character_no_person_can_see_is_the_fault(bootstrap_module, + branch, point): + """Since #715: git takes each of these names, and no person reading one + can tell it from the name without the character. Step (a) makes no ref + of it, as `validate-manifest.py` and `siblings.py` refuse it too.""" + if "\udcff" in branch and len(os.fsencode("\udcff")) != 1: + pytest.skip("this system's `os.fsencode` makes no byte of U+DCFF") + assert bootstrap_module._argument_fault(branch) == ( + f"{point}, {bootstrap_module.UNSEEN}") + + @pytest.mark.parametrize("branch", ["a\udcffb", "\udc80", "a\udcfeb"]) def test_where_a_surrogate_is_three_bytes_and_no_byte_it_is_refused( bootstrap_module, monkeypatch, branch): diff --git a/tests/test_bootstrap_spelling.py b/tests/test_bootstrap_spelling.py index 715c05ad..7120f49c 100644 --- a/tests/test_bootstrap_spelling.py +++ b/tests/test_bootstrap_spelling.py @@ -50,6 +50,7 @@ import ast import io +import os import shutil import subprocess import sys @@ -165,6 +166,21 @@ def bootstrap_of(root: Path) -> Path: return root / "scripts" / "bootstrap.py" +def needs_a_byte_name(where: Path) -> None: + """Skip where a branch cannot be named with the byte 0xff, which POSIX + spells U+DCFF: the one name that does not print and that step (a) still + hands to git, since #715 refuses every other (`_argument_fault`). git + stores a branch as a file, so the disk must take the byte too.""" + if len(os.fsencode("\udcff")) != 1: + pytest.skip("this system's `os.fsencode` makes no byte of U+DCFF") + probe = where / "probe-\udcff" + try: + probe.write_bytes(b"") + except (OSError, ValueError): + pytest.skip("this disk takes no file name with the byte 0xff") + probe.unlink() + + def spelled_root(root: Path, encoding: str) -> str: """The root as the first line prints it: resolved, with what the stdout cannot hold escaped, and a line separator in a directory name escaped @@ -381,42 +397,47 @@ def test_a_legs_path_is_spelled_where_it_is_not_initialized( def test_a_tracking_branch_git_will_not_make_is_spelled(project, tmp_path): """Passes on origin/main too, and is here to keep it: that line prints the - branch with `!r`, and a repr has always escaped a control.""" + branch with `!r`, and a repr has always escaped what does not print. + Since #715 a control never reaches git (`_argument_fault`), so the name + holds the byte 0xff, which does, and `..`, which git refuses.""" + needs_a_byte_name(tmp_path) edit(project, "project.yaml", 'tracking_branch: "main"', - r'tracking_branch: "a\x1bb"') + r'tracking_branch: "a\udcff..b"') code, out, err = run(bootstrap_of(project), "--skip-validators", encoding="utf-8:strict", tmp_path=tmp_path, cwd=project) assert code == 0 and "Traceback" not in err, out + err for leg in ("spec", "code"): - assert (rf" [{leg}] {leg}: could not create branch 'a\x1bb' at the " - f"pin {pin12(project, leg)}; leaving detached" + assert (rf" [{leg}] {leg}: could not create branch 'a\udcff..b' at " + f"the pin {pin12(project, leg)}; leaving detached" ) in out.split("\n"), out - assert "\x1b" not in out, out def test_a_branch_name_is_spelled_in_every_line_that_names_it(project, tmp_path): - """A branch git accepts and a terminal should not be handed raw: U+2028. - The lines that name one: created (and where the remote's is), on, and the + """A branch git accepts and a terminal should not be handed raw: the byte + 0xff, U+DCFF, the one such name step (a) hands to git since #715 (a + U+2028 is refused before git, as the test after this one holds). The + lines that name one: created (and where the remote's is), on, and the disagreement.""" + needs_a_byte_name(tmp_path) edit(project, "project.yaml", 'tracking_branch: "main"', - r'tracking_branch: "tb\u2028x"') - named = r"tb\u2028x" + r'tracking_branch: "tb\udcffx"') + named = r"tb\udcffx" spec = project / "spec" pin = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() code_pin = pin12(project, "code") tree = git("rev-parse", f"{pin}^{{tree}}", cwd=spec).stdout.strip() other = git("commit-tree", tree, "-p", pin, "-m", "elsewhere", cwd=spec).stdout.strip() - git("update-ref", "refs/remotes/origin/tb\u2028x", other, cwd=spec) + git("update-ref", "refs/remotes/origin/tb\udcffx", other, cwd=spec) def lines() -> list[str]: code, out, err = run(bootstrap_of(project), "--skip-validators", encoding="utf-8:strict", tmp_path=tmp_path, cwd=project) assert code == 0 and "Traceback" not in err, out + err - assert "\u2028" not in out, out + assert "\udcff" not in out, out return out.split("\n") first = lines() @@ -429,12 +450,38 @@ def lines() -> list[str]: assert (f" [spec] spec: on {named} at {pin[:12]} (branch tip == pin)" ) in lines() - git("commit", "-q", "--allow-empty", "-m", "local work", cwd=spec) - tip = git("rev-parse", "HEAD", cwd=spec).stdout.strip()[:12] - assert (f" [spec] spec: PIN {pin[:12]} != branch {named} tip {tip}" + # The disagreement is asked of `code`, whose branch has no upstream: + # reading `spec`'s, a name that is not UTF-8, is #710's traceback, which + # #824 fixes. This asks only how the line spells the name. + code_leg = project / "code" + git("commit", "-q", "--allow-empty", "-m", "local work", cwd=code_leg) + tip = git("rev-parse", "HEAD", cwd=code_leg).stdout.strip()[:12] + assert (f" [code] code: PIN {code_pin} != branch {named} tip {tip}" ) in lines() +def test_a_branch_no_person_can_see_is_refused_spelled(project, tmp_path): + """U+2028, the old subject of the test above, which git takes and a + terminal should not be handed raw: since #715 step (a) makes no ref of + it (`_argument_fault`), and the line that says so spells it.""" + edit(project, "project.yaml", 'tracking_branch: "main"', + 'tracking_branch: "tb\\' + 'u2028x"') + heads = {leg: git("for-each-ref", "refs/heads", cwd=project / leg).stdout + for leg in ("spec", "code")} + code, out, err = run(bootstrap_of(project), "--skip-validators", + encoding="utf-8:strict", tmp_path=tmp_path, + cwd=project) + assert code == 0 and "Traceback" not in err, out + err + assert "\N{LINE SEPARATOR}" not in out, out + for leg in ("spec", "code"): + assert (f" [{leg}] {leg}: could not create branch tb\\" "u2028x: " + "it holds U+2028, a character that does not print or is " + "white space, which no person can tell from the name " + "without it; the leg is left as it is") in out.split("\n"), out + assert git("for-each-ref", "refs/heads", + cwd=project / leg).stdout == heads[leg] + + #: A register row is a file's text. Each value carries a control or a line #: separator, and the object is a leg's repository with a path after it, so #: the row is one the readout reports. diff --git a/tests/test_yaml_reader_spaces.py b/tests/test_yaml_reader_spaces.py index 19bc5176..09fed7fe 100644 --- a/tests/test_yaml_reader_spaces.py +++ b/tests/test_yaml_reader_spaces.py @@ -33,14 +33,21 @@ from __future__ import annotations import re +import shutil import sys import pytest -from conftest import REPO, run_script +from conftest import REPO, git, run_script +from test_bootstrap_branch_bytes import (ADVICE, ENCODING, leg_branch, + legs_state, tracking, validator_of) +from test_bootstrap_spelling import bootstrap_of, pin12, run +from test_bump_leg import advanced, bump # noqa: F401 - `advanced` from test_leg_path_yaml_scalars import ( # noqa: F401 - `bare_lines` OFFER, SPEC, assert_nothing_was_created, bare_lines, scaffold, sentence) from test_scaffold_pin_and_reuse import scaffolder # noqa: F401 - a fixture +from test_update_shape import (CHANGED_SOURCE, UPDATE, + scaffold_upstream_and_project) from test_yaml_indicator_openers import (EXPLICIT_KEY, NEEDS_PYYAML, OPENS, PAIR, fuzz_documents, ours, refused, stock) @@ -412,6 +419,19 @@ def test_pyyaml_reads_each_as_a_single_pair(text, entry, pyyaml_reads): assert stock(text) == pyyaml_reads +@pytest.mark.parametrize("text, value, held", [ + pytest.param('k: ["a": b[: c]', "b[: c", "[", id="a-bracket"), + pytest.param('k: ["a": b{: c]', "b{: c", "{", id="a-brace"), +]) +def test_a_quoted_items_value_holding_a_bracket_keeps_301s_words( + text, value, held): + """RV823's NIT-1: a QUOTED item skips the bracket test its plain twin + gets, so the value was split again and named as a key holding `[`.""" + assert refusal_words(text) == ( + f"a plain scalar in a flow collection holds `{held}`, which ends it " + f"in YAML: {value!r}") + + # --- L4-153: a template line the reader refuses ----------------------------- #: What the leg-path rule says the project's reader makes of a bare `@x`. @@ -487,3 +507,177 @@ def test_the_docstrings_example_is_one_the_reader_reads( assert parse_yaml(f"id: {value}\n") == {"id": value} assert scaffolder._neutral_pin_entry(entry) == double_quoted(entry) assert refused(ours("id: -")) + + +# --- RV823 MUST-1: a branch no person can tell from another ------------------ +# +# The reader now reads `tracking_branch: main` followed by U+00A0 as PyYAML +# reads it, `'main\xa0'`, where main read `'main'`. git takes that name, so +# the four readers of a branch refuse it as `siblings.py` does: a character +# that does not print or is white space. + +#: `(the YAML line's value, bare as a person types it, the code point named)`. +UNSEEN_BRANCHES = [ + pytest.param("main\xa0", "U+00A0", id="main-and-a-no-break-space"), + pytest.param("\xa0", "U+00A0", id="a-no-break-space-alone"), + pytest.param("main\N{IDEOGRAPHIC SPACE}", "U+3000", + id="main-and-an-ideographic-space"), + pytest.param("a\N{ZERO WIDTH JOINER}b", "U+200D", + id="a-zero-width-joiner"), +] + +#: What `validate-manifest.py` says after the code point. +UNSEEN_WHY = ("a character that does not print or is white space: a person " + "reading the name cannot tell it from the name without it.") + + +def unseen_finding(code: str, what: str, value: str, point: str) -> str: + return (f"FINDING {code}: {what} is {value!r}, which holds {point}, " + f"{UNSEEN_WHY} {ADVICE}") + + +@pytest.mark.parametrize("value, point", UNSEEN_BRANCHES) +def test_the_validator_names_a_tracking_branch_no_person_can_see( + project, tmp_path, value, point): + tracking(project, value) + code, out, err = run(validator_of(project), encoding=ENCODING, + tmp_path=tmp_path, cwd=project) + assert (code, out) == (1, ""), out + err + assert err.split("\n") == [ + unseen_finding("manifest-tracking-branch", "tracking_branch", value, + point), + "", f"1 finding(s) in {project / 'project.yaml'}", ""], err + + +def test_the_validator_names_a_legs_branch_no_person_can_see( + project, tmp_path): + leg_branch(project, "spec", "main\xa0") + code, out, err = run(validator_of(project), encoding=ENCODING, + tmp_path=tmp_path, cwd=project) + assert (code, out) == (1, ""), out + err + assert err.split("\n")[0] == unseen_finding( + "manifest-leg-branch", "leg 'spec': branch", "main\xa0", "U+00A0"), err + + +def not_placed(leg: str, value: str, point: str) -> list[str]: + """Step (a)'s two lines for a leg whose branch no person can see. The + name is printed as `spelled` prints it, which leaves a no-break or an + ideographic space as it is (#391), so the line names its code point.""" + return [f" [{leg}] {leg}: could not create branch " + f"{value}: it holds {point}, a character that does " + "not print or is white space, which no person can tell from the " + "name without it; the leg is left as it is", + " the branch is the leg's `branch:`, else `--branch`, " + "else `tracking_branch`. Give that one a name with no such " + "character and run bootstrap again."] + + +def step_a(out: str) -> list[str]: + lines = out.split("\n") + return lines[lines.index("(a) legs on tracking branches, pins untouched") + + 1:lines.index("(b) neutral validators") - 1] + + +@pytest.mark.parametrize("value, point", UNSEEN_BRANCHES[:3]) +def test_bootstrap_makes_no_ref_of_a_tracking_branch_no_person_can_see( + project, tmp_path, value, point): + """On #823's first head each leg gained `refs/heads/main` followed by + U+00A0 beside `main`, checked out, and the run ended `bootstrap ok`.""" + tracking(project, value) + before = legs_state(project) + code, out, err = run(bootstrap_of(project), encoding=ENCODING, + tmp_path=tmp_path, cwd=project) + assert code == 1 and "Traceback" not in err, out + err + assert step_a(out) == (not_placed("spec", value, point) + + not_placed("code", value, point)), out + assert "bootstrap FAILED: manifest (exit 1)" in err, err + assert legs_state(project) == before + + +def test_bootstrap_makes_no_ref_of_a_legs_branch_no_person_can_see( + project, tmp_path): + leg_branch(project, "spec", "main\xa0") + before = legs_state(project) + code, out, err = run(bootstrap_of(project), encoding=ENCODING, + tmp_path=tmp_path, cwd=project) + assert code == 1 and "Traceback" not in err, out + err + assert step_a(out) == not_placed("spec", "main\xa0", "U+00A0") + [ + f" [code] code: on main at {pin12(project, 'code')} " + "(branch tip == pin)"], out + assert legs_state(project)["spec"] == before["spec"] + + +def committed_tracking(root, value: str) -> str: + """`tracking_branch: ` hand-edited into the root and committed, as + RV823's scenario has it; the root's new HEAD.""" + tracking(root, value) + git("add", "--", "project.yaml", cwd=root) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", + "hand-edit the tracking branch", cwd=root) + return git("rev-parse", "HEAD", cwd=root).stdout.strip() + + +@pytest.mark.parametrize("dry_run", [(), ("--dry-run",)], + ids=["real-run", "dry-run"]) +@pytest.mark.parametrize("value, point", UNSEEN_BRANCHES[:3]) +def test_bump_leg_refuses_a_tracking_branch_no_person_can_see( + project, scaffolded, advanced, value, point, # noqa: F811 + dry_run): + """On #823's first head, on `main`: exit 0, a commit on `main`, and a + NEXT line pushing it, where `bump-leg-on-tracking-branch` was owed. + `checked_value` refuses it now, before anything is fetched or written.""" + head = committed_tracking(project, value) + pin = git("rev-parse", "HEAD:spec", cwd=project).stdout + result = bump(project, scaffolded, advanced, *dry_run) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED unsafe-value: tracking_branch is {value!r}, which is not a " + "value this tool will put on a `git` or `gh` command line"), ( + result.stderr) + assert git("rev-parse", "HEAD", cwd=project).stdout.strip() == head + assert git("rev-parse", "HEAD:spec", cwd=project).stdout == pin + assert git("status", "--porcelain", cwd=project).stdout == "" + + +@pytest.fixture(scope="module") +def older_validator(tmp_path_factory) -> dict: + """A project scaffolded at A, and B: an upstream commit whose manifest + validator asks a branch for a NUL alone, as one older than #715 does. + So the root's own validators pass a hand-edited `main` followed by + U+00A0 after `apply`, and update-shape's own guard is what is asked.""" + _base, upstream, commit_a, clone = scaffold_upstream_and_project( + tmp_path_factory, "w715-older-validator") + source = upstream / CHANGED_SOURCE + text = source.read_text(encoding="utf-8") + asked = "if not c.isprintable() or c.isspace())," + assert text.count(asked) == 1, "the validator no longer asks this" + source.write_text(text.replace(asked, 'if c == "\\x00"),'), + encoding="utf-8") + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", + "An older manifest validator", "--", CHANGED_SOURCE, cwd=upstream) + commit_b = git("rev-parse", "HEAD", cwd=upstream).stdout.strip() + return {"upstream": upstream, "clone": clone, "a": commit_a, + "b": commit_b} + + +def test_update_shape_never_commits_onto_a_tracking_branch_no_one_can_see( + older_validator, tmp_path): + """`apply --branch main` on `main`, `tracking_branch: main` followed by + U+00A0. On #823's first head the guard compared `'main'` with + `'main\xa0'`, found them different, and committed the shape update onto + `main`. Now `checked_value` refuses the value, and the ledger rolls the + rewrite back: the pin, HEAD and the tree are as they were.""" + root = tmp_path / "Atlas" + shutil.copytree(older_validator["clone"], root, symlinks=True) + head = committed_tracking(root, "main\xa0") + result = run_script(UPDATE, "apply", "--root", str(root), "--yes", + "--upstream", str(older_validator["upstream"]), + "--at", older_validator["b"], "--branch", "main") + assert result.returncode == 2, result.stdout + result.stderr + assert ("REFUSED unsafe-value: tracking_branch is 'main\\xa0', which is " + "not a value this tool will put on a `git` or `gh` command line" + in result.stderr), result.stderr + assert git("rev-parse", "HEAD", cwd=root).stdout.strip() == head + pinned = load_yaml(root / "contracts" / "shape-pin.yaml")["commit"] + assert pinned.lower() == older_validator["a"] + assert git("status", "--porcelain", cwd=root).stdout == "" diff --git a/update-shape.py b/update-shape.py index bc6eb703..6b74df5a 100755 --- a/update-shape.py +++ b/update-shape.py @@ -2423,8 +2423,16 @@ def tracking_branch_of(manifest: dict | None) -> str: `bump-leg.py` and `family.py` read the field with. The remote's own HEAD is never asked: a base taken from the network could disagree with the one the `--branch` refusal and `--pr` read from the manifest. + + READ THROUGH `checked_value`, as `bump-leg.py` and `family.py` read it + (#715). Compared raw, `tracking_branch: main` followed by U+00A0, which + the reader reads as YAML does since #715, was not `main`, so + `apply --branch main` committed onto `main` where the root's own + validators passed. A name `checked_value` refuses is `unsafe-value`, + and inside `apply` the ledger rolls the rewrite back. """ - return str((manifest or {}).get("tracking_branch") or "main") + return checked_value("tracking_branch", + (manifest or {}).get("tracking_branch") or "main") def pull_request_repository(manifest: dict | None, From 73b7994cad026f228166b7cbea7a921891e1ca1c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 19:44:41 +0000 Subject: [PATCH 3/6] tests: the #715 rows push their own advanced spec commit, so `test_bump_leg.py` and this file share a session again, and `update-shape.py`'s base test holds that `tracking_branch: "a&b"` is refused before the NEXT line, not quoted into it (#715) Two faults of `53af978e`, found by its targeted run (62 files: 2 failed, 5086 passed, 15 skipped, 1 xfailed, 47 errors). - `tests/test_yaml_reader_spaces.py` imported `test_bump_leg.py`'s module-scoped `advanced`, which pushes a new commit to `bump-leg-advance` on the session's shared spec remote. Each module that names the fixture gets its own instance, so whichever of the two ran second was refused `non-fast-forward`: here all 47 setups of `test_bump_leg.py` errored, and in the full suite, where `test_bump_leg.py` runs first, the six bump-leg rows of this file would have. The file now has its own `advanced`, pushed to `yaml-reader-spaces-advance`, as `test_bump_leg_rollback.py`, `test_bump_leg_paste_lines.py`, `test_bump_leg_unreadable.py` and `test_bump_leg_unopenable.py` each do. - `tests/test_update_shape_pr_base.py`'s `test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_line` gave `next_line` `tracking_branch: "a&b"` and asked for it quoted after `--base`. Since `tracking_branch_of` reads through `checked_value` (RV823's Q2), `a&b` is `REFUSED unsafe-value` there, and in `apply --branch` before the commit. The test keeps the repository's quoting with the base `release/1.2`, and a new one, `test_a_tracking_branch_a_shell_would_misread_never_reaches_the_line`, holds the refusal and that no line is printed. Mutant M15 (`tracking_branch_of` read raw) fails both of its rows. Runs: `test_yaml_reader_spaces.py`, `test_bump_leg.py` and `test_update_shape_pr_base.py` in that order, 390 passed, 1 skipped; `test_bump_leg.py` then `test_yaml_reader_spaces.py`, 361 passed, 1 skipped. The skip is `test_bump_leg.py`'s case-insensitive disk test, on Linux. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- tests/test_update_shape_pr_base.py | 39 ++++++++++++++++++++++++------ tests/test_yaml_reader_spaces.py | 27 ++++++++++++++++++--- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/tests/test_update_shape_pr_base.py b/tests/test_update_shape_pr_base.py index 1b8237ac..7e07233b 100644 --- a/tests/test_update_shape_pr_base.py +++ b/tests/test_update_shape_pr_base.py @@ -367,12 +367,13 @@ def test_nothing_in_the_next_line_names_a_base_when_the_pull_request_is_open( def test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_line( update_shape, tmp_path, capsys, push): """The base and the repository come out of a manifest a human may have - edited, and the line is pasted into a shell. `validate-manifest.py` never - looks at `tracking_branch:`, so `a&b` reaches the line, and an unquoted - `&` would end the command there. What a shell READS from the line is - compared, not the text, so any quoting that says the same thing passes.""" + edited, and the line is pasted into a shell. An unquoted `'` in the + repository would open a quotation there. What a shell READS from the line + is compared, not the text, so any quoting that says the same thing + passes. The base is one `checked_value` takes: the next test holds what + becomes of one it does not.""" (tmp_path / "project.yaml").write_text( - 'tracking_branch: "a&b"\n' + 'tracking_branch: "release/1.2"\n' "legs:\n" " - role: assembly\n" ' repository: "acme/It\'s here"\n', encoding="utf-8") @@ -382,11 +383,35 @@ def test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_line( command = line.partition(" && ")[2] if not push else line[len("NEXT "):] words = shlex.split(command) assert words[:9] == [ - "gh", "pr", "create", "--repo", "acme/It's here", "--base", "a&b", - "--head", "shape/x"], line + "gh", "pr", "create", "--repo", "acme/It's here", "--base", + "release/1.2", "--head", "shape/x"], line assert words[9] == "--title" and words[11] == "--body", line +@pytest.mark.parametrize("push", [True, False], ids=["branch-push", "branch"]) +def test_a_tracking_branch_a_shell_would_misread_never_reaches_the_line( + update_shape, tmp_path, capsys, push): + """`tracking_branch_of` reads through `checked_value` since #715 (RV823's + Q2), so `a&b`, whose `&` would end the pasted command, is refused as + `unsafe-value` and no line is printed. Before, it was quoted into the + line. In `apply --branch` the same refusal comes before the commit, from + `commit_on_branch`'s guard (and, under `--push` or `--pr`, from + `pull_request_line` before it), and is rolled back.""" + (tmp_path / "project.yaml").write_text( + 'tracking_branch: "a&b"\n' + "legs:\n" + " - role: assembly\n" + ' repository: "acme/It\'s here"\n', encoding="utf-8") + args = SimpleNamespace(branch="shape/x", push=push, pr=False) + with pytest.raises(update_shape.Refusal) as refused: + update_shape.next_line(tmp_path, args, [], "0" * 40, []) + assert refused.value.code == "unsafe-value" + assert str(refused.value).startswith( + "REFUSED unsafe-value: tracking_branch is 'a&b', which is not a value " + "this tool will put on a `git` or `gh` command line"), refused.value + assert capsys.readouterr().out == "" + + FAMILY_SOURCE = "templates/family-root/scripts/validate-family.py" FAMILY_FIX_LINE = "\n# An upstream fix that must reach every family.\n" HOLDER = "Contoso" diff --git a/tests/test_yaml_reader_spaces.py b/tests/test_yaml_reader_spaces.py index 09fed7fe..a34bae89 100644 --- a/tests/test_yaml_reader_spaces.py +++ b/tests/test_yaml_reader_spaces.py @@ -38,11 +38,11 @@ import pytest -from conftest import REPO, git, run_script +from conftest import PROJECT, REPO, cloned_just_pushed, git, run_script from test_bootstrap_branch_bytes import (ADVICE, ENCODING, leg_branch, legs_state, tracking, validator_of) from test_bootstrap_spelling import bootstrap_of, pin12, run -from test_bump_leg import advanced, bump # noqa: F401 - `advanced` +from test_bump_leg import bump from test_leg_path_yaml_scalars import ( # noqa: F401 - `bare_lines` OFFER, SPEC, assert_nothing_was_created, bare_lines, scaffold, sentence) from test_scaffold_pin_and_reuse import scaffolder # noqa: F401 - a fixture @@ -617,11 +617,32 @@ def committed_tracking(root, value: str) -> str: return git("rev-parse", "HEAD", cwd=root).stdout.strip() +#: Where this module's advanced spec commit is pushed. Not `test_bump_leg.py`'s +#: branch: both modules share the session's bare remotes, and a second, +#: different commit pushed to the same branch name is refused as +#: non-fast-forward, which errors every test of whichever module runs second. +ADVANCE_BRANCH = "yaml-reader-spaces-advance" + + +@pytest.fixture(scope="module") +def advanced(scaffolded, tmp_path_factory) -> str: + """A real new commit on the spec leg's bare remote; returns its sha.""" + work = cloned_just_pushed( + scaffolded["remotes"] / f"{PROJECT}-spec.git", + tmp_path_factory.mktemp("spec-advance-spaces") / f"{PROJECT}-spec") + (work / "SPACES-CONTRACT.md").write_text("# a contract the leg gained\n") + git("add", "--", "SPACES-CONTRACT.md", cwd=work) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Advance the spec leg for the #715 tests", cwd=work) + git("push", "-q", "origin", f"HEAD:refs/heads/{ADVANCE_BRANCH}", cwd=work) + return git("rev-parse", "HEAD", cwd=work).stdout.strip() + + @pytest.mark.parametrize("dry_run", [(), ("--dry-run",)], ids=["real-run", "dry-run"]) @pytest.mark.parametrize("value, point", UNSEEN_BRANCHES[:3]) def test_bump_leg_refuses_a_tracking_branch_no_person_can_see( - project, scaffolded, advanced, value, point, # noqa: F811 + project, scaffolded, advanced, value, point, dry_run): """On #823's first head, on `main`: exit 0, a commit on `main`, and a NEXT line pushing it, where `bump-leg-on-tracking-branch` was owed. From 716ad45771ae9d199e9f9ab5fae9a49ea62f907a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 20:33:50 +0000 Subject: [PATCH 4/6] =?UTF-8?q?bump-leg.py=20and=20update-shape.py:=20a=20?= =?UTF-8?q?tracking=20branch=20is=20refused=20only=20for=20a=20character?= =?UTF-8?q?=20that=20does=20not=20print=20or=20is=20white=20space,=20so=20?= =?UTF-8?q?`feature/`=20and=20CJK,=20`d=C3=A9veloppement`=20and=20`a&b`=20?= =?UTF-8?q?are=20read=20as=20they=20are,=20#209's=20quoted=20base=20is=20b?= =?UTF-8?q?ack,=20and=20a=20quoted=20key=20whose=20value=20opens=20a=20flo?= =?UTF-8?q?w=20collection=20keeps=20the=20single-pair=20words=20(#715)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 2, after RV823's re-check of `73b7994c` (READY, no MUST), on Brett Heap's ruling on its SHOULD-1, "go with option A for #823" (2026-10-17T20:23:58Z, #823 issuecomment-6240468893). - Option A. `Project.tracking` and `tracking_branch_of` read through `checked_value` since `53af978e`, whose `SAFE_ARG_RE` refused names git creates and the root's own gate passes: `feature/` and CJK, `développement` (NFC and NFD) and `a&b`. They now read through `readable_branch`, a new helper in `scripts/repo_shape.py` beside `checked_value`. It refuses a character for which `str.isprintable()` is false or `str.isspace()` is true, as `validate-manifest.py`'s `_branch_finding` and `bootstrap.py`'s `_argument_fault` do, in their words: `REFUSED unseen-branch: tracking_branch is 'main\xa0', which holds U+00A0, a character that does not print or is white space: a person reading the name cannot tell it from the name without it`. It also refuses a leading `-` (`unsafe-value`): `update-shape.py` hands the name to `gh pr create --base` as one argument, and git creates no branch whose name begins with one. `bump-leg.py` never hands it to a command; it compares it and prints it. - #209's `test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_line` is restored as it was, `tracking_branch: "a&b"` quoted after `--base`, and its refusal twin is now `test_a_tracking_branch_no_person_can_see_never_reaches_the_line`, on `main\xa0`. - Rows: the four names are taken by bump-leg (real run and `--dry-run`, on `main`), by `apply --branch shape/x` on a root (its NEXT line's `--base` reads back as the name), and by `tracking_branch_of` in-process; `tracking_branch_of` refuses every name the validator does, and a leading `-`. - NIT-1. `["a": [b]]`, `["a": {b: c}]` and `['a': [b, c]]` got #301's plain-scalar words from round 1's bracket test of a quoted item's value. A value that opens a flow collection is not asked it, so they keep the single-pair words main gives them; PyYAML reads each as one pair. - NIT-2. The three composite asserts this PR added (`code == … and "Traceback" not in err`) are split. - NIT-3. A validator row for `a b`, the one character `isspace` adds to `isprintable`. - NIT-4. The #710 test is `xfail(strict=True, raises=AssertionError)`, not skipped, so the merge round that brings #823 and #824 together sees it pass. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 25 ++-- scripts/repo_shape.py | 57 ++++++++- tests/test_bootstrap_after_bump.py | 6 +- tests/test_bootstrap_spelling.py | 3 +- tests/test_update_shape_pr_base.py | 37 +++--- tests/test_yaml_reader_spaces.py | 180 ++++++++++++++++++++++++++--- update-shape.py | 20 ++-- 7 files changed, 272 insertions(+), 56 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 2e685f99..0b0b9852 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -121,7 +121,7 @@ sys.path.insert(0, str(SHAPE_ROOT / "scripts")) from repo_shape import ( # noqa: E402 COMMIT_RE, SAFE_PATH_RE, Refusal, checked_value, git_out, load_yaml, - recorded_gitlink, repo_basename, tree_digest, + readable_branch, recorded_gitlink, repo_basename, tree_digest, ) from shape_materialize import ( # noqa: E402 CommandFailed, captured_child_environment, check_program, commit_trailers, @@ -243,14 +243,21 @@ def display_name(self) -> str: @property def tracking(self) -> str: - """The tracking branch, read through `checked_value`, as `family.py` - reads its own (#715). Compared raw, `tracking_branch: main` followed - by U+00A0, which the reader reads as YAML does since #715, was not - `main`, so `_branch_to_bump_from` let a commit be made on `main`. - Every run asks this before it fetches or writes, and a name - `checked_value` refuses is `unsafe-value`, exit 2.""" - return checked_value("tracking_branch", - self.data.get("tracking_branch") or "main") + """The tracking branch, read through `readable_branch` (#715): a + name holding a character that does not print or is white space is + `unseen-branch`, exit 2, and any other name is taken as it is. + + Compared raw, `tracking_branch: main` followed by U+00A0, which the + reader reads as YAML does since #715, was not `main`, so + `_branch_to_bump_from` let a commit be made on `main`. This tool + never hands the name to a command: it compares it + (`_branch_to_bump_from`, the dry run's last line) and prints it. So + `checked_value`'s pattern, which #823's first fix round read it + through, is not asked: it refused `feature/` and CJK, `a&b` and + every other name git creates and the root's own gate passes. Every + run asks this before it fetches or writes.""" + return readable_branch("tracking_branch", + self.data.get("tracking_branch") or "main") # --------------------------------------------------------------------------- diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index e025e063..a6ae2c30 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -664,8 +664,11 @@ def _flow_value(text: str, item: bool = False) -> Any: if pair is not None: # A QUOTED item skipped the test above, so its value is asked # here, before it is split again (#715): `["a": b[: c]` names - # the `[` in #301's words, not a key holding it. - _refuse_flow_indicator(pair[1]) + # the `[` in #301's words, not a key holding it. A value that + # OPENS a flow collection is not a plain scalar: `["a": [b]]` + # is a single pair to YAML, and keeps the single-pair words. + if not pair[1].startswith(("[", "{")): + _refuse_flow_indicator(pair[1]) if _split_key(pair[1]) is not None: raise YamlError("a flow sequence entry with a second `:` " f"opening a value is not valid YAML: {text!r}") @@ -2303,6 +2306,56 @@ def checked_value(what: str, value, pattern: re.Pattern = SAFE_ARG_RE) -> str: return text +#: Why a branch name holding such a character is refused, in the words +#: `validate-manifest.py`'s `_branch_finding` and `bootstrap.py`'s `UNSEEN` +#: say it in (#715). +UNSEEN = "a character that does not print or is white space" + + +def readable_branch(what: str, value) -> str: + """A BRANCH NAME read out of a manifest, refused only for what no person + can see in it (#715; Brett Heap's option A on RV823's SHOULD-1). + + The rule is the root's own. `validate-manifest.py`'s `_branch_finding` + and `bootstrap.py`'s `_argument_fault` refuse a branch holding a + character for which `str.isprintable()` is false or `str.isspace()` is + true, so `main` followed by U+00A0, which the reader reads as YAML does + since #715, is never taken for `main`. Those two ship into a root and + import nothing from here, so the rule is spelled again, not shared. + + It is NOT `checked_value`'s `SAFE_ARG_RE`. A name in any script, or one + holding `&`, is a name git creates and the root's own gate passes. + `bump-leg.py` only compares it and prints it, and `update-shape.py` + compares it, hands it to `gh pr create --base` as one argument and + prints it shell-quoted in the line a person pastes (#209). #823's first + fix round read it through `checked_value`, and refused those names. + + A leading `-` is refused too: `update-shape.py` hands the name to + `gh pr create --base`, where it could be read as an option, and git + creates no branch whose name begins with one, so no root needs it. + """ + text = str(value) + if text.startswith("-"): + raise Refusal( + "unsafe-value", + f"{what} is {text!r}, which begins with `-`: a command it is " + "handed to could read it as an option, and git creates no branch " + "whose name begins with one", + "Remediation: give it a branch name git can create, such as " + "`main` or `release/1.2`.") + char = next((c for c in text if not c.isprintable() or c.isspace()), + None) + if char is not None: + raise Refusal( + "unseen-branch", + f"{what} is {text!r}, which holds U+{ord(char):04X}, {UNSEEN}: " + "a person reading the name cannot tell it from the name without " + "it", + "Remediation: give it a name with no such character, such as " + "`main` or `release/1.2`, and run this again.") + return text + + # --------------------------------------------------------------------------- # A path spelled the one way Git records it (#179, #189) # --------------------------------------------------------------------------- diff --git a/tests/test_bootstrap_after_bump.py b/tests/test_bootstrap_after_bump.py index 1689d58c..fc88deda 100644 --- a/tests/test_bootstrap_after_bump.py +++ b/tests/test_bootstrap_after_bump.py @@ -535,12 +535,12 @@ def printed() -> str: f"{new[:12]}, the pin") in printed() -@pytest.mark.skip(reason=( +@pytest.mark.xfail(strict=True, raises=AssertionError, reason=( "#710: step (a) reads a branch's upstream with a strict UTF-8 decode, " "so an upstream named with the byte 0xff, the one name that does not " "print and still reaches git since #715, ends it in a " - "UnicodeDecodeError. #824 fixes that; whichever of #823 and #824 " - "lands second drops this skip in its merge round.")) + "UnicodeDecodeError. #824 fixes that. Strict, so the merge round that " + "brings #823 and #824 together sees it pass and drops this mark.")) def test_the_branch_and_its_upstream_are_spelled_in_the_move_back(root): """The line that moves a branch back names the branch, its upstream and a command with both in it; each is spelled (#391). The state is built diff --git a/tests/test_bootstrap_spelling.py b/tests/test_bootstrap_spelling.py index 7120f49c..c3472892 100644 --- a/tests/test_bootstrap_spelling.py +++ b/tests/test_bootstrap_spelling.py @@ -471,7 +471,8 @@ def test_a_branch_no_person_can_see_is_refused_spelled(project, tmp_path): code, out, err = run(bootstrap_of(project), "--skip-validators", encoding="utf-8:strict", tmp_path=tmp_path, cwd=project) - assert code == 0 and "Traceback" not in err, out + err + assert code == 0, out + err + assert "Traceback" not in err, err assert "\N{LINE SEPARATOR}" not in out, out for leg in ("spec", "code"): assert (f" [{leg}] {leg}: could not create branch tb\\" "u2028x: " diff --git a/tests/test_update_shape_pr_base.py b/tests/test_update_shape_pr_base.py index 7e07233b..1fbbe419 100644 --- a/tests/test_update_shape_pr_base.py +++ b/tests/test_update_shape_pr_base.py @@ -367,13 +367,12 @@ def test_nothing_in_the_next_line_names_a_base_when_the_pull_request_is_open( def test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_line( update_shape, tmp_path, capsys, push): """The base and the repository come out of a manifest a human may have - edited, and the line is pasted into a shell. An unquoted `'` in the - repository would open a quotation there. What a shell READS from the line - is compared, not the text, so any quoting that says the same thing - passes. The base is one `checked_value` takes: the next test holds what - becomes of one it does not.""" + edited, and the line is pasted into a shell. `validate-manifest.py` never + looks at `tracking_branch:`, so `a&b` reaches the line, and an unquoted + `&` would end the command there. What a shell READS from the line is + compared, not the text, so any quoting that says the same thing passes.""" (tmp_path / "project.yaml").write_text( - 'tracking_branch: "release/1.2"\n' + 'tracking_branch: "a&b"\n' "legs:\n" " - role: assembly\n" ' repository: "acme/It\'s here"\n', encoding="utf-8") @@ -383,32 +382,32 @@ def test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_line( command = line.partition(" && ")[2] if not push else line[len("NEXT "):] words = shlex.split(command) assert words[:9] == [ - "gh", "pr", "create", "--repo", "acme/It's here", "--base", - "release/1.2", "--head", "shape/x"], line + "gh", "pr", "create", "--repo", "acme/It's here", "--base", "a&b", + "--head", "shape/x"], line assert words[9] == "--title" and words[11] == "--body", line @pytest.mark.parametrize("push", [True, False], ids=["branch-push", "branch"]) -def test_a_tracking_branch_a_shell_would_misread_never_reaches_the_line( +def test_a_tracking_branch_no_person_can_see_never_reaches_the_line( update_shape, tmp_path, capsys, push): - """`tracking_branch_of` reads through `checked_value` since #715 (RV823's - Q2), so `a&b`, whose `&` would end the pasted command, is refused as - `unsafe-value` and no line is printed. Before, it was quoted into the - line. In `apply --branch` the same refusal comes before the commit, from - `commit_on_branch`'s guard (and, under `--push` or `--pr`, from - `pull_request_line` before it), and is rolled back.""" + """`tracking_branch_of` reads through `readable_branch` since #715, so + `main` followed by U+00A0, which a person reads as `main`, is refused as + `unseen-branch` and no line is printed: pasted, it would open a pull + request against a second branch beside `main`. A name a person can read, + `a&b` among them, is quoted into the line, as the test above holds.""" (tmp_path / "project.yaml").write_text( - 'tracking_branch: "a&b"\n' + "tracking_branch: main\xa0\n" "legs:\n" " - role: assembly\n" ' repository: "acme/It\'s here"\n', encoding="utf-8") args = SimpleNamespace(branch="shape/x", push=push, pr=False) with pytest.raises(update_shape.Refusal) as refused: update_shape.next_line(tmp_path, args, [], "0" * 40, []) - assert refused.value.code == "unsafe-value" + assert refused.value.code == "unseen-branch" assert str(refused.value).startswith( - "REFUSED unsafe-value: tracking_branch is 'a&b', which is not a value " - "this tool will put on a `git` or `gh` command line"), refused.value + "REFUSED unseen-branch: tracking_branch is 'main\\xa0', which holds " + "U+00A0, a character that does not print or is white space"), ( + refused.value) assert capsys.readouterr().out == "" diff --git a/tests/test_yaml_reader_spaces.py b/tests/test_yaml_reader_spaces.py index a34bae89..6e7ca421 100644 --- a/tests/test_yaml_reader_spaces.py +++ b/tests/test_yaml_reader_spaces.py @@ -32,9 +32,12 @@ from __future__ import annotations +import importlib.util import re +import shlex import shutil import sys +import unicodedata import pytest @@ -432,6 +435,35 @@ def test_a_quoted_items_value_holding_a_bracket_keeps_301s_words( f"in YAML: {value!r}") +#: `(document, the item, what PyYAML reads)`: a quoted key whose value is a +#: flow collection, a single pair to YAML. +OPENS_A_COLLECTION = [ + pytest.param('k: ["a": [b]]', '"a": [b]', {"k": [{"a": ["b"]}]}, + id="a-sequence"), + pytest.param('k: ["a": {b: c}]', '"a": {b: c}', + {"k": [{"a": {"b": "c"}}]}, id="a-mapping"), + pytest.param("k: ['a': [b, c]]", "'a': [b, c]", + {"k": [{"a": ["b", "c"]}]}, id="single-quoted"), +] + + +@pytest.mark.parametrize("text, entry, pyyaml_reads", OPENS_A_COLLECTION) +def test_a_quoted_items_value_that_opens_a_collection_keeps_the_pair_words( + text, entry, pyyaml_reads): + """RV823's round-1 NIT-1: the bracket test asked of a quoted item's + value named `[b]` as a plain scalar holding `[`. It is a flow sequence, + so the item is the single pair YAML reads, refused in the words main + gives it, which this reader does not support.""" + assert refusal_words(text) == f"{PAIR}: {entry!r}" + + +@NEEDS_PYYAML +@pytest.mark.parametrize("text, entry, pyyaml_reads", OPENS_A_COLLECTION) +def test_pyyaml_reads_a_quoted_key_and_a_collection_as_one_pair( + text, entry, pyyaml_reads): + assert stock(text) == pyyaml_reads + + # --- L4-153: a template line the reader refuses ----------------------------- #: What the leg-path rule says the project's reader makes of a bare `@x`. @@ -513,8 +545,11 @@ def test_the_docstrings_example_is_one_the_reader_reads( # # The reader now reads `tracking_branch: main` followed by U+00A0 as PyYAML # reads it, `'main\xa0'`, where main read `'main'`. git takes that name, so -# the four readers of a branch refuse it as `siblings.py` does: a character -# that does not print or is white space. +# the four readers of a branch refuse a character that does not print or is +# white space, the rule of `validate-manifest.py`'s `_branch_finding` +# (`siblings.py` asks `isprintable` alone). bump-leg and update-shape refuse +# nothing else (Brett Heap's option A on RV823's SHOULD-1, 2026-10-17): a +# name git creates and the root's gate passes is read as it is. #: `(the YAML line's value, bare as a person types it, the code point named)`. UNSEEN_BRANCHES = [ @@ -524,8 +559,41 @@ def test_the_docstrings_example_is_one_the_reader_reads( id="main-and-an-ideographic-space"), pytest.param("a\N{ZERO WIDTH JOINER}b", "U+200D", id="a-zero-width-joiner"), + # The one character `isspace` adds to `isprintable` (RV823's MR3): git + # refuses `a b` too, so the refusal costs no name git would make. + pytest.param("a b", "U+0020", id="an-ascii-space"), ] +#: Names git creates and the root's own gate passes, which bump-leg and +#: update-shape read as they are (option A). #823's first fix round read +#: them through `checked_value`, whose pattern refused each, exit 2. +READABLE_BRANCHES = [ + pytest.param("feature/\N{CJK UNIFIED IDEOGRAPH-65E5}" + "\N{CJK UNIFIED IDEOGRAPH-672C}", id="cjk"), + pytest.param("d\N{LATIN SMALL LETTER E WITH ACUTE}veloppement", + id="an-accent-nfc"), + pytest.param("de\N{COMBINING ACUTE ACCENT}veloppement", + id="an-accent-nfd"), + pytest.param("a&b", id="an-ampersand"), +] + + +def test_the_two_spellings_of_the_accent_are_two_names(): + """NFC and NFD are both asked because git keeps the bytes it is given: + the two are one word to a reader and two branches to git.""" + nfc, nfd = (row.values[0] for row in READABLE_BRANCHES[1:3]) + assert nfc != nfd + assert unicodedata.normalize("NFC", nfd) == nfc + + +def unseen_refusal(value: str, point: str) -> str: + """How bump-leg and update-shape refuse such a name (`readable_branch`), + in `validate-manifest.py`'s words.""" + return (f"REFUSED unseen-branch: tracking_branch is {value!r}, which " + f"holds {point}, a character that does not print or is white " + "space: a person reading the name cannot tell it from the name " + "without it") + #: What `validate-manifest.py` says after the code point. UNSEEN_WHY = ("a character that does not print or is white space: a person " "reading the name cannot tell it from the name without it.") @@ -587,7 +655,8 @@ def test_bootstrap_makes_no_ref_of_a_tracking_branch_no_person_can_see( before = legs_state(project) code, out, err = run(bootstrap_of(project), encoding=ENCODING, tmp_path=tmp_path, cwd=project) - assert code == 1 and "Traceback" not in err, out + err + assert code == 1, out + err + assert "Traceback" not in err, err assert step_a(out) == (not_placed("spec", value, point) + not_placed("code", value, point)), out assert "bootstrap FAILED: manifest (exit 1)" in err, err @@ -600,7 +669,8 @@ def test_bootstrap_makes_no_ref_of_a_legs_branch_no_person_can_see( before = legs_state(project) code, out, err = run(bootstrap_of(project), encoding=ENCODING, tmp_path=tmp_path, cwd=project) - assert code == 1 and "Traceback" not in err, out + err + assert code == 1, out + err + assert "Traceback" not in err, err assert step_a(out) == not_placed("spec", "main\xa0", "U+00A0") + [ f" [code] code: on main at {pin12(project, 'code')} " "(branch tip == pin)"], out @@ -646,20 +716,42 @@ def test_bump_leg_refuses_a_tracking_branch_no_person_can_see( dry_run): """On #823's first head, on `main`: exit 0, a commit on `main`, and a NEXT line pushing it, where `bump-leg-on-tracking-branch` was owed. - `checked_value` refuses it now, before anything is fetched or written.""" + `readable_branch` refuses it now, before anything is fetched or + written.""" head = committed_tracking(project, value) pin = git("rev-parse", "HEAD:spec", cwd=project).stdout result = bump(project, scaffolded, advanced, *dry_run) assert result.returncode == 2, result.stdout + result.stderr - assert result.stderr.startswith( - f"REFUSED unsafe-value: tracking_branch is {value!r}, which is not a " - "value this tool will put on a `git` or `gh` command line"), ( - result.stderr) + assert result.stderr.startswith(unseen_refusal(value, point)), ( + result.stderr) assert git("rev-parse", "HEAD", cwd=project).stdout.strip() == head assert git("rev-parse", "HEAD:spec", cwd=project).stdout == pin assert git("status", "--porcelain", cwd=project).stdout == "" +@pytest.mark.parametrize("dry_run", [(), ("--dry-run",)], + ids=["real-run", "dry-run"]) +@pytest.mark.parametrize("value", READABLE_BRANCHES) +def test_bump_leg_takes_a_tracking_branch_a_person_can_read( + project, scaffolded, advanced, value, dry_run): + """Option A: only what no person can see is refused. Read through + `checked_value` (#823's first fix round) each of these was `unsafe-value`, + exit 2, though bump-leg only compares and prints the name. On `main`, + which is not the tracking branch, the bump goes ahead: the dry run + changes nothing, and the real run moves the pin in a commit on `main`.""" + head = committed_tracking(project, value) + result = bump(project, scaffolded, advanced, *dry_run) + assert result.returncode == 0, result.stdout + result.stderr + assert "REFUSED" not in result.stderr, result.stderr + now = git("rev-parse", "HEAD", cwd=project).stdout.strip() + if dry_run: + assert now == head + else: + assert git("rev-parse", "HEAD^", cwd=project).stdout.strip() == head + assert git("rev-parse", "HEAD:spec", + cwd=project).stdout.strip() == advanced + + @pytest.fixture(scope="module") def older_validator(tmp_path_factory) -> dict: """A project scaffolded at A, and B: an upstream commit whose manifest @@ -686,8 +778,8 @@ def test_update_shape_never_commits_onto_a_tracking_branch_no_one_can_see( """`apply --branch main` on `main`, `tracking_branch: main` followed by U+00A0. On #823's first head the guard compared `'main'` with `'main\xa0'`, found them different, and committed the shape update onto - `main`. Now `checked_value` refuses the value, and the ledger rolls the - rewrite back: the pin, HEAD and the tree are as they were.""" + `main`. Now `readable_branch` refuses the value, and the ledger rolls + the rewrite back: the pin, HEAD and the tree are as they were.""" root = tmp_path / "Atlas" shutil.copytree(older_validator["clone"], root, symlinks=True) head = committed_tracking(root, "main\xa0") @@ -695,10 +787,70 @@ def test_update_shape_never_commits_onto_a_tracking_branch_no_one_can_see( "--upstream", str(older_validator["upstream"]), "--at", older_validator["b"], "--branch", "main") assert result.returncode == 2, result.stdout + result.stderr - assert ("REFUSED unsafe-value: tracking_branch is 'main\\xa0', which is " - "not a value this tool will put on a `git` or `gh` command line" - in result.stderr), result.stderr + assert unseen_refusal("main\xa0", "U+00A0") in result.stderr, ( + result.stderr) assert git("rev-parse", "HEAD", cwd=root).stdout.strip() == head pinned = load_yaml(root / "contracts" / "shape-pin.yaml")["commit"] assert pinned.lower() == older_validator["a"] assert git("status", "--porcelain", cwd=root).stdout == "" + + +@pytest.mark.parametrize("value", READABLE_BRANCHES) +def test_update_shape_takes_a_tracking_branch_a_person_can_read( + older_validator, tmp_path, value): + """Option A, on a root: `apply --branch shape/x` commits the update on + `shape/x` and hands a person a `gh pr create` whose base, as a shell + reads it, is the name the manifest holds. Read through `checked_value` + (#823's first fix round) each was `unsafe-value` and rolled back.""" + root = tmp_path / "Atlas" + shutil.copytree(older_validator["clone"], root, symlinks=True) + committed_tracking(root, value) + result = run_script(UPDATE, "apply", "--root", str(root), "--yes", + "--upstream", str(older_validator["upstream"]), + "--at", older_validator["b"], "--branch", "shape/x") + assert result.returncode == 0, result.stdout + result.stderr + assert "\n committed on shape/x: " in result.stdout, result.stdout + assert git("rev-parse", "--abbrev-ref", "HEAD", + cwd=root).stdout.strip() == "shape/x" + line = next(line for line in result.stdout.split("\n") + if line.startswith("NEXT ")) + words = shlex.split(line.partition(" && ")[2]) + assert words[words.index("--base") + 1] == value, line + + +@pytest.fixture(scope="module") +def update_shape_module(): + """`update-shape.py` as a module, so `tracking_branch_of` is asked in + this process, under a name of its own.""" + spec = importlib.util.spec_from_file_location("update_shape_spaces", + UPDATE) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.mark.parametrize("value", READABLE_BRANCHES) +def test_tracking_branch_of_reads_a_name_a_person_can_read_as_it_is( + update_shape_module, value): + manifest = {"tracking_branch": value} + assert update_shape_module.tracking_branch_of(manifest) == value + + +@pytest.mark.parametrize("value, point", UNSEEN_BRANCHES) +def test_tracking_branch_of_refuses_a_name_no_person_can_see( + update_shape_module, value, point): + with pytest.raises(update_shape_module.Refusal) as refused: + update_shape_module.tracking_branch_of({"tracking_branch": value}) + assert str(refused.value).startswith(unseen_refusal(value, point)), ( + refused.value) + + +def test_tracking_branch_of_refuses_a_leading_dash(update_shape_module): + """`gh pr create --base` is handed the name as one argument, where one + that begins with `-` could be read as an option; git creates no branch + whose name begins with one.""" + with pytest.raises(update_shape_module.Refusal) as refused: + update_shape_module.tracking_branch_of({"tracking_branch": "-x"}) + assert str(refused.value).startswith( + "REFUSED unsafe-value: tracking_branch is '-x', which begins with " + "`-`"), refused.value diff --git a/update-shape.py b/update-shape.py index 6b74df5a..f57199f8 100755 --- a/update-shape.py +++ b/update-shape.py @@ -163,7 +163,7 @@ from repo_shape import ( # noqa: E402 COMMIT_RE, PYTHON, TREE_DIGEST_DEFINITION, Refusal, YamlError, checked_value, die, file_sha256, git_out, load_yaml, parse_yaml, - tree_digest, + readable_branch, tree_digest, ) # The reader's own rule for where a comment starts, by its private name on # purpose: `shape_materialize` imports it the same way and says why (#380). A @@ -2424,15 +2424,19 @@ def tracking_branch_of(manifest: dict | None) -> str: is never asked: a base taken from the network could disagree with the one the `--branch` refusal and `--pr` read from the manifest. - READ THROUGH `checked_value`, as `bump-leg.py` and `family.py` read it - (#715). Compared raw, `tracking_branch: main` followed by U+00A0, which - the reader reads as YAML does since #715, was not `main`, so + READ THROUGH `readable_branch`, as `bump-leg.py` reads it (#715). + Compared raw, `tracking_branch: main` followed by U+00A0, which the + reader reads as YAML does since #715, was not `main`, so `apply --branch main` committed onto `main` where the root's own - validators passed. A name `checked_value` refuses is `unsafe-value`, - and inside `apply` the ledger rolls the rewrite back. + validators passed. A name holding a character that does not print or is + white space is now `unseen-branch`, and one that begins with `-`, which + `gh pr create --base` could read as an option, is `unsafe-value`; inside + `apply` the ledger rolls the rewrite back. Any other name is taken as it + is: `a&b` is quoted into the line `next_line` prints (#209), and a name + in any script is one git creates and the root's gate passes. """ - return checked_value("tracking_branch", - (manifest or {}).get("tracking_branch") or "main") + return readable_branch("tracking_branch", + (manifest or {}).get("tracking_branch") or "main") def pull_request_repository(manifest: dict | None, From 4cc9ee2a889da1a92ec43cedc828275b2c5a7c35 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 20:54:55 +0000 Subject: [PATCH 5/6] bump-leg.py: `readable_branch` is imported on a line of its own, so the import list main's #746 extended with `spelled` merges without a conflict (#715) `716ad457` added `readable_branch` to the `from repo_shape import (...)` list, on the line #746 (`fc867207`) also rewrote, and `git merge-tree` with main `190133d5` stopped there. The list is as main's merge base has it again, and the name has an import of its own after it, as `update-shape.py` imports `_strip_comment`. The predicted merge with `190133d5` is now clean. Run: `tests/test_yaml_reader_spaces.py` and `tests/test_bump_leg_dry_run_tracking.py`, `-k "bump_leg or tracking"`: 44 passed. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 0b0b9852..ce3cbf3e 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -121,8 +121,11 @@ sys.path.insert(0, str(SHAPE_ROOT / "scripts")) from repo_shape import ( # noqa: E402 COMMIT_RE, SAFE_PATH_RE, Refusal, checked_value, git_out, load_yaml, - readable_branch, recorded_gitlink, repo_basename, tree_digest, + recorded_gitlink, repo_basename, tree_digest, ) +# On its own line, so the list above, which other changes extend, is +# not a line two of them both rewrite (#715). +from repo_shape import readable_branch # noqa: E402 from shape_materialize import ( # noqa: E402 CommandFailed, captured_child_environment, check_program, commit_trailers, escape_what_stdout_cannot_encode, is_nested_key, rewrite_fault, run, From 8b4dbcf696ceb9674168b01979f8efed1c77d55b Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 22:13:55 +0000 Subject: [PATCH 6/6] shape_materialize.py: `readable_branch` and `UNSEEN` move out of the pinned `repo_shape.py`, a lone surrogate and a NUL are named as the validator names them, and no line says `siblings.py` refuses a space (#715) RV823's re-check of round 2 (#823 issuecomment-6241904307), after the merge with #824. - SHOULD-1. `readable_branch` and `UNSEEN` were in `scripts/repo_shape.py`, a shape copy every root digest-pins, though only `bump-leg.py` and `update-shape.py` call them. They are now in `scripts/shape_materialize.py`, in a section of their own after #283's branch-name rule, under the same "it lives here and not in `scripts/repo_shape.py`" reason. Both tools import the name on a line of its own, and `update-shape.py`'s `from repo_shape import (...)` list is as main has it again. The docstring's reason is restated: the root's validator and bootstrap DO import from `repo_shape.py`, and they spell the rule themselves because a root's copies may run beside an older `repo_shape.py` (the skew rule, `validate-manifest.py`'s `_library_takes_remedies`, #661), which has no such function. - NIT-2. A byte-carrying U+DCFF is refused as "a lone surrogate: no UTF-8 file or console can hold it, so no branch name is spelled with one", and a NUL as "a NUL: no argument, file name or ref name can hold one", as `validate-manifest.py` words them (#280); any other such character keeps the `UNSEEN` words. The leading-`-` refusal says only "which begins with `-`, and git creates no branch whose name begins with one", which holds for bump-leg, which hands the name to no command, as well as for update-shape. New rows hold both. - NIT-1. `bootstrap.py`, `validate-manifest.py` and the fault test's docstring in `tests/test_bootstrap_branch_bytes.py` no longer say `siblings.py` refuses every such name: it asks `isprintable()` alone, and takes a space. The fault test's docstring no longer says git takes each of its names: git refuses `a b` and `a\x1bb`. Run on this tree before the commit: the helper's callers in `tests/test_yaml_reader_spaces.py` and `tests/test_update_shape_pr_base.py`, and the fault test, 75 passed. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 2 +- scripts/repo_shape.py | 50 ------------ scripts/shape_materialize.py | 76 +++++++++++++++++++ templates/assembly-root/scripts/bootstrap.py | 3 +- .../scripts/validate-manifest.py | 13 ++-- tests/test_bootstrap_branch_bytes.py | 8 +- tests/test_yaml_reader_spaces.py | 21 ++++- update-shape.py | 5 +- 8 files changed, 115 insertions(+), 63 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index fe7281c3..89771c30 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -125,7 +125,7 @@ ) # On its own line, so the list above, which other changes extend, is # not a line two of them both rewrite (#715). -from repo_shape import readable_branch # noqa: E402 +from shape_materialize import readable_branch # 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, diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 3b30f57a..a339a9e7 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -2376,56 +2376,6 @@ def checked_value(what: str, value, pattern: re.Pattern = SAFE_ARG_RE) -> str: return text -#: Why a branch name holding such a character is refused, in the words -#: `validate-manifest.py`'s `_branch_finding` and `bootstrap.py`'s `UNSEEN` -#: say it in (#715). -UNSEEN = "a character that does not print or is white space" - - -def readable_branch(what: str, value) -> str: - """A BRANCH NAME read out of a manifest, refused only for what no person - can see in it (#715; Brett Heap's option A on RV823's SHOULD-1). - - The rule is the root's own. `validate-manifest.py`'s `_branch_finding` - and `bootstrap.py`'s `_argument_fault` refuse a branch holding a - character for which `str.isprintable()` is false or `str.isspace()` is - true, so `main` followed by U+00A0, which the reader reads as YAML does - since #715, is never taken for `main`. Those two ship into a root and - import nothing from here, so the rule is spelled again, not shared. - - It is NOT `checked_value`'s `SAFE_ARG_RE`. A name in any script, or one - holding `&`, is a name git creates and the root's own gate passes. - `bump-leg.py` only compares it and prints it, and `update-shape.py` - compares it, hands it to `gh pr create --base` as one argument and - prints it shell-quoted in the line a person pastes (#209). #823's first - fix round read it through `checked_value`, and refused those names. - - A leading `-` is refused too: `update-shape.py` hands the name to - `gh pr create --base`, where it could be read as an option, and git - creates no branch whose name begins with one, so no root needs it. - """ - text = str(value) - if text.startswith("-"): - raise Refusal( - "unsafe-value", - f"{what} is {text!r}, which begins with `-`: a command it is " - "handed to could read it as an option, and git creates no branch " - "whose name begins with one", - "Remediation: give it a branch name git can create, such as " - "`main` or `release/1.2`.") - char = next((c for c in text if not c.isprintable() or c.isspace()), - None) - if char is not None: - raise Refusal( - "unseen-branch", - f"{what} is {text!r}, which holds U+{ord(char):04X}, {UNSEEN}: " - "a person reading the name cannot tell it from the name without " - "it", - "Remediation: give it a name with no such character, such as " - "`main` or `release/1.2`, and run this again.") - return text - - # --------------------------------------------------------------------------- # A path spelled the one way Git records it (#179, #189) # --------------------------------------------------------------------------- diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 39979bd9..74d870eb 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1363,6 +1363,82 @@ def refuse_bad_branch(refusal: str, what: str, value) -> None: raise Refusal(refusal, problem, remediation) +# --------------------------------------------------------------------------- +# A tracking branch a person can read (#715) +# --------------------------------------------------------------------------- +# +# What `bump-leg.py` and `update-shape.py` ask of a manifest's +# `tracking_branch` before they compare it or hand it to `gh` (Brett Heap's +# option A on RV823's SHOULD-1, 2026-10-17): only what no person can see in +# it, never `checked_value`'s alphabet. +# +# IT LIVES HERE AND NOT IN `scripts/repo_shape.py` for the reason the leg-path +# rule and `refuse_bad_branch` do (RV823's round-2 SHOULD-1): that file is a +# SHAPE COPY every root digest-pins, and no root calls this. Both tools +# already import this module. + +#: Why a branch name holding such a character is refused, in the words +#: `validate-manifest.py`'s `_branch_finding` and `bootstrap.py`'s `UNSEEN` +#: say it in (#715). +UNSEEN = "a character that does not print or is white space" + + +def readable_branch(what: str, value) -> str: + """A BRANCH NAME read out of a manifest, refused only for what no person + can see in it (#715; Brett Heap's option A on RV823's SHOULD-1). + + The rule is the root's own: `validate-manifest.py`'s `_branch_finding` + and `bootstrap.py`'s `_argument_fault` refuse a branch holding a + character for which `str.isprintable()` is false or `str.isspace()` is + true, so `main` followed by U+00A0, which the reader reads as YAML does + since #715, is never taken for `main`. Each of those two spells the rule + itself rather than importing it: a root's validator and bootstrap run + beside the `repo_shape.py` that root holds, which may be older than they + are (the skew rule, `validate-manifest.py`'s `_library_takes_remedies`, + #661), and an older one has no such function. The words are theirs: a + NUL and a lone surrogate (#280) have their own, and any other such + character is `UNSEEN`. + + It is NOT `checked_value`'s `SAFE_ARG_RE`. A name in any script, or one + holding `&`, is a name git creates and the root's own gate passes. + `bump-leg.py` only compares it and prints it, and `update-shape.py` + compares it, hands it to `gh pr create --base` as one argument and + prints it shell-quoted in the line a person pastes (#209). #823's first + fix round read it through `checked_value`, and refused those names. + + A leading `-` is refused too, as `unsafe-value`: `update-shape.py` hands + the name to `gh pr create --base`, where it could be read as an option, + and git creates no branch whose name begins with one, so no root needs + it. The refusal's words say only the second, which holds for both + tools. + """ + text = str(value) + if text.startswith("-"): + raise Refusal( + "unsafe-value", + f"{what} is {text!r}, which begins with `-`, and git creates no " + "branch whose name begins with one", + "Remediation: give it a branch name git can create, such as " + "`main` or `release/1.2`.") + char = next((c for c in text if not c.isprintable() or c.isspace()), + None) + if char is None: + return text + if char == "\x00": + kind = "a NUL: no argument, file name or ref name can hold one" + elif "\ud800" <= char <= "\udfff": + kind = ("a lone surrogate: no UTF-8 file or console can hold it, so " + "no branch name is spelled with one") + else: + kind = (f"{UNSEEN}: a person reading the name cannot tell it from the " + "name without it") + raise Refusal( + "unseen-branch", + f"{what} is {text!r}, which holds U+{ord(char):04X}, {kind}", + "Remediation: give it a name with no such character, such as " + "`main` or `release/1.2`, and run this again.") + + # --------------------------------------------------------------------------- # What a run asks before its first write: the folders it makes (#609) # --------------------------------------------------------------------------- diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index cdd7407f..ff84b776 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -275,7 +275,8 @@ def _argument_fault(branch: str) -> str | None: is `os.fsencode`'s own, so it is the system's and not a list kept here. NOR IS A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715), `UNSEEN`, - which `validate-manifest.py` and `siblings.py` refuse in a branch too. + which `validate-manifest.py` refuses in a branch too (`siblings.py` asks + `isprintable()` alone, so a space it takes). git takes `main` followed by U+00A0, and since #715 the reader reads that no-break space as YAML does, where it used to strip it: so this step made a second `main` beside the first in every leg and checked it out. Asked diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index 7bb6f436..c732e0e1 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -601,12 +601,13 @@ def _branch_finding(code: str, what: str, value) -> list[str]: the scaffold asks it for `--tracking-branch` (#283); and a value that is not text is left alone, as `bootstrap.py` turns it into text. - NOR A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715), the branch - `siblings.py` refuses to use. git takes `main` followed by U+00A0, and - since #715 the reader reads that no-break space as YAML does, where it - used to strip it: so `bootstrap.py` made a second `main` beside the - first, and `bump-leg.py`'s guard of the tracking branch let a commit be - made on `main`. No person can tell such a name from the one without it. + NOR A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715); `siblings.py` + uses no branch that does not print, though it takes a space. git takes + `main` followed by U+00A0, and since #715 the reader reads that no-break + space as YAML does, where it used to strip it: so `bootstrap.py` made a + second `main` beside the first, and `bump-leg.py`'s guard of the + tracking branch let a commit be made on `main`. No person can tell such + a name from the one without it. """ if not isinstance(value, str): return [] diff --git a/tests/test_bootstrap_branch_bytes.py b/tests/test_bootstrap_branch_bytes.py index 0e14afc1..a2892e45 100644 --- a/tests/test_bootstrap_branch_bytes.py +++ b/tests/test_bootstrap_branch_bytes.py @@ -484,9 +484,11 @@ def test_a_nul_and_a_surrogate_without_a_byte_are_the_fault( ]) def test_a_character_no_person_can_see_is_the_fault(bootstrap_module, branch, point): - """Since #715: git takes each of these names, and no person reading one - can tell it from the name without the character. Step (a) makes no ref - of it, as `validate-manifest.py` and `siblings.py` refuse it too.""" + """Since #715: no person reading one of these names can tell it from + the name without the character, a visible space aside, which git + refuses too. Step (a) makes no ref of it, as `validate-manifest.py` + refuses it too; `siblings.py` asks `isprintable()` alone, and takes the + space.""" if "\udcff" in branch and len(os.fsencode("\udcff")) != 1: pytest.skip("this system's `os.fsencode` makes no byte of U+DCFF") assert bootstrap_module._argument_fault(branch) == ( diff --git a/tests/test_yaml_reader_spaces.py b/tests/test_yaml_reader_spaces.py index 6e7ca421..bcdf3a5c 100644 --- a/tests/test_yaml_reader_spaces.py +++ b/tests/test_yaml_reader_spaces.py @@ -853,4 +853,23 @@ def test_tracking_branch_of_refuses_a_leading_dash(update_shape_module): update_shape_module.tracking_branch_of({"tracking_branch": "-x"}) assert str(refused.value).startswith( "REFUSED unsafe-value: tracking_branch is '-x', which begins with " - "`-`"), refused.value + "`-`, and git creates no branch whose name begins with one\n"), ( + refused.value) + + +@pytest.mark.parametrize("value, words", [ + pytest.param("a\udcffb", "U+DCFF, a lone surrogate: no UTF-8 file or " + "console can hold it, so no branch name is spelled with one", + id="a-lone-surrogate"), + pytest.param("a\x00b", "U+0000, a NUL: no argument, file name or ref " + "name can hold one", id="a-nul"), +]) +def test_tracking_branch_of_names_a_nul_and_a_surrogate_as_the_validator_does( + update_shape_module, value, words): + """RV823's round-2 NIT-2: a byte-carrying U+DCFF is "a lone surrogate", + as `validate-manifest.py` names it (#280), and a NUL is a NUL.""" + with pytest.raises(update_shape_module.Refusal) as refused: + update_shape_module.tracking_branch_of({"tracking_branch": value}) + assert str(refused.value).startswith( + f"REFUSED unseen-branch: tracking_branch is {value!r}, which holds " + f"{words}\n"), refused.value diff --git a/update-shape.py b/update-shape.py index b7c28b49..b81de67d 100755 --- a/update-shape.py +++ b/update-shape.py @@ -163,7 +163,7 @@ from repo_shape import ( # noqa: E402 COMMIT_RE, PYTHON, TREE_DIGEST_DEFINITION, Refusal, YamlError, checked_value, die, file_sha256, git_out, load_yaml, parse_yaml, - readable_branch, tree_digest, + tree_digest, ) # The reader's own rule for where a comment starts, by its private name on # purpose: `shape_materialize` imports it the same way and says why (#380). A @@ -190,6 +190,9 @@ # Where a root keeps its shape pin. The materializer that writes it names it # once; this module spells the path by that name and never by a literal (#290). from shape_materialize import SHAPE_PIN # noqa: E402 +# The tracking branch's rule (#715), on a line of its own so the lists above, +# which other changes extend, are not lines two of them both rewrite. +from shape_materialize import readable_branch # noqa: E402 #: `owner/repo`, the only remote spelling `--upstream` accepts. REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$")