stevieb9 / berrybrew

Perlbrew for Windows!
Other
63 stars 9 forks source link

berrybrew exec (and other commands) do not exit non-0 on failure #236

Closed autarch closed 3 years ago

autarch commented 4 years ago

Admittedly, I have no idea how this works on Windows. But when running berrybrew exec and other commands in a bash shell on Windows there doesn't seem to be any indication that the command failed.

Here's an example of a command that runs successfully - https://dev.azure.com/houseabsolute/houseabsolute/_build/results?buildId=655&view=logs&j=8896d41c-d541-582c-8ad8-5ca2f02e1047&t=f3bbe9bc-5393-537c-dee6-0162e2f95db4&l=20

And here's the same command failing - https://dev.azure.com/houseabsolute/houseabsolute/_build/results?buildId=658&view=logs&j=8896d41c-d541-582c-8ad8-5ca2f02e1047&t=f3bbe9bc-5393-537c-dee6-0162e2f95db4&l=19

Note that there's no output for any of the commands in the second link. The code that executes these processes is using autodie qw( :all ) and/or IPC::Run3 and checking $?. But it's obviously not seeing any sort of error status from these commands.

I also noticed a similar issue when running berrybrew install in a bash shell on windows where I'd set -e in the same script. The install would fail but the script would keep running.

I can't tell if this is a Windows thing or something specific to berrybrew. I also wonder if perhaps berrybrew is hiding some of the output from things executed via berrybrew exec. In the failure cases I would've expected to see some sort of output.

stevieb9 commented 4 years ago

Admittedly, I don't even use Windows, except for pretty much developing berrybrew, so I'm quite unfamiliar with the Windows bash shell. In fact, I've never used it.

Is what you're seeing an issue just with the most recent version (1.30)?

I'll load up a machine with bash shell here in the coming days and see what I can come up with.

autarch commented 4 years ago

No, it's been like this for other versions too. I think something changed in 1.30 that might be causing the new breakage I'm seeing, but it's tricky to tell what when the failure is so obscured.

stevieb9 commented 4 years ago

Thanks. I'll get to this as quickly and possible and keep you up to date with my findings. The next release is dedicated solely to significantly beefing up and expanding testing, so I'm glad you brought this up.

stevieb9 commented 4 years ago

Ok, so reviewing the code, I definitely need to add different exit statuses when things go awry.

When I do so (for example, exit with -1), run3 will die with the error message that was written to the console. If I eval the run3 call, I get what appears to be a reasonable, non-zero exit status in $?, and the error message in the $err variable passed into the call.

Is this the gist of what you're expecting to happen?

autarch commented 4 years ago

Yes, exactly!

stevieb9 commented 4 years ago

Hey Dave,

If I can cut you a release, would you be willing to throw it into a test to ensure it does the right thing? I'm behind the 8-ball a bit here, but want to ensure I'm on the right track.

autarch commented 4 years ago

Sorry for the slow response. Yes, I can do this either this weekend or more likely the next.

stevieb9 commented 4 years ago

Thank you. I may put out a release by then, but I'm not certain. Please do keep me posted.

On Wed, Jan 29, 2020 at 11:40 AM Dave Rolsky notifications@github.com wrote:

Sorry for the slow response. Yes, I can do this either this weekend or more likely the next.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stevieb9/berrybrew/issues/236?email_source=notifications&email_token=ADDQEP4VRFRHEHBUPX43KALRAHLSPA5CNFSM4J7HCWL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKIPMNI#issuecomment-579925557, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDQEP634T727VFYDK2VIYDRAHLSPANCNFSM4J7HCWLQ .

stevieb9 commented 3 years ago

Fixed in v1.31, released on Jan 17, 2021