From c2d1e08fa6dfd2d9782d595961547b0da9c23449 Mon Sep 17 00:00:00 2001 From: aiolibsbot Date: Sun, 17 May 2026 04:18:30 +0000 Subject: [PATCH 1/3] fix: clean up orphan build dirs from crashed previous runs A crash between the rename and the final symlink-replace inside _atomic_replace_old_index leaves -tmpXXXX directories (and sometimes -build symlinks) behind forever. Two changes: - Sweep matching siblings at startup while holding the lock; skip the one the live symlink points at. - rmtree(temp_dir, ignore_errors=True) in the failure handler so a partial replace (where temp_dir was already renamed) no longer raises FileNotFoundError and masks the original exception. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/index_503/index.py | 29 ++++++++++++++++++++++- tests/test_index.py | 54 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/index_503/index.py b/src/index_503/index.py index 919dd0f..1d46b8f 100644 --- a/src/index_503/index.py +++ b/src/index_503/index.py @@ -6,6 +6,7 @@ from pathlib import Path from shutil import rmtree from tempfile import mkdtemp +from typing import Optional from natsort import natsorted from yarl import URL @@ -52,6 +53,7 @@ def make_index(self) -> Path: """Generate a simple repository of Python wheels.""" target_path = self.target_path old_index = target_path.readlink() if target_path.exists() else None + self._cleanup_orphan_build_dirs(old_index) temp_dir = mkdtemp(None, None, str(self.target_path.parent)) try: self.cache.load() @@ -68,9 +70,34 @@ def make_index(self) -> Path: return target_path except Exception: _LOGGER.exception("Error generating index") - rmtree(temp_dir) + # Use ignore_errors so a partial _atomic_replace_old_index (which + # already renamed temp_dir) doesn't mask the original exception. + # Any leftover dirs are cleaned up on the next run. + rmtree(temp_dir, ignore_errors=True) raise + def _cleanup_orphan_build_dirs(self, current_target: Optional[Path]) -> None: + """Remove build directories left behind by crashed previous runs. + + The atomic-replace flow renames temp dirs to ``-tmpXXXX`` and + briefly creates ``-tmpXXXX-build`` symlinks. A crash between + steps leaves these in place forever; clean them up at startup while we + hold the lock. + """ + parent = self.target_path.parent + prefix = f"{self.target_path.name}-tmp" + keep_name = current_target.name if current_target is not None else None + for entry in parent.iterdir(): + if not entry.name.startswith(prefix): + continue + if entry.name == keep_name: + continue + _LOGGER.warning("Removing orphan build artifact: %s", entry) + if entry.is_symlink() or not entry.is_dir(): + entry.unlink() + else: + rmtree(entry, ignore_errors=True) + def _atomic_replace_old_index(self, temp_dir_path: Path, target_path: Path) -> None: """Atomically replace the old index with the new one.""" final_name = target_path.parent / (target_path.name + "-" + temp_dir_path.name) diff --git a/tests/test_index.py b/tests/test_index.py index e6bdaef..5fb1356 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -1,11 +1,12 @@ import json +import os from pathlib import Path from shutil import copyfile from unittest.mock import ANY, patch import pytest -from index_503.index import make_index +from index_503.index import IndexMaker, make_index from index_503.wheel_file import WHEEL_FILE_VERSION from . import FIXTURES @@ -60,6 +61,57 @@ def test_make_index_fails(tmp_path: Path) -> None: assert parent_dir_contents == {"musllinux", ".musllinux.index_503.lock"} +def test_make_index_fails_partial_replace(tmp_path: Path) -> None: + """If _atomic_replace_old_index renames the temp dir then crashes, the + original exception must propagate (no FileNotFoundError masking it) and + the orphan must be cleaned up on the next run. + """ + origin_path, target_path = setup_wheels(tmp_path, TEST_WHEELS) + parent_dir = origin_path.parent + + original = IndexMaker._atomic_replace_old_index + + def rename_then_fail(self: IndexMaker, temp_dir_path: Path, t: Path) -> None: + # Mimic a partial run: rename succeeds, then we blow up. + final_name = t.parent / (t.name + "-" + temp_dir_path.name) + os.rename(temp_dir_path, final_name) + raise ValueError("boom") + + with pytest.raises(ValueError, match="boom"), patch.object( + IndexMaker, "_atomic_replace_old_index", rename_then_fail + ): + make_index(origin_path) + + # Orphan dir survives the failed run... + orphans = [p for p in parent_dir.iterdir() if p.name.startswith("musllinux-index-tmp")] + assert len(orphans) == 1 + + # ...but the next successful run cleans it up. + assert make_index(origin_path) == target_path + leftover = [p for p in parent_dir.iterdir() if p.name.startswith("musllinux-index-tmp")] + # Exactly one survives — the live target the symlink points at. + assert len(leftover) == 1 + assert leftover[0].name == target_path.readlink().name + + +def test_make_index_cleans_orphan_build_symlink(tmp_path: Path) -> None: + """An orphan -build symlink from a crash between symlink and replace gets removed.""" + origin_path, target_path = setup_wheels(tmp_path, TEST_WHEELS) + parent_dir = origin_path.parent + + # Seed orphans that look exactly like crash leftovers. + orphan_dir = parent_dir / "musllinux-index-tmpDEADBEEF" + orphan_dir.mkdir() + (orphan_dir / "junk").write_text("x") + orphan_symlink = parent_dir / "musllinux-index-tmpFEEDFACE-build" + orphan_symlink.symlink_to(orphan_dir) + + assert make_index(origin_path) == target_path + + assert not orphan_dir.exists() + assert not orphan_symlink.exists() + + def test_make_index_end_to_end(tmp_path: Path) -> None: """Test make_index() end to end.""" origin_path, origin_path_index = setup_wheels(tmp_path, TEST_WHEELS) From 2f04a807c210fa6e25a8c724350480c62a8ab9d9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 17 May 2026 04:19:14 +0000 Subject: [PATCH 2/3] chore(pre-commit.ci): auto fixes --- tests/test_index.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_index.py b/tests/test_index.py index 5fb1356..6b4f654 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -83,12 +83,16 @@ def rename_then_fail(self: IndexMaker, temp_dir_path: Path, t: Path) -> None: make_index(origin_path) # Orphan dir survives the failed run... - orphans = [p for p in parent_dir.iterdir() if p.name.startswith("musllinux-index-tmp")] + orphans = [ + p for p in parent_dir.iterdir() if p.name.startswith("musllinux-index-tmp") + ] assert len(orphans) == 1 # ...but the next successful run cleans it up. assert make_index(origin_path) == target_path - leftover = [p for p in parent_dir.iterdir() if p.name.startswith("musllinux-index-tmp")] + leftover = [ + p for p in parent_dir.iterdir() if p.name.startswith("musllinux-index-tmp") + ] # Exactly one survives — the live target the symlink points at. assert len(leftover) == 1 assert leftover[0].name == target_path.readlink().name From 013247f3c39828b5f0b241734ac77395e66571f6 Mon Sep 17 00:00:00 2001 From: aiolibsbot Date: Sun, 17 May 2026 18:26:18 +0000 Subject: [PATCH 3/3] fix: drop unused 'original' var in test (flake8 F841) Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_index.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_index.py b/tests/test_index.py index 6b4f654..c7cf124 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -69,8 +69,6 @@ def test_make_index_fails_partial_replace(tmp_path: Path) -> None: origin_path, target_path = setup_wheels(tmp_path, TEST_WHEELS) parent_dir = origin_path.parent - original = IndexMaker._atomic_replace_old_index - def rename_then_fail(self: IndexMaker, temp_dir_path: Path, t: Path) -> None: # Mimic a partial run: rename succeeds, then we blow up. final_name = t.parent / (t.name + "-" + temp_dir_path.name)