Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57918 )
Change subject: tests: Add init-shutdown test for raiden_debug_spi ......................................................................
Patch Set 7:
(1 comment)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/57918/comment/b6c41659_96366406 PS6, Line 57: ss
Specifically here, there were no warnings about raiden functions. But CONFIG_RAIDEN_DEBUG_SPI is enabled by default, maybe that's why?
I expected warnings because these functions were neither declared static nor in a forward-declaration (e.g. in a header file). This is usually enabled by GCC's `-Wmissing-prototypes` option. It seems the `meson.build` just uses different warning options than the `Makefile`. That's not a good idea because reviewers are used to these warnings and probably won't check things that they assume the compiled checked.
The `Makefile` also has a switch `WARNERROR` that defaults to `yes`. We could have something similar for the tests that may default to `no` if you prefer but would be enable by `test_build.sh` for the CI?
And even more global question, about switching away from meson for tests, there is a lot to think about. But we do need to think about it, yes.
I guess the problem is that the current `meson.build` was not meant for general use. Somebody added their use case but maintaining it beyond that was not the plan. Thomas is working to re-structure flashrom so we might be able to perform a cleaner switch to Meson in the future. If this will ever happen, it will take quite some time until then.
If we want to use Meson for the tests, it might be better if somebody took over maintenance for it right now.