| Message ID | 20210824105038.1257926-17-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
On 24.08.2021 12:50, Anthony PERARD wrote: > --- a/.gitignore > +++ b/.gitignore > @@ -332,7 +332,6 @@ xen/include/compat/* > xen/include/config/ > xen/include/generated/ > xen/include/public/public > -xen/include/xen/*.new While this indeed looks to only have been here for compile.h, I'm not convinced it is a good idea to delete the entry here. Does it cause any harm if left in place? > --- 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 arch/*/include/asm/asm-offsets.h > - rm -f .banner > + rm -f .banner include/xen/compile.h Isn't this redundant with ... > @@ -425,10 +419,16 @@ include/xen/compile.h: include/xen/compile.h.in .banner > -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 > + < $< > $(dot-target).tmp; \ > + sed -rf tools/process-banner.sed < .banner >> $(dot-target).tmp; \ > + mv -f $(dot-target).tmp $@; \ > + 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 if_changed,compile.h) > +targets += include/xen/compile.h ... this? I would have hoped that $(targets) is included in the generic cleaning logic ... Jan
On Thu, Oct 07, 2021 at 04:55:01PM +0200, Jan Beulich wrote: > On 24.08.2021 12:50, Anthony PERARD wrote: > > --- a/.gitignore > > +++ b/.gitignore > > @@ -332,7 +332,6 @@ xen/include/compat/* > > xen/include/config/ > > xen/include/generated/ > > xen/include/public/public > > -xen/include/xen/*.new > > While this indeed looks to only have been here for compile.h, I'm > not convinced it is a good idea to delete the entry here. Does it > cause any harm if left in place? That's a complicated question. I would prefer to have in this file only artefacts of the build system but other developer and maintainer disagree. So it's fine I guess to leave the entry, it just hide any *.new file from `git status` and make it harder to commit them. > > --- a/xen/Makefile > > +++ b/xen/Makefile > > @@ -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 arch/*/include/asm/asm-offsets.h > > - rm -f .banner > > + rm -f .banner include/xen/compile.h > > Isn't this redundant with ... > > > @@ -425,10 +419,16 @@ include/xen/compile.h: include/xen/compile.h.in .banner > > + > > +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 if_changed,compile.h) > > +targets += include/xen/compile.h > > ... this? I would have hoped that $(targets) is included in the > generic cleaning logic ... Not yet. It's probably a good idea, I'll work on a patch. Thanks,
diff --git a/.gitignore b/.gitignore index bc964663d25c..59a22d1685e2 100644 --- a/.gitignore +++ b/.gitignore @@ -332,7 +332,6 @@ xen/include/compat/* xen/include/config/ xen/include/generated/ xen/include/public/public -xen/include/xen/*.new xen/include/xen/acm_policy.h xen/include/xen/compile.h xen/include/xen/lib/x86/cpuid-autogen.h diff --git a/xen/Makefile b/xen/Makefile index da1b8ddb97ff..b408d35b4af0 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 arch/*/include/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 arch/$(TARGET_ARCH)/include/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,9 +405,11 @@ 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' \ +# Don't refresh this files during e.g., 'sudo make install' +quiet_cmd_compile.h = UPD $@ +define cmd_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' \ @@ -425,10 +419,16 @@ include/xen/compile.h: include/xen/compile.h.in .banner -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 + < $< > $(dot-target).tmp; \ + sed -rf tools/process-banner.sed < .banner >> $(dot-target).tmp; \ + mv -f $(dot-target).tmp $@; \ + 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 if_changed,compile.h) +targets += include/xen/compile.h asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c $(CC) $(call cpp_flags,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
This will avoid regenerating "compile.h" if the content hasn't changed. 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". Use "$(dot-target).tmp" as temporary file as this is already cover by ".*.tmp" partern in ".gitconfig". Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v7: - Use $(if_changed,) instead of importing a new macro from Linux (filechk). - use $(dot-target).tmp as temporary file, that way is hiden, and already cover by .gitignore via ".*.tmp". (filechk was doing the same) - update .gitignore. .gitignore | 1 - xen/Makefile | 34 +++++++++++++++++----------------- 2 files changed, 17 insertions(+), 18 deletions(-)