Skip to content

uucore+timeout: accept signals of any casing #6382

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.

Sign up for GitHub

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

Jump to bottom
Merged
sylvestre merged 2 commits into from
Jul 9, 2024
Merged

uucore+timeout: accept signals of any casing #6382

sylvestre merged 2 commits into from
Jul 9, 2024

Conversation

@BenWiederhake
Copy link
Collaborator

@BenWiederhake BenWiederhake commented May 7, 2024 •
edited
Loading

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

BenWiederhake commented May 7, 2024

Changes since last push:

  • clippy fix (shame on me)

@BenWiederhake
Copy link
Collaborator Author

BenWiederhake commented May 9, 2024

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

BenWiederhake commented Jul 3, 2024

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

BenWiederhake commented Jul 3, 2024

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'

@BenWiederhake BenWiederhake mentioned this pull request Jul 3, 2024
Closed
@sylvestre
Copy link
Contributor

sylvestre commented Jul 9, 2024

sorry for the latency

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

BenWiederhake commented Jul 11, 2024

Thank you <3

@BrewTestBot BrewTestBot mentioned this pull request Nov 16, 2024
Merged
@moonfruit moonfruit mentioned this pull request Nov 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

timeout/uucore: Should accept all casings of signal name

2 participants

@BenWiederhake @sylvestre