Re: Explicitly enable meson features in CI

Hi,

Thank you for looking into this!

On Wed, 2 Jul 2025 at 14:33, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 02.07.25 09:22, Nazir Bilal Yavuz wrote:
> > One thing I’m unsure about the patch is that all these features are
> > stored in the MESON_FEATURES environment variable in each task. I
> > wonder if it might be clearer to rename these variables to
> > ${TASK_NAME}_MESON_FEATURES to avoid confusion.
>
> I would hope we could do this without so much repetition. Why not a
> global variable and then have each task override as needed?
>
> (It would also be nice to extra the corresponding configure arguments
> into a similar variable, so we can easily keep this aligned.)

Common configure arguments are '-Dcassert=true
-Dinjection_points=true', I addressed that in the 0003.

> Here is a sketch of what I mean:
>
> env:
> ...
> PG_TEST_EXTRA: ...
> MESON_COMMON_ARGS: -Dauto_features=disabled -Ddocs=enabled ...
> CONFIGURE_COMMON_ARGS: --with-gssapi --with-icu --with-ldap ...

I agree that this repetition does not look good but my idea was to be
able to see which features are enabled at one glance. I tried to apply
grouping in v2:

1) args that are shared between all meson tasks except SanityCheck
are: auto_features, ldap, ssl=openssl, tap_tests, plperl, plpython
2) args that are shared between all meson tasks except SanityCheck and
Windows VS task are: docs, icu, libxml, libxslt, lz4, pltcl, readline,
zlib, zstd

I stored #1 in the MESON_COMMON_FEATURES variable and #2 in the
MESON_NON_VS_FEATURES variable. Then merged #1 and #2 in the
MESON_COMMON_NON_VS_FEATURES variable. So it is like that:

- All meson tasks excluding the Windows VS and SanityCheck tasks use
MESON_COMMON_NON_VS_FEATURES and overrides it.
- Windows VS task uses MESON_COMMON_FEATURES.

>
> ...
>
> configure_script: |
> su postgres <<-EOF
> meson setup \
> ${MESON_COMMON_ARGS} -Dpltcl=disabled \
> --buildtype=debugoptimized \
> --pkg-config-path ${PKGCONFIG_PATH} \
> -Dcassert=true -Dinjection_points=true \
> -Dssl=openssl ${UUID} ${TCL} \
> -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
> build
> EOF
>
> (There are additional opportunities to refactor this, such as the
> -DPG_TEST_EXTRA option that is repeated everywhere.)

-DPG_TEST_EXTRA option is used only in the NetBSD and OpenBSD tasks
but it is unnecessary. They can use the top level PG_TEST_EXTRA
environment variable. This is fixed in 0001.

To sum up:

0001 is for removing unnecessary PG_TEST_EXTRA from the NetBSD and
OpenBSD tasks.
0002 is the actual patch but this time I tried to address that
repetition problem by storing common meson features in variables.
0003 is storing common configure arguments ('-Dcassert=true
-Dinjection_points=true') in one variable.

--
Regards,
Nazir Bilal Yavuz
Microsoft