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(at)postgresql(dot)org>
Date: Tue Apr 2 22:48:03 2024

Implement pg_wal_replay_wait() stored procedure

and

commit d2b74882cab84b9f4fdce0f2f32e892ba9164f5c
Author: Michael Paquier <michael(at)paquier(dot)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'.