cmake: default to linking 'app' with the interface library 'FS' #8533
Conversation
This file contains 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
This patch removes the need for application build script code to explicitly link 'app' with a filesystem implementation. It does this by introducing a zephyr interface library called 'FS' that contains the usage requirements for linking with the filesystem library subsys__fs and using Kconfig to default to linking the 'app' library with this interface library. Signed-off-by: Sebastian Bøe <[email protected]>
It is not necessary to link 'app' with ELMFAT or NFFS because their usage requirements are covered by the 'APP_LINK_WITH_MBEDTLS' mechanism that automatically links 'app' with the interface library FS. This patch removes the redundant target_link_libraries invocations. Signed-off-by: Sebastian Bøe <[email protected]>
nashif
reviewed
Jun 22, 2018
| @@ -33,6 +33,14 @@ config SYS_LOG_FS_LEVEL | |||
|
|
|||
| if FILE_SYSTEM | |||
|
|
|||
| config APP_LINK_WITH_FS | |||
There was a problem hiding this comment.
Codecov Report
@@ Coverage Diff @@
## master #8533 +/- ##
==========================================
- Coverage 63.8% 63.14% -0.66%
==========================================
Files 426 447 +21
Lines 40890 47803 +6913
Branches 6921 8802 +1881
==========================================
+ Hits 26089 30185 +4096
- Misses 11666 14006 +2340
- Partials 3135 3612 +477
Continue to review full report at Codecov.
|
pboettch
reviewed
Jun 25, 2018
| zephyr_library_link_libraries(FS) | ||
|
|
||
| target_link_libraries_ifdef(CONFIG_FAT_FILESYSTEM_ELM FS INTERFACE ELMFAT) | ||
| target_link_libraries_ifdef(CONFIG_FILE_SYSTEM_NFFS FS INTERFACE NFFS) |
There was a problem hiding this comment.
Shouldn't these two lines be in the CMakeList.txt of NFFS and ELMFAT respectively?
There was a problem hiding this comment.
Maybe, but they can't because FS can only be modified in the CMakeLists.txt directory that created it.
|
in subsys/settings/src/CMakeLists.txt, is it necessary to keep following line: |
The settings subsystem has been adding nffs's include dir to the global set of paths. Presumably because app's will need acces. But this is no longer necessary as we default to linking 'app' with FS, which again has the NFFS include paths. Signed-off-by: Sebastian Bøe <[email protected]>
It is not. Fixed. |
This was referenced Jun 26, 2018
|
recheck |
nashif
approved these changes
Jul 25, 2018
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.
This patch removes the need for application build script code to
explicitly link 'app' with a filesystem implementation.
It does this by introducing a zephyr interface library called 'FS'
that contains the usage requirements for linking with the filesystem
library subsys__fs and using Kconfig to default to linking the 'app'
library with this interface library.