| Message ID | 20230504131245.2985400-4-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fix and improvements to xen-analysis.py | expand |
On Thu, 4 May 2023, Luca Fancellu wrote: > repository in the reports > > Currently the cppcheck report entries shows the relative file path > from the /xen folder of the repository instead of the base folder. > In order to ease the checks, for example, when looking a git diff > output and the report, use the repository folder as base. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Tested-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/scripts/xen_analysis/cppcheck_analysis.py | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py > index c3783e8df343..c8abbe0fca79 100644 > --- a/xen/scripts/xen_analysis/cppcheck_analysis.py > +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py > @@ -149,7 +149,7 @@ def generate_cppcheck_deps(): > --suppress='unusedStructMember:*' > --include={}/include/xen/config.h > -DCPPCHECK > -""".format(settings.xen_dir, settings.outdir, settings.xen_dir) > +""".format(settings.repo_dir, settings.outdir, settings.xen_dir) > > invoke_cppcheck = utils.invoke_command( > "{} --version".format(settings.cppcheck_binpath), > @@ -240,7 +240,7 @@ def generate_cppcheck_report(): > try: > cppcheck_report_utils.cppcheck_merge_txt_fragments(fragments, > report_filename, > - [settings.xen_dir]) > + [settings.repo_dir]) > except cppcheck_report_utils.CppcheckTXTReportError as e: > raise CppcheckReportPhaseError(e) > > @@ -257,7 +257,7 @@ def generate_cppcheck_report(): > try: > cppcheck_report_utils.cppcheck_merge_xml_fragments(fragments, > xml_filename, > - settings.xen_dir, > + settings.repo_dir, > settings.outdir) > except cppcheck_report_utils.CppcheckHTMLReportError as e: > raise CppcheckReportPhaseError(e) > @@ -265,7 +265,7 @@ def generate_cppcheck_report(): > utils.invoke_command( > "{} --file={} --source-dir={} --report-dir={}/html --title=Xen" > .format(settings.cppcheck_htmlreport_binpath, xml_filename, > - settings.xen_dir, html_report_dir), > + settings.repo_dir, html_report_dir), > False, CppcheckReportPhaseError, > "Error occured generating Cppcheck HTML report:\n{}" > ) > @@ -273,7 +273,7 @@ def generate_cppcheck_report(): > html_files = utils.recursive_find_file(html_report_dir, r'.*\.html$') > try: > cppcheck_report_utils.cppcheck_strip_path_html(html_files, > - (settings.xen_dir, > + (settings.repo_dir, > settings.outdir)) > except cppcheck_report_utils.CppcheckHTMLReportError as e: > raise CppcheckReportPhaseError(e) > -- > 2.34.1 >
Hi Luca, On 17/05/2023 02:44, Stefano Stabellini wrote: > > > On Thu, 4 May 2023, Luca Fancellu wrote: >> repository in the reports >> >> Currently the cppcheck report entries shows the relative file path >> from the /xen folder of the repository instead of the base folder. >> In order to ease the checks, for example, when looking a git diff >> output and the report, use the repository folder as base. >> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Tested-by: Stefano Stabellini <sstabellini@kernel.org> I know this patch is now committed but there is something confusing here. At the moment, in the cppcheck report we have paths relative to xen/ e.g.: arch/arm/arm64/lib/bitops.c(117,1):... So after this patch, I would expect to see the path relative to root of repository e.g.: *xen/*arch/arm/arm64/lib/bitops.c(117,1):... However, with or without this patch the behavior is the same. Did I misunderstand your patch? ~Michal
> On 19 May 2023, at 08:08, Michal Orzel <michal.orzel@amd.com> wrote: > > Hi Luca, > > On 17/05/2023 02:44, Stefano Stabellini wrote: >> >> >> On Thu, 4 May 2023, Luca Fancellu wrote: >>> repository in the reports >>> >>> Currently the cppcheck report entries shows the relative file path >>> from the /xen folder of the repository instead of the base folder. >>> In order to ease the checks, for example, when looking a git diff >>> output and the report, use the repository folder as base. >>> >>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> >> Acked-by: Stefano Stabellini <sstabellini@kernel.org> >> Tested-by: Stefano Stabellini <sstabellini@kernel.org> > > I know this patch is now committed but there is something confusing here. > At the moment, in the cppcheck report we have paths relative to xen/ e.g.: > arch/arm/arm64/lib/bitops.c(117,1):... > > So after this patch, I would expect to see the path relative to root of repository e.g.: > *xen/*arch/arm/arm64/lib/bitops.c(117,1):... > > However, with or without this patch the behavior is the same. > Did I misunderstand your patch? Hi Michal, Thank you for having spotted this, during my tests I was using Xen-analysis.py so that it calls the makefile with out-of-tree build, I’ve found after your mail that when it calls the makefile with in-tree-build, cppcheck is run from /xen/xen and it causes it to produce relative path from there in the TXT fragments, showing the issue you observed. I have ready a fix for that and I’ll push that soon. > > ~Michal
On 19/05/2023 09:12, Luca Fancellu wrote: > > >> On 19 May 2023, at 08:08, Michal Orzel <michal.orzel@amd.com> wrote: >> >> Hi Luca, >> >> On 17/05/2023 02:44, Stefano Stabellini wrote: >>> >>> >>> On Thu, 4 May 2023, Luca Fancellu wrote: >>>> repository in the reports >>>> >>>> Currently the cppcheck report entries shows the relative file path >>>> from the /xen folder of the repository instead of the base folder. >>>> In order to ease the checks, for example, when looking a git diff >>>> output and the report, use the repository folder as base. >>>> >>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >>> >>> Acked-by: Stefano Stabellini <sstabellini@kernel.org> >>> Tested-by: Stefano Stabellini <sstabellini@kernel.org> >> >> I know this patch is now committed but there is something confusing here. >> At the moment, in the cppcheck report we have paths relative to xen/ e.g.: >> arch/arm/arm64/lib/bitops.c(117,1):... >> >> So after this patch, I would expect to see the path relative to root of repository e.g.: >> *xen/*arch/arm/arm64/lib/bitops.c(117,1):... >> >> However, with or without this patch the behavior is the same. >> Did I misunderstand your patch? > > Hi Michal, > > Thank you for having spotted this, during my tests I was using Xen-analysis.py so that it > calls the makefile with out-of-tree build, I’ve found after your mail that when it calls the makefile > with in-tree-build, cppcheck is run from /xen/xen and it causes it to produce relative path from > there in the TXT fragments, showing the issue you observed. Ok, the way I test it is the same as in our gitlab CI so this needs to be fixed. > > I have ready a fix for that and I’ll push that soon. Thanks. ~Michal
> On 19 May 2023, at 08:25, Michal Orzel <michal.orzel@amd.com> wrote: > > > On 19/05/2023 09:12, Luca Fancellu wrote: >> >> >>> On 19 May 2023, at 08:08, Michal Orzel <michal.orzel@amd.com> wrote: >>> >>> Hi Luca, >>> >>> On 17/05/2023 02:44, Stefano Stabellini wrote: >>>> >>>> >>>> On Thu, 4 May 2023, Luca Fancellu wrote: >>>>> repository in the reports >>>>> >>>>> Currently the cppcheck report entries shows the relative file path >>>>> from the /xen folder of the repository instead of the base folder. >>>>> In order to ease the checks, for example, when looking a git diff >>>>> output and the report, use the repository folder as base. >>>>> >>>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >>>> >>>> Acked-by: Stefano Stabellini <sstabellini@kernel.org> >>>> Tested-by: Stefano Stabellini <sstabellini@kernel.org> >>> >>> I know this patch is now committed but there is something confusing here. >>> At the moment, in the cppcheck report we have paths relative to xen/ e.g.: >>> arch/arm/arm64/lib/bitops.c(117,1):... >>> >>> So after this patch, I would expect to see the path relative to root of repository e.g.: >>> *xen/*arch/arm/arm64/lib/bitops.c(117,1):... >>> >>> However, with or without this patch the behavior is the same. >>> Did I misunderstand your patch? >> >> Hi Michal, >> >> Thank you for having spotted this, during my tests I was using Xen-analysis.py so that it >> calls the makefile with out-of-tree build, I’ve found after your mail that when it calls the makefile >> with in-tree-build, cppcheck is run from /xen/xen and it causes it to produce relative path from >> there in the TXT fragments, showing the issue you observed. > Ok, the way I test it is the same as in our gitlab CI so this needs to be fixed. Here it is the fix: https://patchwork.kernel.org/project/xen-devel/patch/20230519093019.2131896-4-luca.fancellu@arm.com/ I’ve updated my internal test script to test it on in-tree and out-of-tree makefile invocation. Hope I did not forget anything, apologies for the inconvenience! > >> >> I have ready a fix for that and I’ll push that soon. > Thanks. > > ~Michal
diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py b/xen/scripts/xen_analysis/cppcheck_analysis.py index c3783e8df343..c8abbe0fca79 100644 --- a/xen/scripts/xen_analysis/cppcheck_analysis.py +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py @@ -149,7 +149,7 @@ def generate_cppcheck_deps(): --suppress='unusedStructMember:*' --include={}/include/xen/config.h -DCPPCHECK -""".format(settings.xen_dir, settings.outdir, settings.xen_dir) +""".format(settings.repo_dir, settings.outdir, settings.xen_dir) invoke_cppcheck = utils.invoke_command( "{} --version".format(settings.cppcheck_binpath), @@ -240,7 +240,7 @@ def generate_cppcheck_report(): try: cppcheck_report_utils.cppcheck_merge_txt_fragments(fragments, report_filename, - [settings.xen_dir]) + [settings.repo_dir]) except cppcheck_report_utils.CppcheckTXTReportError as e: raise CppcheckReportPhaseError(e) @@ -257,7 +257,7 @@ def generate_cppcheck_report(): try: cppcheck_report_utils.cppcheck_merge_xml_fragments(fragments, xml_filename, - settings.xen_dir, + settings.repo_dir, settings.outdir) except cppcheck_report_utils.CppcheckHTMLReportError as e: raise CppcheckReportPhaseError(e) @@ -265,7 +265,7 @@ def generate_cppcheck_report(): utils.invoke_command( "{} --file={} --source-dir={} --report-dir={}/html --title=Xen" .format(settings.cppcheck_htmlreport_binpath, xml_filename, - settings.xen_dir, html_report_dir), + settings.repo_dir, html_report_dir), False, CppcheckReportPhaseError, "Error occured generating Cppcheck HTML report:\n{}" ) @@ -273,7 +273,7 @@ def generate_cppcheck_report(): html_files = utils.recursive_find_file(html_report_dir, r'.*\.html$') try: cppcheck_report_utils.cppcheck_strip_path_html(html_files, - (settings.xen_dir, + (settings.repo_dir, settings.outdir)) except cppcheck_report_utils.CppcheckHTMLReportError as e: raise CppcheckReportPhaseError(e)
repository in the reports Currently the cppcheck report entries shows the relative file path from the /xen folder of the repository instead of the base folder. In order to ease the checks, for example, when looking a git diff output and the report, use the repository folder as base. Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> --- xen/scripts/xen_analysis/cppcheck_analysis.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)