From 45ede873b39bde24d3cb9c155735f21359d64bc7 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 6 Oct 2026 12:30:51 +0200 Subject: [PATCH] fix: refetch a large file the release replaced --- scripts/largefiles.py | 63 +++++++++++++++++++++++++--------- tests/test_large_file_cache.py | 58 +++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 16 deletions(-) diff --git a/scripts/largefiles.py b/scripts/largefiles.py index 0596a53e..79b8e9ab 100644 --- a/scripts/largefiles.py +++ b/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(): diff --git a/tests/test_large_file_cache.py b/tests/test_large_file_cache.py index ae2d0043..e951f98a 100644 --- a/tests/test_large_file_cache.py +++ b/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.