fix: read a data directory tag before its download

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)

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()