Re: Make all Perl warnings fatal

Hello,

On 18/01/2024 10:52, Peter Eisentraut wrote:
 > Committed, thanks.

since this patch two .pl files without FATAL in "use warnings" have been 
committed to master:
src/test/recovery/t/043_wal_replay_wait.pl
src/test/modules/test_misc/t/006_signal_autovacuum.pl

They come from
commit 06c418e163e913966e17cb2d3fb1c5f8a8d58308
Author: Alexander Korotkov <akorotkov@postgresql.org>
Date:   Tue Apr 2 22:48:03 2024

     Implement pg_wal_replay_wait() stored procedure

and

commit d2b74882cab84b9f4fdce0f2f32e892ba9164f5c
Author: Michael Paquier <michael@paquier.xyz>
Date:   Tue Jul 16 04:05:46 2024

     Add tap test for pg_signal_autovacuum role

An obvious patch adding FATAL => 'all' is attached.

Cc Alexander Korotkov and Michael Paquier as committers of those commits.

-- 
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

P.S. For what it's worth, this is an adapted snippet from a bash script 
from our (postgrespro.com) internal CI used to detect those in 
REL_17_STABLE+. It detects:
1. .pl .pm files without "use warnings"
2. .pl .pm files where "use warnings" does not have FATAL => 'all'

EXIT_CODE=0
LOG=ci_perl_files_without_warnings.log
grep --include='*.p[lm]' -R -L -w "^\s*use warnings" . | \
   sed -e 's,^\./,,' | \
   grep -v -F src/pl/plperl/plc_trusted.pl \
   > "$LOG"
if [ -s "$LOG" ]; then
     N=$(wc -l < "$LOG");
     echo "ERROR: \"use warnings\" is missing in the following $N Perl 
files:";
     cat "$LOG";
     EXIT_CODE=1;
fi
# force "FATAL => 'all'" after any "use warnings" in Perl files
find . -name '*.p[lm]' -exec perl -i -p -e$'
   s/^(\s*)
     use \s+ warnings
     (?! \s+ FATAL \s* => \s* \'all\' \s* );
    /$1use warnings FATAL => \'all\';/x' {} \+;
PATCH_FILE=ci_perl_warnings.patch
git diff > "$PATCH_FILE"
if [ -s "$PATCH_FILE" ]; then
     N=$(grep ^diff "$PATCH_FILE" | wc -l);
     echo "ERROR: missing \"FATAL => 'all'\" in \"use warnings\" in the 
following $N files:";
     git status --porcelain | awk '/^ M/{print $2}';
     PATCH_URL="$CI_JOB_URL/artifacts/file/$PATCH_FILE";
     echo "NOTE: see $PATCH_URL for a suggested patch.";
     EXIT_CODE=1;
   fi
exit "$EXIT_CODE"

Please let me know if you think it's worth adapting into our general 
cirrus pipeline. I'm not quite sure how to fit it yet.

I've added src/pl/plperl/plc_trusted.pl as an exception to the "contains 
use warnings" check. It has require warnings, though. That's probably a 
reasonable exception.

P.P.S. REL_17_STABLE is fine: all use warnings do have FATAL => 'all'.