From 9f429f40b6910946ef88f6ae44268c816b17f076 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 23:38:02 +0000 Subject: [PATCH 1/8] setup-project.py: a project name typed at the prompt is checked as `--project` is and an empty line says no name was typed, so `../x`, `a/b`, `..` and `--help` are refused by name, not by an `--id` hint, a help page or a probe write (#729) The name typed at the `project name` prompt was used as typed. The same value as `--project` or as the positional goes through `checked_value` at parse time and is refused there. Typed, it reached step (3), which builds the clone path from it and asks the disk about that path: `..` leaves `--into`, an absolute name replaces it, and #608's write probe is made in the nearest folder above it. Step (4) did not catch it either: the naming validator reads only the last part of what it is handed (`../x` passes as `x`), has no length bound and reads a leading `-` as its own option, so `--help` printed its help and exited 0. The scaffold's plan then refused `../x` as `scaffold-bad-id: --id '../x'`, an id nobody passed, and `-h` with a usage dump. Now `checked_value("project name", typed)` runs right after the `ask`, the check `take_positional` makes, so the refusal is the tool's own wording with `project name` as its subject, exit 2, before anything reads or writes a path. `a b` and `Bad_Name` still go to step (4): `NAME_RE` is permissive on purpose and the policy rules on those. Nothing in step (3) changes, and the validator's basename rule stays as it is. An empty typed line (and Ctrl-D) now says "no project name was typed; nothing was created.", exit 2, with no usage text. It said "no --project given, and no terminal to ask on." after the full usage text, which is the refusal for no terminal, and `can_ask()` still gives that one. tests/test_setup_project_typed_name.py types each name at a pseudo-terminal (POSIX only, as the other terminal tests are). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 14 ++- tests/test_setup_project_typed_name.py | 136 +++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 tests/test_setup_project_typed_name.py diff --git a/setup-project.py b/setup-project.py index 1b2cab7f..b774914b 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1477,7 +1477,19 @@ def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: say("") say("(3) project") if not opts.project and can_ask(): - opts.project = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + # AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL (#729): + # `can_ask()` above is the gate that refuses for want of one, so this + # refusal says what happened and does not print the usage text. + if not typed: + die("no project name was typed; nothing was created.") + # THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` + # or as the positional (#729), before anything builds a path from it: + # `..` leaves `--into`, an absolute name replaces it, and step (4)'s + # validator reads only the last part of what it is handed. `a b` and + # `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) + # is where the policy rules on those. + opts.project = checked_value("project name", typed) if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py new file mode 100644 index 00000000..f1a3c26c --- /dev/null +++ b/tests/test_setup_project_typed_name.py @@ -0,0 +1,136 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The project name typed at `setup-project.py`'s prompt (#729). + +A name that arrives as `--project` or as the positional is checked at parse +time, before step (1) prints, by `checked_value`. A name typed at the +`project name` prompt reached step (3) unchecked: `..` left `--into`, an +absolute name replaced it, `--help` printed the naming validator's help at +step (4) and exited 0, and the refusals that did come named an `--id` nobody +passed or dumped the usage text. These tests type each of those at a real +terminal and hold the run to the `--project` refusal, worded for what the +person did: the value is a `project name`. + +POSIX only, as every test that gives a run a terminal is (`PTY_SKIP`). +""" + +from __future__ import annotations + +import sys + +import pytest + +from conftest import PTY_SKIP, REPO, run_in_a_terminal +from test_setup_project_py import (SETUP_PROJECT, SETUP_SH, piped, run_entry, + terminal_env) + +PROMPT = "project name (ONE CamelCase token, e.g. Atlas): " + +#: What a terminal in canonical mode takes as "the next character is text": +#: ^V, which is how a control character is typed into a line. +LNEXT = chr(0x16) +BELL = chr(7) + +#: Names the check refuses, by case. `absolute` is a path under `tmp_path`, +#: so it is built by `typed_value`. +REFUSED = { + "up-and-out": "../x", + "slash": "a/b", + "dot-dot": "..", + "dot": ".", + "dash": "-x", + "help": "--help", + "control": "a" + BELL + "b", + "long": "x" * 256, + "blank": " ", + "absolute": None, +} + +#: Names step (4)'s policy rules on, as it did before: `NAME_RE` is +#: permissive on purpose. +LEFT_TO_STEP_FOUR = ("Bad_Name", "a b") + + +def typed_value(tmp_path, case): + value = REFUSED[case] + return str(tmp_path / "elsewhere" / "Name") if value is None else value + + +def at_the_prompt(tmp_path, typed, entry="py"): + """Setup, with no project, asked at a terminal and answered `typed` + (`"EOF"` is Ctrl-D). `entry` is the file run directly, that file with + its stdin a pipe (the terminal is then the controlling one), or + `setup.sh`. Returns the exit status and the screen.""" + flags = ["--org", "demoorg", "--local-remote-dir", + str(tmp_path / "remotes"), "--into", + str(tmp_path / "parent" / "into")] + argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], + "sh": ["bash", str(SETUP_SH), *flags], + "piped": piped("", *flags)}[entry] + if typed != "EOF": + typed = typed.replace(BELL, LNEXT + BELL) + "\n" + status, screen, answered = run_in_a_terminal( + argv, cwd=REPO, env=terminal_env(), answers=[typed], prompt=PROMPT, + timeout=60) + assert answered == 1, f"it never asked for a project name:\n{screen}" + assert "(3) project" in screen, screen + return status, screen + + +def the_refusal(text): + return text[text.index("REFUSED: "):].strip() + + +def assert_refused_at_the_prompt(tmp_path, value, entry="py"): + status, screen = at_the_prompt(tmp_path, value, entry) + assert status == 2, f"status {status}:\n{screen}" + assert f"project name is {ascii(value)}, which is not a value" in screen + assert "(4) naming policy" not in screen, screen + assert "usage: setup-project.py" not in screen, screen + assert "--id" not in screen, screen + assert list(tmp_path.iterdir()) == [], "the refused name left a trace" + return screen + + +@PTY_SKIP +@pytest.mark.parametrize("case", REFUSED) +def test_a_typed_project_name_is_refused_as_the_flag_refuses_it(tmp_path, + case): + value = typed_value(tmp_path, case) + screen = assert_refused_at_the_prompt(tmp_path, value) + flagged = run_entry("--project", value, "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "parent" / "into")) + assert flagged.returncode == 2, flagged.stderr + assert the_refusal(screen) == the_refusal(flagged.stderr).replace( + "REFUSED: --project is", "REFUSED: project name is", 1) + + +@PTY_SKIP +@pytest.mark.parametrize("entry", ["piped", "sh"]) +def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): + """The same check on the one-liner's shape (stdin a pipe, the answer + read from the terminal) and through `setup.sh`, the shim.""" + assert_refused_at_the_prompt(tmp_path, "../x", entry) + + +@PTY_SKIP +@pytest.mark.parametrize("typed", ["", "EOF"]) +def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal(tmp_path, + typed): + status, screen = at_the_prompt(tmp_path, typed) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: no project name was typed; nothing was created.") + assert "no terminal" not in screen, screen + assert "usage: setup-project.py" not in screen, screen + assert "(4) naming policy" not in screen, screen + assert list(tmp_path.iterdir()) == [] + + +@PTY_SKIP +@pytest.mark.parametrize("value", LEFT_TO_STEP_FOUR) +def test_a_name_the_policy_decides_is_left_to_step_four(tmp_path, value): + status, screen = at_the_prompt(tmp_path, value) + assert "(4) naming policy" in screen, screen + assert "project name is" not in screen, screen + assert status == 1, f"status {status}:\n{screen}" From b3c77cc2d22fea4d64fb7f4407a6fff6e74ee551 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 00:05:46 +0000 Subject: [PATCH 2/8] setup-project.py: the typed project name's prompt, empty-line refusal and check are one helper, and the tests pin where the check sits, so a check made after step (3) has asked the disk fails them (#729) Review round on 9f429f40 (rv729: approve with nits, two SHOULDs). SHOULD-1. SonarCloud reported python:S3776 on resolve_project_and_elector (cognitive complexity 16 of 15). The prompt, the empty-line refusal and the `checked_value("project name", typed)` call move into `ask_for_the_project_name()`, with their comments; the call site is `opts.project = ask_for_the_project_name()`. No behaviour changes. The function is back to 14 by rv729's estimator, and the helper is 1. SHOULD-2. The tests did not pin where the check sits: a check moved to just before `check_names`, or to after `resolve_family`, passed all 16 of them. Now every refused name is typed with a file standing where the folder above `--into` goes, so a check made after step (3) asks the disk would end in "is a file" and not "project name is"; and `[ok] project` and `[ok] legs` must not print. Only the seeded file may be on the disk afterwards, unchanged. Both moved checks now fail 12 tests each. Nits. The empty line and Ctrl-D are also typed through the one-liner's shape (stdin a pipe) and through `setup.sh`, 6 tests in place of 2. `(4) naming policy` is one constant. The module docstring no longer says `--help` "exited 0" of the run: the naming validator exited 0 at step (4), and the run ended at step (5) with exit 2. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 34 +++++++++------ tests/test_setup_project_typed_name.py | 59 +++++++++++++++++++------- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/setup-project.py b/setup-project.py index b774914b..3ec1a195 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1472,24 +1472,32 @@ def run_at_the_terminal(argv) -> subprocess.CompletedProcess: os.close(descriptor) +def ask_for_the_project_name() -> str: + """The `project name` prompt, and the name typed at it, checked (#729). + + AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL: `can_ask()` + at the call is the gate that refuses for want of one, so this refusal says + what happened and does not print the usage text. + + THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` or + as the positional, before anything builds a path from it: `..` leaves + `--into`, an absolute name replaces it, and step (4)'s validator reads + only the last part of what it is handed. `a b` and `Bad_Name` pass here on + purpose (`NAME_RE` is permissive); step (4) is where the policy rules on + those. + """ + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + if not typed: + die("no project name was typed; nothing was created.") + return checked_value("project name", typed) + + def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: """setup.sh:355-386, transcribed.""" say("") say("(3) project") if not opts.project and can_ask(): - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") - # AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL (#729): - # `can_ask()` above is the gate that refuses for want of one, so this - # refusal says what happened and does not print the usage text. - if not typed: - die("no project name was typed; nothing was created.") - # THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` - # or as the positional (#729), before anything builds a path from it: - # `..` leaves `--into`, an absolute name replaces it, and step (4)'s - # validator reads only the last part of what it is handed. `a b` and - # `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) - # is where the policy rules on those. - opts.project = checked_value("project name", typed) + opts.project = ask_for_the_project_name() if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py index f1a3c26c..d27eeb68 100644 --- a/tests/test_setup_project_typed_name.py +++ b/tests/test_setup_project_typed_name.py @@ -4,11 +4,17 @@ A name that arrives as `--project` or as the positional is checked at parse time, before step (1) prints, by `checked_value`. A name typed at the `project name` prompt reached step (3) unchecked: `..` left `--into`, an -absolute name replaced it, `--help` printed the naming validator's help at -step (4) and exited 0, and the refusals that did come named an `--id` nobody -passed or dumped the usage text. These tests type each of those at a real -terminal and hold the run to the `--project` refusal, worded for what the -person did: the value is a `project name`. +absolute name replaced it, `--help` made the naming validator print its help +and exit 0 at step (4) (the run itself ended at step (5), with the scaffold's +usage and exit 2), and the refusals that did come named an `--id` nobody passed +or dumped the usage text. These tests type each of those at a real terminal and +hold the run to the `--project` refusal, worded for what the person did: the +value is a `project name`. + +Two things pin WHERE the check sits, not only that it is made. Nothing of step +(3) may print (`[ok] project`, `[ok] legs`), and a FILE stands where the folder +above `--into` goes, so a check made after step (3) has asked the disk would +say "is a file" and not `project name is`. POSIX only, as every test that gives a run a terminal is (`PTY_SKIP`). """ @@ -25,6 +31,13 @@ PROMPT = "project name (ONE CamelCase token, e.g. Atlas): " +#: Step (4)'s heading, which no refused name may reach. +NAMING_POLICY = "(4) naming policy" + +#: A file where the folder above `--into` goes, so any step that asks the +#: disk about the clone path refuses with "is a file". +FILE_ABOVE_INTO = b"a file where the folder above --into goes\n" + #: What a terminal in canonical mode takes as "the next character is text": #: ^V, which is how a control character is typed into a line. LNEXT = chr(0x16) @@ -55,11 +68,14 @@ def typed_value(tmp_path, case): return str(tmp_path / "elsewhere" / "Name") if value is None else value -def at_the_prompt(tmp_path, typed, entry="py"): +def at_the_prompt(tmp_path, typed, entry="py", seeded=False): """Setup, with no project, asked at a terminal and answered `typed` (`"EOF"` is Ctrl-D). `entry` is the file run directly, that file with its stdin a pipe (the terminal is then the controlling one), or - `setup.sh`. Returns the exit status and the screen.""" + `setup.sh`. `seeded` puts `FILE_ABOVE_INTO` where a later step would + refuse it. Returns the exit status and the screen.""" + if seeded: + (tmp_path / "parent").write_bytes(FILE_ABOVE_INTO) flags = ["--org", "demoorg", "--local-remote-dir", str(tmp_path / "remotes"), "--into", str(tmp_path / "parent" / "into")] @@ -80,14 +96,23 @@ def the_refusal(text): return text[text.index("REFUSED: "):].strip() +def assert_nothing_was_made(tmp_path): + """Only the seeded file is on the disk, and it is as it was.""" + assert sorted(p.name for p in tmp_path.iterdir()) == ["parent"] + assert (tmp_path / "parent").read_bytes() == FILE_ABOVE_INTO + + def assert_refused_at_the_prompt(tmp_path, value, entry="py"): - status, screen = at_the_prompt(tmp_path, value, entry) + status, screen = at_the_prompt(tmp_path, value, entry, seeded=True) assert status == 2, f"status {status}:\n{screen}" assert f"project name is {ascii(value)}, which is not a value" in screen - assert "(4) naming policy" not in screen, screen + assert "is a file" not in screen, screen + assert "[ok] project" not in screen, screen + assert "[ok] legs" not in screen, screen + assert NAMING_POLICY not in screen, screen assert "usage: setup-project.py" not in screen, screen assert "--id" not in screen, screen - assert list(tmp_path.iterdir()) == [], "the refused name left a trace" + assert_nothing_was_made(tmp_path) return screen @@ -114,23 +139,25 @@ def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): @PTY_SKIP +@pytest.mark.parametrize("entry", ["py", "piped", "sh"]) @pytest.mark.parametrize("typed", ["", "EOF"]) -def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal(tmp_path, - typed): - status, screen = at_the_prompt(tmp_path, typed) +def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal( + tmp_path, typed, entry): + status, screen = at_the_prompt(tmp_path, typed, entry, seeded=True) assert status == 2, f"status {status}:\n{screen}" assert the_refusal(screen) == ( "REFUSED: no project name was typed; nothing was created.") + assert "is a file" not in screen, screen assert "no terminal" not in screen, screen assert "usage: setup-project.py" not in screen, screen - assert "(4) naming policy" not in screen, screen - assert list(tmp_path.iterdir()) == [] + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) @PTY_SKIP @pytest.mark.parametrize("value", LEFT_TO_STEP_FOUR) def test_a_name_the_policy_decides_is_left_to_step_four(tmp_path, value): status, screen = at_the_prompt(tmp_path, value) - assert "(4) naming policy" in screen, screen + assert NAMING_POLICY in screen, screen assert "project name is" not in screen, screen assert status == 1, f"status {status}:\n{screen}" From 854f6ed95c890e3f47bab9e8ea4f032813f41a15 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 10:52:28 +0000 Subject: [PATCH 3/8] setup-project.py: a refused value names the check it failed, a project name over 95 characters is refused before step (4) where GitHub will hold the repositories, and the prompt takes the spaces off a typed name, so `../x` is no longer told about a leading `-`, a 100-character name no longer ends at step (5) or at GitHub, and `Atlas ` no longer hides in step (4) (#777) `refuse_value` gave every refusal the one closing sentence, and the sentence explained only a leading `-`: `../x`, `a/b`, ` Atlas`, `Atlas/` and a name with an accented letter were told about a `-` none of them has. It now takes the reason, and `why_refused` names the first check the value failed, in the order empty, leading `-` (which keeps its "git reads its own arguments" sentence), control character, then the pattern, said in words and as the pattern, as `repo_shape.checked_value` prints its own. `checked_ref` says `..` and `.lock` in their own sentences and `checked_passthrough` its control character. `repo_shape.checked_value` is not changed: it is a pinned copy, so a wording change there is a re-pin, and it already prints its pattern. A project name over 95 characters is refused before step (1) for `--project` and the positional, and before step (3) prints for the prompt, unless `--local-remote-dir` is given. GitHub keeps a repository name to at most 100 characters (its documentation: nothing here creates a real repository, so the number is not measured) and `-spec` and `-code` add five. Under `--local-remote-dir` the limit is the disk's and step (5) still asks it as `scaffold-name-too-long`. The check runs at the end of the parse, because `--local-remote-dir` can come after `--project` on the line. The prompt takes the spaces off the ends of what was typed. `Atlas ` passed `NAME_RE` where ` Atlas` was refused, and step (4) judged `Atlas -spec` with no space shown anywhere. A line of spaces is now an empty line. An argv value is deliberate and is held as typed. tests/test_setup_project_name_edges.py is new. #756's file moves its three-spaces case from the refusals to the empty answers, and a comment in tests/test_setup_project_py.py stops saying the two entry points refuse in the same words. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 138 +++++++-- tests/test_setup_project_name_edges.py | 395 +++++++++++++++++++++++++ tests/test_setup_project_py.py | 10 +- tests/test_setup_project_typed_name.py | 7 +- 4 files changed, 525 insertions(+), 25 deletions(-) create mode 100644 tests/test_setup_project_name_edges.py diff --git a/setup-project.py b/setup-project.py index 3ec1a195..dc95a73c 100755 --- a/setup-project.py +++ b/setup-project.py @@ -349,8 +349,53 @@ def program_version(argv, field: int) -> str: CONTROL_RE = re.compile(r"[\x00-\x1f\x7f]") -def refuse_value(what: str, text: str) -> None: - """ONE wording for every value this tool will not pass on. +#: A GITHUB REPOSITORY NAME IS AT MOST 100 CHARACTERS, and a project is three +#: repositories: ``, `-spec` and `-code`. The legs +#: add five characters (`-spec` and `-code` are as long as each other), so a +#: project name over 95 names repositories GitHub cannot create (#777). The +#: 100 is GitHub's own documented limit. No run of this repository has +#: measured it: none creates a real repository, and a probe against a real +#: organisation is another lane's. Asked only where a GitHub repository will +#: be made: under `--local-remote-dir` the limit is the disk's, which step (5) +#: asks as `scaffold-name-too-long` (#261). +GITHUB_NAME_MAX = 100 +PROJECT_NAME_MAX = GITHUB_NAME_MAX - len("-spec") + + +#: WHY A VALUE IS REFUSED, one sentence each, said after the fixed first +#: sentence of `refuse_value` (#777). The old refusal gave every value the same +#: closing sentence, which explained only a leading `-`: `../x`, `a/b` and an +#: accented name were told about a `-` none of them has. +WHY_EMPTY = "It is empty, and a value here needs at least one character." +WHY_DASH = ("It starts with `-`, and git reads its own arguments: a value " + "that starts with `-` is an option to git, not a name to us.") +WHY_CONTROL = ("It holds a control character, which the quotation shows as " + "an escape: a newline ends a line in every terminal and log " + "this output reaches, so a value carrying one could print a " + "line that reads as this tool's own.") +WHY_RANGE = ("It holds `..`, which git reads as a revision range and not as " + "a ref.") +WHY_LOCK = ("It ends in `.lock`, the name git gives the lock file it writes " + "beside a ref.") + +#: What each pattern asks, in words and then as the pattern itself, as the +#: sibling `repo_shape.checked_value` prints its own. `PATH_RE` has no row: it +#: refuses only an empty value and a control character, and both are named +#: before a pattern is asked; a pattern with no row is still named, as the +#: sibling names it, by `why_refused`. +WHY_PATTERN = { + NAME_RE: ("It is not spellable as a name: a letter or a digit first, " + "then letters, digits, spaces and `._'()&,+@-`, at most 255 " + "characters in all (`%s`)." % NAME_RE.pattern), + REF_RE: ("It is not spellable as a ref: a letter or a digit first, then " + "letters, digits and `._/-`, at most 255 characters in all " + "(`%s`)." % REF_RE.pattern), +} + + +def refuse_value(what: str, text: str, why: str) -> None: + """ONE first sentence for every value this tool will not pass on, and the + reason it failed as the second (#777). `ascii()`, never `%r`: `repr` leaves a printable non-ASCII character as itself, and this file's output is ASCII to the byte (see the module @@ -359,9 +404,26 @@ def refuse_value(what: str, text: str) -> None: PURE ASCII rule exists to prevent. """ die("%s is %s, which is not a value this tool will put on a `git` or `gh` " - "command line. git reads its own arguments: a value that starts with " - "`-` is an option to git, not a name to us; the rest must be spellable " - "as a name, a ref or a path." % (what, ascii(text))) + "command line. %s" % (what, ascii(text), why)) + + +def why_refused(text: str, pattern) -> str: + """The reason `checked_value` refuses `text`, or "" when it does not. + + Asked in the order a person would find them: nothing typed, a leading `-`, + a control character, and only then the pattern, so the sentence names the + first check the value failed and never one it passed. + """ + if not text: + return WHY_EMPTY + if text.startswith("-"): + return WHY_DASH + if CONTROL_RE.search(text): + return WHY_CONTROL + if not pattern.fullmatch(text): + return WHY_PATTERN.get(pattern) or ( + "It does not match `%s`." % pattern.pattern) + return "" def checked_value(flag: str, value: str, pattern=NAME_RE) -> str: @@ -376,18 +438,39 @@ def checked_value(flag: str, value: str, pattern=NAME_RE) -> str: write). THE HOUSE PATTERN, COPIED RATHER THAN IMPORTED. `scripts/repo_shape.py`'s - `checked_value` is the same check in the same words for the same reason, - and `scaffold-project.py` calls it on the values this file forwards. This + `checked_value` is the same check for the same reason, and + `scaffold-project.py` calls it on the values this file forwards. This entry point cannot import it: it runs BEFORE there is a checkout to import - it from, which is the whole of self-bootstrap mode. + it from, which is the whole of self-bootstrap mode. The two refuse the + same values; each says why in its own words, and this file's name the + check the value failed (`why_refused`), where the sibling prints one + remediation for all of them. """ text = str(value) - if (not text or text.startswith("-") or CONTROL_RE.search(text) - or not pattern.fullmatch(text)): - refuse_value(flag, text) + why = why_refused(text, pattern) + if why: + refuse_value(flag, text, why) return text +def checked_project_length(what: str, name: str, local_mode: bool) -> str: + """A project name that GitHub will hold three repositories under, or a + refusal before step (4) explains three names it will not use (#777). + + ASKED OF A NAME THAT HAS PASSED `checked_value`, so it is printable ASCII + and is quoted back as it is, both ends of it: the name is long, and a + name this long differs from its neighbours by its tail. + """ + if local_mode or len(name) <= PROJECT_NAME_MAX: + return name + die("%s is %d characters long (%s...%s), and a project is three " + "repositories, ``, `-spec` and `-code`: " + "GitHub keeps a repository name to at most %d characters, so a " + "project name is at most %d. Nothing was created. Use a shorter name." + % (what, len(name), name[:12], name[-12:], GITHUB_NAME_MAX, + PROJECT_NAME_MAX)) + + def checked_ref(flag: str, value: str) -> str: """`--shape-ref`: narrower than a name, because git reads more into one. @@ -397,8 +480,10 @@ def checked_ref(flag: str, value: str) -> str: confusing failure inside the temporary checkout. """ text = checked_value(flag, value, REF_RE) - if ".." in text or text.endswith(".lock"): - refuse_value(flag, text) + if ".." in text: + refuse_value(flag, text, WHY_RANGE) + if text.endswith(".lock"): + refuse_value(flag, text, WHY_LOCK) return text @@ -417,7 +502,8 @@ def checked_passthrough(rest: list) -> list: """ for item in rest: if CONTROL_RE.search(item): - refuse_value("a passthrough argument after `--`", item) + refuse_value("a passthrough argument after `--`", item, + WHY_CONTROL) return rest @@ -580,6 +666,12 @@ def parse_args(argv, invocation_dir: str) -> Options: if opts.visibility not in VISIBILITY_CHOICES: die("--visibility is %s; it must be 'private', 'public' or 'internal'" % opts.visibility) + # ASKED HERE, NOT WHERE `--project` IS TAKEN: `--local-remote-dir` can + # come after it on the line, and decides whether a GitHub repository + # will be made (#777). Still before step (1) prints. + if opts.project: + spelling = "" if state.positional else "--project" + checked_project_length(spelling, opts.project, opts.local_mode) return opts @@ -1472,7 +1564,7 @@ def run_at_the_terminal(argv) -> subprocess.CompletedProcess: os.close(descriptor) -def ask_for_the_project_name() -> str: +def ask_for_the_project_name(local_mode: bool) -> str: """The `project name` prompt, and the name typed at it, checked (#729). AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL: `can_ask()` @@ -1485,11 +1577,21 @@ def ask_for_the_project_name() -> str: only the last part of what it is handed. `a b` and `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) is where the policy rules on those. + + THE SPACES AT THE ENDS ARE TAKEN OFF FIRST (#777). A trailing space + passed `NAME_RE` (a space is allowed after the first character, so `a b` + can reach step (4)) where a leading one was refused, and step (4) then + judged `Atlas `, `Atlas -spec` and `Atlas -code` and showed no space + anywhere. A stray space is what a prompt answer gets from a paste or a + slip of the thumb, and the run ends at the first mistake with no second + ask, so the prompt takes it off; an argv value is deliberate and is held + as typed. A line of nothing but spaces is then an empty line. """ - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ").strip() if not typed: die("no project name was typed; nothing was created.") - return checked_value("project name", typed) + return checked_project_length( + "project name", checked_value("project name", typed), local_mode) def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: @@ -1497,7 +1599,7 @@ def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: say("") say("(3) project") if not opts.project and can_ask(): - opts.project = ask_for_the_project_name() + opts.project = ask_for_the_project_name(opts.local_mode) if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py new file mode 100644 index 00000000..d7f53a4d --- /dev/null +++ b/tests/test_setup_project_name_edges.py @@ -0,0 +1,395 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The edges of a project name typed at `setup-project.py` (#777). + +#729 made the name typed at the prompt answer to the check `--project` gets. +Three things were left, each measured on #729's head: + +* `refuse_value` gave every refusal one closing sentence, and the sentence + explained only a leading `-`, so `../x`, `a/b`, ` Atlas` and a name with an + accented letter were told about a `-` none of them has. Each refusal now + names the check the value failed: empty, a leading `-`, a control character, + or the pattern for a name or a ref, in words and as the pattern. +* A name of 96 to 255 characters passed the prompt and `--project` and ended at + step (5) or at GitHub, after step (4) printed its wall for three names that + could never be repositories. GitHub keeps a repository name to at most 100 + characters (its documentation: nothing here creates a real repository, so the + number is not measured), and `-spec` and `-code` add five, so a project name + is at most 95, asked before step (4) and only where a GitHub repository will + be made. Under `--local-remote-dir` the limit is the disk's, and step (5) + still asks it as `scaffold-name-too-long`. +* `Atlas ` passed `NAME_RE` where ` Atlas` was refused, and step (4) judged + `Atlas -spec` with no space shown anywhere. The prompt now takes the spaces + off the ends of what was typed; an argv value is deliberate and is held as + typed. + +POSIX only where a run needs a terminal or a `gh` stub (`PTY_SKIP`). +""" + +from __future__ import annotations + +import os +import re +import string +import sys + +import pytest + +from conftest import PTY_SKIP, REPO, run_in_a_terminal +from test_setup_project_py import (SETUP_PROJECT, entry_point_module, + run_entry, terminal_env) +from test_setup_project_typed_name import (BELL, LNEXT, NAMING_POLICY, PROMPT, + the_refusal) + +#: The first sentence every refusal says, then its reason. +FIRST = ("{what} is {shown}, which is not a value this tool will put on a " + "`git` or `gh` command line. ") + +EMPTY = "It is empty, and a value here needs at least one character." +DASH = ("It starts with `-`, and git reads its own arguments: a value that " + "starts with `-` is an option to git, not a name to us.") +CONTROL = ("It holds a control character, which the quotation shows as an " + "escape: a newline ends a line in every terminal and log this " + "output reaches, so a value carrying one could print a line that " + "reads as this tool's own.") +NAME = ("It is not spellable as a name: a letter or a digit first, then " + "letters, digits, spaces and `._'()&,+@-`, at most 255 characters in " + "all (`^[A-Za-z0-9][A-Za-z0-9 ._'()&,+@-]{0,254}$`).") +REF = ("It is not spellable as a ref: a letter or a digit first, then " + "letters, digits and `._/-`, at most 255 characters in all " + "(`^[A-Za-z0-9][A-Za-z0-9._/-]{0,254}$`).") +RANGE = ("It holds `..`, which git reads as a revision range and not as a " + "ref.") +LOCK = ("It ends in `.lock`, the name git gives the lock file it writes " + "beside a ref.") + +#: What a sentence for one check must never say about another. +NOT_THE_DASH = ("starts with", "option to git", "reads its own arguments") + +ACCENTED = "Caf\u00e9" + +#: Each refusal: the argv that carries the value (`{}` is the value), the +#: value, how the refusal names it, and the reason it gives. A value that +#: fails two checks is told the first, in the order empty, `-`, control +#: character, pattern. +REFUSALS = { + "org-empty": (("--org", "{}"), "", "--org", EMPTY), + "into-empty": (("--into", "{}"), "", "--into", EMPTY), + "org-dash": (("--org", "{}"), "-evil", "--org", DASH), + "into-dash": (("--into", "{}"), "-x", "--into", DASH), + "org-control": (("--org", "{}"), "a" + BELL + "b", "--org", CONTROL), + "into-control": (("--into", "{}"), "a" + BELL + "b", "--into", CONTROL), + "passthrough-control": (("--", "{}"), "a" + BELL + "b", + "a passthrough argument after `--`", CONTROL), + "project-up-and-out": (("--project", "{}"), "../x", "--project", NAME), + "project-slash": (("--project", "{}"), "a/b", "--project", NAME), + "project-dot-dot": (("--project", "{}"), "..", "--project", NAME), + "project-leading-space": (("--project", "{}"), " Atlas", "--project", + NAME), + "project-trailing-slash": (("--project", "{}"), "Atlas/", "--project", + NAME), + "project-accented": (("--project", "{}"), ACCENTED, "--project", NAME), + "project-blank": (("--project", "{}"), " ", "--project", NAME), + "org-dash-and-control": (("--org", "{}"), "-" + BELL, "--org", DASH), + "org-control-and-slash": (("--org", "{}"), "a" + BELL + "/b", "--org", + CONTROL), + "org-long": (("--org", "{}"), "x" * 256, "--org", NAME), + "positional": (("{}",), "../x", "", NAME), + "ref-space": (("--shape-ref", "{}"), "a b", "--shape-ref", REF), + "ref-long": (("--shape-ref", "{}"), "a" * 256, "--shape-ref", REF), + "ref-dash": (("--shape-ref", "{}"), "-x", "--shape-ref", DASH), + "ref-range": (("--shape-ref", "{}"), "a..b", "--shape-ref", RANGE), + "ref-lock": (("--shape-ref", "{}"), "topic.lock", "--shape-ref", LOCK), +} + + +def a_run_carrying(tmp_path, template, value): + """A run that would otherwise be valid, with `value` where `template` + puts it. A bare `` is the one carrier that cannot also be + given `--project`: two names are refused before either is checked.""" + carried = [part.replace("{}", value) for part in template] + context = ["--org", "demoorg", "--into", str(tmp_path / "work"), + "--local-remote-dir", str(tmp_path / "remotes")] + if template[0] != "{}": + context = ["--project", "Sample", *context] + return run_entry(*context, *carried) + + +@pytest.mark.parametrize("case", REFUSALS) +def test_a_refusal_names_the_check_the_value_failed(tmp_path, case): + template, value, what, why = REFUSALS[case] + result = a_run_carrying(tmp_path, template, value) + assert result.returncode == 2, result.stderr + expected = "REFUSED: " + FIRST.format(what=what, shown=ascii(value)) + why + assert the_refusal(result.stderr) == expected + if why != DASH: + for words in NOT_THE_DASH: + assert words not in result.stderr, result.stderr + assert not (tmp_path / "remotes").exists() + + +#: What typed at the prompt is refused, and why. An empty line is not here: +#: it is "no project name was typed", which #729's file pins. +TYPED = { + "dash": ("-x", DASH), + "control": ("a" + BELL + "b", CONTROL), + "up-and-out": ("../x", NAME), + "slash": ("a/b", NAME), + "trailing-slash": ("Atlas/", NAME), + "accented": (ACCENTED, NAME), + "long": ("x" * 256, NAME), +} + + +def gh_that_is_signed_in(tmp_path): + """The environment of a run that is allowed to call `gh` and gets the + three answers a preflight and step (3) read, and refuses the rest, so no + run here reaches a real GitHub (the real `gh` stays unreachable).""" + stub = tmp_path / "ghbin" + stub.mkdir(exist_ok=True) + gh = stub / "gh" + gh.write_bytes(b"#!/bin/sh\n" + b'case "$1" in\n' + b' --version) echo "gh version 2.50.0 (stub)" ;;\n' + b" auth) exit 0 ;;\n" + b" api) echo tester ;;\n" + b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' + b"esac\n") + gh.chmod(0o755) + return {"PATH": str(stub) + os.pathsep + os.environ["PATH"]} + + +def flags_for(tmp_path, local): + """The flags of a run asked for its project: `--` and `--dry-run` last, + so a name that goes on ends at the plan and creates nothing.""" + return ["--org", "demoorg", "--elected-by", "Tester", "--into", + str(tmp_path / "parent" / "into"), + *(["--local-remote-dir", str(tmp_path / "remotes")] + if local else []), "--", "--dry-run"] + + +def at_the_prompt(tmp_path, typed, local=True): + """Setup asked for its project at a terminal and answered `typed`.""" + env = terminal_env() + env.update(gh_that_is_signed_in(tmp_path)) + status, screen, answered = run_in_a_terminal( + [sys.executable, str(SETUP_PROJECT), *flags_for(tmp_path, local)], + cwd=REPO, env=env, + answers=[typed.replace(BELL, LNEXT + BELL) + "\n"], prompt=PROMPT, + timeout=60) + assert answered == 1, f"it never asked for a project name:\n{screen}" + return status, screen + + +def assert_nothing_was_made(tmp_path): + for name in ("remotes", "parent"): + assert not (tmp_path / name).exists(), name + + +@PTY_SKIP +@pytest.mark.parametrize("case", TYPED) +def test_a_typed_name_is_refused_with_the_check_it_failed(tmp_path, case): + value, why = TYPED[case] + status, screen = at_the_prompt(tmp_path, value) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: " + FIRST.format(what="project name", shown=ascii(value)) + + why) + if why != DASH: + for words in NOT_THE_DASH: + assert words not in screen, screen + assert "[ok] project" not in screen, screen + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) + + +# --- the bound --------------------------------------------------------------- + +#: GitHub's documented limit on a repository name, less `-spec` and `-code`. +BOUND = 95 + + +def a_name(length): + return "A" + "b" * (length - 1) + + +def too_long(what, name): + return ("REFUSED: %s is %d characters long (%s...%s), and a project is " + "three repositories, ``, `-spec` and " + "`-code`: GitHub keeps a repository name to at most " + "100 characters, so a project name is at most 95. Nothing was " + "created. Use a shorter name." + % (what, len(name), name[:12], name[-12:])) + + +def a_run_for(tmp_path, project, local, positional=False, late_local=False): + """The project given on the command line, and `-- --dry-run`.""" + flags = flags_for(tmp_path, local) + if late_local: + flags = [f for f in flags_for(tmp_path, False)] + flags[flags.index("--"):flags.index("--")] = [ + "--local-remote-dir", str(tmp_path / "remotes")] + given = [project] if positional else ["--project", project] + return run_entry(*given, *flags, extra_env=gh_that_is_signed_in(tmp_path)) + + +@PTY_SKIP +@pytest.mark.parametrize("how", ["flag", "positional"]) +@pytest.mark.parametrize("length", [BOUND + 1, 100, 101, 255]) +def test_a_name_over_the_bound_is_refused_before_anything_prints( + tmp_path, length, how): + name = a_name(length) + result = a_run_for(tmp_path, name, local=False, + positional=how == "positional") + assert result.returncode == 2, result.stderr + assert the_refusal(result.stderr) == too_long( + "" if how == "positional" else "--project", name) + assert result.stdout == "", result.stdout + assert_nothing_was_made(tmp_path) + + +@PTY_SKIP +def test_a_name_at_the_bound_goes_on_to_step_four(tmp_path): + result = a_run_for(tmp_path, a_name(BOUND), local=False) + assert NAMING_POLICY in result.stdout, result.stdout + result.stderr + assert "characters long" not in result.stderr, result.stderr + + +@PTY_SKIP +@pytest.mark.parametrize("length", [BOUND + 1, 100, 101, 246]) +def test_a_name_over_the_bound_is_not_asked_of_a_local_run(tmp_path, length): + """No GitHub repository is made under `--local-remote-dir`, so the limit + is the disk's, and step (5) still asks it: 247 characters and more end + there as `scaffold-name-too-long`.""" + result = a_run_for(tmp_path, a_name(length), local=True) + assert NAMING_POLICY in result.stdout, result.stdout + result.stderr + assert "characters long" not in result.stderr, result.stderr + assert result.returncode == 0, result.stderr + + +@PTY_SKIP +def test_the_disk_limit_is_still_step_fives_under_a_local_run(tmp_path): + result = a_run_for(tmp_path, a_name(247), local=True) + assert result.returncode == 2, result.stderr + assert "scaffold-name-too-long" in result.stderr, result.stderr + assert "characters long" not in result.stderr, result.stderr + + +@PTY_SKIP +def test_the_bound_waits_for_a_local_remote_dir_that_comes_later(tmp_path): + """`--local-remote-dir` decides whether a GitHub repository will be made, + and it can come after `--project` on the line.""" + result = a_run_for(tmp_path, a_name(BOUND + 1), local=False, + late_local=True) + assert NAMING_POLICY in result.stdout, result.stdout + result.stderr + assert "characters long" not in result.stderr, result.stderr + + +@PTY_SKIP +@pytest.mark.parametrize("length", [BOUND + 1, 100, 101, 255]) +def test_a_typed_name_over_the_bound_is_refused_at_the_prompt(tmp_path, + length): + name = a_name(length) + status, screen = at_the_prompt(tmp_path, name, local=False) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == too_long("project name", name) + assert "[ok] project" not in screen, screen + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) + + +@PTY_SKIP +@pytest.mark.parametrize("local", [False, True], ids=["github", "local"]) +def test_a_typed_name_at_the_bound_goes_on_to_step_four(tmp_path, local): + status, screen = at_the_prompt(tmp_path, a_name(BOUND), local=local) + assert "[ok] project" in screen, screen + assert NAMING_POLICY in screen, screen + assert "characters long" not in screen, screen + + +@PTY_SKIP +def test_a_typed_name_over_the_bound_goes_on_in_a_local_run(tmp_path): + status, screen = at_the_prompt(tmp_path, a_name(BOUND + 1), local=True) + assert status == 0, f"status {status}:\n{screen}" + assert NAMING_POLICY in screen, screen + assert "characters long" not in screen, screen + + +# --- the spaces at the ends -------------------------------------------------- + +@PTY_SKIP +@pytest.mark.parametrize("typed", ["Atlas ", " Atlas", " Atlas "]) +def test_the_spaces_at_the_ends_of_a_typed_name_are_taken_off(tmp_path, typed): + status, screen = at_the_prompt(tmp_path, typed) + assert status == 0, f"status {status}:\n{screen}" + assert "\n [ok] project Atlas\n" in screen, screen + assert "\n [ok] legs Atlas-spec, Atlas-code\n" in screen, screen + assert "NO FAMILY" not in screen, screen + assert "naming-unclassified" not in screen, screen + assert f"clone into {tmp_path / 'parent' / 'into' / 'Atlas'}\n" in screen + + +@PTY_SKIP +def test_a_line_of_spaces_is_no_name_at_all(tmp_path): + status, screen = at_the_prompt(tmp_path, " ") + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: no project name was typed; nothing was created.") + assert "[ok] project" not in screen, screen + assert_nothing_was_made(tmp_path) + + +def test_an_argv_name_is_held_as_typed(tmp_path): + """The other half of the choice: an argv value is deliberate, so a + trailing space on `--project` reaches step (4) as it was given.""" + result = run_entry("--project", "Atlas ", "--org", "demoorg", + "--elected-by", "Tester", "--into", + str(tmp_path / "work"), "--local-remote-dir", + str(tmp_path / "remotes")) + assert result.returncode == 1, result.stderr + assert "FINDING naming-unclassified: Atlas , Atlas -spec, Atlas -code" \ + in result.stderr, result.stderr + + +# --- the sentences say what the checks are ------------------------------------- + +def test_the_name_sentence_says_what_the_pattern_holds(): + """The words and the pattern cannot drift: every printable character is + in the sentence's list exactly when `NAME_RE` takes it after a letter.""" + module = entry_point_module() + said = set(string.ascii_letters + string.digits + " ._'()&,+@-") + for char in string.printable: + if char in "\r\n\x0b\x0c": + continue + assert bool(module.NAME_RE.fullmatch("A" + char)) == (char in said), \ + repr(char) + assert module.NAME_RE.fullmatch("A" * 255) + assert not module.NAME_RE.fullmatch("A" * 256) + for char in string.digits + string.ascii_letters: + assert module.NAME_RE.fullmatch(char + "x") + assert not module.NAME_RE.fullmatch(" x") + + +def test_the_ref_sentence_says_what_the_pattern_holds(): + module = entry_point_module() + said = set(string.ascii_letters + string.digits + "._/-") + for char in string.printable: + if char in "\r\n\x0b\x0c": + continue + assert bool(module.REF_RE.fullmatch("a" + char)) == (char in said), \ + repr(char) + assert module.REF_RE.fullmatch("a" * 255) + assert not module.REF_RE.fullmatch("a" * 256) + + +def test_a_pattern_with_no_words_is_named_as_the_sibling_names_it(): + """`PATH_RE` has no row in `WHY_PATTERN`: its only failures are an empty + value and a control character, named first. A pattern that did get here + is still named, not a KeyError.""" + module = entry_point_module() + for text in ("", "a" + BELL + "b", "\n"): + assert not module.PATH_RE.fullmatch(text) + assert module.why_refused(text, module.PATH_RE) in ( + EMPTY, CONTROL), repr(text) + with pytest.raises(module.Refusal) as refused: + module.checked_value("--x", "ab", re.compile("^z$")) + assert str(refused.value).endswith(". It does not match `^z$`.") diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index 535aa995..dcb8c822 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -784,10 +784,12 @@ def test_a_flag_with_no_value_refuses(): # ARGUMENT injection, not shell injection: every command this entry point runs # is a list with `shell=False`, so there is no shell - but git reads its own # arguments, and a value that starts with `-` is an option to git rather than -# a name. `scaffold-project.py` refuses the same shape in the same words -# (`tests/test_scaffold_pin_and_reuse.py`), through `repo_shape.checked_value`; -# this file's subject carries its own copy because it runs BEFORE there is a -# checkout to import that from, which is the whole of self-bootstrap mode. +# a name. `scaffold-project.py` refuses the same values +# (`tests/test_scaffold_pin_and_reuse.py`), through `repo_shape.checked_value`, +# each in a sentence of its own: this file's subject names the check a value +# failed (`tests/test_setup_project_name_edges.py`). It carries its own copy +# because it runs BEFORE there is a checkout to import that from, which is the +# whole of self-bootstrap mode. # # Every run here passes `--local-remote-dir` even though none of them reaches # it: the refusal is a parse-time one, and the flag is what keeps diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py index d27eeb68..cac6b3b8 100644 --- a/tests/test_setup_project_typed_name.py +++ b/tests/test_setup_project_typed_name.py @@ -44,7 +44,9 @@ BELL = chr(7) #: Names the check refuses, by case. `absolute` is a path under `tmp_path`, -#: so it is built by `typed_value`. +#: so it is built by `typed_value`. A line of spaces is not here: the prompt +#: takes the spaces off the ends of what was typed (#777), so it is an empty +#: line, and `--project ' '` is refused by the pattern. REFUSED = { "up-and-out": "../x", "slash": "a/b", @@ -54,7 +56,6 @@ "help": "--help", "control": "a" + BELL + "b", "long": "x" * 256, - "blank": " ", "absolute": None, } @@ -140,7 +141,7 @@ def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): @PTY_SKIP @pytest.mark.parametrize("entry", ["py", "piped", "sh"]) -@pytest.mark.parametrize("typed", ["", "EOF"]) +@pytest.mark.parametrize("typed", ["", "EOF", " "]) def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal( tmp_path, typed, entry): status, screen = at_the_prompt(tmp_path, typed, entry, seeded=True) From b5def3f0562f0fb18820134151f7e972a6bdbf1f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 16:12:40 +0000 Subject: [PATCH 4/8] tests/test_setup_project_name_edges.py: the typed runs give git a credential helper for github.com, and one run empties git's own config, so a GitHub run no longer waits on CI at a `gh auth setup-git` offer nothing answers (#777) On CI (run 39027235147, git 2.55.0) five of this file's tests were killed after 60 seconds: the four over-the-bound names typed at the prompt (96, 100, 101, 255) and the 95-character name typed in a GitHub run. Each screen ended in step (1): [??] git has no credential helper for github.com, so the first push over HTTPS will fail. Fix it once: gh auth setup-git Run `gh auth setup-git` now? Type yes to continue: A GitHub run (no `--local-remote-dir`) makes that offer in its preflight when git has no credential helper for github.com, before step (3) asks for the project. The tests type only at the project prompt, so nothing answered the offer, and the run waited. The bound played no part: the same names in local runs passed, and the 95-character GitHub run hung too. On the writer's machine /etc/gitconfig and ~/.gitconfig both name a helper, so the offer never came there. CI's runner names none. With git's system file skipped and its global file empty, the same five hang on that machine with CI's screen. The tool is right: the offer is `_check_credential_helper`'s documented warning and offer. The tests depended on the machine's git config. - `gh_that_is_signed_in` also gives git a credential helper for github.com, in git's command scope (`GIT_CONFIG_COUNT`, appended to what the environment holds): `! auth git-credential`, the line `gh auth setup-git` writes, pointed at the stub. The preflight finds a helper on every machine. No run here pushes, so git never asks it. - A new test, `test_a_github_run_here_is_not_held_by_a_git_with_no_helper` (95 and 96 characters), runs with git's system and global files empty, as CI's runner has them. A run that waits at the offer now fails on every machine, not on CI alone. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- tests/test_setup_project_name_edges.py | 38 ++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py index d7f53a4d..a2e2282c 100644 --- a/tests/test_setup_project_name_edges.py +++ b/tests/test_setup_project_name_edges.py @@ -29,6 +29,7 @@ import os import re +import shlex import string import sys @@ -143,7 +144,16 @@ def test_a_refusal_names_the_check_the_value_failed(tmp_path, case): def gh_that_is_signed_in(tmp_path): """The environment of a run that is allowed to call `gh` and gets the three answers a preflight and step (3) read, and refuses the rest, so no - run here reaches a real GitHub (the real `gh` stays unreachable).""" + run here reaches a real GitHub (the real `gh` stays unreachable). + + Its git has a credential helper for github.com: the line `gh auth + setup-git` writes, pointed at this stub, in git's command scope + (`GIT_CONFIG_COUNT`, appended to what the environment holds). Without + it, the machine's own git config decided whether a GitHub run's + preflight offered `gh auth setup-git`. A machine with a helper anywhere + was never offered it; CI's runner has none, and its runs waited at that + offer's `Type yes to continue:`, which nothing here answers, until they + were killed (#777's first CI run).""" stub = tmp_path / "ghbin" stub.mkdir(exist_ok=True) gh = stub / "gh" @@ -155,7 +165,12 @@ def gh_that_is_signed_in(tmp_path): b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' b"esac\n") gh.chmod(0o755) - return {"PATH": str(stub) + os.pathsep + os.environ["PATH"]} + held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) + return {"PATH": str(stub) + os.pathsep + os.environ["PATH"], + "GIT_CONFIG_COUNT": str(held + 1), + f"GIT_CONFIG_KEY_{held}": "credential.https://github.com.helper", + f"GIT_CONFIG_VALUE_{held}": + "!%s auth git-credential" % shlex.quote(str(gh))} def flags_for(tmp_path, local): @@ -306,6 +321,25 @@ def test_a_typed_name_at_the_bound_goes_on_to_step_four(tmp_path, local): assert "characters long" not in screen, screen +@PTY_SKIP +@pytest.mark.parametrize("length", [BOUND, BOUND + 1]) +def test_a_github_run_here_is_not_held_by_a_git_with_no_helper( + tmp_path, monkeypatch, length): + """A GitHub run's preflight offers `gh auth setup-git` to a machine whose + git has no credential helper for github.com, before step (3) asks for the + project. CI's runner is such a machine, and the typed runs above waited + there on it. Here git's system and global files are empty, as the + runner's are, so a run that would wait at that offer fails on every + machine, not on CI alone.""" + blank = tmp_path / "blank.gitconfig" + blank.write_bytes(b"") + for scope in ("GIT_CONFIG_SYSTEM", "GIT_CONFIG_GLOBAL"): + monkeypatch.setenv(scope, str(blank)) + _, screen = at_the_prompt(tmp_path, a_name(length), local=False) + assert "gh auth setup-git" not in screen, screen + assert ("characters long" in screen) == (length > BOUND), screen + + @PTY_SKIP def test_a_typed_name_over_the_bound_goes_on_in_a_local_run(tmp_path): status, screen = at_the_prompt(tmp_path, a_name(BOUND + 1), local=True) From 050abea6cc7e4fa25b8e3d83dc3b614b9b1bd730 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 16:28:58 +0000 Subject: [PATCH 5/8] setup-project.py: the prompt takes only the space off a typed name, and the name and ref sentences say ASCII, so a typed tab, form feed or no-break space is refused as `--project` refuses it, and `Caf\xe9` is no longer described as spellable (#777) Lane 2's review of #811 (rv811, at 854f6ed9), SHOULDs 2 and 3 and NITs 4 and 5. Its MUST 1 is b5def3f0. - The prompt's `.strip()` took off every kind of whitespace, not only the spaces its docstring names: a tab, VT, FF, `\x1c` to `\x1f`, NEL, NBSP and U+3000. Typed, each gave `[ok] project Atlas` and exit 0, where `--project` refuses the same value as a control character or by the name pattern, and where #756's head refused it at the prompt too. It is now `.strip(" ")`. A typed line holds no newline here (`ask` drops it), so nothing else needs taking off. The test file's typed-refusal test takes the nine values, with stdin the terminal and with stdin a pipe, and the argv refusals take them as `--project`. The same reason is given on every way in. Each control character is typed after ^V, so the terminal takes it as text (`\x1c` alone is the quit key). - The name sentence said "letters" and "spaces", but `NAME_RE` takes only ASCII letters and U+0020, so its words described `Caf\xe9` and a no-break space as values that pass. Both sentences now say "an ASCII letter or digit first, then ASCII letters, digits, the space and ..." (the ref sentence without the space), as `adopt-project.py`'s help says "ASCII letters, digits and ...". The drift guards also walk every character above ASCII and find none taken. README, the handbook, AGENTS.md and docs/cli.md do not state either sentence (`render-cli-reference.py --check` is ok). - A ref is refused for `.lock` only where it ends in it: a new test passes `topic.locked` and `a.lockb` through `checked_ref`. - `a_run_for`'s list comprehension that only copied is gone. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- setup-project.py | 27 +++++--- tests/test_setup_project_name_edges.py | 88 +++++++++++++++++++++----- 2 files changed, 90 insertions(+), 25 deletions(-) diff --git a/setup-project.py b/setup-project.py index dc95a73c..f1052b8d 100755 --- a/setup-project.py +++ b/setup-project.py @@ -382,14 +382,17 @@ def program_version(argv, field: int) -> str: #: sibling `repo_shape.checked_value` prints its own. `PATH_RE` has no row: it #: refuses only an empty value and a control character, and both are named #: before a pattern is asked; a pattern with no row is still named, as the -#: sibling names it, by `why_refused`. +#: sibling names it, by `why_refused`. The words say ASCII because the +#: patterns are: "letters" and "spaces" alone describe `Caf\xe9` and a +#: no-break space as values that pass (rv811). WHY_PATTERN = { - NAME_RE: ("It is not spellable as a name: a letter or a digit first, " - "then letters, digits, spaces and `._'()&,+@-`, at most 255 " - "characters in all (`%s`)." % NAME_RE.pattern), - REF_RE: ("It is not spellable as a ref: a letter or a digit first, then " - "letters, digits and `._/-`, at most 255 characters in all " - "(`%s`)." % REF_RE.pattern), + NAME_RE: ("It is not spellable as a name: an ASCII letter or digit " + "first, then ASCII letters, digits, the space and " + "`._'()&,+@-`, at most 255 characters in all (`%s`)." + % NAME_RE.pattern), + REF_RE: ("It is not spellable as a ref: an ASCII letter or digit first, " + "then ASCII letters, digits and `._/-`, at most 255 characters " + "in all (`%s`)." % REF_RE.pattern), } @@ -1586,8 +1589,16 @@ def ask_for_the_project_name(local_mode: bool) -> str: slip of the thumb, and the run ends at the first mistake with no second ask, so the prompt takes it off; an argv value is deliberate and is held as typed. A line of nothing but spaces is then an empty line. + + ONLY THE SPACE COMES OFF, NOT EVERY KIND OF WHITESPACE. `str.strip()` + with no argument also took off a tab, a form feed, `\\x1c` to `\\x1f`, a + no-break space and the ideographic space, so the prompt accepted each of + them where `--project` refuses it, as a control character or by the + pattern (rv811). Now only the space comes off, and the rest reach + `checked_value` as they were typed. """ - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ").strip() + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = typed.strip(" ") if not typed: die("no project name was typed; nothing was created.") return checked_project_length( diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py index a2e2282c..a43ff981 100644 --- a/tests/test_setup_project_name_edges.py +++ b/tests/test_setup_project_name_edges.py @@ -20,7 +20,8 @@ * `Atlas ` passed `NAME_RE` where ` Atlas` was refused, and step (4) judged `Atlas -spec` with no space shown anywhere. The prompt now takes the spaces off the ends of what was typed; an argv value is deliberate and is held as - typed. + typed. Only the space comes off: a tab, a no-break space or any other + whitespace is refused at the prompt as `--project` refuses it. POSIX only where a run needs a terminal or a `gh` stub (`PTY_SKIP`). """ @@ -37,7 +38,7 @@ from conftest import PTY_SKIP, REPO, run_in_a_terminal from test_setup_project_py import (SETUP_PROJECT, entry_point_module, - run_entry, terminal_env) + piped, run_entry, terminal_env) from test_setup_project_typed_name import (BELL, LNEXT, NAMING_POLICY, PROMPT, the_refusal) @@ -52,11 +53,11 @@ "escape: a newline ends a line in every terminal and log this " "output reaches, so a value carrying one could print a line that " "reads as this tool's own.") -NAME = ("It is not spellable as a name: a letter or a digit first, then " - "letters, digits, spaces and `._'()&,+@-`, at most 255 characters in " - "all (`^[A-Za-z0-9][A-Za-z0-9 ._'()&,+@-]{0,254}$`).") -REF = ("It is not spellable as a ref: a letter or a digit first, then " - "letters, digits and `._/-`, at most 255 characters in all " +NAME = ("It is not spellable as a name: an ASCII letter or digit first, " + "then ASCII letters, digits, the space and `._'()&,+@-`, at most 255 " + "characters in all (`^[A-Za-z0-9][A-Za-z0-9 ._'()&,+@-]{0,254}$`).") +REF = ("It is not spellable as a ref: an ASCII letter or digit first, then " + "ASCII letters, digits and `._/-`, at most 255 characters in all " "(`^[A-Za-z0-9][A-Za-z0-9._/-]{0,254}$`).") RANGE = ("It holds `..`, which git reads as a revision range and not as a " "ref.") @@ -68,6 +69,22 @@ ACCENTED = "Caf\u00e9" +#: Whitespace that is not the space. `str.strip()` took each off a typed name +#: where `--project` refuses the same value (rv811), as a control character +#: or by the name pattern. Only the space comes off at the prompt, so each is +#: refused on every way in, for the same reason. +NOT_THE_SPACE = { + "tab-after": ("Atlas\t", CONTROL), + "tab-before": ("\tAtlas", CONTROL), + "vertical-tab": ("Atlas\x0b", CONTROL), + "form-feed": ("Atlas\x0c", CONTROL), + "file-separator": ("\x1cAtlas", CONTROL), + "unit-separator": ("\x1fAtlas", CONTROL), + "no-break-space": ("Atlas\xa0", NAME), + "next-line": ("Atlas\x85", NAME), + "ideographic-space": ("Atlas\u3000", NAME), +} + #: Each refusal: the argv that carries the value (`{}` is the value), the #: value, how the refusal names it, and the reason it gives. A value that #: fails two checks is told the first, in the order empty, `-`, control @@ -100,6 +117,8 @@ "ref-dash": (("--shape-ref", "{}"), "-x", "--shape-ref", DASH), "ref-range": (("--shape-ref", "{}"), "a..b", "--shape-ref", RANGE), "ref-lock": (("--shape-ref", "{}"), "topic.lock", "--shape-ref", LOCK), + **{"project-" + case: (("--project", "{}"), value, "--project", why) + for case, (value, why) in NOT_THE_SPACE.items()}, } @@ -138,6 +157,7 @@ def test_a_refusal_names_the_check_the_value_failed(tmp_path, case): "trailing-slash": ("Atlas/", NAME), "accented": (ACCENTED, NAME), "long": ("x" * 256, NAME), + **NOT_THE_SPACE, } @@ -182,15 +202,26 @@ def flags_for(tmp_path, local): if local else []), "--", "--dry-run"] -def at_the_prompt(tmp_path, typed, local=True): - """Setup asked for its project at a terminal and answered `typed`.""" +def typed_as_text(typed): + """`typed` as keys at a terminal in canonical mode: ^V before each + control character, so the terminal takes it as text and not as an edit + or a signal (the file separator alone is the quit key, Ctrl-\\).""" + return "".join(LNEXT + char if ord(char) < 0x20 or char == "\x7f" + else char for char in typed) + + +def at_the_prompt(tmp_path, typed, local=True, entry="py"): + """Setup asked for its project at a terminal and answered `typed`. + `entry` is the file run directly, or that file with its stdin a pipe, + when the answer is read from the controlling terminal.""" env = terminal_env() env.update(gh_that_is_signed_in(tmp_path)) + flags = flags_for(tmp_path, local) + argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], + "piped": piped("", *flags)}[entry] status, screen, answered = run_in_a_terminal( - [sys.executable, str(SETUP_PROJECT), *flags_for(tmp_path, local)], - cwd=REPO, env=env, - answers=[typed.replace(BELL, LNEXT + BELL) + "\n"], prompt=PROMPT, - timeout=60) + argv, cwd=REPO, env=env, answers=[typed_as_text(typed) + "\n"], + prompt=PROMPT, timeout=60) assert answered == 1, f"it never asked for a project name:\n{screen}" return status, screen @@ -201,10 +232,12 @@ def assert_nothing_was_made(tmp_path): @PTY_SKIP +@pytest.mark.parametrize("entry", ["py", "piped"]) @pytest.mark.parametrize("case", TYPED) -def test_a_typed_name_is_refused_with_the_check_it_failed(tmp_path, case): +def test_a_typed_name_is_refused_with_the_check_it_failed(tmp_path, case, + entry): value, why = TYPED[case] - status, screen = at_the_prompt(tmp_path, value) + status, screen = at_the_prompt(tmp_path, value, entry=entry) assert status == 2, f"status {status}:\n{screen}" assert the_refusal(screen) == ( "REFUSED: " + FIRST.format(what="project name", shown=ascii(value)) @@ -240,7 +273,7 @@ def a_run_for(tmp_path, project, local, positional=False, late_local=False): """The project given on the command line, and `-- --dry-run`.""" flags = flags_for(tmp_path, local) if late_local: - flags = [f for f in flags_for(tmp_path, False)] + flags = flags_for(tmp_path, False) flags[flags.index("--"):flags.index("--")] = [ "--local-remote-dir", str(tmp_path / "remotes")] given = [project] if positional else ["--project", project] @@ -386,10 +419,21 @@ def test_an_argv_name_is_held_as_typed(tmp_path): # --- the sentences say what the checks are ------------------------------------- +def above_ascii_taken(pattern, letter): + """Every character above ASCII that `pattern` takes first or after + `letter`: none, as the sentences' "ASCII" says. A walk over + `string.printable` alone cannot see an accented letter (rv811).""" + return [hex(code) for code in range(0x80, sys.maxunicode + 1) + if pattern.fullmatch(letter + chr(code)) + or pattern.fullmatch(chr(code) + letter)] + + def test_the_name_sentence_says_what_the_pattern_holds(): """The words and the pattern cannot drift: every printable character is - in the sentence's list exactly when `NAME_RE` takes it after a letter.""" + in the sentence's list exactly when `NAME_RE` takes it after a letter, + and no character above ASCII is taken anywhere.""" module = entry_point_module() + assert above_ascii_taken(module.NAME_RE, "A") == [] said = set(string.ascii_letters + string.digits + " ._'()&,+@-") for char in string.printable: if char in "\r\n\x0b\x0c": @@ -405,6 +449,7 @@ def test_the_name_sentence_says_what_the_pattern_holds(): def test_the_ref_sentence_says_what_the_pattern_holds(): module = entry_point_module() + assert above_ascii_taken(module.REF_RE, "a") == [] said = set(string.ascii_letters + string.digits + "._/-") for char in string.printable: if char in "\r\n\x0b\x0c": @@ -415,6 +460,15 @@ def test_the_ref_sentence_says_what_the_pattern_holds(): assert not module.REF_RE.fullmatch("a" * 256) +def test_a_ref_is_refused_for_lock_only_at_its_end(): + """A ref that ends in `.lock` is refused; `topic.locked` and `a.lockb` + hold it inside a word, are refs git checks out (`git check-ref-format + --branch` takes both), and go on.""" + module = entry_point_module() + for text in ("topic.locked", "a.lockb"): + assert module.checked_ref("--shape-ref", text) == text + + def test_a_pattern_with_no_words_is_named_as_the_sibling_names_it(): """`PATH_RE` has no row in `WHY_PATTERN`: its only failures are an empty value and a control character, named first. A pattern that did get here From b6f56cad59315f3581b76d25d319a8ddd36f2cf4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 02:50:08 +0000 Subject: [PATCH 6/8] tests/test_setup_project_name_edges.py: the credential helper's entry is appended through `appended_to_command_scope`, as #848 asks of every hand-worked count, so the scan in `test_no_auto_maintenance.py` passes again (#777) `gh_that_is_signed_in` worked its `GIT_CONFIG_COUNT` out by hand from the environment's own count (`held + 1`, `GIT_CONFIG_KEY_{held}` and `GIT_CONFIG_VALUE_{held}`). That sum was right, and #848's scan (`test_no_test_sets_a_git_config_count_of_its_own`) refuses it in a test file now: it named `test_setup_project_name_edges.py:190`. #848 landed first, and its second-lander note gives this conversion to whichever of #848 and #811 lands second. The three entries now come from `appended_to_command_scope(os.environ, ...)`, merged into the returned dict beside `PATH`. The same three variables get the same values, under an environment that holds two entries, none, or an empty count. The docstring names the helper. No other line of the file changes. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_setup_project_name_edges.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_setup_project_name_edges.py b/tests/test_setup_project_name_edges.py index a43ff981..1f4aa53f 100644 --- a/tests/test_setup_project_name_edges.py +++ b/tests/test_setup_project_name_edges.py @@ -36,7 +36,8 @@ import pytest -from conftest import PTY_SKIP, REPO, run_in_a_terminal +from conftest import (PTY_SKIP, REPO, appended_to_command_scope, + run_in_a_terminal) from test_setup_project_py import (SETUP_PROJECT, entry_point_module, piped, run_entry, terminal_env) from test_setup_project_typed_name import (BELL, LNEXT, NAMING_POLICY, PROMPT, @@ -168,12 +169,12 @@ def gh_that_is_signed_in(tmp_path): Its git has a credential helper for github.com: the line `gh auth setup-git` writes, pointed at this stub, in git's command scope - (`GIT_CONFIG_COUNT`, appended to what the environment holds). Without - it, the machine's own git config decided whether a GitHub run's - preflight offered `gh auth setup-git`. A machine with a helper anywhere - was never offered it; CI's runner has none, and its runs waited at that - offer's `Type yes to continue:`, which nothing here answers, until they - were killed (#777's first CI run).""" + (`GIT_CONFIG_COUNT`, appended to what the environment holds, by + `appended_to_command_scope`). Without it, the machine's own git config + decided whether a GitHub run's preflight offered `gh auth setup-git`. A + machine with a helper anywhere was never offered it; CI's runner has + none, and its runs waited at that offer's `Type yes to continue:`, which + nothing here answers, until they were killed (#777's first CI run).""" stub = tmp_path / "ghbin" stub.mkdir(exist_ok=True) gh = stub / "gh" @@ -185,12 +186,11 @@ def gh_that_is_signed_in(tmp_path): b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' b"esac\n") gh.chmod(0o755) - held = int(os.environ.get("GIT_CONFIG_COUNT") or 0) + helper = "!%s auth git-credential" % shlex.quote(str(gh)) return {"PATH": str(stub) + os.pathsep + os.environ["PATH"], - "GIT_CONFIG_COUNT": str(held + 1), - f"GIT_CONFIG_KEY_{held}": "credential.https://github.com.helper", - f"GIT_CONFIG_VALUE_{held}": - "!%s auth git-credential" % shlex.quote(str(gh))} + **appended_to_command_scope( + os.environ, + [("credential.https://github.com.helper", helper)])} def flags_for(tmp_path, local): From 79a33f112dda262832cce4867f84414e33f7b2b7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 02:53:19 +0000 Subject: [PATCH 7/8] setup-project.py: a passed-through `--dry-run` makes none of the preflight's offers and `--shape-ref` is held to git's rule part by part, so a rehearsal no longer writes git's global config and `a.lock/b` is refused before the clone (#857) A `--dry-run` passed through after `--` ends the run at the plan, and the help and the plan's last line say nothing was created. The preflight before it still offered, on a terminal, every command that fixes what it found missing: an install of git or gh, `gh auth login` and `gh auth setup-git`. Each changes the machine on a typed `yes` (software, a gh login, a helper in the person's global git config), and a rehearsal that typed only the project name waited at the question until it was killed, as five of #777's tests did on CI. `preflight` now passes `offering` False down to `offer` for such a run, and `offer` says `not offered: --dry-run was passed through. A run without it offers this here, and so does --preflight.` where it would have asked, and asks nothing. Every check still runs and reports what is missing. The plan's last line adds that the preflight made none of its offers. `--preflight` still makes every offer, whatever follows `--`: it is the run that exists for them (#59). The help says so, and docs/cli.md is regenerated. `checked_ref` asked only `..` and a `.lock` at the very end, where git's rule is per part: `why_not_a_ref` refuses an empty part (`//`, a trailing `/`), a part that starts with `.`, a part that ends in `.lock`, and a `.` at the end, each in a sentence of its own. `a.lock/b`, `x/y.lock/z`, `release.lock/v1`, `a/.b`, `a//b`, `a/b/`, `a.lock/` and `a.` are refused before the standard is cloned; `topic.locked` and `a./b` go on. Over the 3110 names of up to five parts from {a, 0, ., /, -, .lock} that `REF_RE` takes, the tool and `git check-ref-format --branch` now give one answer, under git 2.43.0 and 2.47.1 (1122 disagreed before). New tests: tests/test_setup_project_dry_run_offers.py (each offer in process; a terminal run under a stub gh that leaves the global git config's bytes as they were, and one that leaves gh signed out) and tests/test_setup_project_ref_parts.py (the issue's ten names and nine more, each beside git, and three refused before a self-bootstrap clone). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- docs/cli.md | 8 +- setup-project.py | 149 ++++++++++---- tests/test_setup_project_dry_run_offers.py | 219 +++++++++++++++++++++ tests/test_setup_project_ref_parts.py | 99 ++++++++++ 4 files changed, 439 insertions(+), 36 deletions(-) create mode 100644 tests/test_setup_project_dry_run_offers.py create mode 100644 tests/test_setup_project_ref_parts.py diff --git a/docs/cli.md b/docs/cli.md index 369c4781..7a90c702 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -113,7 +113,9 @@ Set $OPENREPOSHAPE_REPO to clone a fork or mirror in self-bootstrap mode instead of opensoft/openRepoShape. Anything after `--` is passed straight through to scaffold-project.py. A -`--dry-run` there ends this run at the plan, creating nothing. +`--dry-run` there ends this run at the plan, creating nothing, and makes +none of the preflight's offers: a run without it, or --preflight, makes +them. ``` ### `openRepoShape` @@ -248,7 +250,9 @@ Set $OPENREPOSHAPE_REPO to clone a fork or mirror in self-bootstrap mode instead of opensoft/openRepoShape. Anything after `--` is passed straight through to scaffold-project.py. A -`--dry-run` there ends this run at the plan, creating nothing. +`--dry-run` there ends this run at the plan, creating nothing, and makes +none of the preflight's offers: a run without it, or --preflight, makes +them. ``` ### `scaffold-project.py` diff --git a/setup-project.py b/setup-project.py index f1052b8d..07560bf4 100755 --- a/setup-project.py +++ b/setup-project.py @@ -177,7 +177,9 @@ instead of opensoft/openRepoShape. Anything after `--` is passed straight through to scaffold-project.py. A -`--dry-run` there ends this run at the plan, creating nothing. +`--dry-run` there ends this run at the plan, creating nothing, and makes +none of the preflight's offers: a run without it, or --preflight, makes +them. """ @@ -333,7 +335,8 @@ def program_version(argv, field: int) -> str: #: What a git REF may contain, conservatively: `--shape-ref` becomes #: `git checkout ` in the temporary checkout. `git check-ref-format` is -#: the authority; this is the subset of it that a person types. +#: the authority; this is the subset of its characters that a person types, +#: and `why_not_a_ref` asks the rest of its rule of a value that passes here. REF_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._/-]{0,254}$") #: What a PATH may contain: whatever the filesystem it names accepts - a drive @@ -377,6 +380,16 @@ def program_version(argv, field: int) -> str: "a ref.") WHY_LOCK = ("It ends in `.lock`, the name git gives the lock file it writes " "beside a ref.") +#: The rest of git's rule for a ref, which is asked of each part between +#: slashes and not of the whole alone (#857). +WHY_LOCK_PART = ("A part of it before a `/` ends in `.lock`, the name git " + "gives the lock file it writes beside a ref, and git takes " + "no ref with such a part.") +WHY_DOT_PART = ("A part of it after a `/` starts with `.`, and git takes no " + "ref with such a part.") +WHY_EMPTY_PART = ("It holds `//` or ends in `/`, and git takes no ref with an " + "empty part between slashes.") +WHY_DOT_END = "It ends in `.`, and git takes no ref that does." #: What each pattern asks, in words and then as the pattern itself, as the #: sibling `repo_shape.checked_value` prints its own. `PATH_RE` has no row: it @@ -474,19 +487,49 @@ def checked_project_length(what: str, name: str, local_mode: bool) -> str: PROJECT_NAME_MAX)) +def why_not_a_ref(text: str) -> str: + """The reason `git check-ref-format` refuses `text`, a value `REF_RE` has + passed, or "" when it takes it. + + GIT'S RULE IS PER PART (#857). Each part between slashes is there (no + `//`, no `/` at the end), does not start with `.` and does not end in + `.lock`, and the whole holds no `..` and does not end in `.`. Only `..` + and a `.lock` at the very end were asked before, so `a.lock/b`, `a/.b`, + `a//b`, `a/b/` and `a.` passed, and in self-bootstrap mode the person met + git's own `pathspec ... did not match` after the standard was cloned. + A `/` or a `.` as the first character is `REF_RE`'s to refuse. The checks + run in this order, so the sentence names the first one the value failed. + """ + parts = text.split("/") + if ".." in text: + return WHY_RANGE + if "" in parts: + return WHY_EMPTY_PART + if any(part.startswith(".") for part in parts): + return WHY_DOT_PART + if text.endswith(".lock"): + return WHY_LOCK + if any(part.endswith(".lock") for part in parts): + return WHY_LOCK_PART + if text.endswith("."): + return WHY_DOT_END + return "" + + def checked_ref(flag: str, value: str) -> str: """`--shape-ref`: narrower than a name, because git reads more into one. - `..` is a revision RANGE rather than a ref, and a ref ending in `.lock` is - the name git gives the lock file it writes beside one - neither is a thing - to check out, and both are refused here rather than discovered as a - confusing failure inside the temporary checkout. + `..` is a revision RANGE rather than a ref, a part ending in `.lock` is + the name git gives the lock file it writes beside one, and git takes no + ref with an empty part, a part that starts with `.`, or a `.` at its end + (`why_not_a_ref`) - none is a thing to check out, and each is refused + here rather than discovered as a confusing failure inside the temporary + checkout. """ text = checked_value(flag, value, REF_RE) - if ".." in text: - refuse_value(flag, text, WHY_RANGE) - if text.endswith(".lock"): - refuse_value(flag, text, WHY_LOCK) + why = why_not_a_ref(text) + if why: + refuse_value(flag, text, why) return text @@ -937,7 +980,12 @@ def run_command_text(text: str) -> int: return run_at_the_terminal(argv).returncode -def offer(command_text: str) -> bool: +#: What the terminal is told where a dry run makes no offer (#857). +NOT_OFFERED = (" not offered: --dry-run was passed through. A run without " + "it offers this here, and so does --preflight.") + + +def offer(command_text: str, offering: bool = True) -> bool: """Show one install command and take exactly one typed `yes` for it. THE SIGNATURE IS THE GUARANTEE, not the care of whoever edits this next: @@ -948,17 +996,27 @@ def offer(command_text: str) -> bool: AGENTS.md says the same thing to an assistant driving this: the human types this one. - NO TERMINAL, NO OFFER. `can_ask()` is the only gate, exactly as in + NO TERMINAL, NO OFFER. `can_ask()` is the gate, exactly as in `plan_and_confirm`: the person's terminal, which is stdin or, when a pipe is, the controlling terminal (#492). A run with neither, scheduled or in an assistant's shell, prints what it printed before any of this existed and stops where it stopped. + A DRY RUN, NO OFFER EITHER (#857). `offering` is False in a run passed + `--dry-run` after `--` (`preflight`), which ends at the plan saying + nothing was created: a `yes` typed here would still have installed + software, signed gh in, or written the person's global git config + (`gh auth setup-git`). The terminal is told the offer was not made, and + asked nothing. A bool, never `Options`, for the reason above. + The `yes` is EXACT and the prompt is the one this file already asks: `y`, `Yes`, an empty line and EOF are all refusals. """ if not can_ask(): return False + if not offering: + say(NOT_OFFERED) + return False say("") if "\n" in command_text: # A BLOCK IS SHOWN BEFORE IT IS OFFERED. GitHub's apt instructions @@ -973,8 +1031,9 @@ def offer(command_text: str) -> bool: def offer_install(tool: str, command_text: str, probe, - platform: str = "") -> bool: + platform: str = "", offering: bool = True) -> bool: """One offer, one run, ONE re-check. True when the check now passes. + `offering` is `offer`'s (#857). Never a loop: a person told twice that the thing is still missing is watching this program guess, and the second guess is no better than the @@ -993,7 +1052,7 @@ def offer_install(tool: str, command_text: str, probe, the next one. """ platform = platform or sys.platform - if not offer(command_text): + if not offer(command_text, offering): return False if run_command_text(command_text) != 0: return False @@ -1015,19 +1074,20 @@ def offer_install(tool: str, command_text: str, probe, return False -def offer_missing(tool: str, probe) -> bool: +def offer_missing(tool: str, probe, offering: bool = True) -> bool: """The offer a failed check makes: look this platform's command up, and - make no offer at all when there is none.""" + make no offer at all when there is none. `offering` is `offer`'s.""" command = offer_for(tool, sys.platform) if command is None: return False - return offer_install(tool, command, probe, sys.platform) + return offer_install(tool, command, probe, sys.platform, offering) -def _check_git() -> bool: +def _check_git(offering: bool = True) -> bool: if not shutil.which("git"): bad(GIT_MISSING) - if not offer_missing("git", lambda: bool(shutil.which("git"))): + if not offer_missing("git", lambda: bool(shutil.which("git")), + offering): return False ok("git %s" % program_version(["git", "--version"], 2)) return True @@ -1080,7 +1140,7 @@ def _gh_login_suffix() -> str: return "" -def _check_gh(opts: Options) -> bool: +def _check_gh(opts: Options, offering: bool = True) -> bool: if opts.local_mode: ok("gh not required (--local-remote-dir: bare repositories on disk, " "no network)") @@ -1097,18 +1157,19 @@ def _check_gh(opts: Options) -> bool: # command that follows it, and that is where we stop. say(" Homebrew installs it with `brew install gh`, and is " "its own install: https://brew.sh") - if not offer_missing("gh", lambda: bool(shutil.which("gh"))): + if not offer_missing("gh", lambda: bool(shutil.which("gh")), + offering): return False ok("gh %s" % program_version(["gh", "--version"], 2)) if run(["gh", "auth", "status"], capture=True).returncode != 0: bad("gh is not authenticated. Run: gh auth login") - if not _offer_login(): + if not _offer_login(offering): return False ok("gh is authenticated%s" % _gh_login_suffix()) return True -def _offer_login() -> bool: +def _offer_login(offering: bool = True) -> bool: """`gh auth login`, offered and then run as a PLAIN ARGV. Never through a shell: it is a terminal UI that ends in a browser and @@ -1117,8 +1178,9 @@ def _offer_login() -> bool: what its prompts and its one-time code need, and what the gate on the offer has already guaranteed is there. The browser half is the human's; this starts the login and asks `gh auth status` once when gh comes back. + `offering` is `offer`'s (#857). """ - if not offer("gh auth login"): + if not offer("gh auth login", offering): return False if run_at_the_terminal(["gh", "auth", "login"]).returncode != 0: return False @@ -1144,7 +1206,7 @@ def _credential_helper_configured() -> bool: return False -def _check_credential_helper(opts: Options) -> None: +def _check_credential_helper(opts: Options, offering: bool = True) -> None: """WARN, DO NOT REFUSE - the class `_warn_autocrlf` belongs to. The scaffold clones and pushes over HTTPS (`clone_and_bootstrap`), so a @@ -1164,6 +1226,10 @@ def _check_credential_helper(opts: Options) -> None: `--preflight`: a warning that refuses is a refusal, and this one would refuse a machine whose remotes are SSH and whose HTTPS push this tool never makes. + + A DRY RUN STILL WARNS AND OFFERS NOTHING (#857): `offering` is `offer`'s. + A `yes` here makes gh write its helper into the person's global git + config, which is no part of a run that ends at the plan. """ if opts.local_mode: return @@ -1171,7 +1237,7 @@ def _check_credential_helper(opts: Options) -> None: return warn("git has no credential helper for github.com, so the first push " "over HTTPS will fail. Fix it once: gh auth setup-git") - if not offer("gh auth setup-git"): + if not offer("gh auth setup-git", offering): return if run_at_the_terminal(["gh", "auth", "setup-git"]).returncode != 0: return @@ -1208,13 +1274,14 @@ def _warn_autocrlf() -> None: "git config --global core.autocrlf false") -def preflight_checks(opts: Options) -> bool: +def preflight_checks(opts: Options, offering: bool = True) -> bool: """Every check, in order, and whether they all passed. SPLIT OUT OF `preflight` FOR `--preflight` (#59, renamed 2026-09-10), which runs exactly this and then reports instead of refusing. The scaffold path is unchanged: the refusal is still one refusal, still at the end, still naming everything - that was missing. + that was missing. `offering` False (`preflight`, #857) runs every check + and makes none of the offers. """ say("openRepoShape setup") say("") @@ -1223,20 +1290,29 @@ def preflight_checks(opts: Options) -> bool: # is told about git AND gh in one go rather than one missing thing per # re-run, which is what the original `failed = True` accumulator bought. # The list is complete before `all` reads it, so nothing short-circuits. - checked = [_check_git(), _check_python(), _check_bootstrap()] + checked = [_check_git(offering), _check_python(), _check_bootstrap()] # THE CREDENTIAL HELPER IS ASKED ABOUT ONLY WHEN gh IS READY. With no gh, # or no login, there is nothing to set up as a helper yet and the person # has already been told the thing that has to happen first. - gh_ready = _check_gh(opts) + gh_ready = _check_gh(opts, offering) checked.append(gh_ready) if gh_ready: - _check_credential_helper(opts) + _check_credential_helper(opts, offering) _warn_autocrlf() return all(checked) def preflight(opts: Options) -> None: - if not preflight_checks(opts): + """The scaffold run's preflight, refusing once if anything is missing. + + A `--dry-run` PASSED THROUGH AFTER `--` MAKES NONE OF ITS OFFERS (#857). + That run ends at the plan, and its help and its last line say nothing was + created, so it reports what is missing and asks nothing that would change + this machine. `--preflight` (`preflight_only`) makes every offer whatever + follows `--`: it is the run that exists for them (#59), and it returns + before anything that reads the passthrough. + """ + if not preflight_checks(opts, not passes_a_dry_run(opts.passthrough)): die(PREREQ_MISSING) @@ -2152,7 +2228,8 @@ def passes_a_dry_run(passthrough: list) -> bool: The scaffold then creates nothing in step (6), and step (7) cloned three repositories that did not exist and refused saying they WERE created. - `plan_and_confirm` ends the run at the plan instead. + `plan_and_confirm` ends the run at the plan instead, and `preflight` + makes none of its offers in it (#857). """ return any(names_flag(word, "--dry-run") for word in scaffold_words(passthrough)) @@ -2410,9 +2487,13 @@ def plan_and_confirm(opts: Options, shape_root: Path, org: str, say("clone into %s" % clone) if passes_a_dry_run(opts.passthrough): + # AND THE PREFLIGHT CHANGED NOTHING EITHER (#857): `preflight` made + # none of its offers, and named each one where it would have asked. say("") say("--dry-run was passed through to the scaffold, so this run ends " - "at the plan above: nothing was created and nothing was cloned.") + "at the plan above: nothing was created and nothing was cloned. " + "The preflight made none of its offers; a run without --dry-run " + "makes each one marked `not offered` above.") return False if opts.assume_yes: return True diff --git a/tests/test_setup_project_dry_run_offers.py b/tests/test_setup_project_dry_run_offers.py new file mode 100644 index 00000000..e47f3dd2 --- /dev/null +++ b/tests/test_setup_project_dry_run_offers.py @@ -0,0 +1,219 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A `--dry-run` passed through `setup-project.py` makes none of the +preflight's offers (#857). + +The help says a `--dry-run` after `--` "ends this run at the plan, creating +nothing", and the plan says "nothing was created". The preflight before it +still offered, on a terminal, each command that fixes what it found missing: +an install of git or gh, `gh auth login`, and `gh auth setup-git`. Every one +of them changes the machine on a typed `yes` - software installed, gh signed +in, a credential helper written into the person's global git config - in a +run that then said nothing was created. And a rehearsal that typed only the +project name waited at the question until it was killed, as five of #777's +tests did on CI. + +So a dry run reports what is missing and asks nothing that would change the +machine: where a terminal would have been offered a command, it is told the +offer was not made, and the plan's last line says so. `--preflight` still +makes every offer, whatever follows `--`: it is the run that exists for +them (#59). + +The terminal runs are POSIX only (`PTY_SKIP`). Their `gh` is a stub first on +`PATH` that answers what the run asks and writes what the real one would +write: a login state of its own, and the helper `gh auth setup-git` adds to +the file `GIT_CONFIG_GLOBAL` names. The real `gh` is never reached. The +in-process runs fake the terminal and record every command, running none. +""" + +from __future__ import annotations + +import os +import sys + +import pytest + +from conftest import DRY_RUN_ENDS_IT, PTY_SKIP, run_in_a_terminal +from test_setup_project_py import (EVERY_MANAGER, SETUP_PROJECT, _answers, + _Runs, _which, entry_point_module, piped, + terminal_env) +from test_setup_project_typed_name import the_refusal + +#: What the terminal is told where an offer was not made. +NOT_OFFERED = (" not offered: --dry-run was passed through. A run " + "without it offers this here, and so does --preflight.") + +#: The plan's last line in a dry run: what it always said, and then that the +#: preflight made no offer. +DRY_RUN_LAST_LINE = (DRY_RUN_ENDS_IT + " The preflight made none of its " + "offers; a run without --dry-run makes each one marked " + "`not offered` above.") + +PREREQ_MISSING = "one or more prerequisites are missing (see above)." + +HELPER_WARNING = ("[??] git has no credential helper for github.com, so the " + "first push over HTTPS will fail. Fix it once: gh auth " + "setup-git") + +NOT_SIGNED_IN = "[!!] gh is not authenticated. Run: gh auth login" + + +# --- in process: every offer, on every platform ------------------------------ + +#: A machine missing what each offer is for: the programs `shutil.which` +#: finds, what `run` answers, the offers a run without `--dry-run` makes, +#: and the refusal the preflight ends in ("" when it passes). +MACHINES = { + "install": (EVERY_MANAGER, {}, 2, PREREQ_MISSING), + "login": (("git", "gh"), {"auth status": (1, "")}, 1, PREREQ_MISSING), + "helper": (("git", "gh"), {"config --get-all": (1, "")}, 1, ""), +} + +#: The commands an offer runs on a `yes`: an install goes through a shell, +#: and the two gh commands are argvs of their own. +OFFERED = (["gh", "auth", "login"], ["gh", "auth", "setup-git"]) + + +def refusal_of(module, opts): + """What `preflight` refused, or "" when it passed.""" + try: + module.preflight(opts) + except module.Refusal as refusal: + return str(refusal) + return "" + + +def ran_an_offer(call): + return call[:2] in (["bash", "-c"], ["cmd", "/c"]) or call in OFFERED + + +@pytest.mark.parametrize("machine", MACHINES) +def test_a_dry_run_makes_none_of_the_offers_preflight_makes( + monkeypatch, capsys, tmp_path, machine): + """A terminal that types `yes` to anything is asked nothing by a dry + run, and nothing an offer runs is run: the offers are named as not made, + and every check still says what is missing. `--preflight -- --dry-run` + makes the same offers a run without `--dry-run` makes, and the recorder + runs none of them.""" + present, answers, offers, refused = MACHINES[machine] + module = entry_point_module() + monkeypatch.setattr(module.shutil, "which", _which(*present)) + runs = _Runs(answers) + monkeypatch.setattr(module, "run", runs) + prompts = _answers(monkeypatch, module, "yes") + rehearsal = module.parse_args(["Sample", "--org", "demoorg", "--", + "--dry-run"], str(tmp_path)) + assert refusal_of(module, rehearsal) == refused + assert prompts == [], "a dry run made an offer" + assert [call for call in runs.calls if ran_an_offer(call)] == [] + assert capsys.readouterr().out.count(NOT_OFFERED) == offers + module.preflight_only(module.parse_args(["--preflight", "--", + "--dry-run"], str(tmp_path))) + assert len(prompts) == offers, "--preflight made no offer" + assert NOT_OFFERED not in capsys.readouterr().out + + +# --- at a terminal ----------------------------------------------------------- + +#: A `gh` that logs each call, answers the preflight and the plan's three +#: questions, and writes what the real one writes. Signed in where its +#: folder holds `signed-in`. +STUB_GH = (b"#!/bin/sh\n" + b'here=$(dirname "$0")\n' + b'printf "%s\\n" "$*" >> "$here/calls.log"\n' + b'case "$*" in\n' + b' --version) echo "gh version 2.50.0 (stub)" ;;\n' + b' "auth status") [ -e "$here/signed-in" ] || exit 1 ;;\n' + b' "auth login") : > "$here/signed-in" ;;\n' + b' "auth setup-git") printf \'[credential "https://github.com"]' + b'\\n\\thelper =\\n\\thelper = !gh auth git-credential\\n\' ' + b'>> "${GIT_CONFIG_GLOBAL:?}" ;;\n' + b" api*) echo tester ;;\n" + b' "repo view"*) echo "GraphQL: Could not resolve to a ' + b'Repository with the name x." >&2; exit 1 ;;\n' + b' *) echo "stub gh: refused $*" >&2; exit 97 ;;\n' + b"esac\n") + +#: The person's global git config: a name, and no credential helper. +GLOBAL_CONFIG = b"[user]\n\tname = Tester\n" + +#: The same, with the helper `gh auth setup-git` writes. +GLOBAL_CONFIG_WITH_A_HELPER = (GLOBAL_CONFIG + + b'[credential "https://github.com"]\n' + b"\thelper =\n" + b"\thelper = !gh auth git-credential\n") + + +class Rehearsal: + """`setup-project.py Sample --org demoorg ... -- --dry-run` at a + terminal that types `yes` to the first `Type yes to continue:` it is + shown, under `STUB_GH`, signed in or not, and a global git config of the + run's own: what the run did, and the files it could have changed. + + Its git reads no config but that file and an empty system one: the + environment's `GIT_CONFIG_COUNT` and `-c` pairs are dropped, and it runs + from a folder that is no repository, so no checkout's config gives it a + helper either.""" + + def __init__(self, tmp_path, signed_in, global_config, entry="py"): + stub = tmp_path / "ghbin" + stub.mkdir() + (stub / "gh").write_bytes(STUB_GH) + (stub / "gh").chmod(0o755) + self.signed_in = stub / "signed-in" + if signed_in: + self.signed_in.write_bytes(b"") + self.global_config = tmp_path / "global.gitconfig" + self.global_config.write_bytes(global_config) + system = tmp_path / "system.gitconfig" + system.write_bytes(b"") + (tmp_path / "gh-config").mkdir() + env = {key: value for key, value in terminal_env().items() + if key not in ("GIT_CONFIG_COUNT", "GIT_CONFIG_PARAMETERS")} + env.update({"PATH": str(stub) + os.pathsep + env["PATH"], + "GH_CONFIG_DIR": str(tmp_path / "gh-config"), + "GIT_CONFIG_GLOBAL": str(self.global_config), + "GIT_CONFIG_SYSTEM": str(system)}) + self.into = tmp_path / "into" + flags = ["Sample", "--org", "demoorg", "--elected-by", "Tester", + "--into", str(self.into), "--", "--dry-run"] + argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], + "piped": piped("", *flags)}[entry] + self.status, self.screen, self.answered = run_in_a_terminal( + argv, cwd=tmp_path, env=env, answers=["yes\n"], timeout=60) + log = stub / "calls.log" + self.calls = (log.read_text(encoding="utf-8").splitlines() + if log.exists() else []) + + def assert_asked_nothing(self): + assert self.answered == 0, f"it made an offer:\n{self.screen}" + assert "Type yes to continue" not in self.screen, self.screen + assert not self.into.exists() + + +@PTY_SKIP +@pytest.mark.parametrize("entry", ["py", "piped"]) +def test_a_dry_run_leaves_git_with_no_helper_as_it_found_it(tmp_path, entry): + """Signed in, and git has no credential helper: the preflight warns, says + `gh auth setup-git` was not offered, and goes on to the plan, which ends + the run. The global git config is the bytes it was.""" + run = Rehearsal(tmp_path, True, GLOBAL_CONFIG, entry) + run.assert_asked_nothing() + assert run.status == 0, run.screen + assert HELPER_WARNING + "\n" + NOT_OFFERED + "\n" in run.screen + assert DRY_RUN_LAST_LINE in run.screen + assert run.global_config.read_bytes() == GLOBAL_CONFIG + assert "auth setup-git" not in run.calls + + +@PTY_SKIP +def test_a_dry_run_does_not_sign_gh_in(tmp_path): + """Not signed in: the preflight says `gh auth login` was not offered and + refuses as it does with nothing to ask on. gh is not signed in. Git has a + helper, so the login is the one offer at stake.""" + run = Rehearsal(tmp_path, False, GLOBAL_CONFIG_WITH_A_HELPER) + run.assert_asked_nothing() + assert run.status == 2, run.screen + assert NOT_SIGNED_IN + "\n" + NOT_OFFERED + "\n" in run.screen + assert the_refusal(run.screen) == "REFUSED: " + PREREQ_MISSING + assert not run.signed_in.exists() + assert "auth login" not in run.calls diff --git a/tests/test_setup_project_ref_parts.py b/tests/test_setup_project_ref_parts.py new file mode 100644 index 00000000..0289fc97 --- /dev/null +++ b/tests/test_setup_project_ref_parts.py @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`--shape-ref` is held to git's rule for a ref part by part (#857). + +`checked_ref` refused `..` and a `.lock` at the very end, where git's rule +is asked of each part between slashes: none is empty, none starts with `.`, +none ends in `.lock`, and the whole does not end in `.`. So `a.lock/b`, +`x/y.lock/z`, `release.lock/v1`, `a/.b`, `a//b`, `a/b/`, `a.lock/` and `a.` +passed, and `git check-ref-format --branch` refuses each of them. In +self-bootstrap mode the person met git's own `pathspec ... did not match` +after the standard was cloned. Each is refused now, before anything, with +the part of the rule it failed; `topic.locked` and `a./b` still go on. + +The table asks git itself, `git check-ref-format --branch`, beside the tool: +no network, and the two give one answer for every name in it. +""" + +from __future__ import annotations + +import subprocess + +import pytest + +from conftest import REPO +from test_setup_project_name_edges import FIRST, LOCK, RANGE +from test_setup_project_py import (entry_point_module, make_outside_checkout, + run_entry) +from test_setup_project_typed_name import the_refusal + +LOCK_PART = ("A part of it before a `/` ends in `.lock`, the name git gives " + "the lock file it writes beside a ref, and git takes no ref with " + "such a part.") +DOT_PART = ("A part of it after a `/` starts with `.`, and git takes no ref " + "with such a part.") +EMPTY_PART = ("It holds `//` or ends in `/`, and git takes no ref with an " + "empty part between slashes.") +DOT_END = "It ends in `.`, and git takes no ref that does." + +#: Each name, and the reason the tool refuses it ("" where it goes on). The +#: issue's ten come first, then the names around them. +NAMES = { + "a.lock": LOCK, + "a/b.lock": LOCK, + "a.lock/b": LOCK_PART, + "x/y.lock/z": LOCK_PART, + "release.lock/v1": LOCK_PART, + "a/.b": DOT_PART, + "a//b": EMPTY_PART, + "a/b/": EMPTY_PART, + "a.lock/": EMPTY_PART, + "topic.locked": "", + "a.": DOT_END, + "a/b.": DOT_END, + "a./b": "", + "a.lockb": "", + "a/b.lock/": EMPTY_PART, + "a/.lock": DOT_PART, + "a..b": RANGE, + "refs/tags/v1.0": "", + "v1.0.0": "", +} + + +def tool_answer(module, name): + """The reason `checked_ref` gives for refusing `name`, or "".""" + try: + module.checked_ref("--shape-ref", name) + except module.Refusal as refusal: + return str(refusal).split(" command line. ", 1)[1] + return "" + + +@pytest.mark.parametrize("name", NAMES) +def test_the_tool_and_git_give_one_answer(tmp_path, name): + git = subprocess.run(["git", "check-ref-format", "--branch", name], + cwd=str(tmp_path), capture_output=True, text=True, + check=False) + why = tool_answer(entry_point_module(), name) + assert (why == "") == (git.returncode == 0), (name, why, git.stderr) + assert why == NAMES[name] + + +@pytest.mark.parametrize("name", ["a.lock/b", "a/.b", "a//b"]) +def test_a_ref_git_refuses_is_refused_before_the_clone(tmp_path, name): + """A ref git refuses ended a self-bootstrap run in git's `pathspec ... + did not match`, after the standard was cloned. It is refused before + `(0) self-bootstrap` prints.""" + outside = make_outside_checkout(tmp_path) + result = run_entry("--org", "demoorg", "--project", "Sample", + "--shape-ref", name, + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), + cwd=outside, script=outside / "setup-project.py", + extra_env={"OPENREPOSHAPE_REPO": str(REPO)}) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "", result.stdout + assert the_refusal(result.stderr) == ( + "REFUSED: " + FIRST.format(what="--shape-ref", shown=ascii(name)) + + NAMES[name]) + assert not (tmp_path / "remotes").exists() From ac570069efbcc47c62ef357850451515f06e9d7f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 04:23:40 +0000 Subject: [PATCH 8/8] tests/test_setup_project_ref_parts.py and tests/test_setup_project_dry_run_offers.py: a part that starts with `.` is held anywhere in a ref, a name that breaks two rules is told the first, and a dry run with no terminal and one spelled `--dry` are held, so five mutants of #857's fix that passed every test are caught (#857) An independent review of #873 made 14 mutants of the fix; five survived the two new files and the setup tests. This closes each one with a test. - `a/.b/c` and `a/b/.c/d` join the table. The only dot-part rows put the part last, so a check of `parts[-1]` alone (R4) and of `parts[:2]` alone (R13) passed, and `git check-ref-format --branch` refuses both names. - Three names that break two rules hold `why_not_a_ref`'s order, which its docstring claims: `a//..b` is a range and not an empty part (R3), `a//.b` an empty part and not a dot part, and `a.lock/b.` a lock part and not a `.` at the end. With `a/.lock` and `a/b.lock` that holds all five pairs of neighbours. - A dry run with no terminal prints no `not offered` line and reads as it does without `--dry-run`, on each of the three machines (R1, which puts the dry-run check before `can_ask()`). - The in-process dry-run test and one terminal run take `--dry`, the short spelling `passes_a_dry_run` reads, so `preflight` cannot test the literal `--dry-run` (R7). setup-project.py is unchanged. The plan's last line still prints where nothing was marked `not offered`: telling the plan whether the preflight marked one takes a count written by `offer`, four calls below `preflight`, and the line is true as written. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_setup_project_dry_run_offers.py | 91 +++++++++++++++------- tests/test_setup_project_ref_parts.py | 16 +++- 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/tests/test_setup_project_dry_run_offers.py b/tests/test_setup_project_dry_run_offers.py index e47f3dd2..df38e7eb 100644 --- a/tests/test_setup_project_dry_run_offers.py +++ b/tests/test_setup_project_dry_run_offers.py @@ -34,8 +34,8 @@ from conftest import DRY_RUN_ENDS_IT, PTY_SKIP, run_in_a_terminal from test_setup_project_py import (EVERY_MANAGER, SETUP_PROJECT, _answers, - _Runs, _which, entry_point_module, piped, - terminal_env) + _no_terminal, _Runs, _which, + entry_point_module, piped, terminal_env) from test_setup_project_typed_name import the_refusal #: What the terminal is told where an offer was not made. @@ -73,6 +73,23 @@ OFFERED = (["gh", "auth", "login"], ["gh", "auth", "setup-git"]) +#: The ways a run passes `--dry-run` through: the flag itself, and the short +#: spelling the scaffold's argparse takes for it (`passes_a_dry_run`). +DRY_RUN_SPELLINGS = ["--dry-run", "--dry"] + + +def a_machine(monkeypatch, machine): + """`setup-project.py` as a module on `machine`, with `run` a recorder: + the module, the recorder, the offers a run without `--dry-run` makes + there, and the refusal its preflight ends in.""" + present, answers, offers, refused = MACHINES[machine] + module = entry_point_module() + monkeypatch.setattr(module.shutil, "which", _which(*present)) + runs = _Runs(answers) + monkeypatch.setattr(module, "run", runs) + return module, runs, offers, refused + + def refusal_of(module, opts): """What `preflight` refused, or "" when it passed.""" try: @@ -86,32 +103,48 @@ def ran_an_offer(call): return call[:2] in (["bash", "-c"], ["cmd", "/c"]) or call in OFFERED +@pytest.mark.parametrize("flag", DRY_RUN_SPELLINGS) @pytest.mark.parametrize("machine", MACHINES) def test_a_dry_run_makes_none_of_the_offers_preflight_makes( - monkeypatch, capsys, tmp_path, machine): + monkeypatch, capsys, tmp_path, machine, flag): """A terminal that types `yes` to anything is asked nothing by a dry - run, and nothing an offer runs is run: the offers are named as not made, - and every check still says what is missing. `--preflight -- --dry-run` - makes the same offers a run without `--dry-run` makes, and the recorder - runs none of them.""" - present, answers, offers, refused = MACHINES[machine] - module = entry_point_module() - monkeypatch.setattr(module.shutil, "which", _which(*present)) - runs = _Runs(answers) - monkeypatch.setattr(module, "run", runs) + run, in either spelling, and nothing an offer runs is run: the offers + are named as not made, and every check still says what is missing. + `--preflight -- --dry-run` makes the same offers a run without + `--dry-run` makes, and the recorder runs none of them.""" + module, runs, offers, refused = a_machine(monkeypatch, machine) prompts = _answers(monkeypatch, module, "yes") - rehearsal = module.parse_args(["Sample", "--org", "demoorg", "--", - "--dry-run"], str(tmp_path)) + rehearsal = module.parse_args(["Sample", "--org", "demoorg", "--", flag], + str(tmp_path)) assert refusal_of(module, rehearsal) == refused assert prompts == [], "a dry run made an offer" assert [call for call in runs.calls if ran_an_offer(call)] == [] assert capsys.readouterr().out.count(NOT_OFFERED) == offers - module.preflight_only(module.parse_args(["--preflight", "--", - "--dry-run"], str(tmp_path))) + module.preflight_only(module.parse_args(["--preflight", "--", flag], + str(tmp_path))) assert len(prompts) == offers, "--preflight made no offer" assert NOT_OFFERED not in capsys.readouterr().out +@pytest.mark.parametrize("machine", MACHINES) +def test_a_dry_run_with_no_terminal_says_what_a_run_without_it_says( + monkeypatch, capsys, tmp_path, machine): + """`offer` asks `can_ask()` before it asks whether it is offering: a run + with no terminal makes no offer and has nothing to name as not made, so + it prints no `not offered` line, and its preflight reads as it does + without `--dry-run`.""" + module, _, _, refused = a_machine(monkeypatch, machine) + _no_terminal(monkeypatch, module) + shown = [] + for passthrough in ([], ["--", "--dry-run"]): + opts = module.parse_args(["Sample", "--org", "demoorg", *passthrough], + str(tmp_path)) + assert refusal_of(module, opts) == refused + shown.append(capsys.readouterr().out) + assert "not offered" not in shown[1] + assert shown[1] == shown[0] + + # --- at a terminal ----------------------------------------------------------- #: A `gh` that logs each call, answers the preflight and the plan's three @@ -144,17 +177,19 @@ def test_a_dry_run_makes_none_of_the_offers_preflight_makes( class Rehearsal: - """`setup-project.py Sample --org demoorg ... -- --dry-run` at a - terminal that types `yes` to the first `Type yes to continue:` it is - shown, under `STUB_GH`, signed in or not, and a global git config of the - run's own: what the run did, and the files it could have changed. + """`setup-project.py Sample --org demoorg ... -- ` (`--dry-run`, or + its short spelling) at a terminal that types `yes` to the first `Type yes + to continue:` it is shown, under `STUB_GH`, signed in or not, and a global + git config of the run's own: what the run did, and the files it could + have changed. Its git reads no config but that file and an empty system one: the environment's `GIT_CONFIG_COUNT` and `-c` pairs are dropped, and it runs from a folder that is no repository, so no checkout's config gives it a helper either.""" - def __init__(self, tmp_path, signed_in, global_config, entry="py"): + def __init__(self, tmp_path, signed_in, global_config, entry="py", + flag="--dry-run"): stub = tmp_path / "ghbin" stub.mkdir() (stub / "gh").write_bytes(STUB_GH) @@ -175,7 +210,7 @@ def __init__(self, tmp_path, signed_in, global_config, entry="py"): "GIT_CONFIG_SYSTEM": str(system)}) self.into = tmp_path / "into" flags = ["Sample", "--org", "demoorg", "--elected-by", "Tester", - "--into", str(self.into), "--", "--dry-run"] + "--into", str(self.into), "--", flag] argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], "piped": piped("", *flags)}[entry] self.status, self.screen, self.answered = run_in_a_terminal( @@ -191,12 +226,16 @@ def assert_asked_nothing(self): @PTY_SKIP -@pytest.mark.parametrize("entry", ["py", "piped"]) -def test_a_dry_run_leaves_git_with_no_helper_as_it_found_it(tmp_path, entry): +@pytest.mark.parametrize("entry, flag", [("py", "--dry-run"), + ("piped", "--dry-run"), + ("py", "--dry")]) +def test_a_dry_run_leaves_git_with_no_helper_as_it_found_it(tmp_path, entry, + flag): """Signed in, and git has no credential helper: the preflight warns, says `gh auth setup-git` was not offered, and goes on to the plan, which ends - the run. The global git config is the bytes it was.""" - run = Rehearsal(tmp_path, True, GLOBAL_CONFIG, entry) + the run. The global git config is the bytes it was, whichever way the + terminal's run spells `--dry-run`.""" + run = Rehearsal(tmp_path, True, GLOBAL_CONFIG, entry, flag) run.assert_asked_nothing() assert run.status == 0, run.screen assert HELPER_WARNING + "\n" + NOT_OFFERED + "\n" in run.screen diff --git a/tests/test_setup_project_ref_parts.py b/tests/test_setup_project_ref_parts.py index 0289fc97..9d5e915b 100644 --- a/tests/test_setup_project_ref_parts.py +++ b/tests/test_setup_project_ref_parts.py @@ -8,7 +8,10 @@ passed, and `git check-ref-format --branch` refuses each of them. In self-bootstrap mode the person met git's own `pathspec ... did not match` after the standard was cloned. Each is refused now, before anything, with -the part of the rule it failed; `topic.locked` and `a./b` still go on. +the part of the rule it failed; `topic.locked` and `a./b` still go on. A part +that starts with `.` is refused wherever it stands (`a/.b/c`, `a/b/.c/d`), and +a name that fails two rules is told the first one in the order +`why_not_a_ref` asks them (`a//..b` is a range, not an empty part). The table asks git itself, `git check-ref-format --branch`, beside the tool: no network, and the two give one answer for every name in it. @@ -44,6 +47,8 @@ "x/y.lock/z": LOCK_PART, "release.lock/v1": LOCK_PART, "a/.b": DOT_PART, + "a/.b/c": DOT_PART, + "a/b/.c/d": DOT_PART, "a//b": EMPTY_PART, "a/b/": EMPTY_PART, "a.lock/": EMPTY_PART, @@ -57,6 +62,15 @@ "a..b": RANGE, "refs/tags/v1.0": "", "v1.0.0": "", + # A name that breaks two rules is told the first one `why_not_a_ref` asks, + # in its order: `..`, an empty part, a part that starts with `.`, a name + # that ends in `.lock`, a part that ends in `.lock`, a `.` at the end. + # Each row below holds one pair of neighbours in that order apart; two + # rows above hold the others (`a/.lock`: a dot part, then a lock at the + # end; `a/b.lock`: a lock at the end, then a lock part). + "a//..b": RANGE, + "a//.b": EMPTY_PART, + "a.lock/b.": LOCK_PART, }