Re: PG_TEST_EXTRA and meson

Hi Nazir,

On Fri, Jul 19, 2024 at 1:37 PM Nazir Bilal Yavuz <byavuz81(at)gmail(dot)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].

Thanks.

>
> The one downside of this approach is that PG_TEXT_EXTRA in user
> defined options in meson setup output could be misleading now.
>

Upthread Tom asked whether we should do a symmetric change to "make".
This set of patches does not do that. Here are my thoughts:
1. Those who use make, are not using configure time PG_TEST_EXTRA
anyway, so they don't need it.
2. Those meson users who use setup time PG_TEST_EXTRA and also want to
use make may find the need for the feature in make.
3. https://www.postgresql.org/docs/devel/install-requirements.html
says that the meson support is currently experimental and only works
when building from a Git checkout. So there's a possibility (even if
theoretical) that make and meson will co-exist. Also that we may
abandon meson?

Considering those, it seems to me that symmetry is required. I don't
know how hard it is to introduce PG_TEST_EXTRA as a configure time
option in "make". If it's simple, we should do that. Otherwise it will
be better to just remove PG_EXTRA_TEST option support from meson
support to keep make and meson symmetrical.

As far as the implementation is concerned the patch seems to be doing
what's expected. If PG_TEST_EXTRA is specified at setup time, it is
not needed to be specified as an environment variable at run time. But
it can also be overridden at runtime. If PG_TEST_EXTRA is not
specified at the time of setup, but specified at run time, it works. I
have tested xid_wraparound and wal_consistency_check.

I wonder whether we really require pg_test_extra argument to testwrap.
Why can't we use the logic in testwrap, to set run time PG_TEST_EXTRA,
in meson.build directly? I.e. set test_env['PG_TEST_EXTRA'] to
os.environ[;PG_TEST_EXTRA'] if the latter is set, otherwise set the
first to get_option('PG_TEST_EXTRA').

> 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.

I think this is useful and allows both make and meson to use the same
logic in cirrus.

--
Best Wishes,
Ashutosh Bapat