diff mbox series

[XEN,v6,20/31] build: generate "include/xen/compile.h" with filechk

Message ID 20210701141011.785641-21-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen: Build system improvements | expand

Commit Message

Anthony PERARD July 1, 2021, 2:10 p.m. UTC
This will always try regenerate the content of compile.h, but if it
didn't change the file isn't updated.

Also, as it's currently the case, the file isn't regenerated during
`sudo make install` if it exist and does belong to a different user.

Thus, we can remove the target "delete-unfresh-files".
Target $(TARGET) still need a phony dependency, so add FORCE.

This patch imports the macro 'filechk' from Linux v5.12.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile               | 51 +++++++++++++++++++-------------------
 xen/scripts/Kbuild.include | 31 +++++++++++++++++++++++
 2 files changed, 56 insertions(+), 26 deletions(-)

Comments

Jan Beulich Aug. 5, 2021, 7:20 a.m. UTC | #1
On 01.07.2021 16:10, Anthony PERARD wrote:
> This will always try regenerate the content of compile.h, but if it
> didn't change the file isn't updated.
> 
> Also, as it's currently the case, the file isn't regenerated during
> `sudo make install` if it exist and does belong to a different user.
> 
> Thus, we can remove the target "delete-unfresh-files".
> Target $(TARGET) still need a phony dependency, so add FORCE.
> 
> This patch imports the macro 'filechk' from Linux v5.12.

Would you mind clarifying why $(if_changed ...) cannot be used here
(unlike for .banner in the earlier patch)?

> @@ -413,22 +405,29 @@ endef
>  	$(call if_changed,banner)
>  targets += .banner
>  
> -# compile.h contains dynamic build info. Rebuilt on every 'make' invocation.
> -include/xen/compile.h: include/xen/compile.h.in .banner
> -	@sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
> -	    -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
> -	    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
> -	    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
> -	    -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
> -	    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
> -	    -e 's/@@version@@/$(XEN_VERSION)/g' \
> -	    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
> -	    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
> -	    -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) || echo "unavailable")!g' \
> -	    < include/xen/compile.h.in > $@.new
> +# Don't refresh this files during e.g., 'sudo make install'
> +define filechk_compile.h
> +    if [ ! -r $@ -o -O $@ ]; then \
> +    sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
> +        -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
> +        -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
> +        -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
> +        -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
> +        -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
> +        -e 's/@@version@@/$(XEN_VERSION)/g' \
> +        -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
> +        -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
> +        -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) || echo "unavailable")!g' \
> +	< $<; \
> +    sed -rf tools/process-banner.sed < .banner; \
> +    else \
> +	cat $@; \
> +    fi

Just like "cat" I think the "sed" invocations should be indented deeper
than if/else/fi.

Jan
Anthony PERARD Aug. 9, 2021, 2:27 p.m. UTC | #2
On Thu, Aug 05, 2021 at 09:20:10AM +0200, Jan Beulich wrote:
> On 01.07.2021 16:10, Anthony PERARD wrote:
> > This will always try regenerate the content of compile.h, but if it
> > didn't change the file isn't updated.
> > 
> > Also, as it's currently the case, the file isn't regenerated during
> > `sudo make install` if it exist and does belong to a different user.
> > 
> > Thus, we can remove the target "delete-unfresh-files".
> > Target $(TARGET) still need a phony dependency, so add FORCE.
> > 
> > This patch imports the macro 'filechk' from Linux v5.12.
> 
> Would you mind clarifying why $(if_changed ...) cannot be used here
> (unlike for .banner in the earlier patch)?

if_changed can be used instead of filechk. I probably use "filechk"
because I was looking for an excuse to use it, so I've used it here.

filechk advantage over if_changed is that the output of the command is
compared so there is no need to have an extra dependency (.*.cmd) file
generated. That probably mostly an advantage when the generated file
changed often, or when the command is simple enough.

But it seems that "filechk" is only used once in this patch series, in
this patch. So I can rework the patch to use "if_changed" instead, that
would avoid the need to import another macro from Linux, and avoid the
weird need to have the command "cat" the target when update isn't
wanted.

Thanks,
diff mbox series

Patch

diff --git a/xen/Makefile b/xen/Makefile
index 267ae77aef7a..4c4990a753df 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -351,7 +351,7 @@  _debug:
 	$(OBJDUMP) -D -S $(TARGET)-syms > $(TARGET).s
 
 .PHONY: _clean
