fix: consolidate per-platform SHA512SUMS into single file to avoid upload collisions#120
Merged
Merged
Conversation
…load collisions Each build artifact previously included a SHA512SUMS file with identical filenames across platforms, causing 'asset under the same name already exists' errors during release upload. Now all per-platform SHA512SUMS files are merged into a single combined SHA512SUMS at the workspace root before upload, matching common open-source conventions (e.g. Node.js SHASUMS256.txt). Per-artifact copies are deleted so they are not uploaded redundantly.
Contributor
WalkthroughThe Release Asset Consolidation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Around line 210-217: Guard the clang-tools-* glob in the build workflow before
invoking find so the step does not fail when no artifact directories exist.
Update the logic around the combined SHA generation and file collection block to
detect whether clang-tools-* matched any directories before running find, and if
not, skip those commands so the existing “No files found” fallback can run. Use
the surrounding upload setup and the mapfile/files collection flow to keep the
behavior safe when no artifacts are downloaded.
- Around line 210-214: The checksum aggregation step in the build workflow
should fail if any `clang-tools-*` artifact is missing its local `SHA512SUMS`,
rather than silently building a partial consolidated file. Update the workflow
logic around the `find ... SHA512SUMS` and combined checksum generation so it
first verifies every expected artifact has a checksum file and exits with an
error if any are absent, before running the aggregation and deletion steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd4d7ad7-d5d6-4dce-bbbb-ea3e7e5471e9
📒 Files selected for processing (1)
.github/workflows/build.yml
Comment on lines
+210
to
+214
| find clang-tools-* -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha" | ||
| echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha" | ||
|
|
||
| # Remove the per-artifact SHA512SUMS files so they are not uploaded individually. | ||
| find clang-tools-* -name SHA512SUMS -type f -delete |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail fast when an artifact is missing its SHA512SUMS.
This currently uploads whatever checksum files happen to exist. If one clang-tools-* artifact is missing its local SHA512SUMS, that asset still gets released but disappears from the consolidated root file, which breaks the documented verification contract for sha512sum -c SHA512SUMS --ignore-missing.
Suggested fix
combined_sha="SHA512SUMS"
- find clang-tools-* -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha"
+ mapfile -d '' -t checksum_files < <(find "${artifact_dirs[@]}" -name SHA512SUMS -type f -print0)
+ if [[ ${`#checksum_files`[@]} -ne ${`#artifact_dirs`[@]} ]]; then
+ echo "::error::Expected one SHA512SUMS per artifact directory"
+ exit 1
+ fi
+ printf '%s\0' "${checksum_files[@]}" | xargs -0 cat > "$combined_sha"
+ [[ -s "$combined_sha" ]] || { echo "::error::Combined SHA512SUMS is empty"; exit 1; }
echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| find clang-tools-* -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha" | |
| echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha" | |
| # Remove the per-artifact SHA512SUMS files so they are not uploaded individually. | |
| find clang-tools-* -name SHA512SUMS -type f -delete | |
| combined_sha="SHA512SUMS" | |
| mapfile -d '' -t checksum_files < <(find "${artifact_dirs[@]}" -name SHA512SUMS -type f -print0) | |
| if [[ ${`#checksum_files`[@]} -ne ${`#artifact_dirs`[@]} ]]; then | |
| echo "::error::Expected one SHA512SUMS per artifact directory" | |
| exit 1 | |
| fi | |
| printf '%s\0' "${checksum_files[@]}" | xargs -0 cat > "$combined_sha" | |
| [[ -s "$combined_sha" ]] || { echo "::error::Combined SHA512SUMS is empty"; exit 1; } | |
| echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha" | |
| # Remove the per-artifact SHA512SUMS files so they are not uploaded individually. | |
| find clang-tools-* -name SHA512SUMS -type f -delete |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build.yml around lines 210 - 214, The checksum aggregation
step in the build workflow should fail if any `clang-tools-*` artifact is
missing its local `SHA512SUMS`, rather than silently building a partial
consolidated file. Update the workflow logic around the `find ... SHA512SUMS`
and combined checksum generation so it first verifies every expected artifact
has a checksum file and exits with an error if any are absent, before running
the aggregation and deletion steps.
Comment on lines
+210
to
+217
| find clang-tools-* -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha" | ||
| echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha" | ||
|
|
||
| # Remove the per-artifact SHA512SUMS files so they are not uploaded individually. | ||
| find clang-tools-* -name SHA512SUMS -type f -delete | ||
|
|
||
| # Collect all files and upload one-by-one with a small delay | ||
| mapfile -t files < <(find clang-* -type f) | ||
| mapfile -t files < <(find clang-tools-* -type f) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the clang-tools-* glob before calling find.
If no artifact directories were downloaded, Bash leaves clang-tools-* unexpanded, find exits non-zero, and the job dies before Line 219’s “No files found” fallback can run.
Suggested fix
+ shopt -s nullglob
+ artifact_dirs=(clang-tools-*)
+ if [[ ${`#artifact_dirs`[@]} -eq 0 ]]; then
+ echo "::warning::No artifact directories found"
+ exit 0
+ fi
+
combined_sha="SHA512SUMS"
- find clang-tools-* -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha"
+ find "${artifact_dirs[@]}" -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha"
echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha"
# Remove the per-artifact SHA512SUMS files so they are not uploaded individually.
- find clang-tools-* -name SHA512SUMS -type f -delete
+ find "${artifact_dirs[@]}" -name SHA512SUMS -type f -delete
# Collect all files and upload one-by-one with a small delay
- mapfile -t files < <(find clang-tools-* -type f)
+ mapfile -t files < <(find "${artifact_dirs[@]}" -type f)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| find clang-tools-* -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha" | |
| echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha" | |
| # Remove the per-artifact SHA512SUMS files so they are not uploaded individually. | |
| find clang-tools-* -name SHA512SUMS -type f -delete | |
| # Collect all files and upload one-by-one with a small delay | |
| mapfile -t files < <(find clang-* -type f) | |
| mapfile -t files < <(find clang-tools-* -type f) | |
| shopt -s nullglob | |
| artifact_dirs=(clang-tools-*) | |
| if [[ ${`#artifact_dirs`[@]} -eq 0 ]]; then | |
| echo "::warning::No artifact directories found" | |
| exit 0 | |
| fi | |
| combined_sha="SHA512SUMS" | |
| find "${artifact_dirs[@]}" -name SHA512SUMS -type f -print0 | xargs -0 cat > "$combined_sha" | |
| echo "Created combined $(wc -l < "$combined_sha")-entry $combined_sha" | |
| # Remove the per-artifact SHA512SUMS files so they are not uploaded individually. | |
| find "${artifact_dirs[@]}" -name SHA512SUMS -type f -delete | |
| # Collect all files and upload one-by-one with a small delay | |
| mapfile -t files < <(find "${artifact_dirs[@]}" -type f) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build.yml around lines 210 - 217, Guard the clang-tools-*
glob in the build workflow before invoking find so the step does not fail when
no artifact directories exist. Update the logic around the combined SHA
generation and file collection block to detect whether clang-tools-* matched any
directories before running find, and if not, skip those commands so the existing
“No files found” fallback can run. Use the surrounding upload setup and the
mapfile/files collection flow to keep the behavior safe when no artifacts are
downloaded.
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
1 participant
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.
Problem
When releasing, each build artifact (version × platform) contains a
SHA512SUMSfile with the same filename. GitHub Release assets are keyed by filename, so everySHA512SUMSafter the first fails to upload:This affected 10 out of 10 SHA512SUMS files per release (all platforms except the first one alphabetically).
Solution
Combine all per-artifact
SHA512SUMSfiles into a single file at the workspace root before uploading. This follows the common open-source convention (e.g. Node.jsSHASUMS256.txt, DebianSHA256SUMS).The combined
SHA512SUMScontains checksums for all platforms and LLVM versions — users can verify any binary with a singlesha512sum -c SHA512SUMS --ignore-missingcommand.Changes
.github/workflows/build.yml— in thedraft-releasejob:SHA512SUMSand concatenate into oneSHA512SUMSalongsideversions.jsonBackward Compatibility
No user-facing change. The Release page still has a single
SHA512SUMSfile; the verification command (sha512sum -c SHA512SUMS --ignore-missing) works the same as before.Summary by CodeRabbit