Use Getopt::Long for catalog scripts

Replace hand-rolled option parsing with the Getopt module. This is
shorter and easier to read. In passing, make some cosmetic adjustments
for consistency.

Author: John Naylor
Reviewed-by: David Fetter
Discussion: https://postgr.es/m/CACPNZCvRjepXh5b2N50njN+rO_2Nzcf=jhMkKX7=79XWUKJyKA@mail.gmail.com

diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index d9f92ba70191c6b00f3ded5a6f15d71b35247349..f186198fc6cb2dc90d8c1eba7b27248ee81de137 100644 (file)
--- a/src/backend/catalog/Makefile
+++ b/src/backend/catalog/Makefile
@@ -68,9 +68,6 @@ POSTGRES_BKI_DATA = $(addprefix $(top_srcdir)/src/include/catalog/,\
        pg_ts_template.dat pg_type.dat \
        )
 
-# location of Catalog.pm
-catalogdir = $(top_srcdir)/src/backend/catalog
-
 all: distprep generated-header-symlinks
 
 distprep: bki-stamp
@@ -88,9 +85,8 @@ generated-header-symlinks: $(top_builddir)/src/include/catalog/header-stamp
 # instead is cheating a bit, but it will achieve the goal of updating the
 # version number when it changes.
 bki-stamp: genbki.pl Catalog.pm $(POSTGRES_BKI_SRCS) $(POSTGRES_BKI_DATA) $(top_srcdir)/configure.in
-       $(PERL) -I $(catalogdir) $< \
-               -I $(top_srcdir)/src/include/ --set-version=$(MAJORVERSION) \
-               $(POSTGRES_BKI_SRCS)
+       $(PERL) $< --include-path=$(top_srcdir)/src/include/ \
+               --set-version=$(MAJORVERSION) $(POSTGRES_BKI_SRCS)
        touch $@
 
 # The generated headers must all be symlinked into builddir/src/include/,

diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index be81094ffb6235068ae803a2ad2f4cab797e3ea8..4935e00fb27d72d2c212bc3f1e2983564088b142 100644 (file)
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -16,6 +16,7 @@
 
 use strict;
 use warnings;
+use Getopt::Long;
 
 use File::Basename;
 use File::Spec;
@@ -23,43 +24,21 @@ BEGIN  { use lib File::Spec->rel2abs(dirname(__FILE__)); }
 
 use Catalog;
 
-my @input_files;
 my $output_path = '';
 my $major_version;
 my $include_path;
 
-# Process command line switches.
-while (@ARGV)
-{
-       my $arg = shift @ARGV;
-       if ($arg !~ /^-/)
-       {
-               push @input_files, $arg;
-       }
-       elsif ($arg =~ /^-I/)
-       {
-               $include_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
-       }
-       elsif ($arg =~ /^-o/)
-       {
-               $output_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
-       }
-       elsif ($arg =~ /^--set-version=(.*)$/)
-       {
-               $major_version = $1;
-               die "Invalid version string.\n"
-                 if !($major_version =~ /^\d+$/);
-       }
-       else
-       {
-               usage();
-       }
-}
+GetOptions(
+       'output:s'       => \$output_path,
+       'set-version:s'  => \$major_version,
+       'include-path:s' => \$include_path) || usage();
 
 # Sanity check arguments.
-die "No input files.\n" if !@input_files;
-die "--set-version must be specified.\n" if !defined $major_version;
-die "-I, the header include path, must be specified.\n" if !$include_path;
+die "No input files.\n" unless @ARGV;
+die "--set-version must be specified.\n" unless $major_version;
+die "Invalid version string: $major_version\n"
+  unless $major_version =~ /^\d+$/;
+die "--include-path must be specified.\n" unless $include_path;
 
 # Make sure paths end with a slash.
 if ($output_path ne '' && substr($output_path, -1) ne '/')
@@ -79,7 +58,7 @@ my @toast_decls;
 my @index_decls;
 my %oidcounts;
 
