Re: [XEN PATCH v7 16/51] build: generate "include/xen/compile.h" with if_changed

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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,

-- 
Anthony PERARD