From fa513848a27cdcb39977f3590a37380f33e6f79a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 13:36:57 +0000 Subject: [PATCH 1/3] WIP (account-swap checkpoint; untested on these bytes): family.py and shape_materialize.py: the NEXT push line names the branch by its full refname on both sides, so a branch beginning with `+` or named like a tag pushes as itself, and a tracking branch whose last part is over 250 bytes, which git cannot lock, is refused by `refuse_bad_branch` before anything exists, `--dry-run` too (#689) THE PUSH LINE. `_print_next` ended `add`, `bump` and `remove` with `git -C push -u origin `, the branch bare. A bare name is a refspec git reads for itself, and a leading `+` in one means FORCE: a holder on `+x`, with an older local `x` and somebody's commit on origin's `x`, was told a line that pushed the LOCAL `x` over origin's (`+ 5f19342...647e6d4 x -> x (forced update)`), exit 0, and never pushed the commit the act made; with no local `x` it failed `src refspec x does not match any`. A branch that shares its name with a tag (`v1`) failed `src refspec v1 matches more than one`, exit 1 (R421 O1, O4). It is now `refs/heads/:refs/heads/`, the spelling `init`'s own push uses (#421), written out on both sides and quoted whole by `shell_line`. Measured: it pushes the named branch whatever is checked out, and `-u` still sets `branch..merge` and the remote. THE LENGTH. `git check-ref-format` has no length limit and `checked_value` allows 255 characters, so a tracking branch of 251 to 255 passed `--dry-run` and the real run died at the first commit with every remote made, `fatal: cannot lock ref 'HEAD': ... File name too long` (R421 O2). Git locks a branch by making `refs/heads/.lock`, and every other part is a directory, so `_branch_verdict` now asks `_branch_length_verdict` of a name git accepted: a last part over `NAME_MAX - len(".lock")` = 250 bytes, or any other part over `NAME_MAX` = 255, is refused with its own `Remediation:` (`BRANCH_LENGTH_REMEDIATION`), because the remedy for a name git judged sends the person to `git check-ref-format`, which accepts it. One helper, so `scaffold-project.py`, `family.py init` and `adopt-project.py` (`plan`, `check`, `execute`) all refuse it before anything exists. DECISION. The issue's direction says a part over 250; git measured here makes `<255 a's>/x` and refuses `x/<251 a's>`, so the 250 limit holds for the last part only, and the rows say so: 250 held, 251 refused, `<255>/x` held. TESTS. A new file holds each case, runs the printed line against a bare repository of the test's own, and holds the old bare spelling to the failure measured. The tests that read the line's last word move with it (`test_family_tracking_branch.py`, `test_family_switch_branch_exists.py`, `test_family_add_refusals.py`, and the sample line in `test_family_printed_quoting.py`); `test_tracking_branch_ref.py`'s corpus, which holds the helper to git's own verdict, swaps its 255-character name for 250, since the length is the one rule git does not state. Not touched: the same bare push in `bump-leg.py` and `update-shape.py` (#458's lines). Closes #689 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- scripts/family.py | 18 +- scripts/shape_materialize.py | 72 ++++- tests/test_family_add_refusals.py | 6 +- tests/test_family_printed_quoting.py | 2 +- tests/test_family_switch_branch_exists.py | 7 +- tests/test_family_tracking_branch.py | 12 +- tests/test_next_refspec_branch_length.py | 325 ++++++++++++++++++++++ tests/test_tracking_branch_ref.py | 7 +- 8 files changed, 432 insertions(+), 17 deletions(-) create mode 100644 tests/test_next_refspec_branch_length.py diff --git a/scripts/family.py b/scripts/family.py index e4f430cb..62fe066b 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -1180,9 +1180,23 @@ def _checked_inside_sparse_rules(root: Path, verb: str, def _print_next(family: "Family", branch: str) -> None: """The NEXT an act that committed ends with (#487): ONE pasteable line that pushes the branch the commit is on, every word quoted for the - reader's shell (#486), then the pull request it is for.""" + reader's shell (#486), then the pull request it is for. + + THE BRANCH BY ITS FULL REFNAME, ON BOTH SIDES (#689). A bare `` + is a REFSPEC git reads for itself, and a leading `+` in one means FORCE: + a holder on `+x` printed `push -u origin +x`, which pushed the LOCAL + branch `x` over origin's `x` (`+ 77de599...261789e x -> x (forced + update)`) and never the commit this act made, or, with no local `x`, + died `src refspec x does not match any`. A branch that shares its name + with a tag (`v1`) died `src refspec v1 matches more than one`. + `refs/heads/:refs/heads/` is the branch it is whatever + is checked out, and `-u` still records `origin/` as its upstream + (`branch..merge`). `init`'s own push spells it the same way + (#421), and the whole refspec is ONE word for `shell_line` to quote. + """ + refspec = f"{HEADS_PREFIX}{branch}:{HEADS_PREFIX}{branch}" print("\nNEXT " + shell_line(["git", "-C", family.root, "push", "-u", - "origin", branch])) + "origin", refspec])) print(f" then open a pull request against {family.tracking()}; " "never push to it directly") diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 351ab603..04b757c4 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1185,6 +1185,23 @@ def run(args: list[str], cwd: Path | None = None, capture: bool = True, #: the person's name: a git that refuses this one too could not answer. BRANCH_CONTROL = "main" +#: What git adds to a branch's last part to lock its ref while it makes it: +#: `refs/heads/.lock`, a file in the same directory. +BRANCH_LOCK_SUFFIX = ".lock" + +#: The remedy for a part too long for the disk (#689), whose own words are +#: the same for a flag and for a plan's key, so `adopt-project.py`'s `check` +#: prints it as it stands. `BRANCH_REMEDIATION` would send the person to +#: `git check-ref-format --branch`, which accepts every such name. +BRANCH_LENGTH_REMEDIATION = ( + "Remediation: shorten the part the name above says is too long. Git " + "makes each part of a branch name a directory, and the last one a file " + f"as well, named `{BRANCH_LOCK_SUFFIX}` while it locks the ref, " + "and a disk keeps a file or directory name to 255 bytes: the last part " + "may be at most 250 bytes and every other part at most 255. " + "`git check-ref-format` has no length limit, so it accepts such a name " + "and the first commit is where git fails.") + def _ask_git_branch(name: str) -> str: """`git check-ref-format --branch `, asked of NO repository and @@ -1213,8 +1230,9 @@ def _ask_git_branch(name: str) -> str: def _branch_verdict(what: str, value) -> tuple[str | None, str]: """`(problem, remediation)`: `branch_problem`'s sentence, and the remedy - that fits it, which is about the NAME when git judged the name and about - GIT when git could not answer.""" + that fits it, which is about the NAME when git judged the name, about + GIT when git could not answer, and about the DISK when a part of the + name is too long for it (#689, `_branch_length_verdict`).""" text = str(value) refused = f"{what} is {text!r}, which is not a branch name git will create" unchecked = f"{what} is {text!r}, which could not be checked" @@ -1250,12 +1268,54 @@ def _branch_verdict(what: str, value) -> tuple[str | None, str]: if answer != text: return (f"{refused}: `git check-ref-format --branch` reads it as " f"{answer!r}, not as written", BRANCH_REMEDIATION) + return _branch_length_verdict(what, text) + + +def _branch_length_verdict(what: str, text: str) -> tuple[str | None, str]: + """`_branch_verdict`'s last question, asked of a name git accepted + (#689): can the disk hold the files git makes for it? + + `git check-ref-format` has NO LENGTH LIMIT, so a tracking branch of 251 + to 255 characters, inside `checked_value`'s 255, passed it and passed + `--dry-run`, and the real run died at the first commit with every remote + already made: `fatal: cannot lock ref 'HEAD': Unable to create + '.../refs/heads/aaa....lock': File name too long`. The limit is the + disk's, not git's, so it is the one question here that is written down + and not asked of git. Each part of the name is a directory under + `refs/heads/` and the last one is also the file `.lock`, so the + last part may be at most `NAME_MAX - len(".lock")` = 250 bytes and any + other part `NAME_MAX` = 255 (measured: `<255 a's>/x` and `x/<250 a's>` + are created, `x/<251 a's>` and `<251 a's>` are not). BYTES, as + `refuse_unstorable_names` counts them, in the form the operating system + is given (`os.fsencode`). One refusal, for the first part over its limit. + """ + parts = text.split("/") + for number, part in enumerate(parts, 1): + last = number == len(parts) + limit = NAME_MAX - (len(BRANCH_LOCK_SUFFIX) if last else 0) + size = len(os.fsencode(part)) + if size <= limit: + continue + # BOTH ENDS, as `refuse_unstorable_names` shows a long name: the + # tail is what tells two long names apart. + shown = text if len(text) <= 27 else f"{text[:12]}...{text[-12:]}" + if len(parts) == 1: + which = "it" + elif last: + which = "its last part" + else: + which = f"its part {number}" + return (f"{what} is {shown!r}, which is not a branch name git will " + f"create: {which} is {size} bytes long, and may be at most " + f"{limit}", BRANCH_LENGTH_REMEDIATION) return None, BRANCH_REMEDIATION def branch_problem(what: str, value) -> str | None: """None when `git check-ref-format --branch` accepts `value` exactly as - written; otherwise the sentence that says what git made of it. + written and the disk can hold the files git makes for it (#689); + otherwise the sentence that says what git made of it, or which part is + too long. `what` names the value as the person met it (`--tracking-branch`, `tracking_branch`), the way `leg_path_problem` is given its flag. Git is @@ -1277,8 +1337,10 @@ def refuse_bad_branch(refusal: str, what: str, value) -> None: `refusal` is the caller's own code (`scaffold-bad-tracking-branch`), so each tool refuses in its own voice, as `refuse_bad_leg_paths` lets it. - The remedy is `BRANCH_REMEDIATION` when git judged the name, and - `BRANCH_UNCHECKED_REMEDIATION` when git could not answer at all. + The remedy is `BRANCH_REMEDIATION` when git judged the name, + `BRANCH_UNCHECKED_REMEDIATION` when git could not answer at all, and + `BRANCH_LENGTH_REMEDIATION` when a part of the name is too long for the + disk (#689). """ problem, remediation = _branch_verdict(what, value) if problem: diff --git a/tests/test_family_add_refusals.py b/tests/test_family_add_refusals.py index 7a037a25..c469b417 100644 --- a/tests/test_family_add_refusals.py +++ b/tests/test_family_add_refusals.py @@ -66,8 +66,10 @@ "their own assembly root{named}.") #: What a good `add` ends with (#487): the push of the branch it committed on, -#: which is the fixture's own, and the pull request it is for. -NEXT = ("NEXT git -C {root} push -u origin {branch}", +#: which is the fixture's own, by its full refname on both sides (#689), and +#: the pull request it is for. +NEXT = ("NEXT git -C {root} push -u origin " + "refs/heads/{branch}:refs/heads/{branch}", " then open a pull request against main; never push to it " "directly") diff --git a/tests/test_family_printed_quoting.py b/tests/test_family_printed_quoting.py index db852475..9e84c3c4 100644 --- a/tests/test_family_printed_quoting.py +++ b/tests/test_family_printed_quoting.py @@ -165,7 +165,7 @@ def test_nothing_is_flattened_and_a_typographic_apostrophe_is_doubled(): def test_a_line_is_its_words_each_quoted(): words = ["git", "-C", "/srv/my work/Sonata", "push", "-u", "origin", - "family/add-irrs"] + "refs/heads/family/add-irrs:refs/heads/family/add-irrs"] assert shlex.split(shell_line(words, "posix")) == words assert powershell_words(shell_line(words, "nt")) == words diff --git a/tests/test_family_switch_branch_exists.py b/tests/test_family_switch_branch_exists.py index e6490ddc..3a4889ab 100644 --- a/tests/test_family_switch_branch_exists.py +++ b/tests/test_family_switch_branch_exists.py @@ -59,7 +59,8 @@ ) from test_family_printed_quoting import host_words from test_family_tracking_branch import ( # noqa: F401 - TRACKING, act, next_push, on_main, remedy_switch, snapshot, suggested, + TRACKING, act, next_push, on_main, refspec, remedy_switch, snapshot, + suggested, ) # `BRANCH_LEFT` is the sentence both tools end the remedy with, pinned as # text: a person, an agent and a holder's CI read it, so it is not rebuilt @@ -192,7 +193,7 @@ def test_the_refusal_typed_as_printed_puts_the_act_on_its_branch( assert git("rev-parse", "HEAD~1", cwd=holder).stdout.strip() == \ earlier push, _ = next_push(done.stdout) - assert push[-1] == branch, push + assert push[-1] == refspec(branch), push assert validate(holder).returncode == 0 @@ -501,5 +502,5 @@ def test_the_branch_filled_in_as_written_is_one_add_commits_on(family, done = act(family, holder, "add") assert done.returncode == 0, done.stdout + done.stderr push, _ = next_push(done.stdout) - assert push[-1] == branch, push + assert push[-1] == refspec(branch), push assert SPARE in [row["project"] for row in manifest(holder)["members"]] diff --git a/tests/test_family_tracking_branch.py b/tests/test_family_tracking_branch.py index 93467d12..85b62ee1 100644 --- a/tests/test_family_tracking_branch.py +++ b/tests/test_family_tracking_branch.py @@ -121,6 +121,13 @@ def next_push(stdout: str) -> tuple[list[str], str]: return host_words(lines[at][len("NEXT "):]), lines[at + 1] +def refspec(branch: str) -> str: + """The word the NEXT push ends with (#689): the branch by its full + refname on both sides, which no `+`, tag or checked-out branch can + change the reading of.""" + return f"refs/heads/{branch}:refs/heads/{branch}" + + @pytest.mark.parametrize("verb", ["add", "bump", "remove"]) def test_each_act_refuses_the_tracking_branch_then_lands_on_a_branch( family, on_main, verb): @@ -156,7 +163,7 @@ def test_each_act_refuses_the_tracking_branch_then_lands_on_a_branch( "the tracking branch did not move") push, then = next_push(done.stdout) assert push == ["git", "-C", str(holder), "push", "-u", "origin", - branch] + refspec(branch)] assert then == (f" then open a pull request against {TRACKING}; " "never push to it directly") assert done.stdout.rstrip().endswith(then), ( @@ -305,7 +312,8 @@ def test_the_tracking_branch_is_the_one_family_yaml_declares(family, done = run_script(FAMILY, *add) assert done.returncode == 0, done.stdout + done.stderr push, then = next_push(done.stdout) - assert push == ["git", "-C", str(holder), "push", "-u", "origin", "main"] + assert push == ["git", "-C", str(holder), "push", "-u", "origin", + refspec("main")] assert then == (" then open a pull request against trunk; never " "push to it directly") diff --git a/tests/test_next_refspec_branch_length.py b/tests/test_next_refspec_branch_length.py new file mode 100644 index 00000000..806d20d9 --- /dev/null +++ b/tests/test_next_refspec_branch_length.py @@ -0,0 +1,325 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The NEXT push line names its branch by its full refname, and a tracking +branch the disk cannot hold is refused before anything exists (#689). + +THE PUSH LINE. `family.py add`, `bump` and `remove` each end with ONE line +that pushes the branch the commit is on, and it spelled the branch bare: +`git -C push -u origin `. A bare name is a refspec git reads +for itself, so, measured on main at d4a7fab: + + * a holder on `+x` (a name `git check-ref-format --branch` accepts) printed + `push -u origin +x`, a FORCED refspec for the LOCAL branch `x`. With an + older local `x` and somebody's commit on origin's `x`, pasting it said + `+ ... x -> x (forced update)` and overwrote origin's `x`, and the commit + the act made was never pushed; with no local `x`, `src refspec x does not + match any`; + * a branch that shares its name with a tag (`v1`) died `src refspec v1 + matches more than one`, exit 1. + +It is `refs/heads/:refs/heads/` now, one word the way the +shell reads it. Every case here runs the line as printed, against a bare +repository of the test's own, and holds the OLD spelling to the failure the +issue measured, so the test cannot pass on a git that reads a bare name +another way. + +THE LENGTH. `git check-ref-format` has no length limit, and `checked_value` +allows 255 characters, so a tracking branch of 251 to 255 passed +`--dry-run`, and the real run died at the first commit with every remote +made: `fatal: cannot lock ref 'HEAD': Unable to create '.../refs/heads/aaa +....lock': File name too long`. Git locks a branch by making +`refs/heads/.lock`, so the last part of the name may be at most +250 bytes and any other 255. `shape_materialize.refuse_bad_branch`, the one +helper `scaffold-project.py`, `family.py init` and `adopt-project.py` ask, +refuses it before anything exists, `--dry-run` too. Where this disk makes the +name, git is held to the same rows. + +THE SAME FAMILY AS `tests/test_family.py`, in `on_main`'s private copy of the +holder on its tracking branch, its `origin` made over to a bare repository +of the test's own so no other test sees a branch pushed here. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from conftest import REPO, findings, git, write_plan +from test_adopt_branch_names import TRACKING_KEY, with_branch, world # noqa: F401 +from test_family import family, family_module, validate # noqa: F401 +from test_family_printed_quoting import host_words +from test_family_tracking_branch import ( # noqa: F401 + act, next_push, on_main, refspec, +) +from test_tracking_branch_ref import ( + FAMILY_CODE, SCAFFOLD_CODE, assert_family_made_nothing, + assert_nothing_was_made, family_init, scaffold, +) + +sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 +from repo_shape import Refusal # noqa: E402 + +# --- the NEXT push line ------------------------------------------------------ + +#: `(branch, an older local x, a tag v1, what the bare name said)`: the three +#: states the issue measured, and the words git gave the line printed before. +STATES = [ + pytest.param("+x", True, False, "(forced update)", id="plus-over-a-local-x"), + pytest.param("+x", False, False, "src refspec x does not match any", + id="plus-and-no-local-x"), + pytest.param("v1", False, True, "matches more than one", + id="named-like-a-tag"), +] + + +def arranged(holder: Path, base: Path, branch: str, local_x: bool, + tag: bool) -> tuple[Path, str]: + """A bare `origin` of this test's own, holding the holder's `main` and + somebody's `x` (one commit above it), made the holder's `origin`; the + holder gets an older local `x` and a tag `v1` where asked, then stands on + `branch`. Returns the bare repository and somebody's commit.""" + bare = base / "origin.git" + git("init", "-q", "--bare", "-b", "main", str(bare), cwd=base) + git("remote", "set-url", "origin", str(bare), cwd=holder) + git("push", "-q", "origin", "HEAD:refs/heads/main", cwd=holder) + somebody = git("commit-tree", "-p", "HEAD", "-m", "Somebody's x", + "HEAD^{tree}", cwd=holder).stdout.strip() + git("push", "-q", "origin", f"{somebody}:refs/heads/x", cwd=holder) + if local_x: + git("branch", "x", "HEAD", cwd=holder) + if tag: + git("tag", "v1", cwd=holder) + git("switch", "-q", "-c", branch, cwd=holder) + return bare, somebody + + +@pytest.mark.parametrize("verb", ["add", "bump", "remove"]) +@pytest.mark.parametrize("branch, local_x, tag, old_words", STATES) +def test_the_printed_push_lands_the_branch_it_names( + family, on_main, tmp_path, verb, branch, local_x, tag, old_words): + """THE ISSUE'S THREE LINES, for each act. The line names the branch by + its full refname on both sides. Typed as printed it pushes that branch, + at the act's commit, leaves somebody's `x` alone, and sets the branch's + upstream to itself. The line printed before, run against a copy of the + same remote, fails or forces exactly as measured.""" + holder = on_main + bare, somebody = arranged(holder, tmp_path, branch, local_x, tag) + done = act(family, holder, verb) + assert done.returncode == 0, done.stdout + done.stderr + head = git("rev-parse", "HEAD", cwd=holder).stdout.strip() + push, _ = next_push(done.stdout) + assert push == ["git", "-C", str(holder), "push", "-u", "origin", + f"refs/heads/{branch}:refs/heads/{branch}"] + + # THE CONTROL: the bare spelling, against a copy of origin. + copy = tmp_path / "old.git" + shutil.copytree(bare, copy) + old = subprocess.run([*push[:4], str(copy), branch], capture_output=True, + text=True, check=False) + assert old_words in old.stderr, old.stderr + assert git("rev-parse", "-q", "--verify", f"refs/heads/{branch}", + cwd=copy, check=False).returncode != 0, ( + "the old line pushed the branch it named") + if local_x: + assert git("rev-parse", "refs/heads/x", cwd=copy).stdout.strip() \ + != somebody, "the old line did not overwrite origin's x" + + pushed = subprocess.run(push, capture_output=True, text=True, check=False) + assert pushed.returncode == 0, pushed.stderr + assert git("rev-parse", f"refs/heads/{branch}", + cwd=bare).stdout.strip() == head + assert git("rev-parse", "refs/heads/x", cwd=bare).stdout.strip() == \ + somebody, "somebody's x on origin was moved" + assert git("config", "--get", f"branch.{branch}.merge", + cwd=holder).stdout.strip() == f"refs/heads/{branch}" + assert git("config", "--get", f"branch.{branch}.remote", + cwd=holder).stdout.strip() == "origin" + assert validate(holder).returncode == 0 + + +@pytest.mark.parametrize("branch", [ + "+x", "v1", "family/add-irrs", "it's", "a$b;c", "lane-3/shape+fix@eagle", + "é"]) +def test_the_refspec_is_one_word_whatever_the_branch(capsys, branch): + """The whole refspec is ONE word for the shell to read, quoted as every + other word of the line is (#486), so a branch holding an apostrophe, a + `$` or a `;` still reaches git as the refname it is.""" + holder = Path("/srv/my work/Sonata") + family_module()._print_next( + SimpleNamespace(root=holder, tracking=lambda: "main"), branch) + (line,) = [line for line in capsys.readouterr().out.splitlines() + if line.startswith("NEXT ")] + assert host_words(line[len("NEXT "):]) == [ + "git", "-C", str(holder), "push", "-u", "origin", refspec(branch)] + + +# --- a branch the disk cannot hold --------------------------------------------- + +A, E = "a", "é" + +#: Names git creates on a disk of 255-byte file names, as `(name)`. +HELD = [ + pytest.param(A * 250, id="250-bytes"), + pytest.param("x/" + A * 250, id="last-part-250"), + pytest.param(A * 255 + "/x", id="first-part-255"), + pytest.param("y/" + "b" * 251 + "/z", id="middle-part-251"), + pytest.param(E * 125, id="125-two-byte-characters"), +] +#: Names it does not, as `(name, the words after "it is"/"its ... is")`. +TOO_LONG = [ + pytest.param(A * 251, "it is 251 bytes long, and may be at most 250", + id="251-bytes"), + pytest.param(A * 255, "it is 255 bytes long, and may be at most 250", + id="255-bytes"), + pytest.param("x/" + A * 251, + "its last part is 251 bytes long, and may be at most 250", + id="last-part-251"), + pytest.param(A * 256 + "/x", + "its part 1 is 256 bytes long, and may be at most 255", + id="first-part-256"), + pytest.param(E * 126, "it is 252 bytes long, and may be at most 250", + id="126-two-byte-characters"), +] + + +def shown(name: str) -> str: + """The name as a long one is said: both ends, where the tail is what + tells two long names apart.""" + return name if len(name) <= 27 else f"{name[:12]}...{name[-12:]}" + + +@pytest.mark.parametrize("name", HELD) +def test_a_name_the_disk_holds_is_not_refused(name): + assert shape_materialize.branch_problem("--tracking-branch", name) is None + assert shape_materialize.refuse_bad_branch( + "any-code", "--tracking-branch", name) is None + + +@pytest.mark.parametrize("name, words", TOO_LONG) +def test_a_name_the_disk_cannot_hold_says_which_part_and_how_long(name, words): + problem = shape_materialize.branch_problem("--tracking-branch", name) + assert problem == ( + f"--tracking-branch is {shown(name)!r}, which is not a branch name " + f"git will create: {words}") + with pytest.raises(Refusal) as caught: + shape_materialize.refuse_bad_branch("any-code", "--tracking-branch", + name) + assert caught.value.code == "any-code" + assert caught.value.detail == problem + remedy = caught.value.remediation + assert remedy == shape_materialize.BRANCH_LENGTH_REMEDIATION + # The remedy names both limits and says why git's own check passes. + assert "at most 250 bytes" in remedy + assert "255" in remedy + assert "no length limit" in remedy + + +@pytest.mark.skipif(os.name == "nt", reason="a Windows path of this length is " + "refused whatever git says") +@pytest.mark.parametrize("name", [ + pytest.param(row.values[0], id=row.id) for row in + [*HELD[:4], *TOO_LONG[:4]]]) +def test_the_rows_are_what_git_makes_on_this_disk(tmp_path, name): + """THE ROWS ARE MEASURED, NOT ASSUMED: `git branch ` in a repository + of its own succeeds exactly where the helper accepts the name (ASCII rows + only: a disk may normalise `é` into more bytes than the helper counts).""" + git("init", "-q", "-b", "main", str(tmp_path), cwd=tmp_path) + git("commit", "-q", "--allow-empty", "-m", "base", cwd=tmp_path) + made = git("branch", name, cwd=tmp_path, check=False) + accepted = shape_materialize.branch_problem("--tracking-branch", + name) is None + assert (made.returncode == 0) == accepted, ( + made.stderr[:200], len(name)) + + +def assert_too_long(result, code: str, name: str) -> None: + """Exit 2, the tool's own code, the name as a long one is said, the size + and the limit, the remedy, and nothing on stdout.""" + assert result.returncode == 2, result.stderr + result.stdout + assert result.stderr.startswith( + f"REFUSED {code}: --tracking-branch is {shown(name)!r}, which is not " + f"a branch name git will create: it is {len(name)} bytes long, and " + "may be at most 250\n" + f"{shape_materialize.BRANCH_LENGTH_REMEDIATION}"), result.stderr + assert result.stdout == "", "a plan was printed for a refused name" + assert "Traceback" not in result.stderr + + +@pytest.mark.parametrize("dry_run", [True, False], ids=["dry-run", "real"]) +@pytest.mark.parametrize("size", [251, 255]) +def test_scaffold_refuses_a_tracking_branch_over_250_before_anything_is_made( + tmp_path, size, dry_run): + """THE ISSUE'S SCAFFOLD: `--dry-run` exited 0, and the real run made all + three bare remotes and then died at the first commit.""" + name = A * size + result = scaffold(tmp_path, f"--tracking-branch={name}", dry_run=dry_run) + assert_too_long(result, SCAFFOLD_CODE, name) + assert_nothing_was_made(tmp_path) + + +@pytest.mark.parametrize("dry_run", [True, False], ids=["dry-run", "real"]) +@pytest.mark.parametrize("size", [251, 255]) +def test_family_init_refuses_a_tracking_branch_over_250_before_anything_is_made( + tmp_path, size, dry_run): + """THE ISSUE'S REPRODUCTION: `r/Orbit.git` and `w/Orbit` existed when the + real run died. Now neither run makes a folder, a remote or a holder.""" + name = A * size + result = family_init(tmp_path, f"--tracking-branch={name}", dry_run=dry_run) + assert_too_long(result, FAMILY_CODE, name) + assert_family_made_nothing(tmp_path) + + +def test_a_dry_run_with_250_still_passes_in_both_tools(tmp_path): + """The other side of the row: 250 is held, so a plan is printed.""" + name = A * 250 + for run in (scaffold, family_init): + result = run(tmp_path, f"--tracking-branch={name}", dry_run=True) + assert result.returncode == 0, result.stderr + result.stdout + assert "--dry-run: nothing was created." in result.stdout + + +@pytest.mark.skipif(os.name == "nt", reason="a Windows path of this length is " + "refused whatever the tool says") +@pytest.mark.parametrize("tool, remote", [(scaffold, "Atlas.git"), + (family_init, "Contoso.git")]) +def test_a_real_run_with_250_makes_every_remote_and_pushes_the_branch( + tmp_path, tool, remote): + """THE CONTROL FOR THE LIMIT: where the check lets a name through, the run + goes to its end and the remote holds exactly that branch.""" + name = A * 250 + result = tool(tmp_path, f"--tracking-branch={name}") + assert result.returncode == 0, result.stderr + result.stdout + bare = tmp_path / "remotes" / remote + assert git("for-each-ref", "--format=%(refname:short)", "refs/heads", + cwd=bare).stdout.split() == [name] + + +def test_adopt_refuses_the_same_name_in_plan_and_in_check(world, tmp_path): + """`adopt-project.py` asks the same helper: `plan` refuses a + `--tracking-branch` of 251 as `adopt-bad-tracking-branch` and writes + nothing, and `check` finds a plan edited to it once, under + `plan-bad-tracking-branch`, in the same words.""" + name = A * 251 + plan = tmp_path / "adoption-plan.yaml" + result = write_plan(world[0], plan, extra=("--tracking-branch", name)) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED adopt-bad-tracking-branch: --tracking-branch is " + f"{shown(name)!r}, which is not a branch name git will create: it is " + "251 bytes long, and may be at most 250\n"), result.stderr + assert not plan.exists() + + checked = tmp_path / "check" + checked.mkdir() + ((code, detail),) = findings(with_branch(world, checked, TRACKING_KEY, + name)) + assert code == "plan-bad-tracking-branch" + assert detail == ( + f"{TRACKING_KEY} is {shown(name)!r}, which is not a branch name git " + "will create: it is 251 bytes long, and may be at most 250") diff --git a/tests/test_tracking_branch_ref.py b/tests/test_tracking_branch_ref.py index f4cab59e..49e2ecea 100644 --- a/tests/test_tracking_branch_ref.py +++ b/tests/test_tracking_branch_ref.py @@ -86,12 +86,15 @@ #: the test that holds the helper to git's. Names git refuses AND accepts, the #: edges of each rule, and a few that only some versions of git have an #: opinion on (`HEAD`, `@`), which is exactly why the expectation is asked of -#: the git that runs the suite and not written here. +#: the git that runs the suite and not written here. Every name is one the +#: disk can hold: a part over its length limit (#689) is the one rule the +#: helper keeps that git does not state, so it is asked in +#: `tests/test_next_refspec_branch_length.py`, not against git's answer. CORPUS = [ *ACCEPTED, "1", "0", "~", "a~1", "a..b", "..", ".", "x.lock", ".lock", "a/", "/a", "a//b", "/", "//", ".a", "a.", "a/.b", "a./b", "a.lock/b", "a/b.lock", "HEAD", "head", "@", "@-1", "a@", "@a", "FETCH_HEAD", "refs/heads/x", - "heads/x", "a/b/c/d", "a" * 255, "-", "a b", "a\tb", "a:b", "a^b", "a*b", + "heads/x", "a/b/c/d", "a" * 250, "-", "a b", "a\tb", "a:b", "a^b", "a*b", "a?b", "a[b", "a\\b", "a@{b", "a@{-1}", "@{-1}", "a\x7fb", ] From b5a7a7f15ad9a288de6e01b8a17039230112929a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:16:09 +0000 Subject: [PATCH 2/3] family.py: `_print_next`'s docstring says `init`'s own push names its destination in full (`HEAD:refs/heads/`), where the WIP commit said it spells the line the same way (#689) The NEXT line is `refs/heads/:refs/heads/`, the source in full as well, so it pushes the branch whatever is checked out when it is pasted. `init`'s push (#421, #643) is `HEAD:refs/heads/`: its destination is in full and its source is HEAD, which is the branch in a clone `init` has just made. The docstring's "the same way" was not true of the source. Docstring only; the test run on the merged bytes (3767 passed) ran the code as it stands, and the new file, the three tracking-branch and quoting files and the hygiene, CLI and AGENTS.md code tests were run again on these bytes. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- scripts/family.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 94f3c179..f281e8e7 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -1203,8 +1203,9 @@ def _print_next(family: "Family", branch: str) -> None: with a tag (`v1`) died `src refspec v1 matches more than one`. `refs/heads/:refs/heads/` is the branch it is whatever is checked out, and `-u` still records `origin/` as its upstream - (`branch..merge`). `init`'s own push spells it the same way - (#421), and the whole refspec is ONE word for `shell_line` to quote. + (`branch..merge`). `init`'s own push names its side of the + remote in full too (`HEAD:refs/heads/`, #421), and the whole + refspec is ONE word for `shell_line` to quote. """ refspec = f"{HEADS_PREFIX}{branch}:{HEADS_PREFIX}{branch}" print("\nNEXT " + shell_line(["git", "-C", family.root, "push", "-u", From 15c3b5ab916ea9e7ea1794cb6770e1cbc9bfd932 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:55:19 +0000 Subject: [PATCH 3/3] shape_materialize.py: the length remedy says how git stores a branch (each part before the last a directory, the last the file) and that git fails when it first writes it, and the split branch and the 253/x name are held by rows (#689) Lane 1's review of #813 at b5a7a7f1, fix round 1. `BRANCH_LENGTH_REMEDIATION`, which every person refused a long name reads, said the last part is a directory as well as a file (it never is: for `x/<251 a's>`, `x` is the directory and the last part the file), said "the part the name above says" (the line above says it), and said "the first commit is where git fails", which is not so for adopt's split branch: on main `execute` failed in the extraction, after both leg repositories existed. The text is the review's, word for word. `_branch_length_verdict`'s docstring said the same untrue thing of the last part and is corrected alike. Rows added to tests/test_next_refspec_branch_length.py: - `adopt_branch:` set to 251 bytes and to `x/` + 251: `check` finds `plan-bad-adopt-branch` once, and `execute` refuses it before any leg exists (the legs' directory is never made). On main `check` said `plan ok` and `execute` made both leg repositories and died; the PR fixed this without a row holding it. - the 250 rows for `--dry-run` and a real run now also hold `<253 a's>/x` (255 characters, a first part over 250 that `checked_value` admits), which both tools complete with: the case the 255 for a part before the last rests on, which M5 (the issue's 250 throughout) would refuse. - the remedy's asserts also pin what it says of how git stores a name. Text only otherwise: no behaviour of the helper changes. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Sonnet 5.5 --- scripts/shape_materialize.py | 20 ++++--- tests/test_next_refspec_branch_length.py | 73 +++++++++++++++++++++--- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index ef5ece2d..c605da0c 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -1208,13 +1208,14 @@ def run(args: list[str], cwd: Path | None = None, capture: bool = True, #: prints it as it stands. `BRANCH_REMEDIATION` would send the person to #: `git check-ref-format --branch`, which accepts every such name. BRANCH_LENGTH_REMEDIATION = ( - "Remediation: shorten the part the name above says is too long. Git " - "makes each part of a branch name a directory, and the last one a file " - f"as well, named `{BRANCH_LOCK_SUFFIX}` while it locks the ref, " - "and a disk keeps a file or directory name to 255 bytes: the last part " - "may be at most 250 bytes and every other part at most 255. " - "`git check-ref-format` has no length limit, so it accepts such a name " - "and the first commit is where git fails.") + "Remediation: shorten the part of the name the line above says is too " + "long. Git keeps a branch as a file under `.git/refs/heads/`, each part " + "before the last a directory and the last part the file, and it locks " + f"the branch by first creating `{BRANCH_LOCK_SUFFIX}` beside " + "it; most disks keep a file or directory name to 255 bytes, so the last " + "part may be at most 250 bytes and every other part at most 255. " + "`git check-ref-format` has no length limit, so it accepts such a name, " + "and git fails only when it first writes the branch.") def _ask_git_branch(name: str) -> str: @@ -1295,8 +1296,9 @@ def _branch_length_verdict(what: str, text: str) -> tuple[str | None, str]: already made: `fatal: cannot lock ref 'HEAD': Unable to create '.../refs/heads/aaa....lock': File name too long`. The limit is the disk's, not git's, so it is the one question here that is written down - and not asked of git. Each part of the name is a directory under - `refs/heads/` and the last one is also the file `.lock`, so the + and not asked of git. Git keeps a branch as a file under `refs/heads/`: + each part before the last is a directory and the last part is the file, + which git locks by first creating `.lock` beside it. So the last part may be at most `NAME_MAX - len(".lock")` = 250 bytes and any other part `NAME_MAX` = 255 (measured: `<255 a's>/x` and `x/<250 a's>` are created, `x/<251 a's>` and `<251 a's>` are not). BYTES, as diff --git a/tests/test_next_refspec_branch_length.py b/tests/test_next_refspec_branch_length.py index 806d20d9..c2a11d4a 100644 --- a/tests/test_next_refspec_branch_length.py +++ b/tests/test_next_refspec_branch_length.py @@ -33,6 +33,15 @@ refuses it before anything exists, `--dry-run` too. Where this disk makes the name, git is held to the same rows. +THE TWO LIMITS ARE REACHED THROUGH THE TOOLS. `checked_value` admits only an +ASCII alphabet, up to 255 characters, so a FIRST part of 251 to 253 bytes +(`<253 a's>/x` is 255 characters) reaches the helper, and `scaffold-project.py` +and `family.py init` complete a run with it: the reason the limit is 255 for +a part before the last and not the issue's 250 throughout. And +`adopt-project.py` asks the helper of `adopt_branch:` as well as +`tracking_branch:`: on main a split branch of 251 checked `plan ok`, and +`execute` made both leg repositories before git died on it. + THE SAME FAMILY AS `tests/test_family.py`, in `on_main`'s private copy of the holder on its tracking branch, its `origin` made over to a bare repository of the test's own so no other test sees a branch pushed here. @@ -49,8 +58,10 @@ import pytest -from conftest import REPO, findings, git, write_plan -from test_adopt_branch_names import TRACKING_KEY, with_branch, world # noqa: F401 +from conftest import REPO, findings, git, refused_before_any_leg, write_plan +from test_adopt_branch_names import ( # noqa: F401 + SPLIT_KEY, TRACKING_KEY, adopter, with_branch, world, +) from test_family import family, family_module, validate # noqa: F401 from test_family_printed_quoting import host_words from test_family_tracking_branch import ( # noqa: F401 @@ -218,6 +229,15 @@ def test_a_name_the_disk_cannot_hold_says_which_part_and_how_long(name, words): assert "at most 250 bytes" in remedy assert "255" in remedy assert "no length limit" in remedy + # What it says of how git stores the name: each part before the last is a + # directory, only the last is the file (and is locked as `.lock`), + # and git fails when it first writes the branch, not at "the first commit". + assert "each part before the last a directory and the last part the file" \ + in remedy + assert "`.lock`" in remedy + assert "line above says is too long" in remedy + assert "when it first writes the branch" in remedy + assert "first commit" not in remedy @pytest.mark.skipif(os.name == "nt", reason="a Windows path of this length is " @@ -275,9 +295,16 @@ def test_family_init_refuses_a_tracking_branch_over_250_before_anything_is_made( assert_family_made_nothing(tmp_path) -def test_a_dry_run_with_250_still_passes_in_both_tools(tmp_path): - """The other side of the row: 250 is held, so a plan is printed.""" - name = A * 250 +#: Names the tools complete a run with: 250 bytes, and `<253 a's>/x`, 255 +#: characters, a first part over 250 that `checked_value` admits. +COMPLETED = [pytest.param(A * 250, id="250-bytes"), + pytest.param(A * 253 + "/x", id="253-then-x")] + + +@pytest.mark.parametrize("name", COMPLETED) +def test_a_dry_run_with_a_name_the_disk_holds_still_passes_in_both_tools( + tmp_path, name): + """The other side of the row: the name is held, so a plan is printed.""" for run in (scaffold, family_init): result = run(tmp_path, f"--tracking-branch={name}", dry_run=True) assert result.returncode == 0, result.stderr + result.stdout @@ -286,13 +313,15 @@ def test_a_dry_run_with_250_still_passes_in_both_tools(tmp_path): @pytest.mark.skipif(os.name == "nt", reason="a Windows path of this length is " "refused whatever the tool says") +@pytest.mark.parametrize("name", COMPLETED) @pytest.mark.parametrize("tool, remote", [(scaffold, "Atlas.git"), (family_init, "Contoso.git")]) -def test_a_real_run_with_250_makes_every_remote_and_pushes_the_branch( - tmp_path, tool, remote): +def test_a_real_run_with_a_name_the_disk_holds_makes_every_remote_and_pushes_it( + tmp_path, tool, remote, name): """THE CONTROL FOR THE LIMIT: where the check lets a name through, the run - goes to its end and the remote holds exactly that branch.""" - name = A * 250 + goes to its end and the remote holds exactly that branch. `<253 a's>/x` is + the row the 255 for a part before the last rests on: refusing it, as the + issue's 250 throughout would, refuses a name both tools complete with.""" result = tool(tmp_path, f"--tracking-branch={name}") assert result.returncode == 0, result.stderr + result.stdout bare = tmp_path / "remotes" / remote @@ -323,3 +352,29 @@ def test_adopt_refuses_the_same_name_in_plan_and_in_check(world, tmp_path): assert detail == ( f"{TRACKING_KEY} is {shown(name)!r}, which is not a branch name git " "will create: it is 251 bytes long, and may be at most 250") + + +#: `adopt_branch:` and a `/`-separated name whose last part is over the limit. +SPLIT_NAMES = [pytest.param(A * 251, id="251-bytes"), + pytest.param("x/" + A * 251, id="last-part-251")] + + +@pytest.mark.parametrize("name", SPLIT_NAMES) +def test_adopt_refuses_a_long_split_branch_in_check_and_before_any_leg( + adopter, world, tmp_path, monkeypatch, name): + """THE SPLIT BRANCH, `adopt_branch:`, is asked the same helper. On main a + plan edited to a part over 250 checked `plan ok`, and `execute` made both + leg repositories (`Northwind-spec.git`, `Northwind-code.git`) and died in + the extraction, `...-spec/.git/refs/heads/aaa....lock': File name too + long`, with nothing rolled back. Now `check` finds `plan-bad-adopt-branch` + once, and `execute` refuses it as that finding before any leg exists: the + directory the legs go in is never made.""" + plan = with_branch(world, tmp_path, SPLIT_KEY, name) + ((code, detail),) = findings(plan) + which = "it" if "/" not in name else "its last part" + assert code == "plan-bad-adopt-branch" + assert detail == ( + f"{SPLIT_KEY} is {shown(name)!r}, which is not a branch name git will " + f"create: {which} is 251 bytes long, and may be at most 250") + refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + assert refusal.code == "plan-bad-adopt-branch"