Thread: perl checking
These two small patches allow us to run "perl -cw" cleanly on all our
perl code.
One patch silences a warning from convutils.pl about the unportability
of the literal 0x100000000. We've run for many years without this giving
us a problem, so I think we can turn the warning off pretty safely.
The other patch provides a dummy library that emulates just enough of
the Win32 perl infrastructure to allow us to run these checks. That
means that Unix-based developers who might want to make changes in the
msvc code can actually run a check against their code without having to
put it on a Windows machine. The invocation goes like this (to check
Mkvcbuild.pl for example):
PERL5LIB=src/tools/msvc/dummylib perl -cw src/tools/Mkvcbuild.pm
This also allows us to check src/tools/win32tzlist.pl.
In due course I'll submit a script to automate this syntax checking.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 5/18/18 14:02, Andrew Dunstan wrote: > These two small patches allow us to run "perl -cw" cleanly on all our > perl code. It's not clear to me what that really means. My understanding is that perl "warnings" are primarily a run-time instrument, unlike 'use strict' and perl -c. I have been playing with a private branch that adds 'use warnings' next to 'use strict' across the perl scripts, and there are a number of warnings that pop up at run time. The fact that you get even more warnings at compile time makes me wonder. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 05/18/2018 09:05 PM, Peter Eisentraut wrote: > On 5/18/18 14:02, Andrew Dunstan wrote: >> These two small patches allow us to run "perl -cw" cleanly on all our >> perl code. > It's not clear to me what that really means. My understanding is that > perl "warnings" are primarily a run-time instrument, unlike 'use strict' > and perl -c. I have been playing with a private branch that adds 'use > warnings' next to 'use strict' across the perl scripts, and there are a > number of warnings that pop up at run time. The fact that you get even > more warnings at compile time makes me wonder. > Mike Blackwell is working on some things that will help us lower the severity of our perlcritic checks. One of those things will almost certainly be to add "use warnings;" in quite a few places, so let's make sure we don't duplicate effort. Essentially "perl -cw" will make dure it can comoile the file and then print warnings about those things it can detect at compile time. I have found it a useful tool. More importantly, there are several files in our Windows suite that a Unix-based developer can't check even for compilation success, let alone warnings, because they refer to libraries that only exist on Windows. That's what the tiny dummy library is designed to fix. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
At Fri, 18 May 2018 14:02:39 -0400, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote in <5a6d6de8-cff8-1ffb-946c-ccf381800ea1@2ndQuadrant.com> > > These two small patches allow us to run "perl -cw" cleanly on all our > perl code. > > > One patch silences a warning from convutils.pl about the unportability > of the literal 0x100000000. We've run for many years without this > giving us a problem, so I think we can turn the warning off pretty > safely. It was introduced by aeed17d000 (in 2017). The history of the file is rather short. Over 32-bit values do not apperar as a character so there's no problem in ignoring the warning for now, but can't we use bigint to silence it instead? > The other patch provides a dummy library that emulates just enough of > the Win32 perl infrastructure to allow us to run these checks. That > means that Unix-based developers who might want to make changes in the > msvc code can actually run a check against their code without having > to put it on a Windows machine. The invocation goes like this (to > check Mkvcbuild.pl for example): > > > PERL5LIB=src/tools/msvc/dummylib perl -cw src/tools/Mkvcbuild.pm > > > This also allows us to check src/tools/win32tzlist.pl. > > > In due course I'll submit a script to automate this syntax checking. > > > cheers > > > andrew regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On 05/22/2018 04:11 AM, Kyotaro HORIGUCHI wrote: > At Fri, 18 May 2018 14:02:39 -0400, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote in <5a6d6de8-cff8-1ffb-946c-ccf381800ea1@2ndQuadrant.com> >> These two small patches allow us to run "perl -cw" cleanly on all our >> perl code. >> >> >> One patch silences a warning from convutils.pl about the unportability >> of the literal 0x100000000. We've run for many years without this >> giving us a problem, so I think we can turn the warning off pretty >> safely. > It was introduced by aeed17d000 (in 2017). The history of the > file is rather short. Over 32-bit values do not apperar as a > character so there's no problem in ignoring the warning for now, > but can't we use bigint to silence it instead? > It would impose an additional dependency. bigint isn't installed by default on many systems AFAICT, so I think we'd need a better reason than this to require it. I was a little optimistic about claiming that 'perl -cw' would run cleanly with these two patches - there's a little remediation that will be required in the src/msvc/tools directory. These patches at least let it run to completion. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 05/22/2018 04:11 AM, Kyotaro HORIGUCHI wrote:
>> At Fri, 18 May 2018 14:02:39 -0400, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote in
<5a6d6de8-cff8-1ffb-946c-ccf381800ea1@2ndQuadrant.com>
>>> One patch silences a warning from convutils.pl about the unportability
>>> of the literal 0x100000000. We've run for many years without this
>>> giving us a problem, so I think we can turn the warning off pretty
>>> safely.
>> It was introduced by aeed17d000 (in 2017). The history of the
>> file is rather short. Over 32-bit values do not apperar as a
>> character so there's no problem in ignoring the warning for now,
>> but can't we use bigint to silence it instead?
> It would impose an additional dependency. bigint isn't installed by
> default on many systems AFAICT, so I think we'd need a better reason
> than this to require it.
I agree with not adding a dependency (although FWIW, bigint does seem
to be there in my minimal perl setups). But can't we fix it like this:
- elsif ($in < 0x100000000)
+ elsif ($in <= 0xffffffff)
At least in a quick test here, "-cw" doesn't moan about 0xffffffff.
For consistency, the other arms of the "if" should be adjusted
similarly.
regards, tom lane
On 05/22/2018 10:09 AM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 05/22/2018 04:11 AM, Kyotaro HORIGUCHI wrote: >>> At Fri, 18 May 2018 14:02:39 -0400, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote in <5a6d6de8-cff8-1ffb-946c-ccf381800ea1@2ndQuadrant.com> >>>> One patch silences a warning from convutils.pl about the unportability >>>> of the literal 0x100000000. We've run for many years without this >>>> giving us a problem, so I think we can turn the warning off pretty >>>> safely. >>> It was introduced by aeed17d000 (in 2017). The history of the >>> file is rather short. Over 32-bit values do not apperar as a >>> character so there's no problem in ignoring the warning for now, >>> but can't we use bigint to silence it instead? >> It would impose an additional dependency. bigint isn't installed by >> default on many systems AFAICT, so I think we'd need a better reason >> than this to require it. > I agree with not adding a dependency (although FWIW, bigint does seem > to be there in my minimal perl setups). But can't we fix it like this: > > - elsif ($in < 0x100000000) > + elsif ($in <= 0xffffffff) > > At least in a quick test here, "-cw" doesn't moan about 0xffffffff. > > For consistency, the other arms of the "if" should be adjusted > similarly. > > Yeah. I tested this on the oldest 32 but perls I could find, the msys and activestate perls on the XP machine that runs frogmouth and friends. Even though they both have an ivsize of 4, the arithmetic seems to work properly. Perhaps they store larger numbers as doubles, which you should be able to do exactly up to about 52 bit integers. The other 32 bit machine I have is an Ubuntu 16.04 VM, but there the perl has an ivsize of 8, so of course it does the right thing. We don't normally use these scripts anyway, so I'll go with this suggestion without further investigation. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
At Tue, 22 May 2018 15:02:46 -0400, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote in <a70c49ec-d816-9fd6-1565-38fb20cc7206@2ndQuadrant.com> > > - elsif ($in < 0x100000000) > > + elsif ($in <= 0xffffffff) This is one of my thougts and the reason for regarding it sour is the following. > > For consistency, the other arms of the "if" should be adjusted > > similarly. > Yeah. I tested this on the oldest 32 but perls I could find, the msys > and activestate perls on the XP machine that runs frogmouth and > friends. Even though they both have an ivsize of 4, the arithmetic > seems to work properly. Perhaps they store larger numbers as doubles, > which you should be able to do exactly up to about 52 bit > integers. The other 32 bit machine I have is an Ubuntu 16.04 VM, but > there the perl has an ivsize of 8, so of course it does the right > thing. > > We don't normally use these scripts anyway, so I'll go with this > suggestion without further investigation. Agreed. I'm fine with the direction. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On 05/18/2018 02:02 PM, Andrew Dunstan wrote:
>
> These two small patches allow us to run "perl -cw" cleanly on all our
> perl code.
>
>
> One patch silences a warning from convutils.pl about the unportability
> of the literal 0x100000000. We've run for many years without this
> giving us a problem, so I think we can turn the warning off pretty
> safely.
>
>
> The other patch provides a dummy library that emulates just enough of
> the Win32 perl infrastructure to allow us to run these checks. That
> means that Unix-based developers who might want to make changes in the
> msvc code can actually run a check against their code without having
> to put it on a Windows machine. The invocation goes like this (to
> check Mkvcbuild.pl for example):
>
>
> PERL5LIB=src/tools/msvc/dummylib perl -cw src/tools/Mkvcbuild.pm
>
>
> This also allows us to check src/tools/win32tzlist.pl.
>
>
> In due course I'll submit a script to automate this syntax checking.
>
>
>
Here is the latest version of the second patch, this time with warnings
about redefinition of some subroutines suppressed. These mostly occur
because in a few cases we have multiple packages in a single file.
This allows the following command to pass cleanly:
{ find . -type f -a -name '*.p[lm]' -print; find . -type f -perm
-100 -exec file {} \; -print | egrep -i ':.*perl[0-9]*\>' |cut -d:
-f1 ; } | sort -u |
PERL5LIB=src/test/perl:src/test/ssl:src/bin/pg_rewind:src/backend/catalog:src/backend/utils/mb/Unicode:src/tools/msvc/dummylib:src/tools/msvc
xargs -L 1 perl -cw
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On Sun, May 27, 2018 at 11:42 AM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
>
>
> On 05/18/2018 02:02 PM, Andrew Dunstan wrote:
>>
>>
>> These two small patches allow us to run "perl -cw" cleanly on all our perl
>> code.
>>
>>
>> One patch silences a warning from convutils.pl about the unportability of
>> the literal 0x100000000. We've run for many years without this giving us a
>> problem, so I think we can turn the warning off pretty safely.
>>
>>
>> The other patch provides a dummy library that emulates just enough of the
>> Win32 perl infrastructure to allow us to run these checks. That means that
>> Unix-based developers who might want to make changes in the msvc code can
>> actually run a check against their code without having to put it on a
>> Windows machine. The invocation goes like this (to check Mkvcbuild.pl for
>> example):
>>
>>
>> PERL5LIB=src/tools/msvc/dummylib perl -cw src/tools/Mkvcbuild.pm
>>
>>
>> This also allows us to check src/tools/win32tzlist.pl.
>>
>>
>> In due course I'll submit a script to automate this syntax checking.
>>
>>
>>
>
>
> Here is the latest version of the second patch, this time with warnings
> about redefinition of some subroutines suppressed. These mostly occur
> because in a few cases we have multiple packages in a single file.
>
> This allows the following command to pass cleanly:
>
> { find . -type f -a -name '*.p[lm]' -print; find . -type f -perm
> -100 -exec file {} \; -print | egrep -i ':.*perl[0-9]*\>' |cut -d:
> -f1 ; } | sort -u |
>
>
PERL5LIB=src/test/perl:src/test/ssl:src/bin/pg_rewind:src/backend/catalog:src/backend/utils/mb/Unicode:src/tools/msvc/dummylib:src/tools/msvc
> xargs -L 1 perl -cw
>
>
Attached version actually does what's advertised above.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services