diff --git a/tools/cmake/project.cmake b/tools/cmake/project.cmake index 97b15d2896..3a61d14c7a 100644 --- a/tools/cmake/project.cmake +++ b/tools/cmake/project.cmake @@ -119,7 +119,7 @@ function(paths_with_spaces_to_list variable_name) endif() endfunction() -function(__component_info components output) +function(__build_component_info components output) set(components_json "") foreach(name ${components}) __component_get_target(target ${name}) @@ -187,6 +187,62 @@ function(__component_info components output) set(${output} "${components_json}" PARENT_SCOPE) endfunction() +function(__all_component_info output) + set(components_json "") + + idf_build_get_property(build_prefix __PREFIX) + idf_build_get_property(component_targets __COMPONENT_TARGETS) + + foreach(target ${component_targets}) + __component_get_property(name ${target} COMPONENT_NAME) + __component_get_property(alias ${target} COMPONENT_ALIAS) + __component_get_property(prefix ${target} __PREFIX) + __component_get_property(dir ${target} COMPONENT_DIR) + __component_get_property(type ${target} COMPONENT_TYPE) + __component_get_property(lib ${target} COMPONENT_LIB) + __component_get_property(reqs ${target} REQUIRES) + __component_get_property(include_dirs ${target} INCLUDE_DIRS) + __component_get_property(priv_reqs ${target} PRIV_REQUIRES) + __component_get_property(managed_reqs ${target} MANAGED_REQUIRES) + __component_get_property(managed_priv_reqs ${target} MANAGED_PRIV_REQUIRES) + + if(prefix STREQUAL build_prefix) + set(name ${name}) + else() + set(name ${alias}) + endif() + + make_json_list("${reqs}" reqs) + make_json_list("${priv_reqs}" priv_reqs) + make_json_list("${managed_reqs}" managed_reqs) + make_json_list("${managed_priv_reqs}" managed_priv_reqs) + make_json_list("${include_dirs}" include_dirs) + + string(JOIN "\n" component_json + " \"${name}\": {" + " \"alias\": \"${alias}\"," + " \"target\": \"${target}\"," + " \"prefix\": \"${prefix}\"," + " \"dir\": \"${dir}\"," + " \"lib\": \"${lib}\"," + " \"reqs\": ${reqs}," + " \"priv_reqs\": ${priv_reqs}," + " \"managed_reqs\": ${managed_reqs}," + " \"managed_priv_reqs\": ${managed_priv_reqs}," + " \"include_dirs\": ${include_dirs}" + " }" + ) + string(CONFIGURE "${component_json}" component_json) + if(NOT "${components_json}" STREQUAL "") + string(APPEND components_json ",\n") + endif() + string(APPEND components_json "${component_json}") + endforeach() + string(PREPEND components_json "{\n") + string(APPEND components_json "\n }") + set(${output} "${components_json}" PARENT_SCOPE) +endfunction() + # # Output the built components to the user. Generates files for invoking esp_idf_monitor # that doubles as an overview of some of the more important build properties. @@ -251,7 +307,8 @@ function(__project_info test_components) make_json_list("${build_component_paths};${test_component_paths}" build_component_paths_json) make_json_list("${common_component_reqs}" common_component_reqs_json) - __component_info("${build_components};${test_components}" build_component_info_json) + __build_component_info("${build_components};${test_components}" build_component_info_json) + __all_component_info(all_component_info_json) # The configure_file function doesn't process generator expressions, which are needed # e.g. to get component target library(TARGET_LINKER_FILE), so the project_description diff --git a/tools/cmake/project_description.json.in b/tools/cmake/project_description.json.in index feee0dd845..1805db5339 100644 --- a/tools/cmake/project_description.json.in +++ b/tools/cmake/project_description.json.in @@ -1,5 +1,5 @@ { - "version": "1", + "version": "1.1", "project_name": "${PROJECT_NAME}", "project_version": "${PROJECT_VER}", "project_path": "${PROJECT_PATH}", @@ -28,5 +28,6 @@ "build_components" : ${build_components_json}, "build_component_paths" : ${build_component_paths_json}, "build_component_info" : ${build_component_info_json}, + "all_component_info" : ${all_component_info_json}, "debug_prefix_map_gdbinit": "${debug_prefix_map_gdbinit}" } diff --git a/tools/cmake/scripts/component_get_requirements.cmake b/tools/cmake/scripts/component_get_requirements.cmake index 21d2da8614..89531881a4 100644 --- a/tools/cmake/scripts/component_get_requirements.cmake +++ b/tools/cmake/scripts/component_get_requirements.cmake @@ -76,6 +76,7 @@ macro(idf_component_register) set(__component_requires "${__REQUIRES}") set(__component_kconfig "${__KCONFIG}") set(__component_kconfig_projbuild "${__KCONFIG_PROJBUILD}") + set(__component_include_dirs "${__INCLUDE_DIRS}") set(__component_registered 1) return() endmacro() @@ -107,11 +108,13 @@ function(__component_get_requirements) spaces2list(__component_requires) spaces2list(__component_priv_requires) + spaces2list(__component_include_dirs) set(__component_requires "${__component_requires}" PARENT_SCOPE) set(__component_priv_requires "${__component_priv_requires}" PARENT_SCOPE) set(__component_kconfig "${__component_kconfig}" PARENT_SCOPE) set(__component_kconfig_projbuild "${__component_kconfig_projbuild}" PARENT_SCOPE) + set(__component_include_dirs "${__component_include_dirs}" PARENT_SCOPE) set(__component_registered ${__component_registered} PARENT_SCOPE) endfunction() @@ -141,7 +144,8 @@ foreach(__component_target ${__component_targets}) set(__contents "__component_set_property(${__component_target} REQUIRES \"${__component_requires}\") __component_set_property(${__component_target} PRIV_REQUIRES \"${__component_priv_requires}\") -__component_set_property(${__component_target} __COMPONENT_REGISTERED ${__component_registered})" +__component_set_property(${__component_target} __COMPONENT_REGISTERED ${__component_registered}) +__component_set_property(${__component_target} INCLUDE_DIRS \"${__component_include_dirs}\")" ) if(__component_kconfig) diff --git a/tools/idf_py_actions/hint_modules/component_requirements.py b/tools/idf_py_actions/hint_modules/component_requirements.py index 3ce6dd9dba..92e3a66825 100644 --- a/tools/idf_py_actions/hint_modules/component_requirements.py +++ b/tools/idf_py_actions/hint_modules/component_requirements.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 import os import re @@ -27,6 +27,10 @@ ENOENT_FULL_RE = re.compile(r'^(In file included.*No such file or directory)$', ORIGINAL_FILE_RE = re.compile(r'.*from (.*):[\d]+:') +def _bug(msg: str) -> str: + return f'BUG: {os.path.basename(__file__)}: {msg}' + + def _get_absolute_path(filename: str, base: str) -> str: # If filename path is relative, return absolute path based # on base directory. The filename is normalized in any case. @@ -58,48 +62,58 @@ def generate_hint(output: str) -> Optional[str]: # find the source_component that contains the source file found_source_component_name = None found_source_component_info = None - for component_name, component_info in proj_desc['build_component_info'].items(): + for component_name, component_info in proj_desc['all_component_info'].items(): # look if the source_filename is within a component directory, not only # at component_info['sources'], because the missing file may be included # from header file, which is not present in component_info['sources'] component_dir = os.path.normpath(component_info['dir']) if source_filename.startswith(component_dir): - found_source_component_name = component_name + if found_source_component_info and len(found_source_component_info['dir']) >= len(component_dir): + continue found_source_component_info = component_info - break + found_source_component_name = component_name if not found_source_component_name: # The source file is not in any component. # It could be in a subproject added via ExternalProject_Add, in which case # we can't help much. return None - # find the original_component, which may be different from sourc_component + # find the original_component, which may be different from source_component found_original_component_name = found_source_component_name found_original_component_info = found_source_component_info original_filename = source_filename hint_match_full = ENOENT_FULL_RE.search(output) if hint_match_full: - lines = hint_match_full.group().splitlines() + # As a precaution remove empty lines, but there should be none. + lines = [line for line in hint_match_full.group().splitlines() if line] # second line from the end contains filename which is part of the # original_component original_file_match = ORIGINAL_FILE_RE.match(lines[-2]) if original_file_match: original_filename = _get_absolute_path(original_file_match.group(1), proj_desc['build_dir']) - for component_name, component_info in proj_desc['build_component_info'].items(): + for component_name, component_info in proj_desc['all_component_info'].items(): component_dir = os.path.normpath(component_info['dir']) if original_filename.startswith(component_dir): found_original_component_name = component_name found_original_component_info = component_info break + else: + # Original component not found. This should never happen, because the + # original_filename has to part of some component, which was added to + # the build. + return _bug((f'cannot find original component for source ' + f'component {found_source_component_name}')) + else: # We should never reach this path. It would probably mean - # the preprocessor output was changed. Anyway we can still - # report something meaningful, so just keep going. - pass + # the preprocessor output was changed or we received malformed + # output. + return _bug((f'cannot match original component filename for ' + f'source component {found_source_component_name}')) # look for the header file in the public include directories of all components found_dep_component_names = [] - for candidate_component_name, candidate_component_info in proj_desc['build_component_info'].items(): + for candidate_component_name, candidate_component_info in proj_desc['all_component_info'].items(): if candidate_component_name == found_source_component_name: # skip the component that contains the source file continue @@ -116,7 +130,7 @@ def generate_hint(output: str) -> Optional[str]: # directories if we can find the missing header there and notify user about possible # missing entry in INCLUDE_DIRS. candidate_component_include_dirs = [] - for component_name, component_info in proj_desc['build_component_info'].items(): + for component_name, component_info in proj_desc['all_component_info'].items(): component_dir = os.path.normpath(component_info['dir']) for root, _, _ in os.walk(component_dir): full_path = os.path.normpath(os.path.join(root, missing_header)) @@ -153,7 +167,8 @@ def generate_hint(output: str) -> Optional[str]: if dep_component_name in all_reqs: # Oops. This component is already in the requirements list. # How did this happen? - return f'BUG: {missing_header} found in component {dep_component_name} which is already in the requirements list of {found_source_component_name}' + return _bug((f'{missing_header} found in component {dep_component_name} ' + f'which is already in the requirements list of {found_source_component_name}')) # try to figure out the correct require type: REQUIRES or PRIV_REQUIRES requires_type = None diff --git a/tools/test_idf_py/test_hints.py b/tools/test_idf_py/test_hints.py index d1164ff0a1..393c48d67e 100755 --- a/tools/test_idf_py/test_hints.py +++ b/tools/test_idf_py/test_hints.py @@ -48,16 +48,17 @@ class TestHintsMassages(unittest.TestCase): self.tmpdir.cleanup() -class TestHintModuleComponentRequirements(unittest.TestCase): - def run_idf(self, args: List[str]) -> str: - # Simple helper to run idf command and return it's stdout. - cmd = [ - sys.executable, - os.path.join(os.environ['IDF_PATH'], 'tools', 'idf.py') - ] - proc = run(cmd + args, capture_output=True, cwd=str(self.projectdir), text=True) - return proc.stdout + proc.stderr +def run_idf(args: List[str], cwd: Path) -> str: + # Simple helper to run idf command and return it's stdout. + cmd = [ + sys.executable, + os.path.join(os.environ['IDF_PATH'], 'tools', 'idf.py') + ] + proc = run(cmd + args, capture_output=True, cwd=cwd, text=True) + return str(proc.stdout + proc.stderr) + +class TestHintModuleComponentRequirements(unittest.TestCase): def setUp(self) -> None: # Set up a dummy project in tmp directory with main and component1 component. # The main component includes component1.h from component1, but the header dir is @@ -87,44 +88,117 @@ class TestHintModuleComponentRequirements(unittest.TestCase): # The main component uses component1.h, but this header is not in component1 public # interface. Hints should suggest that component1.h should be added into INCLUDE_DIRS # of component1. - output = self.run_idf(['app']) + output = run_idf(['app'], self.projectdir) self.assertIn('Missing "component1.h" file name found in the following component(s): component1(', output) # Based on previous hint the component1.h is added to INCLUDE_DIRS, but main still doesn't # have dependency on compoment1. Hints should suggest to add component1 into main component # PRIV_REQUIRES, because foo.h is not in main public interface. - self.run_idf(['fullclean']) + run_idf(['fullclean'], self.projectdir) component1cmake = self.projectdir / 'components' / 'component1' / 'CMakeLists.txt' component1cmake.write_text('idf_component_register(INCLUDE_DIRS ".")') - output = self.run_idf(['app']) + output = run_idf(['app'], self.projectdir) self.assertIn('To fix this, add component1 to PRIV_REQUIRES list of idf_component_register call', output) # Add foo.h into main public interface. Now the hint should suggest to use # REQUIRES instead of PRIV_REQUIRES. - self.run_idf(['fullclean']) + run_idf(['fullclean'], self.projectdir) maincmake = self.projectdir / 'main' / 'CMakeLists.txt' maincmake.write_text(('idf_component_register(SRCS "foo.c" ' 'REQUIRES esp_timer ' 'INCLUDE_DIRS ".")')) - output = self.run_idf(['app']) + output = run_idf(['app'], self.projectdir) self.assertIn('To fix this, add component1 to REQUIRES list of idf_component_register call', output) # Add component1 to REQUIRES as suggested by previous hint, but also # add esp_psram as private req for component1 and add esp_psram.h - # to component1.h. New the hint should report that esp_psram should + # to component1.h. Now the hint should report that esp_psram should # be moved from PRIV_REQUIRES to REQUIRES for component1. - self.run_idf(['fullclean']) + run_idf(['fullclean'], self.projectdir) maincmake.write_text(('idf_component_register(SRCS "foo.c" ' 'REQUIRES esp_timer component1 ' 'INCLUDE_DIRS ".")')) (self.projectdir / 'components' / 'component1' / 'component1.h').write_text('#include "esp_psram.h"') component1cmake.write_text('idf_component_register(INCLUDE_DIRS "." PRIV_REQUIRES esp_psram)') - output = self.run_idf(['app']) + output = run_idf(['app'], self.projectdir) self.assertIn('To fix this, move esp_psram from PRIV_REQUIRES into REQUIRES', output) def tearDown(self) -> None: self.tmpdir.cleanup() +class TestNestedModuleComponentRequirements(unittest.TestCase): + def setUp(self) -> None: + # Set up a nested component structure. The components directory contains + # component1, which also contains foo project with main component. + # components/component1/project/main + # ^^^^^^^^^^ ^^^^ + # component nested component + # Both components include esp_timer.h, but only component1 has esp_timer + # in requirements. + self.tmpdir = tempfile.TemporaryDirectory() + self.tmpdirpath = Path(self.tmpdir.name) + + components = self.tmpdirpath / 'components' + maindir = components / 'component1' + maindir.mkdir(parents=True) + (maindir / 'CMakeLists.txt').write_text('idf_component_register(SRCS "component1.c" PRIV_REQUIRES esp_timer)') + (maindir / 'component1.c').write_text('#include "esp_timer.h"') + + self.projectdir = maindir / 'project' + self.projectdir.mkdir(parents=True) + (self.projectdir / 'CMakeLists.txt').write_text(( + 'cmake_minimum_required(VERSION 3.16)\n' + f'set(EXTRA_COMPONENT_DIRS "{components.as_posix()}")\n' + 'set(COMPONENTS main)\n' + 'include($ENV{IDF_PATH}/tools/cmake/project.cmake)\n' + 'project(foo)')) + + maindir = self.projectdir / 'main' + maindir.mkdir() + (maindir / 'CMakeLists.txt').write_text('idf_component_register(SRCS "foo.c" REQUIRES component1)') + (maindir / 'foo.c').write_text('#include "esp_timer.h"\nvoid app_main(){}') + + def test_nested_component_requirements(self) -> None: + # Verify that source component for a failed include is properly identified + # when components are nested. The main component should be identified as the + # real source, not the component1 component. + output = run_idf(['app'], self.projectdir) + self.assertNotIn('esp_timer.h found in component esp_timer which is already in the requirements list of component1', output) + self.assertIn('To fix this, add esp_timer to PRIV_REQUIRES list of idf_component_register call', output) + + def tearDown(self) -> None: + self.tmpdir.cleanup() + + +class TestTrimmedModuleComponentRequirements(unittest.TestCase): + def setUp(self) -> None: + # Set up a dummy project with a trimmed down list of components and main component. + # The main component includes "esp_http_client.h", but the esp_http_client + # component is not added to main's requirements. + self.tmpdir = tempfile.TemporaryDirectory() + self.tmpdirpath = Path(self.tmpdir.name) + + self.projectdir = self.tmpdirpath / 'project' + self.projectdir.mkdir(parents=True) + (self.projectdir / 'CMakeLists.txt').write_text(( + 'cmake_minimum_required(VERSION 3.16)\n' + 'set(COMPONENTS main)\n' + 'include($ENV{IDF_PATH}/tools/cmake/project.cmake)\n' + 'project(foo)')) + + maindir = self.projectdir / 'main' + maindir.mkdir() + (maindir / 'CMakeLists.txt').write_text('idf_component_register(SRCS "foo.c")') + (maindir / 'foo.c').write_text('#include "esp_http_client.h"\nvoid app_main(){}') + + def test_trimmed_component_requirements(self) -> None: + output = run_idf(['app'], self.projectdir) + self.assertIn('To fix this, add esp_http_client to PRIV_REQUIRES list of idf_component_register call in', output) + + def tearDown(self) -> None: + self.tmpdir.cleanup() + + if __name__ == '__main__': unittest.main()