Make all Perl warnings fatal

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Make all Perl warnings fatal
Date: 2023-08-10 05:58:27
Message-ID: 06f899fd-1826-05ab-42d6-adeb1fd5e200@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

We have a lot of Perl scripts in the tree, mostly code generation and
TAP tests. Occasionally, these scripts produce warnings. These are
AFAICT always mistakes on the developer side (true positives). Typical
examples are warnings from genbki.pl or related when you make a mess in
the catalog files during development, or warnings from tests when they
massage a config file that looks different on different hosts, or
mistakes during merges (e.g., duplicate subroutine definitions), or just
mistakes that weren't noticed, because, you know, there is a lot of
output in a verbose build.

I wanted to figure put if we can catch these more reliably, in the style
of -Werror. AFAICT, there is no way to automatically turn all warnings
into fatal errors. But there is a way to do it per script, by replacing

use warnings;

by

use warnings FATAL => 'all';

See attached patch to try it out.

The documentation at <https://perldoc.perl.org/warnings#Fatal-Warnings>
appears to sort of hand-wave against doing that. Their argument appears
to be something like, the modules you use might in the future produce
additional warnings, thus breaking your scripts. On balance, I'd take
that risk, if it means I would notice the warnings in a more timely and
robust way. But that's just me at a moment in time.

Thoughts?

Now to some funny business. If you apply this patch, the Cirrus CI
Linux tasks will fail, because they get an undefined return from
getprotobyname() in PostgreSQL/Test/Cluster.pm. Huh? I need this patch
to get past that:

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm
b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 3fa679ff97..dfe7bc7b1a 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -1570,7 +1570,7 @@ sub can_bind
my ($host, $port) = @_;
my $iaddr = inet_aton($host);
my $paddr = sockaddr_in($port, $iaddr);
- my $proto = getprotobyname("tcp");
+ my $proto = getprotobyname("tcp") || 6;

socket(SOCK, PF_INET, SOCK_STREAM, $proto)
or die "socket failed: $!";

What is going on there? Does this host not have /etc/protocols, or
something like that?

There are also a couple of issues in the MSVC legacy build system that
would need to be tightened up in order to survive with fatal Perl
warnings. Obviously, there is a question whether it's worth spending
any time on that anymore.

Attachment Content-Type Size
0001-Make-all-Perl-warnings-fatal.patch text/plain 130.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-08-10 05:59:07 Re: Incorrect handling of OOM in WAL replay leading to data loss
Previous Message Kyotaro Horiguchi 2023-08-10 05:47:51 Re: Incorrect handling of OOM in WAL replay leading to data loss