From c71eadbfb5a92e295ab232a74901c9db51a80d8a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:01:11 +0000 Subject: [PATCH 1/4] setup-project.py: its paste lines quote each path and read local legs, the scaffold's NEXT STEPS are left to step (7), step (3) refuses a file above the clone path, `-- --dry-run` ends at the plan, step (4) hears the pins passed through, and the upstream guard ignores case, so no run says what it did not do or creates three repositories and then dies (#538) Six defects in setup-project.py's printed text and step checks, and the two rides-along, from the v1.1.0 triage of the v1.0.0 known limits on #459. On main bfed143 each one reproduces. - The hand-over's `cd ` and step (7)'s backstop printed their paths bare, so `--into "/home/a b"` gave lines the shell split. Both now go through `shell_word`, as the family lines do (#486), and the backstop names the clone's parent as a `cd` line of its own. - Under `--local-remote-dir` the backstop's recursive clone, and the scaffold's own NEXT STEPS clone, lacked `-c protocol.file.allow=always` and failed `transport 'file' not allowed`. Both carry it now; the scaffold's line is POSIX-quoted (`shell_line`, as #532 rules). - Inside setup the scaffold printed its NEXT STEPS (clone, cd, make bootstrap) just before step (7) ran them. The scaffold takes a new `--no-next-steps`, which setup passes in step (6); `Working trees are at` still prints, because README's rehearsal paragraph cites it. - A FILE at `/`, or at `--into`, passed step (3): all three repositories were created and `os.makedirs` ended the run in a `FileExistsError` traceback. Step (3) now refuses a non-folder above the clone path, and step (7)'s `os.makedirs` is a refusal with the lines that finish by hand, never a traceback. - `-- --dry-run` made the scaffold create nothing, then step (7)'s clone failed and said "The three repositories WERE created" (and left the family folder behind). Setup now reads its passthrough as the scaffold's argparse does (whole, a prefix it takes, up to an inner `--`) and ends the run at the plan, exit 0, asking nothing. - Step (4) asked the naming check about the three names alone, so a `--pin` (or `--referent-chain`) passed through was "no referent pin declared" there and a domain descendant in the plan. The validator is now asked with `--pins=` and `--referent-chain=` from the passthrough. - The upstream-org guard compared `opensoft` exactly, so `--org OpenSoft` and an `origin` under `OpenSoft/` scaffolded into opensoft's namespace. Every upstream check casefolds now (`is_upstream_org`). docs/cli.md is re-rendered for the usage line and the new scaffold flag. The tests are shared helpers in tests/conftest.py, called by one-line twins in tests/test_setup_project_py.py and tests/test_setup_sh.py, plus in-process tests and one scaffold test; all 29 new run tests are red on main. Closes #538 Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- docs/cli.md | 11 +- scaffold-project.py | 33 +++- setup-project.py | 262 ++++++++++++++++++++++++++++---- tests/conftest.py | 267 +++++++++++++++++++++++++++++++++ tests/test_scaffold_e2e.py | 24 +++ tests/test_setup_project_py.py | 197 +++++++++++++++++++++++- tests/test_setup_sh.py | 51 +++++++ 7 files changed, 805 insertions(+), 40 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index c793a3e0..45ce0188 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -112,7 +112,8 @@ usage: setup-project.py [] [options] [-- ] 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. +Anything after `--` is passed straight through to scaffold-project.py. A +`--dry-run` there ends this run at the plan, creating nothing. ``` ### `openRepoShape` @@ -233,7 +234,8 @@ usage: setup-project.py [] [options] [-- ] 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. +Anything after `--` is passed straight through to scaffold-project.py. A +`--dry-run` there ends this run at the plan, creating nothing. ``` ### `scaffold-project.py` @@ -253,6 +255,7 @@ usage: scaffold-project.py [-h] --org ORG --project PROJECT [--id ID] [--pin openProduct@COMMIT] [--referent-chain openLayer,openProduct] [--pin-source PIN_SOURCE] [--work-dir WORK_DIR] + [--no-next-steps] Create a project's three repositories in the shape this standard describes. @@ -342,6 +345,10 @@ options: network) --work-dir WORK_DIR where the working trees are built (default: a temporary directory) + --no-next-steps leave out the closing NEXT STEPS (the clone, the cd + and `make bootstrap`): setup-project.py passes it, + because it runs those steps itself next. `Working + trees are at` still prints. ``` ### `adopt-project.py` diff --git a/scaffold-project.py b/scaffold-project.py index 98bb95c5..110e7c01 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -79,7 +79,7 @@ free_plan_secret_hint, COMMIT_RE, NEUTRAL_PRODUCT_OWNER, PROJECT_ID_RE, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, accepts_role, checked_value, - git_out, link_pins_from_trees, repo_basename, tree_digest, + git_out, link_pins_from_trees, repo_basename, shell_line, tree_digest, ) from shape_materialize import ( # noqa: E402 RULESET_HINT, SHAPE_REPOSITORY, CommandFailed, @@ -308,6 +308,11 @@ def main(argv: list[str] | None = None) -> int: parser.add_argument("--work-dir", type=Path, default=None, help="where the working trees are built " "(default: a temporary directory)") + parser.add_argument("--no-next-steps", action="store_true", + help="leave out the closing NEXT STEPS (the clone, " + "the cd and `make bootstrap`): setup-project.py " + "passes it, because it runs those steps itself " + "next. `Working trees are at` still prints.") args = parser.parse_args(argv) try: @@ -1517,22 +1522,36 @@ def _scaffold(args) -> int: return 2 _set_topics(repositories, topic, local) - print(f""" + if not args.no_next_steps: + print(f""" NEXT STEPS - git clone --recurse-submodules {urls['assembly']} + {_next_steps_clone(urls['assembly'], local)} cd {names['assembly']} make bootstrap `make bootstrap` puts each leg on `{args.tracking_branch}` AT its pinned commit, runs the naming, manifest and lockstep-pin validators, and prints the review authority a wallet register names for this project — or says that authority is -not wallet-carried here, and continues. - -Working trees are at {work_root} -""") +not wallet-carried here, and continues.""") + print(f"\nWorking trees are at {work_root}\n") return 0 +def _next_steps_clone(url: str, local: bool) -> str: + """The NEXT STEPS clone, as a person pastes it into a POSIX shell. + + UNDER `--local-remote-dir` IT CARRIES `-c protocol.file.allow=always` + (#538), as setup-project.py's own clone does: the legs are cloned from + the plain paths `.gitmodules` records, the file transport, which git + refuses a submodule by default since the 2022 advisories, so the line as + printed failed `transport 'file' not allowed`. The URL is a path there, + one word whatever folder it is in (`shell_line`, POSIX as #532 rules + for a printed command); a GitHub URL needs no quotes. + """ + words = ["git", "-c", "protocol.file.allow=always"] if local else ["git"] + return shell_line([*words, "clone", "--recurse-submodules", url], "posix") + + if __name__ == "__main__": sys.exit(main()) diff --git a/setup-project.py b/setup-project.py index 3d1d531f..40915e02 100755 --- a/setup-project.py +++ b/setup-project.py @@ -176,7 +176,8 @@ 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. +Anything after `--` is passed straight through to scaffold-project.py. A +`--dry-run` there ends this run at the plan, creating nothing. """ @@ -1222,11 +1223,22 @@ def _org_from_gh(opts: Options, origin_url: str) -> str: return (proc.stdout or "").strip() if proc.returncode == 0 else "" +def is_upstream_org(org: str) -> bool: + """`org` is opensoft, in whatever case it was typed or recorded (#538). + + GitHub reads an organisation's login in any case, so `--org OpenSoft`, + and an `origin` under `OpenSoft/`, name opensoft's own namespace. Every + upstream check below compared exactly, so those passed with no refusal, + and a person with create rights there scaffolded into it. + """ + return org.casefold() == UPSTREAM_ORG.casefold() + + def _org_without_origin(opts: Options, origin_url: str) -> str: """No --org and no organisation parsed out of `origin`: ask, or refuse.""" viewed = _org_from_gh(opts, origin_url) if viewed: - if viewed == UPSTREAM_ORG: + if is_upstream_org(viewed): die("you are running from the upstream checkout (%s); pass " "--org ." % origin_url) ok("organisation %s (from `gh repo view` on the origin URL: %s; " @@ -1245,7 +1257,7 @@ def _choose_org(opts: Options, origin_url: str, origin_org: str) -> str: # `origin` pointing at opensoft itself means this checkout IS the upstream # - there is no fork to inherit an organisation from, exactly like # self-bootstrap mode, so the fix is the same: pass --org. - if not opts.org and origin_org == UPSTREAM_ORG: + if not opts.org and is_upstream_org(origin_org): die("you are running from the upstream checkout (%s); pass --org " "." % origin_url) if opts.org: @@ -1281,14 +1293,22 @@ def _guard_upstream_org(opts: Options, org: str) -> None: records as the owner of all three legs, so a manifest reading `opensoft/Sample` is wrong regardless of whether any network call happened. + + IN ANY CASE (#538): `is_upstream_org`. The refusal names the + organisation as it was typed, and says why another casing is still + opensoft's. """ - if org == UPSTREAM_ORG and not opts.allow_upstream_org: + if is_upstream_org(org) and not opts.allow_upstream_org: + casing = ("" if org == UPSTREAM_ORG else + " (GitHub reads an organisation's name in any case, so " + "'%s' is '%s')" % (org, UPSTREAM_ORG)) die("the organisation is '%s', which is the UPSTREAM owner of " - "openRepoShape itself, and scaffolding here would create three " + "openRepoShape itself%s, and scaffolding here would create three " "repositories in opensoft's own namespace.\n" "\n" " Wrong guess? re-run with --org \n" - " You meant it? re-run with --allow-upstream-org" % UPSTREAM_ORG) + " You meant it? re-run with --allow-upstream-org" + % (org, casing)) def resolve_org(opts: Options, invocation_dir: str) -> str: @@ -1789,8 +1809,15 @@ def refuse_a_taken_clone_path(clone: str, project: str) -> None: The refusal says nothing was created, names the path, and says what is there where `what_stands_at` can tell. Step (7) keeps its own check, for a path that appears while the run is going. + + AND WHAT STANDS ABOVE IT (#538). A FILE at `/`, or at + `--into` itself, is a path the clone can never be made under, and + `lexists` reads `/` as absent: on main this passed, all + three repositories were created, and step (7)'s `os.makedirs` ended the + run in a `FileExistsError` traceback. `refuse_a_file_above` asks it here. """ if not os.path.lexists(clone): + refuse_a_file_above(clone) return what, way_out = what_stands_at(clone, project) die(" ".join(part for part in ( @@ -1799,15 +1826,150 @@ def refuse_a_taken_clone_path(clone: str, project: str) -> None: what, way_out) if part)) +def nearest_entry(path: str): + """`path`, or its nearest parent that anything stands at (`lexists`), or + None when not even the top of the path does - a Windows drive that is + not there.""" + while not os.path.lexists(path): + parent = os.path.dirname(path) + if parent == path: + return None + path = parent + return path + + +def what_blocks(path: str) -> str: + """What `path` is, when it is not a folder, as the refusal says it: a + symbolic link by its target where that prints as itself + (`CONTROL_RE`), as `what_stands_at` says one.""" + if os.path.islink(path): + try: + target = os.readlink(path) + except OSError: + target = "" + if target and not CONTROL_RE.search(target): + return "a symbolic link to %s, which is not a folder" % target + return "a symbolic link that is not to a folder" + if os.path.isfile(path): + return "a file" + return "neither a folder nor a file" + + +def refuse_a_file_above(clone: str) -> None: + """Refuse a clone path that something other than a folder stands above. + + The folders above the clone are made in step (7), by `os.makedirs`, and + nothing exists yet: the first thing found on the way up from the clone's + parent must be a folder (`isdir`, so a link to one is one), or there is + nowhere to make them. Nothing found at all is the case before, and is + left to the clone. + """ + above = nearest_entry(os.path.dirname(clone)) + if above is None or os.path.isdir(above): + return + die("%s is %s, and this project would be cloned inside it, at %s, so the " + "run stops here and nothing was created. %s" + % (above, what_blocks(above), clone, KEEP_IT)) + + +# --------------------------------------------------------------------------- +# what the passthrough tells the scaffold (#538) +# --------------------------------------------------------------------------- + +#: THE SCAFFOLD FLAGS THIS FILE READS OUT OF ITS PASSTHROUGH, each with the +#: shortest spelling the scaffold's argparse takes for it. argparse takes an +#: unambiguous prefix of a long option as that option, so `--dry` is the +#: scaffold's `--dry-run`; `--pin` has no shorter spelling, because +#: `--pin-owner` and `--pin-source` share every one, and a prefix of +#: `--referent-chain` shorter than `--referent` is one of `--reference` +#: too. `tests/test_setup_project_py.py` holds each row to the +#: scaffold's own `--help`. A word this reads that the scaffold finds +#: ambiguous makes the scaffold's plan refuse, in step (5), before anything +#: is created, so reading one too many is never a run the scaffold made. +PASSED_FLAGS = {"--dry-run": "--d", "--pin": "--pin", + "--referent-chain": "--referent"} + + +def scaffold_words(passthrough: list) -> list: + """The passthrough up to an inner `--`: every word after one is a + positional to argparse, never a flag, and the scaffold takes none.""" + if "--" in passthrough: + return passthrough[:passthrough.index("--")] + return list(passthrough) + + +def names_flag(word: str, flag: str) -> bool: + """`word` is the scaffold's `flag` (a `PASSED_FLAGS` key), alone or as + `flag=value`, in a spelling its argparse takes.""" + given = word.split("=", 1)[0] + return len(given) >= len(PASSED_FLAGS[flag]) and flag.startswith(given) + + +def passed_values(passthrough: list, flag: str) -> list: + """Every value the passthrough gives `flag`, in order: `flag=value`, or + the word after `flag`. A value never starts with `-` in the second + spelling, because argparse reads such a word as a flag, so a word this + takes as `flag` is never somebody's value.""" + words = scaffold_words(passthrough) + values = [] + for index, word in enumerate(words): + if not names_flag(word, flag): + continue + if "=" in word: + values.append(word.split("=", 1)[1]) + elif index + 1 < len(words): + values.append(words[index + 1]) + return values + + +def passes_a_dry_run(passthrough: list) -> bool: + """`--dry-run` was passed through after `--` (#538). + + 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. + """ + return any(names_flag(word, "--dry-run") + for word in scaffold_words(passthrough)) + + +def naming_declarations(passthrough: list) -> list: + """The naming validator's arguments for what the passthrough declares + about the project (#538): the products of its `--pin`s, and its + `--referent-chain`. + + A `` name is a descendant only when the project pins + its referent, and the scaffold judges the names with the pins it is + given. Step (4) asked about the three names alone, so it reported `no + referent pin declared` for a project whose plan, a step later, called it + a domain descendant. Each product is read the way the scaffold's + `parse_pin` reads one: the name before `@`, at its last `/`. The LAST + chain, because a repeated flag that takes one value keeps its last. + Each is passed as `--flag=value`, so a value that starts with `-` stays + a value to the validator; one the scaffold refuses is refused at the + plan. + """ + products = [name for name in ( + pin.strip().split("@", 1)[0].rsplit("/", 1)[-1] + for pin in passed_values(passthrough, "--pin")) if name] + chains = passed_values(passthrough, "--referent-chain") + return ((["--pins=" + ",".join(products)] if products else []) + + (["--referent-chain=" + chains[-1]] if chains else [])) + + # --------------------------------------------------------------------------- # 4. the names, before anything exists # --------------------------------------------------------------------------- -def check_names(shape_root: Path, project: str, family: str = "") -> None: +def check_names(shape_root: Path, project: str, family: str = "", + declared=()) -> None: + """The three names against the naming policy, with what the project + declares about itself (`naming_declarations`) as the scaffold will + judge them.""" say("") say("(4) naming policy") proc = run([PYTHON, - str(shape_root / "scripts" / NAMING_VALIDATOR), + str(shape_root / "scripts" / NAMING_VALIDATOR), *declared, "--explain", project, project + "-spec", project + "-code"], cwd=shape_root) if proc.returncode != 0: @@ -1878,7 +2040,11 @@ def scaffold_args(opts: Options, org: str) -> list: def plan_and_confirm(opts: Options, shape_root: Path, org: str, - args: list, clone: str) -> None: + args: list, clone: str) -> bool: + """The plan, then the one question. False when the run ends at the plan + instead: a `--dry-run` passed through after `--` (`passes_a_dry_run`), + which asks nothing, `--yes` or not, because there is nothing to say yes + to.""" say("") say("(5) the plan") planned = run([PYTHON, str(shape_root / SCAFFOLD), *args, @@ -1899,8 +2065,13 @@ def plan_and_confirm(opts: Options, shape_root: Path, org: str, say("") say("clone into %s" % clone) + if passes_a_dry_run(opts.passthrough): + 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.") + return False if opts.assume_yes: - return + return True # THE QUESTION GOES TO THE PERSON'S TERMINAL, NEVER TO A PIPE (#492). # `curl ... | bash -s --` hands this file the rest of the script as its # stdin, so the yes is read from the controlling terminal instead @@ -1927,6 +2098,7 @@ def plan_and_confirm(opts: Options, shape_root: Path, org: str, say("This will create THREE repositories in '%s'." % org) if ask("Type yes to continue: ") != "yes": die("not confirmed; nothing was created.", 1) + return True # --------------------------------------------------------------------------- @@ -1934,9 +2106,13 @@ def plan_and_confirm(opts: Options, shape_root: Path, org: str, # --------------------------------------------------------------------------- def scaffold(shape_root: Path, args: list) -> None: + """The scaffold, for real, told `--no-next-steps` (#538): its closing + block is the `git clone`, `cd` and `make bootstrap` that step (7) runs + next, and printed here it read as work left to the person. Its `Working + trees are at` line still prints.""" say("") say("(6) scaffold") - proc = run([PYTHON, str(shape_root / SCAFFOLD), *args], + proc = run([PYTHON, str(shape_root / SCAFFOLD), *args, "--no-next-steps"], cwd=shape_root) if proc.returncode != 0: raise Refusal("the scaffold failed (see above).", @@ -1979,6 +2155,28 @@ def local_remotes_by_url(directory: str) -> list: return args +def finish_by_hand(parent: str, assembly: str, project: str, + local: bool) -> str: + """The lines that finish a run by hand once the three repositories + exist (#498): from the clone's parent, the recursive clone, then the + bootstrap. + + EVERY PATH IS ONE WORD (#538), through `shell_word`, as the family lines + are (#486): under `--into "/home/a b"` the bare lines split at the space, + and under `--local-remote-dir` `` is a path too. And THAT clone + carries `-c protocol.file.allow=always`, which setup's own clone passes: + the legs are cloned from the plain paths `.gitmodules` records, the file + transport, which git refuses a submodule without it. The project name is + a word already: step (4) held it to the naming policy. + """ + clone = "git -c protocol.file.allow=always clone" if local else "git clone" + return (" cd %s\n" + " %s --recurse-submodules %s\n" + " cd %s && %s %s" + % (shell_word(parent), clone, shell_word(assembly), project, + PYTHON_CMD, BOOTSTRAP)) + + def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: # WHERE IT LANDS WAS DECIDED IN STEP (3), by `resolve_family`: `--into` # or, on the DEVELOPER PATH, the `..` of this checkout - a checkout's @@ -1986,13 +2184,7 @@ def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: # it, and self-bootstrap mode has no such neighbour and passes --into # instead. `--family` adds one level to it there and not here, so the # plan the person read and the clone that happens cannot disagree. - # - # `//` IS CREATED HERE AND NOWHERE EARLIER. A rehearsal - # that stops at the plan (no `--yes`, no terminal) must leave the disk - # exactly as it found it - a family folder made by a run that created - # nothing else would be litter the person did not ask for. parent = str(landing.directory) - os.makedirs(parent, exist_ok=True) clone = landing.clone_path(opts.project) if opts.local_mode: @@ -2012,7 +2204,24 @@ def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: # The three repositories now EXIST. Everything from here is recoverable by # hand, so a failure names the command that finishes the job rather than # leaving the person wondering what was created. - # + by_hand = finish_by_hand(parent, urls["assembly"], opts.project, + opts.local_mode) + + # `//` IS CREATED HERE AND NOWHERE EARLIER. A rehearsal + # that stops at the plan (no `--yes`, no terminal) must leave the disk + # exactly as it found it - a family folder made by a run that created + # nothing else would be litter the person did not ask for. Step (3) + # refused a file standing where it goes (#538); one that appeared since, + # or a folder this account cannot write, is a refusal with the way on, + # never a traceback after three repositories exist. + try: + os.makedirs(parent, exist_ok=True) + except OSError as exc: + die("%s could not be made a folder (%s), so there is nowhere to " + "clone into. The three repositories WERE created. Make it one, " + "then finish by hand:\n%s" + % (parent, exc.strerror or exc.__class__.__name__, by_hand)) + # THE BACKSTOP (#498). `refuse_a_taken_clone_path` asked this in step (3), # before anything existed, so a path taken HERE appeared while the run # was going. `lexists` for the same reason it is there. The way out moves @@ -2022,11 +2231,8 @@ def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: if os.path.lexists(clone): die("%s already exists - it was not there when step (3) looked - so " "there is nowhere to clone into. The three repositories WERE " - "created. Move it aside, then finish by hand in %s:\n" - " git clone --recurse-submodules %s\n" - " cd %s && %s %s" - % (clone, parent, urls["assembly"], opts.project, PYTHON_CMD, - BOOTSTRAP)) + "created. Move it aside, then finish by hand:\n%s" + % (clone, by_hand)) say("") say("(7) clone and bootstrap") @@ -2166,7 +2372,9 @@ def hand_over(project: str, clone: str, urls: dict, org: str, say("") say("Next:") say("") - say(" cd %s" % clone) + # ONE WORD (#538): a clone under `--into "/home/a b"` is a `cd` the + # shell would split. + say(" cd %s" % shell_word(clone)) say(" make validate # naming, manifest, lockstep pins - what " "CI runs") # setup.sh's hand-over block, with ONE line added: this file runs where @@ -2240,9 +2448,11 @@ def _main(argv, invocation_dir: str) -> int: landing = resolve_family(opts, shape_root) clone = landing.clone_path(opts.project) refuse_a_taken_clone_path(clone, opts.project) - check_names(shape_root, opts.project, opts.family) + check_names(shape_root, opts.project, opts.family, + naming_declarations(opts.passthrough)) args = scaffold_args(opts, org) - plan_and_confirm(opts, shape_root, org, args, clone) + if not plan_and_confirm(opts, shape_root, org, args, clone): + return 0 scaffold(shape_root, args) created = clone_and_bootstrap(opts, org, landing) hand_over(opts.project, created["clone"], created["urls"], org, landing) diff --git a/tests/conftest.py b/tests/conftest.py index f6672b8c..33227611 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -940,6 +940,273 @@ def assert_a_taken_clone_path_is_refused(run, tmp_path: Path, assert after == before, f"{taken} was changed by a run that refused" +# --- setup's paste lines, its step checks and the upstream guard (#538) ------ +# +# ONE READING, TWO ENTRY POINTS, as for #498 above: each helper takes the +# runner of the file that calls it, and every run passes `--local-remote-dir` +# for the reason given there. + +#: A folder whose name holds a space, which a shell splits a bare path at: +#: `--into "/home/a b"`, or a Windows profile folder. +SPACED = "a b" + +#: The word setup prints for the interpreter a person types: +#: `setup-project.py`'s `PYTHON_CMD`. +SETUP_PYTHON_CMD = "python" if os.name == "nt" else "python3" + +#: Where a FILE can stand above the clone path: the `--family` folder, +#: `/`, and `--into` itself. +FILE_ABOVE_THE_CLONE = ("family folder", "into") + +#: `(spelling, --yes)`: `--dry-run` passed through after `--` as the +#: scaffold's argparse reads it, whole and as a prefix it takes for the same +#: flag, told `--yes`; and whole with no `--yes` and no terminal to ask on. +DRY_RUNS = (("--dry-run", True), ("--dry", True), ("--dry-run", False)) + +#: What setup says when that `--dry-run` ends the run at its plan. +DRY_RUN_ENDS_IT = ("--dry-run was passed through to the scaffold, so this run " + "ends at the plan above: nothing was created and nothing " + "was cloned.") + +#: What a project declares after `--` that the naming check must hear: a +#: `--pin` in either spelling, and a pin chain. +DECLARATIONS = ("--pin", "--pin=", "--referent-chain") + +#: `opensoft` in a case other than its own, which GitHub reads as opensoft. +UPSTREAM_CASINGS = ("OpenSoft", "OPENSOFT") + + +def read_through_git(remotes: Path) -> dict: + """The environment that makes every git a pasted line starts read the + bare repositories in `remotes` through git, not through their files + (#440): `clone_just_pushed`'s `url..insteadOf` rewrites, as + `GIT_CONFIG_COUNT` entries, because the line under test is run exactly + as it was printed and a submodule's own clone inherits the environment. + The directory as typed and as resolved, so the root's URL and the legs' + `.gitmodules` URLs are both rewritten.""" + uri = remotes.resolve().as_uri().rstrip("/") + "/" + prefixes = list(dict.fromkeys( + spelling for directory in (Path(os.path.abspath(remotes)), + remotes.resolve()) + for spelling in (os.path.join(str(directory), ""), + directory.as_posix().rstrip("/") + "/"))) + env = {"GIT_CONFIG_COUNT": str(len(prefixes))} + for index, prefix in enumerate(prefixes): + env[f"GIT_CONFIG_KEY_{index}"] = f"url.{uri}.insteadOf" + env[f"GIT_CONFIG_VALUE_{index}"] = prefix + return env + + +def pasted(lines, cwd: Path, + env: dict | None = None) -> subprocess.CompletedProcess: + """`lines` pasted into `sh` one after another, stopping at the first that + fails: what a person does with the commands a tool prints.""" + return subprocess.run(["sh", "-c", "set -e\n" + "\n".join(lines)], + cwd=str(cwd), env={**os.environ, **(env or {})}, + capture_output=True, text=True, check=False) + + +def indented_commands(text: str) -> list: + """The commands `text` prints for pasting, each on a line of its own + indented four spaces, as setup indents them, with the indent dropped.""" + return [line.strip() for line in text.splitlines() + if line.startswith(" ") and not line[4:5].isspace()] + + +def assert_the_closing_cd_is_one_word(run, tmp_path: Path) -> None: + """The hand-over's `cd ` names the clone as ONE word, so a folder + with a space in `--into` reaches `cd` whole. On main it printed `cd + /.../a b/work/Sample`, which `cd` refuses as two arguments. Read the way + a POSIX shell reads it on every host, and run in one where there is + one.""" + into = tmp_path / SPACED / "work" + clone = os.path.abspath(str(into / "Sample")) + result = run("--project", "Sample", "--yes", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / SPACED / "remotes"), + "--into", str(into)) + assert result.returncode == 0, result.stderr + result.stdout + [cd] = [line for line in indented_commands( + result.stdout.split("\nNext:\n", 1)[-1]) if line.startswith("cd ")] + assert shlex.split(cd) == ["cd", clone], cd + if os.name != "nt": + ran = pasted([cd, "test -f project.yaml"], cwd=tmp_path) + assert ran.returncode == 0, ran.stderr + + +def assert_the_backstop_is_finished_by_hand_as_printed( + run, tmp_path: Path) -> None: + """Step (7)'s backstop, followed the way it says: move the path aside, + then paste the lines. Each path is ONE word, and under + `--local-remote-dir` the recursive clone carries `-c + protocol.file.allow=always`, as setup's own clone does: the legs are + cloned over the file transport, which git refuses a submodule without + it. On main the lines split at the space, and the clone failed + `transport 'file' not allowed`. + + THE PATH APPEARS WHILE THE RUN IS GOING because the scaffold is told to + build its working trees in `--into` itself (`--work-dir`, passed through + after `--`), so its root tree stands at `/Sample` by step (7).""" + into = tmp_path / SPACED / "work" + remotes = tmp_path / SPACED / "remotes" + clone = into / "Sample" + result = run("--project", "Sample", "--yes", "--org", "demoorg", + "--local-remote-dir", str(remotes), "--into", str(into), + "--", "--work-dir", str(into)) + assert result.returncode == 2, result.stderr + result.stdout + assert ("REFUSED: %s already exists - it was not there when step (3) " + "looked" % os.path.abspath(str(clone))) in result.stderr, ( + result.stderr) + lines = indented_commands(result.stderr.split("REFUSED: ", 1)[-1]) + assert [shlex.split(line) for line in lines] == [ + ["cd", os.path.abspath(str(into))], + ["git", *FILE_PROTOCOL, "clone", "--recurse-submodules", + os.path.join(os.path.abspath(str(remotes)), "Sample.git")], + ["cd", "Sample", "&&", SETUP_PYTHON_CMD, "scripts/bootstrap.py"], + ], lines + if os.name != "nt": + clone.rename(into / "Sample.aside") + ran = pasted(lines, cwd=tmp_path, env=read_through_git(remotes)) + assert ran.returncode == 0, ran.stderr + ran.stdout + assert "bootstrap ok" in ran.stdout, ran.stdout + + +def assert_the_scaffold_leaves_its_next_steps_to_setup(result) -> None: + """Inside setup the scaffold's own NEXT STEPS block (`git clone`, `cd`, + `make bootstrap`) is not printed: step (7) runs those very steps next, + and the block read as work left to the person. Its `Working trees are + at` line stays, after the scaffold and before step (7): README's + rehearsal paragraph sends a person to it.""" + out = result.stdout + assert "NEXT STEPS" not in out, out + trees = out.find("\nWorking trees are at ") + assert out.find("(6) scaffold") < trees, out + assert trees < out.find("(7) clone and bootstrap"), out + + +def assert_a_file_above_the_clone_path_is_refused(run, tmp_path: Path, + where: str) -> None: + """A FILE where a folder above the clone path must be - one of + `FILE_ABOVE_THE_CLONE` - is refused in step (3), naming it, and is left + as it was. On main `lexists(/Sample)` was False, so step (3) + passed, all three repositories were created, and the run ended in a + `FileExistsError` traceback where step (7) made the folder.""" + work = tmp_path / "work" + if where == "into": + blocker, family = work, () + else: + work.mkdir() + blocker, family = work / "Zephyr", ("--family", "Zephyr") + blocker.write_bytes(b"not a folder\n") + result = run("--project", "Sample", "--yes", "--org", "demoorg", *family, + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(work)) + assert result.returncode == 2, result.stderr + result.stdout + assert ("REFUSED: %s is a file, and this project would be cloned inside " + "it, at %s, so the run stops here and nothing was created. %s" + % (os.path.abspath(str(blocker)), + os.path.abspath(str(blocker / "Sample")), KEEP_IT) + ) in result.stderr, result.stderr + assert "Traceback" not in result.stderr, result.stderr + for later in ("(4) naming policy", "(5) the plan", "(6) scaffold"): + assert later not in result.stdout, f"the run reached {later}" + assert not (tmp_path / "remotes").exists() + assert blocker.read_bytes() == b"not a folder\n" + + +def assert_a_dry_run_passed_through_ends_at_the_plan(run, tmp_path: Path, + spelling: str, + yes: bool) -> None: + """`-- --dry-run` (`spelling` and `yes`, one of `DRY_RUNS`) ends the run + at the plan, exit 0, whatever `--yes` says and with no terminal to ask on: + nothing is created, nothing is cloned, and no folder is made. On main + the scaffold made nothing, step (7)'s clone failed, and the refusal said + "The three repositories WERE created"; the family folder was left + behind.""" + work = tmp_path / "work" + result = run("--project", "Sample", *(["--yes"] if yes else []), + "--org", "demoorg", "--family", "Zephyr", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(work), "--", spelling) + assert result.returncode == 0, result.stderr + result.stdout + assert DRY_RUN_ENDS_IT in result.stdout, result.stdout + assert "WERE created" not in result.stderr, result.stderr + assert "no terminal to confirm on" not in result.stderr, result.stderr + for later in ("(6) scaffold", "(7) clone and bootstrap", "DONE."): + assert later not in result.stdout, f"the run reached {later}" + assert not (tmp_path / "remotes").exists() + assert not work.exists(), f"{work} was made by a run that made nothing" + + +def assert_the_naming_check_hears_what_is_passed_through(run, tmp_path: Path, + how: str) -> None: + """Step (4) asks the naming check with the `--pin` products and the + `--referent-chain` passed through after `--` (`how`, one of + `DECLARATIONS`), so a descendant-form name is reported as the + descendant the plan then says it is. On main step (4) asked about the + three names alone and reported `no referent pin declared`. The run + stops at the confirm: no terminal, and nothing is created.""" + source = tmp_path / "product" + source.mkdir() + git("init", "-q", "-b", "main", cwd=source) + (source / "README.md").write_bytes(b"# a neutral product\n") + git("add", "README.md", cwd=source) + git("commit", "-q", "-m", "seed", cwd=source) + commit = git("rev-parse", "HEAD", cwd=source).stdout.strip() + project, declared = "MedxGlass", ("--pin", f"openGlass@{commit}") + if how == "--pin=": + declared = (f"--pin=openGlass@{commit}",) + elif how == "--referent-chain": + project = "codexDox" + declared = ("--pin", f"openXdox@{commit}", + "--referent-chain", "openXdox,openDox") + result = run("--project", project, "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), + "--", *declared, "--pin-source", str(source)) + assert result.returncode == 2, result.stderr + result.stdout + assert_refuses_for_want_of_a_terminal(result.stderr) + step_four = result.stdout.split("(4) naming policy", 1)[-1].split( + "(5) the plan", 1)[0] + assert f"{project}: domain-descendant" in step_four, step_four + assert "no referent pin declared" not in step_four, step_four + assert not (tmp_path / "remotes").exists() + + +def assert_the_upstream_org_is_refused_in_any_case(run, tmp_path: Path, + org: str) -> None: + """`--org OpenSoft` (`org`, one of `UPSTREAM_CASINGS`) is refused as + `--org opensoft` is: GitHub reads an organisation's login in any case, + so it is opensoft's namespace. On main the guard compared exactly and + scaffolded.""" + result = run("--project", "Sample", "--yes", "--org", org, + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work")) + assert result.returncode == 2, result.stderr + result.stdout + assert f"the organisation is '{org}', which is the UPSTREAM" in ( + result.stderr), result.stderr + assert "--allow-upstream-org" in result.stderr, result.stderr + assert not (tmp_path / "remotes").exists() + + +def assert_an_upstream_origin_in_any_case_needs_an_org(run, + tmp_path: Path) -> None: + """A checkout whose `origin` is `OpenSoft/openRepoShape` IS the upstream + checkout, and with no `--org` the run refuses as it does for + `opensoft/`. On main the casing went through as the organisation.""" + fork = tmp_path / "fork" + fork.mkdir() + git("init", "-q", cwd=fork) + git("remote", "add", "origin", + "https://github.com/OpenSoft/openRepoShape.git", cwd=fork) + result = run("--project", "Sample", "--yes", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), cwd=fork) + assert result.returncode == 2, result.stderr + result.stdout + assert "you are running from the upstream checkout" in result.stderr, ( + result.stderr) + assert not (tmp_path / "remotes").exists() + + #: THE PERSON'S OWN TERMINAL, IN A TEST (#492). The one-liner reads its yes #: from the CONTROLLING terminal when its stdin is the pipe the script came #: down, so a test that says it asks has to give the run one: a diff --git a/tests/test_scaffold_e2e.py b/tests/test_scaffold_e2e.py index 45d7ce17..f1046bc9 100644 --- a/tests/test_scaffold_e2e.py +++ b/tests/test_scaffold_e2e.py @@ -9,11 +9,14 @@ import os import re +import shlex import subprocess from conftest import (FILE_PROTOCOL, ORG, PROJECT, REPO, SCAFFOLD, WINDOWS_SKIP, assert_read_through_git, git, run_script) +from conftest import indented_commands, pasted, read_through_git + DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -259,6 +262,27 @@ def test_the_scaffold_creates_three_bare_remotes(scaffolded): assert "make bootstrap" in scaffolded["stdout"] +@WINDOWS_SKIP +def test_the_next_steps_clone_under_local_remotes_reads_the_legs(scaffolded, + tmp_path): + """#538: under `--local-remote-dir` the NEXT STEPS clone carries `-c + protocol.file.allow=always`, as setup's own clone does. The legs are + cloned from the plain paths `.gitmodules` records, the file transport, + which git refuses a submodule by default since the 2022 advisories, so + on main the line as printed failed `transport 'file' not allowed`. Run + as printed, with its `cd`, by `sh`.""" + block = scaffolded["stdout"].split("NEXT STEPS", 1)[1] + clone, cd = indented_commands(block)[:2] + remote = (scaffolded["remotes"] / f"{PROJECT}.git").resolve() + assert shlex.split(clone) == ["git", *FILE_PROTOCOL, "clone", + "--recurse-submodules", str(remote)], clone + assert cd == f"cd {PROJECT}", cd + ran = pasted([clone, cd, "git -C spec rev-parse -q --verify HEAD", + "git -C code rev-parse -q --verify HEAD"], cwd=tmp_path, + env=read_through_git(scaffolded["remotes"])) + assert ran.returncode == 0, ran.stderr + ran.stdout + + def test_the_session_clone_was_read_through_git_not_through_the_remotes( scaffolded): """#440: the root and both legs arrived as ONE PACK each. diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index 95481f11..636e6421 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -45,6 +45,17 @@ assert_a_rehearsals_clone_stops_the_real_run, assert_a_taken_clone_path_is_refused) +from conftest import (DECLARATIONS, DRY_RUNS, FILE_ABOVE_THE_CLONE, + UPSTREAM_CASINGS, + assert_a_dry_run_passed_through_ends_at_the_plan, + assert_a_file_above_the_clone_path_is_refused, + assert_an_upstream_origin_in_any_case_needs_an_org, + assert_the_backstop_is_finished_by_hand_as_printed, + assert_the_closing_cd_is_one_word, + assert_the_naming_check_hears_what_is_passed_through, + assert_the_scaffold_leaves_its_next_steps_to_setup, + assert_the_upstream_org_is_refused_in_any_case) + SETUP_PROJECT = REPO / "setup-project.py" SETUP_SH = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -480,6 +491,46 @@ def test_a_taken_clone_path_is_refused_before_anything(tmp_path, kind): assert_a_taken_clone_path_is_refused(run_entry, tmp_path, kind) +# --- #538, on an interpreter alone: each is `conftest`'s helper of that name - + +def test_the_closing_cd_names_the_clone_as_one_word(tmp_path): + assert_the_closing_cd_is_one_word(run_entry, tmp_path) + + +def test_the_backstop_is_finished_by_hand_as_printed(tmp_path): + assert_the_backstop_is_finished_by_hand_as_printed(run_entry, tmp_path) + + +def test_the_scaffold_leaves_its_next_steps_to_setup(setup_run): + assert_the_scaffold_leaves_its_next_steps_to_setup(setup_run["result"]) + + +@pytest.mark.parametrize("where", FILE_ABOVE_THE_CLONE) +def test_a_file_above_the_clone_path_is_refused(tmp_path, where): + assert_a_file_above_the_clone_path_is_refused(run_entry, tmp_path, where) + + +@pytest.mark.parametrize("spelling,yes", DRY_RUNS) +def test_a_dry_run_passed_through_ends_at_the_plan(tmp_path, spelling, yes): + assert_a_dry_run_passed_through_ends_at_the_plan(run_entry, tmp_path, + spelling, yes) + + +@pytest.mark.parametrize("how", DECLARATIONS) +def test_the_naming_check_hears_what_is_passed_through(tmp_path, how): + assert_the_naming_check_hears_what_is_passed_through(run_entry, tmp_path, + how) + + +@pytest.mark.parametrize("org", UPSTREAM_CASINGS) +def test_the_upstream_org_is_refused_in_any_case(tmp_path, org): + assert_the_upstream_org_is_refused_in_any_case(run_entry, tmp_path, org) + + +def test_an_upstream_origin_in_any_case_needs_an_org(tmp_path): + assert_an_upstream_origin_in_any_case_needs_an_org(run_entry, tmp_path) + + def test_anything_but_yes_refuses_and_creates_nothing(monkeypatch, tmp_path): """The one `yes` is EXACT: `no`, `y`, `Yes` and an empty line all refuse. @@ -1865,7 +1916,8 @@ def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, tmp_path, SHOULD-2). Its way out moves the thing aside and clones from the clone's PARENT: as it used to read, the printed `git clone` met the same directory, and `cd ` from anywhere else entered whatever stood - there. Nothing is cloned.""" + there. Each path is one word, and the local clone can read its legs + (#538). Nothing is cloned.""" module = entry_point_module() runs = _Runs() monkeypatch.setattr(module, "run", runs) @@ -1890,15 +1942,150 @@ def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, tmp_path, assert str(refused.value) == ( "%s already exists - it was not there when step (3) looked - so " "there is nowhere to clone into. The three repositories WERE " - "created. Move it aside, then finish by hand in %s:\n" - " git clone --recurse-submodules %s\n" + "created. Move it aside, then finish by hand:\n" + " cd %s\n" + " git -c protocol.file.allow=always clone --recurse-submodules %s\n" " cd Sample && %s scripts/bootstrap.py" - % (parent / "Sample", parent, - os.path.join(opts.local_remote_dir, "Sample.git"), + % (parent / "Sample", module.shell_word(parent), + module.shell_word(os.path.join(opts.local_remote_dir, + "Sample.git")), module.PYTHON_CMD)) assert runs.calls == [], f"something was run: {runs.calls}" +def test_a_folder_step_seven_cannot_make_is_refused_with_the_way_on( + monkeypatch, tmp_path): + """#538: a file that appeared at `/` after step (3) + looked is a refusal naming it, with the lines that finish the run by + hand, not the `FileExistsError` traceback `os.makedirs` raised after all + three repositories existed. Nothing is cloned.""" + module = entry_point_module() + runs = _Runs() + monkeypatch.setattr(module, "run", runs) + opts = module.parse_args(["--project", "Sample", "--org", "demoorg", + "--family", FAMILY, + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work")], + str(tmp_path)) + folder = Path(opts.into) / FAMILY + folder.parent.mkdir(parents=True) + folder.write_bytes(b"not a folder\n") + with pytest.raises(module.Refusal) as refused: + module.clone_and_bootstrap(opts, "demoorg", + module.Landing(folder, FAMILY, + folder / FAMILY)) + assert refused.value.code == 2 + said = str(refused.value) + assert said.startswith("%s could not be made a folder (" % folder), said + assert said.endswith( + "), so there is nowhere to clone into. The three repositories WERE " + "created. Make it one, then finish by hand:\n" + + module.finish_by_hand(str(folder), + os.path.join(opts.local_remote_dir, + "Sample.git"), + "Sample", True)), said + assert runs.calls == [], f"something was run: {runs.calls}" + assert folder.read_bytes() == b"not a folder\n" + + +@pytest.mark.parametrize("case", ["a-file-two-levels-up", "a-link-to-nothing", + "a-link-to-a-folder", "nothing-there"]) +def test_what_stands_above_a_clone_path(tmp_path, case): + """#538, step (3): the first thing on the way up from the clone's parent + must be a folder, or there is nowhere to make the folders between. A + file is said to be one; a link is said to be one, by its target; a link + TO a folder is a folder; and a path nothing stands on at all is left to + the clone, as it was.""" + module = entry_point_module() + blocker = tmp_path / "work" / FAMILY + blocker.parent.mkdir() + clone = blocker / "Sample" + expected = None + if case == "a-file-two-levels-up": + blocker.write_bytes(b"not a folder\n") + clone = blocker / "deeper" / "Sample" + expected = "a file" + elif case == "nothing-there": + clone = blocker / "deeper" / "Sample" + else: + target = tmp_path / ("real" if case == "a-link-to-a-folder" + else "nowhere") + if case == "a-link-to-a-folder": + target.mkdir() + else: + expected = ("a symbolic link to %s, which is not a folder" + % target) + try: + os.symlink(target, blocker, target_is_directory=True) + except (NotImplementedError, OSError) as exc: + pytest.skip(f"this platform cannot create a symlink here: {exc}") + if expected is None: + assert module.refuse_a_taken_clone_path(str(clone), "Sample") is None + return + with pytest.raises(module.Refusal) as refused: + module.refuse_a_taken_clone_path(str(clone), "Sample") + assert refused.value.code == 2 + assert str(refused.value) == ( + "%s is %s, and this project would be cloned inside it, at %s, so the " + "run stops here and nothing was created. %s" + % (blocker, expected, clone, KEEP_IT)) + + +#: 40 hex, for a `--pin` nothing reads the digest of. +A_COMMIT = "0123456789abcdef0123456789abcdef01234567" + + +@pytest.mark.parametrize("passthrough,dry", [ + ([], False), (["--dry-run"], True), (["--d"], True), + (["--work-dir", "w", "--dry-ru"], True), (["--name=--dry-run"], False), + (["--", "--dry-run"], False), (["--d=x"], True), (["--"], False)]) +def test_a_dry_run_is_read_as_the_scaffold_reads_one(passthrough, dry): + """#538: `--dry-run`, or a prefix of it, up to an inner `--`; never a + value given with `=` to another flag.""" + assert entry_point_module().passes_a_dry_run(passthrough) is dry + + +@pytest.mark.parametrize("passthrough,declared", [ + ([], []), + (["--pin", f"acme/openGlass@{A_COMMIT}", f"--pin=openChart@{A_COMMIT}"], + ["--pins=openGlass,openChart"]), + (["--referent-chain", "openA,openB", "--referent=openXdox,openDox"], + ["--referent-chain=openXdox,openDox"]), + (["--pin-owner", "acme", "--pin-source", "src", "--reference", "doc.md", + "--referen", "x"], []), + (["--pin", f"openGlass@{A_COMMIT}", "--", + "--pin", f"openChart@{A_COMMIT}"], ["--pins=openGlass"]), + (["--pin"], []), (["--pin", f"@{A_COMMIT}"], [])]) +def test_the_declarations_are_read_as_the_scaffold_reads_them(passthrough, + declared): + """#538: each `--pin`'s product, at its last `/` and before `@`; the + last `--referent-chain`, in any spelling the scaffold takes for it; and + nothing from a flag that only shares a prefix with one of them.""" + assert entry_point_module().naming_declarations(passthrough) == declared + + +def test_each_passed_flag_is_spelled_as_the_scaffold_takes_it(): + """`PASSED_FLAGS` against the scaffold's own `--help`: each flag is one + of its options; a shortest spelling shorter than the flag names that + flag alone; and one character less is not that flag alone any more, so + the row is the shortest. A flag the scaffold renamed, or one it grew a + sibling of, fails here instead of being misread.""" + module = entry_point_module() + helped = subprocess.run([sys.executable, str(REPO / "scaffold-project.py"), + "--help"], capture_output=True, text=True, + check=True).stdout + usage = helped.split("\n\n", 1)[0] + options = sorted(set(re.findall(r"--[a-z][a-z-]*", usage))) + for flag, shortest in module.PASSED_FLAGS.items(): + assert flag in options, flag + assert flag.startswith(shortest), flag + if shortest != flag: + assert [o for o in options if o.startswith(shortest)] == [flag] + if len(shortest) > 3: + assert [o for o in options if o.startswith(shortest[:-1])] != [ + flag], flag + + # --- setup.sh is a shim, not a second flag list ----------------------------- #: The only flags `setup.sh` may still NAME, and why each of them is about the diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 9e2c300f..4722eb65 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -32,6 +32,17 @@ assert_a_rehearsals_clone_stops_the_real_run, assert_a_taken_clone_path_is_refused) +from conftest import (DECLARATIONS, DRY_RUNS, FILE_ABOVE_THE_CLONE, + UPSTREAM_CASINGS, + assert_a_dry_run_passed_through_ends_at_the_plan, + assert_a_file_above_the_clone_path_is_refused, + assert_an_upstream_origin_in_any_case_needs_an_org, + assert_the_backstop_is_finished_by_hand_as_printed, + assert_the_closing_cd_is_one_word, + assert_the_naming_check_hears_what_is_passed_through, + assert_the_scaffold_leaves_its_next_steps_to_setup, + assert_the_upstream_org_is_refused_in_any_case) + SETUP = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -410,6 +421,46 @@ def test_a_taken_clone_path_is_refused_before_anything(tmp_path, kind): assert_a_taken_clone_path_is_refused(run_setup, tmp_path, kind) +# --- #538, through the shim: each is `conftest`'s helper of that name ------- + +def test_the_closing_cd_names_the_clone_as_one_word(tmp_path): + assert_the_closing_cd_is_one_word(run_setup, tmp_path) + + +def test_the_backstop_is_finished_by_hand_as_printed(tmp_path): + assert_the_backstop_is_finished_by_hand_as_printed(run_setup, tmp_path) + + +def test_the_scaffold_leaves_its_next_steps_to_setup(setup_run): + assert_the_scaffold_leaves_its_next_steps_to_setup(setup_run["result"]) + + +@pytest.mark.parametrize("where", FILE_ABOVE_THE_CLONE) +def test_a_file_above_the_clone_path_is_refused(tmp_path, where): + assert_a_file_above_the_clone_path_is_refused(run_setup, tmp_path, where) + + +@pytest.mark.parametrize("spelling,yes", DRY_RUNS) +def test_a_dry_run_passed_through_ends_at_the_plan(tmp_path, spelling, yes): + assert_a_dry_run_passed_through_ends_at_the_plan(run_setup, tmp_path, + spelling, yes) + + +@pytest.mark.parametrize("how", DECLARATIONS) +def test_the_naming_check_hears_what_is_passed_through(tmp_path, how): + assert_the_naming_check_hears_what_is_passed_through(run_setup, tmp_path, + how) + + +@pytest.mark.parametrize("org", UPSTREAM_CASINGS) +def test_the_upstream_org_is_refused_in_any_case(tmp_path, org): + assert_the_upstream_org_is_refused_in_any_case(run_setup, tmp_path, org) + + +def test_an_upstream_origin_in_any_case_needs_an_org(tmp_path): + assert_an_upstream_origin_in_any_case_needs_an_org(run_setup, tmp_path) + + def test_no_test_here_depends_on_gh_being_authenticated(): """Every invocation either stops before the preflight or runs offline. From 7baf16b06302aab6779feeb24ab3dafe37721ef3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:47:29 +0000 Subject: [PATCH 2/4] setup-project.py: the paste lines it adds or changes are spelled for `sh` on every host, so a Windows path reaches Git Bash whole, and the link-to-nothing test reads the target as Windows returns it (#538) Review of PR #569 at c71eadb found two things to fix in the PR. The hand-over's `cd ` and step (7)'s `cd ` and `` went through `shell_word`, which quotes for PowerShell on Windows: it left `C:\work\Sample` bare, which Git Bash reads as `C:workSample`, and doubled the apostrophe of `Jane O'Neill`, which sh reads as `ONeill`. #532 rules the printed commands POSIX-only, and this PR already prints the scaffold's NEXT STEPS clone with `shell_line(..., "posix")`. `shell_word` takes a `posix` switch now, over a copy of the standard's `SHELL_BARE_POSIX` (this file runs before any checkout of the standard exists), and these three words pass it. The family lines (`family_next_command`) are #532's and still take the host's dialect. The new test pins the Windows spelling in-process, with `os.name` and `SHELL_BARE` switched for the calls, and reads each printed line back with `shlex.split`. It is red on c71eadb in all three cases. The existing backstop test now expects the POSIX spelling on every host. `test_what_stands_above_a_clone_path[a-link-to-nothing]` compared the refusal with the link's target as it was given. Since Python 3.8 `os.readlink` on Windows returns the substitute name with a `\\?\` prefix, and the refusal names that, so it now compares with `os.readlink(blocker)`. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 41 +++++++++++++------- tests/test_setup_project_py.py | 70 +++++++++++++++++++++++++++++++--- 2 files changed, 93 insertions(+), 18 deletions(-) diff --git a/setup-project.py b/setup-project.py index 40915e02..bcd485f9 100755 --- a/setup-project.py +++ b/setup-project.py @@ -2163,18 +2163,21 @@ def finish_by_hand(parent: str, assembly: str, project: str, EVERY PATH IS ONE WORD (#538), through `shell_word`, as the family lines are (#486): under `--into "/home/a b"` the bare lines split at the space, - and under `--local-remote-dir` `` is a path too. And THAT clone - carries `-c protocol.file.allow=always`, which setup's own clone passes: - the legs are cloned from the plain paths `.gitmodules` records, the file - transport, which git refuses a submodule without it. The project name is - a word already: step (4) held it to the naming policy. + and under `--local-remote-dir` `` is a path too. THE WORD IS + SPELLED FOR `sh` ON EVERY HOST (`posix=True`; #532 rules the printed + commands POSIX-only), so a Windows path reaches Git Bash whole. And THAT + clone carries `-c protocol.file.allow=always`, which setup's own clone + passes: the legs are cloned from the plain paths `.gitmodules` records, + the file transport, which git refuses a submodule without it. The project + name is a word already: step (4) held it to the naming policy. """ clone = "git -c protocol.file.allow=always clone" if local else "git clone" return (" cd %s\n" " %s --recurse-submodules %s\n" " cd %s && %s %s" - % (shell_word(parent), clone, shell_word(assembly), project, - PYTHON_CMD, BOOTSTRAP)) + % (shell_word(parent, posix=True), clone, + shell_word(assembly, posix=True), project, PYTHON_CMD, + BOOTSTRAP)) def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: @@ -2296,24 +2299,36 @@ def clone_and_bootstrap(opts: Options, org: str, landing: Landing) -> dict: # 8. hand over # --------------------------------------------------------------------------- +#: A word `sh` reads as itself, needing no quotes: `scripts/repo_shape.py`'s +#: `SHELL_BARE_POSIX`. A backslash is not in it, so a Windows path is quoted. +SHELL_BARE_POSIX = re.compile(r"[A-Za-z0-9_./:@%+=,-]+") + #: A word this host's shell reads as itself, needing no quotes: #: `scripts/repo_shape.py`'s `SHELL_BARE_NT` on Windows, else its #: `SHELL_BARE_POSIX`. This file runs before any checkout of the standard #: exists, so it carries its own copy of the rule, and #: `tests/test_family_printed_quoting.py` holds the two to one answer. SHELL_BARE = re.compile(r"[A-Za-z0-9_./:%+=\\-]+" if os.name == "nt" - else r"[A-Za-z0-9_./:@%+=,-]+") + else SHELL_BARE_POSIX.pattern) -def shell_word(value) -> str: +def shell_word(value, posix: bool = False) -> str: """`value` as ONE word of a command pasted into this host's shell: bare where every character is itself to it, else in single quotes, doubled inside for PowerShell, closed-escaped-reopened for `sh` (#486). A - leading `=` is quoted too, which zsh would expand.""" + leading `=` is quoted too, which zsh would expand. + + `posix` spells it for `sh` on EVERY host, Windows too: Git Bash reads + the backslash of a bare `C:\\work` as an escape and an apostrophe doubled + for PowerShell as two quotes, and #532 rules the printed commands + POSIX-only. The lines #538 adds or changes pass it, as the scaffold's + NEXT STEPS clone does (`shell_line(..., "posix")`); the family lines + (`family_next_command`) are #532's and still take this host's dialect.""" text = str(value) - if not text.startswith("=") and SHELL_BARE.fullmatch(text): + bare = SHELL_BARE_POSIX if posix else SHELL_BARE + if not text.startswith("=") and bare.fullmatch(text): return text - if os.name == "nt": + if os.name == "nt" and not posix: return "'" + re.sub("(['\u2018\u2019\u201a\u201b])", r"\1\1", text) + "'" return "'" + text.replace("'", "'\\''") + "'" @@ -2374,7 +2389,7 @@ def hand_over(project: str, clone: str, urls: dict, org: str, say("") # ONE WORD (#538): a clone under `--into "/home/a b"` is a `cd` the # shell would split. - say(" cd %s" % shell_word(clone)) + say(" cd %s" % shell_word(clone, posix=True)) say(" make validate # naming, manifest, lockstep pins - what " "CI runs") # setup.sh's hand-over block, with ONE line added: this file runs where diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index 636e6421..d66fc8bb 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -25,6 +25,7 @@ import importlib.util import os import re +import shlex import shutil import subprocess import sys @@ -1946,9 +1947,9 @@ def test_a_path_taken_after_step_three_is_the_backstops(monkeypatch, tmp_path, " cd %s\n" " git -c protocol.file.allow=always clone --recurse-submodules %s\n" " cd Sample && %s scripts/bootstrap.py" - % (parent / "Sample", module.shell_word(parent), + % (parent / "Sample", module.shell_word(parent, posix=True), module.shell_word(os.path.join(opts.local_remote_dir, - "Sample.git")), + "Sample.git"), posix=True), module.PYTHON_CMD)) assert runs.calls == [], f"something was run: {runs.calls}" @@ -1988,6 +1989,62 @@ def test_a_folder_step_seven_cannot_make_is_refused_with_the_way_on( assert folder.read_bytes() == b"not a folder\n" +#: `(a Windows clone path, the word the printed lines spell it as)`. The +#: spelling is `sh`'s: Git Bash is the shell a Windows person pastes these +#: into, and #532 rules the printed commands POSIX-only. PowerShell's rule +#: leaves the first bare, so Git Bash reads `C:workSample`, and doubles the +#: apostrophe of the third, so Git Bash reads `ONeill`. +WINDOWS_CLONES = ( + (r"C:\work\Sample", r"'C:\work\Sample'"), + (r"C:\Users\a b\work\Sample", r"'C:\Users\a b\work\Sample'"), + (r"C:\Users\Jane O'Neill\work\Sample", + r"'C:\Users\Jane O'\''Neill\work\Sample'"), +) +WINDOWS_ASSEMBLY = (r"C:\Users\Jane O'Neill\remotes\Sample.git", + r"'C:\Users\Jane O'\''Neill\remotes\Sample.git'") + + +@pytest.mark.parametrize("clone,spelled", WINDOWS_CLONES) +def test_the_changed_paste_lines_are_posix_quoted_on_windows_too( + monkeypatch, capsys, clone, spelled): + """#538, under #532's ruling that printed commands are POSIX-only: the + hand-over's `cd ` and the backstop's `cd ` and `` + are spelled for `sh` on every host, as the scaffold's own NEXT STEPS + clone is (`shell_line(..., "posix")`). `os.name` and `SHELL_BARE` are + switched for the calls, the two things `shell_word` reads to choose + PowerShell's dialect, so this holds on any host; the lines are then read + back by `shlex.split`, as Git Bash reads them. The family lines are + #532's and are not asked.""" + module = entry_point_module() + assembly, assembly_spelled = WINDOWS_ASSEMBLY + urls = {"assembly": assembly, "spec": assembly, "code": assembly} + monkeypatch.setattr(os, "name", "nt") + monkeypatch.setattr(module, "SHELL_BARE", + re.compile(r"[A-Za-z0-9_./:%+=\\-]+")) + module.hand_over("Sample", clone, urls, "demoorg", + module.Landing(None, "", None)) + local = module.finish_by_hand(clone, assembly, "Sample", True) + github = module.finish_by_hand( + clone, "https://github.com/demoorg/Sample.git", "Sample", False) + monkeypatch.undo() + [closing] = [line for line in capsys.readouterr().out.splitlines() + if line.startswith(" cd ")] + assert closing == " cd " + spelled, closing + assert shlex.split(closing) == ["cd", clone], closing + assert local == ( + " cd %s\n" + " git -c protocol.file.allow=always clone --recurse-submodules %s\n" + " cd Sample && %s scripts/bootstrap.py" + % (spelled, assembly_spelled, module.PYTHON_CMD)), local + assert [shlex.split(line) for line in local.splitlines()[:2]] == [ + ["cd", clone], + ["git", *FILE_PROTOCOL, "clone", "--recurse-submodules", assembly]] + assert github.splitlines()[:2] == [ + " cd " + spelled, + " git clone --recurse-submodules " + "https://github.com/demoorg/Sample.git"], github + + @pytest.mark.parametrize("case", ["a-file-two-levels-up", "a-link-to-nothing", "a-link-to-a-folder", "nothing-there"]) def test_what_stands_above_a_clone_path(tmp_path, case): @@ -2012,13 +2069,16 @@ def test_what_stands_above_a_clone_path(tmp_path, case): else "nowhere") if case == "a-link-to-a-folder": target.mkdir() - else: - expected = ("a symbolic link to %s, which is not a folder" - % target) try: os.symlink(target, blocker, target_is_directory=True) except (NotImplementedError, OSError) as exc: pytest.skip(f"this platform cannot create a symlink here: {exc}") + if case == "a-link-to-nothing": + # THE TARGET AS THE LINK HOLDS IT, not as it was given: + # `os.readlink` on Windows returns the substitute name with its + # `\\?\` prefix (Python 3.8+), and the refusal names that. + expected = ("a symbolic link to %s, which is not a folder" + % os.readlink(blocker)) if expected is None: assert module.refuse_a_taken_clone_path(str(clone), "Sample") is None return From df67b4625841dd0cec326b99dd1a9a845b3175b8 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 15:32:17 +0000 Subject: [PATCH 3/4] tests/test_setup_by_hand_commands.py: `what_setup_runs` asserts the real run is the plan's arguments and a final `--no-next-steps`, and returns it without that flag, so #538's flag no longer reds the by-hand guard while the README's and the handbook's lines stay the ones a person runs (#538) #579 (#550) landed this guard after #569's last CI: it runs `plan_and_confirm` and `scaffold` with `run` recorded and holds README.md's and the handbook's by-hand `scaffold-project.py` lines, and `plan == real + " --dry-run"`, to what setup runs. #538 makes setup's real run carry `--no-next-steps`, because step (7) runs the closing clone, `cd` and `make bootstrap` itself, so three tests went red on main's merge (the README's, the handbook's and the plan-and-real one). By hand there is no step (7): the scaffold's closing block is the person's next work, so the documents' lines correctly omit the flag, and no README or handbook word changes (four open PRs touch both, and each landing would force another conflicted merge here). The test says the difference instead: `recorded_scaffold_runs` keeps the argument vectors as built; `what_setup_runs` asserts the plan ends `--dry-run` and the real run is the plan's arguments plus exactly one final `--no-next-steps`, then returns the real run without it, the reason in its docstring and the module's. A flag added to the plan or the real run alone, or the flag moved, still fails. Mutants run: the flag dropped, an extra flag on the real run, and the flag moved first each fail three of the five tests. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_setup_by_hand_commands.py | 49 +++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/tests/test_setup_by_hand_commands.py b/tests/test_setup_by_hand_commands.py index ec5e42fb..047a954a 100644 --- a/tests/test_setup_by_hand_commands.py +++ b/tests/test_setup_by_hand_commands.py @@ -24,6 +24,15 @@ `--local-remote-dir`) is in neither: the printed lines show the default run, and so do the recorded ones. +One flag is on the real run alone and is left out of the printed line ON +PURPOSE: `--no-next-steps` (#538). `setup-project.py` runs the scaffold's +closing block, the `git clone`, the `cd` and `make bootstrap`, itself as its +step (7), so it tells the scaffold to leave that block out. By hand there is no +step (7): the block IS the person's next work, so the documents' lines do not +carry the flag. `what_setup_runs` asserts the flag is there, last, and that +nothing else differs from the plan's arguments, then returns the real run +without it. + Nothing here starts a process, touches git or GitHub, or reads a terminal, so it holds on every platform; the printed commands are POSIX and are rendered with `shlex.quote`, which is what they are written in. @@ -57,6 +66,10 @@ SCAFFOLD_COMMAND = "python3 scaffold-project.py" +#: What the real run carries and the printed line does not: see the module +#: docstring. +NO_NEXT_STEPS = "--no-next-steps" + def scaffold_lines(block: str) -> list[str]: """The `scaffold-project.py` commands in a printed block, each as one line @@ -70,10 +83,10 @@ def scaffold_lines(block: str) -> list[str]: return lines -def what_setup_runs(monkeypatch, tmp_path: Path) -> list[str]: - """The two `scaffold-project.py` commands `setup-project.py` runs for the - documents' own values, as `python3 scaffold-project.py ` lines: - the plan first, then the real run.""" +def recorded_scaffold_runs(monkeypatch, tmp_path: Path) -> list[list[str]]: + """The argument vectors `setup-project.py` hands `run` for the plan and the + real run, for the documents' own values: the plan first, then the real run, + each exactly as built, interpreter and script included.""" module = entry_point_module() ran: list[list[str]] = [] @@ -92,8 +105,31 @@ def record(argv, *args, **kwargs): module.plan_and_confirm(opts, REPO, ORG, args, str(tmp_path / "Atlas")) module.scaffold(REPO, args) assert len(ran) == 2, ran + return ran + + +def what_setup_runs(monkeypatch, tmp_path: Path) -> list[str]: + """The two `scaffold-project.py` commands `setup-project.py` runs for the + documents' own values, as `python3 scaffold-project.py ` lines: + the plan first, then the real run. + + THE REAL RUN IS RETURNED WITHOUT `--no-next-steps` (#538), after it is + asserted to be there. setup's step (7) clones, `cd`s and bootstraps + itself, so it has the scaffold leave out the closing block that prints the + same three commands, which printed in the middle of setup read as work + left to the person. By hand there is no step (7) and that block is the + person's next work, so the documents' lines stay without the flag, and + this compares them with the run it would be the same as. The assertion + keeps the exception exact: the flag is last, and the real run is otherwise + the plan's arguments, so a flag added to one of the two alone still fails + here.""" + plan, real = recorded_scaffold_runs(monkeypatch, tmp_path) + assert plan[-1] == "--dry-run", plan + assert real == [*plan[:-1], NO_NEXT_STEPS], ( + "the real run is the plan's arguments and a final " + f"{NO_NEXT_STEPS}, nothing else:\n {plan}\n {real}") lines = [] - for argv in ran: + for argv in (plan, real[:-1]): assert Path(argv[1]).name == "scaffold-project.py", argv lines.append(" ".join([SCAFFOLD_COMMAND, *(shlex.quote(part) for part in argv[2:])])) @@ -133,6 +169,9 @@ def test_the_handbook_prints_the_commands_setup_runs(monkeypatch, tmp_path): def test_setup_runs_the_plan_and_the_real_run_with_the_same_arguments( monkeypatch, tmp_path): + recorded = recorded_scaffold_runs(monkeypatch, tmp_path) + assert recorded[1][-1] == NO_NEXT_STEPS, recorded[1] + assert NO_NEXT_STEPS not in recorded[0], recorded[0] plan, real = what_setup_runs(monkeypatch, tmp_path) assert plan == real + " --dry-run" assert "--visibility private" in real and "--elected-by 'Your Name'" in real From 0b6228f38a5401178b57a97b03c892483ffe5c50 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 15 Oct 2026 15:33:52 +0000 Subject: [PATCH 4/4] tests/conftest.py, tests/test_setup_project_py.py, tests/test_setup_sh.py: #569's three #538 blocks sit where #573 inserts nothing, so the merge after #573 lands is clean and needs no new three-OS run (#538) A PURE MOVE; nothing inside a block changes. #569 and #573 both insert at the point after `assert_a_taken_clone_path_is_refused` (conftest.py) and after `test_a_taken_clone_path_is_refused_before_anything` (test_setup_project_py.py and test_setup_sh.py), so once #573 lands, #569 conflicts with main in all three files, and a conflicted merge is a new H0 that needs a full three-OS run (Windows alone takes over an hour and a half). With #569's blocks elsewhere, the merge after #573 is clean and, by ruling 27, needs only Linux and SonarCloud. Moved: tests/conftest.py the `# --- setup's paste lines, its step checks and the upstream guard (#538)` block, to the end of the file tests/test_setup_project_py.py the `# --- #538, on an interpreter alone` wrappers, to just before `# --- setup.sh is a shim, not a second flag list` tests/test_setup_sh.py the `# --- #538, through the shim` wrappers, to just before `# --- self- bootstrap` Checked: each file's lines are the same multiset before and after and so are its top-level definitions; pyflakes is clean; no name the conftest block defines is used at import time outside it (it moved down, so what it needs is still above it); `git merge-tree --write-tree` of this head with the predicted main bd0c6b5c reports no conflict. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 254 ++++++++++++++++----------------- tests/test_setup_project_py.py | 80 +++++------ tests/test_setup_sh.py | 70 ++++----- 3 files changed, 202 insertions(+), 202 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8e4115ac..99eda321 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1021,6 +1021,133 @@ def assert_a_taken_clone_path_is_refused(run, tmp_path: Path, assert after == before, f"{taken} was changed by a run that refused" +#: THE PERSON'S OWN TERMINAL, IN A TEST (#492). The one-liner reads its yes +#: from the CONTROLLING terminal when its stdin is the pipe the script came +#: down, so a test that says it asks has to give the run one: a +#: pseudo-terminal, made the controlling terminal of a session of the run's +#: own. `subprocess` hands a child file descriptors and nothing more, so +#: `_CONTROLLING`, run first by this interpreter, claims the terminal on its +#: stdin (`TIOCSCTTY`) and then becomes the command. POSIX only: Windows has +#: no controlling terminal to give, and its way in is the downloaded file. +PTY_SKIP = pytest.mark.skipif( + os.name == "nt", + reason="a controlling terminal is POSIX; on Windows the way in is the " + "downloaded setup-project.py, and there is no /dev/tty to ask on") + +_CONTROLLING = ("import fcntl, os, sys, termios; " + "fcntl.ioctl(0, termios.TIOCSCTTY, 0); " + "os.execvp(sys.argv[1], sys.argv[1:])") + +#: The same terminal, the command run in a BACKGROUND process group of it: a +#: piped run, `cat setup.sh | timeout N bash -s -- ...`, typed in a shell +#: (GNU `timeout` puts its command in a group of its own), or any runner that +#: backgrounds a job sharing the terminal. `_CONTROLLING_IN_THE_BACKGROUND` +#: stays the session's leader and its foreground group, and the command is a +#: child in a group of its own, so a read of the terminal from it stops on +#: SIGTTIN. A stopped command is killed and shows as a status of 128 + the +#: signal, so a run that waits where it should have refused fails at once and +#: leaves nothing stopped behind it. +_CONTROLLING_IN_THE_BACKGROUND = """ +import fcntl, os, signal, sys, termios +fcntl.ioctl(0, termios.TIOCSCTTY, 0) +child = os.fork() +if child == 0: + os.setpgid(0, 0) + os.execvp(sys.argv[1], sys.argv[1:]) +status = os.waitpid(child, os.WUNTRACED)[1] +if os.WIFSTOPPED(status): + os.killpg(child, signal.SIGKILL) + sys.exit(128 + os.WSTOPSIG(status)) +sys.exit(os.WEXITSTATUS(status) if os.WIFEXITED(status) + else 128 + os.WTERMSIG(status)) +""" + + +def run_in_a_terminal(argv: list, *, cwd: Path, env: dict, answers=(), + prompt="Type yes to continue:", + timeout: float = 180, raw: bool = False, + background: bool = False) -> tuple: + """`argv` run as a person runs it in their own terminal: a + pseudo-terminal is its controlling terminal and its stdin, stdout and + stderr, so a pipe inside `argv` (`cat setup.sh | bash -s -- ...`) is the + only thing that is not the terminal. Each of `answers` is typed once the + screen shows its prompt after the answer before it - `prompt` is one + prompt for every answer, or one per answer, in order - `"EOF"` as Ctrl-D, + and nothing is typed anywhere else. + + Returns `(exit status, the screen, how many answers were typed)`, with + the terminal's `\\r\\n` read as `\\n`. A run still going after `timeout` + seconds is killed and FAILS the test with its screen: a run waiting on a + prompt nobody answers is the defect this exists to show. Reading stops + once the run has exited and its output is drained, so a detached child + still holding the terminal cannot keep the test waiting. + + `raw` puts the terminal in RAW mode before the run starts, as an + assistant's full-screen interface leaves the pane it owns: nothing is + echoed and the Enter key sends a carriage return, so a read for the end + of a line never ends. `background` runs the command in a background + process group of the terminal, not its foreground one. + """ + master, slave = os.openpty() + if raw: + import tty # POSIX only, like the pty itself + tty.setraw(slave) + try: + proc = subprocess.Popen([sys.executable, "-c", + _CONTROLLING_IN_THE_BACKGROUND if background + else _CONTROLLING, *argv], + cwd=str(cwd), env=env, stdin=slave, + stdout=slave, stderr=slave, + start_new_session=True) + finally: + os.close(slave) + prompts = ([prompt] * len(answers) if isinstance(prompt, str) + else list(prompt)) + screen, typed, since = b"", 0, 0 + deadline = time.monotonic() + timeout + try: + while True: + left = deadline - time.monotonic() + if left <= 0: + raise AssertionError( + f"still running after {timeout}s, waiting on its " + "terminal:\n" + screen.decode("utf-8", "replace")) + if not select.select([master], [], [], min(left, 1.0))[0]: + if proc.poll() is not None: + break + continue + try: + chunk = os.read(master, 4096) + except OSError: # EIO: the last holder of the terminal is gone + break + if not chunk: + break + screen += chunk + if typed < len(answers) and \ + prompts[typed].encode("utf-8") in screen[since:]: + answer = answers[typed] + os.write(master, b"\x04" if answer == "EOF" + else answer.encode("utf-8")) + typed, since = typed + 1, len(screen) + status = proc.wait(timeout=max(deadline - time.monotonic(), 1.0)) + finally: + if proc.poll() is None: + os.killpg(proc.pid, signal.SIGKILL) + proc.wait() + os.close(master) + return (status, screen.decode("utf-8", "replace").replace("\r\n", "\n"), + typed) + + +def spelled(path) -> str: + """`path` as `adopt-project.py` names one in a finding, a refusal or the + late-failure notice (#212): between backticks, with a backslash or a + backtick escaped. A path a test builds is otherwise printable, which + `_spelled` writes as it is; a Windows path keeps its separators, each + one escaped.""" + return "`" + str(path).replace("\\", "\\\\").replace("`", "\\`") + "`" + + # --- setup's paste lines, its step checks and the upstream guard (#538) ------ # # ONE READING, TWO ENTRY POINTS, as for #498 above: each helper takes the @@ -1286,130 +1413,3 @@ def assert_an_upstream_origin_in_any_case_needs_an_org(run, assert "you are running from the upstream checkout" in result.stderr, ( result.stderr) assert not (tmp_path / "remotes").exists() - - -#: THE PERSON'S OWN TERMINAL, IN A TEST (#492). The one-liner reads its yes -#: from the CONTROLLING terminal when its stdin is the pipe the script came -#: down, so a test that says it asks has to give the run one: a -#: pseudo-terminal, made the controlling terminal of a session of the run's -#: own. `subprocess` hands a child file descriptors and nothing more, so -#: `_CONTROLLING`, run first by this interpreter, claims the terminal on its -#: stdin (`TIOCSCTTY`) and then becomes the command. POSIX only: Windows has -#: no controlling terminal to give, and its way in is the downloaded file. -PTY_SKIP = pytest.mark.skipif( - os.name == "nt", - reason="a controlling terminal is POSIX; on Windows the way in is the " - "downloaded setup-project.py, and there is no /dev/tty to ask on") - -_CONTROLLING = ("import fcntl, os, sys, termios; " - "fcntl.ioctl(0, termios.TIOCSCTTY, 0); " - "os.execvp(sys.argv[1], sys.argv[1:])") - -#: The same terminal, the command run in a BACKGROUND process group of it: a -#: piped run, `cat setup.sh | timeout N bash -s -- ...`, typed in a shell -#: (GNU `timeout` puts its command in a group of its own), or any runner that -#: backgrounds a job sharing the terminal. `_CONTROLLING_IN_THE_BACKGROUND` -#: stays the session's leader and its foreground group, and the command is a -#: child in a group of its own, so a read of the terminal from it stops on -#: SIGTTIN. A stopped command is killed and shows as a status of 128 + the -#: signal, so a run that waits where it should have refused fails at once and -#: leaves nothing stopped behind it. -_CONTROLLING_IN_THE_BACKGROUND = """ -import fcntl, os, signal, sys, termios -fcntl.ioctl(0, termios.TIOCSCTTY, 0) -child = os.fork() -if child == 0: - os.setpgid(0, 0) - os.execvp(sys.argv[1], sys.argv[1:]) -status = os.waitpid(child, os.WUNTRACED)[1] -if os.WIFSTOPPED(status): - os.killpg(child, signal.SIGKILL) - sys.exit(128 + os.WSTOPSIG(status)) -sys.exit(os.WEXITSTATUS(status) if os.WIFEXITED(status) - else 128 + os.WTERMSIG(status)) -""" - - -def run_in_a_terminal(argv: list, *, cwd: Path, env: dict, answers=(), - prompt="Type yes to continue:", - timeout: float = 180, raw: bool = False, - background: bool = False) -> tuple: - """`argv` run as a person runs it in their own terminal: a - pseudo-terminal is its controlling terminal and its stdin, stdout and - stderr, so a pipe inside `argv` (`cat setup.sh | bash -s -- ...`) is the - only thing that is not the terminal. Each of `answers` is typed once the - screen shows its prompt after the answer before it - `prompt` is one - prompt for every answer, or one per answer, in order - `"EOF"` as Ctrl-D, - and nothing is typed anywhere else. - - Returns `(exit status, the screen, how many answers were typed)`, with - the terminal's `\\r\\n` read as `\\n`. A run still going after `timeout` - seconds is killed and FAILS the test with its screen: a run waiting on a - prompt nobody answers is the defect this exists to show. Reading stops - once the run has exited and its output is drained, so a detached child - still holding the terminal cannot keep the test waiting. - - `raw` puts the terminal in RAW mode before the run starts, as an - assistant's full-screen interface leaves the pane it owns: nothing is - echoed and the Enter key sends a carriage return, so a read for the end - of a line never ends. `background` runs the command in a background - process group of the terminal, not its foreground one. - """ - master, slave = os.openpty() - if raw: - import tty # POSIX only, like the pty itself - tty.setraw(slave) - try: - proc = subprocess.Popen([sys.executable, "-c", - _CONTROLLING_IN_THE_BACKGROUND if background - else _CONTROLLING, *argv], - cwd=str(cwd), env=env, stdin=slave, - stdout=slave, stderr=slave, - start_new_session=True) - finally: - os.close(slave) - prompts = ([prompt] * len(answers) if isinstance(prompt, str) - else list(prompt)) - screen, typed, since = b"", 0, 0 - deadline = time.monotonic() + timeout - try: - while True: - left = deadline - time.monotonic() - if left <= 0: - raise AssertionError( - f"still running after {timeout}s, waiting on its " - "terminal:\n" + screen.decode("utf-8", "replace")) - if not select.select([master], [], [], min(left, 1.0))[0]: - if proc.poll() is not None: - break - continue - try: - chunk = os.read(master, 4096) - except OSError: # EIO: the last holder of the terminal is gone - break - if not chunk: - break - screen += chunk - if typed < len(answers) and \ - prompts[typed].encode("utf-8") in screen[since:]: - answer = answers[typed] - os.write(master, b"\x04" if answer == "EOF" - else answer.encode("utf-8")) - typed, since = typed + 1, len(screen) - status = proc.wait(timeout=max(deadline - time.monotonic(), 1.0)) - finally: - if proc.poll() is None: - os.killpg(proc.pid, signal.SIGKILL) - proc.wait() - os.close(master) - return (status, screen.decode("utf-8", "replace").replace("\r\n", "\n"), - typed) - - -def spelled(path) -> str: - """`path` as `adopt-project.py` names one in a finding, a refusal or the - late-failure notice (#212): between backticks, with a backslash or a - backtick escaped. A path a test builds is otherwise printable, which - `_spelled` writes as it is; a Windows path keeps its separators, each - one escaped.""" - return "`" + str(path).replace("\\", "\\\\").replace("`", "\\`") + "`" diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index d66fc8bb..7bb59862 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -492,46 +492,6 @@ def test_a_taken_clone_path_is_refused_before_anything(tmp_path, kind): assert_a_taken_clone_path_is_refused(run_entry, tmp_path, kind) -# --- #538, on an interpreter alone: each is `conftest`'s helper of that name - - -def test_the_closing_cd_names_the_clone_as_one_word(tmp_path): - assert_the_closing_cd_is_one_word(run_entry, tmp_path) - - -def test_the_backstop_is_finished_by_hand_as_printed(tmp_path): - assert_the_backstop_is_finished_by_hand_as_printed(run_entry, tmp_path) - - -def test_the_scaffold_leaves_its_next_steps_to_setup(setup_run): - assert_the_scaffold_leaves_its_next_steps_to_setup(setup_run["result"]) - - -@pytest.mark.parametrize("where", FILE_ABOVE_THE_CLONE) -def test_a_file_above_the_clone_path_is_refused(tmp_path, where): - assert_a_file_above_the_clone_path_is_refused(run_entry, tmp_path, where) - - -@pytest.mark.parametrize("spelling,yes", DRY_RUNS) -def test_a_dry_run_passed_through_ends_at_the_plan(tmp_path, spelling, yes): - assert_a_dry_run_passed_through_ends_at_the_plan(run_entry, tmp_path, - spelling, yes) - - -@pytest.mark.parametrize("how", DECLARATIONS) -def test_the_naming_check_hears_what_is_passed_through(tmp_path, how): - assert_the_naming_check_hears_what_is_passed_through(run_entry, tmp_path, - how) - - -@pytest.mark.parametrize("org", UPSTREAM_CASINGS) -def test_the_upstream_org_is_refused_in_any_case(tmp_path, org): - assert_the_upstream_org_is_refused_in_any_case(run_entry, tmp_path, org) - - -def test_an_upstream_origin_in_any_case_needs_an_org(tmp_path): - assert_an_upstream_origin_in_any_case_needs_an_org(run_entry, tmp_path) - - def test_anything_but_yes_refuses_and_creates_nothing(monkeypatch, tmp_path): """The one `yes` is EXACT: `no`, `y`, `Yes` and an empty line all refuse. @@ -2146,6 +2106,46 @@ def test_each_passed_flag_is_spelled_as_the_scaffold_takes_it(): flag], flag +# --- #538, on an interpreter alone: each is `conftest`'s helper of that name - + +def test_the_closing_cd_names_the_clone_as_one_word(tmp_path): + assert_the_closing_cd_is_one_word(run_entry, tmp_path) + + +def test_the_backstop_is_finished_by_hand_as_printed(tmp_path): + assert_the_backstop_is_finished_by_hand_as_printed(run_entry, tmp_path) + + +def test_the_scaffold_leaves_its_next_steps_to_setup(setup_run): + assert_the_scaffold_leaves_its_next_steps_to_setup(setup_run["result"]) + + +@pytest.mark.parametrize("where", FILE_ABOVE_THE_CLONE) +def test_a_file_above_the_clone_path_is_refused(tmp_path, where): + assert_a_file_above_the_clone_path_is_refused(run_entry, tmp_path, where) + + +@pytest.mark.parametrize("spelling,yes", DRY_RUNS) +def test_a_dry_run_passed_through_ends_at_the_plan(tmp_path, spelling, yes): + assert_a_dry_run_passed_through_ends_at_the_plan(run_entry, tmp_path, + spelling, yes) + + +@pytest.mark.parametrize("how", DECLARATIONS) +def test_the_naming_check_hears_what_is_passed_through(tmp_path, how): + assert_the_naming_check_hears_what_is_passed_through(run_entry, tmp_path, + how) + + +@pytest.mark.parametrize("org", UPSTREAM_CASINGS) +def test_the_upstream_org_is_refused_in_any_case(tmp_path, org): + assert_the_upstream_org_is_refused_in_any_case(run_entry, tmp_path, org) + + +def test_an_upstream_origin_in_any_case_needs_an_org(tmp_path): + assert_an_upstream_origin_in_any_case_needs_an_org(run_entry, tmp_path) + + # --- setup.sh is a shim, not a second flag list ----------------------------- #: The only flags `setup.sh` may still NAME, and why each of them is about the diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 4722eb65..aee72a91 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -421,6 +421,41 @@ def test_a_taken_clone_path_is_refused_before_anything(tmp_path, kind): assert_a_taken_clone_path_is_refused(run_setup, tmp_path, kind) +def test_no_test_here_depends_on_gh_being_authenticated(): + """Every invocation either stops before the preflight or runs offline. + + The ones that stop early are the help (`--help`, `-h`) and two + argument-parsing refusals (an unknown flag, a bad `--visibility`). Every + `run_setup` runs THIS checkout's `setup.sh` from inside THIS checkout, + where the shim hands all of them to `setup-project.py`, which answers + them before it looks at the machine. From anywhere ELSE, which only the + self-bootstrap section below reaches (through `run_bare_setup`, never + scanned here), the shim answers the help itself before any clone (#341), + and the two refusals are made by the CLONED `setup-project.py`. + Everything else must pass `--local-remote-dir`, or it reaches `gh auth + status` and passes only on a laptop that happens to be logged in — which + is exactly how the first version of this file went green locally and red + in CI. + """ + source = Path(__file__).read_text(encoding="utf-8") + early = ("--help", '"-h"', "--wat", '"secret"') + for match in re.finditer(r"(? --project ` has