diff --git a/shape-doctor.py b/shape-doctor.py index e37ef4d2..f8903571 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -8161,11 +8161,12 @@ def sibling_slot(root: Path, sibling: Path, row: dict, only names. A FILE, OR A FOLDER THAT CANNOT BE LISTED OR ENTERED, IS `not-a-clone` - TOO: nothing can be cloned into it. `make siblings` itself ends in a - traceback on each today (`siblings.py:558`, #735), so the row says what - is there and nothing of what `make siblings` does with it - (`beside_sentence`). A folder that cannot be entered cannot be asked - for its `.git` either, so that question's `OSError` reads "no `.git`". + TOO: nothing can be cloned into it. A holder whose pinned `siblings.py` + predates #735 ends `make siblings` in a traceback on each, and one that + has #735 calls it NOT A CLONE, so the row says what is there and + nothing of what `make siblings` does with it (`beside_sentence`), which + is true of both. A folder that cannot be entered cannot be asked for its + `.git` either, so that question's `OSError` reads "no `.git`". """ if sibling == root: return "is-the-holder", None @@ -8310,9 +8311,9 @@ def url_less_members(root: Path, rows: list) -> list[str]: "project.yaml that says it is this member", ""), } -#: What stands at a `not-a-clone` slot `make siblings` cannot read today, as -#: `unclonable` names it: it ends in a traceback on each (#735), so the row -#: says what is there and no word of what `make siblings` does with it. +#: What stands at a `not-a-clone` slot that a holder's pinned `siblings.py` +#: from before #735 ends in a traceback on, as `unclonable` names it: the +#: row says what is there and no word of what `make siblings` does with it. UNREADABLE_BESIDE = {"file": "is a file, not a folder", "unlistable": "is a folder that cannot be listed"} @@ -8329,8 +8330,9 @@ def beside_sentence(ctx: Context, entry: dict) -> str: `--json` keeps them raw: `json.dumps` escapes them. A FILE, OR A FOLDER THAT CANNOT BE LISTED, is `not-a-clone` with no - word from `make siblings`, which ends in a traceback on it today (#735): - the sentence stops at what stands there (`UNREADABLE_BESIDE`). + word from `make siblings`, which ends in a traceback on it in a holder + whose pinned copy predates #735: the sentence stops at what stands + there (`UNREADABLE_BESIDE`). """ state = entry["sibling"] where = ctx.root.parent / entry["project"] diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 224ef102..e78e2edb 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -504,6 +504,45 @@ def project_id(path: Path) -> str | None: return str(data.get("id")) if isinstance(data, dict) else None +def holds_git(path: Path) -> bool: + """Does `path` hold a `.git`? False where that cannot be asked (#735): + in a folder that cannot be entered (mode `000`), Python 3.12's + `Path.exists` raises `PermissionError`, and `make siblings` and `make + park` ended in that traceback.""" + try: + return (path / ".git").exists() + except OSError: + return False + + +def not_a_clone(target: Path) -> tuple[str, str] | None: + """`(what stands at target, what kind of thing it is)` for a `target` + with no `.git` that this command will not write into, in the words its + `NOT A CLONE` finding says it in; None for nothing at all, or an empty + folder, which a clone goes into. + + A FILE, OR A FOLDER THAT CANNOT BE LISTED, IS NOT A CLONE TOO (#735). + Only a folder with something in it was asked about, and `iterdir` + raised on the other two: `NotADirectoryError` for a file, and + `PermissionError` for a folder that can be entered and not listed (mode + `0311`; `711` is common for another user's directory) or cannot be + entered at all. Each ended `make siblings` in a traceback, exit 1, and + the members after it were never placed. Nothing can tell that such a + folder is empty, and `git clone` refuses one it cannot read. The words + are the ones the doctor's `members` row says of the same two slots. + """ + try: + if not target.exists(): + return None + if not target.is_dir(): + return "is a file, not a folder", "file" + if any(target.iterdir()): + return "exists, is not empty and is not a git clone", "directory" + return None + except OSError: + return "is a folder that cannot be listed", "directory" + + def verify_sibling(row: dict, target: Path, url: str, repository: str, sibling: Sibling) -> bool: """Is the clone at `target` the member this row names? @@ -650,14 +689,15 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], f"the two are not the same path, then re-run.") return sibling - present = (target / ".git").exists() - if not present and target.exists() and any(target.iterdir()): + present = holds_git(target) + occupied = None if present else not_a_clone(target) + if occupied: + found, thing = occupied sibling.state = "NOT A CLONE" sibling.finding = ( - f"{who}: {at} exists, is not empty and is not a git " - "clone, so this command will not write into it. Move it aside or " - "name the directory something else; nothing here overwrites a " - "directory somebody made.") + f"{who}: {at} {found}, so this command will not write into it. " + f"Move it aside or name the {thing} something else; nothing here " + f"overwrites a {thing} somebody made.") return sibling if not present: @@ -828,7 +868,7 @@ def dispatch(root: Path, row: dict, urls: dict[str, str], target: str, "holder keeps its name) so the two are not the same path.") return sibling - if not (path / ".git").exists(): + if not holds_git(path): sibling.state = "SKIPPED no clone" sibling.finding = ( f"{who}: skipped: no working clone at {at}; run `make " diff --git a/tests/test_family.py b/tests/test_family.py index 818e7d17..e94494ed 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -1928,6 +1928,87 @@ def test_siblings_reports_a_directory_that_is_not_a_clone_at_all(estate): assert (estate["folder"] / "IRRS" / "notes.md").read_text() == "mine\n" +#: A mode hides a folder's contents only on POSIX, and never from root. +MODE_HOLDS = pytest.mark.skipif( + os.name == "nt" or getattr(os, "geteuid", lambda: 0)() == 0, + reason="a mode hides a folder's contents only on POSIX, from a user " + "who is not root") + +#: `(the folder's mode, or None for a file; what the finding says is there; +#: what kind of thing it is)` for the slots `make siblings` ended in a +#: traceback on (#735): a file, and a folder holding a file whose mode hides +#: it, one that can be entered and not listed (`711` is common for another +#: user's directory) and one that cannot be entered. +SLOTS_NOT_READ = [ + pytest.param(None, "is a file, not a folder", "file", id="a-file"), + pytest.param(0o311, "is a folder that cannot be listed", "directory", + id="a-folder-it-cannot-list", marks=MODE_HOLDS), + pytest.param(0o000, "is a folder that cannot be listed", "directory", + id="a-folder-it-cannot-enter", marks=MODE_HOLDS), +] + + +def slot_not_read(request, slot, mode): + """A file at `slot`, or, for a `mode`, a folder there holding `notes.md` + with that mode, given its own mode back when the test ends; the file + that says `mine` is returned.""" + if mode is None: + slot.write_text("mine\n", encoding="utf-8") + return slot + slot.mkdir() + (slot / "notes.md").write_text("mine\n", encoding="utf-8") + slot.chmod(mode) + request.addfinalizer(lambda: slot.chmod(0o755)) + return slot / "notes.md" + + +@pytest.mark.parametrize("mode, found, thing", SLOTS_NOT_READ) +def test_siblings_reports_a_slot_it_cannot_read_as_not_a_clone( + estate, request, mode, found, thing): + """#735: A FILE, OR A FOLDER IT CANNOT LIST, IS NOT A CLONE. + + `place` asked only whether a folder had something in it, and `iterdir` + raised on these: `NotADirectoryError` for a file, `PermissionError` for + the two folders. `make siblings` ended in that traceback, exit 1, and + IRSS, the member after it, was never placed. Now each is `NOT A CLONE`, + as the doctor's `members` row calls the slot: the finding is the one a + folder of files gets, saying what stands there, the slot is left as it + is, and the next member is placed.""" + slot = estate["folder"] / "IRRS" + mine = slot_not_read(request, slot, mode) + result = siblings(estate["holder"]) + assert "Traceback" not in result.stderr, result.stderr + assert result.returncode == 1, result.stdout + result.stderr + assert "sibling IRRS NOT A CLONE -" in layout(result.stdout), \ + result.stdout + where = estate["holder"].resolve().parent / "IRRS" + assert (f"FINDING IRRS: {where} {found}, so this command will not write " + f"into it. Move it aside or name the {thing} something else; " + f"nothing here overwrites a {thing} somebody made.") in \ + result.stderr, result.stderr + assert (estate["folder"] / "IRSS" / ".git").is_dir() + if mode is not None: + slot.chmod(0o755) + assert mine.read_text(encoding="utf-8") == "mine\n" + assert not (slot / ".git").exists() + + +@MODE_HOLDS +@pytest.mark.skipif(shutil.which("make") is None, + reason="needs `make` on PATH") +def test_a_make_verb_skips_a_folder_it_cannot_enter(estate, request): + """#735's `--make` half: `dispatch` asked `(path / ".git").exists()`, + which Python 3.12 answers with `PermissionError` in a folder that cannot + be entered, so `make park` ended in that traceback. It is now a member + with no working clone, skipped by name, as every other one is.""" + slot_not_read(request, estate["folder"] / "IRRS", 0o000) + result = siblings(estate["holder"], "--make", "park", "--dry-run") + assert "Traceback" not in result.stderr, result.stderr + assert "sibling IRRS SKIPPED no clone -" in [ + " ".join(line.split()) for line in result.stdout.splitlines()], \ + result.stdout + + def test_siblings_warns_about_the_parent_folder_and_moves_nothing(family, tmp_path): """THE DOCTOR PATTERN, NOT A MOVER (#76). The holder here sits under a diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 731bfeb5..e48685aa 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -4002,9 +4002,9 @@ def test_the_stranger_note_says_from_which_folder_make_siblings_calls_it( "working-clone": "each with a working clone beside the holder", } -#: What the row says instead of a `not-a-clone` slot `make siblings` ends in -#: a traceback on today (#735): what is there, and nothing of what `make -#: siblings` does with it. +#: What the row says instead of a `not-a-clone` slot that a holder's pinned +#: `siblings.py` from before #735 ends in a traceback on: what is there, and +#: nothing of what `make siblings` does with it. UNREADABLE_SAYS = { "file": "is a file, not a folder, so it is not counted as its working " "clone", @@ -4058,9 +4058,15 @@ def test_the_stranger_note_says_from_which_folder_make_siblings_calls_it( pytest.param("clone", "working-clone", id="the-working-clone"), ] -#: A file at `../IRRS`, which `make siblings` ends in a traceback on today -#: (#735). -A_FILE = ("file", "not-a-clone") +#: A file at `../IRRS`, and the two `LOCKED` folders: `not-a-clone`, and the +#: slots `make siblings` ended in a traceback on before #735. +UNREADABLE_SLOTS = [ + pytest.param("file", "not-a-clone", id="a-file"), + pytest.param("unlistable", "not-a-clone", id="a-folder-it-cannot-list", + marks=LOCKED_HOLDS), + pytest.param("unenterable", "not-a-clone", + id="a-folder-it-cannot-enter", marks=LOCKED_HOLDS), +] def holder_with_slot(holder: dict, tmp_path: Path, slot: str) -> Path: @@ -4109,14 +4115,7 @@ def locked(request, path: Path, slot: str) -> None: request.addfinalizer(lambda: path.chmod(0o755)) -@pytest.mark.parametrize("slot, state", [ - *SLOTS, - pytest.param(*A_FILE, id="a-file"), - pytest.param("unlistable", "not-a-clone", id="a-folder-it-cannot-list", - marks=LOCKED_HOLDS), - pytest.param("unenterable", "not-a-clone", - id="a-folder-it-cannot-enter", marks=LOCKED_HOLDS), -]) +@pytest.mark.parametrize("slot, state", [*SLOTS, *UNREADABLE_SLOTS]) def test_the_row_says_what_make_siblings_does_with_each_slot( standard, holder, tmp_path, request, slot, state): """#669: ONE ROW PER SLOT STATE. `--json` carries the state as @@ -4127,10 +4126,10 @@ def test_the_row_says_what_make_siblings_does_with_each_slot( workstation's layout, and `without_working_clone` keeps its meaning. A FILE, OR A FOLDER THAT CANNOT BE LISTED, is `not-a-clone`, and the - row says nothing of `make siblings` there, which ends in a traceback on - it today (#735). The folder was the review's MUST on #705: `iterdir`'s - `PermissionError` made the row a FINDING and the holder DRIFTED, exit - 1, where main says COMPLIANT, exit 0.""" + row says nothing of `make siblings` there, which a holder's pinned copy + from before #735 ends in a traceback on. The folder was the review's + MUST on #705: `iterdir`'s `PermissionError` made the row a FINDING and + the holder DRIFTED, exit 1, where main says COMPLIANT, exit 0.""" root = holder_with_slot(holder, tmp_path, slot) locked(request, root.parent / FAMILY_MEMBER, slot) result = doctor(standard, root, "--json") @@ -4149,24 +4148,20 @@ def test_the_row_says_what_make_siblings_does_with_each_slot( assert json.loads(result.stdout)["verdict"] == "COMPLIANT", result.stdout -@pytest.mark.parametrize("slot, state", [ - *SLOTS, - pytest.param(*A_FILE, id="a-file", marks=pytest.mark.xfail( - strict=True, raises=AssertionError, - reason="#735: `siblings.py:558` raises NotADirectoryError on a " - "file, so `make siblings` ends in a traceback")), -]) +@pytest.mark.parametrize("slot, state", [*SLOTS, *UNREADABLE_SLOTS]) def test_make_siblings_dry_run_gives_the_rows_word_for_each_slot( - standard, holder, tmp_path, slot, state): + standard, holder, tmp_path, request, slot, state): """#669: THE AGREEMENT. The holder's own `scripts/siblings.py --dry-run`, run from its family folder, says of the member what the row's `sibling` state says it does: the same word where it refuses the directory, `would clone` where the row says it places it, and `present, would fetch` where it fetches it as the member. The row restates `place`'s order, and this is what keeps the two tools answering - together about one directory (#146). A FILE is held to it too, as a - strict xfail until #735 makes `make siblings` answer NOT A CLONE.""" + together about one directory (#146). A FILE, and a folder that cannot + be listed or entered, are held to it too: `make siblings` ended in a + traceback on each, and since #735 calls each NOT A CLONE.""" root = holder_with_slot(holder, tmp_path, slot) + locked(request, root.parent / FAMILY_MEMBER, slot) entry = rows_of(doctor(standard, root, "--json"))["members"]["detail"][ "members"][0] assert entry["sibling"] == state, entry