-foreach my $header (@input_files)
+foreach my $header (@ARGV)
 {
        $header =~ /(.+)\.h$/
          or die "Input files need to be header files.\n";
@@ -917,12 +896,12 @@ sub form_pg_type_symbol
 sub usage
 {
        die <<EOM;
-Usage: genbki.pl [options] header...
+Usage: perl -I [directory of Catalog.pm] genbki.pl [--output/-o <path>] [--include-path/-i <path>] header...
 
 Options:
-    -I               include path
-    -o               output path
+    --output         Output directory (default '.')
     --set-version    PostgreSQL version number for initdb cross-check
+    --include-path   Include path in source tree
 
 genbki.pl generates BKI files and symbol definition
 headers from specially formatted header files and .dat

diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index f17a78ebcd20ad5e507fd64f3a63d609f3496af3..2ffacae666b61ebfec02e4b8d2aac5043ca2c980 100644 (file)
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -18,32 +18,14 @@ use Catalog;
 
 use strict;
 use warnings;
+use Getopt::Long;
 
-# Collect arguments
-my @input_files;
 my $output_path = '';
 my $include_path;
 
-while (@ARGV)
-{
-       my $arg = shift @ARGV;
-       if ($arg !~ /^-/)
-       {
-               push @input_files, $arg;
-       }
-       elsif ($arg =~ /^-o/)
-       {
-               $output_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
-       }
-       elsif ($arg =~ /^-I/)
-       {
-               $include_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
-       }
-       else
-       {
-               usage();
-       }
-}
+GetOptions(
+       'output:s'       => \$output_path,
+       'include-path:s' => \$include_path) || usage();
 
 # Make sure output_path ends in a slash.
 if ($output_path ne '' && substr($output_path, -1) ne '/')
@@ -52,8 +34,8 @@ if ($output_path ne '' && substr($output_path, -1) ne '/')
 }
 
 # Sanity check arguments.
-die "No input files.\n"                       if !@input_files;
-die "No include path; you must specify -I.\n" if !$include_path;
+die "No input files.\n"                   unless @ARGV;
+die "--include-path must be specified.\n" unless $include_path;
 
 # Read all the input files into internal data structures.
 # Note: We pass data file names as arguments and then look for matching
@@ -63,7 +45,7 @@ die "No include path; you must specify -I.\n" if !$include_path;
 # more than one data file.
 my %catalogs;
 my %catalog_data;
-foreach my $datfile (@input_files)
+foreach my $datfile (@ARGV)
 {
        $datfile =~ /(.+)\.dat$/
          or die "Input files need to be data (.dat) files.\n";
@@ -292,7 +274,11 @@ Catalog::RenameTempFile($tabfile,    $tmpext);
 sub usage
 {
        die <<EOM;
-Usage: perl -I [directory of Catalog.pm] Gen_fmgrtab.pl -I [include path] [path to pg_proc.dat]
+Usage: perl -I [directory of Catalog.pm] Gen_fmgrtab.pl [--include-path/-i <path>] [path to pg_proc.dat]
+
+Options:
+    --output         Output directory (default '.')
+    --include-path   Include path in source tree
 
 Gen_fmgrtab.pl generates fmgroids.h, fmgrprotos.h, and fmgrtab.c from
 pg_proc.dat

diff --git a/src/backend/utils/Makefile b/src/backend/utils/Makefile
index a5251327e275443f2c5ff476121b3c23eb8b7491..c2618e5272a651eb8271a01f9e08c6b44062eff7 100644 (file)
--- a/src/backend/utils/Makefile
+++ b/src/backend/utils/Makefile
@@ -35,7 +35,7 @@ $(SUBDIRS:%=%-recursive): fmgr-stamp errcodes.h
 # the timestamps of the individual output files, because the Perl script
 # won't update them if they didn't change (to avoid unnecessary recompiles).
 fmgr-stamp: Gen_fmgrtab.pl $(catalogdir)/Catalog.pm $(top_srcdir)/src/include/catalog/pg_proc.dat $(top_srcdir)/src/include/access/transam.h
-       $(PERL) -I $(catalogdir) $< -I $(top_srcdir)/src/include/ $(top_srcdir)/src/include/catalog/pg_proc.dat
+       $(PERL) -I $(catalogdir) $< --include-path=$(top_srcdir)/src/include/ $(top_srcdir)/src/include/catalog/pg_proc.dat
        touch $@
 
 errcodes.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-errcodes.pl