Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Perl: porters

op/magic.t tests 154,157

 

 

Perl porters RSS feed   Index | Next | Previous | View Threaded


steve.m.hay at googlemail

Aug 1, 2012, 2:11 PM

Post #1 of 20 (858 views)
Permalink
op/magic.t tests 154,157

I see op/magic.t failing tests 154 and 157 for me (Windows, VC++), and
also in various other smoke reports (not just Windows).

Is anyone looking into this?


Here's the relevant output on my system;

# Failed test 154 - ENV store downgrades utf8 in setenv at op/magic.t line 71
# got 'foo=eh zero
not ok 154 - ENV store downgrades utf8 in setenv# '

# expected /(?^:^(?:foo=)?eh\ zero\ \á$)/
# Wide character in setenv at op/magic.t line 649.
ok 155 - ENV store warns about wide characters
ok 156 - ENV store encodes high utf8 in SV
# Failed test 157 - ENV store encodes high utf8 in SV at op/magic.t line 71
# got 'foo=X-Day á¦~
not ok 157 - ENV store encodes high utf8 in SV# '

# expected /(?^:^(?:foo=)?X\-Day\ \ß\ª\ÿ$)/

Cmd.exe uses the old DOS/OEM code pages by default (cp850 for me in a
British English setup), but if I switch to the Windows/ANSI code page
(cp1252 for me) then all tests pass, albeit with a "Wide character in
setenv" warning:

mode con cp select=1252

ok 154 - ENV store downgrades utf8 in setenv
# Wide character in setenv at op/magic.t line 649.
ok 155 - ENV store warns about wide characters
ok 156 - ENV store encodes high utf8 in SV
ok 157 - ENV store encodes high utf8 in SV


bulk88 at hotmail

Aug 2, 2012, 9:10 AM

Post #2 of 20 (827 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

> Date: Wed, 1 Aug 2012 22:11:12 +0100
> Subject: op/magic.t tests 154,157
> From: steve.m.hay [at] googlemail
> To: perl5-porters [at] perl
>
> I see op/magic.t failing tests 154 and 157 for me (Windows, VC++), and
> also in various other smoke reports (not just Windows).
>
> Is anyone looking into this?
>
I saw the failed smokes ( http://www.nntp.perl.org/group/perl.daily-build.reports/2012/08/msg124810.html ). I tried a couple days ago to compile a VC (dont remember if it was VC 03 or 08) blead, I got a bunch of other things failing in op/magic.t, I'm not sure if its my setup/my fault, but 154 and 157 didn't fail for me at that time, so I can't pursue it further unless I reproduce it. I see you are running NT 6 and VC 2010 (if its a NT 6 or VC CRT 2010 only problem I don't think I can ever debug it). I only have NT 5 machines at hand and a VC 2003 and a VC 2008. If I can reproduce it on my setups, I willing to try to tackle it with a C debugger. Timeframe wise I'm not sure if I'll get to it or someone like Jan or another Win32 guy will get to it first. Do you or anyone else know what commit started making op/magic.t 154/157 fail? I know there was in improvement to 5.17 a few months ago that added passing through high ascii CP specific chars from the parent perl's fake process (that is really a OS thread) ithreads "ENV" to child processes (system() for example).


Steve.Hay at verosoftware

Aug 2, 2012, 10:02 AM

Post #3 of 20 (828 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

bulk 88 wrote on 2012-08-02:
>
>
>
>> Date: Wed, 1 Aug 2012 22:11:12 +0100
>> Subject: op/magic.t tests 154,157
>> From: steve.m.hay [at] googlemail
>> To: perl5-porters [at] perl
>>
>> I see op/magic.t failing tests 154 and 157 for me (Windows, VC++),
and
>> also in various other smoke reports (not just Windows).
>>
>> Is anyone looking into this?
>>
> I saw the failed smokes ( http://www.nntp.perl.org/group/perl.daily-
> build.reports/2012/08/msg124810.html ). I tried a couple days ago to
> compile a VC (dont remember if it was VC 03 or 08) blead, I got a
> bunch of other things failing in op/magic.t, I'm not sure if its my
> setup/my fault, but 154 and 157 didn't fail for me at that time, so I
> can't pursue it further unless I reproduce it. I see you are running
> NT 6 and VC 2010 (if its a NT 6 or VC CRT 2010 only problem I don't
> think I can ever debug it). I only have NT 5 machines at hand and a VC
> 2003 and a VC 2008. If I can reproduce it on my setups, I willing to
> try to tackle it with a C debugger. Timeframe wise I'm not sure if
> I'll get to it or someone like Jan or another Win32 guy will get to it

I will try to look myself, but didn't want to waste time if someone else
is already looking...


> first. Do you or anyone else know what commit started making
> op/magic.t 154/157 fail? I know there was in improvement to 5.17 a few
> months ago that added passing through high ascii CP specific chars
> from the parent perl's fake process (that is really a OS thread)
> ithreads "ENV" to child processes (system() for example).

I will start with bisecting to find where it was introduced
(unfortunately I've not been doing many smokes recently, and I think it
started while I wasn't smoking...).

It can't be the GetEnvironmentStringsW() change (4f46e52b00) which you
refer to, though, because it happens on platforms other than Windows,
e.g. Debian linux
(http://www.nntp.perl.org/group/perl.daily-build.reports/2012/08/msg1248
64.html) and HP-UX
(http://www.nntp.perl.org/group/perl.daily-build.reports/2012/08/msg1248
59.html) and others...


craig.a.berry at gmail

Aug 2, 2012, 11:18 AM

Post #4 of 20 (828 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On Thu, Aug 2, 2012 at 12:02 PM, Steve Hay <Steve.Hay [at] verosoftware> wrote:


> I will start with bisecting to find where it was introduced
> (unfortunately I've not been doing many smokes recently, and I think it
> started while I wasn't smoking...).

My bet would be on:

<http://perl5.git.perl.org/perl.git/commitdiff/613c63b465f01af4e535fdc6c1c17e7470be5aad>


steve.m.hay at googlemail

Aug 2, 2012, 2:38 PM

Post #5 of 20 (829 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On 2 August 2012 19:18, Craig A. Berry <craig.a.berry [at] gmail> wrote:
> On Thu, Aug 2, 2012 at 12:02 PM, Steve Hay <Steve.Hay [at] verosoftware> wrote:
>
>
>> I will start with bisecting to find where it was introduced
>> (unfortunately I've not been doing many smokes recently, and I think it
>> started while I wasn't smoking...).
>
> My bet would be on:
>
> <http://perl5.git.perl.org/perl.git/commitdiff/613c63b465f01af4e535fdc6c1c17e7470be5aad>

Yes, you're spot on. The failing tests were added by that commit, and
all tests are successful in the previous commit.

As I found before, the results depend on the cmd.exe shell's current
code page, which is the OEM one by default and therefore doesn't match
the ANSI code page which perl.exe uses as its native character set...

The following samples reproduce and explain the first test failure
(#154 - 'ENV store downgrades utf8 in setenv'):

First, with my system's default (OEM) code page (cp850). The byte
\x{A0} in $b is manually upgraded to utf8 in $c (being interpreted as
the character U+00A0 because that's the character found at byte point
0xA0 in perl.exe's native character set, cp1252, on my system -- see
http://unicode.org/Public/MAPPINGS/VENDORS/MICSFT/WINDOWS/CP1252.TXT),
then downgraded again when stored in %ENV, so $f comes out identical
to $b (so test #153 'ENV store downgrades utf8 in SV' passes), but
when the output of cmd.exe's 'set' command is inspected it is found to
contain the byte \x{FF} instead of \x{A0}. This is because cmd.exe is
using cp850, in which character U+00A0 is found at byte point 0xFF
(see http://unicode.org/Public/MAPPINGS/VENDORS/MICSFT/PC/CP850.TXT):

perl.exe -MDevel::Peek -e "$b = qq[\x{A0}]; utf8::upgrade($c = $b); $f
= $ENV{foo} = $c; $e = `set foo`; Dump($b); Dump($c); Dump($f);
Dump($e)"
SV = PV(0x8f61c4) at 0x22a6d24
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x22ae0dc "\240"\0
CUR = 1
LEN = 12
SV = PV(0x8f61ec) at 0x22a6d64
REFCNT = 1
FLAGS = (POK,pPOK,UTF8)
PV = 0x22ae1cc "\302\240"\0 [UTF8 "\x{a0}"]
CUR = 2
LEN = 12
SV = PVMG(0x8ffbac) at 0x22a6d54
REFCNT = 1
FLAGS = (POK,pPOK)
IV = 0
NV = 0
PV = 0x22dd334 "\240"\0
CUR = 1
LEN = 12
SV = PV(0x8f6274) at 0x22a7064
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x22dd244 "foo=\377\n"\0
CUR = 6
LEN = 12

If I switch the cmd.exe code page to cp1252 to match perl.exe the it
all works fine because then there is no mix-up between perl.exe and
cmd.exe about which character is meant by byte \x{A0}:

perl.exe -MDevel::Peek -e "$b = qq[\x{A0}]; utf8::upgrade($c = $b); $f
= $ENV{foo} = $c; $e = `set foo`; Dump($b); Dump($c); Dump($f);
Dump($e)"
SV = PV(0x9f61c4) at 0x5c6d24
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x5ce0dc "\240"\0
CUR = 1
LEN = 12
SV = PV(0x9f61ec) at 0x5c6d64
REFCNT = 1
FLAGS = (POK,pPOK,UTF8)
PV = 0x5ce1cc "\302\240"\0 [UTF8 "\x{a0}"]
CUR = 2
LEN = 12
SV = PVMG(0x9ffbac) at 0x5c6d54
REFCNT = 1
FLAGS = (POK,pPOK)
IV = 0
NV = 0
PV = 0x5fd334 "\240"\0
CUR = 1
LEN = 12
SV = PV(0x9f6274) at 0x5c7064
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x5fd244 "foo=\240\n"\0
CUR = 6
LEN = 12

That's what I think is going on, and probably the other failure is
similar since switching cmd.exe code page also fixes that.

However, I'm not sure what is best to do about this. It is possible to
get the OEM and ANSI code pages on Windows and convert the OEM data
received from 'set' back to ANSI before comparing with the expected
value, but that clearly won't fix failures on other OSes...


nick at ccl4

Aug 3, 2012, 3:33 AM

Post #6 of 20 (827 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On Thu, Aug 02, 2012 at 10:38:58PM +0100, Steve Hay wrote:

> However, I'm not sure what is best to do about this. It is possible to
> get the OEM and ANSI code pages on Windows and convert the OEM data
> received from 'set' back to ANSI before comparing with the expected
> value, but that clearly won't fix failures on other OSes...

a) Which other OSes are failing? I've lost track
b) Assuming it's all "not Unix", seems likely that each is going to need a
different custom fix, because what the test has to do to actually test that
things worked is in OS-dependant territory.

Nicholas Clark


Steve.Hay at verosoftware

Aug 3, 2012, 3:43 AM

Post #7 of 20 (832 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

Nicholas Clark wrote on 2012-08-03:
> On Thu, Aug 02, 2012 at 10:38:58PM +0100, Steve Hay wrote:
>
>> However, I'm not sure what is best to do about this. It is possible
to
>> get the OEM and ANSI code pages on Windows and convert the OEM data
>> received from 'set' back to ANSI before comparing with the expected
>> value, but that clearly won't fix failures on other OSes...
>
> a) Which other OSes are failing? I've lost track
> b) Assuming it's all "not Unix", seems likely that each is going to
> need a
> different custom fix, because what the test has to do to actually
> test that things worked is in OS-dependant territory.
> Nicholas Clark

I haven't had time yet to trawl through enough smoke reports for a
complete answer, but I've seen these two tests failing on at least
Windows, Debian Linux and HP-UX, so it's already not "all "not Unix"".


tony at develop-help

Aug 3, 2012, 8:59 PM

Post #8 of 20 (822 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On Fri, Aug 03, 2012 at 11:33:52AM +0100, Nicholas Clark wrote:
> On Thu, Aug 02, 2012 at 10:38:58PM +0100, Steve Hay wrote:
>
> > However, I'm not sure what is best to do about this. It is possible to
> > get the OEM and ANSI code pages on Windows and convert the OEM data
> > received from 'set' back to ANSI before comparing with the expected
> > value, but that clearly won't fix failures on other OSes...
>
> a) Which other OSes are failing? I've lost track

It failed in Linux in a unicode environment on Linux, eg.

PERL_UNICODE= LC_ALL=en_AU.utf8 ./perl op/magic.t

I've pushed e2e1d5ce8 to fix that.

This doesn't fix the problem on Win32, but that appears to be a
different issue.

Tony


steve.m.hay at googlemail

Aug 4, 2012, 4:34 AM

Post #9 of 20 (824 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On 4 August 2012 04:59, Tony Cook <tony [at] develop-help> wrote:
> On Fri, Aug 03, 2012 at 11:33:52AM +0100, Nicholas Clark wrote:
>> On Thu, Aug 02, 2012 at 10:38:58PM +0100, Steve Hay wrote:
>>
>> > However, I'm not sure what is best to do about this. It is possible to
>> > get the OEM and ANSI code pages on Windows and convert the OEM data
>> > received from 'set' back to ANSI before comparing with the expected
>> > value, but that clearly won't fix failures on other OSes...
>>
>> a) Which other OSes are failing? I've lost track
>
> It failed in Linux in a unicode environment on Linux, eg.
>
> PERL_UNICODE= LC_ALL=en_AU.utf8 ./perl op/magic.t
>
> I've pushed e2e1d5ce8 to fix that.
>
> This doesn't fix the problem on Win32, but that appears to be a
> different issue.

Indeed. In fact it seems to have made things worse on Win32 - I now
have 150-152, 154, 157, 161, 163 all failing, Although it is at least
more consistent now, in that I get the same failures with either cp850
or cp1252!...


steve.m.hay at googlemail

Aug 4, 2012, 6:09 AM

Post #10 of 20 (821 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On 4 August 2012 12:34, Steve Hay <steve.m.hay [at] googlemail> wrote:
> On 4 August 2012 04:59, Tony Cook <tony [at] develop-help> wrote:
>> On Fri, Aug 03, 2012 at 11:33:52AM +0100, Nicholas Clark wrote:
>>> On Thu, Aug 02, 2012 at 10:38:58PM +0100, Steve Hay wrote:
>>>
>>> > However, I'm not sure what is best to do about this. It is possible to
>>> > get the OEM and ANSI code pages on Windows and convert the OEM data
>>> > received from 'set' back to ANSI before comparing with the expected
>>> > value, but that clearly won't fix failures on other OSes...
>>>
>>> a) Which other OSes are failing? I've lost track
>>
>> It failed in Linux in a unicode environment on Linux, eg.
>>
>> PERL_UNICODE= LC_ALL=en_AU.utf8 ./perl op/magic.t
>>
>> I've pushed e2e1d5ce8 to fix that.
>>
>> This doesn't fix the problem on Win32, but that appears to be a
>> different issue.
>
> Indeed. In fact it seems to have made things worse on Win32 - I now
> have 150-152, 154, 157, 161, 163 all failing, Although it is at least
> more consistent now, in that I get the same failures with either cp850
> or cp1252!...

Commit 5ceaabe8e4 fixes the new Windows failures, which were just the
result of :raw input containing unexpected CR characters.

(It's a shame that one can't specify ":raw:crlf" with the open pragma
-- it just fails to find the PerlIO layer "raw:crlf". Could the pragma
be extended to allow a stack of layers to be given?)

I think that just leaves 154 and 157 failing on Windows when the
command prompt uses a different code page to perl, which it sadly does
by default on almost all systems.

I was intending to call GetOEMCP() and GetACP() to check if the code
pages different and convert the encoding if necessary, but
unfortunately I don't see interfaces to those functions in the Win32
module. I guess I normally use Win32::API to call them when I've done
similar things in the past...

Jan: Could we add those two functions to your Win32 module (they're
both very trivial, but rather useful, I find), or is there a better
approach to fixing this?


tony at develop-help

Aug 4, 2012, 11:57 PM

Post #11 of 20 (821 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On Sat, Aug 04, 2012 at 02:09:40PM +0100, Steve Hay wrote:
> On 4 August 2012 12:34, Steve Hay <steve.m.hay [at] googlemail> wrote:
> > On 4 August 2012 04:59, Tony Cook <tony [at] develop-help> wrote:
> >> This doesn't fix the problem on Win32, but that appears to be a
> >> different issue.
> >
> > Indeed. In fact it seems to have made things worse on Win32 - I now
> > have 150-152, 154, 157, 161, 163 all failing, Although it is at least
> > more consistent now, in that I get the same failures with either cp850
> > or cp1252!...
>
> Commit 5ceaabe8e4 fixes the new Windows failures, which were just the
> result of :raw input containing unexpected CR characters.

Thanks, looking at my open files in Emacs it looks like I was
modifying my cygwin checkout instead of my Win32 checkout when I
tested this on Win32. Doh.

Tony


jand at activestate

Aug 6, 2012, 1:03 PM

Post #12 of 20 (810 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

On Sat, 04 Aug 2012, Steve Hay wrote:
> I was intending to call GetOEMCP() and GetACP() to check if the code
> pages different and convert the encoding if necessary, but
> unfortunately I don't see interfaces to those functions in the Win32
> module. I guess I normally use Win32::API to call them when I've done
> similar things in the past...
>
> Jan: Could we add those two functions to your Win32 module (they're
> both very trivial, but rather useful, I find), or is there a better
> approach to fixing this?

Yes, I think this is a good idea. Gisle asked me about adding GetACP()
too already for Encode::Locale, which currently uses, Win32::API,
Win32::Console, or qx(chcp) to determine the information.

I've created a ticket for it:

https://rt.cpan.org/Ticket/Display.html?id=78820

Cheers,
-Jan


Steve.Hay at verosoftware

Aug 7, 2012, 10:05 AM

Post #13 of 20 (799 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

Jan Dubois wrote on 2012-08-06:
> On Sat, 04 Aug 2012, Steve Hay wrote:
>> I was intending to call GetOEMCP() and GetACP() to check if the code
>> pages different and convert the encoding if necessary, but
>> unfortunately I don't see interfaces to those functions in the Win32
>> module. I guess I normally use Win32::API to call them when I've
>> done similar things in the past...
>>
>> Jan: Could we add those two functions to your Win32 module (they're
>> both very trivial, but rather useful, I find), or is there a better
>> approach to fixing this?
>
> Yes, I think this is a good idea. Gisle asked me about adding
> GetACP() too already for Encode::Locale, which currently uses,
> Win32::API, Win32::Console, or qx(chcp) to determine the information.
>
> I've created a ticket for it:
>
> https://rt.cpan.org/Ticket/Display.html?id=78820


Thanks, I've just attached a patch to that ticket to add the necessary functions, plus a couple more to fix this test script more correctly and easily :-)

With that patch dropped into my bleadperl workspace, I can now get the op/magic.t test to pass using the following change to it. I've tested this with VC10 and GCC.

Is there any chance of a new release of Win32 with this or something similar in it soonish? It would be nice to have it fixed for 5.17.3 in just under a fortnight's time.


diff --git a/t/op/magic.t b/t/op/magic.t
index ebf99a7..990791d 100644
--- a/t/op/magic.t
+++ b/t/op/magic.t
@@ -70,7 +70,11 @@ sub env_is {
if ($Is_MSWin32) {
# cmd.exe will echo 'variable=value' but 4nt will echo just the value
# -- Nikola Knezevic
+ require Win32;
+ my $cp = Win32::GetConsoleOutputCP();
+ Win32::SetConsoleOutputCP(Win32::GetACP());
(my $set = `set $key`) =~ s/\r\n$/\n/;
+ Win32::SetConsoleOutputCP($cp);
like $set, qr/^(?:\Q$key\E=)?\Q$val\E$/, $desc;
} elsif ($Is_VMS) {
is `write sys\$output f\$trnlnm("\Q$key\E")`, "$val\n", $desc;


jand at activestate

Aug 15, 2012, 12:55 AM

Post #14 of 20 (770 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

On Tue, 07 Aug 2012, Steve Hay wrote:
>> I've created a ticket for it:
>>
>> https://rt.cpan.org/Ticket/Display.html?id=78820
>
> Thanks, I've just attached a patch to that ticket to add the necessary
> functions, plus a couple more to fix this test script more correctly
> and easily :-)
>
> With that patch dropped into my bleadperl workspace, I can now get the
> op/magic.t test to pass using the following change to it. I've tested
> this with VC10 and GCC.
>
> Is there any chance of a new release of Win32 with this or something
> similar in it soonish? It would be nice to have it fixed for 5.17.3 in
> just under a fortnight's time.

I've released Win32-0.45 with your changes (and a fix for the weird
Cygwin readdir() behavior). I've already merged it into blead. I've
switched to Unix line-endings for the CPAN release, so you need to
ignore whitespace changes if you want to look at the commit:

git show -w c3c0674

While testing I noticed that blead no longer compiles with VC6. The
breakage started in commit bb02a38:

..\pp_ctl.c(1976) : error C2105: '++' needs l-value
..\pp_ctl.c(1979) : error C2105: '--' needs l-value
..\pp_ctl.c(1988) : error C2105: '++' needs l-value
..\pp_ctl.c(2885) : error C2105: '++' needs l-value

Ok, so that one is fixed by commit 1bd3586, but even after cherry-
picking that on top of bb02a38 I get:

..\sv.c(900) : error C2099: initializer is not a constant
..\sv.c(901) : error C2099: initializer is not a constant
..\sv.c(904) : error C2099: initializer is not a constant
..\sv.c(906) : error C2099: initializer is not a constant
..\sv.c(907) : error C2099: initializer is not a constant
..\sv.c(910) : error C2099: initializer is not a constant
..\sv.c(912) : error C2099: initializer is not a constant
..\sv.c(913) : error C2099: initializer is not a constant
..\sv.c(916) : error C2099: initializer is not a constant

This looks like a compiler bug. :( I can work around it by using an
anonymous union instead:

diff --git a/cv.h b/cv.h
index e8cb162..7e7c935 100644
--- a/cv.h
+++ b/cv.h
@@ -66,7 +66,7 @@ S_CvDEPTHp(const CV * const sv)
{
return SvTYPE(sv) == SVt_PVCV
? &((XPVCV*)SvANY(sv))->xcv_depth
- : &((XPVCV*)SvANY(sv))->xpv_cur_u.xpvcuru_fmdepth;
+ : &((XPVCV*)SvANY(sv))->xpvcuru_fmdepth;
}
#if defined(__GNUC__) && !defined(PERL_GCC_BRACE_GROUPS_FORBIDDEN)
# define CvDEPTH(sv) (*({const CV *const _cvdepth = (const CV *)sv; \
diff --git a/sv.h b/sv.h
index b69979f..e255c63 100644
--- a/sv.h
+++ b/sv.h
@@ -418,10 +418,10 @@ perform the upgrade if necessary. See C<svtype>.
union { \
STRLEN xpvcuru_cur; /* length of svu_pv as a C string */ \
I32 xpvcuru_fmdepth; \
- } xpv_cur_u; \
+ }; \
STRLEN xpv_len /* allocated size */

-#define xpv_cur xpv_cur_u.xpvcuru_cur
+#define xpv_cur xpvcuru_cur

union _xnvu {
NV xnv_nv; /* numeric value, if any */

I don't really like making this change to deal with a compiler issue,
but I do want to keep Perl working with VC6 (the last version of VisualC
that links against MSVCRT.dll and not against a version specific CRT).

Cheers,
-Jan


Steve.Hay at verosoftware

Aug 15, 2012, 1:11 AM

Post #15 of 20 (771 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

Jan Dubois wrote on 2012-08-15:
> On Tue, 07 Aug 2012, Steve Hay wrote:
>>> I've created a ticket for it:
>>>
>>> https://rt.cpan.org/Ticket/Display.html?id=78820
>>
>> Thanks, I've just attached a patch to that ticket to add the necessary
>> functions, plus a couple more to fix this test script more correctly
>> and easily :-)
>>
>> With that patch dropped into my bleadperl workspace, I can now get the
>> op/magic.t test to pass using the following change to it. I've tested
>> this with VC10 and GCC.
>>
>> Is there any chance of a new release of Win32 with this or something
>> similar in it soonish? It would be nice to have it fixed for 5.17.3 in
>> just under a fortnight's time.
>
> I've released Win32-0.45 with your changes (and a fix for the weird
> Cygwin readdir() behavior). I've already merged it into blead. I've
> switched to Unix line-endings for the CPAN release, so you need to
> ignore whitespace changes if you want to look at the commit:
>
> git show -w c3c0674

