diff --git a/tools/idf_py_actions/hint_modules/component_requirements.py b/tools/idf_py_actions/hint_modules/component_requirements.py index 3ce6dd9dba..ab555b9545 100644 --- a/tools/idf_py_actions/hint_modules/component_requirements.py +++ b/tools/idf_py_actions/hint_modules/component_requirements.py @@ -64,9 +64,10 @@ def generate_hint(output: str) -> Optional[str]: # 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 diff --git a/tools/test_idf_py/test_hints.py b/tools/test_idf_py/test_hints.py index d1164ff0a1..18c6092430 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,88 @@ 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}")\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('BUG: 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() + + if __name__ == '__main__': unittest.main()