Perl-Toolchain-Gang / ExtUtils-MakeMaker

Perl module to make Makefiles and build modules (what backs Makefile.PL)
https://metacpan.org/release/ExtUtils-MakeMaker
64 stars 77 forks source link

WriteMakefile() should warn more on invalid arguments #214

Open karenetheridge opened 9 years ago

karenetheridge commented 9 years ago

e.g. the NAME field should be a module name, so we should warn when passed something that doesn't look like a module (e.g. contains a -)

examples of distributions passing an invalid NAME: http://grep.cpan.me/?q=file%3AMakefile.PL+%28^|%5B^\w%5D%29NAME\s*%3D%3E.*\w-\w

karenetheridge commented 9 years ago

somewhat orthogonal note: there is some confusion about what the correct regex is for a valid package name; IMHO this should be a core API somewhere, but whatever we use in EUMM, we can easily refine it later.

Even just qr/^(?:[^-]+(?:'|::))*[^-]+$/ would be fine initially.

grtodd commented 9 years ago

Module::Runtime has is_module_name() which uses $module_name_rx so I resolved part of my situation with that:

 perl -MModule::Runtime=is_module_name -MExtUtils::Installed -E '
                my $installed = ExtUtils::Installed->new();
                 for ($installed->modules) {
                   say $_, $installed->version($_)  if is_module_name($_);
                 }'

For comparison:

$module_name_rx = qr/[A-Z_a-z][0-9A-Z_a-z]*(?:::[0-9A-Z_a-z]+)*/

karenetheridge commented 9 years ago

Module::Runtime has is_module_name() which uses $module_name_rx

for EUMM this isn't good because MR isn't a core module and this regex isn't correct (it is far too restrictive; see also recent discussions in Sub::Name's queues, and I'm also amassing lots of notes in this space), but it's fine for gtodd's cleanup purposes.

grtodd commented 9 years ago

++ I think I may have been hinting that some of the simple functions for interrogating/inspecting and modules in Module::Runtime would be nice to have somewhere in Core (so a perl can know about its dists modules). The functionality may already exist and just be under a layer of API.

haarg commented 9 years ago

It's worth distinguishing between package names and module names (something that maps to a file). Package names can include unicode just fine, but for module names, the regex used by Module::Runtime is pretty reasonable, since handling unicode filenames portably is very tricky.