diff --git a/scripts/refresh_data_dirs.py b/scripts/refresh_data_dirs.py index 45bcb129..50d2c783 100644 --- a/scripts/refresh_data_dirs.py +++ b/scripts/refresh_data_dirs.py @@ -447,6 +447,15 @@ def _refresh_entry( ) return True + if remote_tag is None: + # Read before the download. A release published in between then + # leaves an older tag on newer content, which the next run refreshes; + # read after, it left a newer tag on older content, trusted for good. + if source_type == "zip": + remote_tag = _get_remote_etag(source_url) + else: + remote_tag = get_remote_sha(entry["source_url"], version) + try: if source_type == "zip": strip = entry.get("strip_components", 0) @@ -468,11 +477,6 @@ def _refresh_entry( log.warning("[%s] download failed: %s", key, exc) return None - if remote_tag is None: - if source_type == "zip": - remote_tag = _get_remote_etag(source_url) - else: - remote_tag = get_remote_sha(entry["source_url"], version) _record_version(key, {"sha": remote_tag or "", "version": version}, versions_path) log.info("[%s] refreshed: %d files extracted to %s", key, file_count, local_cache) diff --git a/tests/test_refresh_failures.py b/tests/test_refresh_failures.py index f36ea99a..2e55074b 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 json import shutil import subprocess import sys @@ -105,5 +106,33 @@ class AnEmptyExtractionKeepsTheCache(unittest.TestCase): self.assertEqual((self.cache / "keep.dat").read_bytes(), b"cached") +class TheVersionIsReadBeforeTheDownload(unittest.TestCase): + """The buildbot republishes nightly. Read after the download, the tag + named the new build while the cache held the old one, and every later + run called it up to date.""" + + def test_a_build_published_during_the_download(self): + import refresh_data_dirs as rdd + + root = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp")) + self.addCleanup(shutil.rmtree, root, True) + state = {"downloaded": False} + + def etag(_url): + return "build-2" if state["downloaded"] else "build-1" + + def download(*_args, **_kwargs): + state["downloaded"] = True + return 3 + + versions = root / "versions.json" + entry = {"source_type": "zip", "source_url": "https://example.invalid/Sys.zip", + "local_cache": str(root / "cache")} + with mock.patch.object(rdd, "_get_remote_etag", etag), \ + mock.patch.object(rdd, "_download_and_extract_zip", download): + self.assertTrue(rdd._refresh_entry("sys", entry, True, False, str(versions))) + self.assertEqual(json.loads(versions.read_text())["sys"]["sha"], "build-1") + + if __name__ == "__main__": unittest.main()