fix: portable SHA256 verification and direct sccache invocation#209
Merged
fix: portable SHA256 verification and direct sccache invocation#209
Conversation
Fixes CI failure where autoconf download via ftpmirror.gnu.org failed because --proto =https blocked HTTP mirror redirects. Downloads now go to files with SHA256 verification instead of piping to tar, making HTTP redirects safe. Also adds checksums for all 8 library downloads in build-config.sh and auto-updates them in check-native-updates.sh.
Add portable sha256_check/sha256_compute helpers that work with both sha256sum (Linux) and shasum (macOS), failing explicitly if neither is available. Validate computed hashes are 64-char hex before writing to build-config.sh. Use sha256sum in build.yml since the runner is Ubuntu.
macOS BSD sha256sum doesn't support --check/--status flags. Replace flag-based verification with compute-and-compare approach. Prefer shasum (available on macOS and most Linux) over sha256sum.
sha256_compute was defined in build-config.sh but only sourced in a subshell for URL extraction, making it unavailable for hash computation. Source in current shell so both the URL variables and helper functions are accessible.
Replace per-compiler shell script wrappers with CC="sccache compiler" directly, eliminating an extra fork+exec per compilation. Quote CC=$CC in make commands to handle the space in the value. Windows build script auto-detects sccache availability for local use without it.
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 6dbcd67..2e8f206
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (8)
• .github/workflows/build.yml
• native/build-config.sh
• native/build-linux-arm.sh
• native/build-linux-arm64.sh
• native/build-linux-x86.sh
• native/build-linux.sh
• native/build-windows.sh
• scripts/check-native-updates.sh
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
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.
Summary
Follow-up to #208, addressing issues found in CI:
sha256sumdoesn't support--check --status. Replaced with compute-and-compare approach usingshasum(preferred) orsha256sumcheck-native-updates.shnow validates computed hashes are 64-char hex before writing tobuild-config.sh, and sourcesbuild-config.shin the current shell sosha256_computeis actually availableexec sccache compiler) withCC="sccache compiler"directly, eliminating a fork+exec per compilation. Quoted"CC=$CC"in make commands to handle the spaceTest plan
shasum)High-level PR Summary
This PR fixes cross-platform SHA256 verification issues and optimizes compiler caching. The main changes include replacing GNU-specific
sha256sum --check --statuswith a portable compute-and-compare approach usingshasumorsha256sum(compatible with macOS BSD and Linux), adding SHA256 checksums tobuild-config.shwith validation, and replacing shell script wrappers with directCC="sccache compiler"invocation to eliminate unnecessary fork+exec overhead per compilation. The update script now computes and validates checksums when updating dependency versions.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
native/build-config.shscripts/check-native-updates.shnative/build-linux.shnative/build-linux-arm.shnative/build-linux-arm64.shnative/build-linux-x86.shnative/build-windows.sh.github/workflows/build.ymlSummary by cubic
Make SHA256 verification portable across macOS and Linux and invoke
sccachedirectly to improve CI reliability and build performance.Bug Fixes
shasumorsha256sum(no--checkflags) with newsha256_computeandsha256_checkhelpers.native/build-config.sh.autoconfandautomaketo files and verify SHA256, allowing HTTP mirror redirects safely.build-config.shin the current shell so helpers are available.Refactors
CC="sccache <compiler>"andCXX="sccache <compiler>"; quote"CC=$CC","AR=$AR", and"RANLIB=$RANLIB"inmakecalls to handle spaces.sccacheon Windows and use it when present.Written for commit 5b0dfb6. Summary will update on new commits.