Skip Menu |
 

This queue is for tickets about the Module-Install CPAN distribution.

Report information
The Basics
Id: 56589
Status: resolved
Priority: 0/
Queue: Module-Install

People
Owner: Nobody in particular
Requestors: florent.angly [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Bundle's refactor
Download (untitled) / with headers
text/plain 846b
I tested Kenichi's refactor of M::I::Bundle. The code is much cleaner, it is nice. I attached some patches concerning: * some misc cleaning * BUNDLE.yml is not needed anymore, so, some code went away * _install_bundle should be performed only once (even if called several times) * more checks for bundling by the admin My tests of the new refactor are pretty positive so far. I seem to have an issue with make test though, as it does not seem to be aware of all the modules that it should use from inc/BUNDLES. I checked that @INC did not contain any of the modules inside /inc/BUNDLES. I suspect that there is something to setup in M::I::Bundle so that the Makefile contains the appropriate instructions. Also, I seem to have the same(?) problem with make install. It does not seem to want to install any of the files for the bundled modules.
Subject: bundle.diff
Download bundle.diff
text/x-diff 4.7k
Index: lib/Module/Install/Bundle.pm =================================================================== --- lib/Module/Install/Bundle.pm (revision 11940) +++ lib/Module/Install/Bundle.pm (working copy) @@ -1,10 +1,7 @@ package Module::Install::Bundle; use strict; -use Cwd (); -use File::Find (); -use File::Copy (); -use File::Basename (); +use File::Spec; use Module::Install::Base (); use vars qw{$VERSION @ISA $ISCORE}; @@ -52,10 +49,7 @@ return $self->_install_bundled_dists unless $self->is_admin; my $deps = $self->admin->scan_dependencies($pkg); - if (scalar keys %$deps == 0) { - # Probably a user trying to install the package, read the dependencies from META.yml - %$deps = ( map { $$_[0] => undef } (@{$self->requires()}) ); - } + foreach my $key (sort keys %$deps) { $self->bundle($key, ($key eq $pkg) ? $version : 0); } @@ -64,29 +58,38 @@ sub _install_bundled_dists { my $self = shift; - my @dirs; - foreach my $entry (glob "inc/BUNDLES/*") { - # ignore BUNDLES.yml - next if -f $entry; + # process bundle only the first time this function is called + return if $self->{bundle_processed}; + $self->makemaker_args->{DIR} ||= []; + + # process all dists bundled in inc/BUNDLES/ + my $bundle_dir = $self->_top->{bundle}; + foreach my $sub_dir (glob File::Spec->catfile($bundle_dir,"*")) { + + next if -f $sub_dir; + # ignore dot dirs/files if any - next if $entry =~ m{^inc/BUNDLES/\.}; + my $dot_file = File::Spec->catfile($bundle_dir,'\.'); + next if $sub_dir =~ m{^$dot_file}; # EU::MM can't handle Build.PL based distributions - if (-f File::Spec->catfile($entry, 'Build.PL')) { - warn "Skipped: $entry has Build.PL."; + if (-f File::Spec->catfile($sub_dir, 'Build.PL')) { + warn "Skipped: $sub_dir has Build.PL."; next; } # EU::MM can't handle distributions without Makefile.PL # (actually this is to cut blib in a wrong directory) - if (!-f File::Spec->catfile($entry, 'Makefile.PL')) { - warn "Skipped: $entry has no Makefile.PL."; + if (!-f File::Spec->catfile($sub_dir, 'Makefile.PL')) { + warn "Skipped: $sub_dir has no Makefile.PL."; next; } - push @dirs, $entry; + push @{ $self->makemaker_args->{DIR} }, $sub_dir; } - push @{ $self->makemaker_args->{DIR} ||= [] }, @dirs; + + $self->{bundle_processed} = 1; + } 1; Index: lib/Module/Install/Admin/Bundle.pm =================================================================== --- lib/Module/Install/Admin/Bundle.pm (revision 11940) +++ lib/Module/Install/Admin/Bundle.pm (working copy) @@ -35,55 +35,44 @@ mkdir( $bundle_dir, 0777 ); - my %bundles; + $self->{bundled} ||= {}; while ( my ( $name, $version ) = splice( @_, 0, 2 ) ) { + + next if exists $self->{bundled}->{$name}; + my $mod = $cp->module_tree($name); if (not $mod) { warn "Warning: Could not find distribution for module $name on CPAN. Bundle it manually.\n"; next; } - if ( $mod->package_is_perl_core or $self->{already_bundled}{$mod->package} ) { - next; + next if $mod->package_is_perl_core; + + my $file = $mod->fetch( fetchdir => $bundle_dir, ); + if (not $file) { + warn "Warning: Could not download distribution $bundle_dir. Download it manually.\n"; + next; } - my $where = $mod->fetch( fetchdir => $bundle_dir, ); - next unless ($where); - my $file = Cwd::abs_path($where); - my $extract_result = $mod->extract( - files => [ $file ], + files => [ Cwd::abs_path($file) ], extractdir => $bundle_dir, ); - unlink $file; - next unless ($extract_result); - - $extract_result =~ s|\\|/|g if $^O eq 'MSWin32'; - my $location = ''; - if ($extract_result =~ /$bundle_dir\/(.*)/) { - $location = 'inc/BUNDLES/'.$1; - } else { - $location = $extract_result; + if (not $extract_result) { + warn "Warning: Could not extract distribution $file to $bundle_dir\n. Extract it manually.\n"; + next; } for my $submod ($mod->contains) { - $bundles{$submod->name} = $location; + $self->{bundled}->{$submod->name} = undef; } - $self->{already_bundled}{ $mod->package }++; - } chdir $cwd; - local *FH; - open FH, ">> $bundle_dir.yml" or die "Cannot write to $bundle_dir.yml: $!"; - foreach my $name ( sort keys %bundles ) { - print FH "$name: '$bundles{$name}'\n"; - } - close FH; } 1;
Hi, sorry for the late reply. Cherry-picked your patch. ::Admin::Bundle still needs refactoring though... On 2010-4-14 Wed 11:35:49, FANGLY wrote: Show quoted text
> I tested Kenichi's refactor of M::I::Bundle. The code is much cleaner, > it is nice. > > I attached some patches concerning: > * some misc cleaning > * BUNDLE.yml is not needed anymore, so, some code went away > * _install_bundle should be performed only once (even if called
several Show quoted text
> times) > * more checks for bundling by the admin > > My tests of the new refactor are pretty positive so far. I seem to
have Show quoted text
> an issue with make test though, as it does not seem to be aware of all > the modules that it should use from inc/BUNDLES. I checked that @INC
did Show quoted text
> not contain any of the modules inside /inc/BUNDLES. I suspect that
there Show quoted text
> is something to setup in M::I::Bundle so that the Makefile contains
the Show quoted text
> appropriate instructions. > > Also, I seem to have the same(?) problem with make install. It does
not Show quoted text
> seem to want to install any of the files for the bundled modules.
Download (untitled) / with headers
text/plain 143b
New Module::Install 0.96/0.97 is out. I know we need more refactoring on this issue but in the meantime I'd like to close this ticket. Thanks.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.