Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Check for correct version of perltidy

  1. The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-25T14:50:01Z

    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?

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.

--
Daniel Gustafsson

  2. Re: The pgperltidy diffs in HEAD

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-25T15:49:32Z

    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/

  3. Re: The pgperltidy diffs in HEAD

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-25T16:00:51Z

    =?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

  4. Re: The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-25T16:29:05Z

    > 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

  5. Re: The pgperltidy diffs in HEAD

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-25T17:35:52Z

    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

  6. Re: The pgperltidy diffs in HEAD

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-25T17:53:05Z

    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/

  7. Re: The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-25T19:18:39Z

    > 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

  8. Re: The pgperltidy diffs in HEAD

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-25T19:31:09Z

    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)

  9. Re: The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-25T19:36:33Z

    > 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

  10. Re: The pgperltidy diffs in HEAD

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-25T20:04:50Z

    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

  11. Re: The pgperltidy diffs in HEAD

    Andrew Dunstan <andrew@dunslane.net> — 2025-11-25T21:03:02Z

    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

  12. Re: The pgperltidy diffs in HEAD

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-11-25T22:03:26Z

    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

  13. Re: The pgperltidy diffs in HEAD

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-25T22:04:53Z

    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

  14. Re: The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-25T22:28:35Z

    > 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

  15. Re: The pgperltidy diffs in HEAD

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-11-26T11:11:05Z

    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

  16. Re: The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-26T11:16:52Z

    > 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

  17. Re: The pgperltidy diffs in HEAD

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-26T16:19:40Z

    > 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

  18. Re: The pgperltidy diffs in HEAD

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-11-26T16:22:50Z

    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