From b22415c8471fed691a8a97cb564baf7eb83989b9 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Mon, 10 Feb 2025 23:38:41 +0200 Subject: [PATCH 1/3] grep, pkill: Move match-related clap args to process_matcher --- src/uu/pgrep/src/pgrep.rs | 55 +++------------------------ src/uu/pgrep/src/process_matcher.rs | 59 ++++++++++++++++++++++++++++- src/uu/pkill/src/pkill.rs | 54 +++----------------------- 3 files changed, 68 insertions(+), 100 deletions(-) diff --git a/src/uu/pgrep/src/pgrep.rs b/src/uu/pgrep/src/pgrep.rs index c947c2d8..b1856abb 100644 --- a/src/uu/pgrep/src/pgrep.rs +++ b/src/uu/pgrep/src/pgrep.rs @@ -7,7 +7,7 @@ pub mod process; pub mod process_matcher; -use clap::{arg, crate_version, Arg, ArgAction, ArgGroup, Command}; +use clap::{arg, crate_version, Command}; use uucore::{error::UResult, format_usage, help_about, help_usage}; const ABOUT: &str = help_about!("pgrep.md"); @@ -76,61 +76,16 @@ pub fn uu_app() -> Command { .about(ABOUT) .override_usage(format_usage(USAGE)) .args_override_self(true) - .group(ArgGroup::new("oldest_newest").args(["oldest", "newest", "inverse"])) .args([ arg!(-d --delimiter "specify output delimiter") .default_value("\n") .hide_default_value(true), arg!(-l --"list-name" "list PID and process name"), arg!(-a --"list-full" "list PID and full command line"), - arg!(-H --"require-handler" "match only if signal handler is present"), - arg!(-v --inverse "negates the matching"), // arg!(-w --lightweight "list all TID"), - arg!(-c --count "count of matching processes"), - arg!(-f --full "use full process name to match"), - // arg!(-g --pgroup ... "match listed process group IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - // arg!(-G --group ... "match real group IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - arg!(-i --"ignore-case" "match case insensitively"), - arg!(-n --newest "select most recently started"), - arg!(-o --oldest "select least recently started"), - arg!(-O --older "select where older than seconds") - .value_parser(clap::value_parser!(u64)), - arg!(-P --parent "match only child processes of the given parent") - .value_delimiter(',') - .value_parser(clap::value_parser!(u64)), - // arg!(-s --session "match session IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - arg!(--signal "signal to send (either number or name)") - .default_value("SIGTERM"), - arg!(-t --terminal "match by controlling terminal") - .value_delimiter(','), - // arg!(-u --euid ... "match by effective IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - // arg!(-U --uid ... "match by real IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - arg!(-x --exact "match exactly with the command name"), - // arg!(-F --pidfile "read PIDs from file"), - // arg!(-L --logpidfile "fail if PID file is not locked"), - arg!(-r --runstates "match runstates [D,S,Z,...]"), - // arg!(-A --"ignore-ancestors" "exclude our ancestors from results"), - // arg!(--cgroup "match by cgroup v2 names") - // .value_delimiter(','), - // arg!( --ns "match the processes that belong to the same namespace as "), - // arg!( --nslist ... "list which namespaces will be considered for the --ns option.") - // .value_delimiter(',') - // .value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]), ]) - .arg( - Arg::new("pattern") - .help("Name of the program to find the PID of") - .action(ArgAction::Append) - .index(1), - ) + .args(process_matcher::clap_args( + "Name of the program to find the PID of", + true, + )) } diff --git a/src/uu/pgrep/src/process_matcher.rs b/src/uu/pgrep/src/process_matcher.rs index 51a9b2b2..e5472461 100644 --- a/src/uu/pgrep/src/process_matcher.rs +++ b/src/uu/pgrep/src/process_matcher.rs @@ -7,7 +7,7 @@ use std::collections::HashSet; -use clap::ArgMatches; +use clap::{arg, Arg, ArgAction, ArgMatches}; use regex::Regex; use uucore::error::{UResult, USimpleError}; #[cfg(unix)] @@ -248,3 +248,60 @@ fn parse_signal_value(signal_name: &str) -> UResult { signal_by_name_or_value(signal_name) .ok_or_else(|| USimpleError::new(1, format!("Unknown signal {}", signal_name.quote()))) } + +#[allow(clippy::cognitive_complexity)] +pub fn clap_args(pattern_help: &'static str, enable_v_flag: bool) -> Vec { + vec![ + if enable_v_flag { + arg!(-v --inverse "negates the matching").group("oldest_newest_inverse") + } else { + arg!(--inverse "negates the matching").group("oldest_newest_inverse") + }, + arg!(-H --"require-handler" "match only if signal handler is present"), + arg!(-c --count "count of matching processes"), + arg!(-f --full "use full process name to match"), + // arg!(-g --pgroup "match listed process group IDs") + // .value_delimiter(',') + // .value_parser(clap::value_parser!(u64)), + // arg!(-G --group "match real group IDs") + // .value_delimiter(',') + // .value_parser(clap::value_parser!(u64)), + arg!(-i --"ignore-case" "match case insensitively"), + arg!(-n --newest "select most recently started") + .group("oldest_newest_inverse"), + arg!(-o --oldest "select least recently started") + .group("oldest_newest_inverse"), + arg!(-O --older "select where older than seconds") + .value_parser(clap::value_parser!(u64)), + arg!(-P --parent "match only child processes of the given parent") + .value_delimiter(',') + .value_parser(clap::value_parser!(u64)), + // arg!(-s --session "match session IDs") + // .value_delimiter(',') + // .value_parser(clap::value_parser!(u64)), + arg!(--signal "signal to send (either number or name)") + .default_value("SIGTERM"), + arg!(-t --terminal "match by controlling terminal").value_delimiter(','), + // arg!(-u --euid "match by effective IDs") + // .value_delimiter(',') + // .value_parser(clap::value_parser!(u64)), + // arg!(-U --uid "match by real IDs") + // .value_delimiter(',') + // .value_parser(clap::value_parser!(u64)), + arg!(-x --exact "match exactly with the command name"), + // arg!(-F --pidfile "read PIDs from file"), + // arg!(-L --logpidfile "fail if PID file is not locked"), + arg!(-r --runstates "match runstates [D,S,Z,...]"), + // arg!(-A --"ignore-ancestors" "exclude our ancestors from results"), + // arg!(--cgroup "match by cgroup v2 names") + // .value_delimiter(','), + // arg!(--ns "match the processes that belong to the same namespace as "), + // arg!(--nslist "list which namespaces will be considered for the --ns option.") + // .value_delimiter(',') + // .value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]), + Arg::new("pattern") + .help(pattern_help) + .action(ArgAction::Append) + .index(1), + ] +} diff --git a/src/uu/pkill/src/pkill.rs b/src/uu/pkill/src/pkill.rs index 3c121e2d..07b8b0e5 100644 --- a/src/uu/pkill/src/pkill.rs +++ b/src/uu/pkill/src/pkill.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // Pid utils -use clap::{arg, crate_version, Arg, ArgAction, ArgGroup, Command}; +use clap::{arg, crate_version, Command}; #[cfg(unix)] use nix::{ sys::signal::{self, Signal}, @@ -109,57 +109,13 @@ pub fn uu_app() -> Command { .about(ABOUT) .override_usage(format_usage(USAGE)) .args_override_self(true) - .group(ArgGroup::new("oldest_newest").args(["oldest", "newest", "inverse"])) .args([ // arg!(- "signal to send (either number or name)"), - arg!(-H --"require-handler" "match only if signal handler is present"), // arg!(-q --queue "integer value to be sent with the signal"), arg!(-e --echo "display what is killed"), - arg!(--inverse "negates the matching"), - arg!(-c --count "count of matching processes"), - arg!(-f --full "use full process name to match"), - // arg!(-g --pgroup "match listed process group IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - // arg!(-G --group "match real group IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - arg!(-i --"ignore-case" "match case insensitively"), - arg!(-n --newest "select most recently started"), - arg!(-o --oldest "select least recently started"), - arg!(-O --older "select where older than seconds") - .value_parser(clap::value_parser!(u64)), - arg!(-P --parent "match only child processes of the given parent") - .value_delimiter(',') - .value_parser(clap::value_parser!(u64)), - // arg!(-s --session "match session IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - arg!(--signal "signal to send (either number or name)") - .default_value("SIGTERM"), - arg!(-t --terminal "match by controlling terminal").value_delimiter(','), - // arg!(-u --euid "match by effective IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - // arg!(-U --uid "match by real IDs") - // .value_delimiter(',') - // .value_parser(clap::value_parser!(u64)), - arg!(-x --exact "match exactly with the command name"), - // arg!(-F --pidfile "read PIDs from file"), - // arg!(-L --logpidfile "fail if PID file is not locked"), - arg!(-r --runstates "match runstates [D,S,Z,...]"), - // arg!(-A --"ignore-ancestors" "exclude our ancestors from results"), - // arg!(--cgroup "match by cgroup v2 names") - // .value_delimiter(','), - // arg!(--ns "match the processes that belong to the same namespace as "), - // arg!(--nslist "list which namespaces will be considered for the --ns option.") - // .value_delimiter(',') - // .value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]), ]) - .arg( - Arg::new("pattern") - .help("Name of the program to find the PID of") - .action(ArgAction::Append) - .index(1), - ) + .args(process_matcher::clap_args( + "Name of the process to kill", + false, + )) } From 74a2ffdddf98e9f50473026d6dd52f240b772b91 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Mon, 10 Feb 2025 23:26:08 +0200 Subject: [PATCH 2/3] pgrep, pkill: Add check for pattern length pidwait already has a check for this, copy it to the common process_matcher implementation. Some tests need adjustment to not use such long fake names. --- src/uu/pgrep/src/process_matcher.rs | 6 ++++++ tests/by-util/test_pgrep.rs | 12 +++++++++++- tests/by-util/test_pkill.rs | 12 +++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/uu/pgrep/src/process_matcher.rs b/src/uu/pgrep/src/process_matcher.rs index e5472461..7e3ca240 100644 --- a/src/uu/pgrep/src/process_matcher.rs +++ b/src/uu/pgrep/src/process_matcher.rs @@ -78,6 +78,12 @@ pub fn get_match_settings(matches: &ArgMatches) -> UResult { )); } + if !settings.full && pattern.len() >= 15 { + let msg = format!("pattern that searches for process name longer than 15 characters will result in zero matches\n\ + Try `{} -f' option to match against the complete command line.", uucore::util_name()); + return Err(USimpleError::new(1, msg)); + } + Ok(settings) } diff --git a/tests/by-util/test_pgrep.rs b/tests/by-util/test_pgrep.rs index 19d73f0b..0f33494c 100644 --- a/tests/by-util/test_pgrep.rs +++ b/tests/by-util/test_pgrep.rs @@ -31,7 +31,7 @@ fn test_no_args() { #[test] fn test_non_matching_pattern() { new_ucmd!() - .arg("THIS_PATTERN_DOES_NOT_MATCH") + .arg("NONMATCHING") .fails() .code_is(1) .no_output(); @@ -368,3 +368,13 @@ fn test_invalid_signal() { .fails() .stderr_contains("Unknown signal 'foo'"); } + +#[test] +#[cfg(target_os = "linux")] +fn test_too_long_pattern() { + new_ucmd!() + .arg("THIS_IS_OVER_16_CHARS") + .fails() + .code_is(1) + .stderr_contains("pattern that searches for process name longer than 15 characters will result in zero matches"); +} diff --git a/tests/by-util/test_pkill.rs b/tests/by-util/test_pkill.rs index 801e617d..f9686ddd 100644 --- a/tests/by-util/test_pkill.rs +++ b/tests/by-util/test_pkill.rs @@ -20,7 +20,7 @@ fn test_no_args() { #[test] fn test_non_matching_pattern() { new_ucmd!() - .arg("THIS_PATTERN_DOES_NOT_MATCH") + .arg("NONMATCHING") .fails() .code_is(1) .no_output(); @@ -60,3 +60,13 @@ fn test_inverse() { fn test_help() { new_ucmd!().arg("--help").succeeds(); } + +#[test] +#[cfg(target_os = "linux")] +fn test_too_long_pattern() { + new_ucmd!() + .arg("THIS_IS_OVER_16_CHARS") + .fails() + .code_is(1) + .stderr_contains("pattern that searches for process name longer than 15 characters will result in zero matches"); +} From b633a46d5f57b5143d7d98ec4aee7edaec8f51fb Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Mon, 10 Feb 2025 23:24:01 +0200 Subject: [PATCH 3/3] pidwait: Use common process_matcher Its matching functionality should be a superset of what pidwait can currently do. --- src/uu/pidwait/src/pidwait.rs | 231 ++-------------------------------- 1 file changed, 12 insertions(+), 219 deletions(-) diff --git a/src/uu/pidwait/src/pidwait.rs b/src/uu/pidwait/src/pidwait.rs index 62977ea3..1a110a6e 100644 --- a/src/uu/pidwait/src/pidwait.rs +++ b/src/uu/pidwait/src/pidwait.rs @@ -3,14 +3,9 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use clap::{arg, crate_version, value_parser, Arg, ArgAction, ArgMatches, Command}; -use regex::Regex; -use std::{collections::HashSet, env, sync::OnceLock}; -use uu_pgrep::process::{walk_process, ProcessInformation, RunState, Teletype}; -use uucore::{ - error::{UResult, USimpleError}, - format_usage, help_about, help_usage, -}; +use clap::{arg, crate_version, Command}; +use uu_pgrep::process_matcher; +use uucore::{error::UResult, format_usage, help_about, help_usage}; use wait::wait; mod wait; @@ -18,64 +13,12 @@ mod wait; const ABOUT: &str = help_about!("pidwait.md"); const USAGE: &str = help_usage!("pidwait.md"); -static REGEX: OnceLock = OnceLock::new(); - -#[derive(Debug)] -struct Settings { - echo: bool, - count: bool, - full: bool, - ignore_case: bool, - newest: bool, - oldest: bool, - older: Option, - terminal: Option>, - exact: bool, - runstates: Option>, -} - #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; - let settings = Settings { - echo: matches.get_flag("echo"), - count: matches.get_flag("count"), - full: matches.get_flag("full"), - ignore_case: matches.get_flag("ignore-case"), - newest: matches.get_flag("newest"), - oldest: matches.get_flag("oldest"), - older: matches.get_one::("older").copied(), - terminal: matches.get_many::("terminal").map(|ttys| { - ttys.cloned() - .flat_map(Teletype::try_from) - .collect::>() - }), - exact: matches.get_flag("exact"), - runstates: matches - .get_many::("runstates") - .map(|it| it.cloned().flat_map(RunState::try_from).collect()), - }; - - let pattern = initialize_pattern(&matches, &settings)?; - REGEX - .set(Regex::new(&pattern).map_err(|e| USimpleError::new(2, e.to_string()))?) - .unwrap(); - - if (!settings.newest - && !settings.oldest - && settings.runstates.is_none() - && settings.older.is_none() - && settings.terminal.is_none()) - && pattern.is_empty() - { - return Err(USimpleError::new( - 2, - "no matching criteria specified\nTry `pidwait --help' for more information.", - )); - } - - let mut proc_infos = collect_proc_infos(&settings); + let settings = process_matcher::get_match_settings(&matches)?; + let mut proc_infos = process_matcher::find_matching_pids(&settings); // For empty result if proc_infos.is_empty() { @@ -83,11 +26,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } // Process outputs - if settings.count { + if matches.get_flag("count") { println!("{}", proc_infos.len()); } - if settings.echo { + if matches.get_flag("echo") { if settings.newest || settings.oldest { for ele in &proc_infos { println!("waiting for (pid {})", ele.pid); @@ -104,165 +47,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { Ok(()) } -fn initialize_pattern(matches: &ArgMatches, settings: &Settings) -> UResult { - let pattern = match matches.get_many::("pattern") { - Some(patterns) if patterns.len() > 1 => { - return Err(USimpleError::new( - 2, - "only one pattern can be provided\nTry `pidwait --help' for more information.", - )) - } - Some(mut patterns) => patterns.next().unwrap(), - None => return Ok(String::new()), - }; - - let pattern = if settings.ignore_case { - &pattern.to_lowercase() - } else { - pattern - }; - - let pattern = if settings.exact { - &format!("^{}$", pattern) - } else { - pattern - }; - - if !settings.full && pattern.len() >= 15 { - const MSG_0: &str= "pidwait: pattern that searches for process name longer than 15 characters will result in zero matches"; - const MSG_1: &str = "Try `pidwait -f' option to match against the complete command line."; - return Err(USimpleError::new(1, format!("{MSG_0}\n{MSG_1}"))); - } - - Ok(pattern.to_string()) -} - -fn collect_proc_infos(settings: &Settings) -> Vec { - // Process pattern - let proc_infos = { - let mut temp = Vec::new(); - for mut it in walk_process() { - let matched = { - let binding = it.status(); - let name = binding.get("Name").unwrap(); - let name = if settings.ignore_case { - name.to_lowercase() - } else { - name.into() - }; - - let want = if settings.exact { - &name - } else if settings.full { - &it.cmdline - } else { - &it.proc_stat()[..15] - }; - - REGEX.get().unwrap().is_match(want) - }; - if matched { - temp.push(it); - } - } - temp - }; - - // Process `-O` - let proc_infos = { - let mut temp: Vec = Vec::new(); - let older = settings.older.unwrap_or_default(); - for mut proc_info in proc_infos { - if proc_info.start_time().unwrap() >= older { - temp.push(proc_info); - } - } - temp - }; - - let mut proc_infos = { - if let Some(terminals) = &settings.terminal { - proc_infos - .into_iter() - .filter(|it| terminals.contains(&it.tty())) - .collect() - } else { - proc_infos - } - }; - - if proc_infos.is_empty() { - return proc_infos; - } - - // Sorting oldest and newest - let proc_infos = if settings.oldest || settings.newest { - proc_infos.sort_by(|a, b| { - b.clone() - .start_time() - .unwrap() - .cmp(&a.clone().start_time().unwrap()) - }); - - let start_time = if settings.newest { - proc_infos.first().cloned().unwrap().start_time().unwrap() - } else { - proc_infos.last().cloned().unwrap().start_time().unwrap() - }; - - // There might be some process start at same time, so need to be filtered. - let mut filtered = proc_infos - .iter() - .filter(|it| (*it).clone().start_time().unwrap() == start_time) - .collect::>(); - - if settings.newest { - filtered.sort_by(|a, b| b.pid.cmp(&a.pid)); - } else { - filtered.sort_by(|a, b| a.pid.cmp(&b.pid)); - } - - vec![filtered.first().cloned().unwrap().clone()] - } else { - proc_infos - }; - - proc_infos -} - -#[allow(clippy::cognitive_complexity)] pub fn uu_app() -> Command { Command::new(env!("CARGO_PKG_NAME")) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) .infer_long_args(true) - .args([ - arg!(-e --echo "display PIDs before waiting"), - arg!(-c --count "count of matching processes"), - arg!(-f --full "use full process name to match"), - // arg!(-g --pgroup "match listed process group IDs"), - // arg!(-G --group "match real group IDs"), - arg!(-i --"ignore-case" "match case insensitively"), - arg!(-n --newest "select most recently started"), - arg!(-o --oldest "select least recently started"), - arg!(-O --older "select where older than seconds") - .value_parser(value_parser!(u64)), - // arg!(-P --parent "match only child processes of the given parent"), - // arg!(-s --session "match session IDs"), - arg!(-t --terminal "match by controlling terminal"), - // arg!(-u --euid "match by effective IDs"), - // arg!(-U --uid "match by real IDs"), - arg!(-x --exact "match exactly with the command name"), - // arg!(-F --pidfile "read PIDs from file"), - // arg!(-L --logpidfile "fail if PID file is not locked"), - arg!(-r --runstates "match runstates [D,S,Z,...]"), - // arg!(-A --"ignore-ancestors" "exclude our ancestors from results"), - ]) - .arg( - Arg::new("pattern") - .help("Name of the program to find the PID of") - .action(ArgAction::Append) - .index(1), - ) + .args([arg!(-e --echo "display PIDs before waiting")]) + .args(process_matcher::clap_args( + "Name of the program to wait for", + true, + )) }