8000 add BashWithOpts command for strict bash exec by burmudar · Pull Request #45 · sourcegraph/run · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@burmudar
Copy link
Contributor
@burmudar burmudar commented Oct 31, 2022

In #44 a command seems to not return an error from a command that is executed. Upon further investigation it is because the command is executed with bash -c, which means bash default options which means that if the last command of a pipe succeeds, the entire pipe statement is considered to have succeeded regardless if earlier statements have failed. A common "fix" for this is to enable pipefail and errexit options in bash.

This PR adds easier way for users to execute some bash scripts with "strict" bash and possibly avoid unexpected errors.

Closes #44

@burmudar burmudar requested review from a team and bobheadxi October 31, 2022 10:46
@burmudar burmudar requested a review from michaellzc as a code owner October 31, 2022 10:46
@burmudar burmudar self-assigned this Oct 31, 2022
Copy link
Contributor
@sanderginn sanderginn left a comment

Choose a reason for hiding this comment

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

Instinctively I would want pipe failures to exit with error codes by default. What's the motivation to implement it as a standalone option?

@sanderginn
Copy link
Contributor

Also thank you for picking this up 🚀

Co-authored-by: Sander Ginn <sanderginn@users.noreply.github.com>
burmudar and others added 2 commits October 31, 2022 18:20
Co-authored-by: Robert Lin <robert@bobheadxi.dev>
@burmudar burmudar requested a review from bobheadxi October 31, 2022 16:40
@burmudar
Copy link
Contributor Author

Instinctively I would want pipe failures to exit with error codes by default. What's the motivation to implement it as a standalone option?

Sorry @sanderginn missed your comment and only saw it now. Forcing it now to run with bash strictness might break other scripts so I opted to add it as a separate call.

@burmudar burmudar merged commit 9f0a46a into main Oct 31, 2022
@burmudar burmudar deleted the wb/add-bash-options branch October 31, 2022 17:02
@bobheadxi
Copy link
Member

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.

sourcegraph/run: investigate errored command not being thrown as an error

5 participants

0