fix: run refused writers in a copy of the tree

scripts/generate_site.py

@@ -56,6 +56,7 @@ from common import (
     load_platform_config,
     load_provenance_snapshots,
     native_layer,
+    require_directory,
     require_yaml,
     resolution_is_hash_exact,
     resolve_local_file,
@@ -12390,6 +12391,10 @@ def main() -> None:
     args = parser.parse_args()
     if args.docs_dir is None:
         args.docs_dir = str(Path(args.site_dir) / STARLIGHT_CONTENT)
+    # Before any write: the public assets and the bootstraps went out first,
+    # and a mistyped folder stopped the run only after them.
+    require_directory(args.emulators_dir, "--emulators-dir")
+    require_directory(args.platforms_dir, "--platforms-dir")
     try:
         # Two runs over one docs/ delete each other's pages and scratch
         # files in the sweep, so the second is refused instead.

tests/scratchtree.py

@@ -0,0 +1,78 @@
+"""A disposable copy of the checkout, for tests that run a writer and expect
+it to refuse.
+
+Run in the checkout, a regression made the very write the test guards
+against: generate_site rewrote website/src/data/published-parts.json and the
+FAQ page whatever --docs-dir said, and a pipeline whose lock probe failed
+would regenerate database.json and install/. A file another session had
+already changed listed the same name in `git diff --name-only` before and
+after, so its edits were lost without a signal.
+
+The copy holds every file git tracks or would track, as the working tree has
+it. bios/ and data/ (17 GB and 600 MB of inputs no such test writes) are
+links to the checkout's.
+"""
+
+from __future__ import annotations
+
+import atexit
+import hashlib
+import os
+import shutil
+import subprocess
+import tempfile
+from functools import cache
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+LINKED = ("bios", "data")
+
+
+@cache
+def scratch_checkout() -> Path:
+    """The copy, built once per test process and removed at its exit."""
+    root = Path(tempfile.mkdtemp(prefix="checkout-", dir=REPO_ROOT / "tmp" / "tests"))
+    # A detached worktree with nothing checked out: the scripts that read
+    # HEAD (the site's bootstrap pin) find this commit.
+    subprocess.run(
+        ["git", "worktree", "add", "--quiet", "--detach", "--no-checkout", str(root), "HEAD"],
+        cwd=REPO_ROOT, capture_output=True, check=True,
+    )
+    atexit.register(_remove, root)
+    listed = subprocess.run(
+        ["git", "ls-files", "-z", "--cached", "--others", "--exclude-standard"],
+        cwd=REPO_ROOT, capture_output=True, check=True,
+    ).stdout.decode("utf-8")
+    for name in filter(None, listed.split("\0")):
+        source = REPO_ROOT / name
+        if name.split("/", 1)[0] in LINKED or not source.is_file() or source.is_symlink():
+            continue
+        target = root / name
+        target.parent.mkdir(parents=True, exist_ok=True)
+        shutil.copy2(source, target)
+    for name in LINKED:
+        if (REPO_ROOT / name).exists():
+            (root / name).symlink_to(REPO_ROOT / name, target_is_directory=True)
+    return root
+
+
+def _remove(root: Path) -> None:
+    subprocess.run(["git", "worktree", "remove", "--force", str(root)],
+                   cwd=REPO_ROOT, capture_output=True, check=False)
+    shutil.rmtree(root, ignore_errors=True)
+
+
+def snapshot(root: Path) -> dict[str, str]:
+    """The SHA-1 of every file under root, links not followed. A file
+    written with the bytes it held is no change: what is caught is a file
+    whose content a run replaced. Scratch space, caches, bytecode and lock
+    files are no output."""
+    seen = {}
+    for directory, dirs, files in os.walk(root):
+        dirs[:] = [d for d in dirs if d != "__pycache__"
+                   and Path(directory, d) not in (root / "tmp", root / ".cache")]
+        for name in files:
+            path = Path(directory) / name
+            if name != ".lock" and not path.is_symlink():
+                seen[str(path.relative_to(root))] = hashlib.sha1(path.read_bytes()).hexdigest()
+    return seen

tests/test_artifact_lock.py

@@ -25,6 +25,7 @@ from common import (  # noqa: E402
     hold_repository_outputs,
     publish_hold,
 )
+from tests.scratchtree import scratch_checkout  # noqa: E402
 
 
 def _tempdir(test: unittest.TestCase) -> str:
@@ -115,11 +116,13 @@ class PackLockCliTest(unittest.TestCase):
         self.dir = _tempdir(self)
 
     def _run(self, argv: list[str]) -> subprocess.CompletedProcess:
+        # In a copy: a pipeline that missed the lock regenerated
+        # database.json and install/ wherever it ran.
         return subprocess.run(
             [sys.executable, *argv],
             capture_output=True,
             text=True,
-            cwd=str(REPO_ROOT),
+            cwd=str(scratch_checkout()),
             timeout=300,
         )
 
@@ -234,11 +237,12 @@ class PipelineHoldsItsFixedOutputs(unittest.TestCase):
 
     def test_a_second_pipeline_is_refused_whatever_its_output_dir(self):
         scratch = _tempdir(self)
-        with hold_repository_outputs(str(REPO_ROOT)):
+        checkout = scratch_checkout()
+        with hold_repository_outputs(str(checkout)):
             proc = subprocess.run(
                 [sys.executable, "scripts/pipeline.py", "--offline", "--skip-docs",
                  "--output-dir", scratch],
-                capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=120,
+                capture_output=True, text=True, cwd=str(checkout), timeout=120,
             )
         self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
         self.assertIn("another run is writing database.json", proc.stderr)
@@ -260,11 +264,12 @@ class StandaloneWritersHonourTheRepositoryHold(unittest.TestCase):
         env = {k: v for k, v in os.environ.items() if k != "RETROBIOS_HELD_ARTIFACT_DIR"}
         return subprocess.run(
             [sys.executable, *argv], capture_output=True, text=True,
-            cwd=str(REPO_ROOT), timeout=300, env=env, check=False,
+            cwd=str(scratch_checkout()), timeout=300, env=env, check=False,
         )
 
     def test_a_writer_of_a_fixed_output_is_refused(self):
-        with hold_repository_outputs(str(REPO_ROOT)):
+        # The writers run in a copy, where a missed lock writes nothing of ours.
+        with hold_repository_outputs(str(scratch_checkout())):
             for argv in (
                 ["scripts/generate_db.py", "--bios-dir", "bios", "--output", "database.json"],
                 ["scripts/generate_pack.py", "--all", "--manifest", "--offline",
@@ -369,7 +374,7 @@ class SiteGenerationHoldsDocs(unittest.TestCase):
         with artifact_lock(docs):
             result = subprocess.run(
                 [sys.executable, "scripts/generate_site.py", "--docs-dir", docs],
-                cwd=REPO_ROOT, capture_output=True, text=True, timeout=600,
+                cwd=scratch_checkout(), capture_output=True, text=True, timeout=600,
             )
         self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
         self.assertIn("in use by another run", result.stderr)

tests/test_system_selection.py

@@ -23,6 +23,7 @@ sys.path.insert(0, str(REPO_ROOT / "scripts"))
 
 import common  # noqa: E402
 import generate_pack as builder  # noqa: E402
+from tests.scratchtree import scratch_checkout, snapshot  # noqa: E402
 
 
 class SpellingsAgree(unittest.TestCase):
@@ -219,7 +220,7 @@ class AMissingDatabaseIsNamed(unittest.TestCase):
             with self.subTest(script=script):
                 proc = subprocess.run(
                     [sys.executable, f"scripts/{script}", "--db", "nosuch.json", *extra],
-                    capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600,
+                    capture_output=True, text=True, cwd=str(scratch_checkout()), timeout=600,
                     check=False,
                 )
                 self.assertNotEqual(proc.returncode, 0, proc.stdout[-500:])
@@ -254,7 +255,7 @@ class AMissingBiosDirIsRefused(unittest.TestCase):
             with self.subTest(script=script):
                 proc = subprocess.run(
                     [sys.executable, f"scripts/{script}", "--bios-dir", "nosuch", *extra],
-                    capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600,
+                    capture_output=True, text=True, cwd=str(scratch_checkout()), timeout=600,
                     check=False,
                 )
                 self.assertNotEqual(proc.returncode, 0)
@@ -307,14 +308,15 @@ class AMissingProfileDirIsRefused(unittest.TestCase):
                   "dat": str(root / "empty.dat")}
         taking = _scripts_taking("--emulators-dir")
         self.assertGreater(len(taking), 15)
-        tracked = ["git", "diff", "--name-only"]
-        before = subprocess.run(tracked, cwd=REPO_ROOT, capture_output=True, text=True).stdout
+        # In a copy: run in the checkout, a regression wrote there first.
+        checkout = scratch_checkout()
+        before = snapshot(checkout)
         for path in taking:
             extra = [arg.format(**values) for arg in self.EXTRA.get(path.stem, [])]
             with self.subTest(script=path.stem):
                 proc = subprocess.run(
                     [*_command(path), "--emulators-dir", "nosuch", *extra],
-                    capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600,
+                    capture_output=True, text=True, cwd=str(checkout), timeout=600,
                     check=False, env={**os.environ, "RETROBIOS_OFFLINE": "1"},
                 )
                 self.assertNotEqual(proc.returncode, 0, proc.stdout[-500:])
@@ -326,9 +328,11 @@ class AMissingProfileDirIsRefused(unittest.TestCase):
         ]
         self.assertEqual(written, [])
         # Read as empty, generate_site rewrote published-parts.json and the
-        # FAQ, and sitesources source-paths.json.
-        after = subprocess.run(tracked, cwd=REPO_ROOT, capture_output=True, text=True).stdout
-        self.assertEqual(after, before)
+        # FAQ, and sitesources source-paths.json. The copy compares the
+        # files themselves: a name `git diff` already listed hid a rewrite.
+        after = snapshot(checkout)
+        self.assertEqual(sorted(k for k in before.keys() | after.keys()
+                                if before.get(k) != after.get(k)), [])
 
 
 class AMissingPlatformsDirIsRefused(unittest.TestCase):
@@ -367,7 +371,7 @@ class AMissingPlatformsDirIsRefused(unittest.TestCase):
             with self.subTest(script=path.stem):
                 proc = subprocess.run(
                     [*_command(path), "--platforms-dir", "nosuch", *extra],
-                    capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600,
+                    capture_output=True, text=True, cwd=str(scratch_checkout()), timeout=600,
                     check=False, env={**os.environ, "RETROBIOS_OFFLINE": "1"},
                 )
                 self.assertNotEqual(proc.returncode, 0, proc.stdout[-500:])
@@ -375,6 +379,28 @@ class AMissingPlatformsDirIsRefused(unittest.TestCase):
                 self.assertRegex(proc.stderr, r"ERROR: .*nosuch")
 
 
+class TheCopyKeepsTheCheckoutOutOfReach(unittest.TestCase):
+    """The refusal tests above run their writers in a copy."""
+
+    def test_a_script_in_the_copy_writes_to_the_copy(self):
+        checkout = scratch_checkout()
+        proc = subprocess.run(
+            [sys.executable, "-c", "import sys; sys.path.insert(0, 'scripts'); "
+             "import generate_site; print(generate_site._REPO_ROOT)"],
+            capture_output=True, text=True, cwd=str(checkout), timeout=300, check=True,
+        )
+        self.assertEqual(Path(proc.stdout.strip()), checkout.resolve())
+
+    def test_a_rewrite_of_a_file_is_seen(self):
+        checkout = scratch_checkout()
+        page = checkout / "wiki" / "index.md"
+        before = snapshot(checkout)
+        original = page.read_bytes()
+        self.addCleanup(page.write_bytes, original)
+        page.write_bytes(original + b"\n")
+        self.assertNotEqual(snapshot(checkout), before)
+
+
 class AMissingInputFileIsNamed(unittest.TestCase):
     """The documented `--from-md5-file missing.txt` ended on a
     FileNotFoundError at the bottom of a stack trace."""