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

PICARD-1382: Fail macOS packaging scripts on error #1083

Merged
merged 2 commits into from Jan 29, 2019

Conversation

phw
Copy link
Member

@phw phw commented Jan 29, 2019

Summary

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences:

Problem

The packaging scripts for macOS currently do not fail on error but continue running and return with success.

Solution

Use set -e to have the script terminate if any command fails.

Action

Note: Expected result of this PR is that the build fails due to https://tickets.metabrainz.org/browse/PICARD-1456 . Actually a failing build indicates this PR is a success. If the build succeeds this PR is probably not working as expected ;)

...and not after successful build. That makes failing packaging also fail the build.
@phw
Copy link
Member Author

phw commented Jan 29, 2019

Builds failed successfully :D

@phw phw merged commit 01bd341 into metabrainz:master Jan 29, 2019
@phw phw deleted the PICARD-1382-fail-macos-packaging-on-error branch January 29, 2019 17:25
@@ -1,4 +1,6 @@
#!/usr/bin/env bash
set -e
Copy link
Member

Choose a reason for hiding this comment

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

Is the curl --upload-file "$dmg" https://transfer.sh/ code still used? If so, then this is not safe because curl returns an exit code of 0 for a lot of HTTP status codes a human might consider errors (401, 403, maybe even 502). It's possible to work around that by using --fail, but even that's not guaranteed to result in a non-zero exit code. To make that uploads with curl robust, you need `curl --write-out '%{http_code}' and check whatever that writes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants