Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. ci: Add missing "set -e" to scripts run by su.

  2. Don't put library-supplied -L/-I switches before user-supplied ones.

  1. meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-02-03T00:28:40Z

    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.
    
    Reordering obvious mentions of libpq, as in the attached, gets past
    most of them but I couldn't immediately figure out how to reorder
    src/test/isolation/meson.build and gave up and uninstalled the
    interfering libpq package for now.  Dumping these observations here as
    this is as far as I got with this impediment today.
    
  2. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-07-14T00:48:23Z

    I took another look and got it working, after something else I'm using
    insisted on installing libpq.  It's mostly no-brainer hunks like this:
    
    -  dependencies: [frontend_code, libpq],
    +  dependencies: [libpq, frontend_code],
    
    For the new src/test/modules/test_oat_hooks/meson.build, I copied what
    I'd seen in contrib modules that use libpq:
    
    -  kwargs: pg_test_mod_args,
    +  kwargs: pg_test_mod_args + {
    +       'dependencies': [libpq] + pg_test_mod_args['dependencies'],
    +  }
    
    (I'm still green enough with Meson that I had to look that syntax up:
    it's how you replace the values of common keys with the values from
    the right hand dict[1], which ain't Pythonesque AFAIK.)
    
    For ecpg and pg_regress, where I got stuck last time, I am still a bit
    confused about whether it should go here:
    
    -ecpg_inc = include_directories('.')
    +ecpg_inc = [libpq_inc, include_directories('.')]
    
    ... or perhaps in intermediate ecpgXXX_inc variables or the final
    include_directories directives.  In this version I just did that easy
    thing and it works for me.  But is it the right place?
    
    Likewise for pg_regress_inc, also used by ECPG and isolation tests:
    
    -pg_regress_inc = include_directories('.')
    +pg_regress_inc = [libpq_inc, include_directories('.')]
    
    [1] https://mesonbuild.com/Reference-manual_elementary_dict.html
    
  3. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-09-14T03:23:55Z

    Added to commitfest: https://commitfest.postgresql.org/patch/6056/
    
    I'm having to carry this patch in all my development branches for now
    or I can't build on one of my regular dev machines, so I'm quite keen
    to get this into the tree soon and would back-patch to 16.
    
    I gather no one else is affected yet, but I assume you can see the
    problem on a Mac by installing PostgreSQL with Homebrew/MacPorts, or
    on CI if you do this to .cirrus.tasks.yml:
    
       setup_additional_packages_script: |
    -    #pkg install -y ...
    +    pkg install -y postgresql17-client
    
    
    
    
  4. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Mario Gonzalez <gonzalemario@gmail.com> — 2025-10-09T19:09:17Z

    On Thu, 9 Oct 2025 at 16:06, Thomas Munro <thomas.munro@gmail.com> wrote:
    >
    > Added to commitfest: https://commitfest.postgresql.org/patch/6056/
    >
    > I'm having to carry this patch in all my development branches for now
    > or I can't build on one of my regular dev machines, so I'm quite keen
    > to get this into the tree soon and would back-patch to 16.
    >
    
    Can you confirm you still have this problem on current master? I tried
    to reproduce it in debian 12 by installing `postgresql-server-dev-15`
    and adding the -Dextra_include_dirs
    
    > I gather no one else is affected yet, but I assume you can see the
    > problem on a Mac by installing PostgreSQL with Homebrew/MacPorts, or
    > on CI if you do this to .cirrus.tasks.yml:
    >
    >    setup_additional_packages_script: |
    > -    #pkg install -y ...
    > +    pkg install -y postgresql17-client
    >
    
    -- 
    https://www.linkedin.com/in/gonzalemario
    
    
    
    
  5. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-10-09T20:26:08Z

    On Fri, Oct 10, 2025 at 8:09 AM Mario González Troncoso
    <gonzalemario@gmail.com> wrote:
    > Can you confirm you still have this problem on current master? I tried
    
    Thanks for looking!  Yes.
    
    > to reproduce it in debian 12 by installing `postgresql-server-dev-15`
    > and adding the -Dextra_include_dirs
    
    The problem is with libpq headers, not server headers.
    
    Thinking about how to show this on Debian...  Also it'll fail to fail
    if your libpq headers are new enough to be compatible, eg 18.  I guess
    if you find an older libpq-dev .deb file, v16 is what my system is
    clashing with, and unpack it into a temporary directory (something
    like ar x XXX.deb then tar xf data.tar.XXX), and then point to the
    headers in -Dextra_include_dirs, you should see it?
    
    Or maybe just make a temporary file somewhere called libpq-fe.h that contains:
    
    #error "wrong header included"
    
    ... and point to its parent with -Dextra_include_dirs.  The goal is
    for the in-tree libpq-fe.h to be found sooner in the search path (as
    it is with configure), completely hiding that poisoned file.
    
    
    
    
  6. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Heikki Linnakangas <hlinnaka@iki.fi> — 2025-10-31T18:14:38Z

    On 14/09/2025 06:23, Thomas Munro wrote:
    > Added to commitfest: https://commitfest.postgresql.org/patch/6056/
    > 
    > I'm having to carry this patch in all my development branches for now
    > or I can't build on one of my regular dev machines, so I'm quite keen
    > to get this into the tree soon and would back-patch to 16.
    > 
    > I gather no one else is affected yet, but I assume you can see the
    > problem on a Mac by installing PostgreSQL with Homebrew/MacPorts, or
    > on CI if you do this to .cirrus.tasks.yml:
    > 
    >     setup_additional_packages_script: |
    > -    #pkg install -y ...
    > +    pkg install -y postgresql17-client
    
    The patch seems harmless enough to me, no objections to committing it. 
    However, I'm worried that we'll soon break it again. The new rule is 
    apparently "include libpq first", but we have no way of enforcing it.
    
    - Heikki
    
    
    
    
    
  7. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-10-31T21:13:38Z

    On Sat, Nov 1, 2025 at 7:14 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
    > The patch seems harmless enough to me, no objections to committing it.
    
    Thanks!
    
    > However, I'm worried that we'll soon break it again. The new rule is
    > apparently "include libpq first", but we have no way of enforcing it.
    
    Hmm, my meson-fu is weak, but there must surely be some way to declare
    that you want libpq and have it automatically put things in the right
    order, will look into that...
    
    
    
    
  8. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-01T05:21:35Z

    On Sat, Nov 1, 2025 at 10:13 AM Thomas Munro <thomas.munro@gmail.com> wrote:
    > On Sat, Nov 1, 2025 at 7:14 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
    > > However, I'm worried that we'll soon break it again. The new rule is
    > > apparently "include libpq first", but we have no way of enforcing it.
    >
    > Hmm, my meson-fu is weak, but there must surely be some way to declare
    > that you want libpq and have it automatically put things in the right
    > order, will look into that...
    
    If we really don't want every site that depends on both libpq and
    frontend_code to have to remember to respect that order when declaring
    dependencies, then we could instead change frontend_code to force
    libpq_inc to appear in its include_directories before postgres_inc (=
    where extra_include_dirs comes from).  This one-liner fixes the build
    on my system:
    
     # for frontend binaries
     frontend_code = declare_dependency(
    -  include_directories: [postgres_inc],
    +  include_directories: [libpq_inc, postgres_inc],
    
    That feels a little odd, because libpq is not really a dependency of
    frontend_code, and not all frontend_code users also use libpq (though
    almost all do).  Does this have any unwanted side-effects?  Is there a
    better way to do this in a central place?
    
    There are two other places that already have those two in
    include_directories already, so their order should surely be flipped
    to match, they just didn't happen to break on my system (I guess by
    luck, ie not accessing APIs that changed incompatibly since the
    version in my system-installed libpq headers).
    
  9. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-03T02:09:43Z

    On Sat, Nov 1, 2025 at 7:14 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
    > The patch seems harmless enough to me, no objections to committing it.
    > However, I'm worried that we'll soon break it again. The new rule is
    > apparently "include libpq first", but we have no way of enforcing it.
    
    Here's a new version of the original approach with fixes for some
    copy-and-pastes I'd missed, and a new patch to check the search order
    in CI.  It adds a "poisoned" libpq-fe.h header into the search path to
    break the build step if you get it wrong in meson or configure (cf
    commit 4300d8b6 which fixed a case of this there).  It fails like
    this:
    
    [01:45:21.825] In file included from ../src/include/fe_utils/connect_utils.h:15,
    [01:45:21.825]                  from ../src/bin/scripts/common.h:13,
    [01:45:21.825]                  from ../src/bin/scripts/vacuumdb.c:17:
    [01:45:21.825] /tmp/poisoned_headers/libpq-fe.h:1:2: error: #error
    "external header hides in-tree header"
    [01:45:21.825]     1 | #error "external header hides in-tree header"
    [01:45:21.825]       |  ^~~~~
    
    There must surely be a more mesonic way to do this with declarations
    rather than the order in a flimsy list that is copied all over the
    place (I guess that is probably supposed to be thought of as an
    unordered set...?), but I'm not sure I was on the right path with my
    v2 as mentioned and it didn't survive this test.  This approach is OK
    for now, I think... if someone ever shows up with a patch to tackle it
    more fundamentally, the order will presumably become more flexible, so
    the new order will still be valid but unimportant.
    
    Will wait another day for a better idea or objection to show up, and
    otherwise commit these to 16+.
    
  10. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Tristan Partin <tristan@partin.io> — 2025-11-03T05:43:58Z

    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)
    
    
    
    
  11. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-03T06:14:48Z

    "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
    
    
    
    
  12. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-04T05:30:51Z

    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
    
  13. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Tristan Partin <tristan@partin.io> — 2025-11-04T06:17:45Z

    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)
    
    
    
    
  14. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Tristan Partin <tristan@partin.io> — 2025-11-04T06:25:05Z

    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)
    
    
    
    
  15. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Andres Freund <andres@anarazel.de> — 2025-11-04T19:55:56Z

    Hi,
    
    On 2025-11-04 18:30:51 +1300, Thomas Munro wrote:
    > First two patches as before, except for a couple of unnecessary hunks
    > I deleted based on an off-list review from Bilal.
    
    I think there may be a less verbose way to do this:
    
    The problem is caused by us adding extra_include_dirs to postgres_inc_d, which
    does not include the private include dir for e.g. libpq. As it is added to
    frontend_code etc as a flat list, there's no way for meson to know that
    src/interfaces/libpq should be added earlier in the commandline.
    
    The position we add extra_include_dirs right now also seems wrong on windows,
    leaving libpq aside, as it's added *before* src/include/port/win32 etc.
    
    
    The easiest way to fix that seems to be to simply not extra_include_dirs to
    postgres_inc_d, but instead either add it to cppflags (the meson variable, not
    the environment) or add it to the project C flags.
    
    It seems that with autoconf we add the --with-includes to the pg_config
    --cppflags, but we don't today with meson. Adding it to the cppflags variable
    would take care of that too.
    
    A quick prototype of that is attached.
    
    Thoughts?
    
    Greetings,
    
    Andres Freund
    
  16. Re: meson's in-tree libpq header search order vs -Dextra_include_dirs

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-05T01:50:55Z

    On Wed, Nov 5, 2025 at 8:55 AM Andres Freund <andres@anarazel.de> wrote:
    > The problem is caused by us adding extra_include_dirs to postgres_inc_d, which
    > does not include the private include dir for e.g. libpq. As it is added to
    > frontend_code etc as a flat list, there's no way for meson to know that
    > src/interfaces/libpq should be added earlier in the commandline.
    
    Check.
    
    > The position we add extra_include_dirs right now also seems wrong on windows,
    > leaving libpq aside, as it's added *before* src/include/port/win32 etc.
    
    Yeah.
    
    > The easiest way to fix that seems to be to simply not extra_include_dirs to
    > postgres_inc_d, but instead either add it to cppflags (the meson variable, not
    > the environment) or add it to the project C flags.
    
    I see.  Similar to what Tristan said except higher level than having
    the human write command line arguments -I/foo/bar as C/CPP flags.  I
    actually tried to do something like that myself early on, but I
    couldn't figure out how to expand include directories to command line
    arguments using meson machinery, as it does for dependencies.  I had a
    notion that raw -I strings weren't OK, but I see that we already do
    that while probing perl.
    
    > It seems that with autoconf we add the --with-includes to the pg_config
    > --cppflags, but we don't today with meson. Adding it to the cppflags variable
    > would take care of that too.
    
    Right!
    
    To experience a problem I suppose you'd need meson-built PostgreSQL, an
    extension running pg_config, and a PostgreSQL header that wants to
    include something from an external package on a platform that puts
    them outside the base system, and that hasn't come up in the wild yet.
    Meson adoption in the relevant packaging projects may not be happening
    yet.
    
    > A quick prototype of that is attached.
    
    I think where cc.find_library() probe headers, we need
    s/header_include_directories: postgres_inc/header_args: test_c_args/.
    Otherwise for example the libintl probe fails on my system, and in
    that particular case it doesn't seem necessary to probe the header
    anyway so that can just be removed.
    
    Similarly, cc.has_function() and cc.has_header() could probably drop
    include_directories as they already have test_c_args, as long as they
    are probing external libraries.
    
    Some probes do still need postgres_inc, though: I think it's the
    category that is testing OS headers, but we've interposed a bunch of
    wrappers on Windows eg sockets.  Maybe that's confusing enough that we
    should just leave postgres_inc everywhere but supplement it with
    header_args: test_c_args where it's missing, or should we add
    postgres_inc to test_c_args?
    
    With extra_include_dirs in cpp_flags, bitcode_cflags also needs to be
    reordered, or else:
    
    ... clang ... -I/usr/local/include -I./src/include
    -I./src/backend/utils/misc -I../src/include
    ../src/include/c.h:113:9: warning: 'pg_restrict' macro redefined
    [-Wmacro-redefined]
      113 | #define pg_restrict restrict
          |         ^
    /usr/local/include/pg_config.h:806:9: note: previous definition is here
      806 | #define pg_restrict __restrict
          |         ^
    
    I have some rough patches for those changes...
    
    I think there might be a fundamental problem with this though.
    Although it fixes my original complaint today, all it would take to
    make it break again would be something to appear with "dependencies:
    [<something-found-by-pkg-config> libpq]", or libintl with the v4-0003
    patch I posted yesterday (which finds the headers and declares the
    dependency, an extended fuzzy version of the sort of thing meson does
    inside dependency(...) IIUC,  the point of my patch being to make it
    so that you don't need -Dextra_include_dirs at all on Macs and *BSD).
    In other words, this problem isn't really just about
    extra_include_dirs, it's also about paths produced by dependencies,
    which can bring external libpq-fe.h back into visibility.  Hence my
    attempt to engage with the dependency system.