diff --git a/AGENTS.md b/AGENTS.md index 9c424083..58bb0772 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,13 +115,13 @@ scripts/bootstrap.py`. README's "What setup.sh does" carries the exact commands. ## Adopting an existing repository `adopt-project.py` converts a repository that already exists, IN PLACE: it keeps -its name, its identity and its full history and becomes the assembly root, and -the two legs are new repositories extracted with `git filter-repo` (a hard -dependency: `execute` refuses without it, printing the install command). Seven -steps, in order, and step 4 is the one you must not skip. +its name, which `--project` must be, its identity and its full history and +becomes the assembly root, and the two legs are new repositories extracted with +`git filter-repo` (a hard dependency: `execute` refuses without it, printing the +install command). Seven steps, in order, and step 4 is the one you must not skip. ```sh -./adopt-project.py plan --source --project +./adopt-project.py plan --source --project # 2. answer every `review_required: true` entry IN THE PLAN ./adopt-project.py check --plan adoption-plan.yaml # 4. show the human the plan and the follow-ups; get an explicit yes @@ -167,7 +167,7 @@ history, not an empty repository. `plan` and `check` both WARN; `check` still passes, because a spec-only repository is a legitimate thing to adopt. ```sh -./adopt-project.py plan --source / --project \ +./adopt-project.py plan --source / --project \ --allow-empty-leg code ``` diff --git a/README.md b/README.md index 8527950a..6bf94d58 100644 --- a/README.md +++ b/README.md @@ -727,14 +727,14 @@ it satisfies no leg form to be admitted into, and is still refused as any leg. ## Adopting an existing repository -A repository that already exists is converted **in place**, by -`adopt-project.py`, and this is a ruling (**Brett Heap, 2026-09-02**): it KEEPS -ITS NAME, ITS IDENTITY AND ITS FULL HISTORY and becomes the assembly root, while -`-spec` and `-code` are NEW repositories extracted with -history-preserving filters. The source is never deleted, never renamed and never -force-pushed; the only change to its history is ONE split commit, arriving on a -branch by pull request — the organisations this serves are pull-request only, -and a tool needing a bypass cannot be used where it is needed. +A repository that already exists is converted **in place**, by `adopt-project.py`, +and this is a ruling (**Brett Heap, 2026-09-02**): it KEEPS ITS NAME, which +`--project` must be, ITS IDENTITY AND ITS FULL HISTORY and becomes the assembly +root, while `-spec` and `-code` are NEW repositories extracted +with history-preserving filters. The source is never deleted, never renamed and +never force-pushed; the only change to its history is ONE split commit, arriving +on a branch by pull request — the organisations this serves are pull-request +only, and a tool needing a bypass cannot be used where it is needed. ```sh ./adopt-project.py plan --source MedxSoft/MedxEHR --project MedxEHR diff --git a/adopt-project.py b/adopt-project.py index 179bcc8b..cdd45383 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -31,7 +31,13 @@ --pin` reads it, its digest included, and recorded in the plan, and `execute` writes its `contracts/-pin.yaml`; `check` finds, and `execute` refuses, a pin the plan does not record - whole (#273). + whole (#273). Where the source names a repository, `--org` and + `--project` name that one, compared without case, or `plan` + refuses them (`adopt-not-the-source`), and `check` finds and + `execute` refuses a plan edited to name another + (`plan-not-the-source`): in place, the source IS the assembly + root, and its own name is renamed first, on GitHub, never + retyped (#499). check validates a plan against the source: each entry path bound for a leg one `execute` will extract, no symlink in the source with an empty target (Linux cannot make one, so a clone there checks @@ -2300,6 +2306,10 @@ def _checked_plan_inputs(args, source: Source, "URL ends in no `org/repo`", "Remediation: pass --org .") args.org = checked_value("--org", args.org) + # IN PLACE, THE SOURCE'S OWN NAME (#499), as soon as both halves are + # known and before any pin's digest is read: `execute` acts on the + # repository `--org` and `--project` name, so they must be the source. + _refuse_another_repository(args, source, naming, set(products)) args.elected_on = args.elected_on or _dt.date.today().isoformat() # Read before the plan is written, whether or not it is what chooses the # reference: a date the tools cannot parse would reach `execute` as a @@ -2328,7 +2338,7 @@ def _checked_plan_inputs(args, source: Source, _refuse_an_unrecordable_pin_source(args.pin_source, products) names = _names(args.project) pins = declared_pin_values(args) - _check_names(naming, names, set(pins)) + _check_names(naming, names, set(pins), source) return names, pins @@ -5117,6 +5127,12 @@ def cmd_check(args) -> int: names = plan.names() pins = set(plan.pins) + # The plan names the source, or `execute` acts on another repository + # (#499): said before the names' own findings, as `execute` refuses it. + another = _not_the_source(plan, source, names) + if another: + findings.append(f"FINDING {PLAN_NOT_THE_SOURCE}: {another}") + remediation.append(_own_name_remediation(source, naming, pins, "")) try: _check_names(naming, names, pins) for role, name in names.items(): @@ -6040,6 +6056,18 @@ def _checked_plan_values(plan: Plan) -> tuple[str, str, str, str]: return spec_path, code_path, branch, tracking +def _plan_org(plan: Plan) -> str: + """The plan's `org:` as `execute` puts it into each repository's name + and URL: `str()` of whatever the file holds, which is the text + `checked_value` asks about and hands back. THE ONE READ for + `_repository_urls` and for `_not_the_source` (#499), so that the + repository `check` and `execute` compare with the source is the one + `execute` acts on: `org: 2026`, `org: true`, `org: ~` and no `org:` line + at all are `2026`, `True` and `None` there, and were let by while the + comparison read only text (#513's review, SHOULD-1).""" + return str(plan.get("org")) + + def _repository_urls(args, plan: Plan, names: dict, source: Source) -> tuple[bool, dict, dict]: """Where the three repositories are: `(local, repositories, urls)`. @@ -6048,7 +6076,7 @@ def _repository_urls(args, plan: Plan, names: dict, itself under `--local-remote-dir`, because an in-place adoption pushes its split branch back to the repository it read. """ - org = checked_value("org", plan.get("org")) + org = checked_value("org", _plan_org(plan)) repositories = {role: f"{org}/{name}" for role, name in names.items()} local = args.local_remote_dir is not None # Each name is a directory's too: the work directory's for all three, and @@ -6708,7 +6736,8 @@ def cmd_execute(args) -> int: names = plan.names() pins = set(plan.pins) - _check_names(naming, names, pins) + _refuse_a_plan_for_another_repository(plan, source, names, naming, pins) + _check_names(naming, names, pins, source) _refuse_an_unrunnable_plan(plan, source) # THE PLAN IS UNTRUSTED INPUT. It is a file a human or an AI edited, and @@ -7689,13 +7718,28 @@ def _names(project: str) -> dict[str, str]: def _check_names(policy: NamingPolicy, names: dict[str, str], - pins: set[str]) -> None: + pins: set[str], source: Source | None = None) -> None: + """Each of the three names classifies in the naming policy, in its role. + + `source` is the source an in-place adoption keeps the name of, where it + names a repository and `names` are made from its name (#499). The + assembly root's name is then not the person's to retype: a refusal of it + says that the source cannot be adopted in place under that name, and + that renaming it comes first (`_rename_first`; at that path, where its + `origin` is one, `_origin_path`), never that `--project` takes one + CamelCase token. A leg's name is the plan's own to edit, and keeps the + remediation it had. + """ for role, name in names.items(): found = policy.classify(name, role, pins) + renamed = (_rename_first(source.repository, path=_origin_path(source)) + if role == "assembly" and source is not None + and source.repository is not None else None) if found is None: raise Refusal( "naming-unclassified", f"{name!r} matches no family in the naming policy", + renamed or "Remediation: --project takes one CamelCase token with no " "hyphen, underscore, dot or space.") if not accepts_role(found, role): @@ -7705,6 +7749,7 @@ def _check_names(policy: NamingPolicy, names: dict[str, str], + (f"/{found.role}" if found.role else "") + f", not as the {role!r} form of a project leg " f"({found.reason})", + renamed or "Remediation: --project takes one CamelCase token. Two forms " "beyond the bare one MAY be the assembly root: a declared " "descendant (declare the pin with `--pin " @@ -7715,6 +7760,211 @@ def _check_names(policy: NamingPolicy, names: dict[str, str], "an `-Install` may be no leg at all.") +# --------------------------------------------------------------------------- +# An in-place adoption keeps the source's own name (#499) +# --------------------------------------------------------------------------- +# +# `--project` names all three repositories (`_names`), and without +# `--local-remote-dir` `execute` acts on `https://github.com//.git` +# for each (`_remote_urls`): the split branch it asks for and pushes, the pull +# request it opens and the topic it sets are the assembly root's, which in +# place IS the source. Nothing compared the two. `--help` said `--project` is +# the source's own name, and a source named `billing-service` was refused +# `naming-unclassified` with "--project takes one CamelCase token"; the +# `--project BillingService` that steered to planned and checked `plan ok` +# against `acme/billing-service`, and `execute` then ran `git ls-remote` on +# `acme/BillingService`: a raw failure where no such repository exists, and, +# where another one does, both legs made from it, the split pushed to it and +# its pull request opened there. A typo (`Atlsa` for `Atlas`) and an `--org` +# that is not the source's did the same. Under `--local-remote-dir` the split +# goes to the source's own path, so no test met it. +# +# RULED BY BRETT HEAP, 2026-10-13: "Casefold; rename first". Where the source +# names a repository -- its `origin` ends in an `org/repo` +# (`_remote_repository`), or `--source` is one -- `/` must be +# that repository, compared without case, because GitHub resolves `acme/atlas` +# and `acme/Atlas` to one. `plan` refuses another as `adopt-not-the-source`, +# naming both, before it writes anything. `check` finds, and `execute` +# refuses before it asks the naming policy or any remote, a plan whose `org:` +# names another with its `legs.assembly:`, which is what `execute` acts on, or +# with its `project:`, as `plan-not-the-source`: a hand edit is caught too, +# whatever `org:` then holds, because it is read as `execute` reads it +# (`_plan_org`). A source that names no repository (no `origin`, or one that +# ends in no `org/repo`) is asked nothing, as before. +# +# AN `origin` THAT IS A PATH NAMES ITS LAST TWO DIRECTORIES, as `plan` has +# read it for `--org` all along, and is compared like any other: a clone of +# `/srv/git/acme/Atlas.git` is `acme/Atlas`. A clone of `/srv/clones/Atlas.git` +# is then `clones/Atlas`, no repository on GitHub, so the remediation +# says where the name was read from (`_origin_path`) before it offers one, +# and never offers a directory's name as an organisation without saying so. +# +# AND A NAME THE POLICY REFUSES IS RENAMED FIRST, NOT RETYPED. Where the +# source's own name is one the naming policy refuses (`billing-service`), it +# cannot be adopted in place under that name, and no `--project` changes +# that: whether to rename the repository is a human's decision, made first on +# GitHub, or at the path an `origin` that is one names, and `plan` runs again +# after it. So these refusals say that, and `_check_names` says it in place of +# its hint to retype `--project`. + +#: `plan`'s refusal of an `--org` and a `--project` that name another +#: repository than the source's (#499). +NOT_THE_SOURCE = "adopt-not-the-source" +#: `check`'s finding, and `execute`'s refusal, of a plan that does. +PLAN_NOT_THE_SOURCE = "plan-not-the-source" + + +def _is_the_source(repository: str, org: str, name: str) -> bool: + """Whether `/` is `repository` as GitHub resolves a name: + without case (#499).""" + return f"{org}/{name}".casefold() == repository.casefold() + + +def _rename_first(repository: str, said: str = "", + path: str | None = None) -> str: + """The remediation where the source's own name is one the naming policy + refuses: it cannot be adopted in place under that name, and renaming it + is a human's decision that comes first (#499). `said` is what the + refusal says before that, a sentence or none. THE ONE TEXT of it. + + `path` is the source's `origin` where that is a filesystem path + (`_origin_path`). The repository is then the one at that path, which + names none on GitHub by itself, so it is renamed there, or the adoption + is planned from a clone of the repository on GitHub instead: `gh repo + rename` renames a repository on GitHub, and sending the person to it + contradicted the sentence that a path names none there (#513's review, + NIT-3).""" + where = ( + "on GitHub (`gh repo rename`); then run `plan` again against the " + "renamed repository (`--source /`, or a clone whose " + "`origin` names it)" if path is None else + f"where it is kept, and its `origin` is the path {_spelled(path)}, " + "not a repository on GitHub: rename the repository at that path, or " + "plan instead from a clone of the repository on GitHub, whose " + "`origin` names it there; then run `plan` again against the renamed " + "repository") + return (f"Remediation: {said}{_shown(repository)} cannot be adopted in " + "place under that name: an in-place adoption keeps the source's " + "own name, and `--project` cannot rename it. Whether to rename " + f"it is a human's decision, made first {where}, with its new " + "name as `--project`.") + + +def _origin_path(source: Source) -> str | None: + """The source's `origin` where `source.repository` was read from it and + it is a filesystem PATH (`remote_is_a_path`), not a url (#499): the + repository is then its last two directories, which name no repository + on GitHub by themselves, and an `--org` read from them may be none. None + for an `org/repo` source, which names its repository itself.""" + if source.cloned or source.repository is None: + return None + url = _config_value(source.path, "remote.origin.url") + return url if url is not None and remote_is_a_path(url) else None + + +def _own_name_remediation(source: Source, naming: NamingPolicy, + pins: set[str], done: str) -> str: + """What `adopt-not-the-source` and `plan-not-the-source` say to do: + `plan` again under the source's own name, or, where the naming policy + refuses that name, `_rename_first`. `done` is what the refusing command + left as it was, said first; then, where the name was read from a path + (`_origin_path`), that it was, so that a directory's name is never + offered as an organisation without saying where it came from.""" + repository = str(source.repository) + org, _, name = repository.partition("/") + path = _origin_path(source) + if path is not None: + done += (f"The source's repository, {_shown(repository)}, is read " + "from the last two directories of its `origin`, the path " + f"{_spelled(path)}, and names no repository on GitHub by " + "itself. ") + try: + _check_names(naming, _names(name), pins) + except Refusal as exc: + return _rename_first(repository, f"{done}The source's own name is no " + f"remedy either: {exc.detail}. ", path) + return (f"Remediation: {done}Run `plan` again with `--project " + f"{_shown(name)} --org {_shown(org)}`, the source's own name, " + "which `plan` writes into the plan's `org:`, `project:` and " + "`legs:`. Any other name is another repository; if " + f"{_shown(repository)} is not the one to adopt, plan from a clone " + "of the one that is, or with `--source /`.") + + +def _refuse_another_repository(args, source: Source, naming: NamingPolicy, + pins: set[str]) -> None: + """`plan` refuses an `--org` and a `--project` that name another + repository than the one the source names, as `adopt-not-the-source`, + before anything is written (#499). `args.org` is settled: given, or read + from the source.""" + repository = source.repository + if repository is None or _is_the_source(repository, args.org, + args.project): + return + raise Refusal( + NOT_THE_SOURCE, + f"--org {args.org} and --project {args.project} name " + f"{args.org}/{args.project}, and the source is {_shown(repository)}: " + "an in-place adoption keeps the source's own name, and `execute` " + "pushes the split to the repository they name, opens its pull " + "request there and sets its topic on it", + _own_name_remediation(source, naming, pins, "no plan was written. ")) + + +def _not_the_source(plan: Plan, source: Source, + names: dict[str, str]) -> str | None: + """`plan-not-the-source`'s detail, or None: the plan's `org:` with its + `legs.assembly:` (`names`, read by `Plan.names`) or with its `project:` + names another repository than the one the source names (#499). + + `org:` is read as `execute` reads it (`_plan_org`), whatever the file + holds. While it was compared only when it was text, a number, a + boolean, a null or a deleted `org:` line checked `plan ok`, and + `execute` acted on `2026/Atlas`, `True/Atlas` or `None/Atlas` (#513's + review, SHOULD-1). Where it is not text, the detail says what the file + holds beside how `execute` spells it. `project:` is read only as text: + a misread one is not this question's to answer, and `execute` acts on + `legs.assembly:`.""" + repository = source.repository + if repository is None: + return None + org, held = _plan_org(plan), plan.get("org") + wrong = [(key, value) for key, value in + (("legs.assembly", names["assembly"]), + ("project", plan.get("project"))) + if isinstance(value, str) + and not _is_the_source(repository, org, value)] + if not wrong: + return None + said = (f"`org: {_shown(org)}`" if isinstance(held, str) else + f"`org:` ({_read_as(held) if 'org' in plan.data else 'absent'}, " + f"which `execute` spells {_shown(org)})") + keys = " and ".join(f"`{key}: {_shown(value)}`" for key, value in wrong) + named = _and_listed(sorted({f"{_shown(org)}/{_shown(value)}" + for _, value in wrong})) + return (f"the plan's {said} with its {keys} names {named}, " + f"and the source is {_shown(repository)}: an in-place adoption " + "keeps the source's own name, and `execute` acts on the " + "repository `org:` and `legs.assembly:` name") + + +def _refuse_a_plan_for_another_repository( + plan: Plan, source: Source, names: dict[str, str], + naming: NamingPolicy, pins: set[str]) -> None: + """`execute` refuses `check`'s `plan-not-the-source` before it asks the + naming policy about the plan's names or any remote anything (#499): + what it would ask of GitHub it would ask of the repository the plan + names, and a raw `git ls-remote` failure on that repository was the + first thing it said.""" + detail = _not_the_source(plan, source, names) + if detail: + raise Refusal( + PLAN_NOT_THE_SOURCE, f"{plan.path}: {detail}", + _own_name_remediation( + source, naming, pins, + "no leg repository was created and nothing was pushed. ")) + + def _elector() -> str: """`git config user.name`, which `plan` records when `--elected-by` is not given; "" when it names nobody. diff --git a/docs/handbook.html b/docs/handbook.html index 707bbffc..695a9cd1 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1168,7 +1168,7 @@

