Re: [HACKERS] Plan for straightening out the include-file mess
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
Re: [HACKERS] Plan for straightening out the include-file mess
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 say make 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
[HACKERS] 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.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
Re: [HACKERS] Plan for straightening out the include-file mess
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
Re: [HACKERS] Plan for straightening out the include-file mess
Alex Pilosov [EMAIL PROTECTED] 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
Re: [HACKERS] Plan for straightening out the include-file mess
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 [EMAIL PROTECTED] http://yi.org/peter-e/
Re: [HACKERS] Plan for straightening out the include-file mess
Peter Eisentraut [EMAIL PROTECTED] 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
