Skip to content

Conversation

@mathieuchopstm
Copy link
Contributor

This PR modifies the DTS CMake module to capture stderr output of the gen_defines.py script and print it as part of the CMake FATAL_ERROR message in case of failure. With this, the usual "devicetree error" message is no longer buried in the build log, which is a huge quality-of-life improvement when debugging DT errors.

Example output log before PR: (note how line 2 is the one containing useful information)

   Found BOARD.dts: ~/zephyrproject/zephyr/boards/st/nucleo_f401re/nucleo_f401re.dts
devicetree error: ~/zephyrproject/zephyr/boards/st/nucleo_f401re/nucleo_f401re.dts:27 (column 2): parse error: expected ';', not 'leds'
   In: ~/zephyrproject/zephyr/build/zephyr, command: ~/zephyrproject/.venv/bin/python3;~/zephyrproject/zephyr/scripts/dts/gen_defines.py;--dts;~/zephyrproject/zephyr/build/zephyr/zephyr.dts.pre;--dtc-flags;'';--bindings-dirs;~/zephyrproject/zephyr/dts/bindings;--header-out;~/zephyrproject/zephyr/build/zephyr/include/generated/zephyr/devicetree_generated.h.new;--dts-out;~/zephyrproject/zephyr/build/zephyr/zephyr.dts.new;--edt-pickle-out;~/zephyrproject/zephyr/build/zephyr/edt.pickle;--vendor-prefixes;~/zephyrproject/zephyr/dts/bindings/vendor-prefixes.txt
-CMake Error at ~/zephyrproject/zephyr/cmake/modules/dts.cmake:301 (message):
-   gen_defines.py failed with return code: 1
- Call Stack (most recent call first):
-   ~/zephyrproject/zephyr/cmake/modules/zephyr_default.cmake:132 (include)
-   ~/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
-   ~/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:97 (include_boilerplate)
-   CMakeLists.txt:5 (find_package)


   Configuring incomplete, errors occurred!

(N.B.: the log has been slightly modified to be able to highlight the error message in red using the diff syntax, as CMake would be doing in a real terminal. Note also that GitHub does not wrap lines unlike (some) terminal emulators, which makes this even more unreadable)

Example log after PR:

   Found BOARD.dts: ~/zephyrproject/zephyr/boards/st/nucleo_f401re/nucleo_f401re.dts
   In: ~/zephyrproject/zephyr/build/zephyr, command: ~/zephyrproject/.venv/bin/python3;~/zephyrproject/zephyr/scripts/dts/gen_defines.py;--dts;~/zephyrproject/zephyr/build/zephyr/zephyr.dts.pre;--dtc-flags;'';--bindings-dirs;~/zephyrproject/zephyr/dts/bindings;--header-out;~/zephyrproject/zephyr/build/zephyr/include/generated/zephyr/devicetree_generated.h.new;--dts-out;~/zephyrproject/zephyr/build/zephyr/zephyr.dts.new;--edt-pickle-out;~/zephyrproject/zephyr/build/zephyr/edt.pickle;--vendor-prefixes;~/zephyrproject/zephyr/dts/bindings/vendor-prefixes.txt
-CMake Error at ~/zephyrproject/zephyr/cmake/modules/dts.cmake:299 (message):
-  gen_defines.py failed due to devicetree error:
-  ~/zephyrproject/zephyr/boards/st/nucleo_f401re/nucleo_f401re.dts:27
-  (column 2): parse error: expected ';', not 'leds'
-
- Call Stack (most recent call first):
-  ~/zephyrproject/zephyr/cmake/modules/zephyr_default.cmake:132 (include)
-  ~/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
-  ~/zephyrproject/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:97 (include_boilerplate)
-  CMakeLists.txt:5 (find_package)

   Configuring incomplete, errors occurred!

The message gen_defines.py failed due to ${stderr} is chosen because gen_defines may only print one of these messages to stderr (see sys.exit calls):

  • devicetree error: {e}
  • ERROR: Duplicate 'zephyr,memory-region' ({region}) properties between {regions[region].path} and {node.path}

pdgendt
pdgendt previously approved these changes Jul 30, 2024
Comment on lines 298 to 309
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just print all knowledge we have whenever there is a failure.

Suggested change
if (stderr)
message(FATAL_ERROR "gen_defines.py failed due to ${stderr}")
else()
message(FATAL_ERROR "gen_defines.py failed with return code: ${ret}")
endif()
message(FATAL_ERROR "gen_defines.py failed with return code: ${ret}\nstderr: ${stderr}")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about the following?
It avoids printing the stderr:\n when there's nothing in stderr (which, arguably, should never happen...)

Suggested change
if (stderr)
message(FATAL_ERROR "gen_defines.py failed due to ${stderr}")
else()
message(FATAL_ERROR "gen_defines.py failed with return code: ${ret}")
endif()
if (stderr)
set(stderr "\nScript stderr: ${stderr}")
endif()
message(FATAL_ERROR "gen_defines.py failed with return code: ${ret}${stderr}")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It avoids printing the stderr:\n when there's nothing in stderr

I actually think that a stderr: with no content actually indicates to users that there actually was no error message, which can also be useful information.

If not, then users would actually need to know that stderr is only missing if it is empty, something they can only know if they look in the CMake file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. However, I think an empty stderr: could be confusing.
Would displaying stderr: <empty> instead be fine by you?

Copy link
Contributor

@tejlmand tejlmand Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would displaying stderr: <empty> instead be fine by you?

yes, that's a clear indication to the user that an error ocured, but that there were no messages printed to stderr.

