Обсуждение: Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

On Sun Feb 2, 2025 at 6:29 PM CST, Thomas Munro wrote:
> When I use configure/make and --with-includes=/usr/local/include, I
> see compile lines like this:
>
> ... -I../../src/interfaces/libpq -I../../src/include  -I/usr/local/include ...
>
> That's important, because if I happen to have libpq headers installed
> on the system I don't want to be confused by them.  Yet meson's
> -Dextra_include_dirs=/usr/local/include compiles like this:
>
> ... -I../src/include -I/usr/local/include -Isrc/interfaces/libpq ...
>
> ... which gives me errors due to the v16 headers I happen to have installed:
>
> ../src/fe_utils/connect_utils.c:164:3: error: unknown type name
> 'PGcancelConn'; did you mean 'PGcancel'?
>   164 |                 PGcancelConn *cancelConn = PQcancelCreate(conn);
>
> I guess this affects Macs, BSDs and a few lesser used Linux distros
> that put optional packages outside the base system and default search
> paths.  Perhaps you can see this on everything-goes-into-base-system
> distros if you redundantly say -Dextra_include_dirs=/usr/include; I
> didn't check if that is true, it wouldn't be if meson is opinionated
> enough to remove it.

What is the benefit of -Dextra_include_dirs when you could use -Dc_flags
or CFLAGS to specify extra include directories? If you tried either of
those as an alternative, would they fix the issue? The existence of the
option goes all the way back to the initial commit of the meson port, so
I presume it exists to mirror --with-includes, but why does that exist?

--
Tristan Partin
Databricks (https://databricks.com)

"Tristan Partin" <tristan@partin.io> writes:
> What is the benefit of -Dextra_include_dirs when you could use -Dc_flags 
> or CFLAGS to specify extra include directories? If you tried either of 
> those as an alternative, would they fix the issue? The existence of the 
> option goes all the way back to the initial commit of the meson port, so 
> I presume it exists to mirror --with-includes, but why does that exist?

--with-includes, and likewise --with-libs, exist because there are
platforms that require it.  For example, on pretty much any
BSD-derived system, the core /usr/include and /usr/lib files are very
limited and you'll need to point at places like /usr/pkg/include or
/opt/local/include or whatever to pull in non-core packages.

I see your point about putting -I flags into CFLAGS, but what you
are missing is that the order of these flags is unbelievably critical,
so "just shove it into CFLAGS" is a recipe for failure, especially
if you haven't even stopped to think about which end to add it at.
We've learned over decades of trial-and-error with the makefile system
that treating -I and -L flags specially is more reliable.  (See for
example all the logic in our autoconf scripts to forcibly separate
-I and -L out of sources like pkg-config.  Tracing that stuff back to
the originating commits and mail discussions would be a constructive
learning exercise.)

I'm not sure that the meson crew have learned all that yet.
But this does not suggest to me that they know more than we do.

            regards, tom lane

On Mon, Nov 3, 2025 at 7:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Tristan Partin" <tristan@partin.io> writes:
> > What is the benefit of -Dextra_include_dirs when you could use -Dc_flags
> > or CFLAGS to specify extra include directories? If you tried either of
> > those as an alternative, would they fix the issue? The existence of the
> > option goes all the way back to the initial commit of the meson port, so
> > I presume it exists to mirror --with-includes, but why does that exist?

Good questions.  Sounds plausible and probably more conventional.  It
doesn't work right now but probably only for superficial reasons.  But
I also worry about hiding current or future problems and generally
being too blunt.  But that's also true of -Dextra_XXX.

> --with-includes, and likewise --with-libs, exist because there are
> platforms that require it.  For example, on pretty much any
> BSD-derived system, the core /usr/include and /usr/lib files are very
> limited and you'll need to point at places like /usr/pkg/include or
> /opt/local/include or whatever to pull in non-core packages.

Yeah.  I for one cargo-culted that configure habit across to the new
world, but you can actually build without -Dextra_XXX on FreeBSD and
maybe macOS + MacPorts too with -Dnls=disabled, as long as your $PATH
contains pkg-config (FreeBSD: yes by default, macOS: probably yes if
you can type "meson").  NetBSD handles even -Dnls=enabled thanks to
its built-in libintl, as CI already demonstrates.  So I wonder if we
have this only because out-of-libc libintl and pkg-config have some
unresolved beef[1][2]...?  Know of any other complications in our
universe or libraries that I might be overlooking?

If that really is the last reason we need this on typical
package-managed non-Linux environments (not sure!)... here's an
experimental patch that probes the conventional paths relative to
msgfmt to hunt for them.  They were all compiled from the same source
package by the same packaging pipeline so I think it has pretty good
odds.  It's not quite $PATH -> pkg-config -> .pc -> exact results, but
it's ... well, something vaguely similar, instead of nothing.  Too
crazy an overreaching assumption, or friendly out-of-the-box support
for lots of common systems including Macs?  It passes on all CI OSes.
No loss if it guesses wrong on your DeathStation 9000: you can still
supply -Dextra_XXX as before.

On the flip side, if you stop using a "catch-all" include path, you
have to declare dependencies accurately or some combinations might
break, for example that gssapi I had to add, because the CI macOS task
found Apple's base system OpenSSL and then didn't think it needed any
extra includes, because we forgot to say that libpq_oauth_st also
depends on gssapi.  That's an example of a "broad"
-Dextra_include_dirs hiding something that is technically incorrect,
or being forgiving depending on your perspective...

First two patches as before, except for a couple of unnecessary hunks
I deleted based on an off-list review from Bilal.

[1] https://lists.gnu.org/archive/html/bug-gettext/2012-03/msg00022.html
[2] https://lists.nongnu.org/archive/html/bug-gettext/2019-10/msg00003.html

On Mon Nov 3, 2025 at 12:14 AM CST, Tom Lane wrote:
> "Tristan Partin" <tristan@partin.io> writes:
>> What is the benefit of -Dextra_include_dirs when you could use -Dc_flags
>> or CFLAGS to specify extra include directories? If you tried either of
>> those as an alternative, would they fix the issue? The existence of the
>> option goes all the way back to the initial commit of the meson port, so
>> I presume it exists to mirror --with-includes, but why does that exist?
>
> --with-includes, and likewise --with-libs, exist because there are
> platforms that require it.  For example, on pretty much any
> BSD-derived system, the core /usr/include and /usr/lib files are very
> limited and you'll need to point at places like /usr/pkg/include or
> /opt/local/include or whatever to pull in non-core packages.
>
> I see your point about putting -I flags into CFLAGS, but what you
> are missing is that the order of these flags is unbelievably critical,
> so "just shove it into CFLAGS" is a recipe for failure, especially
> if you haven't even stopped to think about which end to add it at.
> We've learned over decades of trial-and-error with the makefile system
> that treating -I and -L flags specially is more reliable.  (See for
> example all the logic in our autoconf scripts to forcibly separate
> -I and -L out of sources like pkg-config.  Tracing that stuff back to
> the originating commits and mail discussions would be a constructive
> learning exercise.)

I'm well aware of ordering dependencies, and how annoying they can be.
The perils of C will outlive us all! Given your message and Thomas's
patch, I decided to take a look at the autotools build on PG 16 (since
that is what I had checked out, but maybe also maybe useful as the
original Meson build) and how it compared. Thomas has this hunk in his
patch:

    diff --git a/src/test/isolation/meson.build b/src/test/isolation/meson.build
    index a180e4e2741..660b11eff8b 100644
    --- a/src/test/isolation/meson.build
    +++ b/src/test/isolation/meson.build
    @@ -35,7 +35,7 @@ pg_isolation_regress = executable('pg_isolation_regress',
       isolation_sources,
       c_args: pg_regress_cflags,
       include_directories: pg_regress_inc,
    -  dependencies: [frontend_code, libpq],
    +  dependencies: [libpq, frontend_code],
       kwargs: default_bin_args + {
         'install_dir': dir_pgxs / 'src/test/isolation',
       },
    @@ -52,7 +52,7 @@ endif
     isolationtester = executable('isolationtester',
       isolationtester_sources,
       include_directories: include_directories('.'),
    -  dependencies: [frontend_code, libpq],
    +  dependencies: [libpq, frontend_code],
       kwargs: default_bin_args + {
         'install_dir': dir_pgxs / 'src/test/isolation',
       },

Taking a look, specifically, at isolationtester, I get the following
final compilation line for isolationtester.o (please forgive the macOS garbage):

    gcc -Wall -Wmissing-prototypes -Wpointer-arith \
        -Wdeclaration-after-statement -Werror=vla \
        -Werror=unguarded-availability-new -Wendif-labels \
        -Wmissing-format-attribute -Wcast-function-type \
        -Wformat-security -fno-strict-aliasing -fwrapv \
        -fexcess-precision=standard -Wno-unused-command-line-argument \
        -Wno-compound-token-split-by-macro -Wno-format-truncation \
        -Wno-cast-function-type-strict -O2 -I. \
        -I/Users/tristan.partin/Projects/work/postgres/build-autotools/../src/test/isolation \
        -I/Users/tristan.partin/Projects/work/postgres/build-autotools/../src/interfaces/libpq \
        -I/Users/tristan.partin/Projects/work/postgres/build-autotools/../src/test/isolation/../regress \
        -I../../../src/include \
        -I/Users/tristan.partin/Projects/work/postgres/build-autotools/../src/include \
        -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX26.0.sdk \
        -c -o isolationtester.o \
        /Users/tristan.partin/Projects/work/postgres/build-autotools/../src/test/isolation/isolationtester.c

The first include directory that isn't the source or build directory is
the in-tree libpq headers. I assume that is controlled by:

    override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) \
        -I$(srcdir)/../regress $(CPPFLAGS)

Thomas's patch seemingly makes the meson build equivalent to the
autotools build. Just for good measure, following --with-includes, the
include directories in that configure argument end up in CPPFLAGS:

    CPPFLAGS="$CPPFLAGS $INCLUDES"

where INCLUDES is:

    #
    # Include directories
    #
    ac_save_IFS=$IFS
    IFS="${IFS}${PATH_SEPARATOR}"
    # SRCH_INC comes from the template file
    for dir in $with_includes $SRCH_INC; do
      if test -d "$dir"; then
        INCLUDES="$INCLUDES -I$dir"
      else
        { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: *** Include directory $dir does not exist." >&5
    $as_echo "$as_me: WARNING: *** Include directory $dir does not exist." >&2;}
      fi
    done
    IFS=$ac_save_IFS

From my perspective, it looks like the autotools build has just been
_lucky_ to avoid this problem. I don't see anything that inherently
prevents the problem that Thomas ran into. In my opinion Thomas's patch
is just a parity patch. It's incredible that it took this long to find.

--
Tristan Partin
Databricks (https://databricks.com)

After my analysis in the other subthread, patch 1 looks ready to be
committed. Patch 2 looks good to go, but maybe you should split the "set
-e" changes into a separate commit. Not sure how much I like patch
3 since it hardcodes $libdir as "lib" for the purposes of finding the
header, but it's also kind of a last resort thing, so ¯\_(ツ)_/¯. Maybe
it's best if the meson build just copies whatever the autotools build
does?

--
Tristan Partin
Databricks (https://databricks.com)