Skip to content

Commit

Permalink
Merge pull request #92 from espressif/feat/improve_log
Browse files Browse the repository at this point in the history
fix: improve logging output. Differentiate print and logging.info
  • Loading branch information
hfudev authored Dec 8, 2023
2 parents eac8538 + b03813c commit 8b9d457
Show file tree
Hide file tree
Showing 16 changed files with 254 additions and 166 deletions.
23 changes: 18 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
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
- id: mixed-line-ending
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$
Expand All @@ -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]
Expand All @@ -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
15 changes: 15 additions & 0 deletions docs/find_build.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <file>` 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.
Expand Down
3 changes: 2 additions & 1 deletion idf_build_apps/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -41,4 +41,5 @@
'find_apps',
'build_apps',
'setup_logging',
'SESSION_ARGS',
]
95 changes: 60 additions & 35 deletions idf_build_apps/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
SESSION_ARGS,
)

from . import (
LOGGER,
)
from .build_job import (
BuildAppJob,
)
Expand All @@ -60,7 +57,6 @@
from .utils import (
BaseModel,
BuildError,
Literal,
files_matches_patterns,
find_first_match,
rmdir,
Expand All @@ -69,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):
Expand Down Expand Up @@ -175,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(
Expand All @@ -189,8 +194,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()
Expand All @@ -202,6 +207,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,
Expand All @@ -223,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.
"""
Expand Down Expand Up @@ -258,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:
Expand All @@ -281,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)
Expand All @@ -293,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':
Expand All @@ -309,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
Expand Down Expand Up @@ -449,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,
Expand Down Expand Up @@ -508,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
Expand Down Expand Up @@ -559,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
Expand All @@ -585,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(
Expand Down Expand Up @@ -632,7 +657,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:
Expand Down Expand Up @@ -672,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
Expand Down Expand Up @@ -742,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]:
Expand All @@ -758,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
Expand Down Expand Up @@ -819,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,
Expand Down
6 changes: 3 additions & 3 deletions idf_build_apps/build_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,23 @@ 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:
return self.expand(self._collect_app_info)

return None

@computed_field
@computed_field # type: ignore
@property
def collect_size_info(self) -> t.Optional[str]:
if self._collect_size_info:
return self.expand(self._collect_size_info)

return None

@computed_field
@computed_field # type: ignore
@property
def junitxml(self) -> t.Optional[str]:
if self._junitxml:
Expand Down
2 changes: 1 addition & 1 deletion idf_build_apps/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 8b9d457

Please sign in to comment.