From 0a178e49909b934a2bdede12f3d053440ca683a8 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:18:12 +0000 Subject: [PATCH] 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) L1-J3 of #183's re-verify. Under `core.symlinks=false` (Git for Windows' default, and what `git init` writes on a filesystem that cannot hold a symlink), a symlink the plan keeps ABOVE a path the shape writes is a plain file in the assembly's clone, and nothing can be written beneath it. The finding `check` printed for such a link gave the reason for a disk that holds symlinks ("it lands at `tools/deep/validate-pins.py`, ..."), and its fix, "change where it points", led on: with `scripts -> ci`, `check` says `plan ok` and `execute` refuses `adopt-symlinks-off`. `_a_file_above` asks `_PlannedRoot._a_file_here`'s question of the link a walk meets first ABOVE its path, of the same `_SymlinkProbe`, and only for a walk `_root_symlink_findings` is about to report. Where it is true, the finding says `adopt-symlinks-off`'s reason and sends the link to a leg or `drop`. One finding per link, in the same place in the list; a link the walk follows into something the root keeps is still passed by `check` and refused by `execute` as `adopt-symlinks-off`, after `check`'s list. With symlinks on, or no probe possible, every finding is byte for byte as before. Tests: tests/test_adopt_root_symlinks.py, 149 -> 156. Six cases of the new line (check and execute alike, before any leg) and the fix it gives, followed, passing both of `execute`'s refusals; main's own test of the old line under symlinks off replaced; `check` passing `.github -> ci` under symlinks off asserted, pinning the order; `links_as_links` given to three tests of the symlinks-on line that ran with the runner's default, which is `core.symlinks=false` on Windows. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 80 ++++++++++++++---- tests/test_adopt_root_symlinks.py | 130 +++++++++++++++++++++++++----- 2 files changed, 174 insertions(+), 36 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index e4fa0451..3c9ece0b 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -5276,7 +5276,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 @@ -5809,11 +5811,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 @@ -5909,7 +5915,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 @@ -6055,9 +6063,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()) @@ -6400,6 +6408,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): @@ -6414,17 +6431,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 " @@ -6475,6 +6521,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), # and `execute` refuses with it, where both would otherwise # end in a traceback. self._answer = True @@ -6493,8 +6540,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): diff --git a/tests/test_adopt_root_symlinks.py b/tests/test_adopt_root_symlinks.py index f3f56ab4..a592e33b 100644 --- a/tests/test_adopt_root_symlinks.py +++ b/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 @@ -351,13 +353,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: @@ -598,7 +602,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"}, @@ -611,7 +616,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"}, @@ -670,7 +675,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.""" @@ -1283,10 +1289,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() @@ -1413,17 +1425,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: