fix: use docker for arm builds - #36
Conversation
Code Review: ARM Build FixThank you for working on ARM build support! Here's my detailed review of the changes: 🟢 Positive Aspects
🔴 Critical Issues1. Duplicate "Install Build Dependencies" step (.github/workflows/release.yaml:31-40 and 72-78)The workflow now has TWO steps with the same name installing build dependencies. The second one (lines 72-78) will overwrite some of what the first one does and is missing multi-arch setup. Impact: The Fix: Remove the duplicate step at lines 72-78 and keep only the comprehensive one at lines 31-40. 2. Missing
|
There was a problem hiding this comment.
Pull request overview
This PR updates the release pipeline and GoReleaser config to support Linux/arm64 builds that require CGO (libvirt), primarily by adding cross-compilation dependencies and arm64-specific linker/search paths.
Changes:
- Add arm64 CGO environment overrides in
.goreleaser.yaml(includingCGO_LDFLAGSand explicitly enabling CGO). - Update the GitHub release workflow to install multi-arch build dependencies (arm64 + amd64 libvirt headers/libs and cross-compilers).
- Add QEMU/Buildx setup steps in the release workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.goreleaser.yaml |
Adds/adjusts CGO-related env vars for linux/arm64 builds (libvirt tag builds). |
.github/workflows/release.yaml |
Installs arm64 toolchain + libvirt multiarch deps and adds Docker/QEMU setup steps to the release job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Set up QEMU | ||
| uses: docker/setup-qemu-action@v3 | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
There was a problem hiding this comment.
The workflow sets up QEMU and Docker Buildx, but nothing later in this job uses Docker/Buildx (no docker buildx commands and no dockers: section in .goreleaser.yaml). This adds extra time/complexity without affecting the build. Either remove these steps, or update the release process/GoReleaser config to actually perform the ARM build inside Docker as intended by the PR title.
| libvirt-dev:arm64 \ | ||
| qemu-user-static | ||
|
|
There was a problem hiding this comment.
qemu-user-static is installed via apt while the job also runs docker/setup-qemu-action. If the workflow isn't running target (arm64) binaries, neither is needed for cross-compiling with gcc-aarch64-linux-gnu; and if it is needed, having both is redundant. Consider dropping one (or both) to avoid longer installs and reduce maintenance surface.
| libvirt-dev:arm64 \ | |
| qemu-user-static | |
| libvirt-dev:arm64 |
| - CC=aarch64-linux-gnu-gcc | ||
| - CXX=aarch64-linux-gnu-g++ | ||
| - CGO_ENABLED=1 | ||
| - CGO_LDFLAGS=-L/usr/lib/aarch64-linux-gnu | ||
| - PKG_CONFIG_PATH=/usr/lib/aarch64-linux-gnu/pkgconfig |
There was a problem hiding this comment.
CGO_ENABLED=1 is already set at the build level for fluid-remote, so repeating it in the per-arch override doesn’t change behavior and makes the config harder to scan. Consider keeping CGO_ENABLED only at the build level and leaving overrides for arch-specific settings (CC/CXX/PKG_CONFIG/CGO_LDFLAGS).
| env: | ||
| - CC=aarch64-linux-gnu-gcc | ||
| - CXX=aarch64-linux-gnu-g++ | ||
| - CGO_ENABLED=1 |
There was a problem hiding this comment.
Same redundancy here: CGO_ENABLED=1 is already set at the build level for fluid, so duplicating it in the linux/arm64 override adds noise without changing behavior. Consider removing it from the override and keeping only the arch-specific env vars.
| - CGO_ENABLED=1 |
Description
Type of Change
Checklist
Release Notes
Labels