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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> 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.