From 6c75f5ae15cd1f5230bf906b384208359585647e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sat, 10 Oct 2026 04:42:58 +0200 Subject: [PATCH] fix: keep the cache when an archive yields nothing --- scripts/refresh_data_dirs.py | 14 ++++++++++++ tests/test_refresh_failures.py | 42 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/scripts/refresh_data_dirs.py b/scripts/refresh_data_dirs.py index 50a24411..45bcb129 100644 --- a/scripts/refresh_data_dirs.py +++ b/scripts/refresh_data_dirs.py @@ -210,6 +210,15 @@ def get_remote_sha(source_url: str, version: str) -> str | None: return None +class NothingExtracted(Exception): + """An archive that yields no file for the cache. + + Promoting it replaced the cache with an empty directory, recorded the new + version and reported success, so the next run read "up to date" and the + packs left without the directory. + """ + + def _is_safe_tar_member(member: tarfile.TarInfo, dest: Path) -> bool: """Reject path traversal, absolute paths, and symlinks in tar members.""" if member.issym() or member.islnk(): @@ -293,6 +302,8 @@ def _download_and_extract( shutil.copyfileobj(src, dst) file_count += 1 + if not file_count: + raise NothingExtracted(f"no file under {source_path} in the archive") _promote(extract_dir, cache_dir, Path(tmpdir)) return file_count @@ -354,6 +365,8 @@ def _download_and_extract_zip( shutil.copyfileobj(src, dst) file_count += 1 + if not file_count: + raise NothingExtracted("the archive holds no file to extract") # The old tree is stepped aside rather than deleted: removing it # first and then failing to move the new one in left the cache with # nothing at all, and the next run reads that as "never fetched". @@ -450,6 +463,7 @@ def _refresh_entry( OSError, tarfile.TarError, zipfile.BadZipFile, + NothingExtracted, ) as exc: log.warning("[%s] download failed: %s", key, exc) return None diff --git a/tests/test_refresh_failures.py b/tests/test_refresh_failures.py index 61367320..f36ea99a 100644 --- a/tests/test_refresh_failures.py +++ b/tests/test_refresh_failures.py @@ -8,6 +8,7 @@ failed, the cache left at the old version. from __future__ import annotations +import shutil import subprocess import sys import tempfile @@ -63,5 +64,46 @@ class RefreshFailuresAreFailures(unittest.TestCase): self.assertIn("sys.exit(1)", source[source.index("failed = update_changed(report)"):]) +class AnEmptyExtractionKeepsTheCache(unittest.TestCase): + """An archive without the cited subtree replaced the cache with an empty + directory, recorded the version and reported success.""" + + def setUp(self): + import refresh_data_dirs + + self.module = refresh_data_dirs + scratch = Path(__file__).resolve().parent.parent / "tmp" + scratch.mkdir(exist_ok=True) + self.root = Path(tempfile.mkdtemp(dir=scratch)) + self.addCleanup(shutil.rmtree, self.root, True) + self.cache = self.root / "cache" + self.cache.mkdir() + (self.cache / "keep.dat").write_bytes(b"cached") + + def test_a_tarball_without_the_subtree(self): + import tarfile + + archive = self.root / "archive.tar.gz" + payload = self.root / "a.txt" + payload.write_bytes(b"x") + with tarfile.open(archive, "w:gz") as tf: + tf.add(payload, arcname="repo-main/OtherDir/a.txt") + with self.assertRaises(self.module.NothingExtracted): + self.module._download_and_extract( + archive.as_uri(), "repo-main/Data/Sys", str(self.cache), [] + ) + self.assertEqual((self.cache / "keep.dat").read_bytes(), b"cached") + + def test_a_zip_with_no_file(self): + import zipfile + + archive = self.root / "archive.zip" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("only/", b"") + with self.assertRaises(self.module.NothingExtracted): + self.module._download_and_extract_zip(archive.as_uri(), str(self.cache)) + self.assertEqual((self.cache / "keep.dat").read_bytes(), b"cached") + + if __name__ == "__main__": unittest.main()