adopt-project.py: where git checks a symlink out as a plain file, check's finding for a link kept above a path the shape writes says the clone holds it as a file, so scripts -> tools/deep no longer says where the scripts land and sends the person to change where it points, on to adopt-symlinks-off (#531)

adopt-project.py

@@ -5639,7 +5639,9 @@ def _entry_findings(plan: Plan, source: Source,
     disk by each command when it runs (#183): whether anything is at the
     place outside the assembly it leads to, a climb above the root from its
     own work directory; and whether `git` there checks it out as a symlink at
-    all, or as a plain file the shape's copy goes beside.
+    all, or as a plain file the shape's copy goes beside. The second is asked
+    too of a symlink ABOVE a shape path that a finding names, which then
+    says the clone holds it as a file (#531).
 
     `_leg_findings` is handed the TREE, because the paths the plan keeps in
     the root are read out of it (#172, #191): without it a mount path at, or
@@ -6174,11 +6176,15 @@ def _print_what_will_happen(plan: Plan, source: Source, names: dict,
 # shape writes is a plain file in the clone, which `place` finds there and
 # writes the copy beside, so nothing about where that link leads is refused
 # (B-OFF of #183's re-verify); a link ABOVE one is `execute`'s
-# `adopt-symlinks-off`, since nothing can be written beneath a file. A
-# target is absolute as this machine's `open()` reads it: `C:/x`, a drive
-# letter's path, is a place on this disk only on Windows. On Linux and macOS
-# it is a relative path, into a directory named `C:`, and it is followed as
-# one, as `open()` and the materializer follow it there.
+# `adopt-symlinks-off`, since nothing can be written beneath a file. A link
+# ABOVE one that `check` reports anyway, for where it would lead on a disk
+# that holds symlinks, is named there as that file too, as
+# `adopt-symlinks-off` names it, and not by where it leads (L1-J3 of #183's
+# re-verify, #531): no target makes the file a directory. A target is
+# absolute as this machine's `open()` reads it: `C:/x`, a drive letter's
+# path, is a place on this disk only on Windows. On Linux and macOS it is a
+# relative path, into a directory named `C:`, and it is followed as one, as
+# `open()` and the materializer follow it there.
 #
 # EACH DECLARED PIN'S FILE IS ONE OF THOSE PATHS (#273). `execute` hands the
 # materializer `_recorded_pins`, and `place` writes each
@@ -6277,7 +6283,9 @@ class _PlannedRoot:
     and nothing else in `.git`. Where `git` checks a symlink out as a plain
     file, a symlink AT a path the shape writes is that file, wherever it
     leads: `probe` asks this command's work directory, and only for a link
-    a walk cannot follow (`_a_file_here`).
+    a walk cannot follow (`_a_file_here`). The walk itself still follows a
+    symlink ABOVE a path the shape writes: `_root_symlink_findings` asks the
+    same `probe` of one it reports (`_a_file_above`, #531).
     `walks` adds what the shape itself writes, path by path, as `place` does,
     so `CLAUDE.md -> AGENTS.md` finds the `AGENTS.md` the shape wrote first.
     The paths are those of the pins the plan declares too (`pins`), each
@@ -6423,9 +6431,9 @@ def _a_file_here(self, walk: _Walk) -> bool:
         false, Git for Windows' default), `place` finds that file there and
         writes the shape's copy beside it, whatever the link leads to, as on
         `main` (B-OFF of #183's re-verify). A link the walk meets first
-        ABOVE its path is `execute`'s `adopt-symlinks-off`. The disk is
-        asked only for a link the walk could not follow, so a plan with none
-        makes no probe."""
+        ABOVE its path is `execute`'s `adopt-symlinks-off`, or, where `check`
+        reports it, `_a_file_above`'s. The disk is asked only for a link the
+        walk could not follow, so a plan with none makes no probe."""
         return (bool(walk.links)
                 and walk.links[0][0].casefold() == walk.path.casefold()
                 and not self.probe.symlinks())
@@ -6768,6 +6776,15 @@ def _root_symlink_findings(plan: Plan, source: Source,
     `plan-empty-symlink` names every such symlink in the source, first in
     `_entry_findings`' list, with the fix in the source, so it is not said
     again here, wherever in the walk's chain that symlink is.
+
+    Where `git` checks a symlink out as a plain file, the link a reported
+    walk meets first ABOVE its path is that file in the clone, and the
+    finding says so, as `adopt-symlinks-off` does, in place of where the
+    link leads (`_a_file_above`, L1-J3 of #183's re-verify, #531). It is
+    still the one finding for that link, in the same place in the list, and
+    a link the walk follows into something the root keeps is still no
+    finding of this check's: `execute` refuses it after this list, as
+    `adopt-symlinks-off`.
     """
     findings: list[str] = []
     if not any(mode == SYMLINK_MODE for _, mode, _, _ in tree):
@@ -6782,17 +6799,46 @@ def _root_symlink_findings(plan: Plan, source: Source,
                 continue
             seen.add(walk.links[0][0])
             findings.append(_root_symlink_finding(
-                walk, why, _CASE_ASIDE if fold else ""))
+                walk, why, _CASE_ASIDE if fold else "",
+                as_a_file=_a_file_above(walk, probe)))
     return findings
 
 
-def _root_symlink_finding(walk: _Walk, why: str, aside: str) -> str:
+def _a_file_above(walk: _Walk, probe: _SymlinkProbe) -> bool:
+    """Whether the clone holds the symlink `walk` meets first, ABOVE its own
+    path, as a plain file: where `git` checks a symlink out as one
+    (`core.symlinks` false, Git for Windows' default), nothing is written
+    beneath it, wherever it leads (#531). `_PlannedRoot._a_file_here`'s
+    question, of a link above the path rather than at it, and asked of the
+    same `probe`. The disk is asked only for a link `check` reports, so a
+    plan with none still makes no probe."""
+    return (bool(walk.links)
+            and walk.links[0][0].casefold() != walk.path.casefold()
+            and not probe.symlinks())
+
+
+def _root_symlink_finding(walk: _Walk, why: str, aside: str, *,
+                          as_a_file: bool = False) -> str:
     """The line `check` prints for a symlink `walk` cannot be written
     through, and what to do. A walk that stopped at a symlink with an empty
-    target never comes here: that symlink is #303's `plan-empty-symlink`."""
+    target never comes here: that symlink is #303's `plan-empty-symlink`.
+
+    `as_a_file` (`_a_file_above`): the clone holds the link as a plain file.
+    That is the reason then, in `adopt-symlinks-off`'s words, and not `why`,
+    which is the reason on a disk that holds symlinks; and the fix is in the
+    plan, because on that disk no target the link is given makes it a
+    directory, and "change where it points" led on to `adopt-symlinks-off`
+    (#531)."""
     link, target = walk.links[0]
     fix = (f"Send {_spelled(link)} to a leg or `drop` it, or change where it "
            "points in the source and re-run `plan`.")
+    if as_a_file:
+        why = ("`git` here checks a symlink out as a plain file "
+               "(`core.symlinks` is false), so the assembly's clone would "
+               f"hold {_spelled(link)} as a file, wherever it points, and "
+               "nothing can be written beneath a file")
+        fix = (f"Send {_spelled(link)} to a leg or `drop` it in the plan, "
+               "and run `check` again.")
     return (f"FINDING {ROOT_SYMLINK}: the plan keeps {_spelled(link)} in the "
             f"root, a symlink to {_spelled(target)}, and "
             f"{_spelled(walk.path)}, a path the shape writes, runs through "
@@ -6843,6 +6889,7 @@ 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 the finding of such a disk for a link ABOVE one (#531),
                 # beside `adopt-work-dir-unwritable` (#524), and
                 # `_refuse_what_check_finds` refuses with it, where both
                 # would otherwise end in a traceback. `execute` itself
@@ -6863,8 +6910,11 @@ def _refuse_symlinks_checked_out_as_files(plan: Plan, source: Source,
     directory where a file is. It used to die there in a traceback after
     both legs were pushed. A symlink AT a shape path is unaffected: as a
     file it is still taken, and the shape's copy goes beside it, as `check`
-    counts it too (`_PlannedRoot._a_file_here`). The probe is made only when
-    a plan needs it, so an adoption with no such link runs exactly as before.
+    counts it too (`_PlannedRoot._a_file_here`). A link ABOVE one that
+    `check` reports is refused before this, by `_refuse_what_check_finds`,
+    whose line says the same of the file (`_a_file_above`, #531); this
+    refuses the rest, which `check` passes. The probe is made only when a
+    plan needs it, so an adoption with no such link runs exactly as before.
     """
     tree = source.tree()
     if not any(mode == SYMLINK_MODE for _, mode, _, _ in tree):

tests/test_adopt_root_symlinks.py

@@ -38,8 +38,10 @@
   shape would write beneath, because the assembly's clone holds it as a file.
   A link AT a path the shape writes is then that file, which `place` writes
   the copy beside, so neither `check` nor `execute` refuses it, wherever it
-  leads (B-OFF). The tests of a link `check` follows tell `git` to check a
-  symlink out as one, since Git for Windows' own default is a file.
+  leads (B-OFF). A link ABOVE one that `check` reports says it is that file
+  there, not where it leads (#531). The tests of a link `check` follows tell
+  `git` to check a symlink out as one, since Git for Windows' own default is
+  a file.
 - The materializer's own refusals, for a caller that skipped `check`.
 
 The end-to-end tests at the bottom need `git filter-repo` and a disk that
@@ -352,13 +354,15 @@ def ticked(path: str) -> str:
 
 
 def finding(link: str, target: str, shape_path: str, why: str,
-            aside: str = "") -> str:
-    """The line `check` prints, and `execute`'s refusal carries."""
+            aside: str = "", *, fix: str = "") -> str:
+    """The line `check` prints, and `execute`'s refusal carries. `fix` is
+    its last sentence where that is not the one for a disk that holds
+    symlinks."""
+    fix = fix or (f"Send {ticked(link)} to a leg or `drop` it, or change "
+                  "where it points in the source and re-run `plan`.")
     return (f"{FINDING}the plan keeps {ticked(link)} in the root, a symlink "
             f"to {ticked(target)}, and {ticked(shape_path)}, a path the shape "
-            f"writes, runs through it{aside}, but {why}. Send {ticked(link)} "
-            "to a leg or `drop` it, or change where it points in the source "
-            "and re-run `plan`.")
+            f"writes, runs through it{aside}, but {why}. {fix}")
 
 
 def leads(link: str, here: str, which: str) -> str:
@@ -599,7 +603,8 @@ def test_check_reports_the_link_and_execute_refuses_it_before_any_leg(
     refused_alike(adopter, tmp_path, plan, finding(*expected))
 
 
-def test_a_link_into_a_submodule_kept_in_the_root_is_reported(tmp_path):
+def test_a_link_into_a_submodule_kept_in_the_root_is_reported(
+        links_as_links, tmp_path):
     """A gitlink is a directory in a checkout, holding another repository:
     a link into it is not a link into `a file`."""
     _, plan = planned(tmp_path, {}, {GITHUB: "vendor/lib/ci"},
@@ -612,7 +617,7 @@ def test_a_link_into_a_submodule_kept_in_the_root_is_reported(tmp_path):
 
 
 def test_one_finding_per_link_however_many_shape_paths_run_through_it(
-        tmp_path):
+        links_as_links, tmp_path):
     """`contracts -> /srv` carries four shape paths; the finding names the
     first one the materializer writes."""
     _, plan = planned(tmp_path, {}, {CONTRACTS: "/srv"},
@@ -671,7 +676,8 @@ def test_a_link_at_the_copy_is_one_finding_by_the_rule_that_sees_it(
 
 
 @pytest.mark.parametrize(("raw", "spelled"), SPELLED)
-def test_a_target_that_is_not_plain_text_is_spelled(tmp_path, raw, spelled):
+def test_a_target_that_is_not_plain_text_is_spelled(links_as_links, tmp_path,
+                                                    raw, spelled):
     """As #166's findings spell a path: between backticks, a byte that is
     not UTF-8 as `\\udcff`, the plan's own spelling of it (#329), and a
     newline as `\\n`, so the finding is one line that reads one way."""
@@ -1284,10 +1290,16 @@ def test_execute_refuses_a_link_it_would_check_out_as_a_file(
         adopter, no_filter_repo_probe, tmp_path, monkeypatch):
     """The assembly's clone would hold `.github` as a plain file, and the
     materializer died there with NotADirectoryError after both legs were
-    pushed. It is refused before them, and the probe leaves nothing."""
+    pushed. It is refused before them, and the probe leaves nothing.
+    `check` passes the plan, as #183 left it: the walk follows `.github`
+    into the `ci/` the root keeps, so no finding of `check`'s comes first,
+    and the refusal is `execute`'s own, after that list (#531 changes what a
+    finding says, not that order)."""
     _, plan = github_to_ci(tmp_path)
     for key, value in SYMLINKS_OFF.items():
         monkeypatch.setenv(key, value)
+    checked = check(plan)
+    assert checked.returncode == 0, checked.stderr + checked.stdout
     refusal = refused_by(adopter, execute_args(tmp_path, plan))
     assert refusal.code == SYMLINKS_OFF_CODE
     work = (tmp_path / WORK).resolve()
@@ -1414,17 +1426,93 @@ def test_with_symlinks_off_a_link_at_a_path_is_a_file_passed_by_both(
     assert list(work.iterdir()) == []
 
 
-def test_with_symlinks_off_a_link_above_a_path_is_still_followed(
-        tmp_path, monkeypatch):
-    """Only a link AT a path the shape writes is the file a checkout without
-    symlinks makes of it (`_a_file_here`). A link the shape writes BENEATH
-    is still followed by `check`, and its finding stands."""
-    _, plan = planned(tmp_path, {}, {GITHUB: OUTSIDE_CI}, {GITHUB: ROOT})
+#: Why a link a path the shape writes runs BENEATH cannot be written
+#: through where `git` checks a symlink out as a plain file (#531): `{}` is
+#: the link. `adopt-symlinks-off`'s reason, as `check` says it.
+HELD_AS_A_FILE = ("`git` here checks a symlink out as a plain file "
+                  "(`core.symlinks` is false), so the assembly's clone would "
+                  "hold {} as a file, wherever it points, and nothing can be "
+                  "written beneath a file")
+
+
+def as_a_file(link: str, target: str, shape_path: str,
+              aside: str = "") -> str:
+    """The line `check` prints, and `execute`'s refusal carries, for a link
+    the shape writes beneath, where `git` checks it out as a plain file: no
+    target it is given changes that there, so the fix is in the plan."""
+    return finding(link, target, shape_path,
+                   HELD_AS_A_FILE.format(ticked(link)), aside,
+                   fix=f"Send {ticked(link)} to a leg or `drop` it in the "
+                       "plan, and run `check` again.")
+
+
+#: L1-J3's source: `scripts -> tools/deep`, whose scripts would run from
+#: two directories down, where a disk holds symlinks.
+TOOLS_DEEP = "tools/deep"
+SCRIPTS_DEEP = ({"tools/deep/run.sh": ECHO}, {SCRIPTS: TOOLS_DEEP},
+                {TOOLS_DIR: ROOT, SCRIPTS: ROOT})
+
+#: `(*PLANNED, (link, target, shape path[, aside]))`: links `check` reports
+#: where a disk holds symlinks, each ABOVE the first path the shape writes
+#: through it. Each is L1-J3's question: the clone holds the link as a file.
+ABOVE_AS_A_FILE = [
+    pytest.param(*SCRIPTS_DEEP, (SCRIPTS, TOOLS_DEEP, VALIDATE_PINS),
+                 id="L1-J3-scripts-two-directories-down"),
+    pytest.param({}, {GITHUB: OUTSIDE_CI}, {GITHUB: ROOT},
+                 (GITHUB, OUTSIDE_CI, WORKFLOW), id="D6-a-relative-link-out"),
+    pytest.param(LINK_TO_GH, {GITHUB: TOOLING_GH},
+                 {"tooling/": CODE, GITHUB: ROOT},
+                 (GITHUB, TOOLING_GH, WORKFLOW),
+                 id="D5-into-a-path-sent-to-a-leg"),
+    pytest.param({}, {CONTRACTS: "/srv"}, {CONTRACTS: ROOT},
+                 (CONTRACTS, "/srv", "contracts/spec-pin.yaml"),
+                 id="one-finding-for-four-paths"),
+    pytest.param({}, {SHAPE_DIR: "."}, {SHAPE_DIR: ROOT},
+                 (SHAPE_DIR, ".", SHAPE_README),
+                 id="the-collision-directory-is-the-root"),
+    pytest.param({}, {GITHUB_TWIN: OUTSIDE_CI}, {GITHUB_TWIN: ROOT},
+                 (GITHUB_TWIN, OUTSIDE_CI, WORKFLOW, CASE_ASIDE),
+                 id="a-case-twin-above-a-shape-path"),
+]
+
+
+@pytest.mark.parametrize((*PLANNED, "expected"), ABOVE_AS_A_FILE)
+def test_with_symlinks_off_a_link_above_a_path_is_named_as_a_file(
+        adopter, no_filter_repo_probe, tmp_path, monkeypatch, files, links,
+        legs, expected):
+    """L1-J3 of #183's re-verify (#531). Where `git` checks a symlink out as
+    a plain file, a link the shape writes BENEATH is that file in the
+    assembly's clone, and nothing can be written beneath a file, wherever
+    the link leads. `check`'s one finding for it said where it led, and
+    sent the person to change that, which leads on to `execute`'s
+    `adopt-symlinks-off` there. Now it says what `adopt-symlinks-off` says,
+    with a fix that works on that disk, and `execute` refuses with the same
+    line, word for word, before any leg."""
+    _, plan = planned(tmp_path, files, links, legs)
     links_as_files(monkeypatch)
-    checked = check(plan)
-    assert checked.returncode == 1, checked.stderr + checked.stdout
-    assert symlink_findings(checked) == [
-        finding(GITHUB, OUTSIDE_CI, WORKFLOW, climbs(GITHUB))]
+    refused_alike(adopter, tmp_path, plan, as_a_file(*expected))
+
+
+@pytest.mark.parametrize("leg", [CODE, "drop"])
+def test_with_symlinks_off_the_fix_the_finding_gives_works_there(
+        adopter, tmp_path, monkeypatch, leg):
+    """L1-J3's finding sent the person to change where `scripts` points,
+    and on that disk no target helps. The fix it gives now works there:
+    `scripts` answered as it says, `check` passes, and so do `execute`'s
+    list of `check`'s findings and its own probe for a link the shape
+    writes beneath."""
+    links_as_files(monkeypatch)
+    files, links, legs = SCRIPTS_DEEP
+    _, kept = planned(tmp_path / ROOT, files, links, legs)
+    said = symlink_findings(check(kept))
+    assert said == [as_a_file(SCRIPTS, TOOLS_DEEP, VALIDATE_PINS)]
+    answered = tmp_path / leg
+    _, plan = planned(answered, files, links, {**legs, SCRIPTS: leg})
+    passed_alike(adopter, answered, plan)
+    loaded = adopter.Plan.load(plan)
+    work = answered / WORK
+    assert adopter._refuse_symlinks_checked_out_as_files(
+        loaded, loaded.open_source(None, work), work) is None
 
 
 def git_reads_symlinks_as(directory: Path) -> str: