REGRESS_OPTS versus MSVC build scripts

Tom Lane wrote:

I believe (but won't be able to prove for another few hours) that the
reason the MSVC buildfarm members are failing on contrib/unaccent,
as seen here:
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mastodon&dt=2009-08-18%2013:00:00
is that they are not running the test in a UTF8-encoded database.
And the reason for *that* is that the MSVC build infrastructure pays no
attention to REGRESS_OPTS. Instead it's got hardwired values for all
the pg_regress command line options, and of course it needs to hardwire
several different sets of values for different tests :-(.

I think it's time to fix that properly by making the perl scripts
dredge REGRESS_OPTS out of the makefiles. My perl-fu is not up to
the task, however. Anybody want to do it?

This seems like a stop-ship issue for 8.5alpha1, even though it's only
a contrib regression test that's certain to fail; so I think it's a
bit urgent.

I will take a look.

cheers

andrew

Andrew Dunstan wrote:

Tom Lane wrote:

I believe (but won't be able to prove for another few hours) that the
reason the MSVC buildfarm members are failing on contrib/unaccent,
as seen here:
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mastodon&dt=2009-08-18%2013:00:00

is that they are not running the test in a UTF8-encoded database.
And the reason for *that* is that the MSVC build infrastructure pays no
attention to REGRESS_OPTS. Instead it's got hardwired values for all
the pg_regress command line options, and of course it needs to hardwire
several different sets of values for different tests :-(.

I think it's time to fix that properly by making the perl scripts
dredge REGRESS_OPTS out of the makefiles. My perl-fu is not up to
the task, however. Anybody want to do it?

This seems like a stop-ship issue for 8.5alpha1, even though it's only
a contrib regression test that's certain to fail; so I think it's a
bit urgent.

I will take a look.

Here's an untested patch ... I think it should do the job, however, at
least for contrib modules, which is the immediate problem.

cheers

andrew

On Tue, Aug 18, 2009 at 02:15:48PM -0400, Andrew Dunstan wrote:

Andrew Dunstan wrote:

Tom Lane wrote:

I believe (but won't be able to prove for another few hours) that the
reason the MSVC buildfarm members are failing on contrib/unaccent,
as seen here:

http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mastodon&dt=2009-08-18%2013:00:00

is that they are not running the test in a UTF8-encoded database.
And the reason for *that* is that the MSVC build infrastructure pays no
attention to REGRESS_OPTS. Instead it's got hardwired values for all
the pg_regress command line options, and of course it needs to hardwire
several different sets of values for different tests :-(.

I think it's time to fix that properly by making the perl scripts
dredge REGRESS_OPTS out of the makefiles. My perl-fu is not up to
the task, however. Anybody want to do it?

This seems like a stop-ship issue for 8.5alpha1, even though it's only
a contrib regression test that's certain to fail; so I think it's a
bit urgent.

I will take a look.

Here's an untested patch ... I think it should do the job, however, at
least for contrib modules, which is the immediate problem.

cheers

andrew

Index: src/tools/msvc/vcregress.pl
===================================================================
RCS file: /cvsroot/pgsql/src/tools/msvc/vcregress.pl,v
retrieving revision 1.10
diff -c -r1.10 vcregress.pl
*** src/tools/msvc/vcregress.pl	1 Dec 2008 13:39:45 -0000	1.10
--- src/tools/msvc/vcregress.pl	18 Aug 2009 18:12:59 -0000
***************
*** 185,194 ****
print "============================================================\n";
print "Checking $module\n";
my @tests = fetchTests();
my @args = (
"../../$Config/pg_regress/pg_regress",
"--psqldir=../../$Config/psql",
!             "--dbname=contrib_regression",@tests
);
system(@args);
my $status = $? >> 8;
--- 185,195 ----
print "============================================================\n";
print "Checking $module\n";
my @tests = fetchTests();
+ 		my @opts = fetchRegressOpts();
my @args = (
"../../$Config/pg_regress/pg_regress",
"--psqldir=../../$Config/psql",
!             "--dbname=contrib_regression",@opts,@tests
);
system(@args);
my $status = $? >> 8;
***************
*** 198,203 ****
--- 199,223 ----
exit $mstat if $mstat;
}

+ sub fetchRegressOpts
+ {
+     my $handle;
+     open($handle,"<Makefile")

This section is probably better done with Tie::File.

Cheers,
David.

+       || open($handle,"<GNUmakefile")
+       || die "Could not open Makefile";
+     local($/) = undef;
+     my $m = <$handle>;
+     close($handle);
+ 	my @opts;
+ 	if ($m =~ /^\s*REGRESS_OPTS\s*=(.*)/m)
+ 	{
+ 		# ignore options that use makefile variables - can't handle those
+ 		# ignore anything that isn't an option staring with --
+ 		@opts = grep { $_ !~ /\$\(/ && $_ =~ /^--/ } split(/\s+/,$1);
+ 	}
+ 	return @opts;
+ }
+ 
sub fetchTests
{

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Andrew Dunstan wrote:

+ sub fetchRegressOpts
+ {
+     my $handle;
+     open($handle,"<Makefile")
+       || open($handle,"<GNUmakefile")
+       || die "Could not open Makefile";

I think you should try GNUmakefile first, Makefile second. That's what
gmake does.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

David Fetter wrote:

+ my $handle;
+ open($handle,"<Makefile")

This section is probably better done with Tie::File.

You keep pushing that barrow. I ripped Tie::File OUT of the buildfarm -
it is not universally available. Besides, as the perl world says, TIMTOWTDI.

cheers

andrew

Alvaro Herrera wrote:

Andrew Dunstan wrote:

+ sub fetchRegressOpts
+ {
+     my $handle;
+     open($handle,"<Makefile")
+       || open($handle,"<GNUmakefile")
+       || die "Could not open Makefile";

I think you should try GNUmakefile first, Makefile second. That's what
gmake does.

Hmm, ok. It doesn't matter much in the present case - none of the
contrib modules actually has a GNUmakefile, but I'll change it. I
actually copied that code from elsewhere in the script.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

David Fetter wrote:

This section is probably better done with Tie::File.

You keep pushing that barrow. I ripped Tie::File OUT of the buildfarm -
it is not universally available. Besides, as the perl world says, TIMTOWTDI.

Also, there's some value in having the code be readable by perl tyros
(like me). I know what open() does, but I'd be clueless about whatever
it is you're proposing he write instead.

regards, tom lane

---------- Forwarded message ----------
From: David Fetter <david@fetter.org>
To: Andrew Dunstan <andrew@dunslane.net>
Date: Tue, 18 Aug 2009 11:31:41 -0700
Subject: Re: REGRESS_OPTS versus MSVC build scripts
On Tue, Aug 18, 2009 at 02:15:48PM -0400, Andrew Dunstan wrote:

Andrew Dunstan wrote:

Here's an untested patch ... I think it should do the job, however, at
least for contrib modules, which is the immediate problem.

cheers

andrew

Index: src/tools/msvc/vcregress.pl
===================================================================
RCS file: /cvsroot/pgsql/src/tools/msvc/vcregress.pl,v
retrieving revision 1.10
diff -c -r1.10 vcregress.pl
*** src/tools/msvc/vcregress.pl       1 Dec 2008 13:39:45 -0000       1.10
--- src/tools/msvc/vcregress.pl       18 Aug 2009 18:12:59 -0000

...

*** 198,203 ****
--- 199,223 ----
exit $mstat if $mstat;
}

+ sub fetchRegressOpts
+ {
+     my $handle;
+     open($handle,"<Makefile")

This section is probably better done with Tie::File.

No, absolutely not. I would have a hard time believing any computer
that would be
compiling or testing pg would not have enough memory to slurp a
Makefile into memory.
And if it did, Tie::File would not be the answer. For files with
small line lengths the
memory overhead of Tie::File is generally more than the memory savings.
And it gives us the added bonus of letting us accidentally change
files, when we
thought we were just changing an in memory copy.

The real solution in that highly unlikely case would be to read the
file line by line in
a loop, rather than slurping it, as the existing code doesn't search
across line
boundaries anyway.

Cheers,

Jeff