From 151a0861dd6ce4a82cf78100fd23b86932c166a2 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:01:17 +0000 Subject: [PATCH] render-cli-reference.py and tests/: the CLI reference's children write no bytecode under templates/, the launcher guard reads family.py's siblings child, the quoting-sentence inventory follows an aliased quoter, a suffixless tool and each tool's calling functions, and an S5778 pytest.raises builds its argument first, so each guard fails when what it names breaks (#665) Four guards held less than they said. - scripts/render-cli-reference.py: `child_env()` sets PYTHONDONTWRITEBYTECODE=1, so the template's `siblings.py` (`from bootstrap import ...` from its own directory) leaves no `bootstrap.cpython-312.pyc` under templates/family-root/scripts/ when the generator runs its `--help` children. No `--help` text changes, so docs/cli.md is not regenerated (`render-cli-reference.py --check` still says it is what the generator produces). tests/test_cli_reference.py holds the variable in the environment and holds that an interpreter started under it writes none. - tests/test_validator_child_encoding.py: `interpreter_runs` accepts `[sys.executable or PYTHON, ...]` as well as `[sys.executable, ...]`, and LAUNCHERS gains scripts/family.py with `inherited_child_environment`, so dropping `env=` from `family.py siblings`' child (#214) fails the guard. - tests/test_readme_windows_quoting_sentence.py: the inventory follows `from repo_shape import shell_line as joined` (and `joined = shell_line`), scans every file `git ls-files` lists that is a `.py` file or has a python shebang (the root `bootstrap`), pins the functions that call a host-dialect quoter for scripts/family.py and setup-project.py instead of the file (shape-doctor.py stays every line, as the sentence says), and exempts the bodies of `shell_quoted` and `shell_line` in scripts/repo_shape.py by name, not the whole file. No quoting changes: the Git Bash dialect is a separate issue. - tests/test_bump_leg_paste_lines.py: `leg_placement(leg)` is evaluated before the `with pytest.raises(LegNotAsFound)` in `test_pasting_the_status_line_reads_that_leg` (Sonar S5778), as the test above it does. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scripts/render-cli-reference.py | 8 + tests/test_bump_leg_paste_lines.py | 5 +- tests/test_cli_reference.py | 17 ++ tests/test_readme_windows_quoting_sentence.py | 234 +++++++++++++++--- tests/test_validator_child_encoding.py | 19 +- 5 files changed, 242 insertions(+), 41 deletions(-) diff --git a/scripts/render-cli-reference.py b/scripts/render-cli-reference.py index 03cb489d..ee8c0330 100755 --- a/scripts/render-cli-reference.py +++ b/scripts/render-cli-reference.py @@ -286,6 +286,13 @@ def child_env() -> dict: If that ever stops being true, git refuses the transport, the tool's `--help` exits non-zero, and `capture_help` turns that into a refusal naming the tool — the regression fails closed, offline. + + `PYTHONDONTWRITEBYTECODE=1` (#665): the template's `siblings.py` does + `from bootstrap import ...` from its own directory, so a `--help` child + that may write bytecode leaves a `__pycache__/bootstrap.*.pyc` inside + `templates/family-root/scripts/`, ignored by git and so silent in + `git status`. A person who runs this generator gets the same tree as the + test that runs it. """ env = dict(os.environ) env.pop("LINES", None) @@ -297,6 +304,7 @@ def child_env() -> dict: "PYTHONPATH": str(REPO / "scripts"), "PYTHONUTF8": "1", "PYTHONIOENCODING": "utf-8", + "PYTHONDONTWRITEBYTECODE": "1", "NO_COLOR": "1", "TERM": "dumb", "LC_ALL": "C", diff --git a/tests/test_bump_leg_paste_lines.py b/tests/test_bump_leg_paste_lines.py index 5763c3b4..f28cc0e6 100644 --- a/tests/test_bump_leg_paste_lines.py +++ b/tests/test_bump_leg_paste_lines.py @@ -398,10 +398,11 @@ def test_pasting_the_status_line_reads_that_leg(make_root, bump_leg_module, name): root, _ = make_root(name) leg = root.resolve() / "spec" + placement = bump_leg_module.leg_placement(leg) was = git("rev-parse", "HEAD", cwd=leg).stdout.strip() with pytest.raises(bump_leg_module.LegNotAsFound) as raised: - bump_leg_module._restore_leg_if_moved( - leg, bump_leg_module.leg_placement(leg), was, b"not its status") + bump_leg_module._restore_leg_if_moved(leg, placement, was, + b"not its status") pasted = paste(status_remedy(str(raised.value)), root.parent) assert pasted.returncode == 0, pasted.stderr + pasted.stdout diff --git a/tests/test_cli_reference.py b/tests/test_cli_reference.py index 45fac457..a770e1bf 100644 --- a/tests/test_cli_reference.py +++ b/tests/test_cli_reference.py @@ -472,6 +472,23 @@ def test_child_env_never_lets_a_child_reach_the_network(monkeypatch): assert generator_module().child_env()["GIT_ALLOW_PROTOCOL"] == "file" +def test_child_env_leaves_no_bytecode_in_the_trees_it_runs(monkeypatch): + """`PYTHONDONTWRITEBYTECODE=1` in every child, whatever the caller had + (#665): the template's `siblings.py` imports `bootstrap` from its own + directory, so a `--help` child that may write bytecode leaves a + `bootstrap.*.pyc` under `templates/family-root/scripts/`, which git + ignores and which nothing reads today. Two checks: the variable is in the + environment, and an interpreter started under that environment really + does not write bytecode, so the assertion does not rest on the name alone.""" + monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False) + env = generator_module().child_env() + assert env["PYTHONDONTWRITEBYTECODE"] == "1" + proc = subprocess.run( + [sys.executable, "-c", "import sys; print(sys.dont_write_bytecode)"], + env=env, capture_output=True, check=False) + assert proc.stdout.strip() == b"True", proc.stderr + + def test_the_generators_checkout_test_is_the_shims_own(): """The question `require_a_checkout` asks is `setup.sh`'s `is_shape_checkout`, read from the shim and not guessed: the top level diff --git a/tests/test_readme_windows_quoting_sentence.py b/tests/test_readme_windows_quoting_sentence.py index 2812a7c7..9604054c 100644 --- a/tests/test_readme_windows_quoting_sentence.py +++ b/tests/test_readme_windows_quoting_sentence.py @@ -14,10 +14,14 @@ HASH of the README, not what the two say); * the tools that call a quoter that follows the host (`quote_arg`, `shell_quoted`, `shell_line`, `shell_word`) are exactly the three the - sentence names, so a fourth is a fault here until the sentence names it. + sentence names, and for the two it names by the lines of particular + commands, the functions that print those lines are the ones pinned + below, so a fourth tool, or a new line in a named one, is a fault here + until the sentence names it (#665). -Nothing runs a tool and nothing touches git: the first reads two files, the -second reads every shipped `.py` file with `ast`. +Nothing runs a tool: the first reads two files, the second reads every +shipped Python file with `ast`, the files `git ls-files` lists (a `.py` +suffix, or a `python` shebang, as the root `bootstrap` has). """ from __future__ import annotations @@ -25,6 +29,9 @@ import ast import html import re +import subprocess + +import pytest from conftest import REPO @@ -36,8 +43,37 @@ #: `os.name` is `nt`, `sh`'s elsewhere. HOST_QUOTERS = {"quote_arg", "shell_quoted", "shell_line", "shell_word"} -#: The tools the sentence names, which is every shipped file that calls one. -NAMED = {"shape-doctor.py", "scripts/family.py", "setup-project.py"} +#: A tool the sentence names for ALL its lines: any function of it may call +#: a host-dialect quoter. +EVERY_LINE = None + +#: The tools the sentence names, which is every shipped file that calls one, +#: and where it names a tool by the lines of particular commands, the +#: functions that print them (`Class.method` where a method does). A call +#: switched to `"posix"` in one of these leaves its function out of what the +#: file calls, and a new function that quotes for the host is not one the +#: sentence names: either way the sentence and this table change together. +NAMED = { + # "`shape-doctor.py`'s lines": every NEXT, fix and command line it prints. + "shape-doctor.py": EVERY_LINE, + # "`setup-project.py`'s family lines": the `switch -c` and `add` it prints. + "setup-project.py": {"family_next_command"}, + # "`family.py`'s `NEXT`, `switch -c`, `add` and `git rm -r --cached` + # lines". + "scripts/family.py": { + "_print_next", # the `NEXT` line: `git push -u origin ...` + "_switch_command", # the `git switch -c ` line + "_add_command", # the `family.py add --family-root ...` line + "_print_next_steps", # `init`'s block: its `switch -c` and `cd` + "_checked_mount_path", # the `git rm -r --cached -- ` remedy + }, +} + +#: The quoters' own home is not a tool, and only these two bodies are what the +#: sentence need not name: `shell_line` spells each word by `shell_quoted`, and +#: `shell_quoted` is the host-dialect rule itself. Any other function of +#: `scripts/repo_shape.py` that calls a host-dialect quoter is a tool's line. +QUOTERS_OWN_BODIES = {"scripts/repo_shape.py": {"shell_quoted", "shell_line"}} def squashed(text: str) -> str: @@ -79,42 +115,168 @@ def test_the_handbooks_windows_panel_says_the_same_words(): f"what README's \"Windows, natively\" says:\n {sentence}") -def host_calls(path) -> list[int]: - """The lines of every call in `path` to a quoter that follows the host: - not one that forces `sh`'s dialect with a `'posix'` argument or a - `posix=True` keyword.""" - tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) - lines = [] - for node in ast.walk(tree): - if not isinstance(node, ast.Call): - continue - func = node.func - name = func.id if isinstance(func, ast.Name) else getattr( - func, "attr", None) - if name not in HOST_QUOTERS: +def quoter_names(tree: ast.AST) -> set: + """The names a host-dialect quoter answers to in this module: the four of + `HOST_QUOTERS`, and every other name bound to one, by `from repo_shape + import shell_line as joined` or by `joined = shell_line` (or + `... = repo_shape.shell_line`), however many steps it takes (#665).""" + names = set(HOST_QUOTERS) + while True: + before = len(names) + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + names.update(alias.asname for alias in node.names + if alias.asname and alias.name in names) + elif (isinstance(node, ast.Assign) and len(node.targets) == 1 + and isinstance(node.targets[0], ast.Name)): + value = node.value + bound = value.id if isinstance(value, ast.Name) else getattr( + value, "attr", None) + if bound in names: + names.add(node.targets[0].id) + if len(names) == before: + return names + + +def host_calls(source: str, filename: str = "") -> list: + """`(function, line)` for every call in `source` to a quoter that follows + the host: not one that forces `sh`'s dialect with a `'posix'` argument or + a `posix=True` keyword. `function` is the innermost `def` the call is + in (`Class.method` for a method), or ``. A quoter handed on as a + value (`map(shell_quoted, words)`) is not a call and is not seen.""" + tree = ast.parse(source, filename=filename) + quoters = quoter_names(tree) + found = [] + + def walk(node, scope): + for child in ast.iter_child_nodes(node): + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, + ast.ClassDef)): + walk(child, scope + [child.name]) + else: + if isinstance(child, ast.Call) and host_call(child, quoters): + found.append((".".join(scope) or "", + child.lineno)) + walk(child, scope) + + walk(tree, []) + return found + + +def host_call(node: ast.Call, quoters: set) -> bool: + func = node.func + name = func.id if isinstance(func, ast.Name) else getattr( + func, "attr", None) + if name not in quoters: + return False + forced = any(isinstance(arg, ast.Constant) and arg.value == "posix" + for arg in node.args[1:]) or any( + isinstance(kw.value, ast.Constant) + and (kw.value.value == "posix" or kw.value.value is True) + for kw in node.keywords if kw.arg in ("platform", "posix")) + return not forced + + +def is_python_source(relative: str, first_line: bytes) -> bool: + """A `.py` file, or a file whose first line is a shebang naming `python`: + the root `bootstrap` is a shipped tool with no suffix (#665).""" + return relative.endswith(".py") or ( + first_line.startswith(b"#!") and b"python" in first_line) + + +def tool_sources() -> dict: + """`{relative path: text}` of every shipped Python file: what `git + ls-files` lists (tracked, or new and not ignored), outside `tests/` and + the dot-directories, that is a `.py` file or has a `python` shebang.""" + listed = subprocess.run( + ["git", "ls-files", "-z", "--cached", "--others", + "--exclude-standard"], + cwd=str(REPO), capture_output=True, check=True).stdout + sources = {} + for raw in sorted(set(listed.split(b"\0"))): + relative = raw.decode("utf-8") + parts = relative.split("/") + if not raw or parts[0] == "tests" or parts[0].startswith("."): continue - forced = any(isinstance(arg, ast.Constant) and arg.value == "posix" - for arg in node.args[1:]) or any( - isinstance(kw.value, ast.Constant) - and (kw.value.value == "posix" or kw.value.value is True) - for kw in node.keywords if kw.arg in ("platform", "posix")) - if not forced: - lines.append(node.lineno) - return lines + path = REPO / relative + if not path.is_file(): + continue # a gitlink, or deleted and not staged + data = path.read_bytes() + if is_python_source(relative, data.split(b"\n", 1)[0]): + sources[relative] = data.decode("utf-8") + return sources -def test_the_tools_that_quote_for_the_host_are_the_ones_the_sentence_names(): +def host_quoting(sources: dict) -> dict: + """`{relative path: {function: [lines]}}` of the tools that call a quoter + that follows the host, the quoters' own bodies left out.""" quoting = {} - for path in REPO.rglob("*.py"): - relative = path.relative_to(REPO) - if (relative.parts[0] == "tests" or relative.parts[0].startswith(".") - or relative.as_posix() == "scripts/repo_shape.py"): - continue # the quoters' own home is not a tool - lines = host_calls(path) - if lines: - quoting[relative.as_posix()] = lines - assert set(quoting) == NAMED, ( + for relative, source in sources.items(): + own = QUOTERS_OWN_BODIES.get(relative, set()) + for function, line in host_calls(source, relative): + if function not in own: + quoting.setdefault(relative, {}).setdefault( + function, []).append(line) + return quoting + + +def test_the_tools_that_quote_for_the_host_are_the_ones_the_sentence_names(): + quoting = host_quoting(tool_sources()) + assert set(quoting) == set(NAMED), ( "README's \"Windows, natively\" and the handbook's Windows panel name " "the tools whose printed lines are quoted for PowerShell on Windows. " f"The files that call a host-dialect quoter are {sorted(quoting)}: " "change the sentence in both places, then NAMED here.") + + +@pytest.mark.parametrize("tool", sorted( + tool for tool, functions in NAMED.items() if functions is not EVERY_LINE)) +def test_the_functions_that_quote_for_the_host_are_the_ones_the_sentence_names( + tool): + """`family.py` and `setup-project.py` are named for the lines of certain + commands, not for every line: a call switched to POSIX quoting leaves its + function out, and a new function that calls a host-dialect quoter is a + line the sentence does not name.""" + calling = set(host_quoting(tool_sources()).get(tool, {})) + assert calling == NAMED[tool], ( + f"{tool}'s functions that call a host-dialect quoter are " + f"{sorted(calling)}, and the sentence names {sorted(NAMED[tool])}: " + "change the sentence in both places, then NAMED here.") + + +def test_a_quoter_imported_under_another_name_is_a_host_call(): + source = ("from repo_shape import shell_line as joined, shell_quoted\n" + "spelled = joined\n" + "def a():\n return joined(['git'])\n" + "def b():\n return spelled(['git'])\n" + "def c():\n return shell_quoted('x', 'posix')\n" + "def d():\n return joined(['git'], platform='posix')\n") + assert host_calls(source) == [("a", 4), ("b", 6)] + + +def test_a_method_is_named_with_its_class(): + source = ("import repo_shape\n" + "class Plan:\n def line(self):\n" + " return repo_shape.shell_line(['git'])\n") + assert host_calls(source) == [("Plan.line", 4)] + + +def test_only_the_two_quoter_bodies_of_repo_shape_are_exempt(): + own = ("def shell_quoted(value, platform=None):\n return value\n" + "def shell_line(words, platform=None):\n" + " return ' '.join(shell_quoted(w, platform) for w in words)\n") + assert host_quoting({"scripts/repo_shape.py": own}) == {} + third = own + "def next_line():\n return shell_line(['git'])\n" + assert host_quoting({"scripts/repo_shape.py": third}) == { + "scripts/repo_shape.py": {"next_line": [6]}} + assert host_quoting({"scripts/other.py": own}) != {}, ( + "the exemption is for repo_shape.py's bodies, not for any file that " + "defines functions of those names") + + +def test_a_suffixless_python_file_is_scanned_and_another_language_is_not(): + assert is_python_source("bootstrap", b"#!/usr/bin/env python3\n") + assert is_python_source("scripts/x.py", b"") + assert not is_python_source("openRepoShape", b"#!/usr/bin/env bash\n") + assert not is_python_source("README.md", b"# Title\n") + assert "bootstrap" in tool_sources() diff --git a/tests/test_validator_child_encoding.py b/tests/test_validator_child_encoding.py index 04a754fd..cba5a44b 100644 --- a/tests/test_validator_child_encoding.py +++ b/tests/test_validator_child_encoding.py @@ -606,15 +606,28 @@ def test_the_handler_a_child_would_not_know_is_never_passed_on(): #: The shipped tools that run a Python child of the standard's own, and so #: build its environment. `shape-doctor.py` sets UTF-8 itself (it reads the -#: pipe as UTF-8 too); the other two use the helpers above. +#: pipe as UTF-8 too); the other three use the helpers above. `family.py`'s +#: `siblings` runs the holder's own `siblings.py` that way (#214, #543). LAUNCHERS = { "scripts/bump-leg.py": "captured_child_environment", "update-shape.py": "inherited_child_environment", + "scripts/family.py": "inherited_child_environment", } +def runs_the_interpreter(argv0: ast.expr) -> bool: + """`sys.executable`, or `sys.executable or PYTHON`: the interpreter that + is running this process, with the name the standard prints for it + (`repo_shape.PYTHON`) when `sys.executable` is empty or `None`, as + `family.py`'s `siblings` child spells it.""" + if isinstance(argv0, ast.BoolOp) and isinstance(argv0.op, ast.Or): + argv0 = argv0.values[0] + return ast.unparse(argv0) == "sys.executable" + + def interpreter_runs(tree: ast.AST) -> list[ast.Call]: - """Every `subprocess.run([sys.executable, ...])` in `tree`.""" + """Every `subprocess.run([sys.executable, ...])` in `tree`, and every + `subprocess.run([sys.executable or PYTHON, ...])`.""" found = [] for node in ast.walk(tree): if not (isinstance(node, ast.Call) @@ -625,7 +638,7 @@ def interpreter_runs(tree: ast.AST) -> list[ast.Call]: continue argv = node.args[0] if (isinstance(argv, ast.List) and argv.elts - and ast.unparse(argv.elts[0]) == "sys.executable"): + and runs_the_interpreter(argv.elts[0])): found.append(node) return found