Hi,
Thank you for looking into this!
On Wed, 2 Jul 2025 at 14:33, Peter Eisentraut <pe...@eisentraut.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
From b8846511c07c7008e742de42b954b67b25a6194b Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Wed, 2 Jul 2025 18:07:51 +0300
Subject: [PATCH v2 1/3] ci: Remove PG_TEST_EXTRA from NetBSD and OpenBSD
PG_TEST_EXTRA environment variable is already set at the top level.
NetBSD and OpenBSD tasks will use this top level PG_TEST_EXTRA as
default.
Discussion: https://www.postgresql.org/message-id/flat/CAN55FZ0aO8d_jkyRijcGP8qO%3DXH09qG%3Dpw0ZZDvB4LMzuXYU1w%40mail.gmail.com
---
.cirrus.tasks.yml | 1 -
1 file changed, 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 92057006c93..a7f75c5b59f 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -318,7 +318,6 @@ task:
--pkg-config-path ${PKGCONFIG_PATH} \
-Dcassert=true -Dinjection_points=true \
-Dssl=openssl ${UUID} ${TCL} \
- -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
EOF
--
2.49.0
From 89f786fd408179f77b5a94a079cb2826f7dc0c6d Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Wed, 25 Jun 2025 11:14:26 +0300
Subject: [PATCH v2 2/3] ci: meson: Explicitly enable meson features
By default, Meson silently disables features it cannot detect, which can
lead to incomplete builds. Explicitly enabling these features causes the
build to fail if they are missing, making such issues visible early.
Please note that, libcurl is disabled for OpenBSD until the fix in the
https://www.postgresql.org/message-id/CAOYmi%2BkdR218ke2zu74oTJvzYJcqV1MN5%3DmGAPqZQuc79HMSVA%40mail.gmail.com
is committed.
Suggested-by: Jacob Champion <jacob.champ...@enterprisedb.com>
Reviewed-by: Peter Eisentraut <pe...@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/CAN55FZ0aO8d_jkyRijcGP8qO%3DXH09qG%3Dpw0ZZDvB4LMzuXYU1w%40mail.gmail.com
---
.cirrus.tasks.yml | 107 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 89 insertions(+), 18 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index a7f75c5b59f..ca679025a8f 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -31,6 +31,29 @@ env:
TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption load_balance oauth
+ # Meson feature flags, shared between all meson tasks except the
+ # 'SanityCheck' task
+ MESON_COMMON_FEATURES: >-
+ -Dauto_features=disabled
+ -Dldap=enabled
+ -Dssl=openssl
+ -Dtap_tests=enabled
+ -Dplperl=enabled
+ -Dplpython=enabled
+
+ # Like 'MESON_COMMON_FEATURES' but not shared with 'Windows - VS' task too
+ MESON_NON_VS_FEATURES: >-
+ -Ddocs=enabled
+ -Dicu=enabled
+ -Dlibxml=enabled
+ -Dlibxslt=enabled
+ -Dlz4=enabled
+ -Dpltcl=enabled
+ -Dreadline=enabled
+ -Dzlib=enabled
+ -Dzstd=enabled
+ MESON_COMMON_NON_VS_FEATURES: "${MESON_COMMON_FEATURES} ${MESON_NON_VS_FEATURES}"
+
# What files to preserve in case tests fail
on_failure_ac: &on_failure_ac
@@ -164,6 +187,15 @@ task:
-c debug_parallel_query=regress
PG_TEST_PG_UPGRADE_MODE: --link
+ MESON_FEATURES: >-
+ -Ddtrace=enabled
+ -Dgssapi=enabled
+ -Dlibcurl=enabled
+ -Dnls=enabled
+ -Dpam=enabled
+ -Dtcl_version=tcl86
+ -Duuid=bsd
+
<<: *freebsd_task_template
depends_on: SanityCheck
@@ -198,8 +230,8 @@ task:
meson setup \
--buildtype=debug \
-Dcassert=true -Dinjection_points=true \
- -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
+ ${MESON_COMMON_NON_VS_FEATURES} ${MESON_FEATURES} \
build
EOF
build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS} ${MBUILD_TARGET}'
@@ -267,8 +299,15 @@ task:
# See https://postgr.es/m/2490325.1734471752%40sss.pgh.pa.us
LANG: "C"
LC_ALL: "C"
+
# -Duuid is not set for the NetBSD, see the comment below, above
# configure_script, for more information.
+ MESON_FEATURES: >-
+ -Dgssapi=enabled
+ -Dlibcurl=enabled
+ -Dnls=enabled
+ -Dpam=enabled
+
setup_additional_packages_script: |
#pkgin -y install ...
<<: *netbsd_task_template
@@ -279,8 +318,16 @@ task:
OS_NAME: openbsd
IMAGE_FAMILY: pg-ci-openbsd-postgres
PKGCONFIG_PATH: '/usr/lib/pkgconfig:/usr/local/lib/pkgconfig'
- UUID: -Duuid=e2fs
- TCL: -Dtcl_version=tcl86
+
+ # libcurl is disabled until the fix in the
+ # https://www.postgresql.org/message-id/CAOYmi%2BkdR218ke2zu74oTJvzYJcqV1MN5%3DmGAPqZQuc79HMSVA%40mail.gmail.com
+ # is committed.
+ MESON_FEATURES: >-
+ -Dbsd_auth=enabled
+ -Dlibcurl=disabled
+ -Dtcl_version=tcl86
+ -Duuid=e2fs
+
setup_additional_packages_script: |
#pkg_add -I ...
# Always core dump to ${CORE_DUMP_DIR}
@@ -317,7 +364,7 @@ task:
--buildtype=debugoptimized \
--pkg-config-path ${PKGCONFIG_PATH} \
-Dcassert=true -Dinjection_points=true \
- -Dssl=openssl ${UUID} ${TCL} \
+ ${MESON_COMMON_NON_VS_FEATURES} ${MESON_FEATURES} \
build
EOF
@@ -364,10 +411,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
--with-uuid=ossp
--with-zstd
-LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
- -Dllvm=enabled
- -Duuid=e2fs
-
# Check SPECIAL in the matrix: below
task:
@@ -408,7 +451,6 @@ task:
LLVM_CONFIG: llvm-config-16
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
- LINUX_MESON_FEATURES: *LINUX_MESON_FEATURES
<<: *linux_task_template
@@ -502,12 +544,37 @@ task:
PG_TEST_INITDB_EXTRA_OPTS: >-
-c io_method=io_uring
+ LINUX_64_MESON_FEATURES: >-
+ -Ddocs_pdf=enabled
+ -Dgssapi=enabled
+ -Dlibcurl=enabled
+ -Dlibnuma=enabled
+ -Dliburing=enabled
+ -Dllvm=enabled
+ -Dnls=enabled
+ -Dpam=enabled
+ -Dplperl=enabled
+ -Dselinux=enabled
+ -Dsystemd=enabled
+ -Duuid=e2fs
+
+ LINUX_32_MESON_FEATURES: >-
+ -Ddocs_pdf=enabled
+ -Dgssapi=enabled
+ -Dlibcurl=enabled
+ -Dliburing=enabled
+ -Dnls=enabled
+ -Dpam=enabled
+ -Dselinux=enabled
+ -Dsystemd=enabled
+ -Duuid=e2fs
+
configure_script: |
su postgres <<-EOF
meson setup \
--buildtype=debug \
-Dcassert=true -Dinjection_points=true \
- ${LINUX_MESON_FEATURES} \
+ ${MESON_COMMON_NON_VS_FEATURES} ${LINUX_64_MESON_FEATURES} \
build
EOF
@@ -519,11 +586,9 @@ task:
meson setup \
--buildtype=debug \
-Dcassert=true -Dinjection_points=true \
- ${LINUX_MESON_FEATURES} \
- -Dllvm=disabled \
--pkg-config-path /usr/lib/i386-linux-gnu/pkgconfig/ \
-DPERL=perl5.36-i386-linux-gnu \
- -Dlibnuma=disabled \
+ ${MESON_COMMON_NON_VS_FEATURES} ${LINUX_32_MESON_FEATURES} \
build-32
EOF
@@ -587,6 +652,14 @@ task:
CCACHE_DIR: ${HOME}/ccache
MACPORTS_CACHE: ${HOME}/macports-cache
+ MESON_FEATURES: >-
+ -Dbonjour=enabled
+ -Ddtrace=enabled
+ -Dgssapi=enabled
+ -Dlibcurl=enabled
+ -Dnls=enabled
+ -Duuid=e2fs
+
MACOS_PACKAGE_LIST: >-
ccache
icu
@@ -660,7 +733,7 @@ task:
-Dextra_include_dirs=/opt/local/include \
-Dextra_lib_dirs=/opt/local/lib \
-Dcassert=true -Dinjection_points=true \
- -Duuid=e2fs -Ddtrace=auto \
+ ${MESON_COMMON_NON_VS_FEATURES} ${MESON_FEATURES} \
build
build_script: ninja -C build -j${BUILD_JOBS} ${MBUILD_TARGET}
@@ -732,7 +805,7 @@ task:
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
configure_script: |
vcvarsall x64
- meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Dinjection_points=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% build
+ meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Dinjection_points=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% %MESON_COMMON_FEATURES% build
build_script: |
vcvarsall x64
@@ -792,7 +865,7 @@ task:
# disable -Dnls as the number of files it creates cause a noticable slowdown
configure_script: |
- %BASH% -c "meson setup -Ddebug=true -Doptimization=g -Dcassert=true -Dinjection_points=true -Db_pch=true -Dnls=disabled -DTAR=%TAR% build"
+ %BASH% -c "meson setup -Ddebug=true -Doptimization=g -Dcassert=true -Dinjection_points=true -Db_pch=true %MESON_COMMON_NON_VS_FEATURES% -DTAR=%TAR% build"
build_script: |
%BASH% -c "ninja -C build ${MBUILD_TARGET}"
@@ -830,8 +903,6 @@ task:
CCACHE_DIR: "/tmp/ccache_dir"
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
- LINUX_MESON_FEATURES: *LINUX_MESON_FEATURES
-
# GCC emits a warning for llvm-14, so switch to a newer one.
LLVM_CONFIG: llvm-config-16
--
2.49.0
From 16831a93f5f4e51175ba028ebe65a432606e781c Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Wed, 2 Jul 2025 17:30:59 +0300
Subject: [PATCH v2 3/3] ci: meson: Store common Postgres configuration options
in one variable
This helps to keep things aligned.
Suggested-by: Peter Eisentraut <pe...@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/CAN55FZ0aO8d_jkyRijcGP8qO%3DXH09qG%3Dpw0ZZDvB4LMzuXYU1w%40mail.gmail.com
---
.cirrus.tasks.yml | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index ca679025a8f..da1959984f1 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -31,6 +31,10 @@ env:
TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption load_balance oauth
+ # Postgres config args for the meson builds, shared between all meson tasks
+ # except the 'SanityCheck' task
+ MESON_COMMON_PG_CONFIG_ARGS: -Dcassert=true -Dinjection_points=true
+
# Meson feature flags, shared between all meson tasks except the
# 'SanityCheck' task
MESON_COMMON_FEATURES: >-
@@ -228,8 +232,8 @@ task:
configure_script: |
su postgres <<-EOF
meson setup \
+ ${MESON_COMMON_PG_CONFIG_ARGS} \
--buildtype=debug \
- -Dcassert=true -Dinjection_points=true \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
${MESON_COMMON_NON_VS_FEATURES} ${MESON_FEATURES} \
build
@@ -361,9 +365,9 @@ task:
configure_script: |
su postgres <<-EOF
meson setup \
+ ${MESON_COMMON_PG_CONFIG_ARGS} \
--buildtype=debugoptimized \
--pkg-config-path ${PKGCONFIG_PATH} \
- -Dcassert=true -Dinjection_points=true \
${MESON_COMMON_NON_VS_FEATURES} ${MESON_FEATURES} \
build
EOF
@@ -572,8 +576,8 @@ task:
configure_script: |
su postgres <<-EOF
meson setup \
+ ${MESON_COMMON_PG_CONFIG_ARGS} \
--buildtype=debug \
- -Dcassert=true -Dinjection_points=true \
${MESON_COMMON_NON_VS_FEATURES} ${LINUX_64_MESON_FEATURES} \
build
EOF
@@ -584,8 +588,8 @@ task:
su postgres <<-EOF
export CC='ccache gcc -m32'
meson setup \
+ ${MESON_COMMON_PG_CONFIG_ARGS} \
--buildtype=debug \
- -Dcassert=true -Dinjection_points=true \
--pkg-config-path /usr/lib/i386-linux-gnu/pkgconfig/ \
-DPERL=perl5.36-i386-linux-gnu \
${MESON_COMMON_NON_VS_FEATURES} ${LINUX_32_MESON_FEATURES} \
@@ -729,10 +733,10 @@ task:
configure_script: |
export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/"
meson setup \
+ ${MESON_COMMON_PG_CONFIG_ARGS} \
--buildtype=debug \
-Dextra_include_dirs=/opt/local/include \
-Dextra_lib_dirs=/opt/local/lib \
- -Dcassert=true -Dinjection_points=true \
${MESON_COMMON_NON_VS_FEATURES} ${MESON_FEATURES} \
build
@@ -805,7 +809,7 @@ task:
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
configure_script: |
vcvarsall x64
- meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Dinjection_points=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% %MESON_COMMON_FEATURES% build
+ meson setup --backend ninja %MESON_COMMON_PG_CONFIG_ARGS% --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% %MESON_COMMON_FEATURES% build
build_script: |
vcvarsall x64
@@ -865,7 +869,7 @@ task:
# disable -Dnls as the number of files it creates cause a noticable slowdown
configure_script: |
- %BASH% -c "meson setup -Ddebug=true -Doptimization=g -Dcassert=true -Dinjection_points=true -Db_pch=true %MESON_COMMON_NON_VS_FEATURES% -DTAR=%TAR% build"
+ %BASH% -c "meson setup %MESON_COMMON_PG_CONFIG_ARGS% -Ddebug=true -Doptimization=g -Db_pch=true %MESON_COMMON_NON_VS_FEATURES% -DTAR=%TAR% build"
build_script: |
%BASH% -c "ninja -C build ${MBUILD_TARGET}"
--
2.49.0
