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

Set pipefail so that makes error code is propagated to the shell #523

Merged
merged 5 commits into from
Apr 27, 2024

Conversation

matsl
Copy link
Collaborator

@matsl matsl commented Apr 26, 2024

What

Current pipe with tee swallows the error code if any from make.

Why

The pipe to tee for both showing the output from make and storing it
for processing swallows the error code from make if it fails. Setting shell
to bash explicitly gets the pipefail option set which makes the
pipe a whole fail if make fails.

Note

I noticed this incorrectness when looking at why the rsw branch with
the hywiki did not fail since it should. (But it actually did fail. It was not
noticed since the compilation error was swallowed. This PR fixes this.)

There are some test commits before I got it right but I will squash them
on merge.

Current pipe with tee swallows the error code if any from make.
@matsl matsl force-pushed the fix_pipe_fail_not_propagated_as_error branch from 3fd775f to 4ed91bb Compare April 26, 2024 22:15
@matsl matsl requested a review from rswgnu April 26, 2024 22:41
Copy link
Owner

@rswgnu rswgnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t ‘sh’ the same as ‘bash’ on Linux? If so, the former will work on more platforms that do not have bash.

@matsl
Copy link
Collaborator Author

matsl commented Apr 27, 2024

Isn’t ‘sh’ the same as ‘bash’ on Linux? If so, the former will work on more platforms that do not have bash.

No, sh is posix compliant whereas bash contains enhancements such as pipefail that we want to activate in this case. This also only affects the github workflow so platform compatibility is of no concern.

@matsl matsl requested a review from rswgnu April 27, 2024 08:59
@matsl matsl merged commit 63ed77a into master Apr 27, 2024
4 checks passed
@matsl matsl deleted the fix_pipe_fail_not_propagated_as_error branch April 27, 2024 20:22
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.

None yet

2 participants