diff --git a/build.py b/build.py index 9685855df..88cb3dc72 100644 --- a/build.py +++ b/build.py @@ -69,6 +69,90 @@ def split_diagnostic_logd(logd_path: Path, chunk_size: int = DIAGNOSTIC_CHUNK_SI return chunks +def find_stale_diagnostic_artifacts( + diagnostic_dir: Path, + current_commit_id: str, +) -> list[Path]: + """Return diagnostic artifacts that do NOT belong to the current commit. + + Scans ``diagnostic_dir`` for files matching the standard naming patterns + (``build-XXXXXXXX.logd``, ``build-XXXXXXXX-partNNN.logd``, + ``build-XXXXXXXX.json``) and returns those whose embedded commit ID + differs from *current_commit_id*. + + This is a read-only operation -- nothing is deleted. + """ + if not diagnostic_dir.exists(): + return [] + + stale: list[Path] = [] + for entry in sorted(diagnostic_dir.iterdir()): + if not entry.is_file(): + continue + name = entry.name + # Match build-XXXXXXXX.logd, build-XXXXXXXX-partNNN.logd, build-XXXXXXXX.json + if name.startswith("build-") and ( + name.endswith(".logd") or name.endswith(".json") + ): + # Extract the 8-char commit id after "build-" + # For "build-ec966bc5.logd" -> "ec966bc5" + # For "build-ec966bc5-part001.logd" -> "ec966bc5" + # For "build-ec966bc5.json" -> "ec966bc5" + suffix = name[len("build-"):] + commit_part = suffix.split("-")[0] # handles both with and without -part + if len(commit_part) == 8 and all(c in "0123456789abcdef" for c in commit_part): + if commit_part != current_commit_id: + stale.append(entry) + return stale + + +def run_diagnostic_cleanup( + diagnostic_dir: Path, + current_commit_id: str, + force: bool = False, +) -> int: + """List (dry-run) or delete (force) stale diagnostic artifacts. + + Returns the number of stale artifacts found (or deleted when *force* is + True). Current-commit artifacts are never removed. + """ + stale = find_stale_diagnostic_artifacts(diagnostic_dir, current_commit_id) + + if not stale: + print(f" {color('✓', Colors.GREEN)} No stale diagnostic artifacts found.") + return 0 + + mode_label = "DELETING" if force else "WOULD DELETE (dry-run)" + mode_color = Colors.RED if force else Colors.YELLOW + print(f"\n {color(mode_label, mode_color)} {len(stale)} stale diagnostic artifact(s):") + + total_bytes = 0 + for artifact in stale: + try: + size = artifact.stat().st_size + total_bytes += size + size_str = f"{size / 1024:.1f} KiB" if size < 1024 * 1024 else f"{size / (1024 * 1024):.1f} MiB" + except OSError: + size_str = "unknown size" + rel = artifact.relative_to(ROOT) if artifact.is_relative_to(ROOT) else artifact + print(f" {color('▸', mode_color)} {rel} ({size_str})") + + total_str = f"{total_bytes / 1024:.1f} KiB" if total_bytes < 1024 * 1024 else f"{total_bytes / (1024 * 1024):.1f} MiB" + print(f"\n Total space that {'would be freed' if not force else 'freed'}: {total_str}") + + if not force: + print(f" {color('Run with --force-cleanup to actually delete these artifacts.', Colors.GRAY)}") + else: + for artifact in stale: + try: + artifact.unlink() + except OSError as e: + print(f" {color('✗', Colors.RED)} Failed to delete {artifact.name}: {e}") + print(f" {color('✓', Colors.GREEN)} Stale diagnostic artifacts deleted.") + + return len(stale) + + @dataclass class Module: name: str @@ -827,6 +911,10 @@ def main(): Diagnostic bundle: python3 build.py + +Stale diagnostic cleanup: + python3 build.py --diagnostic-cleanup Dry-run: list stale artifacts + python3 build.py --diagnostic-cleanup --force-cleanup Actually delete stale artifacts """, ) parser.add_argument( @@ -850,6 +938,16 @@ def main(): "--list", action="store_true", help="List available modules and exit", ) + parser.add_argument( + "--diagnostic-cleanup", action="store_true", + help="List stale diagnostic artifacts (dry-run by default; " + "current commit artifacts are never removed)", + ) + parser.add_argument( + "--force-cleanup", action="store_true", + help="When used with --diagnostic-cleanup, actually delete stale " + "artifacts instead of just listing them", + ) args = parser.parse_args() @@ -865,6 +963,17 @@ def main(): print(f" build: {' '.join(m.build_cmd)}") return 0 + if args.diagnostic_cleanup: + commit_id = current_commit_id() + print(f" {color('Stale diagnostic cleanup', Colors.BOLD)}") + print(f" Current commit: {commit_id}") + if args.force_cleanup: + print(f" {color('Mode: FORCE (artifacts will be deleted)', Colors.RED)}") + else: + print(f" {color('Mode: DRY-RUN (no files will be modified)', Colors.YELLOW)}") + count = run_diagnostic_cleanup(DIAGNOSTIC_DIR, commit_id, force=args.force_cleanup) + return 0 + print(f" {color('Checking prerequisites...', Colors.GRAY)}") missing = check_prerequisites() if missing: diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index 58642e7b9..0eadc7808 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -206,6 +206,98 @@ FROM pg_stat_activity WHERE state = 'idle' AND age > interval '1 hour'; ``` +## Diagnostic Artifact Cleanup + +Over time, the `diagnostic/` directory accumulates build artifacts (`.logd` and +`.json` files) from previous commits. Stale artifacts from old or interrupted +builds can make the directory noisy and hard to audit. The build system +provides a dedicated cleanup mode that safely identifies and optionally removes +these stale files. + +### Overview + +Each build generates diagnostic artifacts named after the current Git commit: + +- `diagnostic/build-XXXXXXXX.logd` -- encrypted diagnostic log bundle +- `diagnostic/build-XXXXXXXX-partNNN.logd` -- chunked parts of oversized logs +- `diagnostic/build-XXXXXXXX.json` -- diagnostic metadata + +Artifacts whose commit ID does not match the current HEAD are considered +**stale**. The current commit's artifacts are always preserved. + +### Dry-Run Mode (Default) + +To see which stale artifacts would be removed without actually deleting +anything: + +```bash +python3 build.py --diagnostic-cleanup +``` + +This is safe for CI pipelines and local inspection. It prints a list of stale +files with their sizes and the total space that would be freed, but does not +modify any files. + +### Force Mode (Actual Deletion) + +To actually delete the stale artifacts: + +```bash +python3 build.py --diagnostic-cleanup --force-cleanup +``` + +This removes all stale diagnostic files while preserving the current commit's +artifacts. Use this when you need to reclaim disk space or clean up before a +release. + +### Safety Guarantees + +- **Dry-run is the default**: running `--diagnostic-cleanup` alone never + modifies or deletes any files. +- **Current commit artifacts are always preserved**: the cleanup logic + explicitly excludes any file whose embedded commit ID matches the current + HEAD, including chunked `.logd` parts. +- **Only standard diagnostic files are targeted**: files that do not match the + `build-XXXXXXXX.logd`, `build-XXXXXXXX-partNNN.logd`, or + `build-XXXXXXXX.json` naming pattern are ignored. + +### Running Tests + +The cleanup logic is tested independently using temporary directories: + +```bash +python3 -m pytest tools/test_stale_cleanup.py -v +``` + +Or with unittest directly: + +```bash +python3 tools/test_stale_cleanup.py +``` + +### Example Session + +``` +$ python3 build.py --diagnostic-cleanup + + Tent of Trials: building + Working directory: /home/user/TentOfTrials + + Stale diagnostic cleanup + Current commit: ec966bc5 + Mode: DRY-RUN (no files will be modified) + + WOULD DELETE (dry-run) 12 stale diagnostic artifact(s): + ▸ diagnostic/build-0f9ce9bb.json (0.5 KiB) + ▸ diagnostic/build-0f9ce9bb.logd (12.3 KiB) + ▸ diagnostic/build-11b17016.json (0.5 KiB) + ▸ diagnostic/build-11b17016-part001.logd (40960.0 KiB) + ... + + Total space that would be freed: 245.8 MiB + Run with --force-cleanup to actually delete these artifacts. +``` + ## Capacity Planning ### Resource Utilization diff --git a/tools/test_stale_cleanup.py b/tools/test_stale_cleanup.py new file mode 100644 index 000000000..e8c3818b1 --- /dev/null +++ b/tools/test_stale_cleanup.py @@ -0,0 +1,242 @@ +#!/usr/bin/env python3 +"""Tests for stale diagnostic cleanup in build.py. + +Uses temporary diagnostic directories to verify dry-run, force, and +current-artifact preservation behavior without touching the real +diagnostic/ directory. +""" + +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path + +# Make build.py importable from the project root +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT)) + +from build import find_stale_diagnostic_artifacts, run_diagnostic_cleanup + + +class StaleDiagnosticCleanupTests(unittest.TestCase): + """Test suite for --diagnostic-cleanup functionality.""" + + def setUp(self): + """Create a temporary diagnostic directory with sample artifacts.""" + self.tmpdir = tempfile.TemporaryDirectory() + self.diag_dir = Path(self.tmpdir.name) + + # Create artifacts for several commits + self.commits = { + "aaaa0001": ["build-aaaa0001.logd", "build-aaaa0001.json"], + "aaaa0002": [ + "build-aaaa0002.logd", + "build-aaaa0002.json", + "build-aaaa0002-part001.logd", + "build-aaaa0002-part002.logd", + ], + "aaaa0003": ["build-aaaa0003.logd", "build-aaaa0003.json"], + } + + for commit_id, filenames in self.commits.items(): + for filename in filenames: + filepath = self.diag_dir / filename + filepath.write_text(f"diagnostic data for {commit_id}\n", encoding="utf-8") + + # Current commit is aaaa0003 + self.current_commit = "aaaa0003" + + def tearDown(self): + self.tmpdir.cleanup() + + def test_find_stale_excludes_current_commit(self): + """Current commit artifacts must never appear in the stale list.""" + stale = find_stale_diagnostic_artifacts(self.diag_dir, self.current_commit) + stale_names = {p.name for p in stale} + + # Current commit artifacts should NOT be stale + for name in self.commits["aaaa0003"]: + self.assertNotIn(name, stale_names, f"Current commit artifact {name} should not be stale") + + def test_find_stale_includes_other_commits(self): + """Artifacts from other commits should be reported as stale.""" + stale = find_stale_diagnostic_artifacts(self.diag_dir, self.current_commit) + stale_names = {p.name for p in stale} + + # All non-current commit artifacts should be stale + expected_stale = set() + for commit_id in ["aaaa0001", "aaaa0002"]: + expected_stale.update(self.commits[commit_id]) + + self.assertEqual(stale_names, expected_stale) + + def test_find_stale_empty_directory(self): + """An empty diagnostic directory should return no stale artifacts.""" + empty_dir = Path(tempfile.mkdtemp()) + try: + stale = find_stale_diagnostic_artifacts(empty_dir, "aaaa0003") + self.assertEqual(stale, []) + finally: + empty_dir.rmdir() + + def test_find_stale_nonexistent_directory(self): + """A non-existent directory should return no stale artifacts.""" + nonexistent = Path("/tmp/this-should-not-exist-xyz-123") + stale = find_stale_diagnostic_artifacts(nonexistent, "aaaa0003") + self.assertEqual(stale, []) + + def test_find_stale_ignores_non_matching_files(self): + """Files that don't match the build-XXXXXXXX pattern should be ignored.""" + # Add some non-matching files + (self.diag_dir / "readme.txt").write_text("not a diagnostic", encoding="utf-8") + (self.diag_dir / "build-short.logd").write_text("too short", encoding="utf-8") + (self.diag_dir / "random.json").write_text("{}", encoding="utf-8") + + stale = find_stale_diagnostic_artifacts(self.diag_dir, self.current_commit) + stale_names = {p.name for p in stale} + + self.assertNotIn("readme.txt", stale_names) + self.assertNotIn("build-short.logd", stale_names) + self.assertNotIn("random.json", stale_names) + + def test_dry_run_does_not_delete(self): + """Dry-run mode should list stale artifacts without deleting them.""" + # Capture output by redirecting stdout + import io + captured = io.StringIO() + old_stdout = sys.stdout + sys.stdout = captured + + try: + count = run_diagnostic_cleanup(self.diag_dir, self.current_commit, force=False) + finally: + sys.stdout = old_stdout + + # Should have found stale artifacts + self.assertGreater(count, 0) + + # All files should still exist + for commit_id, filenames in self.commits.items(): + for filename in filenames: + self.assertTrue( + (self.diag_dir / filename).exists(), + f"Dry-run should not delete {filename}", + ) + + def test_force_actually_deletes(self): + """Force mode should delete stale artifacts but preserve current commit ones.""" + import io + captured = io.StringIO() + old_stdout = sys.stdout + sys.stdout = captured + + try: + count = run_diagnostic_cleanup(self.diag_dir, self.current_commit, force=True) + finally: + sys.stdout = old_stdout + + # Should have found and deleted stale artifacts + self.assertGreater(count, 0) + + # Stale artifacts should be gone + for commit_id in ["aaaa0001", "aaaa0002"]: + for filename in self.commits[commit_id]: + self.assertFalse( + (self.diag_dir / filename).exists(), + f"Force mode should delete stale artifact {filename}", + ) + + # Current commit artifacts should still exist + for filename in self.commits["aaaa0003"]: + self.assertTrue( + (self.diag_dir / filename).exists(), + f"Force mode should preserve current commit artifact {filename}", + ) + + def test_no_stale_artifacts(self): + """When all artifacts belong to the current commit, nothing is stale.""" + # Remove all non-current commit artifacts + for commit_id in ["aaaa0001", "aaaa0002"]: + for filename in self.commits[commit_id]: + (self.diag_dir / filename).unlink() + + stale = find_stale_diagnostic_artifacts(self.diag_dir, self.current_commit) + self.assertEqual(stale, []) + + def test_dry_run_returns_correct_count(self): + """Dry-run should return the exact count of stale artifacts.""" + import io + captured = io.StringIO() + old_stdout = sys.stdout + sys.stdout = captured + + try: + count = run_diagnostic_cleanup(self.diag_dir, self.current_commit, force=False) + finally: + sys.stdout = old_stdout + + # aaaa0001: 2 files, aaaa0002: 4 files = 6 total stale + expected_count = len(self.commits["aaaa0001"]) + len(self.commits["aaaa0002"]) + self.assertEqual(count, expected_count) + + def test_force_returns_correct_count(self): + """Force mode should return the same count as dry-run for the same directory.""" + import io + + # Dry-run first + captured = io.StringIO() + old_stdout = sys.stdout + sys.stdout = captured + try: + dry_count = run_diagnostic_cleanup(self.diag_dir, self.current_commit, force=False) + finally: + sys.stdout = old_stdout + + # Force on a fresh copy + self.setUp() + captured = io.StringIO() + sys.stdout = captured + try: + force_count = run_diagnostic_cleanup(self.diag_dir, self.current_commit, force=True) + finally: + sys.stdout = old_stdout + + self.assertEqual(dry_count, force_count) + + def test_chunked_logd_parts_are_stale(self): + """Chunked .logd parts (build-XXXXXXXX-partNNN.logd) from stale commits should be detected.""" + stale = find_stale_diagnostic_artifacts(self.diag_dir, self.current_commit) + stale_names = {p.name for p in stale} + + # aaaa0002 has chunked parts + self.assertIn("build-aaaa0002-part001.logd", stale_names) + self.assertIn("build-aaaa0002-part002.logd", stale_names) + + def test_current_commit_chunked_parts_preserved(self): + """Chunked .logd parts for the current commit should be preserved even in force mode.""" + # Add chunked parts for the current commit + (self.diag_dir / "build-aaaa0003-part001.logd").write_text("chunk1", encoding="utf-8") + (self.diag_dir / "build-aaaa0003-part002.logd").write_text("chunk2", encoding="utf-8") + + import io + captured = io.StringIO() + old_stdout = sys.stdout + sys.stdout = captured + + try: + run_diagnostic_cleanup(self.diag_dir, self.current_commit, force=True) + finally: + sys.stdout = old_stdout + + # Current commit chunked parts should still exist + self.assertTrue((self.diag_dir / "build-aaaa0003-part001.logd").exists()) + self.assertTrue((self.diag_dir / "build-aaaa0003-part002.logd").exists()) + # Current commit main artifacts should still exist + self.assertTrue((self.diag_dir / "build-aaaa0003.logd").exists()) + self.assertTrue((self.diag_dir / "build-aaaa0003.json").exists()) + + +if __name__ == "__main__": + unittest.main()