Skip Menu |

This queue is for tickets about the version CPAN distribution.

Report information
The Basics
Id: 92051
Status: resolved
Priority: 0/
Queue: version

People
Owner: Nobody in particular
Requestors: dexter [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.9906
Fixed in: (no value)



Subject: [PATCH] Android doesn't support POSIX::localeconv()
version module doesn't work on Android. Its libc doesn't provide localeconf() function. This is patch: --- version-0.9906/t/07locale.t +++ version-0.9906_01/t/07locale.t @@ -36,10 +36,10 @@ while (<DATA>) { chomp; $loc = setlocale( LC_ALL, $_); - last if $loc && localeconv()->{decimal_point} eq ','; + last if $loc && eval { localeconv()->{decimal_point} eq ',' }; } skip 'Cannot test locale handling without a comma locale', 5 - unless $loc and localeconv()->{decimal_point} eq ','; + unless $loc and eval { localeconv()->{decimal_point} eq ',' }; setlocale(LC_NUMERIC, $loc); $ver = 1.23; # has to be floating point number --- version-0.9906/t/survey_locales +++ version-0.9906_01/t/survey_locales @@ -9,7 +9,7 @@ while (<$LOCALES>) { chomp; $loc = setlocale( LC_ALL, $_); - print $_ if localeconv()->{decimal_point} eq ','; + print $_ if eval { localeconv()->{decimal_point} eq ',' }; } close $LOCALES; $loc = setlocale( LC_ALL, $orig_loc); --- version-0.9906/vperl/vpp.pm +++ version-0.9906_01/vperl/vpp.pm @@ -654,12 +654,12 @@ return $self; } - my $currlocale = setlocale(LC_ALL); + my $currlocale = do { local $@; local $SIG{__DIE__}; eval { setlocale(LC_ALL) } }; # if the current locale uses commas for decimal points, we # just replace commas with decimal places, rather than changing # locales - if ( localeconv()->{decimal_point} eq ',' ) { + if ( do { local $@; local $SIG{__DIE__}; eval { localeconv()->{decimal_point} eq ',' } } ) { $value =~ tr/,/./; }
It would probably be a good idea to smoke all of cpan on android, to see what issues there are, and then come up with a more general strategy for dealing with them -- wrapping eval {} arund lots of things seems suboptimal. Checking the POSIX docs for android would be a good start, too.
From: piotr.roszatycki [...] gmail.com
Android has very poor support for locales. Even with LC_ALL=C Perl warns about missing locales: perl: warning: Please check that your locale settings: I've tested this module with Perl on Android compiled with -Ui_locale -Ud_setlocale flags Then it is not possible to use locale module: Your vendor does not support locales, you cannot use the locale module. at /data/perl/lib/5.18.2/locale.pm line 75. It means that vpp shouldn't load this module at all. Patch: --- version-0.9906/t/07locale.t +++ version-0.9906_02/t/07locale.t @@ -26,20 +26,19 @@ my $ver = 1.23; # has to be floating point number my $loc; - my $orig_loc = setlocale(LC_NUMERIC); + my $orig_loc = eval { setlocale(LC_NUMERIC) }; ok ($ver eq "1.23", 'Not using locale yet'); # Don't use is(), # because have to # evaluate in current # scope - use locale; while (<DATA>) { chomp; $loc = setlocale( LC_ALL, $_); - last if $loc && localeconv()->{decimal_point} eq ','; + last if $loc && eval { localeconv()->{decimal_point} eq ',' }; } skip 'Cannot test locale handling without a comma locale', 5 - unless $loc and localeconv()->{decimal_point} eq ','; + unless $loc and eval { localeconv()->{decimal_point} eq ',' }; setlocale(LC_NUMERIC, $loc); $ver = 1.23; # has to be floating point number --- version-0.9906/t/survey_locales +++ version-0.9906_02/t/survey_locales @@ -9,7 +9,7 @@ while (<$LOCALES>) { chomp; $loc = setlocale( LC_ALL, $_); - print $_ if localeconv()->{decimal_point} eq ','; + print $_ if eval { localeconv()->{decimal_point} eq ',' }; } close $LOCALES; $loc = setlocale( LC_ALL, $orig_loc); --- version-0.9906/vperl/vpp.pm +++ version-0.9906_02/vperl/vpp.pm @@ -121,7 +121,6 @@ use strict; use POSIX qw/locale_h/; -use locale; use vars qw($VERSION $CLASS @ISA $LAX $STRICT); $VERSION = 0.9906; $CLASS = 'version::vpp'; @@ -654,12 +653,12 @@ return $self; } - my $currlocale = setlocale(LC_ALL); + my $currlocale = do { local $@; local $SIG{__DIE__}; eval { setlocale(LC_ALL) } }; # if the current locale uses commas for decimal points, we # just replace commas with decimal places, rather than changing # locales - if ( localeconv()->{decimal_point} eq ',' ) { + if ( do { local $@; local $SIG{__DIE__}; eval { localeconv()->{decimal_point} eq ',' } } ) { $value =~ tr/,/./; }
From: piotr.roszatycki [...] gmail.com
On Sob 11 Sty 2014, 13:41:00, ETHER wrote: Show quoted text
> It would probably be a good idea to smoke all of cpan on android, to > see what issues there are, and then come up with a more general > strategy for dealing with them -- wrapping eval {} arund lots of > things seems suboptimal.
It is very good idea but it has one backdraw: cpanminus requires version.pm to its work so it have to be fixed in the first place. Without cpanminus it is more difficult to test CPAN modules. Another module to fix is ExtUtils::MakeMaker and I'm just sending the patch to github for this module.
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Sat, 11 Jan 2014 17:17:54 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/11/2014 10:20 AM, Piotr Roszatycki via RT wrote: Show quoted text
> Sat Jan 11 10:20:10 2014: Request 92051 was acted upon. > Transaction: Ticket created by DEXTER > Queue: version > Subject: [PATCH] Android doesn't support POSIX::localeconv() > Broken in: 0.9906 > Severity: Important > Owner: Nobody > Requestors: dexter@cpan.org > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=92051 > > > > version module doesn't work on Android. Its libc doesn't provide localeconf() function.
Can you show me the output of this? perl -MConfig -e 'print $Config{d_setlocale};' There's really no point in testing locale support if we know ahead of time that it isn't supported fully (hence I don't like your first patch). If I can figure out some way to detect this situation, I will just skip the tests entirely and change version:vpp to skip the locale testing in the same way. John
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Sat, 11 Jan 2014 23:50:31 +0100
To: bug-version [...] rt.cpan.org
From: Piotr Roszatycki <dexter [...] cpan.org>
2014/1/11 John Peacock via RT <bug-version@rt.cpan.org>: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=92051 > > Can you show me the output of this? > > perl -MConfig -e 'print $Config{d_setlocale};' > > There's really no point in testing locale support if we know ahead of > time that it isn't supported fully (hence I don't like your first > patch). If I can figure out some way to detect this situation, I will > just skip the tests entirely and change version:vpp to skip the locale > testing in the same way.
There are two cases: $ perl -V:d_setlocale d_setlocale='undef'; Then POSIX's setlocale, localeconv, LC_ALL, LC_NUMBERIC, are failed Another case: $ perl -V:d_setlocale d_setlocale='defined'; but: $ grep -A3 -B3 localeconv /data/data/com.pdaxrom.cctools/root/cctools/arm-linux-androideabi/include/locale.h #if 1 /* MISSING FROM BIONIC - DEFINED TO MAKE libstdc++-v3 happy */ struct lconv { }; struct lconv *localeconv(void); #endif /* MISSING */ so localeconv is failed
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Sun, 12 Jan 2014 10:59:56 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/11/2014 05:50 PM, dexter@cpan.org via RT wrote: Show quoted text
> There are two cases: > > $ perl -V:d_setlocale > d_setlocale='undef'; > > Then POSIX's setlocale, localeconv, LC_ALL, LC_NUMBERIC, are failed > > Another case: > > $ perl -V:d_setlocale > d_setlocale='defined'; > > but: > > $ grep -A3 -B3 localeconv > /data/data/com.pdaxrom.cctools/root/cctools/arm-linux-androideabi/include/locale.h > > #if 1 /* MISSING FROM BIONIC - DEFINED TO MAKE libstdc++-v3 happy */ > struct lconv { }; > struct lconv *localeconv(void); > #endif /* MISSING */ > > so localeconv is failed >
That is pretty unhelpful to say the least. I have installed Android4.3 x86 in VirtualBox. Can someone point me at documentation to go about installing Perl (and the toolchain required to build it presumably) and I can take a look at it? For now, I'm going to say that version.pm is unsupportable on Android, since I cannot remove locale support from vpp like that; that would break everything that does locales reasonably. I have a bunch of other fixes I'd like to release now, so I'm not going to wait on Android. If you can provide a way to determine whether the Makefile.PL is running on Android, I can look at adding a patch step for that platform only. But that is a horrible hack... John
From: dexter [...] cpan.org
On Nd 12 Sty 2014, 11:00:11, john.peacock@havurah-software.org wrote: Show quoted text
> That is pretty unhelpful to say the least. I have installed > Android4.3 > x86 in VirtualBox. Can someone point me at documentation to go about > installing Perl (and the toolchain required to build it presumably) > and > I can take a look at it?
https://github.com/dex4er/perl5/wiki/Installing-Perl-on-Android The binaries are also avaiable. I've changed $^O from 'linux' to 'android', as far as there is so many changes between standard linux+glibc and android+bionic. It is similar to cygwin and MSWin32 - the same OS and different libc I think you can assume that d_setlocale='undef' as far as Android has broken this mechanizm. Then I need just to not load "locale" module. It simpifies the whole patch: --- version-0.9906/t/07locale.t +++ version-0.9906_03/t/07locale.t @@ -31,7 +31,6 @@ # because have to # evaluate in current # scope - use locale; while (<DATA>) { chomp; --- version-0.9906/vperl/vpp.pm +++ version-0.9906_03/vperl/vpp.pm @@ -120,8 +120,8 @@ use 5.005_04; use strict; +use Config; use POSIX qw/locale_h/; -use locale; use vars qw($VERSION $CLASS @ISA $LAX $STRICT); $VERSION = 0.9906; $CLASS = 'version::vpp'; @@ -654,13 +654,15 @@ return $self; } - my $currlocale = setlocale(LC_ALL); + if ($Config{d_setlocale}) { + my $currlocale = setlocale(LC_ALL); - # if the current locale uses commas for decimal points, we - # just replace commas with decimal places, rather than changing - # locales - if ( localeconv()->{decimal_point} eq ',' ) { - $value =~ tr/,/./; + # if the current locale uses commas for decimal points, we + # just replace commas with decimal places, rather than changing + # locales + if ( localeconv()->{decimal_point} eq ',' ) { + $value =~ tr/,/./; + } } if ( not defined $value or $value =~ /^undef$/ ) {
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Sun, 12 Jan 2014 20:00:14 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/12/2014 05:54 PM, Piotr Roszatycki via RT wrote: Show quoted text
> The binaries are also avaiable. > > I've changed $^O from 'linux' to 'android', as far as there is so many changes between standard linux+glibc and android+bionic. It is similar to cygwin and MSWin32 - the same OS and different libc > > I think you can assume that d_setlocale='undef' as far as Android has broken this mechanizm. > > Then I need just to not load "locale" module. It simpifies the whole patch:
As I said, I will not be globally removing the 'use locale' at this time, because that will break every other platform. If you set $^O to something like 'android' like your other thread suggests, I will simply make those changes in Makefile.PL for android only, and skip the locale tests entirely. 'use locale' is technically a pragma, not actually a module, so it should be possible to leave it in and only enable it in that block. If that is not possible, I can do something "clever" instead. JohN
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Sun, 12 Jan 2014 20:49:07 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/12/2014 08:00 PM, John Peacock via RT wrote: Show quoted text
> something like 'android' like your other thread suggests, I will simply > make those changes in Makefile.PL for android only, and skip the locale > tests entirely.
I've pushed changes to the version repo(s): ssh://hg@bitbucket.org/jpeacock/version/ (master) git+ssh://git@github.com/JohnPeacock/version.git (mirror) that should fix the problems with Android. It assumes that $^O matches /android/ and Makefile.PL just strips out the offending 'use locale'. If you can let me know if this works for you, I will release to CPAN... John
From: piotr.roszatycki [...] gmail.com
Show quoted text
> git+ssh://git@github.com/JohnPeacock/version.git (mirror)
github mirror does work. I couldn't clone bitbucket's repo. Show quoted text
> If you can let me know if this works for you, I will release to CPAN...
cc -c -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -DVERSION=\"0.9907\" -DXS_VERSION=\"0.9907\" -fPIC "-I/data/data/com.pdaxrom.cctools/root/cctools/lib/perl5/5.18.2/armv7l-android/CORE" vutil.c In file included from /data/data/com.pdaxrom.cctools/root/cctools/lib/perl5/5.18.2/armv7l-android/CORE/perl.h:5156:0, from vutil.c:7: vutil.c: In function 'Perl_upg_version2': vutil.h:229:31: error: 'LC_NUMERIC' undeclared (first use in this function) char *loc = savepv(setlocale(LC_NUMERIC, NULL)); \ ^ /data/data/com.pdaxrom.cctools/root/cctools/lib/perl5/5.18.2/armv7l-android/CORE/embed.h:523:38: note: in definition of macro 'savepv' #define savepv(a) Perl_savepv(aTHX_ a) ^ vutil.c:566:9: note: in expansion of macro 'STORE_NUMERIC_LOCAL_SET_STANDARD' STORE_NUMERIC_LOCAL_SET_STANDARD(); ^ vutil.h:229:31: note: each undeclared identifier is reported only once for each function it appears in char *loc = savepv(setlocale(LC_NUMERIC, NULL)); \ ^ /data/data/com.pdaxrom.cctools/root/cctools/lib/perl5/5.18.2/armv7l-android/CORE/embed.h:523:38: note: in definition of macro 'savepv' #define savepv(a) Perl_savepv(aTHX_ a) ^ vutil.c:566:9: note: in expansion of macro 'STORE_NUMERIC_LOCAL_SET_STANDARD' STORE_NUMERIC_LOCAL_SET_STANDARD(); ^ Makefile:314: recipe for target 'vutil.o' failed make[1]: *** [vutil.o] Error 1 The previous version with my patch compiled cleanly. I'll try to investigate.
From: piotr.roszatycki [...] gmail.com
Seems that you should check USE_LOCALE https://github.com/JohnPeacock/version/pull/1 make test passes :) Thanks :)
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Mon, 13 Jan 2014 06:24:40 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/13/2014 06:06 AM, Piotr Roszatycki via RT wrote: Show quoted text
> Queue: version > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=92051 > > > Seems that you should check USE_LOCALE > > https://github.com/JohnPeacock/version/pull/1 > > make test passes :) Thanks :) >
Applied and also made changes suggested by Aristotle (though I object to them in principle). You will probably have better luck with this URL for the mercurial repo: https://jpeacock@bitbucket.org/jpeacock/version unless you have an account and ssh key configured (that was my fault). John
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Mon, 13 Jan 2014 12:38:58 +0100
To: bug-version [...] rt.cpan.org
From: Piotr Roszatycki <dexter [...] cpan.org>
Ah sorry, I didn't noticed that it is mercurial repo. Unfortunately there is no mercurial for Android so I'll stay with Github. Thanks for your work.
Fixed in 0.9907
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Mon, 13 Jan 2014 07:05:21 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/13/2014 06:39 AM, dexter@cpan.org via RT wrote: Show quoted text
> Queue: version > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=92051 > > > Ah sorry, I didn't noticed that it is mercurial repo. Unfortunately > there is no mercurial for Android so I'll stay with Github. > > Thanks for your work. >
Unless I hear otherwise, I'm going to close this last bug and release 0.9907 this evening (EST). John
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Mon, 13 Jan 2014 13:20:56 +0100
To: bug-version [...] rt.cpan.org
From: Piotr Roszatycki <dexter [...] cpan.org>
2014/1/13 John Peacock via RT <bug-version@rt.cpan.org>: Show quoted text
> Unless I hear otherwise, I'm going to close this last bug and release > 0.9907 this evening (EST).
Cool. Everything compiled and test passed. cpanminus works correctly
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Tue, 14 Jan 2014 21:00:20 -0500
To: bug-version [...] rt.cpan.org, undisclosed-recipients:;
From: John Peacock <john.peacock [...] havurah-software.org>
On 01/13/2014 07:21 AM, dexter@cpan.org via RT wrote: Show quoted text
> Queue: version > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=92051 > > > 2014/1/13 John Peacock via RT <bug-version@rt.cpan.org>:
>> Unless I hear otherwise, I'm going to close this last bug and release >> 0.9907 this evening (EST).
> > Cool. > > Everything compiled and test passed. cpanminus works correctly >
Can I ask you to pull the latest and test again? Per some other discussions on p5p, I made the code to skip loading locale more generic. John
Subject: Re: [rt.cpan.org #92051] [PATCH] Android doesn't support POSIX::localeconv()
Date: Wed, 15 Jan 2014 09:59:44 +0100
To: bug-version [...] rt.cpan.org
From: Piotr Roszatycki <dexter [...] cpan.org>
Result: PASS Also cpanminus works correctly. Great work, Thank you!