Multi-Release Builder (Linux, MacOS, Windows)#30
Multi-Release Builder (Linux, MacOS, Windows)#30ServeurpersoCom merged 2 commits intoServeurpersoCom:masterfrom
Conversation
Signed-off-by: Lorenzo Mangani <lorenzo.mangani@gmail.com>
📝 WalkthroughWalkthroughRefactors the release workflow by replacing a single matrix-based Changes
Sequence Diagram(s)sequenceDiagram
participant WF as "GitHub Workflow\n(.github/workflows/release.yml)"
participant Runner as "Runner\n(linux/mac/windows)"
participant Cache as "Cache (ccache/gha-cache)"
participant Artifact as "Packager"
participant Release as "GitHub Releases"
WF->>Runner: start per-OS job (unless skipped)
Runner->>Cache: restore cache (ccache, deps)
Runner->>Runner: install toolchain & deps (CUDA/Vulkan where needed)
Runner->>Runner: configure & build binaries
Runner->>Runner: run smoke tests
Runner->>Artifact: package artifacts (tar.gz / zip)
Artifact->>Release: upload release artifact (derive tag or use input tag)
Release-->>WF: publish status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
|
Tested quick generation on Windows: backends are correctly identified, and the generation process works fine. I’ve verified Vulkan and CPU support. I skipped the pre-built CUDA binaries because my local setup uses an older GPU with CUDA 12.4, while the project currently targets CUDA 13.1 (I compiled the files locally instead). Suggestions for legacy hardware support:
P.S. The |
Example of a "Full" Build Configuration (CUDA 12.4.99)I've decided to share a "full" build configuration that might be useful for others. This setup is tailored for CUDA 12.4.99, targeting common consumer GPU architectures (pro-grade architectures are excluded here but can be added if needed). Build Commandcmake -B build-full \
-DGGML_VULKAN=ON \
-DGGML_CUDA=ON \
-DCMAKE_CUDA_ARCHITECTURES="50-real;52-real;61-real;75-real;86-real;89" \
-DGGML_NATIVE=OFF \
-DGGML_CPU_ALL_VARIANTS=ON \
-DGGML_OPENMP=ON \
-DGGML_BACKEND_DL=ON \
-DGGML_BLAS=OFF \
-DCMAKE_BUILD_TYPE=Release
cmake --build build-full --config Release -j$(nproc)Note on OpenMP Final File List in build-full (Release)
Important Manual Runtime Inclusion: The runtime DLLs listed above must be added to the build folder manually. You should copy them directly from your Visual Studio installation directory (typically under VC\Redist\MSVC...).
External Dependencies |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
113-116: Pin the validated macOS image.GitHub treats
macos-latestas a moving label and recommends versioned labels when you want to avoid image migrations. For a release artifact namedacestep-macos-arm64-metal.tar.gz, pin the exact image you validated (for examplemacos-15) so SDK/Xcode changes do not silently change the shipped binary. (github.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 113 - 116, The build job "build-mac" currently uses the moving label "runs-on: macos-latest"; change it to the exact macOS runner you validated (e.g., "runs-on: macos-15") so the released artifact "acestep-macos-arm64-metal.tar.gz" is built on a pinned image and not affected by upstream image updates—update the runs-on value in the build-mac job to the validated version (replace macos-latest with the concrete runner label you tested).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 261-267: The "Package binaries" job only copies files from
build-msvc\Release and omits required redistributable runtime DLLs (MSVC
CRT/MSVCP, matching CUDA runtime DLLs) while also potentially including
build-only tools like vulkan-shaders-gen.exe; update the step that currently
runs Copy-Item and Compress-Archive so it collects and copies the MSVC runtime
DLLs and the matching CUDA runtime DLLs into dist (explicitly list or resolve
them from the VC++/CUDA redistributable folders) and ensure you filter out
build-only artifacts (e.g., exclude vulkan-shaders-gen.exe) before creating the
ZIP so the archive is portable on clean Windows hosts and only contains runtime
redistributables plus the final executables and required DLLs.
- Around line 58-61: The "Install CUDA toolkit" steps using
Jimver/cuda-toolkit@v0.2.30 in both the Linux and Windows jobs are missing an
explicit cuda: input and thus default to CUDA 13.1.0; update both occurrences of
the "Install CUDA toolkit" step (the ones that call Jimver/cuda-toolkit@v0.2.30)
to include a cuda: input pinned to your CUDA 12 toolchain (e.g., cuda: '12.1.0'
or whatever exact 12.x version your cache key uses) so the runtime/toolchain
version matches the Windows cache key (cuda-12-windows-latest).
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 113-116: The build job "build-mac" currently uses the moving label
"runs-on: macos-latest"; change it to the exact macOS runner you validated
(e.g., "runs-on: macos-15") so the released artifact
"acestep-macos-arm64-metal.tar.gz" is built on a pinned image and not affected
by upstream image updates—update the runs-on value in the build-mac job to the
validated version (replace macos-latest with the concrete runner label you
tested).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7ceb249-0f3c-4399-ac47-435326944983
📒 Files selected for processing (1)
.github/workflows/release.yml
| - name: Install CUDA toolkit | ||
| uses: Jimver/cuda-toolkit@v0.2.30 | ||
| if: matrix.install_cuda == true | ||
| with: | ||
| log-file-suffix: '${{matrix.os}}.txt' | ||
| log-file-suffix: 'ubuntu-22.04.txt' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the workflow file
cat -n .github/workflows/release.yml | head -100Repository: ServeurpersoCom/acestep.cpp
Length of output: 3435
🏁 Script executed:
# Also check if the file exists and get its total line count
wc -l .github/workflows/release.ymlRepository: ServeurpersoCom/acestep.cpp
Length of output: 104
🏁 Script executed:
# Read the specific sections mentioned (lines 58-61 and 193-205)
sed -n '50,70p' .github/workflows/release.yml | cat -nRepository: ServeurpersoCom/acestep.cpp
Length of output: 789
🏁 Script executed:
# Read the Windows section around lines 193-205
sed -n '185,210p' .github/workflows/release.yml | cat -nRepository: ServeurpersoCom/acestep.cpp
Length of output: 1025
🌐 Web query:
Jimver/cuda-toolkit v0.2.30 default CUDA version
💡 Result:
In Jimver/cuda-toolkit v0.2.30, if you don’t specify the cuda input, the action installs CUDA 13.1.0 by default (inputs.cuda.default: '13.1.0'). [1]
Pin CUDA toolkit version explicitly in both Linux and Windows jobs.
Both Jimver/cuda-toolkit steps omit the cuda: input and default to 13.1.0, while the Windows cache key is labeled cuda-12-windows-latest, creating a mismatch between the expected and actual toolchain version. Define the cuda: version explicitly in both steps (lines 58-61 and ~202-206) to ensure consistency and prevent DLL ABI drift between builds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 58 - 61, The "Install CUDA
toolkit" steps using Jimver/cuda-toolkit@v0.2.30 in both the Linux and Windows
jobs are missing an explicit cuda: input and thus default to CUDA 13.1.0; update
both occurrences of the "Install CUDA toolkit" step (the ones that call
Jimver/cuda-toolkit@v0.2.30) to include a cuda: input pinned to your CUDA 12
toolchain (e.g., cuda: '12.1.0' or whatever exact 12.x version your cache key
uses) so the runtime/toolchain version matches the Windows cache key
(cuda-12-windows-latest).
| - name: Package binaries | ||
| shell: pwsh | ||
| run: | | ||
| New-Item -ItemType Directory -Path dist | Out-Null | ||
| Copy-Item "build-msvc\Release\*.exe" dist\ -ErrorAction SilentlyContinue | ||
| Copy-Item "build-msvc\Release\*.dll" dist\ -ErrorAction SilentlyContinue | ||
| Compress-Archive -Path dist\* -DestinationPath "acestep-windows-x64.zip" |
There was a problem hiding this comment.
The Windows ZIP is not portable yet.
This step only copies files from build-msvc\Release. The PR notes call out extra MSVC runtime DLLs and matching CUDA runtime DLLs as required for portability, so the uploaded archive will still fail on clean Windows hosts. While curating that list, exclude build-only tools like vulkan-shaders-gen.exe from the release payload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 261 - 267, The "Package binaries"
job only copies files from build-msvc\Release and omits required redistributable
runtime DLLs (MSVC CRT/MSVCP, matching CUDA runtime DLLs) while also potentially
including build-only tools like vulkan-shaders-gen.exe; update the step that
currently runs Copy-Item and Compress-Archive so it collects and copies the MSVC
runtime DLLs and the matching CUDA runtime DLLs into dist (explicitly list or
resolve them from the VC++/CUDA redistributable folders) and ensure you filter
out build-only artifacts (e.g., exclude vulkan-shaders-gen.exe) before creating
the ZIP so the archive is portable on clean Windows hosts and only contains
runtime redistributables plus the final executables and required DLLs.
There was a problem hiding this comment.
A community user in this thread confirmed the Windows builds are working
There was a problem hiding this comment.
@lmangani, thanks for the update! Good to know the builds are working for the community. It's worth noting that cynodesmus did mention having to manually copy the MSVC runtime DLLs (msvcp140.dll, vcruntime140.dll, etc.) for portability on clean hosts — but if that's an acceptable workflow for your users right now, that's totally fine to defer to a later improvement. Thanks for the context!
🧠 Learnings used
Learnt from: lmangani
Repo: ServeurpersoCom/acestep.cpp PR: 29
File: .github/workflows/release.yml:140-143
Timestamp: 2026-03-16T12:06:46.219Z
Learning: In `.github/workflows/release.yml` of the `ServeurpersoCom/acestep.cpp` repository, the Windows packaging/upload steps (including the `$bins` array and `foreach` loop) are intentionally present without a corresponding Windows matrix entry. They serve as a reference scaffold for future Windows build support, which is currently a work-in-progress. Do not flag these blocks as dead/unreachable code in future reviews.
Thanks @cynodesmus this is super useful! The builds are experimental in preparation for a proper release in the near future, please feel free to PR against the builder action if you have suggestions for better defaults 👍 |
|
Thanks @lmangani! Submitting formal PRs isn't really my forte, so I’d appreciate it if you could just use the information below to improve the current one. I’ve carefully weighed the pros and cons of certain flags:
Legacy CPU Support: In Before configure_msvc_target(ggml-cpu-sandybridge)
# __FMA__ and __F16C__ are not defined in MSVC, however they are implied with AVX2/AVX512
# skipping ggml-cpu-ivybridge
# skipping ggml-cpu-piledriver
configure_msvc_target(ggml-cpu-haswell)After configure_msvc_target(ggml-cpu-sandybridge)
configure_msvc_target(ggml-cpu-ivybridge)
configure_msvc_target(ggml-cpu-piledriver)
configure_msvc_target(ggml-cpu-haswell)And in ggml/src/CMakeLists.txt: ggml_add_cpu_backend_variant(sandybridge SSE42 AVX)
if (NOT MSVC)
# __FMA__ and __F16C__ are not defined in MSVC, however they are implied with AVX2/AVX512
ggml_add_cpu_backend_variant(ivybridge SSE42 AVX F16C)
ggml_add_cpu_backend_variant(piledriver SSE42 AVX F16C FMA)
endif()
ggml_add_cpu_backend_variant(haswell SSE42 AVX F16C FMA AVX2 BMI2)After ggml_add_cpu_backend_variant(sandybridge SSE42 AVX)
ggml_add_cpu_backend_variant(ivybridge SSE42 AVX F16C)
ggml_add_cpu_backend_variant(piledriver SSE42 AVX F16C FMA)
ggml_add_cpu_backend_variant(haswell SSE42 AVX F16C FMA AVX2 BMI2)I have verified these changes in my local builds. I'm not quite ready to submit a separate PR to GGML myself, so I'm sharing these findings here. |
Removed commented sections for Linux, macOS, and Windows builds to clean up the workflow file. Signed-off-by: Lorenzo Mangani <lorenzo.mangani@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
54-58:⚠️ Potential issue | 🟠 MajorPin CUDA version explicitly (and align cache key).
Line 57 and Line 193 omit
cuda:forJimver/cuda-toolkit@v0.2.30, while Line 187 implies CUDA 12 in cache naming. This can silently drift toolchains between runs.Suggested patch
- key: cuda-12-windows-latest + key: cuda-13.1-windows-latest @@ - name: Install CUDA toolkit uses: Jimver/cuda-toolkit@v0.2.30 with: + cuda: '13.1.0' log-file-suffix: 'ubuntu-22.04.txt' @@ - name: Install CUDA toolkit if: steps.cache-cuda.outputs.cache-hit != 'true' uses: Jimver/cuda-toolkit@v0.2.30 with: + cuda: '13.1.0' log-file-suffix: 'windows-latest.txt'For Jimver/cuda-toolkit@v0.2.30, what is the default value of the `cuda` input when omitted, and is it stable across action updates?Also applies to: 187-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 54 - 58, The workflow is omitting the explicit cuda input for Jimver/cuda-toolkit@v0.2.30 which can let the toolchain drift; update the action invocation (the Jimver/cuda-toolkit@v0.2.30 step) to include an explicit cuda: '12' (or the exact CUDA minor/patch you need) and then update the related cache key construction (the cache key that references CUDA in the later steps) to include the same explicit cuda value so the cache aligns with the installed toolkit; also confirm the action's default for the cuda input in its docs and lock the value you want to guarantee reproducible runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 70-71: The workflow sets continue-on-error: true at multiple steps
(the continue-on-error key present near the shell: bash entries), which allows
smoke-test failures to not block releases; update each occurrence (the
continue-on-error entries at the three locations) to either remove the
continue-on-error key or set it to false so failures fail the job and prevent
packaging/uploading of bad binaries, then re-run CI to ensure smoke-test
failures now fail the release workflow.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 54-58: The workflow is omitting the explicit cuda input for
Jimver/cuda-toolkit@v0.2.30 which can let the toolchain drift; update the action
invocation (the Jimver/cuda-toolkit@v0.2.30 step) to include an explicit cuda:
'12' (or the exact CUDA minor/patch you need) and then update the related cache
key construction (the cache key that references CUDA in the later steps) to
include the same explicit cuda value so the cache aligns with the installed
toolkit; also confirm the action's default for the cuda input in its docs and
lock the value you want to guarantee reproducible runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a698d953-de52-43dc-a8db-36b756850231
📒 Files selected for processing (1)
.github/workflows/release.yml
| continue-on-error: true | ||
| shell: bash |
There was a problem hiding this comment.
Smoke-test failures currently do not block release uploads.
Line 70, Line 131, and Line 227 set continue-on-error: true, so failed binaries can still be packaged and uploaded in release runs.
Suggested patch
- - name: Smoke test
- continue-on-error: true
+ - name: Smoke test
+ continue-on-error: ${{ github.event_name == 'workflow_dispatch' }}
@@
- - name: Smoke test
- continue-on-error: true
+ - name: Smoke test
+ continue-on-error: ${{ github.event_name == 'workflow_dispatch' }}
@@
- - name: Smoke test
- continue-on-error: true
+ - name: Smoke test
+ continue-on-error: ${{ github.event_name == 'workflow_dispatch' }}Also applies to: 131-132, 227-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 70 - 71, The workflow sets
continue-on-error: true at multiple steps (the continue-on-error key present
near the shell: bash entries), which allows smoke-test failures to not block
releases; update each occurrence (the continue-on-error entries at the three
locations) to either remove the continue-on-error key or set it to false so
failures fail the job and prevent packaging/uploading of bad binaries, then
re-run CI to ensure smoke-test failures now fail the release workflow.
3 participants
Extended builder action with multi-os matrix. MacOS validated working, Linux/Windows binaries need testing.
Test packages: https://github.com/audiohacking/acestep.cpp/releases/tag/v0.0.2
Summary by CodeRabbit