cmake: new add_option_defs helper, add KEYLOG_EXPORT build param #8174
Conversation
|
Can one of the admins verify this patch? |
| endif() | ||
|
|
||
| if(DEFINED ${NAME}) | ||
| list(FIND VALUES ${${NAME}} IDX) | ||
| if(DEFINED ${arg_NAME}) |
There was a problem hiding this comment.
I don't understand purpose this if() in original function. I transformed it mechanically to new args so it should work as before.
when building there is no difference in CMakeCache.txt before and after, so this branch either never triggered or mechanical transformation works as before.
There was a problem hiding this comment.
OK, I think I get it. It triggers when build options is passed via command line with value which is not among the list of supported values, like we pass "ON" for values like "yes|no" This works with booleans, but doesn't it break build options where string input is expected?
There was a problem hiding this comment.
@redbaron sorry accidentally deleted my previous response.
You are correct, this looks like it squashes non-boolean values inappropriately. Could you add a check for this case, to ensure this doesn't happen? Something like this to fall back to defaults maybe?
if(DEFINED ${NAME})
list(FIND VALUES ${${NAME}} IDX)
# Apply override only if VALUES is yes/no
if (VALUES STREQUAL "yes;no" OR VALUES STREQUAL "no;yes")
if (${IDX} EQUAL -1)
# Interpret CMake truthiness for boolean-like values
if(${${NAME}})
override_cache(${NAME} "yes")
else()
override_cache(${NAME} "no")
endif()
endif()
else()
# For non-boolean VALUES, reset to default or error on invalid input
if (${IDX} EQUAL -1)
message(WARNING "Invalid value for ${NAME}. Resetting to default: ${DEFAULT}")
set(${NAME} ${DEFAULT} CACHE STRING "${HELP_STRING}")
endif()
endif()
endif()
|
Hi @redbaron Thanks for opening this PR. We require contributors to be approved by the wolfSSL team. Could you please send an email to [email protected] to request an application form? Thanks, |
Add add_option_defs helper which cover simple case of setting compile definitions when build option is selected. New helper uses named arguments and adds following QoL improvements: - optionally infer default values from list of possible values. First value of allowed values list becomes default. - optionally append default value to helper string. Helper string always reflects accurate default value. - support adding defines if build option is set. It covers many simple cases like new WOLFSSL_KEYLOG_EXPORT.
redbaron
force-pushed
the
keylog-export
branch
from
ff6c097
to
a8fda5f
Compare
|
@redbaron is approved as a wolfSSL contributor |
| endif() | ||
|
|
||
| if(DEFINED ${NAME}) | ||
| list(FIND VALUES ${${NAME}} IDX) | ||
| if(DEFINED ${arg_NAME}) |
There was a problem hiding this comment.
@redbaron sorry accidentally deleted my previous response.
You are correct, this looks like it squashes non-boolean values inappropriately. Could you add a check for this case, to ensure this doesn't happen? Something like this to fall back to defaults maybe?
if(DEFINED ${NAME})
list(FIND VALUES ${${NAME}} IDX)
# Apply override only if VALUES is yes/no
if (VALUES STREQUAL "yes;no" OR VALUES STREQUAL "no;yes")
if (${IDX} EQUAL -1)
# Interpret CMake truthiness for boolean-like values
if(${${NAME}})
override_cache(${NAME} "yes")
else()
override_cache(${NAME} "no")
endif()
endif()
else()
# For non-boolean VALUES, reset to default or error on invalid input
if (${IDX} EQUAL -1)
message(WARNING "Invalid value for ${NAME}. Resetting to default: ${DEFAULT}")
set(${NAME} ${DEFAULT} CACHE STRING "${HELP_STRING}")
endif()
endif()
endif()
Description
Add
add_option_defshelper which cover simple case of setting compile definitions when build option is selected. New helper uses named arguments and adds following QoL improvements:WOLFSSL_KEYLOG_EXPORTFixes #8165
Testing
Checklist