automating perl compile time checking

Started by Andrew Dunstanover 8 years ago11 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t38821
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 27, 2026 at 08:20 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t38821_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t38821_1 && git checkout t38821_1

Patchset v1 (message #1) is on t38821_1

Jump to latest
#1Andrew Dunstan
andrew@dunslane.net

Here is a follow up patch to last weeks commit allowing all perl files
to be checked clean for compile time errors and warnings.

The patch contains a simple script to run the checks. The code that
finds perl files is put in a function in a single file that is sourced
by the three locations that need it.

The directory pgperlcritic is renamed to perlcheck, as it not contains
the new script as well as pgperlcritic.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

t38821_1
0001-Extend-and-slightly-refactor-perl-checking.patchtext/x-patch; name=0001-Extend-and-slightly-refactor-perl-checking.patchDownload+55-40
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Andrew Dunstan (#1)
Re: automating perl compile time checking

On 5 Jun 2018, at 16:31, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:

The patch contains a simple script to run the checks. The code that finds perl files is put in a function in a single file that is sourced by the three locations that need it.

+1 on centralizing the find-files function.

The directory pgperlcritic is renamed to perlcheck, as it not contains the new script as well as pgperlcritic.

This comment should say “perlcheck/..” and not “pgperlcritic/.." I assume:

--- /dev/null
+++ b/src/tools/perlcheck/pgperlcritic
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+# src/tools/pgperlcritic/pgperlcritic

cheers ./daniel

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Daniel Gustafsson (#2)
Re: automating perl compile time checking

On 06/05/2018 05:14 PM, Daniel Gustafsson wrote:

This comment should say “perlcheck/..” and not “pgperlcritic/.." I assume:

Thanks. will fix.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Daniel Gustafsson (#2)
Re: automating perl compile time checking

On 2018-Jun-05, Daniel Gustafsson wrote:

On 5 Jun 2018, at 16:31, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:

The patch contains a simple script to run the checks. The code that finds perl files is put in a function in a single file that is sourced by the three locations that need it.

+1 on centralizing the find-files function.

+1 on that. Why do we need to make the new find_perl_files file
executable, given it's always sourced? (I would have given a .sh
extension because it's a lib not an executable, but I suppose that's
just matter of taste; we certainly don't have a policy about it).

Looks fine to me either way.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#4)
Re: automating perl compile time checking

On 06/11/2018 02:33 PM, Alvaro Herrera wrote:

On 2018-Jun-05, Daniel Gustafsson wrote:

On 5 Jun 2018, at 16:31, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
The patch contains a simple script to run the checks. The code that finds perl files is put in a function in a single file that is sourced by the three locations that need it.

+1 on centralizing the find-files function.

+1 on that. Why do we need to make the new find_perl_files file
executable, given it's always sourced? (I would have given a .sh
extension because it's a lib not an executable, but I suppose that's
just matter of taste; we certainly don't have a policy about it).

Looks fine to me either way.

I've committed this, but I'm fine if people want to tweak the names. It
probably doesn't need to be executable.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#5)
Re: automating perl compile time checking

On 6/11/18 15:38, Andrew Dunstan wrote:

On 5 Jun 2018, at 16:31, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
The patch contains a simple script to run the checks. The code that finds perl files is put in a function in a single file that is sourced by the three locations that need it.

+1 on centralizing the find-files function.

+1 on that. Why do we need to make the new find_perl_files file
executable, given it's always sourced? (I would have given a .sh
extension because it's a lib not an executable, but I suppose that's
just matter of taste; we certainly don't have a policy about it).

Looks fine to me either way.

I've committed this, but I'm fine if people want to tweak the names. It
probably doesn't need to be executable.

Why is this being committed after feature freeze?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#6)
Re: automating perl compile time checking

On 06/11/2018 04:01 PM, Peter Eisentraut wrote:

On 6/11/18 15:38, Andrew Dunstan wrote:

On 5 Jun 2018, at 16:31, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
The patch contains a simple script to run the checks. The code that finds perl files is put in a function in a single file that is sourced by the three locations that need it.

+1 on centralizing the find-files function.

+1 on that. Why do we need to make the new find_perl_files file
executable, given it's always sourced? (I would have given a .sh
extension because it's a lib not an executable, but I suppose that's
just matter of taste; we certainly don't have a policy about it).

Looks fine to me either way.

I've committed this, but I'm fine if people want to tweak the names. It
probably doesn't need to be executable.

Why is this being committed after feature freeze?

This affects pretty much nothing. In fact some of the other changes I've
recently committed were arguably more dangerous. Do you want me to
revert the whole lot?

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#7)
Re: automating perl compile time checking

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 06/11/2018 04:01 PM, Peter Eisentraut wrote:

Why is this being committed after feature freeze?

This affects pretty much nothing. In fact some of the other changes I've
recently committed were arguably more dangerous. Do you want me to
revert the whole lot?

AFAICS this is adding testing, not "features", so it seems fine from here.

regards, tom lane

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#5)
Re: automating perl compile time checking

On 06/11/2018 03:38 PM, Andrew Dunstan wrote:

On 06/11/2018 02:33 PM, Alvaro Herrera wrote:

On 2018-Jun-05, Daniel Gustafsson wrote:

On 5 Jun 2018, at 16:31, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
The patch contains a simple script to run the checks. The code that
finds perl files is put in a function in a single file that is
sourced by the three locations that need it.

+1 on centralizing the find-files function.

+1 on that.  Why do we need to make the new find_perl_files file
executable, given it's always sourced?  (I would have given a .sh
extension because it's a lib not an executable, but I suppose that's
just matter of taste; we certainly don't have a policy about it).

Looks fine to me either way.

I've committed this, but I'm fine if people want to tweak the names.
It probably doesn't need to be executable.

People might be interested to see the perlcritic and 'perl -cw" checks
in operation:

https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&amp;dt=2018-06-11%2021%3A47%3A18&amp;stg=perl-check

The module isn't actually using the scripts in src/tools/perlcheck,
because they are designed to be quiet and it's designed to be more
verbose, but apart from that it's doing exactly the same thing.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#7)
Re: automating perl compile time checking

On 6/11/18 16:06, Andrew Dunstan wrote:

This affects pretty much nothing. In fact some of the other changes I've
recently committed were arguably more dangerous. Do you want me to
revert the whole lot?

No, but this whole let's clean up the Perl code initiative seemed to
have come out of nowhere after feature freeze. The Perl code was fine
before, so if we're going to be polishing it around it should go into
the next release. I would actually have liked to have had more time to
discuss some of the changes, since I didn't seem to agree to some of
them at first reading.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#10)
Re: automating perl compile time checking

On 06/15/2018 12:21 AM, Peter Eisentraut wrote:

On 6/11/18 16:06, Andrew Dunstan wrote:

This affects pretty much nothing. In fact some of the other changes I've
recently committed were arguably more dangerous. Do you want me to
revert the whole lot?

No, but this whole let's clean up the Perl code initiative seemed to
have come out of nowhere after feature freeze. The Perl code was fine
before, so if we're going to be polishing it around it should go into
the next release. I would actually have liked to have had more time to
discuss some of the changes, since I didn't seem to agree to some of
them at first reading.

There were changes made for perlcritic, but almost none for this check.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services