perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Add exit code test (GH #19020):

Open xsawyerx opened this issue 4 years ago • 3 comments

This tests that when we kill a process through a shell, we get the correct exit code and signal.

Unfortunately, this fails on some versions of Ubuntu and dash, which I believe shows a bug somewhere. This PR cannot be merged until that is resolved one way (like fixing it in the core) or another (like turning this into a TODO or something). I'm hoping to get some smoke tests from this to provide more information.

There's a sleep() included because some systems might delay the signal until the -e'' execution would already finish.

xsawyerx avatar Sep 23 '21 18:09 xsawyerx

Smoke test results also spot the issue:

# Failed test 2 - $? (35072) from a KILL signal is 9 at op/kill9_shell_exit.t line 18
t/op/kill9_shell_exit ............................................ FAILED at test 2
#      got "35072"
# expected "9"
# Failed test 3 - OS exit code (shifted) is 0 at op/kill9_shell_exit.t line 19
#      got "137"
# expected "0"
# Failed test 4 - KILL signal (bitwise ANDed exit) is 9 at op/kill9_shell_exit.t line 20
#      got "0"
# expected "9"

xsawyerx avatar Sep 23 '21 19:09 xsawyerx

There's been no activity in this p.r. since last September.

@xsawyerx, do you intend to pursue this p.r. further?

jkeenan avatar Jun 27 '22 13:06 jkeenan

I'm not sure what's left to pursue. This ticket was left unattended and the discussion on the issue itself seems to have petered off.

  • There is a bug in Debian's dash (which seems like a purposeful change with unintended consequences)
  • It was also inherited by Ubuntu
  • This bug affects Perl users in some narrow situations
  • That narrow situation includes the perl smokers
  • I suggested a few options - the most (and maybe only) practical of which is to document this (should probably be in perlport and under system or backticks in perlfunc) and move on
  • No additional comments were made on the discussion or on this test PR

This PR includes the test to prove the problem. I would suggest this be added as TODO so it wouldn't break the testing suite but developers would still be notified when this issue is no longer relevant (namely, when the affected dash version is not in circulation as much).

I have no intention of pursuing either this PR or the original ticket I opened on it. If no one wishes to act on it (or resolve the discussion on it), you can close it as "ENOFIX".

xsawyerx avatar Jun 27 '22 17:06 xsawyerx

Anyone want to pick up turning this into a set of TODO tests?

demerphq avatar Feb 08 '23 06:02 demerphq