Hi Jan, > On 20 May 2022, at 14:56, Jan Beulich <[email protected]> wrote: > > On 20.05.2022 15:23, Bertrand Marquis wrote: >>> On 20 May 2022, at 13:51, Jan Beulich <[email protected]> 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.
Agree hence my first implementation. > >> 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. There are always thousands of ways to achieve the same and here this is only a matter of taste. I must admit that I did not think of using that solution this way. If you prefer this I have nothing against it and I will ack a patch changing to this. > >>> 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. > Feel free to replace @ by $(Q) in my patch on commit. Cheers Bertrand > Jan >
