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

build_framework.ios.sh: echo/exit from current shell #900

Merged
merged 1 commit into from
Aug 26, 2018
Merged

build_framework.ios.sh: echo/exit from current shell #900

merged 1 commit into from
Aug 26, 2018

Conversation

rye
Copy link
Contributor

@rye rye commented Aug 26, 2018

The () syntax spawns a subshell and executes the commands. Hence, the "successful" exit 0 is trapped and is effectively a no-op. This change instead moves the execution of the echo statement to the parent shell which causes the exit 0 to actually kill the process where it is.

This is part of the fixes to the build_framework.ios.sh shell script that I had requested in #897.

This PR changes from subprocess execution to in-shell execution. The biggest change that this makes is that it causes the exit 0 to actually stop the shell script.

The `()` syntax spawns a subshell and executes the commands. Hence, the "successful" `exit 0` is trapped and is effectively a no-op. This change instead moves the execution of the `echo` statement to the parent shell which causes the `exit 0` to actually kill the process where it is.
@rye rye requested a review from rotemmiz as a code owner August 26, 2018 21:44
@rotemmiz
Copy link
Member

Thanks for this!

@rotemmiz rotemmiz merged commit a321e9b into wix:master Aug 26, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants