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

Scripts: Set -o pipefail #2794

Merged
merged 1 commit into from Aug 22, 2022

Conversation

hoffie
Copy link
Member

@hoffie hoffie commented Aug 20, 2022

Short description of changes
This is supposed to avoid silent failures when using pipes.
Example: #2793 (https://github.com/hoffie/jamulus/runs/7930874876?check_suite_focus=true#step:9:42)

CHANGELOG: Internal: Hardened build scripts and tooling against silent failures.

Context: Fixes an issue?

Does this change need documentation? What needs to be documented and how?
No.

Status of this Pull Request

What is missing until this pull request can be merged?

  • CI green
  • tools/ needs explicit review/testing for pipe usage.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@hoffie hoffie added this to the Release 3.9.1 milestone Aug 20, 2022
@hoffie hoffie added this to Triage in Tracking (old) via automation Aug 20, 2022
@hoffie hoffie moved this from Triage to In Progress in Tracking (old) Aug 20, 2022
@hoffie hoffie mentioned this pull request Aug 20, 2022
5 tasks
@hoffie
Copy link
Member Author

hoffie commented Aug 22, 2022

Testable tooling (e.g. translation checker, copyright updates) tested. All scripts reviewed for potential unwanted side effects (little pipe usage and where pipes are used, they should be checked properly).

The Linux CI failure is expected and is a proof of this change working properly. It will go once #2793 is merged and the PR is rebased.

@ann0see
Copy link
Member

ann0see commented Aug 22, 2022

I think you should now rebase this PR?

This is supposed to avoid silent failures when using pipes.
@hoffie
Copy link
Member Author

hoffie commented Aug 22, 2022

Rebased. Linux builds are green now as well, as expected.

@hoffie hoffie merged commit cafdbaf into jamulussoftware:master Aug 22, 2022
Tracking (old) automation moved this from In Progress to Done Aug 22, 2022
@pljones
Copy link
Collaborator

pljones commented Sep 21, 2022

Hi @hoffie,

I had to do this:
b333de3
I'd guess the "pipefail" change meant I now got a silent failure - rather than preventing one, it caused it. || true ignores the fail... Just a hack to get the script working for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants