Fix Windows (MSYS2 MINGW64) build - #41
Conversation
Three fixes found while building on Windows with the MINGW64 toolchain: - Set _WIN32_WINNT/WINVER to 0x0A00 project-wide: MinGW headers default to an older Windows API level, hiding GetCurrentThreadStackLimits, CreateFile2, and other Win8+/Win10 APIs the code uses. - Same defines for the WAMR subproject (CREATEFILE2_EXTENDED_PARAMETERS in core/shared/platform/windows/win_file.c). - Build BoringSSL with OPENSSL_NO_ASM on Windows: its x86_64 perlasm needs NASM, which Meson's CMake subproject translation does not drive, so the asm objects were silently missing and linking failed with undefined references (aes_hw_*, ChaCha20_*, ecp_nistz256_*, ...). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR modifies Meson build files to improve Windows/MinGW builds: it disables BoringSSL assembly optimizations via OPENSSL_NO_ASM on MinGW, and adds Windows 10 API version macros (_WIN32_WINNT, WINVER) as compile definitions in both the main meson build and the wasm-micro-runtime vendor package. ChangesWindows Build Configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Related Issues: None specified. Related PRs: None specified. Suggested labels: build, windows, meson Suggested reviewers: theMackabu PoemA rabbit tweaks the build with care, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![lintling[bot]](https://avatars.githubusercontent.com/u/289077755?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found 1 actionable comment.
The diff looks tidy and the changes are purposeful, though one build-correctness concern is worth flagging before merge.
Review info
Files selected for processing (3)
meson/deps/meson.buildmeson/meson.buildvendor/packagefiles/wasm-micro-runtime/meson.buildRun details
- Run ID:
309416d1-450c-47aa-b33f-238479ac9bf1- Delivery ID:
57366260-77f9-11f1-9c30-cd0105d74627
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
meson/deps/meson.build (1)
26-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant explicit
-DOPENSSL_NO_ASMcompile args.Setting the
OPENSSL_NO_ASMCMake variable already causes BoringSSL's ownCMakeLists.txtto add the-DOPENSSL_NO_ASMdefinition for its targets, so the explicitappend_compile_argscalls are likely unnecessary duplication. Not harmful, but worth double-checking whether they're needed for correctness given howcmake.subproject_optionscompile args interact with the subproject's own build files.🤖 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 `@meson/deps/meson.build` around lines 26 - 30, The BoringSSL Meson setup is redundantly adding the same OPENSSL_NO_ASM definition twice. In the meson/deps logic around boringssl_opts, keep the CMake define via add_cmake_defines and remove the explicit append_compile_args calls for both c and cpp unless you confirm they are required by the subproject translation path; use the existing boringssl_opts configuration as the place to make this change.
🤖 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.
Nitpick comments:
In `@meson/deps/meson.build`:
- Around line 26-30: The BoringSSL Meson setup is redundantly adding the same
OPENSSL_NO_ASM definition twice. In the meson/deps logic around boringssl_opts,
keep the CMake define via add_cmake_defines and remove the explicit
append_compile_args calls for both c and cpp unless you confirm they are
required by the subproject translation path; use the existing boringssl_opts
configuration as the place to make this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ec34e4c-33af-45ce-a0a7-c8834fbac126
📒 Files selected for processing (3)
meson/deps/meson.buildmeson/meson.buildvendor/packagefiles/wasm-micro-runtime/meson.build
1 participant
Three fixes found while building on Windows with the MINGW64 toolchain:
an older Windows API level, hiding GetCurrentThreadStackLimits,
CreateFile2, and other Win8+/Win10 APIs the code uses.
core/shared/platform/windows/win_file.c).
NASM, which Meson's CMake subproject translation does not drive, so the
asm objects were silently missing and linking failed with undefined
references (aes_hw_, ChaCha20_, ecp_nistz256_*, ...).
Summary by CodeRabbit