From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: automating pg_config.h.win32 maintenance |
Date: | 2019-12-17 06:30:38 |
Message-ID: | 20191217063038.GM2344@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro Horiguchi | 2019-12-17 06:30:52 | Re: non-exclusive backup cleanup is mildly broken |
Previous Message | Kyotaro Horiguchi | 2019-12-17 06:11:55 | Re: non-exclusive backup cleanup is mildly broken |