Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WriteMakefile() should warn more on invalid arguments #214

Open
karenetheridge opened this issue Apr 8, 2015 · 5 comments
Open

WriteMakefile() should warn more on invalid arguments #214

karenetheridge opened this issue Apr 8, 2015 · 5 comments

Comments

@karenetheridge
Copy link
Member

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
Copy link
Member Author

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
Copy link

grtodd commented Apr 8, 2015

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
Copy link
Member Author

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
Copy link

grtodd commented Apr 8, 2015

++ 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
Copy link
Member

haarg commented Apr 9, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants