dankogai / p5-encode

Encode - character encodings (for Perl 5.8 or better)
https://metacpan.org/release/Encode
37 stars 51 forks source link

"define_alias" no longer works with just "use Encode::Alias" #116

Closed arc closed 7 years ago

arc commented 7 years ago

In Encode 2.91, this code

use Encode::Alias;
use open ":std", ":locale";

yields this error:

Undefined subroutine &Encode::define_alias called at lib/Encode.pm line 102.
Compilation failed in require at lib/Encode/Alias.pm line 7.
BEGIN failed--compilation aborted at lib/Encode/Alias.pm line 7.
Compilation failed in require at -e line 1.
BEGIN failed--compilation aborted at -e line 1.

This was discovered when trying to import 2.91 into blead, because the Perl test suite does precisely that.

Reverting the change that 55364c9dfa1e3ce611cdff08fc576c86a0bdcdf4 made to lib/Encode/Alias.pm makes this problem go away. That may not be the ideal fix, though.

Since there's a circular dependency here — Encode.pm does use Encode::Alias, and Encode/Alias.pm does use Encode () perhaps it would be better to break the shared pieces out into a separate (internal) module that both Encode and Encode::Alias use?

pali commented 7 years ago

It is not simple... because functions from Encode.pm calls functions from Encode/Alias.pm and also functions from Encode/Alias.pm calls functions from Encode.pm...

Looks like those functions from Enocde.pm and Encode/Alias.pm needs to be moved into one module... I'm thinking about merging Encode/Alias.pm into Encode.pm. But because Encode/Alias.pm has global variables there can be other problems.

@arc what do you think?

pali commented 7 years ago

@arc Here is less invasive patch which could to work. Can you test if it works for you too?

diff --git a/Encode.pm b/Encode.pm
index a38bf0f..62a143c 100644
--- a/Encode.pm
+++ b/Encode.pm
@@ -49,7 +49,7 @@ our %EXPORT_TAGS = (

 our $ON_EBCDIC = ( ord("A") == 193 );

-use Encode::Alias;
+use Encode::Alias ();
 use Encode::MIME::Name;

 use Storable;
@@ -138,6 +138,15 @@ sub getEncoding {
     return;
 }

+# HACK: These two functions must be defined in Encode and because of
+# cyclic dependency between Encode and Encode::Alias, Exporter does not work
+sub find_alias {
+    goto &Encode::Alias::find_alias;
+}
+sub define_alias {
+    goto &Encode::Alias::define_alias;
+}
+
 sub find_encoding($;$) {
     my ( $name, $skip_external ) = @_;
     return __PACKAGE__->getEncoding( $name, $skip_external );
diff --git a/lib/Encode/Alias.pm b/lib/Encode/Alias.pm
index 7020e1e..eb73b87 100644
--- a/lib/Encode/Alias.pm
+++ b/lib/Encode/Alias.pm
@@ -4,8 +4,6 @@ use warnings;
 our $VERSION = do { my @r = ( q$Revision: 2.22 $ =~ /\d+/g ); sprintf "%d." . "%02d" x $#r, @r };
 use constant DEBUG => !!$ENV{PERL_ENCODE_DEBUG};

-use Encode ();
-
 use Exporter 'import';

 # Public, encouraged API is exported by default
@@ -109,6 +107,9 @@ sub define_alias {
     }
 }

+# HACK: Encode must be used after define_alias is declarated as Encode calls define_alias
+use Encode ();
+
 # Allow latin-1 style names as well
 # 0  1  2  3  4  5   6   7   8   9  10
 our @Latin2iso = ( 0, 1, 2, 3, 4, 9, 10, 13, 14, 15, 16 );
arc commented 7 years ago

Yes, importing Encode 2.91 into blead and then applying that patch makes blead's tests pass. Thanks!

I'll hold off on actually importing a newer Encode until there's a release including this change.

pali commented 7 years ago

Thanks for testing, I created pull request: https://github.com/dankogai/p5-encode/pull/119

pali commented 7 years ago

@arc New version with this fix was released.