On Mon, Dec 16, 2019 at 01:12:27PM +0100, Peter Eisentraut wrote:
> OK, here is an updated patch set that has all defines in one big Perl hash,
> and also requires that all symbols in pg_config.h.in are accounted for.
> (The indentation is from pgperltidy.)
The patch looks pretty clean. I have a few minor comments.
- if (/^AC_INIT\(\[PostgreSQL\], \[([^\]]+)\]/)
+ if (/^AC_INIT\(\[([^\]]+)\], \[([^\]]+)\], \[([^\]]+)\]/)
{
Why did you remove the bit about "PostgreSQL"?
+ ENABLE_GSS => $self->{options}->{gss} ? 1 :
undef,
[...]
- if ($self->{options}->{gss})
- {
- print $o "#define ENABLE_GSS 1\n";
I found the part about gss and nls better with the old style. A
matter of taste, not really an objection. And your style is actually
consistent with USE_ASSERT_CHECKING as well.
+ else
+ {
+ croak "missing: $macro";
+ }
[...]
+ if (scalar(keys %define) > 0)
+ {
+ croak "unused defines: @{[%define]}";
+ }
So you have checks both ways. That's nice.
+ open(my $i, '<', "src/include/pg_config.h.in")
+ || confess "Could not open pg_config.h.in\n";
+ open(my $o, '>', "src/include/pg_config.h")
+ || confess "Could not write to pg_config.h\n";
Failure to open pg_config.h.
Wouldn't it be better to remove pg_config_ext.h.win32 as well?
--
Michael