siblings.py: a file, or a folder make siblings cannot list or enter, at ../<Project> is NOT A CLONE, reported and left as it is, so the run no longer ends in a traceback at iterdir before the members after it are placed, and make park no longer ends in one over a folder it cannot enter (#735)

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"]

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 "

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

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