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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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

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.

This is one of those cases where I think "less is more".