[RFC] llext-edk: capture full build properties for EDK export #98348
Open
pillo79 wants to merge 3 commits intozephyrproject-rtos:mainfrom
Open
[RFC] llext-edk: capture full build properties for EDK export #98348pillo79 wants to merge 3 commits intozephyrproject-rtos:mainfrom
pillo79 wants to merge 3 commits intozephyrproject-rtos:mainfrom
Conversation
fcf2334 to
7df1965
Compare
This changes the LLEXT EDK flag filtering to be done after the main Zephyr build is completed. Instead of filtering the flags via genexes, the full set is passed to the script along with the set of filters to be applied, and the filter is applied later. No functional change is intended. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
The current implementation of the LLEXT EDK exports only a partial set of compiler settings to the EDK build system, specifically only the ones defined by the Zephyr core (the 'zephyr_interface' target). Extending this to include application and module-specific settings (from the 'app' target and its dependencies) is tricky, because: - while evaluating the main CMakeLists.txt, the 'app' target is not fully configured yet, and - using generator expressions is not possible due to CMake not allowing expansion of $<COMPILE_LANGUAGE:...> outside of a proper compile context. To work around this, this change introduces a new CMake subdirectory (misc/llext_edk) that creates a dummy target which imports the build configuration from 'app', but overrides the C compile rules to simply output the resulting properties (defines, include directories, flags) to text files. The EDK generation script then reads those text files to get the full set of build properties, and includes them in the EDK tarball. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
Add an interface library to the test application that defines a macro whose value is later used in the extension. This verifies the full dependency chain for 'app' is evaluated during EDK generation. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
7df1965 to
514c30d
Compare
|
Contributor
edersondisouza
left a comment
There was a problem hiding this comment.
Interesting hack! Two questions:
1- I noticed that there seems to be less duplication of flags - like, less times the drivers include dir appears on the flags after your changes - is this expected? Why did we have some many duplicates anyway?
2 - Where is this documented as a stable behaviour? Would it make sense to have a link to that for future reference?
|
|
||
| # Filter out non LLEXT and LLEXT_EDK flags - and add required ones | ||
| llext_filter_zephyr_flags(LLEXT_REMOVE_FLAGS ${zephyr_flags} llext_filt_flags) | ||
| llext_filter_zephyr_flags(LLEXT_EDK_REMOVE_FLAGS ${llext_filt_flags} llext_filt_flags) |
Contributor
There was a problem hiding this comment.
After this, llext_filter_zephyr_flags isn't used anymore, right? Couldn't it be removed then?
Contributor
|
Ederson is the author of the EDK and I've reassigned it to match this |
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
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.



Not for merge in 4.3, obviously - at this stage, looking for feedback on the approach and especially if others had similar issues with the EDK.
The symptom
While developing the Arduino core, we encountered an issue compiling code that used the
fatfsmodule via the EDK; it looked like some defaults were being used in the extension, even though specific compile options were applied during the main Zephyr build.The actual problem
Tracking down the source revealed that the EDK is not exporting all build information, but only those defined by the Zephyr core (the
zephyr_interfacetarget). Modules do not interfere with the Zephyr core but insert themselves as dependencies of theapptarget, so transitive evaluation applies there only.Ruled-out solutions
The current way of reading properties (e.g.
zephyr_get_compile_options_for_lang) cannot be used for theapptarget, because at the time the main ZephyrCMakeLists.txtis evaluated, that target is not yet available (and definitely not fully configured).Trying to use generator expressions is also not possible, due to CMake not allowing expansion of
$<COMPILE_LANGUAGE:...>outside of a proper compile context. These expressions were stripped by complex code inget_*_for_langfunctions, but that's no longer possible if the actual flags are hidden in a genex. The process errors out at configuration time.The proposed solution
To work around this, this change introduces a new CMake subdirectory (
misc/llext_edk) that creates a dummy target which imports the build configuration fromapp, but overrides the C compile rules to simply output the resulting properties (defines, include directories, flags) to text files. The EDK generation script then reads those text files to get the full set of build properties, and includes them in the EDK tarball.Important
This (ab-)uses the CMake build process in a way that is often cited as an error - setting toolchain variables from a
CMakeLists.txtselectively overrides an existing configuration, and is typically discouraged. However, that's 1) documented, stable behavior and 2) exactly what we are after in this case: use every option from the existing toolchain, but "discard" the compiler executable.A test has been added to verify the functionality by checking if a symbol defined in an interface library that is a dependency of
appis properly exported to the extension.