Re: includedir_internal headers are not self-contained

Re: Tom Lane 2014-04-26 <21449.1398524746@sss.pgh.pa.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
commandscommon 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@df7cb.de | http://www.df7cb.de/