forked from espressif/esp-idf
Merge branch 'fix/harden_hints_parsing' into 'master'
fix: harden input parsing in component_requirements hint module Closes IDF-9082 See merge request espressif/esp-idf!28661
This commit is contained in:
@@ -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
|
# SPDX-License-Identifier: Apache-2.0
|
||||||
import os
|
import os
|
||||||
import re
|
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]+:')
|
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:
|
def _get_absolute_path(filename: str, base: str) -> str:
|
||||||
# If filename path is relative, return absolute path based
|
# If filename path is relative, return absolute path based
|
||||||
# on base directory. The filename is normalized in any case.
|
# 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.
|
# we can't help much.
|
||||||
return None
|
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_name = found_source_component_name
|
||||||
found_original_component_info = found_source_component_info
|
found_original_component_info = found_source_component_info
|
||||||
original_filename = source_filename
|
original_filename = source_filename
|
||||||
hint_match_full = ENOENT_FULL_RE.search(output)
|
hint_match_full = ENOENT_FULL_RE.search(output)
|
||||||
if hint_match_full:
|
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
|
# second line from the end contains filename which is part of the
|
||||||
# original_component
|
# original_component
|
||||||
original_file_match = ORIGINAL_FILE_RE.match(lines[-2])
|
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_name = component_name
|
||||||
found_original_component_info = component_info
|
found_original_component_info = component_info
|
||||||
break
|
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:
|
else:
|
||||||
# We should never reach this path. It would probably mean
|
# We should never reach this path. It would probably mean
|
||||||
# the preprocessor output was changed. Anyway we can still
|
# the preprocessor output was changed or we received malformed
|
||||||
# report something meaningful, so just keep going.
|
# output.
|
||||||
pass
|
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
|
# look for the header file in the public include directories of all components
|
||||||
found_dep_component_names = []
|
found_dep_component_names = []
|
||||||
@@ -154,7 +167,8 @@ def generate_hint(output: str) -> Optional[str]:
|
|||||||
if dep_component_name in all_reqs:
|
if dep_component_name in all_reqs:
|
||||||
# Oops. This component is already in the requirements list.
|
# Oops. This component is already in the requirements list.
|
||||||
# How did this happen?
|
# 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
|
# try to figure out the correct require type: REQUIRES or PRIV_REQUIRES
|
||||||
requires_type = None
|
requires_type = None
|
||||||
|
@@ -149,7 +149,7 @@ class TestNestedModuleComponentRequirements(unittest.TestCase):
|
|||||||
self.projectdir.mkdir(parents=True)
|
self.projectdir.mkdir(parents=True)
|
||||||
(self.projectdir / 'CMakeLists.txt').write_text((
|
(self.projectdir / 'CMakeLists.txt').write_text((
|
||||||
'cmake_minimum_required(VERSION 3.16)\n'
|
'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'
|
'set(COMPONENTS main)\n'
|
||||||
'include($ENV{IDF_PATH}/tools/cmake/project.cmake)\n'
|
'include($ENV{IDF_PATH}/tools/cmake/project.cmake)\n'
|
||||||
'project(foo)'))
|
'project(foo)'))
|
||||||
@@ -164,7 +164,7 @@ class TestNestedModuleComponentRequirements(unittest.TestCase):
|
|||||||
# when components are nested. The main component should be identified as the
|
# when components are nested. The main component should be identified as the
|
||||||
# real source, not the component1 component.
|
# real source, not the component1 component.
|
||||||
output = run_idf(['app'], self.projectdir)
|
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)
|
self.assertIn('To fix this, add esp_timer to PRIV_REQUIRES list of idf_component_register call', output)
|
||||||
|
|
||||||
def tearDown(self) -> None:
|
def tearDown(self) -> None:
|
||||||
|
Reference in New Issue
Block a user