From e1b96227349ee2bcc89e75c34153d2262d3db095 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Thu, 7 Dec 2023 15:43:31 +0100 Subject: [PATCH 1/3] fix: improve logging output. Differentiate print and logging.info --- docs/find_build.md | 15 ++++++ idf_build_apps/__init__.py | 3 +- idf_build_apps/app.py | 18 ++++--- idf_build_apps/constants.py | 6 +-- idf_build_apps/finder.py | 8 ++-- idf_build_apps/log.py | 15 +++--- idf_build_apps/main.py | 68 ++++++++++++++------------- idf_build_apps/manifest/manifest.py | 10 ++-- idf_build_apps/manifest/soc_header.py | 9 ++-- idf_build_apps/utils.py | 11 +++-- 10 files changed, 93 insertions(+), 70 deletions(-) diff --git a/docs/find_build.md b/docs/find_build.md index 1cef55e..92ed50d 100644 --- a/docs/find_build.md +++ b/docs/find_build.md @@ -199,6 +199,21 @@ The output would be: (cmake) App ./test-2, target esp32, sdkconfig (default), build in /tmp/build/test-2_esp32 ``` +### Output in Text File + +For `find` command, you may use `--output ` to output the result to a text file. Each line of the text file represents an app. + +You may reuse the file with python code: + +```python +from idf_build_apps import AppDeserializer + +with open("output.txt", "r") as f: + for line in f: + app = AppDeserializer.from_json(line) + print(app) +``` + ## Build Apps Building apps is a process that build all the applications that are collected by the "find" process. diff --git a/idf_build_apps/__init__.py b/idf_build_apps/__init__.py index d34eaaa..9aba448 100644 --- a/idf_build_apps/__init__.py +++ b/idf_build_apps/__init__.py @@ -11,7 +11,7 @@ import logging __version__ = '2.0.0b4' -LOGGER = logging.getLogger('idf_build_apps') +LOGGER = logging.getLogger(__package__) from .session_args import ( SessionArgs, @@ -41,4 +41,5 @@ 'find_apps', 'build_apps', 'setup_logging', + 'SESSION_ARGS', ] diff --git a/idf_build_apps/app.py b/idf_build_apps/app.py index 2abf923..894af17 100644 --- a/idf_build_apps/app.py +++ b/idf_build_apps/app.py @@ -35,9 +35,6 @@ SESSION_ARGS, ) -from . import ( - LOGGER, -) from .build_job import ( BuildAppJob, ) @@ -189,8 +186,8 @@ def __init__( } ) self._initialize_hook(**kwargs) - # create logger and process sdkconfig files - self._logger = LOGGER.getChild(str(hash(self))) + + self._logger = logging.getLogger(f'{__name__}.{hash(self)}') self._logger.addFilter(_AppBuildStageFilter(app=self)) self._process_sdkconfig_files() @@ -202,6 +199,15 @@ def _initialize_hook(self, **kwargs): pass def __str__(self): + if self.build_status in (BuildStatus.UNKNOWN, BuildStatus.SHOULD_BE_BUILT): + return '({}) App {}, target {}, sdkconfig {}, build in {}'.format( + self.build_system, + self.app_dir, + self.target, + self.sdkconfig_path or '(default)', + self.build_path, + ) + return '({}) App {}, target {}, sdkconfig {}, build in {}, {} in {}s'.format( self.build_system, self.app_dir, @@ -632,7 +638,7 @@ def _write_size_json(self) -> None: check=True, ) - self._logger.info('Generated size info to %s', self.size_json_path) + self._logger.debug('Generated size info to %s', self.size_json_path) def write_size_json(self) -> None: try: diff --git a/idf_build_apps/constants.py b/idf_build_apps/constants.py index c3df4a1..bd68cd4 100644 --- a/idf_build_apps/constants.py +++ b/idf_build_apps/constants.py @@ -3,6 +3,7 @@ import enum import importlib +import logging import os import re import sys @@ -12,13 +13,12 @@ Path, ) -from . import ( - LOGGER, -) from .utils import ( to_version, ) +LOGGER = logging.getLogger(__name__) + _BUILDING_DOCS = bool(os.getenv('BUILDING_DOCS')) if _BUILDING_DOCS: print('Building Docs... Faking lots of constant values') diff --git a/idf_build_apps/finder.py b/idf_build_apps/finder.py index 4ec7c6e..a3a7439 100644 --- a/idf_build_apps/finder.py +++ b/idf_build_apps/finder.py @@ -1,16 +1,14 @@ # SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 -import os.path +import logging +import os import re import typing as t from pathlib import ( Path, ) -from . import ( - LOGGER, -) from .app import ( App, CMakeApp, @@ -24,6 +22,8 @@ to_list, ) +LOGGER = logging.getLogger(__name__) + def _get_apps_from_path( path: str, diff --git a/idf_build_apps/log.py b/idf_build_apps/log.py index abc331b..efb9ce4 100644 --- a/idf_build_apps/log.py +++ b/idf_build_apps/log.py @@ -5,9 +5,6 @@ import sys import typing as t -from . import ( - LOGGER, -) from .constants import ( BuildStage, ) @@ -65,7 +62,7 @@ def setup_logging(verbose: int = 0, log_file: t.Optional[str] = None, colored: b """ Setup logging stream handler - :param verbose: 0 - WARNING, 1 - INFO, 2+ - DEBUG + :param verbose: 0 - WARNING, 1 - INFO, 2 - DEBUG :param log_file: log file path :param colored: colored output or not :return: None @@ -77,13 +74,13 @@ def setup_logging(verbose: int = 0, log_file: t.Optional[str] = None, colored: b else: level = logging.DEBUG - LOGGER.setLevel(level) + package_logger = logging.getLogger(__package__) + package_logger.setLevel(level) if log_file: handler = logging.FileHandler(log_file) else: handler = logging.StreamHandler(sys.stderr) - - handler.setLevel(level) handler.setFormatter(ColoredFormatter(colored)) - LOGGER.handlers = [handler] - LOGGER.propagate = False + + package_logger.addHandler(handler) + package_logger.propagate = False # don't propagate to root logger diff --git a/idf_build_apps/main.py b/idf_build_apps/main.py index f2f65fb..cc3a02a 100644 --- a/idf_build_apps/main.py +++ b/idf_build_apps/main.py @@ -3,6 +3,7 @@ import argparse import json +import logging import os import re import shutil @@ -14,7 +15,6 @@ ) from . import ( - LOGGER, SESSION_ARGS, ) from .app import ( @@ -56,6 +56,8 @@ to_list, ) +LOGGER = logging.getLogger(__name__) + def _check_app_dependency( manifest_rootpath: str, @@ -73,8 +75,8 @@ def _check_app_dependency( and modified_files is not None and files_matches_patterns(modified_files, ignore_app_dependencies_filepatterns, manifest_rootpath) ): - LOGGER.debug( - 'Skipping check component dependencies for apps since files %s matches patterns: %s', + LOGGER.info( + 'Build all apps since patterns %s matches modified files %s', ', '.join(modified_files), ', '.join(ignore_app_dependencies_filepatterns), ) @@ -106,6 +108,7 @@ def find_apps( ignore_app_dependencies_filepatterns: t.Optional[t.Union[t.List[str], str]] = None, sdkconfig_defaults: t.Optional[str] = None, include_skipped_apps: bool = False, + output: t.Optional[str] = None, ) -> t.List[App]: """ Find app directories in paths (possibly recursively), which contain apps for the given build system, compatible @@ -137,11 +140,12 @@ def find_apps( :param sdkconfig_defaults: semicolon-separated string, pass to idf.py -DSDKCONFIG_DEFAULTS if specified, also could be set via environment variables "SDKCONFIG_DEFAULTS" :param include_skipped_apps: include skipped apps or not + :param output: write the found apps to the specified file instead of stdout :return: list of found apps """ if default_build_targets: default_build_targets = to_list(default_build_targets) - LOGGER.info('Overriding DEFAULT_BUILD_TARGETS to %s', default_build_targets) + LOGGER.info('Overriding default build targets to %s', default_build_targets) FolderRule.DEFAULT_BUILD_TARGETS = default_build_targets if isinstance(build_system, str): @@ -205,10 +209,18 @@ def find_apps( include_skipped_apps=include_skipped_apps, ) ) - apps.sort() - LOGGER.info('Found %d apps in total', len(apps)) - return apps + LOGGER.info(f'Found {len(apps)} apps in total') + if output: + os.makedirs(os.path.dirname(os.path.realpath(output)), exist_ok=True) + with open(output, 'w') as fw: + for app in apps: + fw.write(app.model_dump_json() + '\n') + else: + for app in apps: + print(app) + + return sorted(apps) def build_apps( @@ -277,14 +289,6 @@ def build_apps( start, stop = get_parallel_start_stop(len(apps), parallel_count, parallel_index) LOGGER.info('Total %s apps. running build for app %s-%s', len(apps), start, stop) - exit_code = 0 - LOGGER.info('Building the following apps:') - if apps[start - 1 : stop]: - for app in apps[start - 1 : stop]: - LOGGER.info(' %s (preserve: %s)', app, app.preserve) - else: - LOGGER.info(' parallel count is too large. build nothing...') - build_job = BuildAppJob( parallel_count=parallel_count, parallel_index=parallel_index, @@ -299,9 +303,10 @@ def build_apps( for f in (build_job.collect_app_info, build_job.collect_size_info, build_job.junitxml): if f and os.path.isfile(f): os.remove(f) - LOGGER.info('Remove existing collect file %s', f) + LOGGER.debug('Remove existing collect file %s', f) Path(f).touch() + exit_code = 0 for i, app in enumerate(apps): index = i + 1 # we use 1-based if index < start or index > stop: @@ -334,7 +339,7 @@ def build_apps( if build_job.collect_app_info: with open(build_job.collect_app_info, 'a') as fw: fw.write(app.to_json() + '\n') - LOGGER.info('Recorded app info in %s', build_job.collect_app_info) + LOGGER.debug('Recorded app info in %s', build_job.collect_app_info) if copy_sdkconfig: try: @@ -345,7 +350,7 @@ def build_apps( except Exception as e: LOGGER.warning('Copy sdkconfig file from failed: %s', e) else: - LOGGER.info('Copied sdkconfig file from %s to %s', app.work_dir, app.build_path) + LOGGER.debug('Copied sdkconfig file from %s to %s', app.work_dir, app.build_path) if app.build_status == BuildStatus.FAILED: if not keep_going: @@ -367,30 +372,31 @@ def build_apps( ) + '\n' ) - LOGGER.info('Recorded size info file path in %s', build_job.collect_size_info) + LOGGER.debug('Recorded size info file path in %s', build_job.collect_size_info) LOGGER.info('') # add one empty line for separating different builds built_apps = [app for app in apps if app.build_status == BuildStatus.SUCCESS] if built_apps: - LOGGER.info('Built the following apps:') + print('Successfully built the following apps:') for app in built_apps: - LOGGER.info(' %s', app) + print(f' {app}') skipped_apps = [app for app in apps if app.build_status == BuildStatus.SKIPPED] if skipped_apps: - LOGGER.info('Skipped the following apps:') + print('Skipped building the following apps:') for app in skipped_apps: - LOGGER.info(' %s', app) + print(f' {app}') failed_apps = [app for app in apps if app.build_status == BuildStatus.FAILED] if failed_apps: - LOGGER.error('Build failed for the following apps:') + print('Failed building the following apps:') for app in failed_apps: - LOGGER.error(' %s', app) + print(f' {app}') if build_job.junitxml: TestReport([test_suite], build_job.junitxml).create_test_report() + LOGGER.info('Generated junit report for build apps: %s', build_job.junitxml) return exit_code @@ -732,6 +738,10 @@ def main(): validate_args(parser, args) SESSION_ARGS.set(args) + + if args.action == 'build': + args.output = None # build action doesn't support output option + # real call starts here apps = find_apps( args.paths, @@ -753,16 +763,10 @@ def main(): modified_files=args.modified_files, ignore_app_dependencies_filepatterns=args.ignore_app_dependencies_filepatterns, sdkconfig_defaults=args.sdkconfig_defaults, + output=args.output, ) if args.action == 'find': - if args.output: - with open(args.output, 'w') as f: - for app in apps: - f.write(str(app) + '\n') - else: - print('\n'.join([str(app) for app in apps])) - sys.exit(0) if args.no_preserve: diff --git a/idf_build_apps/manifest/manifest.py b/idf_build_apps/manifest/manifest.py index 795a025..f408b44 100644 --- a/idf_build_apps/manifest/manifest.py +++ b/idf_build_apps/manifest/manifest.py @@ -1,7 +1,8 @@ # SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 -import os.path +import logging +import os import typing as t import warnings from pathlib import ( @@ -13,9 +14,6 @@ ParseException, ) -from .. import ( - LOGGER, -) from ..constants import ( ALL_TARGETS, SUPPORTED_TARGETS, @@ -29,6 +27,8 @@ BoolStmt, ) +LOGGER = logging.getLogger(__name__) + class IfClause: def __init__(self, stmt: str, temporary: bool = False, reason: t.Optional[str] = None) -> None: @@ -120,7 +120,7 @@ def enable_build_targets( if default_sdkconfig_target: if default_sdkconfig_target not in res: - LOGGER.warning( + LOGGER.debug( 'sdkconfig defined `CONFIG_IDF_TARGET=%s` is not enabled for folder %s. Skip building this App...', default_sdkconfig_target, self.folder, diff --git a/idf_build_apps/manifest/soc_header.py b/idf_build_apps/manifest/soc_header.py index c44e350..eae4b8e 100644 --- a/idf_build_apps/manifest/soc_header.py +++ b/idf_build_apps/manifest/soc_header.py @@ -25,14 +25,13 @@ nums, ) -from .. import ( - LOGGER, -) from ..constants import ( ALL_TARGETS, IDF_PATH, ) +LOGGER = logging.getLogger(__name__) + # Group for parsing literal suffix of a numbers, e.g. 100UL _literal_symbol = Group(CaselessLiteral('L') | CaselessLiteral('U')) _literal_suffix = OneOrMore(_literal_symbol) @@ -53,7 +52,7 @@ def get_defines(header_path: Path) -> t.List[str]: defines = [] - logging.debug('Reading macros from %s...', header_path) + LOGGER.debug('Reading macros from %s...', header_path) with open(str(header_path)) as f: output = f.read() @@ -86,7 +85,7 @@ def __init__(self, target: str) -> None: def _get_dir_from_candidates(candidates: t.List[Path]) -> t.Optional[Path]: for d in candidates: if not d.is_dir(): - logging.debug('folder "%s" not found. Skipping...', d.absolute()) + LOGGER.debug('folder "%s" not found. Skipping...', d.absolute()) else: return d diff --git a/idf_build_apps/utils.py b/idf_build_apps/utils.py index 4d8b22c..4921b1d 100644 --- a/idf_build_apps/utils.py +++ b/idf_build_apps/utils.py @@ -3,6 +3,7 @@ import fnmatch import glob +import logging import os import shutil import subprocess @@ -20,12 +21,8 @@ ) from pydantic import BaseModel as _BaseModel -from . import ( - LOGGER, -) - try: - from typing import ( + from typing import ( # type: ignore Literal, ) except ImportError: @@ -34,12 +31,16 @@ ) +LOGGER = logging.getLogger(__name__) + + class ConfigRule: def __init__(self, file_name: str, config_name: t.Optional[str]) -> None: """ ConfigRule represents the sdkconfig file and the config name. For example: + - filename='', config_name='default' - represents the default app configuration, and gives it a name 'default' - filename='sdkconfig.*', config_name=None - represents the set of configurations, names match the wildcard From 49c88efc5bf7b0f956caad97ca69fd9b896e563f Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Thu, 7 Dec 2023 14:35:22 +0100 Subject: [PATCH 2/3] style: introduce mypy check. Fix all mypy warnings --- .pre-commit-config.yaml | 23 ++++++-- idf_build_apps/app.py | 77 +++++++++++++++++---------- idf_build_apps/build_job.py | 6 +-- idf_build_apps/config.py | 2 +- idf_build_apps/finder.py | 11 ++-- idf_build_apps/junit/report.py | 10 ++-- idf_build_apps/log.py | 2 +- idf_build_apps/main.py | 13 ++--- idf_build_apps/manifest/manifest.py | 14 ++--- idf_build_apps/manifest/soc_header.py | 2 +- idf_build_apps/utils.py | 77 +++++++++++++++++---------- pyproject.toml | 6 +++ 12 files changed, 151 insertions(+), 92 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index acde990..b1db90b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v4.5.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -8,7 +8,7 @@ repos: args: [ '-f=lf' ] - id: double-quote-string-fixer - repo: https://github.com/Lucas-C/pre-commit-hooks - rev: v1.5.1 + rev: v1.5.4 hooks: - id: insert-license files: \.py$ @@ -17,7 +17,7 @@ repos: - license_header.txt # defaults to: LICENSE.txt - --use-current-year - repo: https://github.com/asottile/pyupgrade - rev: v3.10.1 + rev: v3.15.0 hooks: - id: pyupgrade args: [--py37-plus] @@ -26,11 +26,24 @@ repos: hooks: - id: isort - repo: https://github.com/psf/black - rev: 23.7.0 + rev: 23.11.0 hooks: - id: black - repo: https://github.com/charliermarsh/ruff-pre-commit - rev: 'v0.0.281' + rev: 'v0.1.7' hooks: - id: ruff args: ['--fix'] + - repo: https://github.com/pre-commit/mirrors-mypy + rev: 'v1.7.1' + hooks: + - id: mypy + args: ['--warn-unused-ignores'] + additional_dependencies: + - pydantic + - packaging + - toml + - pyparsing + - types-PyYAML + - types-toml + - pytest diff --git a/idf_build_apps/app.py b/idf_build_apps/app.py index 894af17..fe5b621 100644 --- a/idf_build_apps/app.py +++ b/idf_build_apps/app.py @@ -57,7 +57,6 @@ from .utils import ( BaseModel, BuildError, - Literal, files_matches_patterns, find_first_match, rmdir, @@ -66,6 +65,15 @@ to_list, ) +if sys.version_info < (3, 8): + from typing_extensions import ( + Literal, + ) +else: + from typing import ( + Literal, + ) + class _AppBuildStageFilter(logging.Filter): def __init__(self, *args, app, **kwargs): @@ -172,8 +180,8 @@ def __init__( # sdkconfig attrs, use properties instead self._sdkconfig_defaults = self._get_sdkconfig_defaults(sdkconfig_defaults_str) - self._sdkconfig_files = None - self._sdkconfig_files_defined_target = None + self._sdkconfig_files: t.List[str] = None # type: ignore + self._sdkconfig_files_defined_target: str = None # type: ignore # pass all parameters to initialize hook method kwargs.update( @@ -229,7 +237,15 @@ def _get_sdkconfig_defaults(sdkconfig_defaults_str: t.Optional[str] = None) -> t return candidates + @t.overload + def _expand(self, path: None) -> None: + ... + + @t.overload def _expand(self, path: str) -> str: + ... + + def _expand(self, path): """ Internal method, expands any of the placeholders in {app,work,build} paths. """ @@ -264,21 +280,21 @@ def _expand(self, path: str) -> str: def name(self) -> str: return os.path.basename(os.path.realpath(self.app_dir)) - @computed_field + @computed_field # type: ignore @property def work_dir(self) -> str: """ :return: directory where the app should be copied to, prior to the build. """ - return self._expand(self._work_dir) + return self._expand(self._work_dir) # type: ignore - @computed_field + @computed_field # type: ignore @property def build_dir(self) -> str: """ :return: build directory, either relative to the work directory (if relative path is used) or absolute path. """ - return self._expand(self._build_dir) + return self._expand(self._build_dir) # type: ignore @property def build_path(self) -> str: @@ -287,7 +303,7 @@ def build_path(self) -> str: return os.path.join(self.work_dir, self.build_dir) - @computed_field + @computed_field # type: ignore @property def build_log_filename(self) -> t.Optional[str]: return self._expand(self._build_log_filename) @@ -299,7 +315,7 @@ def build_log_path(self) -> t.Optional[str]: return None - @computed_field + @computed_field # type: ignore @property def size_json_filename(self) -> t.Optional[str]: if self.target == 'linux': @@ -315,7 +331,7 @@ def size_json_path(self) -> t.Optional[str]: return None - @computed_field + @computed_field # type: ignore @property def config(self) -> t.Optional[str]: return self.config_name @@ -455,7 +471,7 @@ def wrapper(self, *args, **kwargs): return wrapper - @record_build_duration + @record_build_duration # type: ignore def build( self, manifest_rootpath: t.Optional[str] = None, @@ -514,7 +530,7 @@ def build( return if self.build_log_path: - logfile = open(self.build_log_path, 'w') + logfile: t.IO[str] = open(self.build_log_path, 'w') keep_logfile = True else: # delete manually later, used for tracking debugging info @@ -565,7 +581,7 @@ def build( self._logger.debug('Removed temporary build log file: %s', logfile.name) # Generate Size Files - if self.build_status == BuildStatus.SUCCESS and self.size_json_path: + if self.build_status == BuildStatus.SUCCESS: self.write_size_json() # Cleanup build directory if not preserving @@ -591,15 +607,18 @@ def build( def _build( self, - logfile: t.TextIO, + logfile: t.IO[str], manifest_rootpath: t.Optional[str] = None, modified_components: t.Optional[t.List[str]] = None, modified_files: t.Optional[t.List[str]] = None, check_app_dependencies: bool = False, - ) -> bool: + ) -> None: pass def _write_size_json(self) -> None: + if not self.size_json_path: + return + map_file = find_first_match('*.map', self.build_path) if not map_file: self._logger.warning( @@ -678,12 +697,12 @@ def is_modified(self, modified_files: t.Optional[t.List[str]]) -> bool: return False - def check_should_build( + def _check_should_build( self, - manifest_rootpath: str, - check_app_dependencies: bool, - modified_components: t.Union[t.List[str], str, None], - modified_files: t.Union[t.List[str], str, None], + manifest_rootpath: t.Optional[str] = None, + check_app_dependencies: bool = False, + modified_components: t.Optional[t.List[str]] = None, + modified_files: t.Optional[t.List[str]] = None, ) -> None: if self.build_status != BuildStatus.UNKNOWN: return @@ -748,7 +767,7 @@ def check_should_build( class MakeApp(App): MAKE_PROJECT_LINE: t.ClassVar[str] = r'include $(IDF_PATH)/make/project.mk' - build_system: Literal['make'] = 'make' + build_system: Literal['make'] = 'make' # type: ignore @property def supported_targets(self) -> t.List[str]: @@ -764,10 +783,10 @@ def supported_targets(self) -> t.List[str]: def _build( self, - logfile: t.TextIO, + logfile: t.IO[str], manifest_rootpath: t.Optional[str] = None, - modified_components: t.Union[t.List[str], str, None] = None, - modified_files: t.Union[t.List[str], str, None] = None, + modified_components: t.Optional[t.List[str]] = None, + modified_files: t.Optional[t.List[str]] = None, check_app_dependencies: bool = False, ) -> None: # additional env variables @@ -825,20 +844,20 @@ class CMakeApp(App): # there is no equivalent for the project CMakeLists files. This seems to be the best option... CMAKE_PROJECT_LINE: t.ClassVar[str] = r'include($ENV{IDF_PATH}/tools/cmake/project.cmake)' - build_system: Literal['cmake'] = 'cmake' + build_system: Literal['cmake'] = 'cmake' # type: ignore cmake_vars: t.Dict[str, str] = {} def _build( self, - logfile: t.TextIO, + logfile: t.IO[str], manifest_rootpath: t.Optional[str] = None, - modified_components: t.Union[t.List[str], str, None] = None, - modified_files: t.Union[t.List[str], str, None] = None, + modified_components: t.Optional[t.List[str]] = None, + modified_files: t.Optional[t.List[str]] = None, check_app_dependencies: bool = False, ) -> None: if not self._checked_should_build: - self.check_should_build( + self._check_should_build( manifest_rootpath=manifest_rootpath, modified_components=modified_components, modified_files=modified_files, diff --git a/idf_build_apps/build_job.py b/idf_build_apps/build_job.py index 96c6774..53fae80 100644 --- a/idf_build_apps/build_job.py +++ b/idf_build_apps/build_job.py @@ -36,7 +36,7 @@ def __init__( self._collect_app_info = collect_app_info self._collect_size_info = collect_size_info - @computed_field + @computed_field # type: ignore @property def collect_app_info(self) -> t.Optional[str]: if self._collect_app_info: @@ -44,7 +44,7 @@ def collect_app_info(self) -> t.Optional[str]: return None - @computed_field + @computed_field # type: ignore @property def collect_size_info(self) -> t.Optional[str]: if self._collect_size_info: @@ -52,7 +52,7 @@ def collect_size_info(self) -> t.Optional[str]: return None - @computed_field + @computed_field # type: ignore @property def junitxml(self) -> t.Optional[str]: if self._junitxml: diff --git a/idf_build_apps/config.py b/idf_build_apps/config.py index 69547de..a86dd8e 100644 --- a/idf_build_apps/config.py +++ b/idf_build_apps/config.py @@ -23,7 +23,7 @@ def __init__(self, filepath: t.Union[str, Path], msg: str) -> None: def load_toml(filepath: t.Union[str, Path]) -> dict: try: - import tomllib # python 3.11 + import tomllib # type: ignore # python 3.11 try: with open(str(filepath), 'rb') as fr: diff --git a/idf_build_apps/finder.py b/idf_build_apps/finder.py index a3a7439..2c89891 100644 --- a/idf_build_apps/finder.py +++ b/idf_build_apps/finder.py @@ -31,7 +31,7 @@ def _get_apps_from_path( app_cls: t.Type[App] = CMakeApp, work_dir: t.Optional[str] = None, build_dir: str = 'build', - config_rules_str: t.Union[t.List[str], str, None] = None, + config_rules_str: t.Optional[t.List[str]] = None, build_log_filename: t.Optional[str] = None, size_json_filename: t.Optional[str] = None, check_warnings: bool = False, @@ -51,7 +51,7 @@ def _validate_app(_app: App) -> bool: LOGGER.debug('=> Ignored. %s only supports targets: %s', _app, ', '.join(_app.supported_targets)) return False - _app.check_should_build( + _app._check_should_build( manifest_rootpath=manifest_rootpath, modified_components=modified_components, modified_files=modified_files, @@ -80,8 +80,7 @@ def _validate_app(_app: App) -> bool: default_config_name = rule.config_name continue - sdkconfig_paths = Path(path).glob(rule.file_name) - sdkconfig_paths = sorted([str(p.relative_to(path)) for p in sdkconfig_paths]) + sdkconfig_paths = sorted([str(p.relative_to(path)) for p in Path(path).glob(rule.file_name)]) if sdkconfig_paths: sdkconfig_paths_matched = True # skip the next block for no wildcard config rules @@ -170,11 +169,11 @@ def _find_apps( # The remaining part is for recursive == True apps = [] # handle the exclude list, since the config file might use linux style, but run in windows - exclude_list = [to_absolute_path(p) for p in exclude_list] + exclude_paths_list = [to_absolute_path(p) for p in exclude_list] for root, dirs, _ in os.walk(path): LOGGER.debug('Entering %s', root) root_path = to_absolute_path(root) - if root_path in exclude_list: + if root_path in exclude_paths_list: LOGGER.debug('=> Skipping %s (excluded)', root) del dirs[:] continue diff --git a/idf_build_apps/junit/report.py b/idf_build_apps/junit/report.py index 0084218..e6d5c1e 100644 --- a/idf_build_apps/junit/report.py +++ b/idf_build_apps/junit/report.py @@ -34,6 +34,7 @@ """ import json +import logging import os.path import typing as t from datetime import ( @@ -44,9 +45,6 @@ escape, ) -from .. import ( - LOGGER, -) from ..app import ( App, ) @@ -57,6 +55,8 @@ get_sys_info, ) +LOGGER = logging.getLogger(__name__) + class TestCase: def __init__( @@ -89,7 +89,7 @@ def from_app(cls, app: App) -> 'TestCase': if app.build_status not in (BuildStatus.FAILED, BuildStatus.SUCCESS, BuildStatus.SKIPPED): raise ValueError(f'Cannot create test case from app with build status {app.build_status}') - kwargs = { + kwargs: t.Dict[str, t.Any] = { 'name': app.build_path, 'duration_sec': app._build_duration, 'timestamp': app._build_timestamp, @@ -105,7 +105,7 @@ def from_app(cls, app: App) -> 'TestCase': for k, v in json.load(f).items(): kwargs['properties'][f'{k}'] = str(v) - return cls(**kwargs) + return cls(**kwargs) # type @property def is_failed(self) -> bool: diff --git a/idf_build_apps/log.py b/idf_build_apps/log.py index efb9ce4..8403318 100644 --- a/idf_build_apps/log.py +++ b/idf_build_apps/log.py @@ -77,7 +77,7 @@ def setup_logging(verbose: int = 0, log_file: t.Optional[str] = None, colored: b package_logger = logging.getLogger(__package__) package_logger.setLevel(level) if log_file: - handler = logging.FileHandler(log_file) + handler: logging.Handler = logging.FileHandler(log_file) else: handler = logging.StreamHandler(sys.stderr) handler.setFormatter(ColoredFormatter(colored)) diff --git a/idf_build_apps/main.py b/idf_build_apps/main.py index cc3a02a..71aaeae 100644 --- a/idf_build_apps/main.py +++ b/idf_build_apps/main.py @@ -60,10 +60,10 @@ def _check_app_dependency( - manifest_rootpath: str, - modified_components: t.Optional[t.List[str]], - modified_files: t.Optional[t.List[str]], - ignore_app_dependencies_filepatterns: t.Optional[t.List[str]], + manifest_rootpath: t.Optional[str] = None, + modified_components: t.Optional[t.List[str]] = None, + modified_files: t.Optional[t.List[str]] = None, + ignore_app_dependencies_filepatterns: t.Optional[t.List[str]] = None, ) -> bool: # not check since modified_components and modified_files are not passed if modified_components is None and modified_files is None: @@ -173,6 +173,7 @@ def find_apps( modified_components = to_list(modified_components) modified_files = to_list(modified_files) ignore_app_dependencies_filepatterns = to_list(ignore_app_dependencies_filepatterns) + config_rules_str = to_list(config_rules_str) apps = [] if target == 'all': @@ -240,8 +241,8 @@ def build_apps( # BuildAppJob parallel_count: int = 1, parallel_index: int = 1, - collect_size_info: t.Optional[t.Union[str, t.TextIO]] = None, - collect_app_info: t.Optional[t.Union[str, t.TextIO]] = None, + collect_size_info: t.Optional[str] = None, + collect_app_info: t.Optional[str] = None, junitxml: t.Optional[str] = None, ) -> int: """ diff --git a/idf_build_apps/manifest/manifest.py b/idf_build_apps/manifest/manifest.py index f408b44..1186637 100644 --- a/idf_build_apps/manifest/manifest.py +++ b/idf_build_apps/manifest/manifest.py @@ -53,9 +53,9 @@ class FolderRule: def __init__( self, folder: Path, - enable: t.Optional[t.List[t.Dict[str, str]]] = None, - disable: t.Optional[t.List[t.Dict[str, str]]] = None, - disable_test: t.Optional[t.List[t.Dict[str, str]]] = None, + enable: t.Optional[t.List[t.Dict[str, t.Any]]] = None, + disable: t.Optional[t.List[t.Dict[str, t.Any]]] = None, + disable_test: t.Optional[t.List[t.Dict[str, t.Any]]] = None, depends_components: t.Optional[t.List[str]] = None, depends_filepatterns: t.Optional[t.List[str]] = None, ) -> None: @@ -104,7 +104,7 @@ def _enable_test( if self.disable or self.disable_test: for clause in self.disable + self.disable_test: - if clause.get_value(target, config_name): + if clause.get_value(target, config_name or ''): res = False break @@ -115,7 +115,7 @@ def enable_build_targets( ) -> t.List[str]: res = [] for target in ALL_TARGETS: - if self._enable_build(target, config_name): + if self._enable_build(target, config_name or ''): res.append(target) if default_sdkconfig_target: @@ -154,7 +154,7 @@ def __init__(self, folder: Path) -> None: class Manifest: # could be reassigned later - ROOTPATH = os.curdir + ROOTPATH = Path(os.curdir) CHECK_MANIFEST_RULES = False def __init__( @@ -168,7 +168,7 @@ def from_file(cls, path: str) -> 'Manifest': with open(path) as f: manifest_dict = yaml.safe_load(f) or {} - rules = [] # type: list[FolderRule] + rules: t.List[FolderRule] = [] for folder, folder_rule in manifest_dict.items(): # not a folder, but a anchor if folder.startswith('.'): diff --git a/idf_build_apps/manifest/soc_header.py b/idf_build_apps/manifest/soc_header.py index eae4b8e..52cc9ce 100644 --- a/idf_build_apps/manifest/soc_header.py +++ b/idf_build_apps/manifest/soc_header.py @@ -45,7 +45,7 @@ _int_value = Word(nums)('int_value') + ~Char('.') + Optional(_literal_suffix)('literal_suffix') # Remove optional parenthesis around values -_value = Optional('(').suppress() + MatchFirst(_hex_value | _str_value | _int_value)('value') + Optional(')').suppress() +_value = Optional('(').suppress() + MatchFirst([_hex_value, _str_value, _int_value])('value') + Optional(')').suppress() _define_expr = '#define' + Optional(_name)('name') + Optional(_value) diff --git a/idf_build_apps/utils.py b/idf_build_apps/utils.py index 4921b1d..d759af8 100644 --- a/idf_build_apps/utils.py +++ b/idf_build_apps/utils.py @@ -21,21 +21,11 @@ ) from pydantic import BaseModel as _BaseModel -try: - from typing import ( # type: ignore - Literal, - ) -except ImportError: - from typing_extensions import ( # isort: skip # noqa: F401 - Literal, - ) - - LOGGER = logging.getLogger(__name__) class ConfigRule: - def __init__(self, file_name: str, config_name: t.Optional[str]) -> None: + def __init__(self, file_name: str, config_name: str = '') -> None: """ ConfigRule represents the sdkconfig file and the config name. @@ -55,7 +45,7 @@ def __init__(self, file_name: str, config_name: t.Optional[str]) -> None: self.config_name = config_name -def config_rules_from_str(rule_strings: t.Union[t.List[str], str]) -> t.List[ConfigRule]: +def config_rules_from_str(rule_strings: t.Optional[t.List[str]]) -> t.List[ConfigRule]: """ Helper function to convert strings like 'file_name=config_name' into `ConfigRule` objects @@ -68,7 +58,7 @@ def config_rules_from_str(rule_strings: t.Union[t.List[str], str]) -> t.List[Con rules = [] for rule_str in to_list(rule_strings): items = rule_str.split('=', 2) - rules.append(ConfigRule(items[0], items[1] if len(items) == 2 else None)) + rules.append(ConfigRule(items[0], items[1] if len(items) == 2 else '')) # '' is the default config, sort this one to the front return sorted(rules, key=lambda x: x.file_name) @@ -147,7 +137,7 @@ def find_first_match(pattern: str, path: str) -> t.Optional[str]: def subprocess_run( cmd: t.List[str], log_terminal: bool = True, - log_fs: t.Optional[t.TextIO] = None, + log_fs: t.Optional[t.IO[str]] = None, check: bool = False, additional_env_dict: t.Optional[t.Dict[str, str]] = None, **kwargs, @@ -170,15 +160,16 @@ def subprocess_run( subprocess_env.update(additional_env_dict) p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=subprocess_env, **kwargs) - for line in p.stdout: - if isinstance(line, bytes): - line = line.decode('utf-8') + if p.stdout: + for line in p.stdout: + if isinstance(line, bytes): + line = line.decode('utf-8') - if log_terminal: - sys.stdout.write(line) + if log_terminal: + sys.stdout.write(line) - if log_fs: - log_fs.write(line) + if log_fs: + log_fs.write(line) returncode = p.wait() if check and returncode != 0: @@ -187,7 +178,25 @@ def subprocess_run( return returncode -def to_list(s: t.Any) -> t.Optional[t.List[t.Any]]: +_T = t.TypeVar('_T') + + +@t.overload +def to_list(s: None) -> None: + ... + + +@t.overload +def to_list(s: t.Iterable[_T]) -> t.List[_T]: + ... + + +@t.overload +def to_list(s: _T) -> t.List[_T]: + ... + + +def to_list(s): """ Turn all objects to lists @@ -211,7 +220,22 @@ def to_list(s: t.Any) -> t.Optional[t.List[t.Any]]: return [s] -def to_set(s: t.Any) -> t.Optional[t.Set[t.Any]]: +@t.overload +def to_set(s: None) -> None: + ... + + +@t.overload +def to_set(s: t.Iterable[_T]) -> t.Set[_T]: + ... + + +@t.overload +def to_set(s: _T) -> t.Set[_T]: + ... + + +def to_set(s): """ Turn all objects to sets @@ -273,14 +297,11 @@ def files_matches_patterns( ) -> bool: # can't match an absolute pattern with a relative path # change all to absolute paths - files = [to_absolute_path(f, rootpath) for f in to_list(files)] - patterns = [to_absolute_path(p, rootpath) for p in to_list(patterns)] - matched_paths = set() - for pat in patterns: + for pat in [to_absolute_path(p, rootpath) for p in to_list(patterns)]: matched_paths.update(glob.glob(str(pat), recursive=True)) - for f in files: + for f in [to_absolute_path(f, rootpath) for f in to_list(files)]: if str(f) in matched_paths: return True diff --git a/pyproject.toml b/pyproject.toml index fba1058..0aa7784 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,6 +30,9 @@ dependencies = [ ] [project.optional-dependencies] +dev = [ + "typing-extensions; python_version < '3.8'", +] test = [ "pytest", "pytest-cov", @@ -130,3 +133,6 @@ target-version = "py37" typing-modules = [ "idf_build_apps.utils" ] + +[tool.mypy] +python_version = "3.7" From b03813c7dba703dd6281ae6c510807b119b31cd0 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Thu, 7 Dec 2023 15:15:53 +0100 Subject: [PATCH 3/3] tests: fix pytest cases warnings --- tests/test_finder.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/test_finder.py b/tests/test_finder.py index 39854d0..6856c54 100644 --- a/tests/test_finder.py +++ b/tests/test_finder.py @@ -42,7 +42,10 @@ def test_manifest_rootpath_chdir(self): # manifest folder invalid os.chdir(test_dir) - assert find_apps(str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file)) + with pytest.warns( + UserWarning, match=f'Folder "{IDF_PATH}/examples/get-started/examples/get-started" does not exist' + ): + assert find_apps(str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file)) def test_manifest_rootpath_specified(self): test_dir = IDF_PATH / 'examples' / 'get-started' @@ -56,10 +59,10 @@ def test_manifest_rootpath_specified(self): ''', encoding='utf8', ) - - assert find_apps( - str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file), manifest_rootpath=str(IDF_PATH) - ) + with pytest.warns(UserWarning, match=f'Folder "{IDF_PATH}/get-started" does not exist.'): + assert find_apps( + str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file), manifest_rootpath=str(IDF_PATH) + ) assert not find_apps( str(test_dir), @@ -95,13 +98,16 @@ def test_keyword_idf_version(self): yaml_file = test_dir / 'test.yml' yaml_file.write_text( ''' -get-started: +examples/get-started: enable: - if: IDF_VERSION_MAJOR > 0 and IDF_VERSION_MINOR < 999 and IDF_VERSION_PATCH in [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] ''', encoding='utf8', ) - assert find_apps(str(test_dir), 'esp32', recursive=True, manifest_files=str(yaml_file)) == apps + assert ( + find_apps(str(test_dir), 'esp32', recursive=True, manifest_rootpath=IDF_PATH, manifest_files=str(yaml_file)) + == apps + ) class TestFindWithModifiedFilesComponents: