-
-
Notifications
You must be signed in to change notification settings - Fork 977
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 with underlying exit code #98
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
@@ -32,7 +34,13 @@ func checkForErrorsAndExit(err error) { | |||
} else { | |||
logger.Println(err) | |||
} | |||
os.Exit(1) | |||
// exit with the underlying error code | |||
var retCode int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should default this to 1. Otherwise, if the error is not an ExitError
, this will exit with a 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, of course! Done.
// exit with the underlying error code | ||
var retCode int | ||
if exiterr, ok := errors.Unwrap(err).(*exec.ExitError); ok { | ||
status := exiterr.Sys().(syscall.WaitStatus) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work on all OS's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work on darwin, linux, and windows (all the terragrunt OS's).
It's not completely portable (plan9 prob won't work) but unfortunately there's no portable way to get the exit code. Fortunately, windows and *nix have the same method signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it!
LGTM. Thank you! |
Just created a new release here: https://github.com/gruntwork-io/terragrunt/releases/tag/v0.8.1. If the build passes, the new binaries should show up in a few minutes. |
fixes #37