diff --git a/adopt-project.py b/adopt-project.py index 40619b54..52244191 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -258,7 +258,9 @@ #: The two a pin is a fact by. A pin without either is a name. PIN_RECORDED = ("commit", "tree_sha256") -#: `check`'s findings about the plan's `pins:`. +#: `check`'s findings about the plan's `pins:`, and the last of them about a +#: path the root keeps where `execute` writes a pin: a declared pin's file +#: (#273), or the shape's own pin (#524). PIN_UNRECORDED = "plan-pin-unrecorded" PIN_MALFORMED = "plan-pin-malformed" PIN_COLLIDES = "plan-pin-collides" @@ -443,23 +445,40 @@ def _recorded_pins(plan: "Plan") -> dict[str, dict[str, str]]: return {product: values for _, product, values in _read_pins(plan)[0]} +def _pin_files(plan: "Plan") -> list[tuple[str, str | None]]: + """`(path, product)` for each pin file `execute` writes, in the order + `place` writes them: each declared pin's, once, then the shape's own + pin, whose product is None (#524).""" + return [*((neutral_pin_path(product), product) + for product in dict.fromkeys(_declared_pins(plan))), + (SHAPE_PIN, None)] + + def _kept_pin_findings(plan: "Plan", tree: list) -> list[str]: - """A FINDING for each declared pin whose file a path the plan keeps in - the root already holds. The materializer writes the shape's pin beside - such a path, under `COLLISION_DIR`, as it writes any shape file the root - holds; for a pin that is no copy to merge but the wrong file, because the - root's own `validate-pins.py` reads `contracts/-pin.yaml` and - would read the source's. Asked of a disk that keeps case and, where that - finds nothing, of one that ignores it.""" - products = list(dict.fromkeys(_declared_pins(plan))) - kept = _kept_in_root(plan.entries, tree) if products else [] + """A FINDING for each pin file a path the plan keeps in the root already + holds: a declared pin's (#273), or the shape's own pin (#524). The + materializer writes the pin beside such a path, under `COLLISION_DIR`, + as it writes any shape file the root holds; for a pin that is no copy to + merge but the wrong file at the pin's path, which the root reads there: + its own `validate-pins.py` reads `contracts/-pin.yaml`, and + `execute`'s verification, `validate-pins.py` and `make bootstrap` read + `contracts/shape-pin.yaml`. Asked of a disk that keeps case and, where + that finds nothing, of one that ignores it. + + THE SHAPE'S OWN PIN WAS NEVER ASKED (#524). A file the source keeps at + `contracts/shape-pin.yaml`, with a pin declared or without, or a link + there that leads to something, passed `check`; `execute` made and + pushed both legs, wrote the shape's pin beside it and failed its + verification as `adopt-shape-pin-unreadable`. A link there that leads + nowhere, which `place` writes the pin through, is named here too, and + not again by the walk (`_links_at_a_pin`).""" + kept = _kept_in_root(plan.entries, tree) if not kept: return [] disks = ((_KeptTree(kept, fold=False), ""), (_KeptTree(kept, fold=True), _CASE_ASIDE)) out = [] - for product in products: - rel = neutral_pin_path(product) + for rel, product in _pin_files(plan): for disk, aside in disks: at = disk.at(rel) rows = [row for row in ([at] if at else disk.under(rel)) @@ -469,23 +488,45 @@ def _kept_pin_findings(plan: "Plan", tree: list) -> list[str]: path, mode, entry = rows[0] under = "" if entry == path else f" under the entry {_spelled(entry)}" where = "at" if at else "inside" + written = (_declared_pin_held(product) if product + else _shape_pin_held(mode if at else None)) out.append( f"FINDING {ROOT_HOLDS_PIN}: the plan keeps {_kind_of(mode)} " f"{_spelled(path)} in the assembly root{under}, {where} " - f"`{rel}`{aside}, where `execute` writes the pin the plan " - f"declares on `{product}`. The shape's pin would be written " - f"beside it, under `{COLLISION_DIR}/`, and the root's own " - "`validate-pins.py` would read the source's file as that " - f"pin: {_fix_the_kept_path(entry)}.") + f"`{rel}`{aside}, where `execute` writes {written}: " + f"{_fix_the_kept_path(entry)}.") break return out +def _declared_pin_held(product: str) -> str: + """What `plan-root-holds-pin` says of a declared pin's file (#273).""" + return (f"the pin the plan declares on `{product}`. The shape's pin would " + f"be written beside it, under `{COLLISION_DIR}/`, and the root's " + "own `validate-pins.py` would read the source's file as that pin") + + +def _shape_pin_held(mode: str | None) -> str: + """What `plan-root-holds-pin` says of the shape's own pin (#524), where + the root keeps an entry of `mode` AT it, or, with None, a directory + there.""" + found = "directory" if mode is None else _kind_of(mode).split()[-1] + through = (", or written through it where it leads nowhere" + if mode == SYMLINK_MODE else "") + return ("the shape's own pin: the openRepoShape commit the root's copies " + "came from and each copy's digest, which `execute`'s " + "verification, the root's own `validate-pins.py` and `make " + "bootstrap` read at that path. The split would hold the source's " + f"{found} there, and the shape's pin beside it, under " + f"`{COLLISION_DIR}/`{through}") + + def _pin_findings(plan: "Plan", tree: list) -> list[str]: """Every FINDING about the plan's pins: each entry `execute` cannot write a pin file from, two pins at one file or one at a file the shape writes, two whose products one `SHAPE_PIN_SOURCE_` names - (#275), and a path the root keeps where a pin is written.""" + (#275), and a path the root keeps where a pin is written, the shape's + own among them (#524).""" return _read_pins(plan)[1] + _kept_pin_findings(plan, tree) @@ -3717,8 +3758,9 @@ def _leg_findings(plan: Plan, tree: list | None) -> list[str]: findings += _root_shape_findings(plan, tree) # THE PINS, LAST (#273): each `pins:` entry `execute` cannot write a # pin file from, two pins at one file, and a path the root keeps - # where a pin is written. Each finding names the `plan` that records - # the pin whole. A leg mounted over a pin's file is + # where a pin is written, the shape's own pin among them (#524). + # Each finding about an entry names the `plan` that records the pin + # whole. A leg mounted over a pin's file is # `plan-leg-path-collides`, above, with the shape's own paths. findings += _pin_findings(plan, tree) return findings @@ -5847,7 +5889,10 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict, # them, a link that only a pin's file runs through passed `check`, and # `execute` stopped at it after both legs; and a link to a pin's file was # taken to lead nowhere, though `place` meets the pin there. A symlink AT a -# pin's file is #273's `plan-root-holds-pin`, said once there. +# pin's file is #273's `plan-root-holds-pin`, said once there, and so is one +# AT the shape's own pin, wherever it leads (#524): where it leads to +# something, `place` writes the shape's pin beside it and the root's +# `contracts/shape-pin.yaml` is the source's link, which no walk refuses. #: The finding for a symlink the plan keeps in the root that the shape cannot #: write through. @@ -6368,20 +6413,20 @@ def _links_at_a_copy(plan: Plan, tree: list) -> set[str]: def _links_at_a_pin(plan: Plan, tree: list) -> set[str]: - """Each symlink the plan keeps AT the file a declared pin is written to, - `contracts/-pin.yaml`, that #273's `plan-root-holds-pin` names - (`_kept_pin_findings`): the walks follow each pin's file too, and the - link is not said again by them. Asked as `_kept_pin_findings` asks it, - in `_leg_findings`, of a plan whose `legs` it can read: of each declared - pin, on a disk that keeps case and, where that finds nothing there, on + """Each symlink the plan keeps AT a file a pin is written to, a declared + pin's `contracts/-pin.yaml` or the shape's own + `contracts/shape-pin.yaml`, that #273's `plan-root-holds-pin` names + (`_kept_pin_findings`, #524): the walks follow each pin's file too, and + the link is not said again by them. Asked as `_kept_pin_findings` asks + it, in `_leg_findings`, of a plan whose `legs` it can read: of each pin + file, on a disk that keeps case and, where that finds nothing there, on one that ignores it.""" - products = list(dict.fromkeys(_declared_pins(plan))) - if not products or not isinstance(plan.legs, dict): + if not isinstance(plan.legs, dict): return set() kept = _kept_in_root(plan.entries, tree) disks = (_KeptTree(kept, fold=False), _KeptTree(kept, fold=True)) named: set[str] = set() - for rel in map(neutral_pin_path, products): + for rel, _ in _pin_files(plan): # The disk `_kept_pin_findings` names its row from. disk = next((one for one in disks if _holds_a_pin(one, rel)), None) at = disk.at(rel) if disk else None @@ -6417,8 +6462,8 @@ def _root_symlink_findings(plan: Plan, source: Source, this check's, because #262's rule follows no link. A symlink AT `project.yaml` is #305's `plan-root-blocks-manifest`, which `_shape_blocks` reports the same way, and is not said again either; nor - is a symlink AT a declared pin's file, #273's `plan-root-holds-pin` - (`_links_at_a_pin`). + is a symlink AT a declared pin's file or AT the shape's own pin, #273's + `plan-root-holds-pin` (`_links_at_a_pin`, #524). A walk that stops at a symlink with an empty target is #303's to say: `plan-empty-symlink` names every such symlink in the source, first in @@ -6499,8 +6544,10 @@ def symlinks(self) -> bool: # user cannot write (J4 of #183's re-verify). Taken as a disk # that holds symlinks, `check` prints the finding it printed # before B-OFF for a link AT a path that a walk cannot follow, - # and `execute` refuses with it, where both would otherwise - # end in a traceback. + # beside `adopt-work-dir-unwritable` (#524), and + # `_refuse_what_check_finds` refuses with it, where both + # would otherwise end in a traceback. `execute` itself + # refuses such a directory before it gets here (#524). self._answer = True return self._answer @@ -6562,9 +6609,23 @@ def cmd_check(args) -> int: print(f"\n{len(mistyped)} finding(s) in {plan.path}", file=sys.stderr) return 1 work_root = _work_root(args) - source = plan.open_source(args.source, work_root) + # Asked before the source is opened, as `execute` asks it (#524). An + # `org/repo` source is cloned into the work directory, so where that + # directory cannot be written there is no source to read: refused as + # `execute` refuses it, where it ended in a PermissionError traceback. + unwritable = _unwritable_work_dir(args, work_root) + try: + source = plan.open_source(args.source, work_root) + except OSError: + if not unwritable: + raise + raise Refusal(WORK_DIR_UNWRITABLE, unwritable, + f"Remediation: {WORK_DIR_FIX}") from None naming = NamingPolicy.load(NAMING_POLICY) findings: list[str] = [] + if unwritable: + findings.append( + f"FINDING {WORK_DIR_UNWRITABLE}: {unwritable}. {WORK_DIR_FIX}") # FIRST, because it is said where work starts; it is never one of the # findings below and never moves this command's exit code. _print_triad_advisory(source.path, @@ -7024,6 +7085,54 @@ def _refuse_an_occupied_work_dir(work_root: Path, names: dict, "one, each run makes a new one.") +#: `check`'s finding, and the code `execute` refuses it under, for a +#: `--work-dir` this user cannot write in (#524); and what both say to do. +WORK_DIR_UNWRITABLE = "adopt-work-dir-unwritable" +WORK_DIR_FIX = ("Pass a --work-dir this user can write in, or none: without " + "one, each run makes a new one.") + + +def _unwritable_work_dir(args, work_root: Path) -> str | None: + """Why this run cannot build in the `--work-dir` it was given, or None: + a directory made in it, and removed again, could not be (#524). + + `_work_root` only makes the directory, and what `execute` builds there, + the legs and the split (`BUILT_IN`), it builds only after + `_create_leg_remotes` has made both leg repositories. So with a + `--work-dir` this user cannot write in (`chmod 500`), `check` said `plan + ok`, and `execute` made and pushed both legs and stopped `extracting the + spec leg`, exit 2, as on main at 8beb3d92 and bfed143. Each command now + asks the directory itself, as `execute` will use it: `execute` refuses + it first, before it opens the source (an `org/repo` source is cloned + there), and `check` names it beside the plan's findings, so the two + agree. The probe has a name of its own (`tempfile.mkdtemp`), so it + meets nothing an earlier run left there, and it is removed again. + + Without `--work-dir`, `_work_root` makes a new directory, which this + user owns and can write in, and nothing is asked.""" + if not getattr(args, "work_dir", None): + return None + try: + os.rmdir(tempfile.mkdtemp(prefix="write-probe-", dir=work_root)) + except OSError as exc: + return (f"--work-dir {_spelled(str(work_root))} cannot be written: " + "making a directory in it, and removing it again, failed " + f"({exc.strerror or exc}), and `execute` builds " + f"{_and_listed(list(BUILT_IN.values()))} there, only after " + "it has made both leg repositories") + return None + + +def _refuse_an_unwritable_work_dir(args, work_root: Path) -> None: + """Refuse `_unwritable_work_dir`'s `--work-dir`, before any leg + repository exists (#524).""" + unwritable = _unwritable_work_dir(args, work_root) + if unwritable: + raise Refusal(WORK_DIR_UNWRITABLE, unwritable, + "Remediation: no leg repository was created and " + f"nothing was pushed. {WORK_DIR_FIX}") + + def _why_occupied(built: list[tuple[str, str]], names: dict, seeded: list[str]) -> str: """Why `built`'s directories are refused: each builder's reason once, @@ -8210,6 +8319,8 @@ def cmd_execute(args) -> int: plan = Plan.load(Path(args.plan)) _refuse_a_mistyped_plan(plan) work_root = _work_root(args) + # Before the source is opened, so before any leg exists (#524). + _refuse_an_unwritable_work_dir(args, work_root) source = plan.open_source(args.source, work_root) naming = NamingPolicy.load(NAMING_POLICY) _require_filter_repo() diff --git a/tests/conftest.py b/tests/conftest.py index 9f517dc8..f6b85afa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -716,6 +716,29 @@ def refused_before_any_leg(adopter, monkeypatch: pytest.MonkeyPatch, return refusal +#: A file the shape's pin names, which `a_hook_that_unstages` takes out of +#: the split commit in the tests that need a verification to fail. +PINNED_COPY = "scripts/validate-pins.py" + + +def a_hook_that_unstages(hooks: Path, path: str = PINNED_COPY) -> Path: + """`hooks`, holding a `pre-commit` hook that takes `path` out of the + index before every commit, and nothing else: a commit without `path` is + left as it is (`--ignore-unmatch`). Pointed at by `core.hooksPath`, it + leaves `execute`'s split commit without a file the shape's pin names, so + the verification fails, after both legs are pushed. The plan answer that + stood for that failure until #524, the source's own + `contracts/shape-pin.yaml` kept in the root, is refused before any leg + now. LF, because `sh` reads it on every platform.""" + hooks.mkdir(parents=True, exist_ok=True) + hook = hooks / "pre-commit" + with hook.open("w", encoding="utf-8", newline="\n") as handle: + handle.write("#!/bin/sh\n" + f"exec git rm -q --cached --ignore-unmatch -- {path}\n") + hook.chmod(0o755) + return hooks + + # --- the `gh` guard (#296) --------------------------------------------------- # # NO TEST REACHES THE OPERATOR'S `gh`, and the doctor's `machine` row is still diff --git a/tests/test_adopt_github_exits.py b/tests/test_adopt_github_exits.py index 5c757f12..12878062 100644 --- a/tests/test_adopt_github_exits.py +++ b/tests/test_adopt_github_exits.py @@ -58,8 +58,8 @@ import pytest -from conftest import (ADOPT, REPO, WINDOWS_SKIP, git, make_source_repo, - run_script, write_plan) +from conftest import (ADOPT, REPO, WINDOWS_SKIP, a_hook_that_unstages, git, + make_source_repo, run_script, write_plan) #: `execute` REQUIRES `git filter-repo`; without it the runs below are #: skipped, as every `execute` test in this suite is. @@ -75,18 +75,12 @@ #: the plan as written. SOURCE_TREE = {"README.md": "# Northwind\n", "src/app.py": "print(1)\n", "docs/spec.md": "# Spec\n"} -#: The same source with a `contracts/shape-pin.yaml` of its OWN, naming a file -#: the plan sends to the code leg. `plan` sends `contracts/` to the spec leg, -#: where the adoption verifies; kept in the root, as `KEEP_CONTRACTS` answers, -#: it is the pin the first bootstrap would read, naming a file that is not -#: there, and the verification fails (`adopt-shape-file-missing`). A REAL -#: failure of `_verify`, with nothing in the tool replaced. -FAILING_TREE = {**SOURCE_TREE, "contracts/shape-pin.yaml": ( - "schema_version: 1\nkind: pinned_contract_manifest\nfiles:\n" - f" - path: src/app.py\n sha256: \"{'0' * 64}\"\n")} -CONTRACTS_TO_SPEC = " - path: contracts/\n leg: spec\n" -KEEP_CONTRACTS = (" - path: contracts/\n leg: root\n" - " resolution: \"kept beside the shape's own\"\n") +# A failing verification is a REAL failure of `_verify`, with nothing in the +# tool replaced: a `pre-commit` hook takes a file the shape's pin names out +# of the split commit (`conftest.a_hook_that_unstages`, `github_run`'s +# `failing`), and the verification fails (`adopt-shape-file-missing`). The +# source's own `contracts/shape-pin.yaml` kept in the root stood for it until +# #524, which refuses that plan before any leg exists. #: The heading of step 7, which the point runs from to the next blank line. STEP_7 = "7. **Read the verification table back to them.**" @@ -208,20 +202,20 @@ def step_7() -> str: def github_run(tmp_path: Path, refuses: str, tree: dict = SOURCE_TREE, - keep_contracts: bool = False, merge_stderr: bool = False, + failing: bool = False, merge_stderr: bool = False, stdout_closed: bool = False, branch: str = "main", plan_flags: tuple = (), before_plan=None): """Plan a small source and `execute` it in GitHub mode, with a `gh` that refuses `refuses`. Returns `(result, github)`: the run, and the directory that stands for github.com, with `testorg/.git` in it. - `keep_contracts` answers the plan so that the source's own pin stays in - the root, which fails the verification. `merge_stderr` runs with both - streams in one pipe, as a person redirecting a log has them: stdout is - then block-buffered, and the order the lines arrive in is the order the - tool flushed them. `stdout_closed` gives the run a stdout whose reading end - is already closed, as `| head` leaves it once `head` has quit, so that - `result.stdout` is None. `branch` is the source's own (default) branch, + `failing` gives git the hook that takes a file the shape's pin names + out of the split commit, which fails the verification. `merge_stderr` + runs with both streams in one pipe, as a person redirecting a log has + them: stdout is then block-buffered, and the order the lines arrive in + is the order the tool flushed them. `stdout_closed` gives the run a + stdout whose reading end is already closed, as `| head` leaves it once + `head` has quit, so that `result.stdout` is None. `branch` is the source's own (default) branch, and `plan_flags` go to `plan` after the ones `write_plan` passes. `before_plan`, when given, is called with the source and the directory that stands for github.com once the assembly is there, before `plan`.""" @@ -242,8 +236,12 @@ def github_run(tmp_path: Path, refuses: str, tree: dict = SOURCE_TREE, shim.write_text(GH_SHIM, encoding="utf-8") shim.chmod(shim.stat().st_mode | stat.S_IXUSR) config = tmp_path / "gitconfig" + hooks = (a_hook_that_unstages(tmp_path / "commit-hooks") if failing + else None) config.write_text(f'[url "{github.as_posix()}/"]\n' - "\tinsteadOf = https://github.com/\n", encoding="utf-8") + "\tinsteadOf = https://github.com/\n" + + (f"[core]\n\thooksPath = {hooks.as_posix()}\n" + if hooks else ""), encoding="utf-8") # The operator's `gh` must not be reachable from here: no token it would # accept, no config to read a login from, and (where it can be done # without losing `git`) no directory that holds one on PATH. @@ -253,12 +251,6 @@ def github_run(tmp_path: Path, refuses: str, tree: dict = SOURCE_TREE, plan = tmp_path / "adoption-plan.yaml" written = write_plan(source, plan, extra=plan_flags) assert written.returncode == 0, written.stderr + written.stdout - if keep_contracts: - answered = plan.read_text(encoding="utf-8") - assert CONTRACTS_TO_SPEC in answered, answered - with plan.open("w", encoding="utf-8", newline="\n") as handle: - handle.write(answered.replace(CONTRACTS_TO_SPEC, KEEP_CONTRACTS, - 1)) stdout = subprocess.PIPE if stdout_closed: read_end, stdout = os.pipe() @@ -525,7 +517,7 @@ def test_a_failed_verification_outranks_a_refused_pull_request(tmp_path): the split. The topic's way out is the verification's too: the re-run it names sets the topic (#481).""" result, github = github_run(tmp_path, "pr create|repo edit", - tree=FAILING_TREE, keep_contracts=True) + failing=True) assert result.returncode == 1, result.stdout + result.stderr assert FINDING in result.stderr, result.stderr @@ -549,10 +541,10 @@ def test_a_failed_verification_outranks_a_refused_pull_request(tmp_path): @WINDOWS_SKIP # the `gh` is a `#!` script, which Windows does not run @needs_filter_repo def test_the_same_source_verifies_when_gh_opens_the_pull_request(tmp_path): - """The control for the test above: it is the plan's answer that fails the - verification, not the refused pull request. Sent to the spec leg, as - `plan` sends it, the pin verifies and the run is exit 0.""" - result, github = github_run(tmp_path, "never", tree=FAILING_TREE) + """The control for the test above: it is the split the hook leaves that + fails the verification, not the refused pull request. Without the hook, + the same source verifies and the run is exit 0.""" + result, github = github_run(tmp_path, "never") assert result.returncode == 0, result.stdout + result.stderr assert VERIFIED in result.stdout, result.stdout diff --git a/tests/test_adopt_ignored_shape_files.py b/tests/test_adopt_ignored_shape_files.py index 5feb03dd..46841f0e 100644 --- a/tests/test_adopt_ignored_shape_files.py +++ b/tests/test_adopt_ignored_shape_files.py @@ -755,14 +755,20 @@ def test_the_assembly_with_a_root_kept_symlink_bootstraps( @needs_filter_repo def test_a_source_pin_kept_in_the_root_fails_verification_with_the_plan_answer( - tmp_path): + adopter, tmp_path, monkeypatch, capsys): """The case the finding is left for once the add is forced: the source has a `contracts/shape-pin.yaml` of its OWN, and the plan keeps `contracts/` in the root. The shape's pin lands beside it, under `shape/`; the one at the pinned path names a file the plan moved to the code leg, and the first bootstrap would refuse with shape-copy-missing. `execute` says so, with the plan answer as the exit, never a hand edit - of the split.""" + of the split. + + Since #524 `check` finds that plan, and `execute` refuses it before any + leg exists, as `plan-root-holds-pin` + (`tests/test_adopt_shape_pin_and_work_dir.py`). So the verification is + the backstop here: `execute` runs in this process with the shape's pin + left out of what that rule asks, as before #524, and still says so.""" own_pin = pin_of(README, "src/app/main.py") source = make_source_repo( tmp_path / "Thing", @@ -781,18 +787,26 @@ def test_a_source_pin_kept_in_the_root_fails_verification_with_the_plan_answer( " resolution: \"kept beside the shape's own\"\n", 1), encoding="utf-8") checked = run_script(ADOPT, "check", "--plan", str(plan)) - assert checked.returncode == 0, checked.stderr + checked.stdout + assert checked.returncode == 1, checked.stderr + checked.stdout + assert (f"FINDING plan-root-holds-pin: the plan keeps the file `{PIN}` " + in checked.stderr), checked.stderr - result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", - "--local-remote-dir", str(tmp_path / "remotes"), - "--work-dir", str(tmp_path / "work")) + pin_files = adopter._pin_files + monkeypatch.setattr(adopter, "_pin_files", lambda read: [ + pin for pin in pin_files(read) if pin[0] != PIN]) + monkeypatch.setenv("LANES_LANE", "") + clear_ambient_pin_sources(monkeypatch) + code = adopter.main(["execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(tmp_path / "remotes"), + "--work-dir", str(tmp_path / "work")]) + result = capsys.readouterr() - assert result.returncode == 1, result.stderr + result.stdout - assert "adoption verified" not in result.stdout - assert COLLISION_PIN in result.stdout - findings = [line for line in result.stderr.splitlines() + assert code == 1, result.err + result.out + assert "adoption verified" not in result.out + assert COLLISION_PIN in result.out + findings = [line for line in result.err.splitlines() if line.startswith("FINDING ")] - assert len(findings) == 1, result.stderr + assert len(findings) == 1, result.err assert findings[0].startswith( f"{MISSING} `src/app/main.py` is named by {PIN} but is not in the " "split " @@ -803,9 +817,9 @@ def test_a_source_pin_kept_in_the_root_fails_verification_with_the_plan_answer( # The finding points at the end of the report, so that is where the way # out must be: after the finding, ending with the split branch's # deletion, which a re-run otherwise meets as `adopt-branch-exists`. - after = result.stderr.split(findings[0], 1)[1] + after = result.err.split(findings[0], 1)[1] assert "fix the plan and re-run" in after - assert after.rstrip().endswith(f"--delete {BRANCH}"), result.stderr + assert after.rstrip().endswith(f"--delete {BRANCH}"), result.err assert git("show", f"{BRANCH}:{PIN}", cwd=source).stdout == own_pin diff --git a/tests/test_adopt_late_failure.py b/tests/test_adopt_late_failure.py index b6f64a27..91376cf2 100644 --- a/tests/test_adopt_late_failure.py +++ b/tests/test_adopt_late_failure.py @@ -48,8 +48,8 @@ import pytest -from conftest import (ADOPT, REPO, git, make_source_repo, run_script, - spelled, write_plan) +from conftest import (ADOPT, PINNED_COPY, REPO, a_hook_that_unstages, git, + make_source_repo, run_script, spelled, write_plan) SPEC, CODE = "spec", "code" LEGS = (SPEC, CODE) @@ -100,21 +100,6 @@ SMALL_TREE = {README: "# Thing\n", "src/app/util.py": "VALUE = 1\n", "specs/001-feature/tasks.md": "- [ ] one\n"} -#: A source with a `contracts/shape-pin.yaml` of its OWN, naming a file the -#: plan sends to the code leg. Kept in the root, it is what the first -#: bootstrap would read, so the verification fails; sent to the spec leg, as -#: `plan` sends it, the adoption verifies. -OWN_PIN = ("schema_version: 1\nkind: pinned_contract_manifest\nfiles:\n" - + "".join(f" - path: {path}\n sha256: \"{'0' * 64}\"\n" - for path in (README, "src/app/main.py"))) -OWN_PIN_TREE = {README: "# Thing\n", ".gitignore": "__pycache__/\n", - "specs/001-feature/spec.md": "# The feature\n", - "src/app/main.py": "import contracts_reader\n", - "contracts/shape-pin.yaml": OWN_PIN} -CONTRACTS_TO_SPEC = " - path: contracts/\n leg: spec\n" -CONTRACTS_KEPT = (" - path: contracts/\n leg: root\n" - " resolution: \"kept beside the shape's own\"\n") - needs_filter_repo = pytest.mark.skipif( shutil.which("git-filter-repo") is None, reason="git filter-repo is not installed: `pip install git-filter-repo`") @@ -193,6 +178,12 @@ def hooks_everywhere(tmp_path: Path, ref_pattern: str) -> dict: test meets it at the leg remotes and at the source alike.""" hooks = tmp_path / "hooks" write_refusing_hook(hooks, ref_pattern) + return global_hooks(tmp_path, hooks) + + +def global_hooks(tmp_path: Path, hooks: Path) -> dict: + """The environment whose git takes every repository's hooks from + `hooks`: a global `core.hooksPath`.""" config = tmp_path / "gitconfig" config.write_text(f"[core]\n\thooksPath = {hooks.as_posix()}\n", encoding=UTF8) @@ -1639,28 +1630,32 @@ def test_a_failed_verification_is_flushed_and_its_way_out_runs(tmp_path): and the exit names the legs, the work directory and the branch with the command that deletes it. - Then the way out as the review of #188 followed it (F1): the plan fixed - and fresh legs, but the work directory and the branch kept. That re-run - is refused for the work directory first, before anything exists; its - way out, a new one, is refused for the branch; and that refusal's way - out, followed as written -- the deletion it prints, then `execute` - again -- ends in `adoption verified`.""" - source = make_source_repo(tmp_path / SOURCE, tree=OWN_PIN_TREE, - edits=()) + Then the way out as the review of #188 followed it (F1): what failed + fixed and fresh legs, but the work directory and the branch kept. That + re-run is refused for the work directory first, before anything exists; + its way out, a new one, is refused for the branch; and that refusal's + way out, followed as written -- the deletion it prints, then `execute` + again -- ends in `adoption verified`. + + What fails is a `pre-commit` hook that takes a file the shape's pin + names out of the split commit (`a_hook_that_unstages`), and the fix is + to run without it. The plan answer that stood for it until #524 -- the + source's own `contracts/shape-pin.yaml` kept in the root -- is refused + before any leg exists now.""" + source = make_source_repo(tmp_path / SOURCE, tree=SMALL_TREE, edits=()) plan = planned(tmp_path, source) - answered = plan.read_text(encoding=UTF8) - assert CONTRACTS_TO_SPEC in answered - plan.write_text(answered.replace(CONTRACTS_TO_SPEC, CONTRACTS_KEPT, 1), - encoding=UTF8) checked = run_script(ADOPT, "check", PLAN_FLAG, str(plan)) assert checked.returncode == 0, checked.stderr + checked.stdout failed = execute(plan, tmp_path / REMOTES, tmp_path / WORK, + env=global_hooks(tmp_path, a_hook_that_unstages( + tmp_path / "commit-hooks")), merge_stderr=True) assert failed.returncode == 1, failed.stdout lines = failed.stdout.splitlines() finding = first_line(lines, "FINDING adopt-shape-file-missing") + assert f"`{PINNED_COPY}` is named by" in lines[finding] printed = [index for index, line in enumerate(lines) if line.startswith((BARE_LINE, " spec", " code", " split", " added"))] @@ -1678,7 +1673,7 @@ def test_a_failed_verification_is_flushed_and_its_way_out_runs(tmp_path): f"{spelled(BRANCH)}") in failed.stdout delete = printed_command(failed.stdout, DELETE_COMMAND) - plan.write_text(answered, encoding=UTF8) + # Fixed: the runs below have no such hook. fresh_legs, new_work = tmp_path / "remotes 2", tmp_path / "work 2" kept = execute(plan, fresh_legs, tmp_path / WORK) assert kept.returncode == 2, kept.stderr + kept.stdout diff --git a/tests/test_adopt_root_symlinks.py b/tests/test_adopt_root_symlinks.py index 5a098072..2e566ee5 100644 --- a/tests/test_adopt_root_symlinks.py +++ b/tests/test_adopt_root_symlinks.py @@ -1029,10 +1029,10 @@ def test_a_link_to_a_pins_file_finds_the_pin_place_wrote_first( under `shape/`, as it does for any path the root holds. The walk now says the same, where it said the link led to what is not in the source tree, and `check` prints no link finding for it. (The root's - `contracts/shape-pin.yaml` is then not the shape's pin, and `execute`'s - verification fails after both legs, as on `main` and as for a file the - source keeps there: a question of what the root keeps at the shape's - pin, not of where this link leads.)""" + `contracts/shape-pin.yaml` is then not the shape's pin: a question of + what the root keeps at the shape's pin, not of where this link leads, + which `plan-root-holds-pin` now answers before any leg, as it does for + a file the source keeps there, #524.)""" files, legs = REAL_CONTRACTS _, plan = planned(tmp_path, files, {SHAPE_PIN: PIN_NAME}, legs) declare(plan, A_PIN) diff --git a/tests/test_adopt_shape_pin_and_work_dir.py b/tests/test_adopt_shape_pin_and_work_dir.py new file mode 100644 index 00000000..9740e9e0 --- /dev/null +++ b/tests/test_adopt_shape_pin_and_work_dir.py @@ -0,0 +1,269 @@ +# SPDX-License-Identifier: Apache-2.0 +"""What the root keeps at the shape's own pin, and a work directory this user +cannot write in, refused before any leg exists (#524). + +Two of the v1.0.0 known limits on #457, L1-K1 and L1-K10, measured on main +8beb3d92 and again on bfed143, the v1.0.0 release commit: + +- L1-K1. `_kept_pin_findings` (#273) asked whether the plan keeps a path + where `execute` writes each DECLARED pin's file, and never asked it of the + shape's own pin, `contracts/shape-pin.yaml`. A file the root kept there, + with a pin or without, passed `check`, and so did a link there that leads + to something: `contracts/shape-pin.yaml -> openglass-pin.yaml`, which finds + the pin `place` writes first, or `-> README.md`. `execute` made and pushed + both legs, wrote the shape's pin beside the source's file, under + `shape/`, and failed its verification as `adopt-shape-pin-unreadable`, + exit 1. Now the same rule asks it of the shape's pin, under #273's code, + `plan-root-holds-pin`, its sentence naming the shape's pin. +- One link, one finding. A link there that leads nowhere was the walk's + (#178, #183: `plan-root-symlink`), because the shape would write its pin + through it. The kept-pin rule names it now, as it names a link AT a + declared pin's file, and the walk does not say it again + (`_links_at_a_pin`). +- L1-K10. `--work-dir` was made (`mkdir(exist_ok=True)`) and never written + in before `_create_leg_remotes`. With `chmod 500` on it, `check --work-dir` + said `plan ok`, and `execute --work-dir` made and pushed both legs and + stopped `extracting the spec leg`, exit 2, with or without a link above a + path the shape writes (`.github -> ci`). Now `execute` makes and removes a + directory in it first, before it opens the source, and refuses as + `adopt-work-dir-unwritable`, naming it; `check --work-dir` names the same + as a finding, beside the plan's own, and refuses it as `execute` does + where it would clone an `org/repo` source there. + +`chmod` keeps neither root nor a Windows user from making a directory, so +the work-directory tests skip where it cannot (`unwritable_work`, as #183's +J4 test does). +""" + +from __future__ import annotations + +import errno +import os +import shutil +from pathlib import Path + +import pytest + +from conftest import ADOPT, run_script +from test_adopt_root_symlinks import ( # noqa: F401 + A_PIN, CASE_ASIDE, CONTRACTS, GITHUB, PIN_NAME, README, REAL_CONTRACTS, + REMOTES, ROOT, ROOT_HOLDS_PIN, SHAPE_PIN, WORK, WORK_FLAG, adopter, + check, declare, execute_args, links_as_links, no_filter_repo_probe, + no_leg_was_created, planned, refused_by, ticked, unwritable_work, +) + +WORK_DIR_UNWRITABLE = "adopt-work-dir-unwritable" +MINE = "mine: 1\n" +NOWHERE = "nowhere.yaml" +#: The two kinds of source entry the finding names here. +A_FILE, A_SYMLINK = "file", "symlink" +SHAPE_PIN_TWIN = "contracts/Shape-Pin.yaml" +#: The entry the source's `contracts/` is kept in the root under. +CONTRACTS_ENTRY = f"{CONTRACTS}/" + + +def finding_lines(result) -> list[str]: + """Every FINDING `check` printed, whatever its code.""" + return [line for line in result.stderr.splitlines() + if line.startswith("FINDING ")] + + +def held(kind: str, path: str, where: str = "at", aside: str = "") -> str: + """The line `check` prints, and `execute`'s refusal carries, for the + `kind` (`file`, `symlink`) at `path` that the plan keeps `where` the + shape writes its own pin.""" + found = "directory" if where == "inside" else kind + through = (", or written through it where it leads nowhere" + if kind == A_SYMLINK else "") + return (f"FINDING {ROOT_HOLDS_PIN}: the plan keeps the {kind} " + f"{ticked(path)} in the assembly root under the entry " + f"{ticked(CONTRACTS_ENTRY)}, {where} {ticked(SHAPE_PIN)}{aside}, " + "where `execute` writes the shape's own pin: the openRepoShape " + "commit the root's copies came from and each copy's digest, " + "which `execute`'s verification, the root's own " + "`validate-pins.py` and `make bootstrap` read at that path. The " + f"split would hold the source's {found} there, and the shape's " + f"pin beside it, under `shape/`{through}: send " + f"{ticked(CONTRACTS_ENTRY)} to a leg, `drop` it, or rename it in " + "the source and re-run `plan`.") + + +def refused_alone(adopter, base: Path, plan: Path, line: str) -> None: + """`check` exits 1 with `line` and no other finding, and `execute` + refuses with that line alone, before any leg exists.""" + checked = check(plan) + assert checked.returncode == 1, checked.stderr + checked.stdout + assert finding_lines(checked) == [line], checked.stderr + refusal = refused_by(adopter, execute_args(base, plan)) + assert refusal.code == ROOT_HOLDS_PIN + assert refusal.detail.splitlines()[1:] == [f" {line}"], refusal.detail + no_leg_was_created(base) + + +# --------------------------------------------------------------------------- +# L1-K1: a path the root keeps at the shape's own pin +# --------------------------------------------------------------------------- + +#: `(the source's own file, the kind the finding names, where, aside)`. +KEPT_AT_THE_PIN = [ + pytest.param(SHAPE_PIN, "at", "", id="a-file"), + pytest.param(SHAPE_PIN_TWIN, "at", CASE_ASIDE, id="a-case-twin"), + pytest.param(f"{SHAPE_PIN}/notes.md", "inside", "", id="a-directory"), +] + + +@pytest.mark.parametrize("pinned", [ + pytest.param(False, id="no-pin"), pytest.param(True, id="a-pin")]) +@pytest.mark.parametrize(("path", "where", "aside"), KEPT_AT_THE_PIN) +def test_a_path_kept_at_the_shapes_pin_is_found_and_refused_before_any_leg( + adopter, no_filter_repo_probe, tmp_path, pinned, path, where, + aside): + """The source's `contracts/` answered `root` keeps its own + `contracts/shape-pin.yaml`, a case twin of it, or a directory of that + name. `place` finds it there and writes the shape's pin beside it, under + `shape/`, so the root's `contracts/shape-pin.yaml` is the source's. With + a pin declared or without, `check` names it once, and `execute` refuses + with that line before any leg exists.""" + files, legs = REAL_CONTRACTS + _, plan = planned(tmp_path, {**files, path: MINE}, {}, legs) + if pinned: + declare(plan, A_PIN) + refused_alone(adopter, tmp_path, plan, held(A_FILE, path, where, aside)) + + +#: `(the symlink, where it leads, whether a pin is declared, aside)`. +LINKS_AT_THE_PIN = [ + # The issue's own: `place` writes the pin's file first, so the link + # finds a file and the shape's pin goes beside it. + pytest.param(SHAPE_PIN, PIN_NAME, True, "", + id="to-the-pins-file-place-writes-first"), + pytest.param(SHAPE_PIN, README, False, "", id="to-a-kept-file"), + # The walk's before (#183): the shape would write its pin through it. + pytest.param(SHAPE_PIN, NOWHERE, False, "", id="leading-nowhere"), + pytest.param(SHAPE_PIN, NOWHERE, True, "", + id="leading-nowhere-with-a-pin"), + pytest.param(SHAPE_PIN_TWIN, NOWHERE, False, CASE_ASIDE, + id="a-case-twin-leading-nowhere"), +] + + +@pytest.mark.parametrize(("link", "target", "pinned", "aside"), + LINKS_AT_THE_PIN) +def test_a_link_at_the_shapes_pin_is_one_finding_by_the_rule_that_sees_it( + adopter, no_filter_repo_probe, links_as_links, tmp_path, link, + target, pinned, aside): + """A symlink kept AT the shape's pin is the kept-pin rule's, wherever it + leads, as a symlink AT a declared pin's file is (#273). The walk follows + the same path and, where the link leads nowhere, would refuse it too; + it does not say the same link again: one link, one finding, and + `execute` refuses with that line alone.""" + files, legs = REAL_CONTRACTS + _, plan = planned(tmp_path, files, {link: target}, legs) + if pinned: + declare(plan, A_PIN) + refused_alone(adopter, tmp_path, plan, + held(A_SYMLINK, link, aside=aside)) + + +@pytest.mark.skipif(shutil.which("git-filter-repo") is None, + reason="`execute` refuses without git filter-repo first") +def test_execute_refuses_the_issues_link_and_makes_no_leg(tmp_path): + """The issue's run, as a person makes it: with a pin, and + `contracts/shape-pin.yaml -> openglass-pin.yaml` kept, `execute` made + both legs and exited 1 with `adopt-shape-pin-unreadable`. It now exits 2 + with `plan-root-holds-pin`, and no leg repository exists.""" + files, legs = REAL_CONTRACTS + _, plan = planned(tmp_path, files, {SHAPE_PIN: PIN_NAME}, legs) + declare(plan, A_PIN) + remotes = tmp_path / REMOTES + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + WORK_FLAG, str(tmp_path / WORK)) + assert result.returncode == 2, result.stdout + result.stderr + assert f"\n {held(A_SYMLINK, SHAPE_PIN)}\n" in result.stderr + assert result.stderr.startswith(f"REFUSED {ROOT_HOLDS_PIN}: ") + assert not remotes.exists() + + +# --------------------------------------------------------------------------- +# L1-K10: a work directory this user cannot write in +# --------------------------------------------------------------------------- + +FIX = ("Pass a --work-dir this user can write in, or none: without one, " + "each run makes a new one.") + +#: `(files, links, legs)`: a plan `check` passes, and the same with a link +#: above a path the shape writes, which `execute` asks `git` about in its +#: work directory (#183). +CHECKED_OK = [ + pytest.param(REAL_CONTRACTS[0], {}, REAL_CONTRACTS[1], id="no-link"), + pytest.param({"ci/workflows/ci.yml": "on: push\n"}, {GITHUB: "ci"}, + {"ci/": ROOT, GITHUB: ROOT}, id="a-link-above"), +] + + +def cannot_write(work: Path) -> str: + """Why `--work-dir work` is refused, as `check` and `execute` say it.""" + return (f"--work-dir {ticked(str(work.resolve()))} cannot be written: " + "making a directory in it, and removing it again, failed " + f"({os.strerror(errno.EACCES)}), and `execute` builds the spec " + "leg, the code leg and the split there, only after it has made " + "both leg repositories") + + +@pytest.mark.parametrize(("files", "links", "legs"), CHECKED_OK) +def test_check_names_a_work_directory_it_cannot_write_in( + tmp_path, unwritable_work, files, links, legs): + """`check --work-dir` asks the directory as `execute` will: a writable + one passes and is left as it was, and one this user cannot write in is + a finding that names it, where it said `plan ok`.""" + _, plan = planned(tmp_path, files, links, legs) + writable = tmp_path / "writable" + passed = check(plan, WORK_FLAG, str(writable)) + assert passed.returncode == 0, passed.stderr + passed.stdout + assert list(writable.iterdir()) == [] + checked = check(plan, WORK_FLAG, str(unwritable_work)) + assert checked.returncode == 1, checked.stderr + checked.stdout + assert finding_lines(checked) == [ + f"FINDING {WORK_DIR_UNWRITABLE}: {cannot_write(unwritable_work)}. " + f"{FIX}"], checked.stderr + + +def test_check_refuses_a_source_it_would_clone_into_that_directory( + tmp_path, unwritable_work): + """An `org/repo` source is cloned into the work directory to be read, so + in one this user cannot write in `check` has no source to read: it + refuses as `execute` does, where it ended in a PermissionError + traceback. Nothing is cloned, so nothing asks `gh`.""" + files, legs = REAL_CONTRACTS + _, plan = planned(tmp_path, files, {}, legs) + checked = check(plan, "--source", "acme/thing", + WORK_FLAG, str(unwritable_work)) + assert checked.returncode == 2, checked.stderr + checked.stdout + assert checked.stderr == ( + f"REFUSED {WORK_DIR_UNWRITABLE}: {cannot_write(unwritable_work)}\n" + f"Remediation: {FIX}\n") + + +@pytest.mark.parametrize(("files", "links", "legs"), CHECKED_OK) +def test_execute_refuses_a_work_directory_it_cannot_write_in_before_any_leg( + adopter, no_filter_repo_probe, tmp_path, unwritable_work, files, + links, legs): + """`execute` asks first, before it opens the source, so before any leg + exists, and names the directory; from the command line too, where the + refusal comes before it asks for `git filter-repo`. It made and pushed + both legs and stopped `extracting the spec leg`.""" + _, plan = planned(tmp_path, files, links, legs) + refusal = refused_by(adopter, execute_args(tmp_path, plan)) + assert refusal.code == WORK_DIR_UNWRITABLE + assert refusal.detail == cannot_write(unwritable_work) + assert refusal.remediation == ( + f"Remediation: no leg repository was created and nothing was " + f"pushed. {FIX}") + no_leg_was_created(tmp_path) + result = run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(tmp_path / REMOTES), + WORK_FLAG, str(unwritable_work)) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr == f"{refusal}\n" + no_leg_was_created(tmp_path)