Re: [PATCH 10/11] include/exec: fix kerneldoc definition

From: Peter Maydell <peter.maydell@linaro.org>
To: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	qemu-devel@nongnu.org, "David Hildenbrand" <david@redhat.com>,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-arm@nongnu.org, "Peter Xu" <peterx@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Beraldo Leal" <bleal@redhat.com>
Subject: Re: [PATCH 10/11] include/exec: fix kerneldoc definition
Date: Mon, 13 Mar 2023 17:30:33 +0000	[thread overview]
Message-ID: <CAFEAcA9gdHi0QV1zj7nMNz1=NQjPyMisqU_Wqdc-HaLZg45HYA@mail.gmail.com> (raw)
In-Reply-To: <b23fcfab-cc1b-a861-94ed-217af69f1ef8@redhat.com>

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