Re: Make all Perl warnings fatal

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Make all Perl warnings fatal
Date: 2023-08-21 06:20:08
Message-ID: 47ec651c-7be2-4a8a-85de-293be1904a8c@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

To avoid a complete bloodbath on cfbot, here is an updated patch set
that includes a workaround for the getprotobyname() issue mentioned below.

On 10.08.23 07:58, Peter Eisentraut wrote:
> 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
v2-0001-Make-all-Perl-warnings-fatal.patch text/plain 130.4 KB
v2-0002-Avoid-use-of-Perl-getprotobyname.patch text/plain 1.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gurjeet Singh 2023-08-21 06:32:30 Re: Doc limitation update proposal: include out-of-line OID usage per TOAST-ed columns
Previous Message Masahiro Ikeda 2023-08-21 04:38:45 Re: Fix pg_stat_reset_single_table_counters function