Re: [RFC PATCH 1/4] xen/Makefile: add analysis-coverity and analysis-eclair

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.11.2022 21:52, Stefano Stabellini wrote:
> On Fri, 11 Nov 2022, Jan Beulich wrote:
>> On 11.11.2022 11:42, Luca Fancellu wrote:
>>>> On 9 Nov 2022, at 10:36, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 09.11.2022 11:08, Luca Fancellu wrote:
>>>>>>> On 07.11.2022 11:47, Luca Fancellu wrote:
>>>>>>> Now analysis-build-coverity will be called, the best match is 
>>>>>>> analysis-build-%, so again the dependency
>>>>>>> which is analysis-parse-tags-%, will be translated to 
>>>>>>> analysis-parse-tags-coverity.
>>>>>>>
>>>>>>> Now analysis-parse-tags-coverity will be called, the best match is 
>>>>>>> analysis-parse-tags-%, so the % will
>>>>>>> Have the ‘coverity’ value and in the dependency we will have 
>>>>>>> $(objtree)/%.sed -> $(objtree)/coverity.sed.
>>>>>>>
>>>>>>> Looking for $(objtree)/coverity.sed the best match is $(objtree)/%.sed, 
>>>>>>> which will have $(JUSTIFICATION_FILES)
>>>>>>> and the python script in the dependency, here we will use the second 
>>>>>>> expansion to solve
>>>>>>> $(XEN_ROOT)/docs/misra/false-positive-$$*.json in 
>>>>>>> $(XEN_ROOT)/docs/misra/false-positive-coverity.json
>>>>>>>
>>>>>>> So now after analysis-parse-tags-coverity has ended its dependency it 
>>>>>>> will start with its recipe, after it finishes,
>>>>>>> the recipe of analysis-build-coverity will start and it will call make 
>>>>>>> to actually build Xen.
>>>>>>
>>>>>> Okay, I see now - this building of Xen really _is_ independent of the
>>>>>> checker chosen. I'm not sure though whether it is a good idea to
>>>>>> integrate all this, including ...
>>>>>>
>>>>>>> After the build finishes, if the status is good, the 
>>>>>>> analysis-build-coverity has finished and the _analysis-coverity
>>>>>>> recipe can now run, it will call make with the analysis-clean target, 
>>>>>>> restoring any <file>.{c,h}.safparse to <file>.{c,h}.
>>>>>>
>>>>>> ... the subsequent cleaning. The state of the _source_ tree after a
>>>>>> build failure would be different from that after a successful build.
>>>>>> Personally I consider this at best surprising.
>>>>>>
>>>>>> I wonder whether instead there could be a shell(?) script driving a
>>>>>> sequence of make invocations, leaving the new make goals all be self-
>>>>>> contained. Such a script could revert the source tree to its original
>>>>>> state even upon build failure by default, with an option allowing to
>>>>>> suppress this behavior.
>>>>>
>>>>> Instead of adding another tool, so another layer to the overall system, I 
>>>>> would be more willing to add documentation
>>>>> about this process, explaining how to use the analysis-* build targets, 
>>>>> what to expect after a successful run and what
>>>>> to expect after a failure.
>>>>>
>>>>> What do you think?
>>>>
>>>> Personally I'd prefer make goals to behave as such, with no surprises.
>>>
>>> The analysis-* goal requires a build step, otherwise no analysis can be 
>>> performed by the analysis tools, so I hope we agree
>>> we need to integrate that step as a dependency of the analysis-*.
>>
>> No, I'm afraid we don't agree. But like said for another piece we didn't
>> initially agree on - if others think what you propose is fine, so be it.
>> I'm specifically adding Anthony to Cc, as he's been working on make rules
>> the most of all of us in the recent past.
>>
>>> I understand that the analysis-clean might be a “surprise” if not well 
>>> documented, this comes from the need to substitute the
>>> tags in the tree (to keep the real path in the report log) and to revert 
>>> them back at the end of the analysis.
>>>
>>> So, such script should just mask to the user the analysis-clean invocation 
>>> in case of errors (with an option to don’t do that)?
>>
>> Hmm, here you're saying "such script", which looks to not fit with the
>> earlier part of your reply above. (Just in case that's what I was to read
>> out of this: I wouldn't see value in a script which existed _solely_ to
>> make the cleaning conditional.)
>>
>> Did you consider the alternative approach of copying the tree, altering
>> it (while or after copying), running the build there, pulling out the
>> result files, and delete the entire copy? Such a model would likely get
>> away without introducing surprising make rules.
> 
> Another, maybe simpler idea: what if the build step is not a dependency
> of the analysis-* goals?
> 
> Basically, the user is supposed to:
> 
> 1) call analysis-parse-tags-*
> 2) build Xen (in any way they like)
> 3) call analysis-clean

Well, that's exactly what I've been proposing, with the (optional)
addition of a small (shell) script doing all of the three for ...

> Making steps 1-3 into a single step is slightly more convenient for the
> user but the downside is that dealing with build errors becomes
> problematic.
> 
> On the other hand, if we let the user call steps 1-3 by hand
> individually, it is slightly less convenient for the user but they can
> more easily deal with any build error and sophisticated build
> configurations.

... convenience.

Jan