Skip to content

Conversation

@rettichschnidi
Copy link
Contributor

@rettichschnidi rettichschnidi commented Sep 10, 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.

Warning: this half-way undoes #76472

Copy link
Contributor

@pdgendt pdgendt left a comment

Choose a reason for hiding this comment

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

IMO it should fail the same way as if(NOT "${ret}" STREQUAL "0")

@rettichschnidi
Copy link
Contributor Author

rettichschnidi commented Sep 11, 2024

IMO it should fail the same way as if(NOT "${ret}" STREQUAL "0")

Just the printed message or should it also be message(FATAL_ERROR ...) and therefore breaking the build the same was as if(NOT "${ret}" STREQUAL "0")? I'd love to see the later, but that might have quite some effect on many builds.

@pdgendt
Copy link
Contributor

pdgendt commented Sep 11, 2024

IMO it should fail the same way as if(NOT "${ret}" STREQUAL "0")

Just the printed message or should it also be message(FATAL_ERROR ...) and therefore breaking the build the same was as if(NOT "${ret}" STREQUAL "0")?

I think the check above should be

if(NOT "${ret}" STREQUAL "0" OR stderr)

@rettichschnidi rettichschnidi force-pushed the gardena/rs/upstream/restore-gen_defines.py-warnings branch from 7c1a7b3 to 49c76b7 Compare September 11, 2024 07:51
@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Sep 11, 2024
@rettichschnidi
Copy link
Contributor Author

rettichschnidi commented Sep 11, 2024

Adapted.

Warning: If merged as is, this will cause some fallout, e.g. #78251. I'd be willing to help cleaning up first however.

@rettichschnidi rettichschnidi force-pushed the gardena/rs/upstream/restore-gen_defines.py-warnings branch from 49c76b7 to bef6e82 Compare September 11, 2024 07:54
@rettichschnidi
Copy link
Contributor Author

Did some smoke testing:

twister -T samples/userspace/hello_world_user --all

Result:

INFO    - 460 of 829 test configurations passed (92.93%), 0 failed, 35 errored, 334 skipped with 0 warnings in 1537.25 seconds
$ rg -A4 "gen_defines.py failed with result code:" --glob build.log
legend@35/samples/userspace/hello_world_user/sample.helloworld/build.log
18:  gen_defines.py failed with result code: 0 - stderr contents:
19-
20-  unit address and first address in 'reg' (0x1000) don't match for
21-  /soc/spi@40003800/spi_nor@0/partitions/partition@10000
22-

legend@25ssd/samples/userspace/hello_world_user/sample.helloworld/build.log
18:  gen_defines.py failed with result code: 0 - stderr contents:
19-
20-  unit address and first address in 'reg' (0x1000) don't match for
21-  /soc/spi@40003800/spi_nor@0/partitions/partition@10000
22-

legend@25hdd/samples/userspace/hello_world_user/sample.helloworld/build.log
18:  gen_defines.py failed with result code: 0 - stderr contents:
19-
20-  unit address and first address in 'reg' (0x1000) don't match for
21-  /soc/spi@40003800/spi_nor@0/partitions/partition@10000
22-

nrf52dk_nrf52805/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x20000) don't match for
20-  /soc/flash-controller@4001e000/flash@0/partitions/partition@19000
21-

cyw920829m2evk_02/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x60000) don't match for
20-  /qspi_flash@40890000/flash@60000000/partitions/storage_partition@70000
21-

imx93_evk_mimx9352_a55/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x2) don't match for
20-  /enet@42890000/mdio/phy@0
21-

da1469x_dk_pro/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x82000) don't match for
20-  /soc/flash-controller@38000000/flash@16000000/partitions/partition@80000
21-

slstk3401a/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x3e800) don't match for
20-  /soc/flash-controller@400e0000/flash@0/partitions/partition@fe800
21-

imx8mp_evk_mimx8ml8_a53/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x1) don't match for
20-  /soc/enet@30be0000/mdio/phy@0
21-

imx8mp_evk_mimx8ml8_a53_smp/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x1) don't match for
20-  /soc/enet@30be0000/mdio/phy@0
21-

lpcxpresso55s06/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x3fc70) don't match for
20-  /soc/peripheral@40000000/flash-controller@34000/flash@9fc70
21-

google_twinkie_v2/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0xf) don't match for
20-  /soc/adc@40012400/channel@15
21-

mimxrt595_evk_mimxrt595s_cm33/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x1a0000000000000000) don't match
20-  for /soc/peripheral@50000000/i3c@36000/wm8904@1a
21-

da14695_dk_usb/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x82000) don't match for
20-  /soc/flash-controller@38000000/flash@16000000/partitions/partition@80000
21-

mimxrt685_evk_mimxrt685s_cm33/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x1a0000000000000000) don't match
20-  for /soc/peripheral@50000000/i3c@36000/wm8904@1a
21-

nrf9280pdk_nrf9280_cpuppr_xip/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x2f0d0000) don't match for
20-  /reserved-memory/memory@2
21-

nrf9280pdk_nrf9280_cpurad/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x2f0d0000) don't match for
20-  /reserved-memory/memory@2
21-

nrf9280pdk_nrf9280_cpuapp/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x2f0d0000) don't match for
20-  /reserved-memory/memory@2
21-

ctcc_nrf52840/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x88000) don't match for
20-  /soc/flash-controller@4001e000/flash@0/partitions/partition@87000
21-

usb_kw24d512/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x70000) don't match for
20-  /soc/flash-controller@40020000/flash@0/partitions/partition@700000
21-

frdm_kw41z/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x70000) don't match for
20-  /soc/flash-controller@40020000/flash@0/partitions/partition@700000
21-

arduino_nano_33_iot/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x6a) don't match for
20-  /soc/sercom@42001800/atecc608a@15
21-

rzt2m_starter_kit/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x80280000) don't match for
20-  /soc/sckcr@81280004
21-

frdm_mcxn236/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x88000) don't match for
20-  /soc/peripheral@50000000/flash-controller@43000/flash@0/partitions/partition@80000
21-

altera_max10/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x1002c0) don't match for
20-  /soc/dma@100200
21-

lpcxpresso11u68/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0xa0000000) don't match for
20-  /soc/gpio@0
21-

lpcxpresso55s28/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x40094000) don't match for
20-  /soc/peripheral@40000000/usbhs@144000
21-

lpcxpresso55s16/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x3fc70) don't match for
20-  /soc/peripheral@40000000/flash-controller@34000/flash@9fc70
21-

nrf54l15dk_nrf54l15_cpuflpr/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x20028000) don't match for
20-  /soc/memory@2002f000
21-

nrf54l15dk_nrf54l15_cpuapp/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x5004c000) don't match for
20-  /soc/peripheral@50000000/vpr@4c000/mailbox@1
21-

v2m_musca_s1_musca_s1_ns/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0xa080000) don't match for
20-  /mram@a080400
21-

nrf54l15pdk_nrf54l15_cpuflpr/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x20028000) don't match for
20-  /soc/memory@2002f000
21-

nrf54l15pdk_nrf54l15_cpuapp/samples/userspace/hello_world_user/sample.helloworld/build.log
17:  gen_defines.py failed with result code: 0 - stderr contents:
18-
19-  unit address and first address in 'reg' (0x5004c000) don't match for
20-  /soc/peripheral@50000000/vpr@4c000/mailbox@1
21-

@tejlmand
Copy link
Contributor

tejlmand commented Sep 12, 2024

IMO it should fail the same way as if(NOT "${ret}" STREQUAL "0")

Not against failing gen_defines on warnings like mentioned in PR description, but I don't think the decision to fail should not be done in CMake based on content of stderr, instead this should be decided at the earliest point in time, ie. here https://github.com/zephyrproject-rtos/zephyr/blob/main/scripts/dts/gen_defines.py.

Meaning, if gen_defines should fail on warnings, then it should be gen_defines itself which has a return code != 0.
Whether to treat warnings as errors could be controlled by allowing a --warnings-as-errors flag when calling gen_defines.

But short-comings in underlying scripts should not be handled by adjusting CMake.

Note, we need to cleanup devicetree files before being able to enable such a --warnings-as-errors.

Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Perhaps better to take a step back and cleanup this code and instead use the COMMAND_ERROR_IS_FATAL feature of execute_process, and then a followup step of having a --warnings-as-errors in the gen_defines script.

Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps better to take a step back and remove the patching of paching by simply allow CMake to handle this situation and to ensure CMake always prints both stdout and stderror.

Thus simplify the code to:

execute_process(
  COMMAND ${CMD_GEN_DEFINES}
  WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
  COMMAND_ERROR_IS_FATAL ANY
)

and then completely remove the logic of

if(NOT "${ret}" STREQUAL "0" OR stderr)
  if (stderr)
    ...
  else()
    ..
  endif()
else()

That will ensure:

  • Warnings from gen_defines are printed on console
  • Errors in gen_defines will fail CMake and print the error.

And if we then extend gen_defines to take --warnings-as-errors flag, then it's clearer where the responsibility of dts warnings / errors belongs, and CMake can then apply the flag when the devicetree code has been cleaned up and thereby failing the build on dts warnings (the --warning-as-errors can be done as independent PR).

@rettichschnidi
Copy link
Contributor Author

rettichschnidi commented Sep 12, 2024

Note, we need to cleanup devicetree files before being able to enable such a --warnings-as-errors.

@tejlmand My understanding is that --edtlib-Werror would (should) do pretty much this. Do you think that #78253 goes in the right direction? If so I can flesh it out before continuing with this PR.

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 <[email protected]>
@rettichschnidi rettichschnidi force-pushed the gardena/rs/upstream/restore-gen_defines.py-warnings branch from bef6e82 to 6f4635f Compare September 12, 2024 22:38
@tejlmand
Copy link
Contributor

@tejlmand My understanding is that --edtlib-Werror would (should) do pretty much this. Do you think that #78253 goes in the right direction? If so I can flesh it out before continuing with this PR.

definitely the right way to go. Thanks for pointing to #78253 👍

@carlescufi carlescufi merged commit db7a390 into zephyrproject-rtos:main Sep 13, 2024
@rettichschnidi rettichschnidi deleted the gardena/rs/upstream/restore-gen_defines.py-warnings branch September 13, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Build System size: XS A PR changing only a single line of code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants