Support unix feature on OpenBSD (utmpx not supported) #5620

lcheylus · 2023-12-05T20:40:16Z

"unix" feature requires utmpx support for pinky, users, uptime and who tools.
utmpx is not supported on OpenBSD => add conditional compilation for these 4 tools to disable it and display "unsupported command on OpenBSD" when called.

Build OK with feature "unix" on OpenBSD-current/amd64 with Rust 1.74

$ cargo build --release --features unix
(...)

$ target/release/coreutils
target/release/coreutils
coreutils 0.0.23 (multi-call binary)

Usage: coreutils [function [arguments...]]

Currently defined functions:

    [, arch, b2sum, b3sum, base32, base64, basename, basenc, cat, chgrp, chmod, chown, chroot,
    cksum, comm, cp, csplit, cut, date, dd, df, dir, dircolors, dirname, du, echo, env, expand,
    expr, factor, false, fmt, fold, groups, hashsum, head, hostid, hostname, id, install, join,
    kill, link, ln, logname, ls, md5sum, mkdir, mkfifo, mknod, mktemp, more, mv, nice, nl,
    nohup, nproc, numfmt, od, paste, pathchk, pinky, pr, printenv, printf, ptx, pwd, readlink,
    realpath, rm, rmdir, seq, sha1sum, sha224sum, sha256sum, sha3-224sum, sha3-256sum, sha3-
    384sum, sha3-512sum, sha384sum, sha3sum, sha512sum, shake128sum, shake256sum, shred, shuf,
    sleep, sort, split, stat, stdbuf, stty, sum, sync, tac, tail, tee, test, timeout, touch,
    tr, true, truncate, tsort, tty, uname, unexpand, uniq, unlink, uptime, users, vdir, wc, who,
    whoami, yes

$ target/release/coreutils uname -a
OpenBSD openbsd-dev.home.lan 7.4 GENERIC.MP#1471 amd64 OpenBSD

$ target/release/coreutils pinky
unsupported
$ target/release/coreutils uptime
unsupported
$ target/release/coreutils users
unsupported
$ target/release/coreutils who
unsupported

No regression on Linux : build OK with Rust v1.74.0 on Debian testing/amd64.
Tests on OpenBSD with feature "unix"

$ cargo test --tests --features unix
(...)
failures:
    test_cat::test_error_loop
    test_chown::test_chown_no_change_to_group
    test_chown::test_chown_no_change_to_user_group
    test_chown::test_chown_only_group
    test_chown::test_chown_only_group_id
    test_chown::test_chown_owner_group
    test_chown::test_chown_owner_group_id
    test_chown::test_chown_owner_group_mix
    test_chown::test_chown_various_input
    test_cp::test_copy_contents_fifo
    test_cp::test_cp_parents_2_dirs
    test_cp::test_cp_preserve_xattr
    test_cp::test_no_preserve_mode
    test_cp::test_preserve_hardlink_attributes_in_directory
    test_cp::test_preserve_mode
    test_du::test_du_basics_subdir
    test_du::test_du_d_flag
    test_du::test_du_dereference
    test_du::test_du_hard_link
    test_du::test_du_no_dereference
    test_du::test_du_no_permission
    test_du::test_du_one_file_system
    test_du::test_du_soft_link
    test_du::test_du_symlink_multiple_fail
    test_du::test_du_threshold
    test_hostname::test_hostname_ip
    test_install::test_install_compare_option
    test_install::test_install_copy_then_compare_file
    test_ls::test_ls_allocation_size
    test_ls::test_ls_human_si
    test_ls::test_ls_long_total_size
    test_pinky::test_long_format
    test_pinky::test_long_format_multiple_users
    test_pinky::test_no_flag
    test_pinky::test_short_format_i
    test_pinky::test_short_format_q
    test_split::test_dev_zero
    test_split::test_filter_broken_pipe
    test_split::test_number_by_bytes_dev_zero
    test_stat::test_format_created_time
    test_stat::test_multi_files
    test_stat::test_normal_format
    test_stat::test_pipe_fifo
    test_stat::test_stdin_pipe_fifo1
    test_stat::test_stdin_pipe_fifo2
    test_stat::test_stdin_redirect
    test_stat::test_symlinks
    test_stdbuf::test_stdbuf_line_buffered_stdout
    test_stdbuf::test_stdbuf_trailing_var_arg
    test_stdbuf::test_stdbuf_unbuffered_stdout
    test_tail::test_follow_descriptor_vs_rename1
    test_tail::test_follow_descriptor_vs_rename2
    test_tail::test_follow_name_move1
    test_tail::test_follow_name_move2
    test_tail::test_follow_name_move_create1
    test_tail::test_follow_name_move_create2
    test_tail::test_follow_name_move_retry1
    test_tail::test_follow_name_move_retry2
    test_tail::test_follow_name_retry_headers
    test_tail::test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same_size
    test_tail::test_retry3
    test_tail::test_retry4
    test_tail::test_retry5
    test_tail::test_retry7
    test_tail::test_retry8
    test_tail::test_stdin_redirect_dir
    test_test::test_file_is_sticky
    test_test::test_file_owned_by_egid
    test_touch::test_touch_changes_time_of_file_in_stdout
    test_touch::test_touch_dash
    test_uptime::test_uptime
    test_uptime::test_uptime_since
    test_who::test_arg1_arg2
    test_who::test_boot
    test_who::test_dead
    test_who::test_login
    test_who::test_m
    test_who::test_process
    test_who::test_runlevel
    test_who::test_time

test result: FAILED. 2716 passed; 80 failed; 40 ignored; 0 measured; 0 filtered out; finished in 231.09s

src/uu/pinky/src/pinky.rs

src/uu/uptime/src/uptime.rs

src/uu/pinky/src/pinky.rs

src/uu/pinky/src/platform/openbsd.rs

github-actions · 2023-12-11T11:33:15Z

GNU testsuite comparison:

GNU test failed: tests/tail/inotify-dir-recreate. tests/tail/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

sylvestre · 2023-12-11T11:43:54Z

Looks great to me. @tertsdiepraam what do you think?

tertsdiepraam

I'm honestly not really sure about this. I don't really like to introduce utils that compile but don't work at all. That's because if someone installs uutils, it usually overrides the standard implementation. If we have a stub for, say, pinky, then the standard pinky gets overridden, so can't be used easily anymore. So I think disabling them in Cargo.toml until we support them makes more sense. I do agree that this is the right folder structure though. I just think we should tackle it one at a time by implementing them.

Why don't we start with a feature flag like this:

openbsd = [
  "feat_Tier1",
  #
  "feat_require_crate_cpp",
  "feat_require_unix",
  "feat_require_unix_hostid",
]

Then we can add the utils as they are implemented (and once all of them are done we set it to openbsd = ["unix"]). Maybe our feature flags in general are overdue for a refactor, too.

Also, I think other.rs should be called unix.rs or utmpx.rs :)

