Package: automake-patches;
Reported by: Zack Weinberg <zack <at> owlfolio.org>
Date: Fri, 15 Dec 2023 20:45:02 UTC
Severity: normal
Tags: patch
Done: Karl Berry <karl <at> freefriends.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Karl Berry <karl <at> freefriends.org> Cc: tracker <at> debbugs.gnu.org Subject: bug#67841: closed ([PATCH] Clarify error messages for misuse of m4_warn and --help for -W.) Date: Sat, 23 Dec 2023 18:55:02 +0000
[Message part 1 (text/plain, inline)]
Your message dated Sat, 23 Dec 2023 11:54:35 -0700 with message-id <202312231854.3BNIsZPn004099 <at> freefriends.org> and subject line Re: [PATCH] Clarify error messages for misuse of m4_warn and --help for -W. has caused the debbugs.gnu.org bug report #67841, regarding [PATCH] Clarify error messages for misuse of m4_warn and --help for -W. to be marked as done. (If you believe you have received this mail in error, please contact help-debbugs <at> gnu.org.) -- 67841: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67841 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Zack Weinberg <zack <at> owlfolio.org> To: autoconf-patches <at> gnu.org, automake-patches <at> gnu.org Subject: [PATCH] Clarify error messages for misuse of m4_warn and --help for -W. Date: Fri, 15 Dec 2023 15:44:35 -0500In autoconf 2.69, the manual said that you could use “all” and the empty string as the first argument to m4_warn. As far as I can tell this was never actually true, and the manual was corrected in 2.70, but we still got a bug report from someone who tried it and got a confusing internal error from the guts of autom4te. This patch makes us issue a nice helpful user error, instead of a confusing internal error, when Autom4te::Channels::msg is called with a bogus channel argument. If the bogus channel is “all” or the empty string, that error is demoted to a -Wobsolete warning. If it is one of the other special tokens recognized by -W, we customize the error message, since someone might’ve thought that “none” being acceptable to -W meant it was also acceptable to m4_warn. The --help output for autoconf, autoheader, autom4te, and autoreconf is also adjusted to clarify that not all of the tokens recognized by -W count as warning categories. (Oddly enough, m4_warn([error], […]) actually issues an error, because the argument to m4_warn is passed straight through as the channel argument to Autom4te::Channels::msg. Since neither m4_errprintn(…) nor m4_fatal(…) does quite the same thing as m4_warn([error], […]), I propose to leave that alone for now and promote it to a proper, documented feature (probably spelled m4_error(…)) post-2.72.) Since this touches code shared between Autoconf and Automake, I’m not checking it in yet and I’m requesting comments from both sides of the fence. Also, there’s a perl 2.14ism in one place (s///a) which I need to figure out how to make 2.6-compatible before it can land. Addresses <https://savannah.gnu.org/support/?110872>. * lib/Autom4te/Channels.pm (msg): If the channel argument is invalid, don’t crash; report the mistake and use the ‘syntax’ channel. (report_bad_channel): New function for reporting invalid channels. * lib/Autom4te/ChannelDefs.pm (usage): Clarify that the list of warning categories is exhaustive, and that “all”, “none”, “no-CATEGORY”, and “error” are not warning categories. * bin/autoconf.in, bin/autoheader.in, bin/autom4te.in, bin/autoreconf.in: Tweak --help text. * tests/m4sugar.at (m4@&t <at> _warn (bad categories)): New test. --- bin/autoconf.in | 1 + bin/autoheader.in | 1 + bin/autom4te.in | 1 + bin/autoreconf.in | 10 +++--- lib/Autom4te/ChannelDefs.pm | 10 +++--- lib/Autom4te/Channels.pm | 48 +++++++++++++++++++++++++++- tests/m4sugar.at | 63 +++++++++++++++++++++++++++++++++++++ 7 files changed, 123 insertions(+), 11 deletions(-) diff --git a/bin/autoconf.in b/bin/autoconf.in index 060a9a04..5f4a2e5c 100644 --- a/bin/autoconf.in +++ b/bin/autoconf.in @@ -64,6 +64,7 @@ Operation modes: -f, --force consider all files obsolete -o, --output=FILE save output in FILE (stdout is the default) -W, --warnings=CATEGORY report the warnings falling in CATEGORY + (comma-separated list accepted) " . Autom4te::ChannelDefs::usage . " diff --git a/bin/autoheader.in b/bin/autoheader.in index 4afa5f13..f5af0df2 100644 --- a/bin/autoheader.in +++ b/bin/autoheader.in @@ -74,6 +74,7 @@ or else 'configure.in'. -d, --debug don\'t remove temporary files -f, --force consider all files obsolete -W, --warnings=CATEGORY report the warnings falling in CATEGORY + (comma-separated list accepted) " . Autom4te::ChannelDefs::usage . " diff --git a/bin/autom4te.in b/bin/autom4te.in index 8957a6c9..2e4e7d52 100644 --- a/bin/autom4te.in +++ b/bin/autom4te.in @@ -160,6 +160,7 @@ Operation modes: -o, --output=FILE save output in FILE (defaults to '-', stdout) -f, --force don't rely on cached values -W, --warnings=CATEGORY report the warnings falling in CATEGORY + (comma-separated list accepted) -l, --language=LANG specify the set of M4 macros to use -C, --cache=DIRECTORY preserve results for future runs in DIRECTORY --no-cache disable the cache diff --git a/bin/autoreconf.in b/bin/autoreconf.in index c6077efe..285d0f49 100644 --- a/bin/autoreconf.in +++ b/bin/autoreconf.in @@ -84,20 +84,18 @@ Operation modes: --no-recursive don't rebuild sub-packages -s, --symlink with -i, install symbolic links instead of copies -m, --make when applicable, re-run ./configure && make - -W, --warnings=CATEGORY report the warnings falling in CATEGORY [syntax] + -W, --warnings=CATEGORY report the warnings falling in CATEGORY + (comma-separated list accepted) " . Autom4te::ChannelDefs::usage . " -The environment variable 'WARNINGS' is honored. Some subtools might -support other warning types, using 'all' is encouraged. - Library directories: -B, --prepend-include=DIR prepend directory DIR to search path -I, --include=DIR append directory DIR to search path The environment variables AUTOCONF, ACLOCAL, AUTOHEADER, AUTOM4TE, -AUTOMAKE, AUTOPOINT, GTKDOCIZE, INTLTOOLIZE, LIBTOOLIZE, M4, and MAKE -are honored. +AUTOMAKE, AUTOPOINT, GTKDOCIZE, INTLTOOLIZE, LIBTOOLIZE, M4, MAKE, +and WARNINGS are honored. Report bugs to <bug-autoconf\@gnu.org>. GNU Autoconf home page: <https://www.gnu.org/software/autoconf/>. diff --git a/lib/Autom4te/ChannelDefs.pm b/lib/Autom4te/ChannelDefs.pm index 85ea6f82..9682e8db 100644 --- a/lib/Autom4te/ChannelDefs.pm +++ b/lib/Autom4te/ChannelDefs.pm @@ -197,7 +197,7 @@ Return the warning category descriptions. sub usage () { - return "Warning categories include: + return "Warning categories are: cross cross compilation issues gnu GNU coding standards (default in gnu and gnits modes) obsolete obsolete features or constructions (default) @@ -207,10 +207,12 @@ sub usage () extra-portability extra portability issues related to obscure tools syntax dubious syntactic constructs (default) unsupported unsupported or incomplete features (default) - all all the warnings - no-CATEGORY turn off warnings in CATEGORY + +-W also understands: + all turn on all the warnings none turn off all the warnings - error treat warnings as errors"; + no-CATEGORY turn off warnings in CATEGORY + error treat all enabled warnings as errors"; } =item C<prog_error ($MESSAGE, [%OPTIONS])> diff --git a/lib/Autom4te/Channels.pm b/lib/Autom4te/Channels.pm index a2ed0a0b..634a61a6 100644 --- a/lib/Autom4te/Channels.pm +++ b/lib/Autom4te/Channels.pm @@ -628,7 +628,13 @@ sub msg ($$;$%) $location = ''; } - confess "unknown channel $channel" unless exists $channels{$channel}; + if (!exists $channels{$channel}) + { + # This can happen as a result of e.g. m4_warn([nonsense], [message]) + # so it should not crash. + report_bad_channel($channel, $location); + $channel = 'syntax'; + } my %opts = %{$channels{$channel}}; _merge_options (%opts, %options); @@ -662,6 +668,46 @@ sub msg ($$;$%) } } +sub report_bad_channel ($$) +{ + my ($channel, $location) = @_; + my $message; + my $report_as = 'error'; + + # quotemeta is both too aggressive (e.g. it escapes '-') and + # too generous (it turns control characters into \ + themselves, + # not into symbolic escapes). + my $q_channel = $channel; + $q_channel =~ s/\\/\\\\/g; + $q_channel =~ s/([^\x20-\x7e])/"\\x".sprintf("%02x", ord($1))/aeg; + $q_channel =~ s/(?=[\"\$\'\@\`])/\\/g; + $q_channel = '"' . $q_channel . '"'; + + if ($channel eq '' || $channel eq 'all') + { + # Prior to version 2.70, the Autoconf manual said it was valid to use + # "all" and the empty string as the category argument to m4_warn, so + # don't treat those cases as errors. + $report_as = 'obsolete'; + $message = "use of $q_channel as a diagnostic category is obsolete\n"; + $message .= "(see autom4te --help for a list of valid categories)"; + } + elsif ($channel eq 'none' + || ($channel =~ /^no-/ && exists $channels{substr($channel, 3)})) + { + # Also recognize "none" and "no-[category]", as someone might have + # thought anything acceptable to -W is also acceptable to m4_warn. + # Note: m4_warn([error], [...]) does actually issue an error. + $message = "-W accepts $q_channel, but it is not a diagnostic category"; + } + else + { + $message = "unknown diagnostic category " . $q_channel; + } + + msg $report_as, $location, $message; +} + =item C<setup_channel ($channel, %options)> diff --git a/tests/m4sugar.at b/tests/m4sugar.at index 2f1dbe37..9a6be0d2 100644 --- a/tests/m4sugar.at +++ b/tests/m4sugar.at @@ -244,6 +244,7 @@ AT_CLEANUP ## --------- ## AT_SETUP([m4@&t <at> _warn]) +AT_KEYWORDS([m4@&t <at> _warn]) AT_DATA_M4SUGAR([script.4s], [[m4_init @@ -297,6 +298,68 @@ script.4s:8: the top level AT_CLEANUP +## ----------------------------------------------- ## +## Bad m4_warn categories (Autoconf bug #110872). ## +## ----------------------------------------------- ## + +AT_SETUP([m4@&t <at> _warn (bad categories)]) +AT_KEYWORDS([m4@&t <at> _warn]) + +AT_DATA_M4SUGAR([script.4s], +[[m4_init +m4_warn([], [empty category])dnl +m4_warn([bogus], [invalid category])dnl +m4_warn([!"%^&*], [line noise category])dnl " unconfuse editor +m4_warn([all], ['all' used as a category])dnl +m4_warn([none], ['none' used as a category])dnl +m4_warn([no-syntax], ['no-syntax' used as a category])dnl +m4_warn([no-bogus], ['no-bogus' used as a category])dnl +m4_warn([error], [oddly enough, this works])dnl +]]) + +AT_CHECK_M4SUGAR([-o- -Wnone], 1, [], +[script.4s:3: error: unknown diagnostic category "bogus" +script.4s:4: error: unknown diagnostic category "!\"%^&*" +script.4s:6: error: -W accepts "none", but it is not a diagnostic category +script.4s:7: error: -W accepts "no-syntax", but it is not a diagnostic category +script.4s:8: error: unknown diagnostic category "no-bogus" +script.4s:9: error: oddly enough, this works +]) + +AT_CHECK_M4SUGAR([-o- -Wnone,obsolete], 1, [], +[script.4s:2: warning: use of "" as a diagnostic category is obsolete +script.4s:2: (see autom4te --help for a list of valid categories) +script.4s:3: error: unknown diagnostic category "bogus" +script.4s:4: error: unknown diagnostic category "!\"%^&*" +script.4s:5: warning: use of "all" as a diagnostic category is obsolete +script.4s:5: (see autom4te --help for a list of valid categories) +script.4s:6: error: -W accepts "none", but it is not a diagnostic category +script.4s:7: error: -W accepts "no-syntax", but it is not a diagnostic category +script.4s:8: error: unknown diagnostic category "no-bogus" +script.4s:9: error: oddly enough, this works +]) + +AT_CHECK_M4SUGAR([-o- -Wnone,obsolete,syntax], 1, [], +[script.4s:2: warning: use of "" as a diagnostic category is obsolete +script.4s:2: (see autom4te --help for a list of valid categories) +script.4s:2: warning: empty category +script.4s:3: error: unknown diagnostic category "bogus" +script.4s:3: warning: invalid category +script.4s:4: error: unknown diagnostic category "!\"%^&*" +script.4s:4: warning: line noise category +script.4s:5: warning: use of "all" as a diagnostic category is obsolete +script.4s:5: (see autom4te --help for a list of valid categories) +script.4s:5: warning: 'all' used as a category +script.4s:6: error: -W accepts "none", but it is not a diagnostic category +script.4s:6: warning: 'none' used as a category +script.4s:7: error: -W accepts "no-syntax", but it is not a diagnostic category +script.4s:7: warning: 'no-syntax' used as a category +script.4s:8: error: unknown diagnostic category "no-bogus" +script.4s:8: warning: 'no-bogus' used as a category +script.4s:9: error: oddly enough, this works +]) + +AT_CLEANUP ## ----------------- ## ## m4_divert_stack. ## -- 2.41.0
[Message part 3 (message/rfc822, inline)]
From: Karl Berry <karl <at> freefriends.org> To: 67841 <at> debbugs.gnu.org Subject: Re: [PATCH] Clarify error messages for misuse of m4_warn and --help for -W. Date: Sat, 23 Dec 2023 11:54:35 -0700This was changed in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67971 and autoconf-2.72 has been released, so closing. Without Jacob's suggested use of encode(), I see (Jacob, thanks for the explanations), but Zack, I'm not sure you intended this bug to track that. Feel free to reopen if so. Thanks, Karl
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.