From 0c7e7a2215cb017ba59dc20258f9edfb19a14c1a Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 31 Dec 2025 11:07:58 +0100 Subject: [PATCH v2 3/7] 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 | 185 ++++++++++++++++++++++++++++------ src/tools/pgindent/pgperltidy | 18 ---- 3 files changed, 161 insertions(+), 56 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 6570726381c..f8ac9c8268b 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'; @@ -32,10 +32,15 @@ my $indent_opts = my $devnull = File::Spec->devnull; my ($typedefs_file, $typedef_str, @excludes, $indent, $diff, - $check, $help, @commits,); + $check, $help, @commits, $perltidy_arg, $perl_only,); $help = 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, @@ -43,10 +48,21 @@ 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,); GetOptions(%options) || usage("bad command line argument"); +if (defined($perltidy_arg) && $perltidy_arg ne '') +{ + unless (grep { $_ eq "--perltidy=$perltidy_arg" } @orig_argv) + { + unshift(@ARGV, $perltidy_arg); + $perltidy_arg = ''; + } +} + usage() if $help; usage("Cannot use --commit with command line file list") @@ -56,6 +72,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 @@ -142,6 +164,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 @@ -327,6 +373,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; @@ -356,6 +439,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 The --excludes and --commit options can be given more than once. @@ -374,20 +459,64 @@ 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(); +} + +if ($do_perl) +{ + check_perltidy(); +} -check_indent(); +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; +} 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); + || return; + if ($do_c && is_c_file($File::Find::name)) + { + push(@files, $File::Find::name); + } + elsif ($do_perl && is_perl_file($File::Find::name)) + { + push(@files, $File::Find::name); + } }; # any non-option arguments are files or directories to be processed -File::Find::find({ wanted => $wanted }, @ARGV) if @ARGV; +File::Find::find({ wanted => $wanted, no_chdir => 1 }, @ARGV) if @ARGV; # commit file locations are relative to the source root chdir "$sourcedir/../../.." if @commits && $sourcedir; @@ -399,7 +528,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; @@ -416,51 +546,42 @@ foreach my $source_filename (@files) next if $processed{$source_filename}; $processed{$source_filename} = 1; - # ignore anything that's not a .c or .h file - next 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"; next; } - # 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; - $otherfile =~ s/\.y$/.l/; - next 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"; $status = 3; next; } - $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) { - print diff($source, $source_filename); + print diff($formatted, $source_filename); } if ($check) 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