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. pgindent: Always clean up .BAK files from pg_bsd_indent

  2. pgindent: Clean up temp files created by File::Temp on SIGINT

  1. Add "format" target to make and ninja to run pgindent and pgperltidy

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2025-12-31T11:35:51Z

    This tries to make running formatting a lot easier for committers, but
primarily for new contributors. You can not format the files by simply
running one of the folowing:

make format
ninja -C build format

My primary goal is to introduce Python formatting too (using ruff), and
integrate that in a similar manner. Right now we don't have many Python
files, but hopefully the pytest patch gets merged soonish and then we'll
get more and more Python files. So I'd like to get autoformatting out of
the gate.

But even without that, these rules should make it simpler to run the
current formatters:

Running pgindent is still not trivial for new users. You need to add our
pg_bsd_indent to PATH. And if you use meson to build you have to manually
specify src/ and contrib/ instead of ./ because otherwise pgindent will
try to format files in the build directory.

Running pgperltidy is even more complicated because you need to install
a specific version of perltidy. Recently we started erroring when that
version was not the one we expect.

  2. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-12-31T13:54:12Z

    On Wed, Dec 31, 2025 at 5:06 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> This tries to make running formatting a lot easier for committers, but
> primarily for new contributors. You can not format the files by simply
> running one of the folowing:
>
> make format
> ninja -C build format
>

I generally like the idea. Since perltidy is not enforced regularly
(like pgindent), running it usually ends up modifying files which are
not part of the patch. So I avoid it if not necessary. Do you propose
to make it optional?

-- 
Best Wishes,
Ashutosh Bapat

  3. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-31T15:26:44Z

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
> On Wed, Dec 31, 2025 at 5:06 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>> This tries to make running formatting a lot easier for committers, but
>> primarily for new contributors.

> I generally like the idea. Since perltidy is not enforced regularly
> (like pgindent), running it usually ends up modifying files which are
> not part of the patch. So I avoid it if not necessary. Do you propose
> to make it optional?

The other obstacle is that not everybody will have the right version
of perltidy installed, but using some other version will create a
whole lot of extraneous noise.

On the whole I'd recommend not trying to automate the perltidy
step yet.  Cost/benefit is just not very good.

On the substance of the patch: I wonder whether we could make things
more reliable by using git metadata to figure out which .h and .c
files to point pgindent at.

			regards, tom lane

  4. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Andrew Dunstan <andrew@dunslane.net> — 2025-12-31T15:48:00Z

    On 2025-12-31 We 10:26 AM, Tom Lane wrote:
> Ashutosh Bapat<ashutosh.bapat.oss@gmail.com> writes:
>> On Wed, Dec 31, 2025 at 5:06 PM Jelte Fennema-Nio<postgres@jeltef.nl> wrote:
>>> This tries to make running formatting a lot easier for committers, but
>>> primarily for new contributors.
>> I generally like the idea. Since perltidy is not enforced regularly
>> (like pgindent), running it usually ends up modifying files which are
>> not part of the patch. So I avoid it if not necessary. Do you propose
>> to make it optional?
> The other obstacle is that not everybody will have the right version
> of perltidy installed, but using some other version will create a
> whole lot of extraneous noise.



pgperltidy actually checks the version now.


>
> On the whole I'd recommend not trying to automate the perltidy
> step yet.  Cost/benefit is just not very good.


I'd kinda like to unify these universes, though. We could import the 
pgperltidy logic into pgindent but disable it unless some flag were 
given and the correct version of perltidy were present.


>
> On the substance of the patch: I wonder whether we could make things
> more reliable by using git metadata to figure out which .h and .c
> files to point pgindent at.
>
> 			


The git pre-commit hook I use operates with this set of files:

     files=$(git diff --cached --name-only --diff-filter=ACMR)

pgindent just currently looks at that set of files and ignores 
everything that's not a .c or .h file.

I guess what you're wanting is a test to see if the file is in git or a 
generated file? That doesn't really arise for me as I always do vpath 
builds, so generated files are always elsewhere.


cheers


andrew


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

  5. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-31T15:54:06Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> On 2025-12-31 We 10:26 AM, Tom Lane wrote:
