Re: meson: pass OpenSSL/ICU include dirs to extensions

From: Aidar Imamov <imamovaj22(at)gmail(dot)com>
To: Grigorev Jurij <ju(dot)grigorev(at)ftdata(dot)ru>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: meson: pass OpenSSL/ICU include dirs to extensions
Date: 2026-10-14 17:11:38
Message-ID: D7443D30-8CBD-47F0-A45B-54EC015679E5@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Oct 14, 2026, at 18:08, Aidar Imamov <imamovaj22(at)gmail(dot)com> wrote:
>
>
>
>> On Sep 30, 2026, at 07:53, Grigorev Jurij <ju(dot)grigorev(at)ftdata(dot)ru> wrote:
>>
>> Hi Aidar,
>>
>> Thanks for the quick update. v2 looks much closer to me!
>>
>> I like that both pkgconfig and cmake are covered now, and that all include dirs are kept.
>> Placement still looks right too: OpenSSL in CPPFLAGS and ICU in ICU_CFLAGS,
>> matching Makefile.global and autoconf.
>>
>> Three small worries, all about cases where flags end up silently empty:
>>
>> First, the hand-rolled pkg-config call:
>>> if ssl.type_name() == 'pkgconfig' and pkg_config.found()
>>> openssl_cflags = run_command(pkg_config, '--cflags', 'openssl',
>>> check: false).stdout().strip()
>>
>> Could we check returncode() here, use native: true, and query "icu-uc icu-i18n" together like configure does?
>> Right now a failed pkg-config just gives empty flags, and the extension fails much later with a missing header.
>>
>>
>> Second, pkg_config is defined in one file but used in another:
>>> src/include/meson.build:
>>> pkg_config = find_program( 'pkg-config', required: false)
>>
>>> src/makefiles/meson.build:
>>> if icu.type_name() == 'pkgconfig' and pkg_config.found()
>>
>> It works today because of subdir ordering, but it is fragile.
>> Maybe move it up or look it up locally?
>>
>>
>> Third, the cmake branch:
>>> foreach d : ssl.get_variable(cmake: 'OPENSSL_INCLUDE_DIR',
>>> default_value: '').split(';')
>>
>> Are OPENSSL_INCLUDE_DIR and ICU_INCLUDE_DIRS valid for
>> both Find-modules and upstream config files? With default_value: ''
>> a wrong name just means empty flags. Same for find_library deps --
>> I agree extra_include_dirs is a separate gap, but a short comment would help.
>>
>> One question, not a blocker: with this split, pg_config --cppflags still misses ICU,
>> unlike autoconf. PGXS make is fine through $(ICU_CFLAGS), but direct
>> pg_config users are not. Is that an acceptable divergence, or should ICU
>> go to var_cppflags as well?
>>
>> Btw the fake .pc test idea sounds good as a follow-up! No need to block v3 on it from my side.
>> Thanks again for picking this up!
>>
>> Kind regards,
>> Yuriy
>
> Hi Yuriy,
>
> Thanks for the careful review. v3 attached.
>
>> Could we check returncode(), use native: true, and query
>> "icu-uc icu-i18n" together like configure does?
>
> returncode() is checked now, and a failure is a hard error carrying
> pkg-config's stderr, same as python_includespec effectively does. ICU is
> queried as "icu-uc icu-i18n" in one call. While there I made the call
> mirror Meson's own pkg-config invocation: it sets PKG_CONFIG_PATH from
> the pkg_config_path option, and PKG_CONFIG_SYSROOT_DIR /
> PKG_CONFIG_LIBDIR from the sys_root / pkg_config_libdir properties.
> v2 silently used the shell environment instead, so with
> -Dpkg_config_path=... it could query a different .pc than Meson used.
> This also covers the sysroot concern from your first review.
>
> I did not use native: true, though. Meson looks up pkg-config for host
> dependencies on the host machine (cross-file entry, then PKG_CONFIG,
> then PATH). native: true would skip the cross-file entry and query the
> build machine's .pc files. The default find_program() lookup is the
> same one Meson uses.
>
>> pkg_config is defined in one file but used in another
>
> All flag extraction now lives in one block in the top-level meson.build,
> after all dependencies are resolved. src/include and src/makefiles only
> consume the result.
>
>> Are OPENSSL_INCLUDE_DIR and ICU_INCLUDE_DIRS valid for both
>> Find-modules and upstream config files?
>
> Good catch, and the answer was "no" for OpenSSL. Meson's CMake trace
> only records set() calls, and FindOpenSSL assigns OPENSSL_INCLUDE_DIR
> via find_path() only, so in the default module mode v2 always produced
> empty flags. Only OpenSSL's upstream OpenSSLConfig.cmake set()s it.
> v3 uses PACKAGE_INCLUDE_DIRS instead, which Meson's CMake wrapper
> derives from <Name>_INCLUDE_DIRS / _INCLUDES / _INCLUDE_DIR. I checked
> it against FindOpenSSL, OpenSSLConfig.cmake and FindICU (ICU ships no
> config files). It's internal to Meson, but has been there since at
> least 0.57.2. If it ever comes up empty, setup emits a warning instead
> of silently producing nothing. A comment now explains the 'system'
> method (headers already on the default path) and the find_library /
> extra_include_dirs gap.
>
>> should ICU go to var_cppflags as well?
>
> pg_config --cppflags now includes ICU, for parity with autoconf. It's
> added to VAL_CPPFLAGS only, not Makefile.global's CPPFLAGS, so PGXS
> doesn't get it twice. One note: since af2d4ca191a no installed header
> includes ICU anymore, so on master the ICU part is about autoconf
> parity and extensions using ICU themselves.
>
>
> Regards,
> Aidar Imamov
>
> <v3-0001-meson-Expose-OpenSSL-and-ICU-compiler-flags-to-PG.patch>

