Re: [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig"

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.09.2021 11:46, Anthony PERARD wrote:
> On Thu, Sep 16, 2021 at 05:34:00PM +0200, Jan Beulich wrote:
>> On 08.09.2021 13:17, Anthony PERARD wrote:
>>> @@ -189,14 +193,24 @@ ifeq ($(config-build),y)
>>>  # *config targets only - make sure prerequisites are updated, and descend
>>>  # in tools/kconfig to make the *config target
>>>  
>>> +# Create a file for KCONFIG_ALLCONFIG which depends on the environment.
>>> +# This will be use by kconfig targets 
>>> allyesconfig/allmodconfig/allnoconfig/randconfig
>>> +filechk_kconfig_allconfig = \
>>> +    $(if $(findstring n,$(HAS_CHECKPOLICY)),echo 
>>> 'CONFIG_XSM_FLASK_POLICY=n';) \
>>> +    $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG), :)
>>
>> Nit: It would be nice if you were consistent with the blanks after
>> commas in $(if ...). Personally I'm also considering $(if ...)s the
>> more difficult to follow the longer they are. Hence for the 2nd one
>> I wonder whether
>>
>>     $(if $(KCONFIG_ALLCONFIG),cat,:) $(KCONFIG_ALLCONFIG)
>>
>> wouldn't be easier to read.
> 
> How about:
> 
>     $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
>     :
> 
> .. instead, as that would be more consistent with the previous line,
> that is there would be only one branch to the $(if ) and no else, and
> thus probably easier to read.

Oh, sure, even better if that works.

>>> +.allconfig.tmp: FORCE
>>> +   set -e; { $(call filechk_kconfig_allconfig); } > $@
>>
>> Is there a particular reason for the .tmp suffix?
> 
> Yes, .*.tmp are already ignored via .gitignore.

I see. Could you add two words to the description saying so? Or
maybe even just a post-commit-message remark would do.

Jan