Support unix feature on OpenBSD (utmpx not supported) by lcheylus · Pull Request #5620 · uutils/coreutils

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

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

Uh oh!

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

Support unix feature on OpenBSD (utmpx not supported)#5620
sylvestre merged 11 commits intouutils:mainfrom
lcheylus:openbsd-utmpx

lcheylus commented Dec 5, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 11, 2023

Uh oh!

sylvestre commented Dec 11, 2023

Uh oh!

tertsdiepraam left a comment •

edited

Loading

Uh oh!

lcheylus commented Dec 11, 2023

Uh oh!

tertsdiepraam commented Dec 11, 2023

Uh oh!

lcheylus commented Dec 16, 2023 •

edited

Loading

Uh oh!

sylvestre commented Dec 16, 2023

Uh oh!

sylvestre commented Dec 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

lcheylus commented Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 11, 2023

Uh oh!

sylvestre commented Dec 11, 2023

Uh oh!

tertsdiepraam left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcheylus commented Dec 11, 2023

Uh oh!

tertsdiepraam commented Dec 11, 2023

Uh oh!

lcheylus commented Dec 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sylvestre commented Dec 16, 2023

Uh oh!

sylvestre commented Dec 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lcheylus commented Dec 5, 2023 •

edited

Loading

tertsdiepraam left a comment •

edited

Loading

lcheylus commented Dec 16, 2023 •

edited

Loading