From aee12f298039b01ec2a9be33246490794371e5c3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:26:07 +0000 Subject: [PATCH] adopt-project.py: `check` finds, and `execute` refuses before any leg, the branch names `execute` failed on, so `adopt_branch: a..b`, a source branch `Adopt` or one hidden by `hideRefs` no longer pass `check` and fail after both legs exist (#520, #420) `check` now asks of the plan's `adopt_branch:` and `tracking_branch:` what `execute` asks: `checked_value`'s alphabet first (`unsafe-value`, execute's own words, L2-1 on #457), then git (`git check-ref-format --branch`, #283's `refuse_bad_branch`), as `plan-bad-adopt-branch` and `plan-bad-tracking-branch` in `_entry_findings`, which `execute` refuses before any leg. One finding per value: a bad tracking branch name is no longer also `plan-tracking-branch-missing`, and a bad split branch name is not looked for where the split is pushed. `plan` refuses a `--tracking-branch` git will not create as `adopt-bad-tracking-branch` and writes no plan. `_in_the_way` compares branch names without case too (A4, ruled 2026-10-12 for every system), with the finding saying so. `_branches_at` reads a local repository's own refs with `git for-each-ref`, so a branch hidden by `uploadpack.hideRefs` or `transfer.hideRefs` is seen; the extraction's view of the source stays `git ls-remote`, which a clone obeys, so a hidden branch is a finding about the push into the source alone, and `_refuse_a_split_branch_left_behind` reads the source's own refs under `--local-remote-dir`. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 401 ++++++++++++++++++++------ docs/cli.md | 4 +- tests/test_adopt_branch_names.py | 264 +++++++++++++++++ tests/test_adopt_plan_yaml_quoting.py | 14 +- tests/test_adopt_split_branch.py | 197 +++++++++++-- tests/test_adopt_template_values.py | 8 +- 6 files changed, 752 insertions(+), 136 deletions(-) create mode 100644 tests/test_adopt_branch_names.py diff --git a/adopt-project.py b/adopt-project.py index e4fa0451..1d87ff56 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -213,9 +213,14 @@ # Whether a value this tool writes into a file is text a file can hold # (#255), asked beside `one_printable_line`, the trailer's question. from shape_materialize import unwritable_reason # noqa: E402 -# Whether git creates a tracking branch as it is written (#283), asked only -# of one the project's loader misreads (`_a_branch_git_creates`, #181). -from shape_materialize import branch_problem # noqa: E402 +# Whether git creates a tracking branch as it is written (#283), asked of one +# the project's loader misreads (`_a_branch_git_creates`, #181), of `plan`'s +# `--tracking-branch`, and of both branch names a plan gives `execute` +# (`_branch_name_refusal`, #420, #520). `BRANCH_REMEDIATION` is the remedy +# it gives a name git judged, worded for a flag. +from shape_materialize import ( # noqa: E402 + BRANCH_REMEDIATION, branch_problem, refuse_bad_branch, +) # The shape's bootstrap script, which its `make bootstrap` runs: where it # went beside one the source kept decides what NEXT can name (#480). from shape_materialize import BOOTSTRAP # noqa: E402 @@ -2530,13 +2535,14 @@ def _pushed_in_every_mode(source: Source, tracking: str) -> bool: What GitHub has NOW is `execute`'s to ask (`git ls-remote`, before any leg): `check` asks nothing of the network, as #479 has it, and reads what the clone remembers. The source's own branches are asked as - `execute --local-remote-dir` asks them, of the source as a remote - (`_branches_at`); one that cannot be listed is no branch `execute` - pushes to there.""" + `execute --local-remote-dir` asks them, of the source as a remote, with + `git ls-remote` (`_branches_a_clone_gets`); one that cannot be listed is + no branch `execute` pushes to there.""" if _tracking_branch_problem(source, TRACKING_KEY, tracking) is not None: return False try: - return tracking in _branches_at(str(source.path), source.path) + return tracking in _branches_a_clone_gets(str(source.path), + source.path) except (CommandFailed, OSError): return False @@ -2840,10 +2846,16 @@ def _checked_plan_inputs(args, source: Source, # A default this tool filled in is named by where it came from, as # `--elected-by` is below, and not as a flag the person never passed. given = args.tracking_branch is not None + tracking = ("--tracking-branch" if given else "--tracking-branch " + "(not given, so the source's default branch)") args.tracking_branch = checked_value( - "--tracking-branch" if given else - "--tracking-branch (not given, so the source's default branch)", - args.tracking_branch if given else source.branch) + tracking, args.tracking_branch if given else source.branch) + # Then git's own question (#420): a name inside that alphabet that git + # will not create (`a..b`, `~`, `x.lock`, `a/`) is refused here, in git's + # words, and is not written for `check` to find it no branch of the + # source and the plan to be written again. + refuse_bad_branch("adopt-bad-tracking-branch", tracking, + args.tracking_branch) # The ONE question first -- spelling, Git's and Windows's names, then the # alphabet `checked_value` allows -- so `spec/` is refused here with the # spelling to retype, before a plan that `check` and `execute` would have @@ -5244,6 +5256,87 @@ def _unsafe_path_findings(plan: Plan) -> tuple[list[str], list[str]]: return found, [UNSAFE_PATH_REMEDIATION] if found else [] +# --------------------------------------------------------------------------- +# The two branch names a plan gives `execute` (#420, #520) +# --------------------------------------------------------------------------- +# +# `execute` hands two plan values to git as branch names: `adopt_branch:`, +# the split branch, which each extraction's clone checks out and the split +# is pushed to, and `tracking_branch:`, the branch both legs are pushed to. +# `_checked_plan_values` holds each to `checked_value`'s alphabet, which +# keeps an option off a command line and is wider than the names git +# creates. Measured on main at bfed143: an `adopt_branch:` edited to `a..b`, +# `~`, `x.lock` or `a/` checked `plan ok`, and `execute --local-remote-dir` +# made both legs and died at the extraction's `git checkout -B`, the +# corrected plan then meeting `leg-remote-exists`; a name outside the +# alphabet that git accepts (`a=b`) checked `plan ok`, and `execute` refused +# it as `unsafe-value` (L2-1 on #457). A `tracking_branch:` git refuses was +# found since #479, as `plan-tracking-branch-missing`, a sentence about the +# source's branches that named no rule. +# +# So `check` asks both, as `execute` reads them: the constant `ADOPT_BRANCH` +# and `main` where the key is absent, and `str` of whatever the file holds. +# Each is held to the alphabet first, and a name outside it is `execute`'s +# own refusal, `unsafe-value`, in its own words; a name inside it is asked of +# git with #283's `refuse_bad_branch`, under the key's own code. ONE FINDING +# PER VALUE: a name outside the alphabet is not asked of git, and no other +# question is asked about a name found here (`_check_split_branch`, and +# `plan-tracking-branch-missing` in `_entry_findings`). `execute` refuses +# `unsafe-value` in `_checked_plan_values`, as it always did, and git's +# verdict among `check`'s other findings in `_refuse_what_check_finds`, both +# before any leg exists. `plan` refuses its own `--tracking-branch` the same +# way, as `adopt-bad-tracking-branch`, before it writes a plan. + +#: Each branch name `execute` hands git, in the order `_checked_plan_values` +#: asks them: `(key, what execute reads where the key is absent, the code +#: check finds a name git will not create under)`. +SPLIT_BRANCH_KEY = ("adopt_branch", ADOPT_BRANCH, "plan-bad-adopt-branch") +BRANCH_KEYS = (SPLIT_BRANCH_KEY, + (TRACKING_KEY, "main", "plan-bad-tracking-branch")) +#: What `check` says once after a finding git made about either name: the +#: remedy `refuse_bad_branch` gives a name git judged is worded for a flag. +BRANCH_NAME_REMEDIATION = ( + "Remediation: set each branch name a finding names to one git creates, " + "such as `main` or `release/1.2`, in the plan, and run `check` again: " + "`git check-ref-format --branch ` prints such a name back as it " + "is written, and `git help check-ref-format` lists every rule.") + + +def _branch_name_refusal(plan: Plan, key: str, default: str, + code: str) -> Refusal | None: + """Why `execute` will not hand git the plan's `key` as a branch name, as + the refusal `check` reports, or None: `checked_value`'s, `execute`'s own, + for a name outside its alphabet, and otherwise git's verdict under + `code`. The value is `execute`'s (`_checked_plan_values`): `default` + where the key is absent, and `str` of what the file holds.""" + value = plan.get(key, default) + try: + checked_value(key, value) + refuse_bad_branch(code, key, value) + except Refusal as exc: + return exc + return None + + +def _branch_name_findings(plan: Plan) -> tuple[list[str], list[str], set]: + """`_branch_name_refusal` of each of `BRANCH_KEYS`, as `check` prints + them, in `execute`'s order; what it prints once after them; and the + keys they name, which nothing else asks about.""" + found, remedies, named = [], [], set() + for key, default, code in BRANCH_KEYS: + refusal = _branch_name_refusal(plan, key, default, code) + if refusal is None: + continue + found.append(f"FINDING {refusal.code}: {refusal.detail}") + named.add(key) + remedy = (BRANCH_NAME_REMEDIATION + if refusal.remediation == BRANCH_REMEDIATION + else refusal.remediation) + if remedy not in remedies: + remedies.append(remedy) + return found, remedies, named + + def _entry_findings(plan: Plan, source: Source, local_remote_dir: Path | None = None ) -> tuple[list[str], list[str], list[str]]: @@ -5257,10 +5350,13 @@ def _entry_findings(plan: Plan, source: Source, be written through (#178, #183), then each value `execute` writes into a file that no file can hold (#255), then what the project's own `validate-manifest.py` would refuse of the values `execute` writes into - its manifest (#261), then a tracking branch that is no branch of the - source (#479), then each relative url of a kept submodule that names - another repository from its leg (#190), and last each plan value - `execute` reads late and `_plan_field_findings` asks about (#293, #294). + its manifest (#261), then the split branch's name and the tracking + branch's, in that order, the order `execute` asks them, each held to + `checked_value`'s alphabet and otherwise asked of git (#420, #520), then + a tracking branch that is no branch of the source (#479), then each + relative url of a kept submodule that names another repository from its + leg (#190), and last each plan value `execute` reads late and + `_plan_field_findings` asks about (#293, #294). ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` @@ -5325,6 +5421,9 @@ def _entry_findings(plan: Plan, source: Source, remote `origin` has, which `execute --local-remote-dir` refuses, is named by neither (`_a_branch_execute_writes`); and a pin is named only where the plan's `pins:` has no finding of #273's (`_plan_fields`). + And a tracking branch whose NAME is wrong is that finding alone (#520): + one outside the alphabet or one git will not create is no branch of any + source, and is not also said to be missing from this one. """ tree = source.tree() unsafe, unsafe_remediation = _source_path_findings(plan, tree) @@ -5338,8 +5437,10 @@ def _entry_findings(plan: Plan, source: Source, unwritable_findings, unwritable_remediation = _value_findings(plan) value_findings, value_remediation = _manifest_findings(plan) field_findings, field_remediation = _plan_field_findings(plan) - tracking_findings, tracking_remediation = _tracking_branch_findings( - plan, source) + names, names_remediation, misnamed = _branch_name_findings(plan) + tracking_findings, tracking_remediation = ( + ([], []) if TRACKING_KEY in misnamed + else _tracking_branch_findings(plan, source)) # Asked after the validator, whose judgement it follows, and of the # tracking branch only where `execute` pushes the legs to it in every # mode (`_a_branch_execute_writes`, #479), so that a branch the source @@ -5353,12 +5454,13 @@ def _entry_findings(plan: Plan, source: Source, else (MANIFEST_FIELDS[TRACKING_KEY],)) return (findings + misread + split + _root_symlink_findings(plan, source, tree) - + unwritable_findings + value_findings + tracking_findings - + moved + field_findings, + + unwritable_findings + value_findings + names + + tracking_findings + moved + field_findings, unsafe_remediation + ([MANIFEST_MISREAD_REMEDIATION] if misread else []) + split_remediation + unwritable_remediation + value_remediation - + tracking_remediation + moved_remediation + field_remediation, + + names_remediation + tracking_remediation + moved_remediation + + field_remediation, notes) @@ -5394,6 +5496,28 @@ def _entry_findings(plan: Plan, source: Source, # So `check` reports both, and `execute` refuses the second, before any leg # exists, as `plan-split-branch-taken`. # +# ONCE CASE IS IGNORED TOO (#520's A4; ruled 2026-10-12: on every system). +# Git keeps a branch as a file named after it, and a macOS or Windows disk +# keeps `Adopt` and `adopt` as one path, so a source branch `Adopt`, or one +# that is the split branch in another case, passed `check` there and +# `execute` made both legs before git refused it (#457). `_in_the_way` +# compares as a leg's mount path is compared (`path_overlap`), and a finding +# about a name in the way only once case is ignored says so. It is refused +# on Linux too, where git would hold the pair: one plan is then not good on +# one machine and refused after both legs exist on another. +# +# A BRANCH THE SOURCE HIDES (#520). A local repository's branches were read +# with `git ls-remote`, which asks `git upload-pack`, and that leaves out a +# branch hidden by `uploadpack.hideRefs` or `transfer.hideRefs`: `check` said +# `plan ok`, and `execute --local-remote-dir` made both legs before the push +# of the split met the branch on git's ref lock, measured on main at bfed143 +# with git 2.43. A push into a repository meets its own refs, so those are +# what `_branches_at` reads of a local one, with `git for-each-ref`. A clone +# of it obeys the setting, as `ls-remote` does, so the extraction never +# meets such a branch: a source branch the clone gets is about either run, +# and one the source hides is about the push into it alone +# (`_places_in_the_source`). +# # ONE CODE FOR ONE SITUATION. `check`'s finding for the split branch itself # is `adopt-branch-exists`: the code `execute` refuses that branch under, and # the one `_left_by_the_split` tells a run that pushed its split the re-run @@ -5477,11 +5601,22 @@ def _entry_findings(plan: Plan, source: Source, SPLIT_BRANCH_TAKEN_REMEDIATION = ( "Remediation: no leg repository was created and nothing was pushed. Git " "holds no branch whose name is a directory of another's, or lies under " - "it, so the plan's split branch cannot be made beside a branch named " - "above, where it is named. Delete that branch there, once neither it " - "nor a pull request from it is wanted, or set `adopt_branch:` in the " - "plan to a name git can create there, then run `check` and `execute` " - "again.") + "it, and a macOS or Windows disk holds none whose name is another's, or " + "such a name, in another case, so the plan's split branch cannot be made " + "beside a branch named above, where it is named. Delete that branch " + "there, once neither it nor a pull request from it is wanted, or set " + "`adopt_branch:` in the plan to a name git can create there, then run " + "`check` and `execute` again.") +#: Why a branch is in git's way of the split branch only once case is +#: ignored, said after `CASE_ASIDE` in a finding (#520's A4). +SPLIT_BRANCH_CASE_CLASH = ( + ": git keeps each branch as a file named after it, and such a disk keeps " + "two paths that differ only by case as one. `check` and `execute` refuse " + "such a pair on every system") +#: The listing `git for-each-ref` prints of a repository's own branches, in +#: the shape `git ls-remote` prints them: ` TAB `. +REF_LISTING = "--format=%(objectname)%09%(refname)" +BRANCH_REFS = "refs/heads/" def _listed_branches(raw: bytes, prefix: str) -> dict[str, str]: @@ -5498,29 +5633,55 @@ def _listed_branches(raw: bytes, prefix: str) -> dict[str, str]: return out -def _branches_at(url: str, cwd: Path) -> dict[str, str]: - """Every branch of the repository at `url`, as a push to it meets them: - `git ls-remote --heads`. One that cannot be listed fails as `run` fails - a command, `CommandFailed`, and in `execute` before any leg exists: - `_refuse_a_split_branch_left_behind` has just listed the same - repository, and a failure there is the same.""" - args = ["git", *FILE_PROTOCOL, "ls-remote", "--heads", "--", url] +def _listed_by(args: list[str], cwd: Path) -> dict[str, str]: + """`_listed_branches` of the branches `args` prints in `cwd`. A listing + that fails raises as `run` fails a command, `CommandFailed`.""" listed = subprocess.run(args, cwd=cwd, capture_output=True, check=False) if listed.returncode: raise CommandFailed(args, cwd, listed.returncode, listed.stderr.decode("utf-8", "replace")) - return _listed_branches(listed.stdout, "refs/heads/") + return _listed_branches(listed.stdout, BRANCH_REFS) + + +def _branches_at(url: str, cwd: Path) -> dict[str, str]: + """Every branch of the repository at `url`, as a push to it meets them. + A LOCAL path's are its own refs, `git for-each-ref` (#520): `git + ls-remote` leaves out a branch the repository hides with `hideRefs`, and + a push into it meets that branch all the same. Any other url is listed + with `git ls-remote --heads`, as the push will meet it + (`_branches_a_clone_gets`). One that cannot be listed fails as `run` + fails a command, `CommandFailed`, and in `execute` before any leg + exists: `_refuse_a_split_branch_left_behind` has just asked the same + repository, and a failure there is the same.""" + if remote_is_a_path(url): + return _listed_by(["git", "for-each-ref", REF_LISTING, BRANCH_REFS], + Path(url)) + return _branches_a_clone_gets(url, cwd) + + +def _branches_a_clone_gets(url: str, cwd: Path) -> dict[str, str]: + """The branches the repository at `url` gives a fetch or a clone: `git + ls-remote --heads`, which leaves out one it hides with `hideRefs`. Of a + local source, these are the branches `_extract_leg`'s `git clone` gets, + and so the ones its `git filter-repo` meets.""" + return _listed_by(["git", *FILE_PROTOCOL, "ls-remote", "--heads", "--", + url], cwd) def _in_the_way(branches: dict[str, str], branch: str) -> list[tuple]: - """`(name, commit)` of each of `branches` that stops `refs/heads/` - being created as a new branch: `branch` itself, first, then each whose - name is a directory of it or lies under it, which git cannot hold beside - it.""" - return sorted(((name, commit) for name, commit in branches.items() - if name == branch or name.startswith(branch + "/") - or branch.startswith(name + "/")), - key=lambda found: (found[0] != branch, found[0])) + """`(name, commit, aside)` of each of `branches` that stops + `refs/heads/` being created as a new branch: `branch` itself, + first, then each whose name is a directory of it or lies under it, which + git cannot hold beside it. COMPARED WITHOUT CASE TOO (#520's A4), as a + leg's mount path is (`path_overlap`): `aside` is `CASE_ASIDE` for a name + in the way only once case is ignored, and empty for one that is in the + way on every disk.""" + found = [] + for name, commit in branches.items(): + overlap = _overlap(branch, name) + if overlap: + found.append((name, commit, overlap[1])) + return sorted(found, key=lambda each: (each[0] != branch, each[0])) def _left_behind(where: str, run: str, branch: str, commit: str, @@ -5540,39 +5701,45 @@ def _left_behind(where: str, run: str, branch: str, commit: str, "`adopt_branch:` in the plan to a name no branch has there.") -def _taken(where: str, run: str, branch: str, name: str, - commit: str) -> str: +def _taken(where: str, run: str, branch: str, name: str, commit: str, + aside: str = "") -> str: """One `plan-split-branch-taken` finding about the push: `where` holds the branch `name`, at `commit`, which git cannot hold beside the split - branch `branch`, and `run` pushes the split there.""" + branch `branch` (once case is ignored, where `aside` says so), and `run` + pushes the split there.""" split, held = _spelled(branch), _spelled(name) # git's own words, each name spelled as `_spelled` spells it, so a - # backtick a branch name may hold cannot end the quotation. + # backtick a branch name may hold cannot end the quotation. A disk that + # ignores case is not measured here, so it is not quoted. + why = (aside + SPLIT_BRANCH_CASE_CLASH if aside else + f": `'refs/heads/{held[1:-1]}' exists; cannot create " + f"'refs/heads/{split[1:-1]}'`") return (f"{where} holds the branch {held}, at {commit[:12]}. {run} " f"pushes its split commit to {split} there, after both leg " "repositories were created and pushed, and git cannot create " - f"{split} beside {held}: `'refs/heads/{held[1:-1]}' exists; " - f"cannot create 'refs/heads/{split[1:-1]}'`. Delete {held} " - "there, or set `adopt_branch:` in the plan to a branch git can " - "create there.") + f"{split} beside {held}{why}. Delete {held} there, or set " + "`adopt_branch:` in the plan to a branch git can create there.") -def _in_the_extraction(where: str, branch: str, name: str, - commit: str) -> str: +def _in_the_extraction(where: str, branch: str, name: str, commit: str, + aside: str = "") -> str: """One `plan-split-branch-taken` finding: the source, `where`, holds the branch `name` at `commit`, which git cannot hold beside the split branch - `branch`, and `_extract_leg`'s `git filter-repo` makes each branch of - the source a branch of a clone where `branch` is checked out. Either - run extracts, so the finding is about both.""" + `branch` (once case is ignored, where `aside` says so), and + `_extract_leg`'s `git filter-repo` makes each branch of the source a + branch of a clone where `branch` is checked out. Either run extracts, so + the finding is about both.""" split, held = _spelled(branch), _spelled(name) + why = (aside + SPLIT_BRANCH_CASE_CLASH if aside else + ": git holds no branch whose name is a directory of another's, or " + "lies under it") return (f"{where} holds the branch {held}, at {commit[:12]}. " f"{EITHER_RUN} extracts a leg from a clone of the source with " f"the plan's split branch {split} checked out in it, after both " "leg repositories were created, and `git filter-repo` cannot " - f"create {held} beside {split} there: git holds no branch whose " - "name is a directory of another's, or lies under it. Delete " - f"{held} there, or set `adopt_branch:` in the plan to a branch " - "git can create there.") + f"create {held} beside {split} there{why}. Delete {held} there, " + "or set `adopt_branch:` in the plan to a branch git can create " + "there.") def _split_branch_findings(branch: str | None, places: list, @@ -5595,14 +5762,15 @@ def _split_branch_findings(branch: str | None, places: list, and nothing is asked about it here.""" found = [] for where, pushed_by, extracted, branches in (places if branch else []): - for name, commit in _in_the_way(branches, branch): + for name, commit, aside in _in_the_way(branches, branch): if name != branch and extracted: found.append(f"FINDING {SPLIT_BRANCH_TAKEN}: " + _in_the_extraction(where, branch, name, - commit)) + commit, aside)) elif name != branch and pushed_by: found.append(f"FINDING {SPLIT_BRANCH_TAKEN}: " - + _taken(where, pushed_by, branch, name, commit)) + + _taken(where, pushed_by, branch, name, commit, + aside)) elif (name == branch and pushed_by and source is not None and not _in_the_history_of(source, commit)): found.append(f"FINDING {SPLIT_BRANCH_EXISTS}: " @@ -5630,19 +5798,39 @@ def _branches_in(path: Path, refs: str) -> dict[str, str]: """`_listed_branches` of the refs under `refs` in the repository at `path`, read from it with `git for-each-ref`: no network.""" return _listed_branches( - git_out(["for-each-ref", "--format=%(objectname)%09%(refname)", - refs], cwd=path, binary=True), refs) + git_out(["for-each-ref", REF_LISTING, refs], cwd=path, binary=True), + refs) + + +def _places_in_the_source(where: str, path: Path, extracting: bool) -> list: + """The source at `path`, named `where`, as the place a run with + `--local-remote-dir` pushes the split to, `(where, pushed_by, extracted, + branches)`, as `_split_branch_findings` asks it: its own branches, as + the push meets them (`_branches_at`). While `extracting`, those a clone + of it gets (`_branches_a_clone_gets`) are met by either run's extraction + too, and one it hides from a clone with `hideRefs` (#520) is met by the + push alone, a second place, so a finding about it is about that run.""" + branches = _branches_at(str(path), path) + if not extracting: + return [(where, LOCAL_RUN, False, branches)] + cloned = _branches_a_clone_gets(str(path), path) + return [(where, LOCAL_RUN, True, + {name: tip for name, tip in branches.items() if name in cloned}), + (where, LOCAL_RUN, False, + {name: tip for name, tip in branches.items() + if name not in cloned})] def _places_check_asks(plan: Plan, source: Source, extracting: bool) -> list: """The places `check` asks for the split branch, `(where, pushed_by, extracted, branches)`: the source, which a run with `--local-remote-dir` - pushes to and either run extracts from while `extracting`, and, where - the source's `origin` is the assembly repository, that repository's - branches as the source last fetched them, which a run without it pushes - to. `check` reads no network for either.""" - places = [(f"the source {_shown(str(source.path))}", LOCAL_RUN, - extracting, _branches_at(str(source.path), source.path))] + pushes to and either run extracts from while `extracting` + (`_places_in_the_source`), and, where the source's `origin` is the + assembly repository, that repository's branches as the source last + fetched them, which a run without it pushes to. `check` reads no + network for either.""" + places = _places_in_the_source(f"the source {_shown(str(source.path))}", + source.path, extracting) assembly = _assembly_repository(plan) if assembly and (source.repository or "").lower() == assembly.lower(): places.append((f"`{assembly}` on GitHub, as the source's `origin` " @@ -5655,14 +5843,14 @@ def _check_split_branch(plan: Plan, source: Source ) -> tuple[list[str], list[str]]: """`_split_branch_findings` as `check` asks it, the split branch's own name included, and what to print ONCE after every finding, as - `_submodule_findings` returns it. A split branch that is not a value - `execute` puts on a command line is not asked about: it is `execute`'s - refusal `unsafe-value`, before any leg exists.""" - try: - branch = checked_value("adopt_branch", - plan.get("adopt_branch", ADOPT_BRANCH)) - except Refusal: + `_submodule_findings` returns it. A split branch whose name `execute` + will not hand git is not asked about (#520): `_entry_findings` names it, + as `unsafe-value` or `plan-bad-adopt-branch`, and `execute` refuses it + before any leg exists.""" + if _branch_name_refusal(plan, *SPLIT_BRANCH_KEY) is not None: return [], [] + key, default, _ = SPLIT_BRANCH_KEY + branch = checked_value(key, plan.get(key, default)) extracting = _extracts_a_leg(assigned_paths(plan.entries)) found = _split_branch_findings( branch, _places_check_asks(plan, source, extracting), source) @@ -5684,22 +5872,24 @@ def _refuse_a_branch_in_the_split_branchs_way( nothing then. Under `--local-remote-dir` the split is pushed to the source, which is - also what the extraction clones: one listing, `git ls-remote --heads`, - as the push will meet it, and each finding is `check`'s, word for word. - Without it, the source's own branches are asked for the extraction, - reading no network, and each finding is `check`'s, word for word; then - the assembly repository on GitHub, as the push will meet it, which - `check` reads as the source's `origin` last fetched it, if at all. + also what the extraction clones: the places `check` asks there + (`_places_in_the_source`), its own branches as the push meets them and + those a clone of it gets, and each finding is `check`'s, word for word. + Without it, the branches a clone of the source gets are asked for the + extraction, reading no network, and each finding is `check`'s, word for + word: a branch the source hides from a clone (#520) is met by no part + of this run. Then the assembly repository on GitHub, as the push will + meet it, which `check` reads as the source's `origin` last fetched it, + if at all. """ if local and any(Path(urls[role]).exists() for role in EXTRACTED_LEGS): return the_source = f"the source {_shown(str(source.path))}" if local: - places = [(the_source, LOCAL_RUN, extracting, - _branches_at(urls["assembly"], source.path))] + places = _places_in_the_source(the_source, source.path, extracting) else: places = [(the_source, None, True, - _branches_in(source.path, "refs/heads/")) + _branches_a_clone_gets(str(source.path), source.path)) ] if extracting else [] places.append((f"`{repositories['assembly']}` on GitHub", GITHUB_RUN, False, _branches_at(urls["assembly"], source.path))) @@ -7030,6 +7220,16 @@ def _in_the_history_of(source: Source, tip: str) -> bool: return asked.returncode == 0 +def _tip_at(url: str, branch: str) -> str | None: + """The commit the repository at `url` has `branch` at, as `git ls-remote` + reads it now, or None where it has no such branch.""" + ref = f"refs/heads/{branch}" + listed = run(["git", "ls-remote", url, ref]) + tips = [line.split("\t")[0] for line in listed.splitlines() + if line.split("\t")[-1] == ref] + return tips[0] if tips else None + + def _refuse_a_split_branch_left_behind(source: Source, urls: dict, branch: str, local: bool) -> None: """Refuse a run whose split branch is already on the assembly, BEFORE a @@ -7041,8 +7241,11 @@ def _refuse_a_split_branch_left_behind(source: Source, urls: dict, a failed verification prescribed, into fresh legs, created and pushed two more legs and was then refused at the split push, every time: against GitHub, two more real repositories per attempt. The branch is - asked of the very remote the split is pushed to, with `git ls-remote`, - so the answer is the push's own. + asked of the very remote the split is pushed to, with `git ls-remote` + (`_tip_at`), so the answer is the push's own; under `--local-remote-dir` + that remote is the source, whose own refs are read (`_branches_at`), so + that one it hides from `git ls-remote` with `hideRefs`, which `check` + finds, is refused too (#520): the push into it meets that branch. ONLY A BRANCH THE PUSH CANNOT FAST-FORWARD IS REFUSED (#188's review, N1). One at the source commit, or behind it, is moved forward by the @@ -7064,17 +7267,17 @@ def _refuse_a_split_branch_left_behind(source: Source, urls: dict, """ if local and any(Path(urls[role]).exists() for role in EXTRACTED_LEGS): return - ref = f"refs/heads/{branch}" - listed = run(["git", "ls-remote", urls["assembly"], ref]) - tips = [line.split("\t")[0] for line in listed.splitlines() - if line.split("\t")[-1] == ref] - if not tips or _in_the_history_of(source, tips[0]): + # Under `--local-remote-dir` the push goes into the source, and meets its + # own refs, a branch it hides from `git ls-remote` too (#520). + tip = (_branches_at(urls["assembly"], source.path).get(branch) if local + else _tip_at(urls["assembly"], branch)) + if tip is None or _in_the_history_of(source, tip): return raise Refusal( "adopt-branch-exists", f"{_spelled(urls['assembly'])} already has {_spelled(branch)}, the " "branch this adoption pushes its split commit to, at " - f"{tips[0][:12]}: neither the source commit {source.commit[:12]} nor " + f"{tip[:12]}: neither the source commit {source.commit[:12]} nor " "a commit behind it", "Remediation: no leg repository was created and nothing was pushed. " "The split commit is made on top of the source commit, git moves a " @@ -7485,6 +7688,10 @@ def _checked_plan_values(plan: Plan) -> tuple[str, str, str, str]: `(spec_path, code_path, branch, tracking)`. Split out of `cmd_execute` for #138, in the order the refusals came in. + A branch name `checked_value` refuses is `check`'s finding too, in the + same words (`_branch_name_refusal`, #520); one inside its alphabet that + git will not create is refused with `check`'s other findings, by + `_refuse_what_check_finds`, and before any leg exists as well. """ spec_leg, code_leg = _plan_leg_paths(plan) # Before ANY leg exists: `check` finds a bad path and `execute` refuses it @@ -7624,7 +7831,10 @@ def _refuse_what_check_finds(plan: Plan, source: Source, to lack. A VALUE THAT IS AN OPTION AND A PATH THAT IS A FACT ARE TWO TESTS (#196). A leg's mount path, a branch, a repository name and the organisation are options: `checked_value` refuses them as `unsafe-value` - in `_checked_plan_values` and `_repository_urls`, before this runs. An + in `_checked_plan_values` and `_repository_urls`, before this runs, and + a branch name inside its alphabet that git will not create is one of + `check`'s findings, refused here (`plan-bad-adopt-branch`, + `plan-bad-tracking-branch`, #520). An entry's path is a fact of the source, judged by `entry_path_problem` as `check`'s own finding `plan-unsafe-path`, which `_entry_findings` lists FIRST: a path that is a `git` option is refused as that, and never as the @@ -9611,7 +9821,10 @@ def main(argv: list[str] | None = None) -> int: "plan records as `source.default_branch`). " "`check` finds, and `execute` refuses before any " "leg exists, one that is no branch of the source " - "(`plan-tracking-branch-missing`).") + "(`plan-tracking-branch-missing`), and `plan` " + "refuses one git will not create as a branch " + "(`adopt-bad-tracking-branch`) and writes no " + "plan.") plan.add_argument("--spec-path", default="spec", help=_leg_path_help("spec", "spec")) plan.add_argument("--code-path", default="code", diff --git a/docs/cli.md b/docs/cli.md index c793a3e0..80b3f872 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -406,7 +406,9 @@ options: source's default branch, which the plan records as `source.default_branch`). `check` finds, and `execute` refuses before any leg exists, one that is no branch - of the source (`plan-tracking-branch-missing`). + of the source (`plan-tracking-branch-missing`), and + `plan` refuses one git will not create as a branch + (`adopt-bad-tracking-branch`) and writes no plan. --spec-path SPEC_PATH where the spec leg is mounted, as a path in the assembly root (default: spec); the plan records it as diff --git a/tests/test_adopt_branch_names.py b/tests/test_adopt_branch_names.py new file mode 100644 index 00000000..0489bea1 --- /dev/null +++ b/tests/test_adopt_branch_names.py @@ -0,0 +1,264 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The plan's two branch names are asked what `execute` asks of them (#420, +#520, and L2-1 on #457). + +`execute` hands two plan values to git as branch names: `adopt_branch:`, the +split branch, and `tracking_branch:`, the branch both legs are pushed to. It +holds each to `checked_value`'s alphabet first (`_checked_plan_values`), and +git refuses names inside that alphabet. Measured on main at bfed143, under +`--local-remote-dir`: + +- an `adopt_branch:` edited to a name git will not create (`a..b`, `~`, + `x.lock`, `a/`) checked `plan ok`, and `execute` made both leg + repositories and died at the extraction's `git checkout -B`; the same plan, + corrected, was then refused `leg-remote-exists`; +- such a `tracking_branch:` was found since #479, but as + `plan-tracking-branch-missing`, a branch the source does not have, and + `plan --tracking-branch a..b` wrote the plan with a NOTE saying so; +- either name outside the alphabet that git accepts (`a=b`) checked `plan + ok`, and `execute` refused it as `unsafe-value`. + +Now `plan` refuses such a `--tracking-branch` as `adopt-bad-tracking-branch` +and writes nothing, and `check` finds, and `execute` refuses before any leg +exists, each of the two names: held to the alphabet first, as `execute`'s own +`unsafe-value` in its own words, and otherwise asked of git with #283's +`refuse_bad_branch`, as `plan-bad-adopt-branch` or `plan-bad-tracking-branch`. +One finding per value: a name outside the alphabet is not asked of git, a +split branch that is either is not looked for where the split is pushed, and +a tracking branch that is either is not also said to be missing from the +source. + +Every run here is `plan` or `check`, which read a local source, or `execute` +in this process, refused before any leg; the one end-to-end test needs `git +filter-repo`. Each plan is edited as bytes, so it stays LF on every platform, +and a name git refuses is only ever a plan value, never a branch on disk. +""" + +from __future__ import annotations + +import importlib.util +import shutil +from argparse import Namespace +from pathlib import Path + +import pytest + +from conftest import (ADOPT, findings, git, make_source_repo, + refused_before_any_leg, run_script, write_plan) + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +#: A source small enough that `plan` asks no question. +TREE = {"README.md": "# Northwind\n", "src/app.py": "print(1)\n", + "docs/spec.md": "# Spec\n"} +SPLIT_KEY, TRACKING_KEY = "adopt_branch", "tracking_branch" +SPLIT_BRANCH = "adopt/three-repo-shape" +#: Where `plan` writes the plan, and where the leg repositories go. +PLAN_FILE, REMOTES = "adoption-plan.yaml", "remotes" +UNSAFE = "unsafe-value" +BAD_SPLIT, BAD_TRACKING = "plan-bad-adopt-branch", "plan-bad-tracking-branch" +#: Each key, with the code `check` finds a name git refuses under. +KEYS = pytest.mark.parametrize( + "key, code", [(SPLIT_KEY, BAD_SPLIT), (TRACKING_KEY, BAD_TRACKING)], + ids=["adopt-branch", "tracking-branch"]) +#: Inside `checked_value`'s alphabet and refused by git: #420's table. +DOT_DOT = "a..b" +GIT_REFUSES = [DOT_DOT, "~", "x.lock", "a/", "a//b", ".a"] +#: A name git creates, inside the alphabet, which the source holds: one of +#: #283's own examples. +GIT_CREATES = "lane-3/shape+fix@eagle" +#: Outside the alphabet: `a=b` git accepts, and the source holds (L2-1); +#: `a:b` git refuses too. +EQUALS = "a=b" +OUTSIDE_THE_ALPHABET = [EQUALS, "a:b"] +#: Git's verdict on a name, after the key and the name. +NOT_A_BRANCH = ("which is not a branch name git will create: `git " + "check-ref-format --branch` refuses it") +#: `execute`'s own words for a value outside the alphabet. +NOT_ON_A_COMMAND_LINE = ("which is not a value this tool will put on a `git` " + "or `gh` command line") +#: What `check` says once after a finding about either name. +NAME_REMEDIATION = "Remediation: set each branch name a finding names" + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_branch_names", ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture(scope="module") +def world(tmp_path_factory) -> tuple[Path, Path]: + """One source, holding every name here git creates as a branch, and + `a`, which is in git's way of a split branch `a/`, and one plan of it. + `plan` and `check` only read the source, and a test that edits the plan + edits a copy (`with_branch`).""" + root = tmp_path_factory.mktemp("branch-names") + source = make_source_repo(root / "Northwind", tree=TREE, edits=()) + for name in (GIT_CREATES, EQUALS, "a"): + git("branch", name, cwd=source) + plan = root / PLAN_FILE + written = write_plan(source, plan) + assert written.returncode == 0, written.stderr + written.stdout + return source, plan + + +def edited(plan: Path, key: str, name: str | None) -> Path: + """`plan` with its `key:` line set to `name`, double-quoted, as a person + edits it (`~` alone is YAML's null), or deleted when `name` is None, + which leaves `execute`'s default. Bytes in and out: LF, as `plan` wrote + it.""" + lines = plan.read_bytes().split(b"\n") + (at,) = [index for index, line in enumerate(lines) + if line.startswith(f"{key}: ".encode())] + if name is None: + del lines[at] + else: + lines[at] = f'{key}: "{name}"'.encode() + plan.write_bytes(b"\n".join(lines)) + return plan + + +def with_branch(world, base: Path, key: str, name: str | None) -> Path: + """A private copy of the world's plan, `edited`.""" + copy = base / PLAN_FILE + copy.write_bytes(world[1].read_bytes()) + return edited(copy, key, name) + + +def execute_args(plan: Path, base: Path) -> Namespace: + """`execute --yes --local-remote-dir /remotes` of `plan`.""" + return Namespace(plan=str(plan), source=None, + local_remote_dir=base / REMOTES, allow_empty_leg=[], + yes=True, work_dir=base / "work") + + +# --- `plan` ------------------------------------------------------------------ + +@pytest.mark.parametrize("name", GIT_REFUSES) +def test_plan_refuses_a_tracking_branch_git_will_not_create(world, tmp_path, + name): + """#420's first row. On main `plan` wrote it and said in a NOTE only that + the source has no such branch, which `check` then found, and the plan + was written again. It is refused now, in git's words, and nothing is + written.""" + plan = tmp_path / PLAN_FILE + + result = write_plan(world[0], plan, extra=("--tracking-branch", name)) + + assert result.returncode == 2, result.stdout + result.stderr + assert (f"REFUSED adopt-bad-tracking-branch: --tracking-branch is " + f"{name!r}, {NOT_A_BRANCH}") in result.stderr, result.stderr + assert not plan.exists() + + +# --- `check` ----------------------------------------------------------------- + +@KEYS +@pytest.mark.parametrize("name", [DOT_DOT, "x.lock", "~", "a/"]) +def test_check_finds_a_name_git_will_not_create_once(world, tmp_path, key, + code, name): + """#420's second and third rows: a hand-edited name git refuses is ONE + finding, in git's words, under the key's own code. The tracking branch + is not also said to be missing from the source, and the split branch is + not looked for where the split is pushed: the name is what is wrong.""" + ((found, detail),) = findings(with_branch(world, tmp_path, key, name)) + assert found == code + assert detail.startswith(f"{key} is {name!r}, {NOT_A_BRANCH} ("), detail + + +@pytest.mark.parametrize("key", [SPLIT_KEY, TRACKING_KEY]) +def test_check_holds_both_names_to_the_alphabet_execute_holds_them_to( + world, tmp_path, key): + """L2-1: `a=b`, which git accepts and the source holds, checked `plan + ok`, and `execute` refused it as `unsafe-value`; `a:b`, which git + refuses too, is one finding and not two. Each is `execute`'s own + refusal, as a finding, in its words. A name git creates inside the + alphabet checks ok, and so does a plan without the key, which `execute` + reads as its default.""" + for name in OUTSIDE_THE_ALPHABET: + assert findings(with_branch(world, tmp_path, key, name)) == [ + (UNSAFE, f"{key} is {name!r}, {NOT_ON_A_COMMAND_LINE}")] + assert findings(with_branch(world, tmp_path, key, GIT_CREATES)) == [] + assert findings(with_branch(world, tmp_path, key, None)) == [] + + +def test_both_names_are_found_in_one_run_and_the_way_on_said_once( + world, tmp_path): + """Two names wrong, two findings, the split branch's first, as + `execute` asks them; and what to do about them, once.""" + plan = edited(with_branch(world, tmp_path, SPLIT_KEY, DOT_DOT), + TRACKING_KEY, EQUALS) + + result = run_script(ADOPT, "check", "--plan", str(plan)) + + assert result.returncode == 1, result.stdout + result.stderr + found = [line.split(":", 1)[0] for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + assert found == [f"FINDING {BAD_SPLIT}", f"FINDING {UNSAFE}"], found + assert result.stderr.count(NAME_REMEDIATION) == 1, result.stderr + assert "\n2 finding(s) in " in result.stderr + + +# --- `execute` --------------------------------------------------------------- + +@KEYS +def test_execute_refuses_a_name_git_will_not_create_before_any_leg( + adopter, world, tmp_path, monkeypatch, key, code): + """`check`'s finding, refused by `_refuse_what_check_finds` before any + leg exists, word for word: on main a split branch was refused only by + the extraction's `git checkout -B`, after both legs were made.""" + plan = with_branch(world, tmp_path, key, DOT_DOT) + refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + assert refusal.code == code + + +@pytest.mark.parametrize("key", [SPLIT_KEY, TRACKING_KEY]) +def test_execute_refuses_a_name_outside_the_alphabet_as_check_finds_it( + adopter, world, tmp_path, monkeypatch, key): + """`_checked_plan_values` refuses it first, as it always did, and its + refusal is now `check`'s finding: one code, one sentence.""" + plan = with_branch(world, tmp_path, key, EQUALS) + ((code, detail),) = findings(plan) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + args = execute_args(plan, tmp_path) + + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(args) + + assert (caught.value.code, caught.value.detail) == (code, detail) + assert not (tmp_path / REMOTES).exists() + + +@needs_filter_repo +def test_a_split_branch_git_will_not_create_makes_no_leg_and_the_fix_adopts( + tmp_path): + """#420's third and fifth rows, end to end. On main `execute` made both + leg repositories, died at the extraction's `git checkout -B`, and the + corrected plan, run into the same `--local-remote-dir`, was refused + `leg-remote-exists`. Now nothing is made, and the corrected plan adopts + there.""" + source = make_source_repo(tmp_path / "Northwind", tree=TREE, edits=()) + plan = tmp_path / PLAN_FILE + written = write_plan(source, plan) + assert written.returncode == 0, written.stderr + written.stdout + remotes = tmp_path / REMOTES + flags = ("--plan", str(plan), "--yes", "--local-remote-dir", str(remotes), + "--work-dir", str(tmp_path / "work")) + + edited(plan, SPLIT_KEY, DOT_DOT) + refused = run_script(ADOPT, "execute", *flags) + + assert refused.returncode == 2, refused.stderr + refused.stdout + assert f"REFUSED {BAD_SPLIT}: 1 finding(s) in " in refused.stderr + assert not remotes.exists() + assert git("for-each-ref", "--format=%(refname:short)", "refs/heads/", + cwd=source).stdout.split() == ["main"] + edited(plan, SPLIT_KEY, SPLIT_BRANCH) + adopted = run_script(ADOPT, "execute", *flags) + assert adopted.returncode == 0, adopted.stderr + adopted.stdout + assert "adoption verified" in adopted.stdout diff --git a/tests/test_adopt_plan_yaml_quoting.py b/tests/test_adopt_plan_yaml_quoting.py index 0511c0c0..a9ca7a4d 100644 --- a/tests/test_adopt_plan_yaml_quoting.py +++ b/tests/test_adopt_plan_yaml_quoting.py @@ -1189,20 +1189,22 @@ def test_the_misread_rule_asks_nothing_of_a_value_execute_never_writes( BRANCH_GIT_REFUSES = "~" -def test_plan_records_a_branch_git_will_not_create_as_on_main( +def test_plan_refuses_a_branch_git_will_not_create_in_gits_words( answered, tmp_path): """`plan` refused `--tracking-branch ~` as misread, with a remediation saying `execute` writes it and the run would still end `adoption - verified`. Git will not create `~`, so no source has it: `check` finds - it `plan-tracking-branch-missing` and `execute` refuses it before any - leg exists (#479). `plan` writes the plan, with #479's note and no - misread refusal, as on main (rv181f's H1).""" + verified` (rv181f's H1). Git will not create `~`, so no source has it, + and since #520 `plan` asks git about the flag: it refuses it as + `adopt-bad-tracking-branch`, in git's words, and never as misread, and + writes no plan (#420).""" source, _ = answered plan = tmp_path / PLAN_FILE result = write_plan(source, plan, extra=(TRACKING_FLAG, BRANCH_GIT_REFUSES)) - assert result.returncode == 0, result.stdout + result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert "REFUSED adopt-bad-tracking-branch: " in result.stderr assert "adopt-manifest-misread" not in result.stderr, result.stderr + assert not plan.exists() def test_git_is_asked_only_about_a_branch_the_loader_misreads( diff --git a/tests/test_adopt_split_branch.py b/tests/test_adopt_split_branch.py index 375eaef7..21707036 100644 --- a/tests/test_adopt_split_branch.py +++ b/tests/test_adopt_split_branch.py @@ -50,6 +50,8 @@ SPLIT_BRANCH = "adopt/three-repo-shape" PROJECT, ORG = "Northwind", "testorg" REMOTES, WORK = "remotes", "work" +#: How a finding names the run that pushes the split into the source. +LOCAL_RUN = "`execute --local-remote-dir`" #: A source `plan` asks no question about: `specs/` and `src/` classify #: themselves and `README.md` stays in the root. SMALL_TREE = {"README.md": "# Thing\n", "src/app/util.py": "VALUE = 1\n", @@ -149,7 +151,7 @@ def test_check_finds_an_earlier_split_branch_in_the_source(tmp_path): found = findings(plan_for(source, tmp_path)) head = git("rev-parse", "main", cwd=source).stdout.strip() assert found == [(BRANCH_EXISTS, left_behind( - f"the source {source}", "`execute --local-remote-dir`", old, head))] + f"the source {source}", LOCAL_RUN, old, head))] def test_check_says_how_to_go_on_once_after_its_findings(adopter, tmp_path): @@ -213,36 +215,79 @@ def test_a_branch_the_split_push_fast_forwards_is_no_finding( "three-repo-shape"] -def in_the_extraction(source: Path, name: str, at: str) -> str: +#: Why a name is in git's way of the split branch, as a finding says it: a +#: directory of it, or a name under it. +DIRECTORY_CLASH = (": git holds no branch whose name is a directory of " + "another's, or lies under it") + + +def in_the_extraction(source: Path, name: str, at: str, + why: str = DIRECTORY_CLASH) -> str: """`check`'s finding for a source branch `name` at `at` that git cannot - hold beside the split branch: about the extraction, which both runs - make.""" + hold beside the split branch, for the reason `why`: about the + extraction, which both runs make.""" return (f"the source {source} holds the branch `{name}`, at {at[:12]}. " "`execute`, with `--local-remote-dir` or without it, extracts a " "leg from a clone of the source with the plan's split branch " f"`{SPLIT_BRANCH}` checked out in it, after both leg repositories " f"were created, and `git filter-repo` cannot create `{name}` " - f"beside `{SPLIT_BRANCH}` there: git holds no branch whose name " - "is a directory of another's, or lies under it. Delete " - f"`{name}` there, or set `adopt_branch:` in the plan to a branch " - "git can create there.") - - -@pytest.mark.parametrize("name", IN_THE_WAY) + f"beside `{SPLIT_BRANCH}` there{why}. Delete `{name}` there, or " + "set `adopt_branch:` in the plan to a branch git can create " + "there.") + + +#: Names in git's way of the split branch only once case is ignored: a +#: directory of it, the split branch itself and a name under it, each in +#: another case. #457 reports `execute` making both legs on macOS and +#: Windows before git refused such a name; on Linux, at bfed143, each +#: checked `plan ok` and adopted. Ruled 2026-10-12: compared without case on +#: every system. +ADOPT_IN_ANOTHER_CASE = "Adopt" +BUT_FOR_CASE = [ADOPT_IN_ANOTHER_CASE, SPLIT_BRANCH.upper(), + "Adopt/Three-Repo-Shape/old"] +#: Why such a name is in the way, as a finding says it. +CASE_CLASH = (" once case is ignored, as a macOS or Windows disk does: git " + "keeps each branch as a file named after it, and such a disk " + "keeps two paths that differ only by case as one. `check` and " + "`execute` refuse such a pair on every system") + + +def pushed_beside(where: str, run: str, name: str, at: str, + why: str | None = None) -> str: + """`check`'s finding for a branch `name` at `at`, held where `run` + pushes the split, that git cannot hold beside the split branch: in + git's own words, or for the reason `why`.""" + why = why or (f": `'refs/heads/{name}' exists; cannot create " + f"'refs/heads/{SPLIT_BRANCH}'`") + return (f"{where} holds the branch `{name}`, at {at[:12]}. {run} pushes " + f"its split commit to `{SPLIT_BRANCH}` there, after both leg " + "repositories were created and pushed, and git cannot create " + f"`{SPLIT_BRANCH}` beside `{name}`{why}. Delete `{name}` there, " + "or set `adopt_branch:` in the plan to a branch git can create " + "there.") + + +@pytest.mark.parametrize( + "name, why", [(name, DIRECTORY_CLASH) for name in IN_THE_WAY] + + [(name, CASE_CLASH) for name in BUT_FOR_CASE], + ids=IN_THE_WAY + BUT_FOR_CASE) def test_a_branch_in_gits_way_of_the_split_branch_is_taken(adopter, tmp_path, monkeypatch, - name): + name, why): """Said as the extraction's failure, for both runs: the review of #320 measured a run without `--local-remote-dir` failing there too, after both GitHub leg repositories were made, while this finding said it was about `execute --local-remote-dir` alone. `execute` refuses it with - `check`'s finding, word for word, before any leg exists.""" + `check`'s finding, word for word, before any leg exists. The names in + the way only once case is ignored (#520's A4) were no finding on main, + and are refused the same way, saying why.""" source = small_source(tmp_path) git("branch", name, cwd=source) at = git("rev-parse", name, cwd=source).stdout.strip() plan = plan_for(source, tmp_path) - assert findings(plan) == [(TAKEN, in_the_extraction(source, name, at))] + assert findings(plan) == [(TAKEN, in_the_extraction(source, name, at, + why))] refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) assert refusal.code == TAKEN assert refusal.remediation == adopter.SPLIT_BRANCH_TAKEN_REMEDIATION @@ -543,28 +588,28 @@ def test_execute_refuses_a_repository_it_cannot_list(adopter, tmp_path, assert asked == [] +@pytest.mark.parametrize("name, why", [ + ("adopt", None), (ADOPT_IN_ANOTHER_CASE, CASE_CLASH)], + ids=["in-the-way", "once-case-is-ignored"]) def test_a_branch_in_gits_way_on_github_is_refused_in_the_pushs_words( - adopter, tmp_path, monkeypatch): + adopter, tmp_path, monkeypatch, name, why): """GitHub holds `adopt`, and the source does not: the push of the split would meet it there, so the finding is about the run that pushes - there, in git's own words.""" + there, in git's own words. `Adopt` is in the way once case is ignored + (#520's A4), and is refused the same way, saying so.""" source = small_source(tmp_path) plan = plan_for(source, tmp_path) - bare = a_github(tmp_path, monkeypatch, branches=("adopt",)) - at = git("rev-parse", "adopt", cwd=bare).stdout.strip() + bare = a_github(tmp_path, monkeypatch, branches=(name,)) + at = git("rev-parse", name, cwd=bare).stdout.strip() asked = no_gh(adopter, monkeypatch) + args = execute_args(plan, tmp_path, local=False) with pytest.raises(adopter.Refusal) as caught: - adopter.cmd_execute(execute_args(plan, tmp_path, local=False)) + adopter.cmd_execute(args) assert caught.value.code == TAKEN - assert (f" FINDING {TAKEN}: `{ORG}/{PROJECT}` on GitHub holds the " - f"branch `adopt`, at {at[:12]}. `execute` without " - "`--local-remote-dir` pushes its split commit to " - f"`{SPLIT_BRANCH}` there, after both leg repositories were " - "created and pushed, and git cannot create " - f"`{SPLIT_BRANCH}` beside `adopt`: `'refs/heads/adopt' exists; " - f"cannot create 'refs/heads/{SPLIT_BRANCH}'`. Delete `adopt` " - "there, or set `adopt_branch:` in the plan to a branch git can " - "create there.") in caught.value.detail.splitlines() + finding = f" FINDING {TAKEN}: " + pushed_beside( + f"`{ORG}/{PROJECT}` on GitHub", "`execute` without " + "`--local-remote-dir`", name, at, why) + assert finding in caught.value.detail.splitlines(), caught.value.detail assert asked == [] @@ -729,6 +774,89 @@ def test_a_fetched_branch_the_split_push_fast_forwards_is_no_finding( assert findings(plan_for(source, tmp_path)) == [] +# --- a branch the source hides (#520) ---------------------------------------- + +#: A branch in git's way of the split branch, hidden by the source's own +#: `.hideRefs`, which `git ls-remote` and a clone of the source obey +#: and a push into it does not: on main at bfed143 `check` said `plan ok`, +#: and `execute --local-remote-dir` made both legs and failed at the push +#: of the split, on git's ref lock. `transfer.hideRefs` hides the branch +#: UNDER the split branch's name, since one that hid `adopt` would hide the +#: split branch from that push too. +UPLOADPACK = "uploadpack" +HIDDEN = [pytest.param(UPLOADPACK, "adopt", id=UPLOADPACK), + pytest.param("transfer", f"{SPLIT_BRANCH}/old", id="transfer")] + + +def hide(source: Path, knob: str, branch: str) -> None: + """`branch` hidden by the source's `.hideRefs`, appended to its + own `.git/config`, LF, as `git config --add` would write it.""" + with (source / ".git" / "config").open("a", encoding="utf-8", + newline="\n") as handle: + handle.write(f"[{knob}]\n\thideRefs = refs/heads/{branch}\n") + + +@pytest.mark.parametrize("knob, name", HIDDEN) +def test_a_branch_the_source_hides_is_taken_where_the_split_is_pushed( + adopter, tmp_path, monkeypatch, knob, name): + """`_branches_at` read the source with `git ls-remote`, which leaves the + branch out. It reads a local repository's own refs now. The extraction's + clone never gets the branch, so the finding is about the push alone, in + the push's words, and `execute --local-remote-dir` refuses it before any + leg exists.""" + source = small_source(tmp_path) + git("branch", name, cwd=source) + hide(source, knob, name) + at = git("rev-parse", name, cwd=source).stdout.strip() + plan = plan_for(source, tmp_path) + assert findings(plan) == [(TAKEN, pushed_beside( + f"the source {source}", LOCAL_RUN, name, at))] + refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + assert refusal.code == TAKEN + + +@pytest.mark.parametrize("knob, name", HIDDEN) +def test_a_github_run_is_not_refused_for_a_branch_the_source_hides( + adopter, tmp_path, monkeypatch, knob, name): + """Without `--local-remote-dir` the split goes to GitHub, which holds no + such branch, and the extraction's clone of the source never gets it: + nothing stops this run. On main `execute` read the source's own refs for + the extraction and refused it, in words about a `git filter-repo` that + never meets the branch.""" + source = small_source(tmp_path) + git("branch", name, cwd=source) + hide(source, knob, name) + plan = plan_for(source, tmp_path) + a_github(tmp_path, monkeypatch, branches=()) + asked = no_gh(adopter, monkeypatch) + args = execute_args(plan, tmp_path, local=False) + with pytest.raises(Proceeded): + adopter.cmd_execute(args) + assert asked[0][:3] == ["gh", "repo", "create"] + + +def test_an_earlier_split_branch_the_source_hides_is_found_and_refused( + adopter, tmp_path, monkeypatch): + """`execute` asked the source for the split branch an earlier run left + with `git ls-remote`, which leaves out one the source hides from a + fetch, and the push of the split still meets it there. `check` names it + as `execute --local-remote-dir`'s `adopt-branch-exists`, and `execute` + refuses it, before any leg exists.""" + source = small_source(tmp_path) + old = split_branch_by_hand(source) + hide(source, UPLOADPACK, SPLIT_BRANCH) + head = git("rev-parse", "main", cwd=source).stdout.strip() + plan = plan_for(source, tmp_path) + assert findings(plan) == [(BRANCH_EXISTS, left_behind( + f"the source {source}", LOCAL_RUN, old, head))] + stop_at_the_legs(adopter, monkeypatch) + args = execute_args(plan, tmp_path) + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(args) + assert caught.value.code == BRANCH_EXISTS + assert not (tmp_path / REMOTES).exists() + + # --- end to end ------------------------------------------------------------- @needs_filter_repo @@ -773,14 +901,19 @@ def test_the_issue_re_run_is_refused_and_runs_once_the_branch_is_gone( @needs_filter_repo +@pytest.mark.parametrize("hidden", [False, True], ids=["seen", "hidden"]) def test_a_source_branch_in_the_way_is_refused_and_runs_once_it_is_gone( - tmp_path): + tmp_path, hidden): """A source branch `adopt`: on main at ec4a7a0 `check` said `plan ok` - and `execute` made both legs and exited 2 at `git filter-repo`. Now - `check` names the branch, `execute` refuses before any leg exists, and - once the branch is deleted the same command adopts.""" + and `execute` made both legs and exited 2 at `git filter-repo`. Hidden + by the source's `uploadpack.hideRefs`, on main at bfed143, `check` said + `plan ok` and `execute` made both legs and exited 2 at the push of the + split (#520). Now `check` names the branch, `execute` refuses before any + leg exists, and once the branch is deleted the same command adopts.""" source = small_source(tmp_path) git("branch", "adopt", cwd=source) + if hidden: + hide(source, UPLOADPACK, "adopt") plan = plan_for(source, tmp_path) flags = ("--plan", str(plan), "--yes") diff --git a/tests/test_adopt_template_values.py b/tests/test_adopt_template_values.py index 2748530b..faf3a008 100644 --- a/tests/test_adopt_template_values.py +++ b/tests/test_adopt_template_values.py @@ -460,8 +460,9 @@ def test_plan_and_check_say_the_same_sentence(tmp_path, answered): #: HAPPEN`, which raised on a strict stdout before any finding about it was #: printed. It is shown as a name is (`_shown`), a byte that is not UTF-8 as #: `\udce9`, as the plan's own `path:` spells it (#329). A leg path is -#: `plan-bad-leg-path`; the adopt branch passes `check` and is `unsafe-value` -#: to `execute`, as each was on main before #247. +#: `plan-bad-leg-path`; the adopt branch is `unsafe-value` to `execute`, as +#: on main before #247, and since #520 (L2-1 on #457) a finding of `check`'s +#: too, in `execute`'s words, where it passed `check` before. PRINTED = [ ("spec_path-surrogate", " spec_path", "sp\ud800ec", rb"`sp\ud800ec`/", 1, b"FINDING plan-bad-leg-path: legs.spec_path is ", @@ -469,7 +470,8 @@ def test_plan_and_check_say_the_same_sentence(tmp_path, answered): ("code_path-byte", " code_path", "co\udce9de", rb"`co\udce9de`/", 1, b"FINDING plan-bad-leg-path: legs.code_path is ", b"plan-bad-leg-path"), ("adopt_branch-surrogate", "adopt_branch", "adopt/x\ud800", - rb"`adopt/x\ud800` of", 0, PLAN_OK, b"unsafe-value"), + rb"`adopt/x\ud800` of", 1, b"FINDING unsafe-value: adopt_branch is ", + b"unsafe-value"), ]