Thread: Plan for straightening out the include-file mess
I have been looking at making a split between client-side and server-side
include files as we discussed earlier this week (pghackers thread
"Include files for SPI are not installed", if you missed it). It turns
out that the major issue here is not just divvying up the files; the
problem is that we have never had a clear concept of such a division
before, and so the core include files like postgres.h, c.h, config.h
contain a rather unholy mixture of things that are definitely
backend-only with things that are relevant to both clients and backends.
I think we need to start by clarifying the roles of these include files
and moving their contents around as necessary.
Currently, almost every .c in the distribution starts out by including
postgres.h, which in turn includes these other files:
postgres.hpostgres_ext.hc.h config.h os.hutils/elog.hutils/palloc.h
Now elog.h and palloc.h are server-only facilities and certainly don't
belong in a client file's include set. I think what we want to do is
decree that postgres.h is the primary include file for backend .c files
only, and that frontend .c files should include something else.
postgres_ext.h would be a candidate to be that something else, except
that it's included by libpq-fe.h, so anything we add to postgres_ext.h
represents namespace pollution for libpq clients. I think we should be
very wary about adding a lot of stuff to postgres_ext.h. This suggests
that we'd best create a new primary include file for client-side .c files,
say "postgres_fe.h" or "postgres_client.h". (Anyone have a better naming
idea? Does the old 14-character limit still pose a problem anywhere?)
That would leave us with include trees like this:
backend .c file:postgres.h postgres_ext.h c.h config.h os.h utils/elog.h utils/palloc.h
frontend .c file:postgres_fe.h postgres_ext.h c.h config.h os.h
where the include files have these roles:
postgres_ext.h: definitions needed in frontend, backend, *and* by clients;by design an extremely small file
postgres.h: backend-wide definitions
postgres_fe.h: definitions common to all client-side interface libraries
c.h: basic typedefs and macros needed by both frontend and backend, butnot intended to be exported to clients of
frontendlibraries
config.h: configuration definitions, not intended to be client-visible
os.h: platform-specific configuration hacks, not intended to beclient-visible (this comes from one of the
src/include/portfiles)
config.h and os.h don't need to change, I think, but I'll go through the
definitions in the other four files and make sure everything is classified
reasonably.
It's possible that postgres_fe.h will end up containing nothing except
the inclusions of postgres_ext.h and c.h, in which case we wouldn't really
need to invent that file, but I'm still inclined to do so. I think it's
good practice to have a single include file that's the basic "must haves"
for all client-side code.
Now, since the intent is that the basic install provide headers needed
for client-side programming, we'd want to add postgres_fe.h to the
installed header set. But the following files can be removed from the
basic install:
access/attnum.h
commands/trigger.h
executor/spi.h
fmgr.h
postgres.h
utils/elog.h
utils/geo_decls.h
utils/palloc.h
We might also remove utils/fmgroids.h. I'm uncertain about this one.
The function OID macros it contains are potentially useful to clients,
but do we really want people hard-wiring function OIDs on the client
side? I doubt it.
There are two or three other include files, such as lib/dllist.h, that are
needed on the client side only because libpq-int.h includes them, and we
want to support client code that includes libpq-int.h. I am going to look
at skinnying that list down too. libpq-fs.h, in particular, looks like
mostly legacy junk ...
As we discussed, there'll be an additional install target (or RPM) that
installs these files and everything else from the src/include tree.
Comments?
regards, tom lane
On Thu, 8 Feb 2001, Alex Pilosov wrote: > Great! :) > > It might also clean up something that I've been fighting against for > awhile: when I include files needed for SPI, it drags also a lot of other > garbage in, which conflicts with other things (namely, trying to get a > file to simultaneously include SPI and perl headers is impossible). > > I realise it might be a lot of pain to clean up, but, you may consider > having a separate top-level include for SPI, which would not define (by > default) things like DEBUG, USE_LOCALE, union semun, etc. > > IMHO, it should really include only definitions of relevant data > structures which interface with SPI code... > > I realize that complete split for SPI/module from "core backend" might be > very hard, so a thing to consider would be to have (like linux kernel code > has) #define IN_CORE (you are welcome to come up with better name), and > include "core backend"-specific things conditionally on that being > defined. > > > -alex > > On Thu, 8 Feb 2001, Tom Lane wrote: > > > I have been looking at making a split between client-side and server-side > > include files as we discussed earlier this week (pghackers thread > > "Include files for SPI are not installed", if you missed it). It turns > > out that the major issue here is not just divvying up the files; the > > problem is that we have never had a clear concept of such a division > > before, and so the core include files like postgres.h, c.h, config.h > > contain a rather unholy mixture of things that are definitely > > backend-only with things that are relevant to both clients and backends. > > I think we need to start by clarifying the roles of these include files > > and moving their contents around as necessary. > > > > Currently, almost every .c in the distribution starts out by including > > postgres.h, which in turn includes these other files: > > > > postgres.h > > postgres_ext.h > > c.h > > config.h > > os.h > > utils/elog.h > > utils/palloc.h > > > > Now elog.h and palloc.h are server-only facilities and certainly don't > > belong in a client file's include set. I think what we want to do is > > decree that postgres.h is the primary include file for backend .c files > > only, and that frontend .c files should include something else. > > > > postgres_ext.h would be a candidate to be that something else, except > > that it's included by libpq-fe.h, so anything we add to postgres_ext.h > > represents namespace pollution for libpq clients. I think we should be > > very wary about adding a lot of stuff to postgres_ext.h. This suggests > > that we'd best create a new primary include file for client-side .c files, > > say "postgres_fe.h" or "postgres_client.h". (Anyone have a better naming > > idea? Does the old 14-character limit still pose a problem anywhere?) > > > > That would leave us with include trees like this: > > > > backend .c file: > > postgres.h > > postgres_ext.h > > c.h > > config.h > > os.h > > utils/elog.h > > utils/palloc.h > > > > frontend .c file: > > postgres_fe.h > > postgres_ext.h > > c.h > > config.h > > os.h > > > > where the include files have these roles: > > > > postgres_ext.h: definitions needed in frontend, backend, *and* by clients; > > by design an extremely small file > > > > postgres.h: backend-wide definitions > > > > postgres_fe.h: definitions common to all client-side interface libraries > > > > c.h: basic typedefs and macros needed by both frontend and backend, but > > not intended to be exported to clients of frontend libraries > > > > config.h: configuration definitions, not intended to be client-visible > > > > os.h: platform-specific configuration hacks, not intended to be > > client-visible (this comes from one of the src/include/port files) > > > > config.h and os.h don't need to change, I think, but I'll go through the > > definitions in the other four files and make sure everything is classified > > reasonably. > > > > It's possible that postgres_fe.h will end up containing nothing except > > the inclusions of postgres_ext.h and c.h, in which case we wouldn't really > > need to invent that file, but I'm still inclined to do so. I think it's > > good practice to have a single include file that's the basic "must haves" > > for all client-side code. > > > > > > Now, since the intent is that the basic install provide headers needed > > for client-side programming, we'd want to add postgres_fe.h to the > > installed header set. But the following files can be removed from the > > basic install: > > > > access/attnum.h > > commands/trigger.h > > executor/spi.h > > fmgr.h > > postgres.h > > utils/elog.h > > utils/geo_decls.h > > utils/palloc.h > > > > We might also remove utils/fmgroids.h. I'm uncertain about this one. > > The function OID macros it contains are potentially useful to clients, > > but do we really want people hard-wiring function OIDs on the client > > side? I doubt it. > > > > There are two or three other include files, such as lib/dllist.h, that are > > needed on the client side only because libpq-int.h includes them, and we > > want to support client code that includes libpq-int.h. I am going to look > > at skinnying that list down too. libpq-fs.h, in particular, looks like > > mostly legacy junk ... > > > > As we discussed, there'll be an additional install target (or RPM) that > > installs these files and everything else from the src/include tree. > > > > Comments? > > > > regards, tom lane > > > > > >
Alex Pilosov <alex@acecape.com> writes:
> when I include files needed for SPI, it drags also a lot of other
> garbage in, which conflicts with other things (namely, trying to get a
> file to simultaneously include SPI and perl headers is impossible).
> I realise it might be a lot of pain to clean up, but, you may consider
> having a separate top-level include for SPI, which would not define (by
> default) things like DEBUG, USE_LOCALE, union semun, etc.
Unless you want to write SPI code that never calls elog(), it's gonna be
tough to avoid the conflict on DEBUG. I suppose sooner or later we'll
have to rename the elog severity symbols ... but it's not a change that
I'm looking forward to making.
In any case there's too much time pressure to consider wide-ranging code
changes for 7.1. Right now I don't want to do more than rearrange the
contents of a small number of include files.
regards, tom lane
On Thu, 8 Feb 2001, Tom Lane wrote: > Alex Pilosov <alex@acecape.com> writes: > > when I include files needed for SPI, it drags also a lot of other > > garbage in, which conflicts with other things (namely, trying to get a > > file to simultaneously include SPI and perl headers is impossible). > > I realise it might be a lot of pain to clean up, but, you may consider > > having a separate top-level include for SPI, which would not define (by > > default) things like DEBUG, USE_LOCALE, union semun, etc. > > Unless you want to write SPI code that never calls elog(), it's gonna be > tough to avoid the conflict on DEBUG. I suppose sooner or later we'll > have to rename the elog severity symbols ... but it's not a change that > I'm looking forward to making. Yes, how about ELOG_DEBUG, etc? Global search and replace should get it correctly, shouldn't be TOO painful. It would indeed break other people's internally-written modules, I don't know a good solution. Maybe do it like perl, have a #define ELOG_POLLUTE which would pull in defined by their old names? > In any case there's too much time pressure to consider wide-ranging code > changes for 7.1. Right now I don't want to do more than rearrange the > contents of a small number of include files. Oh no, I'd plan SPI cleanup for 7.2 timeframe, if possible. -- -- Alex Pilosov | http://www.acecape.com/dsl CTO - Acecape, Inc. | AceDSL:The best ADSL in Bell Atlantic area 325 W 38 St. Suite 1005 | (Stealth Marketing Works! :) New York, NY 10018 |
Tom Lane writes: > where the include files have these roles: This plan looks good in general. It's the same I've been pondering for a while. But maybe this should receive more extensive thought than what would be appropriate to implement now. > postgres_ext.h: definitions needed in frontend, backend, *and* by clients; > by design an extremely small file The problem here will be that when we move to 8 byte oids as an option this file will depend on config.h, so this "design" will break. I have worked on a private branch with 8 byte oids a while ago and stumbled over this. This is only part of a larger problem, namely that over time it gets more likely that some public header file will depend on config.h. For example, the libpq++ one's already do. The SSL support in libpq currently requires the user to define USE_SSL themselves. int8 support in ecpg also requires configuration information. Installing config.h is a pretty drastic namespace violation. No other package that links against some PostgreSQL component can use Autoconf out of the box. This "may I install config.h" is a very heated debate around the autotools discussion fora. I don't see a consensus, but most people agree that you need to butcher config.h is some way before installing it, like prefixing all defines with a string, and renaming the file to <package>-config.h. > > postgres.h: backend-wide definitions > > postgres_fe.h: definitions common to all client-side interface libraries > > c.h: basic typedefs and macros needed by both frontend and backend, but > not intended to be exported to clients of frontend libraries > > config.h: configuration definitions, not intended to be client-visible > > os.h: platform-specific configuration hacks, not intended to be > client-visible (this comes from one of the src/include/port files) > > config.h and os.h don't need to change, I think, but I'll go through the > definitions in the other four files and make sure everything is classified > reasonably. > > It's possible that postgres_fe.h will end up containing nothing except > the inclusions of postgres_ext.h and c.h, in which case we wouldn't really > need to invent that file, but I'm still inclined to do so. I think it's > good practice to have a single include file that's the basic "must haves" > for all client-side code. -- Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
Peter Eisentraut <peter_e@gmx.net> writes:
> This is only part of a larger problem, namely that over time it gets more
> likely that some public header file will depend on config.h. For example,
> the libpq++ one's already do. The SSL support in libpq currently requires
> the user to define USE_SSL themselves. int8 support in ecpg also requires
> configuration information.
Yes, I had noticed that but didn't have a solution, so I was just
leaving the issue at status quo for now. As you say, pulling config.h
into client-visible headers is a big pollution of their namespace,
so we need to avoid it if possible.
As far as the Oid typedef goes, it'd be possible to have configure
generate postgres_ext.h from postgres_ext.h.in and insert the proper
typedef, so that there's no added namespace pollution there. This
answer does not scale real well, but it might be enough for Oid and
int8 support.
regards, tom lane
I have committed changes implementing the plan I sketched yesterday.
A basic install now installs just the include files needed for
client-side programming; to get all the include files, also saymake install-all-headers
I have verified that the header files installed by default are enough
to build the /interfaces and /bin directories, with two exceptions:
1. pg_dump.c includes a bunch of backend header files, mostly from the
src/include/catalog directory. This could perhaps be worked around,
but since pg_dump has always been pretty closely tied to the backend,
I'm not sure it's worth the trouble.
2. libpq's MULTIBYTE support pulls in a number of backend source files
which in turn want to include some backend-only headers. I'm going to
leave it to the MULTIBYTE gurus to clean that up if they feel like it.
We had talked about making separate 'client' and 'server' devel RPMs
to correspond to the minimal and full header sets. However, I measure
the extra install footprint at not very much over a megabyte, so maybe
it's not worth the trouble. If Lamar wants to just install the full
header set as part of the existing postgres-devel RPM, I won't object.
BTW, I have not done anything about Peter E's concern about config.h
polluting client namespaces. That's a valid concern but we'll have
to come back to it another day.
regards, tom lane
Great! :) It might also clean up something that I've been fighting against for awhile: when I include files needed for SPI, it drags also a lot of other garbage in, which conflicts with other things (namely, trying to get a file to simultaneously include SPI and perl headers is impossible). I realise it might be a lot of pain to clean up, but, you may consider having a separate top-level include for SPI, which would not define (by default) things like DEBUG, USE_LOCALE, union semun, etc. IMHO, it should really include only definitions of relevant data structures which interface with SPI code... I realize that complete split for SPI/module from "core backend" might be very hard, so a thing to consider would be to have (like linux kernel code has) #define IN_CORE (you are welcome to come up with better name), and include "core backend"-specific things conditionally on that being defined. -alex On Thu, 8 Feb 2001, Tom Lane wrote: > I have been looking at making a split between client-side and server-side > include files as we discussed earlier this week (pghackers thread > "Include files for SPI are not installed", if you missed it). It turns > out that the major issue here is not just divvying up the files; the > problem is that we have never had a clear concept of such a division > before, and so the core include files like postgres.h, c.h, config.h > contain a rather unholy mixture of things that are definitely > backend-only with things that are relevant to both clients and backends. > I think we need to start by clarifying the roles of these include files > and moving their contents around as necessary. > > Currently, almost every .c in the distribution starts out by including > postgres.h, which in turn includes these other files: > > postgres.h > postgres_ext.h > c.h > config.h > os.h > utils/elog.h > utils/palloc.h > > Now elog.h and palloc.h are server-only facilities and certainly don't > belong in a client file's include set. I think what we want to do is > decree that postgres.h is the primary include file for backend .c files > only, and that frontend .c files should include something else. > > postgres_ext.h would be a candidate to be that something else, except > that it's included by libpq-fe.h, so anything we add to postgres_ext.h > represents namespace pollution for libpq clients. I think we should be > very wary about adding a lot of stuff to postgres_ext.h. This suggests > that we'd best create a new primary include file for client-side .c files, > say "postgres_fe.h" or "postgres_client.h". (Anyone have a better naming > idea? Does the old 14-character limit still pose a problem anywhere?) > > That would leave us with include trees like this: > > backend .c file: > postgres.h > postgres_ext.h > c.h > config.h > os.h > utils/elog.h > utils/palloc.h > > frontend .c file: > postgres_fe.h > postgres_ext.h > c.h > config.h > os.h > > where the include files have these roles: > > postgres_ext.h: definitions needed in frontend, backend, *and* by clients; > by design an extremely small file > > postgres.h: backend-wide definitions > > postgres_fe.h: definitions common to all client-side interface libraries > > c.h: basic typedefs and macros needed by both frontend and backend, but > not intended to be exported to clients of frontend libraries > > config.h: configuration definitions, not intended to be client-visible > > os.h: platform-specific configuration hacks, not intended to be > client-visible (this comes from one of the src/include/port files) > > config.h and os.h don't need to change, I think, but I'll go through the > definitions in the other four files and make sure everything is classified > reasonably. > > It's possible that postgres_fe.h will end up containing nothing except > the inclusions of postgres_ext.h and c.h, in which case we wouldn't really > need to invent that file, but I'm still inclined to do so. I think it's > good practice to have a single include file that's the basic "must haves" > for all client-side code. > > > Now, since the intent is that the basic install provide headers needed > for client-side programming, we'd want to add postgres_fe.h to the > installed header set. But the following files can be removed from the > basic install: > > access/attnum.h > commands/trigger.h > executor/spi.h > fmgr.h > postgres.h > utils/elog.h > utils/geo_decls.h > utils/palloc.h > > We might also remove utils/fmgroids.h. I'm uncertain about this one. > The function OID macros it contains are potentially useful to clients, > but do we really want people hard-wiring function OIDs on the client > side? I doubt it. > > There are two or three other include files, such as lib/dllist.h, that are > needed on the client side only because libpq-int.h includes them, and we > want to support client code that includes libpq-int.h. I am going to look > at skinnying that list down too. libpq-fs.h, in particular, looks like > mostly legacy junk ... > > As we discussed, there'll be an additional install target (or RPM) that > installs these files and everything else from the src/include tree. > > Comments? > > regards, tom lane > > -- -- Alex Pilosov | http://www.acecape.com/dsl CTO - Acecape, Inc. | AceDSL:The best ADSL in Bell Atlantic area 325 W 38 St. Suite 1005 | (Stealth Marketing Works! :) New York, NY 10018 |