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

Exit script/build immediately upon error #1108

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Conversation

razzius
Copy link
Contributor

@razzius razzius commented Aug 28, 2024

Without this change, errors in the build process do not cancel the build, leading to additional errors that can obscure the cause of the build failure.

When I attempted to build planck locally lacking the jar command, the following happened:

planck $ ./script/build
Creating an optimized build, using build cache to speed up build.
Specifying --fast skips optimizations and takes less than two minutes.
Fetching Google Closure Compiler...
Fetching Google Closure Library...
script/get-closure-library: line 17: jar: command not found
script/get-closure-library: line 23: jar: command not found
script/get-closure-library: line 37: cd: planck-cljs/lib/closure/goog: No such file or directory
script/get-closure-library: line 64: cd: planck-cljs/lib/third_party/closure/goog: No such file or directory
find: ‘./var/spool/cron/crontabs’: Permission denied
find: ‘./var/spool/cups’: Permission denied
find: ‘./var/spool/rsyslog’: Permission denied
find: ‘./var/spool/postfix/bounce’: Permission denied
...

It appears that after jar was missing, a cd command didn't work and it took me to the root directory, where further commands attempted to read unrelated directories.

With this change, the build errors immediately once jar fails:

planck $ ./script/build
Creating an optimized build, using build cache to speed up build.
Specifying --fast skips optimizations and takes less than two minutes.
Fetching Google Closure Compiler...
Fetching Google Closure Library...
script/get-closure-library: line 19: jar: command not found
planck $

This seems like preferable behavior to me, but it might lead to issues if other commands depended on the build script to continue through some types of errors. I'm open to alternative approaches to set -e.

It's also worth considering whether such a setting should be in every bash script, if it's considered an improvement.

Without this change, errors in the build process do not
cancel the build, leading to additional errors that can obscure the
cause of the build failure.
@mfikes
Copy link
Member

mfikes commented Aug 28, 2024

I think the unit tests are failing simply because I had recently shut down an HTTP echo server that the HTTP tests depended on.

@mfikes
Copy link
Member

mfikes commented Aug 28, 2024

Re-started the echo HTTP server and re-ran unit tests, which are passing now.

@mfikes mfikes merged commit 116c5eb into planck-repl:master Aug 28, 2024
2 checks passed
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.

2 participants