Fail build.sh if any command in it fails
#2931
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mbg
force-pushed
the
mbg/fail-build.sh-on-error
branch
from
71d8dee to
63beaad
Compare
mbg
force-pushed
the
mbg/fail-build.sh-on-error
branch
from
63beaad to
8f71d47
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that Go is explicitly installed in all local workflow runs by adding an actions/setup-go@v5 step, so that build.sh and related scripts will correctly fail if Go is missing.
- Introduces an Install Go step to every workflow to guarantee a supported Go version is available.
- Aligns all workflows on
go-version: '>=1.21.0'and disables caching of Go installs. - Prepares the ground for
build.shto error out early when dependencies are not met.
Reviewed Changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/__test-local-codeql.yml | Added Install Go step before fetching bundle |
| .github/workflows/__swift-custom-build.yml | Added Install Go step before init action |
| .github/workflows/__split-workflow.yml | Added Install Go step before init action |
| .github/workflows/__remote-config.yml | Added Install Go step before init action |
| .github/workflows/__packaging-inputs-js.yml | Added Install Go step before init action |
| .github/workflows/__packaging-config-js.yml | Added Install Go step before init action |
| .github/workflows/__packaging-config-inputs-js.yml | Added Install Go step before init action |
| .github/workflows/__packaging-codescanning-config-inputs-js.yml | Added Install Go step before init action |
| .github/workflows/__multi-language-autodetect.yml | Added Install Go step before init action |
| .github/workflows/__go-tracing-legacy-workflow.yml | Replaced old setup-go usage with named step |
| .github/workflows/__go-tracing-custom-build-steps.yml | Replaced old setup-go usage with named step |
| .github/workflows/__go-tracing-autobuilder.yml | Replaced old setup-go usage with named step |
| .github/workflows/__go-indirect-tracing-workaround.yml | Added Install Go step before init action |
| .github/workflows/__go-indirect-tracing-workaround-no-file-program.yml | Added Install Go step before remove step |
| .github/workflows/__go-indirect-tracing-workaround-diagnostic.yml | Added Install Go step before init action |
| .github/workflows/__go-custom-queries.yml | Added Install Go step before init action |
| .github/workflows/__export-file-baseline-information.yml | Added Install Go step before init action |
| .github/workflows/__build-mode-manual.yml | Added Install Go step before init action |
| .github/workflows/__analyze-ref-input.yml | Added Install Go step before init action |
| .github/workflows/__all-platform-bundle.yml | Added Install Go step before init action |
Comments suppressed due to low confidence (1)
.github/workflows/__test-local-codeql.yml:48
- This
Install Gostep is duplicated across many workflows. Consider extracting it into a reusable workflow snippet or composite action to simplify updates and reduce duplication.
- - name: Install Go
redsun82
approved these changes
Jun 16, 2025
Comment on lines
+111
to
+113
| installGo = False | ||
| if checkSpecification.get('installGo'): | ||
| installGo = True if checkSpecification['installGo'].lower() == "true" else False |
There was a problem hiding this comment.
if we just use
Suggested change
| installGo = False | |
| if checkSpecification.get('installGo'): | |
| installGo = True if checkSpecification['installGo'].lower() == "true" else False | |
| installGo = checkSpecification.get('installGo') |
then we can use a more natural
installGo: truein the test specification, without using stringly typed values. Although I see we use "true" for other cases as well.
There was a problem hiding this comment.
Yeah, like with the other point, I went with this to mirror the existing inputs. I would be happy to change this though if we can't think of a downside / motivation for why the others are strings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates
build.shso that it fails if any step in the script fails. Previously, the script would continue running and thus mask unexpected failures. For example, Go is no longer available by default on runners and so an explicitsetup-gostep was required. However, because the script didn't fail, we didn't notice that failure until now.To address the Go issue, I updated
sync.shto understand a newinstallGoinput that injects thesetup-gostep into the workflow.I tried adding it to the local
prepare-testaction first, but this resulted in a number of further issues:setup-gothere, which has a post-action, forced the entireprepare-testaction to have a post-action, which caused problems because the workflow also moves the location of theprepare-testaction to outside of the workspace..githubfolder in the workspace.Merge / deployment checklist