mirror of
https://github.com/espressif/esp-idf.git
synced 2024-10-05 20:47:46 -04:00
Merge branch 'feature/requires_hints' into 'master'
feat(tools): show hints for component dependencies Closes IDF-6642 See merge request espressif/esp-idf!23950
This commit is contained in:
commit
33205dad4c
214
tools/idf_py_actions/hint_modules/component_requirements.py
Normal file
214
tools/idf_py_actions/hint_modules/component_requirements.py
Normal file
@ -0,0 +1,214 @@
|
||||
# SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
import os
|
||||
import re
|
||||
from typing import Optional
|
||||
|
||||
from idf_py_actions.tools import get_build_context
|
||||
|
||||
'''
|
||||
glossary:
|
||||
orignal_component: component which compilation failed
|
||||
source_component: component containing file which is including the missing header file
|
||||
candidate_component: component which contain the missing header file
|
||||
original_filename: abs path of file(compilation unit) in original_component
|
||||
source_filename: abs path of file in source_component which is including the missing header file
|
||||
missing_header: filename of the missing header included in source_filename
|
||||
'''
|
||||
|
||||
# Regex to find source_filename in preprocessor's error message
|
||||
ENOENT_RE = re.compile(r'^(.+):\d+:\d+: fatal error: (.+): No such file or directory$',
|
||||
flags=re.MULTILINE)
|
||||
# Regex to find full preprocessor's error message to identify the original_filename
|
||||
# in case the missing_header is reported in indirect include.
|
||||
ENOENT_FULL_RE = re.compile(r'^(In file included.*No such file or directory)$',
|
||||
flags=re.MULTILINE | re.DOTALL)
|
||||
# Regex to find original_filename in preprocessor's error message
|
||||
ORIGINAL_FILE_RE = re.compile(r'.*from (.*):[\d]+:')
|
||||
|
||||
|
||||
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.
|
||||
if not os.path.isabs(filename):
|
||||
filename = os.path.join(base, filename)
|
||||
filename = os.path.normpath(filename)
|
||||
return filename
|
||||
|
||||
|
||||
def generate_hint(output: str) -> Optional[str]:
|
||||
# get the project description
|
||||
proj_desc = get_build_context().get('proj_desc')
|
||||
if not proj_desc:
|
||||
# hints cannot be generated because we are not in the build context,
|
||||
# meaning ensure_build_directory() was not ran and project description
|
||||
# is not available
|
||||
return None
|
||||
|
||||
hint_match = ENOENT_RE.search(output)
|
||||
if not hint_match:
|
||||
return None
|
||||
|
||||
# this is the file where the error has occurred
|
||||
source_filename = _get_absolute_path(hint_match.group(1), proj_desc['build_dir'])
|
||||
|
||||
# this is the header file we tried to include
|
||||
missing_header = hint_match.group(2)
|
||||
|
||||
# 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():
|
||||
# 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
|
||||
found_source_component_info = component_info
|
||||
break
|
||||
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
|
||||
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()
|
||||
# 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():
|
||||
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:
|
||||
# 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
|
||||
|
||||
# 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():
|
||||
if candidate_component_name == found_source_component_name:
|
||||
# skip the component that contains the source file
|
||||
continue
|
||||
candidate_component_include_dirs = candidate_component_info['include_dirs']
|
||||
component_dir = os.path.normpath(candidate_component_info['dir'])
|
||||
for candidate_component_include_dir in candidate_component_include_dirs:
|
||||
candidate_header_path = os.path.join(component_dir, candidate_component_include_dir, missing_header)
|
||||
if os.path.exists(candidate_header_path):
|
||||
found_dep_component_names.append(candidate_component_name)
|
||||
break # no need to look further in this component
|
||||
|
||||
if not found_dep_component_names:
|
||||
# Header file not found in any component INCLUDE_DIRS. Try to scan whole component
|
||||
# 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():
|
||||
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))
|
||||
# sanity check that the full_path is still within component's directory
|
||||
if not full_path.startswith(component_dir):
|
||||
continue
|
||||
if os.path.isfile(full_path):
|
||||
candidate_component_include_dirs.append(f'{component_name}({full_path})')
|
||||
|
||||
if candidate_component_include_dirs:
|
||||
candidates = ', '.join(candidate_component_include_dirs)
|
||||
return (f'Missing "{missing_header}" file name found in the following component(s): {candidates}. '
|
||||
f'Maybe one of the components needs to add the missing header directory to INCLUDE_DIRS '
|
||||
f'of idf_component_register call in CMakeLists.txt.')
|
||||
|
||||
# The missing header not found anywhere, nothing much we can do here.
|
||||
return None
|
||||
|
||||
assert found_source_component_info is not None # to help mypy
|
||||
assert found_original_component_info is not None # to help mypy
|
||||
|
||||
# Sanity check: verify we didn't somehow find a component which is already in the requirements list
|
||||
all_reqs = (found_source_component_info['reqs']
|
||||
+ found_source_component_info['managed_reqs'])
|
||||
if found_original_component_name == found_source_component_name:
|
||||
# Add also private reqs, but only if source_component is same original_component.
|
||||
# The missing_header may be part of component which is already added as private
|
||||
# req for source_component. Meaning it's not part of source_component public
|
||||
# interface.
|
||||
all_reqs += (found_source_component_info['priv_reqs']
|
||||
+ found_source_component_info['managed_priv_reqs'])
|
||||
|
||||
for dep_component_name in found_dep_component_names:
|
||||
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}'
|
||||
|
||||
# try to figure out the correct require type: REQUIRES or PRIV_REQUIRES
|
||||
requires_type = None
|
||||
source_component_has_priv_dep = False
|
||||
if original_filename == source_filename:
|
||||
# The error is reported directly in compilation unit, so
|
||||
# missing_header should not be part of public interface.
|
||||
requires_type = 'PRIV_REQUIRES'
|
||||
elif found_original_component_name == found_source_component_name:
|
||||
# The original_component and source_component are the same and original_filename
|
||||
# is different from source_filename. Check if the source_file is part of the
|
||||
# original_component's public interface. If so, the REQUIRES should be used.
|
||||
for include_dir in found_original_component_info['include_dirs']:
|
||||
include_dir = _get_absolute_path(found_original_component_info['dir'], include_dir)
|
||||
if source_filename.startswith(include_dir):
|
||||
# source_filename is part of public interface
|
||||
requires_type = 'REQUIRES'
|
||||
break
|
||||
if not requires_type:
|
||||
# source_file not part of public interface, suggest PRIV_REQUIRES
|
||||
requires_type = 'PRIV_REQUIRES'
|
||||
else:
|
||||
# The source_filename is part of different component than the original_component, so
|
||||
# the source_component needs to use REQUIRES to make the missing_header available for
|
||||
# original_component.
|
||||
requires_type = 'REQUIRES'
|
||||
if len(found_dep_component_names) == 1:
|
||||
# If there is only one component found as missing dependency, look at
|
||||
# source_component private requires to see if the missing dependency is
|
||||
# already there. If so, we suggest to move it from PRIV_REQUIRES to REQUIRES.
|
||||
# This is done only if there is one component in found_dep_component_names, because
|
||||
# otherwise we cannot be sure which component should be moved.
|
||||
priv_reqs = (found_source_component_info['priv_reqs']
|
||||
+ found_source_component_info['managed_priv_reqs'])
|
||||
if found_dep_component_names[0] in priv_reqs:
|
||||
source_component_has_priv_dep = True
|
||||
|
||||
found_dep_component_names_list = ', '.join(found_dep_component_names)
|
||||
source_filename_short = os.path.basename(source_filename)
|
||||
cmakelists_file_to_fix = os.path.normpath(os.path.join(found_source_component_info['dir'], 'CMakeLists.txt'))
|
||||
problem_description = (
|
||||
f'Compilation failed because {source_filename_short} (in "{found_source_component_name}" component) '
|
||||
f'includes {missing_header}, provided by {found_dep_component_names_list} component(s).\n')
|
||||
|
||||
if source_component_has_priv_dep:
|
||||
problem_solution = (
|
||||
f'However, {found_dep_component_names_list} component(s) is in the private requirements list '
|
||||
f'of "{found_source_component_name}".\n'
|
||||
f'To fix this, move {found_dep_component_names_list} from PRIV_REQUIRES into '
|
||||
f'REQUIRES list of idf_component_register call in {cmakelists_file_to_fix}.')
|
||||
else:
|
||||
problem_solution = (
|
||||
f'However, {found_dep_component_names_list} component(s) is not in the requirements list '
|
||||
f'of "{found_source_component_name}".\n'
|
||||
f'To fix this, add {found_dep_component_names_list} to {requires_type} list '
|
||||
f'of idf_component_register call in {cmakelists_file_to_fix}.')
|
||||
|
||||
return problem_description + problem_solution
|
@ -1,6 +1,7 @@
|
||||
# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
import asyncio
|
||||
import importlib
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
@ -8,6 +9,7 @@ import subprocess
|
||||
import sys
|
||||
from asyncio.subprocess import Process
|
||||
from io import open
|
||||
from pkgutil import iter_modules
|
||||
from types import FunctionType
|
||||
from typing import Any, Dict, Generator, List, Match, Optional, TextIO, Tuple, Union
|
||||
|
||||
@ -30,6 +32,37 @@ SHELL_COMPLETE_VAR = '_IDF.PY_COMPLETE'
|
||||
SHELL_COMPLETE_RUN = SHELL_COMPLETE_VAR in os.environ
|
||||
|
||||
|
||||
# The ctx dict "abuses" how python evaluates default parameter values.
|
||||
# https://docs.python.org/3/reference/compound_stmts.html#function-definitions
|
||||
# Default parameter values are evaluated from left to right
|
||||
# when the function definition is executed
|
||||
def get_build_context(ctx: Dict={}) -> Dict:
|
||||
"""
|
||||
The build context is set in the ensure_build_directory function. It can be used
|
||||
in modules or other code, which don't have direct access to such information.
|
||||
It returns dictionary with the following keys:
|
||||
|
||||
'proj_desc' - loaded project_description.json file
|
||||
|
||||
Please make sure that ensure_build_directory was called otherwise the build
|
||||
context dictionary will be empty. Also note that it might not be thread-safe to
|
||||
modify the returned dictionary. It should be considered read-only.
|
||||
"""
|
||||
return ctx
|
||||
|
||||
|
||||
def _set_build_context(args: 'PropertyDict') -> None:
|
||||
# private helper to set global build context from ensure_build_directory
|
||||
ctx = get_build_context()
|
||||
|
||||
proj_desc_fn = f'{args.build_dir}/project_description.json'
|
||||
try:
|
||||
with open(proj_desc_fn, 'r') as f:
|
||||
ctx['proj_desc'] = json.load(f)
|
||||
except (OSError, ValueError) as e:
|
||||
raise FatalError(f'Cannot load {proj_desc_fn}: {e}')
|
||||
|
||||
|
||||
def executable_exists(args: List) -> bool:
|
||||
try:
|
||||
subprocess.check_output(args)
|
||||
@ -134,16 +167,50 @@ def debug_print_idf_version() -> None:
|
||||
print_warning(f'ESP-IDF {idf_version() or "version unknown"}')
|
||||
|
||||
|
||||
def load_hints() -> Any:
|
||||
def load_hints() -> Dict:
|
||||
"""Helper function to load hints yml file"""
|
||||
with open(os.path.join(os.path.dirname(__file__), 'hints.yml'), 'r') as file:
|
||||
hints = yaml.safe_load(file)
|
||||
hints: Dict = {
|
||||
'yml': [],
|
||||
'modules': []
|
||||
}
|
||||
|
||||
current_module_dir = os.path.dirname(__file__)
|
||||
with open(os.path.join(current_module_dir, 'hints.yml'), 'r') as file:
|
||||
hints['yml'] = yaml.safe_load(file)
|
||||
|
||||
hint_modules_dir = os.path.join(current_module_dir, 'hint_modules')
|
||||
if not os.path.exists(hint_modules_dir):
|
||||
return hints
|
||||
|
||||
sys.path.append(hint_modules_dir)
|
||||
for _, name, _ in iter_modules([hint_modules_dir]):
|
||||
# Import modules for hint processing and add list of their 'generate_hint' functions into hint dict.
|
||||
# If the module doesn't have the function 'generate_hint', it will raise an exception
|
||||
try:
|
||||
hints['modules'].append(getattr(importlib.import_module(name), 'generate_hint'))
|
||||
except ModuleNotFoundError:
|
||||
red_print(f'Failed to import "{name}" from "{hint_modules_dir}" as a module')
|
||||
raise SystemExit(1)
|
||||
except AttributeError:
|
||||
red_print('Module "{}" does not have function generate_hint.'.format(name))
|
||||
raise SystemExit(1)
|
||||
|
||||
return hints
|
||||
|
||||
|
||||
def generate_hints_buffer(output: str, hints: list) -> Generator:
|
||||
def generate_hints_buffer(output: str, hints: Dict) -> Generator:
|
||||
"""Helper function to process hints within a string buffer"""
|
||||
for hint in hints:
|
||||
# Call modules for possible hints with unchanged output. Note that
|
||||
# hints in hints.yml expect new line trimmed, but modules should
|
||||
# get the output unchanged. Please see tools/idf_py_actions/hints.yml
|
||||
for generate_hint in hints['modules']:
|
||||
module_hint = generate_hint(output)
|
||||
if module_hint:
|
||||
yield module_hint
|
||||
|
||||
# hints expect new lines trimmed
|
||||
output = ' '.join(line.strip() for line in output.splitlines() if line.strip())
|
||||
for hint in hints['yml']:
|
||||
variables_list = hint.get('variables')
|
||||
hint_list, hint_vars, re_vars = [], [], []
|
||||
match: Optional[Match[str]] = None
|
||||
@ -183,8 +250,7 @@ def generate_hints(*filenames: str) -> Generator:
|
||||
hints = load_hints()
|
||||
for file_name in filenames:
|
||||
with open(file_name, 'r') as file:
|
||||
output = ' '.join(line.strip() for line in file if line.strip())
|
||||
yield from generate_hints_buffer(output, hints)
|
||||
yield from generate_hints_buffer(file.read(), hints)
|
||||
|
||||
|
||||
def fit_text_in_terminal(out: str) -> str:
|
||||
@ -567,6 +633,9 @@ def ensure_build_directory(args: 'PropertyDict', prog_name: str, always_run_cmak
|
||||
except KeyError:
|
||||
pass
|
||||
|
||||
# set global build context
|
||||
_set_build_context(args)
|
||||
|
||||
|
||||
def merge_action_lists(*action_lists: Dict) -> Dict:
|
||||
merged_actions: Dict = {
|
||||
|
@ -1,14 +1,24 @@
|
||||
#!/usr/bin/env python
|
||||
#
|
||||
# SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
|
||||
# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from subprocess import run
|
||||
from typing import List
|
||||
|
||||
import yaml
|
||||
|
||||
try:
|
||||
EXT_IDF_PATH = os.environ['IDF_PATH'] # type: str
|
||||
except KeyError:
|
||||
print(('This test needs to run within ESP-IDF environmnet. '
|
||||
'Please run export script first.'), file=sys.stderr)
|
||||
exit(1)
|
||||
|
||||
CWD = os.path.join(os.path.dirname(__file__))
|
||||
ERR_OUT_YML = os.path.join(CWD, 'error_output.yml')
|
||||
|
||||
@ -20,15 +30,100 @@ except ImportError:
|
||||
|
||||
|
||||
class TestHintsMassages(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.tmpdir = tempfile.TemporaryDirectory()
|
||||
|
||||
def test_output(self) -> None:
|
||||
with open(ERR_OUT_YML) as f:
|
||||
error_output = yaml.safe_load(f)
|
||||
|
||||
error_filename = os.path.join(self.tmpdir.name, 'hint_input')
|
||||
for error, hint in error_output.items():
|
||||
with tempfile.NamedTemporaryFile(mode='w') as f:
|
||||
with open(error_filename, 'w') as f:
|
||||
f.write(error)
|
||||
f.flush()
|
||||
for generated_hint in generate_hints(f.name):
|
||||
self.assertEqual(generated_hint, hint)
|
||||
for generated_hint in generate_hints(f.name):
|
||||
self.assertEqual(generated_hint, hint)
|
||||
|
||||
def tearDown(self) -> None:
|
||||
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 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
|
||||
# not added in INCLUDE_DIRS and main doesn't have REQUIRES for component1.
|
||||
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'
|
||||
'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 esp_timer)')
|
||||
(maindir / 'foo.h').write_text('#include "component1.h"')
|
||||
(maindir / 'foo.c').write_text('#include "foo.h"\nvoid app_main(){}')
|
||||
|
||||
component1dir = self.projectdir / 'components' / 'component1'
|
||||
component1dir.mkdir(parents=True)
|
||||
(component1dir / 'CMakeLists.txt').write_text('idf_component_register()')
|
||||
(component1dir / 'component1.h').touch()
|
||||
|
||||
def test_component_requirements(self) -> None:
|
||||
# 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'])
|
||||
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'])
|
||||
component1cmake = self.projectdir / 'components' / 'component1' / 'CMakeLists.txt'
|
||||
component1cmake.write_text('idf_component_register(INCLUDE_DIRS ".")')
|
||||
output = self.run_idf(['app'])
|
||||
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'])
|
||||
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'])
|
||||
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
|
||||
# be moved from PRIV_REQUIRES to REQUIRES for component1.
|
||||
self.run_idf(['fullclean'])
|
||||
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'])
|
||||
self.assertIn('To fix this, move esp_psram from PRIV_REQUIRES into REQUIRES', output)
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self.tmpdir.cleanup()
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
|
Loading…
Reference in New Issue
Block a user