The first four commits are definitely good though!

lcheylus · 2023-12-11T16:34:27Z

Why don't we start with a feature flag like this:
openbsd = [
  "feat_Tier1",
  #
  "feat_require_crate_cpp",
  "feat_require_unix",
  "feat_require_unix_hostid",
]
Then we can add the utils as they are implemented (and once all of them are done we set it to openbsd = ["unix"]). Maybe our feature flags in general are overdue for a refactor, too.

I have already proposed a PR with this solution, @sylvestre rejected it and asked me to implement as in this PR. See PR#5613

My goal is to implement these 4 utils on OpenBSD, without using utmpx.

Also, I think other.rs should be called unix.rs or utmpx.rs :)

OK, rename platform/other.rs => platform/unix.rs for the 4 utils.

tertsdiepraam · 2023-12-11T16:54:16Z

Alright, I talked with @sylvestre and we'll follow his plan! Thanks for working on this!

- add target_os cfg for pline function - add target_os cfg for auditd function (void on OpenBSD)

- See issue uutils#5596 for details

- utmpx not supported on OpenBSD - add src/uu/pinky/src/platform directory and platform/mod.rs for conditional compilation according to target_os - platform/openbsd.rs: implementation on OpenBSD (unsupported tool) - platform/unix.rs: implementation on other OS - src/uu/pinky/src/pinky.rs: use platform module for uucore::main function

- utmpx not supported on OpenBSD - add src/uu/uptime/src/platform directory and platform/mod.rs for conditional compilation according to target_os - platform/openbsd.rs: implementation on OpenBSD (unsupported tool) - platform/unix.rs: implementation on other OS - src/uu/uptime/src/uptime.rs: use platform module for uucore::main function

- utmpx not supported on OpenBSD - add src/uu/users/src/platform directory and platform/mod.rs for conditional compilation according to target_os - platform/openbsd.rs: implementation on OpenBSD (unsupported tool) - platform/unix.rs: implementation on other OS - src/uu/users/src/users.rs: use platform module for uucore::main function

