Hi,
On Wed, 17 Jul 2024 at 13:13, Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
>
> If you are willing to work on this further, please add it to the commitfest.
Since general consensus is more towards having an environment variable
to override Meson configure option, I converted solution-3 to
something more like a patch. I updated the docs, added more comments
and added this to the commitfest [1].
The one downside of this approach is that PG_TEXT_EXTRA in user
defined options in meson setup output could be misleading now.
Also, with this change; PG_TEST_EXTRA from configure_scripts in the
.cirrus.tasks.yml file should be removed as they are useless now. I
added that as a second patch.
[1] https://commitfest.postgresql.org/49/5134/
--
Regards,
Nazir Bilal Yavuz
Microsoft
From 7e6c31c73d14f8e50d13ad7ce4aa1aa167193afd Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Fri, 19 Jul 2024 09:48:47 +0300
Subject: [PATCH v1 1/2] Make PG_TEST_EXTRA env variable override its Meson
configure option
Since PG_TEST_EXTRA configure option is set while configuring Meson,
each change on PG_TEST_EXTRA needs reconfigure and this is costly. So,
first try to use PG_TEST_EXTRA from environment. If it does not exist,
then try to use it from its configure option in Meson builds.
---
doc/src/sgml/installation.sgml | 6 ++++--
meson.build | 11 ++++++-----
meson_options.txt | 2 +-
src/tools/testwrap | 6 ++++++
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 4784834ab9f..a78ef8cb78b 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -3132,8 +3132,10 @@ ninja install
<listitem>
<para>
Enable test suites which require special software to run. This option
- accepts arguments via a whitespace-separated list. See <xref
- linkend="regress-additional"/> for details.
+ accepts arguments via a whitespace-separated list. Please note that
+ this configure option is overridden by PG_TEST_EXTRA environment
+ variable if it exists. See <xref linkend="regress-additional"/> for
+ details.
</para>
</listitem>
</varlistentry>
diff --git a/meson.build b/meson.build
index 5387bb6d5fd..e0b04b01756 100644
--- a/meson.build
+++ b/meson.build
@@ -3224,11 +3224,6 @@ test_env.set('PG_REGRESS', pg_regress.full_path())
test_env.set('REGRESS_SHLIB', regress_module.full_path())
test_env.set('INITDB_TEMPLATE', test_initdb_template)
-# Test suites that are not safe by default but can be run if selected
-# by the user via the whitespace-separated list in variable PG_TEST_EXTRA.
-# Export PG_TEST_EXTRA so it can be checked in individual tap tests.
-test_env.set('PG_TEST_EXTRA', get_option('PG_TEST_EXTRA'))
-
# Add the temporary installation to the library search path on platforms where
# that works (everything but windows, basically). On windows everything
# library-like gets installed into bindir, solving that issue.
@@ -3292,6 +3287,12 @@ foreach test_dir : tests
testwrap,
'--basedir', meson.build_root(),
'--srcdir', test_dir['sd'],
+ # Test suites that are not safe by default but can be run if selected
+ # by the user via the whitespace-separated list in variable PG_TEST_EXTRA.
+ # Export PG_TEST_EXTRA so it can be checked in individual tap tests.
+ # This configure option is overridden by PG_TEST_EXTRA environment variable
+ # if it exists.
+ '--pg_test_extra', get_option('PG_TEST_EXTRA'),
]
foreach kind, v : test_dir
diff --git a/meson_options.txt b/meson_options.txt
index 246cecf3827..e1d55311702 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -47,7 +47,7 @@ option('injection_points', type: 'boolean', value: false,
description: 'Enable injection points')
option('PG_TEST_EXTRA', type: 'string', value: '',
- description: 'Enable selected extra tests')
+ description: 'Enable selected extra tests, please note that this configure option is overridden by PG_TEST_EXTRA environment variable if it exists')
option('atomics', type: 'boolean', value: true,
description: 'Use atomic operations')
diff --git a/src/tools/testwrap b/src/tools/testwrap
index 9a270beb72d..0dca4c26f2b 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -13,6 +13,7 @@ parser.add_argument('--basedir', help='base directory of test', type=str)
parser.add_argument('--testgroup', help='test group', type=str)
parser.add_argument('--testname', help='test name', type=str)
parser.add_argument('--skip', help='skip test (with reason)', type=str)
+parser.add_argument('--pg_test_extra', help='extra tests', type=str)
parser.add_argument('test_command', nargs='*')
args = parser.parse_args()
@@ -41,6 +42,11 @@ env_dict = {**os.environ,
'TESTDATADIR': os.path.join(testdir, 'data'),
'TESTLOGDIR': os.path.join(testdir, 'log')}
+# If PG_TEST_EXTRA is not set in the environment, then look for its Meson
+# configure option.
+if "PG_TEST_EXTRA" not in os.environ and args.pg_test_extra:
+ env_dict["PG_TEST_EXTRA"] = args.pg_test_extra
+
sp = subprocess.Popen(args.test_command, env=env_dict, stdout=subprocess.PIPE)
# Meson categorizes a passing TODO test point as bad
# (https://github.com/mesonbuild/meson/issues/13183). Remove the TODO
--
2.45.2
From 9e3de3b53943501c0aa60f4f1f84802451519982 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Fri, 19 Jul 2024 09:59:40 +0300
Subject: [PATCH v1 2/2] Remove PG_TEST_EXTRA from configure_scripts in
.cirrus.tasks.yml
Meson builds are able to get PG_TEST_EXTRA from environment now and
this overrides the configure option. PG_TEST_EXTRA environment variable
is set at the top of the .cirrus.tasks.yml file. So, PG_TEXT_EXTRA in
configure scripts became useless and were removed because of that.
---
.cirrus.tasks.yml | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 99ca74d5133..8449addbdcf 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -174,7 +174,6 @@ task:
--buildtype=debug \
-Dcassert=true -Dinjection_points=true \
-Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
- -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
EOF
@@ -362,7 +361,6 @@ task:
--buildtype=debug \
-Dcassert=true -Dinjection_points=true \
${LINUX_MESON_FEATURES} \
- -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
EOF
@@ -378,7 +376,6 @@ task:
-Dllvm=disabled \
--pkg-config-path /usr/lib/i386-linux-gnu/pkgconfig/ \
-DPERL=perl5.36-i386-linux-gnu \
- -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build-32
EOF
@@ -492,7 +489,6 @@ task:
-Dextra_lib_dirs=/opt/local/lib \
-Dcassert=true -Dinjection_points=true \
-Duuid=e2fs -Ddtrace=auto \
- -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
build_script: ninja -C build -j${BUILD_JOBS}
@@ -564,7 +560,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% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" 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% build
build_script: |
vcvarsall x64
--
2.45.2
