From ae7d1fff49f60624b8a0008f21ddf90d4a1512d9 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 1 Dec 2021 23:00:01 +0100 Subject: [PATCH 1/2] cmake: add component dependency graph generation helpers These optional feature produces a graphviz file showing component dependencies. It is useful for debugging reasons why certain components got added to the build. --- Kconfig | 5 +++ tools/cmake/build.cmake | 3 ++ tools/cmake/component_deps.dot.in | 30 +++++++++++++++ tools/cmake/depgraph.cmake | 61 +++++++++++++++++++++++++++++++ tools/cmake/idf.cmake | 1 + tools/cmake/project.cmake | 3 ++ 6 files changed, 103 insertions(+) create mode 100644 tools/cmake/component_deps.dot.in create mode 100644 tools/cmake/depgraph.cmake diff --git a/Kconfig b/Kconfig index 1325799097..8edda6b8a5 100644 --- a/Kconfig +++ b/Kconfig @@ -22,6 +22,11 @@ mainmenu "Espressif IoT Development Framework Configuration" bool default "n" + config IDF_TARGET_ARCH + string + default "riscv" if IDF_TARGET_ARCH_RISCV + default "xtensa" if IDF_TARGET_ARCH_XTENSA + config IDF_TARGET # This option records the IDF target when sdkconfig is generated the first time. # It is not updated if environment variable $IDF_TARGET changes later, and diff --git a/tools/cmake/build.cmake b/tools/cmake/build.cmake index 3352238ef6..94a2f8ccc5 100644 --- a/tools/cmake/build.cmake +++ b/tools/cmake/build.cmake @@ -221,13 +221,16 @@ function(__build_expand_requirements component_target) get_property(reqs TARGET ${component_target} PROPERTY REQUIRES) get_property(priv_reqs TARGET ${component_target} PROPERTY PRIV_REQUIRES) + __component_get_property(component_name ${component_target} COMPONENT_NAME) foreach(req ${reqs}) + depgraph_add_edge(${component_name} ${req} REQUIRES) __build_resolve_and_add_req(_component_target ${component_target} ${req} __REQUIRES) __build_expand_requirements(${_component_target}) endforeach() foreach(req ${priv_reqs}) + depgraph_add_edge(${component_name} ${req} PRIV_REQUIRES) __build_resolve_and_add_req(_component_target ${component_target} ${req} __PRIV_REQUIRES) __build_expand_requirements(${_component_target}) endforeach() diff --git a/tools/cmake/component_deps.dot.in b/tools/cmake/component_deps.dot.in new file mode 100644 index 0000000000..fd49b8aad8 --- /dev/null +++ b/tools/cmake/component_deps.dot.in @@ -0,0 +1,30 @@ +digraph G { + +${depgraph} + +subgraph cluster_g0 { + rank = same; + label = "G0"; + + soc; + hal; + esp_common; + esp_rom; + ${CONFIG_IDF_TARGET_ARCH}; + ${CONFIG_IDF_TARGET}; +} + +subgraph cluster_g1 { + rank = same; + label = "G1"; + + spi_flash; + freertos; + log; + heap; + newlib; + esp_system; + esp_hw_support; +} + +} diff --git a/tools/cmake/depgraph.cmake b/tools/cmake/depgraph.cmake new file mode 100644 index 0000000000..fc312db7ad --- /dev/null +++ b/tools/cmake/depgraph.cmake @@ -0,0 +1,61 @@ +# Component dependency graph generation helpers. +# To enable this functionality, add: +# idf_build_set_property(__BUILD_COMPONENT_DEPGRAPH_ENABLED 1) +# in the project CMakeLists.txt file between include(project.cmake) and project(name) calls. +# +# Graphviz file component_deps.dot will be produced in the build directory. +# To change the template, see component_deps.dot.in. + +# depgraph_add_edge +# +# @brief Record an edge in the component dependency graph (dependent -> dependency) +# +# @param[in] dep_from dependent name +# @param[in] dep_to dependency name +# @param[in, optional] REQUIRES if given, record this as "REQUIRES" (public/interface) dependency +# @param[in, optional] PRIV_REQUIRES if given, record this as "PRIV_REQUIRES" (private) dependency +# +function(depgraph_add_edge dep_from dep_to) + cmake_parse_arguments(_ "REQUIRES;PRIV_REQUIRES" "" "" ${ARGN}) + idf_build_get_property(depgraph_enabled __BUILD_COMPONENT_DEPGRAPH_ENABLED) + if(NOT depgraph_enabled) + return() + endif() + idf_build_get_property(common_reqs __COMPONENT_REQUIRES_COMMON) + + set(attr) + if(__REQUIRES) + set(attr "[class=\"requires\"]") + elseif(__PRIV_REQUIRES) + set(attr "[class=\"priv_requires\" style=\"dotted\"]") + endif() + + if(dep_to IN_LIST common_reqs) + # Don't record graph edges leading to "common" components, since every component has these dependencies. + # However, show which components are "common" by adding an edge from a node named "common". + # If necessary, add a new build property to customize this behavior. + if(NOT dep_from IN_LIST common_reqs) + idf_build_set_property(__BUILD_COMPONENT_DEPGRAPH "common -> ${dep_to}" APPEND) + endif() + else() + idf_build_set_property(__BUILD_COMPONENT_DEPGRAPH "${dep_from} -> ${dep_to} ${attr}" APPEND) + endif() +endfunction() + +# depgraph_generate +# +# @brief Write the collected component dependency graph to a file. The file is in Graphviz (.dot) format. +# +# @param[in] out_path name of the output file +# +function(depgraph_generate out_path) + idf_build_get_property(depgraph_enabled __BUILD_COMPONENT_DEPGRAPH_ENABLED) + if(NOT depgraph_enabled) + return() + endif() + idf_build_get_property(depgraph __BUILD_COMPONENT_DEPGRAPH) + list(REMOVE_DUPLICATES depgraph) + string(REPLACE ";" ";\n" depgraph "${depgraph}") + configure_file("${IDF_PATH}/tools/cmake/component_deps.dot.in" + "${out_path}") +endfunction() diff --git a/tools/cmake/idf.cmake b/tools/cmake/idf.cmake index ea096b96c1..311346b5fe 100644 --- a/tools/cmake/idf.cmake +++ b/tools/cmake/idf.cmake @@ -41,6 +41,7 @@ if(NOT __idf_env_set) include(kconfig) include(component) include(utilities) + include(depgraph) include(targets) include(ldgen) include(dfu) diff --git a/tools/cmake/project.cmake b/tools/cmake/project.cmake index 4f0b33d9dc..2d56126a09 100644 --- a/tools/cmake/project.cmake +++ b/tools/cmake/project.cmake @@ -120,6 +120,9 @@ function(__project_info test_components) configure_file("${idf_path}/tools/cmake/project_description.json.in" "${build_dir}/project_description.json") + # Generate component dependency graph + depgraph_generate("${build_dir}/component_deps.dot") + # We now have the following component-related variables: # # build_components is the list of components to include in the build. From ac99a93f3359c98f047a7bed77bdf049ad6bbcde Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 2 Dec 2021 00:07:11 +0100 Subject: [PATCH 2/2] tools: add a test app to check "G1-only" build At the moment the list of components is far from being "G1-only". "extra_components_which_shouldnt_be_included" list will be reduced in future MRs. --- .../system/g1_components/CMakeLists.txt | 127 ++++++++++++++++++ .../test_apps/system/g1_components/README.md | 41 ++++++ .../system/g1_components/main/CMakeLists.txt | 2 + .../system/g1_components/main/g1_components.c | 11 ++ 4 files changed, 181 insertions(+) create mode 100644 tools/test_apps/system/g1_components/CMakeLists.txt create mode 100644 tools/test_apps/system/g1_components/README.md create mode 100644 tools/test_apps/system/g1_components/main/CMakeLists.txt create mode 100644 tools/test_apps/system/g1_components/main/g1_components.c diff --git a/tools/test_apps/system/g1_components/CMakeLists.txt b/tools/test_apps/system/g1_components/CMakeLists.txt new file mode 100644 index 0000000000..3a6de2bbf7 --- /dev/null +++ b/tools/test_apps/system/g1_components/CMakeLists.txt @@ -0,0 +1,127 @@ +# For more information about build system see +# https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/build-system.html +# The following five lines of boilerplate have to be in your project's +# CMakeLists in this exact order for cmake to work correctly +cmake_minimum_required(VERSION 3.5) + +set(g0_components soc hal esp_common esp_rom) # also , i.e. xtensa or riscv, will be added below +set(g1_components spi_flash freertos log heap newlib esp_system esp_hw_support) +set(COMPONENTS ${g0_components} ${g1_components} main) + +include($ENV{IDF_PATH}/tools/cmake/project.cmake) + +idf_build_set_property(__BUILD_COMPONENT_DEPGRAPH_ENABLED 1) + +project(g1_components) + +set(extra_allowed_components + ${CONFIG_IDF_TARGET_ARCH} + ${CONFIG_IDF_TARGET} +) + +# 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 + # (via weak CMake dependency and #if __has_include in C code) + app_update + # of G1 components, bootloader is only included from spi_flash + # [refactor-todo]: see if this dependency from spi_flash can be made weak + bootloader + # bootloader_support is a dependency of efuse, app_update, spi_flash. + # 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} + # all of these should be removed from G1 except for spi_flash. + driver + # [refactor-todo]: efuse is a dependency of esp_hw_support, esp_system. + # Figure out if these components can exist without a dependency on efuse. + # If not, see if esp_hw_support can provide minimal efuse component replacement in G1 build. + efuse + # esp_eth is a dependency of esp_netif, esp_event, lwip + # Once they are removed from G1, esp_eth will be removed as well. + 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. + esp_phy + # esp_pm is pulled in by esp_system and freertos, can be made a weak dependency + # conditional on related Kconfig option. It is also used by esp_wifi, driver, mbedtls, + # all of which should be removed from G1-only build. + esp_pm + # esp_ringbuf is a dependency of driver, which should be removed. + esp_ringbuf + # esp_timer is a dependency of freertos, esp_event, esp_wifi, driver. + # For freertos, it can be made a weak dependency conditional on FREERTOS_RUN_TIME_STATS_USING_ESP_TIMER + esp_timer + # 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 + # mbedtls is a dependency of bootloader_support (plus other easier-to-remove ones) + # it is hard to make it conditional, need to remove bootloader_support. + 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) + # and cxx. See also [refactor-todo] about cxx, can it work without pthread? + pthread + # tcpip_adapter is a dependency of esp_netif and lwip, both of which should be removed + tcpip_adapter + # vfs is a dependency of lwip. It can be made conditional, while lwip is still a common component. + vfs + # wpa_supplicant is a dependency of esp_wifi, which is to be removed from G1-only build + wpa_supplicant +) + +set(expected_components + ${COMPONENTS} + ${extra_allowed_components} + ${extra_components_which_shouldnt_be_included} +) + +list(SORT expected_components) + + +idf_build_get_property(build_components BUILD_COMPONENTS) +list(SORT build_components) + +if(NOT "${expected_components}" STREQUAL "${build_components}") + message(FATAL_ERROR "Unexpected components list in G1 build. " + "Expected: ${expected_components}. " + "Actual: ${build_components}") +endif() diff --git a/tools/test_apps/system/g1_components/README.md b/tools/test_apps/system/g1_components/README.md new file mode 100644 index 0000000000..fd2c1b7095 --- /dev/null +++ b/tools/test_apps/system/g1_components/README.md @@ -0,0 +1,41 @@ +# "G1"-components-only app + +This test application checks the list of components included into the build when "G1" components are added to the build. If G1 components don't have any dependencies outside of G1, then only G1 components themselves should be built. + +Currently, this is not the case, and many other components are added to the build. See `extra_components_which_shouldnt_be_included` list inside CMakeLists.txt. + +The purpose of this example is to: + +- Document which extra components are known to be included into G1 build. +- Fail the build if additional components are added to G1 build, preventing new dependencies outside of G1 from being added to G1 components. + +Once all the unwanted dependencies of G1 components are removed, the `extra_components_which_shouldnt_be_included` list should become empty. + +Please note, if an extra component B is already added to the build as a dependency of component A (A -> B), the build of this example will not fail when a new dependency onto the same component is added (C -> B). It will only fail when a new component is added to the build. Diff-ing `build/component_deps.dot` (see below) against a known-good one could be an alternative, but it will likely be quite fragile. + +# Using this test app + +To check G1 components using this app, run: +```bash +idf.py reconfigure +``` +(optionally, set IDF_TARGET to check this for a different target than the default one) + +# Troubleshooting + +If you get a build error in this example, + +``` +Unexpected components list in G1 build. Expected: . Actual: . +``` + +it means that the list of extra components added to G1 build has changed compared to `extra_components_which_shouldnt_be_included`. + +- If "Expected" contains more components than "Actual" — great! You have simplified the build dependency graph. Identify the component which is no longer added to the build and remove it from `extra_components_which_shouldnt_be_included`. +- If "Actual" contains more components than "Expected" — not so great. It means that some change has caused additional component to be "pulled into" the build. If you don't know which additional component was added, check `component_deps.dot` file in the build directory (see below). + +Please try very hard not to increase the number of components in `extra_components_which_shouldnt_be_included`. + +# Component dependencies graph (`component_deps.dot`) + +When this project is configured, `component_deps.dot` file in the build directory is generated. This file contains a Graphviz graph showing the component dependencies. You can visualize this graph (using `dot` tool or online at https://dreampuf.github.io/GraphvizOnline/) to see why an extra component got added. You can also build the project for the base branch, to compare the graph to a known good one. diff --git a/tools/test_apps/system/g1_components/main/CMakeLists.txt b/tools/test_apps/system/g1_components/main/CMakeLists.txt new file mode 100644 index 0000000000..7f64ebdc92 --- /dev/null +++ b/tools/test_apps/system/g1_components/main/CMakeLists.txt @@ -0,0 +1,2 @@ +idf_component_register(SRCS "g1_components.c" + INCLUDE_DIRS ".") diff --git a/tools/test_apps/system/g1_components/main/g1_components.c b/tools/test_apps/system/g1_components/main/g1_components.c new file mode 100644 index 0000000000..d69ff76b42 --- /dev/null +++ b/tools/test_apps/system/g1_components/main/g1_components.c @@ -0,0 +1,11 @@ +/* + * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include + +void app_main(void) +{ + +}