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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan