diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 72d397e5..02b92c1a 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -1141,14 +1141,45 @@ def current_branch(root: Path) -> str | None: 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. + + READ AS THE BYTES GIT WROTE (#765). Exactly one newline and exactly + `refs/heads/` come off the bytes, and the rest is decoded with + `surrogateescape`, as `family.py` reads its holder's branch. Decoded + with `errors="replace"`, a branch `caf<0xE9>` read as `caf`, a + branch nobody has, and the push line named that on both sides and + failed pasted (`src refspec ... does not match any`). A NAME THAT IS NOT + UTF-8 IS REFUSED here, `bump-leg-branch-not-utf8`, the dry run alike, + before the fetch and before anything is written: no line this tool + prints can spell it, and a push line naming another branch is worse + than none. `git branch -m` gives the branch a name that can be spelled. """ 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.rstrip("\n") - if not ref.startswith(HEADS): + cwd=str(root), capture_output=True, check=False) + ref = proc.stdout[:-1] if proc.stdout.endswith(b"\n") else proc.stdout + heads = HEADS.encode("ascii") + if not ref.startswith(heads): return None - return ref[len(HEADS):] or None + raw = ref[len(heads):] + branch = raw.decode("utf-8", "surrogateescape") + try: + branch.encode("utf-8") + except UnicodeEncodeError: + # NAMED BY ITS BYTES, `b'caf<0xE9>'`, as `trailer-not-utf8` names a + # trailer, and THE ROOT as `shown_path` names a path, as + # `_branch_to_bump_from`'s other refusals name it (#766). The remedy + # is a command to paste, and holds the root raw inside its shell + # quotes, as their `switch -c` does. + raise Refusal( + "bump-leg-branch-not-utf8", + f"{shown_path(root)} is on the branch {raw!r}, whose name is not " + "UTF-8, so no line this tool prints can name it: its push line " + "would name a branch that is not there. Nothing was fetched, " + "written or committed.", + "Remediation: give the branch a name that is UTF-8 text, then " + "run the same command again: " + + shlex.join(["git", "-C", str(root), "branch", "-m"]) + + " ") from None + return branch or None # --------------------------------------------------------------------------- diff --git a/scripts/family.py b/scripts/family.py index 5e9c3f8d..372e3b92 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -771,13 +771,49 @@ def _read_member_project(path: Path, project: str, repository: str, def _holder_branch(root: Path) -> str | None: """The branch the holder's HEAD is on, or None: detached, or a HEAD - that names something other than a branch.""" + that names something other than a branch. + + READ AS THE BYTES GIT WROTE (#765). Exactly one newline and exactly + `refs/heads/` come off the bytes, and the rest is decoded with + `surrogateescape`. `strip()` also took a trailing U+00A0, U+2028 or + U+0085, which a branch name may end in: a holder on `feat` was + read as `feat`, a branch of its own, and the NEXT line pushed that one + and not the commit the act made. A NAME THAT IS NOT UTF-8 IS REFUSED + here, `family-branch-not-utf8`: no line this tool prints can spell it, + and a push line naming another branch is worse than none. The real run + of each act asks this before it reads the member or writes anything, + and its dry run at the line that would name the branch.""" try: - ref = _git_bytes(["symbolic-ref", "-q", "HEAD"], root).decode( - "utf-8", "surrogateescape").strip() + ref = _git_bytes(["symbolic-ref", "-q", "HEAD"], root) except CommandFailed: return None - return ref[len(HEADS_PREFIX):] if ref.startswith(HEADS_PREFIX) else None + ref = ref[:-1] if ref.endswith(b"\n") else ref + heads = HEADS_PREFIX.encode("ascii") + if not ref.startswith(heads): + return None + raw = ref[len(heads):] + branch = raw.decode("utf-8", "surrogateescape") + try: + branch.encode("utf-8") + except UnicodeEncodeError: + # NAMED BY ITS BYTES, `b'caf<0xE9>'`, as `trailer-not-utf8` names a + # trailer, and THE HOLDER as `shown_path` names a path, as + # `_checked_branch`'s other refusals name it (#766). The remedy is + # quoted for a POSIX shell, as bump-leg's and update-shape's are, so + # README's sentence on the lines quoted for PowerShell stays true; + # a command to paste, it holds the holder's path raw inside its + # quotes, as their `switch -c` does (#459). + raise Refusal( + "family-branch-not-utf8", + f"{shown_path(root)} is on the branch {raw!r}, whose name is not " + "UTF-8, so no line this tool prints can name it: the NEXT line " + "would push a branch that is not there. Nothing was committed, " + "and the holder's work tree and index are as they were.", + "Remediation: give the branch a name that is UTF-8 text, then " + "run the same command again:\n " + + shell_line(["git", "-C", root, "branch", "-m"], "posix") + + " ") from None + return branch #: A branch-name component the suggestion may use as it stands: dot-separated diff --git a/tests/conftest.py b/tests/conftest.py index 1ddd93cb..222e4256 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1177,6 +1177,64 @@ def skip_where_a_name_cannot_be_not_utf_8(directory: Path) -> None: probe.rmdir() +#: A branch git makes and no text can spell (#765): `caf` and the byte 0xE9, +#: Latin-1's e-acute, a UTF-8 lead byte with nothing after it. Built with +#: `chr`, not `os.fsdecode`, which raises on Windows, where this module +#: must still import. +NOT_UTF_8_BRANCH = "caf" + chr(0xDCE9) +#: How a refusal names `NOT_UTF_8_BRANCH`: by its bytes, `b'caf<0xE9>'`, as +#: `trailer-not-utf8` names a trailer. +NOT_UTF_8_BRANCH_NAMED = repr(NOT_UTF_8_BRANCH.encode("utf-8", + "surrogateescape")) + + +def git_answer_bytes(repo: Path, *args: str) -> bytes: + """`git ` in `repo`, its stdout as the bytes git wrote, whatever + the exit: `git` above decodes as text, which a name that is not UTF-8 + fails.""" + return subprocess.run(["git", *args], cwd=str(repo), capture_output=True, + check=False).stdout + + +def held_by_git(repo: Path) -> dict: + """What a refusal must leave in `repo` as it was, as bytes (#765): the + commit HEAD names, the ref it is attached to, every ref and its commit, + the index and the status; and for each git directory under + `.git/modules`, a leg's or a member's, its refs, its object count and + its FETCH_HEAD with the time it was written, which every fetch into it + rewrites.""" + held = {"head": git_answer_bytes(repo, "rev-parse", "HEAD"), + "attached": git_answer_bytes(repo, "symbolic-ref", "-q", "HEAD"), + "refs": git_answer_bytes(repo, "for-each-ref", + "--format=%(objectname) %(refname)"), + "index": git_answer_bytes(repo, "ls-files", "--stage", "-z"), + "status": git_answer_bytes(repo, "status", "--porcelain", "-z")} + modules = repo / ".git" / "modules" + gitdirs = sorted(found.parent for found in modules.rglob("HEAD") + if (found.parent / "objects").is_dir() + ) if modules.is_dir() else [] + for gitdir in gitdirs: + fetched = gitdir / "FETCH_HEAD" + held[gitdir.relative_to(modules).as_posix()] = ( + git_answer_bytes(gitdir, "for-each-ref", + "--format=%(objectname) %(refname)"), + git_answer_bytes(gitdir, "count-objects", "-v"), + (fetched.read_bytes(), fetched.stat().st_mtime_ns) + if fetched.is_file() else None) + return held + + +def on_a_branch_that_is_not_utf_8(repo: Path) -> dict: + """`repo` switched to a new branch `NOT_UTF_8_BRANCH` where HEAD is, or + the test skipped where this system cannot make the name + (`skip_where_a_name_cannot_be_not_utf_8`); returns `held_by_git(repo)` + on it.""" + skip_where_a_name_cannot_be_not_utf_8(repo) + subprocess.run(["git", "switch", "-q", "-c", NOT_UTF_8_BRANCH], + cwd=str(repo), capture_output=True, check=True) + return held_by_git(repo) + + 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. Bytes diff --git a/tests/test_bump_leg_branch_and_push_line.py b/tests/test_bump_leg_branch_and_push_line.py index 52e35ac3..79fa62ba 100644 --- a/tests/test_bump_leg_branch_and_push_line.py +++ b/tests/test_bump_leg_branch_and_push_line.py @@ -32,10 +32,15 @@ they already refuse it exits 0 and the real run ends `bump-leg-validators-red`. Its closing paragraph says so. +AND THE BRANCH'S BYTES (#765): the name is read as git wrote it, so a +trailing no-break space stays on it, and a name that is not UTF-8 is refused +by name (`bump-leg-branch-not-utf8`) where `errors="replace"` read another. + THE ROOT ABOVE THEM (#766). The three refusals and the dry run's `the real run needs a branch` line printed the root as it was, so a root under a name with an ESC, a line break or a bidi override sent it to the terminal raw. -Each names it as `repo_shape.shown_path` does now. +Each names it as `repo_shape.shown_path` does now, and so does #765's +`bump-leg-branch-not-utf8`, raised in the same check. 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 @@ -52,7 +57,9 @@ import pytest -from conftest import FILE_PROTOCOL, PROJECT, REPO, cloned_just_pushed, git +from conftest import (FILE_PROTOCOL, NOT_UTF_8_BRANCH_NAMED, PROJECT, REPO, + cloned_just_pushed, git, held_by_git, + on_a_branch_that_is_not_utf_8) #: The names no line may print raw, and how a line spells them (#766). from conftest import (POSIX_NAMES, SHOWN_NAMES, UNSAFE_NAMES, assert_shown_safe, copied_under, named_as, @@ -241,16 +248,23 @@ 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" +@pytest.mark.parametrize("neighbour", ["feat", "main"], + ids=["beside-a-branch", "beside-the-tracking-branch"]) def test_a_branch_ending_in_a_no_break_space_is_pushed_as_itself( - make_root, scaffolded, advanced): + make_root, scaffolded, advanced, neighbour): """`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.""" + (rv672's S2). And `main` beside the tracking branch `main` read + as `main`, so the bump was refused `bump-leg-on-tracking-branch` on a + branch that is not it (#765). Only git's newline is taken off now: the + line names the branch the bump committed on, and run, pushes that + commit, leaving origin's neighbour where it was.""" root, remote = make_root("plain") - name = "feat\u00a0" - git("branch", "feat", cwd=root) + name = neighbour + chr(0xA0) + if neighbour != "main": + git("branch", neighbour, cwd=root) + was = head_of(remote, f"refs/heads/{neighbour}") git("checkout", "-q", "-b", name, cwd=root) result = bump(root, scaffolded, advanced) @@ -261,7 +275,33 @@ def test_a_branch_ending_in_a_no_break_space_is_pushed_as_itself( 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" + assert head_of(remote, f"refs/heads/{neighbour}") == was, ( + f"`{neighbour}` was not pushed") + + +@pytest.mark.parametrize("extra", [(), ("--dry-run",)], + ids=["bump", "dry-run"]) +def test_a_branch_named_with_a_byte_that_is_not_utf8_is_refused_by_name( + project, scaffolded, advanced, extra): + """`caf<0xE9>` (#765): decoded with `errors="replace"`, it read as + `caf`, a branch nobody has, so the bump committed on `caf<0xE9>` + and the line named `caf` on both sides, which pasted fails `src + refspec ... does not match any`. Refused by name now, by its bytes, the + dry run alike, before the fetch: no line names the branch but the + refusal, which says how to rename it, and the root, its refs and each + leg's git directory are as they were, FETCH_HEAD and objects too.""" + before = on_a_branch_that_is_not_utf_8(project) + + result = bump(project, scaffolded, advanced, *extra) + + assert result.returncode == 2, result.stderr + result.stdout + assert (f"REFUSED bump-leg-branch-not-utf8: {project.resolve()} is on " + f"the branch {NOT_UTF_8_BRANCH_NAMED}, whose name is not UTF-8" + in result.stderr), result.stderr + assert result.stderr.rstrip().endswith(" branch -m "), ( + result.stderr) + assert result.stdout == "" + assert held_by_git(project) == before # --- the dirty root, spelled ------------------------------------------------- @@ -606,3 +646,25 @@ def test_the_dry_runs_branch_line_names_the_root_safe_to_show( assert said[0].startswith( f"the real run needs a branch: {spelt(root)} is on 'main', the " "tracking branch. Create one first with: git -C "), said + + +@pytest.mark.parametrize("name", SHOWN_NAMES) +def test_a_root_on_a_branch_that_is_not_utf8_is_named_safe_to_show( + scaffolded, advanced, tmp_path, name): + """#765's `bump-leg-branch-not-utf8` is raised in `_branch_to_bump_from`, + the check that gives `bump-leg-root-detached` and + `bump-leg-on-tracking-branch`, and names the root as they do (#766): as + `repr` spells it, on the one line it is, and a printable root as it is, + byte for byte. The remedy's `branch -m` is a command to paste, quoted + for a shell, and is not this row's.""" + root = copied_under(scaffolded["clone"], tmp_path, name) + on_a_branch_that_is_not_utf_8(root) + result = bump(root, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert result.stdout == "" + assert refusal_detail(result.stderr, "bump-leg-branch-not-utf8") == [ + f"REFUSED bump-leg-branch-not-utf8: {named_as(root, name)} is on the " + f"branch {NOT_UTF_8_BRANCH_NAMED}, whose name is not UTF-8, so no " + "line this tool prints can name it: its push line would name a " + "branch that is not there. Nothing was fetched, written or " + "committed."] diff --git a/tests/test_family_tracking_branch.py b/tests/test_family_tracking_branch.py index 54f543f4..f3a1c349 100644 --- a/tests/test_family_tracking_branch.py +++ b/tests/test_family_tracking_branch.py @@ -25,6 +25,10 @@ again and goes through; its NEXT is split and run, and the holder's remote then has the branch at the commit the act wrote. +THE BRANCH IS READ AS GIT WROTE IT (#765): a trailing no-break space stays +on it, and a name that is not UTF-8 is refused by name +(`family-branch-not-utf8`), since no line could spell it. + THE SAME FAMILY AS `test_family.py`, in a private copy put back on its tracking branch, which is where `init` leaves a holder. The remote a push reaches is this module's own instance of the fixture's, and every branch a @@ -38,9 +42,11 @@ import pytest -from conftest import REPO, git, run_script +from conftest import (NOT_UTF_8_BRANCH_NAMED, REPO, git, held_by_git, + on_a_branch_that_is_not_utf_8, run_script) #: The names no line may print raw, and how a line spells them (#766). -from conftest import UNSAFE_NAMES, copied_under, refusal_detail, spelt +from conftest import (SHOWN_NAMES, UNSAFE_NAMES, copied_under, named_as, + refusal_detail, spelt) #: The family fixture, its names and the work branch it grows the holder on, #: imported from the module that owns them. from test_family import ( # noqa: F401 @@ -200,6 +206,62 @@ def test_a_holder_on_no_branch_is_refused_too(family, on_main): assert snapshot(holder) == before +def test_a_branch_ending_in_a_no_break_space_is_pushed_as_itself(family, + on_main): + """`feat` beside a branch `feat` (#765): `strip()` read the + holder's branch as `feat`, so the act committed on `feat` and + its NEXT pushed `feat`, the other branch, which holds no such commit. + Read as git wrote it, the NEXT names the branch the commit is on, and + run, pushes that commit there and leaves `feat` alone.""" + holder = on_main + name = "feat" + chr(0xA0) + git("branch", "feat", cwd=holder) + git("switch", "-q", "-c", name, cwd=holder) + + done = act(family, holder, "remove") + assert done.returncode == 0, done.stdout + done.stderr + push, _ = next_push(done.stdout) + assert push == ["git", "-C", str(holder), "push", "-u", "origin", + refspec(name)] + remote = family["remotes"] / f"{NAME}.git" + pushed = subprocess.run(push, capture_output=True, text=True, check=False) + try: + assert pushed.returncode == 0, pushed.stderr + assert git("rev-parse", f"refs/heads/{name}", cwd=remote).stdout == \ + git("rev-parse", "HEAD", cwd=holder).stdout + assert git("rev-parse", "--verify", "-q", "refs/heads/feat", + cwd=remote, check=False).stdout == "" + finally: + git("push", "-q", "origin", "--delete", name, cwd=holder, + check=False) + + +@pytest.mark.parametrize("verb", ["add", "bump", "remove"]) +def test_a_branch_named_with_a_byte_that_is_not_utf8_is_refused_by_name( + family, on_main, verb): + """`caf<0xE9>` (#765): the act committed on it and printed a NEXT whose + spelling of the byte was the stream's: the raw byte, which a UTF-8 + terminal shows and copies as U+FFFD, or the six characters of its + escape, which name another branch. Refused by name now, by its bytes, + before anything is fetched or written, with how to rename it: no NEXT, + `add` prints not even the line that says the member is being read, and + the holder, its refs and each member's git directory are as they were, + FETCH_HEAD and objects too.""" + holder = on_main + before = on_a_branch_that_is_not_utf_8(holder) + + refused = act(family, holder, verb) + + assert refused.returncode == 2, refused.stdout + refused.stderr + assert refused.stdout == "" + assert (f"REFUSED family-branch-not-utf8: {holder} is on the branch " + f"{NOT_UTF_8_BRANCH_NAMED}, whose name is not UTF-8" + in refused.stderr), refused.stderr + assert refused.stderr.rstrip().endswith(" branch -m "), ( + refused.stderr) + assert held_by_git(holder) == before + + @pytest.mark.parametrize("verb", ["add", "bump", "remove"]) def test_a_dry_run_runs_on_the_tracking_branch_and_says_so(family, on_main, verb): @@ -366,3 +428,25 @@ def test_a_holder_refused_for_its_branch_is_named_safe_to_show( assert refusal_detail(result.stderr, code) == [ f"REFUSED {code}: {spelt(holder)} {said}. No git command that writes " "was run and nothing was written."] + + +@pytest.mark.parametrize("name", SHOWN_NAMES) +def test_a_holder_on_a_branch_that_is_not_utf8_is_named_safe_to_show( + family, tmp_path, name): + """#765's `family-branch-not-utf8` is raised in `_checked_branch`, the + check that gives the two refusals above, and names the holder as they + do (#766): as `repr` spells it, on the one line it is, and a printable + holder as it is, byte for byte. The remedy's `branch -m` is a command + to paste, quoted for a POSIX shell, and is not this row's.""" + holder = copied_under(family["root"], tmp_path, name) + git("switch", "-q", "-C", TRACKING, cwd=holder) + on_a_branch_that_is_not_utf_8(holder) + result = act(family, holder, "remove") + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "" + assert refusal_detail(result.stderr, "family-branch-not-utf8") == [ + f"REFUSED family-branch-not-utf8: {named_as(holder, name)} is on the " + f"branch {NOT_UTF_8_BRANCH_NAMED}, whose name is not UTF-8, so no " + "line this tool prints can name it: the NEXT line would push a " + "branch that is not there. Nothing was committed, and the holder's " + "work tree and index are as they were."] diff --git a/tests/test_update_shape_apply_lines_and_headers.py b/tests/test_update_shape_apply_lines_and_headers.py index 07743e86..5d2bc764 100644 --- a/tests/test_update_shape_apply_lines_and_headers.py +++ b/tests/test_update_shape_apply_lines_and_headers.py @@ -30,6 +30,10 @@ `update-manifest-no-shape`, "has no `shape:` block". The headers are found with the reader's own `_split_key` now, and written back as they were. +AND THE BRANCH HEAD IS ON, READ AS GIT WROTE IT (#765): a trailing no-break +space stays on it, and a name that is not UTF-8 is refused by name +(`update-branch-not-utf8`) before the first write of a `--branch` run. + THE HARNESSES ARE THE NEIGHBOURS'. `tests/test_update_shape_partial_rollback.py` gives the upstream (A, B above it, C above that), the project cut at A into local bare remotes, and the @@ -55,7 +59,8 @@ import pytest -from conftest import REPO, WINDOWS_SKIP, git, run_script +from conftest import (NOT_UTF_8_BRANCH_NAMED, REPO, WINDOWS_SKIP, git, + held_by_git, on_a_branch_that_is_not_utf_8, run_script) from test_pin_rewrite_layout import MANIFEST as MANIFEST_TEXT from test_pin_rewrite_layout import (NEW_COMMIT, NEW_ROWS, NEW_TREE, OLD_COMMIT, OLD_ROWS, OLD_TREE, make_pin, @@ -346,16 +351,22 @@ def test_a_run_that_only_drops_merged_away_rows_says_it_committed_the_pin( # --------------------------------------------------------------------------- -def tagged(root: Path, tmp_path: Path) -> Path: - """`root` with a tag `v1.0.0` at HEAD, pushing to an EMPTY bare remote of - this test's own: the bare remote.""" - git("tag", TAGGED, cwd=root) +def own_remote(root: Path, tmp_path: Path) -> Path: + """`root` pushing to an EMPTY bare remote of this test's own: the bare + remote.""" remote = tmp_path / "own-remote.git" git("init", "-q", "--bare", str(remote), cwd=tmp_path) git("remote", "set-url", "origin", str(remote), cwd=root) return remote +def tagged(root: Path, tmp_path: Path) -> Path: + """`root` with a tag `v1.0.0` at HEAD, pushing to `own_remote`: the bare + remote.""" + git("tag", TAGGED, cwd=root) + return own_remote(root, tmp_path) + + def on_the_remote(remote: Path) -> str | None: """What `remote` holds at the BRANCH `v1.0.0`, or None.""" proc = git("rev-parse", "--verify", "--quiet", f"refs/heads/{TAGGED}", @@ -486,6 +497,85 @@ def test_a_pr_rerun_on_a_branch_named_like_a_tag_opens_the_missing_one( assert gh.calls() == [listing(TAGGED), create] +# --------------------------------------------------------------------------- +# The branch HEAD is on, read as git wrote it (#765) +# --------------------------------------------------------------------------- + + +def test_a_branch_ending_in_a_no_break_space_is_not_taken_for_its_neighbour( + standard, root, tmp_path): + """`feat` checked out beside `feat`, and `--branch feat --push`: + `strip()` read HEAD as `feat`, so the run took `feat` for the branch + checked out, committed onto `feat` and pushed `feat`, which + holds no re-pin. Read as git wrote it, `feat` is a branch that exists + and is not checked out: refused `update-branch-exists`, naming the + branch HEAD is on, with both where they were and nothing pushed.""" + remote = own_remote(root, tmp_path) + name = "feat" + chr(0xA0) + git("branch", "feat", cwd=root) + git("switch", "-q", "-c", name, cwd=root) + before = held_by_git(root) + + result = apply(standard, root, "b", "--branch", "feat", "--push") + + assert result.returncode == 2, result.stdout + result.stderr + assert ("REFUSED update-branch-exists: --branch feat already exists and " + f"is not the current branch ({name!r} is checked out)" + in result.stderr), result.stderr + assert held_by_git(root) == before + assert git("for-each-ref", cwd=remote).stdout == "" + + +def test_a_branch_named_with_a_byte_that_is_not_utf8_is_refused_first( + standard, root, tmp_path): + """HEAD on `caf<0xE9>` (#765): `errors="replace"` read it as + `caf`, a branch nobody has, which the lines that name the branch + HEAD is on then named. Refused by name now, by its bytes, before the + first write, so there is nothing to roll back, with how to rename it, + the root and its refs as they were, and nothing pushed.""" + remote = own_remote(root, tmp_path) + before = on_a_branch_that_is_not_utf_8(root) + + result = apply(standard, root, "b", "--branch", BRANCH, "--push") + + assert result.returncode == 2, result.stdout + result.stderr + assert (f"REFUSED update-branch-not-utf8: {root} is on the branch " + f"{NOT_UTF_8_BRANCH_NAMED}, whose name is not UTF-8" + in result.stderr), result.stderr + assert result.stderr.rstrip().endswith(" branch -m "), ( + result.stderr) + assert "ROLLED BACK" not in result.stderr, "refused before any write" + assert held_by_git(root) == before + assert git("for-each-ref", cwd=remote).stdout == "" + + +@WINDOWS_SKIP +@pytest.mark.parametrize("env", [{}, {"LC_ALL": "C", "PYTHONUTF8": "0"}], + ids=["locale", "c-locale"]) +def test_a_rollback_puts_head_back_on_the_branch_it_started_on( + standard, root, env): + """A run from `feat`, beside `feat`, whose commit a `pre-commit` + hook refuses (#765, r765's findings 2 and 3). `strip()` read the branch + HEAD started on as `feat`, so the rollback "switched HEAD back to feat", + the neighbour. And handed back to git as text, the name raised + `UnicodeEncodeError` under `LC_ALL=C PYTHONUTF8=0` part way through the + rollback, leaving the copies written and HEAD on the new branch. HEAD + goes back on `feat` now, in both locales, with every byte git + holds as it was.""" + git("branch", "feat", cwd=root) + git("switch", "-q", "-c", "feat" + chr(0xA0), cwd=root) + hook = root / ".git" / "hooks" / "pre-commit" + hook.parent.mkdir(exist_ok=True) + hook.write_bytes(b"#!/bin/sh\nexit 1\n") + hook.chmod(0o755) + before = held_by_git(root) + + result = apply(standard, root, "b", "--branch", BRANCH, env=env) + + assert result.returncode == 2, result.stdout + result.stderr + assert held_by_git(root) == before + + # --------------------------------------------------------------------------- # A rollback line for a file a validator deleted under a shut directory # --------------------------------------------------------------------------- diff --git a/update-shape.py b/update-shape.py index 461d5c99..e8e54b7e 100755 --- a/update-shape.py +++ b/update-shape.py @@ -2337,11 +2337,33 @@ def current_branch(root: Path) -> str | None: the ref is read whole and `refs/heads/` taken off by `_branch_of`, as `Rollback.new_branch` takes it off; a HEAD attached anywhere else is on no branch, as a detached one is. + + ONE READ, BYTE FOR BYTE (#765): `_symbolic_head`'s, which `Rollback` + reads HEAD with too, and which keeps a trailing U+00A0 and a byte that + is not UTF-8 as they are. A NAME THAT IS NOT UTF-8 IS REFUSED here, + `update-branch-not-utf8`, and `cmd_apply` asks this before the first + write of every `--branch` run: no line this tool prints can spell it, + and the lines that would name it, `update-branch-exists` and a + rollback's way back to it, would name a branch that is not there. """ - proc = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], - cwd=str(root), capture_output=True, text=True, - encoding="utf-8", errors="replace", check=False) - return _branch_of(proc.stdout.strip()) + branch = _branch_of(_symbolic_head(root)) + if branch is None: + return None + try: + branch.encode("utf-8") + except UnicodeEncodeError: + # NAMED BY ITS BYTES, `b'caf<0xE9>'`, as `trailer-not-utf8` names a + # trailer. + raw = branch.encode("utf-8", "surrogateescape") + raise Refusal( + "update-branch-not-utf8", + f"{root} is on the branch {raw!r}, whose name is not UTF-8, so " + "no line this tool prints can name it: a line that named it " + "would name a branch that is not there. Nothing was written.", + "Remediation: give the branch a name that is UTF-8 text, then " + "run the same command again: " + + _by_hand(root, "branch", "-m") + " ") from None + return branch def _branch_of(ref: str | None) -> str | None: @@ -3283,7 +3305,10 @@ def _undo_branch(self, done: list[str], left: list[str]) -> None: back = [start] if start else ["--detach", commit] moved = _symbolic_head(self.root) == ref if moved: - _git_quietly(["git", "checkout", "-q", start, "--"] if start + # `start` AS GIT'S OWN BYTES (#765), whatever the locale; the + # by-hand lines below print it as it is. + _git_quietly(["git", "checkout", "-q", _as_git_argument(start), + "--"] if start else ["git", "checkout", "-q", "--detach", commit, "--"], self.root) if self._head_is(start, commit): @@ -3339,10 +3364,46 @@ def _said(done: list[str], left: list[str]) -> str: def _symbolic_head(root: Path) -> str | None: """The FULL ref HEAD is attached to (`refs/heads/`), or None when it is detached — the spelling `Rollback` keeps, for the reason - `Rollback.new_branch` gives. Asked through `_git_answer`, as - `readme_is_committed` asks its own, rather than a second copy of it.""" - answer = _git_answer(root, ["symbolic-ref", "--quiet", "HEAD"]) - return (answer or "").strip() or None + `Rollback.new_branch` gives, and the one read `current_branch` takes + the branch from. + + READ AS THE BYTES GIT WROTE (#765), not as `_git_answer`'s text. Exactly + one newline comes off the bytes, and the rest is decoded with + `surrogateescape`; `refs/heads/` is ASCII, so `_branch_of` taking it off + the text takes off exactly its bytes. `strip()` also took a trailing + U+00A0, U+2028 or U+0085: HEAD on `feat` read as `feat`, so + `commit_on_branch` took `--branch feat` for the branch checked out and + committed onto `feat`, the push named `feat`, and a rollback + "returned" HEAD to `feat`, another branch. `errors="replace"` read + `caf<0xE9>` as `caf`, which no command can check out. As + through `_git_answer`, no git and a git that refuses answer None: + neither may raise inside a rollback.""" + try: + proc = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], + cwd=str(root), capture_output=True, + check=False) + except OSError: + return None + ref = proc.stdout if proc.returncode == 0 else b"" + ref = ref[:-1] if ref.endswith(b"\n") else ref + return ref.decode("utf-8", "surrogateescape") or None + + +def _as_git_argument(name: str) -> str: + """`name`, as `_symbolic_head` decoded it, spelled as the argument that + hands git the same bytes back under any locale (#765), as #824 does for + `bootstrap.py`'s branches. + + `subprocess` writes an argument with `os.fsencode`, the locale's codec: + under `LC_ALL=C PYTHONUTF8=0` that is ASCII, which cannot write U+00A0, + so the rollback's checkout from `feat` raised + `UnicodeEncodeError` part way and left the copies written. Decoded back + with `os.fsdecode`, the name's own UTF-8 bytes go to git under every + codec. Windows hands git UTF-16, so the name goes as it is there. A line + printed for a person keeps the name as it is.""" + if os.name == "nt": + return name + return os.fsdecode(name.encode("utf-8", "surrogateescape")) def _by_hand(root: Path, *args: str) -> str: @@ -4606,6 +4667,10 @@ def cmd_apply(args) -> int: nothing = _nothing_to_do(rows, pinned, target, taking) rerun = nothing and _awaits_its_pull_request(root, args, kind) if args.branch: + # THE BRANCH HEAD IS ON, BEFORE THE FIRST WRITE (#765): one whose + # name is not UTF-8 is refused here, with nothing to roll back, + # not by `commit_on_branch` after the copies are written. + current_branch(root) # BEFORE THE FIRST WRITE, AND BEFORE "NOTHING TO DO" (#436): that # answer is read from the working tree too, and over a re-pin # left uncommitted it made no branch where one was asked for.