On Thu Mar 12, 2026 at 10:10 AM CET, Peter Eisentraut wrote:
Apparently, this is still work in progress, but here are some comments from me.
Thanks for the review. I went over the code in detail now myself, and haven't found anything hugehely weird (although I did a bit of cleanup and additional comments). As said before I'm definetly not a Perl expert though, so I might have missed some wrong details. But all the logic at least seems sound. I also reorderd the commits to have the perltidy integration as the last ones, and the general pgindent QoL imorovements first.
I'm very interested in the patch "pgindent: Clean up temp files on SIGINT", because this is an annoying problem. But I didn't find any documentation about why this is the correct solution. I didn't find anything on the File::Temp man page, for example. Do you have more details?
I explained this black magic better now, and also did the same for SIGTERM. I also added a second commit which cleans up the .BAK files that pg_bsd_indent creates. Feel free to combine those commits if you think that's better.
The patch "pgindent: Use git ls-files to discover files" is also interested and pretty straightforward. I guess we don't really care about being able to run this in non-git trees?
I don't think we care. This is a tool for Postgres developers, and I think we can assume all of those are using git to work on Postgres.
The other stuff I don't think I'm really on board with. In particular, I don't like integrating pgperltidy into pgindent. These things are in practice run at different times and the underlying tools update differently and require different management, so integrating them all might lead to various annoyances.
So, to be clear. My goal is to get to a point where they ARE run at the same time, because their goal is the same, just for different filetypes. Afaict the primary reason that pg_bsd_indent is run on every commit, but perltidy is not, is that the later is currently much more annoying to run. Integrating it into pgindent is my attempt at making that barier much lower, so that we can make it part of the standard workflow.
I kind of liked the original idea of a "make format", and then you could have subtargets like "make format-c" and "make format-perl" if you don't have all the tools.
I liked it too initially, but I then realized that means you lose all the nice flags that pgindent provides, e.g. --commit=@ or --check. Ofcourse you could add a FORMATARGS variable, but at that point why not just have people run pgindent directly? Also, perltidy is very slow to run. Running it on the whole codebase takes about a minute for me. So the new parallel and git ls-files support in pgindent are really helpful to have it finish in a reasonable time.
The parallel pgindent is pretty nice, but I wonder how "use POSIX" works on Windows?
Removed the POSIX thing. That wasn't needed. I'm wondering how many people use pginden ton Windows though.
(But in practice I mostly use pgindent --commit=@, which is still much faster.)
(--commit=@ should also get faster with the parallelization, because now it can run of the files from the commit in parallel)
From 8862ec4be4139f6d7e84932b3435a4ea5940037d Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Wed, 4 Mar 2026 10:02:39 +0100 Subject: [PATCH v3 1/8] pgindent: Clean up temp files created by File::Temp on SIGINT When pressing Ctrl+C while running pgindent, it would often leave around files like pgtypedefAXUEEA. This slightly changes SIGINT handling so those files are cleaned up. --- src/tools/pgindent/pgindent | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 7481696a584..b96891b2252 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -19,6 +19,14 @@ use File::Temp; use IO::Handle; use Getopt::Long; +# By default Perl's SIGINT/SIGTERM kill the process without running END blocks, +# so File::Temp never gets to clean up. Converting the signal into an exit() +# makes END-block cleanup run. See: http://www.perlmonks.org/?node_id=714225 +# Exit codes use the 128+signum convention so callers can tell the process was +# killed by a signal. +$SIG{INT} = sub { exit 130; }; # 128 + 2 (SIGINT) +$SIG{TERM} = sub { exit 143; }; # 128 + 15 (SIGTERM) + # Update for pg_bsd_indent version my $INDENT_VERSION = "2.1.2"; base-commit: a0b6ef29a51818a4073a5f390ed10ef6453d5c11 -- 2.53.0
From f63ca9cb6869452e85aec7b09d4ae471ab1f3030 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Fri, 13 Mar 2026 00:21:04 +0100 Subject: [PATCH v3 2/8] pgindent: Always clean up .BAK files from pg_bsd_indent The previous commit let pgindent clean up File::Temp files on SIGINT. This extends that to also cleaning up the .BAK files, created by pg_bsd_indent. --- src/tools/pgindent/pgindent | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index b96891b2252..6ce695895eb 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -27,6 +27,12 @@ use Getopt::Long; $SIG{INT} = sub { exit 130; }; # 128 + 2 (SIGINT) $SIG{TERM} = sub { exit 143; }; # 128 + 15 (SIGTERM) +# pg_bsd_indent creates a .BAK file that File::Temp doesn't know about. This +# END block makes sure that that file is cleaned up in case someone presses +# Ctrl+C during pgindent. +my $bak_to_cleanup; +END { unlink $bak_to_cleanup if defined $bak_to_cleanup; } + # Update for pg_bsd_indent version my $INDENT_VERSION = "2.1.2"; @@ -295,11 +301,14 @@ sub run_indent print $tmp_fh $source; $tmp_fh->close(); + $bak_to_cleanup = "$filename.BAK"; + $$error_message = `$cmd $filename 2>&1`; return "" if ($? || length($$error_message) > 0); - unlink "$filename.BAK"; + unlink $bak_to_cleanup; + $bak_to_cleanup = undef; open(my $src_out, '<', $filename) || die $!; local ($/) = undef; -- 2.53.0
From ae515e0d4a848da800ecf7e7743902a80a2344ea Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Tue, 3 Mar 2026 09:43:33 +0100 Subject: [PATCH v3 3/8] pgindent: Use git ls-files to discover files When running pgindent on the whole source tree, it would often also indent build artifacts. This changes the pgindent directory search logic to become git-aware, and only indent files that are actually tracked by git. Any files that are explicitly part of the pgindent its arguments, are always indented though, even if they are not tracked by git. This can be useful to force indentation of an untracked file and/or for editor integration. --- src/tools/pgindent/pgindent | 44 +++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 6ce695895eb..b6fbda9a298 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -13,7 +13,6 @@ use strict; use warnings FATAL => 'all'; use Cwd qw(abs_path getcwd); -use File::Find; use File::Spec; use File::Temp; use IO::Handle; @@ -369,16 +368,43 @@ $filtered_typedefs_fh = load_typedefs(); check_indent(); -my $wanted = sub { - my ($dev, $ino, $mode, $nlink, $uid, $gid); - (($dev, $ino, $mode, $nlink, $uid, $gid) = lstat($_)) - && -f _ - && /^.*\.[ch]\z/s - && push(@files, $File::Find::name); -}; +sub discover_files +{ + my @paths = @_; + my @discovered; + + # Separate individual files from directories + my @dirs; + foreach my $path (@paths) + { + if (-f $path) + { + push(@discovered, $path); + } + elsif (-d $path) + { + push(@dirs, $path); + } + else + { + warn "Could not find $path"; + } + } + + # Use git ls-files for directories to avoid searching build trees etc. + if (@dirs) + { + my @git_files = `git ls-files -- @dirs`; + die "git ls-files error" if $?; + chomp(@git_files); + push(@discovered, grep { /\.[ch]$/ } @git_files); + } + + return @discovered; +} # any non-option arguments are files or directories to be processed -File::Find::find({ wanted => $wanted }, @ARGV) if @ARGV; +push(@files, discover_files(@ARGV)) if @ARGV; # commit file locations are relative to the source root chdir "$sourcedir/../../.." if @commits && $sourcedir; -- 2.53.0
From 1433ef85b04bb772f3e470a1d41a17c7c96392cd Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Tue, 3 Mar 2026 20:56:15 +0100 Subject: [PATCH v3 4/8] pgindent: Default to indenting the current directory if no files are given Previously running pgindent without giving it any files would result in this output: src/tools/pgindent/pgindent No files to process at src/tools/pgindent/pgindent line 526. This instead indents the current directory, which is probably what the user intended. --- src/tools/pgindent/pgindent | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index b6fbda9a298..e6bba48893c 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -62,6 +62,9 @@ usage() if $help; usage("Cannot use --commit with command line file list") if (@commits && @ARGV); +# default to current directory if no files/dirs given +@ARGV = ('.') unless @ARGV || @commits; + # command line option wins, then environment, then locations based on current # dir, then default location $typedefs_file ||= $ENV{PGTYPEDEFS}; -- 2.53.0
From 5a17a9619c1a6a611fbfdeb313c32f35e1149502 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Wed, 4 Mar 2026 09:24:24 +0100 Subject: [PATCH v3 5/8] pgindent: Allow parallel pgindent runs Running pgindent on the whole source tree can take a while. This adds support for pgindent to indent files in parallel. This speeds up a full pgindent run from ~7 seconds to 1 second on my machine. Especially with future commits that integrate perltidy into pgindent the wins are huge, because perltidy is much slower at formatting than pg_bsd_indent. With those later commits the time it takes to do a full pgindent run (including perltidy) takes more than a minute on my machine without the parallelization, but only take ~7 seconds when run in parallel. --- src/tools/pgindent/pgindent | 114 ++++++++++++++++++++++++++++++------ 1 file changed, 95 insertions(+), 19 deletions(-) diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index e6bba48893c..85271c9986e 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -17,6 +17,7 @@ use File::Spec; use File::Temp; use IO::Handle; use Getopt::Long; +use Fcntl qw(:flock); # By default Perl's SIGINT/SIGTERM kill the process without running END blocks, # so File::Temp never gets to clean up. Converting the signal into an exit() @@ -41,10 +42,11 @@ my $indent_opts = my $devnull = File::Spec->devnull; -my ($typedefs_file, $typedef_str, @excludes, $indent, $diff, - $check, $help, @commits,); +my ($typedefs_file, $typedef_str, @excludes, $indent, + $diff, $check, $help, @commits, $jobs,); $help = 0; +$jobs = 0; my %options = ( "help" => \$help, @@ -54,9 +56,32 @@ my %options = ( "excludes=s" => \@excludes, "indent=s" => \$indent, "diff" => \$diff, - "check" => \$check,); + "check" => \$check, + "jobs|j=i" => \$jobs,); GetOptions(%options) || usage("bad command line argument"); +sub get_num_cpus +{ + # Try nproc (Linux, some BSDs), then sysctl (macOS, FreeBSD). + for my $cmd ('nproc', 'sysctl -n hw.ncpu') + { + my $n = `$cmd 2>$devnull`; + chomp $n; + return $n + 0 if ($? == 0 && $n =~ /^\d+$/ && $n > 0); + } + return 1; +} + +if ($jobs == 0) +{ + $jobs = get_num_cpus(); +} +elsif ($jobs < 0) +{ + usage("--jobs must be a non-negative number"); +} + + usage() if $help; usage("Cannot use --commit with command line file list") @@ -351,6 +376,7 @@ Options: --indent=PATH path to pg_bsd_indent program --diff show the changes that would be made --check exit with status 2 if any changes would be made + --jobs=N, -j N number of parallel workers (0 = num CPUs, default 0) The --excludes and --commit options can be given more than once. EOF if ($help) @@ -427,32 +453,30 @@ warn "No files to process" unless @files; # remove excluded files from the file list process_exclude(); -my %processed; -my $status = 0; +# Used by forked children to serialize diff output to STDOUT via flock(). +my $stdout_lock_fh = new File::Temp(TEMPLATE => "pglockXXXXX"); -foreach my $source_filename (@files) +sub process_file { - # skip duplicates - next if $processed{$source_filename}; - $processed{$source_filename} = 1; + my $source_filename = shift; # ignore anything that's not a .c or .h file - next unless $source_filename =~ /\.[ch]$/; + return 0 unless $source_filename =~ /\.[ch]$/; # don't try to indent a file that doesn't exist unless (-f $source_filename) { warn "Could not find $source_filename"; - next; + return 0; } # Automatically ignore .c and .h files that correspond to a .y or .l # file. indent tends to get badly confused by Bison/flex output, # and there's no value in indenting derived files anyway. my $otherfile = $source_filename; $otherfile =~ s/\.[ch]$/.y/; - next if $otherfile ne $source_filename && -f $otherfile; + return 0 if $otherfile ne $source_filename && -f $otherfile; $otherfile =~ s/\.y$/.l/; - next if $otherfile ne $source_filename && -f $otherfile; + return 0 if $otherfile ne $source_filename && -f $otherfile; my $source = read_source($source_filename); my $orig_source = $source; @@ -464,8 +488,7 @@ foreach my $source_filename (@files) if ($source eq "") { print STDERR "Failure in $source_filename: " . $error_message . "\n"; - $status = 3; - next; + return 3; } $source = post_indent($source); @@ -480,14 +503,67 @@ foreach my $source_filename (@files) { if ($diff) { - print diff($source, $source_filename); + my $output = diff($source, $source_filename); + flock($stdout_lock_fh, LOCK_EX); + print $output; + STDOUT->flush(); + flock($stdout_lock_fh, LOCK_UN); } - if ($check) + return 2 if $check; + } + } + + return 0; +} + +# deduplicate file list +my %seen; +@files = grep { !$seen{$_}++ } @files; + +my $status = 0; + +if ($jobs <= 1) +{ + foreach my $source_filename (@files) + { + my $file_status = process_file($source_filename); + $status = $file_status if $file_status > $status; + last if $check && $status >= 2 && !$diff; + } +} +else +{ + my %children; # pid => 1 + + my $file_idx = 0; + while ($file_idx < scalar(@files) || %children) + { + # Fork new children up to $jobs limit + while ($file_idx < scalar(@files) && scalar(keys %children) < $jobs) + { + my $source_filename = $files[ $file_idx++ ]; + + my $pid = fork(); + die "fork failed: $!\n" unless defined $pid; + + if ($pid == 0) { - $status ||= 2; - last unless $diff; + # child + my $child_status = process_file($source_filename); + exit $child_status; } + + $children{$pid} = 1; + } + + # Wait for at least one child to finish + my $pid = waitpid(-1, 0); + if ($pid > 0 && exists $children{$pid}) + { + delete $children{$pid}; + my $child_status = $? >> 8; + $status = $child_status if $child_status > $status; } } } -- 2.53.0
From b6c4a895ff08b46d65114728f413ace63a143729 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Tue, 3 Mar 2026 23:46:52 +0100 Subject: [PATCH v3 6/8] pgindent: Try to find pg_bsd_indent binary in common locations To run pgindent you need to to have the right version of pg_bsd_indent in your PATH, or specify it manually. This is a bit of a hassle, especially for newcomers or when working on backbranches. So this chnages pgindent to search for a pg_bsd_indent that's built from the current sources, both in-tree (for in-tree autoconf builds) and in a build directory (for meson or autoconf vpath builds). --- src/tools/pgindent/pgindent | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 85271c9986e..97bebc6282e 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -94,11 +94,34 @@ usage("Cannot use --commit with command line file list") # dir, then default location $typedefs_file ||= $ENV{PGTYPEDEFS}; -# get indent location for environment or default -$indent ||= $ENV{PGINDENT} || $ENV{INDENT} || "pg_bsd_indent"; - my $sourcedir = locate_sourcedir(); +# get indent location: command line wins, then environment, then try to find +# a compiled pg_bsd_indent in the source tree, then fall back to PATH. +$indent ||= $ENV{PGINDENT} || $ENV{INDENT}; +if (!$indent && $sourcedir) +{ + my $srcroot = "$sourcedir/../../.."; + my $bsd_indent_subdir = "src/tools/pg_bsd_indent/pg_bsd_indent"; + + # Look for pg_bsd_indent: first in-tree (autoconf in-tree build), + # then in a "build" directory (meson or autoconf vpath), + # then any "build*" directory. + foreach my $candidate ( + "$srcroot/$bsd_indent_subdir", + "$srcroot/build/$bsd_indent_subdir", + glob("$srcroot/build*/$bsd_indent_subdir")) + { + if (-x $candidate) + { + $indent = $candidate; + last; + } + } +} +$indent ||= "pg_bsd_indent"; + + # if it's the base of a postgres tree, we will exclude the files # postgres wants excluded if ($sourcedir) -- 2.53.0
From 861edbfb36b063d13d52da73956f6b2faf8f72ef Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Wed, 31 Dec 2025 11:07:58 +0100 Subject: [PATCH v3 7/8] pgindent: Integrate pgperltidy functionality into pgindent Over time our pgindent script has gotten a lot of quality of life features, like the --commit, --diff and --check flags. This integrates the functionality of pgperltidy into pgindent, so it can benefit from these same quality of life improvements, as well as future ones. This commit adds a --perltidy flag to pgindent, which when given will cause pgindent to also format Perl files in addition to the C files it would normally format. It also adds a --perl-only flag, to (as the name suggests) only format Perl files. --- src/tools/pgindent/README | 14 +-- src/tools/pgindent/pgindent | 191 ++++++++++++++++++++++++++++------ src/tools/pgindent/pgperltidy | 18 ---- 3 files changed, 168 insertions(+), 55 deletions(-) delete mode 100755 src/tools/pgindent/pgperltidy diff --git a/src/tools/pgindent/README b/src/tools/pgindent/README index b6cd4c6f6b7..2e31471d7e6 100644 --- a/src/tools/pgindent/README +++ b/src/tools/pgindent/README @@ -35,6 +35,10 @@ DOING THE INDENT RUN BEFORE A NORMAL COMMIT: src/tools/pgindent/pgindent . + To also format Perl files at the same time, add --perltidy: + + src/tools/pgindent/pgindent --perltidy=perltidy . + If any files generate errors, restore their original versions with "git checkout", and see below for cleanup ideas. @@ -76,12 +80,12 @@ AT LEAST ONCE PER RELEASE CYCLE: 2) Run pgindent as above. -3) Indent the Perl code using perltidy: +3) Indent the Perl code using perltidy (if not already done in step 2): - src/tools/pgindent/pgperltidy . + src/tools/pgindent/pgindent --perl-only . If you want to use some perltidy version that's not in your PATH, - first set the PERLTIDY environment variable to point to it. + use --perltidy=PATH or set the PERLTIDY environment variable. 4) Reformat the bootstrap catalog data files: @@ -166,6 +170,4 @@ Note that we do not exclude ecpg's header files from the run. Some of them get copied verbatim into ecpg's output, meaning that ecpg's expected files may need to be updated to match. -The perltidy run processes all *.pl and *.pm files, plus a few -executable Perl scripts that are not named that way. See the "find" -rules in pgperltidy for details. +When --perltidy is given, pgindent also processes *.pl and *.pm files. diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 97bebc6282e..9d635b7a22b 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -2,12 +2,12 @@ # Copyright (c) 2021-2026, PostgreSQL Global Development Group -# Program to maintain uniform layout style in our C code. +# Program to maintain uniform layout style in our C and Perl code. # Exit codes: # 0 -- all OK # 1 -- error invoking pgindent, nothing done # 2 -- --check mode and at least one file requires changes -# 3 -- pg_bsd_indent failed on at least one file +# 3 -- pg_bsd_indent or perltidy failed on at least one file use strict; use warnings FATAL => 'all'; @@ -43,11 +43,17 @@ my $indent_opts = my $devnull = File::Spec->devnull; my ($typedefs_file, $typedef_str, @excludes, $indent, - $diff, $check, $help, @commits, $jobs,); + $diff, $check, $help, @commits, + $perltidy_arg, $perl_only, $jobs,); $help = 0; $jobs = 0; +# Save @ARGV before parsing so we can distinguish --perltidy=PATH (where +# the value should be used as the perltidy path) from --perltidy PATH +# (where PATH is a file to format that Getopt::Long greedily consumed). +my @orig_argv = @ARGV; + my %options = ( "help" => \$help, "commit=s" => \@commits, @@ -55,6 +61,8 @@ my %options = ( "list-of-typedefs=s" => \$typedef_str, "excludes=s" => \@excludes, "indent=s" => \$indent, + "perltidy:s" => \$perltidy_arg, + "perl-only" => \$perl_only, "diff" => \$diff, "check" => \$check, "jobs|j=i" => \$jobs,); @@ -81,6 +89,16 @@ elsif ($jobs < 0) usage("--jobs must be a non-negative number"); } +if (defined($perltidy_arg) && $perltidy_arg ne '') +{ + # Check wheter we have --perltidy=PATH or --perltidy PATH, and if the + # latter, consider the argument to be a file to format, not a perltidy path. + unless (grep { $_ eq "--perltidy=$perltidy_arg" } @orig_argv) + { + unshift(@ARGV, $perltidy_arg); + $perltidy_arg = ''; + } +} usage() if $help; @@ -94,6 +112,12 @@ usage("Cannot use --commit with command line file list") # dir, then default location $typedefs_file ||= $ENV{PGTYPEDEFS}; +# get perltidy location: command line wins, then environment, then default. +# --perltidy (with or without a path) and --perl-only all imply perltidy is +# wanted, falling back to the PERLTIDY env var, then "perltidy" in PATH. +my $perltidy = $perltidy_arg || $ENV{PERLTIDY} + || (defined($perltidy_arg) || $perl_only ? "perltidy" : undef); + my $sourcedir = locate_sourcedir(); # get indent location: command line wins, then environment, then try to find @@ -180,6 +204,30 @@ sub check_indent return; } +my $PERLTIDY_VERSION = "20230309"; + +sub check_perltidy +{ + my $ver = `$perltidy -v 2>&1`; + if ($? != 0) + { + print STDERR + "You do not appear to have $perltidy installed on your system.\n" + . "See src/tools/pgindent/README for installation instructions.\n"; + exit 1; + } + + if ($ver !~ m/$PERLTIDY_VERSION/) + { + print STDERR + "You do not appear to have $perltidy version $PERLTIDY_VERSION installed on your system.\n" + . "See src/tools/pgindent/README for installation instructions.\n"; + exit 1; + } + + return; +} + sub locate_sourcedir { # try fairly hard to locate the sourcedir @@ -368,6 +416,43 @@ sub run_indent return $source; } +sub format_c +{ + my $source = shift; + my $source_filename = shift; + my $error_message = ''; + + my $formatted = pre_indent($source); + $formatted = run_indent($formatted, \$error_message); + if ($formatted eq "") + { + print STDERR "Failure in $source_filename: " . $error_message . "\n"; + return ("", 1); + } + return post_indent($formatted); +} + +sub format_perl +{ + my $source = shift; + my $source_filename = shift; + + my $tmp_fh = new File::Temp(TEMPLATE => "pgperltidyXXXXX"); + my $tmp_filename = $tmp_fh->filename; + print $tmp_fh $source; + $tmp_fh->close(); + + my $perltidy_profile = "$sourcedir/perltidyrc"; + my $err = + `$perltidy --profile=$perltidy_profile -b -bext='/' $tmp_filename 2>&1`; + if ($? != 0) + { + print STDERR "Failure in $source_filename: " . $err . "\n"; + return ("", 1); + } + return read_source($tmp_filename); +} + sub diff { my $indented = shift; @@ -397,6 +482,8 @@ Options: --list-of-typedefs=STR string containing typedefs, space separated --excludes=PATH file containing list of filename patterns to ignore --indent=PATH path to pg_bsd_indent program + --perltidy[=PATH] enable Perl formatting (optionally set perltidy path) + --perl-only format only Perl files, skip C formatting --diff show the changes that would be made --check exit with status 2 if any changes would be made --jobs=N, -j N number of parallel workers (0 = num CPUs, default 0) @@ -416,9 +503,46 @@ EOF # main -$filtered_typedefs_fh = load_typedefs(); +my $do_c = !$perl_only; +my $do_perl = defined($perltidy); + +if ($do_c) +{ + $filtered_typedefs_fh = load_typedefs(); + check_indent(); +} -check_indent(); +if ($do_perl) +{ + check_perltidy(); +} + +sub is_c_file +{ + my $filename = shift; + # It needs to have .c or .h extension + return 0 unless $filename =~ /\.[ch]$/; + # Automatically ignore .c and .h files that correspond to a .y or .l file. + # pg_bsd_indent tends to get badly confused by Bison/flex output, and + # there's no value in indenting derived files anyway. + my $otherfile = $filename; + $otherfile =~ s/\.[ch]$/.y/; + return 0 if $otherfile ne $filename && -f $otherfile; + $otherfile =~ s/\.y$/.l/; + return 0 if $otherfile ne $filename && -f $otherfile; + return 1; +} + +sub is_perl_file +{ + my $filename = shift; + # take all .pl and .pm files + return 1 if $filename =~ /\.p[lm]$/; + # take executable files that file(1) thinks are perl files + return 0 unless -x $filename; + my $file_out = `file "$filename"`; + return $file_out =~ /:.*perl[0-9]*\b/i; +} sub discover_files { @@ -449,14 +573,27 @@ sub discover_files my @git_files = `git ls-files -- @dirs`; die "git ls-files error" if $?; chomp(@git_files); - push(@discovered, grep { /\.[ch]$/ } @git_files); + push(@discovered, @git_files); } return @discovered; } # any non-option arguments are files or directories to be processed -push(@files, discover_files(@ARGV)) if @ARGV; +if (@ARGV) +{ + foreach my $f (discover_files(@ARGV)) + { + if ($do_c && is_c_file($f)) + { + push(@files, $f); + } + elsif ($do_perl && is_perl_file($f)) + { + push(@files, $f); + } + } +} # commit file locations are relative to the source root chdir "$sourcedir/../../.." if @commits && $sourcedir; @@ -468,7 +605,8 @@ foreach my $commit (@commits) my @affected = `git diff --diff-filter=ACMR --name-only $prev $commit`; die "git error" if $?; chomp(@affected); - push(@files, @affected); + push(@files, grep { is_c_file($_) } @affected) if $do_c; + push(@files, grep { is_perl_file($_) } @affected) if $do_perl; } warn "No files to process" unless @files; @@ -483,50 +621,41 @@ sub process_file { my $source_filename = shift; - # ignore anything that's not a .c or .h file - return 0 unless $source_filename =~ /\.[ch]$/; - - # don't try to indent a file that doesn't exist + # don't try to format a file that doesn't exist unless (-f $source_filename) { warn "Could not find $source_filename"; return 0; } - # Automatically ignore .c and .h files that correspond to a .y or .l - # file. indent tends to get badly confused by Bison/flex output, - # and there's no value in indenting derived files anyway. - my $otherfile = $source_filename; - $otherfile =~ s/\.[ch]$/.y/; - return 0 if $otherfile ne $source_filename && -f $otherfile; - $otherfile =~ s/\.y$/.l/; - return 0 if $otherfile ne $source_filename && -f $otherfile; my $source = read_source($source_filename); - my $orig_source = $source; - my $error_message = ''; + my ($formatted, $failure); - $source = pre_indent($source); + if ($source_filename =~ /\.[ch]$/) + { + ($formatted, $failure) = format_c($source, $source_filename); + } + else + { + ($formatted, $failure) = format_perl($source, $source_filename); + } - $source = run_indent($source, \$error_message); - if ($source eq "") + if ($failure) { - print STDERR "Failure in $source_filename: " . $error_message . "\n"; return 3; } - $source = post_indent($source); - - if ($source ne $orig_source) + if ($formatted ne $source) { if (!$diff && !$check) { - write_source($source, $source_filename); + write_source($formatted, $source_filename); } else { if ($diff) { - my $output = diff($source, $source_filename); + my $output = diff($formatted, $source_filename); flock($stdout_lock_fh, LOCK_EX); print $output; STDOUT->flush(); diff --git a/src/tools/pgindent/pgperltidy b/src/tools/pgindent/pgperltidy deleted file mode 100755 index 87838d6bde3..00000000000 --- a/src/tools/pgindent/pgperltidy +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/sh - -# src/tools/pgindent/pgperltidy - -set -e - -# set this to override default perltidy program: -PERLTIDY=${PERLTIDY:-perltidy} - -PERLTIDY_VERSION=20230309 -if ! $PERLTIDY -v | grep -q $PERLTIDY_VERSION; then - echo "You do not appear to have $PERLTIDY version $PERLTIDY_VERSION installed on your system." >&2 - exit 1 -fi - -. src/tools/perlcheck/find_perl_files - -find_perl_files "$@" | xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc -- 2.53.0
From 3f5ea701cf8b9abf4fb97804daaee15889b15987 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <[email protected]> Date: Wed, 4 Mar 2026 09:04:15 +0100 Subject: [PATCH v3 8/8] pgindent: Add easy way of getting perltidy We need a very specific perltidy version, but getting that installed is quite a hassle. Especially for people who don't use perl on a daily basis. This adds a small script to download the exact perltidy version that we need. --- src/tools/pgindent/.gitignore | 1 + src/tools/pgindent/README | 16 ++++----- src/tools/pgindent/get_perltidy | 62 +++++++++++++++++++++++++++++++++ src/tools/pgindent/pgindent | 30 +++++++++++++--- 4 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 src/tools/pgindent/.gitignore create mode 100755 src/tools/pgindent/get_perltidy diff --git a/src/tools/pgindent/.gitignore b/src/tools/pgindent/.gitignore new file mode 100644 index 00000000000..1497217113b --- /dev/null +++ b/src/tools/pgindent/.gitignore @@ -0,0 +1 @@ +/perltidy/ diff --git a/src/tools/pgindent/README b/src/tools/pgindent/README index 2e31471d7e6..264038e7e09 100644 --- a/src/tools/pgindent/README +++ b/src/tools/pgindent/README @@ -17,14 +17,14 @@ PREREQUISITES: 2) Install perltidy. Please be sure it is version 20230309 (older and newer versions make different formatting choices, and we want consistency). - You can get the correct version from - https://cpan.metacpan.org/authors/id/S/SH/SHANCOCK/ - To install, follow the usual install process for a Perl module - ("man perlmodinstall" explains it). Or, if you have cpan installed, - this should work: - cpan SHANCOCK/Perl-Tidy-20230309.tar.gz - Or if you have cpanm installed, you can just use: - cpanm https://cpan.metacpan.org/authors/id/S/SH/SHANCOCK/Perl-Tidy-20230309.tar.gz + + The easiest way is to use the get_perltidy script, which downloads, + verifies, and installs the correct version into the source tree: + + src/tools/pgindent/get_perltidy + + This installs perltidy into src/tools/pgindent/perltidy/, which + pgindent will find automatically. DOING THE INDENT RUN BEFORE A NORMAL COMMIT: diff --git a/src/tools/pgindent/get_perltidy b/src/tools/pgindent/get_perltidy new file mode 100755 index 00000000000..09ce4a195cb --- /dev/null +++ b/src/tools/pgindent/get_perltidy @@ -0,0 +1,62 @@ +#!/bin/sh + +# src/tools/pgindent/get_perltidy +# +# Downloads and installs perltidy 20230309 into src/tools/pgindent/perltidy/. + +set -e + +PERLTIDY_VERSION=20230309 +PERLTIDY_TARBALL="Perl-Tidy-${PERLTIDY_VERSION}.tar.gz" +PERLTIDY_URL="https://cpan.metacpan.org/authors/id/S/SH/SHANCOCK/${PERLTIDY_TARBALL}"; +PERLTIDY_SHA256="e22949a208c618d671a18c5829b451abbe9da0da2cddd78fdbfcb036c7361c18" + +# Determine the directory this script lives in, i.e. src/tools/pgindent +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) + +PERLTIDY_DIR="$SCRIPT_DIR/perltidy" +PERLTIDY_BIN="$PERLTIDY_DIR/bin/perltidy" + +# Check if already installed with the correct version +if [ -x "$PERLTIDY_BIN" ]; then + perltidy_lib="$PERLTIDY_DIR/lib/perl5" + installed_version=$(PERL5LIB="$perltidy_lib${PERL5LIB:+:$PERL5LIB}" "$PERLTIDY_BIN" -v 2>/dev/null || true) + if echo "$installed_version" | grep -q "$PERLTIDY_VERSION"; then + echo "perltidy $PERLTIDY_VERSION is already installed in $PERLTIDY_DIR" + exit 0 + fi +fi + +WORK_DIR=$(mktemp -d) +trap 'rm -rf "$WORK_DIR"' EXIT + +# Download +TARBALL="$WORK_DIR/$PERLTIDY_TARBALL" +if command -v curl >/dev/null 2>&1; then + curl -sSL -o "$TARBALL" "$PERLTIDY_URL" +elif command -v wget >/dev/null 2>&1; then + wget -q -O "$TARBALL" "$PERLTIDY_URL" +else + echo "error: neither curl nor wget found" >&2 + exit 1 +fi + +# Verify SHA256 +if command -v sha256sum >/dev/null 2>&1; then + echo "$PERLTIDY_SHA256 $TARBALL" | sha256sum -c - >/dev/null +elif command -v shasum >/dev/null 2>&1; then + echo "$PERLTIDY_SHA256 $TARBALL" | shasum -a 256 -c - >/dev/null +else + echo "error: neither sha256sum nor shasum found" >&2 + exit 1 +fi + +# Extract, build, install +cd "$WORK_DIR" +tar xzf "$TARBALL" +cd "Perl-Tidy-${PERLTIDY_VERSION}" +perl Makefile.PL INSTALL_BASE="$PERLTIDY_DIR" >/dev/null +make >/dev/null +make install >/dev/null + +echo "perltidy $PERLTIDY_VERSION installed in $PERLTIDY_DIR" diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 9d635b7a22b..8003dadc4e1 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -112,14 +112,34 @@ usage("Cannot use --commit with command line file list") # dir, then default location $typedefs_file ||= $ENV{PGTYPEDEFS}; -# get perltidy location: command line wins, then environment, then default. -# --perltidy (with or without a path) and --perl-only all imply perltidy is -# wanted, falling back to the PERLTIDY env var, then "perltidy" in PATH. -my $perltidy = $perltidy_arg || $ENV{PERLTIDY} - || (defined($perltidy_arg) || $perl_only ? "perltidy" : undef); +# get perltidy location: command line wins, then environment, then try to +# find a get_perltidy-installed copy in the source tree, then fall back to +# PATH. --perltidy (with or without a path) and --perl-only all imply +# perltidy is wanted. +my $perltidy = $perltidy_arg || $ENV{PERLTIDY}; my $sourcedir = locate_sourcedir(); +if (!$perltidy && (defined($perltidy_arg) || $perl_only) && $sourcedir) +{ + # Look for a get_perltidy-installed perltidy in the source tree. + my $candidate = "$sourcedir/perltidy/bin/perltidy"; + if (-x $candidate) + { + $perltidy = $candidate; + + # Local installs need PERL5LIB set so perltidy can find its + # modules. + my $libdir = "$sourcedir/perltidy/lib/perl5"; + if (-d $libdir) + { + $ENV{PERL5LIB} = + $ENV{PERL5LIB} ? "$libdir:$ENV{PERL5LIB}" : $libdir; + } + } +} +$perltidy ||= (defined($perltidy_arg) || $perl_only ? "perltidy" : undef); + # get indent location: command line wins, then environment, then try to find # a compiled pg_bsd_indent in the source tree, then fall back to PATH. $indent ||= $ENV{PGINDENT} || $ENV{INDENT}; -- 2.53.0
