-
Notifications
You must be signed in to change notification settings - Fork 1.2k
ci: consolidate 2 jobs to 1, add arm64 image #6706
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
base: develop
Are you sure you want to change the base?
ci: consolidate 2 jobs to 1, add arm64 image #6706
Conversation
Replace cross-compilation approach with native ARM64 builds to resolve build failures and improve performance. Changes: - Remove QEMU setup and multi-platform builds from build-container.yml - Add container-arm64 and container-slim-arm64 jobs using ubuntu-24.04-arm64 runners - Parameterize build-container.yml to accept runs-on input - Simplify shellcheck architecture detection in ci-slim.Dockerfile - Use same container names for both architectures This approach eliminates GCC segfaults during cppcheck compilation on ARM64 and provides faster, more reliable builds compared to emulated cross-compilation.
5d7e8da
to
b071188
Compare
WalkthroughThe changes introduce support for building and pushing both "main" and "slim" Docker containers in the GitHub Actions workflows, with explicit input parameters for each container's Dockerfile path and image name. The workflow runner is now configurable via an input parameter instead of being hardcoded. Additionally, a new workflow job is added to build ARM64 containers using the same Dockerfiles, and a corresponding ARM64 test job is introduced. Test jobs are updated to reference the new slim container output path. The slim container's Dockerfile is updated to select the correct ShellCheck binary based on the system architecture. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contrib/containers/ci/ci-slim.Dockerfile (1)
91-93
: Consider more robust architecture mapping
The dynamicuname -m
detection is a solid improvement to fetch the correct ShellCheck binary. To make this even more resilient across environments (e.g., some systems reportarm64
instead ofaarch64
), consider normalizing the architecture string or expanding theif
to cover both variants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-container.yml
(2 hunks).github/workflows/build.yml
(2 hunks)contrib/containers/ci/ci-slim.Dockerfile
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: arm-linux-build / Build source
🔇 Additional comments (9)
.github/workflows/build.yml (3)
22-29
: Slim inputs consolidate two jobs into one
Renaming the container job to “Build containers (amd64)” and addingslim-file
/slim-name
inputs aligns the slim build under the same workflow call. This reduces duplication and makes the slim/main relationship explicit.
31-40
: ARM64 container job mirrors AMD64 logic
The newcontainer-arm64
job correctly reusesbuild-container.yml
with the same inputs and overridesruns-on
toubuntu-24.04-arm
. This ensures parity between architectures.
201-205
: Test jobs updated to consume slim image
Alltest-…
jobs now referenceneeds.container.outputs.slim-path
(andneeds.container-arm64.outputs.slim-path
for ARM), ensuring they pull the slim image. The dependency and path updates look correct.Also applies to: 210-214, 219-223, 227-232, 237-242, 247-250, 253-260
.github/workflows/build-container.yml (6)
9-16
: Clearer input definitions for main Docker image
The inputsfile
(main Dockerfile path) andname
(main container name) now have explicit descriptions, improving readability for users of this reusable workflow.
17-24
: Added inputs for slim container
Introducingslim-file
andslim-name
allows building a second, minimal image in the same job without duplicating steps—this cleanly supports the slim/main consolidation goal.
25-30
: Configurable runner with sensible default
Parameterizingruns-on
with a default ofubuntu-24.04
gives callers flexibility to target different environments without hardcoding the runner.
31-37
: Workflow outputs expose both image paths
Definingpath
and the newslim-path
outputs ensures downstream consumers can reference the correct image tags for both the main and slim builds.
41-41
: Runs-on now parameterized in build job
Using the${{ inputs.runs-on }}
expression for thebuild
job correctly applies the caller’s runner choice, enabling multi-architecture invocation.
69-84
: Sequential slim image build/push with cache hints
Building and pushing the slim image first, keyed byhashFiles(inputs.slim-file)
, is a smart move to maximize cache hits and streamline the subsequent main image build. The tags and cache-from are well-structured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It failed. Do you have another link maybe? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest CI indicates pending functionality
https://github.com/PastaPastaPasta/dash/actions/runs/15408556076
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
CI in my fork: https://github.com/PastaPastaPasta/dash/actions/runs/15408489687
Breaking Changes
None
Checklist:
Go over all the following points, and put an
x
in all the boxes that apply.