[RFC] Proposal to relax warnings of htmldocs

Hello all,

I would like to discuss a recurring issue that we all have encountered
while running `make htmldocs`. The process often generates an overwhelming
amount of warnings that are not relevant to our work, which makes it harder
to identify and address actual warning messages related to our changes.

One of the reasons for this is the variation in warnings raised by
different compilers. For instance, the Linux kernel robot employs Sparse,
which recently raised a warning that we (Avadhut in CC, and then me
reviewing his patch) did not catch during our testing [1,2].

Particularly annoying -to me personally- are warnings of the form:

"warning: Function parameter or member 'field' not described in 'struct'"

that seem to enumerate every single undocumented field within a struct.

I would like to propose something to alleviate this issue before the list
of warnings keeps piling up. I suggest for the command `make htmldocs` to
only display, by default, warnings directly related to the changes being
made, unless explicitly requested otherwisee.

I'm thinking we could do this, for example, by making hmtldocs a two-step
process: First running htmldocs as usual but with warnings disabled, and
then generating docs again but only for the new files (see $git diff
--name-only HEAD), with warnings active but limited to the scope of the
changes made.

I'm interested in your thoughts and comments. Does anyone have a better
idea regarding this? Maybe someone more experienced in Sphinx can tell us
an alternative way to disable unrelated warnings.

Thanks,
Carlos

Signed-off-by: Carlos Bilbao <redacted>

[1] https://lore.kernel.org/all/20230717181602.3468421-1-avadhut.naik@amd.com/
[2] https://download.01.org/0day-ci/archive/20230722/202307221642.nEBAfROh-lkp@intel.com/reproduce

Carlos Bilbao [off-list ref] writes:

Hello all,

I would like to discuss a recurring issue that we all have encountered
while running `make htmldocs`. The process often generates an overwhelming
amount of warnings that are not relevant to our work, which makes it harder
to identify and address actual warning messages related to our changes.

One of the reasons for this is the variation in warnings raised by
different compilers. For instance, the Linux kernel robot employs Sparse,
which recently raised a warning that we (Avadhut in CC, and then me
reviewing his patch) did not catch during our testing [1,2].

Particularly annoying -to me personally- are warnings of the form:

"warning: Function parameter or member 'field' not described in 'struct'"

that seem to enumerate every single undocumented field within a struct.

I would like to propose something to alleviate this issue before the list
of warnings keeps piling up.

...other than fixing the actual problems? :)

I suggest for the command `make htmldocs` to
only display, by default, warnings directly related to the changes being
made, unless explicitly requested otherwisee.

I'm thinking we could do this, for example, by making hmtldocs a two-step
process: First running htmldocs as usual but with warnings disabled, and
then generating docs again but only for the new files (see $git diff
--name-only HEAD), with warnings active but limited to the scope of the
changes made.

A normal build should just generate warnings for files that have
changed (since the last build).  Does that not do what you want?

Trying to get Sphinx to do smarter things with partial builds seems like
a path to frustration.  Since the specific warnings you're talking about
are generated by kernel-doc, a better solution would probably just
invoke it directly.  It wouldn't be that hard to bash out a script to
feed a given set of files to kernel-doc and see what it says.

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

Thanks,

jon

On Tue, Aug 15, 2023 at 8:23 PM Jonathan Corbet [off-list ref] wrote:

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

It could be nice to get to enforce warning-free builds as soon as possible.

Perhaps we could move those to a `W=1`-like group and clean them over
time instead? Or do we have that already?

Cheers,
Miguel

On Tue, Aug 15, 2023 at 08:35:40PM +0200, Miguel Ojeda wrote:

On Tue, Aug 15, 2023 at 8:23 PM Jonathan Corbet [off-list ref] wrote:

quoted

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

It could be nice to get to enforce warning-free builds as soon as possible.

Perhaps we could move those to a `W=1`-like group and clean them over
time instead? Or do we have that already?

I think the problem is that we don't run kernel-doc by default.  Instead,
it's only run for W=1 (and higher) builds.  That's why Carlos doesn't
see the problems he is introducing in his own builds.  Of course, if
AMD required building with W=1 then they'd see these problems earlier
in their own testing.  Apparently they don't.

