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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

This trick only works for trivial cases, and interferes with diff's when
the logic inevitably becomes less trivial.  LTO support in particular
needs a complete overhaul, but there is no way I'm going to touch that
with a barge pole until this series is in place.

~Andrew