From 71a884571c94fc32c8e20379172334d58b35b557 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 1 Dec 2021 23:09:12 +0100 Subject: [PATCH 1/3] cmake: add idf_component_optional_requires utility function It can be used in component CMakeLists.txt files when adding "weak" dependencies between component. A "weak" dependency from A to B is similar to target_link_libraries(A PRIVATE B), but it gets added only if B is included in the build. Use it instead of specifying B as part of PRIV_REQUIRES when the component can be built (even if with reduced functionality) without component B being available. --- tools/cmake/component.cmake | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tools/cmake/component.cmake b/tools/cmake/component.cmake index 5e3053dba5..e28a52f4ca 100644 --- a/tools/cmake/component.cmake +++ b/tools/cmake/component.cmake @@ -576,6 +576,28 @@ function(idf_component_mock) ) endfunction() +# idf_component_optional_requires +# +# @brief Add a dependency on a given component only if it is included in the build. +# +# Calling idf_component_optional_requires(PRIVATE dependency_name) has the similar effect to +# target_link_libraries(${COMPONENT_LIB} PRIVATE idf::dependency_name), only if 'dependency_name' +# component is part of the build. Otherwise, no dependency gets added. Multiple names may be given. +# +# @param[in] type of the dependency, one of: PRIVATE, PUBLIC, INTERFACE +# @param[in, multivalue] list of component names which should be added as dependencies +# +function(idf_component_optional_requires req_type) + set(optional_reqs ${ARGN}) + idf_build_get_property(build_components BUILD_COMPONENTS) + foreach(req ${optional_reqs}) + if(req IN_LIST build_components) + idf_component_get_property(req_lib ${req} COMPONENT_LIB) + target_link_libraries(${COMPONENT_LIB} ${req_type} ${req_lib}) + endif() + endforeach() +endfunction() + # # Deprecated functions # From 7a5ed12c6b163754a6d5a1f8d6fc5665c7edec5b Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 13 Dec 2021 10:04:43 +0100 Subject: [PATCH 2/3] lwip: make some of the component dependencies optional --- components/lwip/CMakeLists.txt | 19 +++++++++++++++++-- .../system/g1_components/CMakeLists.txt | 8 -------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/components/lwip/CMakeLists.txt b/components/lwip/CMakeLists.txt index acd587de92..dda2742f09 100644 --- a/components/lwip/CMakeLists.txt +++ b/components/lwip/CMakeLists.txt @@ -86,7 +86,6 @@ set(srcs "port/esp32/hooks/lwip_default_hooks.c" "port/esp32/debug/lwip_debug.c" "port/esp32/freertos/sys_arch.c" - "port/esp32/netif/dhcp_state.c" "port/esp32/netif/wlanif.c") if(CONFIG_LWIP_PPP_SUPPORT) @@ -148,11 +147,15 @@ if(CONFIG_LWIP_DHCPS) list(APPEND srcs "apps/dhcpserver/dhcpserver.c") endif() +if(CONFIG_LWIP_DHCP_RESTORE_LAST_IP) + list(APPEND srcs "port/esp32/netif/dhcp_state.c") +endif() + idf_component_register(SRCS "${srcs}" INCLUDE_DIRS "${include_dirs}" LDFRAGMENTS linker.lf REQUIRES vfs esp_wifi - PRIV_REQUIRES esp_eth esp_netif tcpip_adapter nvs_flash openthread) + PRIV_REQUIRES esp_netif tcpip_adapter) # lots of LWIP source files evaluate macros that check address of stack variables target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-address) @@ -174,3 +177,15 @@ set_source_files_properties( PROPERTIES COMPILE_FLAGS -Wno-type-limits ) + +if(CONFIG_OPENTHREAD_ENABLED) + idf_component_optional_requires(PRIVATE openthread) +endif() + +if(CONFIG_ETH_ENABLED) + idf_component_optional_requires(PRIVATE esp_eth) +endif() + +if(CONFIG_LWIP_DHCP_RESTORE_LAST_IP) + idf_component_optional_requires(PRIVATE nvs_flash) +endif() diff --git a/tools/test_apps/system/g1_components/CMakeLists.txt b/tools/test_apps/system/g1_components/CMakeLists.txt index 3a6de2bbf7..98d99530e6 100644 --- a/tools/test_apps/system/g1_components/CMakeLists.txt +++ b/tools/test_apps/system/g1_components/CMakeLists.txt @@ -37,8 +37,6 @@ set(extra_components_which_shouldnt_be_included # efuse and app_update should be removed from G1 build; # [refactor-todo]: see if the dependency from spi_flash can be made weak bootloader_support - # console is pulled in by openthread, which should be removed - console # [refactor-todo]: should cxx be in G1? Can it exist without FreeRTOS? cxx # [refactor-todo]: driver is a dependency of esp_pm, esp_timer, spi_flash, vfs, esp_wifi, ${IDF_TARGET} @@ -79,9 +77,6 @@ set(extra_components_which_shouldnt_be_included # esptool_py is a dependency of bootloader, esp_wifi, app_update, partition_table, all of which # should be removed from G1-only build. esptool_py - # a recent addition to the G1-only build, ieee802154 is a dependency of openthread, to be removed - # once openthread is (easily) removed. - ieee802154 # lwip is a common component due to "sys/socket.h" header. # [refactor-todo] consider adding a system-level sys/socket.h in newlib instead lwip @@ -90,11 +85,8 @@ set(extra_components_which_shouldnt_be_included mbedtls # nvs_flash is required by: # esp_system — no obvious reason, [refactor-todo] why? - # lwip — can be turned into a weak dependency # esp_wifi, esp_phy — both should be removed nvs_flash - # openthread is a dependency of lwip, it is easy to turn it into a conditional one - openthread # partition_table is pulled in by app_update, esptool_py, bootloader; all to be removed partition_table # pthread is required by esp_system (for initialization only, can be made a weak dependency) From 8df306ab6007941851485bee46fb5c33c1da0d55 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 13 Dec 2021 10:52:22 +0100 Subject: [PATCH 3/3] freertos,esp_system: make dependencies on some components optional Dependencies on gdbstub, espcoredump, app_trace will only be added if these components are present in the build. --- components/esp_system/CMakeLists.txt | 21 +++++++++++++++++-- components/esp_system/startup.c | 3 +++ components/freertos/CMakeLists.txt | 18 ++++++++++++---- .../FreeRTOS-Kernel/portable/xtensa/port.c | 2 ++ .../system/g1_components/CMakeLists.txt | 10 --------- tools/test_apps/system/panic/CMakeLists.txt | 2 +- 6 files changed, 39 insertions(+), 17 deletions(-) diff --git a/components/esp_system/CMakeLists.txt b/components/esp_system/CMakeLists.txt index 77b7f567f3..53c5cd7ced 100644 --- a/components/esp_system/CMakeLists.txt +++ b/components/esp_system/CMakeLists.txt @@ -40,8 +40,8 @@ else() # [refactor-todo] requirements due to init code, # should be removable once using component init functions # link-time registration is used. - esp_pm app_update nvs_flash pthread app_trace esp_gdbstub - espcoredump esp_phy efuse + esp_pm app_update nvs_flash pthread + esp_phy efuse LDFRAGMENTS "linker.lf" "app.lf") add_subdirectory(port) @@ -72,3 +72,20 @@ endif() # Force linking UBSAN hooks. If UBSAN is not enabled, the hooks will ultimately be removed # due to -ffunction-sections -Wl,--gc-sections options. target_link_libraries(${COMPONENT_LIB} INTERFACE "-u __ubsan_include") + + +# [refactor-todo] requirements due to init code, should be removable +# once link-time registration of component init functions is used. +if(CONFIG_APPTRACE_ENABLE) + idf_component_optional_requires(PRIVATE app_trace) +endif() + +if(CONFIG_ESP_COREDUMP_ENABLE) + idf_component_optional_requires(PRIVATE espcoredump) +endif() + +# [refactor-todo] requirement from the panic handler, +# need to introduce panic "event" concept to remove this dependency (IDF-2194) +if(CONFIG_ESP_SYSTEM_PANIC_GDBSTUB) + idf_component_optional_requires(PRIVATE esp_gdbstub) +endif() diff --git a/components/esp_system/startup.c b/components/esp_system/startup.c index 8886fda52a..34d53099e2 100644 --- a/components/esp_system/startup.c +++ b/components/esp_system/startup.c @@ -45,7 +45,10 @@ #include "esp_core_dump.h" #endif +#if CONFIG_APPTRACE_ENABLE #include "esp_app_trace.h" +#endif + #include "esp_private/dbg_stubs.h" #include "esp_pm.h" #include "esp_private/pm_impl.h" diff --git a/components/freertos/CMakeLists.txt b/components/freertos/CMakeLists.txt index 69961a95ef..93e3c5aa39 100644 --- a/components/freertos/CMakeLists.txt +++ b/components/freertos/CMakeLists.txt @@ -68,10 +68,11 @@ if(CONFIG_ESP32_IRAM_AS_8BIT_ACCESSIBLE_MEMORY) list(APPEND srcs "FreeRTOS-Kernel/portable/xtensa/xtensa_loadstore_handler.S") endif() -# esp_timer is required by FreeRTOS because we use esp_tiemr_get_time() to do profiling -# app_trace is required by FreeRTOS headers only when CONFIG_APPTRACE_SV_ENABLE=y, -# REQUIRES can't depend on config options, so always require it. -set(required_components app_trace esp_timer) +# esp_timer is required by FreeRTOS when we use esp_timer_get_time() to do profiling +# [refactor-todo]: make this an optional requirement depending on CONFIG_FREERTOS_RUN_TIME_STATS_USING_ESP_TIMER +# Note, many other components implicitly include esp_timer.h via freertos portmacro.h and some +# components implicitly depend on esp_timer via freertos. +set(required_components esp_timer) idf_component_register(SRCS "${srcs}" INCLUDE_DIRS ${include_dirs} @@ -104,3 +105,12 @@ set_source_files_properties( # we can't establish dependency on what we don't yet know, so we force the # linker to not drop this symbol. target_link_libraries(${COMPONENT_LIB} INTERFACE "-u app_main") + +if(CONFIG_APPTRACE_SV_ENABLE) + # FreeRTOS headers have a dependency on app_trace when SystemView tracing is enabled + idf_component_optional_requires(PUBLIC app_trace) +elseif(CONFIG_APPTRACE_ENABLE) + # [refactor-todo]: port.c has a dependency on esp_apptrace_init, for running it on APP CPU. + # this should be resolved when link-time registration of startup functions is added. + idf_component_optional_requires(PRIVATE app_trace) +endif() diff --git a/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c b/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c index 3508653730..70a5b38494 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c +++ b/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c @@ -61,7 +61,9 @@ #include "esp_system.h" #include "esp_log.h" #include "esp_int_wdt.h" +#ifdef CONFIG_APPTRACE_ENABLE #include "esp_app_trace.h" /* Required for esp_apptrace_init. [refactor-todo] */ +#endif #include "FreeRTOS.h" /* This pulls in portmacro.h */ #include "task.h" /* Required for TaskHandle_t, tskNO_AFFINITY, and vTaskStartScheduler */ #include "port_systick.h" diff --git a/tools/test_apps/system/g1_components/CMakeLists.txt b/tools/test_apps/system/g1_components/CMakeLists.txt index 98d99530e6..ad57901d32 100644 --- a/tools/test_apps/system/g1_components/CMakeLists.txt +++ b/tools/test_apps/system/g1_components/CMakeLists.txt @@ -22,9 +22,6 @@ set(extra_allowed_components # These components are currently included into "G1" build, but shouldn't. # After removing the extra dependencies, remove the components from this list as well. set(extra_components_which_shouldnt_be_included - # app_trace is a public dependency of FreeRTOS because of SystemView trace headers. - # To fix, convert this into a weak public dependency conditional on CONFIG_APPTRACE_SV_ENABLE. - app_trace # app_update gets added because of bootloader_support, spi_flash, espcoredump, esp_system. # bootloader_support, spi_flash, espcoredump should be removed from dependencies; # esp_system code that reads app version should be made conditional on app_update being included @@ -51,8 +48,6 @@ set(extra_components_which_shouldnt_be_included esp_eth # esp_event is a dependency of esp_wifi, esp_eth. Both should be removed from G1. esp_event - # esp_gdbstub is a dependency of esp_system, can easily be made conditional on related Kconfig option. - esp_gdbstub # esp_netif is a dependency of lwip and esp_event, should disappear once lwip is removed. esp_netif # esp_phy is a dependency of esp_system and esp_wifi. For the former, it can be made a weak dependency. @@ -69,11 +64,6 @@ set(extra_components_which_shouldnt_be_included # esp_wifi is a dependency of lwip. # [refactor-todo]: investigate making esp_wifi a conditional dependency. esp_wifi - # Similar to esp_gdbstub, this dependency from esp_system can be made weakly dependent - # on CONFIG_ESP_COREDUMP_ENABLE. - # [refactor-todo]: how will the user set CONFIG_ESP_COREDUMP_ENABLE if the component - # is not included into the build, hence Kconfig option is not visible in menuconfig? - espcoredump # esptool_py is a dependency of bootloader, esp_wifi, app_update, partition_table, all of which # should be removed from G1-only build. esptool_py diff --git a/tools/test_apps/system/panic/CMakeLists.txt b/tools/test_apps/system/panic/CMakeLists.txt index a53a793ff7..2132f5b91f 100644 --- a/tools/test_apps/system/panic/CMakeLists.txt +++ b/tools/test_apps/system/panic/CMakeLists.txt @@ -2,7 +2,7 @@ # in this exact order for cmake to work correctly cmake_minimum_required(VERSION 3.5) -set(COMPONENTS esptool_py main) +set(COMPONENTS main espcoredump esp_gdbstub) include($ENV{IDF_PATH}/tools/cmake/project.cmake) project(test_panic)