Re: [PATCH] build: correct cppcheck-misra make rule

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jan,

> On 9 Sep 2022, at 15:26, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 09.09.2022 15:50, Bertrand Marquis wrote:
>>> On 9 Sep 2022, at 14:41, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> 
>>> It has been bothering me for a while that I made a bad suggestion during
>> 
>> This is not a sentence for a commit message.
> 
> How else should I express the motivation for the change?

I would say, start with “cppcheck-misra.json depend on …” and remove everything 
before.

Being bothered is not really something interesting to read in the git log.

> 
>>> review: Having cppcheck-misra.json depend on cppcheck-misra.txt does not
>>> properly address the multiple targets problem. If cppcheck-misra.json
>>> is deleted from the build tree but cppcheck-misra.txt is still there,
>>> nothing will re-generate cppcheck-misra.json.
>>> 
>>> With GNU make 4.3 or newer we could use the &: grouped target separator,
>>> but since we support older make as well we need to use some other
>>> mechanism. Convert the rule to a pattern one (with "cppcheck"
>>> arbitrarily chosen as the stem), thus making known to make that both
>>> files are created by a single command invocation. Since, as a result,
>>> the JSON file is now "intermediate" from make's perspective, prevent it
>>> being deleted again by making it a prereq of .PRECIOUS.
>>> 
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> ---
>>> I've not been able to spot where / how cppcheck-misra.txt is used. If
>>> it's indeed unused, a perhaps better alternative would be to convert the
>>> original rule to specify cppcheck-misra.json as (the only) target. One
>>> might then even consider using "-o /dev/null" instead of producing an
>>> unused *.txt file.
>> 
>> Txt file is used by cppcheck to give a text description of the rule.
>> If you look inside the json content you will see it mentioned.
> 
> Oh, that's properly hidden then.

This is how cppcheck needs it and why I added a comment but it might needs 
improving.

> 
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -746,11 +746,9 @@ cppcheck-version:
>>> # documentation file. Also generate a json file with the right arguments for
>>> # cppcheck in json format including the list of rules to ignore.
>>> #
>>> -cppcheck-misra.txt: $(XEN_ROOT)/docs/misra/rules.rst 
>>> $(srctree)/tools/convert_misra_doc.py
>>> -   $(Q)$(PYTHON) $(srctree)/tools/convert_misra_doc.py -i $< -o $@ -j 
>>> $(@:.txt=.json)
>>> -
>>> -# convert_misra_doc is generating both files.
>>> -cppcheck-misra.json: cppcheck-misra.txt
>>> +.PRECIOUS: %-misra.json
>>> +%-misra.txt %-misra.json: $(XEN_ROOT)/docs/misra/rules.rst 
>>> $(srctree)/tools/convert_misra_doc.py
>>> +   $(Q)$(PYTHON) $(srctree)/tools/convert_misra_doc.py -i $< -o 
>>> $*-misra.txt -j $*-misra.json
>> 
>> As far as I know, this is not saying to make that both files are generated 
>> by this rule,
>> but that this rule can generate both files so nothing is telling make here 
>> that calling
>> it once is enough I think.
> 
> As said in the description - it specifically has this effect. We're
> using this elsewhere already, see e.g. tools/libs/light/Makefile
> generating three headers and a C file all in one go. Iirc this is
> also explicitly described in make documentation (and contrasted to
> the different behavior for non-pattern rules).

Then I think the comment suggested by Anthony makes sense to add.

Cheers
Bertrand

> 
> Jan