>> On the substance of the patch: I wonder whether we could make things
>> more reliable by using git metadata to figure out which .h and .c
>> files to point pgindent at.

> I guess what you're wanting is a test to see if the file is in git or a 
> generated file? That doesn't really arise for me as I always do vpath 
> builds, so generated files are always elsewhere.

Right.  But if we're trying to make this easy, we need to make
the automation work for all three use-cases (in-tree makefiles,
vpath makefiles, meson).  I was just wondering if relying on
git would simplify getting the same results in all three.

			regards, tom lane

  6. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Andrew Dunstan <andrew@dunslane.net> — 2025-12-31T16:13:44Z

    On 2025-12-31 We 10:54 AM, Tom Lane wrote:
> Andrew Dunstan<andrew@dunslane.net> writes:
>> On 2025-12-31 We 10:26 AM, Tom Lane wrote:
>>> On the substance of the patch: I wonder whether we could make things
>>> more reliable by using git metadata to figure out which .h and .c
>>> files to point pgindent at.
>> I guess what you're wanting is a test to see if the file is in git or a
>> generated file? That doesn't really arise for me as I always do vpath
>> builds, so generated files are always elsewhere.
> Right.  But if we're trying to make this easy, we need to make
> the automation work for all three use-cases (in-tree makefiles,
> vpath makefiles, meson).  I was just wondering if relying on
> git would simplify getting the same results in all three.
>
> 			



I think we could use

     git ls-files -t $file

or similar.

cheers

andrew

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

  7. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2025-12-31T18:17:39Z

    On Wed Dec 31, 2025 at 4:26 PM CET, Tom Lane wrote:
> Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
>> On Wed, Dec 31, 2025 at 5:06 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>>> This tries to make running formatting a lot easier for committers, but
>>> primarily for new contributors.
>
>> I generally like the idea. Since perltidy is not enforced regularly
>> (like pgindent), running it usually ends up modifying files which are
>> not part of the patch. So I avoid it if not necessary. Do you propose
>> to make it optional?
>
> The other obstacle is that not everybody will have the right version
> of perltidy installed, but using some other version will create a
> whole lot of extraneous noise.
>
> On the whole I'd recommend not trying to automate the perltidy
> step yet.  Cost/benefit is just not very good.

I would like to get to a point where it is enforced for every commit
pushed by committers, so the same as with pgindent. I agree that the
cost/benefit is not there currently, but that's what this patchset is
trying to address. The docs in the last patch try to explain clearly how
to get and configure the correct version of perltidy. And once you've
done that it's as easy as running a single make/ninja command to format
all the files.

Do you think that's not still not easy enough for committers? What would
be needed instead? Automatically fetching and installing perltidy to the
local build path when running make/ninja format?

> On the substance of the patch: I wonder whether we could make things
> more reliable by using git metadata to figure out which .h and .c
> files to point pgindent at.

I think that would definitely be helpful. The fact that pgindent runs on
files in the meson build directory when passing "." would be solved by that.

  8. Re: Add "format" target to make and ninja to run pgindent and pgperltidy

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-31T18:37:21Z

    "Jelte Fennema-Nio" <postgres@jeltef.nl> writes:
> On Wed Dec 31, 2025 at 4:26 PM CET, Tom Lane wrote:
>> On the whole I'd recommend not trying to automate the perltidy
>> step yet.  Cost/benefit is just not very good.

> I would like to get to a point where it is enforced for every commit
> pushed by committers, so the same as with pgindent.

As an affected committer, I want to push back against having such
a requirement, because I don't think it is reasonable to require
everybody to have precisely version XYZ of perltidy installed.
If that's not the version provided by their platform-of-choice,
it's an annoying hurdle.

As a comparison point, we did not start requiring pgindent cleanliness
until we imported bsdindent into our tree, so as not to have an
external dependency for that.  (But I can't see vendoring perltidy,
even if there weren't license issues involved.)

I recognize the analogy to requiring a specific version of autoconf,
but the difference is that without autoconf you just plain can't work
on the configure code.  Here, the hurdle would be erected for no
reason stronger than neatnik-ism, and IMO that's not a good enough
reason to put yet another burden on committers.

I'm even less pleased by the notion that we'd soon add still another
such requirement for python.

			regards, tom lane