- utmpx not supported on OpenBSD - add src/uu/who/src/platform directory and platform/mod.rs for conditional compilation according to target_os - platform/openbsd.rs: implementation on OpenBSD (unsupported tool) - platform/unix.rs: implementation on other OS - src/uu/who/src/who.rs: use platform module for uucore::main function

lcheylus · 2023-12-16T17:01:53Z

@sylvestre Build and tests OK on OpenBSD/amd64 with your 2 additional commits from c32e730

sylvestre · 2023-12-16T17:15:05Z

Super, merci :)

sylvestre · 2023-12-16T17:15:28Z

Félicitations!

lcheylus mentioned this pull request Dec 5, 2023

Add specific openbsd feature to disable use of utmpx #5613

Closed

sylvestre reviewed Dec 5, 2023

View reviewed changes

src/uu/pinky/src/pinky.rs Outdated Show resolved Hide resolved

sylvestre reviewed Dec 5, 2023

View reviewed changes

src/uu/uptime/src/uptime.rs Show resolved Hide resolved

sylvestre reviewed Dec 5, 2023

View reviewed changes

src/uu/pinky/src/pinky.rs Outdated Show resolved Hide resolved

lcheylus force-pushed the openbsd-utmpx branch from 2106c11 to fccbc8c Compare December 10, 2023 16:22

sylvestre reviewed Dec 10, 2023

View reviewed changes

src/uu/pinky/src/platform/openbsd.rs Outdated Show resolved Hide resolved

lcheylus force-pushed the openbsd-utmpx branch from fccbc8c to 23b7196 Compare December 11, 2023 11:00

sylvestre requested a review from tertsdiepraam December 11, 2023 11:43

tertsdiepraam reviewed Dec 11, 2023

View reviewed changes

lcheylus added 8 commits December 11, 2023 18:23

chroot: add OpenBSD support for set_groups function

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

d239b2f

id: fix support for OpenBSD

94537d3

- add target_os cfg for pline function - add target_os cfg for auditd function (void on OpenBSD)

nohup: fix for OpenBSD with _vprocmgr_detach_from_console function

38ea40d

uucore: disable utmpx feature on OpenBSD

e6d1273

- See issue uutils#5596 for details

lcheylus force-pushed the openbsd-utmpx branch from 23b7196 to e1032e1 Compare December 11, 2023 17:23

sylvestre added 3 commits December 16, 2023 10:29

Merge branch 'main' into openbsd-utmpx

1bfac9b

Apply the change from c32e730 too

6621a1d

Apply change from c32e730

afc6ac8

sylvestre merged commit 35ae43e into uutils:main Dec 16, 2023

lcheylus deleted the openbsd-utmpx branch December 16, 2023 17:44

This was referenced Dec 17, 2023

pinky: support on OpenBSD #5663

Open

uptime: support on OpenBSD #5664

Closed

users: support on OpenBSD #5665

Closed

who: support on OpenBSD #5666

Open

lcheylus mentioned this pull request Jan 19, 2024

tests: disable tests for tools unsupported on OpenBSD #5863

Merged

LionelMeli mentioned this pull request Mar 22, 2025

last: support on OpenBSD uutils/util-linux#267

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support unix feature on OpenBSD (utmpx not supported) #5620

Support unix feature on OpenBSD (utmpx not supported) #5620

lcheylus commented Dec 5, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023

sylvestre commented Dec 11, 2023

tertsdiepraam left a comment •

edited

Loading

lcheylus commented Dec 11, 2023

tertsdiepraam commented Dec 11, 2023

lcheylus commented Dec 16, 2023 •

edited

Loading

sylvestre commented Dec 16, 2023

sylvestre commented Dec 16, 2023

Support unix feature on OpenBSD (utmpx not supported) #5620

Support unix feature on OpenBSD (utmpx not supported) #5620

Conversation

lcheylus commented Dec 5, 2023 • edited Loading

github-actions bot commented Dec 11, 2023

sylvestre commented Dec 11, 2023

tertsdiepraam left a comment • edited Loading

Choose a reason for hiding this comment

lcheylus commented Dec 11, 2023

tertsdiepraam commented Dec 11, 2023

lcheylus commented Dec 16, 2023 • edited Loading

sylvestre commented Dec 16, 2023

sylvestre commented Dec 16, 2023

lcheylus commented Dec 5, 2023 •

edited

Loading

tertsdiepraam left a comment •

edited

Loading

lcheylus commented Dec 16, 2023 •

edited

Loading