diff --git a/setup-project.py b/setup-project.py index bcd485f9..51c3c020 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1698,6 +1698,12 @@ def resolve_family(opts: Options, shape_root: Path) -> Landing: DISCARD_IT_IF_UNUSED = ("Move it, or delete it if nothing in it is yours, then " "run this again.") KEEP_IT = "Move it, or choose another --into, then run this again." +#: What step (3) says to do about a parent nobody can write in, and about a +#: path whose drive is not there (#608). +WRITE_IT = ("Choose another --into, or make it a folder you can write in, " + "then run this again.") +DRIVE_WAY_OUT = ("Choose another --into on a drive that is there, then run " + "this again.") def origin_recorded_in(clone: str) -> str: @@ -1815,9 +1821,16 @@ def refuse_a_taken_clone_path(clone: str, project: str) -> None: `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. + + AND WHETHER THE CLONE CAN BE MADE THERE AT ALL (#608). A folder the + person cannot write in, or a Windows drive that is not there, passed + too: the three repositories were created and step (7) then refused + `could not be made a folder (Permission denied)`. + `refuse_a_parent_nobody_can_write_in` asks both here. """ if not os.path.lexists(clone): refuse_a_file_above(clone) + refuse_a_parent_nobody_can_write_in(clone) return what, way_out = what_stands_at(clone, project) die(" ".join(part for part in ( @@ -1861,8 +1874,8 @@ def refuse_a_file_above(clone: str) -> None: 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. + nowhere to make them. Nothing found at all is a drive that is not there, + and `refuse_a_parent_nobody_can_write_in` refuses that (#608). """ above = nearest_entry(os.path.dirname(clone)) if above is None or os.path.isdir(above): @@ -1872,6 +1885,69 @@ def refuse_a_file_above(clone: str) -> None: % (above, what_blocks(above), clone, KEEP_IT)) +def drive_of(path: str) -> str: + r"""The drive, or the network share, `path` is on: `C:`, or + `\\server\share`; nothing on a POSIX host.""" + return os.path.splitdrive(path)[0] + + +def refuse_a_parent_nobody_can_write_in(clone: str) -> None: + """Refuse a clone path whose nearest folder the person cannot write in, or + whose drive is not there (#608, L2-26). + + The clone is made in step (7), by `os.makedirs` and then `git clone`, in + the nearest folder that exists. Step (3) asked that this be a folder and + not whether the person can make one in it, so a parent at mode 555, a + read-only disk or an ACL that says no passed, and the run created all + three repositories before step (7) refused. + + THE QUESTION IS ASKED BY DOING IT: a folder is made in the nearest + existing one and taken away again, which is what step (7) does and the + only answer that holds for a read-only file system, an ACL, a quota and + root alike - `os.access` says what the mode bits say, and on Windows + nothing about a folder at all. It leaves the disk as it found it; the + folder carries a name of its own so a person who finds one after a + killed run knows whose it was. Only the making is the question: the + taking away is best-effort, and says where the folder is when it fails. + + A FILE standing there is `refuse_a_file_above`'s, which runs first and + has said so, so what is found here is a folder. NOTHING FOUND AT ALL, + `nearest_entry` None, is a Windows drive that is not there (a + disconnected network share, a letter nobody has): there is no folder to + ask, and step (7) would create the repositories before it found out. + The code is `die`'s, as for every refusal of step (3). + """ + above = nearest_entry(os.path.dirname(clone)) + if above is None: + drive = drive_of(clone) + die("%s, and this project would be cloned at %s, so the run stops " + "here and nothing was created. %s" + % (("The drive %s is not there" % drive) if drive + else "Not even the top of the path is there", clone, + DRIVE_WAY_OUT)) + try: + probe = tempfile.mkdtemp(prefix=".openreposhape-write-probe-", + dir=above) + except OSError as exc: + die("%s is a folder you cannot write in (%s), and this project would " + "be cloned inside it, at %s, so the run stops here and nothing " + "was created. %s" + % (above, exc.strerror or exc.__class__.__name__, clone, + WRITE_IT)) + # TAKING IT AWAY IS BEST-EFFORT. The refusal above is for a folder that + # could not be MADE; one that was made says the person can write here, + # and a failing `rmdir` (on Windows an indexer or an antivirus can hold a + # handle on a new folder for a moment) is no reason to say they cannot. + # The folder left behind is named instead, empty and the person's to + # delete. + try: + os.rmdir(probe) + except OSError as exc: + warn("%s, the folder made in %s to try it, could not be taken away " + "again (%s); it is empty, and yours to delete." + % (probe, above, exc.strerror or exc.__class__.__name__)) + + # --------------------------------------------------------------------------- # what the passthrough tells the scaffold (#538) # --------------------------------------------------------------------------- @@ -1882,12 +1958,14 @@ def refuse_a_file_above(clone: str) -> None: #: 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 +#: too. `--pin-source`, read since #608 for the tree a chain's links are +#: looked for in, is `--pin-s`: `--pin-owner` ends the sharing there. +#: `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"} + "--referent-chain": "--referent", "--pin-source": "--pin-s"} def scaffold_words(passthrough: list) -> list: @@ -1908,8 +1986,10 @@ def names_flag(word: str, flag: str) -> bool: 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.""" + spelling, because argparse reads such a word as a flag, so a word that + does is not this flag's value (`--pin --referent-chain x` gives `--pin` + none, and the scaffold's plan refuses it) and is read again as the flag + it is (#608, L2-27). The `=` spelling takes whatever follows it.""" words = scaffold_words(passthrough) values = [] for index, word in enumerate(words): @@ -1917,7 +1997,7 @@ def passed_values(passthrough: list, flag: str) -> list: continue if "=" in word: values.append(word.split("=", 1)[1]) - elif index + 1 < len(words): + elif index + 1 < len(words) and not words[index + 1].startswith("-"): values.append(words[index + 1]) return values @@ -1933,10 +2013,30 @@ def passes_a_dry_run(passthrough: list) -> bool: for word in scaffold_words(passthrough)) +def as_a_bare_link_source(path: str) -> str: + """`path` as the naming validator reads a `--link-source` that names no + product: it takes the text before the first `=` for a product when that + is a link of the chain, so `openXdox=layer`, which the scaffold reads as + the one path it is, would be read as `layer` for `openXdox`. A path that + starts `=` is spelled `./=`; one with a separator or a drive + before its first `=` cannot be a link's name, and stays as it is. + + AN EMPTY PATH IS NEVER GIVEN TO IT. Since #570 the scaffold takes + `--pin-source` as text and refuses an exactly empty value as + `pin-source-no-path`, before anything exists (`pin_source_path`), so it + no longer reads `''` as the directory it runs in: `.` would be a folder + the plan never reads, and `--link-source=` is refused by the validator as + `link-source-no-path` (#576). Either would end step (4) naming a flag the + person never typed, where the plan refuses the value by name. + `naming_declarations` leaves an empty source out. A BLANK path is a path: + the scaffold reads `Path(' ')`, so it goes as it is.""" + return "./" + path if re.match(r"[^/\\:=]+=", path) else path + + 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`. + `--referent-chain`, with the tree its links are read from (#608). A `` name is a descendant only when the project pins its referent, and the scaffold judges the names with the pins it is @@ -1948,13 +2048,41 @@ def naming_declarations(passthrough: list) -> list: 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. + + THE LINKS ARE VERIFIED FROM THE SAME TREE (#608, L2-25). The scaffold + reads a chain's links from the tree `--pin-source` names, and the naming + validator reads them from a `--link-source` it is given; step (4) was + given none, so a chain the plan verified showed + `[declared-unverified]` and a WARNING that the link's tree was not + reachable there, and one the tree broke showed no break at all. The LAST + `--pin-source`, as the scaffold keeps its last, goes as a bare + `--link-source` - and only WITH a chain of two links at least: the + scaffold reads the tree for the links alone, a chain of one link has no + holder whose declaration a source could answer, and a source no link + used is a NOTE the plan never prints. + + AN EMPTY LAST `--pin-source` IS LEFT TO THE PLAN (#608, rv608 on #623). + Since #570 the scaffold refuses `--pin-source ''` as `pin-source-no-path` + before anything exists, so there is no tree it reads, and step (4) is + given no `--link-source` for it: it says nothing about a flag the person + never typed, and step (5)'s plan refuses the value by name, with exit 2. + The scaffold's own test is `not str(typed)` (`pin_source_path`), so a + BLANK value is a path and goes as it is, and the rule reads the LAST + value alone: `--pin-source src --pin-source ''` is empty, + `--pin-source '' --pin-source src` is not. """ 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") + sources = passed_values(passthrough, "--pin-source") + has_links = bool(chains) and sum( + 1 for link in chains[-1].split(",") if link.strip()) > 1 + names_a_source = bool(sources) and bool(str(sources[-1])) return ((["--pins=" + ",".join(products)] if products else []) - + (["--referent-chain=" + chains[-1]] if chains else [])) + + (["--referent-chain=" + chains[-1]] if chains else []) + + (["--link-source=" + as_a_bare_link_source(sources[-1])] + if has_links and names_a_source else [])) # --------------------------------------------------------------------------- @@ -2039,6 +2167,58 @@ def scaffold_args(opts: Options, org: str) -> list: return args + list(opts.passthrough) +#: What the scaffold's plan starts with, at the start of a line: the first +#: thing `_print_plan` prints, and nothing printed before it begins so. +PLAN_STARTS = "project " + +#: What step (5) says when the plan did not go through (#608, L2-37): a plan +#: the scaffold printed and then refused, a plan it never printed, and a plan +#: that stopped the first time and not the second. The last names no plan, and +#: no refusal, because the second run is all it has seen. +PLAN_REFUSED = "the plan was refused (see above)." +PLAN_NOT_PRODUCED = "the plan could not be produced (see above)." +PLAN_STOPPED_ONCE = ("the scaffold stopped at the plan (see above). Asked a " + "second time it did not stop, so the refusal did not " + "happen again: run setup again.") + + +def why_the_plan_did_not_go_through(shape_root: Path, plan: list, + planned: subprocess.CompletedProcess + ) -> str: + """What step (5) says of a scaffold `--dry-run`, `plan`, that exited + nonzero (#608, L2-37). + + Since #573 the dry run prints the plan and THEN asks whether the remotes + exist, so a refusal can come either side of the plan, and "the plan could + not be produced (see above)" under a plan that is right there says + something false. The scaffold's output goes straight to the terminal, not + through this process, so the question is put once more, captured, and + only after a refusal: the dry run reads and creates nothing, a run that + printed and refused prints and refuses again, and a plan that went well + is never asked twice. Reading every run's plan through a pipe instead + would put each of them behind the child's buffering and its output + encoding, on every platform, to word one refusal. A child killed by a + signal refused nothing, and is not asked. + + THE SECOND RUN IS NOT THE ONE THE PERSON SAW. The words describe what it + printed, and what is above is the first run's. A passing gh or network + failure before the plan, or a remote made between the two runs, makes the + first refuse and the second print the plan and exit 0: it refused + nothing, and neither "refused" nor "could not be produced" is known to be + true of what is above. So a second run that exits 0 gets its own words, + that the refusal did not happen again, and the person runs setup again. + """ + if planned.returncode < 0: + return PLAN_NOT_PRODUCED + again = run(plan, cwd=shape_root, capture=True) + if again.returncode == 0: + return PLAN_STOPPED_ONCE + if any(line.startswith(PLAN_STARTS) + for line in (again.stdout or "").splitlines()): + return PLAN_REFUSED + return PLAN_NOT_PRODUCED + + def plan_and_confirm(opts: Options, shape_root: Path, org: str, args: list, clone: str) -> bool: """The plan, then the one question. False when the run ends at the plan @@ -2047,10 +2227,11 @@ def plan_and_confirm(opts: Options, shape_root: Path, org: str, to.""" say("") say("(5) the plan") - planned = run([PYTHON, str(shape_root / SCAFFOLD), *args, - "--dry-run"], cwd=shape_root) + plan = [PYTHON, str(shape_root / SCAFFOLD), *args, "--dry-run"] + planned = run(plan, cwd=shape_root) if planned.returncode != 0: - raise Refusal("the plan could not be produced (see above).", + raise Refusal(why_the_plan_did_not_go_through(shape_root, plan, + planned), shell_status(planned.returncode)) # THE ONE THING THE RUN WRITES ON THIS DISK IS IN THE PLAN TOO (#498). diff --git a/tests/conftest.py b/tests/conftest.py index 4bc356f8..33cbf476 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1213,8 +1213,12 @@ def assert_the_plan_refuses_a_remote_that_exists(run, tmp_path: Path) -> None: assert result.returncode == 2, result.stderr + result.stdout assert (f"REFUSED scaffold-remote-exists: {leg.resolve()} already exists " "and has commits" in result.stderr), result.stderr - assert "REFUSED: the plan could not be produced" in result.stderr + # THE PLAN WAS PRINTED BEFORE THE REMOTES WERE ASKED (#573), so it is + # refused, not "could not be produced" (#608, L2-37). + assert "REFUSED: the plan was refused (see above)." in result.stderr + assert "could not be produced" not in result.stderr, result.stderr assert "(5) the plan" in result.stdout, result.stdout + assert "\nproject Sample" in result.stdout, result.stdout assert "--dry-run: nothing was created." not in result.stdout assert "(6) scaffold" not in result.stdout, "the run reached (6)" assert "This will create THREE repositories" not in result.stdout @@ -1617,3 +1621,205 @@ 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() + + +# --- setup's step (3), (4) and (5) checks (#608) ----------------------------- +# +# ONE READING, TWO ENTRY POINTS, as above: each helper takes the runner of the +# file that calls it, and every run passes `--local-remote-dir`. + +#: What step (3) says a person can do about a parent folder nobody can write +#: in, and about a path whose drive is not there. +WRITE_IT = ("Choose another --into, or make it a folder you can write in, " + "then run this again.") +DRIVE_WAY_OUT = ("Choose another --into on a drive that is there, then run " + "this again.") + +#: Where the naming check can be asked about a chain, and what each makes of +#: it: `(the product the project pins, what its own tree declares)`. `verified` +#: is a link tree that declares the chain's next link; `broken` one that +#: declares another product, which the plan refuses as `chain-not-declared`. +CHAIN_LINK_TREES = {"verified": ("openXdox", "openDox"), + "broken": ("openYdox", "openOther")} + + +def a_folder_nobody_can_write_in(tmp_path: Path) -> Path: + """`tmp_path/shut`, an existing folder at mode 555, or a skip where this + account can make a folder in one anyway (root) or the platform has no such + mode (Windows). The test puts the mode back: pytest cannot delete it + otherwise.""" + if os.name == "nt": + pytest.skip("a folder's mode does not stop Windows from writing in it") + shut = tmp_path / "shut" + shut.mkdir() + shut.chmod(0o555) + try: + (shut / "probe").mkdir() + except OSError: + return shut + (shut / "probe").rmdir() + shut.chmod(0o755) + pytest.skip("this account can make a folder in a folder at mode 555") + + +def assert_a_parent_nobody_can_write_in_is_refused(run, + tmp_path: Path) -> None: + """`--into /work` is refused in step (3), naming the + folder and the reason the system gave, before the naming check, the plan + or any repository, and leaves the folder as it was. On main step (3) + asked only that what it found was a folder, so all three repositories were + created and step (7) refused `could not be made a folder (Permission + denied)`.""" + shut = a_folder_nobody_can_write_in(tmp_path) + try: + into = shut / "work" + result = run("--project", "Sample", "--yes", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(into)) + assert result.returncode == 2, result.stderr + result.stdout + assert ("REFUSED: %s is a folder you cannot write in (Permission " + "denied), and this project would be cloned inside it, at %s, " + "so the run stops here and nothing was created. %s" + % (os.path.abspath(str(shut)), + os.path.abspath(str(into / "Sample")), WRITE_IT) + ) in result.stderr, result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert "(3) project" in result.stdout, result.stdout + 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 list(shut.iterdir()) == [] + finally: + shut.chmod(0o755) + + +def assert_a_plan_that_never_printed_could_not_be_produced( + run, tmp_path: Path) -> None: + """A plan the scaffold refuses BEFORE it prints one - here a + `--tracking-branch` git will not create, passed through - keeps step + (5)'s words, `the plan could not be produced (see above).`: there is no + plan above to call refused (#608, L2-37). The refusal after a PRINTED plan + is `assert_the_plan_refuses_a_remote_that_exists`'s.""" + result = run("--project", "Sample", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), + "--", "--tracking-branch", "a..b") + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED scaffold-bad-tracking-branch:" in result.stderr + assert ("REFUSED: the plan could not be produced (see above)." + in result.stderr), result.stderr + assert "was refused" not in result.stderr, result.stderr + assert "(5) the plan" in result.stdout, result.stdout + assert "\nproject Sample" not in result.stdout, result.stdout + assert not (tmp_path / "remotes").exists() + assert not (tmp_path / "work").exists() + + +def assert_step_four_reads_a_chain_as_the_plan_does(run, tmp_path: Path, + how: str) -> None: + """Step (4) is given the passthrough's `--pin-source` as the naming + check's `--link-source`, so the chain it reports on is the chain the plan + reads (#608, L2-25): a link tree that declares the next link shows + `[verified]`, as the plan has it, and one that declares another product + shows `[broken]`, which the plan then refuses as `chain-not-declared`. On + main step (4) showed both as `[declared-unverified]` with a WARNING that + the link's tree was not reachable, though the tree was named with + `--pin-source`.""" + product, declares = CHAIN_LINK_TREES[how] + tree = tmp_path / "link" + tree.mkdir() + git("init", "-q", "-b", "main", cwd=tree) + (tree / "project.yaml").write_bytes( + ("schema_version: 1\nkind: project-manifest\n" + f"neutral_product_pins:\n - {declares}\n").encode("utf-8")) + git("add", "project.yaml", cwd=tree) + git("commit", "-q", "-m", "seed", cwd=tree) + commit = git("rev-parse", "HEAD", cwd=tree).stdout.strip() + result = run("--project", "codexDox", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), + "--", "--pin", f"{product}@{commit}", + "--referent-chain", f"{product},openDox", + "--pin-source", str(tree)) + assert result.returncode == 2, result.stderr + result.stdout + step_four = result.stdout.split("(4) naming policy", 1)[-1].split( + "(5) the plan", 1)[0] + assert f"[{how}]" in step_four, step_four + assert "declared-unverified" not in step_four + result.stderr, ( + step_four + result.stderr) + assert "NOTE --link-source" not in step_four + result.stderr + if how == "verified": + assert_refuses_for_want_of_a_terminal(result.stderr) + else: + assert "REFUSED chain-not-declared:" in result.stderr, result.stderr + assert (f"{product} declares {declares}, not openDox" + in step_four + result.stderr) + assert not (tmp_path / "remotes").exists() + + +def assert_an_empty_pin_source_is_the_plans_to_refuse(run, + tmp_path: Path) -> None: + """`--pin-source ''` is refused by the scaffold's plan as + `pin-source-no-path` (#570), so step (4) is given no `--link-source` for + it and leaves the value to step (5) (#608, rv608 on #623). On the merge of + #608 with #576 step (4) passed `--link-source=`, which the naming check + refuses as `link-source-no-path`, naming a flag the person never typed, + and the run ended there with exit 1 where the plan refuses the same value + with exit 2; passing `.` instead, as the scaffold's `Path('')` once read + it, made the naming check WARN about `--link-source`, `.` and a folder the + plan never reads. The naming check is given no source, so it says of the + chain what it says of any it cannot verify, `[declared-unverified]`, and + its hint there, `--link-source =`, one per holder it + cannot verify, is the one spelling of the flag in a run that never typed + it.""" + result = run("--project", "codexDox", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), + "--", "--pin", "openXdox@" + "0123456789abcdef" * 2 + + "01234567", "--referent-chain", "openXdox,openDox", + "--pin-source", "") + heard = result.stderr + result.stdout + assert "(5) the plan" in result.stdout, result.stdout + assert result.returncode == 2, heard + assert "REFUSED pin-source-no-path:" in result.stderr, result.stderr + assert "link-source-no-path" not in heard, heard + # The validator's own hint for a chain it cannot verify names the flag, + # `--link-source openXdox=`, whatever the run typed, so it is the + # one spelling left out; a `--link-source` that is a REPORT is not. + assert "--link-source" not in re.sub(r"--link-source \w+=", "", + heard), heard + step_four = result.stdout.split("(4) naming policy", 1)[-1].split( + "(5) the plan", 1)[0] + assert "[declared-unverified]" in step_four, step_four + assert "do not satisfy the naming policy" not in result.stderr, ( + result.stderr) + assert not (tmp_path / "remotes").exists() + + +def assert_a_chain_of_one_link_has_no_source_to_note(run, + tmp_path: Path) -> None: + """A chain of ONE link, which the plan accepts, has no holder whose + declaration a source could answer, so step (4) is not given the + `--pin-source` as a `--link-source`: the naming check would say `NOTE + --link-source ...: not used`, which the plan never prints for that chain + (#608, rv608 on #623).""" + 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() + result = run("--project", "codexDox", "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "work"), + "--", "--pin", f"openDox@{commit}", + "--referent-chain", "openDox", "--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 "codexDox: domain-descendant" in step_four, step_four + assert "NOTE --link-source" not in step_four + result.stderr, ( + step_four + result.stderr) + assert not (tmp_path / "remotes").exists() diff --git a/tests/test_setup_project_py.py b/tests/test_setup_project_py.py index d9b2b87c..5b8bb65b 100644 --- a/tests/test_setup_project_py.py +++ b/tests/test_setup_project_py.py @@ -22,13 +22,16 @@ from __future__ import annotations import ast +import errno import importlib.util +import ntpath import os import re import shlex import shutil import subprocess import sys +import tempfile from pathlib import Path @@ -58,6 +61,13 @@ assert_the_scaffold_leaves_its_next_steps_to_setup, assert_the_upstream_org_is_refused_in_any_case) +from conftest import (CHAIN_LINK_TREES, DRIVE_WAY_OUT, WRITE_IT, + assert_a_parent_nobody_can_write_in_is_refused, + assert_a_plan_that_never_printed_could_not_be_produced, + assert_step_four_reads_a_chain_as_the_plan_does, + assert_a_chain_of_one_link_has_no_source_to_note, + assert_an_empty_pin_source_is_the_plans_to_refuse) + SETUP_PROJECT = REPO / "setup-project.py" SETUP_SH = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -499,6 +509,139 @@ def test_the_plan_refuses_a_remote_that_exists_before_the_yes(tmp_path): assert_the_plan_refuses_a_remote_that_exists(run_entry, tmp_path) +#: What the scaffold's plan starts with, and a few of the lines after it. +A_PRINTED_PLAN = ("project Sample (sample) topic xf-project-sample\n" + "triad the Triad\n" + "topics skipped for local remotes\n") + + +def a_refused_plan(monkeypatch, tmp_path, code, asked, again=None): + """`plan_and_confirm` with the scaffold's plan exiting `code`, and `asked` + the stdout a second, captured run of that same plan gives, which exits + `again` (`code`, unless it is given: the two runs need not agree): the + refusal it raises and every call it made, as `(argv, cwd, capture)`.""" + module = entry_point_module() + calls = [] + + def run(argv, cwd=None, capture=False): + calls.append((list(argv), cwd, capture)) + exits = again if again is not None and len(calls) > 1 else code + return subprocess.CompletedProcess(argv, exits, + asked if capture else None, None) + + monkeypatch.setattr(module, "run", run) + opts = module.parse_args(["--project", "Sample", "--org", "demoorg"], + str(tmp_path)) + with pytest.raises(module.Refusal) as refused: + module.plan_and_confirm(opts, REPO, "demoorg", [], + str(tmp_path / "Sample")) + return refused.value, calls + + +def test_a_plan_that_printed_and_was_refused_is_said_to_be_refused( + monkeypatch, tmp_path): + """#608, L2-37: after #573 the scaffold's `--dry-run` prints the plan and + then refuses an existing remote, so "the plan could not be produced" + pointed at a plan that was right above it. Setup's own run inherits the + terminal, so it asks the question the scaffold asked once more, captured, + to learn whether the plan was there.""" + refusal, calls = a_refused_plan(monkeypatch, tmp_path, 2, A_PRINTED_PLAN) + assert str(refusal) == "the plan was refused (see above)." + assert refusal.code == 2 + [(first, first_cwd, first_captured), + (again, again_cwd, again_captured)] = calls + assert first_captured is False + assert again_captured is True + assert again == first + assert first[-1] == "--dry-run", calls + # Both are asked from the shape checkout: a relative `--pin-source` or + # `--work-dir` means what it means to the first run. + assert first_cwd == REPO + assert again_cwd == REPO + + +@pytest.mark.parametrize("shown", [ + "", "NOTE demoorg/Sample is already private.\n", + " [ok] project Sample (sample)\n", "a project Sample\n", + "projects of the family were read\n"]) +def test_a_plan_that_never_printed_could_not_be_produced(monkeypatch, tmp_path, + shown): + """#608, L2-37: the old words stay for a plan that was not printed. The + plan's own first line, at the start of a line, is what says it was; a + line that only holds those words, one that starts with the word and not + its column of spaces, or a NOTE the scaffold prints before the plan, is + not.""" + refusal, calls = a_refused_plan(monkeypatch, tmp_path, 2, shown) + assert str(refusal) == "the plan could not be produced (see above)." + assert refusal.code == 2 + assert len(calls) == 2, calls + + +def test_a_plan_whose_scaffold_was_killed_is_not_asked_again(monkeypatch, + tmp_path): + """A child killed by a signal (`-2`, Ctrl-C) is not refusing anything: + setup says what it said, with the exit status a shell reports (130), and + does not start the scaffold again to ask.""" + refusal, calls = a_refused_plan(monkeypatch, tmp_path, -2, A_PRINTED_PLAN) + assert str(refusal) == "the plan could not be produced (see above)." + assert refusal.code == 130 + assert len(calls) == 1, calls + + +#: What step (5) says when the plan stopped the first time and did not the +#: second: it names neither a plan above nor a refusal, because it cannot know +#: what the first run printed, and it says what to do. +PLAN_STOPPED_ONCE = ("the scaffold stopped at the plan (see above). Asked a " + "second time it did not stop, so the refusal did not " + "happen again: run setup again.") + + +@pytest.mark.parametrize("asked", [A_PRINTED_PLAN, ""]) +def test_a_plan_refused_once_and_not_again_says_to_run_setup_again( + monkeypatch, tmp_path, asked): + """#608, rv608 on #623: the words follow the second run, which the person + never sees, so they are only as good as its agreeing with the first. A + passing gh or network failure before the plan, or a remote made between + the two runs, makes the first run refuse and the second print the plan and + exit 0; "the plan was refused (see above)" then pointed at a plan that was + not there. A second run that exits 0 refused nothing, whatever it + printed, so the person is told it did not happen again and to run setup + again. The exit code stays the first run's: that is the refusal.""" + refusal, calls = a_refused_plan(monkeypatch, tmp_path, 2, asked, again=0) + assert str(refusal) == PLAN_STOPPED_ONCE + assert refusal.code == 2 + assert len(calls) == 2, calls + + +def test_a_plan_refused_twice_is_not_told_to_run_again(monkeypatch, tmp_path): + """The counterpart: a second run that refuses too, with the plan or + without it, keeps the two wordings it had, so the new sentence is for a + refusal that did not repeat and no other.""" + printed, _ = a_refused_plan(monkeypatch, tmp_path, 2, A_PRINTED_PLAN, + again=1) + assert str(printed) == "the plan was refused (see above)." + never, _ = a_refused_plan(monkeypatch, tmp_path, 2, "", again=1) + assert str(never) == "the plan could not be produced (see above)." + + +def test_a_plan_that_was_produced_is_run_once(monkeypatch, tmp_path): + """The second run is the refusal's alone: a plan that exits 0 is run once, + and its output is the terminal's, not captured.""" + module = entry_point_module() + calls = [] + + def run(argv, cwd=None, capture=False): + calls.append(capture) + return subprocess.CompletedProcess(argv, 0, None, None) + + monkeypatch.setattr(module, "run", run) + opts = module.parse_args(["--project", "Sample", "--org", "demoorg", + "--yes"], str(tmp_path)) + assert module.plan_and_confirm(opts, REPO, "demoorg", [], + str(tmp_path / "Sample")) is True + assert calls == [False] + + 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. @@ -2058,6 +2201,131 @@ def test_what_stands_above_a_clone_path(tmp_path, case): % (blocker, expected, clone, KEEP_IT)) +@pytest.mark.parametrize("number,reason", [(13, "Permission denied"), + (30, "Read-only file system")]) +def test_a_parent_nobody_can_write_in_is_refused_by_name(monkeypatch, tmp_path, + number, reason): + """#608, L2-26, on every platform and as every user: step (3) asks the + folder it finds above the clone for a folder of its own + (`tempfile.mkdtemp`), and what the system answers is the reason it gives. + `refuse_a_file_above` read only that what it found was a folder, so a + parent at mode 555, or on a read-only disk, passed and the three + repositories were created before step (7) said so.""" + module = entry_point_module() + + def refuses(*args, **kwargs): + raise OSError(number, reason) + + monkeypatch.setattr(tempfile, "mkdtemp", refuses) + clone = tmp_path / "work" / "Sample" + 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 a folder you cannot write in (%s), and this project would be " + "cloned inside it, at %s, so the run stops here and nothing was " + "created. %s" % (tmp_path, reason, clone, WRITE_IT)) + + +@pytest.mark.parametrize("depth", [0, 1, 2]) +def test_the_write_probe_is_made_in_the_nearest_folder_and_taken_away( + monkeypatch, tmp_path, depth): + """#608, L2-26: the folder the probe makes is made in the NEAREST folder + that exists - the clone's own parent when it is there, else the first one + above it - and is gone again, so a rehearsal leaves the disk as it found + it.""" + module = entry_point_module() + made_in = [] + real = tempfile.mkdtemp + + def spy(*args, **kwargs): + made = real(*args, **kwargs) + made_in.append(made) + return made + + monkeypatch.setattr(tempfile, "mkdtemp", spy) + base = tmp_path / "base" + base.mkdir() + clone = base.joinpath(*("below%d" % n for n in range(depth)), "Sample") + assert module.refuse_a_taken_clone_path(str(clone), "Sample") is None + [made] = made_in + assert Path(made).parent == base, made + assert not os.path.lexists(made), made + assert list(base.iterdir()) == [] + + +def test_a_probe_folder_that_will_not_go_is_no_reason_to_refuse( + monkeypatch, tmp_path, capsys): + """#608, rv608 on #623: the refusal is for a folder that could not be MADE. + Taking the probe away again is best-effort: where a handle on the new + folder (an indexer, an antivirus, on Windows) makes `rmdir` fail after + the folder was made, the parent is still one the person can write in, so + the clone path is not refused for it, and the folder left behind is named + rather than lost.""" + module = entry_point_module() + real_rmdir = os.rmdir + + def held(path, *args, **kwargs): + if ".openreposhape-write-probe-" in os.path.basename(str(path)): + raise OSError(errno.EBUSY, "Device or resource busy") + return real_rmdir(path, *args, **kwargs) + + monkeypatch.setattr(os, "rmdir", held) + base = tmp_path / "base" + base.mkdir() + assert module.refuse_a_taken_clone_path(str(base / "Sample"), + "Sample") is None + [left] = list(base.iterdir()) + real_rmdir(str(left)) + said = capsys.readouterr() + assert said.out == "", said.out + assert str(left) in said.err, said.err + assert "could not be taken away again" in said.err, said.err + assert "Device or resource busy" in said.err, said.err + assert "cannot write in" not in said.err, said.err + + +def test_the_drive_is_what_the_os_path_module_splits_off(monkeypatch): + """#608: `drive_of` is `os.path.splitdrive`'s drive, which a POSIX host + never has, so the Windows module is put in its place for the three calls: + a letter, a network share, and a path with neither.""" + module = entry_point_module() + with monkeypatch.context() as windows: + windows.setattr(os, "path", ntpath) + found = [module.drive_of(path) for path in ( + "C:\\work\\Sample", "\\\\server\\share\\work\\Sample", + "work\\Sample")] + assert found == ["C:", "\\\\server\\share", ""] + + +@pytest.mark.parametrize("drive,said", [ + ("Z:", "The drive Z: is not there"), + ("\\\\server\\share", "The drive \\\\server\\share is not there"), + ("", "Not even the top of the path is there")]) +def test_a_path_whose_drive_is_not_there_is_refused_by_name( + monkeypatch, tmp_path, drive, said): + """#608, L2-26: `nearest_entry` is None where not even the top of the + path is there, which on Windows is a drive that does not exist and on a + POSIX host cannot happen, so it is made to here, with the drive `drive_of` + names. On main that None passed (`refuse_a_file_above` left it "to the + clone"). Nothing is asked of the disk.""" + module = entry_point_module() + monkeypatch.setattr(module, "nearest_entry", lambda path: None) + monkeypatch.setattr(module, "drive_of", lambda path: drive) + + def asked(*args, **kwargs): + raise AssertionError("the disk was asked for a folder to probe in") + + monkeypatch.setattr(tempfile, "mkdtemp", asked) + clone = str(tmp_path / "work" / "Sample") + with pytest.raises(module.Refusal) as refused: + module.refuse_a_taken_clone_path(clone, "Sample") + assert refused.value.code == 2 + assert str(refused.value) == ( + "%s, and this project would be cloned at %s, so the run stops here " + "and nothing was created. %s" % (said, clone, DRIVE_WAY_OUT)) + + #: 40 hex, for a `--pin` nothing reads the digest of. A_COMMIT = "0123456789abcdef0123456789abcdef01234567" @@ -2082,7 +2350,95 @@ def test_a_dry_run_is_read_as_the_scaffold_reads_one(passthrough, dry): "--referen", "x"], []), (["--pin", f"openGlass@{A_COMMIT}", "--", "--pin", f"openChart@{A_COMMIT}"], ["--pins=openGlass"]), - (["--pin"], []), (["--pin", f"@{A_COMMIT}"], [])]) + (["--pin"], []), (["--pin", f"@{A_COMMIT}"], []), + # L2-25 (#608): a chain is verified from the tree `--pin-source` names, so + # that is passed as a bare `--link-source`: the last one, in any spelling + # the scaffold takes for the flag, and only where there is a chain (a + # source no link used is a NOTE the plan never prints). + (["--referent-chain", "openXdox,openDox", "--pin-source", "src"], + ["--referent-chain=openXdox,openDox", "--link-source=src"]), + (["--pin-source=a", "--referent-chain=openXdox,openDox", "--pin-s", "b", + "--pin-so=c"], + ["--referent-chain=openXdox,openDox", "--link-source=c"]), + (["--pin", f"openXdox@{A_COMMIT}", "--referent-chain", "openXdox,openDox", + "--pin-source", "/x/src"], + ["--pins=openXdox", "--referent-chain=openXdox,openDox", + "--link-source=/x/src"]), + (["--pin-source", "src"], []), + (["--referent-chain", " , ", "--pin-source", "src"], + ["--referent-chain= , "]), + (["--referent-chain", "openXdox,openDox", "--pin-", "x", "--pin-sourc"], + ["--referent-chain=openXdox,openDox"]), + (["--referent-chain", "openXdox,openDox", "--", "--pin-source", "src"], + ["--referent-chain=openXdox,openDox"]), + # ...as the scaffold's `Path` reads it: a value that starts with `-` is + # not given as the next word (argparse reads it as a flag), and is kept + # after an `=`. A value `PRODUCT=PATH` would be read as keyed by the + # naming check where the scaffold reads one path, so it is spelled `./`. + (["--referent-chain", "openXdox,openDox", "--pin-source", "--wat"], + ["--referent-chain=openXdox,openDox"]), + (["--referent-chain", "openXdox,openDox", "--pin-source"], + ["--referent-chain=openXdox,openDox"]), + (["--referent-chain", "openXdox,openDox", "--pin-source=-odd"], + ["--referent-chain=openXdox,openDox", "--link-source=-odd"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", + "openXdox=layer"], + ["--referent-chain=openXdox,openDox", "--link-source=./openXdox=layer"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", "a/b=c"], + ["--referent-chain=openXdox,openDox", "--link-source=a/b=c"]), + # ...and where `=` follows a Windows separator or a drive, the text before + # it cannot be a link's name, so nothing is added: `[^/\\:=]` holds each. + (["--referent-chain", "openXdox,openDox", "--pin-source", "a\\b=c"], + ["--referent-chain=openXdox,openDox", "--link-source=a\\b=c"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", "C:x=y"], + ["--referent-chain=openXdox,openDox", "--link-source=C:x=y"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", + "C:\\src\\openXdox=layer"], + ["--referent-chain=openXdox,openDox", + "--link-source=C:\\src\\openXdox=layer"]), + # An EMPTY value is left to the plan: the scaffold refuses it as + # `pin-source-no-path` (#570), so no `--link-source` goes at all, where + # `--link-source=` is refused by the naming check as `link-source-no-path` + # (#576) and `--link-source=.` WARNs about a flag the person never typed + # and a folder the plan never reads. The LAST value is the one that + # counts, empty or not, and a blank one is not an empty one. + (["--referent-chain", "openXdox,openDox", "--pin-source", ""], + ["--referent-chain=openXdox,openDox"]), + (["--referent-chain", "openXdox,openDox", "--pin-source="], + ["--referent-chain=openXdox,openDox"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", "src", + "--pin-source", ""], + ["--referent-chain=openXdox,openDox"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", "", + "--pin-source", "src"], + ["--referent-chain=openXdox,openDox", "--link-source=src"]), + (["--referent-chain", "openXdox,openDox", "--pin-source", " "], + ["--referent-chain=openXdox,openDox", "--link-source= "]), + (["--pin-source", ""], []), + # The naming check reads a source for a chain's holders, and a chain of + # one link has none: it says `NOTE --link-source ...: not used`, which the + # plan never prints for the same chain. So a source goes with a chain of + # TWO links at least, counted as the scaffold counts them (stripped, + # blanks dropped), and the chain is passed either way. + (["--referent-chain", "openDox", "--pin-source", "src"], + ["--referent-chain=openDox"]), + (["--referent-chain", "openDox, ", "--pin-source", "src"], + ["--referent-chain=openDox, "]), + (["--referent-chain", " , openDox ,", "--pin-source=src"], + ["--referent-chain= , openDox ,"]), + (["--referent-chain", "openXdox, ,openDox", "--pin-source", "src"], + ["--referent-chain=openXdox, ,openDox", "--link-source=src"]), + # A repeated `--referent-chain` keeps its last, and so does the source's + # reading of whether there is a chain to read it for. + (["--referent-chain", "openXdox,openDox", "--referent-chain", " , ", + "--pin-source", "src"], + ["--referent-chain= , "]), + (["--referent-chain", " , ", "--referent-chain", "openXdox,openDox", + "--pin-source", "src"], + ["--referent-chain=openXdox,openDox", "--link-source=src"]), + # L2-27 (#608): `--pin` followed by a flag has no value. + (["--pin", "--referent-chain", "x"], ["--referent-chain=x"]), + (["--pin", "--pin", f"openGlass@{A_COMMIT}"], ["--pins=openGlass"])]) def test_the_declarations_are_read_as_the_scaffold_reads_them(passthrough, declared): """#538: each `--pin`'s product, at its last `/` and before `@`; the @@ -2091,6 +2447,26 @@ def test_the_declarations_are_read_as_the_scaffold_reads_them(passthrough, assert entry_point_module().naming_declarations(passthrough) == declared +@pytest.mark.parametrize("passthrough,flag,values", [ + # L2-27 (#608), the issue's own case and its counterpart. + (["--pin", "--referent-chain", "x"], "--pin", []), + (["--pin", "--referent-chain", "x"], "--referent-chain", ["x"]), + (["--pin", "-x", "--pin", "a@b"], "--pin", ["a@b"]), + (["--pin", "a@b", "--pin"], "--pin", ["a@b"]), + # The `=` spelling takes what follows it whatever it starts with, and the + # word after a flag that has its value is not read again as one. + (["--pin=-x"], "--pin", ["-x"]), + (["--pin=a", "--pin", "b", "c"], "--pin", ["a", "b"]), + (["--pin", "a", "--", "--pin", "b"], "--pin", ["a"])]) +def test_a_word_that_starts_with_a_dash_is_not_a_value(passthrough, flag, + values): + """#608, L2-27: `passed_values` says a value never starts with `-` in the + second spelling, because argparse reads such a word as a flag; on main + `--pin --referent-chain x` gave `--pin` the value `--referent-chain`, and + step (4) was handed `--pins=--referent-chain`.""" + assert entry_point_module().passed_values(passthrough, flag) == values + + 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 @@ -2144,6 +2520,29 @@ def test_the_naming_check_hears_what_is_passed_through(tmp_path, how): how) +# --- #608, on an interpreter alone: each is `conftest`'s helper of that name - + +def test_a_parent_nobody_can_write_in_is_refused(tmp_path): + assert_a_parent_nobody_can_write_in_is_refused(run_entry, tmp_path) + + +def test_a_plan_that_never_printed_could_not_be_produced_end_to_end(tmp_path): + assert_a_plan_that_never_printed_could_not_be_produced(run_entry, tmp_path) + + +@pytest.mark.parametrize("how", CHAIN_LINK_TREES) +def test_step_four_reads_a_chain_as_the_plan_does(tmp_path, how): + assert_step_four_reads_a_chain_as_the_plan_does(run_entry, tmp_path, how) + + +def test_an_empty_pin_source_is_the_plans_to_refuse(tmp_path): + assert_an_empty_pin_source_is_the_plans_to_refuse(run_entry, tmp_path) + + +def test_a_chain_of_one_link_has_no_source_to_note(tmp_path): + assert_a_chain_of_one_link_has_no_source_to_note(run_entry, tmp_path) + + @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) diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index 3778c80d..1256c698 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -44,6 +44,13 @@ assert_the_scaffold_leaves_its_next_steps_to_setup, assert_the_upstream_org_is_refused_in_any_case) +from conftest import (CHAIN_LINK_TREES, + assert_a_parent_nobody_can_write_in_is_refused, + assert_a_plan_that_never_printed_could_not_be_produced, + assert_step_four_reads_a_chain_as_the_plan_does, + assert_a_chain_of_one_link_has_no_source_to_note, + assert_an_empty_pin_source_is_the_plans_to_refuse) + SETUP = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -503,6 +510,29 @@ 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) +# --- #608, through the shim: each is `conftest`'s helper of that name ------- + +def test_a_parent_nobody_can_write_in_is_refused(tmp_path): + assert_a_parent_nobody_can_write_in_is_refused(run_setup, tmp_path) + + +def test_a_plan_that_never_printed_could_not_be_produced(tmp_path): + assert_a_plan_that_never_printed_could_not_be_produced(run_setup, tmp_path) + + +@pytest.mark.parametrize("how", CHAIN_LINK_TREES) +def test_step_four_reads_a_chain_as_the_plan_does(tmp_path, how): + assert_step_four_reads_a_chain_as_the_plan_does(run_setup, tmp_path, how) + + +def test_an_empty_pin_source_is_the_plans_to_refuse(tmp_path): + assert_an_empty_pin_source_is_the_plans_to_refuse(run_setup, tmp_path) + + +def test_a_chain_of_one_link_has_no_source_to_note(tmp_path): + assert_a_chain_of_one_link_has_no_source_to_note(run_setup, tmp_path) + + # --- self-bootstrap --------------------------------------------------------- # # `curl -fsSL .../setup.sh | bash -s -- --org --project ` has