The three legs, and naming

Reference

Adopting an existing repository

A repository that already exists is converted in place. It KEEPS ITS NAME, - ITS IDENTITY AND ITS FULL HISTORY and becomes the assembly root, while + which --project must be, ITS IDENTITY AND ITS FULL HISTORY and becomes the assembly root, while <Project>-spec and <Project>-code are NEW repositories extracted with history-preserving filters. The source is never deleted, never renamed and never force-pushed; the only change to its history is ONE split commit, arriving on a branch @@ -1878,9 +1878,10 @@

For AI assistants

own seed, and the one ruleset exit that re-run is, added (#462); family.py add, bump and remove committing on a branch made first, never the tracking branch, and printing the push of it, added (#487); the installed doctor - keeping its clone of the standard, which its NEXT lines run from, added (#500) — a page - cannot name the commit that contains it, and the blob below is the fact a test checks. - README.md blob 8527950a3e47a4b6d87dababe6886155432d5c87. Every command, flag, exit code + keeping its clone of the standard, which its NEXT lines run from, added (#500); an + adoption’s --project as the source’s own name, added (#499) — a + page cannot name the commit that contains it, and the blob below is the fact a test checks. + README.md blob 6bf94d583bb15fc6196c1e0c5232281edc723885. Every command, flag, exit code and file name on this page is quoted from that README and from AGENTS.md.

diff --git a/tests/test_adopt_git_reads.py b/tests/test_adopt_git_reads.py index 69f93c9f..ea5bcfe1 100644 --- a/tests/test_adopt_git_reads.py +++ b/tests/test_adopt_git_reads.py @@ -73,13 +73,14 @@ def by_git_config(base: Path, source: Path, extra: dict, str(base / WORK), *flags, extra=extra) -def without_org(base: Path, source: Path) -> subprocess.CompletedProcess: +def without_org(base: Path, source: Path, + *flags: str) -> subprocess.CompletedProcess: """`plan` as `planned` runs it, but with no --org, so the org is what - the source's `origin` names.""" + the source's `origin` names, and with `flags` after it.""" return adopted("plan", "--source", str(source), "--project", PROJECT, "--elected-by", "Test Human", "--elected-on", "2026-09-02", "--out", str(base / PLAN_FILE), "--work-dir", - str(base / WORK)) + str(base / WORK), *flags) def command_scope(key: str, value: bytes) -> dict: @@ -268,9 +269,11 @@ def test_plan_reads_an_origin_that_is_not_utf8_as_none(tmp_path, url, """The issue's last row, with --org: on the base `plan` ended in a `UnicodeDecodeError` traceback from `_remote_repository`, exit 1, and wrote no plan. Now it plans, and records no repository. The ASCII URL - is the control: its `org/repo` is read, so the None is the byte's.""" + is the control: its `org/repo` is read, so the None is the byte's, and + the plan is written in that name, which `plan` refuses any other for + (#499).""" source = with_origin(make_source_repo(tmp_path / "Thing"), url) - done = planned(tmp_path, source) + done = planned(tmp_path, source, "--org", "cafe", "--project", "repo") assert_no_traceback({"plan": done}) assert done.returncode == 0, said(done) assert load_yaml(tmp_path / PLAN_FILE)["source"]["repository"] \ @@ -319,9 +322,9 @@ def test_plan_refuses_an_unsafe_org_that_is_passed(tmp_path, org): def test_plan_without_org_takes_the_org_an_ascii_origin_names(tmp_path): """Control: an origin that names `cafe/repo` is the org, as on the - base.""" + base, for a plan in the source's own name (#499).""" source = with_origin(make_source_repo(tmp_path / "Thing"), ASCII_URL) - done = without_org(tmp_path, source) + done = without_org(tmp_path, source, "--project", "repo") assert_no_traceback({"plan": done}) assert done.returncode == 0, said(done) assert load_yaml(tmp_path / PLAN_FILE)["org"] == "cafe" diff --git a/tests/test_adopt_late_failure.py b/tests/test_adopt_late_failure.py index ec49e161..b6f64a27 100644 --- a/tests/test_adopt_late_failure.py +++ b/tests/test_adopt_late_failure.py @@ -755,10 +755,12 @@ def test_an_org_repo_run_names_its_clone_when_a_leg_repository_fails( refused at that clone (`source-unclonable`), but the notice read only the legs and the split off the disk, so it named no work directory. It names the clone. The run is not refused for its own clone: that is no - leg and not the split. `gh repo clone` is a local clone here.""" + leg and not the split. `gh repo clone` is a local clone here. The plan + is in the source's own name, as `execute` refuses any other for an + `org/repo` source (#499).""" no_filter_repo_probe(adopter, monkeypatch) origin = make_source_repo(tmp_path / SOURCE, tree=SMALL_TREE, edits=()) - plan = planned(tmp_path, origin) + plan = planned(tmp_path, origin, SOURCE) real_run = adopter.run def gh_clones_the_origin(args, *rest, **keywords): @@ -1173,9 +1175,9 @@ def execute_args(tmp_path: Path, plan: Path) -> Namespace: yes=True, work_dir=tmp_path / WORK) -def planned(tmp_path: Path, source: Path) -> Path: +def planned(tmp_path: Path, source: Path, project: str = ASSEMBLY) -> Path: plan = tmp_path / "adoption-plan.yaml" - written = write_plan(source, plan, project=ASSEMBLY) + written = write_plan(source, plan, project=project) assert written.returncode == 0, written.stderr + written.stdout return plan diff --git a/tests/test_adopt_project_is_the_source.py b/tests/test_adopt_project_is_the_source.py new file mode 100644 index 00000000..e0409934 --- /dev/null +++ b/tests/test_adopt_project_is_the_source.py @@ -0,0 +1,533 @@ +# SPDX-License-Identifier: Apache-2.0 +"""An in-place adoption keeps the source's own name (#499). + +`--project` names all three repositories, and without `--local-remote-dir` +`execute` acts on `https://github.com//.git`: the split branch it +asks for and pushes, the pull request it opens and the topic it sets. In place +that repository IS the source, and nothing compared the two. A source named +`billing-service` was refused `naming-unclassified` with "--project takes one +CamelCase token"; the `--project BillingService` that steered to planned and +checked `plan ok` against `acme/billing-service`, and `execute` then asked +GitHub about `acme/BillingService`. A typo (`Atlsa`) and an `--org` that is not +the source's did the same. + +Brett Heap's ruling, 2026-10-13: "Casefold; rename first". Where the source +names a repository, `plan` refuses an `--org` and a `--project` that name +another, compared without case (`adopt-not-the-source`), before it writes +anything; `check` finds, and `execute` refuses before it asks anything else, a +plan edited to name another (`plan-not-the-source`). Where the source's own +name is one the naming policy refuses, every one of those refusals says that +it cannot be adopted in place under that name and that renaming it on GitHub +comes first, and none says to retype `--project`. A source that names no +repository is asked nothing, as before. + +Nothing here reaches GitHub or runs `gh`: each source's `origin` is an +`https://github.com/` URL that nothing fetches, `plan` and `check` read it +from the source's own config, and `execute` is run in-process with every +`git` and `gh` command it would run replaced by one that fails the test. +""" + +from __future__ import annotations + +import importlib.util +import sys +from argparse import Namespace +from pathlib import Path + +import pytest + +from conftest import ADOPT, REPO, git, make_source_repo, run_script + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import load_yaml # noqa: E402 + +NOT_THE_SOURCE = "adopt-not-the-source" +PLAN_NOT_THE_SOURCE = "plan-not-the-source" +#: The hint `naming-unclassified` gives where `--project` is the person's to +#: choose. In place it is not, and none of these refusals may give it. +RETYPE = "--project takes one CamelCase token" +#: What every refusal of a source's own name says instead. +RENAME_FIRST = ( + "cannot be adopted in place under that name: an in-place adoption keeps " + "the source's own name, and `--project` cannot rename it. Whether to " + "rename it is a human's decision, made first on GitHub (`gh repo " + "rename`); then run `plan` again against the renamed repository " + "(`--source /`, or a clone whose `origin` names it), with " + "its new name as `--project`.") +#: What they say instead where the source's `origin` is a PATH (#513's +#: review, NIT-3): the repository it names is at that path, not on GitHub, +#: so `gh repo rename` is not the way to rename it. `{}` is the path. +RENAME_AT_THE_PATH = ( + "cannot be adopted in place under that name: an in-place adoption keeps " + "the source's own name, and `--project` cannot rename it. Whether to " + "rename it is a human's decision, made first where it is kept, and its " + "`origin` is the path `{}`, not a repository on GitHub: rename the " + "repository at that path, or plan instead from a clone of the repository " + "on GitHub, whose `origin` names it there; then run `plan` again against " + "the renamed repository, with its new name as `--project`.") +#: A source small enough that `plan` asks no question. +TREE = {"README.md": "# here\n", "src/app.py": "print(1)\n", + "specs/001/spec.md": "# Spec\n"} +PLAN = "adoption-plan.yaml" +WHY = ("an in-place adoption keeps the source's own name, and `execute` " + "pushes the split to the repository they name, opens its pull request " + "there and sets its topic on it") + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_is_the_source", + ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def source_of(base: Path, repository: str | None, + directory: str = "source") -> Path: + """A one-commit source whose `origin` is `repository` on GitHub, or + which has none. Nothing fetches it: `plan` reads the URL alone.""" + source = make_source_repo(base / directory, tree=TREE, edits=()) + if repository is not None: + git("remote", "add", "origin", f"https://github.com/{repository}.git", + cwd=source) + return source + + +def plan(base: Path, source: Path, *flags: str): + """`plan` of `source` into `base`, with the flags given and no others + but the ones every run here shares.""" + return run_script(ADOPT, "plan", "--source", str(source), + "--elected-by", "Test Human", "--elected-on", + "2026-10-13", "--out", str(base / PLAN), *flags) + + +def check(plan_path: Path): + return run_script(ADOPT, "check", "--plan", str(plan_path)) + + +def edited(plan_path: Path, *changes: tuple[str, str | None]) -> None: + """The plan with each whole line `old` replaced by `new`, or deleted + where `new` is None, as a hand edit makes it, LF as `plan` wrote it.""" + text = plan_path.read_text(encoding="utf-8") + for old, new in changes: + assert f"\n{old}\n" in text, text + text = text.replace(f"\n{old}\n", + "\n" if new is None else f"\n{new}\n", 1) + with plan_path.open("w", encoding="utf-8", newline="\n") as handle: + handle.write(text) + + +def findings(stderr: str) -> list[str]: + return [line for line in stderr.splitlines() + if line.startswith("FINDING ")] + + +# --- `plan` --------------------------------------------------------------- + +@pytest.mark.parametrize("repository, flags, named, own", [ + # The issue's own: the hint `plan` gave for the source's name, followed. + ("acme/billing-service", ("--project", "BillingService"), + "--org acme and --project BillingService name acme/BillingService", + None), + # A typo of a name the policy accepts. + ("acme/Atlas", ("--project", "Atlsa"), + "--org acme and --project Atlsa name acme/Atlsa", "Atlas"), + # The right name in another organisation. + ("acme/Atlas", ("--project", "Atlas", "--org", "acme-labs"), + "--org acme-labs and --project Atlas name acme-labs/Atlas", "Atlas"), +], ids=["hyphenated-source-retyped", "typo", "another-org"]) +def test_plan_refuses_a_name_that_is_not_the_sources(tmp_path, repository, + flags, named, own): + """Exit 2 before any plan is written, naming both repositories. Where + the source's own name is one `plan` would take, it says to pass that; + where it is not, that renaming comes first; never to retype + `--project`.""" + source = source_of(tmp_path, repository) + + refused = plan(tmp_path, source, *flags) + + assert refused.returncode == 2, refused.stdout + refused.stderr + detail, remediation = refused.stderr.splitlines()[:2] + assert detail == (f"REFUSED {NOT_THE_SOURCE}: {named}, and the source is " + f"{repository}: {WHY}"), refused.stderr + assert remediation.startswith("Remediation: no plan was written. "), \ + remediation + if own: + # An `origin` that is a url: nothing said of where the name came + # from, which `test_a_name_read_from_a_path_says_so` is about. + assert remediation.startswith( + "Remediation: no plan was written. Run `plan` again with " + f"`--project {own} --org acme`, the source's own name"), \ + remediation + assert RENAME_FIRST not in remediation + else: + assert remediation.endswith( + "The source's own name is no remedy either: 'billing-service' " + "matches no family in the naming policy. acme/billing-service " + f"{RENAME_FIRST}"), remediation + assert RETYPE not in refused.stderr + assert not (tmp_path / PLAN).exists() + + +@pytest.mark.parametrize("repository, code", [ + ("acme/billing-service", "naming-unclassified"), + ("acme/Tools-Install", "naming-role-mismatch"), +], ids=["no-family", "not-a-project-leg"]) +def test_the_sources_own_name_the_policy_refuses_is_renamed_first( + tmp_path, repository, code): + """`--project` as the source's own name, which the naming policy refuses: + the policy's own finding, and in place of its hint to retype + `--project`, that the source cannot be adopted in place under that name + and that renaming it on GitHub comes first.""" + source = source_of(tmp_path, repository) + + refused = plan(tmp_path, source, "--project", repository.split("/")[1]) + + assert refused.returncode == 2, refused.stdout + refused.stderr + detail, remediation = refused.stderr.splitlines()[:2] + assert detail.startswith(f"REFUSED {code}: "), refused.stderr + assert remediation == f"Remediation: {repository} {RENAME_FIRST}" + assert RETYPE not in refused.stderr + assert not (tmp_path / PLAN).exists() + + +def test_a_name_in_another_case_is_the_source(tmp_path): + """GitHub resolves `acme/atlas` and `ACME/Atlas` to one repository, so + they are the source, and `plan` and `check` pass.""" + source = source_of(tmp_path, "acme/atlas") + + written = plan(tmp_path, source, "--project", "Atlas", "--org", "ACME") + + assert written.returncode == 0, written.stdout + written.stderr + recorded = load_yaml(tmp_path / PLAN) + assert (recorded["org"], recorded["project"]) == ("ACME", "Atlas") + assert recorded["source"]["repository"] == "acme/atlas" + checked = check(tmp_path / PLAN) + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "\nplan ok\n" in checked.stdout + + +@pytest.mark.parametrize("origin", [None, "repo.git"], + ids=["no-origin", "no-org-repo-in-it"]) +def test_a_source_that_names_no_repository_is_asked_nothing(tmp_path, + origin): + """No `origin`, or one that ends in no `org/repo`: there is no name to + compare, and `plan` takes `--org` and `--project` as it always did.""" + source = source_of(tmp_path, None) + if origin is not None: + git("remote", "add", "origin", origin, cwd=source) + + written = plan(tmp_path, source, "--project", "Northwind", "--org", + "acme") + + assert written.returncode == 0, written.stdout + written.stderr + assert load_yaml(tmp_path / PLAN)["source"]["repository"] is None + + +def test_a_name_read_from_a_path_says_so(tmp_path): + """An `origin` that is a path names its last two directories, as `plan` + reads `--org` from it, and is compared like any other: `mirrors/Northwind` + is not `acme/Northwind`. The refusal says the name was read from that + path before it offers `--org mirrors`, a directory's name, and a plan in + that name is the source's. The path is written with `/`, which git keeps + as it is given on every platform.""" + source = source_of(tmp_path, None) + origin = (tmp_path / "mirrors" / "Northwind.git").as_posix() + git("remote", "add", "origin", origin, cwd=source) + + refused = plan(tmp_path, source, "--project", "Northwind", "--org", + "acme") + + assert refused.returncode == 2, refused.stdout + refused.stderr + detail, remediation = refused.stderr.splitlines()[:2] + assert detail.startswith( + f"REFUSED {NOT_THE_SOURCE}: --org acme and --project Northwind name " + "acme/Northwind, and the source is mirrors/Northwind: "), detail + assert remediation.startswith( + "Remediation: no plan was written. The source's repository, " + "mirrors/Northwind, is read from the last two directories of its " + f"`origin`, the path `{origin}`, and names no repository on GitHub " + "by itself. Run `plan` again with `--project Northwind --org " + "mirrors`"), remediation + assert not (tmp_path / PLAN).exists() + written = plan(tmp_path, source, "--project", "Northwind", "--org", + "mirrors") + assert written.returncode == 0, written.stdout + written.stderr + + +@pytest.mark.parametrize("flags, code", [ + # Another name: `adopt-not-the-source`, whose remediation has said the + # path names no repository on GitHub before it says to rename. + (("--project", "BillingService", "--org", "acme"), NOT_THE_SOURCE), + # The source's own name, which the policy refuses: `_check_names`. + (("--project", "billing-service"), "naming-unclassified"), +], ids=["another-name", "its-own-name"]) +def test_a_name_read_from_a_path_is_renamed_at_that_path(tmp_path, flags, + code): + """#513's review, NIT-3: a source whose `origin` is the path + `.../clones/billing-service.git` names `clones/billing-service`, which the + naming policy refuses. Its rename-first remediation said the path names + no repository on GitHub, then sent the person to `gh repo rename` on + GitHub. It says to rename the repository at that path, or to plan from a + clone of the one on GitHub, and never `gh repo rename`.""" + source = source_of(tmp_path, None) + origin = (tmp_path / "clones" / "billing-service.git").as_posix() + git("remote", "add", "origin", origin, cwd=source) + + refused = plan(tmp_path, source, *flags) + + assert refused.returncode == 2, refused.stdout + refused.stderr + detail, remediation = refused.stderr.splitlines()[:2] + assert detail.startswith(f"REFUSED {code}: "), detail + assert remediation.endswith( + f"clones/billing-service {RENAME_AT_THE_PATH.format(origin)}"), \ + remediation + assert "gh repo rename" not in refused.stderr + assert RETYPE not in refused.stderr + assert not (tmp_path / PLAN).exists() + + +def test_an_org_repo_source_is_the_repository_it_names(adopter, tmp_path, + monkeypatch, capsys): + """`--source acme/Atlas` names the repository it clones, and is compared + as an `origin` is. `git clone` of a local repository stands in for `gh + repo clone`, and is the one command the run is let make.""" + local = source_of(tmp_path, None, "Atlas") + real = adopter.run + + def cloned(args, *rest, **keywords): + assert args[:3] == ["gh", "repo", "clone"], args + return real(["git", "clone", "-q", str(local), args[4]], *rest, + **keywords) + + monkeypatch.setattr(adopter, "run", cloned) + code = adopter.main(["plan", "--source", "acme/Atlas", "--project", + "Atlsa", "--elected-by", "Test Human", + "--elected-on", "2026-10-13", "--out", + str(tmp_path / PLAN), "--work-dir", + str(tmp_path / "work")]) + + assert code == 2 + err = capsys.readouterr().err + assert err.startswith( + f"REFUSED {NOT_THE_SOURCE}: --org acme and --project Atlsa name " + "acme/Atlsa, and the source is acme/Atlas: "), err + assert not (tmp_path / PLAN).exists() + + +# --- `check` and `execute` -------------------------------------------------- + +@pytest.mark.parametrize("changes, said", [ + ((("project: Atlas", "project: Atlsa"),), + "the plan's `org: acme` with its `project: Atlsa` names acme/Atlsa"), + (((" assembly: Atlas", " assembly: Atlsa"),), + "the plan's `org: acme` with its `legs.assembly: Atlsa` names " + "acme/Atlsa"), + ((("project: Atlas", "project: Atlsa"), + (" assembly: Atlas", " assembly: Northwind")), + "the plan's `org: acme` with its `legs.assembly: Northwind` and " + "`project: Atlsa` names acme/Atlsa and acme/Northwind"), + ((("org: acme", "org: acme-labs"),), + "the plan's `org: acme-labs` with its `legs.assembly: Atlas` and " + "`project: Atlas` names acme-labs/Atlas"), +], ids=["project", "legs-assembly", "both-apart", "org"]) +def test_check_finds_a_plan_edited_to_name_another_repository( + tmp_path, changes, said): + """A plan `plan` wrote for the source checks ok; edited to name another + repository, by its `project:`, its `legs.assembly:` or its `org:`, it is + one finding naming what the plan names and what the source is, with + the way back printed once after the findings.""" + source = source_of(tmp_path, "acme/Atlas") + written = plan(tmp_path, source, "--project", "Atlas") + assert written.returncode == 0, written.stdout + written.stderr + assert check(tmp_path / PLAN).returncode == 0 + + edited(tmp_path / PLAN, *changes) + result = check(tmp_path / PLAN) + + assert result.returncode == 1, result.stdout + result.stderr + assert [line for line in findings(result.stderr) + if PLAN_NOT_THE_SOURCE in line] == [ + f"FINDING {PLAN_NOT_THE_SOURCE}: {said}, and the source is " + "acme/Atlas: an in-place adoption keeps the source's own name, and " + "`execute` acts on the repository `org:` and `legs.assembly:` name"] + assert result.stderr.count( + "Remediation: Run `plan` again with `--project Atlas --org acme`, " + "the source's own name,") == 1, result.stderr + + +def refused_before_any_remote(adopter, monkeypatch, plan_path: Path, + work: Path): + """`execute --yes` without `--local-remote-dir`, in-process, with every + command it runs through `run` -- `git ls-remote`, `gh repo create`, a + push -- failing the test. Returns the refusal it raised.""" + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + + def nothing_is_run(args, *rest, **keywords): + raise AssertionError(f"execute ran {args}") + + monkeypatch.setattr(adopter, "run", nothing_is_run) + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(Namespace( + plan=str(plan_path), source=None, local_remote_dir=None, + allow_empty_leg=[], yes=True, work_dir=work)) + return caught.value + + +def test_execute_refuses_a_plan_edited_to_another_repository_first( + adopter, tmp_path, monkeypatch): + """On main the first thing such a plan's `execute` did was `git + ls-remote` of the repository the plan names. It is refused as `check` + finds it, before anything is run.""" + source = source_of(tmp_path, "acme/Atlas") + written = plan(tmp_path, source, "--project", "Atlas") + assert written.returncode == 0, written.stdout + written.stderr + edited(tmp_path / PLAN, ("project: Atlas", "project: Atlsa"), + (" assembly: Atlas", " assembly: Atlsa"), + (" spec: Atlas-spec", " spec: Atlsa-spec"), + (" code: Atlas-code", " code: Atlsa-code")) + + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert refusal.code == PLAN_NOT_THE_SOURCE + assert refusal.detail == ( + f"{tmp_path / PLAN}: the plan's `org: acme` with its `legs.assembly: " + "Atlsa` and `project: Atlsa` names acme/Atlsa, and the source is " + "acme/Atlas: an in-place adoption keeps the source's own name, and " + "`execute` acts on the repository `org:` and `legs.assembly:` name") + assert refusal.remediation.startswith( + "Remediation: no leg repository was created and nothing was pushed. " + "Run `plan` again with `--project Atlas --org acme`"), \ + refusal.remediation + + +@pytest.mark.parametrize("new, held, spelled", [ + ("org: 2026", "the number 2026", "2026"), + ("org: true", "the boolean true", "True"), + ("org: ~", "null", "None"), + (None, "absent", "None"), +], ids=["number", "boolean", "null", "deleted"]) +def test_an_org_that_is_not_text_is_read_as_execute_reads_it( + adopter, tmp_path, monkeypatch, new, held, spelled): + """#513's review, SHOULD-1: `org:` was compared only when it was text, + and `execute` reads it with `str()` (`_repository_urls`). A plan edited + to `org: 2026`, `org: true` or `org: ~`, or with its `org:` line deleted, + checked `plan ok`, and `execute` ran `git ls-remote` on + `https://github.com/2026/Atlas.git`, `.../True/...` or `.../None/...`. + Each is the repository `execute` would act on, and not the source: + `check` finds it and `execute` refuses it, before anything is run.""" + source = source_of(tmp_path, "acme/Atlas") + written = plan(tmp_path, source, "--project", "Atlas") + assert written.returncode == 0, written.stdout + written.stderr + edited(tmp_path / PLAN, ("org: acme", new)) + said = (f"the plan's `org:` ({held}, which `execute` spells {spelled}) " + "with its `legs.assembly: Atlas` and `project: Atlas` names " + f"{spelled}/Atlas, and the source is acme/Atlas: an in-place " + "adoption keeps the source's own name, and `execute` acts on the " + "repository `org:` and `legs.assembly:` name") + + checked = check(tmp_path / PLAN) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert [line for line in findings(checked.stderr) + if PLAN_NOT_THE_SOURCE in line] == [ + f"FINDING {PLAN_NOT_THE_SOURCE}: {said}"], checked.stderr + assert refusal.code == PLAN_NOT_THE_SOURCE + assert refusal.detail == f"{tmp_path / PLAN}: {said}" + assert refusal.remediation.startswith( + "Remediation: no leg repository was created and nothing was pushed. " + "Run `plan` again with `--project Atlas --org acme`"), \ + refusal.remediation + # The comparison above is only as good as its read of `org:` being the one + # `execute` acts on (#513's review, NIT-B): the URL `_repository_urls` + # hands to the pushes spells the owner as `check` compared it. A later + # edit that gives it a default of its own (`plan.get("org", ...)`) answers + # `null` and `deleted` differently, and reopens SHOULD-1 for a source + # whose repository the plan can then reach. + loaded = adopter.Plan.load(tmp_path / PLAN) + urls = adopter._repository_urls( + Namespace(local_remote_dir=None), loaded, loaded.names(), + loaded.open_source(None, tmp_path / "work"))[2] + assert urls["assembly"] == f"https://github.com/{spelled}/Atlas.git" + + +def test_a_plan_older_than_its_sources_origin_is_caught_by_check_and_execute( + adopter, tmp_path, monkeypatch): + """The issue's plan, as `plan` wrote it on main: `BillingService` for a + source named `billing-service`, here planned while the source had no + `origin` and given its `origin` afterwards. `check` finds it and + `execute` refuses it, each saying that renaming comes first, and + neither says to retype `--project`.""" + source = source_of(tmp_path, None) + written = plan(tmp_path, source, "--project", "BillingService", "--org", + "acme") + assert written.returncode == 0, written.stdout + written.stderr + git("remote", "add", "origin", + "https://github.com/acme/billing-service.git", cwd=source) + + checked = check(tmp_path / PLAN) + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert findings(checked.stderr) == [ + f"FINDING {PLAN_NOT_THE_SOURCE}: the plan's `org: acme` with its " + "`legs.assembly: BillingService` and `project: BillingService` names " + "acme/BillingService, and the source is acme/billing-service: an " + "in-place adoption keeps the source's own name, and `execute` acts " + "on the repository `org:` and `legs.assembly:` name"] + rename = (" The source's own name is no remedy either: 'billing-service' " + f"matches no family in the naming policy. acme/billing-service " + f"{RENAME_FIRST}") + assert f"Remediation:{rename}\n" in checked.stderr, checked.stderr + assert refusal.code == PLAN_NOT_THE_SOURCE + assert refusal.remediation == ( + "Remediation: no leg repository was created and nothing was " + f"pushed.{rename}") + assert RETYPE not in checked.stderr + refusal.remediation + + +def test_execute_says_rename_first_of_the_sources_own_name(adopter, tmp_path, + monkeypatch): + """A plan for `billing-service` itself, which `plan` refuses, is what an + edit makes of one planned before the source had an `origin`: `execute` + refuses the name the policy refuses, saying renaming comes first, before + anything is run.""" + source = source_of(tmp_path, None) + written = plan(tmp_path, source, "--project", "BillingService", "--org", + "acme") + assert written.returncode == 0, written.stdout + written.stderr + edited(tmp_path / PLAN, ("project: BillingService", "project: " + "billing-service"), + (" assembly: BillingService", " assembly: billing-service"), + (" spec: BillingService-spec", " spec: billing-service-spec"), + (" code: BillingService-code", " code: billing-service-code")) + git("remote", "add", "origin", + "https://github.com/acme/billing-service.git", cwd=source) + + refusal = refused_before_any_remote(adopter, monkeypatch, + tmp_path / PLAN, tmp_path / "work") + + assert refusal.code == "naming-unclassified" + assert refusal.remediation == \ + f"Remediation: acme/billing-service {RENAME_FIRST}" + + +def test_a_source_with_no_origin_executes_past_the_comparison( + adopter, tmp_path, monkeypatch): + """Control: with no `origin` the plan is not compared, and `execute` + goes on to the first command it runs against GitHub, which the stand-in + turns into the test's own failure.""" + source = source_of(tmp_path, None) + written = plan(tmp_path, source, "--project", "Northwind", "--org", + "acme") + assert written.returncode == 0, written.stdout + written.stderr + with pytest.raises(AssertionError, match=r"execute ran \['git', " + r"'ls-remote', 'https://github" + r"\.com/acme/Northwind\.git'"): + refused_before_any_remote(adopter, monkeypatch, tmp_path / PLAN, + tmp_path / "work") diff --git a/tests/test_adopt_relative_submodule_url.py b/tests/test_adopt_relative_submodule_url.py index e03c5a35..e33e5681 100644 --- a/tests/test_adopt_relative_submodule_url.py +++ b/tests/test_adopt_relative_submodule_url.py @@ -64,8 +64,12 @@ #: every platform and strips it only where `\` separates directories, so on #: Windows this is #190's own case. BACKSLASH = "..\\dependency" +#: The source's own name, which every plan here is written in: an in-place +#: adoption keeps it, and `plan` refuses another for a source whose `origin` +#: names its repository (#499). +THING = "Thing" #: The source's own name for its remote, beside the repositories it makes. -THING_REMOTE = "Thing.git" +THING_REMOTE = f"{THING}.git" #: Where each test's `execute` works, and what a verified one says. WORK = "work" VERIFIED = "adoption verified" @@ -80,9 +84,11 @@ #: ../remotes/Thing.git` writes it from `/Thing`: RELATIVE. RELATIVE_ORIGIN = f"../{REMOTES}/{THING_REMOTE}" #: The code leg's GitHub remote, as `check` resolves from it. -CODE_LEG = "https://github.com/{org}/Northwind-code.git" +CODE_LEG = "https://github.com/{org}/" + THING + "-code.git" #: The code leg's bare remote, as `execute --local-remote-dir` names it. -CODE_BARE = "Northwind-code.git" +CODE_BARE = f"{THING}-code.git" +#: `check`'s finding for a plan whose organisation is not the source's (#499). +NOT_THE_SOURCE = "plan-not-the-source" #: The source's `origin` on GitHub, in the spellings a clone may carry. HTTPS_ORIGIN = "https://github.com/testorg/Thing.git" SCP_ORIGIN = "git@github.com:testorg/Thing.git" @@ -144,18 +150,39 @@ def commit(source: Path, message: str) -> None: def planned(source: Path, base: Path, org: str = ORG, answers: dict | None = None, name: str | None = None) -> Path: - """A plan of `source` for `org`, in a directory of its own under `base` - (`name`, else `plan-`), with `.gitmodules` and DEPENDENCY both kept - in code unless `answers` says otherwise.""" + """A plan of `source` for `org`, in the source's own name, in a + directory of its own under `base` (`name`, else `plan-`), with + `.gitmodules` and DEPENDENCY both kept in code unless `answers` says + otherwise. `org` is the source's, as `plan` reads it from its `origin`, + or `plan` refuses it (#499): see `its_org` for a local one.""" plan = base / (name or f"plan-{org}") / "adoption-plan.yaml" plan.parent.mkdir(parents=True) - written = write_plan(source, plan, org=org) + written = write_plan(source, plan, project=THING, org=org) assert written.returncode == 0, written.stderr + written.stdout for path, leg in (answers or BOTH_IN_CODE).items(): resolve(plan, path, leg) return plan +def its_org(origin: str) -> str: + """The organisation a LOCAL `origin` names, as `plan` reads one (#499): + the directory its repository is in, `remotes` for `remotes/Thing.git`. + (Where git keeps the path with `\\`, as on Windows, `plan` reads no + repository from it, and compares nothing.)""" + return Path(origin).parent.name + + +def in_another_org(plan: Path, org: str) -> Path: + """`plan` with its `org:` edited by hand to `org`: legs in another + organisation than the source's, which no `plan` writes since #499, so + `check` finds the plan `plan-not-the-source` beside anything else.""" + text = plan.read_bytes().decode() + line = f"\norg: {ORG}\n" + assert line in text, text + plan.write_bytes(text.replace(line, f"\norg: {org}\n", 1).encode()) + return plan + + def check(plan: Path): return run_script(ADOPT, "check", "--plan", str(plan)) @@ -246,19 +273,24 @@ def test_check_keeps_a_relative_url_its_github_leg_resolves_alike( organisation it names another repository, and that is found, with the submodule, its url, the leg's remote and both resolutions, and the remediation once after it. A url with a scheme is printed through - `redacted`, which hides its user too (`ssh://***@github.com:22/...`).""" + `redacted`, which hides its user too (`ssh://***@github.com:22/...`). + Only a hand edit puts the legs in another organisation than the + source's, which `check` finds too (#499).""" source = source_with(tmp_path, SIBLING, origin) same = check(planned(source, tmp_path)) passes(same) assert notes(same.stdout) == [alike(ORG, (SIBLING, DEPENDENCY, GITHUB_DEPENDENCY))] - result = check(planned(source, tmp_path, OTHER_ORG)) + result = check(in_another_org( + planned(source, tmp_path, name=f"plan-{OTHER_ORG}"), OTHER_ORG)) assert result.returncode == 1, result.stderr + result.stdout - assert found(result.stderr) == [(RELATIVE, moved( + relative, (other, _) = found(result.stderr) + assert relative == (RELATIVE, moved( adopter, SIBLING, adopter.redacted(origin), ITS_ORIGIN, f"to `{adopter.redacted(resolved)}`", CODE_LEG.format(org=OTHER_ORG), - "to `https://github.com/otherorg/dependency`"))] + "to `https://github.com/otherorg/dependency`")) + assert other == NOT_THE_SOURCE assert notes(result.stdout) == [] # Said once, after the findings, and it is not #166's remediation. assert result.stderr.count(adopter.RELATIVE_URL_REMEDIATION) == 1 @@ -271,7 +303,7 @@ def test_check_keeps_a_relative_url_its_github_leg_resolves_alike( @pytest.mark.parametrize("url, there", [ # Inside the repository that holds it: another one for every leg. (INSIDE, - "to `https://github.com/testorg/Northwind-code.git/dependency`"), + "to `https://github.com/testorg/Thing-code.git/dependency`"), # More `..` than the remote has components: git says so, in words # this pins only the start of, since they are git's to change. ("../" * 8 + "dependency", "not at all (fatal: "), @@ -310,19 +342,21 @@ def test_a_source_with_no_remote_resolves_from_its_own_directory(adopter, def test_the_source_resolves_from_the_remote_its_branch_tracks(tmp_path): """git's default remote is the one the branch tracks, and `origin` only - when it tracks none: a fork's `origin` is not where its `main` comes - from, and `upstream` is.""" - source = source_with(tmp_path, SIBLING, - "https://github.com/someone/Thing.git") - git("remote", "add", "upstream", HTTPS_ORIGIN, cwd=source) - git("config", "branch.main.remote", "upstream", cwd=source) - passes(check(planned(source, tmp_path))) - git("config", "--unset", "branch.main.remote", cwd=source) - result = check(planned(source, tmp_path, name="untracked")) + when it tracks none: a `main` that tracks a fork resolves against the + fork, and against `origin`, the repository being adopted, once it + tracks none. (`origin` is the source's own repository, which the plan + names, #499; the fork is the remote it tracks.)""" + source = source_with(tmp_path, SIBLING, HTTPS_ORIGIN) + git("remote", "add", "fork", "https://github.com/someone/Thing.git", + cwd=source) + git("config", "branch.main.remote", "fork", cwd=source) + result = check(planned(source, tmp_path)) assert [code for code, _ in found(result.stderr)] == [RELATIVE] assert ("from the source, at `https://github.com/someone/Thing.git` " - f"({ITS_ORIGIN}), to `https://github.com/someone/dependency`" + "(its remote `fork`), to `https://github.com/someone/dependency`" ) in result.stderr + git("config", "--unset", "branch.main.remote", cwd=source) + passes(check(planned(source, tmp_path, name="untracked"))) def test_each_relative_url_is_resolved_on_its_own(adopter, tmp_path): @@ -338,7 +372,7 @@ def test_each_relative_url_is_resolved_on_its_own(adopter, tmp_path): adopter, INSIDE, HTTPS_ORIGIN, ITS_ORIGIN, "to `https://github.com/testorg/Thing.git/dependency`", CODE_LEG.format(org=ORG), - "to `https://github.com/testorg/Northwind-code.git/dependency`"))] + "to `https://github.com/testorg/Thing-code.git/dependency`"))] assert notes(result.stdout) == [] both = source_with(tmp_path / "both", SIBLING, HTTPS_ORIGIN, @@ -381,14 +415,19 @@ def test_a_credential_in_the_source_remote_is_never_printed(adopter, secret = "s3cret-token" origin = f"https://x-access-token:{secret}@github.com/testorg/Thing.git" source = source_with(tmp_path, SIBLING, origin) - result = check(planned(source, tmp_path, OTHER_ORG)) + result = check(in_another_org( + planned(source, tmp_path, name=f"plan-{OTHER_ORG}"), OTHER_ORG)) assert secret not in result.stderr + result.stdout - assert found(result.stderr) == [(RELATIVE, moved( + relative, (other, _) = found(result.stderr) + assert relative == (RELATIVE, moved( adopter, SIBLING, "https://***@github.com/testorg/Thing.git", ITS_ORIGIN, "to `https://***@github.com/testorg/dependency`", CODE_LEG.format(org=OTHER_ORG), - "to `https://github.com/otherorg/dependency`"))] + "to `https://github.com/otherorg/dependency`")) + # #499's finding names the source by the `org/repo` its `origin` ends + # in, which holds no credential. + assert other == NOT_THE_SOURCE # And from the plan's own organisation it is one repository: the # credential is not part of a repository's identity. same = check(planned(source, tmp_path)) @@ -440,9 +479,11 @@ def refusal_of(adopter, plan: Path, tmp_path: Path, (HTTPS_ORIGIN, SIBLING, ORG), (SCP_ORIGIN, SIBLING, ORG), (SCP_OTHER_CASE, SIBLING, ORG), - # git's arithmetic, not the organisation: two `..` leave both behind. - (HTTPS_ORIGIN, TWO_UP, OTHER_ORG), -], ids=["https", "scp", "scp-other-case", "dotdot-dotdot-other-org"]) + # git's arithmetic: two `..` leave the organisation behind. It is the + # source's own, as every plan's is since #499; one in another was this + # case's point, and is now `plan-not-the-source` first. + (HTTPS_ORIGIN, TWO_UP, ORG), +], ids=["https", "scp", "scp-other-case", "dotdot-dotdot"]) def test_execute_resolves_again_from_the_remotes_it_would_create( adopter, tmp_path, monkeypatch, origin, url, org): """(c) `check` cannot know where `execute` will make the legs, so it @@ -529,12 +570,13 @@ def test_one_directory_through_a_symlink_is_one_repository(adopter, pytest.skip(f"this platform cannot create a symlink here: {exc}") linked = source_with(tmp_path / "linked", SIBLING, str(link / THING_REMOTE)) - plan = planned(linked, tmp_path / "linked") + plan = planned(linked, tmp_path / "linked", + its_org(str(link / THING_REMOTE))) assert refusal_of(adopter, plan, tmp_path, remotes) is None - elsewhere = source_with(tmp_path / "elsewhere", SIBLING, - str(tmp_path / "elsewhere" / THING_REMOTE)) - plan = planned(elsewhere, tmp_path / "elsewhere") + there = str(tmp_path / "elsewhere" / THING_REMOTE) + elsewhere = source_with(tmp_path / "elsewhere", SIBLING, there) + plan = planned(elsewhere, tmp_path / "elsewhere", its_org(there)) refusal = refusal_of(adopter, plan, tmp_path, remotes) assert refusal is not None and refusal.code == RELATIVE @@ -554,7 +596,7 @@ def test_a_relative_origin_resolves_from_the_source_working_tree( remotes = tmp_path / REMOTES origin = RELATIVE_ORIGIN if relative else str(remotes / THING_REMOTE) source = source_with(tmp_path, SIBLING, origin, remotes / "dependency") - plan = planned(source, tmp_path) + plan = planned(source, tmp_path, its_org(origin)) assert refusal_of(adopter, plan, tmp_path, remotes) is None elsewhere = tmp_path / "elsewhere" @@ -593,14 +635,14 @@ def clone_the_split(source: Path, clone: Path): @pytest.mark.parametrize("url, leg, there", [ # #190 itself: the re-verifier's `relative-url` case. (SIBLING, "code", "remotes/dependency"), - (INSIDE, "code", "remotes/Northwind-code.git/dependency"), + (INSIDE, "code", "remotes/Thing-code.git/dependency"), (TWO_UP, "code", "dependency"), # The SPEC leg's own remote, which `./` names from inside. - (INSIDE, "spec", "remotes/Northwind-spec.git/dependency"), + (INSIDE, "spec", "remotes/Thing-spec.git/dependency"), # Where `\` separates nothing, git appends `..\dependency` whole. (BACKSLASH, "code", "remotes/dependency" if sys.platform == "win32" - else "remotes/Northwind-code.git/..\\dependency"), + else "remotes/Thing-code.git/..\\dependency"), ], ids=["dotdot", "dot-slash", "dotdot-dotdot", "spec-leg", "backslash"]) def test_execute_refuses_a_relative_url_its_leg_resolves_elsewhere( tmp_path, url, leg, there): @@ -679,9 +721,9 @@ def test_a_relative_url_its_leg_resolves_alike_adopts_and_clones( the remotes directory.""" remotes = tmp_path / REMOTES dependency = remotes / "dependency" - source = source_with(tmp_path, url, origin if origin.startswith("..") - else str(remotes / origin), dependency) - plan = planned(source, tmp_path) + origin = origin if origin.startswith("..") else str(remotes / origin) + source = source_with(tmp_path, url, origin, dependency) + plan = planned(source, tmp_path, its_org(origin)) assert [code for code, _ in found(check(plan).stderr)] == [RELATIVE] result = execute(plan, tmp_path / WORK, remotes / legs) assert result.returncode == 0, result.stderr + result.stdout diff --git a/tests/test_adopt_split_branch.py b/tests/test_adopt_split_branch.py index d0c52dba..375eaef7 100644 --- a/tests/test_adopt_split_branch.py +++ b/tests/test_adopt_split_branch.py @@ -681,15 +681,20 @@ def test_check_reads_github_as_the_sources_origin_last_fetched_it( def test_an_origin_that_is_not_the_assembly_repository_is_not_asked( tmp_path): + """Since #499 `plan` writes no such plan, so the assembly is edited by + hand: `check` finds that the plan names another repository than the + source, and asks nothing of the branches the source's `origin` holds, + which are not the ones a run of this plan would push to.""" source = small_source(tmp_path) with_origin(source, github_repository(tmp_path)) - plan = tmp_path / "plan" / "adoption-plan.yaml" - plan.parent.mkdir() - written = run_script(ADOPT, "plan", "--source", str(source), "--project", - "Elsewhere", "--org", ORG, "--elected-by", "Test", - "--elected-on", "2026-09-02", "--out", str(plan)) - assert written.returncode == 0, written.stderr + written.stdout - assert findings(plan) == [] + plan = plan_for(source, tmp_path) + text = plan.read_bytes() + for key in (b"project", b" assembly"): + line = b"\n" + key + b": " + PROJECT.encode() + b"\n" + assert line in text, text + text = text.replace(line, b"\n" + key + b": Elsewhere\n", 1) + plan.write_bytes(text) + assert [code for code, _ in findings(plan)] == ["plan-not-the-source"] def test_a_branch_gone_from_github_leaves_check_once_the_source_prunes( diff --git a/tests/test_adopt_tracking_branch.py b/tests/test_adopt_tracking_branch.py index 68e64846..798c6665 100644 --- a/tests/test_adopt_tracking_branch.py +++ b/tests/test_adopt_tracking_branch.py @@ -179,7 +179,9 @@ def test_the_branches_asked_are_origins_once_the_source_has_fetched_any( `plan` notes it and `check` finds it, naming `origin`.""" seed = make_source_repo(tmp_path / "seed", tree=TREE, edits=()) git("branch", "develop", cwd=seed) - remote = tmp_path / "Northwind.git" + # Under `testorg/`: `plan` reads the source's repository from its + # `origin`'s last two directories, and the plan's must be it (#499). + remote = tmp_path / "testorg" / "Northwind.git" git(*FILE_PROTOCOL, "clone", "-q", "--bare", str(seed), str(remote), cwd=tmp_path) source = tmp_path / "Northwind"