Many thanks for doing this. I will commit my t/op/magic.t fix using the new APIs very soon.


> While testing I noticed that blead no longer compiles with VC6. The
> breakage started in commit bb02a38:
>
> ..\pp_ctl.c(1976) : error C2105: '++' needs l-value
> ..\pp_ctl.c(1979) : error C2105: '--' needs l-value
> ..\pp_ctl.c(1988) : error C2105: '++' needs l-value
> ..\pp_ctl.c(2885) : error C2105: '++' needs l-value
>
> Ok, so that one is fixed by commit 1bd3586, but even after cherry-
> picking that on top of bb02a38 I get:
>
> ..\sv.c(900) : error C2099: initializer is not a constant
> ..\sv.c(901) : error C2099: initializer is not a constant
> ..\sv.c(904) : error C2099: initializer is not a constant
> ..\sv.c(906) : error C2099: initializer is not a constant
> ..\sv.c(907) : error C2099: initializer is not a constant
> ..\sv.c(910) : error C2099: initializer is not a constant
> ..\sv.c(912) : error C2099: initializer is not a constant
> ..\sv.c(913) : error C2099: initializer is not a constant
> ..\sv.c(916) : error C2099: initializer is not a constant
>
> This looks like a compiler bug. :( I can work around it by using an
> anonymous union instead:
>
> diff --git a/cv.h b/cv.h
> index e8cb162..7e7c935 100644
> --- a/cv.h
> +++ b/cv.h
> @@ -66,7 +66,7 @@ S_CvDEPTHp(const CV * const sv) {
> return SvTYPE(sv) == SVt_PVCV
> ? &((XPVCV*)SvANY(sv))->xcv_depth
> - : &((XPVCV*)SvANY(sv))->xpv_cur_u.xpvcuru_fmdepth;
> + : &((XPVCV*)SvANY(sv))->xpvcuru_fmdepth;
> }
> #if defined(__GNUC__) && !defined(PERL_GCC_BRACE_GROUPS_FORBIDDEN)
> # define CvDEPTH(sv) (*({const CV *const _cvdepth = (const CV *)sv;
> \ diff --git a/sv.h b/sv.h index b69979f..e255c63 100644
> --- a/sv.h
> +++ b/sv.h
> @@ -418,10 +418,10 @@ perform the upgrade if necessary. See C<svtype>.
> union {
> \
> STRLEN xpvcuru_cur; /* length of svu_pv as a C string */ \
> I32 xpvcuru_fmdepth;
> \
> - } xpv_cur_u;
> \
> + }; \
> STRLEN xpv_len /* allocated size */
> -#define xpv_cur xpv_cur_u.xpvcuru_cur
> +#define xpv_cur xpvcuru_cur
>
> union _xnvu {
> NV xnv_nv; /* numeric value, if any */
> I don't really like making this change to deal with a compiler issue,
> but I do want to keep Perl working with VC6 (the last version of
> VisualC that links against MSVCRT.dll and not against a version
> specific CRT).

I'm certainly in favour of maintaining support for VC6, but have been neglecting to test it recently so hadn't noticed this. I won't be able to test your patch myself with VC6 until this evening, but I wouldn't expect to have any problem if it works for you, so feel free to commit any time :-)


nick at ccl4

Aug 15, 2012, 1:28 AM

Post #16 of 20 (771 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On Wed, Aug 15, 2012 at 09:11:35AM +0100, Steve Hay wrote:
> Jan Dubois wrote on 2012-08-15:

> > This looks like a compiler bug. :( I can work around it by using an
> > anonymous union instead:

> > I don't really like making this change to deal with a compiler issue,
> > but I do want to keep Perl working with VC6 (the last version of
> > VisualC that links against MSVCRT.dll and not against a version
> > specific CRT).
>

> I'm certainly in favour of maintaining support for VC6, but have been
> neglecting to test it recently so hadn't noticed this. I won't be able to
> test your patch myself with VC6 until this evening, but I wouldn't expect
> to have any problem if it works for you, so feel free to commit any time
> :-)

Me too, but an anonymous union isn't C89, so it's going to break on various
(other) compilers. IIRC Dave inadvertently used them when refactoring the
regex code to be non-recursive, and the smoke signals at the time were clear.

I guess doing it conditionally for VC would keep all the compilers happy.
(But make the humans more upset)

Nicholas Clark


steve.m.hay at googlemail

Aug 17, 2012, 4:35 PM

Post #17 of 20 (764 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On 15 August 2012 09:28, Nicholas Clark <nick [at] ccl4> wrote:
> On Wed, Aug 15, 2012 at 09:11:35AM +0100, Steve Hay wrote:
>> Jan Dubois wrote on 2012-08-15:
>
>> > This looks like a compiler bug. :( I can work around it by using an
>> > anonymous union instead:
>
>> > I don't really like making this change to deal with a compiler issue,
>> > but I do want to keep Perl working with VC6 (the last version of
>> > VisualC that links against MSVCRT.dll and not against a version
>> > specific CRT).
>>
>
>> I'm certainly in favour of maintaining support for VC6, but have been
>> neglecting to test it recently so hadn't noticed this. I won't be able to
>> test your patch myself with VC6 until this evening, but I wouldn't expect
>> to have any problem if it works for you, so feel free to commit any time
>> :-)
>
> Me too, but an anonymous union isn't C89, so it's going to break on various
> (other) compilers. IIRC Dave inadvertently used them when refactoring the
> regex code to be non-recursive, and the smoke signals at the time were clear.
>
> I guess doing it conditionally for VC would keep all the compilers happy.
> (But make the humans more upset)
>

I've just applied the patch with, suitable checks for VC++ 6 only, in
commit f28d8eb1c1.

It's not pretty, but I confirmed the VC6 build was broken without it,
and fixed again with it. Thanks for the spot and for the patch.


sprout at cpan

Aug 17, 2012, 4:56 PM

Post #18 of 20 (766 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

Steve Hay wrote:
> I've just applied the patch with, suitable checks for VC++ 6 only, in
> commit f28d8eb1c1.

Does the shorter version on the sprout/xpvcuru branch work?


steve.m.hay at googlemail

Aug 17, 2012, 5:42 PM

Post #19 of 20 (765 views)
Permalink
Re: op/magic.t tests 154,157 [In reply to]

On 18 August 2012 00:56, Father Chrysostomos <sprout [at] cpan> wrote:
> Steve Hay wrote:
>> I've just applied the patch with, suitable checks for VC++ 6 only, in
>> commit f28d8eb1c1.
>
> Does the shorter version on the sprout/xpvcuru branch work?
>

Yes it does. Thank you!


jand at activestate

Aug 17, 2012, 5:56 PM

Post #20 of 20 (765 views)
Permalink
RE: op/magic.t tests 154,157 [In reply to]

On Fri, 17 Aug 2012, Steve Hay wrote:
> On 18 August 2012 00:56, Father Chrysostomos <sprout [at] cpan> wrote:
>> Steve Hay wrote:
>>> I've just applied the patch with, suitable checks for VC++ 6 only,
>>> in commit f28d8eb1c1.
>>
>> Does the shorter version on the sprout/xpvcuru branch work?
>>
>
> Yes it does. Thank you!

You may want to further sanitize this by defining in sv.h (pseudocode,
should be part of existing conditional):

#if VC6
# define xpv_fmdepth xpvcuru_fmdepth
#else
# define xpv_fmdepth xpv_cur_u.xpvcuru_fmdepth
#endif

and then just use xpv_fmdepth in cv.h instead of having an undocumented
compiler conditional in that place as well.

Sorry, but I'm not set up right now to do and test this myself.

Cheers,
-Jan

Perl porters RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.