Обсуждение: 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

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