diff --git a/shape-doctor.py b/shape-doctor.py index e4287748..af802bf8 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -1293,6 +1293,23 @@ def head_holds(root: Path, relative: str) -> bool: return kind == "blob" +def restore_from_head(root: Path, relative: str, + platform: str | None = None) -> str: + """The line that puts `relative` back as the root's HEAD holds it: + `git -C checkout HEAD -- `, spelled for the READER'S + shell (#298, #697). + + ONE SPELLING FOR EVERY ROW THAT RESTORES THE FILE `head_holds` has + asked about: the unloaded-manifest row and an own-copy validator row + that crashed on the same loss print the same line, and `platform` is + passed on to `quote_arg` as `bump_leg_command`'s is, so both dialects + are asserted from either host. + """ + target = os.name if platform is None else platform + return (f"git -C {quote_arg(root, target)} checkout HEAD -- " + f"{quote_arg(relative, target)}") + + #: The four variables `gh` reads a token from, which no validator's child #: keeps (#297). GH_TOKEN_VARIABLES = ("GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", @@ -1581,6 +1598,9 @@ def run_validator(ctx: Context, script: Path, args: list[str], where the row says FINDING. That is their variable and their site-packages, and the gate has neither: the row is about the gate, and the line it prints is the project's own command, with no `-S` in it. + WHERE THE CRASH IS THE ROOT'S LOST `scripts/repo_shape.py` THE LINE IS + THE CURE INSTEAD, because that is the one crash the reader's own + environment can hide (#697): `lost_repo_shape_cure`. AND NO CHILD REACHES A NETWORK UNLESS ITS CALLER SAYS SO (#297). A project that pins a neutral product has a `validate-pins.py` that, with @@ -1746,6 +1766,48 @@ def validator_command(shape: Path, used: Path, own_copy: bool, args: list, return f"env PYTHONPATH={quote_arg(scripts, target)} {command}" +def lost_repo_shape_cure(ctx: Context, own: str, lost: Path, + rerun: str) -> tuple[str | None, str]: + """`(the NEXT, the why)` for a root's own validator `own` that crashed + because `lost`, the `repo_shape.py` it imports from beside itself, is not + there, where `rerun` is the line that runs `own` (#697). + + THE NEXT IS THE CURE, NOT THE VALIDATOR. The row used to print `rerun`, + and a reader whose own `PYTHONPATH`, or a `.pth` in whose site-packages, + reaches another `repo_shape` ran it and saw a pass, or a different + finding, under a row that said the gate crashed: the doctor starts every + child with no inherited `PYTHONPATH` and no `site` (#306, #342), and a + pasted line has neither guard. `-E -S` on the line would have stopped + both routes and was declined: the line stays the project's own command. + So the NEXT is the one command that puts the file back, the line the + unloaded-manifest row prints for the same loss (`restore_from_head`, + #298), and `rerun` is the why's, to run once the file is back. + + NO NEXT WHERE HEAD DOES NOT HOLD THE FILE (`head_holds`): a deletion + already committed is not in HEAD, so the line could not restore it, and + the gate would only crash again. The why says so, where the file comes + back from, and the gate to run after. + + THE CONDITION IS THE ROW'S REASON'S -- the file is not beside the root's + own copy (`imports_repo_shape`) -- and it is asked of the root's own copy + only: the standard's template copy has none beside it by design. + """ + relative = lost.relative_to(ctx.root).as_posix() + then = (f"run `{rerun}` again once the file is back; before that, a " + "reader whose own PYTHONPATH or site-packages reaches another " + "repo_shape sees it run on that one, which says nothing about " + "this root's own copy") + if head_holds(ctx.root, relative): + return (restore_from_head(ctx.root, relative), + f"it puts back the {relative} HEAD holds, the copy this " + f"project committed and {own} imports, where update-shape.py " + f"apply would copy a newer one. Then {then}") + return None, (f"HEAD does not hold {relative}, so no git checkout HEAD " + f"puts it back and {own} would only crash again: a " + "deletion already committed comes back from the parent of " + f"the commit that made it. Then {then}") + + def validator_row(ctx: Context, check_id: str, label: str, own: str, template: str, args: list[str], *, output: list[str] | None = None) -> Row: @@ -1772,7 +1834,11 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, imports (`imports_repo_shape`). The TEMPLATE copy crashing is this checkout's failure and not the project's: it carries `detail.environment`, so the verdict is CANNOT ANSWER, exit 3. Either - way the NEXT is the validator's own line, which shows where it stopped. + way the NEXT is the validator's own line, which shows where it stopped + -- EXCEPT where the root's own copy crashed for want of that + `repo_shape.py` (#697): its NEXT is `git checkout HEAD` on the file, or + none where HEAD does not hold it, and the validator's line is the why's + (`lost_repo_shape_cure`). AND A TEMPLATE THIS CHECKOUT CANNOT GIVE ITS `PYTHONPATH` IS NOT RUN (#278): `pythonpath_unnamable` says why, the row is an environment row @@ -1837,18 +1903,21 @@ def validator_row(ctx: Context, check_id: str, label: str, own: str, detail["crashed"] = True reason = f"{which} crashed before reaching a verdict (exit {code}): " \ f"{crash}" + command = validator_command(ctx.shape, used, own_copy, args) + why = CRASH_WHY if not imports_repo_shape(ctx.shape, used, own_copy): lost = repo_shape_for(ctx.shape, used, own_copy) reason += (f"; {lost.relative_to(ctx.root).as_posix()}, which it " "imports from beside itself, is not there" if own_copy else f"; {lost} is not there") + if own_copy: + command, why = lost_repo_shape_cure(ctx, own, lost, command) if not own_copy: detail["environment"] = True reason += ("; that copy is this checkout's, not the project's, " "so nothing here is a statement about the project") - command = validator_command(ctx.shape, used, own_copy, args) return Row(check_id, label, FINDING, reason, command, detail, - why=CRASH_WHY) + why=why) verb = "refuses" if code >= 2 else "reports a finding" reason = f"{which} {verb} (exit {code})" + (f": {quoted}" if quoted else "") # A COMMAND THAT RUNS WHEN IT IS PASTED. This row used to print @@ -1988,8 +2057,7 @@ def unloaded_manifest_row(ctx: Context, check_id: str, label: str, own: str, f"that deleted it, and then {own} refuses {name} by " "name until it loads", None, detail) return Row(check_id, label, FINDING, reason, - f"git -C {quote_arg(ctx.root)} checkout HEAD -- " - f"{quote_arg(lost)}", detail, + restore_from_head(ctx.root, lost), detail, why=f"it puts back the {lost} HEAD holds, the copy this " f"project committed and {own} imports, where " "update-shape.py apply would copy a newer one, then " diff --git a/tests/test_doctor_next_is_a_command.py b/tests/test_doctor_next_is_a_command.py index 3dade5e8..11480aad 100644 --- a/tests/test_doctor_next_is_a_command.py +++ b/tests/test_doctor_next_is_a_command.py @@ -109,7 +109,7 @@ ) #: Doctor B's crashing validator and unloaded manifest (#278, #298), from the #: module that owns them. -from test_doctor_validator_rows import crashing, unloaded +from test_doctor_validator_rows import crashing, lost_gate, unloaded # --- making each converted row print its NEXT ------------------------------- @@ -313,6 +313,13 @@ def _unloaded_manifest_restore(standard, project, holder, tmp_path): return rows_of(doctor(standard, root, "--json"))[row_id] +def _validator_crashed_restore(standard, project, holder, tmp_path): + # An own gate that crashed for want of the `repo_shape.py` it lost, which + # HEAD still holds: the same restore line, on a valid manifest (#697). + root = lost_gate("manifest", project, holder, tmp_path) + return rows_of(doctor(standard, root, "--json"))["manifest"] + + def _pin_row(standard, root, restore): """The `shape currency` row for a root whose OWN pin does not read, and the proof that the fixture reached the branch it was made for.""" @@ -632,6 +639,11 @@ def build(standard, project, holder, tmp_path): "it puts back the scripts/repo_shape.py HEAD holds, the copy this " "project committed and scripts/validate-manifest.py imports, where " "update-shape.py apply would copy a newer one"), + "validator_row-crashed-restore": ( + _validator_crashed_restore, + "it puts back the scripts/repo_shape.py HEAD holds, the copy this " + "project committed and scripts/validate-manifest.py imports, where " + "update-shape.py apply would copy a newer one. Then run `"), "check_legs": ( _legs, "checks each leg out at the commit this repository records for it, " diff --git a/tests/test_doctor_validator_rows.py b/tests/test_doctor_validator_rows.py index 564ed618..34345f60 100644 --- a/tests/test_doctor_validator_rows.py +++ b/tests/test_doctor_validator_rows.py @@ -17,6 +17,11 @@ way" for a root that had also lost `scripts/repo_shape.py`, where the gate dies on the import instead. +#697 IS THE CRASH ROW'S NEXT FOR THAT SAME LOSS: the gate, which a reader +whose own `PYTHONPATH` reaches another `repo_shape` runs and sees pass. It is +the restore line #298 gave the unloaded-manifest row, and the gate is the +row's why. + EVERY ASSERTION IS ON WHAT A COMMAND DOES -- the row's status, its `detail`, the verdict, and a NEXT that is run -- and never on the words of a row's `why`, the sentence beside its NEXT (#277). The fixtures are those of @@ -51,8 +56,8 @@ from test_shape_doctor import ( # noqa: F401 BREAKAGES, DOCTOR, FAMILY_BREAKAGES, FAMILY_NAME, TEMPLATE_FALLBACK, assert_next_runs_the_roots_own, break_manifest, doctor, doctor_module, - holder, loader_line, pasted_from_elsewhere, project, red_root, rows_of, - scaffolded_here, standard, + holder, loader_line, pasted_from_elsewhere, project, red_root, + root_without_its_repo_shape, rows_of, scaffolded_here, standard, ) #: Doctor A's builders for a leg, and a member, whose gitlink is not its pin #: (#276): the lockstep line these rows hand over is A's, so its fixtures are @@ -762,6 +767,153 @@ def test_a_crash_for_want_of_repo_shape_says_which_file_is_not_there( in row["reason"]), row +# --- #697: a gate that crashed for want of repo_shape is not its own NEXT ---- +# +# The NEXT of an own-copy gate's crash row was the gate. Run by a reader whose +# own `PYTHONPATH`, or a `.pth` in whose site-packages, reaches a `repo_shape`, +# it printed `manifest ok` under a row that said FINDING. Where the crash is +# the root's lost `scripts/repo_shape.py` the NEXT is the cure instead, the +# line the unloaded-manifest row prints (#298), and the gate is the why's. + +def lost_gate(case: str, project: Path, holder: dict, tmp_path: Path, + committed: bool = False) -> Path: + """The root whose own `case` gate lost `scripts/repo_shape.py`, with a + VALID manifest, so the file is the only fault: the loss left in the tree, + which HEAD still holds, or with `committed` COMMITTED, which it does not. + """ + root = root_without_its_repo_shape(case, project, holder, tmp_path, + red=False) + if committed: + git("rm", "-q", "--", LOST, cwd=root) + git("commit", "-q", "-m", "Drop repo_shape.py", "--", LOST, cwd=root) + return root + + +def rerun_of(standard: Path, root: Path, case: str) -> str: + """The line that runs the root's own `case` gate on the root: what the + row's why tells a reader to run, and what its NEXT was before #697.""" + return doctor_module(standard).validator_command( + standard.resolve(), root / TEMPLATE_FALLBACK[case]["own"], True, + ["--root", root]) + + +@pytest.mark.parametrize("case", sorted(TEMPLATE_FALLBACK)) +def test_a_gate_that_lost_repo_shape_is_sent_to_put_it_back( + standard, project, holder, tmp_path, case): + """The row is what it was -- a crash, a FINDING, exit 1, the verdict + DRIFTED -- and its NEXT is the one command that puts back the file HEAD + holds, the line the unloaded-manifest row prints for the same loss.""" + root = lost_gate(case, project, holder, tmp_path) + result = doctor(standard, root, "--json") + row = rows_of(result)[case] + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["detail"]["own_copy"] is True, row + assert row["detail"]["exit"] == 1, row + assert "ModuleNotFoundError" in row["reason"], row + assert result.returncode == 1, result.stdout + result.stderr + assert payload_of(result)["verdict"].startswith("DRIFTED"), result.stdout + assert command_of(row)[:2] == ["git", "-C"], row["next"] + assert Path(command_of(row)[2]).resolve() == root, row["next"] + assert command_of(row)[3:] == ["checkout", "HEAD", "--", LOST], row["next"] + assert row["next"] == doctor_module(standard).restore_from_head(root, LOST) + + +@pytest.mark.parametrize("case", sorted(TEMPLATE_FALLBACK)) +def test_the_restore_line_puts_the_file_back_and_the_gate_then_passes( + standard, project, holder, tmp_path, case): + """Run, the NEXT exits 0 and the tree matches HEAD again; the why names + the gate to run after it, and that gate, run, passes, so does its row.""" + root = lost_gate(case, project, holder, tmp_path) + row = rows_of(doctor(standard, root, "--json"))[case] + rerun = rerun_of(standard, root, case) + assert rerun in row["why"], row["why"] + restored = run_argv(command_of(row)) + assert restored.returncode == 0, restored.stdout + restored.stderr + assert git("status", "--porcelain", "--", LOST, cwd=root).stdout == "" + gate = run_argv(rerun.split()) + assert gate.returncode == 0, gate.stdout + gate.stderr + assert rows_of(doctor(standard, root, "--json"))[case]["status"] == "ok" + + +@WINDOWS_SKIP +@pytest.mark.skipif(shutil.which("sh") is None, reason="needs sh") +@pytest.mark.parametrize("case", sorted(TEMPLATE_FALLBACK)) +def test_the_restore_line_pasted_from_elsewhere_lets_the_gate_pass( + standard, project, holder, tmp_path, case): + """Both lines PASTED, whole, from `/` with no `PYTHONPATH`: the + restore, then the gate the why names, which prints its `ok` and exits 0 + where it died on `ModuleNotFoundError` before.""" + root = lost_gate(case, project, holder, tmp_path) + row = rows_of(doctor(standard, root, "--json"))[case] + restore = pasted_from_elsewhere(row["next"], tmp_path) + assert restore.returncode == 0, restore.stdout + restore.stderr + gate = pasted_from_elsewhere(rerun_of(standard, root, case), tmp_path) + whole = gate.stdout + gate.stderr + assert gate.returncode == 0, whole + assert "ModuleNotFoundError" not in whole, whole + + +@pytest.mark.parametrize("case", sorted(TEMPLATE_FALLBACK)) +def test_a_gate_whose_repo_shape_left_head_too_gets_no_line( + standard, project, holder, tmp_path, case): + """HEAD does not hold a file whose deletion was COMMITTED, so `checkout + HEAD` could not put it back and the gate would only crash again: the row + has no NEXT, is still the crash, and its why says why, where the file + comes back from, and the gate to run then.""" + root = lost_gate(case, project, holder, tmp_path, committed=True) + result = doctor(standard, root, "--json") + row = rows_of(result)[case] + assert row["status"] == "FINDING", row + assert row["detail"]["crashed"] is True, row + assert row["next"] is None, row + assert result.returncode == 1, result.stdout + result.stderr + for said in (LOST, "HEAD", "parent of the commit", + rerun_of(standard, root, case)): + assert said in row["why"], (said, row["why"]) + + +@pytest.mark.parametrize("case", sorted(TEMPLATE_FALLBACK)) +def test_a_crash_with_another_cause_keeps_the_gate_as_its_next( + standard, project, holder, tmp_path, case): + """`scripts/repo_shape.py` is there and the gate raises on a line of its + own: nothing to put back, so the NEXT is the gate, as it was, and the why + is the crash row's.""" + here, root, row_id, _own = crashing(f"own-{case}", standard, project, + holder, tmp_path) + row = rows_of(doctor(here, root, "--json"))[row_id] + assert row["detail"]["crashed"] is True, row + assert row["next"] == rerun_of(here, root, case), row["next"] + assert row["why"] == doctor_module(here).CRASH_WHY, row["why"] + + +#: `restore_from_head`'s line in each dialect, a path that needs no quoting +#: and one that does (an apostrophe is `sh`'s `'\''` and PowerShell's `''`). +RESTORE_SPELLINGS = [ + ("posix", "/w/a", f"git -C /w/a checkout HEAD -- {LOST}"), + ("posix", "/w/it's here", + f"git -C '/w/it'\\''s here' checkout HEAD -- {LOST}"), + ("nt", "C:\\w\\a", f"git -C C:\\w\\a checkout HEAD -- {LOST}"), + ("nt", "C:\\it's here", f"git -C 'C:\\it''s here' checkout HEAD -- {LOST}"), +] + + +@pytest.mark.parametrize(("platform", "root", "line"), RESTORE_SPELLINGS) +def test_the_restore_line_is_spelled_for_each_dialect(standard, platform, root, + line): + assert doctor_module(standard).restore_from_head(root, LOST, + platform) == line + + +def test_the_unloaded_manifest_row_prints_the_same_restore_line( + standard, project, holder, tmp_path): + """One spelling for the two rows that restore `scripts/repo_shape.py`.""" + root, row_id, _refusal = unloaded("project", project, holder, tmp_path, + "tree") + row = rows_of(doctor(standard, root, "--json"))[row_id] + assert row["next"] == doctor_module(standard).restore_from_head(root, LOST) + + # --- #278: the lockstep line of the `legs` and `members` rows ---------------- def assert_no_pythonpath_line(*streams: str) -> None: diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index d17e86b3..4031c5f2 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -10127,7 +10127,7 @@ def doctor_with_no_pythonpath(standard: Path, root: Path): @pytest.mark.parametrize("case", sorted(TEMPLATE_FALLBACK)) def test_an_own_validator_without_repo_shape_is_a_finding_the_paste_agrees( standard, project, holder, tmp_path, case): - """(a): the row quotes the ImportError, and the own-copy NEXT, pasted from + """(a): the row quotes the ImportError, and the own-copy line, pasted from `/` with no `PYTHONPATH`, dies on that same import with the row's exit. THE RUN AND THE LINE ARE ONE PROCESS FOR THIS COPY: neither has a @@ -10136,6 +10136,12 @@ def test_an_own_validator_without_repo_shape_is_a_finding_the_paste_agrees( produced (`manifest-topic`, ...) is in NEITHER, because neither run got as far as reading it -- which is what separates this from a validator that ran and was red. + + THE LINE IS THE ROW'S WHY'S SINCE #697: the NEXT of this crash is `git + checkout HEAD` on the lost file, so that a reader whose own environment + reaches another `repo_shape` is not sent to a line that passes for them + (`tests/test_doctor_validator_rows.py`), and the gate is what the why + tells them to run after it. This test runs that line as the NEXT once was. """ spec = TEMPLATE_FALLBACK[case] root = root_without_its_repo_shape(case, project, holder, tmp_path, @@ -10148,10 +10154,14 @@ def test_an_own_validator_without_repo_shape_is_a_finding_the_paste_agrees( row["reason"], row assert spec["finding"] not in row["reason"], row # The own copy's line is what it has always been: no environment. - assert "PYTHONPATH" not in row["next"], row["next"] - assert_next_runs_the_roots_own(row, root, Path(spec["own"]).name) + line = doctor_module(standard).validator_command( + standard.resolve(), root / spec["own"], True, ["--root", root]) + assert "PYTHONPATH" not in line, line + assert line in row["why"], row["why"] + assert Path(row["detail"]["validator"]).resolve() == \ + (root / spec["own"]).resolve(), row["detail"] - pasted = pasted_from_elsewhere(row["next"], tmp_path) + pasted = pasted_from_elsewhere(line, tmp_path) whole = pasted.stdout + pasted.stderr assert pasted.returncode == row["detail"]["exit"], whole assert "ModuleNotFoundError: No module named 'repo_shape'" in whole, whole