fix(win): ship the OpenMP runtime the ggml backends actually import - #330
Merged
Conversation
1.9.1 was supposed to have ended the Visual C++ Redistributable dependency. It did not. ggml-base.dll and ggml-cpu.dll are compiled with OpenMP and import vcomp140.dll, which is part of that redistributable and not of Windows, so on a clean machine whisper-stt-server still dies in the loader before main() and transcription and captions fail with the unactionable timeout before-pack.cjs describes. The guard added with that fix reported the payload clean throughout, because it matched msvcp/vcruntime/concrt and `vcomp` starts with none of the three. Two things were wrong with it, and the second matters more than the missing prefix: It enumerated the members that had already bitten rather than the family, so it now covers msvcp, vcruntime, concrt, vcomp, vcamp and mfc. And it tested "imports a redistributable DLL" when the property worth testing is "imports a redistributable DLL that will not be there". Those differ exactly when the DLL ships alongside — which is the remedy — so the old wording forbade its own fix. It now ignores an import that is satisfied from the same directory, the same colocation that already carries the ffmpeg DLLs. stage-vcomp-runtime.mjs copies vcomp140.dll out of the Visual Studio redistributable directory, globbing the toolset tag because it moves with the compiler (VC143 on the 2022 runners, VC145 on a 2026 install), and preferring the redistributable copy over System32 because that is the one Microsoft licenses for redistribution. win.extraResources carries it like everything else in that folder. Shipping it rather than rebuilding whisper with -DGGML_OPENMP=OFF is deliberate. The DLL leaves the computation identical; dropping OpenMP swaps its scheduler for ggml's own and changes transcription throughput by an amount nobody has measured. 200 KB against that unknown is a cheap trade, and the alternative stays open — measure first. Verified both directions against the real payload: with the DLL staged the check passes, and with it removed it names ggml-base.dll and ggml-cpu.dll. The negative case is the one that matters, since it is precisely what the previous version could not see. Found by walking the full import closure of the shipped payload rather than by running anything, which is the only method available for an absence. The same sweep leaves one unresolved item recorded in the documentation and not addressed here: wgc-capture.exe imports mf.dll, mfplat.dll and mfreadwrite.dll, and Media Foundation is absent from Windows N editions unless the user installs the Media Feature Pack. Recording would fail there with the same 0xC0000135. N editions are sold in Europe.
…it is Two review findings, both real, and the first turned out to have a second half. A directory answers `readdirSync` by name exactly as a file does, so an entry called `vcomp140.dll` marked the import satisfied while the loader would still have found nothing — a guard reporting clean for the one reason it exists. That was the finding. The same confusion sat on the scanning side, where such an entry was opened as a binary and killed the hook with a raw `EISDIR`: the build stopped, which is right, on a message naming nothing, which is not. One list of regular files now feeds both questions. This directory really does hold subdirectories — the vendored ffmpeg SDK — so the distinction is unlikely rather than hypothetical. Root discovery no longer assumes the two default install paths. It reuses `findVcVarsAll` from scripts/msvcEnv.mjs, the same lookup the two native build scripts already run, which consults VCVARSALL, then vswhere, then VSINSTALLDIR, then sweeps for the pre-release channels vswhere does not enumerate. That covers the custom installation path the review raised, without adding a vswhere dependency this repo had already decided it could not rely on — vswhere is not on PATH on the machine this was written on, which is why msvcEnv grew its fallbacks in the first place. The derived root is searched alone when it yields anything, which prefers the toolchain that actually compiled the helpers and avoids re-walking a large tree twice. Verified all three states against the real payload: with the DLL present the check passes; with a directory of that name it refuses and names ggml-base.dll and ggml-cpu.dll; with the file simply absent, likewise. Staging still resolves the redistributable copy, now through discovery rather than a fixed path.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Windows build stages ChangesWindows runtime packaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant WindowsBuild
participant RuntimeStager
participant VisualStudio
participant NativePayload
participant PackagingCheck
WindowsBuild->>RuntimeStager: Run stage:vcomp
RuntimeStager->>VisualStudio: Find x64 vcomp140.dll candidates
VisualStudio-->>RuntimeStager: Return candidates
RuntimeStager->>NativePayload: Copy newest supported DLL
WindowsBuild->>PackagingCheck: Scan native binaries
PackagingCheck->>NativePayload: Check shipped redistributable DLLs
NativePayload-->>PackagingCheck: Return colocated DLL names
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
scripts/stage-vcomp-runtime.mjs (1)
45-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winResolve Visual Studio installations instead of relying on fixed roots.
If Visual Studio or Build Tools uses a custom installation path, the native build can succeed but
findRedistCopies()returns no candidates and stops packaging. Usevswhere.exeto discover installed instances and scan their installation paths. If custom paths are unsupported, document and enforce that constraint in CI.# Run in PowerShell on a Windows build agent. $vswhere = Join-Path ${env:ProgramFiles(x86)} "Microsoft Visual Studio\Installer\vswhere.exe" $roots = & $vswhere -products * -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath $roots | ForEach-Object { Get-ChildItem -Path $_ -Filter "vcomp140.dll" -Recurse -File -ErrorAction SilentlyContinue | Where-Object { $_.FullName -match "\\Redist\\" -and $_.FullName -match "\\x64\\" } | Select-Object -ExpandProperty FullName }🤖 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 `@scripts/stage-vcomp-runtime.mjs` around lines 45 - 50, Update findRedistCopies() to discover Visual Studio and Build Tools installations through vswhere.exe, including custom installation paths, instead of only scanning the two hard-coded roots. Query instances requiring Microsoft.VisualStudio.Component.VC.Tools.x86.x64, use each returned installationPath as a scan root, and preserve filtering for the required vcomp140.dll redist candidates.
🤖 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 `@scripts/before-pack.cjs`:
- Around line 366-377: Update the shipped-entry collection near
scanned/offenders to include only directory entries that are regular files, not
directories or other filesystem types. Use the resulting regular-file name set
when applying the !shipped.has(d.toLowerCase()) exclusion in the offenders
calculation.
---
Nitpick comments:
In `@scripts/stage-vcomp-runtime.mjs`:
- Around line 45-50: Update findRedistCopies() to discover Visual Studio and
Build Tools installations through vswhere.exe, including custom installation
paths, instead of only scanning the two hard-coded roots. Query instances
requiring Microsoft.VisualStudio.Component.VC.Tools.x86.x64, use each returned
installationPath as a scan root, and preserve filtering for the required
vcomp140.dll redist candidates.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c90e91e-a421-4fcc-a672-e7c7b03ba216
📒 Files selected for processing (5)
THIRD-PARTY-NOTICES.mdpackage.jsonscripts/before-pack.cjsscripts/stage-vcomp-runtime.mjstechnical-documentation/engineering/build-and-packaging.md
EtienneLescot
force-pushed
the
fix/win-openmp-runtime
branch
from
August 10, 2026 16:26
2bb038a to
3cef11a
Compare
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.
The bug the guard was supposed to prevent, still shipping
1.9.1 was meant to end the Visual C++ Redistributable dependency. It did not.
vcomp140.dllis Microsoft's OpenMP runtime. It comes with the Visual C++ Redistributable and is not part of Windows — verified on this machine: version 14.51.36247, product "Microsoft Visual Studio", the same version as the redistributable entries in Programs and Features.On a clean machine
whisper-stt-server.exetherefore still dies in the loader beforemain(), and transcription and captions fail with the unactionable timeoutbefore-pack.cjsdescribes. It is in the 1.9.1 release and in the package currently in Store certification.Why the guard missed it
Two defects, and the second is the more useful lesson.
It enumerated the members that had already bitten, not the family.
msvcp/vcruntime/concrt— andvcompstarts with none of them. Now:msvcp,vcruntime,concrt,vcomp,vcamp,mfc.It tested the wrong property. It asked "does this import a redistributable DLL?" when what matters is "does this import a redistributable DLL that will not be there?" Those differ in exactly one case — when the DLL ships alongside — which is the remedy. So the check as written forbade its own fix. It now ignores an import satisfied from the same directory, the same colocation that already carries the ffmpeg DLLs.
The fix
scripts/stage-vcomp-runtime.mjscopiesvcomp140.dllout of the Visual Studio redistributable directory into the payload. It globs the toolset tag, which moves with the compiler (Microsoft.VC143.OpenMPon the 2022 runners,VC145on a 2026 install), and prefers the redistributable copy overSystem32because that is the one Microsoft licenses for redistribution.win.extraResourcescarries it like everything else in that folder. Wired intobuild:winandbuild:win:store.Shipping it rather than
-DGGML_OPENMP=OFFis deliberate. The DLL leaves the computation identical. Dropping OpenMP swaps its scheduler for ggml's own and changes transcription throughput by an amount nobody has measured — 200 KB against that unknown is a cheap trade, and the alternative stays open. Measure first.Verified both directions
With the DLL staged, the check passes. With it removed:
The negative case is the one that matters — it is precisely what the previous version could not see. Run against the real payload, not a fixture.
How it was found, and what else the sweep turned up
By walking the full import closure of the shipped payload and asking which DLLs come from outside it. That is the only method available for an absence: no amount of running the app on a machine that has the redistributable can reveal a machine that does not.
71 distinct external DLLs. After this change every one of them is either shipped beside the payload or present on every Windows edition — with one exception, recorded in the documentation and deliberately not fixed here:
Media Foundation is absent from Windows N editions unless the user installs the Media Feature Pack. Recording would fail there with the same
0xC0000135that rejected 1.9.1. Untested and unhandled; N editions are sold in Europe.Summary by CodeRabbit
Bug Fixes
Documentation