[kvm-unit-tests PATCH 0/4] powerpc fix and misc docs/build/CI improvements

[kvm-unit-tests PATCH 0/4] powerpc fix and misc docs/build/CI improvements

[Nicholas Piggin]

This is some fixes and odd bits before I was going to resubmit the
remains of the powerpc series.

Patch 1 is a silly rebase bug. I thought I must have missed a warning
in the build noise, but no gcc just doesn't warn for string literal to
bool conversion.

Patch 2 is the doc fix updated according to feedback from Thomas
(main change, enumerate the other special groups).

Patch 3 is a quick hack to make build warnings more obvious. Thoughts?
Results can be seen here
https://gitlab.com/npiggin/kvm-unit-tests/-/pipelines/1314895857

Patch 4, after debugging something with CI, it doesn't seem to save the
test logs on failure. I can't drive gitlab CI very well but this seemed
to fix it.

Thanks,
Nick

Nicholas Piggin (4):
  powerpc/sprs: Fix report_kfail call
  doc: update unittests doc
  build: Make build output pretty
  gitlab-ci: Always save artifacts

 .gitlab-ci.yml          |  5 +++++
 Makefile                | 14 ++++++++++++++
 arm/Makefile.common     |  7 +++++++
 docs/unittests.txt      | 11 ++++++++---
 powerpc/Makefile.common | 11 +++++++----
 powerpc/sprs.c          |  2 +-
 riscv/Makefile          |  5 +++++
 s390x/Makefile          | 18 +++++++++++++++++-
 scripts/mkstandalone.sh |  2 +-
 x86/Makefile.common     |  5 +++++
 10 files changed, 70 insertions(+), 10 deletions(-)

-- 
2.43.0

[kvm-unit-tests PATCH 1/4] powerpc/sprs: Fix report_kfail call

[Nicholas Piggin]

Parameters to report_kfail are wrong. String to bool conversion is not
warned by gcc, and printf format did not catch it due to string variable
being passed at the format location.

