Merge pull request #85 from delan/non-global-debug-pipefail

Add non-global ways to set debug and pipefail modes

Author: tao-guo

README.md

@@ -339,7 +339,9 @@ That said, there are some limitations to be aware of:
   each thread will have its own independent version of the variable
 - [`set_debug`](https://docs.rs/cmd_lib/latest/cmd_lib/fn.set_debug.html) and
   [`set_pipefail`](https://docs.rs/cmd_lib/latest/cmd_lib/fn.set_pipefail.html) are *global* and affect all threads;
-  there is currently no way to change those settings without affecting other threads
+  to change those settings without affecting other threads, use
+  [`ScopedDebug`](https://docs.rs/cmd_lib/latest/cmd_lib/struct.ScopedDebug.html) and
+  [`ScopedPipefail`](https://docs.rs/cmd_lib/latest/cmd_lib/struct.ScopedPipefail.html)
 
 [std::env::set_var]: https://doc.rust-lang.org/std/env/fn.set_var.html
 [std::env::remove_var]: https://doc.rust-lang.org/std/env/fn.remove_var.html

src/lib.rs

@@ -394,7 +394,7 @@ pub use log as inner_log;
 pub use logger::try_init_default_logger;
 #[doc(hidden)]
 pub use process::{register_cmd, AsOsStr, Cmd, CmdString, Cmds, GroupCmds, Redirect};
-pub use process::{set_debug, set_pipefail, CmdEnv};
+pub use process::{set_debug, set_pipefail, CmdEnv, ScopedDebug, ScopedPipefail};
 
 mod builtins;
 mod child;

src/process.rs

@@ -6,11 +6,13 @@ use crate::{CmdResult, FunResult};
 use faccess::{AccessMode, PathExt};
 use lazy_static::lazy_static;
 use os_pipe::{self, PipeReader, PipeWriter};
+use std::cell::Cell;
 use std::collections::HashMap;
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::fs::{File, OpenOptions};
 use std::io::{Error, ErrorKind, Result};
+use std::marker::PhantomData;
 use std::mem::take;
 use std::path::{Path, PathBuf};
 use std::process::Command;
@@ -91,15 +93,19 @@ pub fn register_cmd(cmd: &'static str, func: FnFun) {
     CMD_MAP.lock().unwrap().insert(OsString::from(cmd), func);
 }
 
+/// Whether debug mode is enabled globally.
+/// Can be overridden by the thread-local setting in [`DEBUG_OVERRIDE`].
 static DEBUG_ENABLED: LazyLock<AtomicBool> =
     LazyLock::new(|| AtomicBool::new(std::env::var("CMD_LIB_DEBUG") == Ok("1".into())));
 
+/// Whether debug mode is enabled globally.
+/// Can be overridden by the thread-local setting in [`PIPEFAIL_OVERRIDE`].
 static PIPEFAIL_ENABLED: LazyLock<AtomicBool> =
     LazyLock::new(|| AtomicBool::new(std::env::var("CMD_LIB_PIPEFAIL") != Ok("0".into())));
 
 /// Set debug mode or not, false by default.
 ///
-/// This is **global**, and affects all threads.
+/// This is **global**, and affects all threads. To set it for the current thread only, use [`ScopedDebug`].
 ///
 /// Setting environment variable CMD_LIB_DEBUG=0|1 has the same effect, but the environment variable is only
 /// checked once at an unspecified time, so the only reliable way to do that is when the program is first started.
@@ -109,7 +115,7 @@ pub fn set_debug(enable: bool) {
 
 /// Set pipefail or not, true by default.
 ///
-/// This is **global**, and affects all threads.
+/// This is **global**, and affects all threads. To set it for the current thread only, use [`ScopedPipefail`].
 ///
 /// Setting environment variable CMD_LIB_DEBUG=0|1 has the same effect, but the environment variable is only
 /// checked once at an unspecified time, so the only reliable way to do that is when the program is first started.
@@ -118,11 +124,99 @@ pub fn set_pipefail(enable: bool) {
 }
 
 pub(crate) fn debug_enabled() -> bool {
-    DEBUG_ENABLED.load(SeqCst)
+    DEBUG_OVERRIDE
+        .get()
+        .unwrap_or_else(|| DEBUG_ENABLED.load(SeqCst))
 }
 
 pub(crate) fn pipefail_enabled() -> bool {
-    PIPEFAIL_ENABLED.load(SeqCst)
+    PIPEFAIL_OVERRIDE
+        .get()
+        .unwrap_or_else(|| PIPEFAIL_ENABLED.load(SeqCst))
+}
+
+thread_local! {
+    /// Whether debug mode is enabled in the current thread.
+    /// None means to use the global setting in [`DEBUG_ENABLED`].
+    static DEBUG_OVERRIDE: Cell<Option<bool>> = Cell::new(None);
+
+    /// Whether pipefail mode is enabled in the current thread.
+    /// None means to use the global setting in [`PIPEFAIL_ENABLED`].
+    static PIPEFAIL_OVERRIDE: Cell<Option<bool>> = Cell::new(None);
+}
+
+/// Overrides the debug mode in the current thread, while the value is in scope.
+///
+/// Each override restores the previous value when dropped, so they can be nested.
+/// Since overrides are thread-local, these values can’t be sent across threads.
+///
+/// ```
+/// # use cmd_lib::{ScopedDebug, run_cmd};
+/// // Must give the variable a name, not just `_`
+/// let _debug = ScopedDebug::set(true);
+/// run_cmd!(echo hello world)?; // Will have debug on
+/// # Ok::<(), std::io::Error>(())
+/// ```
+// PhantomData field is equivalent to `impl !Send for Self {}`
+pub struct ScopedDebug(Option<bool>, PhantomData<*const ()>);
+
+/// Overrides the pipefail mode in the current thread, while the value is in scope.
+///
+/// Each override restores the previous value when dropped, so they can be nested.
+/// Since overrides are thread-local, these values can’t be sent across threads.
+// PhantomData field is equivalent to `impl !Send for Self {}`
+///
+/// ```
+/// # use cmd_lib::{ScopedPipefail, run_cmd};
+/// // Must give the variable a name, not just `_`
+/// let _debug = ScopedPipefail::set(false);
+/// run_cmd!(false | true)?; // Will have pipefail off
+/// # Ok::<(), std::io::Error>(())
+/// ```
+pub struct ScopedPipefail(Option<bool>, PhantomData<*const ()>);
+
+impl ScopedDebug {
+    /// ```compile_fail
+    /// let _: Box<dyn Send> = Box::new(cmd_lib::ScopedDebug::set(true));
+    /// ```
+    /// ```compile_fail
+    /// let _: Box<dyn Sync> = Box::new(cmd_lib::ScopedDebug::set(true));
+    /// ```
+    #[doc(hidden)]
+    pub fn test_not_send_not_sync() {}
+
+    pub fn set(enabled: bool) -> Self {
+        let result = Self(DEBUG_OVERRIDE.get(), PhantomData);
+        DEBUG_OVERRIDE.set(Some(enabled));
+        result
+    }
+}
+impl Drop for ScopedDebug {
+    fn drop(&mut self) {
+        DEBUG_OVERRIDE.set(self.0)
+    }
+}
+
+impl ScopedPipefail {
+    /// ```compile_fail
+    /// let _: Box<dyn Send> = Box::new(cmd_lib::ScopedPipefail::set(true));
+    /// ```
+    /// ```compile_fail
+    /// let _: Box<dyn Sync> = Box::new(cmd_lib::ScopedPipefail::set(true));
+    /// ```
+    #[doc(hidden)]
+    pub fn test_not_send_not_sync() {}
+
+    pub fn set(enabled: bool) -> Self {
+        let result = Self(PIPEFAIL_OVERRIDE.get(), PhantomData);
+        PIPEFAIL_OVERRIDE.set(Some(enabled));
+        result
+    }
+}
+impl Drop for ScopedPipefail {
+    fn drop(&mut self) {
+        PIPEFAIL_OVERRIDE.set(self.0)
+    }
 }
 
 #[doc(hidden)]

tests/test_macros.rs

@@ -108,6 +108,7 @@ fn test_vars_in_str3() {
 }
 
 #[test]
+// FIXME: doctests have no effect here, and we need to split these into one test per error
 /// ```compile_fail
 /// run_cmd!(echo "${msg0}").unwrap();
 /// assert_eq!(run_fun!(echo "${ msg }").unwrap(), "${ msg }");
@@ -144,15 +145,16 @@ fn test_pipe() {
     assert!(run_cmd!(false | wc).is_err());
     assert!(run_cmd!(echo xx | false | wc | wc | wc).is_err());
 
-    set_pipefail(false);
+    let _pipefail = ScopedPipefail::set(false);
     assert!(run_cmd!(du -ah . | sort -hr | head -n 10).is_ok());
-    set_pipefail(true);
+    let _pipefail = ScopedPipefail::set(true);
 
     let wc_cmd = "wc";
     assert!(run_cmd!(ls | $wc_cmd).is_ok());
+}
 
-    // test `ignore` command and pipefail mode
-    // FIXME: make set_pipefail() thread safe, then move this to a separate test_ignore_and_pipefail()
+#[test]
+fn test_ignore_and_pipefail() {
     struct TestCase {
         /// Run the test case, returning whether the result `.is_ok()`.
         code: fn() -> bool,
@@ -270,20 +272,21 @@ fn test_pipe() {
             "{} when pipefail is on",
             case.code_str
         );
-        set_pipefail(false);
+        let _pipefail = ScopedPipefail::set(false);
         ok &= check_eq!(
             (case.code)(),
             case.expected_ok_pipefail_off,
             "{} when pipefail is off",
             case.code_str
         );
-        set_pipefail(true);
+        let _pipefail = ScopedPipefail::set(true);
     }
 
     assert!(ok);
 }
 
 #[test]
+// FIXME: doctests have no effect here, and we need to split these into one test per error
 /// ```compile_fail
 /// run_cmd!(ls > >&1).unwrap();
 /// run_cmd!(ls >>&1).unwrap();
@@ -355,6 +358,7 @@ fn test_current_dir() {
 }
 
 #[test]
+// FIXME: doctests have no effect here, and we need to split these into one test per error
 /// ```compile_fail
 /// run_cmd!(ls / /x &>>> /tmp/f).unwrap();
 /// run_cmd!(ls / /x &> > /tmp/f).unwrap();