Обсуждение: Re: [COMMITTERS] pgsql: Rename pg_regress option --multibyte to --encoding

Peter Eisentraut <peter_e@gmx.net> writes:
> Rename pg_regress option --multibyte to --encoding
> Also refactor things a little bit so that the same methods for setting
> test locale and encoding can be used everywhere.

The buildfarm shows that this patch fails on Windows --- the
contrib/unaccent regression test is getting run in SQL_ASCII encoding
rather than the UTF8 encoding it's supposed to have.  It looks like
the reason is that fetchRegressOpts in vcregress.pl is only looking
for REGRESS_OPTS in the Makefile, and what you've done requires it
to have a great deal more knowledge than that.  I'm not sure what the
least painful fix is, but maybe we should reconsider the idea of not
using REGRESS_OPTS there.
        regards, tom lane

On 04/15/2011 11:18 AM, Tom Lane wrote:
> Peter Eisentraut<peter_e@gmx.net>  writes:
>> Rename pg_regress option --multibyte to --encoding
>> Also refactor things a little bit so that the same methods for setting
>> test locale and encoding can be used everywhere.
> The buildfarm shows that this patch fails on Windows --- the
> contrib/unaccent regression test is getting run in SQL_ASCII encoding
> rather than the UTF8 encoding it's supposed to have.  It looks like
> the reason is that fetchRegressOpts in vcregress.pl is only looking
> for REGRESS_OPTS in the Makefile, and what you've done requires it
> to have a great deal more knowledge than that.  I'm not sure what the
> least painful fix is, but maybe we should reconsider the idea of not
> using REGRESS_OPTS there.

Yeah, what is the point of this?:
   -# Adjust REGRESS_OPTS because we need a UTF8 database   -REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --multibyte=UTF8
--no-locale  +# We need a UTF8 database +ENCODING = UTF8 +NO_LOCALE = 1
 


Wouldn't it have worked just to change the way we spelled the option 
name in the REGRESS_OPTS line?


cheers

andrew

On Fri, 2011-04-15 at 12:28 -0400, Andrew Dunstan wrote:
> Yeah, what is the point of this?:
> 
>     -# Adjust REGRESS_OPTS because we need a UTF8 database
>     -REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --multibyte=UTF8 --no-locale
>     +# We need a UTF8 database +ENCODING = UTF8 +NO_LOCALE = 1
> 
> 
> Wouldn't it have worked just to change the way we spelled the option 
> name in the REGRESS_OPTS line?

It would have, but this way you have the same uniform interface
everywhere.  For example, you can easily override the encoding that that
test prescribes.

On 04/15/2011 04:36 PM, Peter Eisentraut wrote:
> On Fri, 2011-04-15 at 12:28 -0400, Andrew Dunstan wrote:
>> Yeah, what is the point of this?:
>>
>>      -# Adjust REGRESS_OPTS because we need a UTF8 database
>>      -REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --multibyte=UTF8 --no-locale
>>      +# We need a UTF8 database +ENCODING = UTF8 +NO_LOCALE = 1
>>
>>
>> Wouldn't it have worked just to change the way we spelled the option
>> name in the REGRESS_OPTS line?
> It would have, but this way you have the same uniform interface
> everywhere.  For example, you can easily override the encoding that that
> test prescribes.
>


Well whatever we do, the buildfarm is currently broken for all MSVC 
builds because of this change, so I hope you're intending to fix this.

cheers

andrew

On Sat, 2011-04-16 at 15:07 -0400, Andrew Dunstan wrote:
> On 04/15/2011 04:36 PM, Peter Eisentraut wrote:
> > On Fri, 2011-04-15 at 12:28 -0400, Andrew Dunstan wrote:
> >> Yeah, what is the point of this?:
> >>
> >>      -# Adjust REGRESS_OPTS because we need a UTF8 database
> >>      -REGRESS_OPTS = --dbname=$(CONTRIB_TESTDB) --multibyte=UTF8 --no-locale
> >>      +# We need a UTF8 database +ENCODING = UTF8 +NO_LOCALE = 1
> >>
> >>
> >> Wouldn't it have worked just to change the way we spelled the option
> >> name in the REGRESS_OPTS line?
> > It would have, but this way you have the same uniform interface
> > everywhere.  For example, you can easily override the encoding that that
> > test prescribes.

> Well whatever we do, the buildfarm is currently broken for all MSVC 
> builds because of this change, so I hope you're intending to fix this.

I was able to fix this properly now after figuring out that you can
actually run vcregress.pl on Linux.  So it should be easier to deal with
these sorts of issues in the future.