Обсуждение: PG_TEST_EXTRA and meson

Поиск
Список
Период
Сортировка

PG_TEST_EXTRA and meson

От
Ashutosh Bapat
Дата:
Hi All,
Using PG_TEST_EXTRA with make is simple, one just sets that environment variable
$ make check
... snip ...

PG_REGRESS='/home/ashutosh/work/units/pghead_make/coderoot/pg/src/test/modules/xid_wraparound/../../../../src/test/regress/pg_regress'
/usr/bin/prove -I ../../../../src/test/perl/ -I .  t/*.pl
# +++ tap check in src/test/modules/xid_wraparound +++
t/001_emergency_vacuum.pl .. skipped: test xid_wraparound not enabled
in PG_TEST_EXTRA
t/002_limits.pl ............ skipped: test xid_wraparound not enabled
in PG_TEST_EXTRA
t/003_wraparounds.pl ....... skipped: test xid_wraparound not enabled
in PG_TEST_EXTRA
Files=3, Tests=0,  1 wallclock secs ( 0.02 usr  0.00 sys +  0.20 cusr
0.03 csys =  0.25 CPU)
Result: NOTESTS

Set PG_TEST_EXTRA
$ PG_TEST_EXTRA=xid_wraparound make check

PG_REGRESS='/home/ashutosh/work/units/pghead_make/coderoot/pg/src/test/modules/xid_wraparound/../../../../src/test/regress/pg_regress'
/usr/bin/prove -I ../../../../src/test/perl/ -I .  t/*.pl
# +++ tap check in src/test/modules/xid_wraparound +++
t/001_emergency_vacuum.pl .. ok
t/002_limits.pl ............ ok
t/003_wraparounds.pl ....... ok
All tests successful.
Files=3, Tests=11, 181 wallclock secs ( 0.03 usr  0.00 sys +  2.87
cusr  3.10 csys =  6.00 CPU)
Result: PASS

But this simple trick does not work with meson
$ meson test  -C $BuildDir --suite setup --suite xid_wraparound
ninja: Entering directory `/home/ashutosh/work/units/pg_review/build_dev'
ninja: no work to do.
1/6 postgresql:setup / tmp_install
    OK                0.85s
2/6 postgresql:setup / install_test_files
    OK                0.06s
3/6 postgresql:setup / initdb_cache
    OK                1.57s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    SKIP              0.24s
5/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    SKIP              0.26s
6/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    SKIP              0.27s

$ PG_TEST_EXTRA=xid_wraparound meson test  -C $BuildDir --suite setup
--suite xid_wraparound
ninja: Entering directory `/home/ashutosh/work/units/pg_review/build_dev'
ninja: no work to do.
1/6 postgresql:setup / tmp_install
    OK                0.41s
2/6 postgresql:setup / install_test_files
    OK                0.06s
3/6 postgresql:setup / initdb_cache
    OK                1.57s
4/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    SKIP              0.20s
5/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    SKIP              0.24s
6/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    SKIP              0.24s

the tests are still skipped.

In order to run these tests, we have to run meson setup again. There
are couple of problems with this
1. It's not clear why the tests were skipped. Also not clear that we
have to run meson setup again - from the output alone
2. Running meson setup again is costly, every time we have to run a
new test from PG_TEST_EXTRA.
3. Always configuring meson with PG_TEST_EXTRA means we will run heavy
tests every time meson test is run. I didn't find a way to not run
these tests as part of meson test once configured this way.

We should either allow make like behaviour or provide a way to not run
these tests even if configured that way.

-- 
Best Wishes,
Ashutosh Bapat



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Thu, 11 Jul 2024 at 09:30, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> In order to run these tests, we have to run meson setup again. There
> are couple of problems with this
> 1. It's not clear why the tests were skipped. Also not clear that we
> have to run meson setup again - from the output alone
> 2. Running meson setup again is costly, every time we have to run a
> new test from PG_TEST_EXTRA.
> 3. Always configuring meson with PG_TEST_EXTRA means we will run heavy
> tests every time meson test is run. I didn't find a way to not run
> these tests as part of meson test once configured this way.
>
> We should either allow make like behaviour or provide a way to not run
> these tests even if configured that way.

I have a two quick solutions to this:

1- More like make behaviour. PG_TEST_EXTRA is able to be set from the
setup command, delete this feature so it could be set only from the
environment. Then use it from the environment.

2- If PG_TEST_EXTRA is set from the setup command, use it from the
setup command and discard the environment variable. If PG_TEST_EXTRA
is not set from the setup command, then use it from the environment.

I hope these patches help.

-- 
Regards,
Nazir Bilal Yavuz
Microsoft

Вложения

Re: PG_TEST_EXTRA and meson

От
Jacob Champion
Дата:
On Tue, Jul 16, 2024 at 2:12 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> 2- If PG_TEST_EXTRA is set from the setup command, use it from the
> setup command and discard the environment variable. If PG_TEST_EXTRA
> is not set from the setup command, then use it from the environment.

Is there a way for the environment to override the Meson setting
rather than vice-versa? My vote would be to have both available, but
with the implementation in patch 2 I'd still have to reconfigure if I
wanted to change my test setup.

Thanks!
--Jacob



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Wed, 17 Jul 2024 at 00:27, Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
>
> On Tue, Jul 16, 2024 at 2:12 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> >
> > 2- If PG_TEST_EXTRA is set from the setup command, use it from the
> > setup command and discard the environment variable. If PG_TEST_EXTRA
> > is not set from the setup command, then use it from the environment.
>
> Is there a way for the environment to override the Meson setting
> rather than vice-versa? My vote would be to have both available, but
> with the implementation in patch 2 I'd still have to reconfigure if I
> wanted to change my test setup.

I think something like attached does the trick. I did not test it
extensively but it passed the couple of tests I tried.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Вложения

Re: PG_TEST_EXTRA and meson

От
Ashutosh Bapat
Дата:
On Wed, Jul 17, 2024 at 3:31 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> Hi,
>
> On Wed, 17 Jul 2024 at 00:27, Jacob Champion
> <jacob.champion@enterprisedb.com> wrote:
> >
> > On Tue, Jul 16, 2024 at 2:12 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> > >
> > > 2- If PG_TEST_EXTRA is set from the setup command, use it from the
> > > setup command and discard the environment variable. If PG_TEST_EXTRA
> > > is not set from the setup command, then use it from the environment.
> >
> > Is there a way for the environment to override the Meson setting
> > rather than vice-versa? My vote would be to have both available, but
> > with the implementation in patch 2 I'd still have to reconfigure if I
> > wanted to change my test setup.
>
> I think something like attached does the trick. I did not test it
> extensively but it passed the couple of tests I tried.

Thanks a lot for working on this.

I tested this patch with xid_wraparound. It seems to be working.
$ mts xid_wraparound
... snip
1/6 postgresql:setup / tmp_install
    OK                0.36s
2/6 postgresql:setup / install_test_files
    OK                0.10s
3/6 postgresql:setup / initdb_cache
    OK                1.14s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    SKIP              0.14s
5/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    SKIP              0.14s
6/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    SKIP              0.14s

... snip

$ PG_TEST_EXTRA=xid_wraparound mts xid_wraparound
... snip
1/6 postgresql:setup / tmp_install
    OK                0.38s
2/6 postgresql:setup / install_test_files
    OK                0.07s
3/6 postgresql:setup / initdb_cache
    OK                1.13s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    OK               67.33s   7 subtests passed
5/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    OK               70.14s   3 subtests passed
6/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    OK              178.01s   1 subtests passed

... snip

$ mts xid_wraparound
... snip
1/6 postgresql:setup / tmp_install
    OK                0.36s
2/6 postgresql:setup / install_test_files
    OK                0.06s
3/6 postgresql:setup / initdb_cache
    OK                1.14s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    SKIP              0.18s
5/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    SKIP              0.19s
6/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    SKIP              0.19s

... snip

Providing PG_TEST_EXTRA as a configuration option works as well. But I
find it confusing
$ mts xid_wraparound
... snip
1/6 postgresql:setup / tmp_install
    OK                0.71s
2/6 postgresql:setup / install_test_files
    OK                0.06s
3/6 postgresql:setup / initdb_cache
    OK                1.08s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    OK               52.73s   7 subtests passed
5/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    OK               56.36s   3 subtests passed
6/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    OK              160.46s   1 subtests passed
... snip

$ PG_TEST_EXTRA=ldap mts xid_wraparound
... snip
1/6 postgresql:setup / tmp_install
    OK                0.37s
2/6 postgresql:setup / install_test_files
    OK                0.08s
3/6 postgresql:setup / initdb_cache
    OK                1.16s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    SKIP              0.14s
5/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    SKIP              0.15s
6/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    SKIP              0.15s
... snip
$ PG_TEST_EXTRA=xid_wraparound mts xid_wraparound
... snip
1/6 postgresql:setup / tmp_install
    OK                0.36s
2/6 postgresql:setup / install_test_files
    OK                0.06s
3/6 postgresql:setup / initdb_cache
    OK                1.12s
4/6 postgresql:xid_wraparound / xid_wraparound/001_emergency_vacuum
    OK               62.53s   7 subtests passed
5/6 postgresql:xid_wraparound / xid_wraparound/002_limits
    OK               69.78s   3 subtests passed
6/6 postgresql:xid_wraparound / xid_wraparound/003_wraparounds
    OK              186.78s   1 subtests passed

xid_wraparound tests are run if PG_TEST_EXTRA contains xid_wraparound
or it is not set. Any other setting will not run xid_wraparound test.
That's how the patch is coded but it isn't intuitive that changing
whether a test is run by default would require configuring the build
again. Probably we should just get rid of config time PG_TEST_EXTRA
altogether.

I am including +Tristan Partin who knows meson better.

If you are willing to work on this further, please add it to the commitfest.

--
Best Wishes,
Ashutosh Bapat



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Wed, 17 Jul 2024 at 13:13, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> xid_wraparound tests are run if PG_TEST_EXTRA contains xid_wraparound
> or it is not set. Any other setting will not run xid_wraparound test.
> That's how the patch is coded but it isn't intuitive that changing
> whether a test is run by default would require configuring the build
> again. Probably we should just get rid of config time PG_TEST_EXTRA
> altogether.
>
> I am including +Tristan Partin who knows meson better.
>
> If you are willing to work on this further, please add it to the commitfest.

I think I know why there is confusion. Could you try to set
PG_TEST_EXTRA with quotes? Like PG_TEST_EXTRA="ldap mts
xid_wraparound".

-- 
Regards,
Nazir Bilal Yavuz
Microsoft



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Wed, 17 Jul 2024 at 13:23, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> Hi,
>
> On Wed, 17 Jul 2024 at 13:13, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > xid_wraparound tests are run if PG_TEST_EXTRA contains xid_wraparound
> > or it is not set. Any other setting will not run xid_wraparound test.
> > That's how the patch is coded but it isn't intuitive that changing
> > whether a test is run by default would require configuring the build
> > again. Probably we should just get rid of config time PG_TEST_EXTRA
> > altogether.
> >
> > I am including +Tristan Partin who knows meson better.
> >
> > If you are willing to work on this further, please add it to the commitfest.
>
> I think I know why there is confusion. Could you try to set
> PG_TEST_EXTRA with quotes? Like PG_TEST_EXTRA="ldap mts
> xid_wraparound".

Sorry, the previous reply was wrong; I misunderstood what you said.
Yes, that is how the patch was coded and I agree that getting rid of
config time PG_TEST_EXTRA could be a better alternative.

-- 
Regards,
Nazir Bilal Yavuz
Microsoft



Re: PG_TEST_EXTRA and meson

От
Jacob Champion
Дата:
On Wed, Jul 17, 2024 at 3:34 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> Sorry, the previous reply was wrong; I misunderstood what you said.
> Yes, that is how the patch was coded and I agree that getting rid of
> config time PG_TEST_EXTRA could be a better alternative.

Personally I use the config-time PG_TEST_EXTRA extensively. I'd be sad
to see it go, especially if developers are no longer forced to use it.
(In practice, I don't change that setting much after initial
configure, because I use separate worktrees/build directories for
different patchsets. And the reconfiguration is fast when I do need to
modify it.)

Thanks,
--Jacob



Re: PG_TEST_EXTRA and meson

От
Tom Lane
Дата:
Jacob Champion <jacob.champion@enterprisedb.com> writes:
> On Wed, Jul 17, 2024 at 3:34 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>> Sorry, the previous reply was wrong; I misunderstood what you said.
>> Yes, that is how the patch was coded and I agree that getting rid of
>> config time PG_TEST_EXTRA could be a better alternative.

> Personally I use the config-time PG_TEST_EXTRA extensively. I'd be sad
> to see it go, especially if developers are no longer forced to use it.

The existing and documented expectation is that PG_TEST_EXTRA is an
environment variable, ie it's a runtime option not a configure option.
Making it be the latter seems like a significant loss of flexibility
to me.

            regards, tom lane



Re: PG_TEST_EXTRA and meson

От
Andrew Dunstan
Дата:


On 2024-07-17 We 11:01 AM, Tom Lane wrote:
Jacob Champion <jacob.champion@enterprisedb.com> writes:
On Wed, Jul 17, 2024 at 3:34 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
Sorry, the previous reply was wrong; I misunderstood what you said.
Yes, that is how the patch was coded and I agree that getting rid of
config time PG_TEST_EXTRA could be a better alternative.
Personally I use the config-time PG_TEST_EXTRA extensively. I'd be sad
to see it go, especially if developers are no longer forced to use it.
The existing and documented expectation is that PG_TEST_EXTRA is an
environment variable, ie it's a runtime option not a configure option.
Making it be the latter seems like a significant loss of flexibility
to me.
	


AIUI the only reason we have it as a configure option at all is that meson is *very* dogmatic about not using environment variables. I get their POV when it comes to building, but that should not extend to testing. That said, I don't mind if this is a configure option as long as it can be overridden at run time without having to run "meson configure".


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: PG_TEST_EXTRA and meson

От
Jacob Champion
Дата:
On Wed, Jul 17, 2024 at 8:01 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Jacob Champion <jacob.champion@enterprisedb.com> writes:
> > Personally I use the config-time PG_TEST_EXTRA extensively. I'd be sad
> > to see it go, especially if developers are no longer forced to use it.
>
> The existing and documented expectation is that PG_TEST_EXTRA is an
> environment variable, ie it's a runtime option not a configure option.
> Making it be the latter seems like a significant loss of flexibility
> to me.

I think/hope we're saying the same thing -- developers should not be
forced to lock PG_TEST_EXTRA into their configurations; that's
inflexible and unhelpful.

What I'm saying in addition to that is, I really like that I can
currently put a default PG_TEST_EXTRA into my meson config so that I
don't have to keep track of it, and I do that all the time. So I'm in
favor of the "option 3" approach.

Thanks,
--Jacob



Re: PG_TEST_EXTRA and meson

От
Tom Lane
Дата:
Jacob Champion <jacob.champion@enterprisedb.com> writes:
> On Wed, Jul 17, 2024 at 8:01 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The existing and documented expectation is that PG_TEST_EXTRA is an
>> environment variable, ie it's a runtime option not a configure option.
>> Making it be the latter seems like a significant loss of flexibility
>> to me.

> I think/hope we're saying the same thing -- developers should not be
> forced to lock PG_TEST_EXTRA into their configurations; that's
> inflexible and unhelpful.

Indeed.

> What I'm saying in addition to that is, I really like that I can
> currently put a default PG_TEST_EXTRA into my meson config so that I
> don't have to keep track of it, and I do that all the time. So I'm in
> favor of the "option 3" approach.

Ah.  I have no particular objection to that, but I wonder whether
we should make the autoconf/makefile infrastructure do it too.

            regards, tom lane



Re: PG_TEST_EXTRA and meson

От
Jacob Champion
Дата:
On Wed, Jul 17, 2024 at 11:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ah.  I have no particular objection to that, but I wonder whether
> we should make the autoconf/makefile infrastructure do it too.

I don't need it personally, having moved almost entirely to Meson. But
if the asymmetry is a sticking point, I can work on a patch.

Thanks!
--Jacob



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Wed, 17 Jul 2024 at 13:13, Ashutosh Bapat
<ashutosh.bapat.oss@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

Вложения

Re: PG_TEST_EXTRA and meson

От
Ashutosh Bapat
Дата:
Hi Nazir,

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

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



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Tue, 23 Jul 2024 at 12:26, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> 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.

I agree that symmetry should be the ultimate goal.

Upthread Jacob said he could work on a patch about introducing the
PG_TEST_EXTRA configure option to make builds. Would you still be
interested in working on this? If not, I would gladly work on it.

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

Thanks for testing it!

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

When test_env('PG_TEST_EXTRA') is set, it could not be overridden
afterwards. Perhaps there is a way to override test_env() but I do not
know how.

-- 
Regards,
Nazir Bilal Yavuz
Microsoft



Re: PG_TEST_EXTRA and meson

От
Ashutosh Bapat
Дата:
On Tue, Jul 23, 2024 at 4:02 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> > 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').
>
> When test_env('PG_TEST_EXTRA') is set, it could not be overridden
> afterwards. Perhaps there is a way to override test_env() but I do not
> know how.
>

I am not suggesting to override test_env['PG_TEST_EXTRA'] but set it
to the value after overriding if required. meson.build file seems to
allow some conditional variable setting. So I thought this would be
possible, haven't tried myself though.

--
Best Wishes,
Ashutosh Bapat



Re: PG_TEST_EXTRA and meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Tue, 23 Jul 2024 at 13:40, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Tue, Jul 23, 2024 at 4:02 PM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> >
> > > 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').
> >
> > When test_env('PG_TEST_EXTRA') is set, it could not be overridden
> > afterwards. Perhaps there is a way to override test_env() but I do not
> > know how.
> >
>
> I am not suggesting to override test_env['PG_TEST_EXTRA'] but set it
> to the value after overriding if required. meson.build file seems to
> allow some conditional variable setting. So I thought this would be
> possible, haven't tried myself though.

Sorry if I caused a misunderstanding. What I meant was, when the
test_env('PG_TEST_EXTRA') is set, Meson will always use PG_TEST_EXTRA
from the setup. So, Meson needs to be built again to change
PG_TEST_EXTRA.

AFAIK Meson does not support accessing environment variables but
run_command() could be used to test this:

-test_env.set('PG_TEST_EXTRA', get_option('PG_TEST_EXTRA'))
+pg_test_extra_env = run_command(
+  [python,
+  '-c',
+  'import os; print(os.getenv("PG_TEST_EXTRA", ""))'],
+  check: true).stdout()
+
+test_env.set('PG_TEST_EXTRA', pg_test_extra_env != '' ?
+  pg_test_extra_env :
+  get_option('PG_TEST_EXTRA'))
+

--
Regards,
Nazir Bilal Yavuz
Microsoft



Re: PG_TEST_EXTRA and meson

От
Jacob Champion
Дата:
On Tue, Jul 23, 2024 at 3:32 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> Upthread Jacob said he could work on a patch about introducing the
> PG_TEST_EXTRA configure option to make builds. Would you still be
> interested in working on this? If not, I would gladly work on it.

Sure! Attached is a minimalist approach using AC_ARG_VAR.

It works for top-level `make check-world`, or `make check -C
src/test`. If you run `make check` directly from a test subdirectory,
the variable doesn't get picked up, because it's only exported from
the src/test level as of your patch c3382a3c3cc -- but if that turns
out to be a problem, we can plumb it all the way down or expand the
scope of the export.

Thanks,
--Jacob

Вложения