cmake: add more build options in line with automake#10834
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10834
No scan targets match the changed files in this PR. Review skipped.
|
Jenkins: retest this please |
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: APPROVE
Findings: 4 total — 3 posted, 1 skipped
Posted findings
- [Medium] App-bundle sub-option forcing runs before options are defined, degrading cache entries —
CMakeLists.txt:250-534 - [Medium] Inconsistent WOLFSSL_LINUX_KM gating silently no-ops some kernel options —
CMakeLists.txt:2940-2960 - [Low] Repeated empty placeholder comment blocks in options.h.in —
cmake/options.h.in:811-817
Skipped findings
- [Medium] New build options ship with no test/CI coverage
Review generated by Skoll via Claude/Codex
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
Skoll review (follow-up). Thanks for the earlier fixes — I confirmed all three of my previous comments are addressed (force_option via GLOBAL property preserves cache type/help; LINUXKM_PIE/BENCHMARKS now warn when WOLFSSL_LINUX_KM is off; the empty options.h.in parity headers are gone).
One blocking issue remains, plus a few suggestions/nits below.
SparkiDev
force-pushed
the
cmake_update_1
branch
2 times, most recently
from
0394385 to
da5c186
Compare
|
Jenkins: retest this please |
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: REQUEST_CHANGES
Findings: 3 total — 1 posted, 2 skipped
Posted findings
- [High] New RC2/ASCON/SM2/SM3/SM4 options define feature macros but never compile their source files —
CMakeLists.txt:2967
Skipped findings
- [Medium] force_option silently overrides an explicit user setting with no diagnostic
- [Low] A force_option for an option never declared via add_option silently has no effect
Review generated by Skoll via Claude/Codex
Updates - ~100 individual options added (ciphers, TLS features, debug/test, caches, kernel-module, key-size numerics, asm) + fixed WOLFSSL_32BIT's missing define. - 37 app-integration bundles (openssh, nginx, haproxy, openvpn, wpas, apachehttpd, jni, wolfclu, wolfsentry, ...) - each force-enables its sub-options and emits its defines; all build clean and match ./configure --enable-X exactly (miss=0/extra=0). - Each option wired in 3 places: add_option + define in CMakeLists.txt, #cmakedefine in options.h.in, source selection where needed. Verified by real libwolfssl.so builds.
|
Jenkins: retest this please |
4 participants
Description
Updates
Testing
None