Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARGV tests failing with utf8-all 0.013 or later #279

Closed
pghmcfc opened this issue Aug 28, 2014 · 7 comments · Fixed by #281
Closed

ARGV tests failing with utf8-all 0.013 or later #279

pghmcfc opened this issue Aug 28, 2014 · 7 comments · Fixed by #281

Comments

@pghmcfc
Copy link
Contributor

pghmcfc commented Aug 28, 2014

Changing utf8-all from 0.011 to 0.013 or later results in test failures:

$ ./Build test
gcc -I/usr/lib64/perl5/CORE -fPIC -c -D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -o bin/perl5i.o bin/perl5i.c
gcc -fstack-protector -o bin/perl5i bin/perl5i.o
#   Failed test at t/ARGV.t line 12.
#     Structures begin differing at:
#          $got->[0] = 'føø'
#     $expected->[0] = 'føø'
# Looks like you failed 1 test of 1.
t/ARGV.t ..................... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 
#   Failed test at t/ARGV_twice.t line 17.
#     Structures begin differing at:
#          $got->[0] = 'føø'
#     $expected->[0] = 'føø'
# Looks like you failed 1 test of 1.
t/ARGV_twice.t ............... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 
t/CLASS.t .................... ok
t/Child.t .................... ok
t/English.t .................. ok
t/File-stat.t ................ ok
t/List-MoreUtils/all.t ....... ok
t/List-MoreUtils/any.t ....... ok
t/List-MoreUtils/false.t ..... ok
t/List-MoreUtils/mesh.t ...... ok
t/List-MoreUtils/minmax.t .... ok
t/List-MoreUtils/none.t ...... ok
t/List-MoreUtils/true.t ...... ok
t/List-MoreUtils/uniq.t ...... ok
t/List-Util/first.t .......... ok
t/List-Util/max.t ............ ok
t/List-Util/maxstr.t ......... ok
t/List-Util/min.t ............ ok
t/List-Util/minstr.t ......... ok
t/List-Util/reduce.t ......... ok
t/List-Util/shuffle.t ........ ok
t/List-Util/sum.t ............ ok
t/Meta/ISA.t ................. ok
t/Meta/checksum.t ............ ok
t/Meta/class.t ............... ok
t/Meta/id.t .................. ok
t/Meta/is-equal.t ............ ok
t/Meta/linear_isa.t .......... ok
t/Meta/methods.t ............. ok
t/Meta/reftype.t ............. ok
t/Meta/super.t ............... ok
t/Meta/symbol_table.t ........ ok
t/Want.t ..................... ok
t/alias.t .................... ok
t/as_hash.t .................. ok
t/autobox.t .................. ok
t/autodie.t .................. ok
t/autovivification.t ......... ok
t/caller.t ................... ok
t/can.t ...................... ok
t/capture.t .................. ok
t/carp.t ..................... ok
t/center.t ................... ok
t/chdir.t .................... ok
t/command_line_wrapper.t ..... ok
t/commify.t .................. ok
t/datetime.t ................. ok
t/die.t ...................... ok
t/diff.t ..................... ok
t/dump/array.t ............... ok
t/dump/code.t ................ ok
t/dump/formats.t ............. ok
t/dump/hash.t ................ ok
t/dump/obj.t ................. ok
t/dump/scalar.t .............. ok
t/each.t ..................... ok
t/equal.t .................... ok
t/everything_is_an_object.t .. ok
t/flip.t ..................... ok
t/foreach.t .................. ok
t/github164.t ................ ok
t/grep.t ..................... ok
t/hash-diff.t ................ ok
t/hash-intersect.t ........... ok
t/hash-merge.t ............... ok
t/intersect.t ................ ok
t/io-handle.t ................ ok
t/is_module_name.t ........... ok
t/lexical.t .................. ok
t/list-trim.t ................ ok
t/list.t ..................... ok
t/load_together.t ............ ok
t/map.t ...................... ok
t/method_leaking.t ........... ok
t/modern_perl.t .............. ok
t/module2path.t .............. ok
t/no_indirect.t .............. ok
t/number.t ................... ok
t/path/base.t ................ ok
t/perl5i.t ................... ok
t/pick.t ..................... ok
t/popn.t ..................... ok
t/require.t .................. ok
t/require_message.t .......... ok
t/say.t ...................... ok
t/scalar.t ................... ok
t/shiftn.t ................... ok
t/signature.t ................ ok
t/signatures.t ............... ok
t/skip.t ..................... ok
t/taint.t .................... ok
t/time_compat.t .............. ok
t/true.t ..................... ok
t/try-tiny.t ................. ok
t/uniq.t ..................... ok
t/utf8.t ..................... ok
t/version_0/00_compile.t ..... ok
t/version_1/00_compile.t ..... ok
t/vs_listmoreutils.t ......... ok
t/wrap.t ..................... ok
t/y2038.t .................... ok
Test Summary Report
-------------------
t/ARGV.t                   (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
t/ARGV_twice.t             (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=101, Tests=1218, 31 wallclock secs ( 0.38 usr  0.14 sys + 21.38 cusr  1.76 csys = 23.66 CPU)
Result: FAIL
Failed 2/101 test programs. 2/1218 subtests failed.
@xtaran
Copy link

xtaran commented Aug 31, 2014

Also reported in Debian at https://bugs.debian.org/759969

Adding "use utf8;" to each of the failing .t files solves the issue for me, but possibly defeats the purpose of these tests.

@pghmcfc
Copy link
Contributor Author

pghmcfc commented Sep 1, 2014

Fedora bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1134872

@pghmcfc
Copy link
Contributor Author

pghmcfc commented Sep 1, 2014

The test suite also passes if I apply this change, though I don't know if it's the right thing to do really:

--- lib/perl5i/2.pm
+++ lib/perl5i/2.pm
@@ -127,10 +127,7 @@
     },
     'utf8::all' => sub {
         my ($class, $caller) = @_;
-
-        # use utf8::all
-        require utf8::all;
-        utf8::all::import($class);
+        load_in_caller($caller, ['utf8::all']);
     },
     Want => sub {
         my ($class, $caller) = @_;

@ppisar
Copy link

ppisar commented Sep 5, 2014

This failure started with this utf8-all commit:

commit 659dcada3914bab7e90b8be8395d74b5b1b5043b
Author: HayoBaan <info@hayobaan.nl>
Date:   Sun Aug 3 14:01:12 2014 +0200

    Only decoding @ARGV when called from main [GH-18]

which was motivated by [https://github.com/doherty/utf8-all/issues/18] bug report ("Currently, utf8::all decodes @argv the first time it's imported from any module. I think this is wrong. IMHO").

The change in utf8::all::import() was:

-    # utf8 in @ARGV
+    # Make @ARGV utf-8 when called from the main package
     state $have_encoded_argv = 0;
-    _encode_argv() unless $have_encoded_argv++;
+    map { $_ = Encode::decode('UTF-8' ,$_) } @ARGV unless $target ne "main" || $have_encoded_argv++;

     $^H{'utf8::all'} = 1;

     return;
 }

-sub _encode_argv {
-    $_ = Encode::decode('UTF-8', $_) for @ARGV;
-    return;
-}
-

which had wrong condition. The condition changed over releases and current 0.15 contains:

    # Make @ARGV utf-8 when called from the main package
    state $have_encoded_argv = 0;
    if ($target eq 'main' && !$have_encoded_argv++) {
        $_ = Encode::decode('UTF-8' ,$_) for @ARGV;
    }

At my first look, the condition is still wrong: At first call from 'main' package, the $have_encoded_argv is 0, ++ has higher priority than !, so the evaluation is "&& !0++" → "&&!1" → "&&0". I.e. the @argv is never recoded. Am I right?

@ppisar
Copy link

ppisar commented Sep 5, 2014

Taking back the last paragraph. The condition is good. because $variable++ return original value.

@ppisar
Copy link

ppisar commented Sep 5, 2014

The xtaran's propal is wrong as undermines the test (to set @argv as bytes and get it as UTF-8 decoded characters).

The pghmcfc's patch is correct as the issue is appearing perl5i on the call stack.

@schwern
Copy link
Contributor

schwern commented Sep 6, 2014

Fixing how we import utf8::all sounds correct. Would @pghmcfc or @ppisar like to make the pull request?

pghmcfc added a commit to pghmcfc/perl5i that referenced this issue Sep 8, 2014
Addresses evalEmpire#279

Loads utf8::all in the caller to get the desired behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants