As part of inter-procedural optimizations (IPA), the compiler may
perform tasks such as constant propagation for functions. This involves
generating a specialized version of a given function with a new symbol
name that includes a suffix. For example, during constant propagation,
the compiler might create a specialized version named
`spiflash_start_core.constprop.0` for the `spiflash_start_core`
function. Additionally, the compiler may generate multiple clones of a
single function. Currently, when ldgen performs symbol placement, it
does not account for these compiler-generated functions, leading to
their incorrect or unexpected placement in memory (markers).
Consider a linker fragment with:
```
[mapping:spi_flash]
archive: libspi_flash.a
entries:
esp_flash_api: spiflash_start_core (noflash)
```
The `spiflash_start_core` function should be placed in IRAM. However,
the compiler might generate an optimized version of this function with a
`.constprop.0` suffix, resulting in a
`.text.spiflash_start_core.constprop.0` input section. Currently, ldgen
does not handle this situation, leading to misplaced symbols.
Since `.` is not allowed in C identifiers, it should be safe to consider
all input sections for a symbol with any `.` suffix as representing that
symbol. This means considering the symbol suffixes should not cause any
ambiguity.
This change automatically places all input sections, including those
with possible suffixes for a given symbol, into the specified memory. In
other words, specifying a function name like `spiflash_start_core` in a
linker fragment automatically includes input section names matching
`spiflash_start_core(\..*)?$`.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The latest cJSON vulnerabilities appear to list "davegamble" as the
vendor instead of "cjson_project." Add a new CPE with "davegamble" as
the vendor.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
For the Linux target, we currently attempt to fallback to older C/CXX
lagnuage standards in the __build_set_lang_version() function. The
language standard support is checked using CMake's language-specific
functions, such as check_c_compiler_flag(). These functions require the
language to be enabled[1] in CMake beforehand, which is done by calling
project() or by enabling the languages later with enable_language(). At
present, we use enable_language() to enable C and CXX languages in
CMake, allowing us to set the standard early, before invoking project().
However, newer CMake versions (>3.29) issue a warning[2] if
enable_language() is called before project(), as noted in CMP0165[3].
It should generally be acceptable to call __build_set_lang_version()
after __project(), but doing so would alter the behavior of the
COMPILE_OPTIONS also for non-Linux targets. Currently, users can
add to COMPILE_OPTIONS even before calling project() in the project's
CMakeLists.txt and the options will be in the desired order. In other
words, appending to COMPILE_OPTIONS can occur either before or after
calling project() in the project's CMakeLists.txt, with the outcome
remaining consistent. This means the user's settings will appear later
and take priority. However, if __build_set_lang_version() is called
after __project(), the user's COMPILE_OPTIONS settings would be
overridden if set before calling project(). Our documentation[4] explicitly
states that COMPILE_OPTIONS and similar properties should be modified
using idf_build_set_property() after calling project() to prevent
default values from overwriting them.
Even with this guidance, some existing components that modify
COMPILE_OPTIONS before invoking project() might be impacted by this
change. Therefore, separate the language standard settings for non-Linux
and Linux targets. For non-Linux targets, these settings are applied in
__build_set_default_build_specifications(), maintaining the current
behavior. For the Linux target, the language standard is set with
__linux_build_set_lang_version() after calling __project(), ensuring the
languages are already enabled in CMake and no warning is issued. Since the Linux
target is still in preview, this approach should be acceptable,
especially with the existing documentation[4].
Closes https://github.com/espressif/esp-idf/issues/15488
[1] https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#languages
[2] https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9396
[3] https://cmake.org/cmake/help/latest/policy/CMP0165.html#policy:CMP0165
[4] https://docs.espressif.com/projects/esp-idf/en/v5.4/esp32/api-guides/
build-system.html#overriding-default-build-specifications
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The `setuptools` package starting with `v70.1.0`[1] contains built-in
`bdist_wheel` command. Before this version `setuptools` relied on the
`bdist_wheel` command implementation from the `wheel` package. Starting with
`setuptools` `v75.8.1` the `PEP 491`[3] restrictions on the distribution name
of a wheel package are enforced[4], replacting also `.` with `_`. Note that
`PEP 491` actually allows `.` in the distribution name, but for some reason the
latest packaging docs[10][11] does not, stating that `.` should be replaced
with `_`. This was discussion here[12].
Also the `wheel` package starting with `v0.45.0`[5] is using the `bdist_wheel`
command from `setuptools`. This means that any package which has `.` in its
distribution name, like `ruamel.yaml.clib`, can have different wheel name,
depending on which version of the `bdist_wheel` command was used.
The `bdist_wheel` command from setuptools prior `v75.8.1` or `wheel` prior
`v0.45.0` will keep the dots in distribution name preserved. For exaple the
`ruamel.yaml.clib` package will have distribution name
`ruamel.yaml.clib-0.2.12.dist-info. Newer versions will replace the dots with
`_` according to [10][11], creating distribution like
`ruamel_yaml_clib-0.2.12.dist-info`.
From packaging point of view `ruamel.yaml.clib-0.2.12.dist-info` and
`ruamel_yaml_clib-0.2.12.dist-info` are the same packages, but this is not
reflected in `importlib.metadata` prior python 3.10[9], which does not perform
name normalization prior the distribution search. This causes the `version`
from `importlib.metadata` to fail on python prior the 3.10 version if the
package with dots in distribution name was generated with normalized paths with
newer `setuptools`. Note that the distribution name normalization was
backported to some later 3.9 python version.
Let's demonstrate this behavior on a simple package with the
`my.minimal.package` name.
```
my_minimal_package/
├── pkg
│ └── __init__.py
└── setup.py
from setuptools import setup, find_packages
setup(
name='my.minimal.package',
version='0.1.0',
packages=find_packages(),
install_requires=[],
entry_points={},
)
```
With python 3.9.0 search for `my.minimal.package` fails because
of the missing name normalization.
```
docker run --rm -it --platform linux/x86_64 python:3.9.0 bash
python -m venv venv
. venv/bin/activate
pip install setuptools==v75.8.1
python setup.py bdist_wheel
pip install dist/my_minimal_package-0.1.0-py3-none-any.whl
python
Python 3.9.0 (default, Nov 18 2020, 13:28:38)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib.metadata import version as get_version
>>> get_version('my.minimal.package')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/lib/python3.9/importlib/metadata.py", line 551, in version
return distribution(distribution_name).version
File "/usr/local/lib/python3.9/importlib/metadata.py", line 524, in distribution
return Distribution.from_name(distribution_name)
File "/usr/local/lib/python3.9/importlib/metadata.py", line 187, in from_name
raise PackageNotFoundError(name)
importlib.metadata.PackageNotFoundError: my.minimal.package
>>> get_version('my_minimal_package')
'0.1.0'
```
With python 3.10.0 search for both `my.minimal.package` and
`my_minimal_package` succeeds.
```
docker run --rm -it --platform linux/x86_64 python:3.10.0 bash
python
Python 3.10.0 (default, Dec 3 2021, 00:21:30) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib.metadata import version as get_version
>>> get_version('my.minimal.package')
'0.1.0'
>>> get_version('my_minimal_package')
'0.1.0'
```
In our `tools/check_python_dependencies.py` we cannot relay on the default
distribution finder, used in the `version` function from `importlib.metadata`,
to do name normalization on older python versions. To cope with this,
implement a fallback version search. If `version` fails with
`PackageNotFoundError`, do the name normalization according to [10][11] and try
again.
Note: There is also a `wheel`[6][7] `v0.43.0` package embeded in `setuptools`
along with the new implementation[8]. This one seems to be used if the
external `wheel` package is not available but imported. TBH this is all kinda
messy and I may have overlooked something.
* [1] https://setuptools.pypa.io/en/stable/history.html#v70-1-0
* [2] https://setuptools.pypa.io/en/stable/history.html#v75-8-1
* [3] https://peps.python.org/pep-0491/#escaping-and-unicode
* [4] https://github.com/pypa/setuptools/pull/4766/files
* [5] https://wheel.readthedocs.io/en/stable/news.html
* [6] https://github.com/pypa/setuptools/blob/main/setuptools/_vendor/wheel/__init__.py
* [7] https://github.com/pypa/setuptools/issues/1386
* [8] https://github.com/pypa/setuptools/blob/main/setuptools/command/bdist_wheel.py
* [9] https://github.com/python/cpython/commit/c6ca368867bd68d44f333df840aa85d425a51410
* [10] https://packaging.python.org/en/latest/specifications/name-normalization/#name-normalization
* [11] https://packaging.python.org/en/latest/specifications/binary-distribution-format/
#escaping-and-unicode
* [12] https://github.com/pypa/setuptools/issues/3777
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
When a symbol needs to be placed to a different target than the one
designated for the object file, the object file is expanded, which
includes the following steps:
1. Creating a new placement for the symbol's input section with the
specified target.
2. Excluding the object placement from the orignal target.
3. Creating a new intermediate placement for the object for the original
target, where its input sections are expanded, excluding the input
section for the symbol.
Let's illustrate the object expansion process with the following example:
[sections:rodata]
entries:
.rodata+
.sdata2+
.srodata+
[scheme:default]
entries:
text -> flash_text
rodata -> flash_rodata
[scheme:noflash]
entries:
text -> iram0_text
rodata -> dram0_data
[mapping:soc_pm]
archive: libsoc.a
entries:
gpio_periph: GPIO_HOLD_MASK (noflash)
gpio_periph section headers:
[Nr] Name Type Addr Off Size ES Flg Lk Inf Al
[ 0] NULL 00000000 000000 000000 00 0 0 0
[ 1] .text PROGBITS 00000000 000034 000000 00 AX 0 0 2
[ 2] .data PROGBITS 00000000 000034 000000 00 WA 0 0 1
[ 3] .bss NOBITS 00000000 000034 000000 00 WA 0 0 1
[ 4] .rodata.GPIO_HOLD_MASK PROGBITS 00000000 000034 000058 00 A 0 0 4
[ 5] .rodata.GPIO_PIN_MUX_REG PROGBITS 00000000 00008c 000058 00 A 0 0 4
[ 6] .debug_info PROGBITS 00000000 0000e4 0000d8 00 0 0 1
[ 7] .rela.debug_info RELA 00000000 0009d4 000108 0c I 16 6 4
[ 8] .debug_abbrev PROGBITS 00000000 0001bc 000070 00 0 0 1
[ 9] .debug_aranges PROGBITS 00000000 00022c 000018 00 0 0 1
[10] .rela.debug_aranges RELA 00000000 000adc 00000c 0c I 16 9 4
[11] .debug_line PROGBITS 00000000 000244 0001ab 00 0 0 1
[12] .debug_str PROGBITS 00000000 0003ef 00022d 01 MS 0 0 1
[13] .comment PROGBITS 00000000 00061c 000030 01 MS 0 0 1
[14] .note.GNU-stack PROGBITS 00000000 00064c 000000 00 0 0 1
[15] .riscv.attributes RISCV_ATTRIBUTES 00000000 00064c 000044 00 0 0 1
[16] .symtab SYMTAB 00000000 000690 000260 10 17 36 4
[17] .strtab STRTAB 00000000 0008f0 0000e1 00 0 0 1
[18] .shstrtab STRTAB 00000000 000ae8 0000d1 00 0 0 1
1. Creating a new placement
.dram0.data :
{
*libsoc.a:gpio_periph.*(.rodata.GPIO_HOLD_MASK .sdata2.GPIO_HOLD_MASK .srodata.GPIO_HOLD_MASK)
}
2. Excluding the object placement
.flash.rodata :
{
*(EXCLUDE_FILE(*libsoc.a:gpio_periph.*) .rodata.* ...)
}
3. Creating a new intermediate placement
.flash.rodata :
{
*libsoc.a:gpio_periph.*(.rodata.GPIO_PIN_MUX_REG)
}
Now, let's do the same, but also move GPIO_PIN_MUX_REG to noflash with an updated mapping.
[mapping:soc_pm]
archive: libsoc.a
entries:
gpio_periph: GPIO_HOLD_MASK (noflash)
gpio_periph: GPIO_PIN_MUX_REG (noflash)
1. Creating a new placement
.dram0.data :
{
*libsoc.a:gpio_periph.*(.rodata.GPIO_HOLD_MASK .sdata2.GPIO_HOLD_MASK .srodata.GPIO_HOLD_MASK)
*libsoc.a:gpio_periph.*(.rodata.GPIO_PIN_MUX_REG .sdata2.GPIO_PIN_MUX_REG
.srodata.GPIO_PIN_MUX_REG)
}
2. Excluding the object placement
.flash.rodata :
{
*(EXCLUDE_FILE(*libsoc.a:gpio_periph.*) .rodata.* ...)
}
3. Creating a new intermediate placement
.flash.rodata :
{
*libsoc.a:gpio_periph.*
}
The *libsoc.a:gpio_periph.* entity in step 3 no longer has input
sections, as there are no remaining .rodata input sections in the object
file. The linker behavior for this mapping is to include all object
input sections that have not yet been placed as described in
https://sourceware.org/binutils/docs/ld.html#Input-Section-Basics
"If you use a file name without a list of sections, then all sections in
the input file will be included in the output section. This is not
commonly done, but it may by useful on occasion."
The map file for such mapping now contains following input sections
.flash.rodata 0x3c0a0120 0x19b34
*libsoc.a:gpio_periph.*()
.debug_info 0x3c0b95bf 0xd8 esp-idf/soc/libsoc.a(gpio_periph.c.obj)
.debug_abbrev 0x3c0b9697 0x70 esp-idf/soc/libsoc.a(gpio_periph.c.obj)
.debug_aranges
0x3c0b9707 0x18 esp-idf/soc/libsoc.a(gpio_periph.c.obj)
.debug_line 0x3c0b971f 0x1ab esp-idf/soc/libsoc.a(gpio_periph.c.obj)
.debug_str 0x3c0b98ca 0x21a esp-idf/soc/libsoc.a(gpio_periph.c.obj)
0x22d (size before relaxing)
.comment 0x3c0b9ae4 0x30 esp-idf/soc/libsoc.a(gpio_periph.c.obj)
.note.GNU-stack
0x3c0b9ae4 0x0 esp-idf/soc/libsoc.a(gpio_periph.c.obj)
.riscv.attributes
0x3c0b9ae4 0x44 esp-idf/soc/libsoc.a(gpio_periph.c.obj)
This is incorrect, and such intermediate placement should not be
generated. This type of placement can be recognized because it is not
explicitly defined in the mapping and lacks input sections. We can
identify this in the significant function and prevent issuing commands
for such placement.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently, we are only suggesting that the header file is likely not
part of the component's INCLUDE_DIRS. However, the header file may be
missing also because of the configuration settings. For instance, the
component might be disabled in sdkconfig, or the feature that supplies
the header might not be enabled. Enhance the hint message to address
this scenario as well.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The ImportError or ModuleNotFoundError might be raised without
specifying a module name. In this not so common situation, re-raise the
exception to print all the information that could assist in identifying
the problem.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
By default, esp-idf-size.ng displays all sections individually. This can
be confusing, especially if CONFIG_SOC_MEM_NON_CONTIGUOUS_SRAM is
enabled, resulting in sections like .dram0.data and .dram1.data being
abbreviated as two .data sections in the size report. To avoid confusion
for idf.py and cmake users, pass the --unify option to the underlying
esp_idf_size.ng by default.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently, the `SORT` flag mandates the inclusion of at least the
`sort_by_first` argument in the grammar, despite the documentation[1]
indicating that `SORT` can be utilized without any arguments, defaulting
to sorting input sections by name. Fix this by modifying the grammar
to allow a default `SORT` and update a test accordingly.
[1] https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-guides/
linker-script-generation.html
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Setting OUTPUT_FORMAT through the CMake variable probably never
functioned and seems to be a remnant (possibly a typo in the
documentation) from the time when the -DOUTPUT_JSON=1 CMake variable was
used to specify the JSON output format, prior to the introduction of the
--format option. This removes the note regarding the option to choose
the output format using -DOUTPUT_FORMAT.
Closes https://github.com/espressif/esp-idf/issues/14383
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently, when the venv is installed or updated, we attempt to
automatically update pip and setuptools within the venv. Unfortunately,
the setuptools package is installed or updated without adhering to the
constraints file, which restricts the setuptools version due to
https://github.com/pypa/setuptools/issues/4480. Resolve this issue by
applying the constraints file to the installation and update of both pip
and setuptools.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This adds a simple test that tries to run idf_size.py help and check
if the process does not exit with error. This is just to make sure
that idf_size.py can be used with minimum required python version.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Previous 6caa4a17ac ("fix: display correct help in the idf_size.py wrapper")
introduced a regression, because it uses exit_on_error parameter for
argparse.ArgumentParser, which was added in python3.9, making
idf_size.py incompatible with idf.py minimal required python3.8.
The objective is to inspect the arguments of idf_size.py using a wrapper
argparse to determine whether the legacy or refactored version should be
initiated, while always displaying help for the underlying version. The
exit_on_error function was previously utilized to prevent argparse from
exiting and displaying help/usage. This replaces exit_on_error with a
workaround that makes the --format argument optional. Since this is the
sole instance where the wrapper argparse might fail, it achieves the
same outcome as using exit_on_error.
Fixes: 6caa4a17ac ("fix: display correct help in the idf_size.py wrapper")
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently the wrapper tries to figure out which version of
the esp-idf-size should be started. The legacy version is
used if explicitly requested by the -l/--legacy option or
if json format is specified. This works fine, but if help
is requested, it is printed for the wrapper as shown bellow
$ idf_size.py -h
usage: idf_size.py [-h] [--format FORMAT] [-l]
options:
-h, --help show this help message and exit
--format FORMAT
-l, --legacy
This is not convenient and the full help from the underlying
version should be displayed.
Fix this by only peeking into the args to figure out if legacy or
refactored version should be started and always spawn the underlying
esp_idf_size python module. This is done by using exit_on_error=False and
add_help=False for the ArgumentParser. When help for refactored version
is requested a note as following is printed to notify users that the
legacy version can still be used.
$ idf_size.py -h
Note: legacy esp_idf_size version can be invoked by specifying the -l/--legacy
option or by setting the ESP_IDF_SIZE_LEGACY environment variable.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently when process is started through asyncio Runner and it is termited
e.g. with SIGINT(ctrl+c) a traceback is printed instead of gracefully
exit.
Exception ignored in: <function BaseSubprocessTransport.__del__ at 0x7fe980970900>
Traceback (most recent call last):
File "/usr/lib64/python3.12/asyncio/base_subprocess.py", line 129, in __del__
self.close()
File "/usr/lib64/python3.12/asyncio/base_subprocess.py", line 107, in close
proto.pipe.close()
File "/usr/lib64/python3.12/asyncio/unix_events.py", line 568, in close
self._close(None)
File "/usr/lib64/python3.12/asyncio/unix_events.py", line 592, in _close
self._loop.call_soon(self._call_connection_lost, exc)
File "/usr/lib64/python3.12/asyncio/base_events.py", line 793, in call_soon
self._check_closed()
File "/usr/lib64/python3.12/asyncio/base_events.py", line 540, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
This is caused because asyncio Runner context in asyncio.run is closing the event
loop and if exception is unhandled in coroutine(run_command) the transport is not
closed before the even loop is closed and we get RuntimeError: Event loop is closed
in the transport __del__ function because it's trying to use the closed
even loop.
Let's catch asyncio.CancelledError in case the process we are trying to
read from is terminated, print message, let the asyncio finish and exit
gracefully.
Closes https://github.com/espressif/esp-idf/issues/13418
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
It seems gcc is not producing debug information for sections which
are not properly marked as "ax", resulting in missing debug info
for _vector_table, _interrupt_handler and _panic_handler. This can be
verified e.g. with
readelf --debug=info ./build/esp-idf/riscv/CMakeFiles/__idf_riscv.dir/vectors.S.obj
readelf -SW ./build/esp-idf/riscv/CMakeFiles/__idf_riscv.dir/vectors.S.obj
for hello_world example on esp32c3 target. Mark the .exception_vectors.text and
.exception_vectors_table.text sections as writable and allocatable so the debug
info sections are generated.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
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>
Currently RunTool reads command's output with asyncio read, which
returns bytes. This is decoded into python's string and the output already
contains OS specific line endings, which on Windows is CRLF. Problem is
that the command output is saved by using python's text stream/file,
which replaces LF, native python's line ending, with OS specific line ending.
On Windows, and in this particular case, the CRLF from the command output is
translated into CRCRLF and saved in the commands output file. When this
file is read in again, e.g. for hint modules, the CRCRLF is replaced
with LFLF. Again the file is open as text file. Meaning a new emply line
is added.
Fix this by opening the output file with "newline=''", which prevents
this translation. We already have the OS specific line ending in the
command's output.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
With 09ba070b7e ("fix: allow to use older pre-commit hook version")
and b93cc581a5 ("fix: set default pre-commit hook stages to pre-commit..")
we set the default pre-commit hook stage to commit/pre-commit, which avoids
running the pre-commit plugins/hooks multiple times for different stages.
Unfortunately this also affects "pre-commit run" used in CI and
now it runs only plugins which are by default using the
commit/pre-commit stage. Meaning the sbom validation, which uses
post-commit, is not run at all. This adds an explicit pre-commit command
for validate-sbom-manifest plugin, which needs to be started in
post-commit stage.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
With b93cc581a5 ("fix: set default pre-commit hook stages to pre-commit..")
a dependency on pre-commit 3.3 was added. As it turned out, there are
people still using e.g. 2.16 version and this change is causing problems.
1. The minimum_pre_commit_version is checked after the config is parsed.
Meaning if there is a problem in the config file, like unknown
pre-commit for default_stages, pre-commit exits with an error not
suggesting that it should be updated.
2. "pre-commit" was added in 3.2 for consistency with git hook names and
we can use "commit" instead avoiding dependency on newer pre-commit.
For more info please see [1] and [2].
3. default_install_hook_types[4] were added in 2.18.0[3]. This was IMHO actually
a nice feature to have in our .pre-commit-config.yaml, because it
specifies which hooks should be installed when pre-commit install is
executed. Meaning it's not necessary to list the stages explicitly with the -t
options. Anyway we haven't used this feature till now, so it
hopefully should not be a problem if we remove it again.
With this change we can still avoid to start pre-commit plugins multiple
times for different stages, but also make it compatible with older
versions(2.16).
[1] https://github.com/pre-commit/pre-commit/issues/2732
[2] https://github.com/pre-commit/pre-commit/pull/2808
[3] https://github.com/pre-commit/pre-commit/blob/main/CHANGELOG.md#2180---2022-04-02
[4] https://pre-commit.com/#top_level-default_install_hook_types
Fixes: b93cc581a5 ("fix: set default pre-commit hook stages to pre-commit..")
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Following commit c3afbebf23 ("fix: bump esp-idf-sbom to v0.13.0 in pre-commit"),
the validation of submodule hash now relies solely on the information recorded
in the git-tree. Previously, the hash verification used submodule's
working tree hash if available. Since the new submodule hash is recorded
in git-tree only after the commit is created, we need to move the check
into post-commit, otherwise the hash validation checks the old value.
For example:
1. in .gitmodules
[submodule "components/json/cJSON"]
sbom-hash = cb8693b058ba302f4829ec6d03f609ac6f848546
2. update the cJSON
$ git -C components/json/cJSON checkout b45f48e600671feade0b6bd65d1c69de7899f2be
3. update cJSON hash in .gitmodules
[submodule "components/json/cJSON"]
sbom-hash = b45f48e600671feade0b6bd65d1c69de7899f2be
4. commit the changes
$ git commit -a -s
Step 4. will fail, because the validation is currently started in pre-commit stage,
where the hash for cJSON recorded in git-tree is still
cb8693b058ba302f4829ec6d03f609ac6f848546. The new hash b45f48e600671feade0b6bd65d1c69de7899f2be
will be stored in git-tree after the new commit is created.
Note that this means we cannot prevent the commit creation, but only
notify user about the hash inconsistency. If he/she still decides to
push it, it will fail in pre-commit checks in CI.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
If pre-commit hook does not specify stage[1], neither in
.pre-commit-config.yaml nor in .pre-commit-hooks.yaml, it's started for every
installed pre-commit hook. Limit the default stages to pre-commit only by default.
This also sets the default_install_hook_types and
minimum_pre_commit_version and sets conventional-precommit-linter to
v1.4.1.
[1] https://pre-commit.com/#confining-hooks-to-run-at-certain-stages
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
v0.13.0 contains fix for the submodule hash validation. Let's
bump its version in pre-commit, so the fix is used.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This adds a github action, which performs continuous vulnerability
scanning using the esp-idf-sbom-action github action. The test
is scheduled everyday at midnight and it's also possible to start
it as dispatched workflow. This scans all possible manifest files
in repository. The references for scanning are defined in github's
VULNERABILITY_SCAN_REFS variable and a json list. For example
['master', 'release/v5.2', 'release/v5.1', 'release/v5.0', 'release/v4.4']
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently sbom manifest is checked only in .gitmodules and
this check is done in pre-commit and also in CI. Meaning it's running
three times(pre-commit before push if user has it enabled, in CI
as there is the pre-commit run again and again with test in CI). Since
esp-idf-sbom contains a full manifest validation support and pre-commit
plugin for it, let's use it. This removes all the current sbom testing
and replaces it with a signle pre-commit plugin which validates all
manifests files(sbom.yml, idf_component.yml, .gitmodules and also
referenced manifests) in repository. Note that this checks all
manifests, not only ones which were modified. The check is reasonably
fast though, so it should not cause any problem. The reason for
validating all manifest files is that we want to make sure that the sbom
information in .gitmodules is updated too and that the hash
recorded in .gitmodules is up-to-date. Meaning submodule update
would not trigger this plugin, because no manifest was changed.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
MR espressif/esp-idf!27436 added sbom manifest file for fatfs,
but the originator is missing a prefix required by SPDX specification.
This results in malformed SPDX sbom files.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
With 8959555cee7e[1] ("setup_git_directory(): add an owner check for the top..")
git added an ownership check of the git directory and refuses to
run any git commands, even parsing the config file, if the git directory
is not owned by the current user. The "fatal: detected dubious ownership in repository"
is reported.
This fixes CVE-2022-24765[2], which allows to compromise user account. On a
multi-user system or e.g. on a shared file system, one user may create a "rogue"
git repository with e.g. core.fsmonitor set to an arbitrary command. Other user
may unwillingly execute this command by running e.g. git-diff or
git-status within the "rogue" git repository, which may be in one of the parent
directories. If e.g. PS1 is set to display information about a git
repository in CWD, as suggested in Git in Bash[3], the user do not need to run
any git command to trigger this, just entering some subdirectory under
this "rogue" git repository is enough, because the git command will be
started transparently through the script used in PS1. The core.fsmonitor
can be set to arbitrary command. It's purpose is to help git to identify changed files
and speed up the scanning for changed files.
rogue
├── .git # owned by user1
└── dir1 # owned by user2
├── dir2 # owned by user2
└── .git # owned by user2
user1 sets core.fsmonitor for git repository in rogue directory
$ git config --add core.fsmonitor "bash -c 'rm -rf \$HOME'"
user2 enters dir1 and runs e.g. git diff and triggers the core.fsmonitor command.
The ownership check may cause problems when running git commands in
ESP-IDF Docker container. For example user may run the container as
root, but the mounted project may be owned by a particular user.
In this case git will refuse to execute any git command within the
"/project" directory, because it's not owned by root. To overcome this,
git allows to set safe.directories, for which the ownership check is
skipped. The security check may be completely disabled by setting
safe.directories to "*". This solution was proposed in PR 12636[4], but
it would allow make it possible to exploit this vulnerability again.
This fix allows user to specify git's safe.directory in IDF_GIT_SAFE_DIR
environmental variable, which may be set during container startup.
The IDF_GIT_SAFE_DIR has same format as PATH and multiple directories can be
specified by using a ":" separator. To entirely disable this git security check
within the container, user may set IDF_GIT_SAFE_DIR='*'. This might be
heplfull in CI.
Closes https://github.com/espressif/esp-idf/pull/12636
[1] - https://github.com/git/git/commit/8959555cee7ec045958f9b6dd62e541affb7e7d9
[2] - https://nvd.nist.gov/vuln/detail/cve-2022-24765
[3] - https://git-scm.com/book/en/v2/Appendix-A%3A-Git-in-Other-Environments-Git-in-Bash
[4] - https://github.com/espressif/esp-idf/pull/12636
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This integrates esp_idf_size.ng, refactored esp-idf-size version, into
esp-idf and enables it by default. The esp_idf_size.ng may be enabled
by using the --ng option, but also via ESP_IDF_SIZE_NG env. variable,
which is used in this integration.
New -l/--legacy option is added, which enforces usage of the old version.
This option can be also set via "ESP_IDF_SIZE_LEGACY" env. variable.
This should allow to easily switch back to old version if there is any
problem.
The new version is used by default for all formats, except for the "json".
Examples:
$ idf.py size # uses refactored version
$ idf.py size --legacy # uses legacy version
$ idf.py size --l # uses legacy version
$ idf.py size --format json # uses legacy version
$ idf.py size --format json2 # uses refactored version
$ export ESP_IDF_SIZE_LEGACY="1" # use legacy version only from now on
ESP_IDF_SIZE_FORCE_TERMINAL, which forces terminal control codes(colors), is also set
when running from idf.py, so the colors are propagated even if stdout
for esp_idf_size.ng is not attached to tty.
The same changes are applied also to the idf_size.py wrapper.
There is an import check if esp_idf_size.ng is available. If not,
we switch into the legacy mode. This should cover situation when the
esp-idf has support for refactored version, but it's not installed.
This should also allow users to bind to a legacy version(<1.0.0) and the
idf.py size and idf_size.py should still work. This also allow us to
restring the version in constraints file if we need to switch back to
legacy version globally.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
pyyaml have problem with newer cython(3.0), this was fixed in pyyaml
6.0.1, which requires cython<3.0. The pyyaml dependency in astyle
was fixed in v1.0.5, but the pre-commit config is still useing v1.0.2.
As a result the pre-commit hooks installation of astyle can fail
on pyyaml. Fix this by bumping the astyle version for pre-commit.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently the component_requirements hint module does not work
as expected if the component list for a project is trimmed down.
With the new "all_component_info" dictionary info in project_description.json,
the module can produce hints even if cmake's COMPONENTS variable is
set.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Add new "all_component_info" dictionary into the
project_description.json file. It contains information about all
registered components presented in the __COMPONENT_TARGETS list.
Since components in this list are not fully evaluated, because only the first
stage of cmakefiles processing is done, it does not contain the same information
as the "build_component_info" dictionary. The "type", "file" and "sources" variables
are missing.
Most of the properties are already attached to the component target, so
this only adds INCLUDE_DIRS property to the target during the first cmakefiles
processing stage.
The "all_component_info" dict is generated in a separate function, even
though the original function for "build_component_info" could be
adjusted. This introduces a little bit of boilerplate, but keeps it
logically separated and probably easier if we want to extend it in the
future.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
There are orphaned .phyiram sections from components/esp_phy/lib/esp32/libphy.a when ESP_WIFI_SLP_IRAM_OPT
is not set on esp32. It can be seen in the wifi_station example examples/wifi/getting_started/station with
the default configuration. Even though they seem to be correctly placed in flash(linker got it right), I
believe this should be fixed. At least for the esp-idf-size, because these doesn't seem to be accounted.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
If there is a component(child) within a component(parent), like for test_apps, the parent
component may be wrongly identified as source component for the failed include. This may
lead to a false bug report if the parent component has component, which provides the missing
header, in requirements.
Fix this by looking for the longest matching source component directory.
Suggested-by: Ivan Grokhotkov <ivan@espressif.com>
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently idf.py reports just "Please use idf.py only in an ESP-IDF shell environment".
Sometimes it may be useful to know for which module the import failed.
Also the problem does not have to be related to shell environment only, but the
python venv can be corrupted. This adds a little bit more verbose error
message.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
1. The original test, before hint modules support was added, used
tempfile.NamedTemporaryFile in a way which is not supported on windows.
It was having the file open, which the hints tried to read it, leading
the EPERM exception. The docs[1] says this is not supported.
<quote>
Whether the name can be used to open the file a second time, while the
named temporary file is still open, varies across platforms (it can be
so used on Unix; it cannot on Windows)
</quote>
2. The hint module component_requirements test used the idf.py directly,
which is idf.py.exe on windows.Now it's starting idf.py through python.
We could probably used shell=True, but this approach is used in other
tests too.
Anyway the test are now passing on windows.
[1] https://docs.python.org/3/library/tempfile.html
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Following tests were added.
1. Test for missing header directory in component's INCLUDE_DIRS
2. Test for missing dependency in component's PRIV_REQUIRES
3. Test for missing dependency in component's REQUIRES
4. Test for dependency in PRIV_REQUIRES which should be in REQUIRES
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Introduces get_build_context() helper, which allows to get build context, e.g.
project description, at places where this info is not available. The
build context is set within ensure_build_directory.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The ticket below reports some very strange behaviour, where
even a simple command like the following
python3 -c "import subprocess; subprocess.run([''])"
actually spawns some process and pass instead of raising
PermissionError. Even though the problem is most probably somewhere
else, not in idf_tools, we may just return early if there is no
command available for get_version().
Closes https://github.com/espressif/esp-idf/issues/11880
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently ESP_PYTHON is used in the install.sh script, but it's
overwritten in detect_python.sh. This allows to explicitly specify
python binary, which should be used to create venv.
$ ESP_PYTHON=/usr/bin/python3.7 ./install.{sh,fish}
$ ESP_PYTHON=/usr/bin/python3.7 ./export.{sh,fish}
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
idf.py spawns gdb process within a thread and uses Thread.join() to wait
for the gdb process to finish. As CTRL+C(SIGINT) is used by gdb to interrupt the
running program, we catch the SIGINT while waiting on the gdb to finish,
and try Thread.join() again.
With cpython's commit
commit a22be4943c119fecf5433d999227ff78fc2e5741
Author: Victor Stinner <vstinner@python.org>
Date: Mon Sep 27 14:20:31 2021 +0200
bpo-45274: Fix Thread._wait_for_tstate_lock() race condition (GH-28532)
this logic doesn't work anymore, because cpython internally marks the
thread as stopped when join() is interrupted with an exception. IMHO
this is broken in cpython and there is a bug report about this
https://github.com/python/cpython/issues/90882. Problem is that
waiting on a thread to finish is based on acquiring a lock. Meaning
join() is waiting on _tstate_lock. If this wait is interrupted, the
above referenced commit adds a logic that checks if the lock is help,
meaning the thread is done and marks the thread as stopped. But there is
no way to tell if the lock was acquired by us running join() or if it's
held by someone else e.g. still by the thread bootstrap code. Meaning
the thread is still running.
I may be missing something, but I don't see any reason why to spawn gdb
process within a thread. This change removes the thread and spawns gdb
directly. Instead waiting on a thread, we wait on the process to finish,
replacing join() with wait() and avoiding this problem.
Closes https://github.com/espressif/esp-idf/issues/11871
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Current version of the test is using "git-submodule foreach", which
requires submodules to be initialized. Non-initialized submodules are
ignored. Our CI is not performing submodule initialization, but instead
it only downloads the submodule content in tools/ci/ci_fetch_submodule.py
from cache and copies it into the submodule path.
Since we already know the submodule path from .gitconfig, we can use it
as argument to git-ls-tree and avoid calling git-submodule at all. This
allows to perform the test even if the submodules are not initialization
and also it makes the code simpler.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This uses the test from CI introduced in MR !23989. Even though the
original CI test is run with pytest, it doesn't use any pytest specific
code/features and it should never need them. So it make sense just to
re-use the code.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Newer check-copyright version 1.0.2 contains fixes for copyrights
with single year and also the proposed date update is applied only
if number of changed lines is greater or equal to a limit. The default
limit is 5 and can be changed with the --lines-changed option if needed.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
As pointed out by Fu Hanxi, the pytest_build_system job is currently
using --parallel-index and --parallel-count, which are provided by
pytest-embedded, so we should not disable it. Moreover to properly
disable pytest-embedded we should use "no:pytest_embedded". Meaning
this probably was not working as indented anyway.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This adds SBOM information for submodules, which are not managed
by Espressif. Meaning there is no fork for them in the espressif
namespace. Other submodules should add sbom.yml manifest file to
the root of their git repository.
The SBOM information for submodules is stored in the .gitmodules file.
Each SBOM related variable has the "sbom-" prefix and the following
variables may be used:
sbom-version:
submodule version
sbom-cpe:
CPE record if available in NVD. This will be used by the SBOM
tool to check for possible submodule vulnerabilities. The
version in the CPE can be replaced with the "{}" placeholder,
which will be replaced by the "sbom-version" value from above.
sbom-supplier:
Person or organization who is providing the submodule.
It has to start with "Person:" or "Organization:" prefix
as required by the SPDX-2.2 standard.
sbom-url:
URL to the project if exists, e.g. github.
sbom-description:
Project description.
sbom-hash:
Submodule SHA as recorded in the git-tree. This field is used by
CI to check that the submodule checkout hash and info in .gitmodules
are in sync. IOW if submodule is updated and it has SBOM info in
.gitmodules, the .gitmodules has to be updated too. The test is
part of this commit. The checkout has of the submodule can be found
by using "git submodule status".
Example for micro-ecc submodule
---8<---
[submodule "components/bootloader/subproject/components/micro-ecc/micro-ecc"]
path = components/bootloader/subproject/components/micro-ecc/micro-ecc
url = ../../kmackay/micro-ecc.git
sbom-version = 1.0
sbom-cpe = cpe:2.3:a:micro-ecc_project:micro-ecc:{}:*:*:*:*:*:*:*
sbom-supplier = Person: Ken MacKay
sbom-url = https://github.com/kmackay/micro-ecc
sbom-description = A small and fast ECDH and ECDSA implementation for 8-bit, 32-bit, and 64-bit processors
sbom-hash = d037ec89546fad14b5c4d5456c2e23a71e554966
---8<---
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This extends information provided in the project_description.json file.
Newly added information can be used in the SBOM generating tool and
also to improve hints regarding the the component dependency issues.
Added fields
version:
This adds versioning to the project_description.json file,
so it's easy to identify if it contains the required information.
project_version:
Can be used as a version for the resulting binary e.g. `hello_world.bin`.
idf_path:
This one is probably not necessary, but it allows tools to run even without
esp-idf environment exported(e.g. export.sh).
c_compiler:
The `CMAKE_C_COMPILER` value with full path to the compiler binary. This can
be used to get information about toolchain, which was used to build the project.
common_component_reqs:
List of common components as presented in cmake's __COMPONENT_REQUIRES_COMMON
and set in tools/cmake/build.cmake:__build_init().
build_component_info:
Detailed information about components used during build. It's a
dictionary with the component name as a key and each component has
a dictionary with detailed information. Following is an example for
the efuse component.
"efuse": {
"alias": "idf::efuse",
"target": "___idf_efuse",
"prefix": "idf",
"dir": "/home/fhrbata/work/esp-idf/components/efuse",
"type": "LIBRARY",
"lib": "__idf_efuse",
"reqs": [],
"priv_reqs": [ "bootloader_support", "soc", "spi_flash" ],
"managed_reqs": [],
"managed_priv_reqs": [],
"file": "/home/fhrbata/work/blink/build/esp-idf/efuse/libefuse.a",
"sources": [ "/home/fhrbata/work/esp-idf/components/efuse/esp32s3/esp_efuse_table.c", ... ],
"include_dirs": [ "include", "esp32s3/include" ]
}
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently make_json_list() returns '[ "" ]' for empty cmake list. Fix this
so empty json list is returned instead.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently hints are processed only once the process is finished and
exits with non-error exit code. In interactive mode, e.g. for monitor,
we want to process ouput lines for hints right away.
This adds a simple buffer, which keeps the last line and once EOL is
reached, it is processed for hints.
Since the original hints processing was file based, a new helper
function was added to allow processing hints in string.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
On Windows, when path is specified as absolute for cmdl argument, cmake can
interpret parts of the path as invalid escape chars. For example "C:\Users\..."
will result in "Invalid character escape '\U'." Externally specified
paths should be converted into cmake's representation, which uses '/'.
This can be done e.g. by using 'get_filename_component()'. Currently
there doesn't seem to be any problem with this, but let's add a test for
this.
Suggested-by: Ivan Grokhotkov <ivan@espressif.com>
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently the forced progress in RunTool is trying to fit the output
line into a terminal width, but it doesn't take into an account a
situation when the terminal width is reported as zero. This manifests
when running in docker image with redirected output and can be seen
in the github workflow output for esp-idf-ci-action.
docker run --rm -t my_ubuntu_esp python3 -c 'import os,sys; print(os.get_terminal_size(), sys.stdout.isatty())'
os.terminal_size(columns=238, lines=59) True
vs
docker run --rm -t my_ubuntu_esp python3 -c 'import os,sys; print(os.get_terminal_size(), sys.stdout.isatty())' | tee
os.terminal_size(columns=0, lines=0) True
Since the output is reported as tty and the terminal width as 0, the
fit_text_in_terminal() function returns empty string. I also verified this
by running idf.py build inside a testing docker image.
This fix adjusts the fit_text_in_terminal() function to return original
line if the terminal width is zero.
Also simplify the progress print and use same approach as ninja
https://github.com/ninja-build/ninja/blob/master/src/line_printer.cc#L66
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently if the IDF_TARGET env is set, and old sdkconfig exists with
different target value in CONFIG_IDF_TARGET, the set-target action fails
complaining about the IDF_TARGET env and value in sdkconfig being different.
We should ignore IDF_TARGET value from sdkconfig, because we are
actually setting new target and the old sdkconfig is renamed in cmake.
This can be easily reproduced with
---8<---
$ IDF_TARGET=esp32 idf.py set-target esp32
$ IDF_TARGET=esp32s3 idf.py set-target esp32s3
Project sdkconfig '/home/fhrbata/work/hello_world/sdkconfig' was generated
for target 'esp32s3', but environment variable IDF_TARGET is set to 'esp32'.
Run 'idf.py set-target esp32' to generate new sdkconfig file for target esp32.
---8<---
This also adds test for this use case to test_non_default_target.py.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Two new tests are added.
1) test_check_python_dependencies
The test prepares artificial constraints file containing packages from
requirements.core.txt, which are also reported in pip-freeze output
for virtual env. The constraints file requires package versions higher
than currently installed in venv, so check_python_dependencies
should fail for all of them.
2) test_check_required_packages_only
Test for espressif/esp-idf/-/merge_requests/17917. After installing
core requirements, install additional foopackage, which is embedded.
Add version requirement for foopackage, which cannot be satisfied,
to constraints file. Since foopackage is not a direct requirement,
check-python-dependencies should not fail.
This also fixes existing TestCustomPythonPathInstall test, which sets
IDF_PYTHON_ENV_PATH, but does not restore it. Unittest seems to be
running tests in order based on class/test name. Meaning this test runs
before TestPythonInstall and all tests in TestPythonInstall are using
the latest tmpdir from TestCustomPythonPathInstall as IDF_PYTHON_ENV_PATH.
IOW TestPythonInstall is actually testing custom python env path, same as
TestCustomPythonPathInstall. This fixes it by restoring the
IDF_PYTHON_ENV_PATH. Note that since the actual IDF_PYTHON_ENV_PATH
was different(tmpdir) from PYTHON_DIR, the default PYTHON_DIR was never used,
so the tests were running with old python env(no
shutil.rmtree(PYTHON_DIR).
Since TestCustomPythonPathInstall is inheriting from TestPythonInstall
it also runs test_opt_argument and test_no_constraints for the second
time. This seems unnecessary, so this patch also skips these two tests
in TestCustomPythonPathInstall.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently the set-target has sdkconfig file name hardcoded to the
default one and doesn't honor custom config paths or names.
IMHO the only place where we can really now the config file name
is in cmake. But also the config should be really renamed only if
the set-target action is running.
This moves the config file renaming into cmake and it's performed only
when _IDF_PY_SET_TARGET_ACTION env. var. is set to 'ON'. This should
hopefully guarantee that it's really renamed only while set-target is
running.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Extend target checks in cmake, in case it's run directly and not via
idf.py or if idf.py misses something. This may happen
for example if cmake variables are set in project's CMakeLists.txt.
Some clean-ups are included along with the new checks and tests.
1. __target_check() function is removed. IIUC it should never fail,
because the selected target is explicitly passed as environmental
variable to kconfgen. Meaning the IDF_TARGET from environment variable may
not be actually used in kconfgen if IDF_TARGET is already set it cmake cache.
Note that the IDF_TARGET environment variable used for kconfgen is not
based on the actual IDF_TARGET environment variable set for idf.py, but
rather on the value set in __target_init() with
set(IDF_TARGET ${env_idf_target} CACHE STRING "IDF Build Target")
My understanding is that the original check was introduced to handle
situation, where IDF_TARGET was already set in cmake's cache and
the IDF_TARGET from environment variable was different. Since
the kconfgen would use the original environment variable(not
explicitly passed as it is now) the IDF_TARGET in cmake and in
sdkconfig could differ. IOW I think the original check was introduced
to cope with the following cmake behaviour
set(VARIABLE "value1" CACHE STRING "test variable")
set(VARIABLE "value2" CACHE STRING "test variable")
message("Variable value: ${VARIABLE}")
output: Variable value: value1
2. I scratched by head how it is possible that the following code
in __target_check()
if(NOT ${IDF_TARGET} STREQUAL ${env_idf_target})
could fail if IDF_TARGET is not set. For example in clean project
IDF_TARGET=esp32 idf.py reconfigure
Here env_idf_target==esp32 and IDF_TARGET is not set, so I would
expect that cmake will fail with error message that the cache
and env target do not match. The thing is that the variable
evaluation is done before the if command, so it actually
sees this
if(NOT STREQUAL esp32)
which is false and the error is not printed. It can be seen
with 'cmake --trace-expand' command. I don't know if this
was used on purpose or it worked just as a coincidence, but
I find it very confusing, so I added explicit check if the
IDF_TARGET is defined before the actual check. Same for
CMAKE_TOOLCHAIN_FILE.
3. Error messages are not formated(line-wrapped) by cmake's markup
so it's easier to check the output in tests.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Extend existing target consistency checks for the two following cases.
1. Target does not match currently used toolchain
$ IDF_TARGET=esp32s2 idf.py reconfigure
$ idf.py -DIDF_TARGET=esp32c3 build
2. Target is ambiguous, because it's specified also as env. var.
IDF_TARGET=esp32s3 idf.py set-target esp32c2
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The _guess_or_check_idf_target() function has sdkconfig and sdkconfig.defaults
file names hardcoded. Since config file names may be specified with SDKCONFIG
or SDKCONFIG_DEFAULTS cmake vars, directly in CMakeLists.txt or passed in with
the -D cmake option, they are not respected.
Problem is when SDKCONFIG or SDKCONFIG_DEFAULTS is set in
CMakeLists.txt. While idf can detect cmake vars passed through it
to cmake via the -D option, detecting SDKCONFIG and SDKCONFIG_DEFAULTS
vars settings in CMakeLists.txt would require to parse it. This seems
like error prone approach. Also if the vars defined by the -D option
are passed directly to cmake, not via idf, they will not be visible to idf.
It seems reasonable to move the logic into cmake, where we know the correct
SDKCONFIG and SDKCONFIG_DEFAULTS values. So the IDF_TARGET detection/guessing
is moved into targets.cmake, where the IDF_TARGET is actually set. The target
is guessed based on the following precendence.
1) $ENV{IDF_TARGET}
2) IDF_TARGET
3) SDKCONFIG
4) sdkconfig
5) SDKCONFIG_DEFAULTS if non-empty or
$ENV{SDKCONFIG_DEFAULTS} if non-empty or
sdkconfig.defaults
6) esp32
All config files referred in $ENV{SDKCONFIG_DEFAULTS} and SDKCONFIG_DEFAULTS
are searched, compared to the current behaviour. First target found in the
above chain is used.
The original _guess_or_check_idf_target() is renamed to _check_idf_target() and
used for the target consistency checks only.
The get_sdkconfig_filename() helper is now used to get the sdkconfig file
for consistency checks. It looks in SDKCONFIG specified with the -D
option and project_description.json.
With this change config full paths are reported in messages, so it's clear
e.g. from which config the target was guessed from or which config has
consistency problem. test_non_default_target.py was adjusted to this
change and also new test for testing the IDF_TARGET guessing was added.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Get project's current sdkconfig file name. It looks in SDKCONFIG cmake var
defined by the -D option and project_description.json. If not found return
default sdkconfig.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This parses cmakes cache vars defined on command line with -D options
into dictionary. It allows to simplify the check for new cache entries
and also can be re-used for other checks.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
On Windows/Mac the serial port cannot be access directly inside
docker container. This is already mentioned in the documentation.
This expands the documentation for steps which can be used to overcome
this limitation by using remote serial port access via telnet protocol.
Closes https://github.com/espressif/esp-idf/issues/10617
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This adds a new outdated option, which only lists outdated
packages installed in IDF_TOOLS_PATH. It searches for the
latest installed tool version in the IDF_TOOLS_PATH/tools path and
compares it against the latest available version in the tools.json
file. If the latest version of a tool installed in IDF_TOOLS_PATH/tools
is smaller, it's reported as outdated. Nothing is reported if the tool
is up to date.
Two new tests are added. First just checks if nothing is reported in
case there is no update available. The second artificially generates
new tools.json file called tools.outdated.json and sets XTENSA_ESP32_ELF
version to 'zzzzzz'. It then checks if the XTENSA_ESP32_ELF tool
is reported as outdated by the 'zzzzzz' version.
Description of the new outdated option is addedd to docs as well.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Hints should be now working for gdbui and openocd. They are not
produced via RunTool(), but the hints are used directly.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
This moves one hint, which was hardcoded in debug_ext.py to
hints.yml and adds a new one when openocd process does not have
permissions to the USB JTAG/serial device.
Also hint replacing the original 'Please check JTAG connection!'
hardcoded message is added.
Suggested-by: Alexey Lapshin <alexey.lapshin@espressif.com>
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
In a very rare cases there is a need to use custom license, which is not
present on the SPDX list. Now, files under such license are added to the
check_copyright ignore list. For example zigbee examples introduced
by !16205. SPDX has a LicenseRef-[idstring] identifier[1] for such cases,
so let's try to use it in license representation[2]. The idea is that licenses
not on the SPDX list can be added into the LICENSES[3] directory and used as
SPDX-License-Identifier: LicenseRef-Special-License
Or if the custom license is present directly in a source file we can use
a special LicenseRef-Included identifier to state that the license
is included.
SPDX-License-Identifier: LicenseRef-Included
Please note that LicenseRef-Included is just a made up identifier to
state the fact that the license is included directly in the source file.
There is nothing in the SPDX spec about this usage.
This relatively small adjustment allows to refer to custom licenses
without a need to skip check_copyright for them.
[1] https://spdx.github.io/spdx-spec/v2.3/other-licensing-information-detected/#101-license-identifier-field
[2] https://spdx.github.io/spdx-spec/v2.3/using-SPDX-short-identifiers-in-source-files/#e4-representing-multiple-licenses
[3] https://reuse.software/spec/
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The debug targets are currently not utilizing hints, because they
are not using RunTool() helper from tools.py to spawn sub-processes.
Adjusting debug targets to use RunTool() would require some significant
changes to debug targets and RunTool() as well. Since debug targets
are already storing their output in logs, we can use these and process
them for hints.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Examples for zigbee are using custom license, which is not present
on the SPDX license list. This was discussed on !16205 and a decision
was made that the examples will be put on check-copyright's ignore list.
SPDX has LicenseRef-[idstring] identifier for such cases, so let's try
to use it. In this particular case the LicenseRef-Included is used to
express that the full license text is included in the source file.
Note that the LicenseRef-Included is not part of SPDX. It's just
something I used. No change is needed on the check-copyright side.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Currently loading of esp32s3 ROM ELF symbols fails with
"Cannot access memory at address 0x3ff194ad". Let's perform
add-symbol-file after connection to the target.
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
The main idf.py process has a handler for SIGINT, which actually just
ignores it. The get_default_serial_port() function is called within the
idf.py context to detect port for several tools(monitor,flash) and it's
not possible to terminate it because of this. Let's ignore SIGINT only
while running idf_monitor, which uses it to spawn gdb.
Fixes: c6e3eb0922 ("idf.py.exe changes to handle Ctrl+C in correct way. H..")
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
When multiple targets are specified, e.g. idf.py flash monitor, the
automatic port detection is performed twice. Keep the port value in
args.port and avoid multiple calls to get_default_serial_port().
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
Frantisek Hrbata