From 917961f0d9cfc65632e7e7d8614c7545a5ac12dc Mon Sep 17 00:00:00 2001 From: Igor Date: Sun, 27 Oct 2024 01:28:25 +0400 Subject: [PATCH] feat: implement `kill -l` (#221) Leverage nix to enumerate and translate signal values. --- brush-core/src/builtins/kill.rs | 55 ++++++++- brush-core/src/builtins/trap.rs | 54 ++------- brush-core/src/completion.rs | 7 +- brush-core/src/error.rs | 4 +- brush-core/src/sys/stubs/signal.rs | 8 -- brush-core/src/sys/unix/signal.rs | 16 +-- brush-core/src/traps.rs | 131 +++++++++++++++++++-- brush-shell/tests/cases/builtins/kill.yaml | 17 +++ 8 files changed, 202 insertions(+), 90 deletions(-) create mode 100644 brush-shell/tests/cases/builtins/kill.yaml diff --git a/brush-core/src/builtins/kill.rs b/brush-core/src/builtins/kill.rs index f94b978f..3a134104 100644 --- a/brush-core/src/builtins/kill.rs +++ b/brush-core/src/builtins/kill.rs @@ -1,6 +1,7 @@ use clap::Parser; use std::io::Write; +use crate::traps::TrapSignal; use crate::{builtins, commands, error}; /// Signal a job or process. @@ -37,15 +38,13 @@ impl builtins::Command for KillCommand { } if self.list_signals { - error::unimp("kill -l") + return print_signals(&context, self.args.as_ref()); } else { if self.args.len() != 1 { writeln!(context.stderr(), "{}: invalid usage", context.command_name)?; return Ok(builtins::ExitCode::InvalidUsage); } - let exit_code = builtins::ExitCode::Success; - let pid_or_job_spec = &self.args[0]; if pid_or_job_spec.starts_with('%') { // It's a job spec. @@ -67,8 +66,56 @@ impl builtins::Command for KillCommand { nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), nix::sys::signal::SIGKILL) .map_err(|_errno| error::Error::FailedToSendSignal)?; } + } + Ok(builtins::ExitCode::Success) + } +} - Ok(exit_code) +fn print_signals( + context: &commands::ExecutionContext<'_>, + signals: &[String], +) -> Result { + let mut exit_code = builtins::ExitCode::Success; + if !signals.is_empty() { + for s in signals { + // If the user gives us a code, we print the name; if they give a name, we print its + // code. + enum PrintSignal { + Name(&'static str), + Num(i32), + } + + let signal = if let Ok(n) = s.parse::() { + // bash compatibility. `SIGHUP` -> `HUP` + TrapSignal::try_from(n).map(|s| { + PrintSignal::Name(s.as_str().strip_prefix("SIG").unwrap_or(s.as_str())) + }) + } else { + TrapSignal::try_from(s.as_str()).map(|sig| { + i32::try_from(sig).map_or(PrintSignal::Name(sig.as_str()), PrintSignal::Num) + }) + }; + + match signal { + Ok(PrintSignal::Num(n)) => { + writeln!(context.stdout(), "{n}")?; + } + Ok(PrintSignal::Name(s)) => { + writeln!(context.stdout(), "{s}")?; + } + Err(e) => { + writeln!(context.stderr(), "{e}")?; + exit_code = builtins::ExitCode::Custom(1); + } + } } + } else { + return crate::traps::format_signals( + context.stdout(), + TrapSignal::iterator().filter(|s| !matches!(s, TrapSignal::Exit)), + ) + .map(|()| builtins::ExitCode::Success); } + + Ok(exit_code) } diff --git a/brush-core/src/builtins/trap.rs b/brush-core/src/builtins/trap.rs index 1a9cc71f..33cff639 100644 --- a/brush-core/src/builtins/trap.rs +++ b/brush-core/src/builtins/trap.rs @@ -1,7 +1,8 @@ use clap::Parser; use std::io::Write; -use crate::{builtins, commands, error, sys, traps}; +use crate::traps::TrapSignal; +use crate::{builtins, commands, error}; /// Manage signal traps. #[derive(Parser)] @@ -23,13 +24,12 @@ impl builtins::Command for TrapCommand { mut context: commands::ExecutionContext<'_>, ) -> Result { if self.list_signals { - Self::display_signals(&context)?; - Ok(builtins::ExitCode::Success) + crate::traps::format_signals(context.stdout(), TrapSignal::iterator()) + .map(|()| builtins::ExitCode::Success) } else if self.print_trap_commands || self.args.is_empty() { if !self.args.is_empty() { for signal_type in &self.args { - let signal_type = parse_signal(signal_type)?; - Self::display_handlers_for(&context, signal_type)?; + Self::display_handlers_for(&context, signal_type.parse()?)?; } } else { Self::display_all_handlers(&context)?; @@ -37,15 +37,14 @@ impl builtins::Command for TrapCommand { Ok(builtins::ExitCode::Success) } else if self.args.len() == 1 { let signal = self.args[0].as_str(); - let signal_type = parse_signal(signal)?; - Self::remove_all_handlers(&mut context, signal_type); + Self::remove_all_handlers(&mut context, signal.parse()?); Ok(builtins::ExitCode::Success) } else { let handler = &self.args[0]; let mut signal_types = vec![]; for signal in &self.args[1..] { - signal_types.push(parse_signal(signal)?); + signal_types.push(signal.parse()?); } Self::register_handler(&mut context, signal_types, handler.as_str()); @@ -56,16 +55,6 @@ impl builtins::Command for TrapCommand { #[allow(unused_variables)] impl TrapCommand { - #[allow(clippy::unnecessary_wraps)] - fn display_signals(context: &commands::ExecutionContext<'_>) -> Result<(), error::Error> { - #[cfg(unix)] - for signal in nix::sys::signal::Signal::iterator() { - writeln!(context.stdout(), "{}: {signal}", signal as i32)?; - } - - Ok(()) - } - fn display_all_handlers(context: &commands::ExecutionContext<'_>) -> Result<(), error::Error> { for signal in context.shell.traps.handlers.keys() { Self::display_handlers_for(context, *signal)?; @@ -75,7 +64,7 @@ impl TrapCommand { fn display_handlers_for( context: &commands::ExecutionContext<'_>, - signal_type: traps::TrapSignal, + signal_type: TrapSignal, ) -> Result<(), error::Error> { if let Some(handler) = context.shell.traps.handlers.get(&signal_type) { writeln!(context.stdout(), "trap -- '{handler}' {signal_type}")?; @@ -85,14 +74,14 @@ impl TrapCommand { fn remove_all_handlers( context: &mut crate::commands::ExecutionContext<'_>, - signal: traps::TrapSignal, + signal: TrapSignal, ) { context.shell.traps.remove_handlers(signal); } fn register_handler( context: &mut crate::commands::ExecutionContext<'_>, - signals: Vec, + signals: Vec, handler: &str, ) { for signal in signals { @@ -103,26 +92,3 @@ impl TrapCommand { } } } - -fn parse_signal(signal: &str) -> Result { - if signal.chars().all(|c| c.is_ascii_digit()) { - let digits = signal - .parse::() - .map_err(|_| error::Error::InvalidSignal)?; - - sys::signal::parse_numeric_signal(digits) - } else { - let mut signal_to_parse = signal.to_ascii_uppercase(); - - if !signal_to_parse.starts_with("SIG") { - signal_to_parse.insert_str(0, "SIG"); - } - - match signal_to_parse { - s if s == "SIGDEBUG" => Ok(traps::TrapSignal::Debug), - s if s == "SIGERR" => Ok(traps::TrapSignal::Err), - s if s == "SIGEXIT" => Ok(traps::TrapSignal::Exit), - s => sys::signal::parse_os_signal_name(s.as_str()), - } - } -} diff --git a/brush-core/src/completion.rs b/brush-core/src/completion.rs index 450a661d..5a73cee8 100644 --- a/brush-core/src/completion.rs +++ b/brush-core/src/completion.rs @@ -487,10 +487,9 @@ impl Spec { } } CompleteAction::Signal => { - for signal in traps::TrapSignal::all_values() { - let signal_str = signal.to_string(); - if signal_str.starts_with(token) { - candidates.insert(signal_str); + for signal in traps::TrapSignal::iterator() { + if signal.as_str().starts_with(token) { + candidates.insert(signal.as_str().to_string()); } } } diff --git a/brush-core/src/error.rs b/brush-core/src/error.rs index d2e890dc..946e0821 100644 --- a/brush-core/src/error.rs +++ b/brush-core/src/error.rs @@ -148,8 +148,8 @@ pub enum Error { ThreadingError(#[from] tokio::task::JoinError), /// An invalid signal was referenced. - #[error("invalid signal")] - InvalidSignal, + #[error("{0}: invalid signal specification")] + InvalidSignal(String), /// A system error occurred. #[cfg(unix)] diff --git a/brush-core/src/sys/stubs/signal.rs b/brush-core/src/sys/stubs/signal.rs index 22c8932d..6b816cfe 100644 --- a/brush-core/src/sys/stubs/signal.rs +++ b/brush-core/src/sys/stubs/signal.rs @@ -1,13 +1,5 @@ use crate::{error, sys, traps}; -pub(crate) fn parse_numeric_signal(_signal: i32) -> Result { - Err(error::Error::InvalidSignal) -} - -pub(crate) fn parse_os_signal_name(_signal: &str) -> Result { - Err(error::Error::InvalidSignal) -} - pub(crate) fn continue_process(_pid: sys::process::ProcessId) -> Result<(), error::Error> { error::unimp("continue process") } diff --git a/brush-core/src/sys/unix/signal.rs b/brush-core/src/sys/unix/signal.rs index 281f2f50..67cb1a6d 100644 --- a/brush-core/src/sys/unix/signal.rs +++ b/brush-core/src/sys/unix/signal.rs @@ -1,18 +1,4 @@ -use std::str::FromStr; - -use crate::{error, sys, traps}; - -pub(crate) fn parse_numeric_signal(signal: i32) -> Result { - Ok(traps::TrapSignal::Signal( - nix::sys::signal::Signal::try_from(signal).map_err(|_| error::Error::InvalidSignal)?, - )) -} - -pub(crate) fn parse_os_signal_name(signal: &str) -> Result { - Ok(traps::TrapSignal::Signal( - nix::sys::signal::Signal::from_str(signal).map_err(|_| error::Error::InvalidSignal)?, - )) -} +use crate::{error, sys}; pub(crate) fn continue_process(pid: sys::process::ProcessId) -> Result<(), error::Error> { #[allow(clippy::cast_possible_wrap)] diff --git a/brush-core/src/traps.rs b/brush-core/src/traps.rs index f4004ca6..25729f2c 100644 --- a/brush-core/src/traps.rs +++ b/brush-core/src/traps.rs @@ -1,5 +1,10 @@ +use std::str::FromStr; use std::{collections::HashMap, fmt::Display}; +use itertools::Itertools as _; + +use crate::error; + /// Type of signal that can be trapped in the shell. #[derive(Clone, Copy, Eq, Hash, PartialEq)] pub enum TrapSignal { @@ -16,28 +21,128 @@ pub enum TrapSignal { impl Display for TrapSignal { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +impl TrapSignal { + /// Returns all possible values of [`TrapSignal`]. + pub fn iterator() -> impl Iterator { + const SIGNALS: &[TrapSignal] = &[TrapSignal::Debug, TrapSignal::Err, TrapSignal::Exit]; + let iter = SIGNALS.iter().copied(); + + #[cfg(unix)] + let iter = itertools::chain!( + iter, + nix::sys::signal::Signal::iterator().map(TrapSignal::Signal) + ); + + iter + } + + /// Converts [`TrapSignal`] into its corresponding signal name as a [`&'static str`](str) + pub const fn as_str(self) -> &'static str { match self { #[cfg(unix)] - TrapSignal::Signal(s) => s.fmt(f), - TrapSignal::Debug => write!(f, "DEBUG"), - TrapSignal::Err => write!(f, "ERR"), - TrapSignal::Exit => write!(f, "EXIT"), + TrapSignal::Signal(s) => s.as_str(), + TrapSignal::Debug => "DEBUG", + TrapSignal::Err => "ERR", + TrapSignal::Exit => "EXIT", } } } -impl TrapSignal { - /// Returns all possible values of `TrapSignal`. - #[allow(unused_mut)] - pub fn all_values() -> Vec { - let mut signals = vec![TrapSignal::Debug, TrapSignal::Err, TrapSignal::Exit]; +/// Formats [`Iterator`](TrapSignal) to the provided writer. +/// +/// # Arguments +/// +/// * `f` - Any type that implements [`std::io::Write`]. +/// * `it` - An iterator over the signals that will be formatted into the `f`. +pub fn format_signals( + mut f: impl std::io::Write, + it: impl Iterator, +) -> Result<(), error::Error> { + let it = it + .filter_map(|s| i32::try_from(s).ok().map(|n| (s, n))) + .sorted_by(|a, b| Ord::cmp(&a.1, &b.1)) + .format_with("\n", |s, f| f(&format_args!("{}) {}", s.1, s.0))); + write!(f, "{it}")?; + Ok(()) +} - #[cfg(unix)] - for signal in nix::sys::signal::Signal::iterator() { - signals.push(TrapSignal::Signal(signal)); +// implement s.parse::() +impl FromStr for TrapSignal { + type Err = error::Error; + fn from_str(s: &str) -> Result::Err> { + if let Ok(n) = s.parse::() { + TrapSignal::try_from(n) + } else { + TrapSignal::try_from(s) } + } +} + +// from a signal number +impl TryFrom for TrapSignal { + type Error = error::Error; + fn try_from(value: i32) -> Result { + // NOTE: DEBUG and ERR are real-time signals, defined based on NSIG or SIGRTMAX (is not + // available on bsd-like systems), + // and don't have persistent numbers across platforms, so we skip them here. + Ok(match value { + 0 => TrapSignal::Exit, + #[cfg(unix)] + value => TrapSignal::Signal( + nix::sys::signal::Signal::try_from(value) + .map_err(|_| error::Error::InvalidSignal(value.to_string()))?, + ), + #[cfg(not(unix))] + _ => return Err(error::Error::InvalidSignal(value.to_string())), + }) + } +} + +// from a signal name +impl TryFrom<&str> for TrapSignal { + type Error = error::Error; + fn try_from(value: &str) -> Result { + #[allow(unused_mut)] // on not unix platforms + let mut s = value.to_ascii_uppercase(); - signals + Ok(match s.as_str() { + "DEBUG" => TrapSignal::Debug, + "ERR" => TrapSignal::Err, + "EXIT" => TrapSignal::Exit, + + #[cfg(unix)] + _ => { + // Bash compatibility: + // support for signal names without the `SIG` prefix, for example `HUP` -> `SIGHUP` + if !s.starts_with("SIG") { + s.insert_str(0, "SIG"); + } + nix::sys::signal::Signal::from_str(s.as_str()) + .map(TrapSignal::Signal) + .map_err(|_| error::Error::InvalidSignal(value.into()))? + } + #[cfg(not(unix))] + _ => return Err(error::Error::InvalidSignal(value.into())), + }) + } +} + +#[derive(Debug, Clone, Copy)] +pub struct DoesntHaveANumber; + +impl TryFrom for i32 { + type Error = DoesntHaveANumber; + fn try_from(value: TrapSignal) -> Result { + Ok(match value { + #[cfg(unix)] + TrapSignal::Signal(s) => s as i32, + TrapSignal::Exit => 0, + _ => return Err(DoesntHaveANumber), + }) } } diff --git a/brush-shell/tests/cases/builtins/kill.yaml b/brush-shell/tests/cases/builtins/kill.yaml new file mode 100644 index 00000000..3645ab67 --- /dev/null +++ b/brush-shell/tests/cases/builtins/kill.yaml @@ -0,0 +1,17 @@ +name: "Builtins: kill" +cases: + - name: "kill -l" + ignore_stderr: true + stdin: | + for i in $(seq 1 31); do kill -l $i; done + # limit the number of signals to 31. Realtime signals are not implemented yet. + for i in $(kill -l | sed -e "s/[[:digit:]]*)//g"); do echo $i; done | head -31 + # invalid option + kill -l 9999 + kill -l HUP + kill -l iNt + kill -l int + kill -l SIGHUP + kill -l EXIT + +