Windows Build: Fail early on all errors #2280
Conversation
hoffie
left a comment
There was a problem hiding this comment.
Sometimes one gets to be lucky. CI on this very PR failed. It failed due to Qt download flakiness, which is what I'm mostly after with this PR. And it failed as it should, it failed in the very first stage. There can hardly be a better test case... ;)
https://github.com/jamulussoftware/jamulus/runs/4941257866?check_suite_focus=true
The checks completed successfully on the second try though.
This PR is now ready for review.
| ) | ||
|
|
||
| # Fail early on all errors | ||
| $ErrorActionPreference = "Stop" |
There was a problem hiding this comment.
I considered adding this line at the very top of the script, but this breaks param() which has to be the first statement in a script (when used).
| echo "Install Qt..." | ||
| # Install Qt | ||
| pip install aqtinstall | ||
| if ( !$? ) |
There was a problem hiding this comment.
I had really hoped for $ErrorActionPreference to cover external command failures as well, but sadly it doesn't. There doesn't seem to be an equivalent to bash's set -eu either. So, manual error checking is all we're left with.
I considered adding a function to do that to avoid code duplication (similar to deploy_windows.ps1's Invoke-Native-Command), but I figured it's not worth it given the low number of repetitions within a single script.
pljones
left a comment
There was a problem hiding this comment.
Can't see anything wrong. Thanks!
ann0see
left a comment
There was a problem hiding this comment.
Some small clarifications. I hope I found all.
Currently, build errors such as failure to download Qt do not fail the script. As such, they do not fail the workflow step either. The workflow continues and fails at a very late stage, which is annoying and wastes ressources. This commit sets `$ErrorActionPreference = "Stop"` for all relevant PowerShell scripts in order to fail early for PowerShell-internal errors. This commits also adds exit code checks for all relevant external commands as PowerShell does not consider them to be errors otherwise. Fixes jamulussoftware#2276 Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>
9a71c65 to
15db09e
Compare
Short description of changes
Currently, build errors such as failure to download Qt do not fail the script. As such, they do not fail the workflow step either. The workflow continues and fails at a very late stage, which is annoying and wastes ressources.
This commit sets
$ErrorActionPreference = "Stop"for all relevant PowerShell scripts in order to fail early for PowerShell-internal errors.This commits also adds exit code checks for all relevant external commands as PowerShell does not consider them to be errors otherwise.
Context: Fixes an issue?
Fixes #2276
Does this change need documentation? What needs to be documented and how?
No. Does not need a ChangeLog entry either.
CHANGELOG: SKIP
Status of this Pull Request
Ready.
What is missing until this pull request can be merged?
Ready.
Checklist
My code follows the style guideno relevant style guide