From 98fc12a6cbd6f724e31576f4a5f5aad504f9b25b Mon Sep 17 00:00:00 2001
From: Jon Meow <46229924+jonmeow@users.noreply.github.com>
Date: Fri, 25 Feb 2022 16:46:37 -0800
Subject: [PATCH] Try lockf instead of fsync (#1105)

I'm wondering if my issue all along was parallel script executions (I reproduced the error message locally with `pre-commit run -a`, and I think in my case that's what happened).

Adding the file size to errors for sanity checking, since part of what had me looking again was checksum mismatches, and actually the parallel execution offers a consistent story there (checksum mismatch due to file being modified by another download mid-stream).
---
 scripts/scripts_utils.py | 58 +++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/scripts/scripts_utils.py b/scripts/scripts_utils.py
index cc75ef6620d7..f447e1468fcf 100644
--- a/scripts/scripts_utils.py
+++ b/scripts/scripts_utils.py
@@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 """
 
 from enum import Enum
+import fcntl
 import hashlib
 import os
 from pathlib import Path
@@ -81,8 +82,6 @@ def _download(url: str, local_path: Path) -> Optional[int]:
             return int(response.code)
         with local_path.open("wb") as f:
             shutil.copyfileobj(response, f)
-            # Run fsync because of "Text file busy" in GH Actions.
-            os.fsync(f.fileno())
     return None
 
 
@@ -111,31 +110,40 @@ def get_release(release: Release) -> str:
         exit(f"No {release.value} release available for platform: {version}")
     want_hash = _VERSION_SHAS[release.value][version]
 
-    # Check if there's a cached file that can be used.
-    local_path = cache_dir.joinpath(f"{release.value}{ext}")
-    if local_path.is_file() and want_hash == _get_hash(local_path):
-        return str(local_path)
+    # Hold a lock while checksumming and downloading the path. Otherwise,
+    # parallel runs by pre-commit may conflict with one another with
+    # simultaneous downloads.
+    with open(cache_dir.joinpath(f"{release.value}.lock"), "w") as lock_file:
+        fcntl.lockf(lock_file.fileno(), fcntl.LOCK_EX)
 
-    # Download the file.
-    url = f"{_URL}/{release.value}-{version}{ext}"
-    retries = 5
-    while True:
-        err = _download(url, local_path)
-        if err is None:
-            break
-        retries -= 1
-        if retries == 0:
-            exit(f"Failed to download {release.value}-{version}: HTTP {err}.")
-        time.sleep(1)
-    local_path.chmod(0o755)
+        # Check if there's a cached file that can be used.
+        local_path = cache_dir.joinpath(f"{release.value}{ext}")
+        if local_path.is_file() and want_hash == _get_hash(local_path):
+            return str(local_path)
 
-    # Verify the downloaded hash.
-    found_hash = _get_hash(local_path)
-    if want_hash != found_hash:
-        exit(
-            f"Downloaded {release.value}-{version} but found sha256 "
-            f"{found_hash}, wanted {want_hash}"
-        )
+        # Download the file.
+        url = f"{_URL}/{release.value}-{version}{ext}"
+        retries = 5
+        while True:
+            err = _download(url, local_path)
+            if err is None:
+                break
+            retries -= 1
+            if retries == 0:
+                exit(
+                    f"Failed to download {release.value}-{version}: HTTP {err}."
+                )
+            time.sleep(1)
+        local_path.chmod(0o755)
+
+        # Verify the downloaded hash.
+        found_hash = _get_hash(local_path)
+        if want_hash != found_hash:
+            exit(
+                f"Downloaded {release.value}-{version} but found sha256 "
+                f"{found_hash} ({local_path.stat().st_size} bytes), wanted "
+                f"{want_hash}"
+            )
 
     return str(local_path)