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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.02.2020 13:17, Anthony PERARD wrote:
> 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.

I think this is better, yes, albeit I'm still a little unhappy
about the uses of "target" here: It makes it look to me as if
this was primarily about things like

abc.o: CFLAGS += ...

where, unless its rule invokes make, I don't think the adjustment
would propagate anywhere. Instead aiui what you refer to is solely
an effect from the subdir traversal the build system does?

>>> @@ -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.

Apparently a matter of taste then. I generally consider trailing
backslashes a little "unclean", and hence would prefer to avoid
them if I can.

>>> --- 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.

Thank you.

Jan

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