Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uucore+timeout: accept signals of any casing #6382

Merged
merged 2 commits into from
Jul 9, 2024

Conversation

BenWiederhake
Copy link
Collaborator

@BenWiederhake BenWiederhake commented May 7, 2024

This makes #6217 available to timeout and all other utils that try to parse signal names in the future (at least env: #6377).

Fixes #6381.

I don't have GNU kill available to check, but I think this fixes another issue in kill that we didn't even notice: signal names in the obsolete kill invocation-format:

$ sleep 999 &
[3] 419718
$ /usr/bin/kill -tErM 419718  # procps-ng 4.0.4
[3]+  Terminated              sleep 999
  • If this is indeed what GNU does, I need to add a test to make sure we don't regress
  • Otherwise, I need to modify this PR to split signal parsing in two steps.

@BenWiederhake BenWiederhake marked this pull request as draft May 7, 2024 19:40
@BenWiederhake BenWiederhake marked this pull request as ready for review May 7, 2024 20:23
@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • clippy fix (shame on me)

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • Rebased on current main
  • Changed the conceptual definition of UResult::signal_name_is to allow signal names of any casing, too. The only change in the code is to change the tests, going from "expect panic" to "expect success". Note that it already links to signal_by_name_or_value.

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • Rebased on current main
  • Nothing else, I just want to re-run CI, because there were many broken tests last month.

@BenWiederhake
Copy link
Collaborator Author

CI failures seem unrelated:

  • Android can't log into something(?)
  • Windows can't install crate profiler_builtins(?)
  • MacOS has an unrelated error:
--- TRY 3 STDERR:        coreutils::tests test_uptime::test_uptime_with_fifo ---
thread 'main' panicked at tests/by-util/test_uptime.rs:77:10:
'uptime: couldn't get boot time
' does not contain 'uptime: couldn't get boot time: Illegal seek'

@sylvestre
Copy link
Sponsor Contributor

sorry for the latency

@sylvestre sylvestre merged commit c90b693 into uutils:main Jul 9, 2024
63 of 68 checks passed
@BenWiederhake BenWiederhake deleted the dev-signal-casing branch July 11, 2024 08:51
@BenWiederhake
Copy link
Collaborator Author

Thank you <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

timeout/uucore: Should accept all casings of signal name
2 participants