Long story short: It expects an array of paths, but will die if it
can't find the symbol in the first path. Arguably it's a bug, but
since the original use case for multiple paths is long gone anyway, it
might never occur in practice. The attached patch changes it to expect
a single path.
Also attached: a few minor build script cleanups I've noticed along the way:
-more accurate error message
-s/print sprintf/printf/
-a recent perltidy change made a couple multi-line strings look odd,
so use heredocs (which other scripts use in this case anyway)
-make generated file footers match project style
-remove a duplicate comment
-John Naylor
From be3cbb1499e026355697b1aff890a3049cdef4ac Mon Sep 17 00:00:00 2001
From: John Naylor <jcnay...@gmail.com>
Date: Sat, 19 May 2018 13:14:58 +0700
Subject: [PATCH] Update FindDefinedSymbol() to match current practice.
Once upon a time, headers passed to build scripts could live in
either $builddir or $sourcedir, so the Makefiles needed to pass
both directories to the scripts. FindDefinedSymbol() was intended
to deal with muliple directories, but it's actually broken: It dies
if it can't find the symbol in the first given path. This went
unnoticed for so long, because the catalog scripts output distprep
targets, so only need $sourcedir.
Fix so that we expect only one include path.
---
src/backend/catalog/Catalog.pm | 36 ++++++++++++++++--------------------
src/backend/utils/Gen_fmgrtab.pl | 8 ++++----
src/include/catalog/unused_oids | 2 +-
3 files changed, 21 insertions(+), 25 deletions(-)
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 07bd5f3..b88b36b 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -365,34 +365,30 @@ sub RenameTempFile
}
# Find a symbol defined in a particular header file and extract the value.
-#
-# The include path has to be passed as a reference to an array.
sub FindDefinedSymbol
{
my ($catalog_header, $include_path, $symbol) = @_;
+ my $value;
- for my $path (@$include_path)
+ # Make sure include path ends in a slash.
+ if (substr($include_path, -1) ne '/')
{
-
- # Make sure include path ends in a slash.
- if (substr($path, -1) ne '/')
- {
- $path .= '/';
- }
- my $file = $path . $catalog_header;
- next if !-f $file;
- open(my $find_defined_symbol, '<', $file) || die "$file: $!";
- while (<$find_defined_symbol>)
+ $include_path .= '/';
+ }
+ my $file = $include_path . $catalog_header;
+ next if !-f $file;
+ open(my $find_defined_symbol, '<', $file) || die "$file: $!";
+ while (<$find_defined_symbol>)
+ {
+ if (/^#define\s+\Q$symbol\E\s+(\S+)/)
{
- if (/^#define\s+\Q$symbol\E\s+(\S+)/)
- {
- return $1;
- }
+ $value = $1;
+ last;
}
- close $find_defined_symbol;
- die "$file: no definition found for $symbol\n";
}
- die "$catalog_header: not found in any include directory\n";
+ close $find_defined_symbol;
+ return $value if defined $value;
+ die "$file: no definition found for $symbol\n";
}
# Similar to FindDefinedSymbol, but looks in the bootstrap metadata.
diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index 6c9f1a7..465bcd5 100644
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -22,7 +22,7 @@ use warnings;
# Collect arguments
my @input_files;
my $output_path = '';
-my @include_path;
+my $include_path;
while (@ARGV)
{
@@ -37,7 +37,7 @@ while (@ARGV)
}
elsif ($arg =~ /^-I/)
{
- push @include_path, length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
+ $include_path = length($arg) > 2 ? substr($arg, 2) : shift @ARGV;
}
else
{
@@ -53,7 +53,7 @@ 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 at least once.\n" if !@include_path;
+die "No include path; you must specify -I.\n" if !$include_path;
# Read all the input files into internal data structures.
# Note: We pass data file names as arguments and then look for matching
@@ -80,7 +80,7 @@ foreach my $datfile (@input_files)
# Fetch some values for later.
my $FirstBootstrapObjectId =
- Catalog::FindDefinedSymbol('access/transam.h', \@include_path,
+ Catalog::FindDefinedSymbol('access/transam.h', $include_path,
'FirstBootstrapObjectId');
my $INTERNALlanguageId =
Catalog::FindDefinedSymbolFromData($catalog_data{pg_language},
diff --git a/src/include/catalog/unused_oids b/src/include/catalog/unused_oids
index 2780de0..c5fc378 100755
--- a/src/include/catalog/unused_oids
+++ b/src/include/catalog/unused_oids
@@ -34,7 +34,7 @@ my $oids = Catalog::FindAllOidsFromHeaders(@input_files);
# Also push FirstBootstrapObjectId to serve as a terminator for the last gap.
my $FirstBootstrapObjectId =
- Catalog::FindDefinedSymbol('access/transam.h', [".."],
+ Catalog::FindDefinedSymbol('access/transam.h', '..',
'FirstBootstrapObjectId');
push @{$oids}, $FirstBootstrapObjectId;
--
2.7.4
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 2889cb9..07bd5f3 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -196,7 +196,7 @@ sub ParseHeader
else
{
die
- "unknown column option $attopt on column $attname";
+ "unknown or misformatted column option $attopt on column $attname";
}
if ($column{forcenull} and $column{forcenotnull})
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index fb61db0..ebdc919 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -357,8 +357,7 @@ EOM
}
# Emit Anum_* constants
- print $def
- sprintf("#define Anum_%s_%s %s\n", $catname, $attname, $attnum);
+ printf $def "#define Anum_%s_%s %s\n", $catname, $attname, $attnum;
}
print $bki "\n )\n";
@@ -493,7 +492,7 @@ EOM
}
print $bki "close $catname\n";
- print $def sprintf("\n#endif\t\t\t\t\t\t\t/* %s_D_H */\n", uc $catname);
+ printf $def "\n#endif\t\t\t\t\t\t\t/* %s_D_H */\n", uc $catname;
# Close and rename definition header
close $def;
diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index 5fd5313..6c9f1a7 100644
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -117,8 +117,8 @@ open my $pfh, '>', $protosfile . $tmpext
open my $tfh, '>', $tabfile . $tmpext
or die "Could not open $tabfile$tmpext: $!";
-print $ofh
- qq|/*-------------------------------------------------------------------------
+print $ofh <<OFH;
+/*-------------------------------------------------------------------------
*
* fmgroids.h
* Macros that define the OIDs of built-in functions.
@@ -152,10 +152,10 @@ print $ofh
* its equivalent macro will be defined with the lowest OID among those
* entries.
*/
-|;
+OFH
-print $pfh
- qq|/*-------------------------------------------------------------------------
+print $pfh <<PFH;
+/*-------------------------------------------------------------------------
*
* fmgrprotos.h
* Prototypes for built-in functions.
@@ -178,10 +178,10 @@ print $pfh
#include "fmgr.h"
-|;
+PFH
-print $tfh
- qq|/*-------------------------------------------------------------------------
+print $tfh <<TFH;
+/*-------------------------------------------------------------------------
*
* fmgrtab.c
* The function manager's table of internal functions.
@@ -206,7 +206,7 @@ print $tfh
#include "utils/fmgrtab.h"
#include "utils/fmgrprotos.h"
-|;
+TFH
# Emit #define's and extern's -- only one per prosrc value
my %seenit;
@@ -280,8 +280,8 @@ print $tfh "};\n";
# And add the file footers.
-print $ofh "\n#endif /* FMGROIDS_H */\n";
-print $pfh "\n#endif /* FMGRPROTOS_H */\n";
+print $ofh "\n#endif\t\t\t\t\t\t\t/* FMGROIDS_H */\n";
+print $pfh "\n#endif\t\t\t\t\t\t\t/* FMGRPROTOS_H */\n";
close($ofh);
close($pfh);
diff --git a/src/include/catalog/reformat_dat_file.pl b/src/include/catalog/reformat_dat_file.pl
index 00c4f2b..7f6dc6e 100755
--- a/src/include/catalog/reformat_dat_file.pl
+++ b/src/include/catalog/reformat_dat_file.pl
@@ -125,7 +125,6 @@ foreach my $catname (@catnames)
open my $dat, '>', $datfile
or die "can't open $datfile: $!";
- # Write the data.
foreach my $data (@{ $catalog_data{$catname} })
{
