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