Is it time to just run kernel-doc by default?  There aren't _that_
many kernel-doc warnings now.  Not compared to how they used to be.
And enabling them for everyone means that new ones won't sneak in.
I haven't timed how much extra time kernel-doc adds to a build.
Perhaps that's infeasible.

On 8/15/23 13:41, Matthew Wilcox wrote:

On Tue, Aug 15, 2023 at 08:35:40PM +0200, Miguel Ojeda wrote:

quoted

On Tue, Aug 15, 2023 at 8:23 PM Jonathan Corbet [off-list ref] wrote:

quoted

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

It could be nice to get to enforce warning-free builds as soon as possible.

Perhaps we could move those to a `W=1`-like group and clean them over
time instead? Or do we have that already?

I think the problem is that we don't run kernel-doc by default.  Instead,
it's only run for W=1 (and higher) builds.  That's why Carlos doesn't
see the problems he is introducing in his own builds.  Of course, if
AMD required building with W=1 then they'd see these problems earlier
in their own testing.  Apparently they don't.

AMD's testing practices or standards are orthogonal to this discussion. My
contributions to the Documentation effort are entirely voluntary and
independent of my role at AMD.

Is it time to just run kernel-doc by default?  There aren't _that_
many kernel-doc warnings now.  Not compared to how they used to be.
And enabling them for everyone means that new ones won't sneak in.
I haven't timed how much extra time kernel-doc adds to a build.
Perhaps that's infeasible.

Thanks,
Carlos

On Tue, 15 Aug 2023, Matthew Wilcox [off-list ref] wrote:

On Tue, Aug 15, 2023 at 08:35:40PM +0200, Miguel Ojeda wrote:

quoted

On Tue, Aug 15, 2023 at 8:23 PM Jonathan Corbet [off-list ref] wrote:

quoted

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

It could be nice to get to enforce warning-free builds as soon as possible.

Perhaps we could move those to a `W=1`-like group and clean them over
time instead? Or do we have that already?

I think the problem is that we don't run kernel-doc by default.  Instead,
it's only run for W=1 (and higher) builds.  That's why Carlos doesn't
see the problems he is introducing in his own builds.  Of course, if
AMD required building with W=1 then they'd see these problems earlier
in their own testing.  Apparently they don't.

Is it time to just run kernel-doc by default?  There aren't _that_
many kernel-doc warnings now.  Not compared to how they used to be.
And enabling them for everyone means that new ones won't sneak in.
I haven't timed how much extra time kernel-doc adds to a build.
Perhaps that's infeasible.

Personally, I believe it's easier to get at a warning free build (both
compiler W=1 warnings as well as kernel-doc) by doing it driver and
subsystem at a time, instead of, say, one warning at a time across the
entire kernel. It's just too much of a burden to fix the entire kernel
to enable a warning across the board.

To that end, the i915 Makefile enables a lot more warnings than the
defaults, and the developers and CI run the compiler and kernel-doc with
-Werror. No new warnings get introduced.

What I'd hope for is build system support to enable W=1
compiler/kernel-doc warnings for a subdir with a few lines at most,
instead of duplicating and copy-pasting tens of lines from
scripts/Makefile.extrawarn like we have to do now.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

On 8/15/23 13:23, Jonathan Corbet wrote:

Carlos Bilbao [off-list ref] writes:

quoted

Hello all,

I would like to discuss a recurring issue that we all have encountered
while running `make htmldocs`. The process often generates an overwhelming
amount of warnings that are not relevant to our work, which makes it harder
to identify and address actual warning messages related to our changes.

One of the reasons for this is the variation in warnings raised by
different compilers. For instance, the Linux kernel robot employs Sparse,
which recently raised a warning that we (Avadhut in CC, and then me
reviewing his patch) did not catch during our testing [1,2].

Particularly annoying -to me personally- are warnings of the form:

"warning: Function parameter or member 'field' not described in 'struct'"

that seem to enumerate every single undocumented field within a struct.

I would like to propose something to alleviate this issue before the list
of warnings keeps piling up.

...other than fixing the actual problems? :)

I'm happy to fix as many as I can, but there are obstacles e.g. some things
lack documentation, such as undocumented fields in structures with names
that nobody but their creator could decipher. Also, that won't solve the
underlying warning display problem (which maybe it's W!=1, as noted by
Matthew.

quoted

I suggest for the command `make htmldocs` to
only display, by default, warnings directly related to the changes being
made, unless explicitly requested otherwisee.

I'm thinking we could do this, for example, by making hmtldocs a two-step
process: First running htmldocs as usual but with warnings disabled, and
then generating docs again but only for the new files (see $git diff
--name-only HEAD), with warnings active but limited to the scope of the
changes made.

A normal build should just generate warnings for files that have
changed (since the last build).  Does that not do what you want?

That's not the behavior I see on my system, when I run `make htmldocs` I
see many warnings from other places.It floods my screen. The default
behavior appears to change between configurations and compilers.

Trying to get Sphinx to do smarter things with partial builds seems like
a path to frustration.  Since the specific warnings you're talking about
are generated by kernel-doc, a better solution would probably just
invoke it directly.  It wouldn't be that hard to bash out a script to
feed a given set of files to kernel-doc and see what it says.

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

Thanks for explaining, yes, looks like kernel-doc is the place to do
something at.

Thanks,

jon

Thanks,
Carlos

Carlos Bilbao [off-list ref] writes:

quoted

...other than fixing the actual problems? :)

I'm happy to fix as many as I can, but there are obstacles e.g. some things
lack documentation, such as undocumented fields in structures with names
that nobody but their creator could decipher. Also, that won't solve the
underlying warning display problem (which maybe it's W!=1, as noted by
Matthew.

I *did* put a smiley on that...

quoted

quoted

I suggest for the command `make htmldocs` to
only display, by default, warnings directly related to the changes being
made, unless explicitly requested otherwisee.

I'm thinking we could do this, for example, by making hmtldocs a two-step
process: First running htmldocs as usual but with warnings disabled, and
then generating docs again but only for the new files (see $git diff
--name-only HEAD), with warnings active but limited to the scope of the
changes made.

A normal build should just generate warnings for files that have
changed (since the last build).  Does that not do what you want?

That's not the behavior I see on my system, when I run `make htmldocs` I
see many warnings from other places.It floods my screen. The default
behavior appears to change between configurations and compilers.

Warnings that are generated at output time, such as broken references,
will come out every time.  Those generated during the read phase are
generally limited to what has been changed.  I rely on that pretty
heavily to do incremental builds when applying patches.  If you're doing
a full rebuild after changing one file, something strange is happening.

Thanks,

jon

On 8/16/23 06:01, Jani Nikula wrote:

On Tue, 15 Aug 2023, Matthew Wilcox [off-list ref] wrote:

quoted

On Tue, Aug 15, 2023 at 08:35:40PM +0200, Miguel Ojeda wrote:

quoted

On Tue, Aug 15, 2023 at 8:23 PM Jonathan Corbet [off-list ref] wrote:

quoted

As an alternative, of course, we could consider turning off those
specific warnings entirely for normal builds.

It could be nice to get to enforce warning-free builds as soon as possible.

Perhaps we could move those to a `W=1`-like group and clean them over
time instead? Or do we have that already?

I think the problem is that we don't run kernel-doc by default.  Instead,
it's only run for W=1 (and higher) builds.  That's why Carlos doesn't
see the problems he is introducing in his own builds.  Of course, if
AMD required building with W=1 then they'd see these problems earlier
in their own testing.  Apparently they don't.

Is it time to just run kernel-doc by default?  There aren't _that_
many kernel-doc warnings now.  Not compared to how they used to be.
And enabling them for everyone means that new ones won't sneak in.
I haven't timed how much extra time kernel-doc adds to a build.
Perhaps that's infeasible.

Personally, I believe it's easier to get at a warning free build (both
compiler W=1 warnings as well as kernel-doc) by doing it driver and
subsystem at a time, instead of, say, one warning at a time across the
entire kernel. It's just too much of a burden to fix the entire kernel
to enable a warning across the board.

To that end, the i915 Makefile enables a lot more warnings than the
defaults, and the developers and CI run the compiler and kernel-doc with
-Werror. No new warnings get introduced.

What I'd hope for is build system support to enable W=1
compiler/kernel-doc warnings for a subdir with a few lines at most,
instead of duplicating and copy-pasting tens of lines from
scripts/Makefile.extrawarn like we have to do now.