pdgendt
pdgendt previously approved these changes Aug 1, 2024
@mathieuchopstm
Copy link
Contributor Author

Changelog:

  • always print the exit code, but also stderr when available on gen_defines.py failure, to be more in line with what @tejlmand requested
  • also capture dtc warnings and transform them into CMake message(WARNING)s

N.B.: I assumed that gen_defines.py would always have clean stderr when it returns 0 - AFAIK, it should never raise warnings.
Conversely, I assumed that dtc would never fail due to DTS errors, since those should be caught by gen_defines.py earlier in the module's code.
This removes some code that would probably never be used (printing stderr of DTC on failure, and printing gen_defines.py stderr on success).

@mathieuchopstm
Copy link
Contributor Author

Oops, I forgot to check-in the modified code before pushing... 🤦‍♂️

pdgendt
pdgendt previously approved these changes Aug 1, 2024
This commit modifies the DTS cmake module to capture `stderr`
output of the `gen_defines.py` script and `dtc` program during
their execution. The messages can then be printed as CMake
`message`s, which improves QoL when debugging device tree
errors, and reduces the risk of introducing malformed DTS,
as the warning/error messages are made much more visible.

Signed-off-by: Mathieu Choplain <mathieu.choplain@st.com>
@mathieuchopstm
Copy link
Contributor Author

Changelog:

  • modified how gen_defines.py failures are handled to comply with request from @tejlmand
  • change formulation of DTC warnings message

The error logs now look like what follows (truncated):

  • gen_defines.py failure with something in stderr
CMake Error at ~/zephyrproject/zephyr/cmake/modules/dts.cmake:305 (message):
 gen_defines.py failed with result code: 1 - stderr contents:

 devicetree error:
 ~/zephyrproject/zephyr/boards/st/nucleo_f401re/nucleo_f401re.dts:27
 (column 2): parse error: expected ';', not 'leds'

Call Stack (most recent call first):
  • gen_defines.py failure with nothing printed on stderr (simulated by adding sys.exit(1) to script)
CMake Error at ~/zephyrproject/zephyr/cmake/modules/dts.cmake:305 (message):
 gen_defines.py failed with result code: 1 - stderr contents: <empty>
Call Stack (most recent call first):
  • dtc warnings
CMake Warning at ~/zephyrproject/zephyr/cmake/modules/dts.cmake:397 (message):
 dtc raised one or more warnings:

 ~/zephyrproject/build/zephyr/zephyr.dts:647.10-654.5:
 Warning (simple_bus_reg): /soc/clocks: missing or empty reg/ranges property

 ~/zephyrproject/build/zephyr/zephyr.dts:909.30-938.5:
 Warning (spi_bus_bridge): /soc/octospi@a0001400: node name for SPI buses
 should be 'spi'

 <stdout>: Warning (spi_bus_reg): Failed prerequisite 'spi_bus_bridge'

Call Stack (most recent call first):

rettichschnidi added a commit to husqvarnagroup/zephyrproject-rtos-zephyr that referenced this pull request Sep 12, 2024
Ever since 059aae7 (cmake: modules:
dts: make Device Tree error messages more visible, PR zephyrproject-rtos#76472), warnings
generated by gen_defines.py got only printed when the exit code signaled
an error.

Without this patch, the warning gets swallowed and the build continues:

```
$ west build --pristine --board nrf54l15pdk/nrf54l15/cpuapp \
  samples/userspace/hello_world_user
<snip>
-- Found BOARD.dts:
<snip>/boards/nordic/nrf54l15pdk/nrf54l15pdk_nrf54l15_cpuapp.dts
-- Generated zephyr.dts: <snip>/build/zephyr/zephyr.dts
<snip>
```

With this patch, the behavior is back to how it was before
059aae7:
```
$ west build --pristine --board nrf54l15pdk/nrf54l15/cpuapp \
  samples/userspace/hello_world_user
<snip>
-- Found BOARD.dts: <snip>/nrf54l15pdk/nrf54l15pdk_nrf54l15_cpuapp.dts
unit address and first address in 'reg' (0x5004c000) don't match for
/soc/peripheral@50000000/vpr@4c000/mailbox@1
-- Generated zephyr.dts: <snip>zephyr/build/zephyr/zephyr.dts
<snip>
```

Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>
carlescufi pushed a commit that referenced this pull request Sep 13, 2024
Ever since 059aae7 (cmake: modules:
dts: make Device Tree error messages more visible, PR #76472), warnings
generated by gen_defines.py got only printed when the exit code signaled
an error.

Without this patch, the warning gets swallowed and the build continues:

```
$ west build --pristine --board nrf54l15pdk/nrf54l15/cpuapp \
  samples/userspace/hello_world_user
<snip>
-- Found BOARD.dts:
<snip>/boards/nordic/nrf54l15pdk/nrf54l15pdk_nrf54l15_cpuapp.dts
-- Generated zephyr.dts: <snip>/build/zephyr/zephyr.dts
<snip>
```

With this patch, the behavior is back to how it was before
059aae7:
```
$ west build --pristine --board nrf54l15pdk/nrf54l15/cpuapp \
  samples/userspace/hello_world_user
<snip>
-- Found BOARD.dts: <snip>/nrf54l15pdk/nrf54l15pdk_nrf54l15_cpuapp.dts
unit address and first address in 'reg' (0x5004c000) don't match for
/soc/peripheral@50000000/vpr@4c000/mailbox@1
-- Generated zephyr.dts: <snip>zephyr/build/zephyr/zephyr.dts
<snip>
```

Signed-off-by: Reto Schneider <reto.schneider@husqvarnagroup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants