Re: Makefile, environment variables and REGTESTS_TYPES
On Fri, Feb 05, 2021 at 11:06:01AM +0100, William Lallemand wrote:
> On Fri, Feb 05, 2021 at 10:31:53AM +0100, William Lallemand wrote:
>
> > Ok, I'm going to do the change in the help command then.
> >
>
> In fact I just take a look again at this, and I think we've done the
> patch the wrong way.
>
> In 'run-regtests.sh' there is already a default setting:
>
> REGTESTS_TYPES="${REGTESTS_TYPES:-any}"
>
> So I suggest we change this one and remove the one from the Makefile, so we
> could have the previous behavior.
>
> The Makefile is not suppose to change these variables, even
> HAPROXY_PROGRAM and VTEST_PROGRAM are not set in the Makefile.
>
> It makes more sense this way in my opinion.
I think you're right, indeed.
Willy
Re: Makefile, environment variables and REGTESTS_TYPES
On Fri, Feb 05, 2021 at 10:31:53AM +0100, William Lallemand wrote:
> Ok, I'm going to do the change in the help command then.
>
In fact I just take a look again at this, and I think we've done the
patch the wrong way.
In 'run-regtests.sh' there is already a default setting:
REGTESTS_TYPES="${REGTESTS_TYPES:-any}"
So I suggest we change this one and remove the one from the Makefile, so we
could have the previous behavior.
The Makefile is not suppose to change these variables, even
HAPROXY_PROGRAM and VTEST_PROGRAM are not set in the Makefile.
It makes more sense this way in my opinion.
--
William Lallemand
Re: Makefile, environment variables and REGTESTS_TYPES
On Fri, Feb 05, 2021 at 08:41:47AM +0100, Willy Tarreau wrote:
> Hi William,
>
> On Fri, Jan 29, 2021 at 02:44:27PM +0100, William Lallemand wrote:
> > Hello List,
> >
> > According to `make reg-tests-help` the REGTESTS_TYPES parameter must be
> > configured as an environment variable and not a make argument.
> >
> > However since patch 3bad3d5 ("BUILD: Makefile: exclude broken tests by
> > default"), it does not work anymore with an environment variable.
> >
> > Looking at the several CI files we have, it is used as a make
> > argument everywhere.
>
> I must confess I didn't even know it was expected to be used as an
> environment variable and have always used it as a make argument, given
> that these *are* exported as argument variables anyway.
I was surprised it was documented this way too, because we never used
the Makefile with environment variable before.
> Here's what I'm using for the make-reg-test-all script for example:
>
> make reg-tests VTEST_PROGRAM=/usr/local/bin/vtest
> REGTESTS_TYPES=default,bug,devel,slow -- --j 8 "$@" || exit 1
>
> > I'm going to update the `make reg-tests-help` command with the correct
> > syntax if there isn't any complain.
>
> OK.
>
> > We could fix the issue by using "?=" when setting the default value, but
> > it would make it the only variable that use "?=" in the Makefile and I'm
> > not sure we want to proceed this way.
>
> No please avoid this, I find the mixing of arguments and environment
> really confusing to use. The only valid use of "?=" is when you want
> to support pre-setting a variable that will be completed later,
> but this usually remains as it doesn't allow you to remove options,
> and in general it's really bad as you get different behaviors for
>
>$ VAR=xxx make
>
> and:
>
>$ make VAR=xxx
>
> I think the reg-test users are a small enough group that any required
> change can be quickly adopted if needed anyway, without having to take
> too much care about backwards compatibility, as long as it remains
> convenient to use.
>
Ok, I'm going to do the change in the help command then.
--
William Lallemand
Re: Makefile, environment variables and REGTESTS_TYPES
Hi William,
On Fri, Jan 29, 2021 at 02:44:27PM +0100, William Lallemand wrote:
> Hello List,
>
> According to `make reg-tests-help` the REGTESTS_TYPES parameter must be
> configured as an environment variable and not a make argument.
>
> However since patch 3bad3d5 ("BUILD: Makefile: exclude broken tests by
> default"), it does not work anymore with an environment variable.
>
> Looking at the several CI files we have, it is used as a make
> argument everywhere.
I must confess I didn't even know it was expected to be used as an
environment variable and have always used it as a make argument, given
that these *are* exported as argument variables anyway. Here's what I'm
using for the make-reg-test-all script for example:
make reg-tests VTEST_PROGRAM=/usr/local/bin/vtest
REGTESTS_TYPES=default,bug,devel,slow -- --j 8 "$@" || exit 1
> I'm going to update the `make reg-tests-help` command with the correct
> syntax if there isn't any complain.
OK.
> We could fix the issue by using "?=" when setting the default value, but
> it would make it the only variable that use "?=" in the Makefile and I'm
> not sure we want to proceed this way.
No please avoid this, I find the mixing of arguments and environment
really confusing to use. The only valid use of "?=" is when you want
to support pre-setting a variable that will be completed later, but
this usually remains as it doesn't allow you to remove options, and
in general it's really bad as you get different behaviors for
$ VAR=xxx make
and:
$ make VAR=xxx
I think the reg-test users are a small enough group that any required
change can be quickly adopted if needed anyway, without having to take
too much care about backwards compatibility, as long as it remains
convenient to use.
Thanks,
Willy
Re: Makefile, environment variables and REGTESTS_TYPES
On Fri, Jan 29, 2021 at 2:46 PM William Lallemand
wrote:
> According to `make reg-tests-help` the REGTESTS_TYPES parameter must be
> configured as an environment variable and not a make argument.
> However since patch 3bad3d5 ("BUILD: Makefile: exclude broken tests by
> default"), it does not work anymore with an environment variable.
> Looking at the several CI files we have, it is used as a make
> argument everywhere.
wow indeed, I did not realise that; yet another thing I broke ;-)
> I'm going to update the `make reg-tests-help` command with the correct
> syntax if there isn't any complain.
Thanks for looking into this!
--
William
Makefile, environment variables and REGTESTS_TYPES
Hello List,
According to `make reg-tests-help` the REGTESTS_TYPES parameter must be
configured as an environment variable and not a make argument.
However since patch 3bad3d5 ("BUILD: Makefile: exclude broken tests by
default"), it does not work anymore with an environment variable.
Looking at the several CI files we have, it is used as a make
argument everywhere.
I'm going to update the `make reg-tests-help` command with the correct
syntax if there isn't any complain.
We could fix the issue by using "?=" when setting the default value, but
it would make it the only variable that use "?=" in the Makefile and I'm
not sure we want to proceed this way.
Regards,
--
William Lallemand
