pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_recvlogical.c doesn't build with --disable-integer-datetimes
19+ messages / 6 participants
[nested] [flat]
* pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 06:16 Thomas Munro <thomas.munro@enterprisedb.com>
0 siblings, 2 replies; 19+ messages in thread
From: Thomas Munro @ 2017-02-17 06:16 UTC (permalink / raw)
To: pgsql-hackers
Hi,
Since commit 7c030783a5bd07cadffc2a1018bc33119a4c7505 it seems that $SUBJECT.
pg_recvlogical.c:501:37: error: incompatible pointer types passing
'int64 *' (aka 'long *') to parameter of type 'TimestampTz *' (aka
'double *') [-Werror,-Wincompatible-pointer-types]
if (!flushAndSendFeedback(conn, &now))
pg_recvlogical.c:547:36: error: incompatible pointer types passing
'int64 *' (aka 'long *') to parameter of type 'TimestampTz *' (aka
'double *') [-Werror,-Wincompatible-pointer-types]
if (!flushAndSendFeedback(conn, &now))
pg_recvlogical.c:596:36: error: incompatible pointer types passing
'int64 *' (aka 'long *') to parameter of type 'TimestampTz *' (aka
'double *') [-Werror,-Wincompatible-pointer-types]
if (!flushAndSendFeedback(conn, &now))
Maybe the attached is the right fix for that? (Sorry, I didn't test
because I haven't got around to learning how to drive this new stuff
yet.)
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[application/octet-stream] fix-pg-recvlogical-with-float-timestamps.patch (910B, ../../CAEepm=15e9L695yVCO-_OkBVbsPupyXqzYWzzDmj-bdJ6o2+Pw@mail.gmail.com/2-fix-pg-recvlogical-with-float-timestamps.patch)
download | inline diff:
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index d16d08b..54d7819 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -65,7 +65,7 @@ static XLogRecPtr output_fsync_lsn = InvalidXLogRecPtr;
static void usage(void);
static void StreamLogicalLog(void);
static void disconnect_and_exit(int code);
-static bool flushAndSendFeedback(PGconn *conn, TimestampTz *now);
+static bool flushAndSendFeedback(PGconn *conn, int64 *now);
static void prepareToTerminate(PGconn *conn, XLogRecPtr endpos,
bool keepalive, XLogRecPtr lsn);
@@ -1024,7 +1024,7 @@ main(int argc, char **argv)
* feedback.
*/
static bool
-flushAndSendFeedback(PGconn *conn, TimestampTz *now)
+flushAndSendFeedback(PGconn *conn, int64 *now)
{
/* flush data to disk, so that we send a recent flush pointer */
if (!OutputFsync(*now))
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 15:19 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Thomas Munro <thomas.munro@enterprisedb.com>
1 sibling, 0 replies; 19+ messages in thread
From: Alvaro Herrera @ 2017-02-17 15:19 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@enterprisedb.com>; +Cc: pgsql-hackers
Thomas Munro wrote:
> Hi,
>
> Since commit 7c030783a5bd07cadffc2a1018bc33119a4c7505 it seems that $SUBJECT.
Added it to open items list.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 16:14 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@enterprisedb.com>
1 sibling, 1 reply; 19+ messages in thread
From: Tom Lane @ 2017-02-17 16:14 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@enterprisedb.com>; +Cc: pgsql-hackers
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Since commit 7c030783a5bd07cadffc2a1018bc33119a4c7505 it seems that $SUBJECT.
Hmm ... I thought we had at least one buildfarm member still testing
--disable-integer-datetimes. Evidently somebody needs to close that
gap.
> Maybe the attached is the right fix for that?
I'll look at this later today, if no one beats me to it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 17:21 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2017-02-17 17:21 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@enterprisedb.com>; +Cc: pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>
I wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> Since commit 7c030783a5bd07cadffc2a1018bc33119a4c7505 it seems that $SUBJECT.
> Hmm ... I thought we had at least one buildfarm member still testing
> --disable-integer-datetimes. Evidently somebody needs to close that
> gap.
The plot thickens: we *do* have one such critter, mandrill, but for some
reason it's not generating any complaint about this. Must be a very
lax compiler.
My own compilers don't generate errors either, only warnings. Maybe
we could catch this sort of thing mechanically with a critter configured
with -Werror as well as --disable-integer-datetimes, but I'm a tad
hesitant to have a buildfarm machine configured with -Werror. We'd
be raising the stakes quite a bit on keeping the code warning-free
for a specific compiler that might prove to be picky in silly ways.
But if it were a pretty mainstream compiler that hadn't shown much
evidence of wanting to throw useless warnings, maybe that's fine.
Thoughts?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 18:13 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Alvaro Herrera @ 2017-02-17 18:13 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>
Tom Lane wrote:
> My own compilers don't generate errors either, only warnings. Maybe
> we could catch this sort of thing mechanically with a critter configured
> with -Werror as well as --disable-integer-datetimes, but I'm a tad
> hesitant to have a buildfarm machine configured with -Werror. We'd
> be raising the stakes quite a bit on keeping the code warning-free
> for a specific compiler that might prove to be picky in silly ways.
>
> But if it were a pretty mainstream compiler that hadn't shown much
> evidence of wanting to throw useless warnings, maybe that's fine.
>
> Thoughts?
Note that Thomas' compiler was using -Werror.
I think we're at a point where one gcc plus one clang animals set up
with -Werror would be a good thing to have, and keep green. In
practice, this list fulfills the role of such a compiler, only with much
higher communication overhead.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 18:21 Tom Lane <tgl@sss.pgh.pa.us>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2017-02-17 18:21 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> My own compilers don't generate errors either, only warnings. Maybe
>> we could catch this sort of thing mechanically with a critter configured
>> with -Werror as well as --disable-integer-datetimes, but I'm a tad
>> hesitant to have a buildfarm machine configured with -Werror. We'd
>> be raising the stakes quite a bit on keeping the code warning-free
>> for a specific compiler that might prove to be picky in silly ways.
>>
>> But if it were a pretty mainstream compiler that hadn't shown much
>> evidence of wanting to throw useless warnings, maybe that's fine.
> I think we're at a point where one gcc plus one clang animals set up
> with -Werror would be a good thing to have, and keep green. In
> practice, this list fulfills the role of such a compiler, only with much
> higher communication overhead.
Yeah. I have longfin which is running Apple's clang, and is on a machine
that doesn't have much to do otherwise. I propose to turn on -Werror in
its configuration, and to configure a second critter on the same hardware
that runs with -Werror as well as --disable-integer-datetimes. Somebody
else should do similarly with a reasonably modern/stable gcc release.
In the longer term, we might want to encourage more buildfarm owners to
use -Werror, but only with mainstream compilers that are generally pretty
sane about warnings. Some of the older critters generate
hundreds/thousands of useless warnings; I'm not interested in trying to
suppress all of those.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 19:40 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 19+ messages in thread
From: Tom Lane @ 2017-02-17 19:40 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>
I wrote:
> Yeah. I have longfin which is running Apple's clang, and is on a machine
> that doesn't have much to do otherwise. I propose to turn on -Werror in
> its configuration, and to configure a second critter on the same hardware
> that runs with -Werror as well as --disable-integer-datetimes. Somebody
> else should do similarly with a reasonably modern/stable gcc release.
So I tried doing that by adding -Werror to longfin's CFLAGS environment,
and it crashed and burned in configure. The most obvious problem is
that we didn't bother to supply a prototype for does_int64_work() in
the probes for 64-bit ints. However, even after fixing that, configure
produces totally bollixed output because -Werror breaks many of its
built-in tests. We could fix the int64 tests but we have no good way of
fixing the built-in tests.
AFAICS, if you want to build with -Werror, you have to configure without
that and then inject it afterwards. I wonder how people who use -Werror
are handling that. Is it possible to do at all in a buildfarm critter?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 19:48 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 19+ messages in thread
From: Andres Freund @ 2017-02-17 19:48 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org,Tom Lane <tgl@sss.pgh.pa.us>; Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>
On February 17, 2017 11:40:57 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>I wrote:
>> Yeah. I have longfin which is running Apple's clang, and is on a
>machine
>> that doesn't have much to do otherwise. I propose to turn on -Werror
>in
>> its configuration, and to configure a second critter on the same
>hardware
>> that runs with -Werror as well as --disable-integer-datetimes.
>Somebody
>> else should do similarly with a reasonably modern/stable gcc release.
>
>So I tried doing that by adding -Werror to longfin's CFLAGS
>environment,
>and it crashed and burned in configure. The most obvious problem is
>that we didn't bother to supply a prototype for does_int64_work() in
>the probes for 64-bit ints. However, even after fixing that, configure
>produces totally bollixed output because -Werror breaks many of its
>built-in tests. We could fix the int64 tests but we have no good way
>of
>fixing the built-in tests.
>
>AFAICS, if you want to build with -Werror, you have to configure
>without
>that and then inject it afterwards. I wonder how people who use
>-Werror
>are handling that. Is it possible to do at all in a buildfarm critter?
Yea, I complained about that before. I think we'd have to add an extra configure option for it, because we rely in setting it only for certain tests. Personally I use CEXTRA (IIRC) to achieve that result after configure ran.
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 19:51 Mark Dilger <hornschnorter@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 19+ messages in thread
From: Mark Dilger @ 2017-02-17 19:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>
> On Feb 17, 2017, at 11:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
>> Yeah. I have longfin which is running Apple's clang, and is on a machine
>> that doesn't have much to do otherwise. I propose to turn on -Werror in
>> its configuration, and to configure a second critter on the same hardware
>> that runs with -Werror as well as --disable-integer-datetimes. Somebody
>> else should do similarly with a reasonably modern/stable gcc release.
>
> So I tried doing that by adding -Werror to longfin's CFLAGS environment,
> and it crashed and burned in configure. The most obvious problem is
> that we didn't bother to supply a prototype for does_int64_work() in
> the probes for 64-bit ints. However, even after fixing that, configure
> produces totally bollixed output because -Werror breaks many of its
> built-in tests. We could fix the int64 tests but we have no good way of
> fixing the built-in tests.
>
> AFAICS, if you want to build with -Werror, you have to configure without
> that and then inject it afterwards. I wonder how people who use -Werror
> are handling that. Is it possible to do at all in a buildfarm critter?
I ran into the same problem when adding -Werror, and fixed it as follows:
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index d39d6ca..96fbc60 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -240,7 +240,7 @@ endif # not PGXS
CC = @CC@
GCC = @GCC@
SUN_STUDIO_CC = @SUN_STUDIO_CC@
-CFLAGS = @CFLAGS@
+CFLAGS = @CFLAGS@ -Werror
CFLAGS_VECTOR = @CFLAGS_VECTOR@
CFLAGS_SSE42 = @CFLAGS_SSE42@
That adds -Werror to the compilation of sources but not to the compilation
of configure test programs. I'd be happy to hear if anybody has a cleaner
way of doing this.
Mark Dilger
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 20:04 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2017-02-17 20:04 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>; Thomas Munro <thomas.munro@enterprisedb.com>; Andrew Dunstan <andrew@dunslane.net>
Andres Freund <andres@anarazel.de> writes:
> On February 17, 2017 11:40:57 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> AFAICS, if you want to build with -Werror, you have to configure
>> without that and then inject it afterwards. I wonder how people who
>> use -Werror are handling that. Is it possible to do at all in a
>> buildfarm critter?
> Yea, I complained about that before. I think we'd have to add an extra configure option for it, because we rely in setting it only for certain tests. Personally I use CEXTRA (IIRC) to achieve that result after configure ran.
[ pokes around... ] Ah, that's called COPT, and it's entirely
undocumented :-(. Probably ought to fix that.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 20:05 Thomas Munro <thomas.munro@enterprisedb.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Thomas Munro @ 2017-02-17 20:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
On Sat, Feb 18, 2017 at 9:04 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On February 17, 2017 11:40:57 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> AFAICS, if you want to build with -Werror, you have to configure
>>> without that and then inject it afterwards. I wonder how people who
>>> use -Werror are handling that. Is it possible to do at all in a
>>> buildfarm critter?
>
>> Yea, I complained about that before. I think we'd have to add an extra configure option for it, because we rely in setting it only for certain tests. Personally I use CEXTRA (IIRC) to achieve that result after configure ran.
>
> [ pokes around... ] Ah, that's called COPT, and it's entirely
> undocumented :-(. Probably ought to fix that.
One way to set that up is like this:
$ cat src/Makefile.custom
COPT=-Wall -Werror $(CC_OPT)
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 20:21 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@enterprisedb.com>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2017-02-17 20:21 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@enterprisedb.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Sat, Feb 18, 2017 at 9:04 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> [ pokes around... ] Ah, that's called COPT, and it's entirely
>> undocumented :-(. Probably ought to fix that.
> One way to set that up is like this:
> $ cat src/Makefile.custom
> COPT=-Wall -Werror $(CC_OPT)
Well, we don't document Makefile.custom either, and probably now is
not the time to start. I'm inclined to just explain that you can
set COPT in the environment of the "make" step to add flags that
you couldn't or didn't tell configure about.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 21:13 Mark Dilger <hornschnorter@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 3 replies; 19+ messages in thread
From: Mark Dilger @ 2017-02-17 21:13 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
> On Feb 17, 2017, at 12:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> On Sat, Feb 18, 2017 at 9:04 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> [ pokes around... ] Ah, that's called COPT, and it's entirely
>>> undocumented :-(. Probably ought to fix that.
>
>> One way to set that up is like this:
>
>> $ cat src/Makefile.custom
>> COPT=-Wall -Werror $(CC_OPT)
Makefile.custom is deprecated per comment in src/Makefile.global, so
you'd at least have to remove or edit that comment. It was enough to
scare me off using Makefile.custom as a solution for this when I was
working on getting -Werror into all my builds.
> Well, we don't document Makefile.custom either, and probably now is
> not the time to start. I'm inclined to just explain that you can
> set COPT in the environment of the "make" step to add flags that
> you couldn't or didn't tell configure about.
There is a test in configure.in for whether the compiler is GCC, followed
by additional flags if so. On my platform (Mac laptop) I was able to add
additional flags against HEAD without any of them actually triggering a
warning (though my last merge from HEAD was a while ago):
diff --git a/configure.in b/configure.in
index b9831bc..d6e7e24 100644
--- a/configure.in
+++ b/configure.in
@@ -446,6 +446,24 @@ fi
if test "$GCC" = yes -a "$ICC" = no; then
CFLAGS="-Wall -Wmissing-prototypes -Wpointer-arith"
+ PGAC_PROG_CC_CFLAGS_OPT([-Wempty-body])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wignored-qualifiers])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wtype-limits])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wuninitialized])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wshift-negative-value])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-include-dirs])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wshift-overflow])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wswitch-default])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wdangling-else])
+ PGAC_PROG_CC_CFLAGS_OPT([-Waggregate-return])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wstrict-prototypes])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-declarations])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wredundant-decls])
+ PGAC_PROG_CC_CFLAGS_OPT([-Winline])
+ PGAC_PROG_CC_CFLAGS_OPT([-Woverlength-strings])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wc++-compat])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wold-style-definition])
# These work in some but not all gcc versions
PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement])
PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels])
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index d39d6ca..96fbc60 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -240,7 +240,7 @@ endif # not PGXS
CC = @CC@
GCC = @GCC@
SUN_STUDIO_CC = @SUN_STUDIO_CC@
-CFLAGS = @CFLAGS@
+CFLAGS = @CFLAGS@ -Werror
CFLAGS_VECTOR = @CFLAGS_VECTOR@
CFLAGS_SSE42 = @CFLAGS_SSE42@
How about we add (some of) these extra warnings, plus -Werror,
in a section that is only active for platforms/compilers where we
know there aren't spurious warnings? That would make detecting
unintentionally introduced warnings simpler, without the use of
COPT. Perhaps where the compiler is GCC or CLANG, and the
platform is x86_64 redhat, something like that?
Mark Dilger
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 21:18 Andres Freund <andres@anarazel.de>
parent: Mark Dilger <hornschnorter@gmail.com>
2 siblings, 0 replies; 19+ messages in thread
From: Andres Freund @ 2017-02-17 21:18 UTC (permalink / raw)
To: Mark Dilger <hornschnorter@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
On February 17, 2017 1:13:10 PM PST, Mark Dilger <hornschnorter@gmail.com> wrote:
>How about we add (some of) these extra warnings, plus -Werror,
>in a section that is only active for platforms/compilers where we
>know there aren't spurious warnings? That would make detecting
>unintentionally introduced warnings simpler, without the use of
>COPT. Perhaps where the compiler is GCC or CLANG, and the
>platform is x86_64 redhat, something like that?
Strongly against that. I do *not* want Werrror enabled during development. I have a lot of warnings enabled, but while hacking I often have some of them triggering (e.g. an unused variable or static function). Preventing me from compiling in those scenarios has no benefits. Werror it's useful in automated scenarios (e.g. my pre push script compiles with it), a lot less in interactive scenarios where you can just use make -s.
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 21:31 Tom Lane <tgl@sss.pgh.pa.us>
parent: Mark Dilger <hornschnorter@gmail.com>
2 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2017-02-17 21:31 UTC (permalink / raw)
To: Mark Dilger <hornschnorter@gmail.com>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
Mark Dilger <hornschnorter@gmail.com> writes:
> if test "$GCC" = yes -a "$ICC" = no; then
> CFLAGS="-Wall -Wmissing-prototypes -Wpointer-arith"
> + PGAC_PROG_CC_CFLAGS_OPT([-Wempty-body])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wignored-qualifiers])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wtype-limits])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wuninitialized])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wshift-negative-value])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-include-dirs])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wshift-overflow])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wswitch-default])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wdangling-else])
> + PGAC_PROG_CC_CFLAGS_OPT([-Waggregate-return])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wstrict-prototypes])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-declarations])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wredundant-decls])
> + PGAC_PROG_CC_CFLAGS_OPT([-Winline])
> + PGAC_PROG_CC_CFLAGS_OPT([-Woverlength-strings])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wc++-compat])
> + PGAC_PROG_CC_CFLAGS_OPT([-Wold-style-definition])
> # These work in some but not all gcc versions
> PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement])
> PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels])
I'm not terribly for that. For the most part, if a gcc warning isn't
included by '-Wall', there's a reason for it. We could talk about
whether individual extra warnings are useful for the Postgres project,
but I don't want to suddenly move the goalposts to where we are
enforcing a bunch of pedantic warnings that different compiler versions
might not even interpret the same. I'm particularly not for turning
them up in the way you suggest here, where violations would result in
buildfarm failures on machines running with -Werror.
Right now I'd rather focus on getting to where we can have -Werror on in
the buildfarm at all. longfin says we've got work to do on that, at least
in the back branches. It may be that we can't expect near-EOL branches to
always compile perfectly cleanly on newer compilers.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 23:34 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 19+ messages in thread
From: Tom Lane @ 2017-02-17 23:34 UTC (permalink / raw)
To: Mark Dilger <hornschnorter@gmail.com>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
I wrote:
> Right now I'd rather focus on getting to where we can have -Werror on in
> the buildfarm at all. longfin says we've got work to do on that, at least
> in the back branches. It may be that we can't expect near-EOL branches to
> always compile perfectly cleanly on newer compilers.
I found that longfin's compiler is happy with 9.4 and up as-is, and it
required only a few very small and safe back-patches to make 9.3 work
too. However, it's looking like -Werror with 9.2 is a bridge too far.
The thing that seems untenable is that to make 9.2 compile clean, we would
have to back-patch commit 71450d7fd, along with a fair amount of follow-on
work, so that the compiler would understand that ereport(ERROR) doesn't
return. I don't think we want to go that far --- and maybe introduce
portability issues for someone's ancient compiler --- for a branch that's
near EOL anyway.
I'll adjust longfin's configuration so that it only applies -Werror in
9.3 and up.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-17 23:37 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Mark Dilger <hornschnorter@gmail.com>
2 siblings, 1 reply; 19+ messages in thread
From: Peter Eisentraut @ 2017-02-17 23:37 UTC (permalink / raw)
To: Mark Dilger <hornschnorter@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
On 2/17/17 16:13, Mark Dilger wrote:
> + PGAC_PROG_CC_CFLAGS_OPT([-Wc++-compat])
If your compiler isn't warning about anything with that, then there is
something wrong with it.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-18 00:10 Mark Dilger <hornschnorter@gmail.com>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 19+ messages in thread
From: Mark Dilger @ 2017-02-18 00:10 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
> On Feb 17, 2017, at 3:37 PM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
>
> On 2/17/17 16:13, Mark Dilger wrote:
>> + PGAC_PROG_CC_CFLAGS_OPT([-Wc++-compat])
>
> If your compiler isn't warning about anything with that, then there is
> something wrong with it.
$ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.3.0 (clang-703.0.31)
Target: x86_64-apple-darwin15.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: pg_recvlogical.c doesn't build with --disable-integer-datetimes
@ 2017-02-18 00:22 Tom Lane <tgl@sss.pgh.pa.us>
parent: Mark Dilger <hornschnorter@gmail.com>
0 siblings, 0 replies; 19+ messages in thread
From: Tom Lane @ 2017-02-18 00:22 UTC (permalink / raw)
To: Mark Dilger <hornschnorter@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; Thomas Munro <thomas.munro@enterprisedb.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; Alvaro Herrera <alvherre@2ndquadrant.com>; Andrew Dunstan <andrew@dunslane.net>
Mark Dilger <hornschnorter@gmail.com> writes:
>> On Feb 17, 2017, at 3:37 PM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
>> If your compiler isn't warning about anything with that, then there is
>> something wrong with it.
> $ gcc --version
> Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
> Apple LLVM version 7.3.0 (clang-703.0.31)
> Target: x86_64-apple-darwin15.6.0
> Thread model: posix
> InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
Interesting. That option doesn't seem to do anything on my Mac's compiler
either (Apple LLVM version 8.0.0 (clang-800.0.42.1)). However, it surely
spews a metric buttload of warnings with RHEL6's gcc. Points up my
concern upthread about different peoples' compilers interpreting these
less-common switches differently.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2017-02-18 00:22 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-02-17 06:16 pg_recvlogical.c doesn't build with --disable-integer-datetimes Thomas Munro <thomas.munro@enterprisedb.com>
2017-02-17 15:19 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2017-02-17 16:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 17:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 18:13 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2017-02-17 18:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 19:40 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 19:48 ` Andres Freund <andres@anarazel.de>
2017-02-17 20:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 20:05 ` Thomas Munro <thomas.munro@enterprisedb.com>
2017-02-17 20:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 21:13 ` Mark Dilger <hornschnorter@gmail.com>
2017-02-17 21:18 ` Andres Freund <andres@anarazel.de>
2017-02-17 21:31 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 23:34 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 23:37 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-02-18 00:10 ` Mark Dilger <hornschnorter@gmail.com>
2017-02-18 00:22 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-02-17 19:51 ` Mark Dilger <hornschnorter@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox