Re: [PATCH] build: adjust include/xen/compile.h generation

[Anthony PERARD <anthony.perard@citrix.com>]

On Tue, Jan 11, 2022 at 03:16:17PM +0100, Jan Beulich wrote:
> Prior to 19427e439e01 ("build: generate "include/xen/compile.h" with
> if_changed") running "make install-xen" as root would not have printed
> the banner under normal circumstances. Its printing would instead have
> indicated that something was wrong (or during a normal build the lack
> of printing would do so).

So, having several line of logs with one generating "compile.h", and
several object rebuild plus the re-linking of xen isn't enough has to
indicate that something is wrong?

Also, with this patch, the banner would no longer be printed on rebuild,
unless one doesn't try to prevent regeneration of "compile.h" by
exporting two variables.

I kind of like having the banner been nearly always printed, but I'm not
opposed to the change.

> Further aforementioned change had another undesirable effect, which I
> didn't notice during review: Originally compile.h would have been
> re-generated (and final binaries re-linked) when its dependencies were
> updated after an earlier build. This is no longer the case now, which
> means that if some other file also was updated, then the re-build done
> during "make install-xen" would happen with a stale compile.h (as its
> updating is suppressed in this case).

So, the comment:
    Don't refresh this files during e.g., 'sudo make install'
wasn't correct?

On the other hand, it's probably not good to not regen the file when the
prerequisite changes. It's kind of weird to "rm" the target, but I don't
have a better solution at the moment.

> Restore the earlier behavior for both aspects.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> An alternative to removing the target would be to replace "! -r $@" by
> "-n '$?'" in the shell "if", but that would cause unhelpful alteration
> of what gets recorded in include/xen/.compile.h.cmd. (The command
> normally changes every time anyway, due to the inclusion of
> $(XEN_BUILD_TIME), but I consider that different from varying the
> conditions of the "if".)
> 
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -424,6 +424,7 @@ targets += .banner
>  quiet_cmd_compile.h = UPD     $@
>  define cmd_compile.h
>      if [ ! -r $@ -o -O $@ ]; then \
> +	cat .banner; \
>  	sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
>  	    -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
>  	    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
> @@ -441,7 +442,7 @@ define cmd_compile.h
>  endef
>  
>  include/xen/compile.h: include/xen/compile.h.in .banner FORCE
> -	@cat .banner
> +	$(if $(filter-out FORCE,$?),rm -fv $@)

Is there a reason for -v? Do we care if the file existed?

Do we want to log "rm -f compile.h" ? Or could you just prefix the line
with $(Q)?

Thanks,

-- 
Anthony PERARD