Merge branch 'feature/cmake_weak_deps_helper' into 'master'

cmake: add idf_component_optional_requires utility function, simplify some component dependencies

Closes IDF-3086

See merge request espressif/esp-idf!16373

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
+                                    esp_pm app_update nvs_flash pthread
-                                    espcoredump esp_phy efuse
+                                    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()

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"

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
+# esp_timer is required by FreeRTOS when we use esp_timer_get_time() to do profiling
-# app_trace is required by FreeRTOS headers only when CONFIG_APPTRACE_SV_ENABLE=y,
+# [refactor-todo]: make this an optional requirement depending on CONFIG_FREERTOS_RUN_TIME_STATS_USING_ESP_TIMER
-# REQUIRES can't depend on config options, so always require it.
+# Note, many other components implicitly include esp_timer.h via freertos portmacro.h and some
-set(required_components app_trace esp_timer)
+# 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()

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"

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()

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
 #

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
@@ -37,8 +34,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}
@@ -53,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.
@@ -71,17 +64,9 @@ 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
-    # 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 +75,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)

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)