Bug #44239 for Module-Build: ->script_files includes files ignored in MANIFEST.SKIP

Tue Mar 17 06:31:37 2009 dolmen [...] cpan.org - Ticket created

Subject:

->script_files includes files ignored in MANIFEST.SKIP

Test case : - create bin/myscript.old.pl - add "\.old\.pl$" to MANIFEST.SKIP - perl -MModule::Build -e "$,=',';print keys %{Module::Build->new(dist_na me=>'Toto',dist_version=>'0.00')->script_files}" -> it shows bin/myscript.old.pl (bin\myscript.old.pl on MSWin32) As Module::Build::Compat uses Module::Build->script_files, the Makefile.PL generated by Module::Build->create_build_script() includes in EXE_FILES some files which are not in the distribution. The problem seems to come from Module::Build::Base->_files_in() which does not filter against MANIFEST.SKIP content.

Thu Jul 02 16:46:03 2009 SHLOMIF [...] cpan.org - Correspondence added

On Tue Mar 17 06:31:37 2009, DOLMEN wrote: Show quoted text

> Test case : > - create bin/myscript.old.pl > - add "\.old\.pl$" to MANIFEST.SKIP > - perl -MModule::Build -e "$,=',';print keys %{Module::Build->new(dist_na > me=>'Toto',dist_version=>'0.00')->script_files}" > -> it shows bin/myscript.old.pl (bin\myscript.old.pl on MSWin32) > > As Module::Build::Compat uses Module::Build->script_files, the > Makefile.PL generated by Module::Build->create_build_script() includes > in EXE_FILES some files which are not in the distribution. > > > The problem seems to come from Module::Build::Base->_files_in() which > does not filter against MANIFEST.SKIP content.

