[rt.cpan.org #106144] [Patch] Preload dependencies for File::BOM

[Roderich Schupp via RT]

Wed Jul 29 11:10:31 2015: Request 106144 was acted upon.
Transaction: Correspondence added by RSCHUPP
Queue: Module-ScanDeps
Subject: [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: new
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >

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

[Shawn Laffan via RT]

Wed Jul 29 17:45:38 2015: Request 106144 was acted upon.
Transaction: Correspondence added by SLAFFAN
Queue: Module-ScanDeps
Subject: [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >


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.

Post by Roderich Schupp via RT

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

[Shawn Laffan via RT]

Wed Jul 29 19:42:33 2015: Request 106144 was acted upon.
Transaction: Correspondence added by SLAFFAN
Queue: Module-ScanDeps
Subject: [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >


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.

Post by Shawn Laffan via RT
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.

Post by Roderich Schupp via RT

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

[Roderich Schupp via RT]

Thu Jul 30 03:31:22 2015: Request 106144 was acted upon.
Transaction: Correspondence added by RSCHUPP
Queue: Module-ScanDeps
Subject: [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >

Post by Shawn Laffan via RT
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

[Shawn Laffan via RT]

Thu Jul 30 05:02:28 2015: Request 106144 was acted upon.
Transaction: Correspondence added by SLAFFAN
Queue: Module-ScanDeps
Subject: [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >

Post by Roderich Schupp via RT

Post by Shawn Laffan via RT
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.

[Roderich Schupp via RT]

Thu Jul 30 07:04:34 2015: Request 106144 was acted upon.
Transaction: Correspondence added by RSCHUPP
Queue: Module-ScanDeps
Subject: [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: open
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >

Post by Shawn Laffan via RT
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.

Post by Shawn Laffan via RT
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

[Shawn Laffan via RT]

Thu Jul 30 18:02:11 2015: Request 106144 was acted upon.
Transaction: Correspondence added by shawn_laffan
Queue: Module-ScanDeps
Subject: Re: [rt.cpan.org #106144] [Patch] Preload dependencies for File::BOM
Broken in: (no value)
Severity: (no value)
Owner: Nobody
Requestors: ***@cpan.org
Status: resolved
Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=106144 >

Post by Roderich Schupp via RT

Post by Shawn Laffan via RT
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.

Post by Shawn Laffan via RT
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.

[Shawn Laffan]

Post by Roderich Schupp via RT

Post by Shawn Laffan via RT
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.

Post by Shawn Laffan via RT
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.