diff --git a/shape-doctor.py b/shape-doctor.py index 9518ef83..2b20eb0e 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3024,6 +3024,13 @@ def check_shape_currency(ctx: Context) -> Row: without this file being told. """ us = ctx.update_shape() + # A PIN OR MANIFEST KEPT AS A LINK IS ASKED ABOUT FIRST, before anything + # reads the pin or opens this checkout as the upstream (#286): see + # `linked_record_row`. + linked = linked_record_row(ctx, us) + if linked is not None: + return linked + args = argparse.Namespace(root=str(ctx.root), upstream=str(ctx.shape), at=None) try: @@ -3131,6 +3138,52 @@ def check_shape_currency(ctx: Context) -> Row: upstream.close() +def linked_record_row(ctx: Context, us) -> Row | None: + """The `shape currency` row for a root that keeps its pin or its + manifest as a symlink, or under one, or None when it keeps neither + (#286). + + A STATEMENT ABOUT THEIR REPOSITORY, NOT A FAILURE TO ASK. `update-shape.py + apply` rewrites `contracts/shape-pin.yaml` and the manifest where they + stand, so it refuses a root that keeps either as a link + (`update-rewrite-symlink`): the re-pin would land at the link's end, + outside the root if that is where it points, and git, which holds the + link, would commit none of it. That is what the root's own tree holds, + and no `git fetch` in the standard changes it. Filed with `prepare`'s + other refusals, it read `CANNOT ANSWER`, exit 3, with a fetch for its + NEXT. So this row carries no `environment`, which is the only thing that + makes `verdict_for` answer CANNOT ANSWER; the ladder goes on, and where + every gate reads through the link and is green, the verdict is `INVALID + (shape-currency)`. + + ASKED FIRST, AND BY THE SAME FUNCTION. `linked_record` is the question + `read_pin` and `prepare` ask before they refuse, for the KIND of root + `prepare` would see, so this row and `update-shape.py` cannot disagree + about which record is linked. Asked before anything reads the pin, so a + linked pin is answered as a link, whatever else reads it next. + + THE REFUSAL, QUOTED AS `update-shape.py` PRINTS IT, and its remedy for + the NEXT: replace the link with what it points at, commit that, run the + doctor again. Said, not spelled as a command: what the link points at + may be shared with other trees, and removing a link -- a junction above + all -- is a person's call. + """ + linked = us.linked_record(ctx.root, us.root_kind(ctx.root)) + if linked is None: + return None + record, (name, points) = linked + refusal = us.rewrite_link_refusal([linked]) + return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, + "update-shape.py cannot re-pin this root: " + f"REFUSED {refusal.code}: {refusal.detail}", + f"replace {name} with the file or directory it points at, " + "or with what it replaced where it points at nothing, commit " + "that, and run this doctor again -- update-shape.py never " + f"rewrites {record} through a link", + {"error": refusal.code, "path": record, "link": name, + "points": points}) + + def head_of(path: Path) -> str | None: try: return git_out(["rev-parse", "HEAD"], cwd=path).lower() @@ -6249,10 +6302,15 @@ def drifted_file_reasons(counts: dict) -> list: TWO GROUPS AND THEY SAY DIFFERENT THINGS. `locally-modified` and `both` are what MADE this repository drifted -- a copy of the standard carries - an edit -- so they are named first. `upstream-removed`, `unmapped` and - `copy-missing` are states in which the standard and the copy no longer - map onto each other at all, which is worth quoting beside the edits - because it is the same person's next question. + an edit -- so they are named first. `upstream-removed`, `unmapped`, + `copy-missing` and `copy-is-link` are states in which the standard and + the copy no longer map onto each other at all, which is worth quoting + beside the edits because it is the same person's next question. + `copy-is-link` (#286) is a pinned copy the tree keeps as a symlink, or + under one: git holds the link, not the bytes the pin's digest describes, + and `update-shape.py apply` refuses it. It is a conflict there, so the + row is `drifted`; unnamed here, that drift had no reason to print, and + the verdict fell through to the catch-all `INVALID (shape-currency)`. A state is named only when its count is non-zero: a verdict line reading `0 unmapped` would be a parenthetical contradicting the words above it. @@ -6260,7 +6318,8 @@ def drifted_file_reasons(counts: dict) -> list: Split out of `verdict_for` for #134. """ edited = ("locally-modified", "both") - unmapped = ("upstream-removed", "unmapped", "copy-missing") + unmapped = ("upstream-removed", "unmapped", "copy-missing", + "copy-is-link") return [f"{counts[state]} {state}" for state in edited + unmapped if counts.get(state)] diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index e51fbc52..d7dbc33f 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2277,6 +2277,122 @@ def test_a_deleted_pinned_agent_file_is_not_an_add(standard, project): assert "checkout --" in row["next"], row["next"] +@WINDOWS_SKIP +def test_a_pinned_copy_kept_as_a_link_is_drifted_by_name(standard, project, + tmp_path): + """`AGENTS-shape.md` moved outside the root, a committed symlink in its + place, with the bytes unchanged (#286). + + `update-shape.py` reads that row as `copy-is-link`, a conflict its + `apply` refuses: git holds the link, not the bytes the pin's digest + describes. The doctor names it the way it names `copy-missing` -- + `DRIFTED (1 copy-is-link)` -- rather than reaching a verdict whose + parenthesis says nothing about the file. Main read through the link and + called this root COMPLIANT. + """ + outside = tmp_path / "outside" + outside.mkdir() + shutil.move(str(project / "AGENTS-shape.md"), + str(outside / "AGENTS-shape.md")) + (project / "AGENTS-shape.md").symlink_to(outside / "AGENTS-shape.md") + commit_all(project, "keep AGENTS-shape.md as a link") + + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + payload = json.loads(result.stdout) + assert payload["verdict"] == "DRIFTED (1 copy-is-link)", payload["verdict"] + row = rows_of(result)["shape-currency"] + assert row["status"] == "FINDING", row + assert row["detail"]["counts"] == {"copy-is-link": 1, + "unchanged": 10}, row["detail"] + assert row["detail"]["conflicts"] == ["AGENTS-shape.md"], row["detail"] + assert "AGENTS-shape.md" in row["reason"], row["reason"] + + +def keep_as_committed_link(root: Path, linked: str, tmp_path: Path) -> Path: + """`linked` moved beside the root, a symlink to it left in its place and + committed, so the link is what git holds. Returns where it points.""" + end = tmp_path / "outside" / Path(linked).name + end.parent.mkdir(exist_ok=True) + shutil.move(str(root / linked), str(end)) + (root / linked).symlink_to(end, target_is_directory=end.is_dir()) + commit_all(root, f"keep {linked} as a link") + return end + + +def assert_a_linked_record_is_their_finding(result, linked: str, + record: str) -> None: + """The `shape-currency` row for a root that keeps `record`, or the + directory above it, as a committed link at `linked` (#286). + + A STATEMENT ABOUT THEIR REPOSITORY. `update-shape.py` refuses to + re-pin through the link, and the doctor had filed that refusal with + the ones about THIS checkout of the standard: `CANNOT ANSWER`, exit 3, + with a `git fetch --all` that changes nothing. It is a finding now: + `INVALID (shape-currency)`, exit 1, the refusal quoted, and the NEXT is + the refusal's own remedy. + """ + payload = json.loads(result.stdout) + assert (payload["verdict"], payload["exit"]) == ( + "INVALID (shape-currency)", 1), payload["verdict"] + assert result.returncode == 1, result.stdout + result.stderr + assert "cannot-answer" not in result.stderr, result.stderr + row = rows_of(result)["shape-currency"] + assert row["status"] == "FINDING", row + assert "environment" not in row["detail"], row["detail"] + assert row["detail"]["error"] == "update-rewrite-symlink", row["detail"] + assert (row["detail"]["path"], row["detail"]["link"]) == ( + record, linked), row["detail"] + assert "REFUSED update-rewrite-symlink: `apply` would rewrite " \ + f"{record}, but {linked} in this root is a symlink" in row["reason"], \ + row["reason"] + assert "fetch" not in row["next"], row["next"] + assert row["next"].startswith(f"replace {linked} with the file or " + "directory it points at"), row["next"] + assert "run this doctor again" in row["next"], row["next"] + # Every other row reads THROUGH the link and finds nothing wrong: the + # verdict is this row's alone. + red = [key for key, other in rows_of(result).items() + if other["status"] == "FINDING"] + assert red == ["shape-currency"], red + + +@WINDOWS_SKIP +@pytest.mark.parametrize("linked, record", [ + pytest.param("contracts/shape-pin.yaml", "contracts/shape-pin.yaml", + id="pin-at-a-link"), + pytest.param("contracts", "contracts/shape-pin.yaml", + id="contracts-directory-link"), + pytest.param("project.yaml", "project.yaml", id="manifest-at-a-link"), +]) +def test_a_pin_or_manifest_kept_as_a_link_is_a_finding_not_cannot_answer( + standard, project, tmp_path, linked, record): + """The pin, the `contracts` directory above it, or `project.yaml` kept + as a committed link, with the bytes unchanged and the pin naming this + standard's commit. Main read through the link and said COMPLIANT; #286 + made `update-shape.py` refuse the root, and the doctor then answered + CANNOT ANSWER. Neither is the truth: `apply` cannot re-pin this root + until the link is replaced, and that is the root's to fix.""" + keep_as_committed_link(project, linked, tmp_path) + result = doctor(standard, project, "--json") + assert_a_linked_record_is_their_finding(result, linked, record) + + +@WINDOWS_SKIP +def test_a_family_manifest_kept_as_a_link_is_a_finding_not_cannot_answer( + standard, holder, tmp_path): + """A family holder's manifest is `family.yaml`, and `apply` rewrites it + as it rewrites an assembly root's `project.yaml`: the doctor asks after + the manifest of the KIND of root it was pointed at.""" + root = tmp_path / FAMILY_NAME + shutil.copytree(holder["root"], root, symlinks=True) + keep_as_committed_link(root, "family.yaml", tmp_path) + result = doctor(standard, root, "--json") + assert json.loads(result.stdout)["kind"] == "family", result.stdout + assert_a_linked_record_is_their_finding(result, "family.yaml", + "family.yaml") + + #: How the `leg shape files` row spells a copy ON THIS HOST. `cp` is not a #: program on Windows, it is an alias of `Copy-Item` (Microsoft's own #: reference: "PowerShell includes the following aliases for `Copy-Item`: diff --git a/tests/test_update_shape.py b/tests/test_update_shape.py index 25a7bdb8..7dd69cb8 100644 --- a/tests/test_update_shape.py +++ b/tests/test_update_shape.py @@ -179,7 +179,8 @@ def apply(root, upstream_and_project, *extra, env=None): #: Every verdict `check` can print, in the column it prints them in. STATES = ("unchanged", "upstream-changed", "locally-modified", "both", - "already-at-target", "upstream-removed", "unmapped", "copy-missing") + "already-at-target", "upstream-removed", "unmapped", "copy-missing", + "copy-is-link") def verdicts(stdout: str) -> dict: diff --git a/tests/test_update_shape_add.py b/tests/test_update_shape_add.py index 29695f0a..8467e476 100644 --- a/tests/test_update_shape_add.py +++ b/tests/test_update_shape_add.py @@ -281,7 +281,7 @@ def pin_rows(root) -> dict: #: mistaken for one. STATES = ("unchanged", "upstream-changed", "upstream-added", "locally-modified", "both", "already-at-target", "upstream-removed", - "unmapped", "copy-missing") + "unmapped", "copy-missing", "copy-is-link") def verdicts(stdout: str) -> dict: diff --git a/tests/test_update_shape_add_symlink.py b/tests/test_update_shape_add_symlink.py index 14a549b3..405d7c58 100644 --- a/tests/test_update_shape_add_symlink.py +++ b/tests/test_update_shape_add_symlink.py @@ -383,10 +383,11 @@ def test_add_refuses_a_link_by_name_before_it_writes_anything( "the refusal says where the link points, quoted as a literal") assert "remove or move" in result.stderr, ( "and what the operator does about it") - # The promise is `--add`'s, not the whole command's: a pinned copy, or - # the pin itself, kept as a link is still rewritten through it. - assert "`--add` never writes through a link" in result.stderr - assert "this command never writes through a link" not in result.stderr + # The promise is the whole command's again (#286): a pinned copy, the + # pin and the manifest kept as a link are refused too. It says SYMLINK: + # a hard-linked copy is still rewritten through its other name. + assert "this command never writes through a symlink" in result.stderr + assert "never writes through a link" not in result.stderr assert "update-validators-red" not in result.stderr assert "ROLLED BACK" not in result.stderr, ( "nothing was written, so nothing is rolled back") diff --git a/tests/test_update_shape_rewrite_symlink.py b/tests/test_update_shape_rewrite_symlink.py new file mode 100644 index 00000000..332ce89e --- /dev/null +++ b/tests/test_update_shape_rewrite_symlink.py @@ -0,0 +1,646 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`apply` never REWRITES a file through a symlink the root keeps (#286). + +`apply` overwrites three kinds of file: the pinned copies the standard +changed, `contracts/shape-pin.yaml` and the manifest. `Rollback.write` opens +each one `r+b`, which follows a link, so a root that kept any of them as a +committed symlink — `AGENTS-shape.md` pointing at a shared file elsewhere on +disk, say — had the standard's bytes written at the LINK'S END, outside the +root if that is where it points. git holds the link, which had not changed, +so `--branch` committed none of those bytes while printing them among what it +committed, and the pin recorded a digest no commit carries. With the +project's validators red, the rollback wrote the old bytes back through the +same link. + +`tests/test_update_shape_add_symlink.py` holds `--add`'s half (#239). This +file holds the rewrites, in the places they matter: + + * (a) A PINNED COPY kept as a link is `copy-is-link` to `check`, counted + with the conflicts, and `apply` refuses it as `update-rewrite-symlink`, + exit 2, before it writes anything — with or without `--branch`, with the + validators green or red. + * (b) THE PIN OR THE MANIFEST kept as a link, or the pin under a directory + link: `prepare` refuses it, so `check` and `apply` both say so before + anything is written. A DANGLING link at the pin is named as the link it + is, not as a pin that "does not exist". + * (c) A DIRECTORY LINK ABOVE pinned copies (`scripts`, moved outside the + root): every copy under it is `copy-is-link`, and the refusal comes + before the validators, which used to resolve their own location through + the link, go red, and roll back through it. + * (d) THE CONTROLS: a DANGLING link at a pinned copy stays `copy-missing`, + refused as `update-conflict` as before; a root reached THROUGH a link, + with an unrelated link inside it, still updates. + * THE LEDGER'S OWN GUARD: a link that arrives while an interactive `apply` + waits in `confirm` is refused by `Rollback.write` itself, and what was + already written is put back. + * A WINDOWS JUNCTION is a link here too, on every platform, by handing the + walk what `lstat` gives for one. + +"Nothing was written" is the whole tree before and after — every file's +bytes and mode, every link by where it points, git's HEAD, branches, index +and status — and every directory beside the root that a link leads to; the +helpers are `tests/test_update_shape_add_symlink.py`'s. + +THE FIXTURE is `tests/test_update_shape.py`'s: a clone of this repository at +A and `Atlas` scaffolded from it into bare repositories on disk. Commit B +edits two pinned copies, `AGENTS-shape.md` and `scripts/validate-manifest.py`, +so there is a write for a link to be in the way of whichever one is linked. +No network is used and `gh` is never invoked. +""" + +from __future__ import annotations + +import os +import shutil +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from conftest import REPO, WINDOWS_SKIP, git, run_script +from test_scaffold_ignored_shape_files import load +from test_update_shape import (CHANGED, CHANGED_SOURCE, UPSTREAM_FIX_LINE, + scaffold_upstream_and_project) +from test_update_shape_add import commit_all, validators_are_green, verdicts +from test_update_shape_add_symlink import (JUNCTION_TAG, assert_untouched, + beyond, make_validators_red, + outside_dir, snapshot) + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import TREE_DIGEST_DEFINITION, load_yaml # noqa: E402 + +UPDATE = REPO / "update-shape.py" +PROJECT = "Atlas" +ENV = {"PYTHONDONTWRITEBYTECODE": "1"} + +#: The issue's pinned copy, and the edit commit B makes to its source. +AGENTS = "AGENTS-shape.md" +AGENTS_SOURCE = f"templates/assembly-root/{AGENTS}" +AGENTS_EDIT = b"\n" +PIN = "contracts/shape-pin.yaml" +MANIFEST = "project.yaml" +#: What an operator's own file, outside the root, holds. +THEIRS = b"someone else's file, which no tool here may touch\n" + + +@pytest.fixture(scope="module") +def standard(tmp_path_factory) -> dict: + """A clone at A, `Atlas` cut from it, and B editing two pinned copies.""" + _base, upstream, a, clone = scaffold_upstream_and_project( + tmp_path_factory, "update-shape-rewrite-symlink") + for source, edit in ((AGENTS_SOURCE, AGENTS_EDIT), + (CHANGED_SOURCE, UPSTREAM_FIX_LINE.encode("utf-8"))): + path = upstream / source + # Bytes, so the blob is what was appended on every platform. + path.write_bytes(path.read_bytes() + edit) + b = commit_all(upstream, "Edit AGENTS-shape.md and fix the manifest " + "validator", [AGENTS_SOURCE, CHANGED_SOURCE]) + assert a != b + return {"upstream": upstream, "clone": clone, "a": a, "b": b} + + +@pytest.fixture +def root(standard, tmp_path): + """A private, mutable copy of the project as it was cut at A.""" + target = tmp_path / PROJECT + shutil.copytree(standard["clone"], target, symlinks=True) + return target + + +@pytest.fixture(scope="module") +def update_shape(): + """`update-shape.py` as a module, for the ledger and the walk.""" + return load(UPDATE, "update_shape_rewrite_symlink") + + +def check(root, standard, *extra): + return run_script(UPDATE, "check", "--root", str(root), + "--upstream", str(standard["upstream"]), + "--at", standard["b"], *extra, env=ENV) + + +def apply(root, standard, *extra): + return run_script(UPDATE, "apply", "--root", str(root), "--yes", + "--upstream", str(standard["upstream"]), + "--at", standard["b"], *extra, env=ENV) + + +def pinned_commit(root) -> str: + """The pin's `commit:`, read through whatever the root keeps there.""" + return str(load_yaml(root / PIN)["commit"]).lower() + + +def commit_root(root: Path, message: str) -> None: + """Commit everything the test just did to the root, so the tree is + clean and the link is what git holds, as in the issue.""" + git("add", "-A", "--", ".", cwd=root) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", + message, cwd=root) + + +def keep_as_link(root: Path, rel: str, where: Path) -> Path: + """The issue's `mv` and `ln -s`, committed: `rel` moved to `where` + beside the root, with a symlink to it left at `rel`. Returns `where`.""" + where.parent.mkdir(parents=True, exist_ok=True) + shutil.move(str(root / rel), str(where)) + (root / rel).symlink_to(where, target_is_directory=where.is_dir()) + commit_root(root, f"keep {rel} as a link") + return where + + +def said(path: str, name: str, points) -> str: + """How `update-shape.py` names a link on `path`'s way: the phrase + `link_said` prints, spelled out so a change to it is seen here.""" + above = "" if name == path else f", on the way to {path}" + return f"{name} in this root is a symlink{above}, pointing at {points!r}" + + +def one_link_said(path: str, name: str, points) -> str: + """How `update-rewrite-symlink` opens when it names ONE path: on one + line, so the doctor can quote it inside one line of its table.""" + return f"`apply` would rewrite {path}, but " + said(path, name, points) + + +def assert_refused_before_any_write(result) -> None: + """Exit 2, by name, and nothing written to put back: the refusal came + before the first byte, not after a write through the link.""" + assert "ROLLED BACK" not in result.stderr, ( + "the refusal came after a write, and the run rolled back:\n" + + result.stderr) + assert "update-validators-red" not in result.stderr, result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert "REFUSED update-rewrite-symlink" in result.stderr, result.stderr + assert "never writes through a symlink" in result.stderr + assert "copied " not in result.stdout, result.stdout + assert "re-pinned" not in result.stdout, result.stdout + + +# --------------------------------------------------------------------------- +# (a) A pinned copy kept as a link +# --------------------------------------------------------------------------- + + +@WINDOWS_SKIP +def test_check_names_a_pinned_copy_kept_as_a_link(root, standard, tmp_path): + """Main read through the link and called the row `upstream-changed`, + which `apply` then wrote through. The row is the link's now, with where + it points, and the conflicts line says `apply` will refuse it; the other + copy B changed still reports as it did.""" + where = keep_as_link(root, AGENTS, outside_dir(tmp_path) / AGENTS) + before, elsewhere = snapshot(root), beyond(tmp_path) + + result = check(root, standard) + assert result.returncode == 1, result.stdout + result.stderr + found = verdicts(result.stdout) + assert found[AGENTS] == "copy-is-link", result.stdout + assert found[CHANGED] == "upstream-changed", result.stdout + assert said(AGENTS, AGENTS, str(where)) in result.stdout + ahead = [line for line in result.stdout.splitlines() + if line.startswith("REFUSAL AHEAD")] + assert len(ahead) == 1 and AGENTS in ahead[0], result.stdout + assert "copy-is-link 1" in result.stdout, "the summary counts it" + assert_untouched(before, snapshot(root)) + assert beyond(tmp_path) == elsewhere + + +@WINDOWS_SKIP +@pytest.mark.parametrize("validators", ["green", "red"]) +@pytest.mark.parametrize("branch", [None, "shape/update-link"], + ids=["no-branch", "branch"]) +def test_apply_refuses_a_pinned_copy_kept_as_a_link_before_any_write( + root, standard, tmp_path, branch, validators): + """THE ISSUE. Green, main wrote the standard's bytes at the link's end, + exited 0, and with `--branch` named `AGENTS-shape.md` among what it + committed while the commit held only the pin and the manifest. Red, it + rolled back by writing the old bytes through the same link. Now the + run refuses by name before the first write: the file at the link's end, + the tree, the pin, HEAD and every branch are as they were.""" + where = keep_as_link(root, AGENTS, outside_dir(tmp_path) / AGENTS) + if validators == "red": + make_validators_red(root) + held = where.read_bytes() + before, elsewhere = snapshot(root), beyond(tmp_path) + + result = apply(root, standard, *(("--branch", branch) if branch else ())) + # THE INVARIANT FIRST: a failure on main names the write through the + # link, not the wording. + assert where.read_bytes() == held, ( + "the standard's bytes were written THROUGH the link, outside the " + "root") + assert beyond(tmp_path) == elsewhere + assert_untouched(before, snapshot(root)) + assert (root / AGENTS).is_symlink() + assert pinned_commit(root) == standard["a"] + assert_refused_before_any_write(result) + assert one_link_said(AGENTS, AGENTS, str(where)) in result.stderr, ( + result.stderr) + assert "replace the link with what it points at" in result.stderr + + +@WINDOWS_SKIP +def test_accept_local_does_not_take_a_pinned_copy_kept_as_a_link( + root, standard, tmp_path): + """`--accept-local` re-pins a row from the root's bytes, and these are + the link's end's: there is no local edit to accept, only a place this + command does not write, so it is refused the same way.""" + keep_as_link(root, AGENTS, outside_dir(tmp_path) / AGENTS) + before, elsewhere = snapshot(root), beyond(tmp_path) + + result = apply(root, standard, "--accept-local", AGENTS) + assert beyond(tmp_path) == elsewhere + assert_untouched(before, snapshot(root)) + assert_refused_before_any_write(result) + + +# --------------------------------------------------------------------------- +# (b) The pin or the manifest kept as a link +# --------------------------------------------------------------------------- + + +def pin_at_a_link(root: Path, tmp_path: Path): + """The issue's second row: the pin moved outside, a link in its place.""" + keep_as_link(root, PIN, outside_dir(tmp_path) / "shape-pin.yaml") + return PIN, PIN + + +def pin_under_a_directory_link(root: Path, tmp_path: Path): + """`contracts` moved outside whole: the pin is a plain file, under a + link.""" + keep_as_link(root, "contracts", outside_dir(tmp_path) / "contracts") + return PIN, "contracts" + + +def manifest_at_a_link(root: Path, tmp_path: Path): + """`project.yaml`, the record that mirrors the pin, moved outside.""" + keep_as_link(root, MANIFEST, outside_dir(tmp_path) / MANIFEST) + return MANIFEST, MANIFEST + + +RECORD_LINKS = [ + pytest.param(pin_at_a_link, id="pin-at-a-link"), + pytest.param(pin_under_a_directory_link, id="pin-under-a-directory-link"), + pytest.param(manifest_at_a_link, id="manifest-at-a-link"), +] + + +@WINDOWS_SKIP +@pytest.mark.parametrize("make_link", RECORD_LINKS) +def test_check_refuses_a_pin_or_manifest_kept_as_a_link( + root, standard, tmp_path, make_link): + """Main read the pin through its link and reported the copies, and never + read the manifest at all, so `check` exited 1 with a `NEXT` line whose + `apply` would write through either. `prepare` refuses now, as `read_pin` + does for any pin it cannot use.""" + path, name = make_link(root, tmp_path) + points = os.readlink(root / name) + before, elsewhere = snapshot(root), beyond(tmp_path) + + result = check(root, standard) + assert result.returncode == 2, result.stdout + result.stderr + assert "Traceback" not in result.stderr, result.stderr + assert "REFUSED update-rewrite-symlink" in result.stderr, result.stderr + # ONE LINE, link and reason together: the doctor quotes this refusal + # inside one line of its table. + refused = [line for line in result.stderr.splitlines() + if line.startswith("REFUSED")] + assert len(refused) == 1, result.stderr + assert one_link_said(path, name, points) in refused[0], result.stderr + assert refused[0].endswith("never writes through a symlink"), ( + result.stderr) + assert "NEXT" not in result.stdout, result.stdout + assert_untouched(before, snapshot(root)) + assert beyond(tmp_path) == elsewhere + + +@WINDOWS_SKIP +@pytest.mark.parametrize("branch", [None, "shape/update-link"], + ids=["no-branch", "branch"]) +@pytest.mark.parametrize("make_link", RECORD_LINKS) +def test_apply_refuses_a_pin_or_manifest_kept_as_a_link_before_any_write( + root, standard, tmp_path, make_link, branch): + """Main rewrote the pin, or the manifest, at the link's end and exited + 0; with `--branch` the commit moved the copies and the other record but + not the linked one, while the run said it had committed it. Refused now + before the first write, so not even the copies B changed are written.""" + path, name = make_link(root, tmp_path) + before, elsewhere = snapshot(root), beyond(tmp_path) + copy = (root / CHANGED).read_bytes() + + result = apply(root, standard, *(("--branch", branch) if branch else ())) + assert beyond(tmp_path) == elsewhere, ( + "a record was rewritten THROUGH the link, outside the root") + assert_untouched(before, snapshot(root)) + assert (root / name).is_symlink() + assert (root / CHANGED).read_bytes() == copy + assert pinned_commit(root) == standard["a"] + assert_refused_before_any_write(result) + assert one_link_said(path, name, os.readlink(root / name)) \ + in result.stderr, result.stderr + + +@WINDOWS_SKIP +def test_a_dangling_link_at_the_pin_is_named_as_the_link(root, standard, + tmp_path): + """`is_file()` follows a link, so main called a dangling one a pin that + "does not exist" and sent the operator to `adopt-project.py`. The link + is asked first, by `lstat`, and named — with where it points — as any + link at the pin is.""" + (root / PIN).unlink() + gone = tmp_path / "no-such-dir" / "shape-pin.yaml" + (root / PIN).symlink_to(gone) + commit_root(root, "a pin that leads nowhere") + before, elsewhere = snapshot(root), beyond(tmp_path) + + for result in (check(root, standard), apply(root, standard)): + assert result.returncode == 2, result.stdout + result.stderr + assert "REFUSED update-rewrite-symlink" in result.stderr, ( + result.stderr) + assert "update-pin-missing" not in result.stderr + assert said(PIN, PIN, str(gone)) in result.stderr, result.stderr + assert not gone.parent.exists() + assert_untouched(before, snapshot(root)) + assert beyond(tmp_path) == elsewhere + + +# --------------------------------------------------------------------------- +# (c) A directory link above pinned copies +# --------------------------------------------------------------------------- + + +@WINDOWS_SKIP +def test_a_directory_link_above_pinned_copies_is_refused_before_the_validators( + root, standard, tmp_path): + """`scripts` moved outside the root whole, a link in its place. Main + wrote B's `scripts/validate-manifest.py` through it, then ran the + project's validators, which resolve their own location THROUGH the link, + found no repository there, went red, and rolled every byte back through + the same link. Every copy under the link is `copy-is-link` now, and the + refusal comes before anything is written or run.""" + where = keep_as_link(root, "scripts", outside_dir(tmp_path) / "scripts") + pinned = [str(row["path"]) for row in load_yaml(root / PIN)["files"]] + under = [path for path in pinned if path.startswith("scripts/")] + assert CHANGED in under + before, elsewhere = snapshot(root), beyond(tmp_path) + + checked = check(root, standard) + found = verdicts(checked.stdout) + assert {path: found.get(path) for path in under} == { + path: "copy-is-link" for path in under}, checked.stdout + assert found[AGENTS] == "upstream-changed", checked.stdout + assert said(CHANGED, "scripts", str(where)) in checked.stdout + + result = apply(root, standard, "--branch", "shape/update-link") + assert beyond(tmp_path) == elsewhere, ( + "a copy was written THROUGH the directory link, outside the root") + assert_untouched(before, snapshot(root)) + assert (root / AGENTS).read_bytes() == \ + (standard["clone"] / AGENTS).read_bytes(), ( + "the copy outside the link was written too") + assert_refused_before_any_write(result) + assert f" {CHANGED}: " + said(CHANGED, "scripts", str(where)) \ + in result.stderr, result.stderr + assert "the project's own validators" not in result.stdout, ( + "the validators ran: the refusal came after a write") + + +# --------------------------------------------------------------------------- +# (d) The controls +# --------------------------------------------------------------------------- + + +@WINDOWS_SKIP +def test_a_dangling_link_at_a_pinned_copy_is_still_copy_missing( + root, standard, tmp_path): + """There is no file to read at a dangling link, so the row is what main + called it — `copy-missing` — and `apply` refuses it as the conflict it + always was, before any write, without creating the link's end.""" + (root / AGENTS).unlink() + gone = tmp_path / "gone" / AGENTS + (root / AGENTS).symlink_to(gone) + commit_root(root, "AGENTS-shape.md leads nowhere") + before, elsewhere = snapshot(root), beyond(tmp_path) + + checked = check(root, standard) + assert verdicts(checked.stdout)[AGENTS] == "copy-missing", checked.stdout + assert "copy-is-link" not in checked.stdout + + result = apply(root, standard) + assert result.returncode == 2, result.stdout + result.stderr + assert "REFUSED update-conflict" in result.stderr, result.stderr + assert "update-rewrite-symlink" not in result.stderr + assert not gone.parent.exists() + assert_untouched(before, snapshot(root)) + assert beyond(tmp_path) == elsewhere + + +@WINDOWS_SKIP +def test_a_root_reached_through_a_link_still_updates(root, standard, + tmp_path): + """`--root` is resolved before any path is walked, so a root named + through a symlink (macOS's `/var`, a project kept under a link into a + bigger disk) is not "a link on the way", and neither is a link the root + holds somewhere `apply` does not write: B's two copies are written, the + pin moves, the commit holds all of it, and the validators stay green.""" + (root / "notes-link").symlink_to(outside_dir(tmp_path), + target_is_directory=True) + via = tmp_path / "via" + via.symlink_to(root, target_is_directory=True) + + result = apply(via, standard, "--branch", "shape/update-via") + assert result.returncode == 0, result.stdout + result.stderr + assert (root / AGENTS).read_bytes().endswith(AGENTS_EDIT) + assert (root / CHANGED).read_bytes().endswith( + UPSTREAM_FIX_LINE.encode("utf-8")) + assert pinned_commit(root) == standard["b"] + committed = set(git("show", "--name-only", "--format=", "HEAD", + cwd=root).stdout.split()) + assert {AGENTS, CHANGED, PIN, MANIFEST} <= committed, committed + assert os.listdir(tmp_path / "outside") == [] + validators_are_green(root) + + +# --------------------------------------------------------------------------- +# The ledger's own guard: a link that arrives after `prepare` looked +# --------------------------------------------------------------------------- + + +@WINDOWS_SKIP +@pytest.mark.parametrize("arrives_at, rewrites, restored", [ + pytest.param(AGENTS, AGENTS, (CHANGED,), id="at-a-pinned-copy"), + pytest.param(PIN, PIN, (CHANGED, AGENTS), id="at-the-pin"), + pytest.param(MANIFEST, MANIFEST, (CHANGED, AGENTS, PIN), + id="at-the-manifest"), + pytest.param("contracts", PIN, (CHANGED, AGENTS), + id="a-directory-link-above-the-pin"), +]) +def test_a_link_that_arrives_while_apply_waits_is_refused_and_rolled_back( + update_shape, root, standard, tmp_path, monkeypatch, arrives_at, + rewrites, restored): + """`confirm` is where an interactive `apply` waits for a human, after + `prepare` has looked; the operator's link arrives there, at `arrives_at`, + in the way of the write to `rewrites`. Whatever was written before it — + the copy, or both copies, or both copies and the pin when the manifest + is the one linked — is put back by `cmd_apply`'s refusal arm, and the + tree is exactly as it was when the run began to write. Main wrote + through the link and finished the run. `contracts` arrives as a + DIRECTORY link: the pin under it is the write refused.""" + where = outside_dir(tmp_path) / Path(arrives_at).name + end = where / Path(rewrites).relative_to(arrives_at) + seen: dict = {} + + def a_link_arrives(*_args, **_kwargs) -> None: + shutil.move(str(root / arrives_at), str(where)) + (root / arrives_at).symlink_to(where, + target_is_directory=where.is_dir()) + seen["held"] = end.read_bytes() + seen["root"], seen["beyond"] = snapshot(root), beyond(tmp_path) + + monkeypatch.setattr(update_shape, "confirm", a_link_arrives) + monkeypatch.setenv("PYTHONDONTWRITEBYTECODE", "1") + args = update_shape.build_parser().parse_args([ + "apply", "--root", str(root), "--yes", + "--upstream", str(standard["upstream"]), "--at", standard["b"]]) + + with pytest.raises(update_shape.Refusal) as refused: + update_shape.cmd_apply(args) + assert end.read_bytes() == seen["held"], ( + f"{end} was written THROUGH the link") + assert beyond(tmp_path) == seen["beyond"] + assert_untouched(seen["root"], snapshot(root)) + assert refused.value.code == "update-rewrite-symlink" + assert said(rewrites, arrives_at, str(where)) in refused.value.detail + assert "it arrived while `apply` ran" in refused.value.detail + assert "ROLLED BACK: every byte" in refused.value.detail, ( + refused.value.detail) + for rel in restored: + assert f"restored {rel}" in refused.value.detail + + +@WINDOWS_SKIP +@pytest.mark.parametrize("above", [False, True], + ids=["at-the-path", "above-the-path"]) +def test_the_ledger_never_rewrites_a_file_through_a_link(update_shape, + tmp_path, above): + """The guard alone, with nothing in front of it. Main opened the path + `r+b`, followed the link and overwrote what it leads to. Refused by + name now, with nothing read into the ledger, so there is nothing for an + undo to write back through the link either.""" + root = tmp_path / PROJECT + root.mkdir() + outside = outside_dir(tmp_path) + (outside / "agents.md").write_bytes(THEIRS) + if above: + (root / "docs").symlink_to(outside, target_is_directory=True) + rel, name = "docs/agents.md", "docs" + else: + (root / "agents.md").symlink_to(outside / "agents.md") + rel, name = "agents.md", "agents.md" + ledger = update_shape.Rollback(root) + + with pytest.raises(update_shape.Refusal) as refused: + ledger.write(root / rel, b"the standard's bytes\n") + assert (outside / "agents.md").read_bytes() == THEIRS + assert ledger.written == {} + assert refused.value.code == "update-rewrite-symlink" + assert said(rel, name, os.readlink(root / name)) in refused.value.detail + + +def test_the_ledger_still_rewrites_a_plain_file(update_shape, tmp_path): + """The control: no link anywhere on the way, so the file is rewritten + and its first bytes are kept for the undo.""" + root = tmp_path / PROJECT + (root / "docs").mkdir(parents=True) + (root / "docs" / "agents.md").write_bytes(THEIRS) + ledger = update_shape.Rollback(root) + + ledger.write(root / "docs" / "agents.md", b"new\n") + assert (root / "docs" / "agents.md").read_bytes() == b"new\n" + assert ledger.written == {root / "docs" / "agents.md": THEIRS} + + +# --------------------------------------------------------------------------- +# A Windows junction is a link for the rewrites too +# --------------------------------------------------------------------------- +# +# `is_link_info` and the walk are #239's, and its tests hand the walk what +# Windows' `lstat` gives for a junction. These hand it the same at the three +# places a rewrite is decided — a pinned row, the pin, the manifest — on +# every platform, since no POSIX runner can make a real junction. + + +def junction_at(monkeypatch, junctions: dict) -> None: + """`os.lstat` reports each path in `junctions` as a directory carrying + the mount point's reparse tag, and `os.readlink` answers its target.""" + real_lstat, real_readlink = os.lstat, os.readlink + + def lstat(path, *args, **kwargs): + info = real_lstat(path, *args, **kwargs) + if Path(path) in junctions: + return SimpleNamespace(st_mode=info.st_mode, + st_reparse_tag=JUNCTION_TAG) + return info + + def readlink(path, *args, **kwargs): + if Path(path) in junctions: + return junctions[Path(path)] + return real_readlink(path, *args, **kwargs) + + monkeypatch.setattr(os, "lstat", lstat) + monkeypatch.setattr(os, "readlink", readlink) + + +def a_bare_root(tmp_path: Path) -> Path: + """A root holding a pin `read_pin` accepts, a manifest and one pinned + copy under `scripts/` — nothing more is read before the walk decides.""" + root = tmp_path / PROJECT + (root / "contracts").mkdir(parents=True) + (root / "scripts").mkdir() + (root / "scripts" / "x.py").write_bytes(b"pass\n") + (root / MANIFEST).write_bytes(b"name: atlas\n") + (root / PIN).write_text( + f'commit: "{"a" * 40}"\n' + f'digest_definition: "{TREE_DIGEST_DEFINITION}"\n' + "files:\n - path: scripts/x.py\n sha256: \"0\"\n", + encoding="utf-8") + return root + + +def test_a_junction_above_a_pinned_copy_makes_the_row_copy_is_link( + update_shape, tmp_path, monkeypatch): + """`scripts` reads as a junction: the row is `copy-is-link`. The stub + upstream holds the root's own bytes at every commit, so a walk that + missed the junction would go on to compare them and give the row an + ordinary verdict.""" + root = a_bare_root(tmp_path) + junction_at(monkeypatch, {root / "scripts": r"\\?\C:\elsewhere"}) + row = update_shape.Row("scripts/x.py", "0" * 64) + upstream = SimpleNamespace(blob=lambda _rev, _path: b"pass\n") + + update_shape._classify_row(root, row, upstream, "a" * 40, "b" * 40, + update_shape.PROJECT_KIND) + assert row.state == "copy-is-link" + assert row.is_conflict + assert row.link == ("scripts", r"\\?\C:\elsewhere") + + +@pytest.mark.parametrize("junction, refused_at", [ + pytest.param("contracts", PIN, id="above-the-pin"), + pytest.param(MANIFEST, MANIFEST, id="at-the-manifest"), +]) +def test_a_junction_at_or_above_a_record_is_refused_by_prepare( + update_shape, tmp_path, monkeypatch, junction, refused_at): + """The pin under a junction is refused by `read_pin`, the manifest at + one by `prepare`, before `--upstream` is even opened (it names nothing + that exists).""" + root = a_bare_root(tmp_path) + junction_at(monkeypatch, {root / junction: r"\\?\C:\elsewhere"}) + args = SimpleNamespace(root=str(root), upstream=str(tmp_path / "none"), + at=None) + + with pytest.raises(update_shape.Refusal) as refused: + update_shape.prepare(args) + assert refused.value.code == "update-rewrite-symlink" + assert said(refused_at, junction, r"\\?\C:\elsewhere") in \ + refused.value.detail diff --git a/update-shape.py b/update-shape.py index 5895ee36..a4406ede 100755 --- a/update-shape.py +++ b/update-shape.py @@ -71,6 +71,15 @@ branch, and the refusal names the step that failed and lists what it put back. A re-run then finds the tree it found the first time, rather than an uncommitted re-pin it would read as already done. + * Nothing is written THROUGH A SYMLINK (#239, #286). A rewrite opens the + file where it stands, so a pinned copy, `contracts/shape-pin.yaml` or + the manifest kept as a link — or under a directory link — would have the + standard's bytes land at the link's end, outside the root if that is + where it points, while git, which holds the link, commits none of them. + `check` calls such a copy `copy-is-link` and `apply` refuses it; both + refuse a linked pin or manifest. Each refusal is `update-rewrite-symlink`, + before anything is written, as `--add` refuses a link at a path it would + create as `update-add-symlink`. ONLY THE PIN'S OWN ROWS ARE CONSIDERED. The set of shape files is read from `contracts/shape-pin.yaml`, never re-derived from this repository's copy @@ -248,8 +257,9 @@ def root_kind(root: Path) -> Kind: #: States a pinned row can be in. The first four are the vocabulary the task -#: of updating a project is actually about; the last three are the ways the -#: question cannot be asked, and each of them refuses rather than guessing. +#: of updating a project is actually about; the last four are the ways the +#: question cannot be asked, or its answer written, and each of them refuses +#: rather than guessing. UNCHANGED = "unchanged" UPSTREAM_CHANGED = "upstream-changed" #: Not a state a pinned ROW can be in: a path the upstream's copy lists name at @@ -263,12 +273,19 @@ def root_kind(root: Path) -> Kind: UPSTREAM_REMOVED = "upstream-removed" UNMAPPED = "unmapped" COPY_MISSING = "copy-missing" +#: A pinned copy the root keeps as a SYMLINK, or under one (#286). The bytes +#: can be compared — they are read through the link — but the answer cannot be +#: written: a rewrite lands at the link's end, which git does not commit. So +#: it is a fourth way the question cannot be asked, whatever the upstream did, +#: and `apply` refuses it by name. A DANGLING link stays `copy-missing`: there +#: is no file to read, and that refusal already writes nothing. +COPY_IS_LINK = "copy-is-link" #: Anything here is a merge or a decision, never a byte copy. -CONFLICTS = (BOTH, UPSTREAM_REMOVED, UNMAPPED, COPY_MISSING) +CONFLICTS = (BOTH, UPSTREAM_REMOVED, UNMAPPED, COPY_MISSING, COPY_IS_LINK) ORDER = (UPSTREAM_CHANGED, UPSTREAM_ADDED, BOTH, LOCALLY_MODIFIED, ALREADY_AT_TARGET, UPSTREAM_REMOVED, UNMAPPED, COPY_MISSING, - UNCHANGED) + COPY_IS_LINK, UNCHANGED) # --------------------------------------------------------------------------- @@ -406,6 +423,9 @@ def __init__(self, path: str, recorded: str): self.detail = "" self.target_bytes: bytes | None = None self.digest = self.recorded + #: `symlink_on_the_way`'s answer for this path — the link and where + #: it points — when there is one, which makes the row `copy-is-link`. + self.link: tuple[str, str] | None = None @property def is_conflict(self) -> bool: @@ -437,7 +457,20 @@ def pin_rows_remediation(root: Path) -> str: def read_pin(root: Path) -> tuple[dict, list[Row]]: + """The root's `contracts/shape-pin.yaml` and its rows, or a refusal that + says why this command cannot use it. + + A PIN KEPT AS A SYMLINK, OR UNDER ONE, IS ONE IT CANNOT USE (#286), and + that is asked FIRST, by `lstat`: `apply` rewrites the pin where it + stands, so the re-pin would land at the link's end while the commit + `--branch` makes holds the unchanged link. Asked before `is_file()`, + which follows a link, so a dangling one is named as the link it is + rather than as a pin that "does not exist". + """ pin_path = root / "contracts" / "shape-pin.yaml" + linked = linked_record(root) + if linked is not None: + raise rewrite_link_refusal([linked]) if not pin_path.is_file(): raise Refusal( "update-pin-missing", @@ -558,7 +591,11 @@ def _classify_row(root: Path, row: Row, upstream: Upstream, pinned: str, THE THREE EXITS HERE ARE THE ONES WHERE THERE IS NOTHING TO COMPARE: the root has no file at this path, no upstream path corresponds to it, or the - upstream has none at the target commit. Each is a decision for a human + upstream has none at the target commit. A FOURTH COMES SECOND (#286): + the file is there, but the path is a symlink or sits under one, so + whatever the comparison said could not be written where git would commit + it. It is asked after `is_file()`, which follows the link, so a DANGLING + link is still `copy-missing`. Each is a decision for a human rather than a difference this tool can act on, so `_row_verdict` below is only ever reached holding the root's bytes and the target's bytes for certain. THE PINNED BLOB IS THE EXCEPTION and is passed on as `None` when @@ -575,6 +612,12 @@ def _classify_row(root: Path, row: Row, upstream: Upstream, pinned: str, row.detail = ("pinned but absent from the root; a human decides " "whether it was deleted on purpose") return + link = symlink_on_the_way(root, row.path) + if link is not None: + row.state = COPY_IS_LINK + row.link = link + row.detail = linked_copy_detail(row.path, link) + return source = source_for(row.path, upstream, target, kind) row.source = source if source is None: @@ -1016,27 +1059,42 @@ def find_additions(root: Path, rows: list[Row], upstream: Upstream, return additions -def link_refusal(path: str, link: tuple[str, str]) -> Refusal: - """`update-add-symlink`: `--add ` with a link at the path or above - it, `link` being `symlink_on_the_way`'s answer (#239). Raised where - `resolve_additions` finds the link, and again by `Rollback.write_added` - for one that arrived after that. +def link_said(path: str, link: tuple[str, str]) -> str: + """` in this root is a symlink[, on the way to ], pointing at + ''`: the one phrase every refusal and verdict about a link on + `path`'s way prints, `link` being `symlink_on_the_way`'s answer. WHERE THE LINK POINTS IS QUOTED, `!r`, because it is the operator's text and a file name may hold a newline: printed as it is, a target spelled `outREFUSED fake` puts a line on stderr that reads as a second - refusal. And the promise is `--add`'s alone: a pinned copy, or the pin, - kept as a link is still rewritten through it. + refusal (#239). """ name, points = link above = "" if name == path else f", on the way to {path}" + return f"{name} in this root is a symlink{above}, pointing at {points!r}" + + +def link_refusal(path: str, link: tuple[str, str]) -> Refusal: + """`update-add-symlink`: `--add ` with a link at the path or above + it, `link` being `symlink_on_the_way`'s answer (#239). Raised where + `resolve_additions` finds the link, and again by `Rollback.write_added` + for one that arrived after that. + + THE PROMISE IS THE WHOLE COMMAND'S (#286). It said `--add`'s alone while + a pinned copy, or the pin, kept as a link was still rewritten through + it; `rewrite_link_refusal` now refuses those, so the sentence is true of + every write `apply` makes. It says SYMLINK, not "link": a HARD link is + another name for the same bytes, which `Rollback.write` still writes + through everywhere but the README. + """ + name = link[0] return Refusal( "update-add-symlink", - f"--add {path} names a file the upstream's lists add, but {name} in " - f"this root is a symlink{above}, pointing at {points!r}. A write at " + f"--add {path} names a file the upstream's lists add, but " + f"{link_said(path, link)}. A write at " f"{path} would go THROUGH it, to wherever it leads — outside this " - "root, if that is where it points — and `--add` never writes through " - "a link", + "root, if that is where it points — and this command never writes " + "through a symlink", f"Remediation: if the link is yours to keep, leave {path} alone: " "`check` stops reporting a path the root holds, and the standard's " "file is yours to carry in whatever the link leads to. To take the " @@ -1045,6 +1103,98 @@ def link_refusal(path: str, link: tuple[str, str]) -> Refusal: "there, as for any addition.") +def linked_copy_detail(path: str, link: tuple[str, str]) -> str: + """A `copy-is-link` row's detail: the link, and what is done about it. + `check` prints it under the row, and `apply` names the same link in + `rewrite_link_refusal` (#286).""" + return (f"{link_said(path, link)}; `apply` never writes through one, so " + "replace the link with what it points at, or take the update by " + "hand at the link's end") + + +def linked_record(root: Path, kind: Kind | None = None + ) -> tuple[str, tuple[str, str]] | None: + """The first RECORD `apply` rewrites that the root keeps as a symlink, + or under one, as `(record, link)`, `link` being `symlink_on_the_way`'s + answer for it; None when neither is (#286). The records are + `contracts/shape-pin.yaml` and then `kind.manifest`; with no `kind`, the + pin alone, which is all `read_pin` reads. + + ONE QUESTION, AND EVERYONE WHO NEEDS IT ASKS THIS ONE. `read_pin` + refuses a linked pin and `prepare` a linked manifest, each with + `rewrite_link_refusal` over this answer. `shape-doctor.py` asks it + FIRST, before it reads the pin or opens this checkout as the upstream: + a linked record is a fact about the ROOT, which keeps a link where + `apply` has to write, and not this checkout of the standard failing to + ask about a commit. Asking the same function is how the doctor tells the + two apart by which read refused, rather than by a list of refusal codes. + """ + records = ["contracts/shape-pin.yaml"] + if kind is not None: + records.append(kind.manifest) + for record in records: + link = symlink_on_the_way(root, record) + if link is not None: + return record, link + return None + + +def rewrite_link_refusal(linked: list[tuple[str, tuple[str, str]]], + arrived: bool = False) -> Refusal: + """`update-rewrite-symlink`: a file `apply` REWRITES is a symlink in the + root, or sits under one (#286). `linked` is `[(path, link)]`, `link` + being `symlink_on_the_way`'s answer for `path`. + + `--add`'s `update-add-symlink` is the same refusal for a file `apply` + would CREATE. This one is for the three it OVERWRITES — a pinned copy, + `contracts/shape-pin.yaml`, the kind's manifest — which `Rollback.write` + opens where they stand, so each write lands at the link's end, outside + the root if that is where it points. git holds the LINK there, not those + bytes, so the commit `--branch` makes would leave them out while the run + printed them among what it committed, and the pin would record a digest + of bytes no commit carries. Raised before any write: by `read_pin` for + the pin, by `prepare` for the manifest, by `cmd_apply` for every + `copy-is-link` row at once — and, with `arrived`, by `Rollback.write` + for a link that came after `prepare` looked. + + ONE PATH IS ONE LINE, as `update-add-symlink` is. `read_pin` and + `prepare` always name one, and the doctor quotes this refusal, for the + record `linked_record` names, inside one line of its table, which a + detail spread over three lines broke into three. Several `copy-is-link` + rows are a list, one path a line, as `update-conflict` lists its own. + """ + many = len(linked) > 1 + sentences = (["It was not there when this run looked: it arrived while " + "`apply` ran, after this run had decided what to write."] + if arrived else []) + sentences.append( + "A rewrite goes THROUGH a link, to wherever it leads — outside this " + "root, if that is where it points — and git, which holds the link " + "and not those bytes, commits none of them: the commit `--branch` " + "makes would not hold what this run wrote. This command never " + "writes through a symlink") + if many: + detail = ("`apply` would rewrite these files, but each is a symlink " + "in this root, or sits under one:\n" + + "\n".join(f" {path}: {link_said(path, link)}" + for path, link in linked) + + "".join(f"\n {sentence}" for sentence in sentences)) + else: + path, link = linked[0] + detail = " ".join([f"`apply` would rewrite {path}, but " + f"{link_said(path, link)}.", *sentences]) + return Refusal( + "update-rewrite-symlink", + detail, + "Remediation: replace " + + ("each link" if many else "the link") + + " with what it points at — the same bytes at the same path, so git " + "holds them there rather than a link — commit that, and run this " + "command again. A root that keeps a link takes the update by hand, " + "at the link's end, and re-pins by hand: the upstream's bytes are " + "`git show :` in a shape clone.") + + def resolve_additions(rows: list[Row], additions: list[Addition], asked: list[str], target: str) -> list[Addition]: """The `--add` paths as additions, or a refusal that names the path. @@ -1687,6 +1837,12 @@ def prepare(args) -> tuple[Path, dict, list[Row], list[Addition], Upstream, raise Refusal("update-root-missing", f"{root} is not a directory") kind = root_kind(root) pin, rows = read_pin(root) + # THE MANIFEST IS THE OTHER RECORD `apply` REWRITES, so a link at it, or + # above it, is refused here as `read_pin` refuses one at the pin (#286): + # before the upstream is opened, and before any byte is written. + linked = linked_record(root, kind) + if linked is not None: + raise rewrite_link_refusal([linked]) pinned = str(pin["commit"]).lower() declared = str(pin.get("source_repository") or SHAPE_REPOSITORY) upstream = open_upstream(str(args.upstream or declared)) @@ -2185,7 +2341,21 @@ def write(self, path: Path, data: bytes) -> None: before the ledger has promised anything, so a refusal raised over a write that could not happen rolls back a tree this ledger can actually restore. + + NEVER THROUGH A SYMLINK (#286). `r+b` follows a link, at the path or + on its way down. `prepare` refuses a linked pin or manifest and + `apply` a `copy-is-link` row, but this runs later — after `confirm` + has waited for a human — and a link can arrive in between. So the + path is walked again here and a link refused by name before anything + is read or recorded, and `cmd_apply`'s refusal arm puts back what + was written before it. What the walk cannot see is a link that + arrives in the instant between it and the open, as + `write_added` cannot see one that arrives above its path. """ + rel = self._name(path) + link = symlink_on_the_way(self.root, rel) + if link is not None: + raise rewrite_link_refusal([(rel, link)], arrived=True) with path.open("r+b") as handle: self.written.setdefault(path, handle.read()) handle.seek(0) @@ -2603,6 +2773,21 @@ def _refuse_unknown_accept_local(rows: list[Row], accepted: set) -> None: "refusal, so a mistyped path is reported as a mistyped path.") +def _refuse_linked_copies(rows: list[Row]) -> None: + """`update-rewrite-symlink` for every `copy-is-link` row at once (#286). + + ASKED BEFORE `update-conflict`, though the rows count among the + conflicts `check` names: that refusal's remedy is a hand merge and + `--accept-local`, and neither is the answer for a link. `--accept-local` + on such a path accepts nothing — there is no local edit to trust, only + a place this command does not write. + """ + linked = [(row.path, row.link) for row in rows + if row.state == COPY_IS_LINK and row.link is not None] + if linked: + raise rewrite_link_refusal(linked) + + def _refuse_conflicts(rows: list[Row], accepted: set) -> None: """The `update-conflict` refusal, split out of `cmd_apply` for #133. @@ -2612,7 +2797,9 @@ def _refuse_conflicts(rows: list[Row], accepted: set) -> None: and re-pinned from what is on disk. The other three conflict states (upstream-removed, unmapped, copy-missing) have no such exit: there is no local edit to trust a word about, so `--accept-local` naming one of those - is simply not among the paths this can ever exclude. + is simply not among the paths this can ever exclude. The fifth, + `copy-is-link`, never reaches here: `_refuse_linked_copies` refuses it + first, under its own name. """ conflicts = [row for row in rows if row.is_conflict @@ -2872,6 +3059,7 @@ def cmd_apply(args) -> int: f"{upstream.repository} @ {target[:12]}, which the pin " "already names") return 0 + _refuse_linked_copies(rows) _refuse_conflicts(rows, accepted) _refuse_local_drift(rows, accepted) if args.branch: