bootstrap.py and validate-manifest.py: a branch holding a NUL or a lone surrogate is a finding and step (a) does not hand it to git, so tracking_branch: "a\ud83db" or a leg's branch: "a\x00b" no longer ends make bootstrap in a subprocess traceback under a manifest ok (#455)

templates/assembly-root/scripts/bootstrap.py

@@ -28,11 +28,14 @@
       branch is at the pin, behind it, ahead of it or not there at all, and
       whatever the branch or `origin` holds: a commit made there is on no
       branch, and a checkout would leave it only in the reflog, so bootstrap
-      says where the leg is instead. 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 commit is this project"
-      would have two answers.
+      says where the leg is instead. 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
+      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
+      commit is this project" would have two answers.
 
   (b) RUNS THE NEUTRAL VALIDATORS — naming, manifest, lockstep pins. These are
       the only ones a fork with no review lane and no wallet register has, and
@@ -119,6 +122,62 @@ def _try_git(args: list[str], cwd: Path) -> str | None:
         return None
 
 
+def _argument_fault(branch: str) -> str | None:
+    """What stops `branch` being an argument to a command, or `None` (#455).
+
+    WHAT WAS WRONG. A branch reaches git as argv (`rev-parse --verify --quiet
+    refs/heads/<branch>`, `checkout -q -b <branch> <pin>`) through `_try_git`,
+    which catches `Refusal` and nothing else, and `subprocess` cannot hand a
+    command a NUL (`ValueError: embedded null byte`) or a lone surrogate the
+    system cannot encode (`UnicodeEncodeError`). `tracking_branch: "a\\ud83db"`
+    or a leg's `branch: "a\\x00b"` ended `make bootstrap` in a traceback from
+    `subprocess.py` in step (a), one step before `validate-manifest.py`, the
+    validator that names the key, was reached.
+
+    ASKED BEFORE ANY COMMAND IS BUILT FROM THE VALUE, and not read off an
+    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.
+
+    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
+    as the single byte it stands for, so git is handed that byte, as it was
+    before this check. Any other surrogate has no byte: `os.fsencode` raises,
+    or (`surrogatepass`, Windows) writes the three bytes of a sequence that
+    is not UTF-8 for ANY of them, so a lone surrogate is never handed to git
+    there either, where a `CreateProcess` would pass it on mangled. The answer
+    is `os.fsencode`'s own, so it is the system's and not a list kept here.
+    """
+    for char in branch:
+        if char == "\x00":
+            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
+
+
+def _fsencoded_length(char: str) -> int:
+    """How many bytes `os.fsencode` writes for `char`; 0 when it cannot."""
+    try:
+        return len(os.fsencode(char))
+    except UnicodeEncodeError:
+        return 0
+
+
+def _say_not_tried(where: str, named: str, fault: str) -> 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")
+    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 _short(sha: str | None) -> str:
     return sha[:12] if sha else "?"
 
@@ -518,6 +577,10 @@ def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None:
               f"`git submodule update --init {spelled(path)}` — the "
               f"validators below will refuse until you do.")
         return
+    fault = _argument_fault(branch)
+    if fault is not None:
+        _say_not_tried(where, named, fault)
+        return
     pin = recorded_gitlink(root, path)
     local = _try_git(["rev-parse", "--verify", "--quiet",
                       f"refs/heads/{branch}"], sub)

templates/assembly-root/scripts/validate-manifest.py

@@ -18,6 +18,10 @@
   - the envelope: `schema_version`, `kind`, `id`, `name`, `schema`
   - `elected_by` / `elected_on`, because an election with no elector and no
     date is not an election
+  - `tracking_branch`, and a leg's own `branch`, both OPTIONAL: that none
+    holds a NUL or a lone surrogate, which no command can be given and which
+    `make bootstrap` would hand to git (#455). Whether the value is a name
+    git will create is not asked here.
   - `topic` equals `xf-project-<id>` derived from the id
   - `shape`: a commit-and-digest pin of the openRepoShape revision this
     project was scaffolded from, `revision_kind: commit`, never a tag
@@ -506,6 +510,58 @@ def _visibility_finding(manifest: dict) -> list[str]:
     return []
 
 
+def _branch_finding(code: str, what: str, value) -> list[str]:
+    """One branch value of the manifest: `tracking_branch`, or a leg's
+    `branch`.
+
+    A VALUE NO COMMAND CAN BE GIVEN IS A FINDING (#455). `bootstrap.py` hands
+    the branch to git as an argument, and a NUL or a lone surrogate (the
+    double-quoted escapes `"a\\x00b"` and `"a\\ud83db"` read as them, as in
+    #280) cannot be one: step (a) used to end in a traceback from
+    `subprocess.py` and, with that fixed, would only say it could not place
+    the leg, while this file said `manifest ok`. So no tool named the key.
+    Any surrogate is found, as `name` is (#280): no UTF-8 file or console can
+    hold one, so none is a name anybody wrote. That is the whole of the
+    question. Whether the value is a name git will create (`a..b`, `x.lock`)
+    is `git check-ref-format`'s to answer, as the scaffold asks it for
+    `--tracking-branch` (#283); and a value that is not text is left alone,
+    as `bootstrap.py` turns it into text.
+    """
+    if not isinstance(value, str):
+        return []
+    char = next((c for c in value if c == "\x00" or "\ud800" <= c <= "\udfff"),
+                None)
+    if char is None:
+        return []
+    kind = ("a NUL: no argument, file name or ref name can hold one"
+            if char == "\x00" else
+            "a lone surrogate: no UTF-8 file or console can hold it, so no "
+            "branch name is spelled with one")
+    return [_finding(
+        code,
+        f"{what} is {value!r}, which holds U+{ord(char):04X}, {kind}. Name a "
+        "branch git can create, such as `main` or `release/1.2`.")]
+
+
+def _tracking_branch_finding(manifest: dict) -> list[str]:
+    """`tracking_branch`, OPTIONAL (#455); see `_branch_finding`."""
+    return _branch_finding("manifest-tracking-branch", "tracking_branch",
+                           manifest.get("tracking_branch"))
+
+
+def _leg_branch_findings(legs: list) -> list[str]:
+    """Each leg's own `branch`, OPTIONAL, which wins over `tracking_branch`
+    in `bootstrap.py` (#455); see `_branch_finding`. A leg that is not a
+    mapping is `_leg_findings`' to name."""
+    out: list[str] = []
+    for leg in legs:
+        if isinstance(leg, dict):
+            out.extend(_branch_finding(
+                "manifest-leg-branch", f"leg {leg.get('role')!r}: branch",
+                leg.get("branch")))
+    return out
+
+
 def _shape_findings(manifest: dict) -> list[str]:
     """`shape`: the commit-and-digest pin of the openRepoShape revision this
     project was scaffolded from.
@@ -843,6 +899,7 @@ def _findings(manifest: dict, policy: NamingPolicy, root=None) -> list[str]:
         out.extend(_topic_finding(manifest, policy, project_id))
     out.extend(_reference_finding(manifest))
     out.extend(_visibility_finding(manifest))
+    out.extend(_tracking_branch_finding(manifest))
     out.extend(_shape_findings(manifest))
     out.extend(_record_findings(manifest, SUBMODULE_ROWS))
     out.extend(_record_findings(manifest, ANSWER_ROWS))
@@ -855,6 +912,7 @@ def _findings(manifest: dict, policy: NamingPolicy, root=None) -> list[str]:
     pins = _declared_pins(manifest)
     out.extend(_neutral_product_pins_type_finding(manifest))
     out.extend(_roles_finding(legs))
+    out.extend(_leg_branch_findings(legs))
 
     owners: set[str] = set()
     paths: dict[str, str] = {}