refactor: simplify fspy::Command API and add pre_exec hook (#23)

### TL;DR

Refactored the `fspy` crate to simplify the API by removing the `Spy` struct and making `Command` the primary entry point.

### What changed?

- Removed the `Spy` struct and replaced it with a global `SPY_IMPL` static using `LazyLock`
- Made `Command::new()` the primary entry point for creating commands instead of `Spy::new_command()`
- Renamed `SpyInner` to `SpyImpl` and moved spawn implementation into this struct
- Added `pre_exec` support to `Command` on Unix platforms
- Made most `Command` fields private with proper accessors
- Added `derive_more` dependency for better Debug implementations
- Updated all tests and examples to use the new API

### How to test?

- Run the existing test suite to verify the refactored API works correctly
- Try creating and running commands with the new API:
  ```rust
  let mut command = fspy::Command::new("program");
  command.args(["arg1", "arg2"]);
  let child = command.spawn().await?;
  ```

### Why make this change?

This refactoring simplifies the API by removing an unnecessary abstraction layer. The `Spy` struct was mostly a wrapper around creating commands, and having a global instance was redundant. By making `Command` the primary entry point and using a global implementation internally, the API becomes more intuitive and follows patterns similar to standard libraries. The change also improves encapsulation by making fields private and adding proper accessors.

Author: branchseer

Cargo.lock

@@ -54,6 +54,7 @@ crossterm = { version = "0.29.0", features = ["event-stream"] }
 csv-async = { version = "1.3.1", features = ["tokio"] }
 ctor = "0.6"
 dashmap = "6.1.0"
+derive_more = "2.0.1"
 diff-struct = "0.5.3"
 directories = "6.0.0"
 elf = { version = "0.8.0", default-features = false }
@@ -124,8 +125,14 @@ winsafe = { version = "0.0.24", features = ["kernel"] }
 xxhash-rust = { version = "0.8.15", features = ["const_xxh3"] }
 
 [workspace.metadata.cargo-shear]
-# These are artifact dependencies. They are not directly `use`d in Rust code.
-ignored = ["fspy_preload_unix", "fspy_preload_windows", "fspy_test_bin"]
+ignored = [
+  # These are artifact dependencies. They are not directly `use`d in Rust code.
+  "fspy_preload_unix",
+  "fspy_preload_windows",
+  "fspy_test_bin",
+  # used in a macro in crates/fspy_test_utils/src/lib.rs
+  "ctor",
+]
 
 [profile.dev]
 # Disabling debug info speeds up local and CI builds,

Cargo.toml

@@ -10,7 +10,9 @@ bincode = { workspace = true }
 bstr = { workspace = true, default-features = false }
 bumpalo = { workspace = true }
 const_format = { workspace = true, features = ["fmt"] }
+derive_more = { workspace = true, features = ["debug"] }
 fspy_shared = { workspace = true }
+fspy_test_utils = { workspace = true }
 futures-util = { workspace = true }
 libc = { workspace = true }
 ouroboros = { workspace = true }

crates/fspy/Cargo.toml

@@ -16,9 +16,7 @@ async fn main() -> anyhow::Result<()> {
 
     let program = PathBuf::from(args.next().unwrap());
 
-    let spy = fspy::Spy::global()?;
-
-    let mut command = spy.new_command(program);
+    let mut command = fspy::Command::new(program);
     command.envs(std::env::vars_os()).args(args);
 
     let child = command.spawn().await?;

crates/fspy/examples/cli.rs

@@ -9,32 +9,49 @@ use std::{
 use fspy_shared_unix::exec::Exec;
 use tokio::process::Command as TokioCommand;
 
-use crate::{
-    TrackedChild,
-    error::SpawnError,
-    os_impl::{self, spawn_impl},
-};
+use crate::{SPY_IMPL, TrackedChild, error::SpawnError};
 
-#[derive(Debug)]
+#[derive(derive_more::Debug)]
 pub struct Command {
-    pub(crate) program: OsString,
-    pub(crate) args: Vec<OsString>,
-    pub(crate) envs: HashMap<OsString, OsString>,
-    pub(crate) cwd: Option<PathBuf>,
+    program: OsString,
+    args: Vec<OsString>,
+    envs: HashMap<OsString, OsString>,
+    cwd: Option<PathBuf>,
     #[cfg(unix)]
-    pub(crate) arg0: Option<OsString>,
+    arg0: Option<OsString>,
 
-    pub(crate) stderr: Option<Stdio>,
-    pub(crate) stdout: Option<Stdio>,
-    pub(crate) stdin: Option<Stdio>,
+    stderr: Option<Stdio>,
+    stdout: Option<Stdio>,
+    stdin: Option<Stdio>,
 
-    pub(crate) spy_inner: os_impl::SpyInner,
+    #[cfg(unix)]
+    #[debug("({} pre_exec closures)", pre_exec_closures.len())]
+    pre_exec_closures: Vec<Box<dyn FnMut() -> std::io::Result<()> + Send + Sync>>,
 }
 
 impl Command {
+    /// Create a new command to spy on the given program.
+    /// Initially, environment variables are not inherited from the parent.
+    /// To inherit, explicitly use `.envs(std::env::vars_os())`.
+    pub fn new<P: AsRef<OsStr>>(program: P) -> Self {
+        Self {
+            program: program.as_ref().to_os_string(),
+            args: Vec::new(),
+            envs: HashMap::new(),
+            cwd: None,
+            #[cfg(unix)]
+            arg0: None,
+            stderr: None,
+            stdout: None,
+            stdin: None,
+            #[cfg(unix)]
+            pre_exec_closures: Vec::new(),
+        }
+    }
+
     #[cfg(unix)]
     #[must_use]
-    pub fn get_exec(&self) -> Exec {
+    pub(crate) fn get_exec(&self) -> Exec {
         use std::{
             iter::once,
             os::unix::ffi::{OsStrExt, OsStringExt},
@@ -57,7 +74,7 @@ impl Command {
     }
 
     #[cfg(unix)]
-    pub fn set_exec(&mut self, mut exec: Exec) {
+    pub(crate) fn set_exec(&mut self, mut exec: Exec) {
         use std::os::unix::ffi::OsStringExt;
 
         self.program = OsString::from_vec(exec.program.into());
@@ -147,7 +164,7 @@ impl Command {
 
     pub async fn spawn(mut self) -> Result<TrackedChild, SpawnError> {
         self.resolve_program()?;
-        spawn_impl(self).await
+        SPY_IMPL.spawn(self).await
     }
 
     /// Resolve program name to full path using `PATH` and cwd.
@@ -179,7 +196,22 @@ impl Command {
         Ok(())
     }
 
-    pub(crate) fn into_tokio_command(self) -> TokioCommand {
+    /// Schedules a closure to be run just before the exec function is invoked.
+    ///
+    /// # Safety
+    ///
+    /// <https://doc.rust-lang.org/1.91.1/std/os/unix/process/trait.CommandExt.html#tymethod.pre_exec>
+    #[cfg(unix)]
+    pub unsafe fn pre_exec<F>(&mut self, f: F) -> &mut Self
+    where
+        F: FnMut() -> std::io::Result<()> + Send + Sync + 'static,
+    {
+        self.pre_exec_closures.push(Box::new(f));
+        self
+    }
+
+    /// Convert to a `tokio::process::Command` without tracking.
+    pub fn into_tokio_command(self) -> TokioCommand {
         let mut tokio_cmd = TokioCommand::new(self.program);
         if let Some(cwd) = &self.cwd {
             tokio_cmd.current_dir(cwd);
@@ -205,6 +237,12 @@ impl Command {
             tokio_cmd.stderr(stderr);
         }
 
+        #[cfg(unix)]
+        for pre_exec in self.pre_exec_closures {
+            // Safety: The caller of `pre_exec` is responsible for ensuring safety.
+            unsafe { tokio_cmd.pre_exec(pre_exec) };
+        }
+
         tokio_cmd
     }
 }

crates/fspy/src/command.rs

@@ -20,13 +20,13 @@ mod os_impl;
 mod arena;
 mod command;
 
-use std::{env::temp_dir, ffi::OsStr, fs::create_dir, io, process::ExitStatus, sync::OnceLock};
+use std::{env::temp_dir, fs::create_dir, io, process::ExitStatus, sync::LazyLock};
 
 pub use command::Command;
 pub use fspy_shared::ipc::{AccessMode, PathAccess};
 use futures_util::future::BoxFuture;
 pub use os_impl::PathAccessIterable;
-use os_impl::SpyInner;
+use os_impl::SpyImpl;
 use tokio::process::{ChildStderr, ChildStdin, ChildStdout};
 
 /// The result of a tracked child process upon its termination.
@@ -54,33 +54,8 @@ pub struct TrackedChild {
     pub wait_handle: BoxFuture<'static, io::Result<ChildTermination>>,
 }
 
-pub struct Spy(SpyInner);
-impl Spy {
-    pub fn new() -> io::Result<Self> {
-        let tmp_dir = temp_dir().join("fspy");
-        let _ = create_dir(&tmp_dir);
-        Ok(Self(SpyInner::init_in(&tmp_dir)?))
-    }
-
-    pub fn global() -> io::Result<&'static Self> {
-        static GLOBAL_SPY: OnceLock<Spy> = OnceLock::new();
-        GLOBAL_SPY.get_or_try_init(Self::new)
-    }
-
-    pub fn new_command<S: AsRef<OsStr>>(&self, program: S) -> Command {
-        Command {
-            program: program.as_ref().to_os_string(),
-            envs: Default::default(),
-            args: vec![],
-            cwd: None,
-            #[cfg(unix)]
-            arg0: None,
-            spy_inner: self.0.clone(),
-            stderr: None,
-            stdout: None,
-            stdin: None,
-        }
-    }
-}
-
-// pub use fspy_shared::ipc::*;
+pub(crate) static SPY_IMPL: LazyLock<SpyImpl> = LazyLock::new(|| {
+    let tmp_dir = temp_dir().join("fspy");
+    let _ = create_dir(&tmp_dir);
+    SpyImpl::init_in(&tmp_dir).expect("Failed to initialize global spy")
+});