Thread: libpq-fe.h should compile *entirely* standalone
I realized that headerscheck is failing to enforce $SUBJECT. This is bad, since we aren't really using libpq-fe.h ourselves in a way that would ensure that c.h symbols don't creep into it. We can easily do better, as attached, but I wonder which other headers should get the same treatment. regards, tom lane diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck index abbba7aa63..69897092b2 100755 --- a/src/tools/pginclude/headerscheck +++ b/src/tools/pginclude/headerscheck @@ -142,7 +142,14 @@ do # OK, create .c file to include this .h file. { - test "$f" != src/include/postgres_fe.h && echo '#include "postgres.h"' + # Ideally we'd pre-include only the appropriate one of + # postgres.h, postgres_fe.h, or c.h, but we don't have enough + # info here to guess what to do in most cases. + if test "$f" != src/include/postgres_fe.h -a \ + "$f" != src/interfaces/libpq/libpq-fe.h + then + echo '#include "postgres.h"' + fi echo "#include \"$f\"" } >$tmp/test.c
I wrote: > We can easily do better, as attached, but I wonder which other > headers should get the same treatment. After a bit of further research I propose the attached. I'm not sure exactly what subset of ECPG headers is meant to be exposed to clients, but we can adjust these patterns if new info emerges. This is actually moving the inclusion-check goalposts quite far, but HEAD seems to pass cleanly, and again we can always adjust later. Any objections? regards, tom lane diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck index 2c5042eb41..6ad5ef6942 100755 --- a/src/tools/pginclude/cpluspluscheck +++ b/src/tools/pginclude/cpluspluscheck @@ -161,7 +161,31 @@ do # OK, create .c file to include this .h file. { echo 'extern "C" {' - test "$f" != src/include/postgres_fe.h && echo '#include "postgres.h"' + # Ideally we'd pre-include only the appropriate one of + # postgres.h, postgres_fe.h, or c.h. We don't always have enough + # info to guess which, but in some subdirectories there's a + # reasonable choice to make, and otherwise we use postgres.h. + # Also, those three files should compile with no pre-include, as + # should src/interfaces headers meant to be exposed to clients. + case "$f" in + src/include/postgres.h) ;; + src/include/postgres_fe.h) ;; + src/include/c.h) ;; + src/interfaces/libpq/libpq-fe.h) ;; + src/interfaces/libpq/libpq-events.h) ;; + src/interfaces/ecpg/ecpglib/ecpglib_extern.h) + echo '#include "postgres_fe.h"' ;; + src/interfaces/ecpg/ecpglib/*) ;; + src/interfaces/*) + echo '#include "postgres_fe.h"' ;; + src/bin/*) + echo '#include "postgres_fe.h"' ;; + src/port/*) ;; + src/common/*) + echo '#include "c.h"' ;; + *) + echo '#include "postgres.h"' ;; + esac echo "#include \"$f\"" echo '};' } >$tmp/test.cpp diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck index abbba7aa63..f1810c09bb 100755 --- a/src/tools/pginclude/headerscheck +++ b/src/tools/pginclude/headerscheck @@ -142,7 +142,31 @@ do # OK, create .c file to include this .h file. { - test "$f" != src/include/postgres_fe.h && echo '#include "postgres.h"' + # Ideally we'd pre-include only the appropriate one of + # postgres.h, postgres_fe.h, or c.h. We don't always have enough + # info to guess which, but in some subdirectories there's a + # reasonable choice to make, and otherwise we use postgres.h. + # Also, those three files should compile with no pre-include, as + # should src/interfaces headers meant to be exposed to clients. + case "$f" in + src/include/postgres.h) ;; + src/include/postgres_fe.h) ;; + src/include/c.h) ;; + src/interfaces/libpq/libpq-fe.h) ;; + src/interfaces/libpq/libpq-events.h) ;; + src/interfaces/ecpg/ecpglib/ecpglib_extern.h) + echo '#include "postgres_fe.h"' ;; + src/interfaces/ecpg/ecpglib/*) ;; + src/interfaces/*) + echo '#include "postgres_fe.h"' ;; + src/bin/*) + echo '#include "postgres_fe.h"' ;; + src/port/*) ;; + src/common/*) + echo '#include "c.h"' ;; + *) + echo '#include "postgres.h"' ;; + esac echo "#include \"$f\"" } >$tmp/test.c
On 2023-03-03 Fr 13:46, Tom Lane wrote:
I wrote:We can easily do better, as attached, but I wonder which other headers should get the same treatment.After a bit of further research I propose the attached. I'm not sure exactly what subset of ECPG headers is meant to be exposed to clients, but we can adjust these patterns if new info emerges. This is actually moving the inclusion-check goalposts quite far, but HEAD seems to pass cleanly, and again we can always adjust later. Any objections?
LGTM
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes: > On 2023-03-03 Fr 13:46, Tom Lane wrote: >> This is actually moving the inclusion-check goalposts quite far, >> but HEAD seems to pass cleanly, and again we can always adjust later. >> Any objections? > LGTM Pushed, thanks for looking. regards, tom lane