Re: includedir_internal headers are not self-contained

Re: Tom Lane 2014-04-26 <21449(dot)1398524746(at)sss(dot)pgh(dot)pa(dot)us>
> > internal/postgres_fe.h includes
> > common/fe_memutils.h which includes
> > utils/palloc.h
>
> Hm. It seems rather fundamentally broken to me that frontend code is
> including palloc.h --- that file was never intended to be frontend-safe,
> and the #ifdefs that I see in it today don't fill me with any feeling of
> quality workmanship.
>
> I think what we ought to do about this is get rid of the dependency
> on palloc.h.

Thanks for the first tranche of patches on this.

> > Both common/ and utils/ are server-only, so you can't build client
> > apps which need postgres_fe.h with only libpq-dev installed.
>
> Clearly, the idea that common/ is server-only is broken.

The next step should probably be something like this:

diff --git a/src/include/Makefile b/src/include/Makefile
new file mode 100644
index 578a778..d39aa97
*** a/src/include/Makefile
--- b/src/include/Makefile
*************** include $(top_builddir)/src/Makefile.glo
*** 16,21 ****
--- 16,23 ----
all: pg_config.h pg_config_ext.h pg_config_os.h


+ # Subdirectories containing headers for client- and server-side dev
+ SUBDIRS_INTERNAL = common
# Subdirectories containing headers for server-side dev
SUBDIRS = access bootstrap catalog commands common datatype executor foreign \
lib libpq mb nodes optimizer parser postmaster regex replication \
*************** install: all installdirs
*** 38,50 ****
$(INSTALL_DATA) $(srcdir)/port.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/postgres_fe.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/libpq/pqcomm.h '$(DESTDIR)$(includedir_internal)/libpq'
# These headers are needed for server-side development
$(INSTALL_DATA) pg_config.h '$(DESTDIR)$(includedir_server)'
$(INSTALL_DATA) pg_config_ext.h '$(DESTDIR)$(includedir_server)'
$(INSTALL_DATA) pg_config_os.h '$(DESTDIR)$(includedir_server)'
$(INSTALL_DATA) utils/errcodes.h '$(DESTDIR)$(includedir_server)/utils'
$(INSTALL_DATA) utils/fmgroids.h '$(DESTDIR)$(includedir_server)/utils'
- # We don't use INSTALL_DATA for performance reasons --- there are a lot of files
cp $(srcdir)/*.h '$(DESTDIR)$(includedir_server)'/ || exit; \
chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/*.h || exit; \
for dir in $(SUBDIRS); do \
--- 40,56 ----
$(INSTALL_DATA) $(srcdir)/port.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/postgres_fe.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/libpq/pqcomm.h '$(DESTDIR)$(includedir_internal)/libpq'
+ # We don't use INSTALL_DATA for performance reasons --- there are a lot of files
+ for dir in $(SUBDIRS_INTERNAL); do \
+ cp $(srcdir)/$$dir/*.h '$(DESTDIR)$(includedir_internal)'/$$dir/ || exit; \
+ chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_internal)'/$$dir/*.h || exit; \
+ done
# These headers are needed for server-side development
$(INSTALL_DATA) pg_config.h '$(DESTDIR)$(includedir_server)'
$(INSTALL_DATA) pg_config_ext.h '$(DESTDIR)$(includedir_server)'
$(INSTALL_DATA) pg_config_os.h '$(DESTDIR)$(includedir_server)'
$(INSTALL_DATA) utils/errcodes.h '$(DESTDIR)$(includedir_server)/utils'
$(INSTALL_DATA) utils/fmgroids.h '$(DESTDIR)$(includedir_server)/utils'
cp $(srcdir)/*.h '$(DESTDIR)$(includedir_server)'/ || exit; \
chmod $(INSTALL_DATA_MODE) '$(DESTDIR)$(includedir_server)'/*.h || exit; \
for dir in $(SUBDIRS); do \
*************** ifeq ($(vpath_build),yes)
*** 59,65 ****
endif

installdirs:
! $(MKDIR_P) '$(DESTDIR)$(includedir)/libpq' '$(DESTDIR)$(includedir_internal)/libpq'
$(MKDIR_P) $(addprefix '$(DESTDIR)$(includedir_server)'/, $(SUBDIRS))


--- 65,72 ----
endif

installdirs:
! $(MKDIR_P) '$(DESTDIR)$(includedir)/libpq'
! $(MKDIR_P) '$(DESTDIR)$(includedir_internal)/common' '$(DESTDIR)$(includedir_internal)/libpq'
$(MKDIR_P) $(addprefix '$(DESTDIR)$(includedir_server)'/, $(SUBDIRS))

Depending on when 9.4 is coming out, Debian Jessie will probably be
releasing with 9.3. How much of these fixes could we expect to be
backported to 9.3?

> > (Another issue is that client apps frequently seem to want
> > catalog/pg_type.h to get the OID definitions, it might make sense to
> > move that also to internal/.)
>
> That's not happening. We do need some better solution for letting client
> apps get hold of fixed type oids, but moving a catalog header someplace
> else is not it.

Maybe a derived header file generated at build time?

grep '^#define.*OID\>' catalog/pg_type.h > common/pg_staticoids.h

Christoph
--
cb(at)df7cb(dot)de | http://www.df7cb.de/