PROVE_FLAGS
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )
PROVE_FLAGS =
ISTM it's unnecessary, and prevents us from using the same named value
in the environment. I want to be able to use the environment in
vcregress.pl, and I'd like the Make files to work the same way.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017-05-03 15:14:27 -0400, Andrew Dunstan wrote:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )PROVE_FLAGS =
ISTM it's unnecessary, and prevents us from using the same named value
in the environment. I want to be able to use the environment in
vcregress.pl, and I'd like the Make files to work the same way.
Wouldn't it be better to append the environment to the flags here,
that'd allow us to modify flags from both places?
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/03/2017 03:21 PM, Andres Freund wrote:
On 2017-05-03 15:14:27 -0400, Andrew Dunstan wrote:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )PROVE_FLAGS =
ISTM it's unnecessary, and prevents us from using the same named value
in the environment. I want to be able to use the environment in
vcregress.pl, and I'd like the Make files to work the same way.Wouldn't it be better to append the environment to the flags here,
that'd allow us to modify flags from both places?
The Makefile already has:
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) ...
It doesn't set PROVE_FLAGS anywhere.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )
PROVE_FLAGS =
Before that commit it was like
PROVE_FLAGS = --verbose
which had some value. I agree that now we'd be better off to not
set it at all, especially since the convention now appears to be that
automatically-supplied prove options should be set in PG_PROVE_FLAGS.
I'd suggest that the comment just above be replaced by something like
# User-supplied prove flags can be provided in PROVE_FLAGS.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )
PROVE_FLAGS =Before that commit it was like
PROVE_FLAGS = --verbose
right.
which had some value. I agree that now we'd be better off to not
set it at all, especially since the convention now appears to be that
automatically-supplied prove options should be set in PG_PROVE_FLAGS.
Good point.
I'd suggest that the comment just above be replaced by something like
# User-supplied prove flags can be provided in PROVE_FLAGS.
Works for me.
Thanks!
Stephen
On 05/04/2017 12:50 AM, Stephen Frost wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )
PROVE_FLAGS =Before that commit it was like
PROVE_FLAGS = --verbose
right.
which had some value. I agree that now we'd be better off to not
set it at all, especially since the convention now appears to be that
automatically-supplied prove options should be set in PG_PROVE_FLAGS.Good point.
I'd suggest that the comment just above be replaced by something like
# User-supplied prove flags can be provided in PROVE_FLAGS.
Works for me.
Does anyone object to me backpatching this? It seems to me kinda crazy
to have --verbose hardcoded on the back branches and not on the dev branch.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, May 12, 2017 at 9:05 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
Does anyone object to me backpatching this? It seems to me kinda crazy
to have --verbose hardcoded on the back branches and not on the dev branch.
+1. A maximum of consistency with the test code when possible is nice to have.
--
Michael--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier <michael.paquier@gmail.com> writes:
On Fri, May 12, 2017 at 9:05 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:Does anyone object to me backpatching this? It seems to me kinda crazy
to have --verbose hardcoded on the back branches and not on the dev branch.
+1. A maximum of consistency with the test code when possible is nice to have.
No objection here either.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/3/17 15:14, Andrew Dunstan wrote:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )PROVE_FLAGS =
ISTM it's unnecessary, and prevents us from using the same named value
in the environment. I want to be able to use the environment in
vcregress.pl, and I'd like the Make files to work the same way.
I think relying on environment variables like this is a bad design.
Someone could have something left in the environment and it changes
things in mysterious ways. For all other *FLAGS variables, we
explicitly set them in Makefile.global, sometimes to empty values, as
the case may be.
Note that specifying a variable on the make command line overrides
settings in the makefile under certain circumstances, which is a useful
feature.
So I think the above setting was correct, the new behavior is
inconsistent and incorrect, and I think this change should be reverted.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/16/2017 07:44 PM, Peter Eisentraut wrote:
On 5/3/17 15:14, Andrew Dunstan wrote:
Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )PROVE_FLAGS =
ISTM it's unnecessary, and prevents us from using the same named value
in the environment. I want to be able to use the environment in
vcregress.pl, and I'd like the Make files to work the same way.I think relying on environment variables like this is a bad design.
Someone could have something left in the environment and it changes
things in mysterious ways. For all other *FLAGS variables, we
explicitly set them in Makefile.global, sometimes to empty values, as
the case may be.Note that specifying a variable on the make command line overrides
settings in the makefile under certain circumstances, which is a useful
feature.So I think the above setting was correct, the new behavior is
inconsistent and incorrect, and I think this change should be reverted.
It would have been nice if you'd spoken up when the topic was raised.
This doesn't rely on the environment variable, it just enables it. You
can still say "make PROVE_FLAGS=--whatever" and it will override the
environment.
Inheriting variables from the environment is a part of make by design.
We have PG_PROVE_FLAGS for our own forced settings.
Note that the previous setting was done without any thought being given
to how this works with vcregress.pl. I have been trying to make the two
systems more consistent. This was a part of that effort.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, May 16, 2017 at 9:20 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
Inheriting variables from the environment is a part of make by design.
We have PG_PROVE_FLAGS for our own forced settings.
I don't buy this argument. We've had previous cases where we've gone
through and added -X to psql invocations in the regression tests
because, if you don't, some people's .psqlrc files may cause tests to
fail, which nobody wants. The more general principle is that the
regression tests should ideally pass regardless of the local machine
configuration. It's proven impractical to make that universally true,
but that doesn't make it a bad goal. Now, for all I know it may be
that setting PROVE_FLAGS can never cause a regression failure, but I
still tend to agree with Peter that the regression tests framework
should insulate us from the surrounding environment rather than
absorbing settings from it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 17 May 2017 at 14:30, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, May 16, 2017 at 9:20 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:Inheriting variables from the environment is a part of make by design.
We have PG_PROVE_FLAGS for our own forced settings.I don't buy this argument. We've had previous cases where we've gone
through and added -X to psql invocations in the regression tests
because, if you don't, some people's .psqlrc files may cause tests to
fail, which nobody wants. The more general principle is that the
regression tests should ideally pass regardless of the local machine
configuration. It's proven impractical to make that universally true,
but that doesn't make it a bad goal. Now, for all I know it may be
that setting PROVE_FLAGS can never cause a regression failure, but I
still tend to agree with Peter that the regression tests framework
should insulate us from the surrounding environment rather than
absorbing settings from it.
In that case we're going to need to invent a way to do this similarly
in vcregress.pl. I'm not simply going to revert to the situation where
it and the makefiles are completely out of sync on this. The previous
patch was made more or less by ignoring the existence of vcregress.pl.
I will look at this again probably after pgcon. I don't think it's
terribly urgent.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/23/2017 06:59 AM, Andrew Dunstan wrote:
On 17 May 2017 at 14:30, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, May 16, 2017 at 9:20 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:Inheriting variables from the environment is a part of make by design.
We have PG_PROVE_FLAGS for our own forced settings.I don't buy this argument. We've had previous cases where we've gone
through and added -X to psql invocations in the regression tests
because, if you don't, some people's .psqlrc files may cause tests to
fail, which nobody wants. The more general principle is that the
regression tests should ideally pass regardless of the local machine
configuration. It's proven impractical to make that universally true,
but that doesn't make it a bad goal. Now, for all I know it may be
that setting PROVE_FLAGS can never cause a regression failure, but I
still tend to agree with Peter that the regression tests framework
should insulate us from the surrounding environment rather than
absorbing settings from it.In that case we're going to need to invent a way to do this similarly
in vcregress.pl. I'm not simply going to revert to the situation where
it and the makefiles are completely out of sync on this. The previous
patch was made more or less by ignoring the existence of vcregress.pl.I will look at this again probably after pgcon. I don't think it's
terribly urgent.
Here's a patch that should do that. If this is acceptable I'll do this
and put the makefiles back the way they were.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
vcregress-prove-flags.patchtext/x-patch; name=vcregress-prove-flags.patchDownload
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 468a62d..e2d225e 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -178,12 +178,18 @@ sub tap_check
die "Tap tests not enabled in configuration"
unless $config->{tap_tests};
+ my @flags;
+ foreach my $arg (0 .. scalar(@_))
+ {
+ next unless $_[$arg] =~ /^PROVE_FLAGS=(.*)/;
+ @flags = split(/\s+/, $1};
+ splice(@_,$arg,1);
+ last;
+ }
+
my $dir = shift;
chdir $dir;
- my @flags;
- @flags = split(/\s+/, $ENV{PROVE_FLAGS}) if exists $ENV{PROVE_FLAGS};
-
my @args = ("prove", @flags, "t/*.pl");
# adjust the environment for just this test
On 6/5/17 15:06, Andrew Dunstan wrote:
In that case we're going to need to invent a way to do this similarly
in vcregress.pl. I'm not simply going to revert to the situation where
it and the makefiles are completely out of sync on this. The previous
patch was made more or less by ignoring the existence of vcregress.pl.I will look at this again probably after pgcon. I don't think it's
terribly urgent.Here's a patch that should do that.
I'm not able to test this, but it looks like a reasonable approach.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers