From a4952bc029c8452c97c9cb0dae3be73b10c559d8 Mon Sep 17 00:00:00 2001
From: Reilly Wood <26268125+rgwood@users.noreply.github.com>
Date: Sun, 5 Mar 2023 19:04:12 -0800
Subject: [PATCH] Put a lock around `cargo build` invocations for plugin tests
 (#8333)

I think this _might_ fix the issues we've been seeing with plugin tests.

In a nutshell, the plugin tests run `cargo build` to ensure that plugins
have been built:
https://github.com/nushell/nushell/blob/f6ca62384e6140c32894ba94a3d703c7b08ae67f/crates/nu-test-support/src/commands.rs#L6

This PR adds a mutex to ensure that we're never running `cargo build`
concurrently. It also uses an atomic bool to signal when plugins have
already been built, so we can avoid invoking `cargo build` multiple
times unnecessarily.

I can't be certain yet, but I'm guessing the macOS CI problems we've
been seeing come from plugin tests clobbering each other (something
like: test 1 builds the `foo` plugin, then test2 invokes `cargo build`
again and deletes the `foo` plugin from disk).
---
 crates/nu-test-support/src/commands.rs | 22 ++++++++++++++++++++--
 crates/nu-test-support/src/macros.rs   |  2 +-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/crates/nu-test-support/src/commands.rs b/crates/nu-test-support/src/commands.rs
index 61930da1b..ae26b970a 100644
--- a/crates/nu-test-support/src/commands.rs
+++ b/crates/nu-test-support/src/commands.rs
@@ -1,11 +1,27 @@
 use std::{
     io::Read,
     process::{Command, Stdio},
+    sync::{
+        atomic::{AtomicBool, Ordering::Relaxed},
+        Mutex,
+    },
 };
 
-pub fn ensure_binary_present(package: &str) {
+static CARGO_BUILD_LOCK: Mutex<()> = Mutex::new(());
+static PLUGINS_BUILT: AtomicBool = AtomicBool::new(false);
+
+// This runs `cargo build --package nu_plugin_*` to ensure that all plugins
+// have been built before plugin tests run. We use a lock to avoid multiple
+// simultaneous `cargo build` invocations clobbering each other.
+pub fn ensure_plugins_built() {
+    let _guard = CARGO_BUILD_LOCK.lock().expect("could not get mutex lock");
+
+    if PLUGINS_BUILT.load(Relaxed) {
+        return;
+    }
+
     let cargo_path = env!("CARGO");
-    let mut arguments = vec!["build", "--package", package, "--quiet"];
+    let mut arguments = vec!["build", "--package", "nu_plugin_*", "--quiet"];
 
     let profile = std::env::var("NUSHELL_CARGO_TARGET");
     if let Ok(profile) = &profile {
@@ -40,4 +56,6 @@ pub fn ensure_binary_present(package: &str) {
     if !success {
         panic!("cargo build failed");
     }
+
+    PLUGINS_BUILT.store(true, Relaxed);
 }
diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs
index e3643ee2a..726219404 100644
--- a/crates/nu-test-support/src/macros.rs
+++ b/crates/nu-test-support/src/macros.rs
@@ -252,7 +252,7 @@ macro_rules! nu_with_plugins {
         let temp_plugin_file = temp.path().join("plugin.nu");
         std::fs::File::create(&temp_plugin_file).expect("couldn't create temporary plugin file");
 
-        $($crate::commands::ensure_binary_present($plugin_name);)+
+        $crate::commands::ensure_plugins_built();
 
         // TODO: the `$format` is a dummy empty string, but `plugin_name` is repeatable
         // just keep it here for now.  Need to find a way to remove it.