Hi, Willy
> 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@xxxxxxxxxxx>
> > ---
> > 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.
>
Ok, will reserve less ones as possible as we can.
- RUN_OUT is required for architecture specific output
- REPORT_RUN_OUT is not necessary, will remove it
> 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.
>
I have forgetten to pick an old patch to silence the running log like
this:
ifeq ($(QUIET_RUN),1)
LOG_OUT = > "$(RUN_OUT)"
else
LOG_OUT = | tee "$(RUN_OUT)"
endif
Without QUIET_RUN, we can silence the running log with:
$ make run LOG_OUT="> /dev/null"
It is not meaningful like QUIET_RUN, seems the QUIET_RUN is not
necessary for we have 'grep status' now, so, let's remove this RUN_OUT
too.
Thanks,
Zhangjin
> Thanks,
> Willy
