| Message ID | 20230310103123.2118519-11-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tweaks and fixes for 8.0-rc1 (tests, plugins, docs) | expand |
On 10/3/23 11:31, Alex Bennée wrote: > The kerneldoc processor complains about the mismatched variable name. > Fix it. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/exec/memory.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 10/03/2023 11.31, Alex Bennée wrote: > The kerneldoc processor complains about the mismatched variable name. > Fix it. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/exec/memory.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 6fa0b071f0..15ade918ba 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, > * > * @notifier: the notifier to be notified > */ > -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n); > +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier); I also keep running into this problem ... I wonder whether we should run sphinx with "-W" to turn warnings into errors when configure has been run with --enable-werror ...? Anyway, for this patch here: Reviewed-by: Thomas Huth <thuth@redhat.com>
On Mon, 13 Mar 2023 at 17:00, Thomas Huth <thuth@redhat.com> wrote: > > On 10/03/2023 11.31, Alex Bennée wrote: > > The kerneldoc processor complains about the mismatched variable name. > > Fix it. > > > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > --- > > include/exec/memory.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/exec/memory.h b/include/exec/memory.h > > index 6fa0b071f0..15ade918ba 100644 > > --- a/include/exec/memory.h > > +++ b/include/exec/memory.h > > @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, > > * > > * @notifier: the notifier to be notified > > */ > > -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n); > > +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier); > > I also keep running into this problem ... I wonder whether we should run > sphinx with "-W" to turn warnings into errors when configure has been run > with --enable-werror ...? We certainly try to do that: docs/meson.build says: # If we're making warnings fatal, apply this to Sphinx runs as well if get_option('werror') SPHINX_ARGS += [ '-W' ] endif Has that broken ? -- PMM
On 13/03/2023 18.03, Peter Maydell wrote: > On Mon, 13 Mar 2023 at 17:00, Thomas Huth <thuth@redhat.com> wrote: >> >> On 10/03/2023 11.31, Alex Bennée wrote: >>> The kerneldoc processor complains about the mismatched variable name. >>> Fix it. >>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> include/exec/memory.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/exec/memory.h b/include/exec/memory.h >>> index 6fa0b071f0..15ade918ba 100644 >>> --- a/include/exec/memory.h >>> +++ b/include/exec/memory.h >>> @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, >>> * >>> * @notifier: the notifier to be notified >>> */ >>> -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n); >>> +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier); >> >> I also keep running into this problem ... I wonder whether we should run >> sphinx with "-W" to turn warnings into errors when configure has been run >> with --enable-werror ...? > > We certainly try to do that: docs/meson.build says: > > # If we're making warnings fatal, apply this to Sphinx runs as well > if get_option('werror') > SPHINX_ARGS += [ '-W' ] > endif > > Has that broken ? It apparently does not work in our CI, see e.g.: https://gitlab.com/qemu-project/qemu/-/jobs/3922732898#L1420 ... there is a warning here, but the job succeeded happily. Thomas
On Mon, 13 Mar 2023 at 17:14, Thomas Huth <thuth@redhat.com> wrote: > > On 13/03/2023 18.03, Peter Maydell wrote: > > On Mon, 13 Mar 2023 at 17:00, Thomas Huth <thuth@redhat.com> wrote: > >> I also keep running into this problem ... I wonder whether we should run > >> sphinx with "-W" to turn warnings into errors when configure has been run > >> with --enable-werror ...? > > > > We certainly try to do that: docs/meson.build says: > > > > # If we're making warnings fatal, apply this to Sphinx runs as well > > if get_option('werror') > > SPHINX_ARGS += [ '-W' ] > > endif > > > > Has that broken ? > > It apparently does not work in our CI, see e.g.: > > https://gitlab.com/qemu-project/qemu/-/jobs/3922732898#L1420 > > ... there is a warning here, but the job succeeded happily. Specifically: /builds/qemu-project/qemu/docs/../include/exec/memory.h:1741: warning: Function parameter or member 'n' not described in 'memory_region_unmap_iommu_notifier_range' /builds/qemu-project/qemu/docs/../include/exec/memory.h:1741: warning: Excess function parameter 'notifier' description in 'memory_region_unmap_iommu_notifier_range' ninja: bad depfile: multiple outputs: /builds/qemu-project/qemu/docs/devel/secure-coding-practices.rst != docs/docs.stamp Also, what's that 'bad depfile' warning from ninja about ?? I looked at the build.ninja file (which you can fish out of the artifacts for this build), and it shows that we are passing -W to sphinx-build: build docs/docs.stamp: CUSTOM_COMMAND_DEP ../docs/conf.py | /usr/bin/env /usr/bin/sphinx-build DEPFILE = docs/docs.d DEPFILE_UNQUOTED = docs/docs.d COMMAND = /usr/bin/env CONFDIR=etc/qemu /usr/bin/sphinx-build -q -W -Dversion=7.2.50 -Drelease= -Ddepfile=docs/docs.d -Ddepfile_stamp=docs/docs.stamp -b html -d /builds/qemu-project/qemu/build/docs/manual.p /builds/qemu-project/qemu/docs /builds/qemu-project/qemu/build/docs/manual description = Generating$ docs/QEMU$ manual$ with$ a$ custom$ command So I think the problem here is not with Sphinx, but with the kernel-doc script. That script has an option "-Werror" which turns its warnings into errors, but our Sphinx extension docs/sphinx/kerneldoc.py does not set it. I think we need to have the extension say "if Sphinx was run with -W then pass this flag along" (hopefully Sphinx lets us find out...) thanks -- PMM
On Mon, 13 Mar 2023 at 17:30, Peter Maydell <peter.maydell@linaro.org> wrote: > So I think the problem here is not with Sphinx, but with the > kernel-doc script. That script has an option "-Werror" which > turns its warnings into errors, but our Sphinx extension > docs/sphinx/kerneldoc.py does not set it. I think we need to > have the extension say "if Sphinx was run with -W then > pass this flag along" (hopefully Sphinx lets us find out...) This works: --- a/docs/sphinx/kerneldoc.py +++ b/docs/sphinx/kerneldoc.py @@ -74,6 +74,10 @@ def run(self): # Sphinx versions cmd += ['-sphinx-version', sphinx.__version__] + # Pass through the warnings-as-errors flag if appropriate + if env.app.warningiserror: + cmd += ['-Werror'] + filename = env.config.kerneldoc_srctree + '/' + self.arguments[0] export_file_patterns = [] but I think it's prodding undocumented Sphinx internals, so I'm going to check whether there's a better way to do this. It might be more robust to have meson create a commandline with a -Dkerneldoc_werror option that we then pick up in the extension code, rather than trying to find out whether -W was passed. -- PMM
diff --git a/include/exec/memory.h b/include/exec/memory.h index 6fa0b071f0..15ade918ba 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier, * * @notifier: the notifier to be notified */ -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n); +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier); /**
The kerneldoc processor complains about the mismatched variable name. Fix it. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- include/exec/memory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)