pgindent vs. pgperltidy command-line arguments

2023-05-25 Thread Peter Eisentraut 
Until PG15, calling pgindent without arguments would process the whole 
tree.  Now you get


No files to process at ./src/tools/pgindent/pgindent line 372.

Is that intentional?


Also, pgperltidy accepts no arguments and always processes the whole 
tree.  It would be nice if there were a way to process individual files 
or directories, like pgindent can.


Attached is a patch for this.

(It seems that it works ok to pass regular files (not directories) to 
"find", but I'm not sure if it's portable.)From ef9d9cc052d77e1509ce18dc004ac0ab96903a13 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 25 May 2023 11:02:25 +0200
Subject: [PATCH] Allow passing files on command line of pgperltidy

---
 src/tools/perlcheck/find_perl_files | 7 +--
 src/tools/perlcheck/pgperlcritic| 2 +-
 src/tools/perlcheck/pgperlsyncheck  | 2 +-
 src/tools/pgindent/pgperltidy   | 2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/tools/perlcheck/find_perl_files 
b/src/tools/perlcheck/find_perl_files
index fd99dab83b..ad15f6b47a 100644
--- a/src/tools/perlcheck/find_perl_files
+++ b/src/tools/perlcheck/find_perl_files
@@ -3,11 +3,14 @@
 # shell function to find all perl files in the source tree
 
 find_perl_files () {
+   if [ $# -eq 0 ]; then
+   set -- .
+   fi
 {
# take all .pl and .pm files
-   find . -type f -name '*.p[lm]' -print
+   find "$@" -type f -name '*.p[lm]' -print
# take executable files that file(1) thinks are perl files
-   find . -type f -perm -100 -exec file {} \; -print |
+   find "$@" -type f -perm -100 -exec file {} \; -print |
egrep -i ':.*perl[0-9]*\>' |
cut -d: -f1
} | sort -u | grep -v '^\./\.git/'
diff --git a/src/tools/perlcheck/pgperlcritic b/src/tools/perlcheck/pgperlcritic
index 1c2f787580..2ec6f20de3 100755
--- a/src/tools/perlcheck/pgperlcritic
+++ b/src/tools/perlcheck/pgperlcritic
@@ -14,7 +14,7 @@ PERLCRITIC=${PERLCRITIC:-perlcritic}
 
 . src/tools/perlcheck/find_perl_files
 
-find_perl_files | xargs $PERLCRITIC \
+find_perl_files "$@" | xargs $PERLCRITIC \
  --quiet \
  --program-extensions .pl \
  --profile=src/tools/perlcheck/perlcriticrc
diff --git a/src/tools/perlcheck/pgperlsyncheck 
b/src/tools/perlcheck/pgperlsyncheck
index 730f5927cd..da59c9727c 100755
--- a/src/tools/perlcheck/pgperlsyncheck
+++ b/src/tools/perlcheck/pgperlsyncheck
@@ -13,4 +13,4 @@ set -e
 # for zsh
 setopt shwordsplit 2>/dev/null || true
 
-find_perl_files | xargs -L 1 perl $INCLUDES -cw 2>&1 | grep -v OK
+find_perl_files "$@" | xargs -L 1 perl $INCLUDES -cw 2>&1 | grep -v OK
diff --git a/src/tools/pgindent/pgperltidy b/src/tools/pgindent/pgperltidy
index 5e704119eb..6af27d21d5 100755
--- a/src/tools/pgindent/pgperltidy
+++ b/src/tools/pgindent/pgperltidy
@@ -9,4 +9,4 @@ PERLTIDY=${PERLTIDY:-perltidy}
 
 . src/tools/perlcheck/find_perl_files
 
-find_perl_files | xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc
+find_perl_files "$@" | xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc
-- 
2.40.1

Re: pgindent vs. pgperltidy command-line arguments

2023-07-06 Thread Andrew Dunstan 


On 2023-06-21 We 07:35, Andrew Dunstan wrote:



On 2023-06-21 We 05:09, Peter Eisentraut wrote:

On 20.06.23 17:38, Andrew Dunstan wrote:

+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).


That makes sense to me.  Here is a small update with this behavior 
change and associated documentation update.


I'm intending to add some of the new pgindent features to 
pgperltidy. Preparatory to that here's a rewrite of pgperltidy in 
perl - no new features yet but it does remove the hardcoded path, 
and requires you to pass in one or more files / directories as 
arguments.


Are you planning to touch pgperlcritic and pgperlsyncheck as well? 



Yeah, it would make sense to.




Here's a patch that turns all these into perl scripts.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com
diff --git a/src/tools/perlcheck/FindPerlFiles.pm b/src/tools/perlcheck/FindPerlFiles.pm
new file mode 100644
index 00..3a098f6614
--- /dev/null
+++ b/src/tools/perlcheck/FindPerlFiles.pm
@@ -0,0 +1,62 @@
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+=pod
+
+=head1 NAME
+
+FindPerlFiles - module for finding perl files from a list of paths
+
+=head1 SYNOPSIS
+
+  use FindPerlFiles;
+
+  my @files = FindPerlFiles::findperl(path, ...);
+
+
+=head1 DESCRIPTION
+
+FindPerlFiles finds files which either have a perl extension (.pl or .pm) or
+are bot executable and found by the `file` program to be perl scripts.
+
+=cut
+
+
+package FindPerlFiles;
+
+use strict;
+use warnings;
+
+use File::Find;
+use File::stat;
+use Fcntl ':mode';
+
+my @files;
+
+sub _is_perl_exec
+{
+	my $name = shift;
+	my $out = `file $name 2>/dev/null`;
+	return $out =~ /:.*perl[0-9]*\b/i;
+}
+
+sub findperl
+{
+	my @files ;
+	my $wanted = sub
+	{
+		my $name = $File::Find::name;
+		my $st;
+		# check it's a plain file and either it has a perl extension (.p[lm])
+		# or it's executable and `file` thinks it's a perl script.
+		($st = lstat($_))
+		  && -f $st
+		  && (/\.p[lm]$/ || ((($st->mode & S_IXUSR) && _is_perl_exec($_
+		  && push(@files, $name);
+	};
+
+	File::Find::find({ wanted => $wanted }, @_);
+	return @files;
+}
+
+1;
diff --git a/src/tools/perlcheck/find_perl_files b/src/tools/perlcheck/find_perl_files
deleted file mode 100644
index 20dceb800d..00
--- a/src/tools/perlcheck/find_perl_files
+++ /dev/null
@@ -1,18 +0,0 @@
-# src/tools/perlcheck/find_perl_files
-
-# shell function to find all perl files in the source tree
-
-find_perl_files () {
-	if [ $# -eq 0 ]; then
-		echo 'No files to process' 1>&2
-		return
-	fi
-{
-		# take all .pl and .pm files
-		find "$@" -type f -name '*.p[lm]' -print
-		# take executable files that file(1) thinks are perl files
-		find "$@" -type f -perm -100 -exec file {} \; -print |
-		egrep -i ':.*perl[0-9]*\>' |
-		cut -d: -f1
-	} | sort -u | grep -v '^\./\.git/'
-}
diff --git a/src/tools/perlcheck/pgperlcritic b/src/tools/perlcheck/pgperlcritic
index 2ec6f20de3..87d5e92df7 100755
--- a/src/tools/perlcheck/pgperlcritic
+++ b/src/tools/perlcheck/pgperlcritic
@@ -1,20 +1,27 @@
-#!/bin/sh
+#!/usr/bin/perl
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
 
 # src/tools/perlcheck/pgperlcritic
 
-test -f src/tools/perlcheck/perlcriticrc || {
-	echo could not find src/tools/perlcheck/perlcriticrc
-	exit 1
-	}
+use strict;
+use warnings;
 
-set -e
+use FindBin ();
+use lib "$FindBin::Bin";
+use FindPerlFiles;
+
+die "No directories or files specified\n" unless @ARGV;
+
+my $rc_loc = "$FindBin::Bin/perlcriticrc";
+
+die "no $rc_loc\n" unless -f $rc_loc;
 
 # set this to override default perlcritic program:
-PERLCRITIC=${PERLCRITIC:-perlcritic}
+my $perlcritic = $ENV{PERLCRITIC} || 'perlcritic';
 
-. src/tools/perlcheck/find_perl_files
+my @files = FindPerlFiles::findperl(@ARGV);
 
-find_perl_files "$@" | xargs $PERLCRITIC \
-	  --quiet \
-	  --program-extensions .pl \
-	  --profile=src/tools/perlcheck/perlcriticrc
+exit unless @files;
+
+exec $perlcritic, '--program-extensions',  '.pl', "--profile=$rc_loc", @files
diff --git a/src/tools/perlcheck/pgperlsyncheck b/src/tools/perlcheck/pgperlsyncheck
index da59c9727c..70ff248daf 100755
--- a/src/tools/perlcheck/pgperlsyncheck
+++ b/src/tools/perlcheck/pgperlsyncheck
@@ -1,16 +1,48 @@
-#!/bin/sh
+#!/usr/bin/perl
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
 
 # script to detect compile time errors and warnings in all perl files
 
-INCLUDES="-I src/tools/msvc -I src/tools/msvc/dummylib -I src/backend/catalog"
-INCLUDES="-I src/test/perl -I src/backend/utils/mb/Unicode $INCLUDES"
-INCLUDES="-I src/bin/pg_rewind -I src/test/ssl/t $INCLUDES"
+use strict;
+use warnings;
 
-set -e
+use FindBin ();
+use lib "$FindBin::Bin";
 
-. src/tools/perlcheck/find_perl_files
+use Cwd qw(abs_path);
+use FindPerlFiles;
 
-# for zsh
-setopt shwordsplit 2>/dev/null || true
+die "No directories or files specified\n" unless @ARGV;
 
-find_perl_files "$@" | xargs -L 1 perl $INCLUD

Re: pgindent vs. pgperltidy command-line arguments

2023-05-25 Thread Daniel Gustafsson 
> On 25 May 2023, at 11:10, Peter Eisentraut  wrote:

> Also, pgperltidy accepts no arguments and always processes the whole tree.  
> It would be nice if there were a way to process individual files or 
> directories, like pgindent can.

+1, thanks!  I've wanted that several times but never gotten around to doing
anything about it.

--
Daniel Gustafsson

Re: pgindent vs. pgperltidy command-line arguments

2023-05-25 Thread Tom Lane 
Peter Eisentraut  writes:
> Until PG15, calling pgindent without arguments would process the whole 
> tree.  Now you get
> No files to process at ./src/tools/pgindent/pgindent line 372.
> Is that intentional?

It was intentional, cf b16259b3c and the linked discussion.

> Also, pgperltidy accepts no arguments and always processes the whole 
> tree.  It would be nice if there were a way to process individual files 
> or directories, like pgindent can.

+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).

> Attached is a patch for this.
> (It seems that it works ok to pass regular files (not directories) to 
> "find", but I'm not sure if it's portable.)

The POSIX spec for find(1) gives an example of applying find to
what they evidently intend to be a plain file:

if [ -n "$(find file1 -prune -newer file2)" ]; then
printf %s\\n "file1 is newer than file2"
fi

So while I don't see it written in so many words, I think you
can assume it's portable.

regards, tom lane

Re: pgindent vs. pgperltidy command-line arguments

2023-06-14 Thread Peter Eisentraut 

On 25.05.23 15:20, Tom Lane wrote:

Peter Eisentraut  writes:

Until PG15, calling pgindent without arguments would process the whole
tree.  Now you get
No files to process at ./src/tools/pgindent/pgindent line 372.
Is that intentional?


It was intentional, cf b16259b3c and the linked discussion.


Also, pgperltidy accepts no arguments and always processes the whole
tree.  It would be nice if there were a way to process individual files
or directories, like pgindent can.


+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).


That makes sense to me.  Here is a small update with this behavior 
change and associated documentation update.
From 44f7bcdcc0849a55459d4c2da27ee1976c704933 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 14 Jun 2023 09:33:03 +0200
Subject: [PATCH v2] Allow and require passing files on command line of
 pgperltidy

pgperltidy as well as pgperlcritic and pgperlsyncheck now allow
passing files and directories on the command line, like pgindent does.
(Previously, they would always operate on the whole tree.)

Also, for consistency with pgindent's new behavior (as of b16259b3c1),
passing an argument is now required.  To get the previous default
behavior, use "pgperltidy ." for example.

Discussion: 
https://www.postgresql.org/message-id/flat/45aacd8a-5265-d9da-8df2-b8e2c0cf6a07%40eisentraut.org
---
 src/tools/perlcheck/find_perl_files | 8 ++--
 src/tools/perlcheck/pgperlcritic| 2 +-
 src/tools/perlcheck/pgperlsyncheck  | 2 +-
 src/tools/pgindent/README   | 2 +-
 src/tools/pgindent/pgperltidy   | 2 +-
 5 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/tools/perlcheck/find_perl_files 
b/src/tools/perlcheck/find_perl_files
index fd99dab83b..20dceb800d 100644
--- a/src/tools/perlcheck/find_perl_files
+++ b/src/tools/perlcheck/find_perl_files
@@ -3,11 +3,15 @@
 # shell function to find all perl files in the source tree
 
 find_perl_files () {
+   if [ $# -eq 0 ]; then
+   echo 'No files to process' 1>&2
+   return
+   fi
 {
# take all .pl and .pm files
-   find . -type f -name '*.p[lm]' -print
+   find "$@" -type f -name '*.p[lm]' -print
# take executable files that file(1) thinks are perl files
-   find . -type f -perm -100 -exec file {} \; -print |
+   find "$@" -type f -perm -100 -exec file {} \; -print |
egrep -i ':.*perl[0-9]*\>' |
cut -d: -f1
} | sort -u | grep -v '^\./\.git/'
diff --git a/src/tools/perlcheck/pgperlcritic b/src/tools/perlcheck/pgperlcritic
index 1c2f787580..2ec6f20de3 100755
--- a/src/tools/perlcheck/pgperlcritic
+++ b/src/tools/perlcheck/pgperlcritic
@@ -14,7 +14,7 @@ PERLCRITIC=${PERLCRITIC:-perlcritic}
 
 . src/tools/perlcheck/find_perl_files
 
-find_perl_files | xargs $PERLCRITIC \
+find_perl_files "$@" | xargs $PERLCRITIC \
  --quiet \
  --program-extensions .pl \
  --profile=src/tools/perlcheck/perlcriticrc
diff --git a/src/tools/perlcheck/pgperlsyncheck 
b/src/tools/perlcheck/pgperlsyncheck
index 730f5927cd..da59c9727c 100755
--- a/src/tools/perlcheck/pgperlsyncheck
+++ b/src/tools/perlcheck/pgperlsyncheck
@@ -13,4 +13,4 @@ set -e
 # for zsh
 setopt shwordsplit 2>/dev/null || true
 
-find_perl_files | xargs -L 1 perl $INCLUDES -cw 2>&1 | grep -v OK
+find_perl_files "$@" | xargs -L 1 perl $INCLUDES -cw 2>&1 | grep -v OK
diff --git a/src/tools/pgindent/README b/src/tools/pgindent/README
index b2b134ee6a..f5fdfc5d2f 100644
--- a/src/tools/pgindent/README
+++ b/src/tools/pgindent/README
@@ -45,7 +45,7 @@ DOING THE INDENT RUN:
 
 4) Indent the Perl code using perltidy:
 
-   src/tools/pgindent/pgperltidy
+   src/tools/pgindent/pgperltidy .
 
If you want to use some perltidy version that's not in your PATH,
first set the PERLTIDY environment variable to point to it.
diff --git a/src/tools/pgindent/pgperltidy b/src/tools/pgindent/pgperltidy
index 5e704119eb..6af27d21d5 100755
--- a/src/tools/pgindent/pgperltidy
+++ b/src/tools/pgindent/pgperltidy
@@ -9,4 +9,4 @@ PERLTIDY=${PERLTIDY:-perltidy}
 
 . src/tools/perlcheck/find_perl_files
 
-find_perl_files | xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc
+find_perl_files "$@" | xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc

base-commit: 0f8cfaf8921fed35f0b92d918ce95eec7b46ff05
-- 
2.41.0

Re: pgindent vs. pgperltidy command-line arguments

2023-06-20 Thread Andrew Dunstan 


On 2023-06-14 We 03:37, Peter Eisentraut wrote:

On 25.05.23 15:20, Tom Lane wrote:

Peter Eisentraut  writes:

Until PG15, calling pgindent without arguments would process the whole
tree.  Now you get
No files to process at ./src/tools/pgindent/pgindent line 372.
Is that intentional?


It was intentional, cf b16259b3c and the linked discussion.


Also, pgperltidy accepts no arguments and always processes the whole
tree.  It would be nice if there were a way to process individual files
or directories, like pgindent can.


+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).


That makes sense to me.  Here is a small update with this behavior 
change and associated documentation update.



I'm intending to add some of the new pgindent features to pgperltidy. 
Preparatory to that here's a rewrite of pgperltidy in perl - no new 
features yet but it does remove the hardcoded path, and requires you to 
pass in one or more files / directories as arguments.



cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com
#!/usr/bin/perl

# Copyright (c) 2023, PostgreSQL Global Development Group

# src/tools/pgindent/pgperltidy

use strict;
use warnings;

use File::Find;

my $perltidy = $ENV{PERLTIDY} || 'perltidy';

my @files;

die "No directories or files specified" unless @ARGV;

sub is_perl_exec
{
my $name = shift;
my $out = `file $name 2>/dev/null`;
return $out =~ /:.*perl[0-9]*\b/i;
}

my $wanted = sub {

my $name = $File::Find::name;
my ($dev, $ino, $mode, $nlink, $uid, $gid);

# check it's a plain file and either it has a perl extension (.p[lm])
# or it's executable and `file` thinks it's a perl script.

(($dev, $ino, $mode, $nlink, $uid, $gid) = lstat($_))
  && -f _
  && (/\.p[lm]$/ || ((($mode & 0100) == 0100) && is_perl_exec($_)))
  && push(@files, $name);
};

File::Find::find({ wanted => $wanted }, @ARGV);

my $list = join(" ", @files);

system "$perltidy --profile=src/tools/pgindent/perltidyrc $list";

Re: pgindent vs. pgperltidy command-line arguments

2023-06-20 Thread Dagfinn Ilmari Mannsåker 
Andrew Dunstan  writes:

> I'm intending to add some of the new pgindent features to
> pgperltidy. Preparatory to that here's a rewrite of pgperltidy in perl -
> no new features yet but it does remove the hardcoded path, and requires
> you to pass in one or more files / directories as arguments.

Good idea, here's some comments.

> #!/usr/bin/perl
>
> # Copyright (c) 2023, PostgreSQL Global Development Group
>
> # src/tools/pgindent/pgperltidy
>
> use strict;
> use warnings;
>
> use File::Find;
>
> my $perltidy = $ENV{PERLTIDY} || 'perltidy';
>
> my @files;
>
> die "No directories or files specified" unless @ARGV;

It's not really useful to have the file name and line in errors like
this, adding a "\n" to the end of the message suppresses that.

> sub is_perl_exec
> {
>   my $name = shift;
>   my $out = `file $name 2>/dev/null`;
>   return $out =~ /:.*perl[0-9]*\b/i;
> }

> my $wanted = sub {
>
>   my $name = $File::Find::name;
>   my ($dev, $ino, $mode, $nlink, $uid, $gid);
>
>   # check it's a plain file and either it has a perl extension (.p[lm])
>   # or it's executable and `file` thinks it's a perl script.
>
>   (($dev, $ino, $mode, $nlink, $uid, $gid) = lstat($_))
> && -f _
> && (/\.p[lm]$/ || ((($mode & 0100) == 0100) && is_perl_exec($_)))
> && push(@files, $name);
> };

The core File::stat and Fcntl modules can make this neater:

use File::stat;
use Fcntl ':mode';
 
my $wanted = sub {
my $st;
push @files, $File::Find::name
if $st = lstat($_) && -f $st
&& (/\.p[lm]$/ || (($st->mode & S_IXUSR) && 
is_perl_exec($_)));
};

> File::Find::find({ wanted => $wanted }, @ARGV);
>
> my $list = join(" ", @files);
>
> system "$perltidy --profile=src/tools/pgindent/perltidyrc $list";

It's better to use the list form of system, to avoid shell escaping
issues.  Also, since this is the last thing in the script we might as
well exec it instead:

exec $perltidy, '--profile=src/tools/pgindent/perltidyrc', @files;

- ilmari

Re: pgindent vs. pgperltidy command-line arguments

2023-06-21 Thread Peter Eisentraut 

On 20.06.23 17:38, Andrew Dunstan wrote:

+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).


That makes sense to me.  Here is a small update with this behavior 
change and associated documentation update.


I'm intending to add some of the new pgindent features to pgperltidy. 
Preparatory to that here's a rewrite of pgperltidy in perl - no new 
features yet but it does remove the hardcoded path, and requires you to 
pass in one or more files / directories as arguments.


Are you planning to touch pgperlcritic and pgperlsyncheck as well?  If 
not, part of my patch would still be useful.  Maybe I should commit my 
posted patch for PG16, to keep consistency with pgindent, and then your 
work would presumably be considered for PG17.

Re: pgindent vs. pgperltidy command-line arguments

2023-06-21 Thread Andrew Dunstan 


On 2023-06-21 We 05:09, Peter Eisentraut wrote:

On 20.06.23 17:38, Andrew Dunstan wrote:

+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).


That makes sense to me.  Here is a small update with this behavior 
change and associated documentation update.


I'm intending to add some of the new pgindent features to pgperltidy. 
Preparatory to that here's a rewrite of pgperltidy in perl - no new 
features yet but it does remove the hardcoded path, and requires you 
to pass in one or more files / directories as arguments.


Are you planning to touch pgperlcritic and pgperlsyncheck as well? 



Yeah, it would make sense to.


If not, part of my patch would still be useful.  Maybe I should commit 
my posted patch for PG16, to keep consistency with pgindent, and then 
your work would presumably be considered for PG17.



That sounds like a good plan.


cheers


andrew

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

Re: pgindent vs. pgperltidy command-line arguments

2023-06-21 Thread Peter Eisentraut 

On 21.06.23 13:35, Andrew Dunstan wrote:
If not, part of my patch would still be useful.  Maybe I should commit 
my posted patch for PG16, to keep consistency with pgindent, and then 
your work would presumably be considered for PG17.


That sounds like a good plan.


done