Repository: uutils/procps
Pull request #323 (2025-02-15 17:21:24)

pidwait: Use common process_matcher (+ move a pidwait feature there)
Copy remaining process-matching feature of pidwait to process_matcher so pgrep and pkill can utilize it, then move pidwait to use the common process_matcher as well.


Patch:
diff --git a/src/uu/pgrep/src/pgrep.rs b/src/uu/pgrep/src/pgrep.rs
index c947c2d..b1856ab 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 <string>    "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 <PGID>     ... "match listed process group IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            // arg!(-G     --group <GID>       ... "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 <seconds>       "select where older than seconds")
-                .value_parser(clap::value_parser!(u64)),
-            arg!(-P     --parent <PPID>         "match only child processes of the given parent")
-                .value_delimiter(',')
-                .value_parser(clap::value_parser!(u64)),
-            // arg!(-s     --session <SID>         "match session IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            arg!(--signal <sig>                 "signal to send (either number or name)")
-                .default_value("SIGTERM"),
-            arg!(-t     --terminal <tty>        "match by controlling terminal")
-                .value_delimiter(','),
-            // arg!(-u     --euid <ID>         ... "match by effective IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            // arg!(-U     --uid <ID>          ... "match by real IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            arg!(-x     --exact                 "match exactly with the command name"),
-            // arg!(-F     --pidfile <file>        "read PIDs from file"),
-            // arg!(-L     --logpidfile            "fail if PID file is not locked"),
-            arg!(-r     --runstates <state>     "match runstates [D,S,Z,...]"),
-            // arg!(-A     --"ignore-ancestors"    "exclude our ancestors from results"),
-            // arg!(--cgroup <grp>                 "match by cgroup v2 names")
-            //     .value_delimiter(','),
-            // arg!(       --ns <PID>              "match the processes that belong to the same namespace as <pid>"),
-            // arg!(       --nslist <ns>       ... "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 51a9b2b..7e3ca24 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)]
@@ -78,6 +78,12 @@ pub fn get_match_settings(matches: &ArgMatches) -> UResult<Settings> {
         ));
     }
 
+    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)
 }
 