-_clean: delete-unfresh-files
+_clean:
 	$(MAKE) -C tools clean
 	$(MAKE) $(clean) include
 	$(MAKE) $(clean) common
@@ -368,7 +368,7 @@  _clean: delete-unfresh-files
 		-o -name "*.gcno" -o -name ".*.cmd" -o -name "lib.a" \) -exec rm -f {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
 	rm -f asm-offsets.s include/arch-*/asm/asm-offsets.h
-	rm -f .banner
+	rm -f .banner include/xen/compile.h
 
 .PHONY: _distclean
 _distclean: clean
@@ -378,7 +378,7 @@  $(TARGET).gz: $(TARGET)
 	gzip -n -f -9 < $< > $@.new
 	mv $@.new $@
 
-$(TARGET): delete-unfresh-files
+$(TARGET): FORCE
 	$(MAKE) -C tools
 	$(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
 	[ -e arch/$(TARGET_ARCH)/efi ] && for f in $$(cd common/efi; echo *.[ch]); \
@@ -391,14 +391,6 @@  $(TARGET): delete-unfresh-files
 	$(MAKE) -f $(BASEDIR)/Rules.mk include/arch-$(TARGET_ARCH)/asm/asm-offsets.h
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@
 
-# drivers/char/console.o contains static banner/compile info. Blow it away.
-# Don't refresh these files during e.g., 'sudo make install'
-.PHONY: delete-unfresh-files
-delete-unfresh-files:
-	@if [ ! -r include/xen/compile.h -o -O include/xen/compile.h ]; then \
-		rm -f include/xen/compile.h; \
-	fi
-
 quiet_cmd_banner = BANNER  $@
 define cmd_banner
     if which figlet >/dev/null 2>&1 ; then \
@@ -413,22 +405,29 @@  endef
 	$(call if_changed,banner)
 targets += .banner
 
-# compile.h contains dynamic build info. Rebuilt on every 'make' invocation.
-include/xen/compile.h: include/xen/compile.h.in .banner
-	@sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
-	    -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
-	    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
-	    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
-	    -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
-	    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
-	    -e 's/@@version@@/$(XEN_VERSION)/g' \
-	    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
-	    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
-	    -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) || echo "unavailable")!g' \
-	    < include/xen/compile.h.in > $@.new
+# Don't refresh this files during e.g., 'sudo make install'
+define filechk_compile.h
+    if [ ! -r $@ -o -O $@ ]; then \
+    sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
+        -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
+        -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
+        -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
+        -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
+        -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
+        -e 's/@@version@@/$(XEN_VERSION)/g' \
+        -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
+        -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
+        -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) || echo "unavailable")!g' \
+	< $<; \
+    sed -rf tools/process-banner.sed < .banner; \
+    else \
+	cat $@; \
+    fi
+endef
+
+include/xen/compile.h: include/xen/compile.h.in .banner FORCE
 	@cat .banner
-	@sed -rf tools/process-banner.sed < .banner >> $@.new
-	@mv -f $@.new $@
+	$(call filechk,compile.h)
 
 asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
 	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index 83c7e1457baa..838c9440f35e 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -26,6 +26,37 @@  real-prereqs = $(filter-out $(PHONY), $^)
 # Escape single quote for use in echo statements
 escsq = $(subst $(squote),'\$(squote)',$1)
 
+###
+# Easy method for doing a status message
+       kecho := :
+ quiet_kecho := echo
+silent_kecho := :
+kecho := $($(quiet)kecho)
+
+###
+# filechk is used to check if the content of a generated file is updated.
+# Sample usage:
+#
+# filechk_sample = echo $(KERNELRELEASE)
+# version.h: FORCE
+#	$(call filechk,sample)
+#
+# The rule defined shall write to stdout the content of the new file.
+# The existing file will be compared with the new one.
+# - If no file exist it is created
+# - If the content differ the new file is used
+# - If they are equal no change, and no timestamp update
+define filechk
+	$(Q)set -e;						\
+	mkdir -p $(dir $@);					\
+	trap "rm -f $(dot-target).tmp" EXIT;			\
+	{ $(filechk_$(1)); } > $(dot-target).tmp;		\
+	if [ ! -r $@ ] || ! cmp -s $@ $(dot-target).tmp; then	\
+		$(kecho) '  UPD     $@';			\
+		mv -f $(dot-target).tmp $@;			\
+	fi
+endef
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \