| Message ID | 20230602230014.a435aab03cee.I21ab3b54eeebd638676bead3b2f87417944e44f3@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] kernel-doc: don't let V=1 change outcome | expand |
On Sat, Jun 3, 2023 at 6:00 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > From: Johannes Berg <johannes.berg@intel.com> > > The kernel-doc script currently reports a number of issues > only in "verbose" mode, but that's initialized from V=1 > (via KBUILD_VERBOSE), so if you use KDOC_WERROR=1 then > adding V=1 might actually break the build. This is rather > unexpected. Agree. > > Change kernel-doc to not change its behaviour wrt. errors > (or warnings) when verbose mode is enabled, but rather add > separate warning flags (and -Wall) for it. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > scripts/kernel-doc | 41 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 36 insertions(+), 5 deletions(-) > > diff --git a/scripts/kernel-doc b/scripts/kernel-doc > index 2486689ffc7b..1eb1819fbe13 100755 > --- a/scripts/kernel-doc > +++ b/scripts/kernel-doc > @@ -23,7 +23,7 @@ kernel-doc - Print formatted kernel documentation to stdout > > =head1 SYNOPSIS > > - kernel-doc [-h] [-v] [-Werror] > + kernel-doc [-h] [-v] [-Werror] [-Wreturn] [-Wshort-description] [-Wcontents-before-sections] [-Wall] > [ -man | > -rst [-sphinx-version VERSION] [-enable-lineno] | > -none > @@ -133,6 +133,9 @@ my $dohighlight = ""; > > my $verbose = 0; > my $Werror = 0; > +my $Wreturn = 0; > +my $Wshort_desc = 0; > +my $Wcontents_before_sections = 0; > my $output_mode = "rst"; > my $output_preformatted = 0; > my $no_doc_sections = 0; > @@ -191,6 +194,24 @@ if (defined($ENV{'KDOC_WERROR'})) { > $Werror = "$ENV{'KDOC_WERROR'}"; > } > > +if (defined($ENV{'KDOC_WRETURN'})) { > + $Wreturn = "$ENV{'KDOC_WRETURN'}"; > +} > + > +if (defined($ENV{'KDOC_WSHORT_DESC'})) { > + $Wshort_desc = "$ENV{'KDOC_WSHORT_DESC'}"; > +} > + > +if (defined($ENV{'KDOC_WCONTENTS_BEFORE_SECTION'})) { > + $Wcontents_before_sections = "$ENV{'KDOC_WCONTENTS_BEFORE_SECTION'}"; > +} > + > +if (defined($ENV{'KDOC_WALL'})) { > + $Wreturn = "$ENV{'KDOC_WALL'}"; > + $Wshort_desc = "$ENV{'KDOC_WALL'}"; > + $Wcontents_before_sections = "$ENV{'KDOC_WALL'}"; > +} Adding an environment variable to each of them is tedious. If you enable -Wall via the command line option, these lines are unneeded? For example, ifneq ($(KBUILD_EXTRA_WARN),) cmd_checkdoc = $(srctree)/scripts/kernel-doc -none \ $(if $(findstring 2, $(KBUILD_EXTRA_WARN)), -Wall) $< endif
On Mon, 2023-06-05 at 09:36 +0900, Masahiro Yamada wrote: > > +if (defined($ENV{'KDOC_WRETURN'})) { > > + $Wreturn = "$ENV{'KDOC_WRETURN'}"; > > +} > > + > > +if (defined($ENV{'KDOC_WSHORT_DESC'})) { > > + $Wshort_desc = "$ENV{'KDOC_WSHORT_DESC'}"; > > +} > > + > > +if (defined($ENV{'KDOC_WCONTENTS_BEFORE_SECTION'})) { > > + $Wcontents_before_sections = "$ENV{'KDOC_WCONTENTS_BEFORE_SECTION'}"; > > +} > > + > > +if (defined($ENV{'KDOC_WALL'})) { > > + $Wreturn = "$ENV{'KDOC_WALL'}"; > > + $Wshort_desc = "$ENV{'KDOC_WALL'}"; > > + $Wcontents_before_sections = "$ENV{'KDOC_WALL'}"; > > +} > > > > Adding an environment variable to each of them is tedious. Agree. And adding one for -Wall is especially tedious because you have to spell out the list of affected warnings in two places :) > If you enable -Wall via the command line option, > these lines are unneeded? > > For example, > > ifneq ($(KBUILD_EXTRA_WARN),) > cmd_checkdoc = $(srctree)/scripts/kernel-doc -none \ > $(if $(findstring 2, $(KBUILD_EXTRA_WARN)), -Wall) $< > endif > Yes, that should be possible. I feel like maybe we should still have individual settings for the three different classes, because you might want to have -Wshort-desc without the extra -Wcontents-before-sections (which I thought about removing entirely, kernel-doc seems to parse just fine that way, what's the point of it?) But we could even move the env var handling _completely_ to the Makefile if you don't mind, and then we don't have to have two places in the script that need to be aligned all the time. johannes
On Mon, Jun 5, 2023 at 5:19 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Mon, 2023-06-05 at 09:36 +0900, Masahiro Yamada wrote: > > > > +if (defined($ENV{'KDOC_WRETURN'})) { > > > + $Wreturn = "$ENV{'KDOC_WRETURN'}"; > > > +} > > > + > > > +if (defined($ENV{'KDOC_WSHORT_DESC'})) { > > > + $Wshort_desc = "$ENV{'KDOC_WSHORT_DESC'}"; > > > +} > > > + > > > +if (defined($ENV{'KDOC_WCONTENTS_BEFORE_SECTION'})) { > > > + $Wcontents_before_sections = "$ENV{'KDOC_WCONTENTS_BEFORE_SECTION'}"; > > > +} > > > + > > > +if (defined($ENV{'KDOC_WALL'})) { > > > + $Wreturn = "$ENV{'KDOC_WALL'}"; > > > + $Wshort_desc = "$ENV{'KDOC_WALL'}"; > > > + $Wcontents_before_sections = "$ENV{'KDOC_WALL'}"; > > > +} > > > > > > > > Adding an environment variable to each of them is tedious. > > Agree. And adding one for -Wall is especially tedious because you have > to spell out the list of affected warnings in two places :) > > > If you enable -Wall via the command line option, > > these lines are unneeded? > > > > For example, > > > > ifneq ($(KBUILD_EXTRA_WARN),) > > cmd_checkdoc = $(srctree)/scripts/kernel-doc -none \ > > $(if $(findstring 2, $(KBUILD_EXTRA_WARN)), -Wall) $< > > endif > > > > Yes, that should be possible. > > I feel like maybe we should still have individual settings for the three > different classes, because you might want to have -Wshort-desc without > the extra -Wcontents-before-sections (which I thought about removing > entirely, kernel-doc seems to parse just fine that way, what's the point > of it?) > > But we could even move the env var handling _completely_ to the Makefile > if you don't mind, and then we don't have to have two places in the > script that need to be aligned all the time. Yes, probably that will be a good idea.
diff --git a/scripts/kernel-doc b/scripts/kernel-doc index 2486689ffc7b..1eb1819fbe13 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -23,7 +23,7 @@ kernel-doc - Print formatted kernel documentation to stdout =head1 SYNOPSIS - kernel-doc [-h] [-v] [-Werror] + kernel-doc [-h] [-v] [-Werror] [-Wreturn] [-Wshort-description] [-Wcontents-before-sections] [-Wall] [ -man | -rst [-sphinx-version VERSION] [-enable-lineno] | -none @@ -133,6 +133,9 @@ my $dohighlight = ""; my $verbose = 0; my $Werror = 0; +my $Wreturn = 0; +my $Wshort_desc = 0; +my $Wcontents_before_sections = 0; my $output_mode = "rst"; my $output_preformatted = 0; my $no_doc_sections = 0; @@ -191,6 +194,24 @@ if (defined($ENV{'KDOC_WERROR'})) { $Werror = "$ENV{'KDOC_WERROR'}"; } +if (defined($ENV{'KDOC_WRETURN'})) { + $Wreturn = "$ENV{'KDOC_WRETURN'}"; +} + +if (defined($ENV{'KDOC_WSHORT_DESC'})) { + $Wshort_desc = "$ENV{'KDOC_WSHORT_DESC'}"; +} + +if (defined($ENV{'KDOC_WCONTENTS_BEFORE_SECTION'})) { + $Wcontents_before_sections = "$ENV{'KDOC_WCONTENTS_BEFORE_SECTION'}"; +} + +if (defined($ENV{'KDOC_WALL'})) { + $Wreturn = "$ENV{'KDOC_WALL'}"; + $Wshort_desc = "$ENV{'KDOC_WALL'}"; + $Wcontents_before_sections = "$ENV{'KDOC_WALL'}"; +} + # Generated docbook code is inserted in a template at a point where # docbook v3.1 requires a non-zero sequence of RefEntry's; see: # https://www.oasis-open.org/docbook/documentation/reference/html/refentry.html @@ -318,6 +339,16 @@ while ($ARGV[0] =~ m/^--?(.*)/) { $verbose = 1; } elsif ($cmd eq "Werror") { $Werror = 1; + } elsif ($cmd eq "Wreturn") { + $Wreturn = 1; + } elsif ($cmd eq "Wshort-desc") { + $Wshort_desc = 1; + } elsif ($cmd eq "Wcontents-before-sections") { + $Wcontents_before_sections = 1; + } elsif ($cmd eq "Wall") { + $Wreturn = 1; + $Wshort_desc = 1; + $Wcontents_before_sections = 1; } elsif (($cmd eq "h") || ($cmd eq "help")) { pod2usage(-exitval => 0, -verbose => 2); } elsif ($cmd eq 'no-doc-sections') { @@ -1748,9 +1779,9 @@ sub dump_function($$) { # This check emits a lot of warnings at the moment, because many # functions don't have a 'Return' doc section. So until the number # of warnings goes sufficiently down, the check is only performed in - # verbose mode. + # -Wreturn mode. # TODO: always perform the check. - if ($verbose && !$noret) { + if ($Wreturn && !$noret) { check_return_section($file, $declaration_name, $return_type); } @@ -2054,7 +2085,7 @@ sub process_name($$) { $state = STATE_NORMAL; } - if (($declaration_purpose eq "") && $verbose) { + if (($declaration_purpose eq "") && $Wshort_desc) { emit_warning("${file}:$.", "missing initial short description on line:\n$_"); } @@ -2103,7 +2134,7 @@ sub process_body($$) { } if (($contents ne "") && ($contents ne "\n")) { - if (!$in_doc_sect && $verbose) { + if (!$in_doc_sect && $Wcontents_before_sections) { emit_warning("${file}:$.", "contents before sections\n"); } dump_section($file, $section, $contents);