From e8b5311316aff5229b6b5cf64da6c6ca4b786f46 Mon Sep 17 00:00:00 2001 From: Kent Fredric Date: Thu, 30 Jun 2016 17:16:18 +1200 Subject: [PATCH 1/3] Update perltidyrc --- perlcritic.rc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/perlcritic.rc b/perlcritic.rc index 9053c62..f7c254a 100644 --- a/perlcritic.rc +++ b/perlcritic.rc @@ -305,7 +305,7 @@ allowed_pragmata = diagnostics feature perlversion strict warnings utf8 [RegularExpressions::RequireExtendedFormatting] -[RegularExpressions::RequireLineBoundaryMatching] +[-RegularExpressions::RequireLineBoundaryMatching] [Subroutines::ProhibitAmpersandSigils] @@ -318,7 +318,7 @@ exempt_subs = MooseX::Types::subtype MooseX::Types::where MooseX::Types::as Moos [Subroutines::ProhibitExcessComplexity] -[Subroutines::ProhibitExplicitReturnUndef] +[-Subroutines::ProhibitExplicitReturnUndef] [Subroutines::ProhibitExportingUndeclaredSubs] @@ -337,9 +337,9 @@ private_name_regex = _(?!build_)\w [Subroutines::ProtectPrivateSubs] -[Subroutines::RequireArgUnpacking] +[-Subroutines::RequireArgUnpacking] -[Subroutines::RequireFinalReturn] +[-Subroutines::RequireFinalReturn] [TestingAndDebugging::ProhibitNoStrict] @@ -374,7 +374,7 @@ base_max = 130 [ValuesAndExpressions::ProhibitComplexVersion] -[ValuesAndExpressions::ProhibitConstantPragma] +[-ValuesAndExpressions::ProhibitConstantPragma] [ValuesAndExpressions::ProhibitDuplicateHashKeys] From adb8718978b41ec6cc6f5762c53b340f2e30321b Mon Sep 17 00:00:00 2001 From: Kent Fredric Date: Thu, 7 Jul 2016 22:47:46 +1200 Subject: [PATCH 2/3] Warn about Provides entries pointing to non-existent files. This is mostly a failsafe trapping for bad logic and people who misuse MetaProvides::FromFile Aims to guard against not only "Bad filename" but "bad Path convention", but this latter is weakly enforced by "using what dzil uses". CPAN::Meta::Spec says Provides entries must use UNIX conventions and use paths relative to base. --- Changes | 6 ++ Makefile.PL | 2 + lib/Dist/Zilla/Role/MetaProvider/Provider.pm | 24 +++++-- misc/Changes.deps | 2 + misc/Changes.deps.all | 2 + t/01-Provider/05-filenames.t | 67 ++++++++++++++++++++ 6 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 t/01-Provider/05-filenames.t diff --git a/Changes b/Changes index 36bd816..b3b9f64 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,12 @@ Revision history for {{$dist->name}} {{$NEXT}} + - Adds logging warnings for Provides entries that don't map to shipped files. This should not be a problem for anyone + unless you're using custom MetaProvides. + + [Dependencies::Stats] + - Dependencies changed since 2.002001, see misc/*.deps* for details + - test: +1 2.002001 2016-06-29T06:09:34Z 2be5312 - Remove usage of ConfigDumper and inline dump_config code instead. diff --git a/Makefile.PL b/Makefile.PL index c93805f..99aafd1 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -29,6 +29,7 @@ my %WriteMakefileArgs = ( "warnings" => 0 }, "TEST_REQUIRES" => { + "Dist::Zilla::Role::Plugin" => 0, "ExtUtils::MakeMaker" => 0, "File::Spec" => 0, "Path::Tiny" => 0, @@ -48,6 +49,7 @@ my %WriteMakefileArgs = ( my %FallbackPrereqs = ( "Carp" => 0, "Dist::Zilla::Role::MetaProvider" => 0, + "Dist::Zilla::Role::Plugin" => 0, "ExtUtils::MakeMaker" => 0, "File::Spec" => 0, "Hash::Merge::Simple" => 0, diff --git a/lib/Dist/Zilla/Role/MetaProvider/Provider.pm b/lib/Dist/Zilla/Role/MetaProvider/Provider.pm index 588e879..fe048f9 100644 --- a/lib/Dist/Zilla/Role/MetaProvider/Provider.pm +++ b/lib/Dist/Zilla/Role/MetaProvider/Provider.pm @@ -272,10 +272,26 @@ results returned from C<$self-Eprovides>. =cut sub metadata { - my ($self) = @_; - my $discover = {}; - for ( $self->provides ) { - $_->copy_into($discover); + my ($self) = @_; + my $discover = {}; + my (%all_filenames) = map { $_->name => 1 } @{ $self->zilla->files || [] }; + my (%missing_files); + + for my $provide_record ( $self->provides ) { + my $file = $provide_record->file; + + if ( not exists $all_filenames{$file} ) { + $missing_files{$file} = 1; + $self->log_debug( 'Provides entry states missing file <' . $file . '>' ); + } + + $provide_record->copy_into($discover); + } + + ## no critic (RestrictLongStrings) + if ( my $nkeys = scalar keys %missing_files ) { + $self->log( "$nkeys provide map entries did not map to distfiles: " . join q[, ], + sort keys %missing_files ); } return { provides => $discover }; } diff --git a/misc/Changes.deps b/misc/Changes.deps index 43a9c0e..df10989 100644 --- a/misc/Changes.deps +++ b/misc/Changes.deps @@ -1,6 +1,8 @@ This file contains changes in REQUIRED dependencies for standard CPAN phases (configure/build/runtime/test) 2.002002 + [Added / test requires] + - Dist::Zilla::Role::Plugin 2.002001 2016-06-29T06:09:34Z [Added / test requires] diff --git a/misc/Changes.deps.all b/misc/Changes.deps.all index 3da8e36..e6daf81 100644 --- a/misc/Changes.deps.all +++ b/misc/Changes.deps.all @@ -1,6 +1,8 @@ This file contains ALL changes in dependencies in both REQUIRED / OPTIONAL dependencies for all phases (configure/build/runtime/test/develop) 2.002002 + [Added / test requires] + - Dist::Zilla::Role::Plugin 2.002001 2016-06-29T06:09:34Z [Added / test recommends] diff --git a/t/01-Provider/05-filenames.t b/t/01-Provider/05-filenames.t new file mode 100644 index 0000000..666abfe --- /dev/null +++ b/t/01-Provider/05-filenames.t @@ -0,0 +1,67 @@ + +use strict; +use warnings; + +use Test::More 0.96; +use Test::Fatal; +use Path::Tiny qw( path ); +use Test::DZil qw( simple_ini Builder ); + +{ + package # PAUSE + Dist::Zilla::Plugin::FakePlugin; + + use Moose; + use Dist::Zilla::MetaProvides::ProvideRecord; + + with 'Dist::Zilla::Role::Plugin'; + with 'Dist::Zilla::Role::MetaProvider::Provider'; + + sub provides { + my $self = shift; + return $self->_apply_meta_noindex( + Dist::Zilla::MetaProvides::ProvideRecord->new( + module => 'FakeModule', + file => 'C:\temp\notevenonwindows.pl', + version => '3.1414', + parent => $self, + ), + Dist::Zilla::MetaProvides::ProvideRecord->new( + module => 'Example', + file => 'lib/Example.pm', + version => '3.1414', + parent => $self, + ), + ); + } + + __PACKAGE__->meta->make_immutable; + $INC{'Dist/Zilla/Plugin/FakePlugin.pm'} = 1; +} + +my $test_module = <<'EOF'; +package Example; + +1; +EOF + +my $builder = Builder->from_config( + { + dist_root => 'invalid', + }, + { + add_files => { + path('source/dist.ini') => simple_ini( 'GatherDir', [ 'FakePlugin' => { meta_noindex => 1 } ] ), + path('source/lib/Example.pm') => $test_module, + }, + }, +); + +$builder->chrome->logger->set_debug(1); +$builder->build; + +ok( ( grep { /missing file log_messages } ), "Bogus file at C:\\ warned" ); +ok( !( grep { /missing file log_messages } ), "Example.pm is not in error" ); + +note explain $builder->log_messages; +done_testing; From 2a9e8222191f3e3b3606828cb898e01964e154af Mon Sep 17 00:00:00 2001 From: Kent Fredric Date: Thu, 30 Jun 2016 17:14:33 +1200 Subject: [PATCH 3/3] Add warnings for all providers w/ package-name missmatches. Presently if you have nested modules, the modules inside may be indexed, but the package itself can't be "use"'d. This is not a definte issue, but often when it happens its by accident. --- Changes | 2 + lib/Dist/Zilla/Role/MetaProvider/Provider.pm | 12 ++++ t/01-Provider/06-cuckoo.t | 75 ++++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 t/01-Provider/06-cuckoo.t diff --git a/Changes b/Changes index b3b9f64..69717b4 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,8 @@ {{$NEXT}} - Adds logging warnings for Provides entries that don't map to shipped files. This should not be a problem for anyone unless you're using custom MetaProvides. + - Warn about cuckooed packages at the "provides-to-dzil" level. This enables automatic exposure of all packages where + "$filename" !~ "/Module/Name.pm" as being potentially problematic. [Dependencies::Stats] - Dependencies changed since 2.002001, see misc/*.deps* for details diff --git a/lib/Dist/Zilla/Role/MetaProvider/Provider.pm b/lib/Dist/Zilla/Role/MetaProvider/Provider.pm index fe048f9..d942352 100644 --- a/lib/Dist/Zilla/Role/MetaProvider/Provider.pm +++ b/lib/Dist/Zilla/Role/MetaProvider/Provider.pm @@ -276,15 +276,23 @@ sub metadata { my $discover = {}; my (%all_filenames) = map { $_->name => 1 } @{ $self->zilla->files || [] }; my (%missing_files); + my (%unmapped_modules); for my $provide_record ( $self->provides ) { my $file = $provide_record->file; + my $module = $provide_record->module; if ( not exists $all_filenames{$file} ) { $missing_files{$file} = 1; $self->log_debug( 'Provides entry states missing file <' . $file . '>' ); } + my $notional_filename = do { ( join q[/], split /::|'/sx, $module ) . '.pm' }; + if ( $file !~ /\b\Q$notional_filename\E\z/sx ) { + $unmapped_modules{$module} = 1; + $self->log_debug( 'Provides entry for module <' . $module . '> mapped to problematic <' . $file . '> ( want: <.*/' . $notional_filename . '> )' ); + } + $provide_record->copy_into($discover); } @@ -293,6 +301,10 @@ sub metadata { $self->log( "$nkeys provide map entries did not map to distfiles: " . join q[, ], sort keys %missing_files ); } + if ( my $nkeys = scalar keys %unmapped_modules ) { + $self->log( "$nkeys provide map entries did not map to .pm files and may not be loadable at install time: " . join q[, ], + sort keys %unmapped_modules ); + } return { provides => $discover }; } diff --git a/t/01-Provider/06-cuckoo.t b/t/01-Provider/06-cuckoo.t new file mode 100644 index 0000000..b4a08c9 --- /dev/null +++ b/t/01-Provider/06-cuckoo.t @@ -0,0 +1,75 @@ + +use strict; +use warnings; + +use Test::More 0.96; +use Test::Fatal; +use Path::Tiny qw( path ); +use Test::DZil qw( simple_ini Builder ); + +{ + package # PAUSE + Dist::Zilla::Plugin::FakePlugin; + + use Moose; + use Dist::Zilla::MetaProvides::ProvideRecord; + + with 'Dist::Zilla::Role::Plugin'; + with 'Dist::Zilla::Role::MetaProvider::Provider'; + + sub provides { + my $self = shift; + return $self->_apply_meta_noindex( + Dist::Zilla::MetaProvides::ProvideRecord->new( + module => 'FakeModule', + file => 'C:\temp\notevenonwindows.pl', + version => '3.1414', + parent => $self, + ), + Dist::Zilla::MetaProvides::ProvideRecord->new( + module => 'Example', + file => 'lib/Example.pm', + version => '3.1414', + parent => $self, + ), + Dist::Zilla::MetaProvides::ProvideRecord->new( + module => 'Example::Hidden', + file => 'lib/Example.pm', + version => '3.1414', + parent => $self, + ), + + ); + } + + __PACKAGE__->meta->make_immutable; + $INC{'Dist/Zilla/Plugin/FakePlugin.pm'} = 1; +} + +my $test_module = <<'EOF'; +package Example; + +1; +EOF + +my $builder = Builder->from_config( + { + dist_root => 'invalid', + }, + { + add_files => { + path('source/dist.ini') => simple_ini( 'GatherDir', [ 'FakePlugin' => { meta_noindex => 1 } ] ), + path('source/lib/Example.pm') => $test_module, + }, + }, +); + +$builder->chrome->logger->set_debug(1); +$builder->build; + +ok( ( grep { /for module log_messages } ), "Fake namespace without matching file warned" ); +ok( !( grep { /for module / } @{ $builder->log_messages } ), "Example with matching file doesnt warn" ); +ok( ( grep { /for module / } @{ $builder->log_messages } ), "Example::Hidden cuckoo warns" ); + +note explain $builder->log_messages; +done_testing;