Bug #106144 for Module-ScanDeps: [Patch] Preload dependencies for File::BOM

Wed Jul 29 08:44:36 2015 SLAFFAN [...] cpan.org - Ticket created

Subject:

[Patch] Preload dependencies for File::BOM

Dependency scanning when File::BOM is used in a via construct misses several dependencies, resulting in errors when calling PAR packed executables. This particularly applies to CJK locales. The attached patch adds a preload section to include them. It could perhaps also load Encode::HanExtra and Encode::JIS2K, but I have not done so in this version. The patch was tested using the code below. The file open is needed to trigger the loading of the extra modules under "pp -x". A more general solution might be to scan for "open .+?'<:via(\w+)'" constructs. Regards, Shawn. use File::BOM; open my $fh, '<:via(File::BOM)', $0 # just read ourselves or die "Cannot open own code via File::BOM\n"; $fh->close;

Subject:

ScanDeps.pm.FileBOM.patch

Index: lib/Module/ScanDeps.pm =================================================================== --- lib/Module/ScanDeps.pm (revision 1595) +++ lib/Module/ScanDeps.pm (working copy) @@ -331,6 +331,16 @@ grep /\bMM_/, _glob_in_inc('ExtUtils', 1); }, 'File/Basename.pm' => [qw( re.pm )], + 'File/BOM.pm' => [ + qw ( + Encode/Unicode.pm + PerlIO/via.pm + Encode/CN.pm + Encode/JP.pm + Encode/KR.pm + Encode/TW.pm + ) # load extra Encode pages just in case - should probably also do the extended ones + ], 'File/HomeDir.pm' => 'sub', 'File/Spec.pm' => sub { require File::Spec;

Wed Jul 29 11:10:31 2015 RSCHUPP [...] cpan.org - Correspondence added

Show quoted text

> The patch was tested using the code below. The file open is needed to > trigger the loading of the extra modules under "pp -x". > > use File::BOM; > open my $fh, '<:via(File::BOM)', $0 # just read ourselves > or die "Cannot open own code via File::BOM\n"; > $fh->close;

'File/BOM.pm' => [qw( Encode/Unicode.pm PerlIO/via.pm )], is enough to make the code above work. No need to load a bunch of unrelated encodings. Cheers, Roderich

Wed Jul 29 11:10:32 2015 The RT System itself - Status changed from 'new' to 'open'

Wed Jul 29 17:45:38 2015 SLAFFAN [...] cpan.org - Correspondence added

Thanks Roderich. The issue is that, without the additional encodings, a PAR packed executable fails in those locales that need them. An example from my use case is here: https://github.com/shawnlaffan/biodiverse/issues/506 The issue boils down to whether they should be implicitly loaded by Module::ScanDeps, or should be explicitly packaged in the pp call. Regards, Shawn. On Wed Jul 29 11:10:31 2015, RSCHUPP wrote: Show quoted text

> > The patch was tested using the code below. The file open is needed > > to > > trigger the loading of the extra modules under "pp -x". > > > > use File::BOM; > > open my $fh, '<:via(File::BOM)', $0 # just read ourselves > > or die "Cannot open own code via File::BOM\n"; > > $fh->close;

> > 'File/BOM.pm' => [qw( Encode/Unicode.pm PerlIO/via.pm )], > > is enough to make the code above work. No need to load a bunch of > unrelated encodings. > > Cheers, Roderich

Wed Jul 29 19:42:33 2015 SLAFFAN [...] cpan.org - Correspondence added

Actually, if you agree that the Encode::* modules do need to be preloaded then it should be under Encode or Encode::Config, not File::BOM. Encode::Config lists a number of other modules that are loaded on demand, and seem not to be detected by static scanning or using the execute flag. Regards, Shawn. On Wed Jul 29 17:45:38 2015, SLAFFAN wrote: Show quoted text

> Thanks Roderich. > > The issue is that, without the additional encodings, a PAR packed > executable fails in those locales that need them. An example from my > use case is here: https://github.com/shawnlaffan/biodiverse/issues/506 > > The issue boils down to whether they should be implicitly loaded by > Module::ScanDeps, or should be explicitly packaged in the pp call. > > > > Regards, > Shawn. > > > On Wed Jul 29 11:10:31 2015, RSCHUPP wrote:

> > > The patch was tested using the code below. The file open is needed > > > to > > > trigger the loading of the extra modules under "pp -x". > > > > > > use File::BOM; > > > open my $fh, '<:via(File::BOM)', $0 # just read ourselves > > > or die "Cannot open own code via File::BOM\n"; > > > $fh->close;

> > > > 'File/BOM.pm' => [qw( Encode/Unicode.pm PerlIO/via.pm )], > > > > is enough to make the code above work. No need to load a bunch of > > unrelated encodings. > > > > Cheers, Roderich

Thu Jul 30 03:31:22 2015 RSCHUPP [...] cpan.org - Correspondence added

On 2015-07-29 19:42:33, SLAFFAN wrote: Show quoted text

> Actually, if you agree that the Encode::* modules do need to be > preloaded then it should be under Encode or Encode::Config, not > File::BOM.

Well, adding 'Encode.pm' => 'sub', to %Preload will pack ALL of them for a whopping 21 MB of stuff for anyone who just wants to use Encode qw(encode decode); ... $string = decode("utf8", $bytes); So I'm hesitant to do that. If someone knows in advance which encodings they need they can add an appropriate -M option to their pp command. If they want to be flexible (e.g. program takes the encoding as input), this could get cumbersome, though. There was once a suggestion on the list to add an option to pp that would mean "include Foo.pm and everything below Foo/". Implementation would be almost trivial (just call _glob_in_inc("Foo.pm")), the problem is to come up with a good name for this option. Cheers, Roderich

Thu Jul 30 05:02:28 2015 SLAFFAN [...] cpan.org - Correspondence added

On Thu Jul 30 03:31:22 2015, RSCHUPP wrote: Show quoted text

> On 2015-07-29 19:42:33, SLAFFAN wrote:

> > Actually, if you agree that the Encode::* modules do need to be > > preloaded then it should be under Encode or Encode::Config, not > > File::BOM.

> > Well, adding > > 'Encode.pm' => 'sub', > > to %Preload will pack ALL of them for a whopping 21 MB of stuff for > anyone > who just wants to > > use Encode qw(encode decode); > ... > $string = decode("utf8", $bytes); > > So I'm hesitant to do that. If someone knows in advance which > encodings they need > they can add an appropriate -M option to their pp command. If they > want to be > flexible (e.g. program takes the encoding as input), this could get > cumbersome, though. > > There was once a suggestion on the list to add an option to pp that > would mean > "include Foo.pm and everything below Foo/". Implementation would be > almost trivial > (just call _glob_in_inc("Foo.pm")), the problem is to come up with a > good name > for this option. > > Cheers, Roderich > >

That's a fair point about the size surprise. 21MB isn't that big these days, but it would be excessive for a simple application. The "everything below Foo" option could possibly be added to pp by adding wildcard or regexp handling to the -M argument, similar to --add in perlapp. http://docs.activestate.com/pdk/9.4/PerlApp.html#perlapp_top Coming back to File::BOM, do you want a revised patch for it? Regards, Shawn.

Thu Jul 30 07:04:34 2015 RSCHUPP [...] cpan.org - Correspondence added

On 2015-07-30 05:02:28, SLAFFAN wrote: Show quoted text

> Coming back to File::BOM, do you want a revised patch for it?

Not needed, thanks. I just added 'File/BOM.pm' => [qw( Encode/Unicode.pm )], to %Preload and a separate rule to recognize constructs like open FH, '<:via(Foo)',... So I'm resolving this ticket. Show quoted text

> The "everything below Foo" option could possibly be added to pp by > adding wildcard or regexp handling to the -M argument, similar to > --add in perlapp. > http://docs.activestate.com/pdk/9.4/PerlApp.html#perlapp_top

Looks like overkill to me. Also, pp allows for both -M Foo::Bar and -M Foo/Bar.pm so wildcards will be tricky. Perhaps a separate option "--modtree Foo::Bar" for adding Foo/Bar.pm and anything below Foo/Bar? Cheers, Roderich

Thu Jul 30 07:04:49 2015 RSCHUPP [...] cpan.org - Status changed from 'open' to 'resolved'

Thu Jul 30 18:02:11 2015 shawn.laffan [...] unsw.edu.au - Correspondence added

Subject:	Re: [rt.cpan.org #106144] [Patch] Preload dependencies for File::BOM
Date:	Fri, 31 Jul 2015 08:01:51 +1000
To:	<bug-Module-ScanDeps [...] rt.cpan.org>, <par [...] perl.org>
From:	Shawn Laffan <shawn.laffan [...] unsw.edu.au>

On 30/07/2015 21:04, Roderich Schupp via RT wrote: Show quoted text

> On 2015-07-30 05:02:28, SLAFFAN wrote:

>> Coming back to File::BOM, do you want a revised patch for it?

> Not needed, thanks. I just added > > 'File/BOM.pm' => [qw( Encode/Unicode.pm )], > > to %Preload and a separate rule to recognize constructs like > > open FH, '<:via(Foo)',... > > So I'm resolving this ticket. >

>> The "everything below Foo" option could possibly be added to pp by >> adding wildcard or regexp handling to the -M argument, similar to >> --add in perlapp. >> http://docs.activestate.com/pdk/9.4/PerlApp.html#perlapp_top

> Looks like overkill to me. Also, pp allows for both > > -M Foo::Bar > > and > > -M Foo/Bar.pm > > so wildcards will be tricky. Perhaps a separate option "--modtree Foo::Bar" > for adding Foo/Bar.pm and anything below Foo/Bar? > > Cheers, Roderich > > >

Thanks again Roderich, How about --moduletree for the option name? --modtree makes me think of modifying a tree. The wildcard approach would be tricky but could use some sort of file find approach. If I get a chance I'll look into it. Regards, Shawn.