Discussion:
Moving of INT64_FORMAT to c.h
Jan Wieck
2014-10-16 12:04:17 UTC
Permalink
Hi,

PostgreSQL has for ages defined INT64_FORMAT and UINT64_FORMAT in
pg_config.h. This commit

http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ce486056ecd28050

moved those definitions to c.h, which breaks compilation of all released
Slony-I versions against current master. Can those be moved back to
where they used to be?

Slony uses the definitions in external tools, like slon and slonik, to
format sequence numbers in log output.


Regards,
Jan
--
Jan Wieck
Senior Software Engineer
http://slony.info
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund
2014-10-16 12:08:07 UTC
Permalink
Post by Jan Wieck
Hi,
PostgreSQL has for ages defined INT64_FORMAT and UINT64_FORMAT in
pg_config.h. This commit
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ce486056ecd28050
moved those definitions to c.h, which breaks compilation of all released
Slony-I versions against current master. Can those be moved back to where
they used to be?
Well, you could add additional configure stuff to also emit what you
want.
Post by Jan Wieck
Slony uses the definitions in external tools, like slon and slonik, to
format sequence numbers in log output.
Then it should include c.h/postgres_fe.h?

Greetings,

Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Steve Singer
2014-10-22 20:12:06 UTC
Permalink
Post by Andres Freund
Post by Jan Wieck
Hi,
PostgreSQL has for ages defined INT64_FORMAT and UINT64_FORMAT in
pg_config.h. This commit
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ce486056ecd28050
moved those definitions to c.h, which breaks compilation of all released
Slony-I versions against current master. Can those be moved back to where
they used to be?
Well, you could add additional configure stuff to also emit what you
want.
Post by Jan Wieck
Slony uses the definitions in external tools, like slon and slonik, to
format sequence numbers in log output.
Then it should include c.h/postgres_fe.h?
So the header of c.h says "Note that the definitions here are not
intended to be exposed to clients"
but
postgres_fe.h says "This should be the first file included by PostgreSQL
client libraries and"

Should client programs that live outside the postgres source tree be
including postgres_fe.h ? I have a feeling the answer is no. If the
answer is no, then why does a make install install postgres_fe.h ?

Slonik used to include postgre_fe.h but back in 2011 or so we stopped
doing so because it was causing issues (I think on win32 builds)

Maybe slony client programs shouldn't be trying to steal portability
definitions from postgres headers, but I doubt we are the only ones
doing that. It isn't a big deal for slony to define it's own
INT64_FORMAT for 9.5+ but third party projects that have been including
pg_config.h will hit similar issues. if there was good reason for the
change then fine (Postgres isn't intended to be a general purpose C
portability layer).



Steve
Post by Andres Freund
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas
2014-10-22 20:17:28 UTC
Permalink
So the header of c.h says "Note that the definitions here are not intended
to be exposed to clients"
but
postgres_fe.h says "This should be the first file included by PostgreSQL
client libraries and"
Should client programs that live outside the postgres source tree be
including postgres_fe.h ? I have a feeling the answer is no. If the answer
is no, then why does a make install install postgres_fe.h ?
I think the answer is yes.
Slonik used to include postgre_fe.h but back in 2011 or so we stopped doing
so because it was causing issues (I think on win32 builds)
That seems like something we ought to consider fixing, but obviously
we'd need more details.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane
2014-10-22 20:31:30 UTC
Permalink
Post by Robert Haas
So the header of c.h says "Note that the definitions here are not intended
to be exposed to clients"
but
postgres_fe.h says "This should be the first file included by PostgreSQL
client libraries and"
Should client programs that live outside the postgres source tree be
including postgres_fe.h ? I have a feeling the answer is no. If the answer
is no, then why does a make install install postgres_fe.h ?
I think the answer is yes.
Yeah. In particular, postgres_fe.h sees to it that FRONTEND is #define'd;
without that there is *not* a guarantee that c.h will work for non-backend
compiles. We would much rather you were including postgres_fe.h than c.h
directly. Having said that, there is not and never will be a guarantee
that everything that postgres_fe.h declares is immutable, and that
certainly applies to pg_config.h in particular. There's a reason why
libpq doesn't include that into its public headers ...
Post by Robert Haas
Slonik used to include postgre_fe.h but back in 2011 or so we stopped doing
so because it was causing issues (I think on win32 builds)
That seems like something we ought to consider fixing, but obviously
we'd need more details.
Indeed. But I suspect the core of it is going to be that clients of
postgres_fe.h are expected to be linking in libpgport.a ...

regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Steve Singer
2014-10-23 01:10:07 UTC
Permalink
Post by Robert Haas
Slonik used to include postgre_fe.h but back in 2011 or so we stopped doing
so because it was causing issues (I think on win32 builds)
That seems like something we ought to consider fixing, but obviously
we'd need more details.
When I'll try to find sometime to get a windows environment running and
try to figure out what the problems were.
It's also possible that I removed the postgres_fe include at the same
time as I was fixing other win32 issues and the postgres_fe include
wasn't causing a problem it was just removed because it was unnecessary.

At the moment slony only includes the server include dir if we are
building with pgport (which we normally do on windows)
We have had reports of issues like described
(http://www.slony.info/bugzilla/show_bug.cgi?id=315) where some installs
make us pick up server and client includes from different versions of PG.
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Loading...