fix: harden input parsing in component_requirements hint module

Currently we silently ignore when the original component is not found
in a hope we can provide at least some meaningful hint. As it turned
out it's not true. Instead of providing misleading hint, just return
error. This adds several checks for situations, which should not happen,
but when they do it should be easier to identify the root cause of the
problem.

For example when hint module received malformed output with extra new
lines, e.g. caused by a bug in RunTool, it wrongly reported the original
component as source component.

This should also fix the tests on Windows.

Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This commit is contained in:
Frantisek Hrbata 2024-01-23 18:22:08 +01:00
parent 0fc2e77017
commit 6133810392
2 changed files with 23 additions and 9 deletions

View File

@ -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.
@ -74,13 +78,14 @@ def generate_hint(output: str) -> Optional[str]:
# 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])
@ -92,11 +97,19 @@ def generate_hint(output: str) -> Optional[str]:
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 = []
@ -154,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

View File

@ -149,7 +149,7 @@ class TestNestedModuleComponentRequirements(unittest.TestCase):
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'
f'set(EXTRA_COMPONENT_DIRS "{components.as_posix()}")\n'
'set(COMPONENTS main)\n'
'include($ENV{IDF_PATH}/tools/cmake/project.cmake)\n'
'project(foo)'))
@ -164,7 +164,7 @@ class TestNestedModuleComponentRequirements(unittest.TestCase):
# 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.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: