From 977945c644fd1fc3357ad7d03a14ee2d6e433017 Mon Sep 17 00:00:00 2001
From: Marek Kaput <marek.kaput@swmansion.com>
Date: Wed, 11 Oct 2023 15:58:34 +0200
Subject: [PATCH] Extract `protected_run_if_not_ok!` macro (#787)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

**Stack**:
- #783
- #767
- #787 ⬅


⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do
not merge manually using the UI - doing so may have unexpected results.*
---
 scarb/src/flock.rs                | 27 +++++++++++++++++++++
 scarb/src/sources/standard_lib.rs | 39 ++++++++++---------------------
 2 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/scarb/src/flock.rs b/scarb/src/flock.rs
index 48be1fc3b..36dddde60 100644
--- a/scarb/src/flock.rs
+++ b/scarb/src/flock.rs
@@ -319,6 +319,33 @@ impl<'a> fmt::Debug for Filesystem<'a> {
     }
 }
 
+/// The following sequence of if statements & advisory locks implements a file system-based
+/// mutex, that synchronizes extraction logic. The first condition checks if extraction has
+/// happened in the past. If not, then we acquire the advisory lock (which means waiting for
+/// our time slice to do the job). Successful lock acquisition does not mean though that we
+/// still have to perform the extraction! While we waited for our time slice, another process
+/// could just do the extraction! The second condition prevents repeating the work.
+///
+/// This is actually very important for correctness. The another process that performed
+/// the extraction, will highly probably soon try to read the extracted files. If we recreate
+/// the filesystem now, we will cause that process to crash. That's what happened on Windows
+/// in examples tests, when the second condition was missing.
+macro_rules! protected_run_if_not_ok {
+    ($fs:expr, $lock:expr, $body:block) => {{
+        let fs: &$crate::flock::Filesystem<'_> = $fs;
+        let lock: &$crate::flock::AdvisoryLock<'_> = $lock;
+        if !fs.is_ok() {
+            let _lock = lock.acquire_async().await?;
+            if !fs.is_ok() {
+                $body
+                fs.mark_ok()?;
+            }
+        }
+    }};
+}
+
+pub(crate) use protected_run_if_not_ok;
+
 fn acquire(
     file: &File,
     path: &Utf8Path,
diff --git a/scarb/src/sources/standard_lib.rs b/scarb/src/sources/standard_lib.rs
index f7ece02d0..697295f90 100644
--- a/scarb/src/sources/standard_lib.rs
+++ b/scarb/src/sources/standard_lib.rs
@@ -12,6 +12,7 @@ use crate::core::manifest::{ManifestDependency, Summary, TomlManifest};
 use crate::core::package::{Package, PackageId};
 use crate::core::source::Source;
 use crate::core::SourceId;
+use crate::flock::protected_run_if_not_ok;
 use crate::internal::fsx;
 use crate::internal::fsx::PathUtf8Ext;
 use crate::sources::PathSource;
@@ -50,35 +51,19 @@ impl<'c> StandardLibSource<'c> {
 
         let tag_path = tag_fs.path_existent()?;
 
-        // The following sequence of if statements & advisory locks implements a file system-based
-        // mutex, that synchronizes extraction logic. The first condition checks if extraction has
-        // happened in the past. If not, then we acquire the advisory lock (which means waiting for
-        // our time slice to do the job). Successful lock acquisition does not mean though that we
-        // still have to perform the extraction! While we waited for our time slice, another process
-        // could just do the extraction! The second condition prevents repeating the work.
-        //
-        // This is actually very important for correctness. The another process that performed
-        // the extraction, will highly probably soon try to read the extracted files. If we recreate
-        // the filesystem now, we will cause that process to crash. That's what happened on Windows
-        // in examples tests, when the second condition was missing.
-        if !tag_fs.is_ok() {
-            let _lock = self.config.package_cache_lock().acquire_async().await?;
-            if !tag_fs.is_ok() {
-                trace!("extracting Cairo standard library: {tag}");
-                unsafe {
-                    tag_fs.recreate()?;
-                }
-
-                let base_path = tag_fs.path_existent()?;
+        protected_run_if_not_ok!(&tag_fs, &self.config.package_cache_lock(), {
+            trace!("extracting Cairo standard library: {tag}");
+            unsafe {
+                tag_fs.recreate()?;
+            }
 
-                extract_with_templating(&CORELIB, &base_path.join("core"))
-                    .context("failed to extract Cairo standard library (corelib)")?;
-                extract_with_templating(&SCARBLIB, base_path)
-                    .context("failed to extract Cairo standard library (scarblib)")?;
+            let base_path = tag_fs.path_existent()?;
 
-                tag_fs.mark_ok()?;
-            }
-        }
+            extract_with_templating(&CORELIB, &base_path.join("core"))
+                .context("failed to extract Cairo standard library (corelib)")?;
+            extract_with_templating(&SCARBLIB, base_path)
+                .context("failed to extract Cairo standard library (scarblib)")?;
+        });
 
         check_corelib_version(&tag_fs.path_existent()?.join("core").join("Scarb.toml"))?;