diff --git a/adopt-project.py b/adopt-project.py index d44df71e..0626745c 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -158,7 +158,11 @@ gives no organisation (`.`), ending in a hyphen or holding two in a row (`adopt-bad-org`, #863). `check` finds each of these, the disk's included, under the same code: `execute` passed what `check` had called `plan ok` -(#863). Both read one list, `_what_cannot_be_held`. +(#863). Both read one list, `_what_cannot_be_held`. Its first question is +the one the scaffold asks of its `--project` (#989): an assembly root no +Windows disk can make a folder of (`NUL`, `Con`, `com1`, `Lpt9`), which +`plan` refuses as `--project` before anything exists, and `check` and +`execute` find in `legs.assembly` (`adopt-bad-project-name`, #1014). WHY `git filter-repo` AND NOT A VENDORED COPY. Extracting history correctly is a solved problem with one correct implementation, and a vendored copy of it @@ -287,6 +291,9 @@ ) # And the characters GitHub gives an organisation's name (#863). from shape_materialize import refuse_an_owner_github_cannot_spell # noqa: E402 +# Whether a Windows disk can make a folder of a name (#1014): the one question +# the scaffold asks of its `--project` and `family.py init` of `--family`. +from shape_materialize import refuse_an_unholdable_name # noqa: E402 # 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 @@ -4854,6 +4861,54 @@ def _names_held(naming: NamingPolicy, project: str, pins: set[str]) -> bool: return True +#: `adopt-bad-project-name` (#1014): an assembly root no Windows disk can make +#: a folder of. One code for the three subcommands, as `adopt-name-too-long` +#: is: `plan` asks it of `--project`, and `check` and `execute` of the plan's +#: `legs.assembly`, the name `execute` clones the root into a folder of. +BAD_PROJECT_NAME = "adopt-bad-project-name" +#: What the refusal's sentence says the name names, after "which names". +PROJECT_NAMES = "the assembly root and the folder `execute` clones it into" +#: And what it is told to do, in the words the scaffold gives +#: `scaffold-bad-project-name` and `family.py init` `family-bad-holder-name` +#: (#989), of what `--project` names here. A plan's name is mended by `plan` +#: again, which writes `project:`, `id:` and the three `legs:` from one +#: `--project`. Where the name is the source's own, adopted in place, it is +#: `_rename_first`'s way out, in its words: `plan` asks before it has read +#: the source, so it cannot yet say which of the two it is. +PROJECT_NAME_FIX = ( + "Run `plan` again with another --project. It names the assembly root, " + "and so the folder `execute` clones the root into and the folder every " + "clone of it is made in; and a root is cloned on Windows too, where no " + "file or folder can be called a device name (`CON`, `PRN`, `AUX`, " + "`NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`, in any case, with or without an " + "extension), and a name ending in a dot or a space is made under another " + "name. Where --project is the source's own name, it cannot be changed " + "here: an in-place adoption keeps the source's own name, and whether to " + "rename the source is a human's decision, made first on GitHub (`gh " + "repo rename`); then run `plan` again against the renamed repository, " + "with its new name as --project.") + + +def _refuse_a_root_no_disk_holds(name: str, flag: str, made: str) -> None: + """`adopt-bad-project-name`: `name`, the assembly root's, as `flag` + names it, is one a Windows disk can make a folder of, or this refuses + it, saying `made` (what was not made) before `PROJECT_NAME_FIX` (#1014). + + The question is `shape_materialize.refuse_an_unholdable_name`, the one + the scaffold asks of its `--project` and `family.py init` of `--family` + (#989): a Windows device name, with or without an extension, in any case + (`NUL`, `Con`, `com1`, `Lpt9`), a name ending in a dot or a space, or one + of Git's own names. `plan --project NUL` wrote a plan, and `check` said + `plan ok` over it, for a root those two refuse. A device name is the + WHOLE name: `Nullable`, and the legs `NUL-spec` and `NUL-code`, are + folders Windows makes, so only the root's name is asked. It starts no + process.""" + refuse_an_unholdable_name(BAD_PROJECT_NAME, flag, name, + names=PROJECT_NAMES, + remediation=f"Remediation: {made} " + f"{PROJECT_NAME_FIX}") + + #: What `plan` says after `checked_value`'s remedy when the value refused is #: the source's own branch, which nobody passed (#797, L1-71's first case on #: #457). The alphabet alone said nothing of the way past it, which works. @@ -5068,6 +5123,18 @@ def _checked_flag_inputs(args, naming: NamingPolicy) -> None: _leg_collision_remediation(products)) args.spec_path = checked_value(*spec) args.code_path = checked_value(*code) + # A `--project` NO WINDOWS DISK CAN MAKE A FOLDER OF (#1014), in the + # order #972 set for the scaffold and `init`: the naming refusal first, + # then this one, then a passed `--id`'s sentence. The naming policy is + # asked after the source is read, its remedy needing the source, so this + # is asked only of names it holds (`_names_held`): one it refuses too + # (`NUL.git`, `Nul.`) is left to the refusals asked after the source, as + # on main. Before the work directory, the source, `gh` and the plan + # file, as the scaffold asks it before anything exists. + held = _names_held(naming, args.project, set(products)) + if held: + _refuse_a_root_no_disk_holds(args.project, "--project", + PLAN_NOT_WRITTEN) # An `--id` that was passed is held to its rule here, named as the flag # (#601): that branch of `_refuse_bad_id` reads no source, so it is # given none. One derived from `--project` is refused where the source's @@ -5078,8 +5145,7 @@ def _checked_flag_inputs(args, naming: NamingPolicy) -> None: # and its remedy names the source where `--project` is the source's own # name, so a passed id beside such a name is refused where the source # is known, as the derived one is (`_refuse_bad_id`). - if args.id is not None and _names_held(naming, args.project, - set(products)): + if args.id is not None and held: _refuse_bad_id(args.id, args.project, {"id"}, None, naming, set(products)) # An `--org` that was passed, as `_planned_org` holds it, `''` included @@ -12507,12 +12573,13 @@ def _leg_remote_findings(plan: Plan, names: dict, #: The codes of `_what_cannot_be_held` whose fix changes the names the two #: leg repositories are asked under, `/-spec` and #: `/-code` (#871's review, S1): `adopt-name-too-long` is -#: mended with a shorter --project, `adopt-org-too-long` and `adopt-bad-org` -#: with another `org:`. `check` asks #861's leg question only where none of -#: them is found. `adopt-topic-too-long` is not one: it is mended with -#: another `id:`, and an id names no repository. -RENAMES_THE_LEGS = frozenset({"adopt-name-too-long", "adopt-org-too-long", - "adopt-bad-org"}) +#: mended with a shorter --project, `adopt-bad-project-name` with another +#: (#1014), `adopt-org-too-long` and `adopt-bad-org` with another `org:`. +#: `check` asks #861's leg question only where none of them is found. +#: `adopt-topic-too-long` is not one: it is mended with another `id:`, and an +#: id names no repository. +RENAMES_THE_LEGS = frozenset({"adopt-name-too-long", BAD_PROJECT_NAME, + "adopt-org-too-long", "adopt-bad-org"}) def _held_findings(plan: Plan, names: dict, local_remote_dir: Path | None, @@ -13150,8 +13217,10 @@ def _what_cannot_be_held(plan: Plan, names: dict, org: str, local: bool, naming: NamingPolicy | None = None): """Each `Refusal` of what the disk or GitHub will not hold of the three names, the organisation and the topic, in the order `execute` asks them, - one at a time: the names, and without `--local-remote-dir` the `org:`, - its length and then its characters, and the topic. + one at a time: the assembly root's name, of which a Windows disk must be + able to make a folder (`adopt-bad-project-name`, #1014), the names' + lengths, and without `--local-remote-dir` the `org:`, its length and + then its characters, and the topic. ONE LIST, TWO READERS (#863). `execute` refuses the first one (`_repository_urls`), before either leg exists; `check` finds every one, @@ -13161,7 +13230,10 @@ def _what_cannot_be_held(plan: Plan, names: dict, org: str, local: bool, `_refuse_what_check_finds` makes `check` and `execute` read one list of the manifest's values, means the two cannot disagree again. It is a generator, so `execute` asks nothing after the first refusal.""" - asked = [lambda: _refuse_names_nothing_keeps(names, org, local)] + asked = [lambda: _refuse_a_root_no_disk_holds( + names["assembly"], f"{plan.path}: `legs.assembly`", + "no repository was created."), + lambda: _refuse_names_nothing_keeps(names, org, local)] if not local: named = f"{plan.path}: `org:`" asked += [ diff --git a/scaffold-project.py b/scaffold-project.py index 546a1344..b97df2f1 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -1070,9 +1070,9 @@ def _refuse_an_unholdable_project_name(policy, project: str) -> None: THE NAMING REFUSAL SPEAKS FIRST, as `init` asks `_classify_family_name` before its own: a name this refuses that the policy refuses too - (`NUL.git`, `Nul.`) is refused as the policy refuses it, whatever the - id, and only a name the policy holds as an assembly root is told it is - a device name.""" + (`NUL.git`, `Nul.`) is refused as the policy refuses it, whatever id + is passed, and only a name the policy holds as an assembly root is + told it is a device name.""" try: refuse_an_unholdable_name( "scaffold-bad-project-name", "--project", project, diff --git a/tests/test_adopt_device_names.py b/tests/test_adopt_device_names.py new file mode 100644 index 00000000..82a0cba8 --- /dev/null +++ b/tests/test_adopt_device_names.py @@ -0,0 +1,278 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`adopt-project.py` refuses an assembly root no Windows disk can make a +folder of, by the one question the scaffold and `family.py init` ask (#1014). + +ON MAIN AT e2f9fc42, with a stub `gh` that logs its calls and exits 97 first +on PATH, `GH_TOKEN=bogus` and an empty `GH_CONFIG_DIR`: `plan --project NUL` +(and `Con`, `com1`, `Lpt9`) wrote a plan, `project NUL (nul) in acme`, +exit 0, and `check` said `plan ok` over it, exit 0, after asking `gh repo +view` of `acme/NUL-spec` and `acme/NUL-code`. Since #1010 (#989) the +scaffold refuses the same `--project` as `scaffold-bad-project-name`, and +`init` has refused the same `--family` as `family-bad-holder-name` since +#688: a root no Windows disk can clone into a folder of its name. + +NOW `plan` asks `shape_materialize.refuse_an_unholdable_name` of `--project` +as `adopt-bad-project-name`, with the flags (`_checked_flag_inputs`), before +the work directory, the source, `gh` and the plan file: in the order #972 set +for the scaffold and `init`, the naming refusal first (asked after the source +is read, so a name the policy refuses too is left to it, as on main), then +this, then a passed `--id`'s sentence. `check` and `execute` ask it of the +plan's `legs.assembly`, the name `execute` clones the root into a folder of, +first in the one list both read (`_what_cannot_be_held`, #863), under the +same code; it is one of `RENAMES_THE_LEGS`, so `check` asks `gh` nothing of +the legs of a name that has to change. A device name is the WHOLE name: +`Nullable`, `Console` and `Auxo` plan and check as on main. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from conftest import ADOPT, ORG, REPO, make_source_repo, run_script +from test_adopt_plan_flag_refusals_first import ( # noqa: F401 (fixtures) + SMALL_TREE, SOURCES, Harness, harness, refused_first, source, +) + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import NamingPolicy, Refusal # noqa: E402 + +POSIX_ONLY = pytest.mark.skipif( + sys.platform == "win32", + reason="the `gh` stub is a `#!/bin/sh` script, which Windows runs not") + +CODE = "adopt-bad-project-name" +#: The names no Windows disk can make a folder of, one per case the question +#: folds, and the controls, each of which only starts with one. +DEVICES = ["NUL", "Con", "com1", "Lpt9"] +CONTROLS = ["Nullable", "Console", "Auxo"] +#: The project the plan `check` is given is written for, then renamed. +STAND_IN = "Zqxj" + + +def planned(name: str, *flags: str) -> tuple: + """`plan`'s arguments after `--source `, for `--project name`.""" + return ("--project", name, "--org", ORG, "--elected-by", "Test Human", + "--elected-on", "2026-09-02", *flags) + + +def detail(name: str, named: str = "--project") -> str: + """What the refusal says of `name`, named as `named`.""" + return (f"{named} is {name!r}, which names the assembly root and the " + f"folder `execute` clones it into, and `{name}` is a Windows " + "device name (with or without an extension), which cannot be " + "created there.") + + +# --- plan ------------------------------------------------------------------- + +@POSIX_ONLY +@pytest.mark.parametrize("cloned, work", SOURCES) +@pytest.mark.parametrize("name", DEVICES) +def test_plan_refuses_a_device_name_before_anything_exists( + harness, source, name, cloned, work): + """`--project NUL`, `Con`, `com1` and `Lpt9`, from an `org/repo` source + and a local one: refused with the flags, no `gh`, no `git`, no work + directory, nothing in the temp folder and no plan. On main each wrote + a plan, exit 0.""" + given = f"{ORG}/{name}" if cloned else str(source) + result = harness.plan(given, work=work, planned=planned(name)) + refused_first(harness, result, CODE, detail(name), work) + assert result.stderr == ( + f"REFUSED {CODE}: {detail(name)}\nRemediation: no plan was written. " + "Run `plan` again with another --project. It names the assembly " + "root, and so the folder `execute` clones the root into and the " + "folder every clone of it is made in; and a root is cloned on " + "Windows too, where no file or folder can be called a device name " + "(`CON`, `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`, in any " + "case, with or without an extension), and a name ending in a dot or " + "a space is made under another name. Where --project is the " + "source's own name, it cannot be changed here: an in-place adoption " + "keeps the source's own name, and whether to rename the source is a " + "human's decision, made first on GitHub (`gh repo rename`); then " + "run `plan` again against the renamed repository, with its new name " + "as --project.\n") + + +@POSIX_ONLY +@pytest.mark.parametrize("name", CONTROLS) +def test_plan_takes_a_name_that_only_starts_with_one(harness, source, name): + """The controls: a device name is the whole name. Planned as on main, + and `plan` asks `gh` nothing of a local source.""" + result = harness.plan(str(source), planned=planned(name)) + assert result.returncode == 0, result.stdout + result.stderr + assert f"project {name} ({name.lower()}) in {ORG}" in result.stdout + assert harness.out.is_file() + assert harness.gh_calls() == [] + + +@POSIX_ONLY +@pytest.mark.parametrize("passed", ["foo_bar", "", "nul\n"]) +def test_plan_says_the_name_before_a_passed_id(harness, source, passed): + """The order #972 set: the name speaks before a passed `--id`, an id + that is no id (`foo_bar`, `''`) and the one `PROJECT_ID_RE.match` lets + through with a line break, which no `--id` mends.""" + result = harness.plan(str(source), work=True, + planned=planned("NUL", "--id", passed)) + refused_first(harness, result, CODE, detail("NUL"), True) + assert "adopt-bad-id" not in result.stderr + assert "unsafe-value" not in result.stderr + + +@POSIX_ONLY +@pytest.mark.parametrize("name", ["NUL.git", "Nul."]) +@pytest.mark.parametrize("passed", ["foo", "foo_bar"]) +def test_plan_leaves_a_name_the_policy_refuses_to_the_policy( + harness, source, name, passed): + """THE NAMING REFUSAL SPEAKS FIRST: a device-shaped name the naming + policy holds as no assembly root is `naming-unclassified`, as on main, + whatever id is passed, and is never told it is a device name; as the + scaffold says it of `NUL.git` and `Nul.` (#1010's decision 1).""" + result = harness.plan(str(source), planned=planned(name, "--id", passed)) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED naming-unclassified: {name!r} matches no family in the " + "naming policy"), result.stderr + assert CODE not in result.stderr + assert not harness.out.exists() + + +# --- check ------------------------------------------------------------------ + +@pytest.fixture(scope="module") +def stand_in_plan(tmp_path_factory) -> tuple[Path, str]: + """A source, and the text of the plan `plan` writes for it as + `STAND_IN`, which each `check` below renames: `plan` writes no plan for + a device name now, and the plan main wrote for one is this one renamed + (`project:`, `id:` and the three `legs:` names).""" + base = tmp_path_factory.mktemp("stand-in") + made = make_source_repo(base / "Thing", tree=SMALL_TREE, edits=()) + planner = Harness(base) + result = planner.plan(str(made), planned=planned(STAND_IN)) + assert result.returncode == 0, result.stdout + result.stderr + return made, planner.out.read_text(encoding="utf-8") + + +def checked(stub: Harness, stand_in_plan, name: str, base: Path): + """`check` of the stand-in plan renamed `name`, with `--work-dir + base/work`, under `stub`'s `gh`.""" + made, text = stand_in_plan + plan = base / f"plan-{name}.yaml" + plan.write_text(text.replace(STAND_IN, name).replace( + STAND_IN.lower(), name.lower()), encoding="utf-8") + return plan, run_script(ADOPT, "check", "--plan", str(plan), "--source", + str(made), "--work-dir", str(base / "work"), + env=stub.env) + + +@POSIX_ONLY +@pytest.mark.parametrize("name", DEVICES) +def test_check_finds_a_device_name_and_asks_gh_nothing( + harness, stand_in_plan, tmp_path, name): + """The finding, alone, exit 1, under `plan`'s code. On main: `plan ok`, + exit 0, and `gh repo view` of both legs. Its fix renames the legs, so + the leg question is not asked (`RENAMES_THE_LEGS`); `check` makes the + missing `--work-dir`, empty, before it reads the plan, as on main, and + nothing in it.""" + plan, result = checked(harness, stand_in_plan, name, tmp_path) + assert result.returncode == 1, result.stdout + result.stderr + findings = [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + assert findings == [ + f"FINDING {CODE}: {detail(name, f'{plan}: `legs.assembly`')}"], \ + result.stderr + assert ("\nRemediation: no repository was created. Run `plan` again " + "with another --project. ") in result.stderr + assert "plan ok" not in result.stdout + assert harness.gh_calls() == [] + assert list((tmp_path / "work").iterdir()) == [] + + +@POSIX_ONLY +@pytest.mark.parametrize("name", CONTROLS) +def test_check_passes_a_name_that_only_starts_with_one( + harness, stand_in_plan, tmp_path, name): + """The controls: `plan ok`, after the leg question, which the stub + cannot answer, so it is a note, as on main.""" + _, result = checked(harness, stand_in_plan, name, tmp_path) + assert result.returncode == 0, result.stdout + result.stderr + assert "\nplan ok" in result.stdout + assert CODE not in result.stderr + assert harness.gh_calls() == [ + f"repo view {ORG}/{name}-{leg} --json nameWithOwner" + for leg in ("spec", "code")] + + +# --- execute: the one list -------------------------------------------------- + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_device_names_1014", + ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class _Plan: + """What the list reads of a plan: its path, its keys and its id.""" + path = Path("adoption-plan.yaml") + + def __init__(self, project_id: str): + self.data = {"org": ORG, "id": project_id} + + def get(self, key, default=None): + return self.data.get(key, default) + + @property + def project_id(self) -> str: + return self.data["id"] + + +def names_of(name: str) -> dict: + return {"assembly": name, "spec": f"{name}-spec", "code": f"{name}-code"} + + +@pytest.mark.parametrize("local", [False, True], ids=["github", "local"]) +@pytest.mark.parametrize("name", DEVICES) +def test_execute_refuses_it_first_of_the_list_check_reads(adopter, name, + local): + """`execute` refuses the first of `_what_cannot_be_held` + (`_repository_urls`), before either leg exists, and `check` finds each + of them: the device name is the one, on GitHub and under + `--local-remote-dir` alike, where a root is cloned too.""" + naming = NamingPolicy.load(REPO / "contracts" / "repository-naming.yaml") + plan = _Plan(name.lower()) + refused = list(adopter._what_cannot_be_held(plan, names_of(name), ORG, + local, naming)) + assert [exc.code for exc in refused] == [CODE] + assert refused[0].detail == detail( + name, "adoption-plan.yaml: `legs.assembly`") + run_args = SimpleNamespace( + local_remote_dir=Path("remotes") if local else None) + with pytest.raises(Refusal) as caught: + adopter._repository_urls(run_args, plan, names_of(name), None, + naming) + assert caught.value.code == CODE + assert CODE in adopter.RENAMES_THE_LEGS + + +def test_the_device_question_comes_before_the_organisations(adopter): + """First in `_what_cannot_be_held`: a root no disk can hold is found + ahead of a 40-character `org:`, so `execute`, which refuses the first, + says `adopt-bad-project-name` and `check` lists it first.""" + naming = NamingPolicy.load(REPO / "contracts" / "repository-naming.yaml") + refused = list(adopter._what_cannot_be_held( + _Plan("nul"), names_of("NUL"), "a" * 40, False, naming)) + assert [exc.code for exc in refused] == [CODE, "adopt-org-too-long"] + + +@pytest.mark.parametrize("name", CONTROLS) +def test_the_list_holds_a_name_that_only_starts_with_one(adopter, name): + naming = NamingPolicy.load(REPO / "contracts" / "repository-naming.yaml") + assert list(adopter._what_cannot_be_held( + _Plan(name.lower()), names_of(name), ORG, False, naming)) == [] diff --git a/tests/test_derived_id_refusals.py b/tests/test_derived_id_refusals.py index 93bee99c..15049664 100644 --- a/tests/test_derived_id_refusals.py +++ b/tests/test_derived_id_refusals.py @@ -273,13 +273,16 @@ def test_a_name_no_holder_can_have_speaks_before_a_passed_id(tmp_path, name, assert "family-bad-id" not in stderr -@pytest.mark.parametrize("passed", ["foo_bar", ""]) +@pytest.mark.parametrize("passed", ["foo_bar", "", "nul\n"]) def test_a_name_no_root_can_have_on_windows_speaks_before_a_passed_id( tmp_path, passed): """The scaffold's half of the row above (#989): `--project NUL`, which no Windows disk can make a folder of, is refused as `scaffold-bad-project-name` before a passed id that is no id, as `init` - refuses `--family NUL` before its own; no `--id` mends it.""" + refuses `--family NUL` before its own; no `--id` mends it. And before a + passed id's alphabet (#1014, RV1010's NIT-1): `nul` and a line break, + the one id `PROJECT_ID_RE.match` lets through, is `checked_value`'s + `unsafe-value` where the name is asked after it.""" stderr = run_tool(tmp_path, "scaffold", "NUL", f"--id={passed}") assert stderr.startswith( "REFUSED scaffold-bad-project-name: --project is 'NUL', which names " diff --git a/tests/test_scaffold_device_names.py b/tests/test_scaffold_device_names.py index 7fbae5a5..7d052535 100644 --- a/tests/test_scaffold_device_names.py +++ b/tests/test_scaffold_device_names.py @@ -176,8 +176,8 @@ def test_a_name_the_policy_refuses_too_is_refused_by_the_policy( tmp_path, project, passed): """THE NAMING REFUSAL SPEAKS FIRST, as `init` asks `_classify_family_name` before its own: a device-shaped name the policy holds as no assembly - root is refused as `naming-unclassified`, whatever the id, and is never - told it is a device name.""" + root is refused as `naming-unclassified`, whatever id is passed, and is + never told it is a device name.""" result = scaffold(tmp_path, project, f"--id={passed}") assert result.returncode == 2, result.stdout + result.stderr assert refused_line(result).startswith(