Can reproduce here: 1. Latest Module-Build trunk. 2. perl-5.10.0-25mdv2009.1 . 3. My own test file: {{{{{{{{{{{{{ #!/usr/bin/perl use strict; use warnings; use Module::Build; my $mb = Module::Build->new(dist_name=>'Test-Count',dist_version=>'0.00'); print "SCRIPTS = <<<\n"; print map { "$_\n" } keys %{ $mb->script_files }; print ">>>\n"; }}}}}}}}}}}}}

Thu Jul 02 16:46:04 2009 The RT System itself - Status changed from 'new' to 'open'

Thu Jul 02 17:23:20 2009 kwilliams [...] cpan.org - Correspondence added

It's very common to have script files which aren't part of the MANIFEST - for example any script file created by a .PL file. So this behavior can't (and shouldn't) be changed. If there's some other motivation behind this that you would like to discuss, the mailing list would be a good place.

Thu Jul 02 17:23:30 2009 kwilliams [...] cpan.org - Status changed from 'open' to 'rejected'

Thu Jul 02 17:24:34 2009 SHLOMIF [...] cpan.org - Correspondence added

Attached is an svn diff adding a test file to the svn trunk Module-Build. Hopefully, I'll write a fix soon. Regards, -- Shlomi Fish On Thu Jul 02 16:46:03 2009, SHLOMIF wrote: Show quoted text

> On Tue Mar 17 06:31:37 2009, DOLMEN wrote:

> > Test case : > > - create bin/myscript.old.pl > > - add "\.old\.pl$" to MANIFEST.SKIP > > - perl -MModule::Build -e "$,=',';print keys

%{Module::Build->new(dist_na Show quoted text

> > me=>'Toto',dist_version=>'0.00')->script_files}" > > -> it shows bin/myscript.old.pl (bin\myscript.old.pl on MSWin32) > > > > As Module::Build::Compat uses Module::Build->script_files, the > > Makefile.PL generated by Module::Build->create_build_script() includes > > in EXE_FILES some files which are not in the distribution. > > > > > > The problem seems to come from Module::Build::Base->_files_in() which > > does not filter against MANIFEST.SKIP content.

> > Can reproduce here: > > 1. Latest Module-Build trunk. > > 2. perl-5.10.0-25mdv2009.1 . > > 3. My own test file: > > {{{{{{{{{{{{{ > #!/usr/bin/perl > > use strict; > use warnings; > > use Module::Build; > > my $mb = Module::Build->new(dist_name=>'Test-Count',dist_version=>'0.00'); > print "SCRIPTS = <<<\n"; > print map { "$_\n" } keys %{ > $mb->script_files > }; > print ">>>\n"; > > }}}}}}}}}}}}}

Index: t/bug44239.t =================================================================== --- t/bug44239.t (revision 0) +++ t/bug44239.t (revision 0) @@ -0,0 +1,53 @@ +#!/usr/bin/perl -w + +use strict; +use lib $ENV{PERL_CORE} ? '../lib/Module/Build/t/lib' : 't/lib'; +use MBTest tests => 4; + +# TEST +use_ok 'Module::Build'; + +# TEST +ensure_blib('Module::Build'); + +use Module::Build::ConfigData; +my $have_yaml = Module::Build::ConfigData->feature('YAML_support'); + +######################### + +my $tmp = MBTest->tmpdir; + +use DistGen; +my $dist = DistGen->new( dir => $tmp ); +$dist->add_file( + "bin/script.good.pl", + qq{#!/usr/bin/perl\nprint "Hello\\n";} +); +$dist->add_file( + "bin/script.old.pl", + qq{#!/usr/bin/perl\nprint "Wrong file\\n";} +); +$dist->add_file( + "MANIFEST.SKIP", + qq{\\.old\\.pl\$}, +); + +$dist->regen; + +$dist->chdir_in; + +######################### + +my $mb = Module::Build->new_from_context; +# TEST +ok ($mb, "Module::Build was initialised"); + +# TEST +is_deeply( + [ sort {$a cmp $b } keys(%{$mb->script_files()}) ], + [ File::Spec->catfile(File::Spec->curdir(), "bin", "script.good.pl") ], + "Only script.good.pl is included with MANIFEST.SKIP.", +); + +# cleanup +$dist->remove;

Thu Jul 02 17:24:35 2009 The RT System itself - Status changed from 'rejected' to 'open'

Thu Jul 02 22:43:34 2009 kwilliams [...] cpan.org - Correspondence added

On Thu Jul 02 17:24:34 2009, SHLOMIF wrote: Show quoted text

> Attached is an svn diff adding a test file to the svn trunk > Module-Build. Hopefully, I'll write a fix soon.

Nope, see my comments above. Not a bug. If we "fixed" it, we'd break a lot of distributions. -Ken

Fri Jul 03 04:22:52 2009 dolmen [...] cpan.org - Correspondence added

Le Jeu. Jul. 02 17:23:20 2009, KWILLIAMS a écrit : Show quoted text

> It's very common to have script files which aren't part of the MANIFEST > - for example any script file created by a .PL file. So this behavior > can't (and shouldn't) be changed.

Unfortunately you're right. I don't see how we can fix this without introducing side effects (scripts not installed) on distributions which do not explicitely declare scripts (including generated ones) in 'script_files'.

Fri Jul 03 08:46:36 2009 dagolden [...] cpan.org - Correspondence added

On Fri Jul 03 04:22:52 2009, DOLMEN wrote: Show quoted text

> Le Jeu. Jul. 02 17:23:20 2009, KWILLIAMS a écrit :

> > It's very common to have script files which aren't part of the MANIFEST > > - for example any script file created by a .PL file. So this behavior > > can't (and shouldn't) be changed.

> > Unfortunately you're right. I don't see how we can fix this without > introducing side effects (scripts not installed) on distributions which > do not explicitely declare scripts (including generated ones) in > 'script_files'.

Can we close this ticket, then?

Fri Jul 17 14:05:07 2009 dagolden [...] cpan.org - Status changed from 'open' to 'resolved'