[RFC][DNM] cmake: RFC for single libraries to use CMake dependencies and remove whole-archive

[tejlmand]

This RFC present a possibility on how subsys Bluetooth and subsys net can be changed into a single library while still use the zephyr KConfig principle as today.
See issue: #8441

This principle can further be extended to other libraries in Zephyr.

The RFC remove the need to use --start-group /--end-group as well as the --whole-archive
The changes made in this RFC remove the need for whole archive on subsys_net and subsys_bluetooth as seen in the output below, where those libraries are outside the whole-archive.
The whole arhive issue is also mentioned in: #6961

Furthermore net and bluetooth are removed from ${ZEPHYR_LIBS_PROPERTY} as described in #8441

This also ensures that CMake normal dependency handling is obtained.

Furthermore, specific compile flags on sources, such as those found in
subsys/bluetooth/controller/bluetooth_controller.cmake (was: subsys/bluetooth/controller/CMakeLists.txt) are now done using,
SET_SOURCE_FILES_PROPERTIES( ${PRIVATE_SOURCES} PROPERTIES COMPILE_FLAGS -Ofast), which has two advantages over current form:

1. Compile clags as property on files automatically has precendence over library link flags, thus no special handling is needed as opposed to: PR cmake: Give zephyr_library_* flags precedence over zephyr_* flags #5110
   where the order of libraries is very important
2. Compile flags only added for source files are not accidentally propagate, cmake: Propagation of library specific compile flag #8438

The principle used when including *.cmake files are further described here:
https://crascit.com/2016/01/31/enhanced-source-file-handling-with-target_sources/

Another nice part of the proposed structured, is that only the entry point to the library contains a CMakeLists.txt file, i.e. subsys/net/CMakeLists.txt and subsys/bluetooth/CMakeLists.txt
and other files are given more descriptive names,making navigation easier when having multiple *.cmake files open, instead of all files being named CMakeLists.txt

Not addressed in this RFC / commit:
Zephyr has an extended version of add_subdirectory called add_subdirectory_ifdef.
In this RFC, the straight forward way of using
if(KCONFIG_FLAG)
include(...)
endif()
but I see no problem in creating an extended version, called include_ifdef(KCONFIG_FLAG ....) if desired.

--whole-archive has not been completely removed, as there are still libraries left which needs tthis flag, but I hope the principle used is enough for discussion and then rest of libraries can be addressed in future commit.

Output from Zephyr when linking sample app bluetooth/peripheral_hr:
[139/144] : && ccache /opt/zephyr-sdk/sysroots/x86_64-pokysdk-linux/usr/bin/arm-zephyr-eabi/arm-zephyr-eabi-gcc zephyr/CMakeFiles/zephyr_prebuilt.dir/misc/empty_file.c.obj -o zephyr/zephyr_prebuilt.elf -T zephyr/linker.cmd -Wl,-Map=/projects/github/tejlmand/zephyr/samples/bluetooth/peripheral_hr/build/zephyr/zephyr.map -u_OffsetAbsSyms -u_ConfigAbsSyms -e__start -Wl,--whole-archive libapp.a zephyr/libzephyr.a zephyr/arch/arm/core/libarch__arm__core.a zephyr/arch/arm/core/cortex_m/libarch__arm__core__cortex_m.a zephyr/arch/arm/core/cortex_m/mpu/libarch__arm__core__cortex_m__mpu.a zephyr/lib/libc/minimal/liblib__libc__minimal.a zephyr/drivers/gpio/libdrivers__gpio.a zephyr/drivers/serial/libdrivers__serial.a zephyr/drivers/entropy/libdrivers__entropy.a -Wl,--no-whole-archive zephyr/kernel/libkernel.a zephyr/CMakeFiles/offsets.dir/arch/arm/core/offsets/offsets.c.obj -L"/opt/zephyr-sdk/sysroots/armv5-zephyr-eabi/usr/lib/arm-zephyr-eabi/6.2.0/armv7e-m" -L/projects/github/tejlmand/zephyr/samples/bluetooth/peripheral_hr/build/zephyr -lgcc -Wl,--print-memory-usage zephyr/subsys/bluetooth/libsubsys_bluetooth.a -u_hci_driver_init zephyr/subsys/net/libsubsys_net.a zephyr/libzephyr.a zephyr/kernel/libkernel.a zephyr/liboffsets.a -nostartfiles -nodefaultlibs -nostdlib -static -no-pie -Wl,-X -Wl,-N -Wl,--gc-sections -Wl,--build-id=none && :