Hi again,

v4 attached. It fixes the configure failure CFBot reported for v3
("environment.set takes at least 2 arguments, but got 1"). I passed
get_option('pkg_config_path') to environment.set() unconditionally,
and that option is empty when PKG_CONFIG_PATH is not set, which is why
it worked in my shell but not in CI. It is now only set when
non-empty, like the sys_root / pkg_config_libdir handling next to it.
Verified with Meson 1.7.0 and 0.57.2 (current min), with and without
PKG_CONFIG_PATH, and with the configure options CFBot uses.

Apart from that, v4 only makes the new block easier to read: the list
of dependencies is now a named table with named fields ('deps',
'pc_modules') instead of positional lists, and the comments are
shorter. Nothing else changed from v3.

Since v3 looked quite different from v2, a word on why the code moved
into one block in the top-level meson.build instead of staying split
between src/include and src/makefiles:

- With ICU added to pg_config --cppflags, the ICU flags are needed
when VAL_CPPFLAGS is built in src/common, which is processed before
src/makefiles. So the ICU part could not stay where v2 had it.

- With the returncode() check, pkg-config has to see the same
PKG_CONFIG_PATH as Meson. Otherwise a build configured with only
-Dpkg_config_path would now fail at setup, where v2 silently
produced empty flags. That environment setup is shared by OpenSSL
and ICU, as is the pkg_config lookup you asked to move, so keeping
everything in one place avoids duplicating it.

The approach itself is unchanged from v2: ask pkg-config or CMake for
the flags, put OpenSSL into CPPFLAGS and ICU into ICU_CFLAGS. Adding
another dependency is one more entry in the table.

Regards,
Aidar Imamov

Attachment Content-Type Size
v4-0001-meson-expose-OpenSSL-and-ICU-compiler-flags-to-PG.patch application/octet-stream 7.3 KB
unknown_filename text/plain 1 byte

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Zsolt Parragi 2026-10-14 16:50:41 Re: pgstat: allow a stats kind to use its own dedicated dsa/dshash