bootstrap.py: step (a) decodes git's output as the system decodes a name it is given, so a branch whose upstream is not UTF-8 is moved back, and a good branch this system's encoding cannot write gets a line of its own, not a traceback (#710)

templates/assembly-root/scripts/bootstrap.py

@@ -41,7 +41,13 @@
       file is. A branch no command can be given (a NUL,
       or a lone surrogate this system cannot encode) is never tried: the leg
       is left as it is and the line says why (#455); step (b)'s manifest
-      validator names the key when the manifest holds it. Advancing a pin
+      validator names the key when the manifest holds it. Nor is a branch
+      holding a character this system's encoding cannot write (#710): where
+      git takes the name in UTF-8, its line says the name is good and to
+      run bootstrap under a UTF-8 locale, and where git refuses it there
+      too, the line gives git's words. Every name git prints back is read
+      as bytes and decoded so that it reaches git again as the bytes it
+      was (#710). Advancing a pin
       remains an explicit commit in this repository — see `README.md`, "The
       lockstep invariant". This is why `.gitmodules` carries no `branch=`:
       that would buy the same ergonomics by weakening the pin, and then "what
@@ -131,6 +137,10 @@
     "Remediation: put git on PATH and run this again. Step (a) reads each "
     "leg's pin and moves the leg through git, and the validators read the "
     "pins through git, so without it nothing here can be placed or checked.")
+#: What `_argument_fault` calls a character this system's encoding cannot
+#: write (#710): the one fault of a name that is good, so `_say_not_tried`
+#: gives it a line of its own.
+UNWRITABLE = "a character this system's encoding cannot write"
 REGISTER_RELPATH = Path("governance") / "review-authority" / "register.yaml"
 VALIDATORS = (
     ("naming", ["scripts/validate-repository-naming.py", "--project",
@@ -151,10 +161,44 @@ def _run(root: Path, args: list[str]) -> int:
 
 
 def _try_git(args: list[str], cwd: Path) -> str | None:
+    """What `git <args>` printed in `cwd`, stripped, or `None` where git
+    exited non-zero.
+
+    STEP (a)'S ONE DECODE RULE (#710). What git prints is read as BYTES and
+    decoded the way the name is encoded when git is handed it again. On
+    POSIX that is `os.fsdecode`, as `_head_of` and `_in_progress` read
+    theirs: the inverse of the `os.fsencode` that `subprocess` applies to
+    each argument, so a name git prints reaches git again as the same bytes
+    under any locale. On Windows `subprocess` encodes no argument: it hands
+    the command line to `CreateProcessW` as UTF-16, and git for Windows
+    reads it, and writes a ref name, in UTF-8. So the bytes are decoded as
+    UTF-8 there, as `git_out` decoded them, and not by `os.fsdecode`, which
+    `PYTHONLEGACYWINDOWSFSENCODING=1` makes `mbcs` with `replace`, losing
+    every character the code page lacks (RV824's N3). Through `git_out`'s
+    strict UTF-8, `_upstream_holding` ended `make bootstrap` in a
+    `UnicodeDecodeError` traceback over an upstream named with a byte that
+    is not UTF-8 (`refs/remotes/origin/a<ff>b`), and, under an `ascii`
+    encoding (`LC_ALL=C PYTHONUTF8=0`), in a `UnicodeEncodeError` over one
+    named in UTF-8, since the text it read could not be handed back to
+    `rev-parse`.
+
+    `git_out` itself keeps its strict decode, as #710's ruling took Option
+    2: the readers of `origin`'s URL in `shape-doctor.py` and in the
+    advisory it reads take its `UnicodeDecodeError` to mean the URL names
+    nothing (#385).
+
+    The BYTES are stripped, of the ASCII whitespace git ends a line with,
+    and not the text: `str.strip` also takes away a U+00A0 or a U+3000 that
+    ends a name, so `refs/remotes/origin/a<U+00A0>` was read as `.../a`, and
+    a branch with no commit of its own past the pin was not moved back.
+    """
     try:
-        return git_out(args, cwd=cwd)
+        raw = git_out(args, cwd=cwd, binary=True).strip()
     except Refusal:
         return None
+    # #710's ruling, Option 2: the decode is step (a)'s own, here, and
+    # `repo_shape.git_out` keeps its strict UTF-8 for every other caller.
+    return raw.decode("utf-8") if os.name == "nt" else os.fsdecode(raw)
 
 
 @contextmanager
@@ -199,8 +243,21 @@ def _argument_fault(branch: str) -> str | None:
     exception `_try_git` would then have to swallow: a `UnicodeDecodeError`,
     which `git_out` raises over output that is not UTF-8, is a `ValueError`
     too, and catching that there would read a leg whose HEAD is on such a
-    branch as detached. Only a NUL and a lone surrogate are looked for; a
-    character the system's encoding merely lacks is not this fix's.
+    branch as detached.
+
+    A CHARACTER THIS SYSTEM'S ENCODING CANNOT WRITE IS A FAULT TOO (#710,
+    Brett Heap's ruling (a) on L4-124), named `UNWRITABLE`. Under
+    `LC_ALL=C PYTHONUTF8=0` the encoding is `ascii`, and
+    `tracking_branch: "feature/日本"`, which `validate-manifest.py` passes
+    and git takes under a UTF-8 locale, ended step (a) in a
+    `UnicodeEncodeError` traceback from `subprocess.py`. The leg is left as
+    it is, and `_say_not_tried` says why. Its line can call the name good,
+    so this fault is asked last, and a name that holds a surrogate at all
+    is named for the surrogate instead, even one that is one byte of a name
+    (RV824's S1): `validate-manifest.py` names every surrogate (#280), so
+    `"日\\udcff"` is not a good name, and #455's line is the true one.
+    Whether git takes the name is asked before a line calls it good
+    (`_git_refuses_in_utf8`).
 
     A surrogate that is ONE BYTE of a name is not refused. POSIX spells a name
     that is not UTF-8 with U+DC80 to U+DCFF and `os.fsencode` writes each back
@@ -216,7 +273,15 @@ def _argument_fault(branch: str) -> str | None:
             return "U+0000, a NUL"
         if "\ud800" <= char <= "\udfff" and _fsencoded_length(char) != 1:
             return f"U+{ord(char):04X}, a lone surrogate"
-    return None
+    unwritable = next((char for char in branch
+                       if _fsencoded_length(char) == 0), None)
+    if unwritable is None:
+        return None
+    surrogate = next((char for char in branch
+                      if "\ud800" <= char <= "\udfff"), None)
+    if surrogate is not None:
+        return f"U+{ord(surrogate):04X}, a lone surrogate"
+    return f"U+{ord(unwritable):04X}, {UNWRITABLE}"
 
 
 def _fsencoded_length(char: str) -> int:
@@ -227,18 +292,66 @@ def _fsencoded_length(char: str) -> int:
         return 0
 
 
-def _say_not_tried(where: str, named: str, fault: str) -> None:
+def _say_not_tried(where: str, named: str, fault: str,
+                   refused: list[str] | None = None) -> None:
     """A branch `_argument_fault` found: no command was run with it. The leg
     stays where it is, and the line says which of the three places the
-    person's value can have come from holds it (#455)."""
-    print(f"  {where}: could not create branch {named}: it holds {fault}, "
-          "which no command can be given as an argument; the leg is left as "
-          "it is")
+    person's value can have come from holds it (#455).
+
+    A character this system's encoding cannot write (`UNWRITABLE`, #710) is
+    a branch step (a) DID NOT TRY, not one it could not create: the branch
+    can be there already, made by a run under UTF-8 (RV824's N2). Where git
+    takes the name in UTF-8 (`refused` is `None`), the line is the one Brett
+    Heap ruled on L4-124: the name is good, the encoding is named
+    (`sys.getfilesystemencoding()`, the one `os.fsencode` writes with), and
+    the two ways to run bootstrap under UTF-8 follow, a UTF-8 locale or
+    Python's UTF-8 mode. Where git refuses it in UTF-8 too, `refused` is
+    git's lines (`_git_refuses_in_utf8`): the name is not called good, git's
+    words are said, and #455's way out follows, since the name itself has
+    to change (RV824's S1)."""
+    unwritable = fault.endswith(UNWRITABLE)
+    tried = "did not try" if unwritable else "could not create"
+    print(f"  {where}: {tried} branch {named}: it holds {fault}, which no "
+          "command can be given as an argument; the leg is left as it is")
+    if unwritable and refused is None:
+        print("          the name is good, and this system's encoding, "
+              f"{sys.getfilesystemencoding()}, cannot write it: run bootstrap "
+              "again under `LC_ALL=C.UTF-8`, or with `PYTHONUTF8=1`.")
+        return
+    if unwritable:
+        print("          and git refuses it as a branch name in UTF-8 too:")
+        _say_lines(refused)
     print("          the branch is the leg's `branch:`, else `--branch`, else "
           "`tracking_branch`. Give that one a name git can create and run "
           "bootstrap again.")
 
 
+def _git_refuses_in_utf8(root: Path, branch: str) -> list[str] | None:
+    """What git says of `branch` as a branch name in the bytes a run under
+    UTF-8 would hand it, or `None` when git takes it (#710, RV824's S1).
+
+    A name `_argument_fault` faults as `UNWRITABLE` is not called good until
+    git has been asked: `feature/日本..x`, `日本.lock` and `日本 x` are names
+    this system's encoding cannot write AND names git refuses in any
+    encoding, and the way out the line gave ended in `fatal: ... is not a
+    valid branch name`. `git check-ref-format --branch` is git's own answer.
+    It is asked of `branch.encode("utf-8", "surrogateescape")`, handed to
+    `subprocess` through `os.fsdecode`, which POSIX's `os.fsencode` writes
+    back as exactly those bytes under any locale. On Windows no character
+    is `UNWRITABLE` (its `os.fsencode` writes UTF-8, or replaces), so this
+    is never asked there.
+
+    ASKED IN THE ROOT, not in the leg (RV824's R1-N1). `--branch` makes git
+    set up the repository it runs in, so in a leg whose `.git` file names a
+    git dir that is gone it exits 128 (`not a git repository`), and a good
+    name read as one git refuses there, while the same name was good in the
+    next leg. The root is the repository this command was found in, and
+    the answer is about the name alone.
+    """
+    utf8 = os.fsdecode(branch.encode("utf-8", "surrogateescape"))
+    return _git_says(["check-ref-format", "--branch", utf8], root)
+
+
 def _short(sha: str | None) -> str:
     return sha[:12] if sha else "?"
 
@@ -301,7 +414,11 @@ def _upstream_holding(sub: Path, branch: str, local: str) -> str | None:
     remote-tracking ref is not there (never fetched, or pruned) and one
     whose tip is not the upstream's. The two are compared by object id.
     `for-each-ref` names the upstream whether or not its ref exists, and
-    takes the branch after `refs/heads/`, where git reads no option.
+    takes the branch after `refs/heads/`, where git reads no option. Its
+    name is read by `_try_git`'s rule, so on POSIX an upstream named with
+    a byte that is not UTF-8, or in UTF-8 under an `ascii` encoding, is the
+    same ref when `rev-parse` is handed it, and no longer a traceback
+    (#710).
     """
     upstream = _try_git(["for-each-ref", "--format=%(upstream)",
                          f"refs/heads/{branch}"], sub)
@@ -812,7 +929,9 @@ def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None:
         return
     fault = _argument_fault(branch)
     if fault is not None:
-        _say_not_tried(where, named, fault)
+        _say_not_tried(where, named, fault,
+                       _git_refuses_in_utf8(root, branch)
+                       if fault.endswith(UNWRITABLE) else None)
         return
     pin = recorded_gitlink(root, path)
     local = _try_git(["rev-parse", "--verify", "--quiet",

tests/test_bootstrap_branch_bytes.py

@@ -81,6 +81,20 @@
                "and run bootstrap again.")
 
 
+def written(branch: str):
+    """`branch` as a row of the no-fault test, skipped where this system's
+    encoding cannot write it: there `_argument_fault` faults it as #710's
+    `UNWRITABLE`, which `tests/test_bootstrap_non_utf8_branch.py` holds
+    (RV824's S2). Asked at collection, of `os.fsencode` itself."""
+    try:
+        os.fsencode(branch)
+    except UnicodeEncodeError:
+        return pytest.param(branch, marks=pytest.mark.skip(
+            reason="this system's encoding cannot write it, so it is #710's "
+                   "UNWRITABLE fault, held in test_bootstrap_non_utf8_branch"))
+    return branch
+
+
 def not_tried(leg: str, printed: str, point: str, kind: str) -> list[str]:
     return [f"  [{leg}] {leg}: could not create branch {printed}: it holds "
             f"{point}, {kind}, which no command can be given as an "
@@ -408,11 +422,14 @@ def validator_module():
 
 
 @pytest.mark.parametrize("branch", [
-    "main", "release/1.2", "feature/日本", "a‍b", "a\xe9b", "a b",
+    "main", "release/1.2", written("feature/日本"), written("a‍b"),
+    written("a\xe9b"), "a b",
     "a\x1bb", "a..b", "", "\udc80", "a\udcffb"])
 def test_a_branch_the_system_carries_has_no_fault(bootstrap_module, branch):
-    """Nothing but a NUL and a lone surrogate with no byte is a fault: a
-    control, a space, `..`, a character the system's encoding may lack, and
+    """Where the system's encoding writes every character of the branch,
+    nothing but a NUL and a lone surrogate with no byte is a fault: a
+    control, a space, `..`, a character that is not ASCII (`written` skips
+    one the encoding cannot write, #710's `UNWRITABLE`), and
     the byte-carrying surrogates U+DC80 to U+DCFF are the commands'. On a
     system whose `os.fsencode` does not carry U+DC80 and U+DCFF (Windows) the
     next test is the one that holds."""