-
Notifications
You must be signed in to change notification settings - Fork 8.3k
soc: nordic: handle -include for IAR #81446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
soc: nordic: handle -include for IAR #81446
Conversation
tejlmand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this cleanup, but please make the changes to the toolchain / compiler infrastructure instead.
soc/nordic/CMakeLists.txt
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should avoid compiler / toolchain specific handling inside the code as it makes support for multiple toolchains much harder.
Instead the code itself should be cleaned up and the include flag should be defined in the compiler properties and picked up where used.
For example like this:
set(include_flag $<TARGET_PROPERTY:compiler,include-file>)
set_source_files_properties(
validate_binding_headers.c
DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
PROPERTIES COMPILE_OPTIONS "${include_flag}$<JOIN:${dt_binding_includes},;${include_flag}>"
)
and then in https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/compiler/gcc/compiler_flags.cmake add a line like this:
set_compiler_property(PROPERTY include-file "-include$<SEMICOLON>")
# Note to @RobinKastberg: check if "-include;" is sufficient in above.
and in the correspong IAR compiler_flags.cmake you can then do similar:
set_compiler_property(PROPERTY include-file "-preinclude$<SEMICOLON>")
and there will no longer be any need for doing compiler check in the CMake code itself, plus a benefit is that another toolchain can just provide its own flag and doesn't have to add an extra if-clause to this CMake code.
cae1de5 to
ea534ef
Compare
1d4d1fa to
e3fa6b9
Compare
tejlmand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, a minor suggestion to keep a general safer pattern.
(devs tends to copy-paste code they find elsewhere)
e3fa6b9 to
096d51e
Compare
|
@RobinKastberg this needs rebasing to get past the unrelated CI failure |
096d51e to
ce322c5
Compare
CMakeLists.txt uses the C compiler parameter -include, This is causing issues for other toolchains and needs to generalized. Signed-off-by: Robin Kastberg <[email protected]>
ce322c5 to
267a790
Compare
CMakeLists.txt uses the C compiler parameter -include, for IAR this is --preinclude.