Re: [HACKERS] warning handling in Perl scripts
On 06/24/2012 04:05 PM, Robert Haas wrote:
On Sun, Jun 24, 2012 at 2:40 PM, Peter Eisentrautpete...@gmx.net wrote:
Every time I make a change to the structure of the catalog files,
genbki.pl produces a bunch of warnings (like Use of uninitialized value
in string eq at genbki.pl line ...), and produces corrupted output
files, that are then (possibly) detected later by the compiler. Also,
getting out of that is difficult because due to the complicated
dependency relationship between the involved files, you need to remove a
bunch of files manually, or clean everything. So error handling could
be better.
It seems that adding
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index ebc4825..7d66da9 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -19,6 +19,8 @@
use strict;
use warnings;
+local $SIG{__WARN__} = sub { die $_[0] };
+
my @input_files;
our @include_path;
my $output_path = '';
would address that.
Could that cause any other problems? Should it be added to all Perl
scripts?
This seems like a band-aid. How about if we instead add whatever
error-handling the script is missing, so that it produces an
appropriate, human-readable error message?
I realise I'm late to this party, but I'm with Robert. The root cause of
the errors should be fixed.
That's not to say that making warnings fatal might not also be a good
idea as a general defense mechanism.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
On Jun 27, 2012, at 4:07 PM, Andrew Dunstan wrote: I realise I'm late to this party, but I'm with Robert. The root cause of the errors should be fixed. That's not to say that making warnings fatal might not also be a good idea as a general defense mechanism. ISTM that if they are fatal, one will be more motivated to fix them. I think that was the point. David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
On sön, 2012-06-24 at 16:05 -0400, Robert Haas wrote:
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index ebc4825..7d66da9 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -19,6 +19,8 @@
use strict;
use warnings;
+local $SIG{__WARN__} = sub { die $_[0] };
+
my @input_files;
our @include_path;
my $output_path = '';
would address that.
Could that cause any other problems? Should it be added to all Perl
scripts?
This seems like a band-aid.
I'd think of it as a safety net.
How about if we instead add whatever error-handling the script is
missing, so that it produces an appropriate, human-readable error
message?
That could also be worthwhile, but I think given the audience of that
script and the complexity of the possible failures, it could be a very
large and aimless project. But there should still be a catch-all for
uncaught failures.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
Peter Eisentraut pete...@gmx.net writes:
On sön, 2012-06-24 at 16:05 -0400, Robert Haas wrote:
+local $SIG{__WARN__} = sub { die $_[0] };
This seems like a band-aid.
I'd think of it as a safety net.
+1 for the concept of turning warnings into errors, but is that really
the cleanest, most idiomatic way to do so in Perl? Sheesh.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
On Jun 25, 2012, at 3:35 PM, Tom Lane wrote:
+1 for the concept of turning warnings into errors, but is that really
the cleanest, most idiomatic way to do so in Perl? Sheesh.
It’s the most backward-compatible, but the most idiomatic way to do it
lexically is:
use warnings 'FATAL';
However, that works only for the current lexical scope. If there are warnings
in the code you are calling from the current scope, the use of `local
$SIG{__WARN__}` is required.
HTH,
David
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
Excerpts from David E. Wheeler's message of lun jun 25 11:23:34 -0400 2012:
On Jun 25, 2012, at 3:35 PM, Tom Lane wrote:
+1 for the concept of turning warnings into errors, but is that really
the cleanest, most idiomatic way to do so in Perl? Sheesh.
It’s the most backward-compatible, but the most idiomatic way to do it
lexically is:
use warnings 'FATAL';
However, that works only for the current lexical scope. If there are warnings
in the code you are calling from the current scope, the use of `local
$SIG{__WARN__}` is required.
So lets add 'FATAL' to the already existing use warnings lines in
Catalog.pm and genbki.pl.
I think the other files we should add this to are generate-errcodes.pl,
generate-plerrorcodes.pl, generate-spiexceptions.pl, Gen_fmgrtab.pl.
Maybe psql/create_help.pl too.
We have a bunch of files in ECPG and MSVC areas and others in src/tools;
not sure about those.
We also have gen_qsort_tuple.pl which amusingly does not even
use warnings.
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
On Jun 25, 2012, at 5:51 PM, Alvaro Herrera wrote:
However, that works only for the current lexical scope. If there are
warnings in the code you are calling from the current scope, the use of
`local $SIG{__WARN__}` is required.
So lets add 'FATAL' to the already existing use warnings lines in
Catalog.pm and genbki.pl.
I think the other files we should add this to are generate-errcodes.pl,
generate-plerrorcodes.pl, generate-spiexceptions.pl, Gen_fmgrtab.pl.
Maybe psql/create_help.pl too.
We have a bunch of files in ECPG and MSVC areas and others in src/tools;
not sure about those.
We also have gen_qsort_tuple.pl which amusingly does not even
use warnings.
Hrm, I think that `use warnings 'FATAL';` might only work for core warnings.
Which is annoying. I missed what was warning up-thread, but the most foolproof
way to make all warnings fatal is the originally suggested
local $SIG{__WARN__} = sub { die shift };
A *bit* cleaner is to use Carp::croak:
use Carp;
local $SIG{__WARN__} = \croak;
Or if you wanted to get a stack trace out of it, use Carp::confess:
use Carp;
local $SIG{__WARN__} = \confess;
Exception-handling in Perl is one of the few places that annoy me regularly.
Best,
David
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
David E. Wheeler da...@justatheory.com writes:
Hrm, I think that `use warnings 'FATAL';` might only work for core warnings.
Which is annoying. I missed what was warning up-thread, but the most
foolproof way to make all warnings fatal is the originally suggested
local $SIG{__WARN__} = sub { die shift };
Sigh, let's do it that way then.
A *bit* cleaner is to use Carp::croak:
use Carp;
local $SIG{__WARN__} = \croak;
Just as soon not add a new module dependency if we don't have to.
In this case, since we're not really expecting the warnings to get
thrown, it seems like there'd be little value added by doing so.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
On Mon, Jun 25, 2012 at 12:07:55PM -0400, Tom Lane wrote:
David E. Wheeler da...@justatheory.com writes:
Hrm, I think that `use warnings 'FATAL';` might only work for core
warnings. Which is annoying. I missed what was warning up-thread, but the
most foolproof way to make all warnings fatal is the originally suggested
local $SIG{__WARN__} = sub { die shift };
Sigh, let's do it that way then.
A *bit* cleaner is to use Carp::croak:
use Carp;
local $SIG{__WARN__} = \croak;
Just as soon not add a new module dependency if we don't have to.
Carp is core since Perl 5 [1994-10-17].
In this case, since we're not really expecting the warnings to get
thrown, it seems like there'd be little value added by doing so.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
-Ryan Kelly
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] warning handling in Perl scripts
Every time I make a change to the structure of the catalog files,
genbki.pl produces a bunch of warnings (like Use of uninitialized value
in string eq at genbki.pl line ...), and produces corrupted output
files, that are then (possibly) detected later by the compiler. Also,
getting out of that is difficult because due to the complicated
dependency relationship between the involved files, you need to remove a
bunch of files manually, or clean everything. So error handling could
be better.
It seems that adding
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index ebc4825..7d66da9 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -19,6 +19,8 @@
use strict;
use warnings;
+local $SIG{__WARN__} = sub { die $_[0] };
+
my @input_files;
our @include_path;
my $output_path = '';
would address that.
Could that cause any other problems? Should it be added to all Perl
scripts?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] warning handling in Perl scripts
On Sun, Jun 24, 2012 at 2:40 PM, Peter Eisentraut pete...@gmx.net wrote:
Every time I make a change to the structure of the catalog files,
genbki.pl produces a bunch of warnings (like Use of uninitialized value
in string eq at genbki.pl line ...), and produces corrupted output
files, that are then (possibly) detected later by the compiler. Also,
getting out of that is difficult because due to the complicated
dependency relationship between the involved files, you need to remove a
bunch of files manually, or clean everything. So error handling could
be better.
It seems that adding
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index ebc4825..7d66da9 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -19,6 +19,8 @@
use strict;
use warnings;
+local $SIG{__WARN__} = sub { die $_[0] };
+
my @input_files;
our @include_path;
my $output_path = '';
would address that.
Could that cause any other problems? Should it be added to all Perl
scripts?
This seems like a band-aid. How about if we instead add whatever
error-handling the script is missing, so that it produces an
appropriate, human-readable error message?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