Fixes: 8f6290f0e6 ("powerpc/sprs: Specify SPRs with data rather than code")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 powerpc/sprs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index de9e87a21..33872136d 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -590,7 +590,7 @@ int main(int argc, char **argv)
 
 		if (sprs[i].width == 32 && !(before[i] >> 32) && !(after[i] >> 32)) {
 			/* known failure KVM migration of CTRL */
-			report_kfail(true && i == 136,
+			report_kfail(i == 136, pass,
 				"%-10s(%4d):\t        0x%08lx <==>         0x%08lx",
 				sprs[i].name, i,
 				before[i], after[i]);
-- 
2.43.0

Re: [kvm-unit-tests PATCH 1/4] powerpc/sprs: Fix report_kfail call

[Thomas Huth]

On 02/06/2024 14.25, Nicholas Piggin wrote:
> Parameters to report_kfail are wrong. String to bool conversion is not
> warned by gcc, and printf format did not catch it due to string variable
> being passed at the format location.
> 
> Fixes: 8f6290f0e6 ("powerpc/sprs: Specify SPRs with data rather than code")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   powerpc/sprs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index de9e87a21..33872136d 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -590,7 +590,7 @@ int main(int argc, char **argv)
>   
>   		if (sprs[i].width == 32 && !(before[i] >> 32) && !(after[i] >> 32)) {
>   			/* known failure KVM migration of CTRL */
> -			report_kfail(true && i == 136,
> +			report_kfail(i == 136, pass,
>   				"%-10s(%4d):\t        0x%08lx <==>         0x%08lx",
>   				sprs[i].name, i,
>   				before[i], after[i]);

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH 2/4] doc: update unittests doc

[Nicholas Piggin]

Document the special groups, check path restrictions, and a small fix
for check option syntax.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 docs/unittests.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 6ff9872cf..509c529d7 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -69,8 +69,11 @@ groups
 groups = <group_name1> <group_name2> ...
 
 Used to group the test cases for the `run_tests.sh -g ...` run group
-option. Adding a test to the nodefault group will cause it to not be
-run by default.
+option. The group name is arbitrary, except for these special groups:
+- Tests in the "nodefault" group are not run by default (with no -g option).
+- Tests in the "migration" group are run with the migration harness and
+  expects the test to make migrate_*() calls.
+- Tests in the "panic" group expect QEMU to enter the GUEST_PANICKED state.
 
 accel
 -----
@@ -89,8 +92,10 @@ Optional timeout in seconds, after which the test will be killed and fail.
 
 check
 -----
-check = <path>=<<value>
+check = <path>=<value>
 
 Check a file for a particular value before running a test. The check line
 can contain multiple files to check separated by a space, but each check
 parameter needs to be of the form <path>=<value>
+
+The path and value can not contain space, =, or shell wildcard characters.
-- 
2.43.0

Re: [kvm-unit-tests PATCH 2/4] doc: update unittests doc

[Thomas Huth]

On 02/06/2024 14.25, Nicholas Piggin wrote:
> Document the special groups, check path restrictions, and a small fix
> for check option syntax.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   docs/unittests.txt | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 2/4] doc: update unittests doc

[Andrew Jones]

On Sun, Jun 02, 2024 at 10:25:56PM GMT, Nicholas Piggin wrote:
> Document the special groups, check path restrictions, and a small fix
> for check option syntax.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  docs/unittests.txt | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index 6ff9872cf..509c529d7 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -69,8 +69,11 @@ groups
>  groups = <group_name1> <group_name2> ...
>  
>  Used to group the test cases for the `run_tests.sh -g ...` run group
> -option. Adding a test to the nodefault group will cause it to not be
> -run by default.
> +option. The group name is arbitrary, except for these special groups:
> +- Tests in the "nodefault" group are not run by default (with no -g option).
> +- Tests in the "migration" group are run with the migration harness and
> +  expects the test to make migrate_*() calls.

expect make migrate_*() calls.

> +- Tests in the "panic" group expect QEMU to enter the GUEST_PANICKED state.
>  
>  accel
>  -----
> @@ -89,8 +92,10 @@ Optional timeout in seconds, after which the test will be killed and fail.
>  
>  check
>  -----
> -check = <path>=<<value>
> +check = <path>=<value>
>  
>  Check a file for a particular value before running a test. The check line
>  can contain multiple files to check separated by a space, but each check
>  parameter needs to be of the form <path>=<value>
> +
> +The path and value can not contain space, =, or shell wildcard characters.

cannot

Otherwise,

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Thanks,
drew

Re: [kvm-unit-tests PATCH 2/4] doc: update unittests doc

[Nicholas Piggin]

On Mon Jun 3, 2024 at 4:47 PM AEST, Andrew Jones wrote:
> On Sun, Jun 02, 2024 at 10:25:56PM GMT, Nicholas Piggin wrote:
> > Document the special groups, check path restrictions, and a small fix
> > for check option syntax.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  docs/unittests.txt | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/docs/unittests.txt b/docs/unittests.txt
> > index 6ff9872cf..509c529d7 100644
> > --- a/docs/unittests.txt
> > +++ b/docs/unittests.txt
> > @@ -69,8 +69,11 @@ groups
> >  groups = <group_name1> <group_name2> ...
> >  
> >  Used to group the test cases for the `run_tests.sh -g ...` run group
> > -option. Adding a test to the nodefault group will cause it to not be
> > -run by default.
> > +option. The group name is arbitrary, except for these special groups:
> > +- Tests in the "nodefault" group are not run by default (with no -g option).
> > +- Tests in the "migration" group are run with the migration harness and
> > +  expects the test to make migrate_*() calls.
>
> expect make migrate_*() calls.

Not sure if I follow you, but the grammar does sound a bit off now that
I read it back. Is this better?

"... are run with the migration harness and are expected to make
 migrate_*() calls."

or

"... are run with the migration harness, which expects them to make
 migrate_*() calls."

>
> > +- Tests in the "panic" group expect QEMU to enter the GUEST_PANICKED state.
> >  
> >  accel
> >  -----
> > @@ -89,8 +92,10 @@ Optional timeout in seconds, after which the test will be killed and fail.
> >  
> >  check
> >  -----
> > -check = <path>=<<value>
> > +check = <path>=<value>
> >  
> >  Check a file for a particular value before running a test. The check line
> >  can contain multiple files to check separated by a space, but each check
> >  parameter needs to be of the form <path>=<value>
> > +
> > +The path and value can not contain space, =, or shell wildcard characters.
>
> cannot

Huh, seems that is the more usual and formal form. I dind't know that.

Thanks,
Nick

>
> Otherwise,
>
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
>
> Thanks,
> drew

Re: [kvm-unit-tests PATCH 2/4] doc: update unittests doc

[Andrew Jones]

On Mon, Jun 03, 2024 at 06:12:05PM GMT, Nicholas Piggin wrote:
> On Mon Jun 3, 2024 at 4:47 PM AEST, Andrew Jones wrote:
> > On Sun, Jun 02, 2024 at 10:25:56PM GMT, Nicholas Piggin wrote:
> > > Document the special groups, check path restrictions, and a small fix
> > > for check option syntax.
> > > 
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > >  docs/unittests.txt | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/docs/unittests.txt b/docs/unittests.txt
> > > index 6ff9872cf..509c529d7 100644
> > > --- a/docs/unittests.txt
> > > +++ b/docs/unittests.txt
> > > @@ -69,8 +69,11 @@ groups
> > >  groups = <group_name1> <group_name2> ...
> > >  
> > >  Used to group the test cases for the `run_tests.sh -g ...` run group
> > > -option. Adding a test to the nodefault group will cause it to not be
> > > -run by default.
> > > +option. The group name is arbitrary, except for these special groups:
> > > +- Tests in the "nodefault" group are not run by default (with no -g option).
> > > +- Tests in the "migration" group are run with the migration harness and
> > > +  expects the test to make migrate_*() calls.
> >
> > expect make migrate_*() calls.
> 
> Not sure if I follow you, but the grammar does sound a bit off now that
> I read it back. Is this better?
> 
> "... are run with the migration harness and are expected to make
>  migrate_*() calls."

This one looks good to me.

Thanks,
drew

> 
> or
> 
> "... are run with the migration harness, which expects them to make
>  migrate_*() calls."
> 
> >
> > > +- Tests in the "panic" group expect QEMU to enter the GUEST_PANICKED state.
> > >  
> > >  accel
> > >  -----
> > > @@ -89,8 +92,10 @@ Optional timeout in seconds, after which the test will be killed and fail.
> > >  
> > >  check
> > >  -----
> > > -check = <path>=<<value>
> > > +check = <path>=<value>
> > >  
> > >  Check a file for a particular value before running a test. The check line
> > >  can contain multiple files to check separated by a space, but each check
> > >  parameter needs to be of the form <path>=<value>
> > > +
> > > +The path and value can not contain space, =, or shell wildcard characters.
> >
> > cannot
> 
> Huh, seems that is the more usual and formal form. I dind't know that.
> 
> Thanks,
> Nick
> 
> >
> > Otherwise,
> >
> > Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> >
> > Thanks,
> > drew
> 

[kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Nicholas Piggin]

Unless make V=1 is specified, silence make recipe echoing and print
an abbreviated line for major build steps.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 Makefile                | 14 ++++++++++++++
 arm/Makefile.common     |  7 +++++++
 powerpc/Makefile.common | 11 +++++++----
 riscv/Makefile          |  5 +++++
 s390x/Makefile          | 18 +++++++++++++++++-
 scripts/mkstandalone.sh |  2 +-
 x86/Makefile.common     |  5 +++++
 7 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 5b7998b79..cbb2fdbf1 100644
--- a/Makefile
+++ b/Makefile
@@ -9,6 +9,11 @@ include config.mak
 # Set search path for all sources
 VPATH = $(SRCDIR)
 
+V=0
+ifeq ($V, 0)
+.SILENT:
+endif
+
 libdirs-get = $(shell [ -d "lib/$(1)" ] && echo "lib/$(1) lib/$(1)/asm")
 ARCH_LIBDIRS := $(call libdirs-get,$(ARCH_LIBDIR)) $(call libdirs-get,$(TEST_DIR))
 OBJDIRS := $(ARCH_LIBDIRS)
@@ -95,11 +100,13 @@ autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
 LDFLAGS += -nostdlib $(no_pie) -z noexecstack
 
 $(libcflat): $(cflatobjs)
+	@echo " [AR]      $@"
 	$(AR) rcs $@ $^
 
 include $(LIBFDT_srcdir)/Makefile.libfdt
 $(LIBFDT_archive): CFLAGS += -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -Wno-sign-compare
 $(LIBFDT_archive): $(addprefix $(LIBFDT_objdir)/,$(LIBFDT_OBJS))
+	@echo " [AR]      $@"
 	$(AR) rcs $@ $^
 
 libfdt_clean: VECHO = echo " "
@@ -112,7 +119,12 @@ libfdt_clean: SHAREDLIB_EXT = so
 directories:
 	@mkdir -p $(OBJDIRS)
 
+%.o: %.c
+	@echo " [CC]      $@"
+	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+
 %.o: %.S
+	@echo " [AS]      $@"
 	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
 
 -include */.*.d */*/.*.d
@@ -123,6 +135,7 @@ standalone: all
 	@scripts/mkstandalone.sh
 
 install: standalone
+	@echo " [INSTALL] tests -> $(DESTDIR)"
 	mkdir -p $(DESTDIR)
 	install tests/* $(DESTDIR)
 
@@ -136,6 +149,7 @@ distclean: clean
 
 cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
 cscope:
+	@echo " [CSCOPE]"
 	$(RM) ./cscope.*
 	find -L $(cscope_dirs) -maxdepth 1 \
 		-name '*.[chsS]' -exec realpath --relative-base=$(CURDIR) {} \; | sort -u > ./cscope.files
diff --git a/arm/Makefile.common b/arm/Makefile.common
index f828dbe01..9d6b31239 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -73,15 +73,18 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
 
 ifeq ($(CONFIG_EFI),y)
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
 
 %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
 %.so: %.o $(FLATLIBS) $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o) %.aux.o
+	@echo " [LD]      $@"
 	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds \
 		$(filter %.o, $^) $(FLATLIBS) $(EFI_LIBS)
 
 %.efi: %.so
+	@echo " [OBJCOPY] $@"
 	$(call arch_elf_check, $^)
 	$(OBJCOPY) --only-keep-debug $^ $@.debug
 	$(OBJCOPY) --strip-debug $^
@@ -93,22 +96,26 @@ ifeq ($(CONFIG_EFI),y)
 		-O binary $^ $@
 else
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(@:.aux.o=.flat)\" -DAUXFLAGS=$(AUXFLAGS)
 
 %.elf: LDFLAGS += $(arch_LDFLAGS)
 %.elf: %.o $(FLATLIBS) $(SRCDIR)/arm/flat.lds $(cstart.o) %.aux.o
+	@echo " [LD]      $@"
 	$(LD) $(LDFLAGS) -o $@ -T $(SRCDIR)/arm/flat.lds \
 		$(filter %.o, $^) $(FLATLIBS)
 	@chmod a-x $@
 
 %.flat: %.elf
+	@echo " [OBJCOPY] $@"
 	$(call arch_elf_check, $^)
 	$(OBJCOPY) -O binary $^ $@
 	@chmod a-x $@
 endif
 
 $(libeabi): $(eabiobjs)
+	@echo " [AR]      $@"
 	$(AR) rcs $@ $^
 
 arm_clean: asm_offsets_clean
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 68165fc25..b0af9fc00 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -51,31 +51,34 @@ cflatobjs += lib/powerpc/smp.o
 OBJDIRS += lib/powerpc
 
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
+	@echo " [LD]      $@"
 	$(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\"
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
 %.elf: LDFLAGS += $(arch_LDFLAGS) -pie -n
 %.elf: %.o $(FLATLIBS) $(SRCDIR)/powerpc/flat.lds $(cstart.o) $(reloc.o) %.aux.o
+	@echo " [LD]      $@"
 	$(LD) $(LDFLAGS) -o $@ \
 		-T $(SRCDIR)/powerpc/flat.lds --build-id=none \
 		$(filter %.o, $^) $(FLATLIBS)
 	@chmod a-x $@
-	@echo -n Checking $@ for unsupported reloc types...
 	@if $(OBJDUMP) -R $@ | grep R_ | grep -v R_PPC64_RELATIVE; then	\
+		@echo "Unsupported reloc types in $@"			\
 		false;							\
-	else								\
-		echo " looks good.";					\
 	fi
 
 $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf
-	dd if=/dev/zero of=$@ bs=256 count=1
+	@echo " [DD]      $@"
+	dd if=/dev/zero of=$@ bs=256 count=1 status=none
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -O binary $^ $@.tmp
 	cat $@.tmp >> $@
 	$(RM) $@.tmp
 
 $(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian
 $(TEST_DIR)/boot_rom.elf: $(TEST_DIR)/boot_rom.o
+	@echo " [LD]      $@"
 	$(LD) -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none -o $@ $<
 	@chmod a-x $@
 
diff --git a/riscv/Makefile b/riscv/Makefile
index 919a3ebb5..ca33d4960 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -82,6 +82,7 @@ asm-offsets = lib/riscv/asm-offsets.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(notdir $(@:.aux.o=.$(exe)))\" -DAUXFLAGS=$(AUXFLAGS)
 
@@ -96,10 +97,12 @@ cflatobjs += lib/efi.o
 
 %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
 %.so: %.o $(FLATLIBS) $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds $(cstart.o) %.aux.o
+	@echo " [LD]      $@"
 	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds \
 		$(filter %.o, $^) $(FLATLIBS) $(EFI_LIBS)
 
 %.efi: %.so
+	@echo " [OBJCOPY] $@"
 	$(call arch_elf_check, $^)
 	$(OBJCOPY) --only-keep-debug $^ $@.debug
 	$(OBJCOPY) --strip-debug $^
@@ -112,11 +115,13 @@ cflatobjs += lib/efi.o
 else
 %.elf: LDFLAGS += -pie -n -z notext
 %.elf: %.o $(FLATLIBS) $(SRCDIR)/riscv/flat.lds $(cstart.o) %.aux.o
+	@echo " [LD]      $@"
 	$(LD) $(LDFLAGS) -o $@ -T $(SRCDIR)/riscv/flat.lds \
 		$(filter %.o, $^) $(FLATLIBS)
 	@chmod a-x $@
 
 %.flat: %.elf
+	@echo " [OBJCOPY] $@"
 	$(call arch_elf_check, $^)
 	$(OBJCOPY) -O binary $^ $@
 	@chmod a-x $@
diff --git a/s390x/Makefile b/s390x/Makefile
index 8603a523c..19c41a2ec 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -148,42 +148,54 @@ endif
 
 # the asm/c snippets %.o have additional generated files as dependencies
 $(SNIPPET_DIR)/asm/%.o: $(SNIPPET_DIR)/asm/%.S $(asm-offsets)
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
 
 $(SNIPPET_DIR)/c/%.o: $(SNIPPET_DIR)/c/%.c $(asm-offsets)
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
 
 $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(SNIPPET_DIR)/asm/flat.lds
+	@echo " [LD]      $@"
 	$(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/asm/flat.lds $<
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
 	truncate -s '%4096' $@
 
 $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS) $(SNIPPET_DIR)/c/flat.lds
+	@echo " [LD]      $@"
 	$(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds $< $(snippet_lib) $(FLATLIBS)
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
 	truncate -s '%4096' $@
 
 %.hdr: %.gbin $(HOST_KEY_DOCUMENT)
+	@echo " [SEHDR  ] $@"
 	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
 
 .SECONDARY:
 %.gobj: %.gbin
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
 
 .SECONDARY:
 %.hdr.obj: %.hdr
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
 
 lds-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@
 %.lds: %.lds.S $(asm-offsets)
+	@echo " [CPP]     $@"
 	$(CPP) $(lds-autodepend-flags) $(CPPFLAGS) -P -C -o $@ $<
 
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\"
 
 .SECONDEXPANSION:
 %.elf: $(FLATLIBS) $(asmlib) $(SRCDIR)/s390x/flat.lds $$(snippets-obj) $$(snippet-hdr-obj) %.o %.aux.o
-	@$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds \
+	@echo " [CC]      $@"
+	$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds \
 		$(filter %.o, $^) $(FLATLIBS) $(snippets-obj) $(snippet-hdr-obj) || \
 		{ echo "Failure probably caused by missing definition of gen-se-header executable"; exit 1; }
 	@chmod a-x $@
@@ -192,9 +204,11 @@ lds-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@
 # 32 bytes of key material, uses existing one if available
 comm-key = $(TEST_DIR)/comm.key
 $(comm-key):
+	@echo " [DD]      $@"
 	dd if=/dev/urandom of=$@ bs=32 count=1 status=none
 
 %.bin: %.elf
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -O binary  $< $@
 
 # The genprotimg arguments for the cck changed over time so we need to
@@ -216,10 +230,12 @@ endif
 
 $(patsubst %.parmfile,%.pv.bin,$(wildcard s390x/*.parmfile)): %.pv.bin: %.parmfile
 %.pv.bin: %.bin $(HOST_KEY_DOCUMENT) $(comm-key)
+	@echo " [GENPROT] $@"
 	$(eval parmfile_args = $(if $(filter %.parmfile,$^),--parmfile $(filter %.parmfile,$^),))
 	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_OPTION) $(comm-key) --x-pcf $(GENPROTIMG_PCF) $(parmfile_args) --image $(filter %.bin,$^) -o $@
 
 $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
+	@echo " [CC]      $@"
 	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
 
 
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 2318a85f0..3307c25b1 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -94,7 +94,7 @@ function mkstandalone()
 	generate_test "$@" > $standalone
 
 	chmod +x $standalone
-	echo Written $standalone.
+	echo " [WRITE]   $standalone"
 }
 
 if [ "$ENVIRON_DEFAULT" = "yes" ] && [ "$ERRATATXT" ] && [ ! -f "$ERRATATXT" ]; then
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 4ae9a5579..96fb0660c 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -49,11 +49,13 @@ ifeq ($(CONFIG_EFI),y)
 .PRECIOUS: %.efi %.so
 
 %.so: %.o $(FLATLIBS) $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(cstart.o)
+	@echo " [LD]      $@"
 	$(LD) -T $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(EFI_LDFLAGS) -o $@ \
 		$(filter %.o, $^) $(FLATLIBS)
 	@chmod a-x $@
 
 %.efi: %.so
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) --only-keep-debug $^ $@.debug
 	$(OBJCOPY) --strip-debug $^
 	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
@@ -67,11 +69,13 @@ else
 
 %.elf: LDFLAGS += $(arch_LDFLAGS)
 %.elf: %.o $(FLATLIBS) $(SRCDIR)/x86/flat.lds $(cstart.o)
+	@echo " [LD]      $@"
 	$(LD) $(LDFLAGS) -T $(SRCDIR)/x86/flat.lds -o $@ \
 		$(filter %.o, $^) $(FLATLIBS)
 	@chmod a-x $@
 
 %.flat: %.elf
+	@echo " [OBJCOPY] $@"
 	$(OBJCOPY) -O elf32-i386 $^ $@
 	@chmod a-x $@
 endif
@@ -104,6 +108,7 @@ test_cases: $(tests-common) $(tests)
 $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/x86 -I lib
 
 $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
+	@echo " [LD]      $@"
 	$(LD) -m elf_i386 -nostdlib -o $@ \
 	      -T $(SRCDIR)/$(TEST_DIR)/realmode.lds $^
 
-- 
2.43.0

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Andrew Jones]

On Sun, Jun 02, 2024 at 10:25:57PM GMT, Nicholas Piggin wrote:
> Unless make V=1 is specified, silence make recipe echoing and print
> an abbreviated line for major build steps.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  Makefile                | 14 ++++++++++++++
>  arm/Makefile.common     |  7 +++++++
>  powerpc/Makefile.common | 11 +++++++----
>  riscv/Makefile          |  5 +++++
>  s390x/Makefile          | 18 +++++++++++++++++-
>  scripts/mkstandalone.sh |  2 +-
>  x86/Makefile.common     |  5 +++++
>  7 files changed, 56 insertions(+), 6 deletions(-)

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Thomas Huth]

On 02/06/2024 14.25, Nicholas Piggin wrote:
> Unless make V=1 is specified, silence make recipe echoing and print
> an abbreviated line for major build steps.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   Makefile                | 14 ++++++++++++++
>   arm/Makefile.common     |  7 +++++++
>   powerpc/Makefile.common | 11 +++++++----
>   riscv/Makefile          |  5 +++++
>   s390x/Makefile          | 18 +++++++++++++++++-
>   scripts/mkstandalone.sh |  2 +-
>   x86/Makefile.common     |  5 +++++
>   7 files changed, 56 insertions(+), 6 deletions(-)

The short lines look superfluous in verbose mode, e.g.:

  [OBJCOPY] s390x/memory-verify.bin
objcopy -O binary  s390x/memory-verify.elf s390x/memory-verify.bin

Could we somehow suppress the echo lines in verbose mode, please?

For example in the SLOF project, it's done like this:

https://gitlab.com/slof/slof/-/blob/master/make.rules?ref_type=heads#L48

By putting the logic into $CC and friends, you also don't have to add 
"@echo" statements all over the place.

  Thomas

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Andrew Jones]

On Mon, Jun 03, 2024 at 10:26:50AM GMT, Thomas Huth wrote:
> On 02/06/2024 14.25, Nicholas Piggin wrote:
> > Unless make V=1 is specified, silence make recipe echoing and print
> > an abbreviated line for major build steps.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   Makefile                | 14 ++++++++++++++
> >   arm/Makefile.common     |  7 +++++++
> >   powerpc/Makefile.common | 11 +++++++----
> >   riscv/Makefile          |  5 +++++
> >   s390x/Makefile          | 18 +++++++++++++++++-
> >   scripts/mkstandalone.sh |  2 +-
> >   x86/Makefile.common     |  5 +++++
> >   7 files changed, 56 insertions(+), 6 deletions(-)
> 
> The short lines look superfluous in verbose mode, e.g.:
> 
>  [OBJCOPY] s390x/memory-verify.bin
> objcopy -O binary  s390x/memory-verify.elf s390x/memory-verify.bin
> 
> Could we somehow suppress the echo lines in verbose mode, please?
> 
> For example in the SLOF project, it's done like this:
> 
> https://gitlab.com/slof/slof/-/blob/master/make.rules?ref_type=heads#L48
> 
> By putting the logic into $CC and friends, you also don't have to add
> "@echo" statements all over the place.

And I presume make will treat the printing and compiling as one unit, so
parallel builds still get the summary above the error messages when
compilation fails. The way this patch is now a parallel build may show
the summary for the last successful build and then error messages for
a build that hasn't output its summary yet, which can be confusing.

So I agree that something more like SLOF's approach would be better.

Thanks,
drew

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Nicholas Piggin]

On Mon Jun 3, 2024 at 6:56 PM AEST, Andrew Jones wrote:
> On Mon, Jun 03, 2024 at 10:26:50AM GMT, Thomas Huth wrote:
> > On 02/06/2024 14.25, Nicholas Piggin wrote:
> > > Unless make V=1 is specified, silence make recipe echoing and print
> > > an abbreviated line for major build steps.
> > > 
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > >   Makefile                | 14 ++++++++++++++
> > >   arm/Makefile.common     |  7 +++++++
> > >   powerpc/Makefile.common | 11 +++++++----
> > >   riscv/Makefile          |  5 +++++
> > >   s390x/Makefile          | 18 +++++++++++++++++-
> > >   scripts/mkstandalone.sh |  2 +-
> > >   x86/Makefile.common     |  5 +++++
> > >   7 files changed, 56 insertions(+), 6 deletions(-)
> > 
> > The short lines look superfluous in verbose mode, e.g.:
> > 
> >  [OBJCOPY] s390x/memory-verify.bin
> > objcopy -O binary  s390x/memory-verify.elf s390x/memory-verify.bin
> > 
> > Could we somehow suppress the echo lines in verbose mode, please?
> > 
> > For example in the SLOF project, it's done like this:
> > 
> > https://gitlab.com/slof/slof/-/blob/master/make.rules?ref_type=heads#L48
> > 
> > By putting the logic into $CC and friends, you also don't have to add
> > "@echo" statements all over the place.
>
> And I presume make will treat the printing and compiling as one unit, so
> parallel builds still get the summary above the error messages when
> compilation fails. The way this patch is now a parallel build may show
> the summary for the last successful build and then error messages for
> a build that hasn't output its summary yet, which can be confusing.
>
> So I agree that something more like SLOF's approach would be better.

Hmm... kbuild type commands is a pretty big patch. I like it though.
Thoughts?

Thanks,
Nick

---

diff --git a/Makefile b/Makefile
index 5b7998b79..56052107f 100644
--- a/Makefile
+++ b/Makefile
@@ -53,6 +53,34 @@ EFI_CFLAGS += -fPIC
 EFI_LDFLAGS := -Bsymbolic -shared -nostdlib
 endif
 
+quiet = quiet_
+quiet_echo = @echo $1
+Q = @
+ifeq ($V, 1)
+	quiet =
+	quiet_echo =
+	Q =
+endif
+
+cmd = @$(echo-cmd) $(cmd_$(1))
+echo-cmd = $(if $($(quiet)cmd_$(1)), echo ' $($(quiet)cmd_$(1))';)
+
+quiet_cmd_cc = [CC]      $@
+      cmd_cc = $(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+
+quiet_cmd_cpp = [CPP]     $@
+      cmd_cpp = $(CPP) $(CPPFLAGS) -P -C -o $@ $<
+
+quiet_cmd_as = [AS]      $@
+      cmd_as = $(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+
+# Link libs last
+quiet_cmd_ld = [LD]      $@
+      cmd_ld = $(LD) $(LDFLAGS) -o $@ $(filter %.o %.gobj, $^) $(filter %.a, $^)
+
+quiet_cmd_ar = [AR]      $@
+      cmd_ar = $(AR) rcs $@ $^
+
 #include architecture specific make rules
 include $(SRCDIR)/$(TEST_DIR)/Makefile
 
@@ -95,14 +123,13 @@ autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d
 LDFLAGS += -nostdlib $(no_pie) -z noexecstack
 
 $(libcflat): $(cflatobjs)
-	$(AR) rcs $@ $^
+	$(call cmd,ar)
 
 include $(LIBFDT_srcdir)/Makefile.libfdt
 $(LIBFDT_archive): CFLAGS += -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -Wno-sign-compare
 $(LIBFDT_archive): $(addprefix $(LIBFDT_objdir)/,$(LIBFDT_OBJS))
-	$(AR) rcs $@ $^
+	$(call cmd,ar)
 
-libfdt_clean: VECHO = echo " "
 libfdt_clean: STD_CLEANFILES = *.o .*.d
 libfdt_clean: LIBFDT_dir = $(LIBFDT_objdir)
 libfdt_clean: SHAREDLIB_EXT = so
@@ -112,8 +139,11 @@ libfdt_clean: SHAREDLIB_EXT = so
 directories:
 	@mkdir -p $(OBJDIRS)
 
+%.o: %.c
+	$(call cmd,cc)
+
 %.o: %.S
-	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+	$(call cmd,as)
 
 -include */.*.d */*/.*.d
 
@@ -123,23 +153,29 @@ standalone: all
 	@scripts/mkstandalone.sh
 
 install: standalone
-	mkdir -p $(DESTDIR)
-	install tests/* $(DESTDIR)
+	$(call quiet_echo, " [INSTALL] tests -> $(DESTDIR)")
+	$(Q)mkdir -p $(DESTDIR)
+	$(Q)install tests/* $(DESTDIR)
 
-clean: arch_clean libfdt_clean
-	$(RM) $(LIBFDT_archive)
-	$(RM) lib/.*.d $(libcflat) $(cflatobjs)
+#clean: arch_clean libfdt_clean
+clean:
+	$(call quiet_echo, " [CLEAN]")
+	$(Q)$(MAKE) --no-print-directory arch_clean libfdt_clean
+	$(Q)$(RM) $(LIBFDT_archive)
+	$(Q)$(RM) lib/.*.d $(libcflat) $(cflatobjs)
 
 distclean: clean
-	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
-	$(RM) -r tests logs logs.old efi-tests
+	$(call quiet_echo, " [DISTCLEAN]")
+	$(Q)$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
+	$(Q)$(RM) -r tests logs logs.old efi-tests
 
 cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
 cscope:
-	$(RM) ./cscope.*
-	find -L $(cscope_dirs) -maxdepth 1 \
+	$(Q)$(RM) ./cscope.*
+	$(Q)find -L $(cscope_dirs) -maxdepth 1 \
 		-name '*.[chsS]' -exec realpath --relative-base=$(CURDIR) {} \; | sort -u > ./cscope.files
-	cscope -bk
+	$(call quiet_echo, " [CSCOPE]")
+	$(Q)cscope -bk
 
 .PHONY: shellcheck
 shellcheck:
diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
index 960880f1c..06e856ba1 100644
--- a/arm/Makefile.arm64
+++ b/arm/Makefile.arm64
@@ -52,4 +52,4 @@ tests += $(TEST_DIR)/debug.$(exe)
 include $(SRCDIR)/$(TEST_DIR)/Makefile.common
 
 arch_clean: arm_clean
-	$(RM) lib/arm64/.*.d
+	$(Q)$(RM) lib/arm64/.*.d
diff --git a/arm/Makefile.common b/arm/Makefile.common
index 5f22c9b08..0def9a327 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -72,48 +72,47 @@ eabiobjs = lib/arm/eabi_compat.o
 FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi)
 
 ifeq ($(CONFIG_EFI),y)
+%.aux.o: CFLAGS += -DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
 $(tests-all:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c
-	$(CC) $(CFLAGS) -c -o $@ $< \
-		-DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS)
+	$(call cmd,cc)
 
-%.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
-%.so: %.o $(FLATLIBS) $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o) %.aux.o
-	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds \
-		$(filter %.o, $^) $(FLATLIBS)
+%.so: LDFLAGS = $(EFI_LDFLAGS) -defsym=EFI_SUBSYSTEM=0xa --no-undefined -T $(SRCDIR)/arm/efi/elf_aarch64_efi.lds
+%.so: %.o $(SRCDIR)/arm/efi/elf_aarch64_efi.lds $(cstart.o) $(FLATLIBS) $(EFI_LIBS) %.aux.o
+	$(call cmd,ld)
 
 %.efi: %.so
 	$(call arch_elf_check, $^)
-	$(OBJCOPY) --only-keep-debug $^ $@.debug
-	$(OBJCOPY) --strip-debug $^
-	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
-	$(OBJCOPY) \
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) --only-keep-debug $^ $@.debug
+	$(Q)$(OBJCOPY) --strip-debug $^
+	$(Q)$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
+	$(Q)$(OBJCOPY) \
 		-j .text -j .sdata -j .data -j .dynamic -j .dynsym \
 		-j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \
 		-j .reloc \
 		-O binary $^ $@
 else
+%.aux.o: CFLAGS += -DPROGNAME=\"$(@:.aux.o=.flat)\" -DAUXFLAGS=$(AUXFLAGS)
 $(tests-all:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c
-%.aux.o: $(SRCDIR)/lib/auxinfo.c
-	$(CC) $(CFLAGS) -c -o $@ $< \
-		-DPROGNAME=\"$(@:.aux.o=.flat)\" -DAUXFLAGS=$(AUXFLAGS)
+	$(call cmd,cc)
 
-%.elf: LDFLAGS += $(arch_LDFLAGS)
-%.elf: %.o $(FLATLIBS) $(SRCDIR)/arm/flat.lds $(cstart.o) %.aux.o
-	$(LD) $(LDFLAGS) -o $@ -T $(SRCDIR)/arm/flat.lds \
-		$(filter %.o, $^) $(FLATLIBS)
-	@chmod a-x $@
+%.elf: LDFLAGS += $(arch_LDFLAGS) -T $(SRCDIR)/arm/flat.lds
+%.elf: %.o $(SRCDIR)/arm/flat.lds $(cstart.o) $(FLATLIBS) %.aux.o
+	$(call cmd,ld)
+	$(Q)@chmod a-x $@
 
 %.flat: %.elf
 	$(call arch_elf_check, $^)
-	$(OBJCOPY) -O binary $^ $@
-	@chmod a-x $@
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -O binary $^ $@
+	$(Q)chmod a-x $@
 endif
 
 $(libeabi): $(eabiobjs)
-	$(AR) rcs $@ $^
+	$(call cmd,ar)
 
 arm_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,flat,elf,so,efi,debug} $(libeabi) $(eabiobjs) \
+	$(Q)$(RM) $(TEST_DIR)/*.{o,flat,elf,so,efi,debug} $(libeabi) $(eabiobjs) \
 	      $(TEST_DIR)/.*.d $(TEST_DIR)/efi/.*.d lib/arm/.*.d
 
 generated-files = $(asm-offsets)
diff --git a/lib/libfdt/Makefile.libfdt b/lib/libfdt/Makefile.libfdt
index b6d8fc02d..3a1bc1128 100644
--- a/lib/libfdt/Makefile.libfdt
+++ b/lib/libfdt/Makefile.libfdt
@@ -13,6 +13,5 @@ LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
 LIBFDT_LIB = libfdt-$(DTC_VERSION).$(SHAREDLIB_EXT)
 
 libfdt_clean:
-	@$(VECHO) CLEAN "(libfdt)"
-	rm -f $(STD_CLEANFILES:%=$(LIBFDT_dir)/%)
-	rm -f $(LIBFDT_dir)/$(LIBFDT_soname)
+	$(Q)rm -f $(STD_CLEANFILES:%=$(LIBFDT_dir)/%)
+	$(Q)rm -f $(LIBFDT_dir)/$(LIBFDT_soname)
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 3b219eee0..2fdf5bdae 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -52,37 +52,37 @@ cflatobjs += lib/powerpc/smp.o
 
 OBJDIRS += lib/powerpc
 
+%.aux.o: CFLAGS += -DPROGNAME=\"$(@:.aux.o=.elf)\"
 $(tests-all:.elf=.aux.o): $(SRCDIR)/lib/auxinfo.c
-	$(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\"
+	$(call cmd,cc)
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
-%.elf: LDFLAGS += $(arch_LDFLAGS) -pie -n
+%.elf: LDFLAGS += $(arch_LDFLAGS) -pie -n --build-id=none -T $(SRCDIR)/powerpc/flat.lds
 %.elf: %.o $(FLATLIBS) $(SRCDIR)/powerpc/flat.lds $(cstart.o) $(reloc.o) %.aux.o
-	$(LD) $(LDFLAGS) -o $@ \
-		-T $(SRCDIR)/powerpc/flat.lds --build-id=none \
-		$(filter %.o, $^) $(FLATLIBS)
-	@chmod a-x $@
-	@echo -n Checking $@ for unsupported reloc types...
+	$(call cmd,ld)
+	$(Q)chmod a-x $@
 	@if $(OBJDUMP) -R $@ | grep R_ | grep -v R_PPC64_RELATIVE; then	\
+		@echo "Unsupported reloc types in $@"			\
 		false;							\
-	else								\
-		echo " looks good.";					\
 	fi
 
 $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf
-	dd if=/dev/zero of=$@ bs=256 count=1
-	$(OBJCOPY) -O binary $^ $@.tmp
-	cat $@.tmp >> $@
-	$(RM) $@.tmp
+	$(call quiet_echo, " [DD]      $@")
+	$(Q)dd if=/dev/zero of=$@ bs=256 count=1 status=none
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -O binary $^ $@.tmp
+	$(Q)cat $@.tmp >> $@
+	$(Q)$(RM) $@.tmp
 
 $(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian
+$(TEST_DIR)/boot_rom.elf: LDFLAGS = -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none
 $(TEST_DIR)/boot_rom.elf: $(TEST_DIR)/boot_rom.o
-	$(LD) -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none -o $@ $<
-	@chmod a-x $@
+	$(call cmd,ld)
+	$(Q)chmod a-x $@
 
 powerpc_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,elf} $(TEST_DIR)/boot_rom.bin \
+	$(Q)$(RM) $(TEST_DIR)/*.{o,elf} $(TEST_DIR)/boot_rom.bin \
 	      $(TEST_DIR)/.*.d lib/powerpc/.*.d
 
 generated-files = $(asm-offsets)
diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
index a18a9628f..b10499229 100644
--- a/powerpc/Makefile.ppc64
+++ b/powerpc/Makefile.ppc64
@@ -27,4 +27,4 @@ tests = $(TEST_DIR)/spapr_vpa.elf
 include $(SRCDIR)/$(TEST_DIR)/Makefile.common
 
 arch_clean: powerpc_clean
-	$(RM) lib/ppc64/.*.d
+	$(Q)$(RM) lib/ppc64/.*.d
diff --git a/riscv/Makefile b/riscv/Makefile
index eacca7ce1..f6fb9488a 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -81,9 +81,9 @@ CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt
 asm-offsets = lib/riscv/asm-offsets.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
+%.aux.o: CFLAGS += -DPROGNAME=\"$(notdir $(@:.aux.o=.$(exe)))\" -DAUXFLAGS=$(AUXFLAGS)
 $(tests:.$(exe)=.aux.o): $(SRCDIR)/lib/auxinfo.c
-	$(CC) $(CFLAGS) -c -o $@ $< \
-		-DPROGNAME=\"$(notdir $(@:.aux.o=.$(exe)))\" -DAUXFLAGS=$(AUXFLAGS)
+	$(call cmd,cc)
 
 ifeq ($(CONFIG_EFI),y)
 # avoid jump tables before all relocations have been processed
@@ -95,36 +95,37 @@ cflatobjs += lib/efi.o
 .PRECIOUS: %.so
 
 %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
-%.so: %.o $(FLATLIBS) $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds $(cstart.o) %.aux.o
-	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds \
-		$(filter %.o, $^) $(FLATLIBS)
+%.so: LDFLAGS = $(EFI_LDFLAGS) -T $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds
+%.so: %.o $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds $(cstart.o) %.aux.o $(FLATLIBS)
+	$(call cmd,ld)
 
 %.efi: %.so
 	$(call arch_elf_check, $^)
-	$(OBJCOPY) --only-keep-debug $^ $@.debug
-	$(OBJCOPY) --strip-debug $^
-	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
-	$(OBJCOPY) \
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) --only-keep-debug $^ $@.debug
+	$(Q)$(OBJCOPY) --strip-debug $^
+	$(Q)$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
+	$(Q)$(OBJCOPY) \
 		-j .text -j .sdata -j .data -j .rodata -j .dynamic -j .dynsym \
 		-j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \
 		-j .reloc \
 		-O binary $^ $@
 else
-%.elf: LDFLAGS += -pie -n -z notext
-%.elf: %.o $(FLATLIBS) $(SRCDIR)/riscv/flat.lds $(cstart.o) %.aux.o
-	$(LD) $(LDFLAGS) -o $@ -T $(SRCDIR)/riscv/flat.lds \
-		$(filter %.o, $^) $(FLATLIBS)
-	@chmod a-x $@
+%.elf: LDFLAGS += -pie -n -z notext -T $(SRCDIR)/riscv/flat.lds
+%.elf: %.o $(SRCDIR)/riscv/flat.lds $(cstart.o) %.aux.o $(FLATLIBS)
+	$(call cmd,ld)
+	$(Q)@chmod a-x $@
 
 %.flat: %.elf
 	$(call arch_elf_check, $^)
+	$(call quiet_echo, " [OBJCOPY] $@")
 	$(OBJCOPY) -O binary $^ $@
-	@chmod a-x $@
+	$(Q)@chmod a-x $@
 endif
 
 generated-files = $(asm-offsets)
 $(tests:.$(exe)=.o) $(cstart.o) $(cflatobjs): $(generated-files)
 
 arch_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,flat,elf,so,efi,debug} \
+	$(Q)$(RM) $(TEST_DIR)/*.{o,flat,elf,so,efi,debug} \
 	      $(TEST_DIR)/.*.d $(TEST_DIR)/efi/.*.d lib/riscv/.*.d
diff --git a/s390x/Makefile b/s390x/Makefile
index 4c0c8085c..28d8a5b07 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -82,7 +82,7 @@ CFLAGS += -O2
 CFLAGS += -march=zEC12
 CFLAGS += -mbackchain
 CFLAGS += -fno-delete-null-pointer-checks
-LDFLAGS += -Wl,--build-id=none
+LDFLAGS += --build-id=none
 
 # We want to keep intermediate files
 .PRECIOUS: %.o %.lds
@@ -148,54 +148,65 @@ endif
 
 # the asm/c snippets %.o have additional generated files as dependencies
 $(SNIPPET_DIR)/asm/%.o: $(SNIPPET_DIR)/asm/%.S $(asm-offsets)
-	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+	$(call cmd,cc)
 
 $(SNIPPET_DIR)/c/%.o: $(SNIPPET_DIR)/c/%.c $(asm-offsets)
-	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
+	$(call cmd,cc)
 
+$(SNIPPET_DIR)/asm/%.gbin: LDFLAGS += -T $(SNIPPET_DIR)/asm/flat.lds
 $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(SNIPPET_DIR)/asm/flat.lds
-	$(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/asm/flat.lds $<
-	$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
+	$(call cmd,ld)
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
 	truncate -s '%4096' $@
 
+$(SNIPPET_DIR)/c/%.gbin: LDFLAGS += -T $(SNIPPET_DIR)/c/flat.lds
 $(SNIPPET_DIR)/c/%.gbin: $(SNIPPET_DIR)/c/%.o $(snippet_lib) $(FLATLIBS) $(SNIPPET_DIR)/c/flat.lds
-	$(CC) $(LDFLAGS) -o $@ -T $(SNIPPET_DIR)/c/flat.lds $< $(snippet_lib) $(FLATLIBS)
-	$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
-	truncate -s '%4096' $@
+	$(call cmd,ld)
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -O binary -j ".rodata" -j ".lowcore" -j ".text" -j ".data" -j ".bss" --set-section-flags .bss=alloc,load,contents $@ $@
+	$(Q)truncate -s '%4096' $@
 
 %.hdr: %.gbin $(HOST_KEY_DOCUMENT)
-	$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
+	$(call quiet_echo, " [SEHDR]   $@")
+	$(Q)$(GEN_SE_HEADER) -k $(HOST_KEY_DOCUMENT) -c $<,0x0,0x00000000000000420000000000000000 --psw-addr 0x4000 -o $@
 
 .SECONDARY:
 %.gobj: %.gbin
-	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
 
 .SECONDARY:
 %.hdr.obj: %.hdr
-	$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -I binary -O elf64-s390 -B "s390:64-bit" $< $@
 
 lds-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@
+%.lds: CPPFLAGS += $(lds-autodepend-flags)
 %.lds: %.lds.S $(asm-offsets)
-	$(CPP) $(lds-autodepend-flags) $(CPPFLAGS) -P -C -o $@ $<
+	$(call cmd,cpp)
 
+%.aux.o: CFLAGS += -DPROGNAME=\"$(@:.aux.o=.elf)\"
 $(tests:.elf=.aux.o): $(SRCDIR)/lib/auxinfo.c
-	$(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\"
+	$(call cmd,cc)
 
 .SECONDEXPANSION:
-%.elf: $(FLATLIBS) $(asmlib) $(SRCDIR)/s390x/flat.lds $$(snippets-obj) $$(snippet-hdr-obj) %.o %.aux.o
-	@$(CC) $(LDFLAGS) -o $@ -T $(SRCDIR)/s390x/flat.lds \
-		$(filter %.o, $^) $(FLATLIBS) $(snippets-obj) $(snippet-hdr-obj) || \
-		{ echo "Failure probably caused by missing definition of gen-se-header executable"; exit 1; }
-	@chmod a-x $@
+%.elf: LDFLAGS += -T $(SRCDIR)/s390x/flat.lds
+%.elf: $(SRCDIR)/s390x/flat.lds $$(snippets-obj) $$(snippet-hdr-obj) %.o %.aux.o $(FLATLIBS) $(asmlib)
+	$(call cmd,ld)
+# XXX: some test for gen-se-header executable?
+	$(Q)@chmod a-x $@
 
 # Secure Execution Customer Communication Key file
 # 32 bytes of key material, uses existing one if available
 comm-key = $(TEST_DIR)/comm.key
 $(comm-key):
-	dd if=/dev/urandom of=$@ bs=32 count=1 status=none
+	$(call quiet_echo, " [DD]      $@")
+	$(Q)dd if=/dev/urandom of=$@ bs=32 count=1 status=none
 
 %.bin: %.elf
-	$(OBJCOPY) -O binary  $< $@
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -O binary  $< $@
 
 # The genprotimg arguments for the cck changed over time so we need to
 # figure out which argument to use in order to set the cck
@@ -221,14 +232,14 @@ endif
 $(patsubst %.parmfile,%.pv.bin,$(wildcard s390x/*.parmfile)): %.pv.bin: %.parmfile
 %.pv.bin: %.bin $(HOST_KEY_DOCUMENT) $(comm-key)
 	$(eval parmfile_args = $(if $(filter %.parmfile,$^),--parmfile $(filter %.parmfile,$^),))
-	$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_OPTION) $(comm-key) --x-pcf $(GENPROTIMG_PCF) $(parmfile_args) --image $(filter %.bin,$^) -o $@
+	$(call quiet_echo, " [GENPROT] $@")
+	$(Q)$(GENPROTIMG) --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_OPTION) $(comm-key) --x-pcf $(GENPROTIMG_PCF) $(parmfile_args) --image $(filter %.bin,$^) -o $@
 
 $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
-	$(CC) $(CFLAGS) -c -nostdlib -o $@ $<
-
+	$(call cmd,cc)
 
 arch_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,elf,bin,lds} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr,lds} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d $(comm-key)
+	$(Q)$(RM) $(TEST_DIR)/*.{o,elf,bin,lds} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr,lds} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d $(comm-key)
 
 generated-files = $(asm-offsets)
 $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
diff --git a/scripts/asm-offsets.mak b/scripts/asm-offsets.mak
index 7b64162dd..db5f741c8 100644
--- a/scripts/asm-offsets.mak
+++ b/scripts/asm-offsets.mak
@@ -16,7 +16,7 @@ define sed-y
 endef
 
 define make_asm_offsets
-	(set -e; \
+	$(Q)(set -e; \
 	 echo "#ifndef __ASM_OFFSETS_H__"; \
 	 echo "#define __ASM_OFFSETS_H__"; \
 	 echo "/*"; \
@@ -29,16 +29,17 @@ define make_asm_offsets
 	 echo "#endif" ) > $@
 endef
 
+$(asm-offsets:.h=.s): CFLAGS += -fverbose-asm -S
 $(asm-offsets:.h=.s): $(asm-offsets:.h=.c)
-	$(CC) $(CFLAGS) -fverbose-asm -S -o $@ $<
+	$(call cmd,cc)
 
 $(asm-offsets): $(asm-offsets:.h=.s)
 	$(call make_asm_offsets)
-	cp -f $(asm-offsets) lib/generated/
+	$(Q)cp -f $(asm-offsets) lib/generated/
 
 OBJDIRS += lib/generated
 
 asm_offsets_clean:
-	$(RM) $(asm-offsets) $(asm-offsets:.h=.s) \
+	$(Q)$(RM) $(asm-offsets) $(asm-offsets:.h=.s) \
 	      $(addprefix lib/generated/,$(notdir $(asm-offsets)))
 
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 2318a85f0..3307c25b1 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -94,7 +94,7 @@ function mkstandalone()
 	generate_test "$@" > $standalone
 
 	chmod +x $standalone
-	echo Written $standalone.
+	echo " [WRITE]   $standalone"
 }
 
 if [ "$ENVIRON_DEFAULT" = "yes" ] && [ "$ERRATATXT" ] && [ ! -f "$ERRATATXT" ]; then
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 4ae9a5579..7896fb6c9 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -48,32 +48,33 @@ FLATLIBS = lib/libcflat.a
 ifeq ($(CONFIG_EFI),y)
 .PRECIOUS: %.efi %.so
 
-%.so: %.o $(FLATLIBS) $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(cstart.o)
-	$(LD) -T $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(EFI_LDFLAGS) -o $@ \
-		$(filter %.o, $^) $(FLATLIBS)
-	@chmod a-x $@
+%.so: LDFLAGS = $(EFI_LDFLAGS) -T $(SRCDIR)/x86/efi/elf_x86_64_efi.lds
+%.so: %.o $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(cstart.o) $(FLATLIBS)
+	$(call cmd,ld)
+	$(Q)@chmod a-x $@
 
 %.efi: %.so
-	$(OBJCOPY) --only-keep-debug $^ $@.debug
-	$(OBJCOPY) --strip-debug $^
-	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
-	$(OBJCOPY) \
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) --only-keep-debug $^ $@.debug
+	$(Q)$(OBJCOPY) --strip-debug $^
+	$(Q)$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
+	$(Q)$(OBJCOPY) \
 		-j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel \
 		-j .rela -j .reloc -S --target=$(FORMAT) $< $@
-	@chmod a-x $@
+	$(Q)chmod a-x $@
 else
 # We want to keep intermediate file: %.elf and %.o
 .PRECIOUS: %.elf %.o
 
-%.elf: LDFLAGS += $(arch_LDFLAGS)
-%.elf: %.o $(FLATLIBS) $(SRCDIR)/x86/flat.lds $(cstart.o)
-	$(LD) $(LDFLAGS) -T $(SRCDIR)/x86/flat.lds -o $@ \
-		$(filter %.o, $^) $(FLATLIBS)
-	@chmod a-x $@
+%.elf: LDFLAGS += $(arch_LDFLAGS) -T $(SRCDIR)/x86/flat.lds
+%.elf: %.o $(SRCDIR)/x86/flat.lds $(cstart.o) $(FLATLIBS)
+	$(call cmd,ld)
+	$(Q)@chmod a-x $@
 
 %.flat: %.elf
-	$(OBJCOPY) -O elf32-i386 $^ $@
-	@chmod a-x $@
+	$(call quiet_echo, " [OBJCOPY] $@")
+	$(Q)$(OBJCOPY) -O elf32-i386 $^ $@
+	$(Q)chmod a-x $@
 endif
 
 tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \
@@ -103,9 +104,9 @@ test_cases: $(tests-common) $(tests)
 
 $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/x86 -I lib
 
+$(TEST_DIR)/realmode.elf: LDFLAGS = -m elf_i386 -nostdlib -T $(SRCDIR)/$(TEST_DIR)/realmode.lds
 $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
-	$(LD) -m elf_i386 -nostdlib -o $@ \
-	      -T $(SRCDIR)/$(TEST_DIR)/realmode.lds $^
+	$(call cmd,ld)
 
 $(TEST_DIR)/realmode.o: bits = $(if $(call cc-option,-m16,""),16,32)
 
@@ -124,7 +125,7 @@ $(TEST_DIR)/hyperv_stimer.$(bin): $(TEST_DIR)/hyperv.o
 $(TEST_DIR)/hyperv_connections.$(bin): $(TEST_DIR)/hyperv.o
 
 arch_clean:
-	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
+	$(Q)$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
 	$(TEST_DIR)/.*.d lib/x86/.*.d \
 	$(TEST_DIR)/efi/*.o $(TEST_DIR)/efi/.*.d \
 	$(TEST_DIR)/*.so $(TEST_DIR)/*.efi $(TEST_DIR)/*.debug

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Thomas Huth]

On 05/06/2024 02.38, Nicholas Piggin wrote:
> On Mon Jun 3, 2024 at 6:56 PM AEST, Andrew Jones wrote:
>> On Mon, Jun 03, 2024 at 10:26:50AM GMT, Thomas Huth wrote:
>>> On 02/06/2024 14.25, Nicholas Piggin wrote:
>>>> Unless make V=1 is specified, silence make recipe echoing and print
>>>> an abbreviated line for major build steps.
>>>>
>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>> ---
>>>>    Makefile                | 14 ++++++++++++++
>>>>    arm/Makefile.common     |  7 +++++++
>>>>    powerpc/Makefile.common | 11 +++++++----
>>>>    riscv/Makefile          |  5 +++++
>>>>    s390x/Makefile          | 18 +++++++++++++++++-
>>>>    scripts/mkstandalone.sh |  2 +-
>>>>    x86/Makefile.common     |  5 +++++
>>>>    7 files changed, 56 insertions(+), 6 deletions(-)
>>>
>>> The short lines look superfluous in verbose mode, e.g.:
>>>
>>>   [OBJCOPY] s390x/memory-verify.bin
>>> objcopy -O binary  s390x/memory-verify.elf s390x/memory-verify.bin
>>>
>>> Could we somehow suppress the echo lines in verbose mode, please?
>>>
>>> For example in the SLOF project, it's done like this:
>>>
>>> https://gitlab.com/slof/slof/-/blob/master/make.rules?ref_type=heads#L48
>>>
>>> By putting the logic into $CC and friends, you also don't have to add
>>> "@echo" statements all over the place.
>>
>> And I presume make will treat the printing and compiling as one unit, so
>> parallel builds still get the summary above the error messages when
>> compilation fails. The way this patch is now a parallel build may show
>> the summary for the last successful build and then error messages for
>> a build that hasn't output its summary yet, which can be confusing.
>>
>> So I agree that something more like SLOF's approach would be better.
> 
> Hmm... kbuild type commands is a pretty big patch. I like it though.
> Thoughts?

Looks pretty complex to me ... do we really need this complexity in the 
k-u-t? If not, I think I'd rather prefer to go with a more simple approach 
like the one from SLOF.

  Thomas

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Nicholas Piggin]

On Wed Jun 12, 2024 at 8:32 PM AEST, Thomas Huth wrote:
> On 05/06/2024 02.38, Nicholas Piggin wrote:
> > On Mon Jun 3, 2024 at 6:56 PM AEST, Andrew Jones wrote:
> >> On Mon, Jun 03, 2024 at 10:26:50AM GMT, Thomas Huth wrote:
> >>> On 02/06/2024 14.25, Nicholas Piggin wrote:
> >>>> Unless make V=1 is specified, silence make recipe echoing and print
> >>>> an abbreviated line for major build steps.
> >>>>
> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>>> ---
> >>>>    Makefile                | 14 ++++++++++++++
> >>>>    arm/Makefile.common     |  7 +++++++
> >>>>    powerpc/Makefile.common | 11 +++++++----
> >>>>    riscv/Makefile          |  5 +++++
> >>>>    s390x/Makefile          | 18 +++++++++++++++++-
> >>>>    scripts/mkstandalone.sh |  2 +-
> >>>>    x86/Makefile.common     |  5 +++++
> >>>>    7 files changed, 56 insertions(+), 6 deletions(-)
> >>>
> >>> The short lines look superfluous in verbose mode, e.g.:
> >>>
> >>>   [OBJCOPY] s390x/memory-verify.bin
> >>> objcopy -O binary  s390x/memory-verify.elf s390x/memory-verify.bin
> >>>
> >>> Could we somehow suppress the echo lines in verbose mode, please?
> >>>
> >>> For example in the SLOF project, it's done like this:
> >>>
> >>> https://gitlab.com/slof/slof/-/blob/master/make.rules?ref_type=heads#L48
> >>>
> >>> By putting the logic into $CC and friends, you also don't have to add
> >>> "@echo" statements all over the place.
> >>
> >> And I presume make will treat the printing and compiling as one unit, so
> >> parallel builds still get the summary above the error messages when
> >> compilation fails. The way this patch is now a parallel build may show
> >> the summary for the last successful build and then error messages for
> >> a build that hasn't output its summary yet, which can be confusing.
> >>
> >> So I agree that something more like SLOF's approach would be better.
> > 
> > Hmm... kbuild type commands is a pretty big patch. I like it though.
> > Thoughts?
>
> Looks pretty complex to me ... do we really need this complexity in the 
> k-u-t? If not, I think I'd rather prefer to go with a more simple approach 
> like the one from SLOF.

The first patch I posted added silent to make too, but I don't love
it because it silences things that you missed or forgot about.

This way is loud by default and you have to adjust recipes to be
quiet. It caught a couple of things I missed the first time around.
I think that's a long term advantage for more short term churn.

Thanks,
Nick

Re: [kvm-unit-tests PATCH 3/4] build: Make build output pretty

[Nicholas Piggin]

On Mon Jun 3, 2024 at 6:26 PM AEST, Thomas Huth wrote:
> On 02/06/2024 14.25, Nicholas Piggin wrote:
> > Unless make V=1 is specified, silence make recipe echoing and print
> > an abbreviated line for major build steps.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   Makefile                | 14 ++++++++++++++
> >   arm/Makefile.common     |  7 +++++++
> >   powerpc/Makefile.common | 11 +++++++----
> >   riscv/Makefile          |  5 +++++
> >   s390x/Makefile          | 18 +++++++++++++++++-
> >   scripts/mkstandalone.sh |  2 +-
> >   x86/Makefile.common     |  5 +++++
> >   7 files changed, 56 insertions(+), 6 deletions(-)
>
> The short lines look superfluous in verbose mode, e.g.:
>
>   [OBJCOPY] s390x/memory-verify.bin
> objcopy -O binary  s390x/memory-verify.elf s390x/memory-verify.bin
>
> Could we somehow suppress the echo lines in verbose mode, please?
>
> For example in the SLOF project, it's done like this:
>
> https://gitlab.com/slof/slof/-/blob/master/make.rules?ref_type=heads#L48
>
> By putting the logic into $CC and friends, you also don't have to add 
> "@echo" statements all over the place.

I'll could try a bit harder at it, this was a pretty quick hack.

I probably prefer the cmd_cc style that Linux uses rather than
overloading CC. But maybe that's more work.

Thanks,
Nick

[kvm-unit-tests PATCH 4/4] gitlab-ci: Always save artifacts

[Nicholas Piggin]

The unit test logs are important to have when a test fails so
mark these as always save.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 .gitlab-ci.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 23bb69e24..c58dcc46c 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -4,14 +4,19 @@ before_script:
  - dnf update -y
  - dnf install -y make python
 
+# Always save logs even for build failure, because the tests are actually
+# run as part of the test step (because there is little need for an
+# additional build step.
 .intree_template:
  artifacts:
+  when: always
   expire_in: 2 days
   paths:
    - logs
 
 .outoftree_template:
  artifacts:
+  when: always
   expire_in: 2 days
   paths:
    - build/logs
-- 
2.43.0

Re: [kvm-unit-tests PATCH 4/4] gitlab-ci: Always save artifacts

[Thomas Huth]

On 02/06/2024 14.25, Nicholas Piggin wrote:
> The unit test logs are important to have when a test fails so
> mark these as always save.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   .gitlab-ci.yml | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 23bb69e24..c58dcc46c 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -4,14 +4,19 @@ before_script:
>    - dnf update -y
>    - dnf install -y make python
>   
> +# Always save logs even for build failure, because the tests are actually
> +# run as part of the test step (because there is little need for an
> +# additional build step.
>   .intree_template:
>    artifacts:
> +  when: always
>     expire_in: 2 days
>     paths:
>      - logs
>   
>   .outoftree_template:
>    artifacts:
> +  when: always
>     expire_in: 2 days
>     paths:
>      - build/logs

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 4/4] gitlab-ci: Always save artifacts

[Nicholas Piggin]

On Mon Jun 3, 2024 at 2:29 PM AEST, Thomas Huth wrote:
> On 02/06/2024 14.25, Nicholas Piggin wrote:
> > The unit test logs are important to have when a test fails so
> > mark these as always save.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   .gitlab-ci.yml | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 23bb69e24..c58dcc46c 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -4,14 +4,19 @@ before_script:
> >    - dnf update -y
> >    - dnf install -y make python
> >   
> > +# Always save logs even for build failure, because the tests are actually
> > +# run as part of the test step (because there is little need for an
> > +# additional build step.

This wording is a bit scrambled too, I wrote it thinking CI was using
build step by default. It seems to be test so changing the words around
didn't work (also I'll fix the closing paren). I'll fix that up.

Thanks,
Nick

> >   .intree_template:
> >    artifacts:
> > +  when: always
> >     expire_in: 2 days
> >     paths:
> >      - logs
> >   
> >   .outoftree_template:
> >    artifacts:
> > +  when: always
> >     expire_in: 2 days
> >     paths:
> >      - build/logs
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>