Re: [XEN PATCH v4 10/18] xen/build: use if_changed on built_in.o

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.04.2020 15:13, Andrew Cooper wrote:
> On 08/04/2020 13:40, Jan Beulich wrote:
>> On 31.03.2020 12:30, Anthony PERARD wrote:
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
>>>  c_flags += $(CFLAGS-y)
>>>  a_flags += $(CFLAGS-y) $(AFLAGS-y)
>>>  
>>> -built_in.o: $(obj-y) $(extra-y)
>>> -ifeq ($(obj-y),)
>>> -   $(CC) $(c_flags) -c -x c /dev/null -o $@
>>> -else
>>> +quiet_cmd_ld_builtin = LD      $@
>>>  ifeq ($(CONFIG_LTO),y)
>>> -   $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
>>> +cmd_ld_builtin = \
>>> +    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>>>  else
>>> -   $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
>>> +cmd_ld_builtin = \
>>> +    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>>>  endif
>> How about going yet one step further and doing away with the
>> ifeq here altogether:
>>
>> cmd_ld_builtin-y = \
>>     $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>> cmd_ld_builtin-$(CONFIG_LTO) = \
>>     $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
> 
> Please don't.
> 
> Logic like this is substantially harder to follow than a plain if/else
> construct, and clarity is of far higher importance than optimising the
> line count in the build system.

I could maybe see the argument if the two definitions were far apart.
This suggestion isn't about line count at all, but about clarity. In
particular because of the need to use ifeq(,) rather than simple "if"
constructs, I view this list model as the better alternative in all
cases where it can be made use of.

> This trick only works for trivial cases, and interferes with diff's when
> the logic inevitably becomes less trivial.

It may, but whether it actually will can't be known until such time
as it would get touched. The suggested model may very well also be
suitable then.

Anyway, Anthony, I'm not going to insist. This is just another aspect
where we would better generally settle on the preferred style to use.

Jan