On Wed, Jul 19, 2023 at 09:19:10PM +0800, Zhangjin Wu wrote:
The kernel targets share the same kernel make operations, the same .config file, the same kernel image, add MAKE_KERNEL, KERNEL_CONFIG and KERNEL_IMAGE for them.
Many targets share the same logging related settings, let's add common variables RUN_OUT, LOG_OUT and REPORT_RUN_OUT for them.
The qemu run/rerun targets share the same qemu system run command, add QEMU_SYSTEM_RUN for them.
Signed-off-by: Zhangjin Wu falcon@tinylab.org
tools/testing/selftests/nolibc/Makefile | 41 ++++++++++++++++--------- 1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 0cd17de2062c..8c531518bb9f 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -166,45 +166,58 @@ endif libc-test: nolibc-test.c $(QUIET_CC)$(CC) -o $@ $< +# common macros for logging +RUN_OUT = $(CURDIR)/run.out +LOG_OUT = > "$(RUN_OUT)" +REPORT_RUN_OUT = $(REPORT) "$(RUN_OUT)"
# local libc-test run-libc-test: libc-test
- $(Q)./libc-test > "$(CURDIR)/run.out" || :
- $(Q)$(REPORT) $(CURDIR)/run.out
- $(Q)./libc-test $(LOG_OUT) || :
- $(Q)$(REPORT_RUN_OUT)
Sorry but I don't find that this improves maintainability, quite the opposite in fact. One reason is that you never visually expect that some shell indirection delimiters are hidden in a macro that seems to only convey a name. Sure there are valid use cases for this, but I think that here it's just adding too much abstraction and it makes it quite hard to unfold all of this mentally.
Please try to keep the number of macros to the minimum that needs to be replaced or forced by the user. Here I'm not seeing a compelling reason for a user to want to force LOG_OUT to something else. Also makefile macros are generally a pain to debug, which is another reason for not putting too many of them.
I noticed that your next patch changes LOG_OUT to tee, it could have done it everywhere and wouldn't affect readability as much.
Thanks, Willy