refactor: pid1-exe

README.md

@@ -109,22 +109,25 @@ CMD [ "your-application", "--arg1" ]
 
 The `pid1` binary supports various command-line options:
 
-``` shellsession
+```shellsession
 ❯ pid1 --help
-Usage:
+A Unix PID1 process wrapper for signal handling and zombie reaping
+
+Usage: pid1 [OPTIONS] <COMMAND> [ARGS]...
 
 Arguments:
   <COMMAND>  Process to run
-  [ARGS]...  Arguments to the process
+  [ARGS]...  Arguments to that process
 
 Options:
-  -w, --workdir <DIR>        Specify working direcory
-  -t, --timeout <TIMEOUT>    Timeout (in seconds) to wait for child proess to exit [default: 2]
+  -w, --workdir <DIR>        Specify working directory
+  -t, --timeout <SECONDS>    Grace period for stopping before escalating to SIGKILL [default: 2]
   -v, --verbose              Turn on verbose output
-  -e, --env <ENV>            Override environment variables. Can specify multiple times
-  -u, --user-id <USER_ID>    Run command with user ID
-  -g, --group-id <GROUP_ID>  Run command with group ID
+  -e, --env <KEY=VALUE>      Override environment variables. Can specify multiple times
+  -u, --user-id <USER ID>    Run command with user ID
+  -g, --group-id <GROUP ID>  Run command with group ID
   -h, --help                 Print help
+  -V, --version              Print version
 ```
 
 ---

pid1-exe/Cargo.toml

@@ -2,14 +2,13 @@
 name = "pid1-exe"
 version = "0.1.6"
 edition = "2021"
-description = "pid1 handling library for proper signal and zombie reaping of the PID1 process"
+description = "A Unix PID1 process wrapper for signal handling and zombie reaping"
 readme = "../README.md"
 homepage = "https://github.com/fpco/pid1-rs"
 repository = "https://github.com/fpco/pid1-rs"
 license = "MIT"
 keywords = ["cli", "init", "pid1", "process"]
 categories = ["command-line-utilities"]
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [[bin]]
 name = "pid1"
@@ -21,5 +20,7 @@ clap = { version = "4.5.41", default-features = false, features = [
   "help",
   "std",
 ] }
-pid1 = { version = "0.1.6", path = "../pid1" }
 signal-hook = "0.4.3"
+
+[target.'cfg(unix)'.dependencies]
+pid1 = { version = "0.1.6", path = "../pid1" }

pid1-exe/src/cli.rs

@@ -1,104 +1,108 @@
-use clap::Parser;
-#[cfg(target_family = "unix")]
 use pid1::Pid1Settings;
-#[cfg(target_family = "unix")]
 use signal_hook::{
     consts::{SIGCHLD, SIGINT, SIGTERM},
     iterator::Signals,
 };
-#[cfg(target_family = "unix")]
 use std::os::unix::process::CommandExt;
-#[cfg(target_family = "unix")]
 use std::time::Duration;
-use std::{error::Error, ffi::OsString, path::PathBuf};
+use std::{str::FromStr, ffi::OsString, path::PathBuf};
 
-#[derive(Parser, Debug, PartialEq)]
+#[derive(clap::Parser, Debug, PartialEq)]
+#[command(version, about, long_about = None)]
 pub(crate) struct Pid1App {
-    /// Specify working direcory
+    /// Specify working directory
     #[arg(short, long, value_name = "DIR")]
     pub(crate) workdir: Option<PathBuf>,
-    /// Timeout (in seconds) to wait for child proess to exit
-    #[arg(short, long, value_name = "TIMEOUT", default_value_t = 2)]
+
+    /// Grace period for stopping before escalating to SIGKILL
+    #[arg(short, long, value_name = "SECONDS", default_value_t = 2)]
     pub(crate) timeout: u8,
+
     /// Turn on verbose output
-    #[arg(short, long, default_value_t = false)]
+    #[arg(short, long)]
     pub(crate) verbose: bool,
+
     /// Override environment variables. Can specify multiple times.
-    #[arg(short, long, value_parser=parse_key_val::<OsString, OsString>)]
-    pub(crate) env: Vec<(OsString, OsString)>,
+    #[arg(short, long, value_name = "KEY=VALUE")]
+    pub(crate) env: Vec<KeyValue>,
+
     /// Run command with user ID
-    #[arg(short, long, value_name = "USER_ID")]
-    user_id: Option<u32>,
+    #[arg(short, long, value_name = "USER ID")]
+    pub(crate) user_id: Option<u32>,
+
     /// Run command with group ID
-    #[arg(short, long, value_name = "GROUP_ID")]
-    group_id: Option<u32>,
+    #[arg(short, long, value_name = "GROUP ID")]
+    pub(crate) group_id: Option<u32>,
+
     /// Process to run
-    #[arg(required = true)]
+    #[arg(trailing_var_arg = true)]
     pub(crate) command: String,
-    /// Arguments to the process
-    #[arg(required = false)]
+
+    /// Arguments to that process
     pub(crate) args: Vec<String>,
 }
 
 impl Pid1App {
-    #[cfg(target_family = "unix")]
+    /// Parses CLI arguments from the environment.
+    pub(crate) fn from_cli() -> Self {
+      use clap::Parser;
+      Self::parse()
+    }
+
     pub(crate) fn run(self) -> ! {
-        let mut child = std::process::Command::new(&self.command);
-        let child = child.args(&self.args[..]);
+        let mut cmd = std::process::Command::new(&self.command);
+        cmd.args(&self.args);
+
         if let Some(workdir) = &self.workdir {
-            child.current_dir(workdir);
+            cmd.current_dir(workdir);
         }
         if let Some(user_id) = &self.user_id {
-            child.uid(*user_id);
+            cmd.uid(*user_id);
         }
         if let Some(group_id) = &self.group_id {
-            child.gid(*group_id);
+            cmd.gid(*group_id);
         }
-        for (key, value) in &self.env {
-            child.env(key, value);
+        for KeyValue(key, value) in &self.env {
+            cmd.env(key, value);
         }
+
         let pid = std::process::id();
         if pid != 1 {
-            let status = child.exec();
+            let status = cmd.exec();
             eprintln!("execvp failed with: {status:?}");
 
             std::process::exit(1);
-        } else {
-            // Install signal handlers before launching child process
-            let signals = Signals::new([SIGTERM, SIGINT, SIGCHLD]).unwrap();
-            let child = child.spawn();
-            let child = match child {
-                Ok(child) => child,
-                Err(err) => {
-                    eprintln!("pid1: {} spawn failed. Got error: {err}", self.command);
-                    std::process::exit(1);
-                }
-            };
-
-            Pid1Settings::new()
-                .enable_log(self.verbose)
-                .timeout(Duration::from_secs(self.timeout.into()))
-                .pid1_handling(signals, child)
         }
-    }
 
-    #[cfg(target_family = "windows")]
-    pub(crate) fn run(self) -> ! {
-        eprintln!("pid1: Not supported on Windows");
-        std::process::exit(1);
+        // CRITICAL: Install signal handlers BEFORE spawning the child.
+        // This prevents a race condition where a fast-failing child sends SIGCHLD 
+        // before we are ready to catch and reap it, creating zombie processes.
+        let signals = Signals::new([SIGTERM, SIGINT, SIGCHLD]).unwrap();
+
+        let child = cmd.spawn().unwrap_or_else(|err| {
+            eprintln!("pid1: {} spawn failed. Got error: {err}", self.command);
+            std::process::exit(1);
+        });
+
+        Pid1Settings::new()
+            .enable_log(self.verbose)
+            .timeout(Duration::from_secs(self.timeout.into()))
+            .pid1_handling(signals, child)
     }
 }
 
-/// Parse a single key-value pair
-fn parse_key_val<T, U>(s: &str) -> Result<(T, U), Box<dyn Error + Send + Sync + 'static>>
-where
-    T: std::str::FromStr,
-    T::Err: Error + Send + Sync + 'static,
-    U: std::str::FromStr,
-    U::Err: Error + Send + Sync + 'static,
-{
-    let pos = s
-        .find('=')
-        .ok_or_else(|| format!("invalid KEY=value: no `=` found in `{s}`"))?;
-    Ok((s[..pos].parse()?, s[pos + 1..].parse()?))
+/// A CLI argument parsed from a `KEY=VALUE` pair.
+#[derive(Debug, Clone, PartialEq)]
+pub(crate) struct KeyValue(OsString, OsString);
+
+impl FromStr for KeyValue {
+    type Err = String;
+
+    /// Parses a CLI flag value into a [`KeyValue`] type.
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let (key, value) = s.split_once('=')
+          .ok_or_else(|| format!("invalid `KEY=VALUE` pair: no `=` found in `{s}`"))?;
+
+        Ok(KeyValue(key.into(), value.into()))
+    }
 }

pid1-exe/src/main.rs

@@ -1,10 +1,10 @@
+#[cfg(unix)]
 mod cli;
 
-use clap::Parser;
-
-use crate::cli::Pid1App;
-
 fn main() {
-    let cli = Pid1App::parse();
-    cli.run()
+    #[cfg(unix)]
+    cli::Pid1App::from_cli().run();
+
+    #[cfg(not(unix))]
+    compile_error!("`pid1` is only compatible with Unix-like operating systems.");
 }