From d6d1df2340652efe87c0cefbcad4eac8f804ce32 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:05:35 +0000 Subject: [PATCH 1/5] 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 2/5] 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 326de720bc67da7f6a059852d3da9e01431855fd Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 17:21:38 +0000 Subject: [PATCH 3/5] 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 4/5] 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 5/5] 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