Re: [PATCH 2/2] xen/misra: diff-report.py: add report patching feature

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 19 May 2023, Luca Fancellu wrote:
> > On 19 May 2023, at 10:46, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
> > 
> > Add a feature to the diff-report.py script that improves the comparison
> > between two analysis report, one from a baseline codebase and the other
> > from the changes applied to the baseline.
> > 
> > The comparison between reports of different codebase is an issue because
> > entries in the baseline could have been moved in position due to addition
> > or deletion of unrelated lines or can disappear because of deletion of
> > the interested line, making the comparison between two revisions of the
> > code harder.
> > 
> > Having a baseline report, a report of the codebase with the changes
> > called "new report" and a git diff format file that describes the
> > changes happened to the code from the baseline, this feature can
> > understand which entries from the baseline report are deleted or shifted
> > in position due to changes to unrelated lines and can modify them as
> > they will appear in the "new report".
> > 
> > Having the "patched baseline" and the "new report", now it's simple
> > to make the diff between them and print only the entry that are new.
> > 
> > Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> > ---
> > Changes from v1:
> > - Made the script compatible with python2 (Stefano)
> > ---
> > xen/scripts/diff-report.py                    |  55 ++++-
> > xen/scripts/xen_analysis/diff_tool/debug.py   |  21 ++
> > xen/scripts/xen_analysis/diff_tool/report.py  |  87 +++++++
> > .../diff_tool/unified_format_parser.py        | 232 ++++++++++++++++++
> > 4 files changed, 393 insertions(+), 2 deletions(-)
> > create mode 100644 
> > xen/scripts/xen_analysis/diff_tool/unified_format_parser.py
> > 
> > diff --git a/xen/scripts/diff-report.py b/xen/scripts/diff-report.py
> > index f97cb2355cc3..d608e3a05aa1 100755
> > --- a/xen/scripts/diff-report.py
> > +++ b/xen/scripts/diff-report.py
> > @@ -7,6 +7,10 @@ from argparse import ArgumentParser
> > from xen_analysis.diff_tool.cppcheck_report import CppcheckReport
> > from xen_analysis.diff_tool.debug import Debug
> > from xen_analysis.diff_tool.report import ReportError
> > +from xen_analysis.diff_tool.unified_format_parser import \
> > +    (UnifiedFormatParser, UnifiedFormatParseError)
> > +from xen_analysis.settings import repo_dir
> > +from xen_analysis.utils import invoke_command
> > 
> > 
> > def log_info(text, end='\n'):
> > @@ -36,9 +40,32 @@ def main(argv):
> >                              "against the baseline.")
> >     parser.add_argument("-v", "--verbose", action='store_true',
> >                         help="Print more informations during the run.")
> > +    parser.add_argument("--patch", type=str,
> > +                        help="The patch file containing the changes to the 
> > "
> > +                             "code, from the baseline analysis result to 
> > the "
> > +                             "'check report' analysis result.\n"
> > +                             "Do not use with --baseline-rev/--report-rev")
> > +    parser.add_argument("--baseline-rev", type=str,
> > +                        help="Revision or SHA of the codebase analysed to "
> > +                             "create the baseline report.\n"
> > +                             "Use together with --report-rev")
> > +    parser.add_argument("--report-rev", type=str,
> > +                        help="Revision or SHA of the codebase analysed to "
> > +                             "create the 'check report'.\n"
> > +                             "Use together with --baseline-rev")
> > 
> >     args = parser.parse_args()
> > 
> > +    if args.patch and (args.baseline_rev or args.report_rev):
> > +        print("ERROR: '--patch' argument can't be used with 
> > '--baseline-rev'"
> > +              " or '--report-rev'.")
> > +        sys.exit(1)
> > +
> > +    if bool(args.baseline_rev) != bool(args.report_rev):
> > +        print("ERROR: '--baseline-rev' must be used together with "
> > +              "'--report-rev'.")
> > +        sys.exit(1)
> > +
> >     if args.out == "stdout":
> >         file_out = sys.stdout
> >     else:
> > @@ -63,11 +90,35 @@ def main(argv):
> >         new_rep.parse()
> >         debug.debug_print_parsed_report(new_rep)
> >         log_info(" [OK]")
> > -    except ReportError as e:
> > +        diff_source = None
> > +        if args.patch:
> > +            diff_source = os.path.realpath(args.patch)
> > +        elif args.baseline_rev:
> > +            git_diff = invoke_command(
> > +                "git diff --git-dir={} -C -C {}..{}".format(repo_dir,
> > +                                                            
> > args.baseline_rev,
> > +                                                            
> > args.report_rev),
> > +                True, "Error occured invoking:\n{}\n\n{}"
> > +            )
> 
> I’ve noticed now an issue here, when using --baseline-rev/--report-rev, the 
> fix is this one:
> 
> diff --git a/xen/scripts/diff-report.py b/xen/scripts/diff-report.py
> index d608e3a05aa1..636f98f5eebe 100755
> --- a/xen/scripts/diff-report.py
> +++ b/xen/scripts/diff-report.py
> @@ -95,9 +95,8 @@ def main(argv):
>              diff_source = os.path.realpath(args.patch)
>          elif args.baseline_rev:
>              git_diff = invoke_command(
> -                "git diff --git-dir={} -C -C {}..{}".format(repo_dir,
> -                                                            
> args.baseline_rev,
> -                                                            args.report_rev),
> +                "git --git-dir={}/.git diff -C -C {}..{}"
> +                .format(repo_dir, args.baseline_rev, args.report_rev),
>                  True, "Error occured invoking:\n{}\n\n{}"
>              )
>              diff_source = git_diff.splitlines(keepends=True)
> 
> I’ll wait for other feedback on the patch before sending it again.

With this change:

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Tested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>