> On Oct 14, 2026, at 18:08, Aidar Imamov <[email protected]> wrote: > > > >> On Sep 30, 2026, at 07:53, Grigorev Jurij <[email protected]> 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
v4-0001-meson-expose-OpenSSL-and-ICU-compiler-flags-to-PG.patch
Description: Binary data
