From cd23f85ed50cecd93f90385b87438b4e4fadfff2 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 01:29:43 +0000 Subject: [PATCH 01/13] Fixing issues with users that have no active pid --- .github/workflows/CICD.yml | 6 ++++-- Cargo.lock | 2 ++ src/uu/users/Cargo.toml | 1 + src/uu/users/src/users.rs | 17 ++++++++++++++++- src/uu/who/Cargo.toml | 1 + src/uu/who/src/platform/unix.rs | 20 ++++++++++++++++++-- tests/by-util/test_users.rs | 1 - tests/by-util/test_who.rs | 9 --------- 8 files changed, 42 insertions(+), 15 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index f0d2c848220..0614adda035 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -746,7 +746,8 @@ jobs: # system boot entry and a login entry for the GH runner account. FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7") | sudo utmpdump -r -o /var/run/utmp + FAKE_UTMP_LIVE='[7] [1] [tty3] [runner] [tty3] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' + (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7 ; echo "$FAKE_UTMP_LIVE") | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for @@ -1108,7 +1109,8 @@ jobs: # To work around these issues for pinky (and who) tests, we create a fake utmp file with a # system boot entry and a login entry for the GH runner account. FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' - FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' + FAKE_UTMP_DEAD='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' + FAKE_UTMP_LIVE='[7] [1] [tty3] [runner] [tty3] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7") | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd diff --git a/Cargo.lock b/Cargo.lock index 909c0351005..deabd50f6b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4109,6 +4109,7 @@ version = "0.5.0" dependencies = [ "clap", "fluent", + "nix", "utmp-classic", "uucore", ] @@ -4144,6 +4145,7 @@ version = "0.5.0" dependencies = [ "clap", "fluent", + "nix", "uucore", ] diff --git a/src/uu/users/Cargo.toml b/src/uu/users/Cargo.toml index fa311e74ac4..ca0e91c3b29 100644 --- a/src/uu/users/Cargo.toml +++ b/src/uu/users/Cargo.toml @@ -26,6 +26,7 @@ path = "src/users.rs" clap = { workspace = true } uucore = { workspace = true, features = ["utmpx"] } fluent = { workspace = true } +nix = { workspace = true, features = ["signal"] } [target.'cfg(target_os = "openbsd")'.dependencies] utmp-classic = { workspace = true } diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index 3fb48a9b613..483c0511f3d 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -10,6 +10,8 @@ use std::path::Path; use clap::builder::ValueParser; use clap::{Arg, Command}; +use nix::sys::signal::kill; +use nix::unistd::Pid; use uucore::error::UResult; use uucore::format_usage; use uucore::translate; @@ -33,6 +35,19 @@ fn get_long_usage() -> String { translate!("users-long-usage", "default_path" => default_path) } +#[inline] +fn pid_is_alive(pid: i32) -> bool { + if pid <= 0 { + return true; + } + + match kill(Pid::from_raw(pid), None) { + Ok(()) => true, + Err(nix::errno::Errno::ESRCH) => false, + Err(_) => true, + } +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; @@ -66,7 +81,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let filename = maybe_file.unwrap_or(utmpx::DEFAULT_FILE.as_ref()); users = Utmpx::iter_all_records_from(filename) - .filter(|ut| ut.is_user_process()) + .filter(|ut| ut.is_user_process() && pid_is_alive(ut.pid())) .map(|ut| ut.user()) .collect::>(); }; diff --git a/src/uu/who/Cargo.toml b/src/uu/who/Cargo.toml index fdb5dd41dfc..4ca101084ef 100644 --- a/src/uu/who/Cargo.toml +++ b/src/uu/who/Cargo.toml @@ -27,6 +27,7 @@ path = "src/who.rs" clap = { workspace = true } uucore = { workspace = true, features = ["utmpx"] } fluent = { workspace = true } +nix = { workspace = true, features = ["signal"] } [[bin]] name = "who" diff --git a/src/uu/who/src/platform/unix.rs b/src/uu/who/src/platform/unix.rs index 8e72a83ba39..eea13e2534b 100644 --- a/src/uu/who/src/platform/unix.rs +++ b/src/uu/who/src/platform/unix.rs @@ -13,6 +13,9 @@ use uucore::error::{FromIo, UResult}; use uucore::libc::{S_IWGRP, STDIN_FILENO, ttyname}; use uucore::translate; +use nix::sys::signal::kill; +use nix::unistd::Pid; + use uucore::utmpx::{self, UtmpxRecord, time}; use std::borrow::Cow; @@ -177,6 +180,19 @@ fn time_string(ut: &UtmpxRecord) -> String { ut.login_time().format(&time_format).unwrap() } +#[inline] +fn pid_is_alive(pid: i32) -> bool { + if pid <= 0 { + return true; + } + + match kill(Pid::from_raw(pid), None) { + Ok(()) => true, + Err(nix::errno::Errno::ESRCH) => false, + Err(_) => true, + } +} + #[inline] fn current_tty() -> String { unsafe { @@ -210,7 +226,7 @@ impl Who { }; if self.short_list { let users = utmpx::Utmpx::iter_all_records_from(f) - .filter(|ut| ut.is_user_process()) + .filter(|ut| ut.is_user_process() && pid_is_alive(ut.pid())) .map(|ut| ut.user()) .collect::>(); println!("{}", users.join(" ")); @@ -229,7 +245,7 @@ impl Who { for ut in records { if !self.my_line_only || cur_tty == ut.tty_device() { - if self.need_users && ut.is_user_process() { + if self.need_users && ut.is_user_process() && pid_is_alive(ut.pid()) { self.print_user(&ut)?; } else { match ut.record_type() { diff --git a/tests/by-util/test_users.rs b/tests/by-util/test_users.rs index 0d3d7772b83..97bb49242eb 100644 --- a/tests/by-util/test_users.rs +++ b/tests/by-util/test_users.rs @@ -18,7 +18,6 @@ fn test_users_no_arg() { #[test] #[cfg(any(target_vendor = "apple", target_os = "linux"))] -#[ignore = "issue #3219"] fn test_users_check_name() { #[cfg(target_os = "linux")] let util_name = util_name!(); diff --git a/tests/by-util/test_who.rs b/tests/by-util/test_who.rs index d94a7b99a8f..9c7184323df 100644 --- a/tests/by-util/test_who.rs +++ b/tests/by-util/test_who.rs @@ -16,7 +16,6 @@ fn test_invalid_arg() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_count() { let ts = TestScenario::new(util_name!()); for opt in ["-q", "--count", "--c"] { @@ -42,7 +41,6 @@ fn test_boot() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_heading() { let ts = TestScenario::new(util_name!()); for opt in ["-H", "--heading", "--head"] { @@ -61,7 +59,6 @@ fn test_heading() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_short() { let ts = TestScenario::new(util_name!()); for opt in ["-s", "--short", "--s"] { @@ -128,7 +125,6 @@ fn test_time() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_mesg() { // -T, -w, --mesg // add user's message status as +, - or ? @@ -172,7 +168,6 @@ fn test_too_many_args() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_users() { let ts = TestScenario::new(util_name!()); for opt in ["-u", "--users", "--us"] { @@ -198,7 +193,6 @@ fn test_users() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_lookup() { let opt = "--lookup"; let ts = TestScenario::new(util_name!()); @@ -219,7 +213,6 @@ fn test_dead() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_all_separately() { if cfg!(target_os = "macos") { // TODO: fix `-u`, see: test_users @@ -237,7 +230,6 @@ fn test_all_separately() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_all() { if cfg!(target_os = "macos") { // TODO: fix `-u`, see: test_users @@ -253,7 +245,6 @@ fn test_all() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_locale() { let ts = TestScenario::new(util_name!()); From 968c4f022cc88f9e088d1de9b9ddfb08d9c57fe0 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 01:35:48 +0000 Subject: [PATCH 02/13] Fixing spell checker TODO's and missing quote in the CICD file --- .github/workflows/CICD.yml | 2 +- src/uu/who/src/platform/unix.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 0614adda035..98a79c4bffd 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -747,7 +747,7 @@ jobs: FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' FAKE_UTMP_LIVE='[7] [1] [tty3] [runner] [tty3] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7 ; echo "$FAKE_UTMP_LIVE") | sudo utmpdump -r -o /var/run/utmp + (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7" ; echo "$FAKE_UTMP_LIVE") | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for diff --git a/src/uu/who/src/platform/unix.rs b/src/uu/who/src/platform/unix.rs index eea13e2534b..1bae21b4ab9 100644 --- a/src/uu/who/src/platform/unix.rs +++ b/src/uu/who/src/platform/unix.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) ttyname hostnames runlevel mesg wtmp statted boottime deadprocs initspawn clockchange curr pidstr exitstr hoststr +// spell-checker:ignore (ToDO) ttyname hostnames runlevel mesg wtmp statted boottime deadprocs initspawn clockchange curr pidstr exitstr hoststr ESRCH use crate::options; use crate::uu_app; From 45c629934b5db87d1bdf08c97f3f97129fe3cb8d Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 01:39:27 +0000 Subject: [PATCH 03/13] One more missing spellcheck entry --- src/uu/users/src/users.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index 483c0511f3d..c5073f7c9a7 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (paths) wtmp +// spell-checker:ignore (paths) wtmp ESRCH use std::ffi::OsString; use std::path::Path; From a5fcb513a2d6567b7fbac8aa167d47ef2252752e Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 01:53:20 +0000 Subject: [PATCH 04/13] Skipping GNU version check test temporarily and fixing other typo in CICD file --- .github/workflows/CICD.yml | 4 ++-- tests/by-util/test_who.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 98a79c4bffd..a5f65e42b69 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -1109,9 +1109,9 @@ jobs: # To work around these issues for pinky (and who) tests, we create a fake utmp file with a # system boot entry and a login entry for the GH runner account. FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' - FAKE_UTMP_DEAD='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' + FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' FAKE_UTMP_LIVE='[7] [1] [tty3] [runner] [tty3] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7") | sudo utmpdump -r -o /var/run/utmp + (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7"; echo "$FAKE_UTMP_LIVE") | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for diff --git a/tests/by-util/test_who.rs b/tests/by-util/test_who.rs index 9c7184323df..4fd9a5996d3 100644 --- a/tests/by-util/test_who.rs +++ b/tests/by-util/test_who.rs @@ -245,6 +245,7 @@ fn test_all() { #[cfg(unix)] #[test] +#[ignore = "This test is the only one in the suite that checks if the reference version is 8.3. This has not been the case for a while"] fn test_locale() { let ts = TestScenario::new(util_name!()); From cb842f068c958b845a0371b56ff28e027d38d5af Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 19:45:11 +0000 Subject: [PATCH 05/13] Fixing the format of the utmp file --- .github/workflows/CICD.yml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index a5f65e42b69..fd9f6ac181e 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -744,10 +744,11 @@ jobs: # The account also has empty gecos fields. # To work around these issues for pinky (and who) tests, we create a fake utmp file with a # system boot entry and a login entry for the GH runner account. - FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' - FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - FAKE_UTMP_LIVE='[7] [1] [tty3] [runner] [tty3] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7" ; echo "$FAKE_UTMP_LIVE") | sudo utmpdump -r -o /var/run/utmp + printf '%s\n%s\n%s' \ + '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ + '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for @@ -1108,10 +1109,11 @@ jobs: # The account also has empty gecos fields. # To work around these issues for pinky (and who) tests, we create a fake utmp file with a # system boot entry and a login entry for the GH runner account. - FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' - FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - FAKE_UTMP_LIVE='[7] [1] [tty3] [runner] [tty3] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7"; echo "$FAKE_UTMP_LIVE") | sudo utmpdump -r -o /var/run/utmp + printf '%s\n%s\n%s' \ + '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ + '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for From f697a4f00c201122a97f0f2d87214ac82b1272c7 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 20:16:02 +0000 Subject: [PATCH 06/13] Using the expected result command from the utils library to match other tests --- tests/by-util/test_users.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/by-util/test_users.rs b/tests/by-util/test_users.rs index 97bb49242eb..ab586029f8a 100644 --- a/tests/by-util/test_users.rs +++ b/tests/by-util/test_users.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. use uutests::new_ucmd; #[cfg(any(target_vendor = "apple", target_os = "linux"))] -use uutests::{util::TestScenario, util_name}; +use uutests::{unwrap_or_return, util::TestScenario, util::expected_result, util_name}; #[test] fn test_invalid_arg() { @@ -19,18 +19,9 @@ fn test_users_no_arg() { #[test] #[cfg(any(target_vendor = "apple", target_os = "linux"))] fn test_users_check_name() { - #[cfg(target_os = "linux")] - let util_name = util_name!(); - #[cfg(target_vendor = "apple")] - let util_name = &format!("g{}", util_name!()); - - let expected = TestScenario::new(util_name) - .cmd(util_name) - .env("LC_ALL", "C") - .succeeds() - .stdout_move_str(); - - new_ucmd!().succeeds().stdout_is(&expected); + let ts = TestScenario::new(util_name!()); + let expected_stdout = unwrap_or_return!(expected_result(&ts, &[])).stdout_move_str(); + ts.ucmd().succeeds().stdout_is(expected_stdout); } #[test] From 14f25784d1da7c998b491d14711018815dfa0900 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 21:04:45 +0000 Subject: [PATCH 07/13] Fixing linting issue and that the user singular does not match GNU implementation --- src/uu/users/src/users.rs | 1 + src/uu/who/locales/en-US.ftl | 5 +---- src/uu/who/locales/fr-FR.ftl | 5 +---- src/uu/who/src/platform/unix.rs | 1 + 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index c5073f7c9a7..a00e78d02d3 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -36,6 +36,7 @@ fn get_long_usage() -> String { } #[inline] +#[cfg(not(target_os = "openbsd"))] fn pid_is_alive(pid: i32) -> bool { if pid <= 0 { return true; diff --git a/src/uu/who/locales/en-US.ftl b/src/uu/who/locales/en-US.ftl index f092489a16a..aa939de3ebe 100644 --- a/src/uu/who/locales/en-US.ftl +++ b/src/uu/who/locales/en-US.ftl @@ -26,10 +26,7 @@ who-help-users = list users logged in who-help-mesg = add user's message status as +, - or ? # Output messages -who-user-count = # { $count -> - [one] user={ $count } - *[other] users={ $count } -} +who-user-count = # users={ $count } # Idle time indicators who-idle-current = . diff --git a/src/uu/who/locales/fr-FR.ftl b/src/uu/who/locales/fr-FR.ftl index ca88a4946c1..92b70806995 100644 --- a/src/uu/who/locales/fr-FR.ftl +++ b/src/uu/who/locales/fr-FR.ftl @@ -25,10 +25,7 @@ who-help-users = liste les utilisateurs connectés who-help-mesg = ajoute le statut de message de l'utilisateur comme +, - ou ? # Output messages -who-user-count = # { $count -> - [one] utilisateur={ $count } - *[other] utilisateurs={ $count } -} +who-user-count = # utilisateurs={ $count } # Idle time indicators who-idle-old = anc. diff --git a/src/uu/who/src/platform/unix.rs b/src/uu/who/src/platform/unix.rs index 1bae21b4ab9..5868d681a61 100644 --- a/src/uu/who/src/platform/unix.rs +++ b/src/uu/who/src/platform/unix.rs @@ -181,6 +181,7 @@ fn time_string(ut: &UtmpxRecord) -> String { } #[inline] +#[cfg(not(target_os = "openbsd"))] fn pid_is_alive(pid: i32) -> bool { if pid <= 0 { return true; From add4d65f41c3367ccceb48592eb974054a3b8f00 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sat, 22 Nov 2025 21:27:13 +0000 Subject: [PATCH 08/13] Fixing last platform clippy specific build issue --- src/uu/users/src/users.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index a00e78d02d3..f39040cb7ce 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -10,7 +10,9 @@ use std::path::Path; use clap::builder::ValueParser; use clap::{Arg, Command}; +#[cfg(not(target_os = "openbsd"))] use nix::sys::signal::kill; +#[cfg(not(target_os = "openbsd"))] use nix::unistd::Pid; use uucore::error::UResult; use uucore::format_usage; From 5b1886c5687c5ac64ac22bfa612b3798dcd94cbb Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Fri, 28 Nov 2025 21:01:47 +0000 Subject: [PATCH 09/13] Addressing feedback about inlining documentation and use of nix --- Cargo.lock | 2 -- src/uu/users/Cargo.toml | 3 +-- src/uu/users/src/users.rs | 20 ++------------------ src/uu/who/Cargo.toml | 3 +-- src/uu/who/src/platform/unix.rs | 19 ++----------------- src/uucore/src/lib/features/process.rs | 25 +++++++++++++++++++++++++ 6 files changed, 31 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index deabd50f6b1..909c0351005 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4109,7 +4109,6 @@ version = "0.5.0" dependencies = [ "clap", "fluent", - "nix", "utmp-classic", "uucore", ] @@ -4145,7 +4144,6 @@ version = "0.5.0" dependencies = [ "clap", "fluent", - "nix", "uucore", ] diff --git a/src/uu/users/Cargo.toml b/src/uu/users/Cargo.toml index ca0e91c3b29..c86f0146c13 100644 --- a/src/uu/users/Cargo.toml +++ b/src/uu/users/Cargo.toml @@ -24,9 +24,8 @@ path = "src/users.rs" [dependencies] clap = { workspace = true } -uucore = { workspace = true, features = ["utmpx"] } +uucore = { workspace = true, features = ["utmpx", "process"] } fluent = { workspace = true } -nix = { workspace = true, features = ["signal"] } [target.'cfg(target_os = "openbsd")'.dependencies] utmp-classic = { workspace = true } diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index f39040cb7ce..79343d254ed 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -10,12 +10,10 @@ use std::path::Path; use clap::builder::ValueParser; use clap::{Arg, Command}; -#[cfg(not(target_os = "openbsd"))] -use nix::sys::signal::kill; -#[cfg(not(target_os = "openbsd"))] -use nix::unistd::Pid; use uucore::error::UResult; use uucore::format_usage; +#[cfg(not(target_os = "openbsd"))] +use uucore::process::pid_is_alive; use uucore::translate; #[cfg(target_os = "openbsd")] @@ -37,20 +35,6 @@ fn get_long_usage() -> String { translate!("users-long-usage", "default_path" => default_path) } -#[inline] -#[cfg(not(target_os = "openbsd"))] -fn pid_is_alive(pid: i32) -> bool { - if pid <= 0 { - return true; - } - - match kill(Pid::from_raw(pid), None) { - Ok(()) => true, - Err(nix::errno::Errno::ESRCH) => false, - Err(_) => true, - } -} - #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; diff --git a/src/uu/who/Cargo.toml b/src/uu/who/Cargo.toml index 4ca101084ef..8a8ac22bde7 100644 --- a/src/uu/who/Cargo.toml +++ b/src/uu/who/Cargo.toml @@ -25,9 +25,8 @@ path = "src/who.rs" [dependencies] clap = { workspace = true } -uucore = { workspace = true, features = ["utmpx"] } +uucore = { workspace = true, features = ["utmpx", "process"] } fluent = { workspace = true } -nix = { workspace = true, features = ["signal"] } [[bin]] name = "who" diff --git a/src/uu/who/src/platform/unix.rs b/src/uu/who/src/platform/unix.rs index 5868d681a61..1df818a91dd 100644 --- a/src/uu/who/src/platform/unix.rs +++ b/src/uu/who/src/platform/unix.rs @@ -11,11 +11,10 @@ use crate::uu_app; use uucore::display::Quotable; use uucore::error::{FromIo, UResult}; use uucore::libc::{S_IWGRP, STDIN_FILENO, ttyname}; +#[cfg(not(target_os = "openbsd"))] +use uucore::process::pid_is_alive; use uucore::translate; -use nix::sys::signal::kill; -use nix::unistd::Pid; - use uucore::utmpx::{self, UtmpxRecord, time}; use std::borrow::Cow; @@ -180,20 +179,6 @@ fn time_string(ut: &UtmpxRecord) -> String { ut.login_time().format(&time_format).unwrap() } -#[inline] -#[cfg(not(target_os = "openbsd"))] -fn pid_is_alive(pid: i32) -> bool { - if pid <= 0 { - return true; - } - - match kill(Pid::from_raw(pid), None) { - Ok(()) => true, - Err(nix::errno::Errno::ESRCH) => false, - Err(_) => true, - } -} - #[inline] fn current_tty() -> String { unsafe { diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 55e8c36482b..02d23231fb1 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -51,6 +51,31 @@ pub fn getpid() -> pid_t { unsafe { libc::getpid() } } +/// Check if a process with the given PID is alive. +/// +/// Uses `kill(pid, 0)` which sends signal 0 (null signal) to check process existence +/// without actually sending a signal. This is a standard POSIX technique for checking +/// if a process exists. +/// +/// Returns `true` if: +/// - The process exists (kill returns 0) +/// - We lack permission to signal the process (errno != ESRCH) +/// This means the process exists but we can't signal it +/// +/// Returns `false` only if: +/// - errno is ESRCH (No such process), confirming the process doesn't exist +/// +/// PIDs <= 0 are considered alive for compatibility with utmp records that may +/// contain special or invalid PID values. +#[cfg(not(target_os = "openbsd"))] +pub fn pid_is_alive(pid: i32) -> bool { + if pid <= 0 { + return true; + } + + unsafe { libc::kill(pid, 0) == 0 || *libc::__errno_location() != libc::ESRCH } +} + /// `getsid()` returns the session ID of the process with process ID pid. /// /// If pid is 0, getsid() returns the session ID of the calling process. From 3c87bc07f5f63db3e6a3199ebf9756cd5e77b856 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Fri, 28 Nov 2025 21:24:51 +0000 Subject: [PATCH 10/13] Multi platform mechanism for getting error message from libc --- src/uucore/src/lib/features/process.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 02d23231fb1..1019dfc5ccf 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -72,8 +72,7 @@ pub fn pid_is_alive(pid: i32) -> bool { if pid <= 0 { return true; } - - unsafe { libc::kill(pid, 0) == 0 || *libc::__errno_location() != libc::ESRCH } + unsafe { libc::kill(pid, 0) == 0 || Errno::last() != Errno::ESRCH } } /// `getsid()` returns the session ID of the process with process ID pid. From 5a4c6b1151a4ef942572590c1531ca0dc004a8c5 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sun, 7 Dec 2025 18:33:31 +0000 Subject: [PATCH 11/13] Removing the unsafe libc call and replacing with nix --- src/uucore/src/lib/features/process.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 1019dfc5ccf..1c83759e112 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -10,6 +10,10 @@ use libc::{gid_t, pid_t, uid_t}; #[cfg(not(target_os = "redox"))] use nix::errno::Errno; +#[cfg(not(target_os = "openbsd"))] +use nix::sys::signal::kill; +#[cfg(not(target_os = "openbsd"))] +use nix::unistd::Pid; use std::io; use std::process::Child; use std::process::ExitStatus; @@ -72,7 +76,7 @@ pub fn pid_is_alive(pid: i32) -> bool { if pid <= 0 { return true; } - unsafe { libc::kill(pid, 0) == 0 || Errno::last() != Errno::ESRCH } + kill(Pid::from_raw(pid), None).is_ok() || Errno::last() != Errno::ESRCH } /// `getsid()` returns the session ID of the process with process ID pid. From f225186cc600849c935022f88ebeedb64e7511f6 Mon Sep 17 00:00:00 2001 From: Chris Dryden Date: Sun, 7 Dec 2025 14:14:21 -0500 Subject: [PATCH 12/13] Remove unused imports for specific OS targets --- src/uucore/src/lib/features/process.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 1c83759e112..7ad98ba3057 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -8,7 +8,6 @@ // spell-checker:ignore pgrep pwait snice getpgrp use libc::{gid_t, pid_t, uid_t}; -#[cfg(not(target_os = "redox"))] use nix::errno::Errno; #[cfg(not(target_os = "openbsd"))] use nix::sys::signal::kill; From 9713e98d9f414c942afd807b1ea08fdf7773bb2a Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Fri, 26 Dec 2025 01:03:07 +0000 Subject: [PATCH 13/13] Address PR review comments: add OpenBSD comments and deduplicate utmp setup --- .github/workflows/CICD.yml | 38 ++------------------------------- src/uu/users/src/users.rs | 5 +++-- src/uu/who/src/platform/unix.rs | 1 - util/setup-gh-runner-utmp.sh | 20 +++++++++++++++++ 4 files changed, 25 insertions(+), 39 deletions(-) create mode 100755 util/setup-gh-runner-utmp.sh diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index fd9f6ac181e..f5ed11a283d 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -739,24 +739,7 @@ jobs: # selinux and systemd headers needed to build tests sudo apt-get -y update sudo apt-get -y install libselinux1-dev libsystemd-dev - # pinky is a tool to show logged-in users from utmp, and gecos fields from /etc/passwd. - # In GitHub Action *nix VMs, no accounts log in, even the "runner" account that runs the commands, and "system boot" entry is missing. - # The account also has empty gecos fields. - # To work around these issues for pinky (and who) tests, we create a fake utmp file with a - # system boot entry and a login entry for the GH runner account. - printf '%s\n%s\n%s' \ - '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ - '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ - '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ - | sudo utmpdump -r -o /var/run/utmp - # ... and add a full name to each account with a gecos field but no full name. - sudo sed -i 's/:,/:runner name,/' /etc/passwd - # We also create a couple optional files pinky looks for - touch /home/runner/.project - echo "foo" > /home/runner/.plan - # add user with digital username for testing with issue #7787 - echo 200:x:2000:2000::/home/200:/bin/bash | sudo tee -a /etc/passwd - echo 200:x:2000: | sudo tee -a /etc/group + bash util/setup-gh-runner-utmp.sh ;; esac - uses: taiki-e/install-action@v2 @@ -1104,24 +1087,7 @@ jobs: # selinux and systemd headers needed to build tests sudo apt-get -y update sudo apt-get -y install libselinux1-dev libsystemd-dev - # pinky is a tool to show logged-in users from utmp, and gecos fields from /etc/passwd. - # In GitHub Action *nix VMs, no accounts log in, even the "runner" account that runs the commands, and "system boot" entry is missing. - # The account also has empty gecos fields. - # To work around these issues for pinky (and who) tests, we create a fake utmp file with a - # system boot entry and a login entry for the GH runner account. - printf '%s\n%s\n%s' \ - '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ - '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ - '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ - | sudo utmpdump -r -o /var/run/utmp - # ... and add a full name to each account with a gecos field but no full name. - sudo sed -i 's/:,/:runner name,/' /etc/passwd - # We also create a couple optional files pinky looks for - touch /home/runner/.project - echo "foo" > /home/runner/.plan - # add user with digital username for testing with issue #7787 - echo 200:x:2000:2000::/home/200:/bin/bash | sudo tee -a /etc/passwd - echo 200:x:2000: | sudo tee -a /etc/group + bash util/setup-gh-runner-utmp.sh ;; esac diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index 79343d254ed..749bf5e22f3 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -12,8 +12,6 @@ use clap::builder::ValueParser; use clap::{Arg, Command}; use uucore::error::UResult; use uucore::format_usage; -#[cfg(not(target_os = "openbsd"))] -use uucore::process::pid_is_alive; use uucore::translate; #[cfg(target_os = "openbsd")] @@ -63,8 +61,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } }; + // OpenBSD uses the older UTMP format (not UTMPX) which doesn't reliably track PIDs, + // so we only filter by pid_is_alive on non-OpenBSD systems. #[cfg(not(target_os = "openbsd"))] { + use uucore::process::pid_is_alive; let filename = maybe_file.unwrap_or(utmpx::DEFAULT_FILE.as_ref()); users = Utmpx::iter_all_records_from(filename) diff --git a/src/uu/who/src/platform/unix.rs b/src/uu/who/src/platform/unix.rs index 1df818a91dd..d490381c204 100644 --- a/src/uu/who/src/platform/unix.rs +++ b/src/uu/who/src/platform/unix.rs @@ -11,7 +11,6 @@ use crate::uu_app; use uucore::display::Quotable; use uucore::error::{FromIo, UResult}; use uucore::libc::{S_IWGRP, STDIN_FILENO, ttyname}; -#[cfg(not(target_os = "openbsd"))] use uucore::process::pid_is_alive; use uucore::translate; diff --git a/util/setup-gh-runner-utmp.sh b/util/setup-gh-runner-utmp.sh new file mode 100755 index 00000000000..9a7923a4eea --- /dev/null +++ b/util/setup-gh-runner-utmp.sh @@ -0,0 +1,20 @@ +#!/bin/bash +# spell-checker:ignore utmpdump gecos +# pinky is a tool to show logged-in users from utmp, and gecos fields from /etc/passwd. +# In GitHub Action *nix VMs, no accounts log in, even the "runner" account that runs the commands, and "system boot" entry is missing. +# The account also has empty gecos fields. +# To work around these issues for pinky (and who) tests, we create a fake utmp file with a +# system boot entry and a login entry for the GH runner account. +printf '%s\n%s\n%s' \ + '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ + '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + | sudo utmpdump -r -o /var/run/utmp +# ... and add a full name to each account with a gecos field but no full name. +sudo sed -i 's/:,/:runner name,/' /etc/passwd +# We also create a couple optional files pinky looks for +touch /home/runner/.project +echo "foo" > /home/runner/.plan +# add user with digital username for testing with issue #7787 +echo 200:x:2000:2000::/home/200:/bin/bash | sudo tee -a /etc/passwd +echo 200:x:2000: | sudo tee -a /etc/group