@@ -248,3 +254,60 @@ fn parse_signal_value(signal_name: &str) -> UResult<usize> {
     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<Arg> {
+    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 <PGID>        "match listed process group IDs")
+        //     .value_delimiter(',')
+        //     .value_parser(clap::value_parser!(u64)),
+        // arg!(-G --group <GID>          "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 <seconds>      "select where older than seconds")
+            .value_parser(clap::value_parser!(u64)),
+        arg!(-P --parent <PPID>        "match only child processes of the given parent")
+            .value_delimiter(',')
+            .value_parser(clap::value_parser!(u64)),
+        // arg!(-s --session <SID>        "match session IDs")
+        //     .value_delimiter(',')
+        //     .value_parser(clap::value_parser!(u64)),
+        arg!(--signal <sig>            "signal to send (either number or name)")
+            .default_value("SIGTERM"),
+        arg!(-t --terminal <tty>       "match by controlling terminal").value_delimiter(','),
+        // arg!(-u --euid <ID>            "match by effective IDs")
+        //     .value_delimiter(',')
+        //     .value_parser(clap::value_parser!(u64)),
+        // arg!(-U --uid <ID>             "match by real IDs")
+        //     .value_delimiter(',')
+        //     .value_parser(clap::value_parser!(u64)),
+        arg!(-x --exact                "match exactly with the command name"),
+        // arg!(-F --pidfile <file>       "read PIDs from file"),
+        // arg!(-L --logpidfile           "fail if PID file is not locked"),
+        arg!(-r --runstates <state>    "match runstates [D,S,Z,...]"),
+        // arg!(-A --"ignore-ancestors"   "exclude our ancestors from results"),
+        // arg!(--cgroup <grp>            "match by cgroup v2 names")
+        //     .value_delimiter(','),
+        // arg!(--ns <PID>                "match the processes that belong to the same namespace as <pid>"),
+        // arg!(--nslist <ns>             "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/pidwait/src/pidwait.rs b/src/uu/pidwait/src/pidwait.rs
index 62977ea..1a110a6 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<Regex> = OnceLock::new();
-
-#[derive(Debug)]
-struct Settings {
-    echo: bool,
-    count: bool,
-    full: bool,
-    ignore_case: bool,
-    newest: bool,
-    oldest: bool,
-    older: Option<u64>,
-    terminal: Option<HashSet<Teletype>>,
-    exact: bool,
-    runstates: Option<HashSet<RunState>>,
-}
-
 #[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::<u64>("older").copied(),
-        terminal: matches.get_many::<String>("terminal").map(|ttys| {
-            ttys.cloned()
-                .flat_map(Teletype::try_from)
-                .collect::<HashSet<_>>()
-        }),
-        exact: matches.get_flag("exact"),
-        runstates: matches
-            .get_many::<String>("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<String> {
-    let pattern = match matches.get_many::<String>("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<ProcessInformation> {
-    // 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<ProcessInformation> = 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::<Vec<_>>();
-
-        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        <PGID>      "match listed process group IDs"),
-            // arg!(-G --group         <GID>       "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         <seconds>   "select where older than seconds")
-                .value_parser(value_parser!(u64)),
-            // arg!(-P --parent        <PPID>      "match only child processes of the given parent"),
-            // arg!(-s --session       <SID>       "match session IDs"),
-            arg!(-t --terminal      <tty>       "match by controlling terminal"),
-            // arg!(-u --euid          <ID>        "match by effective IDs"),
-            // arg!(-U --uid           <ID>        "match by real IDs"),
-            arg!(-x --exact                     "match exactly with the command name"),
-            // arg!(-F --pidfile       <file>      "read PIDs from file"),
-            // arg!(-L --logpidfile                "fail if PID file is not locked"),
-            arg!(-r --runstates     <state>     "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,
+        ))
 }
diff --git a/src/uu/pkill/src/pkill.rs b/src/uu/pkill/src/pkill.rs
index 3c121e2..07b8b0e 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!(-<sig>                    "signal to send (either number or name)"),
-            arg!(-H --"require-handler"    "match only if signal handler is present"),
             // arg!(-q --queue <value>        "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 <PGID>        "match listed process group IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            // arg!(-G --group <GID>          "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 <seconds>      "select where older than seconds")
-                .value_parser(clap::value_parser!(u64)),
-            arg!(-P --parent <PPID>        "match only child processes of the given parent")
-                .value_delimiter(',')
-                .value_parser(clap::value_parser!(u64)),
-            // arg!(-s --session <SID>        "match session IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            arg!(--signal <sig>            "signal to send (either number or name)")
-                .default_value("SIGTERM"),
-            arg!(-t --terminal <tty>       "match by controlling terminal").value_delimiter(','),
-            // arg!(-u --euid <ID>            "match by effective IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            // arg!(-U --uid <ID>             "match by real IDs")
-            //     .value_delimiter(',')
-            //     .value_parser(clap::value_parser!(u64)),
-            arg!(-x --exact                "match exactly with the command name"),
-            // arg!(-F --pidfile <file>       "read PIDs from file"),
-            // arg!(-L --logpidfile           "fail if PID file is not locked"),
-            arg!(-r --runstates <state>    "match runstates [D,S,Z,...]"),
-            // arg!(-A --"ignore-ancestors"   "exclude our ancestors from results"),
-            // arg!(--cgroup <grp>            "match by cgroup v2 names")
-            //     .value_delimiter(','),
-            // arg!(--ns <PID>                "match the processes that belong to the same namespace as <pid>"),
-            // arg!(--nslist <ns>             "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,
+        ))
 }


Test patch:
diff --git a/tests/by-util/test_pgrep.rs b/tests/by-util/test_pgrep.rs
index 19d73f0..0f33494 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 801e617..f9686dd 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");
+}