diff --git a/scaffold-project.py b/scaffold-project.py index 0da39754..6df7fd5b 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -1725,10 +1725,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/bump-leg.py b/scripts/bump-leg.py index f1aa7e28..2491305a 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -123,6 +123,9 @@ COMMIT_RE, SAFE_PATH_RE, Refusal, checked_value, git_out, load_yaml, recorded_gitlink, repo_basename, shown_path, spelled, 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 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, @@ -243,7 +246,21 @@ def display_name(self) -> str: @property def tracking(self) -> str: - return str(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 cba29b7f..a339a9e7 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -207,10 +207,11 @@ def __init__(self, message: str, remediation: str = ""): # 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 @@ -231,7 +232,22 @@ def __init__(self, message: str, remediation: str = ""): # `@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: @@ -329,8 +345,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: @@ -347,14 +363,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 @@ -551,7 +568,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 "\"'": @@ -613,9 +630,19 @@ def _refuse_entry_indicator(text: str, remedy: bool = True) -> 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.startswith("?:") and text[2:3] in ("", " ", "\t") or text == "?"): refusal = _indicator_refusal(text) if not remedy: @@ -648,26 +675,44 @@ 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: + # 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. 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}") 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): @@ -740,9 +785,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: @@ -766,9 +811,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 @@ -807,16 +852,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.startswith(":") 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: @@ -838,13 +892,15 @@ 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 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 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 1aed8ab7..c3af9884 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1413,6 +1413,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) # --------------------------------------------------------------------------- @@ -4388,8 +4464,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 " @@ -4409,7 +4495,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 _reading_in_words(value) if isinstance(value, bool): return f"the boolean {str(value).lower()}" if isinstance(value, (int, float)): @@ -4429,7 +4518,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/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index 52294cd5..fffece49 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -282,6 +282,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). @@ -322,12 +328,29 @@ 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` 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 + 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. Asked + BEFORE `UNWRITABLE`, for that fault's reason: a name holding such a + character is not good under any encoding, so the line that calls a + name good is never said of it (`"日本 x"` under `LC_ALL=C` is named + for its space, which the root's own validator names too). """ 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}" unwritable = next((char for char in branch if _fsencoded_length(char) == 0), None) if unwritable is None: @@ -353,6 +376,10 @@ def _say_not_tried(where: str, named: str, fault: str, stays where it is, and the line says which of the three places the person's value can have come from holds it (#455). + `UNSEEN` (#715) has its own words: git takes `main` followed by U+00A0, + so "which no command can be given" would be wrong of it, and the name + to give is the one without that character. + A character this system's encoding cannot write (`UNWRITABLE`, #710) is a branch step (a) DID NOT TRY, not one it could not create: the branch can be there already, made by a run under UTF-8 (RV824's N2). Where git @@ -364,6 +391,14 @@ def _say_not_tried(where: str, named: str, fault: str, git's lines (`_git_refuses_in_utf8`): the name is not called good, git's words are said, and #455's way out follows, since the name itself has to change (RV824's S1).""" + 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 unwritable = fault.endswith(UNWRITABLE) tried = "did not try" if unwritable else "could not create" print(f" {where}: {tried} branch {named}: it holds {fault}, which no " diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index c5320179..fe9cb2f7 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -639,22 +639,33 @@ 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); `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 [] - 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 623038f5..55d905c5 100644 --- a/tests/test_bootstrap_after_bump.py +++ b/tests/test_bootstrap_after_bump.py @@ -84,12 +84,14 @@ import os import shlex import shutil +import subprocess from pathlib import Path import pytest from conftest import (ORG, PROJECT, REPO, SCAFFOLD, appended_to_command_scope, 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" @@ -490,15 +492,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) @@ -506,7 +510,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() @@ -533,13 +537,18 @@ 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, and read from `tracking_branch:`; + the line is asked as the escape it must print, not as `spelled` makes + it. Its upstream is read as the system decodes it since #824 (#710), and + HEAD is read here as bytes, which git's answer 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}}") @@ -551,12 +560,14 @@ 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 + head = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], + cwd=str(leg), capture_output=True, check=True) + assert head.stdout == b"refs/heads/" + os.fsencode(raw) + b"\n" # --- a fresh clone of a leg whose remote moved past the pin ----------------- diff --git a/tests/test_bootstrap_branch_bytes.py b/tests/test_bootstrap_branch_bytes.py index 5418705f..a2892e45 100644 --- a/tests/test_bootstrap_branch_bytes.py +++ b/tests/test_bootstrap_branch_bytes.py @@ -197,7 +197,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 @@ -205,8 +204,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, @@ -422,15 +423,16 @@ def validator_module(): @pytest.mark.parametrize("branch", [ - "main", "release/1.2", written("feature/日本"), written("a‍b"), - written("a\xe9b"), "a b", - "a\x1bb", "a..b", "", "\udc80", "a\udcffb"]) + "main", "release/1.2", written("feature/日本"), written("a\xe9b"), + "a..b", "", "\udc80", "a\udcffb"]) def test_a_branch_the_system_carries_has_no_fault(bootstrap_module, branch): """Where the system's encoding writes every character of the branch, - nothing but a NUL and a lone surrogate with no byte is a fault: a - control, a space, `..`, a character that is not ASCII (`written` skips - one the encoding cannot write, #710's `UNWRITABLE`), and - the byte-carrying surrogates U+DC80 to U+DCFF are the commands'. On a + nothing but a NUL, a lone surrogate with no byte and, since #715, a + character that does not print or is white space (a control, a space, + a zero-width joiner: `test_a_character_no_person_can_see_is_the_fault`) + is a fault: `..`, a character that is not ASCII (`written` skips one the + encoding cannot write, #710's `UNWRITABLE`), 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.""" skip_where_fsencode_gives_a_surrogate_no_byte(branch) @@ -472,6 +474,27 @@ 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: 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) == ( + 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_non_utf8_branch.py b/tests/test_bootstrap_non_utf8_branch.py index fcd595af..bca0c811 100644 --- a/tests/test_bootstrap_non_utf8_branch.py +++ b/tests/test_bootstrap_non_utf8_branch.py @@ -91,8 +91,12 @@ #: The good name L4-124 measured, and what an `ascii` stdout prints of it. GOOD = "feature/日本" GOOD_PRINTED = r"feature/\u65e5\u672c" +#: And of `"日本 x"`, whose space is `UNSEEN` (#715). +SPACED_PRINTED = r"\u65e5\u672c x" #: What `_argument_fault` calls the character (`UNWRITABLE`). UNWRITABLE = "a character this system's encoding cannot write" +#: And a character no person can see (`UNSEEN`, #715), asked before it. +UNSEEN = "a character that does not print or is white space" #: The line under a name git refuses in UTF-8 too, before git's own words. GIT_REFUSES = " and git refuses it as a branch name in UTF-8 too:" #: A test that hands git a name only UTF-8 can write, or reads a line a @@ -192,32 +196,39 @@ def past_the_pin(leg: Path, upstream: str, branch: str) -> tuple[str, str]: # --- the upstream's name goes back to git as the bytes it was --------------- -@pytest.mark.parametrize("raw, byte", [ - pytest.param("a\udcffb", True, id="the-byte-0xff"), - pytest.param("a\u00a0", False, id="ending-in-U+00A0", +@pytest.mark.parametrize("raw, upstream, byte", [ + pytest.param("a\udcffb", "a\udcffb", True, id="the-byte-0xff"), + # Since #715 step (a) refuses a BRANCH ending in U+00A0 (`UNSEEN`) before + # git is asked, so the branch here is a printable `a\u00e9`, and the + # UPSTREAM keeps the U+00A0 whose stripping this row was written for. + pytest.param("a\u00e9", "a\u00a0", False, + id="an-upstream-ending-in-U+00A0", marks=NEEDS_A_UTF8_PROCESS), ]) def test_a_branch_past_the_pin_is_moved_back_whatever_its_upstream_is_named( - root, tmp_path, bootstrap_module, raw, byte): + root, tmp_path, bootstrap_module, raw, upstream, byte): """LANE 5'S SITE: on main, a `UnicodeDecodeError` traceback over the - byte 0xff, and, over a name ending in U+00A0, `not moving an existing - branch`. Both are moved back now, the line naming the upstream and the - command as `move_back_to_pin` spells them, and HEAD is on the branch - whose name is the bytes git was given.""" + byte 0xff, and, over an upstream ending in U+00A0, `not moving an + existing branch`. Both are moved back now, the line naming the upstream + and the command as `move_back_to_pin` spells them, and HEAD is on the + branch whose name is the bytes git was given.""" if byte: needs_the_byte(tmp_path) leg = root / "spec" - pin, ahead = past_the_pin(leg, raw, raw) + pin, ahead = past_the_pin(leg, upstream, raw) result = run_script(root / "scripts" / "bootstrap.py", "--skip-validators", "--branch", raw, cwd=root) assert result.returncode == 0, result.stderr + result.stdout assert "Traceback" not in result.stderr, result.stderr spelled = bootstrap_module.spelled + held = "origin/" + upstream command = ("git -C spec merge --ff-only " - f"{spelled(shlex.quote('origin/' + raw))}") - assert moved_back(ahead, pin, spelled(raw), command=command) in ( - result.stdout), result.stdout + f"{spelled(shlex.quote(held))}") + assert (f" [spec] spec: branch {spelled(raw)} moved back {ahead[:12]} " + f"-> {pin[:12]}, the pin; nothing is lost, {spelled(held)} still " + f"holds those commits, and `{command}` brings the branch " + "forward") in result.stdout.split("\n"), result.stdout assert rev(leg, f"refs/heads/{raw}") == pin assert rev(leg) == pin assert head_bytes(leg) == b"refs/heads/" + os.fsencode(raw) + b"\n" @@ -352,17 +363,19 @@ def test_a_good_name_this_encoding_cannot_write_is_said_and_the_way_out_works( @pytest.mark.parametrize("name, printed", [ pytest.param("feature/日本..x", r"feature/\u65e5\u672c..x", id="dot-dot"), pytest.param("日本.lock", r"\u65e5\u672c.lock", id="dot-lock"), - pytest.param("日本 x", r"\u65e5\u672c x", id="a-space"), + pytest.param("日本~x", r"\u65e5\u672c~x", id="a-tilde"), ]) def test_a_name_git_refuses_in_utf8_too_is_never_called_good( root, tmp_path, name, printed): """RV824's S1: each of these names this system's encoding cannot - write, and git refuses in any encoding (`..`, `.lock`, a space), was + write, and git refuses in any encoding (`..`, `.lock`, a `~`), was called good, with a way out that ended in git's `is not a valid branch name`. Now `git check-ref-format --branch` is asked of the UTF-8 bytes first: git's words are said, and #455's way out, and each leg is left as it was. The validators pass the value, as they did, so the run - ends `bootstrap ok`, as a branch git refuses always has.""" + ends `bootstrap ok`, as a branch git refuses always has. RV824 asked a + space here too; since #715 a space is `UNSEEN`, named before + `UNWRITABLE` (the next test).""" changes = under_ascii(tmp_path) tracking(root, f'"{name}"') before = legs_state(root) @@ -383,6 +396,31 @@ def test_a_name_git_refuses_in_utf8_too_is_never_called_good( assert legs_state(root) == before +def test_a_name_with_a_space_is_named_for_the_space_under_ascii_too( + root, tmp_path): + """#715 under `LC_ALL=C PYTHONUTF8=0`: `"日本 x"` holds a character this + encoding cannot write and a space. The space is the fault, in #715's + words, so the line that calls a name good is never said of it, and the + root's own validator names the space, so the run ends as it ends it.""" + changes = under_ascii(tmp_path) + tracking(root, '"日本 x"') + before = legs_state(root) + + code, out, err = run_under(root, tmp_path, changes) + said = err.decode("utf-8") + assert code == 1, out + said + assert "Traceback" not in said, said + assert "the name is good" not in out, out + lines = step_a(out) + for leg in ("spec", "code"): + assert (f" [{leg}] {leg}: could not create branch " + f"{SPACED_PRINTED}: it holds U+0020, {UNSEEN}, which no " + "person can tell from the name without it; the leg is left " + "as it is") in lines, lines + assert "bootstrap FAILED: manifest" in said, said + assert legs_state(root) == before + + def test_a_name_holding_a_surrogate_too_is_named_for_the_surrogate( root, tmp_path): """RV824's S1: `"日\\udcff"` under `LC_ALL=C PYTHONUTF8=0` was called @@ -427,7 +465,6 @@ def test_a_legs_own_branch_this_encoding_cannot_write_is_said_too( assert legs_state(root) == before - def test_a_leg_whose_git_dir_is_gone_still_has_a_good_name_called_good( root, tmp_path): """RV824's R1-N1: git is asked about the name in the ROOT. Asked in @@ -502,6 +539,11 @@ def ascii_fsencode(text): # the encoding cannot write: the validator names it (RV824's S1). ("日\udcff", "U+DCFF, a lone surrogate"), ("\udc80日", "U+DC80, a lone surrogate"), + # A character no person can see is named before one the encoding cannot + # write (#715): that name is not good either, and the validator names + # it, so the line calling it good would be false too. + ("日本 x", f"U+0020, {UNSEEN}"), + ("日本\xa0", f"U+00A0, {UNSEEN}"), ]) def test_under_an_ascii_encoding_what_it_cannot_write_is_the_fault( bootstrap_module, monkeypatch, branch, fault): diff --git a/tests/test_bootstrap_spelling.py b/tests/test_bootstrap_spelling.py index 715c05ad..c3472892 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,39 @@ 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, 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: " + "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_update_shape_pr_base.py b/tests/test_update_shape_pr_base.py index 1b8237ac..1fbbe419 100644 --- a/tests/test_update_shape_pr_base.py +++ b/tests/test_update_shape_pr_base.py @@ -387,6 +387,30 @@ def test_a_value_the_manifest_holds_is_quoted_for_the_shell_that_reads_the_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_no_person_can_see_never_reaches_the_line( + update_shape, tmp_path, capsys, push): + """`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: 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 == "unseen-branch" + assert str(refused.value).startswith( + "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 == "" + + 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_indicator_openers.py b/tests/test_yaml_indicator_openers.py index c680baeb..d6b3307d 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. @@ -460,23 +461,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]"] @@ -495,9 +492,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): @@ -512,7 +508,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: @@ -528,7 +524,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..bcdf3a5c --- /dev/null +++ b/tests/test_yaml_reader_spaces.py @@ -0,0 +1,875 @@ +# 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 importlib.util +import re +import shlex +import shutil +import sys +import unicodedata + +import pytest + +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 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 +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) + +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 + + +@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}") + + +#: `(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`. +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: -")) + + +# --- 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 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 = [ + 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"), + # 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.") + + +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, 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 + 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, 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 + 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() + + +#: 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, + 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. + `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(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 + 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 `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") + 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 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 " + "`-`, 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 63154d3e..b81de67d 100755 --- a/update-shape.py +++ b/update-shape.py @@ -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_.-]+$") @@ -2466,8 +2469,20 @@ 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 `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 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 str((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,