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

fix: correct go build error passing #1067

Merged
merged 3 commits into from
Feb 1, 2024
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Jan 31, 2024

Aims to Close #673

The meta reason is the entire build pipeline not being written with set -e, which means every potential error path needs to be manually handled, or it will be ignored.

For fix here, I suspect things worked fine until we added glibc-check here, which made go build no longer be the last command in goBuild, and if go build fails its code is not the code returned by goBuild, but we continue to the next command, which may run ok due to no binaries to check 🙃

TODO

  • fix the way build-log is created and go build errors are handled
  • try to simulate failure on CI and confirm error is handled ok
  • switch back DIST_ROOT to default and merge if passing

@lidel lidel force-pushed the fix/correct-go-build-error-passing branch 3 times, most recently from 2615e67 to d9313fc Compare January 31, 2024 18:12
@lidel lidel force-pushed the fix/correct-go-build-error-passing branch from d9313fc to a2c2732 Compare January 31, 2024 18:52

This comment was marked as outdated.

@lidel lidel marked this pull request as ready for review January 31, 2024 19:04
Copy link
Member Author

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Waiting for CI to pass, if not issues this is ok to merge.

Comment on lines +109 to +112
if ! (go build -mod=mod -o "$output" -trimpath "${package}"); then
warn " go build of $output failed."
return 1
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

ℹ️ error handling here was missing

@@ -144,7 +145,7 @@ function doBuild() {

mkdir -p "$dir"

if ! (cd "$build_dir_name" && goBuild "$package" "$goos" "$goarch") > build-log; then
if ! (cd "$build_dir_name" && goBuild "$package" "$goos" "$goarch") > "$build_dir_name/build-log" 2>&1; then
Copy link
Member Author

Choose a reason for hiding this comment

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

ℹ️ why error logs were not created correctly OR were missing stderr

This comment was marked as outdated.

This comment was marked as outdated.

@lidel lidel merged commit da84766 into master Feb 1, 2024
5 checks passed
@lidel lidel deleted the fix/correct-go-build-error-passing branch February 1, 2024 01:06
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.

Failed binary builds are seen as successful
1 participant