From d6d1df2340652efe87c0cefbcad4eac8f804ce32 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:05:35 +0000 Subject: [PATCH 01/10] conftest.py and four test files: one helper asks whether a system can carry a name or branch that is not UTF-8, `os.fsencode` first and the disk's EILSEQ second, and two pins hold `bump-leg.py`'s UTF-8 decode and the leg finding's role, so a test no longer skips on the disk alone or passes under a decode as ASCII or a finding named by path (#663) #461's five test lines, lane 2's L2-52 and lane 4's R423 N1 and N2 and R455 F1 and F3. Test only: no tool and no template changes, and `validate-manifest.py`, a pinned copy, is read and not edited. - `tests/conftest.py` holds `skip_where_a_name_cannot_be_not_utf_8`, which `tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py` each had a copy of. It asks `skip_where_fsencode_gives_a_surrogate_no_byte` first (Windows' `os.fsencode` makes three bytes of U+DCFF, so no name that stands for a byte is carried there, though NTFS would take the file), then the disk (macOS answers EILSEQ); any other error is the test's own. - `make_file` and the `--branch a\udcffb` test call it, so they skip on Windows too, where the disk takes the name and the child then refuses the branch. `test_a_branch_the_system_carries_has_no_fault` asks the same function instead of its own copy of the question. - `test_a_name_that_is_utf8_under_quote_path_false_is_still_refused` asserts that the line naming `caf.yml` holds no literal `\x`, in any locale, so `bump-leg.py` decoding git's bytes as ASCII fails it. - `_leg_branch_findings` is asked of a leg whose role is not its path, so naming the leg by `path` fails it. - `tests/test_non_utf8_name_helpers.py` holds the helpers to this, with Windows' `os.fsencode` and macOS's EILSEQ emulated: CI is Linux only, and no Windows or macOS run was made. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 83 +++++++++++++++++ tests/test_adopt_pins.py | 20 +--- tests/test_bootstrap_branch_bytes.py | 74 ++++++++++++--- tests/test_non_utf8_git_reads.py | 77 +++++++++++++--- tests/test_non_utf8_name_helpers.py | 133 +++++++++++++++++++++++++++ tests/test_pin_source_unreadable.py | 21 +---- 6 files changed, 346 insertions(+), 62 deletions(-) create mode 100644 tests/test_non_utf8_name_helpers.py diff --git a/tests/conftest.py b/tests/conftest.py index 33cbf476..0e9da01d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,6 +25,7 @@ from __future__ import annotations +import errno import functools import importlib.util import os @@ -864,6 +865,88 @@ def a_hook_that_unstages(hooks: Path, path: str = PINNED_COPY) -> Path: return hooks +# --- a file name or a branch that is not UTF-8 (#663) ------------------------ +# +# A test that needs a file name or a branch holding a byte that is not UTF-8 +# (Python keeps the byte as a lone surrogate, U+DC80 to U+DCFF, which +# `os.fsencode` gives back as the byte on POSIX) asks these helpers whether +# this system can carry it, so each system's answer is written in one place +# and a change to it reaches every test that asks. +# `tests/test_non_utf8_name_helpers.py` holds them to it, Windows included, by +# emulating its `os.fsencode`: CI runs the suite on Linux only (#597). + +#: The surrogate the helpers ask about: the byte 0xFF, which no UTF-8 text has. +NOT_UTF_8_PROBE = "\udcff" + + +def skip_where_fsencode_gives_a_surrogate_no_byte(text: str) -> None: + """Skip where `text` holds a byte-carrying surrogate (U+DC80 to U+DCFF) + and this system's `os.fsencode` makes no single byte of one. + + That is Windows: its `os.fsencode` is UTF-8 with `surrogatepass`, so it + writes the three bytes of a sequence that is not UTF-8 for EVERY + surrogate, and none of them is a byte of a name. A disk probe cannot be + trusted to ask it: CPython hands a str path to `CreateFileW` as UTF-16 + and NTFS most likely stores a lone surrogate unchecked (read, not run), + so the file is made, and git for Windows then sees the name through its + own UTF-16 conversion, which is not the byte the test meant. A command + line is the same (`CreateProcessW`), which is why `bootstrap.py`'s + `_argument_fault` refuses U+DC80 to U+DCFF there. Asked through the `os` + module at call time, so a test emulates Windows with + `monkeypatch.setattr(os, "fsencode", )`. + """ + if (any("\udc80" <= char <= "\udcff" for char in text) + and len(os.fsencode(NOT_UTF_8_PROBE)) != 1): + pytest.skip("this system's `os.fsencode` makes no byte of a surrogate " + "that stands for a byte that is not UTF-8") + + +def skip_where_a_name_cannot_be_not_utf_8(directory: Path) -> None: + """Skip, as `POSIX_NAMES` skips on Windows, where a file name that is not + UTF-8 cannot be made under `directory`. Two systems say so: + + - `skip_where_fsencode_gives_a_surrogate_no_byte` (Windows), asked first, + before the disk is touched; + - a filesystem that refuses the name: macOS answers EILSEQ ("Illegal byte + sequence"), so no clone can sit at such a path there. + + The second is asked by making a directory `probe` under + `directory` and removing it; any other error is the test's own and is + raised. + """ + probe = directory / f"probe{NOT_UTF_8_PROBE}" + skip_where_fsencode_gives_a_surrogate_no_byte(probe.name) + try: + probe.mkdir() + except OSError as exc: + if exc.errno != errno.EILSEQ: + raise + pytest.skip("needs a file name that is not UTF-8, which this " + "filesystem refuses (EILSEQ)") + probe.rmdir() + + +def windows_fsencode(text: str) -> bytes: + """`os.fsencode` as Windows has it: UTF-8 with `surrogatepass`, so every + lone surrogate is the three bytes of a sequence that is not UTF-8. For + `monkeypatch.setattr(os, "fsencode", windows_fsencode)`, which puts the + guard above in front of a test that Linux would let through.""" + return text.encode("utf-8", "surrogatepass") + + +def skip_message(call, *args) -> str | None: + """The reason `call(*args)` skips the running test with, or None where it + returns; any other exception goes on. For a test OF a skip helper: a + skip that escapes a test ends it SKIPPED, which no run fails on, so a + helper that skipped where it should not would pass `pytest.raises` and + every other assertion that expects no skip.""" + try: + call(*args) + except pytest.skip.Exception as skipped: + return skipped.msg + return None + + # --- the `gh` guard (#296) --------------------------------------------------- # # NO TEST REACHES THE OPERATOR'S `gh`, and the doctor's `machine` row is still diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 719af0f7..7b3bb313 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -34,7 +34,6 @@ from __future__ import annotations -import errno import importlib.util import json import os @@ -50,7 +49,8 @@ from conftest import (ADOPT, REPO, SCAFFOLD, SYNTHETIC_TREE, WINDOWS_SKIP, as_human, cloned_just_pushed, findings, git, make_source_repo, planned, refused_before_any_leg, - resolve, rmtree, run_script) + resolve, rmtree, run_script, + skip_where_a_name_cannot_be_not_utf_8) sys.path.insert(0, str(REPO / "scripts")) import shape_materialize # noqa: E402 @@ -855,22 +855,6 @@ def test_a_pin_source_check_would_refuse_is_refused_by_plan( UNREAD = "unreadclone" -def skip_where_a_name_cannot_be_not_utf_8(directory: Path) -> None: - """Skip, as `POSIX_NAMES` skips on Windows, on a filesystem that refuses - a file name that is not UTF-8: macOS answers EILSEQ ("Illegal byte - sequence"), so no clone can sit at such a path there. Asked of a - directory made under `directory`; any other error is the test's own.""" - probe = directory / "probe\udcff" - try: - probe.mkdir() - except OSError as exc: - if exc.errno != errno.EILSEQ: - raise - pytest.skip("needs a file name that is not UTF-8, which this " - "filesystem refuses (EILSEQ)") - probe.rmdir() - - @pytest.mark.parametrize("name, reason", [ pytest.param(f"{UNREAD}‍x", "it holds an invisible formatting character", id="zwj"), diff --git a/tests/test_bootstrap_branch_bytes.py b/tests/test_bootstrap_branch_bytes.py index bab82102..3f63761e 100644 --- a/tests/test_bootstrap_branch_bytes.py +++ b/tests/test_bootstrap_branch_bytes.py @@ -45,7 +45,10 @@ import pytest -from conftest import REPO, git, load_without_bytecode +from conftest import (REPO, git, load_without_bytecode, skip_message, + skip_where_a_name_cannot_be_not_utf_8, + skip_where_fsencode_gives_a_surrogate_no_byte, + windows_fsencode) from test_bootstrap_spelling import (ASSEMBLY_TEMPLATE, DASH, bootstrap_of, edit, pin12, run) from test_stdout_encoding import child_environ, run_tool_bytes @@ -349,12 +352,12 @@ def test_a_name_that_is_not_utf8_is_still_carried_and_the_branch_made( `os.fsencode` carries it, and git is handed that byte, as on origin/main. `--branch` is the only way it reaches step (a) with the validators out of it (the manifest validator names it, #280's rule for a surrogate). Made on - a disk that takes the byte, skipped on one that does not.""" - probe = tmp_path / "probe-\udcff" - try: - probe.write_bytes(b"") - except (OSError, ValueError): - pytest.skip("this disk takes no file name with the byte 0xff") + a system and a disk that take the byte, skipped on one that does not: the + shared helper asks `os.fsencode` first, which is what Windows answers + (NTFS takes the name, and `_argument_fault` refuses the branch there, + which `test_where_a_surrogate_is_three_bytes_and_no_byte_it_is_refused` + holds), and then the disk, which macOS refuses (#663).""" + skip_where_a_name_cannot_be_not_utf_8(tmp_path) code, out, err = run(bootstrap_of(project), "--skip-validators", "--branch", "a\udcffb", encoding=ENCODING, tmp_path=tmp_path, cwd=project) @@ -370,12 +373,40 @@ def test_a_name_that_is_not_utf8_is_still_carried_and_the_branch_made( assert head == b"refs/heads/a\xffb\n", head +def test_the_branch_test_skips_where_fsencode_makes_no_byte_of_the_surrogate( + project, tmp_path, monkeypatch): + """THE WINDOWS GUARD, EMULATED (#663, R455 F1): on NTFS the disk takes + `probe\\udcff`, the child is given `--branch a\\udcffb`, and + `_argument_fault` refuses U+DCFF where `os.fsencode` writes three bytes, + so the `branch a\\udcffb created` line the test expects is absent. With + Windows' `os.fsencode` the test is therefore SKIPPED, before it runs + bootstrap: it is called here as it is, under the emulation, and the legs + stand as they were.""" + before = legs_state(project) + monkeypatch.setattr(os, "fsencode", windows_fsencode) + reason = skip_message( + test_a_name_that_is_not_utf8_is_still_carried_and_the_branch_made, + project, tmp_path) + assert reason is not None, "the test ran where `os.fsencode` gives no byte" + assert "os.fsencode" in reason, reason + assert legs_state(project) == before + + @pytest.fixture(scope="module") def bootstrap_module(): """The template run as a module, no bytecode written beside it.""" return load_without_bytecode(ASSEMBLY_TEMPLATE, "bootstrap_branch_bytes") +@pytest.fixture(scope="module") +def validator_module(): + """`validate-manifest.py`, the same template, as a module: the file is a + pinned copy, so its functions are read and never changed here.""" + return load_without_bytecode( + REPO / "templates" / "assembly-root" / "scripts" + / "validate-manifest.py", "validate_manifest_branch_bytes") + + @pytest.mark.parametrize("branch", [ "main", "release/1.2", "feature/日本", "a‍b", "a\xe9b", "a b", "a\x1bb", "a..b", "", "\udc80", "a\udcffb"]) @@ -385,9 +416,7 @@ def test_a_branch_the_system_carries_has_no_fault(bootstrap_module, branch): 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.""" - if any("\udc80" <= char <= "\udcff" for char in branch) and len( - os.fsencode("\udcff")) != 1: - pytest.skip("this system's `os.fsencode` makes no byte of U+DCFF") + skip_where_fsencode_gives_a_surrogate_no_byte(branch) assert bootstrap_module._argument_fault(branch) is None @@ -424,13 +453,28 @@ def test_where_a_surrogate_is_three_bytes_and_no_byte_it_is_refused( assert fault.startswith("U+DC"), fault -def test_the_two_templates_agree_on_what_a_fault_is(bootstrap_module): +def test_the_two_templates_agree_on_what_a_fault_is(bootstrap_module, + validator_module): """`validate-manifest.py` finds ANY surrogate (#280's rule, which a UTF-8 file settles), where `bootstrap.py` asks only whether a command can be given the value: so every value bootstrap refuses, the validator names.""" - validator = load_without_bytecode( - REPO / "templates" / "assembly-root" / "scripts" - / "validate-manifest.py", "validate_manifest_branch_bytes") for branch in ("a\x00b", "a\ud83db", "a\ude00", "\udc00"): assert bootstrap_module._argument_fault(branch) is not None - assert validator._branch_finding("c", "w", branch) != [] + assert validator_module._branch_finding("c", "w", branch) != [] + + +@pytest.mark.parametrize("role", ["spec", "code"]) +def test_the_leg_finding_names_the_leg_by_its_role_and_not_its_path( + validator_module, role): + """THE NAME IS THE ROLE (#663, R455 F3, mutant R2: naming `path` passed + all 54 tests). The templates' roles equal their paths (`spec` is mounted + at `spec`), so nothing the scaffold writes can tell the two apart, and + the finding is asked of a leg whose path is not its role, as a spec leg + mounted at `contracts` is. A unit test on the validator, which this file + loads for its last test too: it is a pinned copy and is not edited.""" + [finding] = validator_module._leg_branch_findings( + [{"role": role, "path": "contracts", "branch": "a\x00b"}]) + assert finding.startswith( + f"FINDING manifest-leg-branch: leg '{role}': branch is 'a\\x00b', " + "which holds U+0000, a NUL"), finding + assert "contracts" not in finding, finding diff --git a/tests/test_non_utf8_git_reads.py b/tests/test_non_utf8_git_reads.py index c79bada2..18fbb7e9 100644 --- a/tests/test_non_utf8_git_reads.py +++ b/tests/test_non_utf8_git_reads.py @@ -32,11 +32,13 @@ NO NETWORK AND NO GITHUB. The scaffold and `family.py init` here are run as child processes into LOCAL bare remotes (`--local-remote-dir`), which never calls `gh`. A name that is not UTF-8 is made by `os.fsdecode` of its bytes, so -a POSIX filesystem takes it as it is; where the disk refuses (macOS, Windows) -the test skips, and where the ENVIRONMENT cannot carry a byte (Windows') the -`user.name` rows skip. The checkout under test for `_shape_revision` and -`_shape_snapshot` is a COPY of the standard in a repository of its own -(`standard`), made dirty one test at a time, never this checkout. +a POSIX filesystem takes it as it is; where the system cannot carry the byte +the test skips (`make_file`: Windows' `os.fsencode` makes no byte of it, and +macOS's disk refuses it), and where the ENVIRONMENT cannot carry a byte +(Windows') the `user.name` rows skip. The checkout under test for +`_shape_revision` and `_shape_snapshot` is a COPY of the standard in a +repository of its own (`standard`), made dirty one test at a time, never this +checkout. """ from __future__ import annotations @@ -49,7 +51,10 @@ import pytest -from conftest import ORG, PROJECT, REPO, cloned_just_pushed, git, run_script +from conftest import (ORG, PROJECT, REPO, cloned_just_pushed, git, run_script, + skip_message, + skip_where_fsencode_gives_a_surrogate_no_byte, + windows_fsencode) from test_bump_leg import branched, bump sys.path.insert(0, str(REPO / "scripts")) @@ -85,7 +90,11 @@ def make_file(path: Path, data: bytes = b"on: [push]\n") -> None: """Write `data` at `path`, or skip: a name that is not UTF-8 needs a - filesystem that stores bytes (Linux does; APFS and NTFS refuse it).""" + system that carries the byte and a filesystem that stores it (Linux does; + APFS refuses it, and on Windows `os.fsencode` makes no byte of it, which + is asked FIRST: NTFS most likely takes the surrogate, and the file would + then not be the one the test means). A name that is UTF-8 is written.""" + skip_where_fsencode_gives_a_surrogate_no_byte(path.name) try: path.write_bytes(data) except (OSError, UnicodeError) as exc: @@ -93,6 +102,47 @@ def make_file(path: Path, data: bytes = b"on: [push]\n") -> None: f"UTF-8: {exc}") +def test_make_file_skips_where_fsencode_makes_no_byte_and_writes_nothing( + tmp_path, monkeypatch): + """THE WINDOWS GUARD, EMULATED (#663, R423 N1): where `os.fsencode` gives + a surrogate three bytes, NTFS most likely takes `caf\\udce9.yml` anyway + and git for Windows would see another name, so `make_file` skips BEFORE it + writes, and nothing is on the disk. Linux takes the name, so a file + standing there would be the failure.""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + target = tmp_path / NOT_UTF8 + reason = skip_message(make_file, target) + assert reason is not None, "make_file wrote the name instead of skipping" + assert "os.fsencode" in reason, reason + assert not target.exists(), "the file was written before the skip" + + +def test_make_file_writes_a_name_that_is_utf8_where_fsencode_makes_no_byte( + tmp_path, monkeypatch): + """The guard is for the byte and no wider: Windows holds `caf\\u00e9.yml` + as text, and the tests that use it must run there.""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + target = tmp_path / UTF8 + assert skip_message(make_file, target, b"on: [pull_request]\n") is None + assert target.read_bytes() == b"on: [pull_request]\n" + + +def test_make_file_writes_the_name_that_is_not_utf8_where_the_system_has_it( + tmp_path): + """The control: where `os.fsencode` carries the byte and the disk takes it + (Linux), the file is written under the name that is not UTF-8, so the + guard skipped nothing.""" + if len(os.fsencode("\udce9")) != 1: + pytest.skip("this system's `os.fsencode` makes no byte of U+DCE9") + target = tmp_path / NOT_UTF8 + reason = skip_message(make_file, target, b"on: [push]\n") + if reason is not None and "filesystem" in reason: + pytest.skip(reason) + assert reason is None, reason + assert os.fsencode(target.name) == b"caf\xe9.yml" + assert target.read_bytes() == b"on: [push]\n" + + def git_bytes(*args: str, cwd: Path) -> bytes: """`git ` in `cwd`, stdout as the bytes git wrote. `conftest.git` decodes as text, which is the failure under test.""" @@ -205,16 +255,21 @@ def test_a_name_that_is_utf8_under_quote_path_false_is_still_refused( Its bytes decode, and it was refused on main; it still is, and the refusal still prints it as git did. (The name is matched on its ASCII ends: a child's stderr is decoded by the locale here, which is not UTF-8 - on every platform.)""" + on every platform. What the middle of the name is NOT is held instead, in + any locale: the tool decodes git's bytes as UTF-8, so `\\u00e9` is one + character, where a decode as ASCII would show its two bytes as `\\xc3\\xa9`, + and a locale's decode of what the child printed is mojibake at worst and + never a `\\x`: #423's mutant R1, #663.)""" dirty_root(project, UTF8, quote_path=False) before = found(project) result = bump(project, scaffolded, advanced, *extra) output = result.stderr + result.stdout assert result.returncode == 2, output assert "REFUSED bump-leg-root-dirty: " in result.stderr, output - assert any(line.startswith(" M .github/workflows/caf") - and line.endswith(".yml") - for line in result.stderr.splitlines()), output + [shown] = [line for line in result.stderr.splitlines() + if line.startswith(" M .github/workflows/caf")] + assert shown.endswith(".yml"), output + assert "\\x" not in shown, output assert "Traceback" not in output, output assert found(project) == before diff --git a/tests/test_non_utf8_name_helpers.py b/tests/test_non_utf8_name_helpers.py new file mode 100644 index 00000000..8c387eee --- /dev/null +++ b/tests/test_non_utf8_name_helpers.py @@ -0,0 +1,133 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The two helpers a test asks whether a name that is not UTF-8 can be +carried here (#663, #461's L2-52). + +`skip_where_a_name_cannot_be_not_utf_8` was two helpers with one body +(`tests/test_adopt_pins.py` and `tests/test_pin_source_unreadable.py`), and +`make_file` and the `--branch a\\udcffb` test each asked a smaller question of +their own, or none: on Windows the disk takes the name, so no probe of the +disk skips there. Now `tests/conftest.py` holds both questions once, the +system's first (`os.fsencode`) and the disk's second (EILSEQ), and this file +holds them to it. + +CI runs the suite on Linux only (#597), so what Windows answers is EMULATED: +`os.fsencode` is replaced for one test by `windows_fsencode`, which gives a +surrogate three bytes, as Windows' does. No Windows run was made. macOS's +EILSEQ is emulated the same way, by a `mkdir` that answers it. +""" + +from __future__ import annotations + +import errno +import os +import sys +from pathlib import Path + +import pytest + +from conftest import (NOT_UTF_8_PROBE, skip_message, + skip_where_a_name_cannot_be_not_utf_8, + skip_where_fsencode_gives_a_surrogate_no_byte, + windows_fsencode) + +LINUX_ONLY = pytest.mark.skipif( + not sys.platform.startswith("linux"), + reason="Linux stores any byte in a file name; the other systems are " + "asked by the tests below that emulate them") + + +def mkdir_answers(code: int, said: str): + """A `Path.mkdir` that fails as the disk would with `code`.""" + def refuse(self, *args, **kwargs): + raise OSError(code, said, str(self)) + return refuse + + +# --- the system's question: what `os.fsencode` makes of the surrogate -------- + +@LINUX_ONLY +def test_a_system_that_carries_the_byte_is_not_skipped(): + """The control: Linux's `os.fsencode` gives U+DCFF back as the one byte + 0xFF, so no text is skipped.""" + assert os.fsencode(NOT_UTF_8_PROBE) == b"\xff" + assert skip_message(skip_where_fsencode_gives_a_surrogate_no_byte, + f"caf{NOT_UTF_8_PROBE}.yml") is None + + +def test_where_fsencode_makes_three_bytes_of_the_surrogate_the_name_is_skipped( + monkeypatch): + """Windows' `os.fsencode` (emulated): U+DCFF is the three bytes of a + sequence that is not UTF-8, none of them 0xFF, so a name that holds a + byte-carrying surrogate is skipped, and each one from U+DC80 to U+DCFF.""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + assert len(os.fsencode(NOT_UTF_8_PROBE)) == 3 + for surrogate in ("\udc80", "\udce9", "\udcff"): + reason = skip_message(skip_where_fsencode_gives_a_surrogate_no_byte, + f"a{surrogate}b") + assert reason is not None, f"U+{ord(surrogate):04X} was not skipped" + assert "os.fsencode" in reason, reason + + +@pytest.mark.parametrize("text", [ + "plain", "café.yml", "feature/日本", "", "a\ud83db"], + ids=["ascii", "text", "cjk", "empty", "surrogate-without-a-byte"]) +def test_where_fsencode_makes_three_bytes_a_name_that_is_text_is_not_skipped( + monkeypatch, text): + """The guard is for the surrogates that stand for a byte (U+DC80 to + U+DCFF) and no wider: text is the same on every system, and a surrogate + that stands for no byte is not this helper's to judge (`_argument_fault` + refuses it, as `bootstrap.py`'s tests hold).""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + assert skip_message(skip_where_fsencode_gives_a_surrogate_no_byte, + text) is None + + +# --- the shared helper: the system, then the disk ---------------------------- + +@LINUX_ONLY +def test_a_disk_that_takes_the_name_is_not_skipped_and_the_probe_is_removed( + tmp_path): + """The control: no skip, and nothing is left in the directory.""" + assert skip_message(skip_where_a_name_cannot_be_not_utf_8, + tmp_path) is None + assert list(tmp_path.iterdir()) == [] + + +def test_where_fsencode_makes_three_bytes_the_disk_is_not_asked( + tmp_path, monkeypatch): + """THE SYSTEM IS ASKED FIRST (#663): on Windows the disk would take the + name, so a skip that waited for the disk would never come. The directory + does not exist, so an `mkdir` under it would be FileNotFoundError and not + a skip: the skip is the answer, and the disk was not touched.""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + reason = skip_message(skip_where_a_name_cannot_be_not_utf_8, + tmp_path / "absent") + assert reason is not None, "the helper went on to ask the disk" + assert "os.fsencode" in reason, reason + + +def test_a_disk_that_answers_eilseq_is_skipped(tmp_path, monkeypatch): + """macOS (emulated): APFS answers EILSEQ, `Illegal byte sequence`, to a + file name that is not UTF-8, and no clone can sit at such a path.""" + if len(os.fsencode(NOT_UTF_8_PROBE)) != 1: + pytest.skip("on this system the first question is the one that skips") + monkeypatch.setattr(Path, "mkdir", + mkdir_answers(errno.EILSEQ, "Illegal byte sequence")) + reason = skip_message(skip_where_a_name_cannot_be_not_utf_8, tmp_path) + assert reason is not None, "EILSEQ was not a skip" + assert "EILSEQ" in reason, reason + + +@pytest.mark.parametrize("code", [errno.EACCES, errno.ENOSPC, errno.EEXIST], + ids=["EACCES", "ENOSPC", "EEXIST"]) +def test_any_other_error_from_the_disk_is_the_tests_own( + tmp_path, monkeypatch, code): + """Only EILSEQ is the filesystem's refusal of the name: a full disk, a + folder nobody can write in or a probe left by an earlier run is a + failure of the test, raised as it was, and never a skip that hides it.""" + if len(os.fsencode(NOT_UTF_8_PROBE)) != 1: + pytest.skip("on this system the first question is the one that skips") + monkeypatch.setattr(Path, "mkdir", mkdir_answers(code, os.strerror(code))) + with pytest.raises(OSError) as raised: + skip_message(skip_where_a_name_cannot_be_not_utf_8, tmp_path) + assert raised.value.errno == code diff --git a/tests/test_pin_source_unreadable.py b/tests/test_pin_source_unreadable.py index 177e8c94..c43a41ad 100644 --- a/tests/test_pin_source_unreadable.py +++ b/tests/test_pin_source_unreadable.py @@ -41,7 +41,8 @@ import pytest -from conftest import ADOPT, ORG, REPO, SCAFFOLD, git, make_source_repo, run_script +from conftest import (ADOPT, ORG, REPO, SCAFFOLD, git, make_source_repo, + run_script, skip_where_a_name_cannot_be_not_utf_8) sys.path.insert(0, str(REPO / "scripts")) from repo_shape import load_yaml, parse_yaml, tree_digest # noqa: E402 @@ -592,22 +593,6 @@ def test_a_typed_dot_is_still_the_directory_the_tool_runs_in( ] -def skip_where_no_file_name_is_not_utf_8(directory: Path) -> None: - """Skip, as a Windows command line skips, where the filesystem refuses - a file name that is not UTF-8, so that no clone can sit at one: macOS - answers EILSEQ ("Illegal byte sequence"). Asked by making one under - `directory`; any other error is the test's to report.""" - probe = directory / "probe\udcff" - try: - probe.mkdir() - except OSError as exc: - if exc.errno != errno.EILSEQ: - raise - pytest.skip("needs a file name that is not UTF-8, which this " - "filesystem refuses (EILSEQ)") - probe.rmdir() - - @pytest.mark.parametrize("name", CLONE_NAMES) def test_the_pin_file_says_how_the_digest_was_read_and_not_where( name, tmp_path, adoptable, glass): @@ -618,7 +603,7 @@ def test_the_pin_file_says_how_the_digest_was_read_and_not_where( --all`, read as bytes).""" product, commit = glass if "\udcff" in name: - skip_where_no_file_name_is_not_utf_8(tmp_path) + skip_where_a_name_cannot_be_not_utf_8(tmp_path) clones = tmp_path / "clones" shutil.copytree(product, clones / name, symlinks=True) result = run("scaffold-run", tmp_path, adoptable, str(clones / name), From 0b1d1ddbeeca662500534f58c39e5b5a0c67a70d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:15:43 +0000 Subject: [PATCH 02/10] test_bootstrap_branch_bytes.py: the no-fault test's Windows guard is held by an emulation, so dropping the shared helper from it fails a test on Linux too (#663) `test_a_branch_the_system_carries_has_no_fault` now asks `skip_where_fsencode_gives_a_surrogate_no_byte` where it had its own copy of the question. The new test calls it under Windows' `os.fsencode` (emulated): a branch holding a byte-carrying surrogate is skipped, plain text and text with an accent run. Removing the call fails the two skipped rows. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_bootstrap_branch_bytes.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_bootstrap_branch_bytes.py b/tests/test_bootstrap_branch_bytes.py index 3f63761e..5a79ff8e 100644 --- a/tests/test_bootstrap_branch_bytes.py +++ b/tests/test_bootstrap_branch_bytes.py @@ -420,6 +420,24 @@ def test_a_branch_the_system_carries_has_no_fault(bootstrap_module, branch): assert bootstrap_module._argument_fault(branch) is None +@pytest.mark.parametrize("branch, skipped", [ + pytest.param("a\udcffb", True, id="byte-carrying-surrogate"), + pytest.param("\udc80", True, id="the-first-one"), + pytest.param("release/1.2", False, id="plain"), + pytest.param("a\xe9b", False, id="text"), +]) +def test_the_no_fault_test_skips_where_fsencode_makes_no_byte_of_the_branch( + bootstrap_module, monkeypatch, branch, skipped): + """THE WINDOWS GUARD, EMULATED (#663): under Windows' `os.fsencode` the + test above skips a branch holding a byte-carrying surrogate, which + `_argument_fault` refuses there, and runs every other (the next test + holds the refusal).""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + reason = skip_message(test_a_branch_the_system_carries_has_no_fault, + bootstrap_module, branch) + assert (reason is not None) is skipped, reason + + @pytest.mark.parametrize("branch, fault", [ ("a\x00b", "U+0000, a NUL"), ("\x00", "U+0000, a NUL"), From 19d126cd4f7c5c6e0399ad9117ca77d7224c46e7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:19:30 +0000 Subject: [PATCH 03/10] shape_materialize.py, scaffold-project.py and family.py: the folder refusals and `family-operation-in-progress` name a path as `shown_source` names it, so an ESC or a line break in a folder's name no longer reaches the terminal or begins a line of the person's choosing (#692) #622's folder refusals printed a path as it was. `family.py init --into --dry-run` gave `REFUSED family-folder-cannot-be-made: the family folder .../wd-^[[31mRED/Orbit cannot be made: ...` through `cat -v`, and a name holding a line break and then `WARNING forged-line: pwned` printed that line as a line of its own. The same two tools already spell a `--pin-source` that way (`shape_materialize.shown_source`): as it is, and as Python's `repr` spells it where a character in it is not printable. Each path is spelled so now: both answers of `folder_cannot_be_made`, the folder in `refuse_unmakeable_folder` and `make_folder`, `scaffold-target-exists`, `family-target-exists`, and the mark's path in `family-operation-in-progress`. A printable path is named byte for byte, so no existing row changes and `why.endswith(CANNOT_WRITE_IN)` still holds. `test_dry_run_parity.py` gains rows for an ESC and for a line break then `WARNING forged-line: pwned` in `SCAFFOLD_CASES`, `FAMILY_CASES` and the unwritable-folder rows, and `assert_refused` fails any forged line; `test_family_refuse_up_front.py`'s newline-in-the-holder test now expects the one-line `repr` form and covers an ESC and a forged line too. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scaffold-project.py | 5 +- scripts/family.py | 16 ++- scripts/shape_materialize.py | 23 ++-- tests/test_dry_run_parity.py | 167 ++++++++++++++++++++++++++- tests/test_family_refuse_up_front.py | 30 +++-- 5 files changed, 219 insertions(+), 22 deletions(-) diff --git a/scaffold-project.py b/scaffold-project.py index 5a73dd13..29eae142 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -124,6 +124,9 @@ make_folder, refuse_unless_not_found, refuse_unmakeable_folder, target_in_the_way, target_remediation, ) +# How a refusal names a folder: as it is, unless a character in it is not +# printable, and then as `repr` spells it (#692). +from shape_materialize import shown_source # noqa: E402 #: How a refusal names each value the manifest records (#258, #261). The leg #: paths are not here: `refuse_bad_leg_paths` asks for them (#179, #225). @@ -1247,7 +1250,7 @@ def _refuse_existing_trees(work_root: Path, names: dict) -> None: if why: raise Refusal( "scaffold-target-exists", - f"{target} already exists and {why}", + f"{shown_source(target)} already exists and {why}", target_remediation( why, "Remediation: choose an empty --work-dir. There is no " diff --git a/scripts/family.py b/scripts/family.py index e4f430cb..389db20a 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -195,6 +195,9 @@ # holder's clone URL under `--local-remote-dir` (#255, #433). It is the # question the scaffold and `adopt-project.py` ask of theirs. from shape_materialize import unwritable_reason # noqa: E402 +# How a refusal names a path of the person's: as it is, unless a character in +# it is not printable, and then as `repr` spells it (#692). +from shape_materialize import shown_source # noqa: E402 NAMING_POLICY = SHAPE_ROOT / "contracts" / "repository-naming.yaml" MANIFEST = "family.yaml" @@ -1009,6 +1012,13 @@ def _checked_no_operation_in_progress(root: Path, verb: str) -> None: `load_yaml` makes git run before the refusal of a hand-edited `path:`, which this test holds to no git command at all: `test_family_member_path.py::test_the_refusal_is_made_before_any_git_call`. + + THE MARK'S PATH IS NAMED AS `shown_source` NAMES A PATH (#692): as it is, + and as Python's `repr` spells it when a character in it is not printable. + It is the true path, as git keeps it, so a holder whose path holds a line + break printed the refusal across two lines, with a line of the person's + choosing in it, and an ESC reached the terminal. A printable path is + named byte for byte. """ found = _operation_in_progress(root) if found is None: @@ -1029,7 +1039,8 @@ def _checked_no_operation_in_progress(root: Path, verb: str) -> None: raise Refusal( "family-operation-in-progress", f"{what} is unfinished in the family root: git keeps {mark} at " - f"{where.as_posix()}. `{verb}` makes its commit with `git commit -- " + f"{shown_source(where.as_posix())}. `{verb}` makes its commit with " + "`git commit -- " f"`, and {effect.format(verb=verb)}. No git command that " "writes was run and nothing was written.", end) @@ -1788,7 +1799,8 @@ def _refuse_a_taken_holder(folder: Path | None, holder: Path) -> None: why = target_in_the_way(holder) if why: raise Refusal( - "family-target-exists", f"{holder} already exists and {why}", + "family-target-exists", + f"{shown_source(holder)} already exists and {why}", target_remediation( why, "Remediation: choose another --into (or an empty " diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 351ab603..13fcb74e 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1346,15 +1346,21 @@ def folder_cannot_be_made(path: Path) -> str | None: write in (the system's own answer, `os.access`, where it has one; it says yes for root, and for any folder on Windows, as `mkdir` would). A `path` nothing stands at is asked of the folder above it. + + THE FOLDER IS NAMED AS `shown_source` NAMES A PATH (#692): as it is, and + as Python's `repr` spells it when a character in it is not printable. + Printed raw, an ESC in a folder's name reached the terminal and a line + break began a line of the person's choosing. A printable path is named + byte for byte, so `why.endswith(CANNOT_WRITE_IN)` still holds. """ for here in (path, *path.parents): if not os.path.lexists(here): continue why = _not_a_folder(here) if why: - return f"{here} {why}" + return f"{shown_source(here)} {why}" if not os.access(here, os.W_OK | os.X_OK): - return f"{here} {CANNOT_WRITE_IN}" + return f"{shown_source(here)} {CANNOT_WRITE_IN}" return None return None @@ -1382,11 +1388,13 @@ def refuse_unmakeable_folder(refusal: str, named: str, path: Path, or one above it) is not told to "move what stands in the way aside": it is told what `_unwritable_remediation` says, naming the flag `choose`, which is `named` unless it is no flag (`the family folder`, met by - `--into`). Every other answer keeps `FOLDER_REMEDIATION`.""" + `--into`). Every other answer keeps `FOLDER_REMEDIATION`. + + THE PATH IS NAMED AS `folder_cannot_be_made` NAMES IT (#692).""" why = folder_cannot_be_made(path) if why: raise Refusal( - refusal, f"{named} {path} cannot be made: {why}", + refusal, f"{named} {shown_source(path)} cannot be made: {why}", _unwritable_remediation(choose or named) if why.endswith(CANNOT_WRITE_IN) else FOLDER_REMEDIATION) @@ -1398,12 +1406,13 @@ def make_folder(refusal: str, named: str, path: Path) -> None: The real run's `mkdir`, after `refuse_unmakeable_folder` has said yes: what still fails here (a file made in the meantime, a permission `os.access` did not foresee) is the same refusal, before any remote is - made.""" + made. The path is named as `refuse_unmakeable_folder` names it (#692).""" try: path.mkdir(parents=True, exist_ok=True) except OSError as exc: - raise Refusal(refusal, f"{named} {path} cannot be made: " - f"{exc.strerror or exc}", MKDIR_REMEDIATION) from exc + raise Refusal(refusal, f"{named} {shown_source(path)} cannot be " + f"made: {exc.strerror or exc}", + MKDIR_REMEDIATION) from exc def target_in_the_way(target: Path) -> str | None: diff --git a/tests/test_dry_run_parity.py b/tests/test_dry_run_parity.py index 31170cb6..89cad33a 100644 --- a/tests/test_dry_run_parity.py +++ b/tests/test_dry_run_parity.py @@ -37,6 +37,15 @@ The GitHub route runs against a stub `gh` on PATH that answers `repo view` as the test says and refuses anything else, and is POSIX only, as every `#!` stub in this suite is. + +A FOLDER'S NAME IS SPELLED SAFE TO SHOW (#692, from #459's L2-77). Each +refusal above names a folder, and printed it as it was, so an ESC in a +path reached the terminal and a line break began a line of the person's +choosing, a forged `WARNING` or `REFUSED` among them. Each is named as +`shape_materialize.shown_source` names a `--pin-source`: as it is, unless a +character in it is not printable, and then as Python's `repr` spells it. The +rows whose names are `ESC-...` and `line-break-...` below hold such a name, +and a name like that is a POSIX file name only (`POSIX_NAMES`). """ from __future__ import annotations @@ -63,6 +72,17 @@ SCAFFOLD_FOLDER = "REFUSED scaffold-folder-cannot-be-made" FAMILY_FOLDER = "REFUSED family-folder-cannot-be-made" +#: A command line and a file name on Windows hold none of these. +POSIX_NAMES = pytest.mark.skipif( + os.name == "nt", + reason="needs a folder name holding a control character, which a " + "Windows file name cannot hold") + +#: Folder names that are not safe to show raw (#692): an ESC, which a +#: terminal acts on, and a line break then a line that is not the tool's. +ESC_NAME = "wd-\x1b[31mRED" +FORGED_NAME = "wd-\nWARNING forged-line: pwned" + def touch(path: Path) -> None: """A file at `path`, somebody's, made with every folder above it.""" @@ -132,6 +152,8 @@ def assert_refused(result, said: str) -> None: """Exit 2 with the refusal `said`, its whole line, and no traceback.""" assert result.returncode == 2, result.stdout + result.stderr assert refused_line(result) == said, result.stderr + assert not any(line.startswith("WARNING forged-line") + for line in result.stderr.splitlines()), result.stderr assert "Traceback" not in result.stderr, result.stderr assert DRY_RUN_DONE not in result.stdout, result.stdout @@ -183,6 +205,22 @@ def at(tmp_path: Path, relative: str) -> Path: return (tmp_path / relative).resolve() +def spelt(path: Path) -> str: + """A path that holds a control character as a refusal names it (#692): + as Python's `repr` spells it, quotes and `\\x1b` and `\\n` and all. It + is `repr`, and not `shown_source`, so a change to that function moves no + expectation with it.""" + return repr(str(path)) + + +def case_params(cases: dict) -> list: + """One `pytest.param` per row of `cases`, a row named `ESC-...` or + `line-break-...` skipped where no file can be named so (`POSIX_NAMES`).""" + return [pytest.param(name, marks=POSIX_NAMES + if name.startswith(("ESC-", "line-break-")) else ()) + for name in cases] + + #: The scaffold's four measured cases and two beside them: `(what the test #: puts in the way, the --work-dir and --local-remote-dir it passes, the #: REFUSED line both runs end on)`. On main 2989b4c each dry run exited 0; @@ -215,10 +253,40 @@ def at(tmp_path: Path, relative: str) -> Path: "up", ("wd", "up/r"), lambda t: f"{SCAFFOLD_FOLDER}: --local-remote-dir {at(t, 'up/r')} " f"cannot be made: {at(t, 'up')} is a file, not a folder"), + # #692: each folder a refusal names is spelled safe to show, whether the + # name is the folder's own or a folder above it: an ESC, and a line + # break then a forged line. There is no row for a --local-remote-dir + # like that: its clone URL is refused first, as `scaffold-unwritable- + # value`, and it names the path as `repr` spells it already. + "ESC-in-the-work-dir-name": ( + ESC_NAME, (ESC_NAME, "r"), + lambda t: f"{SCAFFOLD_FOLDER}: --work-dir {spelt(at(t, ESC_NAME))} " + f"cannot be made: {spelt(at(t, ESC_NAME))} is a file, not " + "a folder"), + "line-break-in-the-work-dir-name": ( + FORGED_NAME, (FORGED_NAME, "r"), + lambda t: f"{SCAFFOLD_FOLDER}: --work-dir " + f"{spelt(at(t, FORGED_NAME))} cannot be made: " + f"{spelt(at(t, FORGED_NAME))} is a file, not a folder"), + "ESC-in-a-folder-name-above-the-work-dir": ( + ESC_NAME, (f"{ESC_NAME}/wd", "r"), + lambda t: f"{SCAFFOLD_FOLDER}: --work-dir " + f"{spelt(at(t, f'{ESC_NAME}/wd'))} cannot be made: " + f"{spelt(at(t, ESC_NAME))} is a file, not a folder"), + "ESC-in-the-work-dir-name-holding-a-file-named-Atlas": ( + f"{ESC_NAME}/Atlas", (ESC_NAME, "r"), + lambda t: "REFUSED scaffold-target-exists: " + f"{spelt(at(t, f'{ESC_NAME}/Atlas'))} already exists and " + "is a file, not a folder"), + "line-break-in-the-work-dir-name-holding-a-file-named-Atlas": ( + f"{FORGED_NAME}/Atlas", (FORGED_NAME, "r"), + lambda t: "REFUSED scaffold-target-exists: " + f"{spelt(at(t, f'{FORGED_NAME}/Atlas'))} already exists " + "and is a file, not a folder"), } -@pytest.mark.parametrize("case", list(SCAFFOLD_CASES)) +@pytest.mark.parametrize("case", case_params(SCAFFOLD_CASES)) def test_a_scaffold_dry_run_refuses_what_its_real_run_refuses(tmp_path, case): """L2-34, the scaffold: on main every dry run here exited 0 and every real run but the non-empty tree's ended in a traceback, exit 1.""" @@ -265,10 +333,37 @@ def test_a_scaffold_dry_run_refuses_what_its_real_run_refuses(tmp_path, case): "r", (), lambda t: f"{FAMILY_FOLDER}: --local-remote-dir {at(t, 'r')} cannot " f"be made: {at(t, 'r')} is a file, not a folder"), + # #692, as the scaffold's rows beside them: an ESC, and a line break then + # a forged line, in a folder a refusal names or in one above it. + "ESC-in-the-work-dir-name": ( + ESC_NAME, ("--work-dir", ESC_NAME), + lambda t: f"{FAMILY_FOLDER}: --work-dir {spelt(at(t, ESC_NAME))} " + f"cannot be made: {spelt(at(t, ESC_NAME))} is a file, not " + "a folder"), + "line-break-in-the-work-dir-name": ( + FORGED_NAME, ("--work-dir", FORGED_NAME), + lambda t: f"{FAMILY_FOLDER}: --work-dir " + f"{spelt(at(t, FORGED_NAME))} cannot be made: " + f"{spelt(at(t, FORGED_NAME))} is a file, not a folder"), + "ESC-in-the-into-name": ( + ESC_NAME, ("--into", ESC_NAME), + lambda t: f"{FAMILY_FOLDER}: the family folder " + f"{spelt(at(t, f'{ESC_NAME}/Orbit'))} cannot be made: " + f"{spelt(at(t, ESC_NAME))} is a file, not a folder"), + "ESC-in-the-work-dir-name-holding-a-file-named-Orbit": ( + f"{ESC_NAME}/Orbit", ("--work-dir", ESC_NAME), + lambda t: "REFUSED family-target-exists: " + f"{spelt(at(t, f'{ESC_NAME}/Orbit'))} already exists and " + "is a file, not a folder"), + "line-break-in-the-work-dir-name-holding-a-file-named-Orbit": ( + f"{FORGED_NAME}/Orbit", ("--work-dir", FORGED_NAME), + lambda t: "REFUSED family-target-exists: " + f"{spelt(at(t, f'{FORGED_NAME}/Orbit'))} already exists " + "and is a file, not a folder"), } -@pytest.mark.parametrize("case", list(FAMILY_CASES)) +@pytest.mark.parametrize("case", case_params(FAMILY_CASES)) def test_an_init_dry_run_refuses_what_its_real_run_refuses(tmp_path, case): """L2-34, `init`: on main every dry run here exited 0, and every real run but the non-empty holder's ended in a traceback, exit 1.""" @@ -355,6 +450,18 @@ def test_a_folder_that_cannot_be_written_in_is_refused_by_both_runs( lambda t: f"{FAMILY_FOLDER}: --local-remote-dir {at(t, 'r')} cannot " f"be made: {at(t, 'r')} {CANNOT_WRITE_TEXT}", "--local-remote-dir"), + # #692: the folder the refusal says this user cannot write in is spelled + # safe to show too. + "ESC-in-the-scaffold-work-dir": ( + ESC_NAME, lambda t: scaffold_in(t, ESC_NAME), + lambda t: f"{SCAFFOLD_FOLDER}: --work-dir {spelt(at(t, ESC_NAME))} " + f"cannot be made: {spelt(at(t, ESC_NAME))} " + f"{CANNOT_WRITE_TEXT}", "--work-dir"), + "ESC-in-the-family-work-dir": ( + ESC_NAME, lambda t: init_in(t, "--work-dir", ESC_NAME), + lambda t: f"{FAMILY_FOLDER}: --work-dir {spelt(at(t, ESC_NAME))} " + f"cannot be made: {spelt(at(t, ESC_NAME))} " + f"{CANNOT_WRITE_TEXT}", "--work-dir"), } #: What every OTHER refusal of a folder says to do, as it always said it. @@ -365,7 +472,7 @@ def test_a_folder_that_cannot_be_written_in_is_refused_by_both_runs( @pytest.mark.parametrize("dry_run", [True, False], ids=["dry-run", "real-run"]) -@pytest.mark.parametrize("case", list(UNWRITABLE_FOLDER_AT)) +@pytest.mark.parametrize("case", case_params(UNWRITABLE_FOLDER_AT)) def test_an_unwritable_folder_a_run_makes_is_told_what_fits( tmp_path, unwritable_folder, case, dry_run): """rv622r's nit: an existing EMPTY `--work-dir`, `--local-remote-dir` or @@ -431,6 +538,60 @@ def test_the_folder_remediation_is_chosen_by_what_stands_in_the_way( assert refused.value.remediation == unwritable_remediation("--named") +# --- #692: a folder's name is spelled safe to show --------------------------- + +@POSIX_NAMES +@pytest.mark.parametrize("name", [ESC_NAME, FORGED_NAME], + ids=["ESC", "line-break"]) +def test_each_folder_question_spells_a_name_that_is_not_printable( + tmp_path, name): + """#692, one call each, beneath the CLI rows: `folder_cannot_be_made` + names the file that stands in the way, and `refuse_unmakeable_folder` + and `make_folder` name the folder they were asked for, as `repr` spells + it. On main each printed the path as it was, ESC or line break and all + (`make_folder` has no CLI row: the real run's own `mkdir` fails only for + what the question did not foresee).""" + stands = tmp_path / name + touch(stands) + below = stands / "sub" + why = f"{spelt(stands)} is a file, not a folder" + assert shape_materialize.folder_cannot_be_made(below) == why + with pytest.raises(Refusal) as refused: + shape_materialize.refuse_unmakeable_folder("x-code", "--named", below) + assert refused.value.detail == ( + f"--named {spelt(below)} cannot be made: {why}") + with pytest.raises(Refusal) as made: + shape_materialize.make_folder("x-code", "--named", below) + assert made.value.detail == ( + f"--named {spelt(below)} cannot be made: {mkdir_says(below)}") + + +@POSIX_NAMES +def test_a_folder_this_user_cannot_write_in_is_spelled_safe_to_show( + unwritable_folder): + """The other answer `folder_cannot_be_made` gives, `CANNOT_WRITE_IN`, + names its folder the same way (#692).""" + folder = unwritable_folder(ESC_NAME, CANNOT_WRITE["no-w"]) + assert (shape_materialize.folder_cannot_be_made(folder / "sub") + == f"{spelt(folder)} {shape_materialize.CANNOT_WRITE_IN}") + + +def test_a_printable_name_is_named_byte_for_byte(tmp_path): + """#692 changes no row a person could read before: a name with a space + or a letter that is not ASCII is printable, and is named as it is, with + no quotes.""" + stands = tmp_path / "wd \u00e9 and a space" + touch(stands) + assert stands.name.isprintable() + why = f"{stands} is a file, not a folder" + assert shape_materialize.folder_cannot_be_made(stands / "sub") == why + with pytest.raises(Refusal) as refused: + shape_materialize.refuse_unmakeable_folder( + "x-code", "--named", stands / "sub") + assert refused.value.detail == ( + f"--named {stands / 'sub'} cannot be made: {why}") + + # --- L2-60: an EMPTY working tree this user cannot write in ------------------ #: A folder this user cannot write in, two ways: with no `w` (nothing may be diff --git a/tests/test_family_refuse_up_front.py b/tests/test_family_refuse_up_front.py index 8258af96..6eb587b4 100644 --- a/tests/test_family_refuse_up_front.py +++ b/tests/test_family_refuse_up_front.py @@ -536,10 +536,13 @@ def test_a_merge_in_a_linked_worktree_is_found_where_git_keeps_it( @pytest.mark.skipif(sys.platform == "win32", - reason="Windows refuses a newline in a file name; Linux " - "and APFS take it") -def test_a_holder_path_with_a_newline_still_shows_its_merge( - family, holder, offline, tmp_path): + reason="Windows refuses a newline and an ESC in a file " + "name; Linux and APFS take them") +@pytest.mark.parametrize("name", ["Ink\nRouter", "Ink\x1b[31mRouter", + "Ink\nWARNING forged-line: pwned"], + ids=["line-break", "ESC", "forged-line"]) +def test_a_holder_path_with_a_control_character_still_shows_its_merge( + family, holder, offline, tmp_path, name): """V-F1 of #612's verification. A linked worktree's `git rev-parse --git-path` answers with an ABSOLUTE path through the holder's own `.git`, so a holder in a directory whose name holds a newline makes every @@ -547,9 +550,15 @@ def test_a_holder_path_with_a_newline_still_shows_its_merge( only that answer's FIRST line, which named a directory that does not exist: no mark was found, `add --dry-run` printed a plan, and the real `add` mounted and failed at the commit, #612's own defect back for that - path. The whole answer, less git's final newline, is the path.""" + path. The whole answer, less git's final newline, is the path. + + THE PATH IS NAMED AS `repr` SPELLS IT (#692, R612's round 2). It is the + true path, as git keeps it, and the refusal printed it as it was: across + two lines for a newline, with a forged line of the person's choosing for + `WARNING forged-line`, and an ESC reached the terminal. It is one line + now, with the path quoted and its control characters spelled out.""" env = isolated(offline, tmp_path) - broken = tmp_path / "Ink\nRouter" + broken = tmp_path / name shutil.copytree(holder, broken, symlinks=True) linked = tmp_path / "linked" raw(broken, env, "worktree", "add", "-q", "-b", "linked-branch", @@ -557,15 +566,18 @@ def test_a_holder_path_with_a_newline_still_shows_its_merge( begin("merge", linked, env, tmp_path) shown = raw(linked, env, "rev-parse", "--git-path", "MERGE_HEAD" ).stdout.decode() - assert shown.count("\n") == 2, f"premise: a newline in the path: {shown!r}" + where = shown[:-1] + assert name in where, f"premise: the name is in the path: {shown!r}" for extra in ((), ("--dry-run",)): ran = add(linked, env, family["remotes"], *extra) assert ran.returncode == 2, (extra, ran.stdout, ran.stderr) assert ran.stdout == "" - assert ran.stderr.startswith( + line, _ = refusal(ran.stderr) + assert line.startswith( "REFUSED family-operation-in-progress: a merge is unfinished in " "the family root: git keeps MERGE_HEAD at "), ran.stderr - assert "Ink\nRouter/.git/worktrees/" in ran.stderr, ran.stderr + assert f"git keeps MERGE_HEAD at {where!r}. `add` makes " in line + assert "\x1b" not in ran.stderr, ran.stderr def test_with_no_act_and_no_sparse_checkout_each_verb_runs_as_it_always_did( From ab55c29f525b3867126b8d238fc559b78b99f4b3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 17:08:04 +0000 Subject: [PATCH 04/10] scaffold-project.py and family.py: `shown_source` joins the import block each file already has for the folder questions, so the next import another change adds after the block does not meet it (#692) The first commit imported `shown_source` on a line of its own after each block. Lane 4's #660, #688 and #693 add an import of their own at the same place, which `merge-tree` reports as add/add conflicts in `scripts/family.py` and `scaffold-project.py`. The name goes into the existing `from shape_materialize import (...)` block instead, in alphabetical order. No line of behaviour changes. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scaffold-project.py | 5 +---- scripts/family.py | 7 ++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/scaffold-project.py b/scaffold-project.py index 29eae142..5dd2664a 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -122,11 +122,8 @@ # not-found answer as the one failure of `gh repo view` that means absent. from shape_materialize import ( # noqa: E402 make_folder, refuse_unless_not_found, refuse_unmakeable_folder, - target_in_the_way, target_remediation, + shown_source, target_in_the_way, target_remediation, ) -# How a refusal names a folder: as it is, unless a character in it is not -# printable, and then as `repr` spells it (#692). -from shape_materialize import shown_source # noqa: E402 #: How a refusal names each value the manifest records (#258, #261). The leg #: paths are not here: `refuse_bad_leg_paths` asks for them (#179, #225). diff --git a/scripts/family.py b/scripts/family.py index 389db20a..ad196428 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -177,8 +177,8 @@ commit_trailers, env_commit, escape_what_stdout_cannot_encode, force_stage, inherited_child_environment, make_folder, materialize_family_root, refuse_bad_branch, refuse_unless_not_found, - refuse_unmakeable_folder, run, stage_written, target_in_the_way, - target_remediation, trailer_line, write_lf, + refuse_unmakeable_folder, run, shown_source, stage_written, + target_in_the_way, target_remediation, trailer_line, write_lf, ) # What `init` asks of the holder's own reader and validator before it creates # anything (#307), imported as its own unit and not added to the lists above. @@ -195,9 +195,6 @@ # holder's clone URL under `--local-remote-dir` (#255, #433). It is the # question the scaffold and `adopt-project.py` ask of theirs. from shape_materialize import unwritable_reason # noqa: E402 -# How a refusal names a path of the person's: as it is, unless a character in -# it is not printable, and then as `repr` spells it (#692). -from shape_materialize import shown_source # noqa: E402 NAMING_POLICY = SHAPE_ROOT / "contracts" / "repository-naming.yaml" MANIFEST = "family.yaml" From 326de720bc67da7f6a059852d3da9e01431855fd Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 17:21:38 +0000 Subject: [PATCH 05/10] bump-leg.py: the root's branch is read as its full ref and pushed as `refs/heads/:refs/heads/`, `bump-leg-root-dirty` spells each name, `committed` lists what git holds, and the dry run says it runs no validator, so a root on `+x` or beside a tag `main` is no longer told to force-push or push to the default branch (#672) Five lines of #672, each measured on 35133f29 and gone on this head: - The branch: `current_branch` read `git symbolic-ref --short HEAD`, which prints `heads/` beside a tag of the same name. With a tag `main` the tracking-branch refusal was not given, the bump committed on `main` and printed `push -u origin heads/main`; a HEAD on `refs/tags/x` committed through the tag and moved it. It reads the full ref now, and only a ref under `refs/heads/` is a branch. - The push: the NEXT line ended `push -u origin `, a refspec, so a root on `+x` was told `push -u origin +x`, which force-pushed a local `x` over origin's and never pushed `+x`; on `+main`, pasted, it rewound origin's `main`. It prints `refs/heads/:refs/heads/`, the form lane 4's #689 (`family.py`) and #674 (`update-shape.py`) print, which pushes the branch unforced, beside a tag of its name too, and `-u` still records `origin/`. - `bump-leg-root-dirty` printed C1 controls and bidi overrides raw under `core.quotePath=false`. Each line is `repo_shape.spelled` now, split on git's newlines only, so U+0085 and U+2028 stay on their line, named. - `committed :` listed the paths staged; it lists the commit's paths from `git diff-tree --ignore-submodules=none`. Where the gitlink already records the new commit, the `commit` line and the message name the pin's own `commit:` as the old one, not `X -> X`, and say the gitlink does not move. A failure to read the paths after the commit is a sentence, never a refusal over a bump that landed. - `--dry-run` runs no validator; its closing paragraph says so, and that the real run can still end `bump-leg-validators-red`, rolled back. Also: the `BRANCH_LEFT` comment names its four copies (rv647's nit 3). The ride-along (`run_validator`'s environment) is left: no verdict differs, and the caller's `PYTHONPATH` gives the more precise finding. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 143 ++++++- tests/test_bump_leg.py | 10 +- tests/test_bump_leg_branch_and_push_line.py | 425 ++++++++++++++++++++ tests/test_bump_leg_paste_lines.py | 21 +- 4 files changed, 569 insertions(+), 30 deletions(-) create mode 100644 tests/test_bump_leg_branch_and_push_line.py diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index b4e8e2b4..63b476ce 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -121,12 +121,12 @@ sys.path.insert(0, str(SHAPE_ROOT / "scripts")) from repo_shape import ( # noqa: E402 COMMIT_RE, SAFE_PATH_RE, Refusal, checked_value, git_out, load_yaml, - recorded_gitlink, repo_basename, tree_digest, + recorded_gitlink, repo_basename, spelled, tree_digest, ) from shape_materialize import ( # noqa: E402 CommandFailed, captured_child_environment, check_program, commit_trailers, - escape_what_stdout_cannot_encode, is_nested_key, rewrite_fault, run, - trailer_line, with_scalar, write_lf, + escape_what_stdout_cannot_encode, is_nested_key, pin_values_in, + rewrite_fault, run, trailer_line, with_scalar, write_lf, ) MANIFEST = "project.yaml" @@ -1067,11 +1067,29 @@ def commit_once(root: Path, message: str, paths: list[str], return git_out(["rev-parse", "HEAD"], cwd=root).lower() +#: Where git keeps a branch: the root's HEAD names `refs/heads/`. +HEADS = "refs/heads/" + + def current_branch(root: Path) -> str | None: - proc = subprocess.run(["git", "symbolic-ref", "--quiet", "--short", "HEAD"], + """The branch the root's HEAD is on, or None: detached, or a HEAD that + names a ref outside `refs/heads/`, which is no branch. + + THE FULL REF, NOT `--short` (#672). `symbolic-ref --short` gives the + shortest name that is not ambiguous, so beside a tag of the same name it + printed `heads/`. Measured with a tag `main` beside the tracking + branch `main`: the compare with `tracking_branch` read `heads/main` + against `main`, `bump-leg-on-tracking-branch` was not given, and the bump + committed on the default branch and printed a push to it. The name after + `refs/heads/` is the branch's own, whatever tags there are. + """ + proc = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], cwd=str(root), capture_output=True, text=True, encoding="utf-8", errors="replace", check=False) - return proc.stdout.strip() or None + ref = proc.stdout.strip() + if not ref.startswith(HEADS): + return None + return ref[len(HEADS):] or None # --------------------------------------------------------------------------- @@ -1123,11 +1141,17 @@ def _ensure_root_clean(root: Path) -> None: dirt = git_out(["status", "--porcelain", "--untracked-files=no", "--ignore-submodules=untracked"], cwd=root, binary=True) if dirt: - shown = dirt.decode("utf-8", "backslashreplace").strip() + # EACH LINE `spelled` (#672), as `bootstrap.py` spells what it read + # from the disk: under `core.quotePath=false` git prints a name as + # the bytes it holds, so a C1 control or a bidi override in a file + # name reached the terminal raw. Split on git's own newlines and no + # others: `splitlines` also breaks at U+0085 and U+2028, which are + # `spelled`'s to show, and `strip()` took them off the last name. + shown = dirt.decode("utf-8", "backslashreplace").strip(" \n") raise Refusal( "bump-leg-root-dirty", f"{root} has uncommitted changes:\n" - + "\n".join(f" {line}" for line in shown.splitlines()), + + "\n".join(f" {spelled(line)}" for line in shown.split("\n")), "Remediation: commit or stash them first. A lockstep commit that " "cannot be read as exactly one change is the thing the rule " "exists to keep readable.") @@ -1160,9 +1184,10 @@ def _switch_command(root: Path, role: str, commit: str) -> str: #: stands now; going back to the old branch is right only where it holds work #: not merged yet, and `git switch` keeps its commits. IN WORDS, NOT A SECOND #: COMMAND, as `family.py` and the holder's `validate-family.py` say it: no -#: line of ours changes, so none needs quoting, and the three tools follow one -#: rule. THE SAME SENTENCE as their `BRANCH_LEFT` (#607), word for word, with -#: "the project" where they say "the holder". +#: line of ours changes, so none needs quoting. FOUR COPIES OF ONE RULE: the +#: `BRANCH_LEFT` of `family.py`, of the holder's `validate-family.py` (#607) +#: and of `setup-project.py` (#647) is this sentence word for word, with +#: "the holder" where this one says "the project". BRANCH_LEFT = ("If `git switch -c` says that branch already exists, an " "earlier act left it: give the branch another name, which " "starts it where the project stands, or, only if that branch " @@ -1611,11 +1636,69 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, raise +def _moved_from(was: str, commit: str, pin_text: str) -> str: + """The commit a bump moves the leg FROM, as the report's `commit` line + and the commit message name it (#672): the gitlink's, or, where the + gitlink already records `commit`, the pin's own `commit:`. + + A person who moved the gitlink by hand and committed it left the pin + behind, and the bump is what brings the pin to it. `was` is the + gitlink's, so both lines read `X -> X` for a commit that moved the pin + from another one. The pin's value is read as the validators read it, + from the text `rewrite_pin` was given; one that does not read, or is + not a commit, leaves the gitlink's, and the `gitlink` line under it says + which one did not move. + """ + if was != commit: + return was + try: + pinned = pin_values_in(pin_text)[0].strip().lower() + except ValueError: + return was + return pinned if COMMIT_RE.match(pinned) else was + + +def _committed_paths(root: Path, head: str) -> str: + """The paths the commit `head` changes, as git records them, for the + `committed` line (#672), or a sentence saying they could not be read. + + FROM GIT, NOT FROM THE LIST STAGED. The leg's path is staged whether or + not its gitlink moves, so where it already recorded the new commit the + line named `spec` beside the pin while the commit held the pin alone. + `diff-tree` is plumbing: it reads no `diff.ignoreSubmodules` (measured, + `git show` under `=all` drops the leg), and `--ignore-submodules=none` + overrides a `submodule..ignore` (measured, which `diff-tree` does + honour). NUL-separated, so a name is the bytes git holds, and each is + `spelled`, as the dirty-root list is. + + AFTER THE COMMIT, NOTHING HERE MAY REFUSE: the bump has landed, and a + `git-failed` exit 2 would say it had not. + """ + try: + raw = git_out(["diff-tree", "--no-commit-id", "--name-only", "-r", + "-z", "--ignore-submodules=none", head], cwd=root, + binary=True) + except (Refusal, OSError) as exc: + why = " ".join((exc.detail if isinstance(exc, Refusal) + else str(exc)).split()) + return (f"its paths could not be read back ({why}); `git show " + f"--name-only {head[:12]}` lists them") + return ", ".join(spelled(name.decode("utf-8", "backslashreplace")) + for name in raw.split(b"\0") if name) + + def cmd_bump(args) -> int: """Advance one leg. The ORDER is what makes `--dry-run` the real run's answer (#534, #438): everything that can REFUSE the move is asked before the dry-run return, so the dry run and the real run refuse alike and the - only thing the dry run skips is the writing. + only thing the dry run skips is the writing — and the project's own + validators, which judge the rewritten tree and so run only once it is + written. A pin they would refuse as it stands is previewed, exit 0, and + the real run ends `bump-leg-validators-red`, rolled back; the dry run's + closing paragraph says so (#672). Running them over the tree as it + stands would put `validate-pins.py`'s `gh api` call (#411) into a command + that promises to fetch into the leg and write nothing else, and would see + only what is red already. 1. the root: clean, on a branch, the leg initialized and recorded; 2. the fetch, and the commit `--to` names in the leg's objects (an @@ -1661,7 +1744,14 @@ def cmd_bump(args) -> int: pin_text = _leg_pin_text(root, pin_path) plans = plan_workflows(root, repository, commit) - print(f"commit {was[:12]} -> {commit[:12]}") + # `old`, NOT `was`, IS WHAT THE BUMP MOVES FROM (#672), on this line and + # in the commit message; `was` is the gitlink's, which the rollback + # restores. They differ only where the gitlink already records `commit`. + old = _moved_from(was, commit, pin_text) + print(f"commit {old[:12]} -> {commit[:12]}") + if was == commit: + print(f"gitlink {path} already records {commit[:12]}, so this " + "does not move it") print(f"tree {digest[:12]}… ({digest})") print(f"pin {pin_rel}") _print_workflow_plan(root, plans) @@ -1688,7 +1778,10 @@ def cmd_bump(args) -> int: print("\n--dry-run: nothing was changed. The leg's object store was " "fetched into, which is how the digest above was computed; the " "working tree, the pin, the workflows and the index are " - "untouched.") + "untouched. A dry run does not run the project's own " + f"validators ({' and '.join(VALIDATORS)}): the real run runs " + "them after the rewrite, and can still end " + "`bump-leg-validators-red`, with every byte rolled back.") if branch == project.tracking: # LAST, after everything a dry run on a branch prints, so that # line and the sentence under it are the whole of the difference @@ -1705,12 +1798,16 @@ def cmd_bump(args) -> int: staged: list[str] = [path, pin_rel] + [ wf_path.relative_to(root).as_posix() for wf_path, _, _ in plans] body = ( - f"{was[:12]} -> {commit[:12]}, tree {digest}.\n\n" + f"{old[:12]} -> {commit[:12]}, tree {digest}.\n\n" + + (f"The gitlink at {path} already recorded {commit[:12]}, so this " + "commit does not move it.\n\n" if was == commit else "") + (f"{sum(count for _, _, count in plans)} workflow `@` " f"reference(s) in {len(plans)} file(s) moved with it.\n\n" if plans else - "No workflow reference names this leg, so the gitlink and the pin " - "file are the whole of it.\n\n") + "No workflow reference names this leg, so " + + ("the pin file is" if was == commit + else "the gitlink and the pin file are") + + " the whole of it.\n\n") + f"The gitlink at {path}, `commit:` and `digests.tree_sha256` in " f"{pin_rel}, and every workflow reference to {repository} move in " "ONE commit because `scripts/validate-pins.py` refuses when they " @@ -1722,15 +1819,23 @@ def cmd_bump(args) -> int: root, submodule, commit, was, pin_path, new_pin, plans, staged, f"Bump {role} leg to {commit[:12]} in {project.display_name}\n\n" + body, args.trailer) - print(f"\n committed {head[:12]}: " + ", ".join(staged)) + print(f"\n committed {head[:12]}: {_committed_paths(root, head)}") print(f" {path}/ is left DETACHED at {commit[:12]}; `make bootstrap` in " "the root puts it on its tracking branch AT the new pin, or says " "why not") # QUOTED WHOLE (#532), the root and the branch: a root under `My # Projects/` pasted as two arguments, and the line is the one this run - # ends on, the one a person runs next. + # ends on, the one a person runs next. THE BRANCH BY ITS FULL REFNAME, + # ON BOTH SIDES (#672): a bare `` is a refspec, and git reads a + # leading `+` as FORCE, so a root on `+x` was told `push -u origin +x`, + # which pushed a local `x` over origin's, forced, and not `+x` at all. + # `refs/heads/:refs/heads/` is that branch and nothing + # else, beside a tag of its name too, and `-u` still records + # `origin/` as its upstream. It is the form #689 and #674 give + # `family.py`'s and `update-shape.py`'s push lines: one for the three. print("\nNEXT " + shlex.join(["git", "-C", str(root), "push", "-u", - "origin", branch])) + "origin", + f"{HEADS}{branch}:{HEADS}{branch}"])) print(f" then open a pull request against {project.tracking}; never " "push the bump to the default branch directly") return 0 diff --git a/tests/test_bump_leg.py b/tests/test_bump_leg.py index 031d7075..1ddf07cc 100644 --- a/tests/test_bump_leg.py +++ b/tests/test_bump_leg.py @@ -232,10 +232,14 @@ def test_the_message_and_the_next_step_are_a_pull_request(project, scaffolded, assert tree_digest(project / "spec", advanced) in body assert "validate-pins.py" in body - assert f"push -u origin {WORKING_BRANCH}" in result.stdout + # THE BRANCH BY ITS FULL REFNAME (#672): a bare name is a refspec. + pushed = f"refs/heads/{WORKING_BRANCH}" + assert f"push -u origin {pushed}:{pushed}" in result.stdout assert "pull request" in result.stdout - assert "push -u origin main" not in result.stdout, ( - "never a direct push to the default branch") + for default in ("main", "refs/heads/main"): + assert f"push -u origin {default}" not in result.stdout, ( + "never a direct push to the default branch") + assert f":{default}" not in result.stdout def test_bumping_to_the_commit_already_pinned_does_nothing(project, scaffolded): diff --git a/tests/test_bump_leg_branch_and_push_line.py b/tests/test_bump_leg_branch_and_push_line.py new file mode 100644 index 00000000..ce8e4376 --- /dev/null +++ b/tests/test_bump_leg_branch_and_push_line.py @@ -0,0 +1,425 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`scripts/bump-leg.py` reads the root's branch the way git spells it, ends +on a push no branch name can bend, spells what a dirty root lists, names +what its commit holds, and says what a dry run does not run (#672). + +FIVE LINES, EACH MEASURED ON MAIN BEFORE THE FIX: + + * THE BRANCH. `current_branch` asked `git symbolic-ref --short HEAD`, which + gives the shortest name that is not ambiguous: beside a tag `main` it + printed `heads/main`, the compare with the tracking branch read that + against `main`, and the bump COMMITTED ON THE DEFAULT BRANCH and printed + `push -u origin heads/main`. A HEAD on `refs/tags/` read as ``, and + the bump moved the tag. The full ref is read now, and only a ref under + `refs/heads/` is a branch. + * THE PUSH. The NEXT line ended `push -u origin `, and a bare + `` is a refspec: a leading `+` is FORCE. A root on `+x` was told + `push -u origin +x`, which pushed a local `x` over origin's `x`, forced, + and never pushed `+x`; on `+main` it rewinds origin's `main`. It is + `refs/heads/:refs/heads/` now, the branch and nothing + else on both sides, the form #689 and #674 give `family.py`'s and + `update-shape.py`'s push lines. + * THE DIRTY ROOT. Under `core.quotePath=false` git prints a name as the + bytes it holds, and `bump-leg-root-dirty` printed a C1 control and a + bidi override raw. Each line is `repo_shape.spelled` now, split on git's + own newlines, so U+0085 and U+2028 stay on their line and are named. + * THE COMMIT. `committed :` listed the paths STAGED, and the leg is + staged whether or not its gitlink moves: over a gitlink a person had + moved by hand it named `spec` beside the pin, and the report and the + message said `X -> X`. The paths are read from git now, and the old + commit is the pin's own `commit:` where the gitlink already holds it. + * THE DRY RUN. It never runs the project's own validators, so over a pin + they already refuse it exits 0 and the real run ends + `bump-leg-validators-red`. Its closing paragraph says so. + +NO NETWORK, NO `gh`: the bare remotes are the session's own, and every root +here pushes to a private copy of its remote (`make_root`), so a push reaches +nothing another test reads. Every printed line is run as the argv a POSIX +shell reads out of it, without a shell, so these hold on every OS. +""" + +from __future__ import annotations + +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import FILE_PROTOCOL, PROJECT, REPO, cloned_just_pushed, git +# A fixture is used by being named in this module's namespace, which is what +# the imports of `bump_leg_module` and `make_root` are for. +from test_bump_leg import ( # noqa: F401 + branched, bump, bump_leg_module, committed, reference_both_legs, validate, +) +from test_bump_leg_paste_lines import make_root, next_line, words # noqa: F401 + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import spelled # noqa: E402 + +#: Where the advanced spec commit is pushed. Not another module's branch: the +#: bare remotes are session-scoped, and a second, different commit pushed to +#: a name already taken is refused as non-fast-forward. +ADVANCE_BRANCH = "bump-leg-branch-and-push-line-advance" + +SPEC_PIN = "contracts/spec-pin.yaml" + +#: The sentence the dry run's closing paragraph ends with. +NOT_VALIDATED = ( + "A dry run does not run the project's own validators " + "(scripts/validate-pins.py and scripts/validate-manifest.py): the real " + "run runs them after the rewrite, and can still end " + "`bump-leg-validators-red`, with every byte rolled back.") + + +@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.""" + remote = scaffolded["remotes"] / f"{PROJECT}-spec.git" + work = cloned_just_pushed( + remote, tmp_path_factory.mktemp("spec-advance-push-line") / "leg") + (work / "PUSH-LINE.md").write_text("# the push line\n", encoding="utf-8") + git("add", "--", "PUSH-LINE.md", cwd=work) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Advance the spec leg for the push-line tests", cwd=work) + git("push", "-q", "origin", f"HEAD:refs/heads/{ADVANCE_BRANCH}", cwd=work) + return git("rev-parse", "HEAD", cwd=work).stdout.strip() + + +def head_of(repo: Path, ref: str = "HEAD") -> str: + return git("rev-parse", "--verify", "-q", ref, cwd=repo, + check=False).stdout.strip() + + +def commit_empty(root: Path, message: str) -> str: + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "--allow-empty", "-m", message, cwd=root) + return head_of(root) + + +def lines_of(result) -> list[str]: + return result.stdout.splitlines() + + +# --- the branch, read in full ------------------------------------------------ + + +def test_a_tag_named_like_the_tracking_branch_does_not_let_the_bump_onto_it( + project, scaffolded, advanced): + """THE ISSUE'S MEASUREMENT: a tag `main` beside the tracking branch + `main`. `--short` read `heads/main`, the refusal was not given, the bump + committed on the default branch and printed a push to it. Refused now, + in the words a root on `main` with no such tag is given, and the dry run + names the tracking branch as itself.""" + git("tag", "main", cwd=project) + head = head_of(project) + + real = bump(project, scaffolded, advanced) + assert real.returncode == 2, real.stderr + real.stdout + assert "REFUSED bump-leg-on-tracking-branch" in real.stderr, real.stderr + assert "is on 'main', which project.yaml declares" in real.stderr + assert real.stdout == "" + assert head_of(project) == head, "nothing was committed on main" + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert f"{project.resolve()} is on 'main', the tracking branch." in \ + dry.stdout, dry.stdout + assert "heads/main" not in dry.stdout + + +def test_a_head_on_a_tag_ref_is_no_branch_to_bump_on(project, scaffolded, + advanced): + """`symbolic-ref` lets HEAD name `refs/tags/`, and `--short` read it + as the branch ``: the bump committed THROUGH it, which moved the tag, + and printed `push -u origin x`. A ref outside `refs/heads/` is no + branch: refused as a root on no branch is, with the tag where it was.""" + git("tag", "x", cwd=project) + tagged = head_of(project, "refs/tags/x") + git("symbolic-ref", "HEAD", "refs/tags/x", cwd=project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED bump-leg-root-detached" in result.stderr, result.stderr + assert result.stdout == "" + assert head_of(project, "refs/tags/x") == tagged, "the tag did not move" + + +# --- the push, by its full ref ----------------------------------------------- + + +def push_argv(root: Path, branch: str) -> list[str]: + """The NEXT line's argv: the branch by its full refname on both sides, + the form #689 and #674 give `family.py`'s and `update-shape.py`'s.""" + return ["git", "-C", str(root.resolve()), "push", "-u", "origin", + f"refs/heads/{branch}:refs/heads/{branch}"] + + +def run_the_line(line: str, cwd: Path) -> subprocess.CompletedProcess: + """The NEXT line as the argv a POSIX shell reads out of it, run without + a shell, from a directory that is not the root's.""" + return subprocess.run(words(line), cwd=str(cwd), capture_output=True, + text=True, encoding="utf-8", errors="replace", + check=False) + + +def upstream_of(root: Path, branch: str) -> str: + return git("for-each-ref", "--format=%(upstream)", + f"refs/heads/{branch}", cwd=root).stdout.strip() + + +@pytest.mark.parametrize("name", ["+x", "+main"]) +def test_a_branch_named_with_a_leading_plus_is_pushed_and_nothing_is_forced( + make_root, scaffolded, advanced, name): + """THE ISSUE'S MEASUREMENT, and its worst case. Origin's `` + has moved on from the local branch of that name. The bare line read + `+` as the FORCED refspec ``: origin's `x` was + overwritten (`forced update`), and on `+main` origin's DEFAULT BRANCH was + rewound, while `+` itself was never pushed. By its full refname + the line pushes `+`, as a new branch, upstream recorded, and + leaves the other where it was.""" + root, remote = make_root("plain") + other = name[1:] + start = head_of(root) + git("checkout", "-q", "-b", "w672-elsewhere", cwd=root) + moved_on = commit_empty(root, f"origin's {other} moves on") + git("push", "-q", "origin", f"HEAD:refs/heads/{other}", cwd=root) + git("checkout", "-q", "main", cwd=root) + if other != "main": + git("branch", other, start, cwd=root) + git("checkout", "-q", "-b", name, cwd=root) + + result = bump(root, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + line = next_line(result.stdout) + assert words(line) == push_argv(root, name) + + pushed = run_the_line(line, root.parent) + assert pushed.returncode == 0, pushed.stderr + pushed.stdout + assert "forced update" not in pushed.stderr, pushed.stderr + assert head_of(remote, f"refs/heads/{name}") == head_of(root) + assert head_of(remote, f"refs/heads/{other}") == moved_on, ( + f"origin's {other} was not touched") + assert upstream_of(root, name) == f"refs/remotes/origin/{name}" + + +def test_a_branch_beside_a_tag_of_its_name_is_pushed_by_its_full_ref( + make_root, scaffolded, advanced): + """A branch `v1` beside a tag `v1`: `--short` read `heads/v1`, and the + line said `push -u origin heads/v1` (git's own bare `v1` there is `src + refspec v1 matches more than one`). The line names `refs/heads/v1` on + both sides, and run, it pushes the branch and records its upstream.""" + root, remote = make_root("plain") + git("tag", "v1", cwd=root) + git("checkout", "-q", "-b", "v1", cwd=root) + + result = bump(root, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + line = next_line(result.stdout) + assert words(line) == push_argv(root, "v1") + + pushed = run_the_line(line, root.parent) + assert pushed.returncode == 0, pushed.stderr + pushed.stdout + assert head_of(remote, "refs/heads/v1") == head_of(root, "refs/heads/v1") + assert upstream_of(root, "v1") == "refs/remotes/origin/v1" + + +# --- the dirty root, spelled ------------------------------------------------- + +#: `(a name, as `bump-leg-root-dirty` lists it under core.quotePath=false)`. +SPELLED = [ + pytest.param("caf\u009b\u202eevil.yml", "caf\\x9b\\u202eevil.yml", + id="c1-and-bidi"), + pytest.param("a\u2028b.yml", "a\\u2028b.yml", id="line-separator"), + pytest.param("c\u0085d.yml", "c\\x85d.yml", id="next-line"), + pytest.param("end\u2028", "end\\u2028", id="ends-in-a-separator"), +] + + +def raw_controls(name: str, text: str) -> list[str]: + """The characters of `name` that `spelled` escapes and that `text` still + holds as they are: what a terminal must not be handed raw.""" + return [char for char in name if spelled(char) != char and char in text] + + +def dirty_with(project: Path, name: str) -> None: + """A tracked workflow file `name`, committed and then edited, under + `core.quotePath=false`, on a bump branch; or skip where the disk cannot + hold the name.""" + branched(project) + path = project / ".github" / "workflows" / name + try: + path.write_bytes(b"on: [push]\n") + except (OSError, UnicodeError) as exc: + pytest.skip(f"this filesystem cannot hold the name: {exc}") + git("add", "--", f".github/workflows/{name}", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A workflow whose name holds a control", cwd=project) + path.write_bytes(path.read_bytes() + b"# edited\n") + git("config", "core.quotePath", "false", cwd=project) + + +@pytest.mark.parametrize("extra", [(), ("--dry-run",)], + ids=["bump", "dry-run"]) +@pytest.mark.parametrize("name, shown", SPELLED) +def test_a_dirty_name_holding_controls_is_listed_spelled( + project, scaffolded, advanced, name, shown, extra): + """THE ISSUE'S MEASUREMENT (`cafevil.yml`), and the two + characters `splitlines` breaks a line at: the name is on ONE line of its + own, every control written as its escape, and none of them raw.""" + dirty_with(project, name) + result = bump(project, scaffolded, advanced, *extra) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED bump-leg-root-dirty" in result.stderr, result.stderr + assert f" M .github/workflows/{shown}" in result.stderr.split("\n"), ( + result.stderr) + assert raw_controls(name, result.stderr) == [], ascii(result.stderr) + + +def test_under_the_default_quoting_git_names_the_file_itself( + project, scaffolded, advanced): + """THE CONTROL: with `core.quotePath` as git ships it, git quotes the + bytes itself, and the line is git's, unchanged.""" + dirty_with(project, "caf\u009b\u202eevil.yml") + git("config", "--unset", "core.quotePath", cwd=project) + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert (' M ".github/workflows/caf\\302\\233\\342\\200\\256evil.yml"' + in result.stderr.splitlines()), result.stderr + + +# --- the commit, as git holds it --------------------------------------------- + + +def body_of(project: Path) -> str: + return git("log", "-1", "--format=%b", cwd=project).stdout + + +@pytest.mark.parametrize("setting", [None, "submodule.spec.ignore", + "diff.ignoreSubmodules"]) +def test_the_committed_line_names_the_paths_git_holds( + project, scaffolded, advanced, setting): + """The paths of the commit, as git lists them, in git's order: a + workflow, the pin and the gitlink. With an `ignore = all` the person set + for the leg, `git show` drops the gitlink from its list, and the line + still names it, because the commit holds it.""" + reference_both_legs(project) + branched(project) + if setting: + git("config", setting, "all", cwd=project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + held = git("show", "--ignore-submodules=none", "--name-only", + "--format=", "HEAD", cwd=project).stdout.split() + assert held == [".github/workflows/legs.yml", SPEC_PIN, "spec"] + assert f" committed {head_of(project)[:12]}: {', '.join(held)}" in \ + lines_of(result), result.stdout + assert not any(line.startswith("gitlink ") for line in lines_of(result)) + + +def test_a_gitlink_moved_by_hand_is_reported_as_the_pins_move( + project, scaffolded, advanced): + """THE ISSUE'S MEASUREMENT (AT-scaffold N9): a person moved the gitlink to + the new commit by hand and committed it; the pin still names the old + one. The report, the dry run's included, and the commit message said + `X -> X`, and `committed` named `spec` beside the pin, which the commit + does not hold. The old commit is the pin's own now, a line says the + gitlink does not move, and the commit's paths are git's.""" + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + branched(project) + leg = project / "spec" + git(*FILE_PROTOCOL, "fetch", "-q", + str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), ADVANCE_BRANCH, + cwd=leg) + git("checkout", "-q", "--detach", advanced, cwd=leg) + git("add", "--", "spec", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Move the gitlink by hand", cwd=project) + moved = f"commit {pinned[:12]} -> {advanced[:12]}" + held = (f"gitlink spec already records {advanced[:12]}, so this " + "does not move it") + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert moved in lines_of(dry), dry.stdout + assert held in lines_of(dry), dry.stdout + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert moved in lines_of(result), result.stdout + assert held in lines_of(result), result.stdout + assert f"{advanced[:12]} -> {advanced[:12]}" not in result.stdout + assert committed(project) == {SPEC_PIN} + assert f" committed {head_of(project)[:12]}: {SPEC_PIN}" in \ + lines_of(result), result.stdout + body = body_of(project) + assert body.startswith(f"{pinned[:12]} -> {advanced[:12]}, tree "), body + assert (f"The gitlink at spec already recorded {advanced[:12]}, so this " + "commit does not move it.") in body, body + assert "so the pin file is the whole of it." in body, body + assert validate(project).returncode == 0, "the project's own gate agrees" + + +def test_a_committed_line_git_cannot_read_back_still_says_the_bump_landed( + project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): + """THE LINE IS READ AFTER THE COMMIT, so nothing in it may refuse: a + `git-failed` there would be exit 2 over a bump that landed. The line + names the commit and says where its paths are, and the run ends on its + NEXT, exit 0.""" + branched(project) + before = head_of(project) + real_git_out = bump_leg_module.git_out + + def no_diff_tree(args, *rest, **kwargs): + if args[:1] == ["diff-tree"]: + raise bump_leg_module.Refusal( + "git-failed", "`git diff-tree` exited 128: fatal: bad object") + return real_git_out(args, *rest, **kwargs) + + monkeypatch.setattr(bump_leg_module, "git_out", no_diff_tree) + code = bump_leg_module.main([ + "--root", str(project), "--leg", "spec", "--to", advanced, + "--local-remote-dir", str(scaffolded["remotes"])]) + out = capsys.readouterr().out + + head = head_of(project) + assert code == 0, out + assert git("rev-parse", f"{head}~1", cwd=project).stdout.strip() == before + assert (f" committed {head[:12]}: its paths could not be read back " + "(`git diff-tree` exited 128: fatal: bad object); `git show " + f"--name-only {head[:12]}` lists them") in out.splitlines(), out + assert next_line(out) == shlex.join(push_argv(project, "bump/spec")) + + +# --- the dry run, and what it does not run ----------------------------------- + + +def test_a_dry_run_says_the_validators_are_the_real_runs(project, scaffolded, + advanced): + """THE ISSUE'S MEASUREMENT: a pin `validate-pins.py` already refuses + (`yaml-unparsable`, a tab-indented line after a `note:`), committed. The + dry run previews the move, exit 0, as before, and now says that it runs + no validator and that the real run can still end red; the real run then + does, rolled back. No behaviour changes.""" + branched(project) + pin = project / SPEC_PIN + pin.write_bytes(pin.read_bytes() + + b"note: a remark\n\tindented with a tab\n") + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A pin its validator cannot parse", "--", SPEC_PIN, cwd=project) + gate = validate(project) + assert gate.returncode == 2, gate.stdout + gate.stderr + assert "yaml-unparsable" in gate.stdout + gate.stderr + head = head_of(project) + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert NOT_VALIDATED in dry.stdout, dry.stdout + + real = bump(project, scaffolded, advanced) + assert real.returncode == 2, real.stderr + real.stdout + assert "REFUSED bump-leg-validators-red" in real.stderr, real.stderr + assert head_of(project) == head + assert git("status", "--porcelain", cwd=project).stdout == "" diff --git a/tests/test_bump_leg_paste_lines.py b/tests/test_bump_leg_paste_lines.py index 32e95199..b8efe96b 100644 --- a/tests/test_bump_leg_paste_lines.py +++ b/tests/test_bump_leg_paste_lines.py @@ -288,8 +288,10 @@ def test_pasting_the_detached_remedy_puts_the_root_on_a_branch( @pytest.mark.parametrize("name", ROOTS) def test_the_closing_next_is_one_command(make_root, scaffolded, advanced, name): - """`NEXT git -C push -u origin `, split into the argv it - means, and spelled exactly as `shlex` spells it.""" + """`NEXT git -C push -u origin refs/heads/:refs/heads/`, + split into the argv it means, and spelled exactly as `shlex` spells it. + The branch by its full refname (#672): a bare one is a refspec, and a + leading `+` forces.""" root, _ = make_root(name) branched(root) result = bump(root, scaffolded, advanced) @@ -297,7 +299,7 @@ def test_the_closing_next_is_one_command(make_root, scaffolded, advanced, line = next_line(result.stdout) argv = ["git", "-C", str(root.resolve()), "push", "-u", "origin", - WORKING_BRANCH] + f"refs/heads/{WORKING_BRANCH}:refs/heads/{WORKING_BRANCH}"] assert words(line) == argv assert line == shlex.join(argv) assert (" then open a pull request against main; never push the " @@ -309,13 +311,15 @@ def test_the_closing_next_is_one_command(make_root, scaffolded, advanced, def test_an_ordinary_root_prints_the_line_it_always_did(make_root, scaffolded, advanced): """Nothing is quoted that needs no quoting: a root whose name is plain - prints the same bytes as before #532.""" + prints the bytes it did before #532, the branch by its full refname on + both sides (#672).""" root, _ = make_root("plain") branched(root) result = bump(root, scaffolded, advanced) assert result.returncode == 0, result.stderr + result.stdout assert next_line(result.stdout) == ( - f"git -C {root.resolve()} push -u origin {WORKING_BRANCH}") + f"git -C {root.resolve()} push -u origin " + f"refs/heads/{WORKING_BRANCH}:refs/heads/{WORKING_BRANCH}") @WINDOWS_SKIP @@ -348,11 +352,12 @@ def test_the_closing_next_carries_the_branch_whole(make_root, scaffolded, assert result.returncode == 0, result.stderr + result.stdout line = next_line(result.stdout) + pushed = f"refs/heads/{branch}:refs/heads/{branch}" assert words(line) == ["git", "-C", str(root.resolve()), "push", "-u", - "origin", branch] - assert words(line)[-1] == branch + "origin", pushed] + assert words(line)[-1] == pushed assert line == shlex.join(["git", "-C", str(root.resolve()), "push", "-u", - "origin", branch]) + "origin", pushed]) @WINDOWS_SKIP From 6ba8c5d2d6fec3a779963713593e5377c76bbadf Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 18:40:04 +0000 Subject: [PATCH 06/10] conftest.py and two test files: `skip_message` is held by a test that fails where a skip escapes it, `windows_fsencode` passes bytes through as `os.fsencode` does, and the no-`\x` check skips where the file name encoding is not UTF-8, so a leaked skip, an emulation that breaks on bytes and a Latin-1 host no longer pass silently or fail on a tool that is right (#663) The review of #723 (0b1d1ddb) found no MUST and three things to take. SHOULD-1: `skip_message` had no test of its own. A skip that escapes it ends the test SKIPPED, and no run fails on a skip, so the mutant where one escapes gave `10 passed, 3 skipped` on the helpers file and seven emulation tests silently skipped. `tests/test_non_utf8_name_helpers.py` now holds it with three tests: it returns the reason a call skips with, it returns None where the call does not skip, and it lets any other exception go on. The first catches the escape and fails it with `pytest.fail`, because a bare assert on an escaped skip would end that test SKIPPED too. NIT-2: `windows_fsencode` took `str` only. `os.fsencode` passes bytes through on every system, so the emulation now does, and two tests hold it (the surrogate and text, and the bytes, compared with `os.fsencode` itself). NIT-1: on a Latin-1 file name encoding `make_file` writes `caf\xe9.yml` as the single byte 0xE9, the tool prints `caf\xe9.yml`, and the new `"\\x" not in shown` assertion would fail although the tool is right. It is now `assert_shown_without_hex_escapes`, which skips where `os.fsencode(UTF8) != UTF8.encode("utf-8")` and is called after the test's other assertions, so a skip loses none of them. Two tests hold it under an emulated Latin-1 `os.fsencode` (it skips) and Windows' (it asserts, and fails a line that holds a `\x`). Tests only; no tool and no template changes. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 7 +++-- tests/test_non_utf8_git_reads.py | 44 ++++++++++++++++++++++++++- tests/test_non_utf8_name_helpers.py | 47 +++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0e9da01d..a5313d72 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -926,11 +926,14 @@ def skip_where_a_name_cannot_be_not_utf_8(directory: Path) -> None: probe.rmdir() -def windows_fsencode(text: str) -> bytes: +def windows_fsencode(text: str | bytes) -> bytes: """`os.fsencode` as Windows has it: UTF-8 with `surrogatepass`, so every - lone surrogate is the three bytes of a sequence that is not UTF-8. For + lone surrogate is the three bytes of a sequence that is not UTF-8. Bytes + go through as they are, as `os.fsencode` passes them on every system. For `monkeypatch.setattr(os, "fsencode", windows_fsencode)`, which puts the guard above in front of a test that Linux would let through.""" + if isinstance(text, bytes): + return text return text.encode("utf-8", "surrogatepass") diff --git a/tests/test_non_utf8_git_reads.py b/tests/test_non_utf8_git_reads.py index 18fbb7e9..4d19a998 100644 --- a/tests/test_non_utf8_git_reads.py +++ b/tests/test_non_utf8_git_reads.py @@ -143,6 +143,48 @@ def test_make_file_writes_the_name_that_is_not_utf8_where_the_system_has_it( assert target.read_bytes() == b"on: [push]\n" +def assert_shown_without_hex_escapes(shown: str, output: str) -> None: + """Assert that the refusal's line for `caf\u00e9.yml` holds no literal + `\\x`, or skip where this system's file name encoding is not UTF-8. + + On a Latin-1 host `make_file` writes the one byte 0xE9 for it, and the + tool, which decodes git's bytes as UTF-8, rightly prints `caf\\xe9.yml`: + the assertion would fail though the tool is right (#663, NIT-1). So it + is asked after the system's encoding is, and a system whose encoding is + not UTF-8 skips. (Where `make_file` could not write the name at all, an + ASCII host, it skipped already.)""" + if os.fsencode(UTF8) != UTF8.encode("utf-8"): + pytest.skip("this system's file name encoding is not UTF-8, so a " + "UTF-8 name is not written as the bytes the tool decodes") + assert "\\x" not in shown, output + + +def test_the_hex_escape_check_skips_where_the_file_name_encoding_is_not_utf_8( + monkeypatch): + """THE LATIN-1 HOST, EMULATED (#663, NIT-1): `os.fsencode` gives the name + `caf\u00e9.yml` as the one byte 0xE9, the line holds a `\\x`, and the + check skips, where it would otherwise fail on a tool that is right.""" + monkeypatch.setattr(os, "fsencode", lambda name: name.encode("latin-1")) + reason = skip_message(assert_shown_without_hex_escapes, + r" M .github/workflows/caf\xe9.yml", "output") + assert reason is not None, "the check asserted instead of skipping" + assert "UTF-8" in reason, reason + + +def test_the_hex_escape_check_asserts_where_the_file_name_encoding_is_utf_8( + monkeypatch): + """The control: Windows' `os.fsencode` (emulated), like Linux's and + macOS's, is UTF-8 for text, so the check is not skipped, and it still + fails a line that holds a `\\x` and passes one that does not.""" + monkeypatch.setattr(os, "fsencode", windows_fsencode) + reason = skip_message(assert_shown_without_hex_escapes, + " M .github/workflows/caf\u00e9.yml", "output") + assert reason is None, reason + with pytest.raises(AssertionError): + skip_message(assert_shown_without_hex_escapes, + r" M .github/workflows/caf\xc3\xa9.yml", "output") + + def git_bytes(*args: str, cwd: Path) -> bytes: """`git ` in `cwd`, stdout as the bytes git wrote. `conftest.git` decodes as text, which is the failure under test.""" @@ -269,9 +311,9 @@ def test_a_name_that_is_utf8_under_quote_path_false_is_still_refused( [shown] = [line for line in result.stderr.splitlines() if line.startswith(" M .github/workflows/caf")] assert shown.endswith(".yml"), output - assert "\\x" not in shown, output assert "Traceback" not in output, output assert found(project) == before + assert_shown_without_hex_escapes(shown, output) def test_the_refusal_names_every_dirty_file_whatever_its_bytes( diff --git a/tests/test_non_utf8_name_helpers.py b/tests/test_non_utf8_name_helpers.py index 8c387eee..47ae0d13 100644 --- a/tests/test_non_utf8_name_helpers.py +++ b/tests/test_non_utf8_name_helpers.py @@ -14,6 +14,10 @@ `os.fsencode` is replaced for one test by `windows_fsencode`, which gives a surrogate three bytes, as Windows' does. No Windows run was made. macOS's EILSEQ is emulated the same way, by a `mkdir` that answers it. + +The two tools the tests below are written with are held here too: +`skip_message`, which every one of them reads a skip with, and +`windows_fsencode`, the emulation itself. """ from __future__ import annotations @@ -43,6 +47,49 @@ def refuse(self, *args, **kwargs): return refuse +# --- `skip_message`: how every test below reads a skip ----------------------- +# +# A skip that ESCAPES a helper ends its test SKIPPED, and no run fails on a +# skip: a `skip_message` that let one out would turn every assertion below +# that expects a skip, or expects none, into a silent skip. So the first test +# catches the escape and fails it. + +def test_skip_message_returns_the_reason_a_call_skips_with(): + try: + reason = skip_message(lambda: pytest.skip("x")) + except pytest.skip.Exception: + pytest.fail("the skip escaped skip_message, which would end the test " + "SKIPPED, and no run fails on that") + assert reason == "x" + + +def test_skip_message_returns_none_where_the_call_does_not_skip(): + assert skip_message(lambda: None) is None + + +def test_skip_message_lets_any_other_exception_go_on(): + def not_a_skip(): + raise ZeroDivisionError("not a skip") + + with pytest.raises(ZeroDivisionError): + skip_message(not_a_skip) + + +# --- `windows_fsencode`: the emulation is Windows' `os.fsencode` -------------- + +def test_windows_fsencode_makes_three_bytes_of_a_surrogate_and_utf_8_of_text(): + assert windows_fsencode("\udcff") == b"\xed\xb3\xbf" + assert windows_fsencode("caf\u00e9.yml") == "caf\u00e9.yml".encode("utf-8") + + +def test_windows_fsencode_passes_bytes_through_as_os_fsencode_does(): + """`os.fsencode` takes bytes on every system and gives them back; the + emulation that stands in for it must not fail on them.""" + data = b"caf\xe9.yml" + assert os.fsencode(data) == data + assert windows_fsencode(data) == data + + # --- the system's question: what `os.fsencode` makes of the surrogate -------- @LINUX_ONLY From a5d473eed9ce7ded1bb1654bd6ce966f40a274ec Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 19:16:13 +0000 Subject: [PATCH 07/10] bump-leg.py: the branch read keeps a trailing no-break space, the dirty list keeps git's leading ` M`, the fallback names a `git show` that lists the gitlink, and a gitlink moved by hand is not said to move, so rv672's S2 and N1-N3 no longer happen (#672) rv672's review of 326de720, one fix round: - S2: `current_branch` takes only git's newline off `symbolic-ref`'s answer. `strip()` also took a trailing U+00A0, U+2028 or U+0085, so a root on `feat` beside a branch `feat` committed on the first and printed a push of the second. - N1: `_ensure_root_clean` takes only the last newline off. `strip(" \n")` took the first line's leading space, so its ` M` read as `M` (staged) and stood a column left of the lines under it. - N2: the `committed` fallback names `git show --name-only --ignore-submodules=none `, which lists the gitlink under `diff.ignoreSubmodules=all` as the line itself does. - N3: where the gitlink already recorded the commit, the message's last sentence says the pin and the workflow references are brought to the gitlink in ONE commit, and no longer that the gitlink moves in it. The ordinary message is byte for byte what it was. Tests: S1 parametrizes the HEAD-outside-`refs/heads/` test over `refs/tags/release-1` and `refs/remotes/origin/main` (rv672's R1 survived on `refs/tags/x`, whose name is as long as `refs/heads/`); N4 adds a pin whose `commit:` is `not-a-sha` (rv672's R2); S2 and N1 each add a test, and the dirty-list expectations in two files now read ` M ...`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/bump-leg.py | 35 +++-- tests/test_bump_leg_branch_and_push_line.py | 147 ++++++++++++++++---- tests/test_non_utf8_git_reads.py | 7 +- 3 files changed, 151 insertions(+), 38 deletions(-) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 63b476ce..226e7b0d 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -1082,11 +1082,17 @@ def current_branch(root: Path) -> str | None: against `main`, `bump-leg-on-tracking-branch` was not given, and the bump committed on the default branch and printed a push to it. The name after `refs/heads/` is the branch's own, whatever tags there are. + + ONLY THE NEWLINE IS TAKEN OFF. A ref name holds no control character, + so git's line ending is all there is to drop; `strip()` also took a + trailing U+00A0, U+2028 or U+0085, which a branch name may end in, and + read a root on `feat` as `feat`, a branch of its own that the + push line then pushed instead of the bump. """ proc = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], cwd=str(root), capture_output=True, text=True, encoding="utf-8", errors="replace", check=False) - ref = proc.stdout.strip() + ref = proc.stdout.rstrip("\n") if not ref.startswith(HEADS): return None return ref[len(HEADS):] or None @@ -1146,8 +1152,11 @@ def _ensure_root_clean(root: Path) -> None: # the bytes it holds, so a C1 control or a bidi override in a file # name reached the terminal raw. Split on git's own newlines and no # others: `splitlines` also breaks at U+0085 and U+2028, which are - # `spelled`'s to show, and `strip()` took them off the last name. - shown = dirt.decode("utf-8", "backslashreplace").strip(" \n") + # `spelled`'s to show, and `strip()` took them off the last name and + # the leading space off the first line's ` M`, which then read as + # `M` (staged). git quotes a name with a control in it or a space at + # its end, so the last newline is all there is to take off. + shown = dirt.decode("utf-8", "backslashreplace").rstrip("\n") raise Refusal( "bump-leg-root-dirty", f"{root} has uncommitted changes:\n" @@ -1682,7 +1691,8 @@ def _committed_paths(root: Path, head: str) -> str: why = " ".join((exc.detail if isinstance(exc, Refusal) else str(exc)).split()) return (f"its paths could not be read back ({why}); `git show " - f"--name-only {head[:12]}` lists them") + f"--name-only --ignore-submodules=none {head[:12]}` lists " + "them") return ", ".join(spelled(name.decode("utf-8", "backslashreplace")) for name in raw.split(b"\0") if name) @@ -1808,11 +1818,18 @@ def cmd_bump(args) -> int: + ("the pin file is" if was == commit else "the gitlink and the pin file are") + " the whole of it.\n\n") - + f"The gitlink at {path}, `commit:` and `digests.tree_sha256` in " - f"{pin_rel}, and every workflow reference to {repository} move in " - "ONE commit because `scripts/validate-pins.py` refuses when they " - "disagree — a gitlink advanced alone is red on the next pull " - "request, and on somebody else's change.\n\nWritten by " + + (f"`commit:` and `digests.tree_sha256` in {pin_rel}, and every " + f"workflow reference to {repository}, are brought to the gitlink " + f"at {path} in ONE commit because `scripts/validate-pins.py` " + "refuses when they disagree — a gitlink advanced alone, as this " + "one was, is red on the next pull request, and on somebody " + "else's change.\n\n" if was == commit else + f"The gitlink at {path}, `commit:` and `digests.tree_sha256` in " + f"{pin_rel}, and every workflow reference to {repository} move " + "in ONE commit because `scripts/validate-pins.py` refuses when " + "they disagree — a gitlink advanced alone is red on the next " + "pull request, and on somebody else's change.\n\n") + + "Written by " "openRepoShape's `scripts/bump-leg.py`; the digest is recomputed " "from the leg's own objects, not adjusted.\n") head = _apply_rewrite( diff --git a/tests/test_bump_leg_branch_and_push_line.py b/tests/test_bump_leg_branch_and_push_line.py index ce8e4376..11dc4bdd 100644 --- a/tests/test_bump_leg_branch_and_push_line.py +++ b/tests/test_bump_leg_branch_and_push_line.py @@ -129,21 +129,28 @@ def test_a_tag_named_like_the_tracking_branch_does_not_let_the_bump_onto_it( assert "heads/main" not in dry.stdout -def test_a_head_on_a_tag_ref_is_no_branch_to_bump_on(project, scaffolded, - advanced): - """`symbolic-ref` lets HEAD name `refs/tags/`, and `--short` read it - as the branch ``: the bump committed THROUGH it, which moved the tag, - and printed `push -u origin x`. A ref outside `refs/heads/` is no - branch: refused as a root on no branch is, with the tag where it was.""" - git("tag", "x", cwd=project) - tagged = head_of(project, "refs/tags/x") - git("symbolic-ref", "HEAD", "refs/tags/x", cwd=project) +@pytest.mark.parametrize("ref", ["refs/tags/release-1", + "refs/remotes/origin/main"]) +def test_a_head_on_a_ref_outside_refs_heads_is_no_branch_to_bump_on( + project, scaffolded, advanced, ref): + """`symbolic-ref` lets HEAD name `refs/tags/` or a remote-tracking + ref, and `--short` read either as a branch (`release-1`, `origin/main`): + the bump committed THROUGH it, which moved the ref, and printed a push + of it. A ref outside `refs/heads/` is no branch: refused as a root on no + branch is, with the ref where it was. Neither name is 11 characters + long, as `refs/heads/` is, so the refusal is the guard's and not an + empty name's (rv672's R1).""" + if ref.startswith("refs/tags/"): + git("tag", ref[len("refs/tags/"):], cwd=project) + was = head_of(project, ref) + assert was, f"fixture: {ref} exists" + git("symbolic-ref", "HEAD", ref, cwd=project) result = bump(project, scaffolded, advanced) assert result.returncode == 2, result.stderr + result.stdout assert "REFUSED bump-leg-root-detached" in result.stderr, result.stderr assert result.stdout == "" - assert head_of(project, "refs/tags/x") == tagged, "the tag did not move" + assert head_of(project, ref) == was, f"{ref} did not move" # --- the push, by its full ref ----------------------------------------------- @@ -225,6 +232,29 @@ def test_a_branch_beside_a_tag_of_its_name_is_pushed_by_its_full_ref( assert upstream_of(root, "v1") == "refs/remotes/origin/v1" +def test_a_branch_ending_in_a_no_break_space_is_pushed_as_itself( + make_root, scaffolded, advanced): + """`feat` beside a branch `feat`: `strip()` took the no-break + space off the name it read, so the bump committed on `feat` and + the line pushed `feat`, the other branch, at the commit before the bump + (rv672's S2). Only git's newline is taken off now: the line names the + branch the bump committed on, and run, pushes that commit.""" + root, remote = make_root("plain") + name = "feat\u00a0" + git("branch", "feat", cwd=root) + git("checkout", "-q", "-b", name, cwd=root) + + result = bump(root, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + line = next_line(result.stdout) + assert words(line) == push_argv(root, name) + + pushed = run_the_line(line, root.parent) + assert pushed.returncode == 0, pushed.stderr + pushed.stdout + assert head_of(remote, f"refs/heads/{name}") == head_of(root) + assert head_of(remote, "refs/heads/feat") == "", "`feat` was not pushed" + + # --- the dirty root, spelled ------------------------------------------------- #: `(a name, as `bump-leg-root-dirty` lists it under core.quotePath=false)`. @@ -272,7 +302,7 @@ def test_a_dirty_name_holding_controls_is_listed_spelled( result = bump(project, scaffolded, advanced, *extra) assert result.returncode == 2, result.stderr + result.stdout assert "REFUSED bump-leg-root-dirty" in result.stderr, result.stderr - assert f" M .github/workflows/{shown}" in result.stderr.split("\n"), ( + assert f" M .github/workflows/{shown}" in result.stderr.split("\n"), ( result.stderr) assert raw_controls(name, result.stderr) == [], ascii(result.stderr) @@ -285,10 +315,33 @@ def test_under_the_default_quoting_git_names_the_file_itself( git("config", "--unset", "core.quotePath", cwd=project) result = bump(project, scaffolded, advanced) assert result.returncode == 2, result.stderr + result.stdout - assert (' M ".github/workflows/caf\\302\\233\\342\\200\\256evil.yml"' + assert (' M ".github/workflows/caf\\302\\233\\342\\200\\256evil.yml"' in result.stderr.splitlines()), result.stderr +def test_every_dirty_line_keeps_gits_status_columns(project, scaffolded, + advanced): + """`strip()` took the leading space off the FIRST line only, so its + ` M` (changed in the working tree) read as `M` (changed in the index) + and stood one column left of the line under it (rv672's N1). Every line + is git's own now, after the same indent.""" + branched(project) + workflows = project / ".github" / "workflows" + for name in ("one.yml", "two.yml"): + (workflows / name).write_bytes(b"on: [push]\n") + git("add", "--", ".github/workflows", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Two workflows", cwd=project) + for name in ("one.yml", "two.yml"): + (workflows / name).write_bytes(b"on: [pull_request]\n") + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + lines = result.stderr.split("\n") + assert " M .github/workflows/one.yml" in lines, result.stderr + assert " M .github/workflows/two.yml" in lines, result.stderr + + # --- the commit, as git holds it --------------------------------------------- @@ -319,28 +372,42 @@ def test_the_committed_line_names_the_paths_git_holds( assert not any(line.startswith("gitlink ") for line in lines_of(result)) -def test_a_gitlink_moved_by_hand_is_reported_as_the_pins_move( - project, scaffolded, advanced): - """THE ISSUE'S MEASUREMENT (AT-scaffold N9): a person moved the gitlink to - the new commit by hand and committed it; the pin still names the old - one. The report, the dry run's included, and the commit message said - `X -> X`, and `committed` named `spec` beside the pin, which the commit - does not hold. The old commit is the pin's own now, a line says the - gitlink does not move, and the commit's paths are git's.""" - pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() +def move_the_gitlink_by_hand(project: Path, scaffolded: dict, + to: str) -> None: + """On a bump branch, the leg checked out at `to` and the gitlink moved + to it in a commit of its own, the pin left as it was: the repair case + of AT-scaffold N9.""" branched(project) leg = project / "spec" git(*FILE_PROTOCOL, "fetch", "-q", str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), ADVANCE_BRANCH, cwd=leg) - git("checkout", "-q", "--detach", advanced, cwd=leg) + git("checkout", "-q", "--detach", to, cwd=leg) git("add", "--", "spec", cwd=project) git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", "Move the gitlink by hand", cwd=project) - moved = f"commit {pinned[:12]} -> {advanced[:12]}" - held = (f"gitlink spec already records {advanced[:12]}, so this " + + +def held_line(commit: str) -> str: + return (f"gitlink spec already records {commit[:12]}, so this " "does not move it") + +def test_a_gitlink_moved_by_hand_is_reported_as_the_pins_move( + project, scaffolded, advanced): + """THE ISSUE'S MEASUREMENT (AT-scaffold N9): a person moved the gitlink to + the new commit by hand and committed it; the pin still names the old + one. The report, the dry run's included, and the commit message said + `X -> X`, and `committed` named `spec` beside the pin, which the commit + does not hold. The old commit is the pin's own now, a line says the + gitlink does not move, and the commit's paths are git's. The message's + last sentence states the rule without saying the gitlink moved in it + (rv672's N3).""" + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + move_the_gitlink_by_hand(project, scaffolded, advanced) + moved = f"commit {pinned[:12]} -> {advanced[:12]}" + held = held_line(advanced) + dry = bump(project, scaffolded, advanced, "--dry-run") assert dry.returncode == 0, dry.stderr + dry.stdout assert moved in lines_of(dry), dry.stdout @@ -359,6 +426,33 @@ def test_a_gitlink_moved_by_hand_is_reported_as_the_pins_move( assert (f"The gitlink at spec already recorded {advanced[:12]}, so this " "commit does not move it.") in body, body assert "so the pin file is the whole of it." in body, body + assert ("are brought to the gitlink at spec in ONE commit because " + "`scripts/validate-pins.py` refuses when they disagree") in \ + " ".join(body.split()), body + assert "The gitlink at spec, `commit:`" not in body, body + assert validate(project).returncode == 0, "the project's own gate agrees" + + +def test_a_pin_commit_that_is_no_commit_is_not_printed_as_the_old_one( + project, scaffolded, advanced): + """The pin's own `commit:` is the old commit only when it IS one: text + such as `not-a-sha` there is not printed on the `commit` line, which + names the gitlink's commit instead, with the line that says the gitlink + does not move (rv672's R2).""" + pin = project / SPEC_PIN + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + move_the_gitlink_by_hand(project, scaffolded, advanced) + pin.write_bytes(pin.read_bytes().replace( + f'commit: "{pinned}"'.encode("ascii"), b'commit: "not-a-sha"', 1)) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A pin whose commit is no commit", "--", SPEC_PIN, cwd=project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert f"commit {advanced[:12]} -> {advanced[:12]}" in \ + lines_of(result), result.stdout + assert held_line(advanced) in lines_of(result), result.stdout + assert "not-a-sha" not in result.stdout assert validate(project).returncode == 0, "the project's own gate agrees" @@ -387,9 +481,10 @@ def no_diff_tree(args, *rest, **kwargs): head = head_of(project) assert code == 0, out assert git("rev-parse", f"{head}~1", cwd=project).stdout.strip() == before - assert (f" committed {head[:12]}: its paths could not be read back " + said = (f" committed {head[:12]}: its paths could not be read back " "(`git diff-tree` exited 128: fatal: bad object); `git show " - f"--name-only {head[:12]}` lists them") in out.splitlines(), out + f"--name-only --ignore-submodules=none {head[:12]}` lists them") + assert said in out.splitlines(), out assert next_line(out) == shlex.join(push_argv(project, "bump/spec")) diff --git a/tests/test_non_utf8_git_reads.py b/tests/test_non_utf8_git_reads.py index c79bada2..49fdaca8 100644 --- a/tests/test_non_utf8_git_reads.py +++ b/tests/test_non_utf8_git_reads.py @@ -161,7 +161,8 @@ def refused_dirty(project: Path, scaffolded: dict, to: str, shown: str, assert result.returncode == 2, output assert "REFUSED bump-leg-root-dirty: " in result.stderr, output assert f"{project} has uncommitted changes:\n" in result.stderr, output - assert f" M {shown}" in result.stderr.splitlines(), output + # Four spaces, then git's own ` M` (#672: its leading space is kept). + assert f" M {shown}" in result.stderr.splitlines(), output assert "Remediation: commit or stash them first." in result.stderr assert "Traceback" not in output, output assert "UnicodeDecodeError" not in output, output @@ -212,7 +213,7 @@ def test_a_name_that_is_utf8_under_quote_path_false_is_still_refused( output = result.stderr + result.stdout assert result.returncode == 2, output assert "REFUSED bump-leg-root-dirty: " in result.stderr, output - assert any(line.startswith(" M .github/workflows/caf") + assert any(line.startswith(" M .github/workflows/caf") and line.endswith(".yml") for line in result.stderr.splitlines()), output assert "Traceback" not in output, output @@ -229,7 +230,7 @@ def test_the_refusal_names_every_dirty_file_whatever_its_bytes( result = bump(project, scaffolded, advanced) lines = result.stderr.splitlines() assert result.returncode == 2, result.stderr - assert r" M .github/workflows/caf\xe9.yml" in lines, result.stderr + assert r" M .github/workflows/caf\xe9.yml" in lines, result.stderr assert any(line.rstrip().endswith("M README.md") for line in lines), ( result.stderr) assert "Traceback" not in result.stderr From 0e516bba346b268b450a2333c353d616686aa7cf Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:02:38 +0000 Subject: [PATCH 08/10] family.py, scaffold-project.py and bump-leg.py: `init`'s plan rows, seven refusals and bump-leg's root name each path as `shown_path` names it, so an ESC or a line break in a holder, folder, lock, local remote or root no longer reaches the terminal (#766) #692 (PR #731) spelled the folder refusals with `shown_source`, and #672 (PR #746) spelled each dirty file's name. The paths beside them were still printed as they were. Measured with a stub `gh` on the stacked base: a `--work-dir` or `--into` holding a line break and then `WARNING forged-line: pwned` printed that line as a line of `init`'s plan, on a dry run and on a run refused after it; each refusal below printed it as a line of its own; and an ESC reached the terminal raw. Each of these now names its path as `repo_shape.shown_path` does: as it is, and as Python's `repr` spells it where a character in it is not printable. A printable path is named byte for byte, so no existing test row changed. - `scripts/family.py`: the plan's `folder` and `landing` rows; `family-holder-detached` and `family-on-tracking-branch` (the holder); `family-landing-in-standard` (the holder, the checkout, and where `--into ..` lands); `family-holder-already-here` (`--into` and the holder in it); the detail of `family-index-locked` (the lock); and `family-remote-exists` for a local remote. - `scaffold-project.py`: `scaffold-remote-exists` for a local remote. - `scripts/bump-leg.py`: `bump-leg-root-dirty`, `bump-leg-root-detached`, `bump-leg-on-tracking-branch` and the dry run's `the real run needs a branch` line (the root). A GitHub `/` is named as it is. `shown_path` is imported from `repo_shape` into each file's existing `from repo_shape import (...)` block. Not changed: the commands a remedy prints for a person to paste (the `switch -c` of `_switch_command` in family.py and bump-leg.py, the `add` of `_add_command`, the lock's `rm -f`, and the `switch -c` line of `add`/`bump`/`remove --dry-run`). They are quoted for a shell as they were, and still hold such a name as it is. Tests: the new rows hold a path with an ESC and one with a line break then `WARNING forged-line: pwned`, POSIX only, skipped where the disk refuses the name. `tests/conftest.py` keeps the names and the helpers the rows share; #731's `ESC_NAME`, `FORGED_NAME`, `POSIX_NAMES` and `spelt` moved there from `tests/test_dry_run_parity.py`, which imports them. The two remote-exists refusals cannot be reached from the command line with such a name (the clone URL is refused first as `*-unwritable-value`, already spelled), so their rows ask the question in process after that CLI run. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- scaffold-project.py | 11 +- scripts/bump-leg.py | 27 +++-- scripts/family.py | 97 +++++++++++----- tests/conftest.py | 78 +++++++++++++ tests/test_bump_leg_branch_and_push_line.py | 86 ++++++++++++++ tests/test_dry_run_asks_remotes.py | 51 +++++++++ tests/test_dry_run_parity.py | 22 +--- tests/test_family_bump_remove_rollback.py | 26 +++++ tests/test_family_init_landing.py | 117 ++++++++++++++++++++ tests/test_family_tracking_branch.py | 36 ++++++ 10 files changed, 489 insertions(+), 62 deletions(-) diff --git a/scaffold-project.py b/scaffold-project.py index 975dab7e..b754959c 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -86,7 +86,8 @@ from repo_shape import ( # noqa: E402 COMMIT_RE, NEUTRAL_PRODUCT_OWNER, PROJECT_ID_RE, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, accepts_role, checked_value, - git_out, link_pins_from_trees, repo_basename, shell_line, tree_digest, + git_out, link_pins_from_trees, repo_basename, shell_line, shown_path, + tree_digest, ) from shape_materialize import ( # noqa: E402 RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, @@ -546,6 +547,11 @@ def _reusable_remotes(names: dict, urls: dict, repositories: dict, always was, of `remote_is_empty` or `local_remote_is_empty`; the seed is asked only of a leg that is not empty, and a leg git cannot read is refused as one with commits, saying so (#542). + + A LOCAL REMOTE IS NAMED AS `shown_path` NAMES A PATH (#766), as + `family.py init` names its holder's: it is a path under + `--local-remote-dir`, where `scaffold-unwritable-value` refuses a + control character first. A GitHub `/` is named as it is. """ reused: dict[str, str | None] = {} with tempfile.TemporaryDirectory(prefix="openreposhape-seed-") as scratch: @@ -567,7 +573,8 @@ def _reusable_remotes(names: dict, urls: dict, repositories: dict, continue raise Refusal( "scaffold-remote-exists", - f"{target} already exists" + ("" if local else " on GitHub") + f"{shown_path(target) if local else target} already exists" + + ("" if local else " on GitHub") + ("" if empty else " and has commits"), _exists_remedy(role, empty, reuse_flag, seed is not None, unread)) diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 226e7b0d..f1aa7e28 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -121,7 +121,7 @@ sys.path.insert(0, str(SHAPE_ROOT / "scripts")) from repo_shape import ( # noqa: E402 COMMIT_RE, SAFE_PATH_RE, Refusal, checked_value, git_out, load_yaml, - recorded_gitlink, repo_basename, spelled, tree_digest, + recorded_gitlink, repo_basename, shown_path, spelled, tree_digest, ) from shape_materialize import ( # noqa: E402 CommandFailed, captured_child_environment, check_program, commit_trailers, @@ -1155,11 +1155,13 @@ def _ensure_root_clean(root: Path) -> None: # `spelled`'s to show, and `strip()` took them off the last name and # the leading space off the first line's ` M`, which then read as # `M` (staged). git quotes a name with a control in it or a space at - # its end, so the last newline is all there is to take off. + # its end, so the last newline is all there is to take off. THE ROOT + # ABOVE THEM is named as `shown_path` names a path (#766): its name + # held a C1 control and a bidi override raw on the first line. shown = dirt.decode("utf-8", "backslashreplace").rstrip("\n") raise Refusal( "bump-leg-root-dirty", - f"{root} has uncommitted changes:\n" + f"{shown_path(root)} has uncommitted changes:\n" + "\n".join(f" {spelled(line)}" for line in shown.split("\n")), "Remediation: commit or stash them first. A lockstep commit that " "cannot be read as exactly one change is the thing the rule " @@ -1217,21 +1219,23 @@ def _branch_to_bump_from(root: Path, project: "Project", role: str, the branch the real run needs (#477); a root on no branch at all is refused either way. Both refusals name that branch's `switch -c` through `_switch_command`, quoted (#532), and end with `BRANCH_LEFT`, the - sentence for a root that has that branch already (#647). + sentence for a root that has that branch already (#647). Each detail + names the root as `shown_path` names a path (#766), as the dirty root's + does. """ branch = current_branch(root) if branch is None: raise Refusal( "bump-leg-root-detached", - f"{root} is not on a branch, so there is no branch to push and " - "open a pull request from", + f"{shown_path(root)} is not on a branch, so there is no branch " + "to push and open a pull request from", f"Remediation: `{_switch_command(root, role, commit)}` and " f"re-run.\n{BRANCH_LEFT}") if branch == project.tracking and not dry_run: raise Refusal( "bump-leg-on-tracking-branch", - f"{root} is on {branch!r}, which {MANIFEST} declares as this " - "project's tracking branch", + f"{shown_path(root)} is on {branch!r}, which {MANIFEST} declares " + "as this project's tracking branch", "Remediation: these organisations are pull-request only, so this " "will not make a commit whose only next step is a push to the " "default branch. Run:\n " @@ -1796,9 +1800,10 @@ def cmd_bump(args) -> int: # LAST, after everything a dry run on a branch prints, so that # line and the sentence under it are the whole of the difference # between the two (#477, #647). The command is quoted, so a root - # under `My Projects/` pastes. - print(f"\nthe real run needs a branch: {root} is on {branch!r}, " - "the tracking branch. Create one first with: " + # under `My Projects/` pastes; the root before it is named as + # `shown_path` names a path, as the refusals name it (#766). + print(f"\nthe real run needs a branch: {shown_path(root)} is on " + f"{branch!r}, the tracking branch. Create one first with: " + _switch_command(root, role, asked)) print(BRANCH_LEFT) return 0 diff --git a/scripts/family.py b/scripts/family.py index 7e7d44f5..23450150 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -169,7 +169,7 @@ COMMIT_RE, PROJECT_ID_RE, PYTHON, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, YamlError, checked_value, double_quoted, git_out, load_yaml, parse_yaml, recorded_gitlink, - repo_basename, same_repository, tree_digest, + repo_basename, same_repository, shown_path, tree_digest, ) from shape_materialize import ( # noqa: E402 FAMILY_RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, check_program, @@ -759,7 +759,12 @@ def _checked_branch(family: "Family", verb: str, suggested: str) -> str: (#487). Asked by the real run, after the refusals about the arguments and the holder and before the member is read or anything is written; `--dry-run` asks `_branch_said` instead. The remedy's `switch -c` is - followed by `BRANCH_LEFT`, for a holder that has the branch (#607).""" + followed by `BRANCH_LEFT`, for a holder that has the branch (#607). + + THE HOLDER IS NAMED AS `shown_path` NAMES A PATH (#766), as #692 names + the folders `init` makes: a holder whose path held an ESC sent it to the + terminal, and one with a line break began a line of its own choosing. + A printable path is named byte for byte.""" branch = _holder_branch(family.root) if branch is not None and branch != family.tracking(): return branch @@ -772,16 +777,17 @@ def _checked_branch(family: "Family", verb: str, suggested: str) -> str: if branch is None: raise Refusal( "family-holder-detached", - f"{family.root} is not on a branch, so `{verb}` would write a " - "commit no branch holds, with nothing to push and open a pull " - "request from. No git command that writes was run and nothing " - "was written.", remedy) + f"{shown_path(family.root)} is not on a branch, so `{verb}` " + "would write a commit no branch holds, with nothing to push and " + "open a pull request from. No git command that writes was run " + "and nothing was written.", remedy) raise Refusal( "family-on-tracking-branch", - f"{family.root} is on {branch!r}, which {MANIFEST} declares as the " - f"holder's tracking branch, and `{verb}` will not write a commit " - "whose only next step is a push to the default branch. No git " - "command that writes was run and nothing was written.", remedy) + f"{shown_path(family.root)} is on {branch!r}, which {MANIFEST} " + "declares as the holder's tracking branch, and " + f"`{verb}` will not write a commit whose only next step is a push to " + "the default branch. No git command that writes was run and nothing " + "was written.", remedy) def _branch_said(family: "Family", verb: str, suggested: str) -> list[str]: @@ -1452,6 +1458,12 @@ def _refuse_landing_in_standard(into: Path, holder: Path, checkout the relative path that reaches its parent from there, so the printed flag lands the family beside the checkout wherever the person stands. Asked before anything is created, under `--dry-run` alike. + + THE THREE PATHS ARE NAMED AS `shown_path` NAMES A PATH (#766): the + holder, the checkout and where `--into ..` lands, each under the + directory the person stands in, whose name can hold an ESC or a line + break. `--into {parent}` is `..` and `..`s, or on Windows a path no + control character can be in, and is named as it is. """ checkout = _standard_checkout(into) if checkout is None: @@ -1464,15 +1476,16 @@ def _refuse_landing_in_standard(into: Path, holder: Path, raise Refusal( "family-landing-in-standard", f"with neither --into nor --work-dir the holder lands where you are " - f"standing, at {holder}, which is inside {checkout}, a checkout of " - "openRepoShape itself (it holds scripts/family.py and " - "templates/family-root/). The family would be part of the " - "standard's tree, and `make siblings` would clone every member into " - "it too. Nothing was created, and `--dry-run` refuses the same.", + f"standing, at {shown_path(holder)}, which is inside " + f"{shown_path(checkout)}, a checkout of openRepoShape itself (it " + "holds scripts/family.py and templates/family-root/). The family " + "would be part of the standard's tree, and `make siblings` would " + "clone every member into it too. Nothing was created, and " + "`--dry-run` refuses the same.", f"Remediation: pass `--into {parent}` to land it beside this " - f"checkout, at {beside / family / family}, or `--into ` for any " - "other parent directory; `--work-dir ` is the override " - "(`/`, no family folder). There is no --force.") + f"checkout, at {shown_path(beside / family / family)}, or `--into " + "` for any other parent directory; `--work-dir ` is the " + "override (`/`, no family folder). There is no --force.") def _landing(args, family: str) -> tuple[Path | None, Path]: @@ -1506,12 +1519,15 @@ def _landing(args, family: str) -> tuple[Path | None, Path]: # `os.path.isfile`, which answers False for a holder this user cannot # search: `Path.is_file()` raised `PermissionError` there, a traceback # before the question about the holder (`_refuse_a_taken_holder`) that - # names it (#609's rule: a refusal by name, never a traceback). + # names it (#609's rule: a refusal by name, never a traceback). The two + # paths are named as `shown_path` names a path (#766): `--into`, or the + # directory the person stands in, can hold an ESC or a line break. if into.name == family and os.path.isfile(into / family / MANIFEST): raise Refusal( "family-holder-already-here", - f"{into} is already the {family} family folder and its holder " - f"is at {into / family}, so there is nothing to init", + f"{shown_path(into)} is already the {family} family folder and " + f"its holder is at {shown_path(into / family)}, so there is " + "nothing to init", f"Remediation: `{_add_command(into / family, args.org)}` grows " "it, and `make siblings` in the holder puts each member's " "working clone beside it. There is no --force.") @@ -1802,6 +1818,13 @@ def _print_init_plan(values: dict, repository: str, url: str, and what a real run shows before it does anything. Split from `cmd_init` for #136. + + THE FOLDER AND THE HOLDER ARE NAMED AS `shown_path` NAMES A PATH (#766). + Both lie under `--into`, `--work-dir` or the directory the person + stands in, and the plan is on stdout before any refusal, so a name with + a line break and then `WARNING forged-line: pwned` printed that line as + a line of the plan, on a dry run and on a run refused after it, and an + ESC reached the terminal. A printable path is named byte for byte. """ print(f"family {values['FAMILY_NAME']} ({values['FAMILY_ID']})") print(f"shape {SHAPE_REPOSITORY} @ {shape_commit[:12]} " @@ -1809,10 +1832,13 @@ def _print_init_plan(values: dict, repository: str, url: str, print(f"created by {values['CREATED_BY']} on {values['CREATED_ON']}") print(f" holder {repository:<28} -> {url}") if folder is None: - print(f"landing {holder} (--work-dir: no family folder)") + print(f"landing {shown_path(holder)} (--work-dir: no family " + "folder)") else: - print(f"folder {folder} a plain folder, NOT a repository") - print(f"landing {holder} the holder clone, inside it") + print(f"folder {shown_path(folder)} a plain folder, NOT a " + "repository") + print(f"landing {shown_path(holder)} the holder clone, inside " + "it") print(f"members mounted under {MEMBERS_DIR}/; none yet — " "`family.py add` puts one there") print("remotes " + ("a bare repository on disk (no network)" if local @@ -1908,6 +1934,12 @@ def _checked_reuse(reuse_empty_repo: bool, local: bool, url: str, Refused when the remote already exists WITH commits, or exists empty without `--reuse-empty-repo`. Split from `cmd_init` for #136. + + A LOCAL REMOTE IS NAMED AS `shown_path` NAMES A PATH (#766): it is a + path under `--local-remote-dir`. `_refuse_an_unwritable_clone_url` + refuses a control character there first, so only a character that is + not printable and not refused (a zero-width space) reaches this from + the command line. A GitHub `/` is named as it is. """ # THE ONE REPOSITORY THAT MAY ALREADY EXIST is the holder, and only with # --reuse-empty-repo, and only with ZERO commits. `InkRouter` in the @@ -1920,7 +1952,8 @@ def _checked_reuse(reuse_empty_repo: bool, local: bool, url: str, if not (reuse_empty_repo and empty): raise Refusal( "family-remote-exists", - f"{target} already exists" + ("" if local else " on GitHub") + f"{shown_path(target) if local else target} already exists" + + ("" if local else " on GitHub") + ("" if empty else " and has commits"), ("Remediation: it has ZERO commits, so it is a reserved name " "rather than a live repository — re-run with " @@ -6348,6 +6381,10 @@ def _checked_index_unlocked(root: Path, path: str) -> None: `.git/index.lock`. A lock taken after this check is the rollback's to meet. The dry run does not ask: it writes nothing, and a lock is a fact about the moment. + + THE DETAIL NAMES THE LOCK AS `shown_path` NAMES A PATH (#766): it lies + under the holder, whose path can hold an ESC or a line break. The + remedy's `rm -f` is quoted for a shell, as it was. """ shown = _git_bytes(["rev-parse", "--git-path", "index.lock"], root).decode("utf-8", "surrogateescape").strip() @@ -6356,11 +6393,11 @@ def _checked_index_unlocked(root: Path, path: str) -> None: return raise Refusal( "family-index-locked", - f"the holder's index is locked: {lock.as_posix()} exists. `git " - f"submodule deinit` takes no such lock, so `remove` would empty " - f"{path} and drop its registration and then fail at `git rm`, its " - "first write to the index; it was refused before either, and " - "nothing was written.", + f"the holder's index is locked: {shown_path(lock.as_posix())} " + "exists. `git submodule deinit` takes no such lock, so `remove` " + f"would empty {path} and drop its registration and then fail at " + "`git rm`, its first write to the index; it was refused before " + "either, and nothing was written.", "Remediation: another git command is running in the holder, or one " "that stopped left the lock behind. Let it finish and run the same " "command again; if no git command is running, delete the lock (`rm " diff --git a/tests/conftest.py b/tests/conftest.py index 63655f76..25ba0954 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1912,3 +1912,81 @@ def assert_step_four_speaks_in_the_plans_words(run, tmp_path: Path, f"({FIRST_LINK_WORDS}). The recorded chain still classifies." ), offered assert not (tmp_path / "remotes").exists() + + +# --- a path a line names, spelled safe to show (#692, #766) ------------------ +# +# A path a person supplied, or one git keeps, can hold an ESC, which a +# terminal acts on, or a line break and then a line that is not the tool's. +# A line that names such a path names it as `repo_shape.shown_path` does: as +# it is, unless a character in it is not printable, and then as Python's +# `repr` spells it. #692 gave the folder refusals that spelling and #766 the +# plan rows and refusals beside them; their tests hand the tools these names +# and read what comes back with these helpers. + +#: Folder names that are not safe to show raw: an ESC, and a line break then +#: a line that is not the tool's. +ESC_NAME = "wd-\x1b[31mRED" +FORGED_NAME = "wd-\nWARNING forged-line: pwned" + +#: A command line and a file name on Windows hold none of these. +POSIX_NAMES = pytest.mark.skipif( + os.name == "nt", + reason="needs a folder name holding a control character, which a " + "Windows file name cannot hold") + +#: The two names, for `parametrize`, by the ids #692's rows use. +UNSAFE_NAMES = [pytest.param(ESC_NAME, id="ESC", marks=POSIX_NAMES), + pytest.param(FORGED_NAME, id="line-break", marks=POSIX_NAMES)] + + +def spelt(path) -> str: + """A path that holds a control character as a line names it: as + Python's `repr` spells it, quotes and `\\x1b` and `\\n` and all. It is + `repr`, and not `shown_path`, so a change to that function moves no + expectation with it.""" + return repr(str(path)) + + +def unsafe_folder(parent: Path, name: str) -> Path: + """`parent / name`, made with every folder above it, or the test skipped + where the disk refuses the name.""" + folder = parent / name + try: + folder.mkdir(parents=True, exist_ok=True) + except (OSError, UnicodeError) as exc: + pytest.skip(f"this filesystem cannot hold the name: {exc}") + return folder + + +def copied_under(source: Path, parent: Path, name: str) -> Path: + """A private copy of the repository `source`, under its own name, in a + folder `name` made under `parent` (`unsafe_folder`): a holder or an + assembly root whose path holds that name. A copy keeps a submodule's + `.git` file, a relative path, true.""" + target = unsafe_folder(parent, name) / source.name + shutil.copytree(source, target, symlinks=True) + return target.resolve() + + +def assert_shown_safe(*texts: str) -> None: + """No ESC in any of `texts`, and no line of them that starts `WARNING + forged-line`: no path reached the terminal raw.""" + for text in texts: + assert "\x1b" not in text, ascii(text) + assert not any(line.startswith("WARNING forged-line") + for line in text.splitlines()), text + + +def refusal_detail(stderr: str, code: str) -> list[str]: + """The lines refusal `code` prints before its `Remediation:` line: the + `REFUSED` line, and any line its detail goes on to, such as a list of + file names. A path that broke the detail in two adds a line here.""" + lines = stderr.splitlines() + starts = [at for at, line in enumerate(lines) + if line.startswith(f"REFUSED {code}: ")] + assert len(starts) == 1, stderr + ends = [at for at in range(starts[0], len(lines)) + if lines[at].startswith("Remediation:")] + assert ends, stderr + return lines[starts[0]:ends[0]] diff --git a/tests/test_bump_leg_branch_and_push_line.py b/tests/test_bump_leg_branch_and_push_line.py index 11dc4bdd..708b215e 100644 --- a/tests/test_bump_leg_branch_and_push_line.py +++ b/tests/test_bump_leg_branch_and_push_line.py @@ -32,6 +32,11 @@ they already refuse it exits 0 and the real run ends `bump-leg-validators-red`. Its closing paragraph says so. +THE ROOT ABOVE THEM (#766). The three refusals and the dry run's `the real +run needs a branch` line printed the root as it was, so a root under a name +with an ESC, a line break or a bidi override sent it to the terminal raw. +Each names it as `repo_shape.shown_path` does now. + NO NETWORK, NO `gh`: the bare remotes are the session's own, and every root here pushes to a private copy of its remote (`make_root`), so a push reaches nothing another test reads. Every printed line is run as the argv a POSIX @@ -48,6 +53,9 @@ import pytest from conftest import FILE_PROTOCOL, PROJECT, REPO, cloned_just_pushed, git +#: The names no line may print raw, and how a line spells them (#766). +from conftest import (POSIX_NAMES, UNSAFE_NAMES, assert_shown_safe, + copied_under, refusal_detail, spelt) # A fixture is used by being named in this module's namespace, which is what # the imports of `bump_leg_module` and `make_root` are for. from test_bump_leg import ( # noqa: F401 @@ -518,3 +526,81 @@ def test_a_dry_run_says_the_validators_are_the_real_runs(project, scaffolded, assert "REFUSED bump-leg-validators-red" in real.stderr, real.stderr assert head_of(project) == head assert git("status", "--porcelain", cwd=project).stdout == "" + + +# --- #766: the root, named safe to show ------------------------------------- + +#: The names a root is put under: an ESC, a line break then a forged line, +#: and rv672's measured name, a bidi override and a C1 control. +ROOT_NAMES = [*UNSAFE_NAMES, + pytest.param("r\u202eo\u009bot", id="bidi-and-C1", + marks=POSIX_NAMES)] + + +@pytest.mark.parametrize("extra", [(), ("--dry-run",)], + ids=["bump", "dry-run"]) +@pytest.mark.parametrize("name", ROOT_NAMES) +def test_a_dirty_root_is_named_safe_to_show(scaffolded, advanced, tmp_path, + name, extra): + """#766's third line, measured by rv672 on #672's head: under a root + named `root`, `bump-leg-root-dirty` put both characters + raw into its first line, above the file names #672 spells. The root is + named as `repr` spells it now, by the real run and the dry run alike, + and nothing on either stream holds the name raw.""" + root = branched(copied_under(scaffolded["clone"], tmp_path, name)) + readme = root / "README.md" + readme.write_bytes(readme.read_bytes() + b"dirty\n") + result = bump(root, scaffolded, advanced, *extra) + assert result.returncode == 2, result.stderr + result.stdout + assert refusal_detail(result.stderr, "bump-leg-root-dirty") == [ + f"REFUSED bump-leg-root-dirty: {spelt(root)} has uncommitted " + "changes:", " M README.md"] + lines = (result.stdout + result.stderr).splitlines() + assert [raw_controls(name, line) for line in lines + if raw_controls(name, line)] == [], ascii(result.stderr) + assert_shown_safe(result.stdout, result.stderr) + + +@pytest.mark.parametrize("state", ["tracking-branch", "detached"]) +@pytest.mark.parametrize("name", UNSAFE_NAMES) +def test_a_root_refused_for_its_branch_is_named_safe_to_show( + scaffolded, advanced, tmp_path, name, state): + """#766: `bump-leg-on-tracking-branch` and `bump-leg-root-detached` + named the root as it was. The detail names it as `repr` spells it, on + the one line it is. The remedy's `switch -c` is a command to paste, + quoted for a shell as it was (`_switch_command`), and is not this + row's.""" + root = copied_under(scaffolded["clone"], tmp_path, name) + if state == "detached": + git("checkout", "-q", "--detach", cwd=root) + result = bump(root, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert result.stdout == "" + code, said = ( + ("bump-leg-root-detached", + "is not on a branch, so there is no branch to push and open a pull " + "request from") + if state == "detached" else + ("bump-leg-on-tracking-branch", + "is on 'main', which project.yaml declares as this project's " + "tracking branch")) + assert refusal_detail(result.stderr, code) == [ + f"REFUSED {code}: {spelt(root)} {said}"] + + +@pytest.mark.parametrize("name", UNSAFE_NAMES) +def test_the_dry_runs_branch_line_names_the_root_safe_to_show( + scaffolded, advanced, tmp_path, name): + """#766: on the tracking branch the dry run ends `the real run needs a + branch: is on 'main'`, and named the root as it was. It names it + as `repr` spells it. The `switch -c` after it is a command to paste, + quoted for a shell as it was, and is not this row's.""" + root = copied_under(scaffolded["clone"], tmp_path, name) + result = bump(root, scaffolded, advanced, "--dry-run") + assert result.returncode == 0, result.stderr + result.stdout + said = [line for line in result.stdout.splitlines() + if line.startswith("the real run needs a branch: ")] + assert len(said) == 1, result.stdout + assert said[0].startswith( + f"the real run needs a branch: {spelt(root)} is on 'main', the " + "tracking branch. Create one first with: git -C "), said diff --git a/tests/test_dry_run_asks_remotes.py b/tests/test_dry_run_asks_remotes.py index 31c87771..52498b84 100644 --- a/tests/test_dry_run_asks_remotes.py +++ b/tests/test_dry_run_asks_remotes.py @@ -37,6 +37,9 @@ import pytest from conftest import ORG, REPO, SCAFFOLD, WINDOWS_SKIP, git, run_script +#: The names no line may print raw, and how a line spells them (#766). +from conftest import (UNSAFE_NAMES, assert_shown_safe, refusal_detail, + spelt, unsafe_folder) from test_scaffold_reuse_seeded_legs import (REPOS, bare, commits, refused_at_the_root, tip) from test_ruleset_hint_tracking_branch import template # noqa: F401 @@ -482,3 +485,51 @@ def test_a_real_run_with_no_gh_is_refused_in_the_dry_runs_words( assert refusal in captured.err, captured.err assert "put `gh` on PATH, logged in" in captured.err assert asked == [first] + + +# --- #766: a local remote that exists, named safe to show ------------------- + +def remote_exists_in_process(tool: str, remote: Path): + """The refusal the tool's own question raises for `remote`, a local + remote with commits, asked in process: `family.py`'s `_checked_reuse` + for its holder, or the scaffold's `_reusable_remotes` for its root.""" + if tool == "family": + module = load("family_766", FAMILY) + with pytest.raises(module.Refusal) as refused: + module._checked_reuse(False, True, str(remote), f"{ORG}/{HOLDER}") + else: + module = load("scaffold_766", SCAFFOLD) + role = "assembly" + with pytest.raises(module.Refusal) as refused: + module._reusable_remotes( + {role: PROJECT}, {role: str(remote)}, + {role: f"{ORG}/{PROJECT}"}, True, False, {}, "main") + return refused.value + + +@pytest.mark.parametrize("name", UNSAFE_NAMES) +@pytest.mark.parametrize("tool", ["family", "scaffold"]) +def test_a_local_remote_that_exists_is_named_safe_to_show(tmp_path, tool, + name): + """#766: `family-remote-exists` and `scaffold-remote-exists` named a + local remote as it was, a path under `--local-remote-dir`. From the + command line a name with an ESC or a line break there never reaches + them: the clone URL is refused first, as `-unwritable-value`, + which names it as `repr` spells it, and nothing on either stream holds + the name raw. So each question is asked in process too, as #692 asks + `make_folder`, and names the remote as `repr` spells it.""" + remotes = unsafe_folder(tmp_path, name).resolve() + remote = live_bare( + remotes / f"{HOLDER if tool == 'family' else PROJECT}.git") + run = family_init if tool == "family" else scaffold + result = run(tmp_path, "--local-remote-dir", str(remotes), local=False) + assert result.returncode == 2, result.stdout + result.stderr + refused = refusal_detail(result.stderr, f"{tool}-unwritable-value") + assert f"own path, {spelt(remote)}, which " in refused[0], refused + assert_shown_safe(result.stdout, result.stderr) + + refusal = remote_exists_in_process(tool, remote) + assert refusal.code == f"{tool}-remote-exists" + assert refusal.detail == ( + f"{spelt(remote)} already exists and has commits") + assert_shown_safe(refusal.detail, refusal.remediation) diff --git a/tests/test_dry_run_parity.py b/tests/test_dry_run_parity.py index 89cad33a..46281ef9 100644 --- a/tests/test_dry_run_parity.py +++ b/tests/test_dry_run_parity.py @@ -59,6 +59,9 @@ import pytest from conftest import ORG, REPO, SCAFFOLD, WINDOWS_SKIP, run_script +#: The names #692's rows hold, and how a line spells them, kept beside the +#: helpers #766's rows in other modules share (#766). +from conftest import ESC_NAME, FORGED_NAME, POSIX_NAMES, spelt from test_dry_run_asks_remotes import (DRY_RUN_DONE, live_bare, load, no_gh_run, scratch_env, without_gh) @@ -72,17 +75,6 @@ SCAFFOLD_FOLDER = "REFUSED scaffold-folder-cannot-be-made" FAMILY_FOLDER = "REFUSED family-folder-cannot-be-made" -#: A command line and a file name on Windows hold none of these. -POSIX_NAMES = pytest.mark.skipif( - os.name == "nt", - reason="needs a folder name holding a control character, which a " - "Windows file name cannot hold") - -#: Folder names that are not safe to show raw (#692): an ESC, which a -#: terminal acts on, and a line break then a line that is not the tool's. -ESC_NAME = "wd-\x1b[31mRED" -FORGED_NAME = "wd-\nWARNING forged-line: pwned" - def touch(path: Path) -> None: """A file at `path`, somebody's, made with every folder above it.""" @@ -205,14 +197,6 @@ def at(tmp_path: Path, relative: str) -> Path: return (tmp_path / relative).resolve() -def spelt(path: Path) -> str: - """A path that holds a control character as a refusal names it (#692): - as Python's `repr` spells it, quotes and `\\x1b` and `\\n` and all. It - is `repr`, and not `shown_source`, so a change to that function moves no - expectation with it.""" - return repr(str(path)) - - def case_params(cases: dict) -> list: """One `pytest.param` per row of `cases`, a row named `ESC-...` or `line-break-...` skipped where no file can be named so (`POSIX_NAMES`).""" diff --git a/tests/test_family_bump_remove_rollback.py b/tests/test_family_bump_remove_rollback.py index 3dcfe47c..153722dd 100644 --- a/tests/test_family_bump_remove_rollback.py +++ b/tests/test_family_bump_remove_rollback.py @@ -78,6 +78,8 @@ BUMP_ORDER, commit_in_mount, isolated, registered_keys, state, ) from test_family_member_path import MEMBER, OTHER, older, raw +#: The names no line may print raw, and how a line spells them (#766). +from conftest import UNSAFE_NAMES, copied_under, refusal_detail, spelt PATH = f"members/{MEMBER}" @@ -426,6 +428,30 @@ def test_a_remove_dry_run_under_a_held_index_lock_is_unchanged( "changed") +@pytest.mark.parametrize("name", UNSAFE_NAMES) +def test_a_held_index_lock_is_named_safe_to_show(family, offline, tmp_path, + name): + """#766: the detail named the lock as it was, under the holder, so a + holder under a name with an ESC sent it to the terminal, and one with a + line break printed a forged line in the detail. It names the lock as + `repr` spells it, on the one line it is. The remedy's `rm -f` is a + command to paste, quoted for a shell as it was, and is not this row's.""" + holder = copied_under(family["root"], tmp_path, name) + env = isolated(offline, tmp_path) + lock = index_lock(holder) + lock.write_bytes(b"") + result = remove(holder, env) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "" + assert lock.is_file(), "the lock is left where it was" + assert refusal_detail(result.stderr, "family-index-locked") == [ + "REFUSED family-index-locked: the holder's index is locked: " + f"{spelt(lock.resolve().as_posix())} exists. `git submodule deinit` " + f"takes no such lock, so `remove` would empty {PATH} and drop its " + "registration and then fail at `git rm`, its first write to the " + "index; it was refused before either, and nothing was written."] + + # --- refusals a dry run asks first are asked first (#365) ----------------- def comment_on_members(root: Path, env: dict) -> None: diff --git a/tests/test_family_init_landing.py b/tests/test_family_init_landing.py index 73c2c6be..0eda7fd0 100644 --- a/tests/test_family_init_landing.py +++ b/tests/test_family_init_landing.py @@ -22,6 +22,12 @@ under the test's own temporary directory (the two paths the detection reads, and nothing else), so a regression writes there and never into the repository under test. NO NETWORK: every remote is `--local-remote-dir`. + +THE PATHS ARE NAMED SAFE TO SHOW (#766). The plan's `folder` and `landing` +rows, `family-landing-in-standard` and `family-holder-already-here` name a +path under the directory the person gave or stands in, and printed it as it +was. Each names it as `repo_shape.shown_path` does now: as it is, or as +`repr` spells it where a character in it is not printable. """ from __future__ import annotations @@ -34,6 +40,9 @@ import pytest from conftest import REPO, run_script +#: The names no line may print raw, and how a line spells them (#766). +from conftest import (UNSAFE_NAMES, assert_shown_safe, refusal_detail, + spelt, unsafe_folder) sys.path.insert(0, str(REPO / "scripts")) from repo_shape import PYTHON # noqa: E402 @@ -196,3 +205,111 @@ def test_the_printed_next_runs_from_where_init_was_run(tmp_path, where): check=False) assert helped.returncode == 0, helped.stderr assert helped.stdout.startswith("usage: family.py add"), helped.stdout + + +# --- #766: the paths the plan and `_landing`'s refusals name ---------------- + +def planned_rows(stdout: str) -> list[str]: + """The plan's `folder` and `landing` rows, in the order printed.""" + return [line for line in stdout.splitlines() + if line.startswith(("folder ", "landing "))] + + +@pytest.mark.parametrize("name", UNSAFE_NAMES) +@pytest.mark.parametrize("flag", ["--work-dir", "--into"]) +def test_the_plan_names_the_folder_and_the_holder_safe_to_show( + tmp_path, flag, name): + """#766's first line, measured on #692's head: the plan printed the + family folder and the holder as they were, on stdout before any + refusal. A `--work-dir` or `--into` holding a line break and then + `WARNING forged-line: pwned` printed that line as a line of the plan, + on a dry run and on a run refused after the plan, and an ESC reached the + terminal. Each row names its path as `repr` spells it now, in both + runs, and nothing else on either stream holds the name raw.""" + place = unsafe_folder(tmp_path, name).resolve() + if flag == "--work-dir": + holder = place / NAME + rows = [f"landing {spelt(holder)} (--work-dir: no family " + "folder)"] + else: + holder = place / NAME / NAME + rows = [f"folder {spelt(place / NAME)} a plain folder, NOT a " + "repository", + f"landing {spelt(holder)} the holder clone, inside it"] + dry = init(tmp_path, tmp_path, flag, str(place), "--dry-run") + assert dry.returncode == 0, dry.stdout + dry.stderr + # A file where the holder goes: the real run prints the same plan, and + # is refused after it (`family-target-exists`, #692's spelling). + holder.parent.mkdir(parents=True, exist_ok=True) + holder.write_bytes(b"somebody's\n") + real = init(tmp_path, tmp_path, flag, str(place)) + assert real.returncode == 2, real.stdout + real.stderr + assert refusal_detail(real.stderr, "family-target-exists") == [ + f"REFUSED family-target-exists: {spelt(holder)} already exists and " + "is a file, not a folder"] + for result in (dry, real): + assert planned_rows(result.stdout) == rows, result.stdout + assert_shown_safe(result.stdout, result.stderr) + + +def test_the_plan_names_a_printable_folder_byte_for_byte(tmp_path): + """#766 changes no row a person could read before: a space and a letter + that is not ASCII are printable, so the folder and the holder are named + as they are, with no quotes.""" + place = tmp_path / "wd é and a space" + place.mkdir() + place = place.resolve() + result = init(tmp_path, tmp_path, "--into", str(place), "--dry-run") + assert result.returncode == 0, result.stdout + result.stderr + assert planned_rows(result.stdout) == [ + f"folder {place / NAME} a plain folder, NOT a repository", + f"landing {place / NAME / NAME} the holder clone, inside it"] + + +@pytest.mark.parametrize("name", UNSAFE_NAMES) +def test_landing_in_a_checkout_names_its_three_paths_safe_to_show( + tmp_path, name): + """#766: `family-landing-in-standard` named the holder, the checkout and + where `--into ..` lands as they were, each under the directory the + person stands in, so a checkout under a name with a line break printed + a forged line in the detail and in the remedy. Each is named as `repr` + spells it, by the dry run and the real run alike, and the whole refusal + is safe to show: `--into ..` holds no name.""" + checkout = fake_checkout(unsafe_folder(tmp_path, name) / "std").resolve() + for extra in (("--dry-run",), ()): + result = init(checkout, tmp_path, *extra) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "" + assert refusal_detail(result.stderr, CODE) == [ + f"REFUSED {CODE}: with neither --into nor --work-dir the holder " + "lands where you are standing, at " + f"{spelt(checkout / NAME / NAME)}, which is inside " + f"{spelt(checkout)}, a checkout of openRepoShape itself (it " + "holds scripts/family.py and templates/family-root/). " + "The family would be part of the standard's tree, and `make " + "siblings` would clone every member into it too. Nothing was " + "created, and `--dry-run` refuses the same."] + assert ("Remediation: pass `--into ..` to land it beside this " + f"checkout, at {spelt(checkout.parent / NAME / NAME)}, or " + "`--into `" in result.stderr), result.stderr + assert_shown_safe(result.stdout, result.stderr) + assert sorted(path.name for path in checkout.iterdir()) == [ + "scripts", "templates"] + + +@pytest.mark.parametrize("name", UNSAFE_NAMES) +def test_a_family_folder_already_here_is_named_safe_to_show(tmp_path, name): + """#766: `family-holder-already-here` named `--into` and the holder in + it as they were. The detail names both as `repr` spells them, on the + one line it is. The remedy's `add` line is a command to paste, quoted + for a shell as it was (`_add_command`), and is not this row's.""" + into = unsafe_folder(tmp_path, name).resolve() / NAME + (into / NAME).mkdir(parents=True) + (into / NAME / "family.yaml").write_bytes(b"name: Orbit\n") + result = init(tmp_path, tmp_path, "--into", str(into), "--dry-run") + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "" + assert refusal_detail(result.stderr, "family-holder-already-here") == [ + f"REFUSED family-holder-already-here: {spelt(into)} is already the " + f"{NAME} family folder and its holder is at {spelt(into / NAME)}, so " + "there is nothing to init"] diff --git a/tests/test_family_tracking_branch.py b/tests/test_family_tracking_branch.py index 93467d12..c7ac18d2 100644 --- a/tests/test_family_tracking_branch.py +++ b/tests/test_family_tracking_branch.py @@ -39,6 +39,8 @@ import pytest from conftest import REPO, git, run_script +#: The names no line may print raw, and how a line spells them (#766). +from conftest import UNSAFE_NAMES, copied_under, refusal_detail, spelt #: The family fixture, its names and the work branch it grows the holder on, #: imported from the module that owns them. from test_family import ( # noqa: F401 @@ -322,3 +324,37 @@ def test_each_doc_makes_the_branch_before_the_add(doc): add = text.index("python3 scripts/family.py add ") block = text[text.rindex("```", 0, add):add] assert " switch -c family/add-" in block, block + + +# --- #766: the holder each refusal names, safe to show ----------------------- + +@pytest.mark.parametrize("name", UNSAFE_NAMES) +@pytest.mark.parametrize("detached", [False, True], + ids=["tracking-branch", "detached"]) +def test_a_holder_refused_for_its_branch_is_named_safe_to_show( + family, tmp_path, detached, name): + """#766: `family-on-tracking-branch` and `family-holder-detached` named + the holder as it was, so a holder under a name with an ESC sent it to + the terminal, and one with a line break printed a forged line in the + detail. The detail names it as `repr` spells it, on the one line it is. + The remedy's `switch -c` is a command to paste, quoted for a shell as + it was (`_switch_command`), and is not this row's.""" + holder = copied_under(family["root"], tmp_path, name) + git("switch", "-q", "-C", TRACKING, cwd=holder) + if detached: + git("checkout", "-q", "--detach", cwd=holder) + result = act(family, holder, "remove") + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "" + code, said = ( + ("family-holder-detached", + "is not on a branch, so `remove` would write a commit no branch " + "holds, with nothing to push and open a pull request from") + if detached else + ("family-on-tracking-branch", + f"is on '{TRACKING}', which family.yaml declares as the holder's " + "tracking branch, and `remove` will not write a commit whose only " + "next step is a push to the default branch")) + assert refusal_detail(result.stderr, code) == [ + f"REFUSED {code}: {spelt(holder)} {said}. No git command that writes " + "was run and nothing was written."] From ef6db40da126dadc065e2e905ffa74fdcd9a313a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 12:02:27 +0000 Subject: [PATCH 09/10] family.py and #766's tests: `family-holder-already-here` and `bump-leg-root-detached` are held to a printable path named unquoted, byte for byte, and two docstrings say the remedy's command still holds the path raw (#766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rv766's review of PR #814, fix round 1. SHOULD 1: three mutants survived. Each replaces `shown_path(X)` with `repr(str(X))`, so a printable path gains quotes: - P8 and P9: the two paths of `family-holder-already-here`; - P14: the root of `bump-leg-root-detached`. No row held a printable path there. `tests/conftest.py` now keeps `PRINTABLE_NAME` (`wd é and a space`, a name `str.isprintable` accepts), `SHOWN_NAMES` (the two unsafe names and that one) and `named_as`, which spells a path as a line names it under each. The two tests take `SHOWN_NAMES` as their parameter, so each gains a printable case that asserts the path unquoted, byte for byte. P8 and P9 fail `test_a_family_folder_already_here_is_named_safe_to_show[printable]`, and P14 fails `test_a_root_refused_for_its_branch_is_named_safe_to_show[printable-detached]`. `test_the_plan_names_a_printable_folder_byte_for_byte` names its folder by `PRINTABLE_NAME` too, where the source held a raw `é`. NIT 1: two docstrings in `scripts/family.py` now also say that the remedy's command still holds the path raw inside its shell quotes (#459). They are `_checked_branch` (`switch -c`) and `_checked_index_unlocked` (`rm -f`). No code changed. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 7 +++++-- tests/conftest.py | 16 ++++++++++++++++ tests/test_bump_leg_branch_and_push_line.py | 16 +++++++++------- tests/test_family_init_landing.py | 20 ++++++++++++-------- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 23450150..2b0f7eff 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -764,7 +764,9 @@ def _checked_branch(family: "Family", verb: str, suggested: str) -> str: THE HOLDER IS NAMED AS `shown_path` NAMES A PATH (#766), as #692 names the folders `init` makes: a holder whose path held an ESC sent it to the terminal, and one with a line break began a line of its own choosing. - A printable path is named byte for byte.""" + A printable path is named byte for byte. The remedy's `switch -c` is + a command to paste, and still holds the holder's path raw inside its + shell quotes (#459).""" branch = _holder_branch(family.root) if branch is not None and branch != family.tracking(): return branch @@ -6384,7 +6386,8 @@ def _checked_index_unlocked(root: Path, path: str) -> None: THE DETAIL NAMES THE LOCK AS `shown_path` NAMES A PATH (#766): it lies under the holder, whose path can hold an ESC or a line break. The - remedy's `rm -f` is quoted for a shell, as it was. + remedy's `rm -f` is quoted for a shell, as it was, and still holds + the lock's path raw inside those quotes (#459). """ shown = _git_bytes(["rev-parse", "--git-path", "index.lock"], root).decode("utf-8", "surrogateescape").strip() diff --git a/tests/conftest.py b/tests/conftest.py index 25ba0954..c955683e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1939,6 +1939,15 @@ def assert_step_four_speaks_in_the_plans_words(run, tmp_path: Path, UNSAFE_NAMES = [pytest.param(ESC_NAME, id="ESC", marks=POSIX_NAMES), pytest.param(FORGED_NAME, id="line-break", marks=POSIX_NAMES)] +#: A name a line names as it is, byte for byte, with no quotes: a space and +#: a letter that is not ASCII are each a character `str.isprintable` +#: accepts. +PRINTABLE_NAME = "wd \u00e9 and a space" + +#: `UNSAFE_NAMES` and `PRINTABLE_NAME`, for a row that holds both kinds +#: (rv766's SHOULD 1: a quoted printable path was caught by no row). +SHOWN_NAMES = [*UNSAFE_NAMES, pytest.param(PRINTABLE_NAME, id="printable")] + def spelt(path) -> str: """A path that holds a control character as a line names it: as @@ -1948,6 +1957,13 @@ def spelt(path) -> str: return repr(str(path)) +def named_as(path, name: str) -> str: + """`path`, made under the folder `name`, as a line names it: as it is, + byte for byte, under `PRINTABLE_NAME`, and `spelt` under one of + `UNSAFE_NAMES`.""" + return str(path) if name == PRINTABLE_NAME else spelt(path) + + def unsafe_folder(parent: Path, name: str) -> Path: """`parent / name`, made with every folder above it, or the test skipped where the disk refuses the name.""" diff --git a/tests/test_bump_leg_branch_and_push_line.py b/tests/test_bump_leg_branch_and_push_line.py index 708b215e..52e35ac3 100644 --- a/tests/test_bump_leg_branch_and_push_line.py +++ b/tests/test_bump_leg_branch_and_push_line.py @@ -54,8 +54,9 @@ from conftest import FILE_PROTOCOL, PROJECT, REPO, cloned_just_pushed, git #: The names no line may print raw, and how a line spells them (#766). -from conftest import (POSIX_NAMES, UNSAFE_NAMES, assert_shown_safe, - copied_under, refusal_detail, spelt) +from conftest import (POSIX_NAMES, SHOWN_NAMES, UNSAFE_NAMES, + assert_shown_safe, copied_under, named_as, + refusal_detail, spelt) # A fixture is used by being named in this module's namespace, which is what # the imports of `bump_leg_module` and `make_root` are for. from test_bump_leg import ( # noqa: F401 @@ -562,14 +563,15 @@ def test_a_dirty_root_is_named_safe_to_show(scaffolded, advanced, tmp_path, @pytest.mark.parametrize("state", ["tracking-branch", "detached"]) -@pytest.mark.parametrize("name", UNSAFE_NAMES) +@pytest.mark.parametrize("name", SHOWN_NAMES) def test_a_root_refused_for_its_branch_is_named_safe_to_show( scaffolded, advanced, tmp_path, name, state): """#766: `bump-leg-on-tracking-branch` and `bump-leg-root-detached` named the root as it was. The detail names it as `repr` spells it, on - the one line it is. The remedy's `switch -c` is a command to paste, - quoted for a shell as it was (`_switch_command`), and is not this - row's.""" + the one line it is, and a printable root as it is, byte for byte, with + no quotes (rv766's P14). The remedy's `switch -c` is a command to + paste, quoted for a shell as it was (`_switch_command`), and is not + this row's.""" root = copied_under(scaffolded["clone"], tmp_path, name) if state == "detached": git("checkout", "-q", "--detach", cwd=root) @@ -585,7 +587,7 @@ def test_a_root_refused_for_its_branch_is_named_safe_to_show( "is on 'main', which project.yaml declares as this project's " "tracking branch")) assert refusal_detail(result.stderr, code) == [ - f"REFUSED {code}: {spelt(root)} {said}"] + f"REFUSED {code}: {named_as(root, name)} {said}"] @pytest.mark.parametrize("name", UNSAFE_NAMES) diff --git a/tests/test_family_init_landing.py b/tests/test_family_init_landing.py index 0eda7fd0..8656d35b 100644 --- a/tests/test_family_init_landing.py +++ b/tests/test_family_init_landing.py @@ -41,7 +41,8 @@ from conftest import REPO, run_script #: The names no line may print raw, and how a line spells them (#766). -from conftest import (UNSAFE_NAMES, assert_shown_safe, refusal_detail, +from conftest import (PRINTABLE_NAME, SHOWN_NAMES, UNSAFE_NAMES, + assert_shown_safe, named_as, refusal_detail, spelt, unsafe_folder) sys.path.insert(0, str(REPO / "scripts")) @@ -256,7 +257,7 @@ def test_the_plan_names_a_printable_folder_byte_for_byte(tmp_path): """#766 changes no row a person could read before: a space and a letter that is not ASCII are printable, so the folder and the holder are named as they are, with no quotes.""" - place = tmp_path / "wd é and a space" + place = tmp_path / PRINTABLE_NAME place.mkdir() place = place.resolve() result = init(tmp_path, tmp_path, "--into", str(place), "--dry-run") @@ -297,12 +298,14 @@ def test_landing_in_a_checkout_names_its_three_paths_safe_to_show( "scripts", "templates"] -@pytest.mark.parametrize("name", UNSAFE_NAMES) +@pytest.mark.parametrize("name", SHOWN_NAMES) def test_a_family_folder_already_here_is_named_safe_to_show(tmp_path, name): """#766: `family-holder-already-here` named `--into` and the holder in it as they were. The detail names both as `repr` spells them, on the - one line it is. The remedy's `add` line is a command to paste, quoted - for a shell as it was (`_add_command`), and is not this row's.""" + one line it is, and a printable path as it is, byte for byte, with no + quotes (rv766's P8 and P9). The remedy's `add` line is a command to + paste, quoted for a shell as it was (`_add_command`), and is not this + row's.""" into = unsafe_folder(tmp_path, name).resolve() / NAME (into / NAME).mkdir(parents=True) (into / NAME / "family.yaml").write_bytes(b"name: Orbit\n") @@ -310,6 +313,7 @@ def test_a_family_folder_already_here_is_named_safe_to_show(tmp_path, name): assert result.returncode == 2, result.stdout + result.stderr assert result.stdout == "" assert refusal_detail(result.stderr, "family-holder-already-here") == [ - f"REFUSED family-holder-already-here: {spelt(into)} is already the " - f"{NAME} family folder and its holder is at {spelt(into / NAME)}, so " - "there is nothing to init"] + "REFUSED family-holder-already-here: " + f"{named_as(into, name)} is already the {NAME} family folder and " + f"its holder is at {named_as(into / NAME, name)}, so there is " + "nothing to init"] From b050dc7124b205ae72df21cc22f1dac4fe995258 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 21:18:30 +0000 Subject: [PATCH 10/10] family.py: one import of shown_path, now that #688 has landed beside #766's (#766) #766 added `shown_path` to family.py's `from repo_shape import (...)` block. #688, which landed on main as 42aed788, imports it on a line of its own, with a comment that says what it is for. After the restack both reached the branch, so the module imported the name twice. This drops #766's addition, so the block is main's again, byte for byte, and #688's line is the one import. Every use of `shown_path` is inside a function, after that line. No other name in either list changes. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/family.py b/scripts/family.py index 4514546f..e7c3e07d 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -169,7 +169,7 @@ COMMIT_RE, PROJECT_ID_RE, PYTHON, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, YamlError, checked_value, double_quoted, git_out, load_yaml, parse_yaml, recorded_gitlink, - repo_basename, same_repository, shown_path, tree_digest, + repo_basename, same_repository, tree_digest, ) from shape_materialize import ( # noqa: E402 FAMILY_RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, check_program,