search

SomeRandomiOSDev / CBORCoding

Easy CBOR encoding and decoding for iOS, macOS, tvOS and watchOS.
MIT License
51 stars 14 forks source link

Ensure that scripts don't swallow errors either #26

Open dabrahams opened 1 year ago

dabrahams commented 1 year ago

As I was attempting to do the same thing for Half I realized that there are a number of places where you use checkresult to attempt to give better diagnostics when there's a failure. This PR will undermine that checking. However, those checkresult $? calls often come after a bunch of steps and $? will only be nonzero if the last of those steps happens to fail. I'm not entirely clear on the best way to proceed here; I never try to write anything more than a few lines using shell scripts because among other things the error-handling model is so screwed up. If it were my project, I'd rewrite these scripts in Swift, have every failing command throw an Error, encode the details of the failure in the error, catch and report all errors in one place, and do some logging to the terminal before each section of work to give me the additional context I need to know what it was doing when the failure occurred.

dabrahams commented 1 year ago

An initial compromise step might be to accept this PR without the changes to workflowtests.sh and let you work out the approach for that file. Please advise.

Update: the latest commit reverts the changes to workflowtests.sh.

SomeRandomiOSDev commented 1 year ago

@dabrahams Good callouts. I hesitate to re-write these scripts in Swift just given how much of a pain it is to invoke terminal commands from Swift. As you pointed out, however, the error model for bash scripts is pretty terrible, so I guess it's a matter of picking a poison. I'll take a deeper look at these scripts overall but for the time being I'd like to keep them as is

dabrahams commented 1 year ago

@dabrahams Good callouts. I hesitate to re-write these scripts in Swift just given how much of a pain it is to invoke terminal commands from Swift.

The principled alternative is to use -e -o pipefail everywhere, and give up on the pretty error messages you get with checkresult. Other ways to know what failed: set +x (ugly but effective) or just echo something about what each step does to the console before each step, much the same way your use of checkresult does so after a failure.

@SomeRandomiOSDev how does that approach sound to you?