Re: [Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Jan 30, 2020 at 02:29:42PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > We would like to calculate CFLAGS once and before calling Rules.mk,
> > so the variable CFLAGS needs to have the same value across the whole
> > build. Thus we need a new variable where some flags can change
> > depending on the target name.
> > 
> > Both the dependency and __OBJECT_FILE__ are such flags that change
> > depending on the target, so there are move out of $(CFLAGS).
> 
> I'm afraid I don't understand: Being a delayed expansion (or
> "recursively expanded") variable, what problem is there when CFLAGS
> references $@? Is there a plan to change the variable's flavor? If
> so, I'd like to ask for this to be mentioned here. "Calculate once",
> at least to me, doesn't imply this.

If I rewrite the first paragraph thus, would that be better?

    In a later patch, we want to calculate the CFLAGS in xen/Makefile,
    then export it. So have Rules.mk use a CFLAGS from the environment
    variables. This will mean that if Rules.mk or a Makefile modify
    CFLAGS for a target, the modification propagates to other targets.
    Thus we will need a different variable name than the one from the
    environment which can have a different value for each target.


> > @@ -141,9 +137,16 @@ endif
> >  # Always build obj-bin files as binary even if they come from C source. 
> >  $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
> >  
> > +c_flags = -MMD -MF $(@D)/.$(@F).d \
> > +          $(CFLAGS) \
> > +          '-D__OBJECT_FILE__="$@"'
> > +
> > +a_flags = -MMD -MF $(@D)/.$(@F).d \
> > +          $(AFLAGS)
> 
> Is there a reason both get extended over multiple lines?

Beside that it looks cleaner to me, not really.

> > --- a/xen/include/Makefile
> > +++ b/xen/include/Makefile
> > @@ -64,7 +64,7 @@ compat/%.h: compat/%.i Makefile 
> > $(BASEDIR)/tools/compat-build-header.py
> >     mv -f $@.new $@
> >  
> >  compat/%.i: compat/%.c Makefile
> > -   $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include 
> > %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
> > +   $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include 
> > %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<
> 
> I think this wants to continue to reference $(CFLAGS) and instead have
> the -M% and %.d patterns dropped. Similarly I guess as-insn in Config.mk
> could then have these two patterns dropped.

It's probably a good idea to keep using CFLAGS, I'll look into it.
As to change as-insn, I can move it out of Config.mk and then change it.
I'll look into that as well.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel