diff --git a/shape-doctor.py b/shape-doctor.py index 5be86db0..ae29579d 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -2009,12 +2009,16 @@ 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]: +def lost_repo_shape_cure(ctx: Context, own: str, lost: Path, rerun: str, + again: bool = True) -> 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). + `again` says "run it AGAIN" where the doctor ran the gate and it crashed, + as the validator rows' crash does; the `legs` and `members` rows' lockstep + line is one the reader never ran, and says "run it" (#750). + 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 @@ -2036,14 +2040,14 @@ def lost_repo_shape_cure(ctx: Context, own: str, lost: Path, 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") + then = (f"run `{rerun}`{' again' if again else ''} 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"root 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 " @@ -2302,7 +2306,7 @@ def unloaded_manifest_row(ctx: Context, check_id: str, label: str, own: str, return Row(check_id, label, FINDING, reason, 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 " + f"root committed and {own} imports, where " "update-shape.py apply would copy a newer one, then " f"run {own} again: it refuses {name} by name until it " "loads. A deletion already committed comes back from " @@ -5523,6 +5527,15 @@ def lockstep_next(ctx: Context, gate: tuple, noun: str) -> tuple: The `legs` and `members` rows say the same thing about a leg and about a member, so the sentence is written here once -- and it is the row's `why`, apart from the line, never a ` # ` tail on it (#277). + + A GATE THAT CANNOT IMPORT `repo_shape` IS NOT THE LINE (#750). The root's + own copy, beside a lost `scripts/repo_shape.py`, dies on that import and + exits 1, the code it uses for findings, so that line cannot do what the + sentence says of it, and the `pins` and `family` rows beside it already + print the cure (#697). So the line is theirs, from + `lost_repo_shape_cure`: the one command that puts the file back, or none + where HEAD no longer holds it. Its `why` says to run the gate once the + file is back, and the sentence above follows it. """ used, _, own_copy = validator_for(ctx, *gate) if (not used.is_file() @@ -5530,10 +5543,16 @@ def lockstep_next(ctx: Context, gate: tuple, noun: str) -> tuple: return None, None command = validator_command(ctx.shape, used, own_copy, ["--root", str(ctx.root)]) - return command, (f"no submodule update cures a {noun} whose gitlink " - "or pin is missing or disagrees, or a mount git cannot " - f"read a HEAD in. {used.name} prints what it finds and " - "names the repair where it has one") + said = (f"no submodule update cures a {noun} whose gitlink " + "or pin is missing or disagrees, or a mount git cannot " + f"read a HEAD in. {used.name} prints what it finds and " + "names the repair where it has one") + if own_copy and not imports_repo_shape(ctx.shape, used, own_copy): + cure, why = lost_repo_shape_cure( + ctx, gate[0], repo_shape_for(ctx.shape, used, own_copy), command, + again=False) + return cure, f"{why}. {said[:1].upper()}{said[1:]}" + return command, said def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: diff --git a/tests/test_doctor_next_is_a_command.py b/tests/test_doctor_next_is_a_command.py index d34530cd..85f2cefb 100644 --- a/tests/test_doctor_next_is_a_command.py +++ b/tests/test_doctor_next_is_a_command.py @@ -109,7 +109,9 @@ ) #: Doctor B's crashing validator and unloaded manifest (#278, #298), from the #: module that owns them. -from test_doctor_validator_rows import crashing, lost_gate, unloaded +from test_doctor_validator_rows import ( + crashing, lockstep_lost, lost_gate, unloaded, +) # --- making each converted row print its NEXT ------------------------------- @@ -320,6 +322,16 @@ def _validator_crashed_restore(standard, project, holder, tmp_path): return rows_of(doctor(standard, root, "--json"))["manifest"] +def _lockstep_restore(case): + """The builder of the `legs` (`pins`) or `members` (`family`) row for a + gitlink off its pin in a root whose own gate lost its `repo_shape.py`: + the lockstep line is the restore line (#750).""" + def build(standard, project, holder, tmp_path): + root, row_id = lockstep_lost(case, project, holder, tmp_path) + return rows_of(doctor(standard, root, "--json"))[row_id] + return build + + 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.""" @@ -637,12 +649,22 @@ def build(standard, project, holder, tmp_path): "unloaded_manifest_row-restore": ( _unloaded_manifest_restore, "it puts back the scripts/repo_shape.py HEAD holds, the copy this " - "project committed and scripts/validate-manifest.py imports, where " + "root 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 " + "root committed and scripts/validate-manifest.py imports, where " + "update-shape.py apply would copy a newer one. Then run `"), + "lockstep_next-legs-restore": ( + _lockstep_restore("pins"), + "it puts back the scripts/repo_shape.py HEAD holds, the copy this " + "root committed and scripts/validate-pins.py imports, where " + "update-shape.py apply would copy a newer one. Then run `"), + "lockstep_next-members-restore": ( + _lockstep_restore("family"), + "it puts back the scripts/repo_shape.py HEAD holds, the copy this " + "root committed and scripts/validate-family.py imports, where " "update-shape.py apply would copy a newer one. Then run `"), "check_legs": ( _legs, diff --git a/tests/test_doctor_validator_rows.py b/tests/test_doctor_validator_rows.py index 34345f60..a8b5282d 100644 --- a/tests/test_doctor_validator_rows.py +++ b/tests/test_doctor_validator_rows.py @@ -23,8 +23,11 @@ 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 +the verdict, and a NEXT that is run -- and not on the prose of a row's `why`, +the sentence beside its NEXT (#277). Where a row's why is the only place a +command or a fact is named, the test reads the why for that and no more: the +gate to run after the cure (#697, #750), the file, the commit a deletion comes +back from. The fixtures are those of `tests/test_shape_doctor.py`, imported, so the standard, the project and the holder are the ones every other doctor test reads. @@ -914,6 +917,85 @@ def test_the_unloaded_manifest_row_prints_the_same_restore_line( assert row["next"] == doctor_module(standard).restore_from_head(root, LOST) +# --- #750: the lockstep line of the legs and members rows is the cure too --- +# +# `lockstep_next` named the root's own gate for a gitlink off its pin, and +# that gate dies on `import repo_shape` where the root lost it: the line the +# `pins` and `family` rows beside it had just stopped printing (#697). + +#: Per gate case: the row that hands over its lockstep line, how the root's +#: gitlink is put off its pin, and the finding the gate prints for it. +LOCKSTEP_OF = { + "pins": ("legs", make_gitlink_moved, "pin-gitlink-mismatch"), + "family": ("members", make_member_gitlink_mismatch, + "member-gitlink-mismatch"), +} + + +def lockstep_lost(case: str, project: Path, holder: dict, tmp_path: Path, + committed: bool = False) -> tuple[Path, str]: + """`(root, the row that hands over the lockstep line)` for a root with a + gitlink off its pin and its own `case` gate's `scripts/repo_shape.py` + lost: in the tree, which HEAD still holds, or `committed`.""" + row_id, off_its_pin, _finding = LOCKSTEP_OF[case] + root = lost_gate(case, project, holder, tmp_path, committed) + off_its_pin(root) + return root, row_id + + +@pytest.mark.parametrize("case", sorted(LOCKSTEP_OF)) +def test_the_lockstep_line_of_a_root_that_lost_repo_shape_is_the_cure( + standard, project, holder, tmp_path, case): + """The `legs` or `members` NEXT is the line the gate's own row prints, + and the gate it used to be is the why's.""" + root, row_id = lockstep_lost(case, project, holder, tmp_path) + rows = rows_of(doctor(standard, root, "--json")) + row = rows[row_id] + assert row["status"] == "FINDING", row + assert row["next"] == doctor_module(standard).restore_from_head(root, LOST) + assert row["next"] == rows[case]["next"], (row["next"], rows[case]["next"]) + assert rerun_of(standard, root, case) in row["why"], row["why"] + + +@WINDOWS_SKIP +@pytest.mark.skipif(shutil.which("sh") is None, reason="needs sh") +@pytest.mark.parametrize("case", sorted(LOCKSTEP_OF)) +def test_the_cure_pasted_and_then_the_lockstep_line_both_run( + standard, project, holder, tmp_path, case): + """Both lines PASTED as printed, from `/` with no `PYTHONPATH`: the cure + exits 0, and the row then names the lockstep gate, which prints its own + finding for the gitlink and exits 1, where it died on + `ModuleNotFoundError` before.""" + root, row_id = lockstep_lost(case, project, holder, tmp_path) + row = rows_of(doctor(standard, root, "--json"))[row_id] + restored = pasted_from_elsewhere(row["next"], tmp_path) + assert restored.returncode == 0, restored.stdout + restored.stderr + after = rows_of(doctor(standard, root, "--json"))[row_id] + assert after["next"] == rerun_of(standard, root, case), after["next"] + gate = pasted_from_elsewhere(after["next"], tmp_path) + whole = gate.stdout + gate.stderr + assert gate.returncode == 1, whole + assert LOCKSTEP_OF[case][2] in whole, whole + assert "ModuleNotFoundError" not in whole, whole + + +@pytest.mark.parametrize("case", sorted(LOCKSTEP_OF)) +def test_a_lockstep_line_whose_repo_shape_left_head_too_is_no_line( + standard, project, holder, tmp_path, case): + """HEAD does not hold a file whose deletion was COMMITTED, so no line puts + it back and the gate would only crash again: the row has no NEXT, and its + why says where the file comes back from, names the gate to run then, and + keeps the sentence about why no checkout cures the gitlink.""" + root, row_id = lockstep_lost(case, project, holder, tmp_path, + committed=True) + row = rows_of(doctor(standard, root, "--json"))[row_id] + assert row["status"] == "FINDING", row + assert row["next"] is None, row + for said in (LOST, "parent of the commit", "No submodule update cures", + rerun_of(standard, root, case)): + assert said in row["why"], (said, row["why"]) + + # --- #278: the lockstep line of the `legs` and `members` rows ---------------- def assert_no_pythonpath_line(*streams: str) -> None: