The pgperltidy diffs in HEAD

On 2025-Nov-25, Daniel Gustafsson wrote:

I routinely run pgperltidy src/ when hacking on things, and am greeted with
lots of diffs like how pgindent runs used to be. Are there objections to
applying the diffs we've accumulated so far with a .git-blame-ignore-revs
update alongside it? Are there reasons not that I am missing?

None here. I tend to run pgperltidy on individual files so this is not
normally a problem for me, but I kinda dislike that our steady status is
not clean.

Attached is the current output from pgperltidy, I haven't looked over it in
detail but I am happy to take that on assuming it's not objected to.

Hmm, I wonder if you ran this with our documented version of perltidy.
I have vague memories of pgperltidy leaving the generate-lwlocknames.pl
script the way it is now, for example. But then, maybe the one who used
the wrong perltidy version is me.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:

On 2025-Nov-25, Daniel Gustafsson wrote:

I routinely run pgperltidy src/ when hacking on things, and am greeted with
lots of diffs like how pgindent runs used to be. Are there objections to
applying the diffs we've accumulated so far with a .git-blame-ignore-revs
update alongside it? Are there reasons not that I am missing?

None here. I tend to run pgperltidy on individual files so this is not
normally a problem for me, but I kinda dislike that our steady status is
not clean.

While I've not got any great objection to running pgperltidy now,
it seems like it'd be better if committers were all on the same page
about this. My understanding of the current policy is that we'll
keep the tree pgindent-clean on the fly, but worry about pgperltidy
only once a year or so. Is there consensus for tightening that up?

Hmm, I wonder if you ran this with our documented version of perltidy.

This sort of thing is why I'm hesitant. We didn't really dare expect
committers to ensure pgindent cleanliness until we had that tool
fully integrated in our tree, so that there was one true (and readily
available) version to use. perltidy still fails that test AFAIK;
you have to go looking for the agreed-on version.

regards, tom lane

On 25 Nov 2025, at 17:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:

On 2025-Nov-25, Daniel Gustafsson wrote:

I routinely run pgperltidy src/ when hacking on things, and am greeted with
lots of diffs like how pgindent runs used to be. Are there objections to
applying the diffs we've accumulated so far with a .git-blame-ignore-revs
update alongside it? Are there reasons not that I am missing?

None here. I tend to run pgperltidy on individual files so this is not
normally a problem for me, but I kinda dislike that our steady status is
not clean.

While I've not got any great objection to running pgperltidy now,
it seems like it'd be better if committers were all on the same page
about this. My understanding of the current policy is that we'll
keep the tree pgindent-clean on the fly, but worry about pgperltidy
only once a year or so. Is there consensus for tightening that up?

I don't think there is, but I wouldn't mind if that was the case given how nice
it is to have a pgindent clean tree at all times.

Hmm, I wonder if you ran this with our documented version of perltidy.

This sort of thing is why I'm hesitant. We didn't really dare expect
committers to ensure pgindent cleanliness until we had that tool
fully integrated in our tree, so that there was one true (and readily
available) version to use. perltidy still fails that test AFAIK;
you have to go looking for the agreed-on version.

..and since I managed to run it with the wrong version for the attached diff that
argument certainly does have merit (I now gave up on having two version
installed for $reasons and will settle on the postgres-mandated version for all
things).

Maybe this is best left alone for now and made into a topic for a committer
meeting at pgconf.dev?

--
Daniel Gustafsson

Daniel Gustafsson <daniel@yesql.se> writes:

On 25 Nov 2025, at 17:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:

Hmm, I wonder if you ran this with our documented version of perltidy.

..and since I managed to run it with the wrong version for the attached diff that
argument certainly does have merit (I now gave up on having two version
installed for $reasons and will settle on the postgres-mandated version for all
things).

pgindent has long had a check that you're running the correct version
of bsdindent, but I just realized that pgperltidy makes no such check
for perltidy. Should we install one?

regards, tom lane

On 2025-Nov-25, Tom Lane wrote:

pgindent has long had a check that you're running the correct version
of bsdindent, but I just realized that pgperltidy makes no such check
for perltidy. Should we install one?

Yeah, we should definitely have that.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/

On 25 Nov 2025, at 18:53, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Nov-25, Tom Lane wrote:

pgindent has long had a check that you're running the correct version
of bsdindent, but I just realized that pgperltidy makes no such check
for perltidy. Should we install one?

Yeah, we should definitely have that.

Agreed. Perhaps something like the attached would work?

--
Daniel Gustafsson

On 2025-Nov-25, Daniel Gustafsson wrote:

Agreed. Perhaps something like the attached would work?

Hmm, I got this

src/tools/pgindent/pgperltidy: 10: [: 0: unexpected operator

$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 feb 4 2025 /bin/sh -> dash*

(Seems to work ok silently with bash.)

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Digital and video cameras have this adjustment and film cameras don't for the
same reason dogs and cats lick themselves: because they can." (Ken Rockwell)

On 25 Nov 2025, at 20:31, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Nov-25, Daniel Gustafsson wrote:

Agreed. Perhaps something like the attached would work?

Hmm, I got this

src/tools/pgindent/pgperltidy: 10: [: 0: unexpected operator

$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 feb 4 2025 /bin/sh -> dash*

(Seems to work ok silently with bash.)

If you replace the if statement with the one below using test, does that make
it work?

if test $? = 1; then

--
Daniel Gustafsson

Daniel Gustafsson <daniel@yesql.se> writes:

On 2025-Nov-25, Tom Lane wrote:

pgindent has long had a check that you're running the correct version
of bsdindent, but I just realized that pgperltidy makes no such check
for perltidy. Should we install one?

Agreed. Perhaps something like the attached would work?

WFM.

regards, tom lane

On 2025-11-25 Tu 2:36 PM, Daniel Gustafsson wrote:

On 25 Nov 2025, at 20:31, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Nov-25, Daniel Gustafsson wrote:

Agreed. Perhaps something like the attached would work?

Hmm, I got this

src/tools/pgindent/pgperltidy: 10: [: 0: unexpected operator

$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 feb 4 2025 /bin/sh -> dash*

(Seems to work ok silently with bash.)

If you replace the if statement with the one below using test, does that make
it work?

if test $? = 1; then

Looks to me like your original patch had == where it should have had =

Your formulation above corrects that.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Andrew Dunstan <andrew@dunslane.net> writes:

On 2025-11-25 Tu 2:36 PM, Daniel Gustafsson wrote:

On 25 Nov 2025, at 20:31, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Nov-25, Daniel Gustafsson wrote:

Agreed. Perhaps something like the attached would work?

Hmm, I got this

src/tools/pgindent/pgperltidy: 10: [: 0: unexpected operator

$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 feb 4 2025 /bin/sh -> dash*

(Seems to work ok silently with bash.)

If you replace the if statement with the one below using test, does that make
it work?

if test $? = 1; then

Looks to me like your original patch had == where it should have had =

Your formulation above corrects that.

Even simpler, and avoiding having to move the `set -e` after the check:

PERLTIDY_VERSION=20230309
if ! $PERLTIDY -v | grep -q $PERLTIDY_VERSION; then
echo "error: pgperltidy requires perltidy v$PERLTIDY_VERSION"
exit 1
fi

- ilmari

On Tue, Nov 25, 2025 at 2:03 PM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:

Even simpler, and avoiding having to move the `set -e` after the check:

+1; you beat me to it by a couple seconds :)

--Jacob

On 25 Nov 2025, at 23:03, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

Even simpler, and avoiding having to move the `set -e` after the check:

PERLTIDY_VERSION=20230309
if ! $PERLTIDY -v | grep -q $PERLTIDY_VERSION; then
echo "error: pgperltidy requires perltidy v$PERLTIDY_VERSION"
exit 1
fi

Ah, even better, thanks!

--
Daniel Gustafsson

Daniel Gustafsson <daniel@yesql.se> writes:

[2. text/x-diff; v2_perltidyversion.diff]
diff --git a/doc/src/sgml/func/func-string.sgml b/doc/src/sgml/func/func-string.sgml
index 7ad1436e5f8..646b5d6d8c4 100644
--- a/doc/src/sgml/func/func-string.sgml
+++ b/doc/src/sgml/func/func-string.sgml
@@ -173,7 +173,7 @@

This seems unrelated to the rest of the patch patch.

diff --git a/src/tools/pgindent/pgperltidy b/src/tools/pgindent/pgperltidy
index 6af27d21d55..6fac758665a 100755
--- a/src/tools/pgindent/pgperltidy
+++ b/src/tools/pgindent/pgperltidy
@@ -7,6 +7,12 @@ set -e
# set this to override default perltidy program:
PERLTIDY=${PERLTIDY:-perltidy}

+PERLTIDY_VERSION=20230309
+if ! $PERLTIDY -v | grep -q $PERLTIDY_VERSION; then
+	echo "error: pgperltidy requires perltidy v$PERLTIDY_VERSION"

I just realised, this message should really go to stderr, i.e. have >&2
on the end.

+	exit 1
+fi
+
. src/tools/perlcheck/find_perl_files

find_perl_files "$@" | xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc

- ilmari

On 26 Nov 2025, at 12:11, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

This seems unrelated to the rest of the patch patch.

Yeah, I was working on a docs-patch in the same tree and only realized this
morning that I had accidentally included that part =)

diff --git a/src/tools/pgindent/pgperltidy b/src/tools/pgindent/pgperltidy
index 6af27d21d55..6fac758665a 100755
--- a/src/tools/pgindent/pgperltidy
+++ b/src/tools/pgindent/pgperltidy
@@ -7,6 +7,12 @@ set -e
# set this to override default perltidy program:
PERLTIDY=${PERLTIDY:-perltidy}

+PERLTIDY_VERSION=20230309
+if ! $PERLTIDY -v | grep -q $PERLTIDY_VERSION; then
+ echo "error: pgperltidy requires perltidy v$PERLTIDY_VERSION"

I just realised, this message should really go to stderr, i.e. have >&2
on the end.

Good point, I'll change that before committing and I'll also reword it to use
the same error as pgindent which has this:

"You do not appear to have $indent version $INDENT_VERSION installed on your system.\n";

Consistency is good.

--
Daniel Gustafsson

On 26 Nov 2025, at 12:16, Daniel Gustafsson <daniel@yesql.se> wrote:

I'll change that before committing and I'll also reword it to use
the same error as pgindent

More specifically, the attached is what I have staged for commit.

--
Daniel Gustafsson

Daniel Gustafsson <daniel@yesql.se> writes:

On 26 Nov 2025, at 12:16, Daniel Gustafsson <daniel@yesql.se> wrote:

I'll change that before committing and I'll also reword it to use
the same error as pgindent

More specifically, the attached is what I have staged for commit.

LGTM

--
Daniel Gustafsson

- ilmari