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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Jan 13, 2022 at 01:11:41PM +0100, Jan Beulich wrote:
> On 13.01.2022 12:22, Anthony PERARD wrote:
> > 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?
> 
> Well, for warnings and errors to be easy to spot (and until your rework
> to make our build more Linux-like is in place) passing -s to make is a
> must, imo.

I see, I guess keeping that behavior is kind of useful (banner been
printed when compile.h is regenerated).

> >> +  $(if $(filter-out FORCE,$?),rm -fv $@)
> > 
> > Is there a reason for -v? Do we care if the file existed?
> 
> That's meant to be an indication of the file getting updated during
> "install-xen" as root. I thought it might be nice to have this extra
> indicator, but I wouldn't mind dropping it if that helps acceptance
> of the change. Can you let me know how important this aspect is to
> you?

I guess it is fine to keep the '-v'. It isn't like it is something that
will happen very often.

> > Do we want to log "rm -f compile.h" ? Or could you just prefix the line
> > with $(Q)?
> 
> I'll add $(Q). As said, I always build with "make -s" (except when
> debugging weird build issues), so this is nothing I would have noticed.

With $(Q) added:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD