Re: [PATCH v2] build: Fix make warning if there is no cppcheck

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.05.2022 15:23, Bertrand Marquis wrote:
>> On 20 May 2022, at 13:51, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 20.05.2022 14:14, Bertrand Marquis wrote:
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -694,12 +694,14 @@ $(objtree)/%.c.cppcheck: $(srctree)/%.c 
>>> $(objtree)/include/generated/autoconf.h
>>>     $(call if_changed,cppcheck_xml)
>>>
>>> cppcheck-version:
>>> -ifeq ($(shell which $(CPPCHECK)),)
>>> -   $(error Cannot find cppcheck executable: $(CPPCHECK))
>>> -endif
>>> -ifeq ($(shell $(CPPCHECK) --version | awk '{print ($$2 < 2.7)}'),1)
>>> -   $(error Please upgrade your cppcheck to version 2.7 or greater)
>>> -endif
>>> +   @if ! which $(CPPCHECK) > /dev/null 2>&1; then \
>>> +           echo "Cannot find cppcheck executable: $(CPPCHECK)"; \
>>> +           exit 1; \
>>> +   fi
>>> +   @if [ "$$($(CPPCHECK) --version | awk '{print ($$2 < 2.7)}')" -eq 1 ]; 
>>> then \
>>> +           echo "Please upgrade your cppcheck to version 2.7 or greater"; \
>>> +           exit 1; \
>>> +   fi
>>>
>>> # Put this in generated headers this way it is cleaned by include/Makefile
>>> $(objtree)/include/generated/compiler-def.h:
>>
>> Fine with me, even if - as said on v1 - I would have preferred $(if ...).
> 
> Could you explain why and what you mean exactly ?

I generally think that make scripts should resort to shell language
only if things cannot reasonably be expressed in make language.

> I thought the code would be more complex and less clear using if and I
> do not see how it would solve the issue with which being called.

The problem to deal with was to move the shell invocation from
makefile parsing time to rule execution time. Hence I don't see
why

cppcheck-version:
        $(if $(shell which ...),,$(error ...))

wouldn't deal with the problem equally well. But I guess I may
not be understanding your question / concern.

>> One question though: Wouldn't it better be $(Q) instead of the two plain
>> @? Preferably with that adjustment (which I guess can be made while
>> committing):
> 
> I thought of it but who would be interested in actually seeing those
> commands which are not “building” anything.

You never know what's relevant to see when hunting down some
obscure build system issue.

Jan