That sounds feasible but, well, I actually proposed the opposite approach.
I wanted to "relax" the warnings (see RFC Subject) rather than making them
more strict by default.

My concern is that W=1 (by default) may theoretically result in a clean
`make htmldocs` but it won't in practice. Not all developers prioritize
good documentation like the folks from i915, and that may lead to
unaddressed warnings or worst, less interest in documenting the code. Isn't
it the case that some of these higher-control warnings don't really have
much effect in real life? And shoukd W=1 become the default, are we going
to be able to enforce that level of control?

If W=1 is decided to be the best approach I'm happy to help with that.

BR,
Jani.

Thanks,
Carlos

On Wed, Aug 16, 2023 at 10:12:56AM -0500, Carlos Bilbao wrote:

On 8/16/23 06:01, Jani Nikula wrote:

quoted

What I'd hope for is build system support to enable W=1
compiler/kernel-doc warnings for a subdir with a few lines at most,
instead of duplicating and copy-pasting tens of lines from
scripts/Makefile.extrawarn like we have to do now.

That sounds feasible but, well, I actually proposed the opposite approach.
I wanted to "relax" the warnings (see RFC Subject) rather than making them
more strict by default.

My concern is that W=1 (by default) may theoretically result in a clean
`make htmldocs` but it won't in practice. Not all developers prioritize
good documentation like the folks from i915, and that may lead to
unaddressed warnings or worst, less interest in documenting the code. Isn't
it the case that some of these higher-control warnings don't really have
much effect in real life? And shoukd W=1 become the default, are we going
to be able to enforce that level of control?

I wasn't proposing making W=1 builds the default; I was proposing
running kernel-doc -none at all levels.

On 8/16/23 10:15, Matthew Wilcox wrote:

On Wed, Aug 16, 2023 at 10:12:56AM -0500, Carlos Bilbao wrote:

quoted

On 8/16/23 06:01, Jani Nikula wrote:

quoted

What I'd hope for is build system support to enable W=1
compiler/kernel-doc warnings for a subdir with a few lines at most,
instead of duplicating and copy-pasting tens of lines from
scripts/Makefile.extrawarn like we have to do now.

That sounds feasible but, well, I actually proposed the opposite approach.
I wanted to "relax" the warnings (see RFC Subject) rather than making them
more strict by default.

My concern is that W=1 (by default) may theoretically result in a clean
`make htmldocs` but it won't in practice. Not all developers prioritize
good documentation like the folks from i915, and that may lead to
unaddressed warnings or worst, less interest in documenting the code. Isn't
it the case that some of these higher-control warnings don't really have
much effect in real life? And shoukd W=1 become the default, are we going
to be able to enforce that level of control?

I wasn't proposing making W=1 builds the default; I was proposing
running kernel-doc -none at all levels.

More strict warning level, right? My concern is the same.

Thanks,
Carlos

On Wed, Aug 16, 2023 at 10:21:07AM -0500, Carlos Bilbao wrote:

On 8/16/23 10:15, Matthew Wilcox wrote:

quoted

I wasn't proposing making W=1 builds the default; I was proposing
running kernel-doc -none at all levels.

More strict warning level, right? My concern is the same.

The problem you're seeing with kernel-doc warning about undocumented
fields / parameters is due to people not running kernel-doc -none.
So I'm proposing this:

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 82e3fb19fdaf..52f57c0c5227 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build

@@ -100,11 +100,9 @@ else ifeq ($(KBUILD_CHECKSRC),2)
         cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
 endif

-ifneq ($(KBUILD_EXTRA_WARN),)
-  cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $(KDOCFLAGS) \
+cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $(KDOCFLAGS) \
         $(if $(findstring 2, $(KBUILD_EXTRA_WARN)), -Wall) \
         $<
-endif

 # Compile C sources (.c)
 # ---------------------------------------------------------------------------

Now everybody runs kernel-doc -none on every build and you don't get to
see that problem any more.

On 8/16/23 10:57, Matthew Wilcox wrote:

Understood, that should do it. I will also try to fix some of the legacy
warnings. Are you interested in submitting this patch? If so:

Reviewed-By: Carlos Bilbao <redacted>

Thanks,
Carlos