beyondgrep / ack2

**ack 2 is no longer being maintained. ack 3 is the latest version.**
https://github.com/beyondgrep/ack3/
Other
1.48k stars 140 forks source link

t/ack-s.t fails for non-English locale #485

Closed eserte closed 7 years ago

eserte commented 10 years ago

Test failure with a German locale:

#   Failed test at t/ack-s.t line 47.
# [
#   'ack: foo: Keine Berechtigung'
# ]
# Looks like you failed 1 test of 4.
t/ack-s.t .................... 

Seen with ack 2.14 on FreeBSD and Linux systems.

hoelzro commented 10 years ago

Confirmed, using de_DE.utf8.

hoelzro commented 10 years ago

https://github.com/petdance/ack2/blob/9443904974cb55f82bed6cd65872ff478be48351/Resources.pm#L16 =(

hoelzro commented 10 years ago

I think if we change that to $! == EACCES (assuming that EACCES works on Windows, and POSIX works on Windows, and File::Next doesn't clobber $!), that should fix this issue.

xtaran commented 10 years ago

Hi,

On Thu, Sep 04, 2014 at 08:06:52AM -0700, Rob Hoelz wrote:

https://github.com/petdance/ack2/blob/9443904974cb55f82bed6cd65872ff478be48351/Resources.pm#L16 =(

The simple way to fix this is probably a "$ENV{LANG} = 'C';" and maybe the same for LC_ALL and LANGUAGE inside the test itself.

    Regards, Axel
hoelzro commented 10 years ago

@xtaran Isn't that ignoring the underlying problem, which is that if a user with a non-English locale is using ack, ack -s doesn't actually work?

xtaran commented 10 years ago

Hi,

On Thu, Sep 04, 2014 at 10:18:37AM -0700, Rob Hoelz wrote:

@xtaran Isn't that ignoring the underlying problem, which is that if a user with a non-English locale is using ack, ack -s doesn't actually work?

Granted. For some reason I thought the test expects the English error message and fails because of that. But that's the test WITHOUT_S in line 13.

As I understand now, it's the WITH_RESTRICTED_DIR test from line 30 on which fails as it expects to see no error message at all and still gets one.

    Regards, Axel
hoelzro commented 10 years ago

@xtaran Ah, that's an interesting point. That seems like a possible bug (which I've filed as #488). Nice catch!

dolmen commented 9 years ago

The real fix is in #515.

eserte commented 9 years ago

Still fails with ack-2.15_01.

hoelzro commented 9 years ago

Relevant PR: https://github.com/petdance/file-next/pull/21

eserte commented 9 years ago

BTW, does not fail with perl 5.22.0, because stringified $! is now always English, regardless of user's locale (unless "use locale" is used within the perl code) (however, even if it's English it is possible that the "permission denied" message differs between operating systems).

eserte commented 8 years ago

Still fails with ack-2.15_02. Just saw a failure on a linux system with perl 5.20.3. More fail reports will probably follow...

dolmen commented 7 years ago

Still fails with ack-2.16 and File::Next 1.16.

petdance commented 7 years ago

This will go out in 2.17_01.