NEILB's Blog

I had a patch accepted which adds an is_core() function to Module::CoreList. This was my first attempt at modifying a core module. This post describes the function, why I wanted it, and my experience adding it to a core module. And then fixing the bug I introduced!

One of the changes to the adoption list I made last month was to add 1 to the score if the module was bundled with Perl itself (ie is a 'core module').

Module::CoreList is a core module which contains information about what versions of which modules shipped with each version of Perl. It has a number of functions for accessing that data, but doesn't have a function for simply checking whether a module is a core module. So for the adoption list I wrote a simple is_core($module) function, which calls some of the functions in Module::CoreList.

Recently I've wanted to use that function for another project. There's a Japanese saying,

if it happens twice, it will happen a third time

So I decided that it was time to see if I could add the function to Module::CoreList. It's a core module, so the process of submitting a patch is a bit different from your average module on CPAN.

First you need to find out what category of core module it is. There are four categories, described in the Core modules section in the perlsource documentation. Module::CoreList is in the dist/ directory, which tells you it's dual-life (is released with Perl, and also released separately on CPAN), but that the blead source is canonical. The name 'blead' refers to the latest development sources for perl (which live at git://perl5.git.perl.org/perl.git). You should double check that though, using the corelist script which is bundled with Module::CoreList:

% corelist --upstream Module::CoreList

Data for 2013-08-20
Module::CoreList was first released with perl v5.8.9
upstream: blead

The upstream tells you where it's maintained, which is confirmed as blead (compare with corelist -u Test::More for example, which has upstream of cpan, telling you to submit patches to the CPAN module, which will then be propagated to blead).

Next you need to read the perlhack doc. Read the SUPER QUICK PATCH GUIDE, and then while Perl is building, read the whole doc.

I wrote some documentation for my simple function, some basic tests, and submitted a patch to p5p. Brad Gilbert pointed out some deficiencies, and LEONT said that he'd wanted such a function in the past, but wanted to specify a minimum version of the module.

Time to get serious.

First I decided on the interface, trying to make it match the existing style of Module::CoreList:

• is_core($module) - returns true if $module was bundled with your (running) version of perl
• is_core($module, $version) - returns true if at least $version of $module was bundled with your perl
• is_core($module, $version, $release) - returns true if the module was bundled with the specified release of perl.
• is_core($module, undef, $release)- if you don't care about the version of the module but want to specify the perl release number.

For each of these cases I found example modules and used them to define tests, adding a new test file. I iteratively wrote the is_core() function, working through each of the cases, until the tests all passed. The documentation for the module is in a separate pod file, so I updated that. Aside: I seem to remember PBP says you shouldn't put pod in a separate file. I should go back and read why, and think about whether that's a change to suggest.

I thought I was good to go, so did a git commit. Bugger, should have run all perl tests, let's hope they all pass. Bugger. Because I added a file, I had to add it to the MANIFEST file in the toplevel directory (I had added it to the MANIFEST file in dist/Module-CoreList/). I just checked out a clean copy of blead, made the changes and did the commit. I really need to improve my git knowledge!

Then I remembered that I should update the perldelta file, which is in pod/perldelta.pod. Another git clone. This time I made all the changes first, ran Configure, and the tests passed. I submitted the patch, and it's already been applied, and Module::CoreList has been released. W00t!

Final thoughts:

• If you're not sure about something, the best place to ask is the p5p channel on IRC. I've asked a few times, and always had prompt helpful responses.
• I now know about git commit --amend
• I'm thinking about submitting a doc update to perlhack
• Read perlgit :-)

I continued to play around with the data in Module::CoreList, and as a result of drawing the perl release graph I realised that there's a bug in is_core(). If a minimum version of a module has been specified, then I need to go through the releases leading to the Perl release specified, to see whether a high enough version of the module is shipped. My first implementation naively assumes the releases are a single sequence, but they're not. For example, here's part of Text::Soundex's history in core:

Text::Soundex 3.03 was included in Perl 5.8.9, but then Perl 5.009003 still included Text::Soundex 1.01.

You'd have to be pretty unlucky to be hit by this bug, but I've now had my second patch accepted, which fixed this bug. Final lesson (re)learned:

• For data of any complexity, come up with a way to visualise it.