EDIT: This is part of umbrella issue: #8827

[codecov-io]

Codecov Report

Merging #8451 into master will increase coverage by 2.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8451      +/-   ##
==========================================
+ Coverage   52.31%   54.36%   +2.05%     
==========================================
  Files         212      208       -4     
  Lines       25937    24958     -979     
  Branches     5589     5394     -195     
==========================================
  Hits        13569    13569              
+ Misses      10122     9143     -979     
  Partials     2246     2246

Impacted Files	Coverage Δ
include/device.h	100% <ø> (ø)	⬆️
subsys/usb/usb_device.c
subsys/net/lib/mqtt/mqtt.c
subsys/usb/usb_descriptor.c
subsys/bluetooth/host/uuid.c

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a166ba7...4be4845. Read the comment docs.

[tejlmand]

Note, the main part of this RFC is the reduced numbers of libs which then leads to the fact that it will be easier to remove the -Wl,--whole-archive.
But the fact that a library really becomes a library and not just a collection of the files in a subfolder gives a much nicer architectural structure.
This gives a better correspondence between a library and a subsystem in Zephyr context.
Which again is an advantage when understanding the structure in Zephyr.

This especially become apparent in an IDE, an example from Eclipse can be seen below.
The screen shots show how the numbers of libs marked in read is reduced by this RFC.
The libs marked in blue are other candidates to merge to single libs.

and after the change:

[carlescufi]

@tejlmand can you rebase please?
@SebastianBoe any thoughts on this one?

[SebastianBoe]

Compile flags only added for source files are not accidentally propagate, #8438

This point can be removed since #8438 has been resolved (the fix is blocked by awaiting PR review).

[SebastianBoe]

Core build script code such as boilerplate.cmake must be agnostic to the details of subsystems.

For this use-case, one should take advantage of the CONFIG_APP_LINK_WITH_${boilerplate_lib_upper_case}
feature. See above foreach loop.

[tejlmand]

yes, as I stated in the commit message:

This is currently a temporary solution to ensure all samples and tests
builds correctly.

As part of the RFC it should be decided what the optimal approach for
linking should be.

The loop you propose is a candidate.
Another approach could be to target_link_libraries where the subsystem is actually included, e.g.
as part of:
https:/zephyrproject-rtos/zephyr/blob/master/subsys/CMakeLists.txt#L1-15

So when a subsystem is included, it is also linked to app.

[SebastianBoe]

This breaks userspace/kernelspace memory separation.

To protect kernelspace and userspace memory regions differently the linker needs to place them in different sections.

Today this is implemented by having CMakeLists.txt iterate over the libraries in ZEPHYR_LIBS_PROPERTY and guess based on their names which one's should be in kernelspace and which ones should be in userspace. Not great, I know.

It is implemented here:
https:/zephyrproject-rtos/zephyr/blob/master/CMakeLists.txt#L526

This breaks when you take subsys_bluetooth out of the global list of Zephyr CMake Libraries.

To do this re-organization we need to first find some other mechanism to split up the kernelspace and userspace memory regions.

[tejlmand]

Yes, this I wasn't aware about, that the list were used for placing libs different mem location.
See further comment below, where I list all places ZEPHYR_LIBS_PROPERTY is used

[SebastianBoe]

Sorry, which comment?

[galak]

@tejlmand any plans to clean this up for 1.14?