fix: refetch a large file the release replaced

scripts/largefiles.py

@@ -163,11 +163,15 @@ def _fetch_asset(
             hashes, expected_sha1, expected_md5
         ):
             return cached
-        else:
-            # A verified copy of this asset that answers another hash: the
-            # caller wants a different revision under the same name. Keeping
-            # it is what lets the next caller for the primary find it.
+        elif offline or _served_size(name) in (None, os.path.getsize(cached)):
+            # A copy of this asset that answers another hash: the caller
+            # wants a different revision under the same name, and the
+            # release still serves this one. Keeping it is what lets the
+            # next caller for the primary find it.
             return None
+        # Otherwise the release was re-uploaded since this copy was cached
+        # (gh release upload --clobber): only a download can answer, and
+        # it replaces the copy only if it verifies.
 
     if offline:
         return None
@@ -179,20 +183,9 @@ def _fetch_asset(
         dir=dest_dir, prefix=os.path.basename(cached) + ".", suffix=".tmp"
     )
     os.close(tmp_fd)
-    # GitHub rewrites spaces to dots in release asset names, so a file whose
-    # name contains spaces is published under a dotted name.
-    candidates = [name]
-    if " " in name:
-        candidates.append(name.replace(" ", "."))
-
     try:
         downloaded = False
-        for candidate in candidates:
-            encoded_name = urllib.parse.quote(candidate)
-            url = (
-                f"https://github.com/{LARGE_FILES_REPO}/releases/download/"
-                f"{LARGE_FILES_RELEASE}/{encoded_name}"
-            )
+        for candidate, url in _asset_urls(name):
             try:
                 req = urllib.request.Request(url, headers={"User-Agent": "retrobios/1.0"})
                 with urllib.request.urlopen(req, timeout=300) as resp:
@@ -220,6 +213,44 @@ def _fetch_asset(
         _drop(tmp_path)
 
 
+def _asset_urls(name: str) -> list[tuple[str, str]]:
+    """Release URLs an asset may be published under, with their names.
+
+    GitHub rewrites spaces to dots in release asset names, so a file whose
+    name contains spaces is published under a dotted name.
+    """
+    candidates = [name]
+    if " " in name:
+        candidates.append(name.replace(" ", "."))
+    return [
+        (
+            candidate,
+            (
+                f"https://github.com/{LARGE_FILES_REPO}/releases/download/"
+                f"{LARGE_FILES_RELEASE}/{urllib.parse.quote(candidate)}"
+            ),
+        )
+        for candidate in candidates
+    ]
+
+
+def _served_size(name: str) -> int | None:
+    """Size of the asset the release serves now, or None when unreadable."""
+    for candidate, url in _asset_urls(name):
+        try:
+            req = urllib.request.Request(
+                url, method="HEAD", headers={"User-Agent": "retrobios/1.0"}
+            )
+            with urllib.request.urlopen(req, timeout=60) as resp:
+                length = resp.headers.get("Content-Length")
+        except (urllib.error.URLError, OSError, http.client.HTTPException) as exc:
+            print(f"  large file {candidate}: {exc}", file=sys.stderr)
+            continue
+        if length and length.isdigit():
+            return int(length)
+    return None
+
+
 def _matches(hashes: dict, expected_sha1: str, expected_md5: str) -> bool:
     """Whether computed hashes answer the caller's declaration."""
     if expected_sha1 and hashes["sha1"].lower() != expected_sha1.lower():

tests/test_large_file_cache.py

@@ -161,6 +161,64 @@ class LargeFileCacheTest(unittest.TestCase):
             str(cached),
         )
 
+    def _serve(self, payload: bytes, gets: list):
+        class _Head(io.BytesIO):
+            headers = {"Content-Length": str(len(payload))}
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *exc):
+                return False
+
+        def urlopen(req, timeout=None):
+            if req.get_method() == "HEAD":
+                return _Head()
+            gets.append(req.full_url)
+            return _SlowResponse(payload)
+
+        largefiles.urllib.request.urlopen = urlopen
+
+    def test_a_reuploaded_asset_replaces_the_cached_revision(self):
+        """--clobber put new bytes under the name: the stale copy answered
+        None on every run, without a single request."""
+        cached = Path(self.dir) / "asset.bin"
+        cached.write_bytes(PAYLOAD_A)
+        rebuilt = b"C" * 1000
+        gets: list = []
+        self._serve(rebuilt, gets)
+        result = common.fetch_large_file(
+            "asset.bin", dest_dir=self.dir,
+            expected_sha1=hashlib.sha1(rebuilt).hexdigest(),
+        )
+        self.assertEqual(result, str(cached))
+        self.assertEqual(cached.read_bytes(), rebuilt)
+        self.assertEqual(len(gets), 1)
+
+    def test_a_revision_the_release_still_serves_is_kept(self):
+        cached = Path(self.dir) / "asset.bin"
+        cached.write_bytes(PAYLOAD_A)
+        gets: list = []
+        self._serve(PAYLOAD_A, gets)
+        result = common.fetch_large_file(
+            "asset.bin", dest_dir=self.dir, expected_sha1="00" * 20
+        )
+        self.assertIsNone(result)
+        self.assertEqual(gets, [])
+        self.assertEqual(cached.read_bytes(), PAYLOAD_A)
+
+    def test_a_download_that_does_not_verify_keeps_the_cache(self):
+        cached = Path(self.dir) / "asset.bin"
+        cached.write_bytes(PAYLOAD_A)
+        gets: list = []
+        self._serve(b"D" * 10, gets)
+        result = common.fetch_large_file(
+            "asset.bin", dest_dir=self.dir, expected_sha1="00" * 20
+        )
+        self.assertIsNone(result)
+        self.assertEqual(cached.read_bytes(), PAYLOAD_A)
+        self.assertEqual(os.listdir(self.dir), ["asset.bin"])
+
 
 class HashCacheKeepsEveryDigest(unittest.TestCase):
     """A cache hit must serve the same five digests a fresh hash produces.