CI: Use goreleaser to build release artifacts#124
Conversation
📝 WalkthroughWalkthroughThis PR centralizes builds with GoReleaser by adding .goreleaser.yaml, a release script (hack/release.sh) that downloads/runs goreleaser and normalizes artifacts into top-level dist/, and a cleanup script (hack/clean.sh). The Makefile gains a release target and delegates clean to hack/clean.sh; .gitignore now ignores /dist. CI workflows are simplified to call make release instead of per-arch build steps, and the release workflow's artifact selector is adjusted to the consolidated artifact name. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a release automation workflow by adding a GoReleaser configuration and helper scripts for building and cleaning artifacts. The feedback focuses on improving the robustness and portability of the shell scripts, specifically addressing a dangerous directory deletion risk, replacing the non-portable realpath command, handling potential null values in JSON processing, and optimizing data piping for efficiency.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
hack/release.shscript downloads the latest goreleaser from the internet, which can make CI runs non-deterministic; consider pinning a specific version (or reading it from an env/var) so builds are reproducible. hack/release.shassumesyq,jq,curl, andtarare present; adding explicit checks with clear error messages for these tools would make failures easier to diagnose when running the script locally or in new environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `hack/release.sh` script downloads the latest goreleaser from the internet, which can make CI runs non-deterministic; consider pinning a specific version (or reading it from an env/var) so builds are reproducible.
- `hack/release.sh` assumes `yq`, `jq`, `curl`, and `tar` are present; adding explicit checks with clear error messages for these tools would make failures easier to diagnose when running the script locally or in new environments.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Coverage Report for CI Build 25744734712Coverage remained the same at 91.698%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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 `@hack/release.sh`:
- Around line 47-48: Replace the recursive delete call that can fail when files
are absent: in the cleanup step that currently calls rm -r on "artifacts.json",
"config.yaml", and "metadata.json", use rm -f (without -r) so missing files are
ignored and the script won’t exit under set -e; update the command that
references dist_dir and those three filenames accordingly.
- Around line 15-23: The script currently uses LATEST="$(curl -sf
https://goreleaser.com/static/latest)" and downloads/verifies nothing; change it
to pin a specific GoReleaser version (or read from an env var like
GORELEASER_VERSION) and download both the matching tarball and its SHA256
checksum file, verify the checksum against the downloaded
"${bin_dir}/goreleaser.tar.gz" before extracting, and fail the script on
mismatch; update uses of LATEST, the curl target URL, and the
goreleaser="${bin_dir}/goreleaser" assignment accordingly so the verification
happens prior to tar -xzf and removal of the archive.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 9f834ce8-d0e9-4732-8dd3-92ad11570042
📒 Files selected for processing (7)
.github/workflows/ci.yaml.github/workflows/release.yaml.gitignore.goreleaser.yamlMakefilehack/clean.shhack/release.sh
Instead of a matrix build job, use goreleaser instead. Signed-off-by: Heathcliff <heathcliff@heathcliff.eu>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
hack/release.sh (2)
48-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
rm -ffor metadata cleanup files.With
set -e, this can fail if any file is absent; these files are not guaranteed in every interrupted/partial run.🤖 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 `@hack/release.sh` at line 48, Replace the fragile recursive remove command that can fail under set -e with a forceful non-recursive delete: change the rm -r invocation that references the artifacts.json, config.yaml and metadata.json variables to rm -f "${dist_dir}/artifacts.json" "${dist_dir}/config.yaml" "${dist_dir}/metadata.json" so missing files don't cause the script to exit; keep the same variable names (dist_dir) and target filenames when making the change.
15-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin and verify downloaded GoReleaser binary before extraction.
The script still pulls
latestand extracts without integrity verification, which is non-reproducible and weakens supply-chain safety.#!/bin/bash # Verify current behavior is still "latest" + no checksum validation sed -n '15,23p' hack/release.sh | cat -n rg -n 'static/latest|checksums|sha256' hack/release.sh🤖 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 `@hack/release.sh` around lines 15 - 23, The script currently downloads the "latest" GoReleaser tarball (variable LATEST) and extracts it without verification; change it to pin and verify the binary before extraction by resolving a specific release tag (do not rely on static/latest), downloading the corresponding checksum or signature for that release, validating the tarball's SHA256 (or signature) against the downloaded checksum, and aborting on mismatch; update the download/extract sequence that uses arch, curl, tar and goreleaser="${bin_dir}/goreleaser" to perform checksum verification (fail early if validation fails) before calling tar -xzf.
🤖 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 `@hack/release.sh`:
- Line 8: This script uses external tools (yq, jq, curl, tar) but doesn't verify
they exist up front; add a fail-fast check near the script start that iterates
over the required commands (yq, jq, curl, tar) using command -v (or which) and
prints a clear error like "required tool X not found, please install" then exits
non-zero if any are missing; place this check before variables like name="$(yq
-r '.project_name' "${base_dir}/.goreleaser.yaml")" so missing dependencies are
detected early.
---
Duplicate comments:
In `@hack/release.sh`:
- Line 48: Replace the fragile recursive remove command that can fail under set
-e with a forceful non-recursive delete: change the rm -r invocation that
references the artifacts.json, config.yaml and metadata.json variables to rm -f
"${dist_dir}/artifacts.json" "${dist_dir}/config.yaml"
"${dist_dir}/metadata.json" so missing files don't cause the script to exit;
keep the same variable names (dist_dir) and target filenames when making the
change.
- Around line 15-23: The script currently downloads the "latest" GoReleaser
tarball (variable LATEST) and extracts it without verification; change it to pin
and verify the binary before extraction by resolving a specific release tag (do
not rely on static/latest), downloading the corresponding checksum or signature
for that release, validating the tarball's SHA256 (or signature) against the
downloaded checksum, and aborting on mismatch; update the download/extract
sequence that uses arch, curl, tar and goreleaser="${bin_dir}/goreleaser" to
perform checksum verification (fail early if validation fails) before calling
tar -xzf.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a0ea6f2f-d493-4e69-ae9b-3a5d10b56f28
📒 Files selected for processing (7)
.github/workflows/ci.yaml.github/workflows/release.yaml.gitignore.goreleaser.yamlMakefilehack/clean.shhack/release.sh
Instead of a matrix build job, use goreleaser instead.
Signed-off-by: Heathcliff heathcliff@heathcliff.eu